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

[Andrew Lunn]

On Thu, Dec 31, 2020 at 01:14:10PM +0100, Pali Rohár wrote:
> On Wednesday 30 December 2020 19:09:58 Russell King - ARM Linux admin wrote:
> > On Wed, Dec 30, 2020 at 06:43:07PM +0100, Pali Rohár wrote:
> > > On Wednesday 30 December 2020 18:13:15 Andrew Lunn wrote:
> > > > Hi Pali
> > > > 
> > > > I have to agree with Russell here. I would rather have no diagnostics
> > > > than untrustable diagnostics.
> > > 
> > > Ok!
> > > 
> > > So should we completely skip hwmon_device_register_with_info() call
> > > if (i2c_block_size < 2) ?
> > 
> > I don't think that alone is sufficient - there's also the matter of
> > ethtool -m which will dump that information as well, and we don't want
> > to offer it to userspace in an unreliable form.
> 
> Any idea/preference how to disable access to these registers?

Page A0, byte 92:

"Diagnostic Monitoring Type" is a 1 byte field with 8 single bit
indicators describing how diagnostic monitoring is implemented in the
particular transceiver.

Note that if bit 6, address 92 is set indicating that digital
diagnostic monitoring has been implemented, received power
monitoring, transmitted power monitoring, bias current monitoring,
supply voltage monitoring and temperature monitoring must all be
implemented. Additionally, alarm and warning thresholds must be
written as specified in this document at locations 00 to 55 on
two-wire serial address 1010001X (A2h) (see Table 8-5).

Unfortunately, we cannot simply set sfp->id.ext.diagmon to false,
because it can also be used to indicate power, software reading of
TX_DISABLE, LOS, etc. These are all single bytes, so could be returned
correctly, assuming they have been implemented according to the spec.

Looking at sfp_module_info(), adding a check for i2c_block_size < 2
when determining what length to return. ethtool should do the right
thing, know that the second page has not been returned to user space.

	Andrew