[RFC PATCH] Fix accelerometer resume path

3 messages Options
Embed this post
Permalink
Michael Trimarchi

[RFC PATCH] Fix accelerometer resume path

Reply Threaded More More options
Print post
Permalink


This fix the resume path of the accelerometer. If the accelerometer
is not power up, restore to initial status.

Signed-off-by: Michael Trimarchi <[hidden email]>

diff --git a/drivers/input/misc/lis302dl.c b/drivers/input/misc/lis302dl.c
index f31e548..69f9938 100644
--- a/drivers/input/misc/lis302dl.c
+++ b/drivers/input/misc/lis302dl.c
@@ -906,11 +906,6 @@ static int lis302dl_resume(struct spi_device *spi)
  if (__lis302dl_reset_device(lis))
  dev_err(&spi->dev, "device BOOT reload failed\n");
 
- lis->regs[LIS302DL_REG_CTRL1] |= LIS302DL_CTRL1_PD |
- LIS302DL_CTRL1_Xen |
- LIS302DL_CTRL1_Yen |
- LIS302DL_CTRL1_Zen;
-
  /* restore registers after resume */
  for (n = 0; n < ARRAY_SIZE(regs_to_save); n++)
  __reg_write(lis, regs_to_save[n], lis->regs[regs_to_save[n]]);
Werner Almesberger

Re: [RFC PATCH] Fix accelerometer resume path

Reply Threaded More More options
Print post
Permalink
Michael Trimarchi wrote:
> This fix the resume path of the accelerometer. If the accelerometer
> is not power up, restore to initial status.

That code looks indeed very fishy.

However, it makes me wonder why the copy is being changed in the first
place. It doesn't look like the sort of thing you do without a reason,
good or bad.

Also, the LIS302 manual is very terse on the semantics of the "power
down mode". If there are operations that aren't permitted in power
down mode, it may well be necessary to keep the chip in power up while
restoring the registers. This could be accomplished by putting
LIS302DL_REG_CTRL1 last in regs_to_save.

If all power down does is stop collecting data, then your patch would
be correct as is.

- Werner

Michael Trimarchi

Re: [RFC PATCH] Fix accelerometer resume path

Reply Threaded More More options
Print post
Permalink
Werner Almesberger wrote:

> Michael Trimarchi wrote:
>  
>> This fix the resume path of the accelerometer. If the accelerometer
>> is not power up, restore to initial status.
>>    
>
> That code looks indeed very fishy.
>
> However, it makes me wonder why the copy is being changed in the first
> place. It doesn't look like the sort of thing you do without a reason,
> good or bad.
>
> Also, the LIS302 manual is very terse on the semantics of the "power
> down mode". If there are operations that aren't permitted in power
> down mode, it may well be necessary to keep the chip in power up while
> restoring the registers. This could be accomplished by putting
> LIS302DL_REG_CTRL1 last in regs_to_save.
>
> If all power down does is stop collecting data, then your patch would
> be correct as is.
>  
Ok, we must check on the datasheet this issue. Experience on the device
show me that the patch is correct so it is possible to write on it.

Another solution can be a reverse restore so the CTRL1 is the last one

Michael

> - Werner
>
>