* [PATCH v1 0/2] backlight: gpio-backlight: Add support for multiple GPIOs @ 2026-01-05 8:51 Sudarshan Shetty 2026-01-05 8:51 ` [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow " Sudarshan Shetty 2026-01-05 8:51 ` [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty 0 siblings, 2 replies; 14+ messages in thread From: Sudarshan Shetty @ 2026-01-05 8:51 UTC (permalink / raw) To: lee, danielt, jingoohan1 Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel, Sudarshan Shetty Hi all, This patch extends the gpio-backlight driver and its Device Tree bindings to support multiple GPIOs for controlling a single backlight device. Some panels require more than one GPIO to enable or disable the backlight, and previously the driver only supported a single GPIO. With this change: - The driver now handles an array of GPIOs and updates all of them based on brightness state. - The Device Tree binding has been updated to allow specifying 1 or 2 GPIOs for a backlight node. This approach avoids describing multiple backlight devices in DT for a single panel. Thanks, Anusha Sudarshan Shetty (2): dt-bindings: backlight: gpio-backlight: allow multiple GPIOs backlight: gpio: add support for multiple GPIOs for backlight control .../leds/backlight/gpio-backlight.yaml | 12 +++- drivers/video/backlight/gpio_backlight.c | 61 ++++++++++++++----- 2 files changed, 56 insertions(+), 17 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs 2026-01-05 8:51 [PATCH v1 0/2] backlight: gpio-backlight: Add support for multiple GPIOs Sudarshan Shetty @ 2026-01-05 8:51 ` Sudarshan Shetty 2026-01-05 9:55 ` Daniel Thompson 2026-01-05 8:51 ` [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty 1 sibling, 1 reply; 14+ messages in thread From: Sudarshan Shetty @ 2026-01-05 8:51 UTC (permalink / raw) To: lee, danielt, jingoohan1 Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel, Sudarshan Shetty Update the gpio-backlight binding to support configurations that require more than one GPIO for enabling/disabling the backlight. Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> --- .../bindings/leds/backlight/gpio-backlight.yaml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml index 584030b6b0b9..1483ce4a3480 100644 --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml @@ -17,7 +17,8 @@ properties: gpios: description: The gpio that is used for enabling/disabling the backlight. - maxItems: 1 + minItems: 1 + maxItems: 2 default-on: description: enable the backlight at boot. @@ -38,4 +39,13 @@ examples: default-on; }; + - | + #include <dt-bindings/gpio/gpio.h> + backlight { + compatible = "gpio-backlight"; + gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>, + <&gpio3 5 GPIO_ACTIVE_HIGH>; + default-on; + }; + ... -- 2.34.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs 2026-01-05 8:51 ` [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow " Sudarshan Shetty @ 2026-01-05 9:55 ` Daniel Thompson 2026-01-13 4:45 ` tessolveupstream 0 siblings, 1 reply; 14+ messages in thread From: Daniel Thompson @ 2026-01-05 9:55 UTC (permalink / raw) To: Sudarshan Shetty Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On Mon, Jan 05, 2026 at 02:21:19PM +0530, Sudarshan Shetty wrote: > Update the gpio-backlight binding to support configurations that require > more than one GPIO for enabling/disabling the backlight. > > Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> > --- > .../bindings/leds/backlight/gpio-backlight.yaml | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml > index 584030b6b0b9..1483ce4a3480 100644 > --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml > +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml > @@ -17,7 +17,8 @@ properties: > > gpios: > description: The gpio that is used for enabling/disabling the backlight. > - maxItems: 1 > + minItems: 1 > + maxItems: 2 Why 2? Daniel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs 2026-01-05 9:55 ` Daniel Thompson @ 2026-01-13 4:45 ` tessolveupstream 2026-01-14 15:53 ` Daniel Thompson 0 siblings, 1 reply; 14+ messages in thread From: tessolveupstream @ 2026-01-13 4:45 UTC (permalink / raw) To: Daniel Thompson Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On 05-01-2026 15:25, Daniel Thompson wrote: > On Mon, Jan 05, 2026 at 02:21:19PM +0530, Sudarshan Shetty wrote: >> Update the gpio-backlight binding to support configurations that require >> more than one GPIO for enabling/disabling the backlight. >> >> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> >> --- >> .../bindings/leds/backlight/gpio-backlight.yaml | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml >> index 584030b6b0b9..1483ce4a3480 100644 >> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml >> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml >> @@ -17,7 +17,8 @@ properties: >> >> gpios: >> description: The gpio that is used for enabling/disabling the backlight. >> - maxItems: 1 >> + minItems: 1 >> + maxItems: 2 > > Why 2? > In the current design, the LVDS panel has a single backlight that is controlled by two GPIOs. Initially, It described as two separate backlight devices using the same gpio-backlight driver, since the existing driver supports only one GPIO per instance. So the maintainer suggested to extend the gpio-backlight driver and bindings to support multiple GPIOs. https://lore.kernel.org/all/q63bdon55app4gb2il5e7skyc6z2amcnaiqbqlhen7arkxphtb@3jejbelji2ti/ > > Daniel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs 2026-01-13 4:45 ` tessolveupstream @ 2026-01-14 15:53 ` Daniel Thompson 2026-01-18 16:48 ` tessolveupstream 0 siblings, 1 reply; 14+ messages in thread From: Daniel Thompson @ 2026-01-14 15:53 UTC (permalink / raw) To: tessolveupstream Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On Tue, Jan 13, 2026 at 10:15:53AM +0530, tessolveupstream@gmail.com wrote: > > > On 05-01-2026 15:25, Daniel Thompson wrote: > > On Mon, Jan 05, 2026 at 02:21:19PM +0530, Sudarshan Shetty wrote: > >> Update the gpio-backlight binding to support configurations that require > >> more than one GPIO for enabling/disabling the backlight. > >> > >> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> > >> --- > >> .../bindings/leds/backlight/gpio-backlight.yaml | 12 +++++++++++- > >> 1 file changed, 11 insertions(+), 1 deletion(-) > >> > >> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml > >> index 584030b6b0b9..1483ce4a3480 100644 > >> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml > >> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml > >> @@ -17,7 +17,8 @@ properties: > >> > >> gpios: > >> description: The gpio that is used for enabling/disabling the backlight. > >> - maxItems: 1 > >> + minItems: 1 > >> + maxItems: 2 > > > > Why 2? > > > > In the current design, the LVDS panel has a single backlight that > is controlled by two GPIOs. Initially, It described as two separate > backlight devices using the same gpio-backlight driver, since the > existing driver supports only one GPIO per instance. > > So the maintainer suggested to extend the gpio-backlight driver > and bindings to support multiple GPIOs. > https://lore.kernel.org/all/q63bdon55app4gb2il5e7skyc6z2amcnaiqbqlhen7arkxphtb@3jejbelji2ti/ Right. So, once we support multiple GPIOs then why limit it to 2? Daniel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs 2026-01-14 15:53 ` Daniel Thompson @ 2026-01-18 16:48 ` tessolveupstream 2026-01-19 14:42 ` Daniel Thompson 0 siblings, 1 reply; 14+ messages in thread From: tessolveupstream @ 2026-01-18 16:48 UTC (permalink / raw) To: Daniel Thompson Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On 14-01-2026 21:23, Daniel Thompson wrote: > On Tue, Jan 13, 2026 at 10:15:53AM +0530, tessolveupstream@gmail.com wrote: >> >> >> On 05-01-2026 15:25, Daniel Thompson wrote: >>> On Mon, Jan 05, 2026 at 02:21:19PM +0530, Sudarshan Shetty wrote: >>>> Update the gpio-backlight binding to support configurations that require >>>> more than one GPIO for enabling/disabling the backlight. >>>> >>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> >>>> --- >>>> .../bindings/leds/backlight/gpio-backlight.yaml | 12 +++++++++++- >>>> 1 file changed, 11 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml >>>> index 584030b6b0b9..1483ce4a3480 100644 >>>> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml >>>> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml >>>> @@ -17,7 +17,8 @@ properties: >>>> >>>> gpios: >>>> description: The gpio that is used for enabling/disabling the backlight. >>>> - maxItems: 1 >>>> + minItems: 1 >>>> + maxItems: 2 >>> >>> Why 2? >>> >> >> In the current design, the LVDS panel has a single backlight that >> is controlled by two GPIOs. Initially, It described as two separate >> backlight devices using the same gpio-backlight driver, since the >> existing driver supports only one GPIO per instance. >> >> So the maintainer suggested to extend the gpio-backlight driver >> and bindings to support multiple GPIOs. >> https://lore.kernel.org/all/q63bdon55app4gb2il5e7skyc6z2amcnaiqbqlhen7arkxphtb@3jejbelji2ti/ > > Right. So, once we support multiple GPIOs then why limit it to 2? > Okay, got the point. I'm removing the maxItems constraint entirely to allow any number of GPIOs as below: diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml index 1483ce4a3480..82698519daff 100644 --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml @@ -16,9 +16,11 @@ properties: const: gpio-backlight gpios: - description: The gpio that is used for enabling/disabling the backlight. + description: | + The gpio that is used for enabling/disabling the backlight. + Multiple GPIOs can be specified for panels that require several + enable signals. minItems: 1 - maxItems: 2 default-on: description: enable the backlight at boot. Does this approach work for you? > > Daniel. ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs 2026-01-18 16:48 ` tessolveupstream @ 2026-01-19 14:42 ` Daniel Thompson 0 siblings, 0 replies; 14+ messages in thread From: Daniel Thompson @ 2026-01-19 14:42 UTC (permalink / raw) To: tessolveupstream Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On Sun, Jan 18, 2026 at 10:18:32PM +0530, tessolveupstream@gmail.com wrote: > On 14-01-2026 21:23, Daniel Thompson wrote: > > On Tue, Jan 13, 2026 at 10:15:53AM +0530, tessolveupstream@gmail.com wrote: > diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml > index 1483ce4a3480..82698519daff 100644 > --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml > +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml > @@ -16,9 +16,11 @@ properties: > const: gpio-backlight > > gpios: > - description: The gpio that is used for enabling/disabling the backlight. > + description: | > + The gpio that is used for enabling/disabling the backlight. > + Multiple GPIOs can be specified for panels that require several > + enable signals. > minItems: 1 > - maxItems: 2 > > default-on: > description: enable the backlight at boot. > > Does this approach work for you? I think so... but I'd rather just review a v2 patch than this kind of meta-patch. Daniel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control 2026-01-05 8:51 [PATCH v1 0/2] backlight: gpio-backlight: Add support for multiple GPIOs Sudarshan Shetty 2026-01-05 8:51 ` [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow " Sudarshan Shetty @ 2026-01-05 8:51 ` Sudarshan Shetty 2026-01-05 10:09 ` Daniel Thompson 1 sibling, 1 reply; 14+ messages in thread From: Sudarshan Shetty @ 2026-01-05 8:51 UTC (permalink / raw) To: lee, danielt, jingoohan1 Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel, Sudarshan Shetty Extend the gpio-backlight driver to handle multiple GPIOs instead of a single one. This allows panels that require driving several enable pins to be controlled by the backlight framework. Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> --- drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++------- 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 728a546904b0..037e1c111e48 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -17,14 +17,18 @@ struct gpio_backlight { struct device *dev; - struct gpio_desc *gpiod; + struct gpio_desc **gpiods; + unsigned int num_gpios; }; static int gpio_backlight_update_status(struct backlight_device *bl) { struct gpio_backlight *gbl = bl_get_data(bl); + unsigned int i; + int br = backlight_get_brightness(bl); - gpiod_set_value_cansleep(gbl->gpiod, backlight_get_brightness(bl)); + for (i = 0; i < gbl->num_gpios; i++) + gpiod_set_value_cansleep(gbl->gpiods[i], br); return 0; } @@ -52,6 +56,7 @@ static int gpio_backlight_probe(struct platform_device *pdev) struct backlight_device *bl; struct gpio_backlight *gbl; int ret, init_brightness, def_value; + unsigned int i; gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL); if (gbl == NULL) @@ -62,10 +67,22 @@ static int gpio_backlight_probe(struct platform_device *pdev) def_value = device_property_read_bool(dev, "default-on"); - gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS); - if (IS_ERR(gbl->gpiod)) - return dev_err_probe(dev, PTR_ERR(gbl->gpiod), - "The gpios parameter is missing or invalid\n"); + gbl->num_gpios = gpiod_count(dev, NULL); + if (gbl->num_gpios == 0) + return dev_err_probe(dev, -EINVAL, + "The gpios parameter is missing or invalid\n"); + gbl->gpiods = devm_kcalloc(dev, gbl->num_gpios, sizeof(*gbl->gpiods), + GFP_KERNEL); + if (!gbl->gpiods) + return -ENOMEM; + + for (i = 0; i < gbl->num_gpios; i++) { + gbl->gpiods[i] = + devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS); + if (IS_ERR(gbl->gpiods[i])) + return dev_err_probe(dev, PTR_ERR(gbl->gpiods[i]), + "Failed to get GPIO at index %u\n", i); + } memset(&props, 0, sizeof(props)); props.type = BACKLIGHT_RAW; @@ -78,22 +95,34 @@ static int gpio_backlight_probe(struct platform_device *pdev) } /* Set the initial power state */ - if (!of_node || !of_node->phandle) + if (!of_node || !of_node->phandle) { /* Not booted with device tree or no phandle link to the node */ bl->props.power = def_value ? BACKLIGHT_POWER_ON - : BACKLIGHT_POWER_OFF; - else if (gpiod_get_value_cansleep(gbl->gpiod) == 0) - bl->props.power = BACKLIGHT_POWER_OFF; - else - bl->props.power = BACKLIGHT_POWER_ON; + : BACKLIGHT_POWER_OFF; + } else { + bool all_high = true; + + for (i = 0; i < gbl->num_gpios; i++) { + if (gpiod_get_value_cansleep(gbl->gpiods[i]) != 0) { + all_high = false; + break; + } + } + + bl->props.power = + all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF; + } bl->props.brightness = 1; init_brightness = backlight_get_brightness(bl); - ret = gpiod_direction_output(gbl->gpiod, init_brightness); - if (ret) { - dev_err(dev, "failed to set initial brightness\n"); - return ret; + + for (i = 0; i < gbl->num_gpios; i++) { + ret = gpiod_direction_output(gbl->gpiods[i], init_brightness); + if (ret) + return dev_err_probe(dev, ret, + "failed to set gpio %u direction\n", + i); } platform_set_drvdata(pdev, bl); -- 2.34.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control 2026-01-05 8:51 ` [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty @ 2026-01-05 10:09 ` Daniel Thompson 2026-01-13 7:17 ` tessolveupstream 0 siblings, 1 reply; 14+ messages in thread From: Daniel Thompson @ 2026-01-05 10:09 UTC (permalink / raw) To: Sudarshan Shetty Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote: > Extend the gpio-backlight driver to handle multiple GPIOs instead of a > single one. This allows panels that require driving several enable pins > to be controlled by the backlight framework. > > Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> > --- > drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++------- > 1 file changed, 45 insertions(+), 16 deletions(-) > > diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c > index 728a546904b0..037e1c111e48 100644 > --- a/drivers/video/backlight/gpio_backlight.c > +++ b/drivers/video/backlight/gpio_backlight.c > @@ -17,14 +17,18 @@ > > struct gpio_backlight { > struct device *dev; > - struct gpio_desc *gpiod; > + struct gpio_desc **gpiods; > + unsigned int num_gpios; Why not use struct gpio_descs for this? Once you do that, then most of the gbl->num_gpios loops can be replaced with calls to the array based accessors. > }; > > static int gpio_backlight_update_status(struct backlight_device *bl) > { > struct gpio_backlight *gbl = bl_get_data(bl); > + unsigned int i; > + int br = backlight_get_brightness(bl); > > - gpiod_set_value_cansleep(gbl->gpiod, backlight_get_brightness(bl)); > + for (i = 0; i < gbl->num_gpios; i++) > + gpiod_set_value_cansleep(gbl->gpiods[i], br); > > return 0; > } > @@ -52,6 +56,7 @@ static int gpio_backlight_probe(struct platform_device *pdev) > struct backlight_device *bl; > struct gpio_backlight *gbl; > int ret, init_brightness, def_value; > + unsigned int i; > > gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL); > if (gbl == NULL) > @@ -62,10 +67,22 @@ static int gpio_backlight_probe(struct platform_device *pdev) > > def_value = device_property_read_bool(dev, "default-on"); > > - gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS); > - if (IS_ERR(gbl->gpiod)) > - return dev_err_probe(dev, PTR_ERR(gbl->gpiod), > - "The gpios parameter is missing or invalid\n"); > + gbl->num_gpios = gpiod_count(dev, NULL); > + if (gbl->num_gpios == 0) > + return dev_err_probe(dev, -EINVAL, > + "The gpios parameter is missing or invalid\n"); > + gbl->gpiods = devm_kcalloc(dev, gbl->num_gpios, sizeof(*gbl->gpiods), > + GFP_KERNEL); > + if (!gbl->gpiods) > + return -ENOMEM; This is definitely easier if you simply use devm_get_array(). > + > + for (i = 0; i < gbl->num_gpios; i++) { > + gbl->gpiods[i] = > + devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS); > + if (IS_ERR(gbl->gpiods[i])) > + return dev_err_probe(dev, PTR_ERR(gbl->gpiods[i]), > + "Failed to get GPIO at index %u\n", i); > + } > > memset(&props, 0, sizeof(props)); > props.type = BACKLIGHT_RAW; > @@ -78,22 +95,34 @@ static int gpio_backlight_probe(struct platform_device *pdev) > } > > /* Set the initial power state */ > - if (!of_node || !of_node->phandle) > + if (!of_node || !of_node->phandle) { > /* Not booted with device tree or no phandle link to the node */ > bl->props.power = def_value ? BACKLIGHT_POWER_ON > - : BACKLIGHT_POWER_OFF; > - else if (gpiod_get_value_cansleep(gbl->gpiod) == 0) > - bl->props.power = BACKLIGHT_POWER_OFF; > - else > - bl->props.power = BACKLIGHT_POWER_ON; > + : BACKLIGHT_POWER_OFF; > + } else { > + bool all_high = true; > + > + for (i = 0; i < gbl->num_gpios; i++) { > + if (gpiod_get_value_cansleep(gbl->gpiods[i]) != 0) { Why is there a != here? > + all_high = false; > + break; > + } > + } > + > + bl->props.power = > + all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF; > + } > > bl->props.brightness = 1; > > init_brightness = backlight_get_brightness(bl); > - ret = gpiod_direction_output(gbl->gpiod, init_brightness); > - if (ret) { > - dev_err(dev, "failed to set initial brightness\n"); > - return ret; > + > + for (i = 0; i < gbl->num_gpios; i++) { > + ret = gpiod_direction_output(gbl->gpiods[i], init_brightness); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to set gpio %u direction\n", > + i); > } > > platform_set_drvdata(pdev, bl); Daniel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control 2026-01-05 10:09 ` Daniel Thompson @ 2026-01-13 7:17 ` tessolveupstream 2026-01-14 16:03 ` Daniel Thompson 0 siblings, 1 reply; 14+ messages in thread From: tessolveupstream @ 2026-01-13 7:17 UTC (permalink / raw) To: Daniel Thompson Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On 05-01-2026 15:39, Daniel Thompson wrote: > On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote: >> Extend the gpio-backlight driver to handle multiple GPIOs instead of a >> single one. This allows panels that require driving several enable pins >> to be controlled by the backlight framework. >> >> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> >> --- >> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++------- >> 1 file changed, 45 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c >> index 728a546904b0..037e1c111e48 100644 >> --- a/drivers/video/backlight/gpio_backlight.c >> +++ b/drivers/video/backlight/gpio_backlight.c >> @@ -17,14 +17,18 @@ >> >> struct gpio_backlight { >> struct device *dev; >> - struct gpio_desc *gpiod; >> + struct gpio_desc **gpiods; >> + unsigned int num_gpios; > > Why not use struct gpio_descs for this? > > Once you do that, then most of the gbl->num_gpios loops can be replaced with > calls to the array based accessors. > Based on your feedback, I have updated the implementation to use struct gpio_descs and array-based accessors, as recommended like below: git diff drivers/video/backlight/gpio_backlight.c diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 037e1c111e48..e99d7a9dc670 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -14,22 +14,37 @@ #include <linux/platform_device.h> #include <linux/property.h> #include <linux/slab.h> +#include <linux/bitmap.h> struct gpio_backlight { struct device *dev; - struct gpio_desc **gpiods; + struct gpio_descs *gpiods; unsigned int num_gpios; }; static int gpio_backlight_update_status(struct backlight_device *bl) { struct gpio_backlight *gbl = bl_get_data(bl); - unsigned int i; + unsigned int n = gbl->num_gpios; int br = backlight_get_brightness(bl); + unsigned long *value_bitmap; + int words = BITS_TO_LONGS(n); + + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL); + if (!value_bitmap) + return -ENOMEM; + + if (br) + bitmap_fill(value_bitmap, n); + else + bitmap_zero(value_bitmap, n); - for (i = 0; i < gbl->num_gpios; i++) - gpiod_set_value_cansleep(gbl->gpiods[i], br); + gpiod_set_array_value_cansleep(gbl->gpiods->ndescs, + gbl->gpiods->desc, + gbl->gpiods->info, + value_bitmap); + kfree(value_bitmap); return 0; } @@ -67,22 +82,18 @@ static int gpio_backlight_probe(struct platform_device *pdev) def_value = device_property_read_bool(dev, "default-on"); - gbl->num_gpios = gpiod_count(dev, NULL); + gbl->gpiods = devm_gpiod_get_array(dev, NULL, GPIOD_ASIS); + if (IS_ERR(gbl->gpiods)) { + if (PTR_ERR(gbl->gpiods) == -ENODEV) + return dev_err_probe(dev, -EINVAL, + "The gpios parameter is missing or invalid\n"); + return PTR_ERR(gbl->gpiods); + } + + gbl->num_gpios = gbl->gpiods->ndescs; if (gbl->num_gpios == 0) return dev_err_probe(dev, -EINVAL, "The gpios parameter is missing or invalid\n"); - gbl->gpiods = devm_kcalloc(dev, gbl->num_gpios, sizeof(*gbl->gpiods), - GFP_KERNEL); - if (!gbl->gpiods) - return -ENOMEM; - - for (i = 0; i < gbl->num_gpios; i++) { - gbl->gpiods[i] = - devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS); - if (IS_ERR(gbl->gpiods[i])) - return dev_err_probe(dev, PTR_ERR(gbl->gpiods[i]), - "Failed to get GPIO at index %u\n", i); - } memset(&props, 0, sizeof(props)); props.type = BACKLIGHT_RAW; @@ -101,14 +112,26 @@ static int gpio_backlight_probe(struct platform_device *pdev) : BACKLIGHT_POWER_OFF; } else { bool all_high = true; - - for (i = 0; i < gbl->num_gpios; i++) { - if (gpiod_get_value_cansleep(gbl->gpiods[i]) != 0) { - all_high = false; - break; - } + unsigned long *value_bitmap; + int words = BITS_TO_LONGS(gbl->num_gpios); + + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL); + if (!value_bitmap) + return -ENOMEM; + + ret = gpiod_get_array_value_cansleep(gbl->gpiods->ndescs, + gbl->gpiods->desc, + gbl->gpiods->info, + value_bitmap); + if (ret) { + kfree(value_bitmap); + return dev_err_probe(dev, ret, + "failed to read initial gpio values\n"); } + all_high = bitmap_full(value_bitmap, gbl->num_gpios); + + kfree(value_bitmap); bl->props.power = all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF; } @@ -118,7 +141,7 @@ static int gpio_backlight_probe(struct platform_device *pdev) init_brightness = backlight_get_brightness(bl); for (i = 0; i < gbl->num_gpios; i++) { - ret = gpiod_direction_output(gbl->gpiods[i], init_brightness); + ret = gpiod_direction_output(gbl->gpiods->desc[i], init_brightness); if (ret) return dev_err_probe(dev, ret, "failed to set gpio %u direction\n", Could you please share your thoughts on whether this approach aligns with your expectations? > >> }; >> >> static int gpio_backlight_update_status(struct backlight_device *bl) >> { >> struct gpio_backlight *gbl = bl_get_data(bl); >> + unsigned int i; >> + int br = backlight_get_brightness(bl); >> >> - gpiod_set_value_cansleep(gbl->gpiod, backlight_get_brightness(bl)); >> + for (i = 0; i < gbl->num_gpios; i++) >> + gpiod_set_value_cansleep(gbl->gpiods[i], br); >> >> return 0; >> } >> @@ -52,6 +56,7 @@ static int gpio_backlight_probe(struct platform_device *pdev) >> struct backlight_device *bl; >> struct gpio_backlight *gbl; >> int ret, init_brightness, def_value; >> + unsigned int i; >> >> gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL); >> if (gbl == NULL) >> @@ -62,10 +67,22 @@ static int gpio_backlight_probe(struct platform_device *pdev) >> >> def_value = device_property_read_bool(dev, "default-on"); >> >> - gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS); >> - if (IS_ERR(gbl->gpiod)) >> - return dev_err_probe(dev, PTR_ERR(gbl->gpiod), >> - "The gpios parameter is missing or invalid\n"); >> + gbl->num_gpios = gpiod_count(dev, NULL); >> + if (gbl->num_gpios == 0) >> + return dev_err_probe(dev, -EINVAL, >> + "The gpios parameter is missing or invalid\n"); >> + gbl->gpiods = devm_kcalloc(dev, gbl->num_gpios, sizeof(*gbl->gpiods), >> + GFP_KERNEL); >> + if (!gbl->gpiods) >> + return -ENOMEM; > > This is definitely easier if you simply use devm_get_array(). > > >> + >> + for (i = 0; i < gbl->num_gpios; i++) { >> + gbl->gpiods[i] = >> + devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS); >> + if (IS_ERR(gbl->gpiods[i])) >> + return dev_err_probe(dev, PTR_ERR(gbl->gpiods[i]), >> + "Failed to get GPIO at index %u\n", i); >> + } >> >> memset(&props, 0, sizeof(props)); >> props.type = BACKLIGHT_RAW; >> @@ -78,22 +95,34 @@ static int gpio_backlight_probe(struct platform_device *pdev) >> } >> >> /* Set the initial power state */ >> - if (!of_node || !of_node->phandle) >> + if (!of_node || !of_node->phandle) { >> /* Not booted with device tree or no phandle link to the node */ >> bl->props.power = def_value ? BACKLIGHT_POWER_ON >> - : BACKLIGHT_POWER_OFF; >> - else if (gpiod_get_value_cansleep(gbl->gpiod) == 0) >> - bl->props.power = BACKLIGHT_POWER_OFF; >> - else >> - bl->props.power = BACKLIGHT_POWER_ON; >> + : BACKLIGHT_POWER_OFF; >> + } else { >> + bool all_high = true; >> + >> + for (i = 0; i < gbl->num_gpios; i++) { >> + if (gpiod_get_value_cansleep(gbl->gpiods[i]) != 0) { > > Why is there a != here? > > >> + all_high = false; >> + break; >> + } >> + } >> + >> + bl->props.power = >> + all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF; >> + } >> >> bl->props.brightness = 1; >> >> init_brightness = backlight_get_brightness(bl); >> - ret = gpiod_direction_output(gbl->gpiod, init_brightness); >> - if (ret) { >> - dev_err(dev, "failed to set initial brightness\n"); >> - return ret; >> + >> + for (i = 0; i < gbl->num_gpios; i++) { >> + ret = gpiod_direction_output(gbl->gpiods[i], init_brightness); >> + if (ret) >> + return dev_err_probe(dev, ret, >> + "failed to set gpio %u direction\n", >> + i); >> } >> >> platform_set_drvdata(pdev, bl); > > > Daniel. ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control 2026-01-13 7:17 ` tessolveupstream @ 2026-01-14 16:03 ` Daniel Thompson 2026-01-20 4:52 ` tessolveupstream 0 siblings, 1 reply; 14+ messages in thread From: Daniel Thompson @ 2026-01-14 16:03 UTC (permalink / raw) To: tessolveupstream Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On Tue, Jan 13, 2026 at 12:47:26PM +0530, tessolveupstream@gmail.com wrote: > > > On 05-01-2026 15:39, Daniel Thompson wrote: > > On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote: > >> Extend the gpio-backlight driver to handle multiple GPIOs instead of a > >> single one. This allows panels that require driving several enable pins > >> to be controlled by the backlight framework. > >> > >> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> > >> --- > >> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++------- > >> 1 file changed, 45 insertions(+), 16 deletions(-) > >> > >> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c > >> index 728a546904b0..037e1c111e48 100644 > >> --- a/drivers/video/backlight/gpio_backlight.c > >> +++ b/drivers/video/backlight/gpio_backlight.c > >> @@ -17,14 +17,18 @@ > >> > >> struct gpio_backlight { > >> struct device *dev; > >> - struct gpio_desc *gpiod; > >> + struct gpio_desc **gpiods; > >> + unsigned int num_gpios; > > > > Why not use struct gpio_descs for this? > > > > Once you do that, then most of the gbl->num_gpios loops can be replaced with > > calls to the array based accessors. > > > > Based on your feedback, I have updated the implementation to use > struct gpio_descs and array-based accessors, as recommended like > below: > > git diff drivers/video/backlight/gpio_backlight.c > diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c > index 037e1c111e48..e99d7a9dc670 100644 > --- a/drivers/video/backlight/gpio_backlight.c > +++ b/drivers/video/backlight/gpio_backlight.c > @@ -14,22 +14,37 @@ > #include <linux/platform_device.h> > #include <linux/property.h> > #include <linux/slab.h> > +#include <linux/bitmap.h> > > struct gpio_backlight { > struct device *dev; > - struct gpio_desc **gpiods; > + struct gpio_descs *gpiods; > unsigned int num_gpios; > }; > > static int gpio_backlight_update_status(struct backlight_device *bl) > { > struct gpio_backlight *gbl = bl_get_data(bl); > - unsigned int i; > + unsigned int n = gbl->num_gpios; > int br = backlight_get_brightness(bl); > + unsigned long *value_bitmap; > + int words = BITS_TO_LONGS(n); > + > + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL); Not sure you need a kcalloc() here. If you want to support more than 32 GPIOs then you can pre-allocate space with a devm_kcalloc() in the probe method rather than reallocate every time it is used. To be honest I don't really mind putting a hard limit on the maximum gpl->num_gpios (so you can just use a local variable) and having no allocation at all. > Could you please share your thoughts on whether this approach > aligns with your expectations? Looks like it is going in the right direction, yes. Daniel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control 2026-01-14 16:03 ` Daniel Thompson @ 2026-01-20 4:52 ` tessolveupstream 2026-01-20 9:38 ` Daniel Thompson 0 siblings, 1 reply; 14+ messages in thread From: tessolveupstream @ 2026-01-20 4:52 UTC (permalink / raw) To: Daniel Thompson Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On 14-01-2026 21:33, Daniel Thompson wrote: > On Tue, Jan 13, 2026 at 12:47:26PM +0530, tessolveupstream@gmail.com wrote: >> >> >> On 05-01-2026 15:39, Daniel Thompson wrote: >>> On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote: >>>> Extend the gpio-backlight driver to handle multiple GPIOs instead of a >>>> single one. This allows panels that require driving several enable pins >>>> to be controlled by the backlight framework. >>>> >>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> >>>> --- >>>> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++------- >>>> 1 file changed, 45 insertions(+), 16 deletions(-) >>>> >>>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c >>>> index 728a546904b0..037e1c111e48 100644 >>>> --- a/drivers/video/backlight/gpio_backlight.c >>>> +++ b/drivers/video/backlight/gpio_backlight.c >>>> @@ -17,14 +17,18 @@ >>>> >>>> struct gpio_backlight { >>>> struct device *dev; >>>> - struct gpio_desc *gpiod; >>>> + struct gpio_desc **gpiods; >>>> + unsigned int num_gpios; >>> >>> Why not use struct gpio_descs for this? >>> >>> Once you do that, then most of the gbl->num_gpios loops can be replaced with >>> calls to the array based accessors. >>> >> >> Based on your feedback, I have updated the implementation to use >> struct gpio_descs and array-based accessors, as recommended like >> below: >> >> git diff drivers/video/backlight/gpio_backlight.c >> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c >> index 037e1c111e48..e99d7a9dc670 100644 >> --- a/drivers/video/backlight/gpio_backlight.c >> +++ b/drivers/video/backlight/gpio_backlight.c >> @@ -14,22 +14,37 @@ >> #include <linux/platform_device.h> >> #include <linux/property.h> >> #include <linux/slab.h> >> +#include <linux/bitmap.h> >> >> struct gpio_backlight { >> struct device *dev; >> - struct gpio_desc **gpiods; >> + struct gpio_descs *gpiods; >> unsigned int num_gpios; >> }; >> >> static int gpio_backlight_update_status(struct backlight_device *bl) >> { >> struct gpio_backlight *gbl = bl_get_data(bl); >> - unsigned int i; >> + unsigned int n = gbl->num_gpios; >> int br = backlight_get_brightness(bl); >> + unsigned long *value_bitmap; >> + int words = BITS_TO_LONGS(n); >> + >> + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL); > > Not sure you need a kcalloc() here. If you want to support more than 32 > GPIOs then you can pre-allocate space with a devm_kcalloc() in the probe > method rather than reallocate every time it is used. > > To be honest I don't really mind putting a hard limit on the maximum > gpl->num_gpios (so you can just use a local variable) and having no > allocation at all. > Thanks for the suggestion. I addressed the kcalloc() concern by moving the bitmap allocation to probe using devm_kcalloc() as below: diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 0eb42d8bf1d9..7af5dc4f0315 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -19,32 +19,25 @@ struct gpio_backlight { struct device *dev; struct gpio_descs *gpiods; - unsigned int num_gpios; + unsigned long *bitmap; }; static int gpio_backlight_update_status(struct backlight_device *bl) { struct gpio_backlight *gbl = bl_get_data(bl); - unsigned int n = gbl->num_gpios; + unsigned int n = gbl->gpiods->ndescs; int br = backlight_get_brightness(bl); - unsigned long *value_bitmap; - int words = BITS_TO_LONGS(n); - - value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL); - if (!value_bitmap) - return -ENOMEM; if (br) - bitmap_fill(value_bitmap, n); + bitmap_fill(gbl->bitmap, n); else - bitmap_zero(value_bitmap, n); + bitmap_zero(gbl->bitmap, n); - gpiod_set_array_value_cansleep(gbl->gpiods->ndescs, + gpiod_set_array_value_cansleep(n, gbl->gpiods->desc, gbl->gpiods->info, - value_bitmap); + gbl->bitmap); - kfree(value_bitmap); return 0; } @@ -67,22 +60,25 @@ static int gpio_backlight_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev); struct device_node *of_node = dev->of_node; - struct backlight_properties props; + struct backlight_properties props = { }; struct backlight_device *bl; struct gpio_backlight *gbl; - int ret, init_brightness, def_value; - unsigned int i; + bool def_value; + enum gpiod_flags flags; + unsigned int n; + int words; - gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL); - if (gbl == NULL) + gbl = devm_kcalloc(dev, 1, sizeof(*gbl), GFP_KERNEL); + if (!gbl) return -ENOMEM; if (pdata) gbl->dev = pdata->dev; def_value = device_property_read_bool(dev, "default-on"); - - gbl->gpiods = devm_gpiod_get_array(dev, NULL, GPIOD_ASIS); + flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW; + + gbl->gpiods = devm_gpiod_get_array(dev, NULL, flags); if (IS_ERR(gbl->gpiods)) { if (PTR_ERR(gbl->gpiods) == -ENODEV) return dev_err_probe(dev, -EINVAL, @@ -90,12 +86,17 @@ static int gpio_backlight_probe(struct platform_device *pdev) return PTR_ERR(gbl->gpiods); } - gbl->num_gpios = gbl->gpiods->ndescs; - if (gbl->num_gpios == 0) + n = gbl->gpiods->ndescs; + if (!n) return dev_err_probe(dev, -EINVAL, - "The gpios parameter is missing or invalid\n"); + "No GPIOs provided\n"); + + words = BITS_TO_LONGS(n); + gbl->bitmap = devm_kcalloc(dev, words, sizeof(unsigned long), + GFP_KERNEL); + if (!gbl->bitmap) + return -ENOMEM; - memset(&props, 0, sizeof(props)); props.type = BACKLIGHT_RAW; props.max_brightness = 1; bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl, @@ -106,50 +107,19 @@ static int gpio_backlight_probe(struct platform_device *pdev) } /* Set the initial power state */ - if (!of_node || !of_node->phandle) { + if (!of_node || !of_node->phandle) /* Not booted with device tree or no phandle link to the node */ bl->props.power = def_value ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF; - } else { - bool all_high = true; - unsigned long *value_bitmap; - int words = BITS_TO_LONGS(gbl->num_gpios); - - value_bitmap = kcalloc(words, sizeof(unsigned long), - GFP_KERNEL); - if (!value_bitmap) - return -ENOMEM; - - ret = gpiod_get_array_value_cansleep(gbl->gpiods->ndescs, - gbl->gpiods->desc, - gbl->gpiods->info, - value_bitmap); - if (ret) { - kfree(value_bitmap); - return dev_err_probe(dev, ret, - "failed to read initial gpio values\n"); - } - - all_high = bitmap_full(value_bitmap, gbl->num_gpios); - - kfree(value_bitmap); - bl->props.power = - all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF; - } - - bl->props.brightness = 1; - - init_brightness = backlight_get_brightness(bl); + else if (gpiod_get_value_cansleep(gbl->gpiods->desc[0]) == 0) + bl->props.power = BACKLIGHT_POWER_OFF; + else + bl->props.power = BACKLIGHT_POWER_ON; - for (i = 0; i < gbl->num_gpios; i++) { - ret = gpiod_direction_output(gbl->gpiods->desc[i], - init_brightness); - if (ret) - return dev_err_probe(dev, ret, - "failed to set gpio %u direction\n", - i); - } + bl->props.brightness = def_value ? 1 : 0; + gpio_backlight_update_status(bl); + platform_set_drvdata(pdev, bl); return 0; } Kindly confirm whether this approach aligns with your expectations. > >> Could you please share your thoughts on whether this approach >> aligns with your expectations? > > Looks like it is going in the right direction, yes. > > > Daniel. ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control 2026-01-20 4:52 ` tessolveupstream @ 2026-01-20 9:38 ` Daniel Thompson 2026-01-20 12:53 ` tessolveupstream 0 siblings, 1 reply; 14+ messages in thread From: Daniel Thompson @ 2026-01-20 9:38 UTC (permalink / raw) To: tessolveupstream Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On Tue, Jan 20, 2026 at 10:22:02AM +0530, tessolveupstream@gmail.com wrote: > > > On 14-01-2026 21:33, Daniel Thompson wrote: > > On Tue, Jan 13, 2026 at 12:47:26PM +0530, tessolveupstream@gmail.com wrote: > >> > >> > >> On 05-01-2026 15:39, Daniel Thompson wrote: > >>> On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote: > >>>> Extend the gpio-backlight driver to handle multiple GPIOs instead of a > >>>> single one. This allows panels that require driving several enable pins > >>>> to be controlled by the backlight framework. > >>>> > >>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> > >>>> --- > >>>> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++------- > >>>> 1 file changed, 45 insertions(+), 16 deletions(-) > >>>> > >>>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c > >>>> index 728a546904b0..037e1c111e48 100644 > >>>> --- a/drivers/video/backlight/gpio_backlight.c > >>>> +++ b/drivers/video/backlight/gpio_backlight.c > >>>> @@ -17,14 +17,18 @@ > >>>> > >>>> struct gpio_backlight { > >>>> struct device *dev; > >>>> - struct gpio_desc *gpiod; > >>>> + struct gpio_desc **gpiods; > >>>> + unsigned int num_gpios; > >>> > >>> Why not use struct gpio_descs for this? > >>> > >>> Once you do that, then most of the gbl->num_gpios loops can be replaced with > >>> calls to the array based accessors. > >>> > >> > >> Based on your feedback, I have updated the implementation to use > >> struct gpio_descs and array-based accessors, as recommended like > >> below: > >> > >> git diff drivers/video/backlight/gpio_backlight.c > >> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c > >> index 037e1c111e48..e99d7a9dc670 100644 > >> --- a/drivers/video/backlight/gpio_backlight.c > >> +++ b/drivers/video/backlight/gpio_backlight.c > >> @@ -14,22 +14,37 @@ > >> #include <linux/platform_device.h> > >> #include <linux/property.h> > >> #include <linux/slab.h> > >> +#include <linux/bitmap.h> > >> > >> struct gpio_backlight { > >> struct device *dev; > >> - struct gpio_desc **gpiods; > >> + struct gpio_descs *gpiods; > >> unsigned int num_gpios; > >> }; > >> > >> static int gpio_backlight_update_status(struct backlight_device *bl) > >> { > >> struct gpio_backlight *gbl = bl_get_data(bl); > >> - unsigned int i; > >> + unsigned int n = gbl->num_gpios; > >> int br = backlight_get_brightness(bl); > >> + unsigned long *value_bitmap; > >> + int words = BITS_TO_LONGS(n); > >> + > >> + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL); > > > > Not sure you need a kcalloc() here. If you want to support more than 32 > > GPIOs then you can pre-allocate space with a devm_kcalloc() in the probe > > method rather than reallocate every time it is used. > > > > To be honest I don't really mind putting a hard limit on the maximum > > gpl->num_gpios (so you can just use a local variable) and having no > > allocation at all. > > > > Thanks for the suggestion. I addressed the kcalloc() concern by > moving the bitmap allocation to probe using devm_kcalloc() as > below: > > diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c > index 0eb42d8bf1d9..7af5dc4f0315 100644 > --- a/drivers/video/backlight/gpio_backlight.c > +++ b/drivers/video/backlight/gpio_backlight.c > @@ -19,32 +19,25 @@ > struct gpio_backlight { > struct device *dev; > struct gpio_descs *gpiods; > - unsigned int num_gpios; > + unsigned long *bitmap; > }; > > static int gpio_backlight_update_status(struct backlight_device *bl) > { > struct gpio_backlight *gbl = bl_get_data(bl); > - unsigned int n = gbl->num_gpios; > + unsigned int n = gbl->gpiods->ndescs; > int br = backlight_get_brightness(bl); > - unsigned long *value_bitmap; > - int words = BITS_TO_LONGS(n); > - > - value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL); > - if (!value_bitmap) > - return -ENOMEM; > > if (br) > - bitmap_fill(value_bitmap, n); > + bitmap_fill(gbl->bitmap, n); > else > - bitmap_zero(value_bitmap, n); > + bitmap_zero(gbl->bitmap, n); > > - gpiod_set_array_value_cansleep(gbl->gpiods->ndescs, > + gpiod_set_array_value_cansleep(n, > gbl->gpiods->desc, > gbl->gpiods->info, > - value_bitmap); > + gbl->bitmap); > > - kfree(value_bitmap); > return 0; > } > > @@ -67,22 +60,25 @@ static int gpio_backlight_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev); > struct device_node *of_node = dev->of_node; > - struct backlight_properties props; > + struct backlight_properties props = { }; > struct backlight_device *bl; > struct gpio_backlight *gbl; > - int ret, init_brightness, def_value; > - unsigned int i; > + bool def_value; > + enum gpiod_flags flags; > + unsigned int n; > + int words; > > - gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL); > - if (gbl == NULL) > + gbl = devm_kcalloc(dev, 1, sizeof(*gbl), GFP_KERNEL); > + if (!gbl) > return -ENOMEM; > > if (pdata) > gbl->dev = pdata->dev; > > def_value = device_property_read_bool(dev, "default-on"); > - > - gbl->gpiods = devm_gpiod_get_array(dev, NULL, GPIOD_ASIS); > + flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW; > + > + gbl->gpiods = devm_gpiod_get_array(dev, NULL, flags); > if (IS_ERR(gbl->gpiods)) { > if (PTR_ERR(gbl->gpiods) == -ENODEV) > return dev_err_probe(dev, -EINVAL, > @@ -90,12 +86,17 @@ static int gpio_backlight_probe(struct platform_device *pdev) > return PTR_ERR(gbl->gpiods); > } > > - gbl->num_gpios = gbl->gpiods->ndescs; > - if (gbl->num_gpios == 0) > + n = gbl->gpiods->ndescs; > + if (!n) > return dev_err_probe(dev, -EINVAL, > - "The gpios parameter is missing or invalid\n"); > + "No GPIOs provided\n"); > + > + words = BITS_TO_LONGS(n); > + gbl->bitmap = devm_kcalloc(dev, words, sizeof(unsigned long), > + GFP_KERNEL); > + if (!gbl->bitmap) > + return -ENOMEM; > > - memset(&props, 0, sizeof(props)); > props.type = BACKLIGHT_RAW; > props.max_brightness = 1; > bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl, > @@ -106,50 +107,19 @@ static int gpio_backlight_probe(struct platform_device *pdev) > } > > /* Set the initial power state */ > - if (!of_node || !of_node->phandle) { > + if (!of_node || !of_node->phandle) > /* Not booted with device tree or no phandle link to the node */ > bl->props.power = def_value ? BACKLIGHT_POWER_ON > : BACKLIGHT_POWER_OFF; > - } else { > - bool all_high = true; > - unsigned long *value_bitmap; > - int words = BITS_TO_LONGS(gbl->num_gpios); > - > - value_bitmap = kcalloc(words, sizeof(unsigned long), > - GFP_KERNEL); > - if (!value_bitmap) > - return -ENOMEM; > - > - ret = gpiod_get_array_value_cansleep(gbl->gpiods->ndescs, > - gbl->gpiods->desc, > - gbl->gpiods->info, > - value_bitmap); > - if (ret) { > - kfree(value_bitmap); > - return dev_err_probe(dev, ret, > - "failed to read initial gpio values\n"); > - } > - > - all_high = bitmap_full(value_bitmap, gbl->num_gpios); > - > - kfree(value_bitmap); > - bl->props.power = > - all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF; > - } > - > - bl->props.brightness = 1; > - > - init_brightness = backlight_get_brightness(bl); > + else if (gpiod_get_value_cansleep(gbl->gpiods->desc[0]) == 0) > + bl->props.power = BACKLIGHT_POWER_OFF; > + else > + bl->props.power = BACKLIGHT_POWER_ON; > > - for (i = 0; i < gbl->num_gpios; i++) { > - ret = gpiod_direction_output(gbl->gpiods->desc[i], > - init_brightness); > - if (ret) > - return dev_err_probe(dev, ret, > - "failed to set gpio %u direction\n", > - i); > - } > + bl->props.brightness = def_value ? 1 : 0; > > + gpio_backlight_update_status(bl); > + > platform_set_drvdata(pdev, bl); > return 0; > } > > Kindly confirm whether this approach aligns with your > expectations. As mentioned yesterday, I'd rather just review a v2 patch than this kind of meta-patch. Please send a v2 patch instead. Daniel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control 2026-01-20 9:38 ` Daniel Thompson @ 2026-01-20 12:53 ` tessolveupstream 0 siblings, 0 replies; 14+ messages in thread From: tessolveupstream @ 2026-01-20 12:53 UTC (permalink / raw) To: Daniel Thompson Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel On 20-01-2026 15:08, Daniel Thompson wrote: > On Tue, Jan 20, 2026 at 10:22:02AM +0530, tessolveupstream@gmail.com wrote: >> >> >> On 14-01-2026 21:33, Daniel Thompson wrote: >>> On Tue, Jan 13, 2026 at 12:47:26PM +0530, tessolveupstream@gmail.com wrote: >>>> >>>> >>>> On 05-01-2026 15:39, Daniel Thompson wrote: >>>>> On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote: >>>>>> Extend the gpio-backlight driver to handle multiple GPIOs instead of a >>>>>> single one. This allows panels that require driving several enable pins >>>>>> to be controlled by the backlight framework. >>>>>> >>>>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com> >>>>>> --- >>>>>> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++------- >>>>>> 1 file changed, 45 insertions(+), 16 deletions(-) >>>>>> >>>>>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c >>>>>> index 728a546904b0..037e1c111e48 100644 >>>>>> --- a/drivers/video/backlight/gpio_backlight.c >>>>>> +++ b/drivers/video/backlight/gpio_backlight.c >>>>>> @@ -17,14 +17,18 @@ >>>>>> >>>>>> struct gpio_backlight { >>>>>> struct device *dev; >>>>>> - struct gpio_desc *gpiod; >>>>>> + struct gpio_desc **gpiods; >>>>>> + unsigned int num_gpios; >>>>> >>>>> Why not use struct gpio_descs for this? >>>>> >>>>> Once you do that, then most of the gbl->num_gpios loops can be replaced with >>>>> calls to the array based accessors. >>>>> >>>> >>>> Based on your feedback, I have updated the implementation to use >>>> struct gpio_descs and array-based accessors, as recommended like >>>> below: >>>> >>>> git diff drivers/video/backlight/gpio_backlight.c >>>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c >>>> index 037e1c111e48..e99d7a9dc670 100644 >>>> --- a/drivers/video/backlight/gpio_backlight.c >>>> +++ b/drivers/video/backlight/gpio_backlight.c >>>> @@ -14,22 +14,37 @@ >>>> #include <linux/platform_device.h> >>>> #include <linux/property.h> >>>> #include <linux/slab.h> >>>> +#include <linux/bitmap.h> >>>> >>>> struct gpio_backlight { >>>> struct device *dev; >>>> - struct gpio_desc **gpiods; >>>> + struct gpio_descs *gpiods; >>>> unsigned int num_gpios; >>>> }; >>>> >>>> static int gpio_backlight_update_status(struct backlight_device *bl) >>>> { >>>> struct gpio_backlight *gbl = bl_get_data(bl); >>>> - unsigned int i; >>>> + unsigned int n = gbl->num_gpios; >>>> int br = backlight_get_brightness(bl); >>>> + unsigned long *value_bitmap; >>>> + int words = BITS_TO_LONGS(n); >>>> + >>>> + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL); >>> >>> Not sure you need a kcalloc() here. If you want to support more than 32 >>> GPIOs then you can pre-allocate space with a devm_kcalloc() in the probe >>> method rather than reallocate every time it is used. >>> >>> To be honest I don't really mind putting a hard limit on the maximum >>> gpl->num_gpios (so you can just use a local variable) and having no >>> allocation at all. >>> >> >> Thanks for the suggestion. I addressed the kcalloc() concern by >> moving the bitmap allocation to probe using devm_kcalloc() as >> below: >> >> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c >> index 0eb42d8bf1d9..7af5dc4f0315 100644 >> --- a/drivers/video/backlight/gpio_backlight.c >> +++ b/drivers/video/backlight/gpio_backlight.c >> @@ -19,32 +19,25 @@ >> struct gpio_backlight { >> struct device *dev; >> struct gpio_descs *gpiods; >> - unsigned int num_gpios; >> + unsigned long *bitmap; >> }; >> >> static int gpio_backlight_update_status(struct backlight_device *bl) >> { >> struct gpio_backlight *gbl = bl_get_data(bl); >> - unsigned int n = gbl->num_gpios; >> + unsigned int n = gbl->gpiods->ndescs; >> int br = backlight_get_brightness(bl); >> - unsigned long *value_bitmap; >> - int words = BITS_TO_LONGS(n); >> - >> - value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL); >> - if (!value_bitmap) >> - return -ENOMEM; >> >> if (br) >> - bitmap_fill(value_bitmap, n); >> + bitmap_fill(gbl->bitmap, n); >> else >> - bitmap_zero(value_bitmap, n); >> + bitmap_zero(gbl->bitmap, n); >> >> - gpiod_set_array_value_cansleep(gbl->gpiods->ndescs, >> + gpiod_set_array_value_cansleep(n, >> gbl->gpiods->desc, >> gbl->gpiods->info, >> - value_bitmap); >> + gbl->bitmap); >> >> - kfree(value_bitmap); >> return 0; >> } >> >> @@ -67,22 +60,25 @@ static int gpio_backlight_probe(struct platform_device *pdev) >> struct device *dev = &pdev->dev; >> struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev); >> struct device_node *of_node = dev->of_node; >> - struct backlight_properties props; >> + struct backlight_properties props = { }; >> struct backlight_device *bl; >> struct gpio_backlight *gbl; >> - int ret, init_brightness, def_value; >> - unsigned int i; >> + bool def_value; >> + enum gpiod_flags flags; >> + unsigned int n; >> + int words; >> >> - gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL); >> - if (gbl == NULL) >> + gbl = devm_kcalloc(dev, 1, sizeof(*gbl), GFP_KERNEL); >> + if (!gbl) >> return -ENOMEM; >> >> if (pdata) >> gbl->dev = pdata->dev; >> >> def_value = device_property_read_bool(dev, "default-on"); >> - >> - gbl->gpiods = devm_gpiod_get_array(dev, NULL, GPIOD_ASIS); >> + flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW; >> + >> + gbl->gpiods = devm_gpiod_get_array(dev, NULL, flags); >> if (IS_ERR(gbl->gpiods)) { >> if (PTR_ERR(gbl->gpiods) == -ENODEV) >> return dev_err_probe(dev, -EINVAL, >> @@ -90,12 +86,17 @@ static int gpio_backlight_probe(struct platform_device *pdev) >> return PTR_ERR(gbl->gpiods); >> } >> >> - gbl->num_gpios = gbl->gpiods->ndescs; >> - if (gbl->num_gpios == 0) >> + n = gbl->gpiods->ndescs; >> + if (!n) >> return dev_err_probe(dev, -EINVAL, >> - "The gpios parameter is missing or invalid\n"); >> + "No GPIOs provided\n"); >> + >> + words = BITS_TO_LONGS(n); >> + gbl->bitmap = devm_kcalloc(dev, words, sizeof(unsigned long), >> + GFP_KERNEL); >> + if (!gbl->bitmap) >> + return -ENOMEM; >> >> - memset(&props, 0, sizeof(props)); >> props.type = BACKLIGHT_RAW; >> props.max_brightness = 1; >> bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl, >> @@ -106,50 +107,19 @@ static int gpio_backlight_probe(struct platform_device *pdev) >> } >> >> /* Set the initial power state */ >> - if (!of_node || !of_node->phandle) { >> + if (!of_node || !of_node->phandle) >> /* Not booted with device tree or no phandle link to the node */ >> bl->props.power = def_value ? BACKLIGHT_POWER_ON >> : BACKLIGHT_POWER_OFF; >> - } else { >> - bool all_high = true; >> - unsigned long *value_bitmap; >> - int words = BITS_TO_LONGS(gbl->num_gpios); >> - >> - value_bitmap = kcalloc(words, sizeof(unsigned long), >> - GFP_KERNEL); >> - if (!value_bitmap) >> - return -ENOMEM; >> - >> - ret = gpiod_get_array_value_cansleep(gbl->gpiods->ndescs, >> - gbl->gpiods->desc, >> - gbl->gpiods->info, >> - value_bitmap); >> - if (ret) { >> - kfree(value_bitmap); >> - return dev_err_probe(dev, ret, >> - "failed to read initial gpio values\n"); >> - } >> - >> - all_high = bitmap_full(value_bitmap, gbl->num_gpios); >> - >> - kfree(value_bitmap); >> - bl->props.power = >> - all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF; >> - } >> - >> - bl->props.brightness = 1; >> - >> - init_brightness = backlight_get_brightness(bl); >> + else if (gpiod_get_value_cansleep(gbl->gpiods->desc[0]) == 0) >> + bl->props.power = BACKLIGHT_POWER_OFF; >> + else >> + bl->props.power = BACKLIGHT_POWER_ON; >> >> - for (i = 0; i < gbl->num_gpios; i++) { >> - ret = gpiod_direction_output(gbl->gpiods->desc[i], >> - init_brightness); >> - if (ret) >> - return dev_err_probe(dev, ret, >> - "failed to set gpio %u direction\n", >> - i); >> - } >> + bl->props.brightness = def_value ? 1 : 0; >> >> + gpio_backlight_update_status(bl); >> + >> platform_set_drvdata(pdev, bl); >> return 0; >> } >> >> Kindly confirm whether this approach aligns with your >> expectations. > > As mentioned yesterday, I'd rather just review a v2 patch than this kind of > meta-patch. Please send a v2 patch instead. > Got it, will send v2 patch. > > Daniel. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-01-20 12:53 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-05 8:51 [PATCH v1 0/2] backlight: gpio-backlight: Add support for multiple GPIOs Sudarshan Shetty 2026-01-05 8:51 ` [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow " Sudarshan Shetty 2026-01-05 9:55 ` Daniel Thompson 2026-01-13 4:45 ` tessolveupstream 2026-01-14 15:53 ` Daniel Thompson 2026-01-18 16:48 ` tessolveupstream 2026-01-19 14:42 ` Daniel Thompson 2026-01-05 8:51 ` [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty 2026-01-05 10:09 ` Daniel Thompson 2026-01-13 7:17 ` tessolveupstream 2026-01-14 16:03 ` Daniel Thompson 2026-01-20 4:52 ` tessolveupstream 2026-01-20 9:38 ` Daniel Thompson 2026-01-20 12:53 ` tessolveupstream
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox