On Tue, Jan 13, 2026 at 12:47:26PM +0530, [email protected] 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 <[email protected]> > >> --- > >> 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.
