* [PATCH v2 0/2] iio: fix bug with triggers not resuming after sleep
@ 2024-07-27 12:30 Denis Benato
2024-07-27 12:30 ` [PATCH v2 1/2] iio: trigger: allow devices to suspend/resume theirs associated trigger Denis Benato
2024-07-27 12:30 ` [PATCH v2 2/2] iio: bmi323: suspend and resume triggering on relevant pm operations Denis Benato
0 siblings, 2 replies; 6+ messages in thread
From: Denis Benato @ 2024-07-27 12:30 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Jagath Jog J, linux-iio, linux-kernel,
Denis Benato, Luke D . Jones, Jonathan LoBue
When a device enters an idle state (for example ASUS RC71L with s2idle)
and an iio driver has a trigger attached such as iio-trig-hrtimer,
after resuming the device the trigger is not triggering data acquisition.
This patch series solves the problem reliably and is well tested after
many cycles and many reboots.
Closes: https://lore.kernel.org/all/31d7f7aa-e834-4fd0-a66a-e0ff528425dc@gmail.com/
Changelog:
- V2: patch 2:
+ Simpliy code
+ Remove unneeded code protections around SET_RUNTIME_PM_OPS()
+ pm_ptr() to let the compiler drop bmi323_core_pm_ops if !CONFIG_PM
Previous patches obsoleted:
https://lore.kernel.org/all/20240725002641.191509-3-benato.denis96@gmail.com/
Denis Benato (2):
iio: trigger: allow devices to suspend/resume theirs associated
trigger
iio: bmi323: suspend and resume triggering on relevant pm operations
drivers/iio/imu/bmi323/bmi323.h | 1 +
drivers/iio/imu/bmi323/bmi323_core.c | 23 +++++++++++++++++++++++
drivers/iio/imu/bmi323/bmi323_i2c.c | 1 +
drivers/iio/imu/bmi323/bmi323_spi.c | 1 +
drivers/iio/industrialio-trigger.c | 26 ++++++++++++++++++++++++++
include/linux/iio/iio.h | 17 +++++++++++++++++
6 files changed, 69 insertions(+)
--
2.45.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] iio: trigger: allow devices to suspend/resume theirs associated trigger
2024-07-27 12:30 [PATCH v2 0/2] iio: fix bug with triggers not resuming after sleep Denis Benato
@ 2024-07-27 12:30 ` Denis Benato
2024-07-27 12:30 ` [PATCH v2 2/2] iio: bmi323: suspend and resume triggering on relevant pm operations Denis Benato
1 sibling, 0 replies; 6+ messages in thread
From: Denis Benato @ 2024-07-27 12:30 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Jagath Jog J, linux-iio, linux-kernel,
Denis Benato, Luke D . Jones, Jonathan LoBue
When a machine enters a sleep state while a trigger is associated to
an iio device that trigger is not resumed after exiting the sleep state:
provide iio device drivers a way to suspend and resume
the associated trigger to solve the aforementioned bug.
Each iio driver supporting external triggers is expected to call
iio_device_suspend_triggering before suspending,
and iio_device_resume_triggering upon resuming.
Signed-off-by: Denis Benato <benato.denis96@gmail.com>
---
drivers/iio/industrialio-trigger.c | 26 ++++++++++++++++++++++++++
include/linux/iio/iio.h | 17 +++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 2e84776f4fbd..2e92283fad0f 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -770,3 +770,29 @@ void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
if (indio_dev->trig)
iio_trigger_put(indio_dev->trig);
}
+
+int iio_device_suspend_triggering(struct iio_dev *indio_dev)
+{
+ struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
+
+ guard(mutex)(&iio_dev_opaque->mlock);
+
+ if ((indio_dev->pollfunc) && (indio_dev->pollfunc->irq > 0))
+ disable_irq(indio_dev->pollfunc->irq);
+
+ return 0;
+}
+EXPORT_SYMBOL(iio_device_suspend_triggering);
+
+int iio_device_resume_triggering(struct iio_dev *indio_dev)
+{
+ struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
+
+ guard(mutex)(&iio_dev_opaque->mlock);
+
+ if ((indio_dev->pollfunc) && (indio_dev->pollfunc->irq > 0))
+ enable_irq(indio_dev->pollfunc->irq);
+
+ return 0;
+}
+EXPORT_SYMBOL(iio_device_resume_triggering);
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 894309294182..c87dfda54681 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -810,6 +810,23 @@ static inline struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev)
}
#endif
+/**
+ * iio_device_suspend_triggering() - suspend trigger attached to an iio_dev
+ * @indio_dev: iio_dev associated with the device that will have triggers suspended
+ *
+ * Return 0 if successful, negative otherwise
+ **/
+int iio_device_suspend_triggering(struct iio_dev *indio_dev);
+
+/**
+ * iio_device_resume_triggering() - resume trigger attached to an iio_dev
+ * that was previously suspended with iio_device_suspend_triggering()
+ * @indio_dev: iio_dev associated with the device that will have triggers resumed
+ *
+ * Return 0 if successful, negative otherwise
+ **/
+int iio_device_resume_triggering(struct iio_dev *indio_dev);
+
#ifdef CONFIG_ACPI
bool iio_read_acpi_mount_matrix(struct device *dev,
struct iio_mount_matrix *orientation,
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] iio: bmi323: suspend and resume triggering on relevant pm operations
2024-07-27 12:30 [PATCH v2 0/2] iio: fix bug with triggers not resuming after sleep Denis Benato
2024-07-27 12:30 ` [PATCH v2 1/2] iio: trigger: allow devices to suspend/resume theirs associated trigger Denis Benato
@ 2024-07-27 12:30 ` Denis Benato
2024-08-03 15:44 ` Jonathan Cameron
1 sibling, 1 reply; 6+ messages in thread
From: Denis Benato @ 2024-07-27 12:30 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Jagath Jog J, linux-iio, linux-kernel,
Denis Benato, Luke D . Jones, Jonathan LoBue
Prevent triggers from stop working after the device has entered sleep:
use iio_device_suspend_triggering and iio_device_resume_triggering helpers.
Signed-off-by: Denis Benato <benato.denis96@gmail.com>
---
drivers/iio/imu/bmi323/bmi323.h | 1 +
drivers/iio/imu/bmi323/bmi323_core.c | 23 +++++++++++++++++++++++
drivers/iio/imu/bmi323/bmi323_i2c.c | 1 +
drivers/iio/imu/bmi323/bmi323_spi.c | 1 +
4 files changed, 26 insertions(+)
diff --git a/drivers/iio/imu/bmi323/bmi323.h b/drivers/iio/imu/bmi323/bmi323.h
index dff126d41658..209bccb1f335 100644
--- a/drivers/iio/imu/bmi323/bmi323.h
+++ b/drivers/iio/imu/bmi323/bmi323.h
@@ -205,5 +205,6 @@
struct device;
int bmi323_core_probe(struct device *dev);
extern const struct regmap_config bmi323_regmap_config;
+extern const struct dev_pm_ops bmi323_core_pm_ops;
#endif
diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
index d708d1fe3e42..4b2b211a3e88 100644
--- a/drivers/iio/imu/bmi323/bmi323_core.c
+++ b/drivers/iio/imu/bmi323/bmi323_core.c
@@ -2121,6 +2121,29 @@ int bmi323_core_probe(struct device *dev)
}
EXPORT_SYMBOL_NS_GPL(bmi323_core_probe, IIO_BMI323);
+#if defined(CONFIG_PM)
+static int bmi323_core_runtime_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+
+ return iio_device_suspend_triggering(indio_dev);
+}
+
+static int bmi323_core_runtime_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+
+ return iio_device_resume_triggering(indio_dev);
+}
+
+#endif
+
+const struct dev_pm_ops bmi323_core_pm_ops = {
+ SET_RUNTIME_PM_OPS(bmi323_core_runtime_suspend,
+ bmi323_core_runtime_resume, NULL)
+};
+EXPORT_SYMBOL_NS_GPL(bmi323_core_pm_ops, IIO_BMI323);
+
MODULE_DESCRIPTION("Bosch BMI323 IMU driver");
MODULE_AUTHOR("Jagath Jog J <jagathjog1996@gmail.com>");
MODULE_LICENSE("GPL");
diff --git a/drivers/iio/imu/bmi323/bmi323_i2c.c b/drivers/iio/imu/bmi323/bmi323_i2c.c
index 52140bf05765..057342f4f816 100644
--- a/drivers/iio/imu/bmi323/bmi323_i2c.c
+++ b/drivers/iio/imu/bmi323/bmi323_i2c.c
@@ -128,6 +128,7 @@ MODULE_DEVICE_TABLE(of, bmi323_of_i2c_match);
static struct i2c_driver bmi323_i2c_driver = {
.driver = {
.name = "bmi323",
+ .pm = pm_ptr(&bmi323_core_pm_ops),
.of_match_table = bmi323_of_i2c_match,
.acpi_match_table = bmi323_acpi_match,
},
diff --git a/drivers/iio/imu/bmi323/bmi323_spi.c b/drivers/iio/imu/bmi323/bmi323_spi.c
index 7b1e8127d0dd..487d4ee05246 100644
--- a/drivers/iio/imu/bmi323/bmi323_spi.c
+++ b/drivers/iio/imu/bmi323/bmi323_spi.c
@@ -79,6 +79,7 @@ MODULE_DEVICE_TABLE(of, bmi323_of_spi_match);
static struct spi_driver bmi323_spi_driver = {
.driver = {
.name = "bmi323",
+ .pm = pm_ptr(&bmi323_core_pm_ops),
.of_match_table = bmi323_of_spi_match,
},
.probe = bmi323_spi_probe,
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] iio: bmi323: suspend and resume triggering on relevant pm operations
2024-07-27 12:30 ` [PATCH v2 2/2] iio: bmi323: suspend and resume triggering on relevant pm operations Denis Benato
@ 2024-08-03 15:44 ` Jonathan Cameron
2024-08-04 15:40 ` Denis Benato
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2024-08-03 15:44 UTC (permalink / raw)
To: Denis Benato
Cc: Lars-Peter Clausen, Jagath Jog J, linux-iio, linux-kernel,
Luke D . Jones, Jonathan LoBue
On Sat, 27 Jul 2024 14:30:34 +0200
Denis Benato <benato.denis96@gmail.com> wrote:
> Prevent triggers from stop working after the device has entered sleep:
> use iio_device_suspend_triggering and iio_device_resume_triggering helpers.
Hi Denis,
I'd got it into my head this was about main suspend / resume, but
it's runtime PM. I assume the s2idle uses only that level which is
interesting.
Anyhow, solution seems safe. We might be able to do something nicer
in the long run as potentially we could have the trigger driver
notified when all consumers have entered this state at which point it
could stop generating triggers at all.
Anyhow, that's a job for when we actually care about it.
Applied to the togreg branch of iio.git and pushed out as testing
for 0-day to poke at it.
For now I'm not keen to see this pushed into drivers where we don't
know if anyone is running into this particular situation. We can
reevaluate that if we start getting lots of reports of this.
I'm also not going to rush this in as a fix. We can consider backporting
it once it's been in mainline for a bit and no side effects have
shown up.
Thanks,
Jonathan
>
> Signed-off-by: Denis Benato <benato.denis96@gmail.com>
> ---
> drivers/iio/imu/bmi323/bmi323.h | 1 +
> drivers/iio/imu/bmi323/bmi323_core.c | 23 +++++++++++++++++++++++
> drivers/iio/imu/bmi323/bmi323_i2c.c | 1 +
> drivers/iio/imu/bmi323/bmi323_spi.c | 1 +
> 4 files changed, 26 insertions(+)
>
> diff --git a/drivers/iio/imu/bmi323/bmi323.h b/drivers/iio/imu/bmi323/bmi323.h
> index dff126d41658..209bccb1f335 100644
> --- a/drivers/iio/imu/bmi323/bmi323.h
> +++ b/drivers/iio/imu/bmi323/bmi323.h
> @@ -205,5 +205,6 @@
> struct device;
> int bmi323_core_probe(struct device *dev);
> extern const struct regmap_config bmi323_regmap_config;
> +extern const struct dev_pm_ops bmi323_core_pm_ops;
>
> #endif
> diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
> index d708d1fe3e42..4b2b211a3e88 100644
> --- a/drivers/iio/imu/bmi323/bmi323_core.c
> +++ b/drivers/iio/imu/bmi323/bmi323_core.c
> @@ -2121,6 +2121,29 @@ int bmi323_core_probe(struct device *dev)
> }
> EXPORT_SYMBOL_NS_GPL(bmi323_core_probe, IIO_BMI323);
>
> +#if defined(CONFIG_PM)
> +static int bmi323_core_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +
> + return iio_device_suspend_triggering(indio_dev);
> +}
> +
> +static int bmi323_core_runtime_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +
> + return iio_device_resume_triggering(indio_dev);
> +}
> +
> +#endif
> +
> +const struct dev_pm_ops bmi323_core_pm_ops = {
> + SET_RUNTIME_PM_OPS(bmi323_core_runtime_suspend,
> + bmi323_core_runtime_resume, NULL)
> +};
> +EXPORT_SYMBOL_NS_GPL(bmi323_core_pm_ops, IIO_BMI323);
> +
> MODULE_DESCRIPTION("Bosch BMI323 IMU driver");
> MODULE_AUTHOR("Jagath Jog J <jagathjog1996@gmail.com>");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/iio/imu/bmi323/bmi323_i2c.c b/drivers/iio/imu/bmi323/bmi323_i2c.c
> index 52140bf05765..057342f4f816 100644
> --- a/drivers/iio/imu/bmi323/bmi323_i2c.c
> +++ b/drivers/iio/imu/bmi323/bmi323_i2c.c
> @@ -128,6 +128,7 @@ MODULE_DEVICE_TABLE(of, bmi323_of_i2c_match);
> static struct i2c_driver bmi323_i2c_driver = {
> .driver = {
> .name = "bmi323",
> + .pm = pm_ptr(&bmi323_core_pm_ops),
> .of_match_table = bmi323_of_i2c_match,
> .acpi_match_table = bmi323_acpi_match,
> },
> diff --git a/drivers/iio/imu/bmi323/bmi323_spi.c b/drivers/iio/imu/bmi323/bmi323_spi.c
> index 7b1e8127d0dd..487d4ee05246 100644
> --- a/drivers/iio/imu/bmi323/bmi323_spi.c
> +++ b/drivers/iio/imu/bmi323/bmi323_spi.c
> @@ -79,6 +79,7 @@ MODULE_DEVICE_TABLE(of, bmi323_of_spi_match);
> static struct spi_driver bmi323_spi_driver = {
> .driver = {
> .name = "bmi323",
> + .pm = pm_ptr(&bmi323_core_pm_ops),
> .of_match_table = bmi323_of_spi_match,
> },
> .probe = bmi323_spi_probe,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] iio: bmi323: suspend and resume triggering on relevant pm operations
2024-08-03 15:44 ` Jonathan Cameron
@ 2024-08-04 15:40 ` Denis Benato
2024-08-07 12:57 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Denis Benato @ 2024-08-04 15:40 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Jagath Jog J, linux-iio, linux-kernel,
Luke D . Jones, Jonathan LoBue
On 03/08/24 17:44, Jonathan Cameron wrote:
> On Sat, 27 Jul 2024 14:30:34 +0200
> Denis Benato <benato.denis96@gmail.com> wrote:
>
>> Prevent triggers from stop working after the device has entered sleep:
>> use iio_device_suspend_triggering and iio_device_resume_triggering helpers.
>
> Hi Denis,
>
Hello Jonathan,
> I'd got it into my head this was about main suspend / resume, but
> it's runtime PM. I assume the s2idle uses only that level which is
> interesting.
>
I have catched the problem with s2idle, but I don-t fully understand
it will manifest outside of said scenario, nor if it will at all and
only s2idle is affected.
> Anyhow, solution seems safe. We might be able to do something nicer
> in the long run as potentially we could have the trigger driver
> notified when all consumers have entered this state at which point it
> could stop generating triggers at all.
> Totally agree.
> Anyhow, that's a job for when we actually care about it.
>
> Applied to the togreg branch of iio.git and pushed out as testing
> for 0-day to poke at it.
>
I have made a mistake while cleaning up patch 1/2 for submission and lost a piece:
the pollfunc->irq=0 you suggested in your first mail.
I would be more than happy to provide a v3, but if you prefer I can also send
a separate patch.
I am sorry about that and I would like guidance on what to do in cases like this.
> For now I'm not keen to see this pushed into drivers where we don't
> know if anyone is running into this particular situation. We can
> reevaluate that if we start getting lots of reports of this.
>
I catched the issue while developing an application for a handheld PC.
As the application will target these kind of devices we can apply the fix
to every relevant driver (bmi260 comes to mind) and have that well-tested
on multiple drivers.
> I'm also not going to rush this in as a fix. We can consider backporting
> it once it's been in mainline for a bit and no side effects have
> shown up.
>
> Thanks,
>
> Jonathan
>
Thanks,
Denis
>>
>> Signed-off-by: Denis Benato <benato.denis96@gmail.com>
>> ---
>> drivers/iio/imu/bmi323/bmi323.h | 1 +
>> drivers/iio/imu/bmi323/bmi323_core.c | 23 +++++++++++++++++++++++
>> drivers/iio/imu/bmi323/bmi323_i2c.c | 1 +
>> drivers/iio/imu/bmi323/bmi323_spi.c | 1 +
>> 4 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/iio/imu/bmi323/bmi323.h b/drivers/iio/imu/bmi323/bmi323.h
>> index dff126d41658..209bccb1f335 100644
>> --- a/drivers/iio/imu/bmi323/bmi323.h
>> +++ b/drivers/iio/imu/bmi323/bmi323.h
>> @@ -205,5 +205,6 @@
>> struct device;
>> int bmi323_core_probe(struct device *dev);
>> extern const struct regmap_config bmi323_regmap_config;
>> +extern const struct dev_pm_ops bmi323_core_pm_ops;
>>
>> #endif
>> diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
>> index d708d1fe3e42..4b2b211a3e88 100644
>> --- a/drivers/iio/imu/bmi323/bmi323_core.c
>> +++ b/drivers/iio/imu/bmi323/bmi323_core.c
>> @@ -2121,6 +2121,29 @@ int bmi323_core_probe(struct device *dev)
>> }
>> EXPORT_SYMBOL_NS_GPL(bmi323_core_probe, IIO_BMI323);
>>
>> +#if defined(CONFIG_PM)
>> +static int bmi323_core_runtime_suspend(struct device *dev)
>> +{
>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +
>> + return iio_device_suspend_triggering(indio_dev);
>> +}
>> +
>> +static int bmi323_core_runtime_resume(struct device *dev)
>> +{
>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +
>> + return iio_device_resume_triggering(indio_dev);
>> +}
>> +
>> +#endif
>> +
>> +const struct dev_pm_ops bmi323_core_pm_ops = {
>> + SET_RUNTIME_PM_OPS(bmi323_core_runtime_suspend,
>> + bmi323_core_runtime_resume, NULL)
>> +};
>> +EXPORT_SYMBOL_NS_GPL(bmi323_core_pm_ops, IIO_BMI323);
>> +
>> MODULE_DESCRIPTION("Bosch BMI323 IMU driver");
>> MODULE_AUTHOR("Jagath Jog J <jagathjog1996@gmail.com>");
>> MODULE_LICENSE("GPL");
>> diff --git a/drivers/iio/imu/bmi323/bmi323_i2c.c b/drivers/iio/imu/bmi323/bmi323_i2c.c
>> index 52140bf05765..057342f4f816 100644
>> --- a/drivers/iio/imu/bmi323/bmi323_i2c.c
>> +++ b/drivers/iio/imu/bmi323/bmi323_i2c.c
>> @@ -128,6 +128,7 @@ MODULE_DEVICE_TABLE(of, bmi323_of_i2c_match);
>> static struct i2c_driver bmi323_i2c_driver = {
>> .driver = {
>> .name = "bmi323",
>> + .pm = pm_ptr(&bmi323_core_pm_ops),
>> .of_match_table = bmi323_of_i2c_match,
>> .acpi_match_table = bmi323_acpi_match,
>> },
>> diff --git a/drivers/iio/imu/bmi323/bmi323_spi.c b/drivers/iio/imu/bmi323/bmi323_spi.c
>> index 7b1e8127d0dd..487d4ee05246 100644
>> --- a/drivers/iio/imu/bmi323/bmi323_spi.c
>> +++ b/drivers/iio/imu/bmi323/bmi323_spi.c
>> @@ -79,6 +79,7 @@ MODULE_DEVICE_TABLE(of, bmi323_of_spi_match);
>> static struct spi_driver bmi323_spi_driver = {
>> .driver = {
>> .name = "bmi323",
>> + .pm = pm_ptr(&bmi323_core_pm_ops),
>> .of_match_table = bmi323_of_spi_match,
>> },
>> .probe = bmi323_spi_probe,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] iio: bmi323: suspend and resume triggering on relevant pm operations
2024-08-04 15:40 ` Denis Benato
@ 2024-08-07 12:57 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2024-08-07 12:57 UTC (permalink / raw)
To: Denis Benato
Cc: Jonathan Cameron, Lars-Peter Clausen, Jagath Jog J, linux-iio,
linux-kernel, Luke D . Jones, Jonathan LoBue
On Sun, 4 Aug 2024 17:40:49 +0200
Denis Benato <benato.denis96@gmail.com> wrote:
> On 03/08/24 17:44, Jonathan Cameron wrote:
> > On Sat, 27 Jul 2024 14:30:34 +0200
> > Denis Benato <benato.denis96@gmail.com> wrote:
> >
> >> Prevent triggers from stop working after the device has entered sleep:
> >> use iio_device_suspend_triggering and iio_device_resume_triggering helpers.
> >
> > Hi Denis,
> >
> Hello Jonathan,
> > I'd got it into my head this was about main suspend / resume, but
> > it's runtime PM. I assume the s2idle uses only that level which is
> > interesting.
> >
> I have catched the problem with s2idle, but I don-t fully understand
> it will manifest outside of said scenario, nor if it will at all and
> only s2idle is affected.
>
> > Anyhow, solution seems safe. We might be able to do something nicer
> > in the long run as potentially we could have the trigger driver
> > notified when all consumers have entered this state at which point it
> > could stop generating triggers at all.
> > Totally agree.
> > Anyhow, that's a job for when we actually care about it.
> >
> > Applied to the togreg branch of iio.git and pushed out as testing
> > for 0-day to poke at it.
> >
> I have made a mistake while cleaning up patch 1/2 for submission and lost a piece:
> the pollfunc->irq=0 you suggested in your first mail.
>
> I would be more than happy to provide a v3, but if you prefer I can also send
> a separate patch.
Send a v3. I'll try and remember to drop the v2 before sending a pull request
upstream.
>
> I am sorry about that and I would like guidance on what to do in cases like this.
Not a problem, we all do stuff like this from time to time!
> > For now I'm not keen to see this pushed into drivers where we don't
> > know if anyone is running into this particular situation. We can
> > reevaluate that if we start getting lots of reports of this.
> >
> I catched the issue while developing an application for a handheld PC.
>
> As the application will target these kind of devices we can apply the fix
> to every relevant driver (bmi260 comes to mind) and have that well-tested
> on multiple drivers.
Ok. Let's see how it goes with a few drivers.
Jonathan
> > I'm also not going to rush this in as a fix. We can consider backporting
> > it once it's been in mainline for a bit and no side effects have
> > shown up.
> >
> > Thanks,
> >
> > Jonathan
> >
> Thanks,
>
> Denis
> >>
> >> Signed-off-by: Denis Benato <benato.denis96@gmail.com>
> >> ---
> >> drivers/iio/imu/bmi323/bmi323.h | 1 +
> >> drivers/iio/imu/bmi323/bmi323_core.c | 23 +++++++++++++++++++++++
> >> drivers/iio/imu/bmi323/bmi323_i2c.c | 1 +
> >> drivers/iio/imu/bmi323/bmi323_spi.c | 1 +
> >> 4 files changed, 26 insertions(+)
> >>
> >> diff --git a/drivers/iio/imu/bmi323/bmi323.h b/drivers/iio/imu/bmi323/bmi323.h
> >> index dff126d41658..209bccb1f335 100644
> >> --- a/drivers/iio/imu/bmi323/bmi323.h
> >> +++ b/drivers/iio/imu/bmi323/bmi323.h
> >> @@ -205,5 +205,6 @@
> >> struct device;
> >> int bmi323_core_probe(struct device *dev);
> >> extern const struct regmap_config bmi323_regmap_config;
> >> +extern const struct dev_pm_ops bmi323_core_pm_ops;
> >>
> >> #endif
> >> diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
> >> index d708d1fe3e42..4b2b211a3e88 100644
> >> --- a/drivers/iio/imu/bmi323/bmi323_core.c
> >> +++ b/drivers/iio/imu/bmi323/bmi323_core.c
> >> @@ -2121,6 +2121,29 @@ int bmi323_core_probe(struct device *dev)
> >> }
> >> EXPORT_SYMBOL_NS_GPL(bmi323_core_probe, IIO_BMI323);
> >>
> >> +#if defined(CONFIG_PM)
> >> +static int bmi323_core_runtime_suspend(struct device *dev)
> >> +{
> >> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> >> +
> >> + return iio_device_suspend_triggering(indio_dev);
> >> +}
> >> +
> >> +static int bmi323_core_runtime_resume(struct device *dev)
> >> +{
> >> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> >> +
> >> + return iio_device_resume_triggering(indio_dev);
> >> +}
> >> +
> >> +#endif
> >> +
> >> +const struct dev_pm_ops bmi323_core_pm_ops = {
> >> + SET_RUNTIME_PM_OPS(bmi323_core_runtime_suspend,
> >> + bmi323_core_runtime_resume, NULL)
> >> +};
> >> +EXPORT_SYMBOL_NS_GPL(bmi323_core_pm_ops, IIO_BMI323);
> >> +
> >> MODULE_DESCRIPTION("Bosch BMI323 IMU driver");
> >> MODULE_AUTHOR("Jagath Jog J <jagathjog1996@gmail.com>");
> >> MODULE_LICENSE("GPL");
> >> diff --git a/drivers/iio/imu/bmi323/bmi323_i2c.c b/drivers/iio/imu/bmi323/bmi323_i2c.c
> >> index 52140bf05765..057342f4f816 100644
> >> --- a/drivers/iio/imu/bmi323/bmi323_i2c.c
> >> +++ b/drivers/iio/imu/bmi323/bmi323_i2c.c
> >> @@ -128,6 +128,7 @@ MODULE_DEVICE_TABLE(of, bmi323_of_i2c_match);
> >> static struct i2c_driver bmi323_i2c_driver = {
> >> .driver = {
> >> .name = "bmi323",
> >> + .pm = pm_ptr(&bmi323_core_pm_ops),
> >> .of_match_table = bmi323_of_i2c_match,
> >> .acpi_match_table = bmi323_acpi_match,
> >> },
> >> diff --git a/drivers/iio/imu/bmi323/bmi323_spi.c b/drivers/iio/imu/bmi323/bmi323_spi.c
> >> index 7b1e8127d0dd..487d4ee05246 100644
> >> --- a/drivers/iio/imu/bmi323/bmi323_spi.c
> >> +++ b/drivers/iio/imu/bmi323/bmi323_spi.c
> >> @@ -79,6 +79,7 @@ MODULE_DEVICE_TABLE(of, bmi323_of_spi_match);
> >> static struct spi_driver bmi323_spi_driver = {
> >> .driver = {
> >> .name = "bmi323",
> >> + .pm = pm_ptr(&bmi323_core_pm_ops),
> >> .of_match_table = bmi323_of_spi_match,
> >> },
> >> .probe = bmi323_spi_probe,
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-07 12:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-27 12:30 [PATCH v2 0/2] iio: fix bug with triggers not resuming after sleep Denis Benato
2024-07-27 12:30 ` [PATCH v2 1/2] iio: trigger: allow devices to suspend/resume theirs associated trigger Denis Benato
2024-07-27 12:30 ` [PATCH v2 2/2] iio: bmi323: suspend and resume triggering on relevant pm operations Denis Benato
2024-08-03 15:44 ` Jonathan Cameron
2024-08-04 15:40 ` Denis Benato
2024-08-07 12:57 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox