linux-kernel - Re: [PATCH] media: i2c: dw9714: use pm_runtime_force

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c9e7799c4b02210c5be29d1c18c4eabd2fe0194b.camel@puri.sm>
Date:   Thu, 18 Nov 2021 17:33:04 +0100
From:   Martin Kepplinger <martin.kepplinger@...i.sm>
To:     Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc:     mchehab@...nel.org, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel@...i.sm,
        linux-pm@...r.kernel.org
Subject: Re: [PATCH] media: i2c: dw9714: use pm_runtime_force_suspend/resume
 for system suspend

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@...i.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