[PATCH] kernal: skip hpower setting for the module which has no revs

照山周一郎 teruyama at springboard-inc.jp
Mon Nov 29 02:46:19 PST 2021


Thanks for all the helpful advice.

The EEPROM dump is as follows.

ーーー

Offset Values
---— ———
0x0000: 03 04 01 00 00 00 80 00 00 00 00 01 0d 00 0a 64
0x0010: 00 00 00 00 4e 54 54 20 20 20 20 20 20 20 20 20
0x0020: 20 20 20 20 00 00 00 00 30 30 30 30 30 30 30 30
0x0030: 30 30 30 30 30 30 30 30 30 30 30 30 05 1e 00 7d
0x0040: 02 00 00 00 30 30 30 30 30 30 30 30 30 30 30 30
0x0050: 30 30 30 30 31 36 30 34 30 38 20 20 00 00 00 75
0x0060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ーーー

The log is as follows.

ーーー

[    8.583797] kmodloader: done loading kernel modules from /etc/modules.d/*
[    8.834704] sfp sfp-eth1: module OEM              10G-SFP-PCU1M
rev G    sn HS18100810001    dc 181008
[    8.856051] sfp sfp-eth3: module NTT              0000000000000000
rev 0000 sn 0000000000000000 dc 160408
[    8.865843] mvpp2 f4000000.ethernet eth3: switched to
inband/1000base-x link mode
[    8.873469] sfp sfp-eth3: Failed to read EEPROM: -5
[    8.983251] sfp sfp-eth3: Failed to read EEPROM: -5
[    9.103250] sfp sfp-eth3: Failed to read EEPROM: -5
ーーー

"Failed to read EEPROM: -5"  output in a loop.
Its caused here.

https://github.com/torvalds/linux/blob/master/drivers/net/phy/sfp.c#L1694

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/phy/sfp.c?id=7cfa9c92d0a325f97ac13f894a7b47d37bd2040
> if (power_mW > sfp->max_power_mW) {

I think the above modification is wrong too.

However, I don't have the SFP module that returns the appropriate
revision, and I don't have the environment to verify it, so I can't
fix the above patch myself.

The NTT OCU that I want to run this time is widely used in Japan, so I
would like to apply the patch with the method I applied.

The build warnings will be fixed later.

Translated with www.DeepL.com/Translator (free version)

2021年11月29日(月) 18:30 Bjørn Mork <bjorn at mork.no>:
>
> 照山周一郎 <teruyama at springboard-inc.jp> writes:
>
> > Thanks for the advice.
> >
> > This is a fix for a Japanese NTT OCU, which always outputs 2W, even
> > though the chip revision always returns 0. This is not a bug, but a
> > specification.
> > https://business.ntt-east.co.jp/service/onu/pdf/interface.pdf
> >
> > Therefore, since the situation is too special to be handled by
> > sfp_module_parse_power(), the existing code was implemented to exit
> > the process in the process just before the error occurs.
>
> It would help understanding if you included the actual error, and maybe
> an eeprom dump.  Something like this (on a patched host where the module
> is supported, of course...):
>
>   ethtool -m ethX raw on | hexdump -C
>
> > The SF-8472 does not normally return 0, so there is no effect on other devices.
> > https://members.snia.org/document/dl/25916
>
> I see.  This makes sense. I wonder if this is actually a bug in mainline
> introduced by commit 7cfa9c92d0a3 ("net: sfp: avoid power switch on
> address-change modules")?
>
> See
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/phy/sfp.c?id=7cfa9c92d0a325f97ac13f894a7b47d37bd2040
>
> If I read that correctly, then the original behaviour was the way you
> want it: We assumed that a module mathcing
>
>  (sfp->id.ext.sff8472_compliance == SFP_SFF8472_COMPLIANCE_NONE &&
>   (sfp->id.ext.diagmon & (SFP_DIAGMON_DDM | SFP_DIAGMON_ADDRMODE)) != SFP_DIAGMON_DDM)
>
> already was in the indicated power mode and skipped the EXT_STATUS
> register update.  But the patch moved most of that logic inside a
>
>  if (power_mW > sfp->max_power_mW) {}
>
> block.  Which I believe is a bug. We're now failing in case the host
> supports higher power but the module doesn't support DOM.
>
> If you think my analysis is correct, then you could you try to revert
> that patch to verify that it fixes your problem.  There is a trivial
> string conflict but otherwise it reverts just fine.
>
> Is verified then a proper fix should go upstream.
>
> > If possible, I would appreciate some practical advice on compile-time warnings.
>
> No warnings before patching:
>
> bjorn at miraculix:/usr/local/src/git/linux$ make -C /lib/modules/$(uname -r)/build M=$(pwd)/drivers/net/phy sfp.ko
> make: Entering directory '/usr/src/linux-headers-5.10.0-9-amd64'
>   CC [M]  /usr/local/src/git/linux/drivers/net/phy/sfp.o
>   MODPOST /usr/local/src/git/linux/drivers/net/phy/Module.symvers
>   CC [M]  /usr/local/src/git/linux/drivers/net/phy/sfp.mod.o
>   LD [M]  /usr/local/src/git/linux/drivers/net/phy/sfp.ko
> make: Leaving directory '/usr/src/linux-headers-5.10.0-9-amd64'
>
> Apply your patch:
>
> bjorn at miraculix:/usr/local/src/git/linux$ patch -p1 < /tmp/771-net-sfp-skip-hpowr-if-no-revision.patch
> patching file drivers/net/phy/sfp.c
> Hunk #1 succeeded at 1634 (offset 44 lines).
>
>
> Verify it:
>
> bjorn at miraculix:/usr/local/src/git/linux$ git diff
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 32c34c728c7a..4099752dbcd0 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -1634,6 +1634,8 @@ static int sfp_module_parse_power(struct sfp *sfp)
>
>  static int sfp_sm_mod_hpower(struct sfp *sfp, bool enable)
>  {
> +       if (sfp->id.ext.sff8472_compliance == SFP_SFF8472_COMPLIANCE_NONE)
> +               return 0;
>         u8 val;
>         int err;
>
>
> Build again and notice a new warning:
>
> bjorn at miraculix:/usr/local/src/git/linux$ make -C /lib/modules/$(uname -r)/build M=$(pwd)/drivers/net/phy sfp.ko
> make: Entering directory '/usr/src/linux-headers-5.10.0-9-amd64'
>   CC [M]  /usr/local/src/git/linux/drivers/net/phy/sfp.o
> /usr/local/src/git/linux/drivers/net/phy/sfp.c: In function ‘sfp_sm_mod_hpower’:
> /usr/local/src/git/linux/drivers/net/phy/sfp.c:1639:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
>  1639 |  u8 val;
>       |  ^~
>   MODPOST /usr/local/src/git/linux/drivers/net/phy/Module.symvers
>   CC [M]  /usr/local/src/git/linux/drivers/net/phy/sfp.mod.o
>   LD [M]  /usr/local/src/git/linux/drivers/net/phy/sfp.ko
> make: Leaving directory '/usr/src/linux-headers-5.10.0-9-amd64'
>
>
>
>
>
> Bjørn



-- 
株式会社スプリングボード
照山 周一郎
teruyama at springboard-inc.jp
http://www.springboard-inc.jp/
〒110-0005
東京都台東区上野3丁目2番2号
アイオス秋葉原505号室



More information about the openwrt-devel mailing list