* [PATCH] BMP085 : Change the macro to swap
@ 2010-06-24 12:41 Datta, Shubhrajyoti
2010-06-24 12:52 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Datta, Shubhrajyoti @ 2010-06-24 12:41 UTC (permalink / raw)
To: linux-kernel; +Cc: Jonathan Cameron, Christoph Mair, Andrew Morton
Changing the macro to swap the bytes as the reason that the first byte is the MSB and the next is LSB.
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/misc/bmp085.c | 41 +++++++++++++++++++++++++++++------------
1 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/drivers/misc/bmp085.c b/drivers/misc/bmp085.c
index 8af6f1b..74c5b12 100644
--- a/drivers/misc/bmp085.c
+++ b/drivers/misc/bmp085.c
@@ -79,18 +79,35 @@ static s32 bmp085_read_calibration_data(struct i2c_client *client)
if (status != BMP085_CALIBRATION_DATA_LENGTH*sizeof(u16))
return -EIO;
-
- cali->AC1 = be16_to_cpu(tmp[0]);
- cali->AC2 = be16_to_cpu(tmp[1]);
- cali->AC3 = be16_to_cpu(tmp[2]);
- cali->AC4 = be16_to_cpu(tmp[3]);
- cali->AC5 = be16_to_cpu(tmp[4]);
- cali->AC6 = be16_to_cpu(tmp[5]);
- cali->B1 = be16_to_cpu(tmp[6]);
- cali->B2 = be16_to_cpu(tmp[7]);
- cali->MB = be16_to_cpu(tmp[8]);
- cali->MC = be16_to_cpu(tmp[9]);
- cali->MD = be16_to_cpu(tmp[10]);
+/*
+ * From the datasheet
+ *
+ * BMP085 Reg Addr
+ * parameter | MSB | LSB
+ * AC1 | 0xAA | 0xAB
+ * AC2 | 0xAC | 0xAD
+ * AC3 | 0xAE | 0xAF
+ * AC4 | 0xB0 | 0xB1
+ * AC5 | 0xB2 | 0xB3
+ * AC6 | 0xB4 | 0xB5
+ * B1 | 0xB6 | 0xB7
+ * B2 | 0xB8 | 0xB9
+ * MB | 0xBA | 0xBB
+ * MC | 0xBC | 0xBD
+ * MC | 0xBE | 0xBF
+ *
+ */
+ cali->AC1 = swab16(tmp[0]);
+ cali->AC2 = swab16(tmp[1]);
+ cali->AC3 = swab16(tmp[2]);
+ cali->AC4 = swab16(tmp[3]);
+ cali->AC5 = swab16(tmp[4]);
+ cali->AC6 = swab16(tmp[5]);
+ cali->B1 = swab16(tmp[6]);
+ cali->B2 = swab16(tmp[7]);
+ cali->MB = swab16(tmp[8]);
+ cali->MC = swab16(tmp[9]);
+ cali->MD = swab16(tmp[10]);
return 0;
}
--
1.5.4.7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] BMP085 : Change the macro to swap
2010-06-24 12:41 [PATCH] BMP085 : Change the macro to swap Datta, Shubhrajyoti
@ 2010-06-24 12:52 ` Jonathan Cameron
2010-06-25 13:00 ` Datta, Shubhrajyoti
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2010-06-24 12:52 UTC (permalink / raw)
To: Datta, Shubhrajyoti; +Cc: linux-kernel, Christoph Mair, Andrew Morton
On 06/24/10 13:41, Datta, Shubhrajyoti wrote:
>
> Changing the macro to swap the bytes as the reason that the first byte is the MSB and the next is LSB.
Are you sure on this one?
The swap would be right if you were using an i2c function that is aware that it is
reading word data (16bit).
Here it just thinks it is reading a string of bytes. Hence the bytes will indeed
come out big endian. So if you have a big endian machine this patch will swap the
bytes round incorrectly.
Of course I might just be half asleep today ;)
>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> drivers/misc/bmp085.c | 41 +++++++++++++++++++++++++++++------------
> 1 files changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/misc/bmp085.c b/drivers/misc/bmp085.c
> index 8af6f1b..74c5b12 100644
> --- a/drivers/misc/bmp085.c
> +++ b/drivers/misc/bmp085.c
> @@ -79,18 +79,35 @@ static s32 bmp085_read_calibration_data(struct i2c_client *client)
>
> if (status != BMP085_CALIBRATION_DATA_LENGTH*sizeof(u16))
> return -EIO;
> -
> - cali->AC1 = be16_to_cpu(tmp[0]);
> - cali->AC2 = be16_to_cpu(tmp[1]);
> - cali->AC3 = be16_to_cpu(tmp[2]);
> - cali->AC4 = be16_to_cpu(tmp[3]);
> - cali->AC5 = be16_to_cpu(tmp[4]);
> - cali->AC6 = be16_to_cpu(tmp[5]);
> - cali->B1 = be16_to_cpu(tmp[6]);
> - cali->B2 = be16_to_cpu(tmp[7]);
> - cali->MB = be16_to_cpu(tmp[8]);
> - cali->MC = be16_to_cpu(tmp[9]);
> - cali->MD = be16_to_cpu(tmp[10]);
> +/*
> + * From the datasheet
> + *
> + * BMP085 Reg Addr
> + * parameter | MSB | LSB
> + * AC1 | 0xAA | 0xAB
> + * AC2 | 0xAC | 0xAD
> + * AC3 | 0xAE | 0xAF
> + * AC4 | 0xB0 | 0xB1
> + * AC5 | 0xB2 | 0xB3
> + * AC6 | 0xB4 | 0xB5
> + * B1 | 0xB6 | 0xB7
> + * B2 | 0xB8 | 0xB9
> + * MB | 0xBA | 0xBB
> + * MC | 0xBC | 0xBD
> + * MC | 0xBE | 0xBF
> + *
> + */
> + cali->AC1 = swab16(tmp[0]);
> + cali->AC2 = swab16(tmp[1]);
> + cali->AC3 = swab16(tmp[2]);
> + cali->AC4 = swab16(tmp[3]);
> + cali->AC5 = swab16(tmp[4]);
> + cali->AC6 = swab16(tmp[5]);
> + cali->B1 = swab16(tmp[6]);
> + cali->B2 = swab16(tmp[7]);
> + cali->MB = swab16(tmp[8]);
> + cali->MC = swab16(tmp[9]);
> + cali->MD = swab16(tmp[10]);
> return 0;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] BMP085 : Change the macro to swap
2010-06-24 12:52 ` Jonathan Cameron
@ 2010-06-25 13:00 ` Datta, Shubhrajyoti
2010-06-25 13:34 ` Christoph Mair
0 siblings, 1 reply; 4+ messages in thread
From: Datta, Shubhrajyoti @ 2010-06-25 13:00 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-kernel, Christoph Mair, Andrew Morton
> -----Original Message-----
> From: J.I. Cameron [mailto:jic23@hermes.cam.ac.uk] On Behalf Of Jonathan
> Cameron
> Sent: Thursday, June 24, 2010 6:23 PM
> To: Datta, Shubhrajyoti
> Cc: linux-kernel@vger.kernel.org; Christoph Mair; Andrew Morton
> Subject: Re: [PATCH] BMP085 : Change the macro to swap
>
> On 06/24/10 13:41, Datta, Shubhrajyoti wrote:
> >
> > Changing the macro to swap the bytes as the reason that the first byte
> is the MSB and the next is LSB.
> Are you sure on this one?
The datasheet says the E2PROM has the bytes as described in the comment below
*
* BMP085 Reg Addr
* parameter | MSB | LSB
* AC1 | 0xAA | 0xAB
...
AA has MSB and AB has lsb so it has to be swapped. My idea is that it does not depend on the endianness of the CPU running the code. So I think that swap should happen unconditionally. The I2C reads a string of bytes and the first byte from AA and next from AB and hence an unconditional swap maybe needed.
Am I missing something.
>
> The swap would be right if you were using an i2c function that is aware
> that it is
> reading word data (16bit).
>
> Here it just thinks it is reading a string of bytes. Hence the bytes will
> indeed
> come out big endian. So if you have a big endian machine this patch will
> swap the
> bytes round incorrectly.
>
> Of course I might just be half asleep today ;)
> >
> > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> > ---
> > drivers/misc/bmp085.c | 41 +++++++++++++++++++++++++++++------------
> > 1 files changed, 29 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/misc/bmp085.c b/drivers/misc/bmp085.c
> > index 8af6f1b..74c5b12 100644
> > --- a/drivers/misc/bmp085.c
> > +++ b/drivers/misc/bmp085.c
> > @@ -79,18 +79,35 @@ static s32 bmp085_read_calibration_data(struct
> i2c_client *client)
> >
> > if (status != BMP085_CALIBRATION_DATA_LENGTH*sizeof(u16))
> > return -EIO;
> > -
> > - cali->AC1 = be16_to_cpu(tmp[0]);
> > - cali->AC2 = be16_to_cpu(tmp[1]);
> > - cali->AC3 = be16_to_cpu(tmp[2]);
> > - cali->AC4 = be16_to_cpu(tmp[3]);
> > - cali->AC5 = be16_to_cpu(tmp[4]);
> > - cali->AC6 = be16_to_cpu(tmp[5]);
> > - cali->B1 = be16_to_cpu(tmp[6]);
> > - cali->B2 = be16_to_cpu(tmp[7]);
> > - cali->MB = be16_to_cpu(tmp[8]);
> > - cali->MC = be16_to_cpu(tmp[9]);
> > - cali->MD = be16_to_cpu(tmp[10]);
> > +/*
> > + * From the datasheet
> > + *
> > + * BMP085 Reg Addr
> > + * parameter | MSB | LSB
> > + * AC1 | 0xAA | 0xAB
> > + * AC2 | 0xAC | 0xAD
> > + * AC3 | 0xAE | 0xAF
> > + * AC4 | 0xB0 | 0xB1
> > + * AC5 | 0xB2 | 0xB3
> > + * AC6 | 0xB4 | 0xB5
> > + * B1 | 0xB6 | 0xB7
> > + * B2 | 0xB8 | 0xB9
> > + * MB | 0xBA | 0xBB
> > + * MC | 0xBC | 0xBD
> > + * MC | 0xBE | 0xBF
> > + *
> > + */
> > + cali->AC1 = swab16(tmp[0]);
> > + cali->AC2 = swab16(tmp[1]);
> > + cali->AC3 = swab16(tmp[2]);
> > + cali->AC4 = swab16(tmp[3]);
> > + cali->AC5 = swab16(tmp[4]);
> > + cali->AC6 = swab16(tmp[5]);
> > + cali->B1 = swab16(tmp[6]);
> > + cali->B2 = swab16(tmp[7]);
> > + cali->MB = swab16(tmp[8]);
> > + cali->MC = swab16(tmp[9]);
> > + cali->MD = swab16(tmp[10]);
> > return 0;
> > }
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] BMP085 : Change the macro to swap
2010-06-25 13:00 ` Datta, Shubhrajyoti
@ 2010-06-25 13:34 ` Christoph Mair
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Mair @ 2010-06-25 13:34 UTC (permalink / raw)
To: Datta, Shubhrajyoti; +Cc: Jonathan Cameron, linux-kernel, Andrew Morton
On Fri, Jun 25, 2010 at 3:00 PM, Datta, Shubhrajyoti
<shubhrajyoti@ti.com> wrote:
>
>
>> -----Original Message-----
>> From: J.I. Cameron [mailto:jic23@hermes.cam.ac.uk] On Behalf Of Jonathan
>> Cameron
>> Sent: Thursday, June 24, 2010 6:23 PM
>> To: Datta, Shubhrajyoti
>> Cc: linux-kernel@vger.kernel.org; Christoph Mair; Andrew Morton
>> Subject: Re: [PATCH] BMP085 : Change the macro to swap
>>
>> On 06/24/10 13:41, Datta, Shubhrajyoti wrote:
>> >
>> > Changing the macro to swap the bytes as the reason that the first byte
>> is the MSB and the next is LSB.
>> Are you sure on this one?
>
> The datasheet says the E2PROM has the bytes as described in the comment below
> *
> * BMP085 Reg Addr
> * parameter | MSB | LSB
> * AC1 | 0xAA | 0xAB
> ...
>
> AA has MSB and AB has lsb so it has to be swapped. My idea is that it does not depend on the endianness of the CPU running the code. So I think that swap should happen unconditionally. The I2C reads a string of bytes and the first byte from AA and next from AB and hence an unconditional swap maybe needed.
>
> Am I missing something.
The data is read bytewise and stored in memory starting with
MSB-LSM-MSB-LSB-... If you have a big endian CPU, it expects a 16 Bit
word to be stored in exactly this format. A little endian CPU expects
the LSB to be stored at the lower memory address: LSB-MSB.
Therefore the bytes have to be swapped only for little-endian cpus.
Best regards,
Christoph
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-06-25 13:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-24 12:41 [PATCH] BMP085 : Change the macro to swap Datta, Shubhrajyoti
2010-06-24 12:52 ` Jonathan Cameron
2010-06-25 13:00 ` Datta, Shubhrajyoti
2010-06-25 13:34 ` Christoph Mair
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).