[PATCH] input:ad7879-i2c use swapped varient of i2c_smbus_read_word_data

[PATCH] input:ad7879-i2c use swapped varient of i2c_smbus_read_word_data

[Jonathan Cameron]

This varient was introduced in
i2c: boilerplate function for byte swapped smbus_write/read_word_data

This also has the side effect of ensuring any errors from the i2c
read and no longer mangled.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

---
The patch introducing this swapped function is working its way through the i2c
tree.

 drivers/input/touchscreen/ad7879-i2c.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
index 4e4e58c..1c1683e 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -47,7 +47,7 @@ static int ad7879_i2c_read(struct device *dev, u8 reg)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 
-	return swab16(i2c_smbus_read_word_data(client, reg));
+	return i2c_smbus_read_word_swapped(client, reg);
 }
 
 static int ad7879_i2c_multi_read(struct device *dev,
-- 
1.7.7

RE: [PATCH] input:ad7879-i2c use swapped varient of i2c_smbus_read_word_data

[Hennerich, Michael]

Jonathan Cameron wrote on 2011-10-21:
> This varient was introduced in
> i2c: boilerplate function for byte swapped smbus_write/read_word_data
>
> This also has the side effect of ensuring any errors from the i2c
> read and no longer mangled.
>
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

Acked-by: Michael Hennerich <michael.hennerich@analog.com>

>
> ---
> The patch introducing this swapped function is working its way through
> the i2c
> tree.
>
>  drivers/input/touchscreen/ad7879-i2c.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> diff --git a/drivers/input/touchscreen/ad7879-i2c.c
> b/drivers/input/touchscreen/ad7879-i2c.c index 4e4e58c..1c1683e 100644
> --- a/drivers/input/touchscreen/ad7879-i2c.c +++
> b/drivers/input/touchscreen/ad7879-i2c.c @@ -47,7 +47,7 @@ static int
> ad7879_i2c_read(struct device *dev, u8 reg)
>  {
>       struct i2c_client *client = to_i2c_client(dev);
> -     return swab16(i2c_smbus_read_word_data(client, reg));
> +     return i2c_smbus_read_word_swapped(client, reg);
>  }
>
>  static int ad7879_i2c_multi_read(struct device *dev,

Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret Seif

RE: [PATCH] input:ad7879-i2c use swapped varient of i2c_smbus_read_word_data

[Hennerich, Michael]

Hennerich, Michael wrote on 2011-10-21:
> Jonathan Cameron wrote on 2011-10-21:
>> This varient was introduced in
>> i2c: boilerplate function for byte swapped smbus_write/read_word_data
>>
>> This also has the side effect of ensuring any errors from the i2c
>> read and no longer mangled.
>>
>> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
>
> Acked-by: Michael Hennerich <michael.hennerich@analog.com>

I think we missing one case in ad7879_i2c_write()...

>
>>
>> --- The patch introducing this swapped function is working its way
>> through the i2c tree.
>>
>>  drivers/input/touchscreen/ad7879-i2c.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> diff --git a/drivers/input/touchscreen/ad7879-i2c.c
>> b/drivers/input/touchscreen/ad7879-i2c.c index 4e4e58c..1c1683e 100644
>> --- a/drivers/input/touchscreen/ad7879-i2c.c +++
>> b/drivers/input/touchscreen/ad7879-i2c.c @@ -47,7 +47,7 @@ static int
>> ad7879_i2c_read(struct device *dev, u8 reg)
>>  {
>>       struct i2c_client *client = to_i2c_client(dev);
>> -     return swab16(i2c_smbus_read_word_data(client, reg));
>> +     return i2c_smbus_read_word_swapped(client, reg);
>>  }
>>
>>  static int ad7879_i2c_multi_read(struct device *dev,
>
> Greetings,
> Michael
>

Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret Seif

[PATCH V2] input:ad7879-i2c use swapped varient of i2c_smbus_read_word_data

[Jonathan Cameron]

This varient was introduced in
i2c: boilerplate function for byte swapped smbus_write/read_word_data

This also has the side effect of ensuring any errors from the i2c
read and no longer mangled.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
V2: include the write function as pointed out by Michael Hennerich.

The patch introducing this swapped function is working its way through the i2c
tree.

 drivers/input/touchscreen/ad7879-i2c.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
index 4e4e58c..cc51392 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -47,7 +47,7 @@ static int ad7879_i2c_read(struct device *dev, u8 reg)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 
-	return swab16(i2c_smbus_read_word_data(client, reg));
+	return i2c_smbus_read_word_swapped(client, reg);
 }
 
 static int ad7879_i2c_multi_read(struct device *dev,
@@ -68,7 +68,7 @@ static int ad7879_i2c_write(struct device *dev, u8 reg, u16 val)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 
-	return i2c_smbus_write_word_data(client, reg, swab16(val));
+	return i2c_smbus_write_word_swapped(client, reg, val);
 }
 
 static const struct ad7879_bus_ops ad7879_i2c_bus_ops = {
-- 
1.7.7

Re: [PATCH V2] input:ad7879-i2c use swapped varient of i2c_smbus_read_word_data

[Dmitry Torokhov]

On Fri, Oct 21, 2011 at 12:57:16PM +0100, Jonathan Cameron wrote:
> This varient was introduced in
> i2c: boilerplate function for byte swapped smbus_write/read_word_data
> 
> This also has the side effect of ensuring any errors from the i2c
> read and no longer mangled.
> 
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
> V2: include the write function as pointed out by Michael Hennerich.
> 
> The patch introducing this swapped function is working its way through the i2c
> tree.
> 
>  drivers/input/touchscreen/ad7879-i2c.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
> index 4e4e58c..cc51392 100644
> --- a/drivers/input/touchscreen/ad7879-i2c.c
> +++ b/drivers/input/touchscreen/ad7879-i2c.c
> @@ -47,7 +47,7 @@ static int ad7879_i2c_read(struct device *dev, u8 reg)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
>  
> -	return swab16(i2c_smbus_read_word_data(client, reg));
> +	return i2c_smbus_read_word_swapped(client, reg);

This is still not endian-safe. I wonder if introducing
i2c_smbus_read_word_swapped() just makes the metter worse by hiding
this...

I'd prefer if we had i2c_smbus_read_be16() instead.

-- 
Dmitry

Re: [PATCH V2] input:ad7879-i2c use swapped varient of i2c_smbus_read_word_data

[Jonathan Cameron]

On 10/21/11 17:09, Dmitry Torokhov wrote:
> On Fri, Oct 21, 2011 at 12:57:16PM +0100, Jonathan Cameron wrote:
>> This varient was introduced in
>> i2c: boilerplate function for byte swapped smbus_write/read_word_data
>>
>> This also has the side effect of ensuring any errors from the i2c
>> read and no longer mangled.
>>
>> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
>> ---
>> V2: include the write function as pointed out by Michael Hennerich.
>>
>> The patch introducing this swapped function is working its way through the i2c
>> tree.
>>
>>  drivers/input/touchscreen/ad7879-i2c.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
>> index 4e4e58c..cc51392 100644
>> --- a/drivers/input/touchscreen/ad7879-i2c.c
>> +++ b/drivers/input/touchscreen/ad7879-i2c.c
>> @@ -47,7 +47,7 @@ static int ad7879_i2c_read(struct device *dev, u8 reg)
>>  {
>>  	struct i2c_client *client = to_i2c_client(dev);
>>  
>> -	return swab16(i2c_smbus_read_word_data(client, reg));
>> +	return i2c_smbus_read_word_swapped(client, reg);
> 
> This is still not endian-safe. I wonder if introducing
> i2c_smbus_read_word_swapped() just makes the metter worse by hiding
> this...
> 
> I'd prefer if we had i2c_smbus_read_be16() instead.
smbus spec says that all transfers must be little endian.
Hence it is type safe because the smbus controllers are guaranteed to have made
that assumption and converted to cpu endianess.  If not they are buggy.
Hence, this is endian safe.

Unfortunately numerous devices do the exact opposite (for successive approximation
ADCs it would be silly to do otherwise.) Hence they use the smbus protocol, but
then have to flip the bytes.

Re: [PATCH V2] input:ad7879-i2c use swapped varient of i2c_smbus_read_word_data

[Dmitry Torokhov]

On Fri, Oct 21, 2011 at 05:14:45PM +0100, Jonathan Cameron wrote:
> On 10/21/11 17:09, Dmitry Torokhov wrote:
> > On Fri, Oct 21, 2011 at 12:57:16PM +0100, Jonathan Cameron wrote:
> >> This varient was introduced in
> >> i2c: boilerplate function for byte swapped smbus_write/read_word_data
> >>
> >> This also has the side effect of ensuring any errors from the i2c
> >> read and no longer mangled.
> >>
> >> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> >> ---
> >> V2: include the write function as pointed out by Michael Hennerich.
> >>
> >> The patch introducing this swapped function is working its way through the i2c
> >> tree.
> >>
> >>  drivers/input/touchscreen/ad7879-i2c.c |    4 ++--
> >>  1 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
> >> index 4e4e58c..cc51392 100644
> >> --- a/drivers/input/touchscreen/ad7879-i2c.c
> >> +++ b/drivers/input/touchscreen/ad7879-i2c.c
> >> @@ -47,7 +47,7 @@ static int ad7879_i2c_read(struct device *dev, u8 reg)
> >>  {
> >>  	struct i2c_client *client = to_i2c_client(dev);
> >>  
> >> -	return swab16(i2c_smbus_read_word_data(client, reg));
> >> +	return i2c_smbus_read_word_swapped(client, reg);
> > 
> > This is still not endian-safe. I wonder if introducing
> > i2c_smbus_read_word_swapped() just makes the metter worse by hiding
> > this...
> > 
> > I'd prefer if we had i2c_smbus_read_be16() instead.
> smbus spec says that all transfers must be little endian.
> Hence it is type safe because the smbus controllers are guaranteed to have made
> that assumption and converted to cpu endianess.  If not they are buggy.
> Hence, this is endian safe.

Ah, right... OK then.

-- 
Dmitry