linux-kernel - Re: [PATCH v4 79/79] media: hantro: do a PM resume earlier

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 29 Apr 2021 09:13:35 +0200
From:   Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To:     Ezequiel Garcia <ezequiel@...labora.com>
Cc:     linuxarm@...wei.com, mauro.chehab@...wei.com,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v4 79/79] media: hantro: do a PM resume earlier

Em Wed, 28 Apr 2021 14:17:50 -0300
Ezequiel Garcia <ezequiel@...labora.com> escreveu:

> Hi Mauro,
> 
> Thanks a lot for taking care of this.
> 
> On Wed, 2021-04-28 at 16:52 +0200, Mauro Carvalho Chehab wrote:
> > The device_run() first enables the clock and then
> > tries to resume PM runtime, checking for errors.
> > 
> > Well, if for some reason the pm_runtime can not resume,
> > it would be better to detect it beforehand.
> > 
> > So, change the order inside device_run().
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>  
> 
> Clocks could be behind power-domains, IIRC, so this change
> is fixing that.
> 
> However, this has ever been a problem for this driver,
> so I don't think it makes sense to bother with Fixes tag.

I would prefer to move this patch to the first part of this
series, together with other fixes, rebasing it to apply cleanly
before the pm_runtime_resume_and_get() patch, with:

    Fixes: 775fec69008d ("media: add Rockchip VPU JPEG encoder driver")

This way, people that could be interested on backporting it will be
capable to apply it as is to stable Kernel releases that came
with this driver.

> 
> Reviewed-by: Ezequiel Garcia <ezequiel@...labora.com>
> 
> Thanks,
> Ezequiel
> 
> > ---
> >  drivers/staging/media/hantro/hantro_drv.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> > index 25fa36e7e773..67de6b15236d 100644
> > --- a/drivers/staging/media/hantro/hantro_drv.c
> > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > @@ -160,14 +160,14 @@ static void device_run(void *priv)
> >         src = hantro_get_src_buf(ctx);
> >         dst = hantro_get_dst_buf(ctx);
> >  
> > -       ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
> > -       if (ret)
> > -               goto err_cancel_job;
> > -
> >         ret = pm_runtime_resume_and_get(ctx->dev->dev);
> >         if (ret < 0)
> >                 goto err_cancel_job;
> >  
> > +       ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
> > +       if (ret)
> > +               goto err_cancel_job;
> > +
> >         v4l2_m2m_buf_copy_metadata(src, dst, true);
> >  
> >         ctx->codec_ops->run(ctx);  
> 
> 



Thanks,
Mauro