Hi

Am 18.03.21 um 11:39 schrieb Geert Uytterhoeven:
> Hi Thomas,
> 
> On Thu, Mar 18, 2021 at 11:29 AM 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>
>> Tested-by: nerdopolis <bluescreen_avenger@verizon.net>
> 
> Thanks for your patch!
> 
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> 
>> +static int simpledrm_device_init_clocks(struct simpledrm_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);
>> +               if (IS_ERR(clock)) {
>> +                       ret = PTR_ERR(clock);
>> +                       if (ret == -EPROBE_DEFER)
>> +                               goto err;
>> +                       drm_err(dev, "clock %u not found: %d\n", i, ret);
>> +                       continue;
>> +               }
>> +               ret = clk_prepare_enable(clock);
>> +               if (ret) {
>> +                       drm_err(dev, "failed to enable clock %u: %d\n",
>> +                               i, ret);
>> +                       clk_put(clock);
>> +               }
>> +               sdev->clks[i] = clock;
>> +       }
> 
> of_clk_bulk_get_all() + clk_bulk_prepare_enable()?
> 
> There's also devm_clk_bulk_get_all(), but not for the OF variant.

Right, you mentioned this on the original patch set. I tried to use the 
functions, but TBH I found them to obfuscate the overall logic of the 
function. So I went back to the original code. Hopefully this is not too 
much of an issue.

Best regards
Thomas

> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer