Re: [PATCH v2 1/3] net: sfp: add workaround for Realtek RTL8672 and RTL9601C chips

[Pali Rohár]

On Thursday 07 January 2021 21:21:16 Marek Behún wrote:
> On Thu, 7 Jan 2021 19:45:49 +0000
> Russell King - ARM Linux admin <linux@xxxxxxxxxxxxxxx> wrote:
> 
> > I think you're not reading the code very well. It checks for bytes at
> > offset 1..blocksize-1, blocksize+1..2*blocksize-1, etc are zero. It
> > does _not_ check that byte 0 or the byte at N*blocksize is zero - these
> > bytes are skipped. In other words, the first byte of each transfer can
> > be any value. The other bytes of the _entire_ ID must be zero.
> 
> Wouldn't it be better, instead of checking if 1..blocksize-1 are zero,
> to check whether reading byte by byte returns the same as reading 16
> bytes whole?

It would means to read EEPROM two times unconditionally for every SFP.
With current solution we read EEPROM two times only for these buggy
RTL-based SFP modules. For all other SFPs EEPROM content is read only
one time. I like current solution because we do not change the way how
are other (non-broken) SFPs detected. It is better to not touch things
which are not broken.

And as we know that these zeros are expected behavior on these broken
RTL-based SFPs I think such test is fine.

Moreover there are Nokia SFPs which do not like one byte read and locks
i2c bus. Yes, it happens only for EEPROM content on second address
(therefore ID part for this test is not affected) but who knows how
broken would be any other SFPs in future.