* [PATCH v4 0/4] fbdev: Make CONFIG_FB_DEVICE optional for drivers
From: Chintan Patel @ 2026-01-07 4:42 UTC (permalink / raw)
To: linux-fbdev, linux-staging, linux-omap
Cc: linux-kernel, dri-devel, tzimmermann, andy, deller, gregkh,
Chintan Patel
This series makes CONFIG_FB_DEVICE optional for fbdev drivers that use
it only for sysfs interfaces, addressing Thomas Zimmermann’s TODO to
remove hard FB_DEVICE dependencies.
The series introduces a small helper, dev_of_fbinfo(), which returns
NULL when CONFIG_FB_DEVICE=n. This allows sysfs code paths to be skipped
via runtime checks, avoids #ifdef CONFIG_FB_DEVICE clutter, and keeps
full compile-time syntax checking.
Signed-off-by: Chintan Patel <chintanlike@gmail.com>
---
Changes in v4:
- PTR_IF() was removed and overlay sysfs is now optional via __maybe_unused
and #ifdef CONFIG_FB_DEVICE (suggested by Helge Deller)
- Decouple variable definition and assignment in
omapfb_remove/create_sysfs (suggested by Andy Shevchenko)
- Added Reviewed-by tags:
- fb: Add dev_of_fbinfo(): Helge Deller, Andy Shevchenko
- staging: fbtft: Helge Deller
Changes in v3:
- Use PTR_IF() to conditionally include overlay_sysfs_group in
overlay_sysfs_groups(suggested by Andy Shevchenko)
- Decouple variable definition and assignment in
fbtft_sysfs_init/exit(suggested by Andy Shevchenko)
Changes in v2:
- Add dev_of_fbinfo() helper (suggested by Helge Deller)
- Replace #ifdef CONFIG_FB_DEVICE blocks with runtime NULL checks
- Switch to fb_dbg() / fb_info() logging (suggested by Thomas Zimmermann)
---
Chintan Patel (4):
fb: Add dev_of_fbinfo() helper for optional sysfs support
staging: fbtft: Make FB_DEVICE dependency optional
fbdev: omapfb: Make FB_DEVICE dependency optional
fbdev: sh_mobile_lcdc: Make FB_DEVICE dependency optional
drivers/staging/fbtft/Kconfig | 5 ++++-
drivers/staging/fbtft/fbtft-sysfs.c | 20 +++++++++++++++----
drivers/video/fbdev/omap2/omapfb/Kconfig | 3 ++-
.../video/fbdev/omap2/omapfb/omapfb-sysfs.c | 18 +++++++++++++----
drivers/video/fbdev/sh_mobile_lcdcfb.c | 5 ++++-
include/linux/fb.h | 9 +++++++++
6 files changed, 49 insertions(+), 11 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v2 6/6] video/logo: move logo selection logic to Kconfig
From: Finn Thain @ 2026-01-06 22:16 UTC (permalink / raw)
To: Vincent Mailhol
Cc: Geert Uytterhoeven, Helge Deller, Greg Kroah-Hartman,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
linux-fbdev, dri-devel, linux-kernel, linux-sh, linux-m68k
In-Reply-To: <dda4052e-b843-43fa-850c-a1bb20e4a8e3@kernel.org>
On Tue, 6 Jan 2026, Vincent Mailhol wrote:
> >
> > MACH_IS_MAC can be a runtime check.
>
> OK. I missed this.
>
You can use "qemu-system-m68k -M q800 ..." to run the code you're
interested in.
$ qemu-system-m68k -M q800 -g 800x600x16
qemu-system-m68k: unknown display mode: width 800, height 600, depth 16
Available modes:
640x480x1
640x480x2
640x480x4
640x480x8
640x480x24
800x600x1
800x600x2
800x600x4
800x600x8
800x600x24
1152x870x1
1152x870x2
1152x870x4
1152x870x8
^ permalink raw reply
* Re: [PATCH v2 6/6] video/logo: move logo selection logic to Kconfig
From: Vincent Mailhol @ 2026-01-06 20:10 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Helge Deller, Greg Kroah-Hartman, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, linux-fbdev, dri-devel, linux-kernel,
linux-sh, linux-m68k
In-Reply-To: <CAMuHMdVy48F5HAfqfJgbY83KDAztb9YWTqm8mT1ntTfj0311oA@mail.gmail.com>
On 06/01/2026 at 12:48, Geert Uytterhoeven wrote:
> Hi Vincent,
>
> CC linux-m68k
>
> Thanks for your patch, which is now commit bd710b3da7308cb1
> ("video/logo: move logo selection logic to Kconfig") in fbdev/for-next.
>
> On Thu, 1 Jan 2026 at 16:26, Vincent Mailhol <mailhol@kernel.org> wrote:
>> Now that the path to the logo file can be directly entered in Kbuild,
>> there is no more need to handle all the logo file selection in the
>> Makefile and the C files.
>
> This may do the wrong thing when booting a multi-platform kernel.
>
>>
>> The only exception is the logo_spe_clut224 which is only used by the
>> Cell processor (found for example in the Playstation 3) [1]. This
>> extra logo uses its own different image which shows up on a separate
>> line just below the normal logo. Because the extra logo uses a
>> different image, it can not be factorized under the custom logo logic.
>>
>> Move all the logo file selection logic to Kbuild (except from the
>> logo_spe_clut224.ppm), this done, clean-up the C code to only leave
>> one entry for each logo type (monochrome, 16-colors and 224-colors).
>>
>> [1] Cell SPE logos
>> Link: https://lore.kernel.org/all/20070710122702.765654000@pademelon.sonytel.be/
>>
>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
>
>> --- a/drivers/video/logo/Kconfig
>> +++ b/drivers/video/logo/Kconfig
>
>> @@ -61,6 +63,12 @@ config LOGO_LINUX_CLUT224
>> config LOGO_LINUX_CLUT224_FILE
>> string "224-color logo .ppm file"
>> depends on LOGO_LINUX_CLUT224
>> + default "drivers/video/logo/logo_dec_clut224.ppm" if MACH_DECSTATION || ALPHA
>> + default "drivers/video/logo/logo_mac_clut224.ppm" if MAC
>
> E.g. an m68k multi-platform kernel including Mac support will scare
> non-Mac users into thinking their machine was assimilated by the
> Apple Empire...
>
>> + default "drivers/video/logo/logo_parisc_clut224.ppm" if PARISC
>> + default "drivers/video/logo/logo_sgi_clut224.ppm" if SGI_IP22 || SGI_IP27 || SGI_IP32
>> + default "drivers/video/logo/logo_sun_clut224.ppm" if SPARC
>> + default "drivers/video/logo/logo_superh_clut224.ppm" if SUPERH
>> default "drivers/video/logo/logo_linux_clut224.ppm"
>> help
>> Takes a path to a 224-color logo in the portable pixmap file
>
>> --- a/drivers/video/logo/logo.c
>> +++ b/drivers/video/logo/logo.c
>> @@ -48,59 +48,21 @@ const struct linux_logo * __ref fb_find_logo(int depth)
>> if (nologo || logos_freed)
>> return NULL;
>>
>> - if (depth >= 1) {
>> #ifdef CONFIG_LOGO_LINUX_MONO
>> - /* Generic Linux logo */
>> + if (depth >= 1)
>> logo = &logo_linux_mono;
>> #endif
>> -#ifdef CONFIG_LOGO_SUPERH_MONO
>> - /* SuperH Linux logo */
>> - logo = &logo_superh_mono;
>> -#endif
>> - }
>>
>> - if (depth >= 4) {
>> #ifdef CONFIG_LOGO_LINUX_VGA16
>> - /* Generic Linux logo */
>> + if (depth >= 4)
>> logo = &logo_linux_vga16;
>> #endif
>> -#ifdef CONFIG_LOGO_SUPERH_VGA16
>> - /* SuperH Linux logo */
>> - logo = &logo_superh_vga16;
>> -#endif
>> - }
>>
>> - if (depth >= 8) {
>> #ifdef CONFIG_LOGO_LINUX_CLUT224
>> - /* Generic Linux logo */
>> + if (depth >= 8)
>> logo = &logo_linux_clut224;
>> #endif
>> -#ifdef CONFIG_LOGO_DEC_CLUT224
>> - /* DEC Linux logo on MIPS/MIPS64 or ALPHA */
>> - logo = &logo_dec_clut224;
>> -#endif
>> -#ifdef CONFIG_LOGO_MAC_CLUT224
>> - /* Macintosh Linux logo on m68k */
>> - if (MACH_IS_MAC)
>
> MACH_IS_MAC can be a runtime check.
OK. I missed this.
I think there are two options to fix this:
1. Keep CONFIG_LOGO_MAC_CLUT224 untouched
2. Remove logo_mac_clut224.ppm
The first option is less controversial but I would like to ask you what
you think about removing the logo_mac_clut224 file.
Here, we are speaking of the Macintosh 68k which ended sales in 1995,
right? So the user base should be rather small, I guess.
And people who still want the custom MAC logo would still be able to add
CONFIG_LOGO_MAC_CLUT224="path/to/logo_mac_clut224.ppm"
to their config to restore the old behaviour anyway.
My choice would go more toward the removal option but what do you think?
>> - logo = &logo_mac_clut224;
>> -#endif
>> -#ifdef CONFIG_LOGO_PARISC_CLUT224
>> - /* PA-RISC Linux logo */
>> - logo = &logo_parisc_clut224;
>> -#endif
>> -#ifdef CONFIG_LOGO_SGI_CLUT224
>> - /* SGI Linux logo on MIPS/MIPS64 */
>> - logo = &logo_sgi_clut224;
>> -#endif
>> -#ifdef CONFIG_LOGO_SUN_CLUT224
>> - /* Sun Linux logo */
>> - logo = &logo_sun_clut224;
>> -#endif
>> -#ifdef CONFIG_LOGO_SUPERH_CLUT224
>> - /* SuperH Linux logo */
>> - logo = &logo_superh_clut224;
>> -#endif
>> - }
>> +
>> return logo;
>> }
>> EXPORT_SYMBOL_GPL(fb_find_logo);
Yours sincerely,
Vincent Mailhol
^ permalink raw reply
* Re: [PATCH v2 6/6] video/logo: move logo selection logic to Kconfig
From: Daniel Palmer @ 2026-01-06 12:56 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Vincent Mailhol, Helge Deller, Greg Kroah-Hartman, Yoshinori Sato,
Rich Felker, John Paul Adrian Glaubitz, linux-fbdev, dri-devel,
linux-kernel, linux-sh, linux-m68k
In-Reply-To: <CAMuHMdVy48F5HAfqfJgbY83KDAztb9YWTqm8mT1ntTfj0311oA@mail.gmail.com>
On Tue, 6 Jan 2026 at 20:54, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> E.g. an m68k multi-platform kernel including Mac support will scare
> non-Mac users into thinking their machine was assimilated by the
> Apple Empire...
>
This is actually how I tell if my Amiga or Mac is currently connected
to my monitor... Amiga has the normal tux, Mac has the tux inside of a
monitor.
^ permalink raw reply
* Re: [PATCH v2 6/6] video/logo: move logo selection logic to Kconfig
From: Geert Uytterhoeven @ 2026-01-06 11:48 UTC (permalink / raw)
To: Vincent Mailhol
Cc: Helge Deller, Greg Kroah-Hartman, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, linux-fbdev, dri-devel, linux-kernel,
linux-sh, linux-m68k
In-Reply-To: <20260101-custom-logo-v2-6-8eec06dfbf85@kernel.org>
Hi Vincent,
CC linux-m68k
Thanks for your patch, which is now commit bd710b3da7308cb1
("video/logo: move logo selection logic to Kconfig") in fbdev/for-next.
On Thu, 1 Jan 2026 at 16:26, Vincent Mailhol <mailhol@kernel.org> wrote:
> Now that the path to the logo file can be directly entered in Kbuild,
> there is no more need to handle all the logo file selection in the
> Makefile and the C files.
This may do the wrong thing when booting a multi-platform kernel.
>
> The only exception is the logo_spe_clut224 which is only used by the
> Cell processor (found for example in the Playstation 3) [1]. This
> extra logo uses its own different image which shows up on a separate
> line just below the normal logo. Because the extra logo uses a
> different image, it can not be factorized under the custom logo logic.
>
> Move all the logo file selection logic to Kbuild (except from the
> logo_spe_clut224.ppm), this done, clean-up the C code to only leave
> one entry for each logo type (monochrome, 16-colors and 224-colors).
>
> [1] Cell SPE logos
> Link: https://lore.kernel.org/all/20070710122702.765654000@pademelon.sonytel.be/
>
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> --- a/drivers/video/logo/Kconfig
> +++ b/drivers/video/logo/Kconfig
> @@ -61,6 +63,12 @@ config LOGO_LINUX_CLUT224
> config LOGO_LINUX_CLUT224_FILE
> string "224-color logo .ppm file"
> depends on LOGO_LINUX_CLUT224
> + default "drivers/video/logo/logo_dec_clut224.ppm" if MACH_DECSTATION || ALPHA
> + default "drivers/video/logo/logo_mac_clut224.ppm" if MAC
E.g. an m68k multi-platform kernel including Mac support will scare
non-Mac users into thinking their machine was assimilated by the
Apple Empire...
> + default "drivers/video/logo/logo_parisc_clut224.ppm" if PARISC
> + default "drivers/video/logo/logo_sgi_clut224.ppm" if SGI_IP22 || SGI_IP27 || SGI_IP32
> + default "drivers/video/logo/logo_sun_clut224.ppm" if SPARC
> + default "drivers/video/logo/logo_superh_clut224.ppm" if SUPERH
> default "drivers/video/logo/logo_linux_clut224.ppm"
> help
> Takes a path to a 224-color logo in the portable pixmap file
> --- a/drivers/video/logo/logo.c
> +++ b/drivers/video/logo/logo.c
> @@ -48,59 +48,21 @@ const struct linux_logo * __ref fb_find_logo(int depth)
> if (nologo || logos_freed)
> return NULL;
>
> - if (depth >= 1) {
> #ifdef CONFIG_LOGO_LINUX_MONO
> - /* Generic Linux logo */
> + if (depth >= 1)
> logo = &logo_linux_mono;
> #endif
> -#ifdef CONFIG_LOGO_SUPERH_MONO
> - /* SuperH Linux logo */
> - logo = &logo_superh_mono;
> -#endif
> - }
>
> - if (depth >= 4) {
> #ifdef CONFIG_LOGO_LINUX_VGA16
> - /* Generic Linux logo */
> + if (depth >= 4)
> logo = &logo_linux_vga16;
> #endif
> -#ifdef CONFIG_LOGO_SUPERH_VGA16
> - /* SuperH Linux logo */
> - logo = &logo_superh_vga16;
> -#endif
> - }
>
> - if (depth >= 8) {
> #ifdef CONFIG_LOGO_LINUX_CLUT224
> - /* Generic Linux logo */
> + if (depth >= 8)
> logo = &logo_linux_clut224;
> #endif
> -#ifdef CONFIG_LOGO_DEC_CLUT224
> - /* DEC Linux logo on MIPS/MIPS64 or ALPHA */
> - logo = &logo_dec_clut224;
> -#endif
> -#ifdef CONFIG_LOGO_MAC_CLUT224
> - /* Macintosh Linux logo on m68k */
> - if (MACH_IS_MAC)
MACH_IS_MAC can be a runtime check.
> - logo = &logo_mac_clut224;
> -#endif
> -#ifdef CONFIG_LOGO_PARISC_CLUT224
> - /* PA-RISC Linux logo */
> - logo = &logo_parisc_clut224;
> -#endif
> -#ifdef CONFIG_LOGO_SGI_CLUT224
> - /* SGI Linux logo on MIPS/MIPS64 */
> - logo = &logo_sgi_clut224;
> -#endif
> -#ifdef CONFIG_LOGO_SUN_CLUT224
> - /* Sun Linux logo */
> - logo = &logo_sun_clut224;
> -#endif
> -#ifdef CONFIG_LOGO_SUPERH_CLUT224
> - /* SuperH Linux logo */
> - logo = &logo_superh_clut224;
> -#endif
> - }
> +
> return logo;
> }
> EXPORT_SYMBOL_GPL(fb_find_logo);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2] fbdev: bitblit: bound-check glyph index in bit_putcs*
From: Thorsten Leemhuis @ 2026-01-06 9:04 UTC (permalink / raw)
To: Barry K. Nathan, Vitaly Chikunov, Junjie Cao, Thomas Zimmermann,
Greg Kroah-Hartman
Cc: Peilin Ye, Daniel Vetter, Shigeru Yoshida, Simona Vetter,
Helge Deller, Zsolt Kajtar, Albin Babu Varghese, linux-fbdev,
dri-devel, linux-kernel, stable, regressions, Ben Hutchings
In-Reply-To: <ccbbf777-cf4e-4c66-856e-282dd9d61970@pobox.com>
[Top posting to make this easy processable]
TWIMC, Ben (now CCed) meanwhile reported the problem as well:
https://lore.kernel.org/all/c5a27a57597c78553bf121d09a1b45ed86dc02a8.camel@decadent.org.uk/
There he wrote
"""
This can be fixed by backporting the following commits from 5.11:
7a089ec7d77f console: Delete unused con_font_copy() callback implementations
259a252c1f4e console: Delete dummy con_font_set() and con_font_default()
callback implementations
4ee573086bd8 Fonts: Add charcount field to font_desc
4497364e5f61 parisc/sticore: Avoid hard-coding built-in font charcount
a1ac250a82a5 fbcon: Avoid using FNTCHARCNT() and hard-coded built-in
font charcount
These all apply without fuzz and builds cleanly for x86_64 and parisc64.
"""
Ciao, Thorsten
On 12/27/25 03:04, Barry K. Nathan wrote:
> On 12/26/25 4:21 AM, Vitaly Chikunov wrote:
>> Dear linux-fbdev, stable,
>>
>> On Fri, Dec 26, 2025 at 01:29:13AM +0300, Vitaly Chikunov wrote:
>>>
>>> On Mon, Oct 20, 2025 at 09:47:01PM +0800, Junjie Cao wrote:
>>>> bit_putcs_aligned()/unaligned() derived the glyph pointer from the
>>>> character value masked by 0xff/0x1ff, which may exceed the actual
>>>> font's
>>>> glyph count and read past the end of the built-in font array.
>>>> Clamp the index to the actual glyph count before computing the address.
>>>>
>>>> This fixes a global out-of-bounds read reported by syzbot.
>>>>
>>>> Reported-by: syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>>>> Closes: https://syzkaller.appspot.com/bug?extid=793cf822d213be1a74f2
>>>> Tested-by: syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>>>> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
>>>
>>> This commit is applied to v5.10.247 and causes a regression: when
>>> switching VT with ctrl-alt-f2 the screen is blank or completely filled
>>> with angle characters, then new text is not appearing (or not visible).
>>>
>>> This commit is found with git bisect from v5.10.246 to v5.10.247:
>>>
>>> 0998a6cb232674408a03e8561dc15aa266b2f53b is the first bad commit
>>> commit 0998a6cb232674408a03e8561dc15aa266b2f53b
>>> Author: Junjie Cao <junjie.cao@intel.com>
>>> AuthorDate: 2025-10-20 21:47:01 +0800
>>> Commit: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> CommitDate: 2025-12-07 06:08:07 +0900
>>>
>>> fbdev: bitblit: bound-check glyph index in bit_putcs*
>>>
>>> commit 18c4ef4e765a798b47980555ed665d78b71aeadf upstream.
>>>
>>> bit_putcs_aligned()/unaligned() derived the glyph pointer from
>>> the
>>> character value masked by 0xff/0x1ff, which may exceed the
>>> actual font's
>>> glyph count and read past the end of the built-in font array.
>>> Clamp the index to the actual glyph count before computing the
>>> address.
>>>
>>> This fixes a global out-of-bounds read reported by syzbot.
>>>
>>> Reported-by:
>>> syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?
>>> extid=793cf822d213be1a74f2
>>> Tested-by: syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>>> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
>>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>
>>> drivers/video/fbdev/core/bitblit.c | 16 ++++++++++++----
>>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> The minimal reproducer in cli, after kernel is booted:
>>>
>>> date >/dev/tty2; chvt 2
>>>
>>> and the date does not appear.
>>>
>>> Thanks,
>>>
>>> #regzbot introduced: 0998a6cb232674408a03e8561dc15aa266b2f53b
>>>
>>>> ---
>>>> v1: https://lore.kernel.org/linux-fbdev/5d237d1a-a528-4205-
>>>> a4d8-71709134f1e1@suse.de/
>>>> v1 -> v2:
>>>> - Fix indentation and add blank line after declarations with
>>>> the .pl helper
>>>> - No functional changes
>>>>
>>>> drivers/video/fbdev/core/bitblit.c | 16 ++++++++++++----
>>>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/
>>>> fbdev/core/bitblit.c
>>>> index 9d2e59796c3e..085ffb44c51a 100644
>>>> --- a/drivers/video/fbdev/core/bitblit.c
>>>> +++ b/drivers/video/fbdev/core/bitblit.c
>>>> @@ -79,12 +79,16 @@ static inline void bit_putcs_aligned(struct
>>>> vc_data *vc, struct fb_info *info,
>>>> struct fb_image *image, u8 *buf, u8 *dst)
>>>> {
>>>> u16 charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
>>>> + unsigned int charcnt = vc->vc_font.charcount;
>>
>> Perhaps, vc->vc_font.charcount (which is relied upon in the following
>> comparison) is not always set correctly in v5.10.247. At least two
>> commits that set vc_font.charcount are missing from v5.10.247:
>>
>> a1ac250a82a5 ("fbcon: Avoid using FNTCHARCNT() and hard-coded
>> built-in font charcount")
>> a5a923038d70 ("fbdev: fbcon: Properly revert changes when
>> vc_resize() failed")
>>
>> Thanks,
>
> I was just about to report this.
>
> I found two ways to fix this bug. One is to revert this patch; the other
> is to apply the following 3 patches, which are already present in 5.11
> and later:
>
> 7a089ec7d77fe7d50f6bb7b178fa25eec9fd822b
> console: Delete unused con_font_copy() callback implementations
>
> 4ee573086bd88ff3060dda07873bf755d332e9ba
> Fonts: Add charcount field to font_desc
>
> a1ac250a82a5e97db71f14101ff7468291a6aaef
> fbcon: Avoid using FNTCHARCNT() and hard-coded built-in font
> charcount
>
> (Oh, by the way, this same regression also affects 5.4.302, and the same
> 3 patches fix the regression on 5.4 as well, once you manually fix merge
> conflicts. Maybe it would be better to backport other additional commits
> instead of fixing the merge conflicts manually, but since 5.4 is now EOL
> I didn't dig that deep.)
>
> Once these 3 patches are applied, I wonder if a5a923038d70 now becomes
> necessary for 5.10.y. For what it's worth, it applies fine and the
> resulting kernel seems to run OK in brief testing.
>
^ permalink raw reply
* Re: [PATCH v3 4/4] fbdev: sh_mobile_lcdc: Make FB_DEVICE dependency optional
From: Chintan Patel @ 2026-01-06 5:07 UTC (permalink / raw)
To: Andy Shevchenko, Helge Deller
Cc: Helge Deller, andy, linux-fbdev, linux-omap, linux-kernel,
dri-devel
In-Reply-To: <aVkWigAQWC1dZBAv@smile.fi.intel.com>
On 1/3/26 05:15, Andy Shevchenko wrote:
> On Sat, Jan 03, 2026 at 10:59:44AM +0100, Helge Deller wrote:
>> On 12/30/25 19:25, Chintan Patel wrote:
>>> On 12/30/25 00:13, Helge Deller wrote:
>
> ...
>
>>>>> -ATTRIBUTE_GROUPS(overlay_sysfs);
>>>>
>>>> Instead of replacing the ^ ATTRIBUTE_GROUPS() by the code below,
>>>> isn't it possible to just mark the overlay_sysfs_attrs[] array
>>>> _maybe_unused, and just do:
>>>> + #ifdef CONFIG_FB_DEVICE
>>>> + ATTRIBUTE_GROUPS(overlay_sysfs);
>>>> + #endif
>>>>
>>>> ?
>>>
>>> Yes, the __maybe_unused + #ifdef ATTRIBUTE_GROUPS() approach would work.
>>>
>>> I went with the PTR_IF(IS_ENABLED()) pattern because Andy suggested
>>> using PTR_IF() to conditionally include overlay_sysfs_group in
>>> overlay_sysfs_groups, and to keep .dev_groups always populated while
>>> letting the device core skip NULL groups. This avoids conditional
>>> wiring via #ifdef and keeps the code type-checked without
>>> CONFIG_FB_DEVICE.
>>> If you still prefer the simpler #ifdef ATTRIBUTE_GROUPS() approach
>>> for this driver, I can switch to that, but I wanted to follow Andy’s
>>> guidance here.
>>
>> I assume Andy will agree to my suggested approach, as it's cleaner
>> and avoids code bloat/duplication. Maybe you send out a v4 with my
>> suggested approach, then it's easier to judge... ?
>
> I'm also fine with original code. But a suggested approach would work as well
> (at least like it sounds from the above description). Ideally would be nice to
> get rid of ifdeffery completely (that's why we have PTR_IF() for), although
> it might be not so readable. TL;DR: the most readable solution is the winner.
>
Thank you both! I will send v4 with Helge's suggestion and take it from
there.
^ permalink raw reply
* Re: [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing
From: sun jian @ 2026-01-06 0:42 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Greg Kroah-Hartman, linux-staging, linux-fbdev,
dri-devel
In-Reply-To: <aVv_w643SMuIELDE@smile.fi.intel.com>
On Tue, Jan 6, 2026 at 2:15 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, Jan 06, 2026 at 01:00:33AM +0800, sun jian wrote:
>
> > Thanks for the feedback.
>
> You're welcome, but please, do not top-post!
Sorry about that - I'll use inline replies.
>
> How can you test without hardware at hand?
>
>
> I already explained in the response to the cover letter. Please, read it.
>
Given that， I will drop all the changes.
Thanks,
Sun Jian
^ permalink raw reply
* Re: [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing
From: Andy Shevchenko @ 2026-01-05 18:15 UTC (permalink / raw)
To: sun jian
Cc: Andy Shevchenko, Greg Kroah-Hartman, linux-staging, linux-fbdev,
dri-devel
In-Reply-To: <CABFUUZFeO51MW5n1uDp0tcwJeJvgxDRxY3rDqkj2Z-6cO23TwA@mail.gmail.com>
On Tue, Jan 06, 2026 at 01:00:33AM +0800, sun jian wrote:
> Thanks for the feedback.
You're welcome, but please, do not top-post!
> You are right: changing the DT init path from write_register() to
> fbtft_write_buf_dc() implicitly assumes "cmd byte + payload bytes" and
> does not preserve the generic write_register() semantics (e.g. regwidth /
> bus-specific handling).I only have clang/arm64 build coverage (no
> access to the actual panels),
> so I can’t provide runtime validation yet. For the remaining 3 driver-local
> patches, all affected drivers have .regwidth = 8 and the sequences are
> “1-byte command + N bytes data” (gamma/LUT). The intent was to avoid the
> huge write_reg() varargs call that triggers -Wframe-larger-than=1024.
>
> Given the lack of hardware, would you prefer one of the following?
How can you test without hardware at hand?
> 1. Drop the driver changes and instead bump -Wframe-larger-than for these
> specific objects in the Makefile as an exception; or
>
> 2. Keep the driver changes but I should provide a detailed test pattern /
> list of tested devices — if so, what level of detail would be acceptable
> (exact panel model + wiring/bus type + expected output), and is “build-only”
> ever sufficient for warning-only changes in fbtft?
>
> Happy to follow the approach you think is appropriate for this staging driver.
I already explained in the response to the cover letter. Please, read it.
> On Tue, Jan 6, 2026 at 12:28 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Sun, Jan 04, 2026 at 07:06:35PM +0800, Sun Jian wrote:
> > > Clang reports a large stack frame for fbtft_init_display_from_property()
> > > (-Wframe-larger-than=1024) when the init sequence is emitted through a
> > > fixed 64-argument write_register() call.
> > >
> > > write_reg()/write_register() relies on NUMARGS((int[]){...}) and large
> > > varargs which inflates stack usage. Switch the DT "init" path to send the
> > > command byte and the payload via fbtft_write_buf_dc() instead.
> > >
> > > No functional change intended: the same register values are sent in the
> > > same order, only the transport is changed.
> >
> > How did you test this?
...
> > > - par->fbtftops.write_register(par, i,
> > > - buf[0], buf[1], buf[2], buf[3],
> > > - buf[4], buf[5], buf[6], buf[7],
> > > - buf[8], buf[9], buf[10], buf[11],
> > > - buf[12], buf[13], buf[14], buf[15],
> > > - buf[16], buf[17], buf[18], buf[19],
> > > - buf[20], buf[21], buf[22], buf[23],
> > > - buf[24], buf[25], buf[26], buf[27],
> > > - buf[28], buf[29], buf[30], buf[31],
> > > - buf[32], buf[33], buf[34], buf[35],
> > > - buf[36], buf[37], buf[38], buf[39],
> > > - buf[40], buf[41], buf[42], buf[43],
> > > - buf[44], buf[45], buf[46], buf[47],
> > > - buf[48], buf[49], buf[50], buf[51],
> > > - buf[52], buf[53], buf[54], buf[55],
> > > - buf[56], buf[57], buf[58], buf[59],
> > > - buf[60], buf[61], buf[62], buf[63]);
> > > + /* buf[0] is command, buf[1..i-1] is data */
> > > + ret = fbtft_write_buf_dc(par, &buf[0], 1, 0);
> > > + if (ret < 0)
> > > + goto out_free;
> > > +
> > > + if (i > 1) {
> > > + ret = fbtft_write_buf_dc(par, &buf[1], i - 1, 1);
> > > + if (ret < 0)
> > > + goto out_free;
> > > + }
> >
> > I believe this is incorrect change and has not to be applied. write !=
> > write_register. Without any evidence of testing, definite NAK to it.
> > Otherwise, please provide detailed testing pattern and which devices were
> > tested.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing
From: sun jian @ 2026-01-05 17:00 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Greg Kroah-Hartman, linux-staging, linux-fbdev,
dri-devel
In-Reply-To: <aVvmr2qOrFvoEKGV@smile.fi.intel.com>
Hi Andy,
Thanks for the feedback.
You are right: changing the DT init path from write_register() to
fbtft_write_buf_dc() implicitly assumes "cmd byte + payload bytes" and
does not preserve the generic write_register() semantics (e.g. regwidth /
bus-specific handling).I only have clang/arm64 build coverage (no
access to the actual panels),
so I can’t provide runtime validation yet. For the remaining 3 driver-local
patches, all affected drivers have .regwidth = 8 and the sequences are
“1-byte command + N bytes data” (gamma/LUT). The intent was to avoid the
huge write_reg() varargs call that triggers -Wframe-larger-than=1024.
Given the lack of hardware, would you prefer one of the following?
1. Drop the driver changes and instead bump -Wframe-larger-than for these
specific objects in the Makefile as an exception; or
2. Keep the driver changes but I should provide a detailed test pattern /
list of tested devices — if so, what level of detail would be acceptable
(exact panel model + wiring/bus type + expected output), and is “build-only”
ever sufficient for warning-only changes in fbtft?
Happy to follow the approach you think is appropriate for this staging driver.
Best regards,
Sun Jian
On Tue, Jan 6, 2026 at 12:28 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Sun, Jan 04, 2026 at 07:06:35PM +0800, Sun Jian wrote:
> > Clang reports a large stack frame for fbtft_init_display_from_property()
> > (-Wframe-larger-than=1024) when the init sequence is emitted through a
> > fixed 64-argument write_register() call.
> >
> > write_reg()/write_register() relies on NUMARGS((int[]){...}) and large
> > varargs which inflates stack usage. Switch the DT "init" path to send the
> > command byte and the payload via fbtft_write_buf_dc() instead.
> >
> > No functional change intended: the same register values are sent in the
> > same order, only the transport is changed.
>
> How did you test this?
>
> ...
>
> > struct device *dev = par->info->device;
> > - int buf[64], count, index, i, j, ret;
> > + u8 buf[64];
> > + int count, index, i, j, ret;
>
> Please, try to preserve reversed xmas tree order.
>
> > u32 *values;
> > u32 val;
> >
>
> ...
>
> > - buf[i++] = val;
> > + buf[i++] = val & 0xFF;
>
> Unneeded change, I suppose.
>
> ...
>
> > - par->fbtftops.write_register(par, i,
> > - buf[0], buf[1], buf[2], buf[3],
> > - buf[4], buf[5], buf[6], buf[7],
> > - buf[8], buf[9], buf[10], buf[11],
> > - buf[12], buf[13], buf[14], buf[15],
> > - buf[16], buf[17], buf[18], buf[19],
> > - buf[20], buf[21], buf[22], buf[23],
> > - buf[24], buf[25], buf[26], buf[27],
> > - buf[28], buf[29], buf[30], buf[31],
> > - buf[32], buf[33], buf[34], buf[35],
> > - buf[36], buf[37], buf[38], buf[39],
> > - buf[40], buf[41], buf[42], buf[43],
> > - buf[44], buf[45], buf[46], buf[47],
> > - buf[48], buf[49], buf[50], buf[51],
> > - buf[52], buf[53], buf[54], buf[55],
> > - buf[56], buf[57], buf[58], buf[59],
> > - buf[60], buf[61], buf[62], buf[63]);
> > + /* buf[0] is command, buf[1..i-1] is data */
> > + ret = fbtft_write_buf_dc(par, &buf[0], 1, 0);
> > + if (ret < 0)
> > + goto out_free;
> > +
> > + if (i > 1) {
> > + ret = fbtft_write_buf_dc(par, &buf[1], i - 1, 1);
> > + if (ret < 0)
> > + goto out_free;
> > + }
>
> I believe this is incorrect change and has not to be applied. write !=
> write_register. Without any evidence of testing, definite NAK to it.
> Otherwise, please provide detailed testing pattern and which devices were
> tested.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v1 4/4] staging: fbtft: hx8353d: send LUT via buffer to reduce stack usage
From: Andy Shevchenko @ 2026-01-05 16:36 UTC (permalink / raw)
To: Sun Jian
Cc: Andy Shevchenko, Greg Kroah-Hartman, linux-staging, linux-fbdev,
dri-devel
In-Reply-To: <20260104110638.532615-5-sun.jian.kdev@gmail.com>
On Sun, Jan 04, 2026 at 07:06:38PM +0800, Sun Jian wrote:
> Clang reports a large stack frame in init_display()
> (-Wframe-larger-than=1024) due to the very large
> write_reg(MIPI_DCS_WRITE_LUT, ...) call.
>
> Send MIPI_DCS_WRITE_LUT followed by the LUT payload using
> fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.
>
> No functional change intended.
...
> +static const u8 lut[] = {
> + 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
> + 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
> + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
> + 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
> + 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
> + 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
> + 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
> + 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
> + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
> + 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
> + 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
> + 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
Two tabs too many on each line.
> + };
> +
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v1 0/4] staging: fbtft: reduce stack usage by avoiding large write_reg() varargs
From: Andy Shevchenko @ 2026-01-05 16:32 UTC (permalink / raw)
To: Sun Jian
Cc: Andy Shevchenko, Greg Kroah-Hartman, linux-staging, linux-fbdev,
dri-devel
In-Reply-To: <20260104110638.532615-1-sun.jian.kdev@gmail.com>
On Sun, Jan 04, 2026 at 07:06:34PM +0800, Sun Jian wrote:
> This series fixes clang `-Wframe-larger-than=1024` warnings in the fbtft
> staging drivers.
>
> The warnings are triggered by very large `write_reg()`/`write_register()`
> varargs calls, which result in excessive stack usage.
>
> Switch the affected paths to send a u8 command byte followed by the u8
> payload using `fbtft_write_buf_dc()`. The register values and ordering are
> kept unchanged; only the transfer method is updated.
Looking at the patches I think this is wrong. W.o. detailed test pattern
provided and the list of the devices, NAK.
If you want to address a warning without HW being accessible, perhaps you just
need a simple bump in the Makefile as an exception, however it's also doubtful
as it will hide a potential issue with the stack in the future.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing
From: Andy Shevchenko @ 2026-01-05 16:28 UTC (permalink / raw)
To: Sun Jian
Cc: Andy Shevchenko, Greg Kroah-Hartman, linux-staging, linux-fbdev,
dri-devel
In-Reply-To: <20260104110638.532615-2-sun.jian.kdev@gmail.com>
On Sun, Jan 04, 2026 at 07:06:35PM +0800, Sun Jian wrote:
> Clang reports a large stack frame for fbtft_init_display_from_property()
> (-Wframe-larger-than=1024) when the init sequence is emitted through a
> fixed 64-argument write_register() call.
>
> write_reg()/write_register() relies on NUMARGS((int[]){...}) and large
> varargs which inflates stack usage. Switch the DT "init" path to send the
> command byte and the payload via fbtft_write_buf_dc() instead.
>
> No functional change intended: the same register values are sent in the
> same order, only the transport is changed.
How did you test this?
...
> struct device *dev = par->info->device;
> - int buf[64], count, index, i, j, ret;
> + u8 buf[64];
> + int count, index, i, j, ret;
Please, try to preserve reversed xmas tree order.
> u32 *values;
> u32 val;
>
...
> - buf[i++] = val;
> + buf[i++] = val & 0xFF;
Unneeded change, I suppose.
...
> - par->fbtftops.write_register(par, i,
> - buf[0], buf[1], buf[2], buf[3],
> - buf[4], buf[5], buf[6], buf[7],
> - buf[8], buf[9], buf[10], buf[11],
> - buf[12], buf[13], buf[14], buf[15],
> - buf[16], buf[17], buf[18], buf[19],
> - buf[20], buf[21], buf[22], buf[23],
> - buf[24], buf[25], buf[26], buf[27],
> - buf[28], buf[29], buf[30], buf[31],
> - buf[32], buf[33], buf[34], buf[35],
> - buf[36], buf[37], buf[38], buf[39],
> - buf[40], buf[41], buf[42], buf[43],
> - buf[44], buf[45], buf[46], buf[47],
> - buf[48], buf[49], buf[50], buf[51],
> - buf[52], buf[53], buf[54], buf[55],
> - buf[56], buf[57], buf[58], buf[59],
> - buf[60], buf[61], buf[62], buf[63]);
> + /* buf[0] is command, buf[1..i-1] is data */
> + ret = fbtft_write_buf_dc(par, &buf[0], 1, 0);
> + if (ret < 0)
> + goto out_free;
> +
> + if (i > 1) {
> + ret = fbtft_write_buf_dc(par, &buf[1], i - 1, 1);
> + if (ret < 0)
> + goto out_free;
> + }
I believe this is incorrect change and has not to be applied. write !=
write_register. Without any evidence of testing, definite NAK to it.
Otherwise, please provide detailed testing pattern and which devices were
tested.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v1 2/4] staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()
From: sun jian @ 2026-01-05 15:09 UTC (permalink / raw)
To: Dan Carpenter
Cc: Andy Shevchenko, Greg Kroah-Hartman, linux-staging, linux-fbdev,
dri-devel
In-Reply-To: <aVvNJV93mprLcZwy@stanley.mountain>
Hi Dan,
Thanks. Agreed.
For v2 I’ll keep this patch minimal: only switch from write_reg() varargs
to fbtft_write_buf_dc() and reduce the stack usage, without other changes.
Any follow-up cleanups (if needed) will be sent as a separate patch.
Thanks,
Sun Jian
On Mon, Jan 5, 2026 at 10:39 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Sun, Jan 04, 2026 at 07:06:36PM +0800, Sun Jian wrote:
> > Clang reports a large stack frame in set_gamma() (-Wframe-larger-than=1024)
> > due to the large write_reg() call emitting 63 gamma bytes via varargs.
> >
> > Send the command byte (0xB8) and the gamma payload using
> > fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.
> >
> > No functional change intended.
> >
> > Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> > ---
> > drivers/staging/fbtft/fb_ssd1351.c | 35 +++++++++++++-----------------
> > 1 file changed, 15 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/staging/fbtft/fb_ssd1351.c b/drivers/staging/fbtft/fb_ssd1351.c
> > index 6736b09b2f45..b4ab2c81e528 100644
> > --- a/drivers/staging/fbtft/fb_ssd1351.c
> > +++ b/drivers/staging/fbtft/fb_ssd1351.c
> > @@ -119,43 +119,38 @@ static int set_var(struct fbtft_par *par)
> > */
> > static int set_gamma(struct fbtft_par *par, u32 *curves)
> > {
> > - unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
> > + u8 data[GAMMA_LEN];
>
> Ugh... GAMMA_NUM is 1 so this is an annoying calculation. So what
> this does is it changes the type from unsigned long to u8 and renames
> the variable. I am fine with renaming the variable it's unrelated and
> makes the review harder.
>
> > + u8 cmd = 0xB8;
> > int i, acc = 0;
> > + int ret;
> >
> > - for (i = 0; i < 63; i++) {
> > + for (i = 0; i < GAMMA_LEN; i++) {
>
> GAMMA_LEN is 63. So this looks like a change, but it's an unrelated
> cleanup.
>
> > if (i > 0 && curves[i] < 2) {
> > dev_err(par->info->device,
> > "Illegal value in Grayscale Lookup Table at index %d : %d. Must be greater than 1\n",
> > i, curves[i]);
> > return -EINVAL;
> > }
> > +
>
> This is an unrelated white space change.
>
> > acc += curves[i];
> > - tmp[i] = acc;
> > +
> > if (acc > 180) {
> > dev_err(par->info->device,
> > "Illegal value(s) in Grayscale Lookup Table. At index=%d : %d, the accumulated value has exceeded 180\n",
> > i, acc);
> > return -EINVAL;
> > }
> > +
> > + data[i] = acc;
>
> Here we move the acc assignment after the sanity check, but it's just
> an unrelated cleanup.
>
> > }
> >
> > - write_reg(par, 0xB8,
> > - tmp[0], tmp[1], tmp[2], tmp[3],
> > - tmp[4], tmp[5], tmp[6], tmp[7],
> > - tmp[8], tmp[9], tmp[10], tmp[11],
> > - tmp[12], tmp[13], tmp[14], tmp[15],
> > - tmp[16], tmp[17], tmp[18], tmp[19],
> > - tmp[20], tmp[21], tmp[22], tmp[23],
> > - tmp[24], tmp[25], tmp[26], tmp[27],
> > - tmp[28], tmp[29], tmp[30], tmp[31],
> > - tmp[32], tmp[33], tmp[34], tmp[35],
> > - tmp[36], tmp[37], tmp[38], tmp[39],
> > - tmp[40], tmp[41], tmp[42], tmp[43],
> > - tmp[44], tmp[45], tmp[46], tmp[47],
> > - tmp[48], tmp[49], tmp[50], tmp[51],
> > - tmp[52], tmp[53], tmp[54], tmp[55],
> > - tmp[56], tmp[57], tmp[58], tmp[59],
> > - tmp[60], tmp[61], tmp[62]);
> > + ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = fbtft_write_buf_dc(par, data, sizeof(data), 1);
> > + if (ret < 0)
> > + return ret;
>
> These are good changes. Just change the type from unsigned long to u8
> and use fbtft_write_buf_dc() instead of write_reg(). Then do the other
> changes in a separate patch.
>
> Same for the other patches.
>
> regards,
> dan carpenter
>
> >
> > return 0;
> > }
> > --
> > 2.43.0
> >
^ permalink raw reply
* Re: [PATCH v1 2/4] staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()
From: Dan Carpenter @ 2026-01-05 14:39 UTC (permalink / raw)
To: Sun Jian
Cc: Andy Shevchenko, Greg Kroah-Hartman, linux-staging, linux-fbdev,
dri-devel
In-Reply-To: <20260104110638.532615-3-sun.jian.kdev@gmail.com>
On Sun, Jan 04, 2026 at 07:06:36PM +0800, Sun Jian wrote:
> Clang reports a large stack frame in set_gamma() (-Wframe-larger-than=1024)
> due to the large write_reg() call emitting 63 gamma bytes via varargs.
>
> Send the command byte (0xB8) and the gamma payload using
> fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.
>
> No functional change intended.
>
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
> drivers/staging/fbtft/fb_ssd1351.c | 35 +++++++++++++-----------------
> 1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fb_ssd1351.c b/drivers/staging/fbtft/fb_ssd1351.c
> index 6736b09b2f45..b4ab2c81e528 100644
> --- a/drivers/staging/fbtft/fb_ssd1351.c
> +++ b/drivers/staging/fbtft/fb_ssd1351.c
> @@ -119,43 +119,38 @@ static int set_var(struct fbtft_par *par)
> */
> static int set_gamma(struct fbtft_par *par, u32 *curves)
> {
> - unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
> + u8 data[GAMMA_LEN];
Ugh... GAMMA_NUM is 1 so this is an annoying calculation. So what
this does is it changes the type from unsigned long to u8 and renames
the variable. I am fine with renaming the variable it's unrelated and
makes the review harder.
> + u8 cmd = 0xB8;
> int i, acc = 0;
> + int ret;
>
> - for (i = 0; i < 63; i++) {
> + for (i = 0; i < GAMMA_LEN; i++) {
GAMMA_LEN is 63. So this looks like a change, but it's an unrelated
cleanup.
> if (i > 0 && curves[i] < 2) {
> dev_err(par->info->device,
> "Illegal value in Grayscale Lookup Table at index %d : %d. Must be greater than 1\n",
> i, curves[i]);
> return -EINVAL;
> }
> +
This is an unrelated white space change.
> acc += curves[i];
> - tmp[i] = acc;
> +
> if (acc > 180) {
> dev_err(par->info->device,
> "Illegal value(s) in Grayscale Lookup Table. At index=%d : %d, the accumulated value has exceeded 180\n",
> i, acc);
> return -EINVAL;
> }
> +
> + data[i] = acc;
Here we move the acc assignment after the sanity check, but it's just
an unrelated cleanup.
> }
>
> - write_reg(par, 0xB8,
> - tmp[0], tmp[1], tmp[2], tmp[3],
> - tmp[4], tmp[5], tmp[6], tmp[7],
> - tmp[8], tmp[9], tmp[10], tmp[11],
> - tmp[12], tmp[13], tmp[14], tmp[15],
> - tmp[16], tmp[17], tmp[18], tmp[19],
> - tmp[20], tmp[21], tmp[22], tmp[23],
> - tmp[24], tmp[25], tmp[26], tmp[27],
> - tmp[28], tmp[29], tmp[30], tmp[31],
> - tmp[32], tmp[33], tmp[34], tmp[35],
> - tmp[36], tmp[37], tmp[38], tmp[39],
> - tmp[40], tmp[41], tmp[42], tmp[43],
> - tmp[44], tmp[45], tmp[46], tmp[47],
> - tmp[48], tmp[49], tmp[50], tmp[51],
> - tmp[52], tmp[53], tmp[54], tmp[55],
> - tmp[56], tmp[57], tmp[58], tmp[59],
> - tmp[60], tmp[61], tmp[62]);
> + ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = fbtft_write_buf_dc(par, data, sizeof(data), 1);
> + if (ret < 0)
> + return ret;
These are good changes. Just change the type from unsigned long to u8
and use fbtft_write_buf_dc() instead of write_reg(). Then do the other
changes in a separate patch.
Same for the other patches.
regards,
dan carpenter
>
> return 0;
> }
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: Daniel Thompson @ 2026-01-05 10:09 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
In-Reply-To: <20260105085120.230862-3-tessolveupstream@gmail.com>
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.
> };
>
> static int gpio_backlight_update_status(struct backlight_device *bl)
> {
> struct gpio_backlight *gbl = bl_get_data(bl);
> + unsigned int i;
> + int br = backlight_get_brightness(bl);
>
> - gpiod_set_value_cansleep(gbl->gpiod, backlight_get_brightness(bl));
> + for (i = 0; i < gbl->num_gpios; i++)
> + gpiod_set_value_cansleep(gbl->gpiods[i], br);
>
> return 0;
> }
> @@ -52,6 +56,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> struct backlight_device *bl;
> struct gpio_backlight *gbl;
> int ret, init_brightness, def_value;
> + unsigned int i;
>
> gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
> if (gbl == NULL)
> @@ -62,10 +67,22 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>
> def_value = device_property_read_bool(dev, "default-on");
>
> - gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
> - if (IS_ERR(gbl->gpiod))
> - return dev_err_probe(dev, PTR_ERR(gbl->gpiod),
> - "The gpios parameter is missing or invalid\n");
> + gbl->num_gpios = gpiod_count(dev, NULL);
> + if (gbl->num_gpios == 0)
> + return dev_err_probe(dev, -EINVAL,
> + "The gpios parameter is missing or invalid\n");
> + gbl->gpiods = devm_kcalloc(dev, gbl->num_gpios, sizeof(*gbl->gpiods),
> + GFP_KERNEL);
> + if (!gbl->gpiods)
> + return -ENOMEM;
This is definitely easier if you simply use devm_get_array().
> +
> + for (i = 0; i < gbl->num_gpios; i++) {
> + gbl->gpiods[i] =
> + devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
> + if (IS_ERR(gbl->gpiods[i]))
> + return dev_err_probe(dev, PTR_ERR(gbl->gpiods[i]),
> + "Failed to get GPIO at index %u\n", i);
> + }
>
> memset(&props, 0, sizeof(props));
> props.type = BACKLIGHT_RAW;
> @@ -78,22 +95,34 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> }
>
> /* Set the initial power state */
> - if (!of_node || !of_node->phandle)
> + if (!of_node || !of_node->phandle) {
> /* Not booted with device tree or no phandle link to the node */
> bl->props.power = def_value ? BACKLIGHT_POWER_ON
> - : BACKLIGHT_POWER_OFF;
> - else if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
> - bl->props.power = BACKLIGHT_POWER_OFF;
> - else
> - bl->props.power = BACKLIGHT_POWER_ON;
> + : BACKLIGHT_POWER_OFF;
> + } else {
> + bool all_high = true;
> +
> + for (i = 0; i < gbl->num_gpios; i++) {
> + if (gpiod_get_value_cansleep(gbl->gpiods[i]) != 0) {
Why is there a != here?
> + all_high = false;
> + break;
> + }
> + }
> +
> + bl->props.power =
> + all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
> + }
>
> bl->props.brightness = 1;
>
> init_brightness = backlight_get_brightness(bl);
> - ret = gpiod_direction_output(gbl->gpiod, init_brightness);
> - if (ret) {
> - dev_err(dev, "failed to set initial brightness\n");
> - return ret;
> +
> + for (i = 0; i < gbl->num_gpios; i++) {
> + ret = gpiod_direction_output(gbl->gpiods[i], init_brightness);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to set gpio %u direction\n",
> + i);
> }
>
> platform_set_drvdata(pdev, bl);
Daniel.
^ permalink raw reply
* [PATCH 5.10.y] fbcon: Fix the issue of uninitialized charcount in the remaining consoles
From: Luo Gengkun @ 2026-01-05 10:28 UTC (permalink / raw)
To: b.zolnierkie; +Cc: dri-devel, linux-fbdev, linux-kernel
After commit 0998a6cb2326 ("fbdev: bitblit: bound-check glyph index in
bit_putcs*") was merged, using alt+ctrl+f1 to switch the tty from tty0 to
tty1 results in garbled display.
The reason is the vc->vc_font.charcount is 0, it is clearly an
uninitialized value. The mainline is fine because commit a1ac250a82a5
("fbcon: Avoid using FNTCHARCNT() and hard-coded built-in font charcount")
assigns the fvc->vc_font.charcount to vc->vc_font.charcount.
Cc: stable@vger.kernel.org
Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
---
drivers/video/fbdev/core/fbcon.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 3dd03e02bf97..900c1ccef98b 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1070,6 +1070,7 @@ static void fbcon_init(struct vc_data *vc, int init)
fvc->vc_font.data);
vc->vc_font.width = fvc->vc_font.width;
vc->vc_font.height = fvc->vc_font.height;
+ vc->vc_font.charcount = fvc->vc_font.charcount;
p->userfont = t->userfont;
if (p->userfont)
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: Daniel Thompson @ 2026-01-05 9:55 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
In-Reply-To: <20260105085120.230862-2-tessolveupstream@gmail.com>
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?
Daniel.
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: Greg KH @ 2026-01-05 9:12 UTC (permalink / raw)
To: sun jian; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev
In-Reply-To: <CABFUUZHRz919=C=fmMqH1sQcURbm+qiQB795xWPCd9Rax_M4ZQ@mail.gmail.com>
On Mon, Jan 05, 2026 at 04:57:48PM +0800, sun jian wrote:
> Hi Greg,
>
> Sorry about that — I mistakenly replied only to you.
>
> As you pointed out, sw_i2c_wait() sits on the bit-banged I2C GPIO
> transitions, so changing the delay semantics without hardware validation
> is risky. I don't have access to the hardware to validate timing/behavior,
> and I can't justify that udelay(1) is equivalent to the existing loop.
>
> Please ignore v2 (and v1). I won't resend a warning-only workaround.
>
> If someone with the hardware can help validate a proper fix (e.g. a
> well-justified time-based delay, or reworking this to use a proper I2C
> bit-banging helper), I'm happy to revisit.
A time-based one is going to be the correct solution as every cpu will
run that "let's count some numbers" loop at a different speed :(
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: sun jian @ 2026-01-05 8:57 UTC (permalink / raw)
To: Greg KH; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev
In-Reply-To: <2026010505-surging-resurface-a7d3@gregkh>
Hi Greg,
Sorry about that — I mistakenly replied only to you.
As you pointed out, sw_i2c_wait() sits on the bit-banged I2C GPIO
transitions, so changing the delay semantics without hardware validation
is risky. I don't have access to the hardware to validate timing/behavior,
and I can't justify that udelay(1) is equivalent to the existing loop.
Please ignore v2 (and v1). I won't resend a warning-only workaround.
If someone with the hardware can help validate a proper fix (e.g. a
well-justified time-based delay, or reworking this to use a proper I2C
bit-banging helper), I'm happy to revisit.
Thanks,
Sun
On Mon, Jan 5, 2026 at 4:05 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jan 05, 2026 at 03:49:17PM +0800, Sun Jian wrote:
> > clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
> >
> > sw_i2c_wait() provides the delay between bit-banged I2C GPIO transitions.
> > Replace the loop-count delay with a time-based udelay(1) to avoid CPU-
> > dependent timing and fix the warning.
>
> Why is udelay(1) the same here?
>
> > Compile-tested with clang W=1; no hardware available to validate timing.
>
> That's going to prevent us from being able to take this, sorry :(
>
>
> >
> > Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> > ---
> > v2:
> > - Replace cpu_relax() delay loop with time-based udelay(1) to avoid
> > CPU-dependent timing (per Greg's feedback).
> >
> > v1:
> > - Used cpu_relax() in the loop to silence -Wunused-but-set-variable.
> > ---
> > drivers/staging/sm750fb/ddk750_swi2c.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> > index 0ef8d4ff2ef9..d5843fa69bfa 100644
> > --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> > +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> > @@ -11,6 +11,7 @@
> > #include "ddk750_reg.h"
> > #include "ddk750_swi2c.h"
> > #include "ddk750_power.h"
> > +#include <linux/delay.h>
>
> Shouldn't this be at the top of the include lines?
>
> >
> > /*
> > * I2C Software Master Driver:
> > @@ -92,12 +93,7 @@ static void sw_i2c_wait(void)
> > * it's more reliable than counter loop ..
> > * write 0x61 to 0x3ce and read from 0x3cf
> > */
> > - int i, tmp;
> > -
> > - for (i = 0; i < 600; i++) {
> > - tmp = i;
> > - tmp += i;
> > - }
> > + udelay(1);
>
> You are ignoring the comments in this function.
>
> Also, if you reduce this to a single call, shouldn't this whole function
> be removed?
>
> thanks,
>
> greg k-h
^ permalink raw reply
* [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: Sudarshan Shetty @ 2026-01-05 8:51 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: <20260105085120.230862-1-tessolveupstream@gmail.com>
Extend the gpio-backlight driver to handle multiple GPIOs instead of a
single one. This allows panels that require driving several enable pins
to be controlled by the backlight framework.
Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
---
drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++-------
1 file changed, 45 insertions(+), 16 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 728a546904b0..037e1c111e48 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -17,14 +17,18 @@
struct gpio_backlight {
struct device *dev;
- struct gpio_desc *gpiod;
+ struct gpio_desc **gpiods;
+ unsigned int num_gpios;
};
static int gpio_backlight_update_status(struct backlight_device *bl)
{
struct gpio_backlight *gbl = bl_get_data(bl);
+ unsigned int i;
+ int br = backlight_get_brightness(bl);
- gpiod_set_value_cansleep(gbl->gpiod, backlight_get_brightness(bl));
+ for (i = 0; i < gbl->num_gpios; i++)
+ gpiod_set_value_cansleep(gbl->gpiods[i], br);
return 0;
}
@@ -52,6 +56,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
struct backlight_device *bl;
struct gpio_backlight *gbl;
int ret, init_brightness, def_value;
+ unsigned int i;
gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
@@ -62,10 +67,22 @@ static int gpio_backlight_probe(struct platform_device *pdev)
def_value = device_property_read_bool(dev, "default-on");
- gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
- if (IS_ERR(gbl->gpiod))
- return dev_err_probe(dev, PTR_ERR(gbl->gpiod),
- "The gpios parameter is missing or invalid\n");
+ gbl->num_gpios = gpiod_count(dev, NULL);
+ if (gbl->num_gpios == 0)
+ return dev_err_probe(dev, -EINVAL,
+ "The gpios parameter is missing or invalid\n");
+ gbl->gpiods = devm_kcalloc(dev, gbl->num_gpios, sizeof(*gbl->gpiods),
+ GFP_KERNEL);
+ if (!gbl->gpiods)
+ return -ENOMEM;
+
+ for (i = 0; i < gbl->num_gpios; i++) {
+ gbl->gpiods[i] =
+ devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
+ if (IS_ERR(gbl->gpiods[i]))
+ return dev_err_probe(dev, PTR_ERR(gbl->gpiods[i]),
+ "Failed to get GPIO at index %u\n", i);
+ }
memset(&props, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
@@ -78,22 +95,34 @@ static int gpio_backlight_probe(struct platform_device *pdev)
}
/* Set the initial power state */
- if (!of_node || !of_node->phandle)
+ if (!of_node || !of_node->phandle) {
/* Not booted with device tree or no phandle link to the node */
bl->props.power = def_value ? BACKLIGHT_POWER_ON
- : BACKLIGHT_POWER_OFF;
- else if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
- bl->props.power = BACKLIGHT_POWER_OFF;
- else
- bl->props.power = BACKLIGHT_POWER_ON;
+ : BACKLIGHT_POWER_OFF;
+ } else {
+ bool all_high = true;
+
+ for (i = 0; i < gbl->num_gpios; i++) {
+ if (gpiod_get_value_cansleep(gbl->gpiods[i]) != 0) {
+ all_high = false;
+ break;
+ }
+ }
+
+ bl->props.power =
+ all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
+ }
bl->props.brightness = 1;
init_brightness = backlight_get_brightness(bl);
- ret = gpiod_direction_output(gbl->gpiod, init_brightness);
- if (ret) {
- dev_err(dev, "failed to set initial brightness\n");
- return ret;
+
+ for (i = 0; i < gbl->num_gpios; i++) {
+ ret = gpiod_direction_output(gbl->gpiods[i], init_brightness);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to set gpio %u direction\n",
+ i);
}
platform_set_drvdata(pdev, bl);
--
2.34.1
^ permalink raw reply related
* [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: Sudarshan Shetty @ 2026-01-05 8:51 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: <20260105085120.230862-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>
---
.../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
default-on:
description: enable the backlight at boot.
@@ -38,4 +39,13 @@ examples:
default-on;
};
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ backlight {
+ compatible = "gpio-backlight";
+ gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>,
+ <&gpio3 5 GPIO_ACTIVE_HIGH>;
+ default-on;
+ };
+
...
--
2.34.1
^ permalink raw reply related
* [PATCH v1 0/2] backlight: gpio-backlight: Add support for multiple GPIOs
From: Sudarshan Shetty @ 2026-01-05 8:51 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 1 or 2
GPIOs for a backlight node.
This approach avoids describing multiple backlight devices in DT for a
single panel.
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 | 12 +++-
drivers/video/backlight/gpio_backlight.c | 61 ++++++++++++++-----
2 files changed, 56 insertions(+), 17 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: Greg KH @ 2026-01-05 8:05 UTC (permalink / raw)
To: Sun Jian; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev
In-Reply-To: <20260105074917.607201-1-sun.jian.kdev@gmail.com>
On Mon, Jan 05, 2026 at 03:49:17PM +0800, Sun Jian wrote:
> clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
>
> sw_i2c_wait() provides the delay between bit-banged I2C GPIO transitions.
> Replace the loop-count delay with a time-based udelay(1) to avoid CPU-
> dependent timing and fix the warning.
Why is udelay(1) the same here?
> Compile-tested with clang W=1; no hardware available to validate timing.
That's going to prevent us from being able to take this, sorry :(
>
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
> v2:
> - Replace cpu_relax() delay loop with time-based udelay(1) to avoid
> CPU-dependent timing (per Greg's feedback).
>
> v1:
> - Used cpu_relax() in the loop to silence -Wunused-but-set-variable.
> ---
> drivers/staging/sm750fb/ddk750_swi2c.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> index 0ef8d4ff2ef9..d5843fa69bfa 100644
> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> @@ -11,6 +11,7 @@
> #include "ddk750_reg.h"
> #include "ddk750_swi2c.h"
> #include "ddk750_power.h"
> +#include <linux/delay.h>
Shouldn't this be at the top of the include lines?
>
> /*
> * I2C Software Master Driver:
> @@ -92,12 +93,7 @@ static void sw_i2c_wait(void)
> * it's more reliable than counter loop ..
> * write 0x61 to 0x3ce and read from 0x3cf
> */
> - int i, tmp;
> -
> - for (i = 0; i < 600; i++) {
> - tmp = i;
> - tmp += i;
> - }
> + udelay(1);
You are ignoring the comments in this function.
Also, if you reduce this to a single call, shouldn't this whole function
be removed?
thanks,
greg k-h
^ permalink raw reply
* [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: Sun Jian @ 2026-01-05 7:49 UTC (permalink / raw)
To: linux-staging; +Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, Sun Jian
In-Reply-To: <20260105021026.556244-1-sun.jian.kdev@gmail.com>
clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
sw_i2c_wait() provides the delay between bit-banged I2C GPIO transitions.
Replace the loop-count delay with a time-based udelay(1) to avoid CPU-
dependent timing and fix the warning.
Compile-tested with clang W=1; no hardware available to validate timing.
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
v2:
- Replace cpu_relax() delay loop with time-based udelay(1) to avoid
CPU-dependent timing (per Greg's feedback).
v1:
- Used cpu_relax() in the loop to silence -Wunused-but-set-variable.
---
drivers/staging/sm750fb/ddk750_swi2c.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index 0ef8d4ff2ef9..d5843fa69bfa 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -11,6 +11,7 @@
#include "ddk750_reg.h"
#include "ddk750_swi2c.h"
#include "ddk750_power.h"
+#include <linux/delay.h>
/*
* I2C Software Master Driver:
@@ -92,12 +93,7 @@ static void sw_i2c_wait(void)
* it's more reliable than counter loop ..
* write 0x61 to 0x3ce and read from 0x3cf
*/
- int i, tmp;
-
- for (i = 0; i < 600; i++) {
- tmp = i;
- tmp += i;
- }
+ udelay(1);
}
/*
--
2.43.0
^ permalink raw reply related
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