Re: [PATCH 6/9] drm/simplekms: Acquire clocks from DT device node

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jun 25, 2020 at 03:34:05PM +0200, Geert Uytterhoeven wrote:
> Hi Thomas,
> 
> On Thu, Jun 25, 2020 at 2:00 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> > Make sure required hardware clocks are enabled while the firmware
> > framebuffer is in use.
> >
> > The basic code has been taken from the simplefb driver and adapted
> > to DRM. Clocks are released automatically via devres helpers.
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Thanks for your patch!
> 
> > --- a/drivers/gpu/drm/tiny/simplekms.c
> > +++ b/drivers/gpu/drm/tiny/simplekms.c
> 
> > @@ -210,6 +218,103 @@ static struct simplekms_device *simplekms_device_of_dev(struct drm_device *dev)
> >         return container_of(dev, struct simplekms_device, dev);
> >  }
> >
> > +/*
> > + * Hardware
> > + */
> > +
> > +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
> > +/*
> > + * Clock handling code.
> > + *
> > + * Here we handle the clocks property of our "simple-framebuffer" dt node.
> > + * This is necessary so that we can make sure that any clocks needed by
> > + * the display engine that the bootloader set up for us (and for which it
> > + * provided a simplefb dt node), stay up, for the life of the simplefb
> > + * driver.
> > + *
> > + * When the driver unloads, we cleanly disable, and then release the clocks.
> > + *
> > + * We only complain about errors here, no action is taken as the most likely
> > + * error can only happen due to a mismatch between the bootloader which set
> > + * up simplefb, and the clock definitions in the device tree. Chances are
> > + * that there are no adverse effects, and if there are, a clean teardown of
> > + * the fb probe will not help us much either. So just complain and carry on,
> > + * and hope that the user actually gets a working fb at the end of things.
> > + */
> > +
> > +static void simplekms_device_release_clocks(void *res)
> > +{
> > +       struct simplekms_device *sdev = simplekms_device_of_dev(res);
> > +       unsigned int i;
> > +
> > +       for (i = 0; i < sdev->clk_count; ++i) {
> > +               if (sdev->clks[i]) {
> > +                       clk_disable_unprepare(sdev->clks[i]);
> > +                       clk_put(sdev->clks[i]);
> > +               }
> > +       }
> > +}
> > +
> > +static int simplekms_device_init_clocks(struct simplekms_device *sdev)
> > +{
> > +       struct drm_device *dev = &sdev->dev;
> > +       struct platform_device *pdev = sdev->pdev;
> > +       struct device_node *of_node = pdev->dev.of_node;
> > +       struct clk *clock;
> > +       unsigned int i;
> > +       int ret;
> > +
> > +       if (dev_get_platdata(&pdev->dev) || !of_node)
> > +               return 0;
> > +
> > +       sdev->clk_count = of_clk_get_parent_count(of_node);
> > +       if (!sdev->clk_count)
> > +               return 0;
> > +
> > +       sdev->clks = drmm_kzalloc(dev, sdev->clk_count * sizeof(sdev->clks[0]),
> > +                                 GFP_KERNEL);
> > +       if (!sdev->clks)
> > +               return -ENOMEM;
> > +
> > +       for (i = 0; i < sdev->clk_count; ++i) {
> > +               clock = of_clk_get(of_node, i);
> 
> clk_bulk_get_all()?

Plus then you can use devm_clk_bulk_get_all, even less code I think.
-Daniel

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel