* [PATCH] backlight: led_bl: Add support for an "enable" GPIO
@ 2021-11-02 10:04 Corentin LABBE
2021-11-02 11:19 ` Daniel Thompson
0 siblings, 1 reply; 4+ messages in thread
From: Corentin LABBE @ 2021-11-02 10:04 UTC (permalink / raw)
To: lee.jones, daniel.thompson, jingoohan1
Cc: dri-devel, linux-fbdev, linux-kernel, khilman,
Jean-Jacques Hiblot, Corentin LABBE
From: Jean-Jacques Hiblot <jjhiblot@ti.com>
This patch adds support for an "enable GPIO".
Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
Signed-off-by: Corentin LABBE <clabbe@baylibre.com>
---
drivers/video/backlight/led_bl.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index f54d256e2d54..ebd7acc32980 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -8,6 +8,7 @@
#include <linux/backlight.h>
#include <linux/leds.h>
+#include <linux/gpio.h>
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -15,6 +16,7 @@ struct led_bl_data {
struct device *dev;
struct backlight_device *bl_dev;
struct led_classdev **leds;
+ struct gpio_desc *enable_gpio;
bool enabled;
int nb_leds;
unsigned int *levels;
@@ -35,6 +37,9 @@ static void led_bl_set_brightness(struct led_bl_data *priv, int level)
for (i = 0; i < priv->nb_leds; i++)
led_set_brightness(priv->leds[i], bkl_brightness);
+ if (!priv->enabled)
+ gpiod_set_value_cansleep(priv->enable_gpio, 1);
+
priv->enabled = true;
}
@@ -48,6 +53,9 @@ static void led_bl_power_off(struct led_bl_data *priv)
for (i = 0; i < priv->nb_leds; i++)
led_set_brightness(priv->leds[i], LED_OFF);
+ if (priv->enabled)
+ gpiod_set_value_cansleep(priv->enable_gpio, 0);
+
priv->enabled = false;
}
@@ -209,6 +217,11 @@ static int led_bl_probe(struct platform_device *pdev)
return PTR_ERR(priv->bl_dev);
}
+ priv->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(priv->enable_gpio))
+ return PTR_ERR(priv->enable_gpio);
+
for (i = 0; i < priv->nb_leds; i++)
led_sysfs_disable(priv->leds[i]);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] backlight: led_bl: Add support for an "enable" GPIO
2021-11-02 10:04 [PATCH] backlight: led_bl: Add support for an "enable" GPIO Corentin LABBE
@ 2021-11-02 11:19 ` Daniel Thompson
2021-11-02 11:25 ` Daniel Thompson
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Thompson @ 2021-11-02 11:19 UTC (permalink / raw)
To: Corentin LABBE
Cc: lee.jones, jingoohan1, dri-devel, linux-fbdev, linux-kernel,
khilman, Jean-Jacques Hiblot, Pavel Machek
On Tue, Nov 02, 2021 at 10:04:55AM +0000, Corentin LABBE wrote:
> From: Jean-Jacques Hiblot <jjhiblot@ti.com>
>
> This patch adds support for an "enable GPIO".
Before taking this kind of change is there a good reason why backlight
should manage the GPIO? I thought that the LED subsystem was a sub
system for LEDs (not LED controllers). In other words if you direct
that the LED be lit up then isn't it the LED driver's job to manage
the GPIO and ensure that it lights up?
Daniel.
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Signed-off-by: Corentin LABBE <clabbe@baylibre.com>
> ---
> drivers/video/backlight/led_bl.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
> index f54d256e2d54..ebd7acc32980 100644
> --- a/drivers/video/backlight/led_bl.c
> +++ b/drivers/video/backlight/led_bl.c
> @@ -8,6 +8,7 @@
>
> #include <linux/backlight.h>
> #include <linux/leds.h>
> +#include <linux/gpio.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
>
> @@ -15,6 +16,7 @@ struct led_bl_data {
> struct device *dev;
> struct backlight_device *bl_dev;
> struct led_classdev **leds;
> + struct gpio_desc *enable_gpio;
> bool enabled;
> int nb_leds;
> unsigned int *levels;
> @@ -35,6 +37,9 @@ static void led_bl_set_brightness(struct led_bl_data *priv, int level)
> for (i = 0; i < priv->nb_leds; i++)
> led_set_brightness(priv->leds[i], bkl_brightness);
>
> + if (!priv->enabled)
> + gpiod_set_value_cansleep(priv->enable_gpio, 1);
> +
> priv->enabled = true;
> }
>
> @@ -48,6 +53,9 @@ static void led_bl_power_off(struct led_bl_data *priv)
> for (i = 0; i < priv->nb_leds; i++)
> led_set_brightness(priv->leds[i], LED_OFF);
>
> + if (priv->enabled)
> + gpiod_set_value_cansleep(priv->enable_gpio, 0);
> +
> priv->enabled = false;
> }
>
> @@ -209,6 +217,11 @@ static int led_bl_probe(struct platform_device *pdev)
> return PTR_ERR(priv->bl_dev);
> }
>
> + priv->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(priv->enable_gpio))
> + return PTR_ERR(priv->enable_gpio);
> +
> for (i = 0; i < priv->nb_leds; i++)
> led_sysfs_disable(priv->leds[i]);
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] backlight: led_bl: Add support for an "enable" GPIO
2021-11-02 11:19 ` Daniel Thompson
@ 2021-11-02 11:25 ` Daniel Thompson
2021-11-02 21:42 ` LABBE Corentin
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Thompson @ 2021-11-02 11:25 UTC (permalink / raw)
To: Corentin LABBE
Cc: lee.jones, jingoohan1, dri-devel, linux-fbdev, linux-kernel,
khilman, Jean-Jacques Hiblot, Pavel Machek
On Tue, Nov 02, 2021 at 11:19:42AM +0000, Daniel Thompson wrote:
> On Tue, Nov 02, 2021 at 10:04:55AM +0000, Corentin LABBE wrote:
> > From: Jean-Jacques Hiblot <jjhiblot@ti.com>
> >
> > This patch adds support for an "enable GPIO".
>
> Before taking this kind of change is there a good reason why backlight
> should manage the GPIO? I thought that the LED subsystem was a sub
> system for LEDs (not LED controllers). In other words if you direct
> that the LED be lit up then isn't it the LED driver's job to manage
> the GPIO and ensure that it lights up?
Sorry... I should have paid more attention to my sense of déjà vu with
this patch.
This approach was discussed and rejected when we first introduced the
led_bl driver:
https://lore.kernel.org/linux-leds/20190705100851.zn2jkipj4fxq5we6@devuan/
Daniel.
> > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> > Signed-off-by: Corentin LABBE <clabbe@baylibre.com>
> > ---
> > drivers/video/backlight/led_bl.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
> > index f54d256e2d54..ebd7acc32980 100644
> > --- a/drivers/video/backlight/led_bl.c
> > +++ b/drivers/video/backlight/led_bl.c
> > @@ -8,6 +8,7 @@
> >
> > #include <linux/backlight.h>
> > #include <linux/leds.h>
> > +#include <linux/gpio.h>
> > #include <linux/module.h>
> > #include <linux/platform_device.h>
> >
> > @@ -15,6 +16,7 @@ struct led_bl_data {
> > struct device *dev;
> > struct backlight_device *bl_dev;
> > struct led_classdev **leds;
> > + struct gpio_desc *enable_gpio;
> > bool enabled;
> > int nb_leds;
> > unsigned int *levels;
> > @@ -35,6 +37,9 @@ static void led_bl_set_brightness(struct led_bl_data *priv, int level)
> > for (i = 0; i < priv->nb_leds; i++)
> > led_set_brightness(priv->leds[i], bkl_brightness);
> >
> > + if (!priv->enabled)
> > + gpiod_set_value_cansleep(priv->enable_gpio, 1);
> > +
> > priv->enabled = true;
> > }
> >
> > @@ -48,6 +53,9 @@ static void led_bl_power_off(struct led_bl_data *priv)
> > for (i = 0; i < priv->nb_leds; i++)
> > led_set_brightness(priv->leds[i], LED_OFF);
> >
> > + if (priv->enabled)
> > + gpiod_set_value_cansleep(priv->enable_gpio, 0);
> > +
> > priv->enabled = false;
> > }
> >
> > @@ -209,6 +217,11 @@ static int led_bl_probe(struct platform_device *pdev)
> > return PTR_ERR(priv->bl_dev);
> > }
> >
> > + priv->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
> > + GPIOD_OUT_LOW);
> > + if (IS_ERR(priv->enable_gpio))
> > + return PTR_ERR(priv->enable_gpio);
> > +
> > for (i = 0; i < priv->nb_leds; i++)
> > led_sysfs_disable(priv->leds[i]);
> >
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] backlight: led_bl: Add support for an "enable" GPIO
2021-11-02 11:25 ` Daniel Thompson
@ 2021-11-02 21:42 ` LABBE Corentin
0 siblings, 0 replies; 4+ messages in thread
From: LABBE Corentin @ 2021-11-02 21:42 UTC (permalink / raw)
To: Daniel Thompson
Cc: lee.jones, jingoohan1, dri-devel, linux-fbdev, linux-kernel,
khilman, Jean-Jacques Hiblot, Pavel Machek
Le Tue, Nov 02, 2021 at 11:25:14AM +0000, Daniel Thompson a écrit :
> On Tue, Nov 02, 2021 at 11:19:42AM +0000, Daniel Thompson wrote:
> > On Tue, Nov 02, 2021 at 10:04:55AM +0000, Corentin LABBE wrote:
> > > From: Jean-Jacques Hiblot <jjhiblot@ti.com>
> > >
> > > This patch adds support for an "enable GPIO".
> >
> > Before taking this kind of change is there a good reason why backlight
> > should manage the GPIO? I thought that the LED subsystem was a sub
> > system for LEDs (not LED controllers). In other words if you direct
> > that the LED be lit up then isn't it the LED driver's job to manage
> > the GPIO and ensure that it lights up?
>
> Sorry... I should have paid more attention to my sense of déjà vu with
> this patch.
>
> This approach was discussed and rejected when we first introduced the
> led_bl driver:
> https://lore.kernel.org/linux-leds/20190705100851.zn2jkipj4fxq5we6@devuan/
>
Hello
I am sorry, I didnt checked if the patch was already submitted or not.
Regards
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-02 21:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-02 10:04 [PATCH] backlight: led_bl: Add support for an "enable" GPIO Corentin LABBE
2021-11-02 11:19 ` Daniel Thompson
2021-11-02 11:25 ` Daniel Thompson
2021-11-02 21:42 ` LABBE Corentin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).