| Message ID | 20191119105118.54285-6-hverkuil-cisco@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | input: rmi4/synaptics fixes | expand |
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. 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;
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; >
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;
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 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(-)