[PATCH v1] HID: cp2112: fix byte order in SMBUS operations

[PATCH v1] HID: cp2112: fix byte order in SMBUS operations

[Ellen Wang]

Change all occurrences of be16 to le16 in cp2112_xfer(),
because SMBUS words are little endian, not big endian.

Signed-off-by: Ellen Wang <ellen-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
---
 drivers/hid/hid-cp2112.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 1d24a65..a3703b8 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -589,7 +589,7 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
 	struct cp2112_device *dev = (struct cp2112_device *)adap->algo_data;
 	struct hid_device *hdev = dev->hdev;
 	u8 buf[64];
-	__be16 word;
+	__le16 word;
 	ssize_t count;
 	size_t read_length = 0;
 	unsigned int retries;
@@ -621,7 +621,7 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
 		break;
 	case I2C_SMBUS_WORD_DATA:
 		read_length = 2;
-		word = cpu_to_be16(data->word);
+		word = cpu_to_le16(data->word);
 
 		if (I2C_SMBUS_READ == read_write)
 			count = cp2112_write_read_req(buf, addr, read_length,
@@ -634,7 +634,7 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
 		size = I2C_SMBUS_WORD_DATA;
 		read_write = I2C_SMBUS_READ;
 		read_length = 2;
-		word = cpu_to_be16(data->word);
+		word = cpu_to_le16(data->word);
 
 		count = cp2112_write_read_req(buf, addr, read_length, command,
 					      (u8 *)&word, 2);
@@ -727,7 +727,7 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
 		data->byte = buf[0];
 		break;
 	case I2C_SMBUS_WORD_DATA:
-		data->word = be16_to_cpup((__be16 *)buf);
+		data->word = le16_to_cpup((__le16 *)buf);
 		break;
 	case I2C_SMBUS_BLOCK_DATA:
 		if (read_length > I2C_SMBUS_BLOCK_MAX) {
-- 
1.9.1

Re: [PATCH v1] HID: cp2112: fix byte order in SMBUS operations

[Jiri Kosina]

On Thu, 9 Jul 2015, Ellen Wang wrote:

> Change all occurrences of be16 to le16 in cp2112_xfer(),
> because SMBUS words are little endian, not big endian.
> 
> Signed-off-by: Ellen Wang <ellen@cumulusnetworks.com>

Alright, this means that I2C_SMBUS_WORD_DATA and I2C_SMBUS_PROC_CALL have 
been terminally broken since the very beginning and noone noticed.

Does this mean that those commands are in practice mostly unused anyway? 
This is important for 4.2 vs 4.3 consideration.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v1] HID: cp2112: fix byte order in SMBUS operations

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Mon, Jul 13, 2015 at 01:23:52PM +0200, Jiri Kosina wrote:
> On Thu, 9 Jul 2015, Ellen Wang wrote:
> 
> > Change all occurrences of be16 to le16 in cp2112_xfer(),
> > because SMBUS words are little endian, not big endian.
> > 
> > Signed-off-by: Ellen Wang <ellen@cumulusnetworks.com>
> 
> Alright, this means that I2C_SMBUS_WORD_DATA and I2C_SMBUS_PROC_CALL have 
> been terminally broken since the very beginning and noone noticed.
> 
> Does this mean that those commands are in practice mostly unused anyway? 
> This is important for 4.2 vs 4.3 consideration.

PROC_CALL is really rare. WORD_DATA depends on the setup, it is used.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v1] HID: cp2112: fix byte order in SMBUS operations

[Ellen Wang]

On 7/13/2015 7:41 AM, Wolfram Sang wrote:
> On Mon, Jul 13, 2015 at 01:23:52PM +0200, Jiri Kosina wrote:
>> On Thu, 9 Jul 2015, Ellen Wang wrote:
>>
>>> Change all occurrences of be16 to le16 in cp2112_xfer(),
>>> because SMBUS words are little endian, not big endian.
>>>
>>> Signed-off-by: Ellen Wang <ellen-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
>>
>> Alright, this means that I2C_SMBUS_WORD_DATA and I2C_SMBUS_PROC_CALL have
>> been terminally broken since the very beginning and noone noticed.
>>
>> Does this mean that those commands are in practice mostly unused anyway?
>> This is important for 4.2 vs 4.3 consideration.
>
> PROC_CALL is really rare. WORD_DATA depends on the setup, it is used.

Yes.  WORD_DATA is common.  It's probably become less used since Antonio 
implemented i2c operations.

I think this really means that the device itself is rarely used.  On the 
other hand, for the people who unfortunately have it, these are 
important fixes.

Re: [PATCH v1] HID: cp2112: fix byte order in SMBUS operations

[Jiri Kosina]

On Thu, 9 Jul 2015, Ellen Wang wrote:

> Change all occurrences of be16 to le16 in cp2112_xfer(),
> because SMBUS words are little endian, not big endian.
> 
> Signed-off-by: Ellen Wang <ellen@cumulusnetworks.com>

I've tagged this for stable and applied to for-4.3/cp2112.

-- 
Jiri Kosina
SUSE Labs