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@xxxxxxxxx> >> --- >> 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; >
