From: Giuseppe Bilotta Date: Tue, 10 Feb 2009 23:51:25 GMT Subject: Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors Message-ID: In-Reply-To: <499211D0.3090705@tremplin-utc.net> On Wed, Feb 11, 2009 at 12:46 AM, Éric Piel wrote: > Giuseppe Bilotta schreef: >> >> 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 >> --- >> >> 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 :-) And don't forget about the v3 for -mm ;-) > However I'm not sure about the conversion between s8 and s16: > >> +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? Well, the previous one works, but I admit it's horribly convoluted. I'll double check with this one and resubmit (I'll only resubmit the one based on Pavel's "don't touch anything on init" probably though, hope it's ok) -- Giuseppe "Oblomov" Bilotta