On Monday 16 May 2011 13:16:46 Éric Piel wrote:It is not normally changing, normally it is set just at init/unsuspend (where the bios can also interfere sometimes) and when the user changes it. So definitely within the same function it's not going to suddenly change. We could avoid calculating/checking it twice in lis3lv02d_selftest(). Care to do a third version with this little clean up?Op 16-05-11 00:46, Christian Lamparter schreef:Do you know how "volatile" this data rate is? If it never changesFrom my POV, it looks like the hardware is not working as expectedHowever, I'd fix it a bit differently: let lis3lv02d_get_odr() return
and returns a bogus data rate. The driver doesn't check the result
and directly uses it as some sort of divisor in some places:
msleep(lis3->pwron_delay / lis3lv02d_get_odr());
Under this circumstances, this could very well cause the
"divide by zero" exception from above.
the raw data, and create a special function
lis3lv02d_get_pwron_delay_ms() which does the "lis3->pwron_delay /
lis3lv02d_get_odr()" with special handling for 0 (returning a large
value and also sending a printk_once() ).
[at least it doesn't here?] then why not read it once in init_device
and store it in the device context?
Sounds even better than using a conservative value!
Anyway, I updated the code... But instead of returning a "large value"
I went for the -ENXIO to bail-out early, so now we won't continue if
something went bad [after resume for instance?].
This is not introduced by your patch, right? So it's fine for now :-)
As you have noted, we might want to check other parts of the driver towell:
validate the data from the device. So far, all the code considers that
the device is flawless :-S
drivers/misc/lis3lv02d/lis3lv02d.c:170:52: warning: cast to restricted __le16