RE: [patch v8 1/1] i2c: add master driver for mellanox systems

[Vadim Pasternak]

Hi Wolfram,

> -----Original Message-----
> From: Wolfram Sang [mailto:wsa-dev@xxxxxxxxxxxxxxxxxxxx]
> Sent: Saturday, November 19, 2016 11:05 PM
> To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> Cc: wsa@xxxxxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; jiri@xxxxxxxxxxx; Michael Shych
> <michaelsh@xxxxxxxxxxxx>
> Subject: Re: [patch v8 1/1] i2c: add master driver for mellanox systems
> 
> Hi,
> 
> looks mostly good. I just found this comment which needs clarification:
> 
> > +	/*
> > +	 * All upper layers currently are never use transfer with more than
> > +	 * 2 messages.
> 
> Have you really checked ALL of the upper layers? And even if so, there is the dev-
> interface to userspace which allows for arbitrary I2C transfers using I2C_RDWR.
> 

We are using the next drivers on our switches: max11603, lm75, pmbus, mlxsw_minimal (last one is new, I wrote it to support Mellanox ASIC and it's has 32 byte limitation for I2C transaction chunk) and at24.

I am planning to add some code in the next patch, for handling bufferization for message length greater then data buffer. Allowed by HW.

Possibly I should add validation for "num" parameter at the beginning of mlxcpld_i2c_xfer?

> >  Actually, it's also not so relevant in Mellanox systems
> > +	 * because of HW limitation.
> 
> What kind of HW limitation do you mean here? Can Mellanox send more than
> two messages?
> 
> > Max size of transfer is o more than 20B
> 
> "is o more"? Typo?

Typo.
Max size of transfer is not more than 32 bytes.

> 
> > +	 * in current x86 LPCI2C bridge.
> 
> What does that mean in result?

In case more than 32 bytes of data length will be set in transaction, such message will fail length validation.
The 32 is HW limitation - 32 registers of DATAx.
In new version of LPCI2C logic, there is 64 registers, which can be used for data, and I am going to support it in next patch for this driver.

> 
> Regards,
> 
>    Wolfram

Thanks,

Vadim.