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
