Re: [PATCH 3/5] iio: imu: st_lsm6dsx: add wakeup-source option

[Sean Nyekjaer <sean@geanix.com>]

On 18/06/2019 22.27, Lorenzo Bianconi wrote:
> On Jun 18, Sean Nyekjaer wrote:
>>
>>
>> On 18/06/2019 17.51, Lorenzo Bianconi wrote:
>>>> this add ways for the SoC to wake from accelerometer wake events.
>>>>
>>>> In the suspend function we skip disabling the sensor if wakeup-source
>>>> and events are activated.
>>>>
>>>> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
>>>> ---
>>>>    drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 15 +++++++++++++++
>>>>    1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>>>> index 351c46f01662..59a34894e495 100644
>>>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>>>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>>>> @@ -1076,6 +1076,10 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id, const char *name,
>>>>    			return err;
>>>>    	}
>>>> +	if (dev->of_node)
>>>> +		if (of_property_read_bool(dev->of_node, "wakeup-source"))
>>>> +			device_init_wakeup(dev, true);
>>>> +
>>>>    	return 0;
>>>>    }
>>>>    EXPORT_SYMBOL(st_lsm6dsx_probe);
>>>> @@ -1088,6 +1092,12 @@ static int __maybe_unused st_lsm6dsx_suspend(struct device *dev)
>>>>    	int i, err = 0;
>>>>    	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
>>>> +		if (device_may_wakeup(dev) && (i == ST_LSM6DSX_ID_ACC)) {
>>>> +			/* Enable wake from IRQ */
>>>> +			enable_irq_wake(hw->irq);
>>>> +			continue;
>>>
>>> This is breaking buffering mode
>>
>> Hmm, what is missing? :-)
>> We need the accelerometer to continue to run in suspend, but skip writing to
>> the internal fifo.
> 
> I think we should keep the accel enabled, but we need to put the FIFO in bypas
> mode
> 

I think it's exactly whats happening here, we keep the accel enabled and 
set the FIFO in bypass mode after.
st_lsm6dsx_flush_fifo (which sets the bypass mode) is called after the 
for loop.

/Sean