> -----Original Message-----
> From: Hans de Goede [mailto:hdego...@redhat.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

Reply via email to