Messages in this thread | | | From | Michal Simek <> | Subject | Re: [PATCH 2/2] usb: misc: usb5744: Add support for USB hub controller | Date | Wed, 10 Feb 2021 12:03:56 +0100 |
| |
Hi Andrew,
On 2/10/21 3:52 AM, Andrew Lunn wrote: > On Tue, Feb 09, 2021 at 10:53:20AM +0100, Michal Simek wrote: >> +static int usb5744_i2c_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + struct device *dev = &client->dev; >> + int ret; >> + >> + /* Trigger gpio reset to the hub. */ >> + ret = usb5744_init_hw(dev); >> + if (ret) >> + return ret; >> + >> + /* Send SMBus command to boot hub. */ >> + ret = i2c_smbus_write_word_data(client, 0xAA, swab16(0x5600)); > > Hi Michal > > This is not my area of the kernel. But that swab16() stood out, and > made me wonder about endianness. Will this work correctly on big and > little endian hosts?
thanks for bringing this up. I didn't test it on BE system.
I have grepped the kernel
[linux](xnext/usb5744)$ git grep i2c_smbus_write_word_data | grep swab drivers/media/i2c/uda1342.c:17: i2c_smbus_write_word_data(client, reg, swab16(value)); drivers/media/i2c/vpx3220.c:97: if (i2c_smbus_write_word_data(client, 0x27, swab16(fpaddr)) == -1) { drivers/media/i2c/vpx3220.c:106: if (i2c_smbus_write_word_data(client, 0x28, swab16(data)) == -1) { drivers/media/i2c/vpx3220.c:120: if (i2c_smbus_write_word_data(client, 0x26, swab16(fpaddr)) == -1) { drivers/usb/misc/usb5744.c:50: ret = i2c_smbus_write_word_data(client, 0xAA, swab16(0x5600)); include/linux/i2c.h:168: return i2c_smbus_write_word_data(client, command, swab16(value));
And last one is interesting
164 static inline s32 165 i2c_smbus_write_word_swapped(const struct i2c_client *client, 166 u8 command, u16 value) 167 { 168 return i2c_smbus_write_word_data(client, command, swab16(value)); 169 }
And this function is also used
[linux](xnext/usb5744)$ git grep i2c_smbus_write_word_swapped | wc -l 76
I think it would be the best to test it and see if this code works on BE but I need to prepare it first. And current code is aligned with others but it doesn't mean that it is correct.
Thanks, Michal
| |