RE: [PATCH] BMP085 : Change the macro to swap

[Datta, Shubhrajyoti]

> -----Original Message-----
> From: J.I. Cameron [mailto:jic23@xxxxxxxxxxxxxxxx] On Behalf Of Jonathan
> Cameron
> Sent: Thursday, June 24, 2010 6:23 PM
> To: Datta, Shubhrajyoti
> Cc: linux-kernel@xxxxxxxxxxxxxxx; 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@xxxxxx>
> > ---
> >  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;
> >  }
> >

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/