Re: [PATCH v2] i2c: xiic: Add max_read_len quirk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2019-06-05 8:18 a.m., Michal Simek wrote:
> On 04. 06. 19 23:55, Robert Hancock wrote:
>> This driver does not support reading more than 255 bytes at once because
>> the register for storing the number of bytes to read is only 8 bits. Add
>> a max_read_len quirk to enforce this.
>>
>> This was found when using this driver with the SFP driver, which was
>> previously reading all 256 bytes in the SFP EEPROM in one transaction.
>> This caused a bunch of hard-to-debug errors in the xiic driver since the
>> driver/logic was treating the number of bytes to read as zero.
>> Rejecting transactions that aren't supported at least allows the problem
>> to be diagnosed more easily.
>>
>> Signed-off-by: Robert Hancock <hancock@xxxxxxxxxxxxx>
>> ---
>>
>> Changes since v1: Added more rationale in description.
>>
>>  drivers/i2c/busses/i2c-xiic.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
>> index 0fea7c5..37b3b93 100644
>> --- a/drivers/i2c/busses/i2c-xiic.c
>> +++ b/drivers/i2c/busses/i2c-xiic.c
>> @@ -709,11 +709,16 @@ static u32 xiic_func(struct i2c_adapter *adap)
>>  	.functionality = xiic_func,
>>  };
>>  
>> +static const struct i2c_adapter_quirks xiic_quirks = {
>> +	.max_read_len = 255,
>> +};
>> +
>>  static const struct i2c_adapter xiic_adapter = {
>>  	.owner = THIS_MODULE,
>>  	.name = DRIVER_NAME,
>>  	.class = I2C_CLASS_DEPRECATED,
>>  	.algo = &xiic_algorithm,
>> +	.quirks = &xiic_quirks,
>>  };
>>  
> 
> Reviewed-by: Michal Simek <michal.simek@xxxxxxxxxx>
> 
> Nit: The same limitation is there for write. Maybe worth to also set it
> up. Anyway this can be done separately.

I am not sure that is the case - can you clarify why you say that? From
what I can tell, the problem on the read side is that the byte count to
receive is stored in the AXI IIC Transmit Data part of the TX FIFO
register, which is only 8 bits. From looking at the code, it doesn't
seem like the same issue exists on the write side - we just shove bytes
into the TX FIFO until the end of the message is reached. However, I
have not tested it, as the application I am using performs I2C reads only.

-- 
Robert Hancock
Senior Software Developer
SED Systems, a division of Calian Ltd.
Email: hancock@xxxxxxxxxxxxx



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux