Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors
- From
- Éric Piel <eric.piel@tremplin-utc.net>
- Date
- Feb 10, 2009, 23:46 UTC
- Message-ID
- <499211D0.3090705@tremplin-utc.net>
- In-Reply-To
- <1234309249-13672-1-git-send-email-giuseppe.bilotta@gmail.com>
Giuseppe Bilotta schreef:
Show 20 quoted lines
> Sensors responding with 0x3B to WHO_AM_I only have one data register per > direction, thus returning a signed byte from the position which is > occupied by the MSB in sensors responding with 0x3A. > > We support both kind of sensors by checking for the sensor type on init > and defining appropriate data-access routines and sensor limits (for the > joystick) depending on what we find. > > Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com> > --- > > This is the other version of the patch, and it changes access to use the > base address from the 8-bit, so the 16-bit routine is changed to access > the PREVIOUS byte for the LSB. > > Choose whichever of the patches is deemed more correct 8-) > > Note that both patches are based off Linus tree, so I don't know how > they cope with Pavel's "don't touch too much on init" one. I'll try to > work out a patch on top of that too.
Yes, great, and this one looks even better than v1 :-) However I'm not sure about the conversion between s8 and s16:
Show 6 quoted lines
> +static s16 lis3lv02d_read_8(acpi_handle handle, int reg)
> +{
> + u8 lo;
> + adev.read(handle, reg, &lo);
> + return *((s8*)(&lo));
> +}Does it really extend the sign to 16 bits? I would have written it this way: +static s16 lis3lv02d_read_8(acpi_handle handle, int reg) +{ + s8 lo; + adev.read(handle, reg, &lo); + return (s16)lo; +} Doesn't it work better?
Eric