Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control

On 20-01-2026 15:08, Daniel Thompson wrote:

On Tue, Jan 20, 2026 at 10:22:02AM +0530, tessolveupstream@gmail.com wrote:

quoted

On 14-01-2026 21:33, Daniel Thompson wrote:

quoted

On Tue, Jan 13, 2026 at 12:47:26PM +0530, tessolveupstream@gmail.com wrote:

quoted

On 05-01-2026 15:39, Daniel Thompson wrote:

quoted

On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote:

quoted

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 <redacted>
---
 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.