* [PATCH] fbdev: sys_fillrect: Add bounds checking to prevent vmalloc-out-of-bounds
From: Osama Abdelkader @ 2026-01-18 0:18 UTC (permalink / raw)
To: Zsolt Kajtar, Simona Vetter, Helge Deller, Osama Abdelkader,
Thomas Zimmermann, linux-fbdev, dri-devel, linux-kernel
Cc: syzbot+7a63ce155648954e749b
The sys_fillrect function was missing bounds validation, which could lead
to vmalloc-out-of-bounds writes when the rectangle coordinates extend
beyond the framebuffer's virtual resolution. This was detected by KASAN
and reported by syzkaller.
Add validation to:
1. Check that width and height are non-zero
2. Verify that dx and dy are within virtual resolution bounds
3. Clip the rectangle dimensions to fit within virtual resolution if needed
This follows the same pattern used in other framebuffer drivers like
pm2fb_fillrect.
Reported-by: syzbot+7a63ce155648954e749b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7a63ce155648954e749b
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
drivers/video/fbdev/core/sysfillrect.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/sysfillrect.c b/drivers/video/fbdev/core/sysfillrect.c
index 12eea3e424bb..73fc322ff8fd 100644
--- a/drivers/video/fbdev/core/sysfillrect.c
+++ b/drivers/video/fbdev/core/sysfillrect.c
@@ -7,6 +7,7 @@
#include <linux/module.h>
#include <linux/fb.h>
#include <linux/bitrev.h>
+#include <linux/string.h>
#include <asm/types.h>
#ifdef CONFIG_FB_SYS_REV_PIXELS_IN_BYTE
@@ -18,10 +19,28 @@
void sys_fillrect(struct fb_info *p, const struct fb_fillrect *rect)
{
+ struct fb_fillrect modded;
+ int vxres, vyres;
+
if (!(p->flags & FBINFO_VIRTFB))
fb_warn_once(p, "%s: framebuffer is not in virtual address space.\n", __func__);
- fb_fillrect(p, rect);
+ vxres = p->var.xres_virtual;
+ vyres = p->var.yres_virtual;
+
+ /* Validate and clip rectangle to virtual resolution */
+ if (!rect->width || !rect->height ||
+ rect->dx >= vxres || rect->dy >= vyres)
+ return;
+
+ memcpy(&modded, rect, sizeof(struct fb_fillrect));
+
+ if (modded.dx + modded.width > vxres)
+ modded.width = vxres - modded.dx;
+ if (modded.dy + modded.height > vyres)
+ modded.height = vyres - modded.dy;
+
+ fb_fillrect(p, &modded);
}
EXPORT_SYMBOL(sys_fillrect);
--
2.43.0
^ permalink raw reply related
* [PATCH] fbdev: Fix slab-out-of-bounds read in fb_pad_unaligned_buffer
From: Osama Abdelkader @ 2026-01-18 13:47 UTC (permalink / raw)
To: Simona Vetter, Helge Deller, Thomas Zimmermann, Lee Jones,
Murad Masimov, Quanmin Yan, Yongzhen Zhang, Osama Abdelkader,
linux-fbdev, dri-devel, linux-kernel
Cc: syzbot+55e03490a0175b8dd81d
The function fb_pad_unaligned_buffer() was reading idx+1 bytes per row
from the source buffer, but when mod == 0 (font width is a multiple of
8 bits), the source buffer only has idx bytes per row. This caused a
slab-out-of-bounds read when accessing src[idx] after the inner loop.
Fix this by only reading the extra byte when mod != 0, ensuring we
never read beyond the source buffer boundaries.
This fixes the KASAN slab-out-of-bounds read reported by syzkaller:
https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
Reported-by: syzbot+55e03490a0175b8dd81d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
drivers/video/fbdev/core/fbmem.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index eff757ebbed1..a0c4932a6758 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -113,15 +113,17 @@ void fb_pad_unaligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 idx, u32 height,
dst[j+1] = tmp;
src++;
}
- tmp = dst[idx];
- tmp &= mask;
- tmp |= *src >> shift_low;
- dst[idx] = tmp;
- if (shift_high < mod) {
- tmp = *src << shift_high;
- dst[idx+1] = tmp;
+ if (mod) {
+ tmp = dst[idx];
+ tmp &= mask;
+ tmp |= *src >> shift_low;
+ dst[idx] = tmp;
+ if (shift_high < mod) {
+ tmp = *src << shift_high;
+ dst[idx+1] = tmp;
+ }
+ src++;
}
- src++;
dst += d_pitch;
}
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: tessolveupstream @ 2026-01-18 16:48 UTC (permalink / raw)
To: Daniel Thompson
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <aWe7_hFpmO0E2sJe@aspen.lan>
On 14-01-2026 21:23, Daniel Thompson wrote:
> On Tue, Jan 13, 2026 at 10:15:53AM +0530, tessolveupstream@gmail.com wrote:
>>
>>
>> On 05-01-2026 15:25, Daniel Thompson wrote:
>>> On Mon, Jan 05, 2026 at 02:21:19PM +0530, Sudarshan Shetty wrote:
>>>> Update the gpio-backlight binding to support configurations that require
>>>> more than one GPIO for enabling/disabling the backlight.
>>>>
>>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>>>> ---
>>>> .../bindings/leds/backlight/gpio-backlight.yaml | 12 +++++++++++-
>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>>> index 584030b6b0b9..1483ce4a3480 100644
>>>> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>>> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>>> @@ -17,7 +17,8 @@ properties:
>>>>
>>>> gpios:
>>>> description: The gpio that is used for enabling/disabling the backlight.
>>>> - maxItems: 1
>>>> + minItems: 1
>>>> + maxItems: 2
>>>
>>> Why 2?
>>>
>>
>> In the current design, the LVDS panel has a single backlight that
>> is controlled by two GPIOs. Initially, It described as two separate
>> backlight devices using the same gpio-backlight driver, since the
>> existing driver supports only one GPIO per instance.
>>
>> So the maintainer suggested to extend the gpio-backlight driver
>> and bindings to support multiple GPIOs.
>> https://lore.kernel.org/all/q63bdon55app4gb2il5e7skyc6z2amcnaiqbqlhen7arkxphtb@3jejbelji2ti/
>
> Right. So, once we support multiple GPIOs then why limit it to 2?
>
Okay, got the point. I'm removing the maxItems constraint entirely
to allow any number of GPIOs as below:
diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
index 1483ce4a3480..82698519daff 100644
--- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
@@ -16,9 +16,11 @@ properties:
const: gpio-backlight
gpios:
- description: The gpio that is used for enabling/disabling the backlight.
+ description: |
+ The gpio that is used for enabling/disabling the backlight.
+ Multiple GPIOs can be specified for panels that require several
+ enable signals.
minItems: 1
- maxItems: 2
default-on:
description: enable the backlight at boot.
Does this approach work for you?
>
> Daniel.
^ permalink raw reply related
* Re: [PATCH] fbdev: sys_fillrect: Add bounds checking to prevent vmalloc-out-of-bounds
From: Thomas Zimmermann @ 2026-01-19 7:38 UTC (permalink / raw)
To: Osama Abdelkader, Zsolt Kajtar, Simona Vetter, Helge Deller,
linux-fbdev, dri-devel, linux-kernel
Cc: syzbot+7a63ce155648954e749b
In-Reply-To: <20260118001852.70173-1-osama.abdelkader@gmail.com>
Hi,
thanks for the patch.
Am 18.01.26 um 01:18 schrieb Osama Abdelkader:
> The sys_fillrect function was missing bounds validation, which could lead
> to vmalloc-out-of-bounds writes when the rectangle coordinates extend
> beyond the framebuffer's virtual resolution. This was detected by KASAN
> and reported by syzkaller.
>
> Add validation to:
> 1. Check that width and height are non-zero
> 2. Verify that dx and dy are within virtual resolution bounds
> 3. Clip the rectangle dimensions to fit within virtual resolution if needed
This is rather a problem with the caller of the fillrect helper and
affects all drivers and all implementations of fb_fillrect. Clipping
should happen in the fbcon functions before invoking ->fb_con.
Best regards
Thomas
>
> This follows the same pattern used in other framebuffer drivers like
> pm2fb_fillrect.
>
> Reported-by: syzbot+7a63ce155648954e749b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7a63ce155648954e749b
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> drivers/video/fbdev/core/sysfillrect.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/sysfillrect.c b/drivers/video/fbdev/core/sysfillrect.c
> index 12eea3e424bb..73fc322ff8fd 100644
> --- a/drivers/video/fbdev/core/sysfillrect.c
> +++ b/drivers/video/fbdev/core/sysfillrect.c
> @@ -7,6 +7,7 @@
> #include <linux/module.h>
> #include <linux/fb.h>
> #include <linux/bitrev.h>
> +#include <linux/string.h>
> #include <asm/types.h>
>
> #ifdef CONFIG_FB_SYS_REV_PIXELS_IN_BYTE
> @@ -18,10 +19,28 @@
>
> void sys_fillrect(struct fb_info *p, const struct fb_fillrect *rect)
> {
> + struct fb_fillrect modded;
> + int vxres, vyres;
> +
> if (!(p->flags & FBINFO_VIRTFB))
> fb_warn_once(p, "%s: framebuffer is not in virtual address space.\n", __func__);
>
> - fb_fillrect(p, rect);
> + vxres = p->var.xres_virtual;
> + vyres = p->var.yres_virtual;
> +
> + /* Validate and clip rectangle to virtual resolution */
> + if (!rect->width || !rect->height ||
> + rect->dx >= vxres || rect->dy >= vyres)
> + return;
> +
> + memcpy(&modded, rect, sizeof(struct fb_fillrect));
> +
> + if (modded.dx + modded.width > vxres)
> + modded.width = vxres - modded.dx;
> + if (modded.dy + modded.height > vyres)
> + modded.height = vyres - modded.dy;
> +
> + fb_fillrect(p, &modded);
> }
> EXPORT_SYMBOL(sys_fillrect);
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v7 1/2] staging: fbtft: Fix build failure when CONFIG_FB_DEVICE=n
From: Andy Shevchenko @ 2026-01-19 7:40 UTC (permalink / raw)
To: Chintan Patel
Cc: linux-fbdev, linux-staging, linux-omap, linux-kernel, dri-devel,
tzimmermann, andy, deller, gregkh, kernel test robot
In-Reply-To: <20260117042931.6088-1-chintanlike@gmail.com>
On Fri, Jan 16, 2026 at 08:29:30PM -0800, Chintan Patel wrote:
> When CONFIG_FB_DEVICE is disabled, struct fb_info does
> not provide a valid dev pointer. Direct dereferences of
> fb_info->dev therefore result in build failures.
>
> Fix this by avoiding direct accesses to fb_info->dev and
> switching the affected debug logging to framebuffer helpers
> that do not rely on a device pointer.
>
> This fixes the following build failure reported by the
> kernel test robot.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v7 2/2] staging: fbtft: Make framebuffer registration message debug-only
From: Andy Shevchenko @ 2026-01-19 7:40 UTC (permalink / raw)
To: Chintan Patel
Cc: linux-fbdev, linux-staging, linux-omap, linux-kernel, dri-devel,
tzimmermann, andy, deller, gregkh
In-Reply-To: <20260117042931.6088-2-chintanlike@gmail.com>
On Fri, Jan 16, 2026 at 08:29:31PM -0800, Chintan Patel wrote:
> The framebuffer registration message is informational only and not
> useful during normal operation. Convert it to debug-level logging to
> keep the driver quiet when working correctly.
Suggested-by: Greg ...?
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] fbdev: Fix slab-out-of-bounds read in fb_pad_unaligned_buffer
From: Thomas Zimmermann @ 2026-01-19 7:45 UTC (permalink / raw)
To: Osama Abdelkader, Simona Vetter, Helge Deller, Lee Jones,
Murad Masimov, Quanmin Yan, Yongzhen Zhang, linux-fbdev,
dri-devel, linux-kernel
Cc: syzbot+55e03490a0175b8dd81d
In-Reply-To: <20260118134735.11507-1-osama.abdelkader@gmail.com>
Hi
Am 18.01.26 um 14:47 schrieb Osama Abdelkader:
> The function fb_pad_unaligned_buffer() was reading idx+1 bytes per row
> from the source buffer, but when mod == 0 (font width is a multiple of
> 8 bits), the source buffer only has idx bytes per row. This caused a
> slab-out-of-bounds read when accessing src[idx] after the inner loop.
>
> Fix this by only reading the extra byte when mod != 0, ensuring we
> never read beyond the source buffer boundaries.
>
> This fixes the KASAN slab-out-of-bounds read reported by syzkaller:
> https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
>
> Reported-by: syzbot+55e03490a0175b8dd81d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> drivers/video/fbdev/core/fbmem.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index eff757ebbed1..a0c4932a6758 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -113,15 +113,17 @@ void fb_pad_unaligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 idx, u32 height,
> dst[j+1] = tmp;
> src++;
> }
> - tmp = dst[idx];
> - tmp &= mask;
> - tmp |= *src >> shift_low;
> - dst[idx] = tmp;
> - if (shift_high < mod) {
> - tmp = *src << shift_high;
> - dst[idx+1] = tmp;
> + if (mod) {
How do we end up here if mod equals 0? When I look at the callers of
this function, cases with (mod == 0) take an entirely different branch.
[1] [2]
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v6.18.6/source/drivers/video/fbdev/core/bitblit.c#L208
[2]
https://elixir.bootlin.com/linux/v6.18.6/source/drivers/video/fbdev/core/fbcon_ud.c#L199
> + tmp = dst[idx];
> + tmp &= mask;
> + tmp |= *src >> shift_low;
> + dst[idx] = tmp;
> + if (shift_high < mod) {
> + tmp = *src << shift_high;
> + dst[idx+1] = tmp;
> + }
> + src++;
> }
> - src++;
> dst += d_pitch;
> }
> }
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH 00/12] Recover sysfb after DRM probe failure
From: Christian König @ 2026-01-19 10:03 UTC (permalink / raw)
To: Zack Rusin, Thomas Zimmermann
Cc: dri-devel, Alex Deucher, amd-gfx, Ard Biesheuvel, Ce Sun,
Chia-I Wu, Danilo Krummrich, Dave Airlie, Deepak Rawat,
Dmitry Osipenko, Gerd Hoffmann, Gurchetan Singh, Hans de Goede,
Hawking Zhang, Helge Deller, intel-gfx, intel-xe, Jani Nikula,
Javier Martinez Canillas, Jocelyn Falempe, Joonas Lahtinen,
Lijo Lazar, linux-efi, linux-fbdev, linux-hyperv, linux-kernel,
Lucas De Marchi, Lyude Paul, Maarten Lankhorst,
Mario Limonciello (AMD), Mario Limonciello, Maxime Ripard,
nouveau, Rodrigo Vivi, Simona Vetter, spice-devel,
Thomas Hellström, Timur Kristóf, Tvrtko Ursulin,
virtualization, Vitaly Prosyak
In-Reply-To: <CABQX2QM0_6DJtrahJS7x9iF_wcSZRc4dohEiPnMCtAg7Vt7JPQ@mail.gmail.com>
On 1/17/26 07:02, Zack Rusin wrote:
> On Fri, Jan 16, 2026 at 2:58 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 16.01.26 um 04:59 schrieb Zack Rusin:
>>> On Thu, Jan 15, 2026 at 6:02 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>> That's really not going to work. For example, in the current series, you
>>>> invoke devm_aperture_remove_conflicting_pci_devices_done() after
>>>> drm_mode_reset(), drm_dev_register() and drm_client_setup().
>>> That's perfectly fine,
>>> devm_aperture_remove_conflicting_pci_devices_done is removing the
>>> reload behavior not doing anything.
>>>
>>> This series, essentially, just adds a "defer" statement to
>>> aperture_remove_conflicting_pci_devices that says
>>>
>>> "reload sysfb if this driver unloads".
>>>
>>> devm_aperture_remove_conflicting_pci_devices_done just cancels that defer.
>>
>> Exactly. And if that reload happens after the hardware state has been
>> changed, the result is undefined.
>
> This is all predicated on drivers actually cleaning up after
> themselves. I don't think any amount of good will or api design is
> going to fix device specific state mismatches.
>
>> The current recovery/reload is not reliable in any case. A number of
>> high-profile devs have also said that it doesn't work with their driver.
>> The same is true for ast. So the current approach is not going to happen.
>>
>>> There also might be the case of some crazy behavior, e.g. pci bar
>>> resize in the driver makes the vga hardware crash or something, in
>>> which case, yea, we should definitely skip this patch, at least until
>>> those drivers properly cleanup on exit.
>>
>> There's nothing crazy here. It's standard probing code.
>>
>> If you want to to move forward, my suggestion is to look at the proposal
>> with the aperture_funcs callbacks that control sysfb device access. And
>> from there, build a full prototype with one or two drivers.
>
> I don't think that approach is going to work. I don't think there's
> anything that can be done if drivers didn't cleanup everything they've
> done that might have broken sysfb on unload. I'm going to drop it
> then, it's obviously a shame because it works fine with virtualized
> drivers and they're ones that would likely profit from this the most
> but I'm sceptical that I could do full system state set reset in a
> generalized fashion for hw drivers or that the work required would be
> worth the payoff.
Well at least for PCI devices you could try doing a function level reset to get the HW back into some usable state.
This does *not* work for AMD HW since we have HW/FW bugs, but at least for your virtualized use case it might work.
All you need then is an EFI, Vesa or int10 call to re-init the HW to the pre-driver load setup.
I know that is not the easiest thing to do, but still better than a black screen.
Regards,
Christian.
>
> z
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: Daniel Thompson @ 2026-01-19 14:42 UTC (permalink / raw)
To: tessolveupstream
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <95a49665-f379-48a7-a2b5-d288cdfdc0a8@gmail.com>
On Sun, Jan 18, 2026 at 10:18:32PM +0530, tessolveupstream@gmail.com wrote:
> On 14-01-2026 21:23, Daniel Thompson wrote:
> > On Tue, Jan 13, 2026 at 10:15:53AM +0530, tessolveupstream@gmail.com wrote:
> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> index 1483ce4a3480..82698519daff 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> @@ -16,9 +16,11 @@ properties:
> const: gpio-backlight
>
> gpios:
> - description: The gpio that is used for enabling/disabling the backlight.
> + description: |
> + The gpio that is used for enabling/disabling the backlight.
> + Multiple GPIOs can be specified for panels that require several
> + enable signals.
> minItems: 1
> - maxItems: 2
>
> default-on:
> description: enable the backlight at boot.
>
> Does this approach work for you?
I think so... but I'd rather just review a v2 patch than this kind of
meta-patch.
Daniel.
^ permalink raw reply
* Re: [PATCH v3 1/7] dt-bindings: backlight: qcom-wled: Document ovp values for PMI8994
From: Daniel Thompson @ 2026-01-19 14:50 UTC (permalink / raw)
To: Barnabás Czémán
Cc: Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Kiran Gunda,
Helge Deller, Luca Weiss, Konrad Dybcio, Eugene Lepshy,
Gianluca Boiano, Alejandro Tafalla, dri-devel, linux-leds,
devicetree, linux-kernel, Daniel Thompson, linux-arm-msm,
linux-fbdev, Konrad Dybcio, Krzysztof Kozlowski
In-Reply-To: <20260116-pmi8950-wled-v3-1-e6c93de84079@mainlining.org>
On Fri, Jan 16, 2026 at 08:07:33AM +0100, Barnabás Czémán wrote:
> Document ovp values supported by wled found in PMI8994.
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Thanks
Daniel
^ permalink raw reply
* Re: [PATCH v3 2/7] backlight: qcom-wled: Support ovp values for PMI8994
From: Daniel Thompson @ 2026-01-19 14:51 UTC (permalink / raw)
To: Barnabás Czémán
Cc: Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Kiran Gunda,
Helge Deller, Luca Weiss, Konrad Dybcio, Eugene Lepshy,
Gianluca Boiano, Alejandro Tafalla, dri-devel, linux-leds,
devicetree, linux-kernel, Daniel Thompson, linux-arm-msm,
linux-fbdev, Konrad Dybcio
In-Reply-To: <20260116-pmi8950-wled-v3-2-e6c93de84079@mainlining.org>
On Fri, Jan 16, 2026 at 08:07:34AM +0100, Barnabás Czémán wrote:
> WLED4 found in PMI8994 supports different ovp values.
>
> Fixes: 6fc632d3e3e0 ("video: backlight: qcom-wled: Add PMI8994 compatible")
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Thanks
Daniel
^ permalink raw reply
* Re: [PATCH v3 3/7] dt-bindings: backlight: qcom-wled: Document ovp values for PMI8950
From: Daniel Thompson @ 2026-01-19 14:51 UTC (permalink / raw)
To: Barnabás Czémán
Cc: Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Kiran Gunda,
Helge Deller, Luca Weiss, Konrad Dybcio, Eugene Lepshy,
Gianluca Boiano, Alejandro Tafalla, dri-devel, linux-leds,
devicetree, linux-kernel, Daniel Thompson, linux-arm-msm,
linux-fbdev, Konrad Dybcio, Krzysztof Kozlowski
In-Reply-To: <20260116-pmi8950-wled-v3-3-e6c93de84079@mainlining.org>
On Fri, Jan 16, 2026 at 08:07:35AM +0100, Barnabás Czémán wrote:
> Document ovp values supported by wled found in PMI8950.
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Thanks
Daniel
^ permalink raw reply
* Re: [PATCH v3 4/7] backlight: qcom-wled: Change PM8950 WLED configurations
From: Daniel Thompson @ 2026-01-19 14:53 UTC (permalink / raw)
To: Barnabás Czémán
Cc: Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Kiran Gunda,
Helge Deller, Luca Weiss, Konrad Dybcio, Eugene Lepshy,
Gianluca Boiano, Alejandro Tafalla, dri-devel, linux-leds,
devicetree, linux-kernel, Daniel Thompson, linux-arm-msm,
linux-fbdev, Konrad Dybcio
In-Reply-To: <20260116-pmi8950-wled-v3-4-e6c93de84079@mainlining.org>
On Fri, Jan 16, 2026 at 08:07:36AM +0100, Barnabás Czémán wrote:
> PMI8950 WLED needs same configurations as PMI8994 WLED.
>
> Fixes: 10258bf4534b ("backlight: qcom-wled: Add PMI8950 compatible")
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
> ---
> drivers/video/backlight/qcom-wled.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
> index 5decbd39b789..8054e4787725 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
> @@ -1455,7 +1455,8 @@ static int wled_configure(struct wled *wled)
> break;
>
> case 4:
> - if (of_device_is_compatible(dev->of_node, "qcom,pmi8994-wled")) {
> + if (of_device_is_compatible(dev->of_node, "qcom,pmi8950-wled") ||
> + of_device_is_compatible(dev->of_node, "qcom,pmi8994-wled")) {
> u32_opts = pmi8994_wled_opts;
I still really dislike naming the structures after a single instance of
the PMIC when, at inception, that name is known to be wrong. However
if the Qualcomm devs are happy with it then I guess I can live with it.
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Thanks
Daniel.
^ permalink raw reply
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: tessolveupstream @ 2026-01-20 4:52 UTC (permalink / raw)
To: Daniel Thompson
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <aWe-QA_grqNwnE4n@aspen.lan>
On 14-01-2026 21:33, Daniel Thompson wrote:
> On Tue, Jan 13, 2026 at 12:47:26PM +0530, tessolveupstream@gmail.com wrote:
>>
>>
>> On 05-01-2026 15:39, Daniel Thompson wrote:
>>> On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote:
>>>> Extend the gpio-backlight driver to handle multiple GPIOs instead of a
>>>> single one. This allows panels that require driving several enable pins
>>>> to be controlled by the backlight framework.
>>>>
>>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>>>> ---
>>>> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++-------
>>>> 1 file changed, 45 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
>>>> index 728a546904b0..037e1c111e48 100644
>>>> --- a/drivers/video/backlight/gpio_backlight.c
>>>> +++ b/drivers/video/backlight/gpio_backlight.c
>>>> @@ -17,14 +17,18 @@
>>>>
>>>> struct gpio_backlight {
>>>> struct device *dev;
>>>> - struct gpio_desc *gpiod;
>>>> + struct gpio_desc **gpiods;
>>>> + unsigned int num_gpios;
>>>
>>> Why not use struct gpio_descs for this?
>>>
>>> Once you do that, then most of the gbl->num_gpios loops can be replaced with
>>> calls to the array based accessors.
>>>
>>
>> Based on your feedback, I have updated the implementation to use
>> struct gpio_descs and array-based accessors, as recommended like
>> below:
>>
>> git diff drivers/video/backlight/gpio_backlight.c
>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
>> index 037e1c111e48..e99d7a9dc670 100644
>> --- a/drivers/video/backlight/gpio_backlight.c
>> +++ b/drivers/video/backlight/gpio_backlight.c
>> @@ -14,22 +14,37 @@
>> #include <linux/platform_device.h>
>> #include <linux/property.h>
>> #include <linux/slab.h>
>> +#include <linux/bitmap.h>
>>
>> struct gpio_backlight {
>> struct device *dev;
>> - struct gpio_desc **gpiods;
>> + struct gpio_descs *gpiods;
>> unsigned int num_gpios;
>> };
>>
>> static int gpio_backlight_update_status(struct backlight_device *bl)
>> {
>> struct gpio_backlight *gbl = bl_get_data(bl);
>> - unsigned int i;
>> + unsigned int n = gbl->num_gpios;
>> int br = backlight_get_brightness(bl);
>> + unsigned long *value_bitmap;
>> + int words = BITS_TO_LONGS(n);
>> +
>> + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL);
>
> Not sure you need a kcalloc() here. If you want to support more than 32
> GPIOs then you can pre-allocate space with a devm_kcalloc() in the probe
> method rather than reallocate every time it is used.
>
> To be honest I don't really mind putting a hard limit on the maximum
> gpl->num_gpios (so you can just use a local variable) and having no
> allocation at all.
>
Thanks for the suggestion. I addressed the kcalloc() concern by
moving the bitmap allocation to probe using devm_kcalloc() as
below:
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 0eb42d8bf1d9..7af5dc4f0315 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -19,32 +19,25 @@
struct gpio_backlight {
struct device *dev;
struct gpio_descs *gpiods;
- unsigned int num_gpios;
+ unsigned long *bitmap;
};
static int gpio_backlight_update_status(struct backlight_device *bl)
{
struct gpio_backlight *gbl = bl_get_data(bl);
- unsigned int n = gbl->num_gpios;
+ unsigned int n = gbl->gpiods->ndescs;
int br = backlight_get_brightness(bl);
- unsigned long *value_bitmap;
- int words = BITS_TO_LONGS(n);
-
- value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL);
- if (!value_bitmap)
- return -ENOMEM;
if (br)
- bitmap_fill(value_bitmap, n);
+ bitmap_fill(gbl->bitmap, n);
else
- bitmap_zero(value_bitmap, n);
+ bitmap_zero(gbl->bitmap, n);
- gpiod_set_array_value_cansleep(gbl->gpiods->ndescs,
+ gpiod_set_array_value_cansleep(n,
gbl->gpiods->desc,
gbl->gpiods->info,
- value_bitmap);
+ gbl->bitmap);
- kfree(value_bitmap);
return 0;
}
@@ -67,22 +60,25 @@ static int gpio_backlight_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
struct device_node *of_node = dev->of_node;
- struct backlight_properties props;
+ struct backlight_properties props = { };
struct backlight_device *bl;
struct gpio_backlight *gbl;
- int ret, init_brightness, def_value;
- unsigned int i;
+ bool def_value;
+ enum gpiod_flags flags;
+ unsigned int n;
+ int words;
- gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
- if (gbl == NULL)
+ gbl = devm_kcalloc(dev, 1, sizeof(*gbl), GFP_KERNEL);
+ if (!gbl)
return -ENOMEM;
if (pdata)
gbl->dev = pdata->dev;
def_value = device_property_read_bool(dev, "default-on");
-
- gbl->gpiods = devm_gpiod_get_array(dev, NULL, GPIOD_ASIS);
+ flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+
+ gbl->gpiods = devm_gpiod_get_array(dev, NULL, flags);
if (IS_ERR(gbl->gpiods)) {
if (PTR_ERR(gbl->gpiods) == -ENODEV)
return dev_err_probe(dev, -EINVAL,
@@ -90,12 +86,17 @@ static int gpio_backlight_probe(struct platform_device *pdev)
return PTR_ERR(gbl->gpiods);
}
- gbl->num_gpios = gbl->gpiods->ndescs;
- if (gbl->num_gpios == 0)
+ n = gbl->gpiods->ndescs;
+ if (!n)
return dev_err_probe(dev, -EINVAL,
- "The gpios parameter is missing or invalid\n");
+ "No GPIOs provided\n");
+
+ words = BITS_TO_LONGS(n);
+ gbl->bitmap = devm_kcalloc(dev, words, sizeof(unsigned long),
+ GFP_KERNEL);
+ if (!gbl->bitmap)
+ return -ENOMEM;
- memset(&props, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = 1;
bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl,
@@ -106,50 +107,19 @@ static int gpio_backlight_probe(struct platform_device *pdev)
}
/* Set the initial power state */
- if (!of_node || !of_node->phandle) {
+ if (!of_node || !of_node->phandle)
/* Not booted with device tree or no phandle link to the node */
bl->props.power = def_value ? BACKLIGHT_POWER_ON
: BACKLIGHT_POWER_OFF;
- } else {
- bool all_high = true;
- unsigned long *value_bitmap;
- int words = BITS_TO_LONGS(gbl->num_gpios);
-
- value_bitmap = kcalloc(words, sizeof(unsigned long),
- GFP_KERNEL);
- if (!value_bitmap)
- return -ENOMEM;
-
- ret = gpiod_get_array_value_cansleep(gbl->gpiods->ndescs,
- gbl->gpiods->desc,
- gbl->gpiods->info,
- value_bitmap);
- if (ret) {
- kfree(value_bitmap);
- return dev_err_probe(dev, ret,
- "failed to read initial gpio values\n");
- }
-
- all_high = bitmap_full(value_bitmap, gbl->num_gpios);
-
- kfree(value_bitmap);
- bl->props.power =
- all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
- }
-
- bl->props.brightness = 1;
-
- init_brightness = backlight_get_brightness(bl);
+ else if (gpiod_get_value_cansleep(gbl->gpiods->desc[0]) == 0)
+ bl->props.power = BACKLIGHT_POWER_OFF;
+ else
+ bl->props.power = BACKLIGHT_POWER_ON;
- for (i = 0; i < gbl->num_gpios; i++) {
- ret = gpiod_direction_output(gbl->gpiods->desc[i],
- init_brightness);
- if (ret)
- return dev_err_probe(dev, ret,
- "failed to set gpio %u direction\n",
- i);
- }
+ bl->props.brightness = def_value ? 1 : 0;
+ gpio_backlight_update_status(bl);
+
platform_set_drvdata(pdev, bl);
return 0;
}
Kindly confirm whether this approach aligns with your
expectations.
>
>> Could you please share your thoughts on whether this approach
>> aligns with your expectations?
>
> Looks like it is going in the right direction, yes.
>
>
> Daniel.
^ permalink raw reply related
* [PATCH 1/2] drm, fbcon, vga_switcheroo: Avoid race condition in fbcon setup
From: Thomas Zimmermann @ 2026-01-20 8:13 UTC (permalink / raw)
To: kernel-kabi
Cc: Thomas Zimmermann, Javier Martinez Canillas, Alex Deucher,
dri-devel, nouveau, amd-gfx, linux-fbdev, stable
In-Reply-To: <20260120081841.9827-1-tzimmermann@suse.com>
From: Thomas Zimmermann <tzimmermann@suse.de>
Protect vga_switcheroo_client_fb_set() with console lock. Avoids OOB
access in fbcon_remap_all(). Without holding the console lock the call
races with switching outputs.
VGA switcheroo calls fbcon_remap_all() when switching clients. The fbcon
function uses struct fb_info.node, which is set by register_framebuffer().
As the fb-helper code currently sets up VGA switcheroo before registering
the framebuffer, the value of node is -1 and therefore not a legal value.
For example, fbcon uses the value within set_con2fb_map() [1] as an index
into an array.
Moving vga_switcheroo_client_fb_set() after register_framebuffer() can
result in VGA switching that does not switch fbcon correctly.
Therefore move vga_switcheroo_client_fb_set() under fbcon_fb_registered(),
which already holds the console lock. Fbdev calls fbcon_fb_registered()
from within register_framebuffer(). Serializes the helper with VGA
switcheroo's call to fbcon_remap_all().
Although vga_switcheroo_client_fb_set() takes an instance of struct fb_info
as parameter, it really only needs the contained fbcon state. Moving the
call to fbcon initialization is therefore cleaner than before. Only amdgpu,
i915, nouveau and radeon support vga_switcheroo. For all other drivers,
this change does nothing.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://elixir.bootlin.com/linux/v6.17/source/drivers/video/fbdev/core/fbcon.c#L2942 # [1]
Fixes: 6a9ee8af344e ("vga_switcheroo: initial implementation (v15)")
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Cc: <stable@vger.kernel.org> # v2.6.34+
Link: https://patch.msgid.link/20251105161549.98836-1-tzimmermann@suse.de
(cherry picked from commit eb76d0f5553575599561010f24c277cc5b31d003)
---
drivers/gpu/drm/drm_fb_helper.c | 6 ------
drivers/gpu/drm/i915/display/intel_fbdev.c | 2 --
drivers/gpu/drm/radeon/radeon_fbdev.c | 3 ---
drivers/video/fbdev/core/fbcon.c | 9 +++++++++
4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b507c1c008a3e..3915a8b17bc4c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -32,7 +32,6 @@
#include <linux/console.h>
#include <linux/pci.h>
#include <linux/sysrq.h>
-#include <linux/vga_switcheroo.h>
#include <drm/drm_atomic.h>
#include <drm/drm_drv.h>
@@ -1668,7 +1667,6 @@ static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper,
static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper)
{
struct drm_client_dev *client = &fb_helper->client;
- struct drm_device *dev = fb_helper->dev;
struct drm_fb_helper_surface_size sizes;
int ret;
@@ -1687,10 +1685,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper)
strcpy(fb_helper->fb->comm, "[fbcon]");
- /* Set the fb info for vgaswitcheroo clients. Does nothing otherwise. */
- if (dev_is_pci(dev->dev))
- vga_switcheroo_client_fb_set(to_pci_dev(dev->dev), fb_helper->info);
-
return 0;
}
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 31d0d695d5671..8bad7e7909f7b 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -36,7 +36,6 @@
#include <linux/string.h>
#include <linux/sysrq.h>
#include <linux/tty.h>
-#include <linux/vga_switcheroo.h>
#include <drm/drm_crtc.h>
#include <drm/drm_fb_helper.h>
@@ -337,7 +336,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
ifbdev->vma_flags = flags;
intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
- vga_switcheroo_client_fb_set(pdev, info);
return 0;
out_unpin:
diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c
index fb70de29545c6..730efabe05f6b 100644
--- a/drivers/gpu/drm/radeon/radeon_fbdev.c
+++ b/drivers/gpu/drm/radeon/radeon_fbdev.c
@@ -303,7 +303,6 @@ static void radeon_fbdev_client_unregister(struct drm_client_dev *client)
struct radeon_device *rdev = dev->dev_private;
if (fb_helper->info) {
- vga_switcheroo_client_fb_set(rdev->pdev, NULL);
drm_helper_force_disable_all(dev);
drm_fb_helper_unregister_info(fb_helper);
} else {
@@ -342,8 +341,6 @@ static int radeon_fbdev_client_hotplug(struct drm_client_dev *client)
if (ret)
goto err_drm_fb_helper_fini;
- vga_switcheroo_client_fb_set(rdev->pdev, fb_helper->info);
-
return 0;
err_drm_fb_helper_fini:
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 79525ee90e3b2..57b21b6869231 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -64,6 +64,7 @@
#include <linux/console.h>
#include <linux/string.h>
#include <linux/kd.h>
+#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/fb.h>
#include <linux/fbcon.h>
@@ -75,6 +76,7 @@
#include <linux/interrupt.h>
#include <linux/crc32.h> /* For counting font checksums */
#include <linux/uaccess.h>
+#include <linux/vga_switcheroo.h>
#include <asm/irq.h>
#include "fbcon.h"
@@ -2909,6 +2911,9 @@ void fbcon_fb_unregistered(struct fb_info *info)
console_lock();
+ if (info->device && dev_is_pci(info->device))
+ vga_switcheroo_client_fb_set(to_pci_dev(info->device), info);
+
fbcon_registered_fb[info->node] = NULL;
fbcon_num_registered_fb--;
@@ -3042,6 +3047,10 @@ static int do_fb_registered(struct fb_info *info)
}
}
+ /* Set the fb info for vga_switcheroo clients. Does nothing otherwise. */
+ if (info->device && dev_is_pci(info->device))
+ vga_switcheroo_client_fb_set(to_pci_dev(info->device), NULL);
+
return ret;
}
--
2.52.0
^ permalink raw reply related
* Re: [PATCH 1/2] drm, fbcon, vga_switcheroo: Avoid race condition in fbcon setup
From: Thomas Zimmermann @ 2026-01-20 8:20 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Javier Martinez Canillas, Alex Deucher, dri-devel, nouveau,
amd-gfx, linux-fbdev, stable
In-Reply-To: <20260120081841.9827-2-tzimmermann@suse.com>
Please ignore. This patch has been merged already.
Am 20.01.26 um 09:13 schrieb Thomas Zimmermann:
> From: Thomas Zimmermann <tzimmermann@suse.de>
>
> Protect vga_switcheroo_client_fb_set() with console lock. Avoids OOB
> access in fbcon_remap_all(). Without holding the console lock the call
> races with switching outputs.
>
> VGA switcheroo calls fbcon_remap_all() when switching clients. The fbcon
> function uses struct fb_info.node, which is set by register_framebuffer().
> As the fb-helper code currently sets up VGA switcheroo before registering
> the framebuffer, the value of node is -1 and therefore not a legal value.
> For example, fbcon uses the value within set_con2fb_map() [1] as an index
> into an array.
>
> Moving vga_switcheroo_client_fb_set() after register_framebuffer() can
> result in VGA switching that does not switch fbcon correctly.
>
> Therefore move vga_switcheroo_client_fb_set() under fbcon_fb_registered(),
> which already holds the console lock. Fbdev calls fbcon_fb_registered()
> from within register_framebuffer(). Serializes the helper with VGA
> switcheroo's call to fbcon_remap_all().
>
> Although vga_switcheroo_client_fb_set() takes an instance of struct fb_info
> as parameter, it really only needs the contained fbcon state. Moving the
> call to fbcon initialization is therefore cleaner than before. Only amdgpu,
> i915, nouveau and radeon support vga_switcheroo. For all other drivers,
> this change does nothing.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://elixir.bootlin.com/linux/v6.17/source/drivers/video/fbdev/core/fbcon.c#L2942 # [1]
> Fixes: 6a9ee8af344e ("vga_switcheroo: initial implementation (v15)")
> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: linux-fbdev@vger.kernel.org
> Cc: <stable@vger.kernel.org> # v2.6.34+
> Link: https://patch.msgid.link/20251105161549.98836-1-tzimmermann@suse.de
> (cherry picked from commit eb76d0f5553575599561010f24c277cc5b31d003)
> ---
> drivers/gpu/drm/drm_fb_helper.c | 6 ------
> drivers/gpu/drm/i915/display/intel_fbdev.c | 2 --
> drivers/gpu/drm/radeon/radeon_fbdev.c | 3 ---
> drivers/video/fbdev/core/fbcon.c | 9 +++++++++
> 4 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b507c1c008a3e..3915a8b17bc4c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -32,7 +32,6 @@
> #include <linux/console.h>
> #include <linux/pci.h>
> #include <linux/sysrq.h>
> -#include <linux/vga_switcheroo.h>
>
> #include <drm/drm_atomic.h>
> #include <drm/drm_drv.h>
> @@ -1668,7 +1667,6 @@ static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper,
> static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper)
> {
> struct drm_client_dev *client = &fb_helper->client;
> - struct drm_device *dev = fb_helper->dev;
> struct drm_fb_helper_surface_size sizes;
> int ret;
>
> @@ -1687,10 +1685,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper)
>
> strcpy(fb_helper->fb->comm, "[fbcon]");
>
> - /* Set the fb info for vgaswitcheroo clients. Does nothing otherwise. */
> - if (dev_is_pci(dev->dev))
> - vga_switcheroo_client_fb_set(to_pci_dev(dev->dev), fb_helper->info);
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 31d0d695d5671..8bad7e7909f7b 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -36,7 +36,6 @@
> #include <linux/string.h>
> #include <linux/sysrq.h>
> #include <linux/tty.h>
> -#include <linux/vga_switcheroo.h>
>
> #include <drm/drm_crtc.h>
> #include <drm/drm_fb_helper.h>
> @@ -337,7 +336,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
> ifbdev->vma_flags = flags;
>
> intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> - vga_switcheroo_client_fb_set(pdev, info);
> return 0;
>
> out_unpin:
> diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c
> index fb70de29545c6..730efabe05f6b 100644
> --- a/drivers/gpu/drm/radeon/radeon_fbdev.c
> +++ b/drivers/gpu/drm/radeon/radeon_fbdev.c
> @@ -303,7 +303,6 @@ static void radeon_fbdev_client_unregister(struct drm_client_dev *client)
> struct radeon_device *rdev = dev->dev_private;
>
> if (fb_helper->info) {
> - vga_switcheroo_client_fb_set(rdev->pdev, NULL);
> drm_helper_force_disable_all(dev);
> drm_fb_helper_unregister_info(fb_helper);
> } else {
> @@ -342,8 +341,6 @@ static int radeon_fbdev_client_hotplug(struct drm_client_dev *client)
> if (ret)
> goto err_drm_fb_helper_fini;
>
> - vga_switcheroo_client_fb_set(rdev->pdev, fb_helper->info);
> -
> return 0;
>
> err_drm_fb_helper_fini:
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 79525ee90e3b2..57b21b6869231 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -64,6 +64,7 @@
> #include <linux/console.h>
> #include <linux/string.h>
> #include <linux/kd.h>
> +#include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/fb.h>
> #include <linux/fbcon.h>
> @@ -75,6 +76,7 @@
> #include <linux/interrupt.h>
> #include <linux/crc32.h> /* For counting font checksums */
> #include <linux/uaccess.h>
> +#include <linux/vga_switcheroo.h>
> #include <asm/irq.h>
>
> #include "fbcon.h"
> @@ -2909,6 +2911,9 @@ void fbcon_fb_unregistered(struct fb_info *info)
>
> console_lock();
>
> + if (info->device && dev_is_pci(info->device))
> + vga_switcheroo_client_fb_set(to_pci_dev(info->device), info);
> +
> fbcon_registered_fb[info->node] = NULL;
> fbcon_num_registered_fb--;
>
> @@ -3042,6 +3047,10 @@ static int do_fb_registered(struct fb_info *info)
> }
> }
>
> + /* Set the fb info for vga_switcheroo clients. Does nothing otherwise. */
> + if (info->device && dev_is_pci(info->device))
> + vga_switcheroo_client_fb_set(to_pci_dev(info->device), NULL);
> +
> return ret;
> }
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* [PATCH 1/2] drm, fbcon, vga_switcheroo: Avoid race condition in fbcon setup
From: Thomas Zimmermann @ 2026-01-20 8:28 UTC (permalink / raw)
To: kernel-kabi
Cc: Thomas Zimmermann, Javier Martinez Canillas, Alex Deucher,
dri-devel, nouveau, amd-gfx, linux-fbdev, stable
In-Reply-To: <20260120082841.11381-1-tzimmermann@suse.com>
From: Thomas Zimmermann <tzimmermann@suse.de>
Protect vga_switcheroo_client_fb_set() with console lock. Avoids OOB
access in fbcon_remap_all(). Without holding the console lock the call
races with switching outputs.
VGA switcheroo calls fbcon_remap_all() when switching clients. The fbcon
function uses struct fb_info.node, which is set by register_framebuffer().
As the fb-helper code currently sets up VGA switcheroo before registering
the framebuffer, the value of node is -1 and therefore not a legal value.
For example, fbcon uses the value within set_con2fb_map() [1] as an index
into an array.
Moving vga_switcheroo_client_fb_set() after register_framebuffer() can
result in VGA switching that does not switch fbcon correctly.
Therefore move vga_switcheroo_client_fb_set() under fbcon_fb_registered(),
which already holds the console lock. Fbdev calls fbcon_fb_registered()
from within register_framebuffer(). Serializes the helper with VGA
switcheroo's call to fbcon_remap_all().
Although vga_switcheroo_client_fb_set() takes an instance of struct fb_info
as parameter, it really only needs the contained fbcon state. Moving the
call to fbcon initialization is therefore cleaner than before. Only amdgpu,
i915, nouveau and radeon support vga_switcheroo. For all other drivers,
this change does nothing.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://elixir.bootlin.com/linux/v6.17/source/drivers/video/fbdev/core/fbcon.c#L2942 # [1]
Fixes: 6a9ee8af344e ("vga_switcheroo: initial implementation (v15)")
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Cc: <stable@vger.kernel.org> # v2.6.34+
Link: https://patch.msgid.link/20251105161549.98836-1-tzimmermann@suse.de
(cherry picked from commit eb76d0f5553575599561010f24c277cc5b31d003)
---
drivers/gpu/drm/drm_fb_helper.c | 6 ------
drivers/gpu/drm/i915/display/intel_fbdev.c | 2 --
drivers/gpu/drm/radeon/radeon_fbdev.c | 3 ---
drivers/video/fbdev/core/fbcon.c | 9 +++++++++
4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b507c1c008a3e..3915a8b17bc4c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -32,7 +32,6 @@
#include <linux/console.h>
#include <linux/pci.h>
#include <linux/sysrq.h>
-#include <linux/vga_switcheroo.h>
#include <drm/drm_atomic.h>
#include <drm/drm_drv.h>
@@ -1668,7 +1667,6 @@ static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper,
static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper)
{
struct drm_client_dev *client = &fb_helper->client;
- struct drm_device *dev = fb_helper->dev;
struct drm_fb_helper_surface_size sizes;
int ret;
@@ -1687,10 +1685,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper)
strcpy(fb_helper->fb->comm, "[fbcon]");
- /* Set the fb info for vgaswitcheroo clients. Does nothing otherwise. */
- if (dev_is_pci(dev->dev))
- vga_switcheroo_client_fb_set(to_pci_dev(dev->dev), fb_helper->info);
-
return 0;
}
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 31d0d695d5671..8bad7e7909f7b 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -36,7 +36,6 @@
#include <linux/string.h>
#include <linux/sysrq.h>
#include <linux/tty.h>
-#include <linux/vga_switcheroo.h>
#include <drm/drm_crtc.h>
#include <drm/drm_fb_helper.h>
@@ -337,7 +336,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
ifbdev->vma_flags = flags;
intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
- vga_switcheroo_client_fb_set(pdev, info);
return 0;
out_unpin:
diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c
index fb70de29545c6..730efabe05f6b 100644
--- a/drivers/gpu/drm/radeon/radeon_fbdev.c
+++ b/drivers/gpu/drm/radeon/radeon_fbdev.c
@@ -303,7 +303,6 @@ static void radeon_fbdev_client_unregister(struct drm_client_dev *client)
struct radeon_device *rdev = dev->dev_private;
if (fb_helper->info) {
- vga_switcheroo_client_fb_set(rdev->pdev, NULL);
drm_helper_force_disable_all(dev);
drm_fb_helper_unregister_info(fb_helper);
} else {
@@ -342,8 +341,6 @@ static int radeon_fbdev_client_hotplug(struct drm_client_dev *client)
if (ret)
goto err_drm_fb_helper_fini;
- vga_switcheroo_client_fb_set(rdev->pdev, fb_helper->info);
-
return 0;
err_drm_fb_helper_fini:
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 79525ee90e3b2..57b21b6869231 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -64,6 +64,7 @@
#include <linux/console.h>
#include <linux/string.h>
#include <linux/kd.h>
+#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/fb.h>
#include <linux/fbcon.h>
@@ -75,6 +76,7 @@
#include <linux/interrupt.h>
#include <linux/crc32.h> /* For counting font checksums */
#include <linux/uaccess.h>
+#include <linux/vga_switcheroo.h>
#include <asm/irq.h>
#include "fbcon.h"
@@ -2909,6 +2911,9 @@ void fbcon_fb_unregistered(struct fb_info *info)
console_lock();
+ if (info->device && dev_is_pci(info->device))
+ vga_switcheroo_client_fb_set(to_pci_dev(info->device), info);
+
fbcon_registered_fb[info->node] = NULL;
fbcon_num_registered_fb--;
@@ -3042,6 +3047,10 @@ static int do_fb_registered(struct fb_info *info)
}
}
+ /* Set the fb info for vga_switcheroo clients. Does nothing otherwise. */
+ if (info->device && dev_is_pci(info->device))
+ vga_switcheroo_client_fb_set(to_pci_dev(info->device), NULL);
+
return ret;
}
--
2.52.0
^ permalink raw reply related
* Re: [PATCH 1/2] drm, fbcon, vga_switcheroo: Avoid race condition in fbcon setup
From: Thomas Zimmermann @ 2026-01-20 8:29 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Javier Martinez Canillas, Alex Deucher, dri-devel, nouveau,
amd-gfx, linux-fbdev, stable
In-Reply-To: <20260120082841.11381-2-tzimmermann@suse.com>
Please ignore. This patch has been merged already.
Am 20.01.26 um 09:28 schrieb Thomas Zimmermann:
> From: Thomas Zimmermann <tzimmermann@suse.de>
>
> Protect vga_switcheroo_client_fb_set() with console lock. Avoids OOB
> access in fbcon_remap_all(). Without holding the console lock the call
> races with switching outputs.
>
> VGA switcheroo calls fbcon_remap_all() when switching clients. The fbcon
> function uses struct fb_info.node, which is set by register_framebuffer().
> As the fb-helper code currently sets up VGA switcheroo before registering
> the framebuffer, the value of node is -1 and therefore not a legal value.
> For example, fbcon uses the value within set_con2fb_map() [1] as an index
> into an array.
>
> Moving vga_switcheroo_client_fb_set() after register_framebuffer() can
> result in VGA switching that does not switch fbcon correctly.
>
> Therefore move vga_switcheroo_client_fb_set() under fbcon_fb_registered(),
> which already holds the console lock. Fbdev calls fbcon_fb_registered()
> from within register_framebuffer(). Serializes the helper with VGA
> switcheroo's call to fbcon_remap_all().
>
> Although vga_switcheroo_client_fb_set() takes an instance of struct fb_info
> as parameter, it really only needs the contained fbcon state. Moving the
> call to fbcon initialization is therefore cleaner than before. Only amdgpu,
> i915, nouveau and radeon support vga_switcheroo. For all other drivers,
> this change does nothing.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://elixir.bootlin.com/linux/v6.17/source/drivers/video/fbdev/core/fbcon.c#L2942 # [1]
> Fixes: 6a9ee8af344e ("vga_switcheroo: initial implementation (v15)")
> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: linux-fbdev@vger.kernel.org
> Cc: <stable@vger.kernel.org> # v2.6.34+
> Link: https://patch.msgid.link/20251105161549.98836-1-tzimmermann@suse.de
> (cherry picked from commit eb76d0f5553575599561010f24c277cc5b31d003)
> ---
> drivers/gpu/drm/drm_fb_helper.c | 6 ------
> drivers/gpu/drm/i915/display/intel_fbdev.c | 2 --
> drivers/gpu/drm/radeon/radeon_fbdev.c | 3 ---
> drivers/video/fbdev/core/fbcon.c | 9 +++++++++
> 4 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b507c1c008a3e..3915a8b17bc4c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -32,7 +32,6 @@
> #include <linux/console.h>
> #include <linux/pci.h>
> #include <linux/sysrq.h>
> -#include <linux/vga_switcheroo.h>
>
> #include <drm/drm_atomic.h>
> #include <drm/drm_drv.h>
> @@ -1668,7 +1667,6 @@ static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper,
> static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper)
> {
> struct drm_client_dev *client = &fb_helper->client;
> - struct drm_device *dev = fb_helper->dev;
> struct drm_fb_helper_surface_size sizes;
> int ret;
>
> @@ -1687,10 +1685,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper)
>
> strcpy(fb_helper->fb->comm, "[fbcon]");
>
> - /* Set the fb info for vgaswitcheroo clients. Does nothing otherwise. */
> - if (dev_is_pci(dev->dev))
> - vga_switcheroo_client_fb_set(to_pci_dev(dev->dev), fb_helper->info);
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 31d0d695d5671..8bad7e7909f7b 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -36,7 +36,6 @@
> #include <linux/string.h>
> #include <linux/sysrq.h>
> #include <linux/tty.h>
> -#include <linux/vga_switcheroo.h>
>
> #include <drm/drm_crtc.h>
> #include <drm/drm_fb_helper.h>
> @@ -337,7 +336,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
> ifbdev->vma_flags = flags;
>
> intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> - vga_switcheroo_client_fb_set(pdev, info);
> return 0;
>
> out_unpin:
> diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c
> index fb70de29545c6..730efabe05f6b 100644
> --- a/drivers/gpu/drm/radeon/radeon_fbdev.c
> +++ b/drivers/gpu/drm/radeon/radeon_fbdev.c
> @@ -303,7 +303,6 @@ static void radeon_fbdev_client_unregister(struct drm_client_dev *client)
> struct radeon_device *rdev = dev->dev_private;
>
> if (fb_helper->info) {
> - vga_switcheroo_client_fb_set(rdev->pdev, NULL);
> drm_helper_force_disable_all(dev);
> drm_fb_helper_unregister_info(fb_helper);
> } else {
> @@ -342,8 +341,6 @@ static int radeon_fbdev_client_hotplug(struct drm_client_dev *client)
> if (ret)
> goto err_drm_fb_helper_fini;
>
> - vga_switcheroo_client_fb_set(rdev->pdev, fb_helper->info);
> -
> return 0;
>
> err_drm_fb_helper_fini:
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 79525ee90e3b2..57b21b6869231 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -64,6 +64,7 @@
> #include <linux/console.h>
> #include <linux/string.h>
> #include <linux/kd.h>
> +#include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/fb.h>
> #include <linux/fbcon.h>
> @@ -75,6 +76,7 @@
> #include <linux/interrupt.h>
> #include <linux/crc32.h> /* For counting font checksums */
> #include <linux/uaccess.h>
> +#include <linux/vga_switcheroo.h>
> #include <asm/irq.h>
>
> #include "fbcon.h"
> @@ -2909,6 +2911,9 @@ void fbcon_fb_unregistered(struct fb_info *info)
>
> console_lock();
>
> + if (info->device && dev_is_pci(info->device))
> + vga_switcheroo_client_fb_set(to_pci_dev(info->device), info);
> +
> fbcon_registered_fb[info->node] = NULL;
> fbcon_num_registered_fb--;
>
> @@ -3042,6 +3047,10 @@ static int do_fb_registered(struct fb_info *info)
> }
> }
>
> + /* Set the fb info for vga_switcheroo clients. Does nothing otherwise. */
> + if (info->device && dev_is_pci(info->device))
> + vga_switcheroo_client_fb_set(to_pci_dev(info->device), NULL);
> +
> return ret;
> }
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v6 2/2] mfd: cgbc: Add support for backlight
From: Thomas Richard @ 2026-01-20 9:13 UTC (permalink / raw)
To: petri.karhula, Lee Jones, Daniel Thompson, Jingoo Han,
Helge Deller
Cc: linux-kernel, dri-devel, linux-fbdev
In-Reply-To: <20251205-cgbc-backlight-v6-2-e4175b0bf406@novatron.fi>
On 12/5/25 1:19 PM, Petri Karhula via B4 Relay wrote:
> From: Petri Karhula <petri.karhula@novatron.fi>
>
> The Board Controller has control for display backlight.
> Add backlight cell for the cgbc-backlight driver which
> adds support for backlight brightness control.
>
Reviewed-by: Thomas Richard <thomas.richard@bootlin.com>
Best Regards,
Thomas
^ permalink raw reply
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: Daniel Thompson @ 2026-01-20 9:38 UTC (permalink / raw)
To: tessolveupstream
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <ec7b7af7-1343-4988-b783-9ce9b045c8ae@gmail.com>
On Tue, Jan 20, 2026 at 10:22:02AM +0530, tessolveupstream@gmail.com wrote:
>
>
> On 14-01-2026 21:33, Daniel Thompson wrote:
> > On Tue, Jan 13, 2026 at 12:47:26PM +0530, tessolveupstream@gmail.com wrote:
> >>
> >>
> >> On 05-01-2026 15:39, Daniel Thompson wrote:
> >>> On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote:
> >>>> Extend the gpio-backlight driver to handle multiple GPIOs instead of a
> >>>> single one. This allows panels that require driving several enable pins
> >>>> to be controlled by the backlight framework.
> >>>>
> >>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
> >>>> ---
> >>>> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++-------
> >>>> 1 file changed, 45 insertions(+), 16 deletions(-)
> >>>>
> >>>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> >>>> index 728a546904b0..037e1c111e48 100644
> >>>> --- a/drivers/video/backlight/gpio_backlight.c
> >>>> +++ b/drivers/video/backlight/gpio_backlight.c
> >>>> @@ -17,14 +17,18 @@
> >>>>
> >>>> struct gpio_backlight {
> >>>> struct device *dev;
> >>>> - struct gpio_desc *gpiod;
> >>>> + struct gpio_desc **gpiods;
> >>>> + unsigned int num_gpios;
> >>>
> >>> Why not use struct gpio_descs for this?
> >>>
> >>> Once you do that, then most of the gbl->num_gpios loops can be replaced with
> >>> calls to the array based accessors.
> >>>
> >>
> >> Based on your feedback, I have updated the implementation to use
> >> struct gpio_descs and array-based accessors, as recommended like
> >> below:
> >>
> >> git diff drivers/video/backlight/gpio_backlight.c
> >> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> >> index 037e1c111e48..e99d7a9dc670 100644
> >> --- a/drivers/video/backlight/gpio_backlight.c
> >> +++ b/drivers/video/backlight/gpio_backlight.c
> >> @@ -14,22 +14,37 @@
> >> #include <linux/platform_device.h>
> >> #include <linux/property.h>
> >> #include <linux/slab.h>
> >> +#include <linux/bitmap.h>
> >>
> >> struct gpio_backlight {
> >> struct device *dev;
> >> - struct gpio_desc **gpiods;
> >> + struct gpio_descs *gpiods;
> >> unsigned int num_gpios;
> >> };
> >>
> >> static int gpio_backlight_update_status(struct backlight_device *bl)
> >> {
> >> struct gpio_backlight *gbl = bl_get_data(bl);
> >> - unsigned int i;
> >> + unsigned int n = gbl->num_gpios;
> >> int br = backlight_get_brightness(bl);
> >> + unsigned long *value_bitmap;
> >> + int words = BITS_TO_LONGS(n);
> >> +
> >> + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL);
> >
> > Not sure you need a kcalloc() here. If you want to support more than 32
> > GPIOs then you can pre-allocate space with a devm_kcalloc() in the probe
> > method rather than reallocate every time it is used.
> >
> > To be honest I don't really mind putting a hard limit on the maximum
> > gpl->num_gpios (so you can just use a local variable) and having no
> > allocation at all.
> >
>
> Thanks for the suggestion. I addressed the kcalloc() concern by
> moving the bitmap allocation to probe using devm_kcalloc() as
> below:
>
> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> index 0eb42d8bf1d9..7af5dc4f0315 100644
> --- a/drivers/video/backlight/gpio_backlight.c
> +++ b/drivers/video/backlight/gpio_backlight.c
> @@ -19,32 +19,25 @@
> struct gpio_backlight {
> struct device *dev;
> struct gpio_descs *gpiods;
> - unsigned int num_gpios;
> + unsigned long *bitmap;
> };
>
> static int gpio_backlight_update_status(struct backlight_device *bl)
> {
> struct gpio_backlight *gbl = bl_get_data(bl);
> - unsigned int n = gbl->num_gpios;
> + unsigned int n = gbl->gpiods->ndescs;
> int br = backlight_get_brightness(bl);
> - unsigned long *value_bitmap;
> - int words = BITS_TO_LONGS(n);
> -
> - value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL);
> - if (!value_bitmap)
> - return -ENOMEM;
>
> if (br)
> - bitmap_fill(value_bitmap, n);
> + bitmap_fill(gbl->bitmap, n);
> else
> - bitmap_zero(value_bitmap, n);
> + bitmap_zero(gbl->bitmap, n);
>
> - gpiod_set_array_value_cansleep(gbl->gpiods->ndescs,
> + gpiod_set_array_value_cansleep(n,
> gbl->gpiods->desc,
> gbl->gpiods->info,
> - value_bitmap);
> + gbl->bitmap);
>
> - kfree(value_bitmap);
> return 0;
> }
>
> @@ -67,22 +60,25 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
> struct device_node *of_node = dev->of_node;
> - struct backlight_properties props;
> + struct backlight_properties props = { };
> struct backlight_device *bl;
> struct gpio_backlight *gbl;
> - int ret, init_brightness, def_value;
> - unsigned int i;
> + bool def_value;
> + enum gpiod_flags flags;
> + unsigned int n;
> + int words;
>
> - gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
> - if (gbl == NULL)
> + gbl = devm_kcalloc(dev, 1, sizeof(*gbl), GFP_KERNEL);
> + if (!gbl)
> return -ENOMEM;
>
> if (pdata)
> gbl->dev = pdata->dev;
>
> def_value = device_property_read_bool(dev, "default-on");
> -
> - gbl->gpiods = devm_gpiod_get_array(dev, NULL, GPIOD_ASIS);
> + flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
> +
> + gbl->gpiods = devm_gpiod_get_array(dev, NULL, flags);
> if (IS_ERR(gbl->gpiods)) {
> if (PTR_ERR(gbl->gpiods) == -ENODEV)
> return dev_err_probe(dev, -EINVAL,
> @@ -90,12 +86,17 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> return PTR_ERR(gbl->gpiods);
> }
>
> - gbl->num_gpios = gbl->gpiods->ndescs;
> - if (gbl->num_gpios == 0)
> + n = gbl->gpiods->ndescs;
> + if (!n)
> return dev_err_probe(dev, -EINVAL,
> - "The gpios parameter is missing or invalid\n");
> + "No GPIOs provided\n");
> +
> + words = BITS_TO_LONGS(n);
> + gbl->bitmap = devm_kcalloc(dev, words, sizeof(unsigned long),
> + GFP_KERNEL);
> + if (!gbl->bitmap)
> + return -ENOMEM;
>
> - memset(&props, 0, sizeof(props));
> props.type = BACKLIGHT_RAW;
> props.max_brightness = 1;
> bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl,
> @@ -106,50 +107,19 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> }
>
> /* Set the initial power state */
> - if (!of_node || !of_node->phandle) {
> + if (!of_node || !of_node->phandle)
> /* Not booted with device tree or no phandle link to the node */
> bl->props.power = def_value ? BACKLIGHT_POWER_ON
> : BACKLIGHT_POWER_OFF;
> - } else {
> - bool all_high = true;
> - unsigned long *value_bitmap;
> - int words = BITS_TO_LONGS(gbl->num_gpios);
> -
> - value_bitmap = kcalloc(words, sizeof(unsigned long),
> - GFP_KERNEL);
> - if (!value_bitmap)
> - return -ENOMEM;
> -
> - ret = gpiod_get_array_value_cansleep(gbl->gpiods->ndescs,
> - gbl->gpiods->desc,
> - gbl->gpiods->info,
> - value_bitmap);
> - if (ret) {
> - kfree(value_bitmap);
> - return dev_err_probe(dev, ret,
> - "failed to read initial gpio values\n");
> - }
> -
> - all_high = bitmap_full(value_bitmap, gbl->num_gpios);
> -
> - kfree(value_bitmap);
> - bl->props.power =
> - all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
> - }
> -
> - bl->props.brightness = 1;
> -
> - init_brightness = backlight_get_brightness(bl);
> + else if (gpiod_get_value_cansleep(gbl->gpiods->desc[0]) == 0)
> + bl->props.power = BACKLIGHT_POWER_OFF;
> + else
> + bl->props.power = BACKLIGHT_POWER_ON;
>
> - for (i = 0; i < gbl->num_gpios; i++) {
> - ret = gpiod_direction_output(gbl->gpiods->desc[i],
> - init_brightness);
> - if (ret)
> - return dev_err_probe(dev, ret,
> - "failed to set gpio %u direction\n",
> - i);
> - }
> + bl->props.brightness = def_value ? 1 : 0;
>
> + gpio_backlight_update_status(bl);
> +
> platform_set_drvdata(pdev, bl);
> return 0;
> }
>
> Kindly confirm whether this approach aligns with your
> expectations.
As mentioned yesterday, I'd rather just review a v2 patch than this kind of
meta-patch. Please send a v2 patch instead.
Daniel.
^ permalink raw reply
* [PATCH v2 0/2] backlight: gpio: add support for multiple GPIOs for backlight control
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
* [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
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
In-Reply-To: <20260120125036.2203995-1-tessolveupstream@gmail.com>
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
* [PATCH v2 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
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
In-Reply-To: <20260120125036.2203995-1-tessolveupstream@gmail.com>
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
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: tessolveupstream @ 2026-01-20 12:53 UTC (permalink / raw)
To: Daniel Thompson
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <aW9NH5GTwSR-m7VQ@aspen.lan>
On 20-01-2026 15:08, Daniel Thompson wrote:
> On Tue, Jan 20, 2026 at 10:22:02AM +0530, tessolveupstream@gmail.com wrote:
>>
>>
>> On 14-01-2026 21:33, Daniel Thompson wrote:
>>> On Tue, Jan 13, 2026 at 12:47:26PM +0530, tessolveupstream@gmail.com wrote:
>>>>
>>>>
>>>> On 05-01-2026 15:39, Daniel Thompson wrote:
>>>>> On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote:
>>>>>> Extend the gpio-backlight driver to handle multiple GPIOs instead of a
>>>>>> single one. This allows panels that require driving several enable pins
>>>>>> to be controlled by the backlight framework.
>>>>>>
>>>>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>>>>>> ---
>>>>>> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++-------
>>>>>> 1 file changed, 45 insertions(+), 16 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
>>>>>> index 728a546904b0..037e1c111e48 100644
>>>>>> --- a/drivers/video/backlight/gpio_backlight.c
>>>>>> +++ b/drivers/video/backlight/gpio_backlight.c
>>>>>> @@ -17,14 +17,18 @@
>>>>>>
>>>>>> struct gpio_backlight {
>>>>>> struct device *dev;
>>>>>> - struct gpio_desc *gpiod;
>>>>>> + struct gpio_desc **gpiods;
>>>>>> + unsigned int num_gpios;
>>>>>
>>>>> Why not use struct gpio_descs for this?
>>>>>
>>>>> Once you do that, then most of the gbl->num_gpios loops can be replaced with
>>>>> calls to the array based accessors.
>>>>>
>>>>
>>>> Based on your feedback, I have updated the implementation to use
>>>> struct gpio_descs and array-based accessors, as recommended like
>>>> below:
>>>>
>>>> git diff drivers/video/backlight/gpio_backlight.c
>>>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
>>>> index 037e1c111e48..e99d7a9dc670 100644
>>>> --- a/drivers/video/backlight/gpio_backlight.c
>>>> +++ b/drivers/video/backlight/gpio_backlight.c
>>>> @@ -14,22 +14,37 @@
>>>> #include <linux/platform_device.h>
>>>> #include <linux/property.h>
>>>> #include <linux/slab.h>
>>>> +#include <linux/bitmap.h>
>>>>
>>>> struct gpio_backlight {
>>>> struct device *dev;
>>>> - struct gpio_desc **gpiods;
>>>> + struct gpio_descs *gpiods;
>>>> unsigned int num_gpios;
>>>> };
>>>>
>>>> static int gpio_backlight_update_status(struct backlight_device *bl)
>>>> {
>>>> struct gpio_backlight *gbl = bl_get_data(bl);
>>>> - unsigned int i;
>>>> + unsigned int n = gbl->num_gpios;
>>>> int br = backlight_get_brightness(bl);
>>>> + unsigned long *value_bitmap;
>>>> + int words = BITS_TO_LONGS(n);
>>>> +
>>>> + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL);
>>>
>>> Not sure you need a kcalloc() here. If you want to support more than 32
>>> GPIOs then you can pre-allocate space with a devm_kcalloc() in the probe
>>> method rather than reallocate every time it is used.
>>>
>>> To be honest I don't really mind putting a hard limit on the maximum
>>> gpl->num_gpios (so you can just use a local variable) and having no
>>> allocation at all.
>>>
>>
>> Thanks for the suggestion. I addressed the kcalloc() concern by
>> moving the bitmap allocation to probe using devm_kcalloc() as
>> below:
>>
>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
>> index 0eb42d8bf1d9..7af5dc4f0315 100644
>> --- a/drivers/video/backlight/gpio_backlight.c
>> +++ b/drivers/video/backlight/gpio_backlight.c
>> @@ -19,32 +19,25 @@
>> struct gpio_backlight {
>> struct device *dev;
>> struct gpio_descs *gpiods;
>> - unsigned int num_gpios;
>> + unsigned long *bitmap;
>> };
>>
>> static int gpio_backlight_update_status(struct backlight_device *bl)
>> {
>> struct gpio_backlight *gbl = bl_get_data(bl);
>> - unsigned int n = gbl->num_gpios;
>> + unsigned int n = gbl->gpiods->ndescs;
>> int br = backlight_get_brightness(bl);
>> - unsigned long *value_bitmap;
>> - int words = BITS_TO_LONGS(n);
>> -
>> - value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL);
>> - if (!value_bitmap)
>> - return -ENOMEM;
>>
>> if (br)
>> - bitmap_fill(value_bitmap, n);
>> + bitmap_fill(gbl->bitmap, n);
>> else
>> - bitmap_zero(value_bitmap, n);
>> + bitmap_zero(gbl->bitmap, n);
>>
>> - gpiod_set_array_value_cansleep(gbl->gpiods->ndescs,
>> + gpiod_set_array_value_cansleep(n,
>> gbl->gpiods->desc,
>> gbl->gpiods->info,
>> - value_bitmap);
>> + gbl->bitmap);
>>
>> - kfree(value_bitmap);
>> return 0;
>> }
>>
>> @@ -67,22 +60,25 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>> struct device *dev = &pdev->dev;
>> struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
>> struct device_node *of_node = dev->of_node;
>> - struct backlight_properties props;
>> + struct backlight_properties props = { };
>> struct backlight_device *bl;
>> struct gpio_backlight *gbl;
>> - int ret, init_brightness, def_value;
>> - unsigned int i;
>> + bool def_value;
>> + enum gpiod_flags flags;
>> + unsigned int n;
>> + int words;
>>
>> - gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
>> - if (gbl == NULL)
>> + gbl = devm_kcalloc(dev, 1, sizeof(*gbl), GFP_KERNEL);
>> + if (!gbl)
>> return -ENOMEM;
>>
>> if (pdata)
>> gbl->dev = pdata->dev;
>>
>> def_value = device_property_read_bool(dev, "default-on");
>> -
>> - gbl->gpiods = devm_gpiod_get_array(dev, NULL, GPIOD_ASIS);
>> + flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
>> +
>> + gbl->gpiods = devm_gpiod_get_array(dev, NULL, flags);
>> if (IS_ERR(gbl->gpiods)) {
>> if (PTR_ERR(gbl->gpiods) == -ENODEV)
>> return dev_err_probe(dev, -EINVAL,
>> @@ -90,12 +86,17 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>> return PTR_ERR(gbl->gpiods);
>> }
>>
>> - gbl->num_gpios = gbl->gpiods->ndescs;
>> - if (gbl->num_gpios == 0)
>> + n = gbl->gpiods->ndescs;
>> + if (!n)
>> return dev_err_probe(dev, -EINVAL,
>> - "The gpios parameter is missing or invalid\n");
>> + "No GPIOs provided\n");
>> +
>> + words = BITS_TO_LONGS(n);
>> + gbl->bitmap = devm_kcalloc(dev, words, sizeof(unsigned long),
>> + GFP_KERNEL);
>> + if (!gbl->bitmap)
>> + return -ENOMEM;
>>
>> - memset(&props, 0, sizeof(props));
>> props.type = BACKLIGHT_RAW;
>> props.max_brightness = 1;
>> bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl,
>> @@ -106,50 +107,19 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>> }
>>
>> /* Set the initial power state */
>> - if (!of_node || !of_node->phandle) {
>> + if (!of_node || !of_node->phandle)
>> /* Not booted with device tree or no phandle link to the node */
>> bl->props.power = def_value ? BACKLIGHT_POWER_ON
>> : BACKLIGHT_POWER_OFF;
>> - } else {
>> - bool all_high = true;
>> - unsigned long *value_bitmap;
>> - int words = BITS_TO_LONGS(gbl->num_gpios);
>> -
>> - value_bitmap = kcalloc(words, sizeof(unsigned long),
>> - GFP_KERNEL);
>> - if (!value_bitmap)
>> - return -ENOMEM;
>> -
>> - ret = gpiod_get_array_value_cansleep(gbl->gpiods->ndescs,
>> - gbl->gpiods->desc,
>> - gbl->gpiods->info,
>> - value_bitmap);
>> - if (ret) {
>> - kfree(value_bitmap);
>> - return dev_err_probe(dev, ret,
>> - "failed to read initial gpio values\n");
>> - }
>> -
>> - all_high = bitmap_full(value_bitmap, gbl->num_gpios);
>> -
>> - kfree(value_bitmap);
>> - bl->props.power =
>> - all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
>> - }
>> -
>> - bl->props.brightness = 1;
>> -
>> - init_brightness = backlight_get_brightness(bl);
>> + else if (gpiod_get_value_cansleep(gbl->gpiods->desc[0]) == 0)
>> + bl->props.power = BACKLIGHT_POWER_OFF;
>> + else
>> + bl->props.power = BACKLIGHT_POWER_ON;
>>
>> - for (i = 0; i < gbl->num_gpios; i++) {
>> - ret = gpiod_direction_output(gbl->gpiods->desc[i],
>> - init_brightness);
>> - if (ret)
>> - return dev_err_probe(dev, ret,
>> - "failed to set gpio %u direction\n",
>> - i);
>> - }
>> + bl->props.brightness = def_value ? 1 : 0;
>>
>> + gpio_backlight_update_status(bl);
>> +
>> platform_set_drvdata(pdev, bl);
>> return 0;
>> }
>>
>> Kindly confirm whether this approach aligns with your
>> expectations.
>
> As mentioned yesterday, I'd rather just review a v2 patch than this kind of
> meta-patch. Please send a v2 patch instead.
>
Got it, will send v2 patch.
>
> Daniel.
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
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
In-Reply-To: <20260120125036.2203995-2-tessolveupstream@gmail.com>
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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox