Subject: RE: [PATCH net-next v3 2/8] i2c: designware: Add driver support for Wangxun 10Gb NIC

From: Jiawen Wu <[email-masked]>
Date: Fri, 21 Apr 2023 10:20:17 +0800

On Thursday, April 20, 2023 9:23 PM, Andrew Lunn wrote:
> On Thu, Apr 20, 2023 at 06:29:11PM +0800, Jiawen Wu wrote:
> > On Thursday, April 20, 2023 4:58 AM, Andrew Lunn wrote:
> > > On Wed, Apr 19, 2023 at 04:27:33PM +0800, Jiawen Wu wrote:
> > > > Wangxun 10Gb ethernet chip is connected to Designware I2C, to communicate
> > > > with SFP.
> > > >
> > > > Add platform data to pass IOMEM base address, board flag and other
> > > > parameters, since resource address was mapped on ethernet driver.
> > > >
> > > > The exists IP limitations are dealt as workarounds:
> > > > - IP does not support interrupt mode, it works on polling mode.
> > > > - I2C cannot read continuously, only one byte can at a time.
> > >
> > > Are you really sure about that?
> > >
> > > It is a major limitation for SFP devices. It means you cannot access
> > > the diagnostics, since you need to perform an atomic 2 byte read.
> > >
> > > Or maybe i'm understanding you wrong.
> > >
> > >    Andrew
> > >
> >
> > Maybe I'm a little confused about this. Every time I read a byte info, I have to
> > write a 'read command'. It can normally get the information for SFP devices.
> > But I'm not sure if this is regular I2C behavior.
> 
> I don't know this hardware, so i cannot say what a 'read command'
> actually does. Can you put a bus pirate or similar sort of device on
> the bus and look at the actual I2C signals. Is it performing one I2C
> transaction per byte? If so, that is not good.
> 
> The diagnostic values, things like temperature sensor, voltage sensor,
> received signal power are all 16 bits. You cannot read them using two
> time one byte reads. Say the first read sees a 16bit value of 0x00FF,
> but only reads the first byte. The second read sees a 16bit value of
> 0x0100 but only reads the second byte. You end up with 0x0000. When
> you do a multi byte read, the SFP should do an atomic read of the
> sensor, so you would see either 0x00FF, or 0x0100.
> 
> If your hardware can only do single byte reads, please make sure the
> I2C framework knows this. The SFP driver should then refuse to access
> the diagnostic parts of the SFP, because your I2C bus master hardware
> is too broken. The rest of the SFP should still work.
> 
> 	Andrew.
> 

You may have misunderstood. If you want to read a 16-bit message, the
size of 'i2c_msg.len' is set to 2 in the array that 'flags = I2C_M_RD'.

For example in sfp_i2c_read(), block_size limits the message length of every
time call i2c_transfer() to read, usually it is 16. The one-byte read limit I
mentioned means that during the 16-byte read, I2C device needs to write a
read command and then read the message 16 times to fill the 16-byte buffer,
instead of reading 16 bytes at once after stop command writes.

Before I thought this behavior might be strange, so I mentioned in the commit.