照山周一郎 <[email protected]> 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@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@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@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@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 _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
