[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