Re: [PATCH 5/5] input/rmi4/rmi_smbus.c: don't increment rmiaddr in rmi_smb_read_block()

[Hans Verkuil <hverkuil-cisco@xs4all.nl>]

Hi Lucas,

On 11/19/19 12:48 PM, Lucas Stach wrote:
> Hi Hans,
> 
> On Di, 2019-11-19 at 11:51 +0100, Hans Verkuil wrote:
>> This increment of rmi_smbus causes garbage to be returned.
>> The first read of SMB_MAX_COUNT bytes is fine, but after that
>> it is nonsense. Trial-and-error showed that by dropping the
>> increment of rmiaddr everything is fine and the F54 function
>> properly works.
>>
>> Even going back to the original code when F54 was added, I
>> could not make it work without this patch. So I do not understand
>> how this ever worked.
> 
> My guess is that F54 has mostly been tested with touchscreens that are
> connected to a regular i2c bus, not smbus. i2c has a separate transport
> implementation in rmi4. Most of the other functions are probably
> reading much smaller block data than F54.

That's my suspicion as well. I tried to configure my kernel so that it
would be using i2c instead of smbus, but I couldn't make it work. I'll
have to try again next week.

I only have Rev E of the RMI4 spec, which does not appear to document
the mapping table entry that rmi_smb_get_command_code() uses.

Do you (or someone else) have access to a newer version? If so, can you check
how this is supposed to work for reading large blocks over smbus?

With the current code it will create a new mapping entry for every 32
byte read. I suspect that that is not how it is supposed to work, but
without a spec this is just trial-and-error.

Regards,

	Hans

> 
> Regards,
> Lucas
> 
>> My guess is that the same change is needed in rmi_smb_write_block,
>> but I wouldn't know how to test this.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>  drivers/input/rmi4/rmi_smbus.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
>> index 2407ea43de59..79ecea5edacc 100644
>> --- a/drivers/input/rmi4/rmi_smbus.c
>> +++ b/drivers/input/rmi4/rmi_smbus.c
>> @@ -215,7 +215,6 @@ static int rmi_smb_read_block(struct rmi_transport_dev *xport, u16 rmiaddr,
>>  		/* prepare to read next block of bytes */
>>  		cur_len -= SMB_MAX_COUNT;
>>  		databuff += SMB_MAX_COUNT;
>> -		rmiaddr += SMB_MAX_COUNT;
>>  	}
>>  
>>  	retval = 0;
>