diff mbox series

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

Message ID 20190618125939.105903-4-sean@geanix.com (mailing list archive)
State New, archived
Headers show
Series iio: imu: st_lsm6dsx: add event reporting and wakeup | expand

Commit Message

Sean Nyekjaer June 18, 2019, 12:59 p.m. UTC
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(+)

Comments

lorenzo@kernel.org June 18, 2019, 3:51 p.m. UTC | #1
> 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

> +		}
> +
>  		sensor = iio_priv(hw->iio_devs[i]);
>  		if (!(hw->enable_mask & BIT(sensor->id)))
>  			continue;
> @@ -1112,6 +1122,11 @@ static int __maybe_unused st_lsm6dsx_resume(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)) {
> +			disable_irq_wake(hw->irq);

same here

> +			continue;
> +		}
> +
>  		sensor = iio_priv(hw->iio_devs[i]);
>  		if (!(hw->enable_mask & BIT(sensor->id)))
>  			continue;
> -- 
> 2.22.0
>
Sean Nyekjaer June 18, 2019, 7:19 p.m. UTC | #2
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.

> 
>> +		}
>> +
>>   		sensor = iio_priv(hw->iio_devs[i]);
>>   		if (!(hw->enable_mask & BIT(sensor->id)))
>>   			continue;
>> @@ -1112,6 +1122,11 @@ static int __maybe_unused st_lsm6dsx_resume(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)) {
>> +			disable_irq_wake(hw->irq);
> 
> same here
> 
>> +			continue;
>> +		}
>> +
>>   		sensor = iio_priv(hw->iio_devs[i]);
>>   		if (!(hw->enable_mask & BIT(sensor->id)))
>>   			continue;
>> -- 
>> 2.22.0
>>
lorenzo@kernel.org June 18, 2019, 8:27 p.m. UTC | #3
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

> 
> > 
> > > +		}
> > > +
> > >   		sensor = iio_priv(hw->iio_devs[i]);
> > >   		if (!(hw->enable_mask & BIT(sensor->id)))
> > >   			continue;
> > > @@ -1112,6 +1122,11 @@ static int __maybe_unused st_lsm6dsx_resume(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)) {
> > > +			disable_irq_wake(hw->irq);
> > 
> > same here
> > 
> > > +			continue;
> > > +		}
> > > +
> > >   		sensor = iio_priv(hw->iio_devs[i]);
> > >   		if (!(hw->enable_mask & BIT(sensor->id)))
> > >   			continue;
> > > -- 
> > > 2.22.0
> > >
Sean Nyekjaer July 1, 2019, 11:06 a.m. UTC | #4
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
diff mbox series

Patch

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;
+		}
+
 		sensor = iio_priv(hw->iio_devs[i]);
 		if (!(hw->enable_mask & BIT(sensor->id)))
 			continue;
@@ -1112,6 +1122,11 @@  static int __maybe_unused st_lsm6dsx_resume(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)) {
+			disable_irq_wake(hw->irq);
+			continue;
+		}
+
 		sensor = iio_priv(hw->iio_devs[i]);
 		if (!(hw->enable_mask & BIT(sensor->id)))
 			continue;