[PATCH v1 0/2] media: i2c: hi846: minor PM fixes

[PATCH v1 0/2] media: i2c: hi846: minor PM fixes

[Martin Kepplinger]

hi Saraki and all,

Here are minor PM fixes for the hi846 sensor while testing system suspend:

thank you very much for your time,

                             martin


Martin Kepplinger (2):
  media: i2c: hi846: check return value of regulator_bulk_disable()
  media: i2c: hi846: use pm_runtime_force_suspend/resume for system
    suspend

 drivers/media/i2c/hi846.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.30.2

[PATCH v1 1/2] media: i2c: hi846: check return value of regulator_bulk_disable()

[Martin Kepplinger]

regulator_bulk_disable can fail and thus suspend() can. Handle that error
gracefully.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---
 drivers/media/i2c/hi846.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c
index 4b94bdf2e5cb..ed55c3894284 100644
--- a/drivers/media/i2c/hi846.c
+++ b/drivers/media/i2c/hi846.c
@@ -1858,7 +1858,7 @@ static int hi846_power_on(struct hi846 *hi846)
 	return ret;
 }
 
-static void hi846_power_off(struct hi846 *hi846)
+static int hi846_power_off(struct hi846 *hi846)
 {
 	if (hi846->rst_gpio)
 		gpiod_set_value_cansleep(hi846->rst_gpio, 1);
@@ -1867,7 +1867,7 @@ static void hi846_power_off(struct hi846 *hi846)
 		gpiod_set_value_cansleep(hi846->shutdown_gpio, 1);
 
 	clk_disable_unprepare(hi846->clock);
-	regulator_bulk_disable(HI846_NUM_SUPPLIES, hi846->supplies);
+	return regulator_bulk_disable(HI846_NUM_SUPPLIES, hi846->supplies);
 }
 
 static int __maybe_unused hi846_suspend(struct device *dev)
@@ -1879,9 +1879,7 @@ static int __maybe_unused hi846_suspend(struct device *dev)
 	if (hi846->streaming)
 		hi846_stop_streaming(hi846);
 
-	hi846_power_off(hi846);
-
-	return 0;
+	return hi846_power_off(hi846);
 }
 
 static int __maybe_unused hi846_resume(struct device *dev)
-- 
2.30.2

[PATCH v1 2/2] media: i2c: hi846: use pm_runtime_force_suspend/resume for system suspend

[Martin Kepplinger]

In cases like this when controlling regulators and clocks the suspend()
and resume() functions are meant to be called balanced toggling the behaviour.

It's wrong to use the same suspend function for runtime and system suspend
in this case and leads to errors like

[   77.718890] Failed to disable vddd: -EIO

Use pm_runtime_force_* helpers in order to support system suspend properly
when runtime pm is already implemented and fix this.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---
 drivers/media/i2c/hi846.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c
index ed55c3894284..9bb96754a091 100644
--- a/drivers/media/i2c/hi846.c
+++ b/drivers/media/i2c/hi846.c
@@ -2418,7 +2418,11 @@ static int hi846_remove(struct i2c_client *client)
 	return 0;
 }
 
-static UNIVERSAL_DEV_PM_OPS(hi846_pm_ops, hi846_suspend, hi846_resume, NULL);
+static const struct dev_pm_ops hi846_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(hi846_suspend, hi846_resume, NULL)
+};
 
 static const struct of_device_id hi846_of_match[] = {
 	{ .compatible = "hynix,hi846", },
-- 
2.30.2

Re: [PATCH v1 0/2] media: i2c: hi846: minor PM fixes

[Martin Kepplinger]

Am Dienstag, dem 09.11.2021 um 14:10 +0100 schrieb Martin Kepplinger:
> hi Saraki and all,
> 
> Here are minor PM fixes for the hi846 sensor while testing system
> suspend:
> 
> thank you very much for your time,
> 
>                              martin
> 
> 
> Martin Kepplinger (2):
>   media: i2c: hi846: check return value of regulator_bulk_disable()
>   media: i2c: hi846: use pm_runtime_force_suspend/resume for system
>     suspend
> 
>  drivers/media/i2c/hi846.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 

hi all. Any objection or other thoughts about this? This fixes system
suspend.

thank you!

                          martin

Re: [PATCH v1 0/2] media: i2c: hi846: minor PM fixes

[Sakari Ailus]

On Tue, Dec 14, 2021 at 02:44:41PM +0100, Martin Kepplinger wrote:
> Am Dienstag, dem 09.11.2021 um 14:10 +0100 schrieb Martin Kepplinger:
> > hi Saraki and all,
> > 
> > Here are minor PM fixes for the hi846 sensor while testing system
> > suspend:
> > 
> > thank you very much for your time,
> > 
> >                              martin
> > 
> > 
> > Martin Kepplinger (2):
> >   media: i2c: hi846: check return value of regulator_bulk_disable()
> >   media: i2c: hi846: use pm_runtime_force_suspend/resume for system
> >     suspend
> > 
> >  drivers/media/i2c/hi846.c | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> 
> hi all. Any objection or other thoughts about this? This fixes system
> suspend.

Thanks for the ping, Martin.

The patches are in my tree now.

-- 
Sakari Ailus