* [PATCH] HID: cp2112: Fix I2C_BLOCK_DATA transactions
@ 2017-11-14 0:03 Eudean Sun
2017-11-21 16:42 ` Jiri Kosina
0 siblings, 1 reply; 4+ messages in thread
From: Eudean Sun @ 2017-11-14 0:03 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, linux-kernel
The existing driver erroneously treats I2C_BLOCK_DATA and BLOCK_DATA
commands the same. Fix the logic for I2C_BLOCK_DATA reads and writes.
Signed-off-by: Eudean Sun <eudean@arista.com>
---
drivers/hid/hid-cp2112.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 078026f..0adece2 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -692,8 +692,16 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
(u8 *)&word, 2);
break;
case I2C_SMBUS_I2C_BLOCK_DATA:
- size = I2C_SMBUS_BLOCK_DATA;
- /* fallthrough */
+ if (read_write == I2C_SMBUS_READ) {
+ read_length = data->block[0];
+ count = cp2112_write_read_req(buf, addr, read_length,
+ command, NULL, 0);
+ } else {
+ count = cp2112_write_req(buf, addr, command,
+ data->block + 1,
+ data->block[0]);
+ }
+ break;
case I2C_SMBUS_BLOCK_DATA:
if (I2C_SMBUS_READ == read_write) {
count = cp2112_write_read_req(buf, addr,
@@ -781,6 +789,9 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
case I2C_SMBUS_WORD_DATA:
data->word = le16_to_cpup((__le16 *)buf);
break;
+ case I2C_SMBUS_I2C_BLOCK_DATA:
+ memcpy(data->block + 1, buf, read_length);
+ break;
case I2C_SMBUS_BLOCK_DATA:
if (read_length > I2C_SMBUS_BLOCK_MAX) {
ret = -EPROTO;
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: cp2112: Fix I2C_BLOCK_DATA transactions
2017-11-14 0:03 [PATCH] HID: cp2112: Fix I2C_BLOCK_DATA transactions Eudean Sun
@ 2017-11-21 16:42 ` Jiri Kosina
2017-11-21 16:45 ` Jiri Kosina
0 siblings, 1 reply; 4+ messages in thread
From: Jiri Kosina @ 2017-11-21 16:42 UTC (permalink / raw)
To: Eudean Sun; +Cc: Benjamin Tissoires, linux-input, linux-kernel, David Barksdale
On Mon, 13 Nov 2017, Eudean Sun wrote:
> The existing driver erroneously treats I2C_BLOCK_DATA and BLOCK_DATA
> commands the same. Fix the logic for I2C_BLOCK_DATA reads and writes.
Addin David, the original author of the driver (and the code in question
as well) to CC.
>
> Signed-off-by: Eudean Sun <eudean@arista.com>
> ---
> drivers/hid/hid-cp2112.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 078026f..0adece2 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
> @@ -692,8 +692,16 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
> (u8 *)&word, 2);
> break;
> case I2C_SMBUS_I2C_BLOCK_DATA:
> - size = I2C_SMBUS_BLOCK_DATA;
> - /* fallthrough */
> + if (read_write == I2C_SMBUS_READ) {
> + read_length = data->block[0];
> + count = cp2112_write_read_req(buf, addr, read_length,
> + command, NULL, 0);
> + } else {
> + count = cp2112_write_req(buf, addr, command,
> + data->block + 1,
> + data->block[0]);
> + }
> + break;
> case I2C_SMBUS_BLOCK_DATA:
> if (I2C_SMBUS_READ == read_write) {
> count = cp2112_write_read_req(buf, addr,
> @@ -781,6 +789,9 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
> case I2C_SMBUS_WORD_DATA:
> data->word = le16_to_cpup((__le16 *)buf);
> break;
> + case I2C_SMBUS_I2C_BLOCK_DATA:
> + memcpy(data->block + 1, buf, read_length);
> + break;
> case I2C_SMBUS_BLOCK_DATA:
> if (read_length > I2C_SMBUS_BLOCK_MAX) {
> ret = -EPROTO;
> --
> 2.7.4
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: cp2112: Fix I2C_BLOCK_DATA transactions
2017-11-21 16:42 ` Jiri Kosina
@ 2017-11-21 16:45 ` Jiri Kosina
2017-11-21 18:40 ` Eudean Sun
0 siblings, 1 reply; 4+ messages in thread
From: Jiri Kosina @ 2017-11-21 16:45 UTC (permalink / raw)
To: Eudean Sun; +Cc: Benjamin Tissoires, linux-input, linux-kernel
On Tue, 21 Nov 2017, Jiri Kosina wrote:
> > The existing driver erroneously treats I2C_BLOCK_DATA and BLOCK_DATA
> > commands the same. Fix the logic for I2C_BLOCK_DATA reads and writes.
>
> Addin David, the original author of the driver (and the code in question
> as well) to CC.
Alright, that address bounces.
In that case -- could you please be a little bit more specific in the
changelog? Namely: what are the observable effects of this bug (did you
find it by code inspection or have you seen it triggering) and why this is
a proper fix for that bug.
Thanks!
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: cp2112: Fix I2C_BLOCK_DATA transactions
2017-11-21 16:45 ` Jiri Kosina
@ 2017-11-21 18:40 ` Eudean Sun
0 siblings, 0 replies; 4+ messages in thread
From: Eudean Sun @ 2017-11-21 18:40 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, linux-input, linux-kernel
On Tue, Nov 21, 2017 at 05:45:32PM +0100, Jiri Kosina wrote:
> On Tue, 21 Nov 2017, Jiri Kosina wrote:
>
> > > The existing driver erroneously treats I2C_BLOCK_DATA and BLOCK_DATA
> > > commands the same. Fix the logic for I2C_BLOCK_DATA reads and writes.
> >
> > Addin David, the original author of the driver (and the code in question
> > as well) to CC.
>
> Alright, that address bounces.
>
> In that case -- could you please be a little bit more specific in the
> changelog? Namely: what are the observable effects of this bug (did you
> find it by code inspection or have you seen it triggering) and why this is
> a proper fix for that bug.
Thanks for the feedback Jiri, I will send out a new patch shortly with more detail in the changelog.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-21 18:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-14 0:03 [PATCH] HID: cp2112: Fix I2C_BLOCK_DATA transactions Eudean Sun
2017-11-21 16:42 ` Jiri Kosina
2017-11-21 16:45 ` Jiri Kosina
2017-11-21 18:40 ` Eudean Sun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).