public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] backlight: gpio: add support for multiple GPIOs for backlight control
@ 2026-01-20 12:50 Sudarshan Shetty
  2026-01-20 12:50 ` [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs Sudarshan Shetty
  2026-01-20 12:50 ` [PATCH v2 2/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty
  0 siblings, 2 replies; 12+ messages in thread
From: Sudarshan Shetty @ 2026-01-20 12:50 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 one
   or more GPIOs for a gpio-backlight node.

This approach avoids describing multiple backlight devices in DT for a
single panel.

Changes in v2:
 - Used devm_gpiod_get_array() and struct gpio_descs
 - Replaced per-index GPIO handling with descriptor array access
 - Moved the bitmap allocation to probe using devm_kcalloc().
 - Updated commit messages.

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        | 24 ++++++-
 drivers/video/backlight/gpio_backlight.c      | 66 +++++++++++++------
 2 files changed, 67 insertions(+), 23 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
  2026-01-20 12:50 [PATCH v2 0/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty
@ 2026-01-20 12:50 ` Sudarshan Shetty
  2026-01-20 14:31   ` Krzysztof Kozlowski
  2026-01-20 12:50 ` [PATCH v2 2/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty
  1 sibling, 1 reply; 12+ messages in thread
From: Sudarshan Shetty @ 2026-01-20 12:50 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>
---
 .../leds/backlight/gpio-backlight.yaml        | 24 +++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
index 584030b6b0b9..4e4a856cbcd7 100644
--- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
@@ -16,8 +16,18 @@ properties:
     const: gpio-backlight
 
   gpios:
-    description: The gpio that is used for enabling/disabling the backlight.
-    maxItems: 1
+    description: |
+      The gpio that is used for enabling/disabling the backlight.
+      Multiple GPIOs can be specified for panels that require several
+      enable signals. All GPIOs are controlled together.
+    type: array
+    minItems: 1
+    items:
+      type: array
+      minItems: 3
+      maxItems: 3
+      items:
+        type: integer
 
   default-on:
     description: enable the backlight at boot.
@@ -38,4 +48,14 @@ examples:
         default-on;
     };
 
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    backlight {
+      compatible = "gpio-backlight";
+      gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>,
+              <&gpio3 5 GPIO_ACTIVE_HIGH>,
+              <&gpio3 6 GPIO_ACTIVE_HIGH>;
+      default-on;
+    };
+
 ...
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
  2026-01-20 12:50 [PATCH v2 0/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty
  2026-01-20 12:50 ` [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs Sudarshan Shetty
@ 2026-01-20 12:50 ` Sudarshan Shetty
  2026-01-28 10:57   ` Daniel Thompson
  1 sibling, 1 reply; 12+ messages in thread
From: Sudarshan Shetty @ 2026-01-20 12:50 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

The gpio-backlight driver currently supports only a single GPIO to
enable or disable a backlight device. Some panels require multiple
enable GPIOs to be asserted together.

Extend the driver to support an array of GPIOs for a single backlight
instance. All configured GPIOs are toggled together based on the
backlight state.

Existing single-GPIO Device Tree users remain unaffected.

Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
---
 drivers/video/backlight/gpio_backlight.c | 66 ++++++++++++++++--------
 1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 728a546904b0..11d21de82cf5 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -14,17 +14,29 @@
 #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 *gpiod;
+	struct gpio_descs *gpiods;
+	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->gpiods->ndescs;
+	int br = backlight_get_brightness(bl);
 
-	gpiod_set_value_cansleep(gbl->gpiod, backlight_get_brightness(bl));
+	if (br)
+		bitmap_fill(gbl->bitmap, n);
+	else
+		bitmap_zero(gbl->bitmap, n);
+
+	gpiod_set_array_value_cansleep(n,
+				       gbl->gpiods->desc,
+				       gbl->gpiods->info,
+				       gbl->bitmap);
 
 	return 0;
 }
@@ -48,26 +60,43 @@ 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;
+	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");
+	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,
+			"The gpios parameter is missing or invalid\n");
+		return PTR_ERR(gbl->gpiods);
+	}
 
-	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");
+	n = gbl->gpiods->ndescs;
+	if (!n)
+		return dev_err_probe(dev, -EINVAL,
+			"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,
@@ -81,21 +110,16 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 	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)
+						    : BACKLIGHT_POWER_OFF;
+	else if (gpiod_get_value_cansleep(gbl->gpiods->desc[0]) == 0)
 		bl->props.power = BACKLIGHT_POWER_OFF;
 	else
 		bl->props.power = BACKLIGHT_POWER_ON;
 
-	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;
-	}
+	bl->props.brightness = def_value ? 1 : 0;
 
+	gpio_backlight_update_status(bl);
+
 	platform_set_drvdata(pdev, bl);
 	return 0;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
  2026-01-20 12:50 ` [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs Sudarshan Shetty
@ 2026-01-20 14:31   ` Krzysztof Kozlowski
  2026-01-23 11:11     ` tessolveupstream
  0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-20 14:31 UTC (permalink / raw)
  To: Sudarshan Shetty, lee, danielt, jingoohan1
  Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
	linux-leds, devicetree, linux-kernel

On 20/01/2026 13:50, Sudarshan Shetty wrote:
> Update the gpio-backlight binding to support configurations that require
> more than one GPIO for enabling/disabling the backlight.


Why? Which devices need it? How a backlight would have three enable
GPIOs? I really do not believe, so you need to write proper hardware
justification.

> 
> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
> ---
>  .../leds/backlight/gpio-backlight.yaml        | 24 +++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> index 584030b6b0b9..4e4a856cbcd7 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> @@ -16,8 +16,18 @@ properties:
>      const: gpio-backlight
>  
>    gpios:
> -    description: The gpio that is used for enabling/disabling the backlight.
> -    maxItems: 1
> +    description: |
> +      The gpio that is used for enabling/disabling the backlight.
> +      Multiple GPIOs can be specified for panels that require several
> +      enable signals. All GPIOs are controlled together.
> +    type: array

There is no such syntax in the bindings, from where did you get it? Type
is already defined.

items:
  minItems: 1
  maxItems: 3


> +    minItems: 1
> +    items:
> +      type: array
> +      minItems: 3
> +      maxItems: 3
> +      items:
> +        type: integer

All this is some odd stuff - just to be clear, don't send us LLM output.
I don't want to waste my time to review microslop.

Was it done with help of Microslop?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
  2026-01-20 14:31   ` Krzysztof Kozlowski
@ 2026-01-23 11:11     ` tessolveupstream
  2026-01-27 12:46       ` tessolveupstream
  2026-01-28 10:11       ` Krzysztof Kozlowski
  0 siblings, 2 replies; 12+ messages in thread
From: tessolveupstream @ 2026-01-23 11:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski, lee, danielt, jingoohan1
  Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
	linux-leds, devicetree, linux-kernel



On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
> On 20/01/2026 13:50, Sudarshan Shetty wrote:
>> Update the gpio-backlight binding to support configurations that require
>> more than one GPIO for enabling/disabling the backlight.
> 
> 
> Why? Which devices need it? How a backlight would have three enable
> GPIOs? I really do not believe, so you need to write proper hardware
> justification.
>

To clarify our hardware setup: 
the panel requires one GPIO for the backlight enable signal, and it 
also has a PWM input. Since the QCS615 does not provide a PWM controller 
for this use case, the PWM input is connected to a GPIO that is driven 
high to provide a constant 100% duty cycle, as explained in the link 
below.
https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5
 
>>
>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>> ---
>>  .../leds/backlight/gpio-backlight.yaml        | 24 +++++++++++++++++--
>>  1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>> index 584030b6b0b9..4e4a856cbcd7 100644
>> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>> @@ -16,8 +16,18 @@ properties:
>>      const: gpio-backlight
>>  
>>    gpios:
>> -    description: The gpio that is used for enabling/disabling the backlight.
>> -    maxItems: 1
>> +    description: |
>> +      The gpio that is used for enabling/disabling the backlight.
>> +      Multiple GPIOs can be specified for panels that require several
>> +      enable signals. All GPIOs are controlled together.
>> +    type: array
> 
> There is no such syntax in the bindings, from where did you get it? Type
> is already defined.
> 
> items:
>   minItems: 1
>   maxItems: 3
> 
> 
>> +    minItems: 1
>> +    items:
>> +      type: array
>> +      minItems: 3
>> +      maxItems: 3
>> +      items:
>> +        type: integer
> 
> All this is some odd stuff - just to be clear, don't send us LLM output.
> I don't want to waste my time to review microslop.
> 
> Was it done with help of Microslop?
>

I understand now that the schema changes I proposed were not correct, 
and I will address this in the next patch series. My intention was to 
check whether the gpio-backlight binding could support more than one 
enable-type GPIO. 
Could you please advise what would be an appropriate maximum number of 
GPIOs for gpio-backlight in such a scenario? For example, would allowing 
2 GPIOs be acceptable, or should this case be handled in a different way?
 
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
  2026-01-23 11:11     ` tessolveupstream
@ 2026-01-27 12:46       ` tessolveupstream
  2026-01-28 10:14         ` Krzysztof Kozlowski
  2026-01-28 10:11       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 12+ messages in thread
From: tessolveupstream @ 2026-01-27 12:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski, lee, danielt, jingoohan1
  Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
	linux-leds, devicetree, linux-kernel



On 23-01-2026 16:41, tessolveupstream@gmail.com wrote:
> 
> 
> On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
>> On 20/01/2026 13:50, Sudarshan Shetty wrote:
>>> Update the gpio-backlight binding to support configurations that require
>>> more than one GPIO for enabling/disabling the backlight.
>>
>>
>> Why? Which devices need it? How a backlight would have three enable
>> GPIOs? I really do not believe, so you need to write proper hardware
>> justification.
>>
> 
> To clarify our hardware setup: 
> the panel requires one GPIO for the backlight enable signal, and it 
> also has a PWM input. Since the QCS615 does not provide a PWM controller 
> for this use case, the PWM input is connected to a GPIO that is driven 
> high to provide a constant 100% duty cycle, as explained in the link 
> below.
> https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5
>  
>>>
>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>>> ---
>>>  .../leds/backlight/gpio-backlight.yaml        | 24 +++++++++++++++++--
>>>  1 file changed, 22 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>> index 584030b6b0b9..4e4a856cbcd7 100644
>>> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>> @@ -16,8 +16,18 @@ properties:
>>>      const: gpio-backlight
>>>  
>>>    gpios:
>>> -    description: The gpio that is used for enabling/disabling the backlight.
>>> -    maxItems: 1
>>> +    description: |
>>> +      The gpio that is used for enabling/disabling the backlight.
>>> +      Multiple GPIOs can be specified for panels that require several
>>> +      enable signals. All GPIOs are controlled together.
>>> +    type: array
>>
>> There is no such syntax in the bindings, from where did you get it? Type
>> is already defined.
>>
>> items:
>>   minItems: 1
>>   maxItems: 3
>>
>>
>>> +    minItems: 1
>>> +    items:
>>> +      type: array
>>> +      minItems: 3
>>> +      maxItems: 3
>>> +      items:
>>> +        type: integer
>>
>> All this is some odd stuff - just to be clear, don't send us LLM output.
>> I don't want to waste my time to review microslop.
>>
>> Was it done with help of Microslop?
>>
> 
> I understand now that the schema changes I proposed were not correct, 
> and I will address this in the next patch series. My intention was to 
> check whether the gpio-backlight binding could support more than one 
> enable-type GPIO. 
> Could you please advise what would be an appropriate maximum number of 
> GPIOs for gpio-backlight in such a scenario? For example, would allowing 
> 2 GPIOs be acceptable, or should this case be handled in a different way?
> 

In line with Daniel’s suggestion, I am planning to adopt a fixed upper 
limit for the number of backlight GPIOs. The current hardware only 
requires two GPIOs, so the maxItems can be set to 2.

If future platforms or customers require support for a higher number 
of GPIOs, this limit can be increased and the driver can be 
updated accordingly.

Kindly advise if this solution aligns with your expectations, or if 
you prefer an alternative maximum value.
 
>> Best regards,
>> Krzysztof
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
  2026-01-23 11:11     ` tessolveupstream
  2026-01-27 12:46       ` tessolveupstream
@ 2026-01-28 10:11       ` Krzysztof Kozlowski
  2026-01-28 11:20         ` Daniel Thompson
  1 sibling, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-28 10:11 UTC (permalink / raw)
  To: tessolveupstream, lee, danielt, jingoohan1
  Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
	linux-leds, devicetree, linux-kernel

On 23/01/2026 12:11, tessolveupstream@gmail.com wrote:
> 
> 
> On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
>> On 20/01/2026 13:50, Sudarshan Shetty wrote:
>>> Update the gpio-backlight binding to support configurations that require
>>> more than one GPIO for enabling/disabling the backlight.
>>
>>
>> Why? Which devices need it? How a backlight would have three enable
>> GPIOs? I really do not believe, so you need to write proper hardware
>> justification.
>>
> 
> To clarify our hardware setup: 
> the panel requires one GPIO for the backlight enable signal, and it 
> also has a PWM input. Since the QCS615 does not provide a PWM controller 
> for this use case, the PWM input is connected to a GPIO that is driven 
> high to provide a constant 100% duty cycle, as explained in the link 
> below.
> https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5

That's not an enable gpio, but PWM.

You write bindings for this device, not for something else - like your
board.


>  
>>>
>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>>> ---
>>>  .../leds/backlight/gpio-backlight.yaml        | 24 +++++++++++++++++--
>>>  1 file changed, 22 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>> index 584030b6b0b9..4e4a856cbcd7 100644
>>> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>> @@ -16,8 +16,18 @@ properties:
>>>      const: gpio-backlight
>>>  
>>>    gpios:
>>> -    description: The gpio that is used for enabling/disabling the backlight.
>>> -    maxItems: 1
>>> +    description: |
>>> +      The gpio that is used for enabling/disabling the backlight.
>>> +      Multiple GPIOs can be specified for panels that require several
>>> +      enable signals. All GPIOs are controlled together.
>>> +    type: array
>>
>> There is no such syntax in the bindings, from where did you get it? Type
>> is already defined.
>>
>> items:
>>   minItems: 1
>>   maxItems: 3
>>
>>
>>> +    minItems: 1
>>> +    items:
>>> +      type: array
>>> +      minItems: 3
>>> +      maxItems: 3
>>> +      items:
>>> +        type: integer
>>
>> All this is some odd stuff - just to be clear, don't send us LLM output.
>> I don't want to waste my time to review microslop.
>>
>> Was it done with help of Microslop?
>>
> 
> I understand now that the schema changes I proposed were not correct, 

How such code could be even created... Just in case, do you understand
that Microslop and LLM is waste of our time?

> and I will address this in the next patch series. My intention was to 
> check whether the gpio-backlight binding could support more than one 
> enable-type GPIO. 
> Could you please advise what would be an appropriate maximum number of 
> GPIOs for gpio-backlight in such a scenario? For example, would allowing 
> 2 GPIOs be acceptable, or should this case be handled in a different way?
We have plenty of examples for this, but anyway you won't need it
because this is not an enable GPIO.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
  2026-01-27 12:46       ` tessolveupstream
@ 2026-01-28 10:14         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-28 10:14 UTC (permalink / raw)
  To: tessolveupstream, lee, danielt, jingoohan1
  Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
	linux-leds, devicetree, linux-kernel

On 27/01/2026 13:46, tessolveupstream@gmail.com wrote:
> 
> 
> On 23-01-2026 16:41, tessolveupstream@gmail.com wrote:
>>
>>
>> On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
>>> On 20/01/2026 13:50, Sudarshan Shetty wrote:
>>>> Update the gpio-backlight binding to support configurations that require
>>>> more than one GPIO for enabling/disabling the backlight.
>>>
>>>
>>> Why? Which devices need it? How a backlight would have three enable
>>> GPIOs? I really do not believe, so you need to write proper hardware
>>> justification.
>>>
>>
>> To clarify our hardware setup: 
>> the panel requires one GPIO for the backlight enable signal, and it 
>> also has a PWM input. Since the QCS615 does not provide a PWM controller 
>> for this use case, the PWM input is connected to a GPIO that is driven 
>> high to provide a constant 100% duty cycle, as explained in the link 
>> below.
>> https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5
>>  
>>>>
>>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>>>> ---
>>>>  .../leds/backlight/gpio-backlight.yaml        | 24 +++++++++++++++++--
>>>>  1 file changed, 22 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>>> index 584030b6b0b9..4e4a856cbcd7 100644
>>>> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>>> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>>> @@ -16,8 +16,18 @@ properties:
>>>>      const: gpio-backlight
>>>>  
>>>>    gpios:
>>>> -    description: The gpio that is used for enabling/disabling the backlight.
>>>> -    maxItems: 1
>>>> +    description: |
>>>> +      The gpio that is used for enabling/disabling the backlight.
>>>> +      Multiple GPIOs can be specified for panels that require several
>>>> +      enable signals. All GPIOs are controlled together.
>>>> +    type: array
>>>
>>> There is no such syntax in the bindings, from where did you get it? Type
>>> is already defined.
>>>
>>> items:
>>>   minItems: 1
>>>   maxItems: 3
>>>
>>>
>>>> +    minItems: 1
>>>> +    items:
>>>> +      type: array
>>>> +      minItems: 3
>>>> +      maxItems: 3
>>>> +      items:
>>>> +        type: integer
>>>
>>> All this is some odd stuff - just to be clear, don't send us LLM output.
>>> I don't want to waste my time to review microslop.
>>>
>>> Was it done with help of Microslop?
>>>
>>
>> I understand now that the schema changes I proposed were not correct, 
>> and I will address this in the next patch series. My intention was to 
>> check whether the gpio-backlight binding could support more than one 
>> enable-type GPIO. 
>> Could you please advise what would be an appropriate maximum number of 
>> GPIOs for gpio-backlight in such a scenario? For example, would allowing 
>> 2 GPIOs be acceptable, or should this case be handled in a different way?
>>
> 
> In line with Daniel’s suggestion, I am planning to adopt a fixed upper 
> limit for the number of backlight GPIOs. The current hardware only 
> requires two GPIOs, so the maxItems can be set to 2.
> 
> If future platforms or customers require support for a higher number 
> of GPIOs, this limit can be increased and the driver can be 
> updated accordingly.
> 
> Kindly advise if this solution aligns with your expectations, or if 
> you prefer an alternative maximum value.


You have entire commit msg to explain the hardware and explain WHY you
are doing this. In a concise and readable way. I will not be going
through 2 different email threads with 20 messages to figure that out.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
  2026-01-20 12:50 ` [PATCH v2 2/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty
@ 2026-01-28 10:57   ` Daniel Thompson
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Thompson @ 2026-01-28 10:57 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 Tue, Jan 20, 2026 at 06:20:36PM +0530, Sudarshan Shetty wrote:
> The gpio-backlight driver currently supports only a single GPIO to
> enable or disable a backlight device. Some panels require multiple
> enable GPIOs to be asserted together.
>
> Extend the driver to support an array of GPIOs for a single backlight
> instance. All configured GPIOs are toggled together based on the
> backlight state.
>
> Existing single-GPIO Device Tree users remain unaffected.
>
> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
> ---
>  drivers/video/backlight/gpio_backlight.c | 66 ++++++++++++++++--------
>  1 file changed, 45 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> index 728a546904b0..11d21de82cf5 100644
> --- a/drivers/video/backlight/gpio_backlight.c
> +++ b/drivers/video/backlight/gpio_backlight.c
> @@ -14,17 +14,29 @@
>  #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 *gpiod;
> +	struct gpio_descs *gpiods;
> +	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->gpiods->ndescs;
> +	int br = backlight_get_brightness(bl);
>
> -	gpiod_set_value_cansleep(gbl->gpiod, backlight_get_brightness(bl));
> +	if (br)
> +		bitmap_fill(gbl->bitmap, n);
> +	else
> +		bitmap_zero(gbl->bitmap, n);
> +
> +	gpiod_set_array_value_cansleep(n,
> +				       gbl->gpiods->desc,
> +				       gbl->gpiods->info,
> +				       gbl->bitmap);
>
>  	return 0;
>  }
> @@ -48,26 +60,43 @@ 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 = { };

This change is unrelated to the patch description. Do not "hide"
changes like this. It you want to replace the memset() it's better to
send a separate patch.


>  	struct backlight_device *bl;
>  	struct gpio_backlight *gbl;
> -	int ret, init_brightness, def_value;
> +	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)

Again, this change is unrelated to the patch description. Do not include
changes that are not described in the patch description.


>  		return -ENOMEM;
>
>  	if (pdata)
>  		gbl->dev = pdata->dev;
>
>  	def_value = device_property_read_bool(dev, "default-on");
> +	flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
> +
> +	gbl->gpiods = devm_gpiod_get_array(dev, NULL, flags);

How is it safe to transition from GPIOD_ASIS to GPIOD_OUT_LOW
here?

Forcing the backlight off if the default-on property is not present
will prevent the backlight state being properly inherited from the
bootloader.


> +	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->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");
> +	n = gbl->gpiods->ndescs;
> +	if (!n)
> +		return dev_err_probe(dev, -EINVAL,
> +			"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,
> @@ -81,21 +110,16 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>  	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)
> +						    : BACKLIGHT_POWER_OFF;
> +	else if (gpiod_get_value_cansleep(gbl->gpiods->desc[0]) == 0)

This logic is broken. This code path needs to be taken is *any* GPIO is
low (and, as mentioned, the initial GPIO state must be GPIOD_ASIS).


>  		bl->props.power = BACKLIGHT_POWER_OFF;
>  	else
>  		bl->props.power = BACKLIGHT_POWER_ON;
>
> -	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;
> -	}
> +	bl->props.brightness = def_value ? 1 : 0;
>
> +	gpio_backlight_update_status(bl);
> +
>  	platform_set_drvdata(pdev, bl);
>  	return 0;
>  }


Daniel.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
  2026-01-28 10:11       ` Krzysztof Kozlowski
@ 2026-01-28 11:20         ` Daniel Thompson
  2026-01-29  5:41           ` tessolveupstream
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Thompson @ 2026-01-28 11:20 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: tessolveupstream, lee, danielt, jingoohan1, deller, pavel, robh,
	krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree,
	linux-kernel

On Wed, Jan 28, 2026 at 11:11:33AM +0100, Krzysztof Kozlowski wrote:
> On 23/01/2026 12:11, tessolveupstream@gmail.com wrote:
> >
> >
> > On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
> >> On 20/01/2026 13:50, Sudarshan Shetty wrote:
> >>> Update the gpio-backlight binding to support configurations that require
> >>> more than one GPIO for enabling/disabling the backlight.
> >>
> >>
> >> Why? Which devices need it? How a backlight would have three enable
> >> GPIOs? I really do not believe, so you need to write proper hardware
> >> justification.
> >>
> >
> > To clarify our hardware setup:
> > the panel requires one GPIO for the backlight enable signal, and it
> > also has a PWM input. Since the QCS615 does not provide a PWM controller
> > for this use case, the PWM input is connected to a GPIO that is driven
> > high to provide a constant 100% duty cycle, as explained in the link
> > below.
> > https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5
>
> That's not an enable gpio, but PWM.
>
> You write bindings for this device, not for something else - like your
> board.

Sudarshan: I believe at one point the intent was to model this hardware
as a pwm-backlight (using enables GPIOs to drive the enable pin)
attached to a pwm-gpio (to drive the PWM pin). Did this approach work?


Daniel.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
  2026-01-28 11:20         ` Daniel Thompson
@ 2026-01-29  5:41           ` tessolveupstream
  2026-02-02 10:28             ` Daniel Thompson
  0 siblings, 1 reply; 12+ messages in thread
From: tessolveupstream @ 2026-01-29  5:41 UTC (permalink / raw)
  To: Daniel Thompson, Krzysztof Kozlowski
  Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
	dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel



On 28-01-2026 16:50, Daniel Thompson wrote:
> On Wed, Jan 28, 2026 at 11:11:33AM +0100, Krzysztof Kozlowski wrote:
>> On 23/01/2026 12:11, tessolveupstream@gmail.com wrote:
>>>
>>>
>>> On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
>>>> On 20/01/2026 13:50, Sudarshan Shetty wrote:
>>>>> Update the gpio-backlight binding to support configurations that require
>>>>> more than one GPIO for enabling/disabling the backlight.
>>>>
>>>>
>>>> Why? Which devices need it? How a backlight would have three enable
>>>> GPIOs? I really do not believe, so you need to write proper hardware
>>>> justification.
>>>>
>>>
>>> To clarify our hardware setup:
>>> the panel requires one GPIO for the backlight enable signal, and it
>>> also has a PWM input. Since the QCS615 does not provide a PWM controller
>>> for this use case, the PWM input is connected to a GPIO that is driven
>>> high to provide a constant 100% duty cycle, as explained in the link
>>> below.
>>> https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5
>>
>> That's not an enable gpio, but PWM.
>>
>> You write bindings for this device, not for something else - like your
>> board.
> 
> Sudarshan: I believe at one point the intent was to model this hardware
> as a pwm-backlight (using enables GPIOs to drive the enable pin)
> attached to a pwm-gpio (to drive the PWM pin). Did this approach work?
> 

Yes, the original plan was to model this using pwm‑gpio, and that 
setup worked. But on the SOC there’s no actual PWM controller available 
for this path— the LED_PWM line is just tied to a GPIO that’s driven 
high (effectively a fixed 100% duty cycle). Because of that, describing 
it as a PWM in DT was flagged as incorrect.

As pointed out during the SoC DTS review, the correct path forward is 
to extend gpio‑backlight to handle multiple GPIOs, rather than 
representing them as multiple separate backlight devices.

> 
> Daniel.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
  2026-01-29  5:41           ` tessolveupstream
@ 2026-02-02 10:28             ` Daniel Thompson
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Thompson @ 2026-02-02 10:28 UTC (permalink / raw)
  To: tessolveupstream
  Cc: Krzysztof Kozlowski, lee, danielt, jingoohan1, deller, pavel,
	robh, krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds,
	devicetree, linux-kernel

On Thu, Jan 29, 2026 at 11:11:34AM +0530, tessolveupstream@gmail.com wrote:
> On 28-01-2026 16:50, Daniel Thompson wrote:
> > On Wed, Jan 28, 2026 at 11:11:33AM +0100, Krzysztof Kozlowski wrote:
> >> On 23/01/2026 12:11, tessolveupstream@gmail.com wrote:
> >>>
> >>>
> >>> On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
> >>>> On 20/01/2026 13:50, Sudarshan Shetty wrote:
> >>>>> Update the gpio-backlight binding to support configurations that require
> >>>>> more than one GPIO for enabling/disabling the backlight.
> >>>>
> >>>>
> >>>> Why? Which devices need it? How a backlight would have three enable
> >>>> GPIOs? I really do not believe, so you need to write proper hardware
> >>>> justification.
> >>>>
> >>>
> >>> To clarify our hardware setup:
> >>> the panel requires one GPIO for the backlight enable signal, and it
> >>> also has a PWM input. Since the QCS615 does not provide a PWM controller
> >>> for this use case, the PWM input is connected to a GPIO that is driven
> >>> high to provide a constant 100% duty cycle, as explained in the link
> >>> below.
> >>> https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5
> >>
> >> That's not an enable gpio, but PWM.
> >>
> >> You write bindings for this device, not for something else - like your
> >> board.
> >
> > Sudarshan: I believe at one point the intent was to model this hardware
> > as a pwm-backlight (using enables GPIOs to drive the enable pin)
> > attached to a pwm-gpio (to drive the PWM pin). Did this approach work?
> >
>
> Yes, the original plan was to model this using pwm‑gpio, and that
> setup worked. But on the SOC there’s no actual PWM controller available
> for this path— the LED_PWM line is just tied to a GPIO that’s driven
> high (effectively a fixed 100% duty cycle). Because of that, describing
> it as a PWM in DT was flagged as incorrect.
>
> As pointed out during the SoC DTS review, the correct path forward is
> to extend gpio‑backlight to handle multiple GPIOs, rather than
> representing them as multiple separate backlight devices.

That not quite what I got from the link above. There is a suggestion to
use gpio-backlight, but the reason it was flagged is because pwm-gpio
was unused... it was not referenced by a pwm-backlight.

Having said that I suspect it is better to model this backlight controller
on this board as a gpio-backlight because from a backlight controller
point of this that is physically what the controller is composed of
(assuming there is not sufficient capacitance on the signal for a
software PWM to work at anything other than 0% and 100%). Even if those
GPIO signals are connected to the panel's PWM input I'm not sure that's
relevant because none of the backlight controller bindings model the
panel anyway.

Whatever route you select, you do need to make it clear in the patch
description *why* it is correct to model the system as a gpio-backlight.
Deferring to (potentially ambiguous) review comments is not sufficient
to explain why changing the gpio-backlight bindings are an improvement.


Daniel.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-02-02 10:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-20 12:50 [PATCH v2 0/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty
2026-01-20 12:50 ` [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs Sudarshan Shetty
2026-01-20 14:31   ` Krzysztof Kozlowski
2026-01-23 11:11     ` tessolveupstream
2026-01-27 12:46       ` tessolveupstream
2026-01-28 10:14         ` Krzysztof Kozlowski
2026-01-28 10:11       ` Krzysztof Kozlowski
2026-01-28 11:20         ` Daniel Thompson
2026-01-29  5:41           ` tessolveupstream
2026-02-02 10:28             ` Daniel Thompson
2026-01-20 12:50 ` [PATCH v2 2/2] backlight: gpio: add support for multiple GPIOs for backlight control Sudarshan Shetty
2026-01-28 10:57   ` Daniel Thompson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox