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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 21 Apr 2023 10:20:17 +0800
From:   Jiawen Wu <jiawenwu@...stnetic.com>
To:     "'Andrew Lunn'" <andrew@...n.ch>
Cc:     <netdev@...r.kernel.org>, <linux@...linux.org.uk>,
        <linux-i2c@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
        <olteanv@...il.com>, <mengyuanlou@...-swift.com>,
        "'Jarkko Nikula'" <jarkko.nikula@...ux.intel.com>
Subject: RE: [PATCH net-next v3 2/8] i2c: designware: Add driver support for Wangxun 10Gb NIC

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.