| Message ID | 20211109125505.2682553-1-martin.kepplinger@puri.sm (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | media: i2c: dw9714: use pm_runtime_force_suspend/resume for system suspend | expand |
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?
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
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) };
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(-)