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.1On 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.
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.
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.
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.
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.
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.
© 2016 - 2026 Red Hat, Inc.