* [PATCH v2] gpio: pca953x: remove incorrect le16_to_cpu calls
@ 2017-07-18 15:51 Andy Shevchenko
2017-07-19 11:56 ` Gregory CLEMENT
0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2017-07-18 15:51 UTC (permalink / raw)
To: Linus Walleij, linux-gpio, Sebastian Reichel, Gregory CLEMENT,
Yong Li, Phil Reid
Cc: Andy Shevchenko
i2c_smbus commands handle the correct byte order for smbus transactions
internally. This will currently result in incorrect operation on big
endian systems.
Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
- add RB tag
drivers/gpio/gpio-pca953x.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 4c9e21300a26..1b9dbf691ae7 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -187,10 +187,9 @@ static int pca953x_write_regs_8(struct pca953x_chip *chip, int reg, u8 *val)
static int pca953x_write_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
{
- __le16 word = cpu_to_le16(get_unaligned((u16 *)val));
+ u16 word = get_unaligned((u16 *)val);
- return i2c_smbus_write_word_data(chip->client,
- reg << 1, (__force u16)word);
+ return i2c_smbus_write_word_data(chip->client, reg << 1, word);
}
static int pca957x_write_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
@@ -241,8 +240,7 @@ static int pca953x_read_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
int ret;
ret = i2c_smbus_read_word_data(chip->client, reg << 1);
- val[0] = (u16)ret & 0xFF;
- val[1] = (u16)ret >> 8;
+ put_unaligned(ret, (u16 *)val);
return ret;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] gpio: pca953x: remove incorrect le16_to_cpu calls
2017-07-18 15:51 [PATCH v2] gpio: pca953x: remove incorrect le16_to_cpu calls Andy Shevchenko
@ 2017-07-19 11:56 ` Gregory CLEMENT
2017-07-19 12:14 ` Andy Shevchenko
2017-07-19 13:39 ` Phil Reid
0 siblings, 2 replies; 4+ messages in thread
From: Gregory CLEMENT @ 2017-07-19 11:56 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, linux-gpio, Sebastian Reichel, Yong Li, Phil Reid
Hi Andy,
On mar., juil. 18 2017, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> i2c_smbus commands handle the correct byte order for smbus
> transactions
I am not sure of this claim, I didn't find any cpu_to_lexx in the sbmus
function.
> internally. This will currently result in incorrect operation on big
> endian systems.
Did you managed to test this driver in a big endian system and to check
that with this patch the driver worked as expected?
Thanks,
Gregory
>
> Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> - add RB tag
> drivers/gpio/gpio-pca953x.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 4c9e21300a26..1b9dbf691ae7 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -187,10 +187,9 @@ static int pca953x_write_regs_8(struct pca953x_chip *chip, int reg, u8 *val)
>
> static int pca953x_write_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
> {
> - __le16 word = cpu_to_le16(get_unaligned((u16 *)val));
> + u16 word = get_unaligned((u16 *)val);
>
> - return i2c_smbus_write_word_data(chip->client,
> - reg << 1, (__force u16)word);
> + return i2c_smbus_write_word_data(chip->client, reg << 1, word);
> }
>
> static int pca957x_write_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
> @@ -241,8 +240,7 @@ static int pca953x_read_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
> int ret;
>
> ret = i2c_smbus_read_word_data(chip->client, reg << 1);
> - val[0] = (u16)ret & 0xFF;
> - val[1] = (u16)ret >> 8;
> + put_unaligned(ret, (u16 *)val);
>
> return ret;
> }
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] gpio: pca953x: remove incorrect le16_to_cpu calls
2017-07-19 11:56 ` Gregory CLEMENT
@ 2017-07-19 12:14 ` Andy Shevchenko
2017-07-19 13:39 ` Phil Reid
1 sibling, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2017-07-19 12:14 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Linus Walleij, linux-gpio, Sebastian Reichel, Yong Li, Phil Reid
On Wed, 2017-07-19 at 13:56 +0200, Gregory CLEMENT wrote:
> Hi Andy,
>
> On mar., juil. 18 2017, Andy Shevchenko <andriy.shevchenko@linux.inte
> l.com> wrote:
>
> > i2c_smbus commands handle the correct byte order for smbus
> > transactions
>
> I am not sure of this claim, I didn't find any cpu_to_lexx in the
> sbmus
> function.
Of course you didn't (I didn't either on the first glance, then I read
the code more carefully).
It does that byte-by-byte.
That explains why people do not notice it AFAIU Sebastian's claim.
>
> > internally. This will currently result in incorrect operation on big
> > endian systems.
>
> Did you managed to test this driver in a big endian system and to
> check
> that with this patch the driver worked as expected?
Unfortunately no. I have big endian systems, though I have no such
device connected to them.
>
> Thanks,
>
> Gregory
>
>
> >
> > Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > - add RB tag
> > drivers/gpio/gpio-pca953x.c | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-
> > pca953x.c
> > index 4c9e21300a26..1b9dbf691ae7 100644
> > --- a/drivers/gpio/gpio-pca953x.c
> > +++ b/drivers/gpio/gpio-pca953x.c
> > @@ -187,10 +187,9 @@ static int pca953x_write_regs_8(struct
> > pca953x_chip *chip, int reg, u8 *val)
> >
> > static int pca953x_write_regs_16(struct pca953x_chip *chip, int
> > reg, u8 *val)
> > {
> > - __le16 word = cpu_to_le16(get_unaligned((u16 *)val));
> > + u16 word = get_unaligned((u16 *)val);
> >
> > - return i2c_smbus_write_word_data(chip->client,
> > - reg << 1, (__force
> > u16)word);
> > + return i2c_smbus_write_word_data(chip->client, reg << 1,
> > word);
> > }
> >
> > static int pca957x_write_regs_16(struct pca953x_chip *chip, int
> > reg, u8 *val)
> > @@ -241,8 +240,7 @@ static int pca953x_read_regs_16(struct
> > pca953x_chip *chip, int reg, u8 *val)
> > int ret;
> >
> > ret = i2c_smbus_read_word_data(chip->client, reg << 1);
> > - val[0] = (u16)ret & 0xFF;
> > - val[1] = (u16)ret >> 8;
> > + put_unaligned(ret, (u16 *)val);
> >
> > return ret;
> > }
> > --
> > 2.11.0
> >
>
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] gpio: pca953x: remove incorrect le16_to_cpu calls
2017-07-19 11:56 ` Gregory CLEMENT
2017-07-19 12:14 ` Andy Shevchenko
@ 2017-07-19 13:39 ` Phil Reid
1 sibling, 0 replies; 4+ messages in thread
From: Phil Reid @ 2017-07-19 13:39 UTC (permalink / raw)
To: Gregory CLEMENT, Andy Shevchenko
Cc: Linus Walleij, linux-gpio, Sebastian Reichel, Yong Li
G'day Gregory,
On 19/07/2017 19:56, Gregory CLEMENT wrote:
> Hi Andy,
>
> On mar., juil. 18 2017, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
>> i2c_smbus commands handle the correct byte order for smbus
>> transactions
>
> I am not sure of this claim, I didn't find any cpu_to_lexx in the sbmus
> function.
For example see: i2c_smbus_xfer_emulated
The I2C_SMBUS_WORD_DATA case. byte copies are done to do the cpu to le conversion.
msgbuf0[1] = data->word & 0xff;
msgbuf0[2] = data->word >> 8;
The patch looks good to me.
Reviewed-by: Phil Reid <preid@electromag.com.au>
>
>> internally. This will currently result in incorrect operation on big
>> endian systems.
>
> Did you managed to test this driver in a big endian system and to check
> that with this patch the driver worked as expected?
>
> Thanks,
>
> Gregory
>
>
>>
>> Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
>> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>> - add RB tag
>> drivers/gpio/gpio-pca953x.c | 8 +++-----
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
>> index 4c9e21300a26..1b9dbf691ae7 100644
>> --- a/drivers/gpio/gpio-pca953x.c
>> +++ b/drivers/gpio/gpio-pca953x.c
>> @@ -187,10 +187,9 @@ static int pca953x_write_regs_8(struct pca953x_chip *chip, int reg, u8 *val)
>>
>> static int pca953x_write_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
>> {
>> - __le16 word = cpu_to_le16(get_unaligned((u16 *)val));
>> + u16 word = get_unaligned((u16 *)val);
>>
>> - return i2c_smbus_write_word_data(chip->client,
>> - reg << 1, (__force u16)word);
>> + return i2c_smbus_write_word_data(chip->client, reg << 1, word);
>> }
>>
>> static int pca957x_write_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
>> @@ -241,8 +240,7 @@ static int pca953x_read_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
>> int ret;
>>
>> ret = i2c_smbus_read_word_data(chip->client, reg << 1);
>> - val[0] = (u16)ret & 0xFF;
>> - val[1] = (u16)ret >> 8;
>> + put_unaligned(ret, (u16 *)val);
>>
>> return ret;
>> }
>> --
>> 2.11.0
>>
>
--
Regards
Phil Reid
ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au
3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@electromag.com.au
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-07-19 13:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-18 15:51 [PATCH v2] gpio: pca953x: remove incorrect le16_to_cpu calls Andy Shevchenko
2017-07-19 11:56 ` Gregory CLEMENT
2017-07-19 12:14 ` Andy Shevchenko
2017-07-19 13:39 ` Phil Reid
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).