[PATCH] media: i2c: dw9714: use pm_runtime_force_suspend/resume for system suspend

[PATCH] media: i2c: dw9714: use pm_runtime_force_suspend/resume for system suspend

[Martin Kepplinger]

Using the same suspend function for runtime and system suspend doesn't
work in this case (when controlling regulators and clocks). suspend() and
resume() are clearly meant to stay balanced.

Use the pm_runtime_force_* helpers for system suspend and fix error like the
following when transitioning to system suspend:

[  559.685921] dw9714 3-000c: I2C write fail
[  559.685941] dw9714 3-000c: dw9714_vcm_suspend I2C failure: -5

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

diff --git a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c
index fcbebb3558b5..f6ddd7c4a1ff 100644
--- a/drivers/media/i2c/dw9714.c
+++ b/drivers/media/i2c/dw9714.c
@@ -267,7 +267,8 @@ static const struct of_device_id dw9714_of_table[] = {
 MODULE_DEVICE_TABLE(of, dw9714_of_table);
 
 static const struct dev_pm_ops dw9714_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(dw9714_vcm_suspend, dw9714_vcm_resume)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
 	SET_RUNTIME_PM_OPS(dw9714_vcm_suspend, dw9714_vcm_resume, NULL)
 };
 
-- 
2.30.2

Re: [PATCH] media: i2c: dw9714: use pm_runtime_force_suspend/resume for system suspend

[Sakari Ailus]

Hi Martin,

On Tue, Nov 09, 2021 at 01:55:05PM +0100, Martin Kepplinger wrote:
> Using the same suspend function for runtime and system suspend doesn't
> work in this case (when controlling regulators and clocks). suspend() and
> resume() are clearly meant to stay balanced.
> 
> Use the pm_runtime_force_* helpers for system suspend and fix error like the
> following when transitioning to system suspend:
> 
> [  559.685921] dw9714 3-000c: I2C write fail
> [  559.685941] dw9714 3-000c: dw9714_vcm_suspend I2C failure: -5
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> ---
>  drivers/media/i2c/dw9714.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c
> index fcbebb3558b5..f6ddd7c4a1ff 100644
> --- a/drivers/media/i2c/dw9714.c
> +++ b/drivers/media/i2c/dw9714.c
> @@ -267,7 +267,8 @@ static const struct of_device_id dw9714_of_table[] = {
>  MODULE_DEVICE_TABLE(of, dw9714_of_table);
>  
>  static const struct dev_pm_ops dw9714_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(dw9714_vcm_suspend, dw9714_vcm_resume)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
>  	SET_RUNTIME_PM_OPS(dw9714_vcm_suspend, dw9714_vcm_resume, NULL)
>  };

The purpose of the vcm suspend / resume callbacks is to gently move the
lens to the resting position without hitting the stopper.

The problem currently appears to be that during system suspend, the VCM may
well be powered off and the driver isn't checking for that. How about
adding that check?

-- 
Sakari Ailus

Re: [PATCH] media: i2c: dw9714: use pm_runtime_force_suspend/resume for system suspend

[Martin Kepplinger]

Am Dienstag, dem 16.11.2021 um 19:27 +0200 schrieb Sakari Ailus:
> Hi Martin,
> 
> On Tue, Nov 09, 2021 at 01:55:05PM +0100, Martin Kepplinger wrote:
> > Using the same suspend function for runtime and system suspend
> > doesn't
> > work in this case (when controlling regulators and clocks).
> > suspend() and
> > resume() are clearly meant to stay balanced.
> > 
> > Use the pm_runtime_force_* helpers for system suspend and fix error
> > like the
> > following when transitioning to system suspend:
> > 
> > [  559.685921] dw9714 3-000c: I2C write fail
> > [  559.685941] dw9714 3-000c: dw9714_vcm_suspend I2C failure: -5
> > 
> > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > ---
> >  drivers/media/i2c/dw9714.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/dw9714.c
> > b/drivers/media/i2c/dw9714.c
> > index fcbebb3558b5..f6ddd7c4a1ff 100644
> > --- a/drivers/media/i2c/dw9714.c
> > +++ b/drivers/media/i2c/dw9714.c
> > @@ -267,7 +267,8 @@ static const struct of_device_id
> > dw9714_of_table[] = {
> >  MODULE_DEVICE_TABLE(of, dw9714_of_table);
> >  
> >  static const struct dev_pm_ops dw9714_pm_ops = {
> > -       SET_SYSTEM_SLEEP_PM_OPS(dw9714_vcm_suspend,
> > dw9714_vcm_resume)
> > +       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > +                               pm_runtime_force_resume)
> >         SET_RUNTIME_PM_OPS(dw9714_vcm_suspend, dw9714_vcm_resume,
> > NULL)
> >  };
> 
> The purpose of the vcm suspend / resume callbacks is to gently move
> the
> lens to the resting position without hitting the stopper.
> 
> The problem currently appears to be that during system suspend, the
> VCM may
> well be powered off and the driver isn't checking for that. How about
> adding that check?
> 

thanks for having a look. Actually, I forgot to add support for a power
supply regulator, so this patch (description) is wrong and I'll send a
different one that adds optional vcc regulator support and splits up
system/runtime suspend functions to handle the regulator correctly.

thank you!

                              martin