On Wed, 12 Aug 2020 15:33:04 +0100
Russell King - ARM Linux admin <li...@armlinux.org.uk> wrote:
> 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 <li...@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.
>
Thank you Russell for this explanation.
