Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors

Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors

[Pavel Machek]

On Wed 2009-02-11 01:21:32, Giuseppe Bilotta wrote:
> [removed git from the cc because that was a mistake]
> 
> On Wed, Feb 11, 2009 at 12:54 AM, Éric Piel <Eric.Piel@tremplin-utc.net> wrote:
> > BTW, now that this is working. Could you double check the correct axes set
> > up. In particular, is it x_inverted or y_inverted, and what about z axis...
> 
> Now that we have the 8 vs 16 bit mess sorted out, I re-checked and
> there my sensor definitely has the X (first) axis inverted, so the
> one-liner patch I sent at the beginning is still valid, at least as
> far as my laptop is involved. It might be that different dv5 Pavilions
> have different sensors though, which will make it quite harder to sort
> out.
> 
> I'd be interested in hearing from the other user (the one reporting
> inverted Y): rather than checking neverball, could he use plain jstest
> and see if the results match the Documentation or not?

Actuall 'watch cat position' is simplest way to test it.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors

[Éric Piel]

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 <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:

> +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

Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors

[Giuseppe Bilotta]

On Wed, Feb 11, 2009 at 12:46 AM, Éric Piel <Eric.Piel@tremplin-utc.net> 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 <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 :-)

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

Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors

[Éric Piel]

Giuseppe Bilotta schreef:

> 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)
Sure, only modified v3 is ok.
BTW, now that this is working. Could you double check the correct axes 
set up. In particular, is it x_inverted or y_inverted, and what about z 
axis...

Thanks,
Eric

Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors

[Giuseppe Bilotta]

[removed git from the cc because that was a mistake]

On Wed, Feb 11, 2009 at 12:54 AM, Éric Piel <Eric.Piel@tremplin-utc.net> wrote:
> BTW, now that this is working. Could you double check the correct axes set
> up. In particular, is it x_inverted or y_inverted, and what about z axis...

Now that we have the 8 vs 16 bit mess sorted out, I re-checked and
there my sensor definitely has the X (first) axis inverted, so the
one-liner patch I sent at the beginning is still valid, at least as
far as my laptop is involved. It might be that different dv5 Pavilions
have different sensors though, which will make it quite harder to sort
out.

I'd be interested in hearing from the other user (the one reporting
inverted Y): rather than checking neverball, could he use plain jstest
and see if the results match the Documentation or not?


-- 
Giuseppe "Oblomov" Bilotta