linux-kernel - RE: [PATCH V4 4/4] video: simplefb: switch to use clk

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: <AM0PR04MB4211E934272EE1A212711FAE80090@AM0PR04MB4211.eurprd04.prod.outlook.com>
Date:   Wed, 29 Aug 2018 13:15:50 +0000
From:   "A.s. Dong" <aisheng.dong@....com>
To:     Hans de Goede <hdegoede@...hat.com>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "sboyd@...nel.org" <sboyd@...nel.org>,
        "mturquette@...libre.com" <mturquette@...libre.com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "thor.thayer@...ux.intel.com" <thor.thayer@...ux.intel.com>,
        dl-linux-imx <linux-imx@....com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        "linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Stephen Boyd <sboyd@...eaurora.org>
Subject: RE: [PATCH V4 4/4] video: simplefb: switch to use clk_bulk API to
 simplify clock operations

> -----Original Message-----
> From: Hans de Goede [mailto:hdegoede@...hat.com]
> Sent: Wednesday, August 29, 2018 9:01 PM

[...]

> > @@ -252,39 +228,21 @@ static int simplefb_clocks_get(struct simplefb_par
> *par,
> >   static void simplefb_clocks_enable(struct simplefb_par *par,
> >   				   struct platform_device *pdev)
> >   {
> > -	int i, ret;
> > +	int ret;
> > +
> > +	ret = clk_bulk_prepare_enable(par->clk_count, par->clks);
> > +	if (ret)
> > +		dev_warn(&pdev->dev, "failed to enable clocks\n");
> 
> If clk_bulk_prepare_enable() fails, it leaves all clocks disabled, so you should
> not set par->clks_enabled = true; then.
> 

Thanks for spotting this.
The original code wanted to keep the behavior as before.
But a bit more thinking that unlike the exist code, clk_bulk_prepare_enable will
automatically do reverse clean up once it fails.
So no need to set par->clks_enabled = true anymore.

Will fix it and resend.

Regards
Dong Aisheng

> Otherwise this patch looks good.
> 
> Regards,
> 
> Hans