lkml.org 
[lkml]   [2021]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v8 4/8] drm/ingenic: Add dw-hdmi driver for jz4780
From
Date
Hi Paul,


>>> You probably should disable the regulator (if not NULL) here.
>> Indeed. Would it be ok to make struct regulator *regulator static
>> or do we need dynamically allocated memory?
>
> static non-const is almost always a bad idea, so avoid it.

Well some years ago it was a perfectly simple solution that still works...
But I asked because I had a lot of doubt.

>
> You can either:
>
> - create a "ingenic_dw_hdmi" struct that will contain a pointer to dw_hdmi and a pointer to the regulator. Instanciate it in the probe with devm_kzalloc() and set the pointers, then set it as the driver data (platform_set_drvdata). In the remove function you can then obtain the pointer to your ingenic_dw_hdmi struct with platform_get_drvdata(), and you can remove the dw_hdmi and unregister the regulator.
>
> - register cleanup functions, using devm_add_action_or_reset(dev, f, priv). When it's time to cleanup, the kernel core will call f(priv) automatically. So you can add a small wrapper around dw_hdmi_remove() and another one around regulator_disable(), and those will be called automatically if your probe function fails, or when the driver is removed. Then you can completely remove the ".remove" callback. There are a few examples of these in the ingenic-drm-drv.c if you want to take a look.

The second one turned out to be cleaner to handle special cases like if there is no regulator. We just register the disabler only if there is a regulator and enable succeeds.

So v9 is coming now.

BR and thanks,
Nikolaus

\
 
 \ /
  Last update: 2021-11-24 22:30    [W:0.598 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site