Hi,
On 28.01.25 17:06, Michael Grzeschik wrote:
It's good style to write some message here. ;)
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
>
> ---
> v3 -> v4: - better bail out if the regulator is set but not available
> v2 -> v3: - reset regulator->power to NULL in case nothing was found
> v1 -> v2: - added the missing signed-off-by
> ---
> drivers/video/simple-panel.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/video/simple-panel.c b/drivers/video/simple-panel.c
> index 7048e2f51b..905fd94fb7 100644
> --- a/drivers/video/simple-panel.c
> +++ b/drivers/video/simple-panel.c
> @@ -12,6 +12,7 @@
> #include <fb.h>
> #include <gpio.h>
> #include <of_gpio.h>
> +#include <regulator.h>
> #include <video/backlight.h>
> #include <video/vpl.h>
> #include <i2c/i2c.h>
> @@ -25,6 +26,7 @@ struct simple_panel {
> struct device_node *backlight_node;
> struct backlight_device *backlight;
> struct device_node *ddc_node;
> + struct regulator *power;
> int enable_delay;
> };
>
> @@ -49,6 +51,8 @@ static int simple_panel_enable(struct simple_panel *panel)
> if (panel->enable_delay)
> mdelay(panel->enable_delay);
>
> + regulator_enable(panel->power);
Conceptually, you power on a device before you toggle its pins, so this
needs to be moved before the enable_gpio toggling.
> +
> if (panel->backlight) {
> ret = backlight_set_brightness_default(panel->backlight);
> if (ret)
> @@ -65,6 +69,8 @@ static int simple_panel_disable(struct simple_panel *panel)
> if (panel->backlight)
> backlight_set_brightness(panel->backlight, 0);
>
> + regulator_disable(panel->power);
Similarly, this should be after the GPIO was dis-asserted.
> +
> if (gpio_is_valid(panel->enable_gpio))
> gpio_direction_output(panel->enable_gpio,
> !panel->enable_active_high);
> @@ -154,6 +160,12 @@ static int simple_panel_probe(struct device *dev)
>
> panel->backlight_node = of_parse_phandle(node, "backlight", 0);
>
> + panel->power = regulator_get(dev, "power");
> + if (IS_ERR(panel->power)) {
> + dev_warn(dev, "Cannot find regulator\n");
> + return PTR_ERR(panel->power);
return dev_errp_probe(dev, panel->power, "Cannot find regulator\n");
So:
- error code is logged
- EPROBE_DEFER doesn't result in a diagnostic yet
- the correct log level is used (warning is for issues that the driver
can recover from)
> + }
> +
> ret = vpl_register(&panel->vpl);
> if (ret)
> return ret;
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
