[PATCH V2 09/16] drm/vc4: v3d: simplify clock retrieval

Maíra Canal mcanal at igalia.com
Wed Aug 7 14:31:28 UTC 2024


Hi Stefan,

On 8/2/24 10:00, Stefan Wahren wrote:
> Hi Maíra,
> 
> Am 02.08.24 um 14:56 schrieb Maíra Canal:
>> Hi Stefan,
>>
>> On 7/31/24 13:41, Stefan Wahren wrote:
>>> Hi Maíra,
>>>
>>> Am 30.07.24 um 13:23 schrieb Maíra Canal:
>>>> On 7/28/24 10:00, Stefan Wahren wrote:
>>>>> Common pattern of handling deferred probe can be simplified with
>>>>> dev_err_probe() and devm_clk_get_optional(). This results in much
>>>>> less code.
>>>>>
>>>>> Signed-off-by: Stefan Wahren <wahrenst at gmx.net>
>>>>> ---
>>>>>   drivers/gpu/drm/vc4/vc4_v3d.c | 13 ++-----------
>>>>>   1 file changed, 2 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c
>>>>> b/drivers/gpu/drm/vc4/vc4_v3d.c
>>>>> index 1ede508a67d3..4bf3a8d24770 100644
>>>>> --- a/drivers/gpu/drm/vc4/vc4_v3d.c
>>>>> +++ b/drivers/gpu/drm/vc4/vc4_v3d.c
>>>>> @@ -441,20 +441,11 @@ static int vc4_v3d_bind(struct device *dev,
>>>>> struct device *master, void *data)
>>>>>       vc4->v3d = v3d;
>>>>>       v3d->vc4 = vc4;
>>>>>
>>>>> -    v3d->clk = devm_clk_get(dev, NULL);
>>>>> +    v3d->clk = devm_clk_get_optional(dev, NULL);
>>>>>       if (IS_ERR(v3d->clk)) {
>>>>>           int ret = PTR_ERR(v3d->clk);
>>>>>
>>>>
>>>> Super nit: you could delete this line ^
>>> Can you please explain? ret is required for dev_err_probe or do you mean
>>> the empty line after the declaration?
>>
>> Just deleting the empty line after the declaration. It is a super small
>> nit indeed.
> AFAIK an empty line after a declaration is part of the coding style. Or
> is different in drm?

TBH I just checked the result of `git grep "dev_err_probe"` and I
noticed that most of the times, we don't add an empty line after the
declaration in this case or we don't even create a variable, something
like:

return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");

But it is a pretty small nit. Feel free to ignore it.

Also, let me know if you need me to apply any patches to drm-misc-next.

Best Regards,
- Maíra

> 
> Best regards
>>
>> Best Regards,
>> - Maíra
>>
>>>>
>>>> Reviewed-by: Maíra Canal <mcanal at igalia.com>
>>>>
>>>> Best Regards,
>>>> - Maíra
>>>>
>>>>> -        if (ret == -ENOENT) {
>>>>> -            /* bcm2835 didn't have a clock reference in the DT. */
>>>>> -            ret = 0;
>>>>> -            v3d->clk = NULL;
>>>>> -        } else {
>>>>> -            if (ret != -EPROBE_DEFER)
>>>>> -                dev_err(dev, "Failed to get V3D clock: %d\n",
>>>>> -                    ret);
>>>>> -            return ret;
>>>>> -        }
>>>>> +        return dev_err_probe(dev, ret, "Failed to get V3D clock\n");
>>>>>       }
>>>>>
>>>>>       ret = platform_get_irq(pdev, 0);
>>>>> -- 
>>>>> 2.34.1
>>>>>
>>>>
>>>
> 


More information about the dri-devel mailing list