Re: [PATCH v2 4/5] i2c: i801: Add SMBUS_LEN_SENTINEL

[Heiner Kallweit <hkallweit1@gmail.com>]

On 01.02.2024 21:48, Andy Shevchenko wrote:
> On Thu, Feb 01, 2024 at 09:12:33PM +0100, Heiner Kallweit wrote:
>> Add a sentinel length value that is used to check whether we should
>> read and use the length value provided by the slave device.
>> This simplifies the currently used checks.
> 
> ...
> 
>> +		/* At transfer start i801_smbus_block_transaction() marks
>> +		 * the block length as invalid. Check for this sentinel value
>> +		 * and read the block length from SMBHSTDAT0.
>> +		 */
> 
> /*
>  * May we use the correct multi-line
>  * comment style, please?
>  */
> 

Right, everybody outside netdev uses this comment style

> ...
> 
>> +		/* At transfer start i801_smbus_block_transaction() marks
>> +		 * the block length as invalid. Check for this sentinel value
>> +		 * and read the block length from SMBHSTDAT0.
>> +		 */
> 
> Ditto.
> 
> ...
> 
>>  	if (read_write == I2C_SMBUS_READ && command == I2C_SMBUS_BLOCK_DATA)
>> -		data->block[0] = I2C_SMBUS_BLOCK_MAX;
>> +		data->block[0] = SMBUS_LEN_SENTINEL; /* Mark block length as invalid */
> 
> I would add a separated comment line on top.
> 
OK
>>  	else if (data->block[0] < 1 || data->block[0] > I2C_SMBUS_BLOCK_MAX)
>>  		return -EPROTO;
>