* [PATCH v4] video: simple-panel: add regulator support @ 2025-01-28 16:06 Michael Grzeschik 2025-01-28 16:49 ` Ahmad Fatoum 0 siblings, 1 reply; 3+ messages in thread From: Michael Grzeschik @ 2025-01-28 16:06 UTC (permalink / raw) To: barebox Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> --- 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); + 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); + 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); + } + ret = vpl_register(&panel->vpl); if (ret) return ret; -- 2.39.5 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] video: simple-panel: add regulator support 2025-01-28 16:06 [PATCH v4] video: simple-panel: add regulator support Michael Grzeschik @ 2025-01-28 16:49 ` Ahmad Fatoum 2025-01-28 16:59 ` Michael Grzeschik 0 siblings, 1 reply; 3+ messages in thread From: Ahmad Fatoum @ 2025-01-28 16:49 UTC (permalink / raw) To: Michael Grzeschik, barebox 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@pengutronix.de> > > --- > 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 | ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] video: simple-panel: add regulator support 2025-01-28 16:49 ` Ahmad Fatoum @ 2025-01-28 16:59 ` Michael Grzeschik 0 siblings, 0 replies; 3+ messages in thread From: Michael Grzeschik @ 2025-01-28 16:59 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox [-- Attachment #1: Type: text/plain, Size: 3472 bytes --] On Tue, Jan 28, 2025 at 05:49:56PM +0100, Ahmad Fatoum wrote: >Hi, > >On 28.01.25 17:06, Michael Grzeschik wrote: > >It's good style to write some message here. ;) Right, but I skipped it for v5. sorry. :) >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> >> >> --- >> 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. This is true for both cases. I fixed that. > >> + >> 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) Great hint. Did that in v5. > >> + } >> + >> 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 | > -- 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 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-01-28 17:00 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-01-28 16:06 [PATCH v4] video: simple-panel: add regulator support Michael Grzeschik 2025-01-28 16:49 ` Ahmad Fatoum 2025-01-28 16:59 ` Michael Grzeschik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox