Re: [PATCH RFC russell-king 2/4] net: phy: sfp: add support for multigig RollBall modules - Russell King

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: "Marek Behún" <marek.behun@nic.cz>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Maxime Chevallier <maxime.chevallier@bootlin.com>,
	Baruch Siach <baruch@tkos.co.il>, Chris Healy <cphealy@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH RFC russell-king 2/4] net: phy: sfp: add support for multigig RollBall modules
Date: Wed, 12 Aug 2020 15:33:04 +0100	[thread overview]
Message-ID: <20200812143303.GO1551@shell.armlinux.org.uk> (raw)
In-Reply-To: <20200812153326.71b84e45@dellmb.labs.office.nic.cz>

On Wed, Aug 12, 2020 at 03:33:26PM +0200, Marek Behún wrote:
> On Tue, 11 Aug 2020 16:15:53 +0100
> Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> 
> > > +	if (rollball) {
> > > +		/* TODO: try to write this to EEPROM */
> > > +		id.base.extended_cc = SFF8024_ECC_10GBASE_T_SFI;  
> > 
> > Should we really be "fixing" vendors EEPROMs for them?
> > 
> 
> Are you reffering to the TODO comment or the id.base.extended_cc
> assignment?
> If the comment, well, your code does it for cotsworks modules, but I am
> actually indifferent.

No, that's Chris' code, and there's quite a bit of history there:
It appears Cotsworks programmed things like the serial number into
the EEPROM and did not update the checksums.  After quite some time,
it seems Cotsworks have seen sense, and have fixed their production
line to properly program the EEPROM, but that leaves a whole bunch
of modules with bad checksums.

I'm more than happy that we should continue issuing the warning, but
Chris has decided to fix them up.  I'm not particularly happy with
that idea, but I didn't get the chance to express it before David
picked up the patch.  So, it's now in mainline.

Fixing the checksum for a module that is known to suffer bad checksums
is one thing - it's a single byte write, and as the checksum is wrong,
it's likely other systems that know about the issue will ignore it.

However, changing the module description to be "correct" is a completely
different level - there are many modules that do not report "correct"
data, and, if we start fixing these up, it's likely that fixups that
other SFP cage implementations have could stop working since they may
not recognise the module.

Remember, things like the extended CC codes are dependent on the SFF
spec revisions, so if we start changing the extended CC code in byte
36, should we also change the SFF8472 compliance code as well (to
be > rev 11.9)?  Since SFF8472 rev 11.9 changed the definition of this
byte.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!