Re: [PATCH] i2c: cadence: Support PEC for SMBus block read

[Michal Simek <michal.simek@amd.com>]

On 7/18/22 14:37, Datta, Shubhrajyoti wrote:
> [AMD Official Use Only - General]
> 
> 
> 
>> -----Original Message-----
>> From: Simek, Michal <michal.simek@amd.com>
>> Sent: Monday, July 18, 2022 2:56 PM
>> To: Lars-Peter Clausen <lars@metafoo.de>; Wolfram Sang
>> <wsa@kernel.org>; Datta, Shubhrajyoti <shubhrajyoti.datta@amd.com>
>> Cc: linux-i2c@vger.kernel.org; git <git@xilinx.com>
>> Subject: Re: [PATCH] i2c: cadence: Support PEC for SMBus block read
>>
>>
>>
>> On 7/17/22 16:52, Lars-Peter Clausen wrote:
>>> SMBus packet error checking (PEC) is implemented by appending one
>>> additional byte of checksum data at the end of the message. This
>>> provides additional protection and allows to detect data corruption on the
>> I2C bus.
>>>
>>> SMBus block reads support variable length reads. The first byte in the
>>> read message is the number of available data bytes.
>>>
>>> The combination of PEC and block read is currently not supported by
>>> the Cadence I2C driver.
>>>    * When PEC is enabled the maximum transfer length for block reads
>>>      increases from 33 to 34 bytes.
>>>    * The I2C core smbus emulation layer relies on the driver updating the
>>>      `i2c_msg` `len` field with the number of received bytes. The updated
>>>      length is used when checking the PEC.
>>>
>>> Add support to the Cadence I2C driver for handling SMBus block reads
>>> with PEC. To determine the maximum transfer length uses the initial
>>> `len` value of the `i2c_msg`. When PEC is enabled this will be 2, when
>>> it is disabled it will be 1.
>>>
>>> Once a read transfer is done also increment the `len` field by the
>>> amount of received data bytes.
>>>
>>> This change has been tested with a UCM90320 PMBus power monitor,
>> which
>>> requires block reads to access certain data fields, but also has PEC
>>> enabled by default.
>>>
>>> Fixes: df8eb5691c48 ("i2c: Add driver for Cadence I2C controller")
>>
>> Subject is saying that you adding support for PEC and here you are saying
>> that it is fixing initial commit.
>>
>> If this is adding new support I think Fixes tag shouldn't be here.
>>
>> If it is fixing issue subject should be updated and this Fixes tag kept here.
>>
>> The rest looks good to me.
>>
>> Shubhrajyoti: Can you please test?
> 
> I have tested the reads and write smbus  without packet error check.

Can you please switch it to formal Tested-by: tag?

M