* [PATCH next] drivers/video/fbdev/s3fb: Use strscpy() to copy strings into arrays
From: david.laight.linux @ 2026-06-08 9:54 UTC (permalink / raw)
To: Kees Cook, linux-hardening, dri-devel, linux-fbdev, linux-kernel
Cc: Arnd Bergmann, Helge Deller, David Laight
From: David Laight <david.laight.linux@gmail.com>
Replacing strcpy() with strscpy() ensures that overflow of the target
buffer cannot happen.
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
This is one of a group of patches that remove potentially unbounded
strcpy() calls.
They are mostly replaced by strscpy() or, when strlen() has just been
called, with memcpy() (usually including the '\0').
Calls with copy string literals into arrays are left unchanged.
They are safe and easily detected as such.
The changes were made by getting the compiler to detect the calls and
then fixing the code by hand.
Note that all the changes are only compile tested.
Some Makefiles were changed to allow files to contain strcpy().
As well as 'difficult to fix' files, this included 'show' functions
as they really need to use sysfs_emit() or seq_printf().
All the patches are being sent individually to avoid very long cc lists.
Apologies for the terse commit messages and likely unexpected tags.
(There are about 100 patches in total.)
drivers/video/fbdev/s3fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index ba30e5568cab..b679ccdb38e2 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -1333,7 +1333,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
vga_wcrt(par->state.vgabase, 0x38, cr38);
vga_wcrt(par->state.vgabase, 0x39, cr39);
- strcpy(info->fix.id, s3_names [par->chip]);
+ strscpy(info->fix.id, s3_names[par->chip]);
info->fix.mmio_start = 0;
info->fix.mmio_len = 0;
info->fix.type = FB_TYPE_PACKED_PIXELS;
--
2.39.5
^ permalink raw reply related
* Re: [PATCH] lib/fonts: Avoid unncessary 64-bit math in font code
From: Thomas Zimmermann @ 2026-06-08 11:25 UTC (permalink / raw)
To: Helge Deller, linux-fbdev, dri-devel; +Cc: Ethan Nelson-Moore
In-Reply-To: <20260607210203.229613-1-deller@gmx.de>
Hi
Am 07.06.26 um 23:02 schrieb Helge Deller:
> The text display code used in the Risc PC kernel image decompression
> code uses arch/arm/boot/compressed/font.c, which includes
> lib/fonts/font_acorn_8x8.c, which further includes <linux/font.h>.
>
> Since commit 97df8960240a ("lib/fonts: Provide helpers for calculating
> glyph pitch and size") <linux/font.h> contains inline functions that
> require __do_div64, which is not linked into the ARM kernel
> decompressor. This makes Risc PC zImages fail to build.
>
> There is no need to use 64-bit division code here, so resolve this issue
> by using plain standard addition and shift maths.
Why is there a 64-bit division at all?
>
> Fixes: 97df8960240a ("lib/fonts: Provide helpers for calculating glyph pitch and size")
> Reported-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
> include/linux/font.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/linux/font.h b/include/linux/font.h
> index 6845f02d739a..67d32268989d 100644
> --- a/include/linux/font.h
> +++ b/include/linux/font.h
> @@ -11,7 +11,6 @@
> #ifndef _VIDEO_FONT_H
> #define _VIDEO_FONT_H
>
> -#include <linux/math.h>
> #include <linux/types.h>
>
> struct console_font;
> @@ -35,7 +34,7 @@ struct console_font;
> */
> static inline unsigned int font_glyph_pitch(unsigned int width)
> {
> - return DIV_ROUND_UP(width, 8);
> + return (width + 7) >> 3;
Ok by me, if that's what's necessary. But can we try to keep a
documented macro for the division to make the code explain itself? Does
it work with DIV_ROUND_UP_POW2() ?
Best regards
Thomas
> }
>
> /**
--
--
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/1] video, sm501: Fix buffer errors in OF binding code
From: David Laight @ 2026-06-08 12:42 UTC (permalink / raw)
To: Heiko Schocher, linux-fbdev, devicetree-discuss, Ben Dooks,
Vincent Sanders, Samuel Ortiz, linux-kernel, Randy Dunlap,
Paul Mundt, Helge Deller, Danila Chernetsov, Kees Cook
Cc: David Laight
The code that gets the frame buffer mode from OF has 'use after free',
'buffer overrun' and memory leaks.
info->edid_data isn't free if the probe functions fail or if
pd->def_mode is set.
If both the CRT and PANEL are enabled info->edid_data is used after
being freed and is freed twice.
The string returned by of_get_property(np, "mode", &len) is just
written over either the static "640x480-16@60" or the module parameter
string without any regard for the length (which is most likely longer).
Use kstrump() for the OF mode and free everything before freeing 'info.
Fixes: 4295f9bf74a88 ("video, sm501: add OF binding to support SM501")
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
drivers/video/fbdev/sm501fb.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c
index fee4b9f84592..ea5375ed4ea6 100644
--- a/drivers/video/fbdev/sm501fb.c
+++ b/drivers/video/fbdev/sm501fb.c
@@ -96,6 +96,7 @@ struct sm501fb_info {
void __iomem *fbmem; /* remapped framebuffer */
size_t fbmem_len; /* length of remapped region */
u8 *edid_data;
+ char *fb_mode;
};
/* per-framebuffer private data */
@@ -1793,12 +1794,11 @@ static int sm501fb_init_fb(struct fb_info *fb, enum sm501_controller head,
fb->var.yres_virtual = fb->var.yres;
} else {
if (info->edid_data) {
- ret = fb_find_mode(&fb->var, fb, fb_mode,
+ ret = fb_find_mode(&fb->var, fb,
+ info->fb_mode ?: fb_mode,
fb->monspecs.modedb,
fb->monspecs.modedb_len,
&sm501_default_mode, default_bpp);
- /* edid_data is no longer needed, free it */
- kfree(info->edid_data);
} else {
ret = fb_find_mode(&fb->var, fb,
NULL, NULL, 0, NULL, 8);
@@ -1974,7 +1974,7 @@ static int sm501fb_probe(struct platform_device *pdev)
/* Get EDID */
cp = of_get_property(np, "mode", &len);
if (cp)
- strcpy(fb_mode, cp);
+ info->fb_mode = kstrdup(cp, GFP_KERNEL);
prop = of_get_property(np, "edid", &len);
if (prop && len == EDID_LENGTH) {
info->edid_data = kmemdup(prop, EDID_LENGTH,
@@ -2031,6 +2031,12 @@ static int sm501fb_probe(struct platform_device *pdev)
goto err_started_crt;
}
+ /* These aren't needed any more */
+ kfree(info->edid_data);
+ kfree(info->fb_mode);
+ info->edid_data = NULL;
+ info->fb_mode = NULL;
+
/* we registered, return ok */
return 0;
@@ -2048,6 +2054,8 @@ static int sm501fb_probe(struct platform_device *pdev)
framebuffer_release(info->fb[HEAD_CRT]);
err_alloc:
+ kfree(info->edid_data);
+ kfree(info->fb_mode);
kfree(info);
return ret;
--
2.39.5
^ permalink raw reply related
* Re: [PATCH] lib/fonts: Avoid unncessary 64-bit math in font code
From: Helge Deller @ 2026-06-08 19:57 UTC (permalink / raw)
To: Thomas Zimmermann, linux-fbdev, dri-devel; +Cc: Ethan Nelson-Moore
In-Reply-To: <b80a379d-87e4-45c4-a078-09e84a840895@suse.de>
On 6/8/26 13:25, Thomas Zimmermann wrote:
> Hi
>
> Am 07.06.26 um 23:02 schrieb Helge Deller:
>> The text display code used in the Risc PC kernel image decompression
>> code uses arch/arm/boot/compressed/font.c, which includes
>> lib/fonts/font_acorn_8x8.c, which further includes <linux/font.h>.
>>
>> Since commit 97df8960240a ("lib/fonts: Provide helpers for calculating
>> glyph pitch and size") <linux/font.h> contains inline functions that
>> require __do_div64, which is not linked into the ARM kernel
>> decompressor. This makes Risc PC zImages fail to build.
>>
>> There is no need to use 64-bit division code here, so resolve this issue
>> by using plain standard addition and shift maths.
>
> Why is there a 64-bit division at all?
Not sure. Might be platform specific.
Maybe, because you add two integers and divide by an integer, that the
compiler then chooses to use 64-bit integer division by 32-bit integer.
>> Fixes: 97df8960240a ("lib/fonts: Provide helpers for calculating glyph pitch and size")
>> Reported-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> ---
>> include/linux/font.h | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/include/linux/font.h b/include/linux/font.h
>> index 6845f02d739a..67d32268989d 100644
>> --- a/include/linux/font.h
>> +++ b/include/linux/font.h
>> @@ -11,7 +11,6 @@
>> #ifndef _VIDEO_FONT_H
>> #define _VIDEO_FONT_H
>> -#include <linux/math.h>
>> #include <linux/types.h>
>> struct console_font;
>> @@ -35,7 +34,7 @@ struct console_font;
>> */
>> static inline unsigned int font_glyph_pitch(unsigned int width)
>> {
>> - return DIV_ROUND_UP(width, 8);
>> + return (width + 7) >> 3;
>
> Ok by me, if that's what's necessary.
> But can we try to keep a documented macro for the division to make the code explain itself?
I'd expect everyone who messes with this kind of low-level graphics and bitmaps
to understand this math addition and bit shift, and as such I think it should
be self-explained.
> Does it work with DIV_ROUND_UP_POW2() ?
IMHO that's even worse than DIV_ROUND_UP().
Heleg
^ permalink raw reply
* Re: [PATCH] lib/fonts: Avoid unncessary 64-bit math in font code
From: Ethan Nelson-Moore @ 2026-06-08 20:14 UTC (permalink / raw)
To: Helge Deller; +Cc: Thomas Zimmermann, linux-fbdev, dri-devel
In-Reply-To: <1492766a-c349-4cca-932e-bf608b922b6b@gmx.de>
Hi, Helge and Thomas,
On Mon, Jun 8, 2026 at 12:58 PM Helge Deller <deller@gmx.de> wrote:
>
> On 6/8/26 13:25, Thomas Zimmermann wrote:
> > Why is there a 64-bit division at all?
>
> Not sure. Might be platform specific.
> Maybe, because you add two integers and divide by an integer, that the
> compiler then chooses to use 64-bit integer division by 32-bit integer.
Actually, I think the real issue is that
arch/arm/boot/compressed/Makefile defines "static" to nothing when
compiling its copy of lib/fonts/font_acorn_8x8.c (via font.c), so that
the font array is available outside of the object file. I assume that
this causes various unused static inline functions in the headers it
includes (such as <linux/math.h>) to be included in the object file
because they become normal inline functions.
Ethan
^ permalink raw reply
* Re: [PATCH] lib/fonts: Avoid unncessary 64-bit math in font code
From: Helge Deller @ 2026-06-08 20:26 UTC (permalink / raw)
To: Ethan Nelson-Moore
Cc: Helge Deller, Thomas Zimmermann, linux-fbdev, dri-devel
In-Reply-To: <CADkSEUg948W-XT3_ODe_6p4i5Y8AKEcP=rmJ+7qJq30Uq0d_EQ@mail.gmail.com>
* Ethan Nelson-Moore <enelsonmoore@gmail.com>:
> Hi, Helge and Thomas,
>
> On Mon, Jun 8, 2026 at 12:58 PM Helge Deller <deller@gmx.de> wrote:
> >
> > On 6/8/26 13:25, Thomas Zimmermann wrote:
> > > Why is there a 64-bit division at all?
> >
> > Not sure. Might be platform specific.
> > Maybe, because you add two integers and divide by an integer, that the
> > compiler then chooses to use 64-bit integer division by 32-bit integer.
>
> Actually, I think the real issue is that
> arch/arm/boot/compressed/Makefile defines "static" to nothing when
> compiling its copy of lib/fonts/font_acorn_8x8.c (via font.c), so that
> the font array is available outside of the object file. I assume that
> this causes various unused static inline functions in the headers it
> includes (such as <linux/math.h>) to be included in the object file
> because they become normal inline functions.
Does this patch fix the issue then?
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index a159120d1e42..e3f550d62857 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -157,4 +157,4 @@ $(obj)/piggy_data: $(obj)/../Image FORCE
$(obj)/piggy.o: $(obj)/piggy_data
-CFLAGS_font.o := -Dstatic=
+CFLAGS_font.o := -DBOOTLOADER
diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c
index 36c51016769d..3327aa6d161d 100644
--- a/lib/fonts/font_acorn_8x8.c
+++ b/lib/fonts/font_acorn_8x8.c
@@ -5,7 +5,11 @@
#define FONTDATAMAX 2048
+#ifndef BOOTLOADER
static const struct font_data acorndata_8x8 = {
+#else
+const struct font_data acorndata_8x8 = {
+#endif
{ 0, 0, FONTDATAMAX, 0 }, {
/* 00 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* ^@ */
/* 01 */ 0x7e, 0x81, 0xa5, 0x81, 0xbd, 0x99, 0x81, 0x7e, /* ^A */
^ permalink raw reply related
* [PATCH] staging: sm750fb: make g_fbmode array const
From: Brock Haftner @ 2026-06-09 1:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman, outreachy
Cc: linux-fbdev, linux-staging, linux-kernel, Brock Haftner
The g_fbmode array is a static array of constant strings, but the pointer
array itself is not marked as const. Fix the checkpatch.pl warning by
adding the const modifier to the array declaration.
Signed-off-by: Brock Haftner <brockhaftner@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 89c811e0806c..8f533f3b1b42 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -21,7 +21,7 @@
static int g_hwcursor = 1;
static int g_noaccel __ro_after_init;
static int g_nomtrr __ro_after_init;
-static const char *g_fbmode[] = {NULL, NULL};
+static const char * const g_fbmode[] = {NULL, NULL};
static const char *g_def_fbmode = "1024x768-32@60";
static char *g_settings;
static int g_dualview __ro_after_init;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] lib/fonts: Avoid unncessary 64-bit math in font code
From: Ethan Nelson-Moore @ 2026-06-09 1:50 UTC (permalink / raw)
To: Helge Deller; +Cc: Helge Deller, Thomas Zimmermann, linux-fbdev, dri-devel
In-Reply-To: <aiclYUfQvMokMu64@carbonx1>
Hi, Helge,
On Mon, Jun 8, 2026 at 1:26 PM Helge Deller <deller@kernel.org> wrote:
> Does this patch fix the issue then?
Yes, it does. I remember you suggested this solution a while ago. Thank you.
Ethan
^ permalink raw reply
* [PATCH] fbcon: correct CONFIG_FB_TILEBLITTING macro name in #endif comment
From: Ethan Nelson-Moore @ 2026-06-09 3:35 UTC (permalink / raw)
To: linux-fbdev
Cc: Ethan Nelson-Moore, Helge Deller, Thomas Zimmermann,
Simona Vetter
A comment in drivers/video/fbdev/core/fbcon.c incorrectly refers to
CONFIG_MISC_TILEBLITTING instead of CONFIG_FB_TILEBLITTING. Correct it.
Discovered while searching for CONFIG_* symbols referenced in code but
not defined in any Kconfig file.
Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
---
drivers/video/fbdev/core/fbcon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index b0e3e765360d..07eab2729895 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -769,7 +769,7 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount)
return 0;
}
-#endif /* CONFIG_MISC_TILEBLITTING */
+#endif /* CONFIG_FB_TILEBLITTING */
static void fbcon_release(struct fb_info *info)
{
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] lib/fonts: Avoid unncessary 64-bit math in font code
From: Thomas Zimmermann @ 2026-06-09 6:14 UTC (permalink / raw)
To: Helge Deller, Ethan Nelson-Moore; +Cc: Helge Deller, linux-fbdev, dri-devel
In-Reply-To: <aiclYUfQvMokMu64@carbonx1>
Hi,
thanks for the fix.
Am 08.06.26 um 22:26 schrieb Helge Deller:
> * Ethan Nelson-Moore <enelsonmoore@gmail.com>:
>> Hi, Helge and Thomas,
>>
>> On Mon, Jun 8, 2026 at 12:58 PM Helge Deller <deller@gmx.de> wrote:
>>> On 6/8/26 13:25, Thomas Zimmermann wrote:
>>>> Why is there a 64-bit division at all?
>>> Not sure. Might be platform specific.
>>> Maybe, because you add two integers and divide by an integer, that the
>>> compiler then chooses to use 64-bit integer division by 32-bit integer.
>> Actually, I think the real issue is that
>> arch/arm/boot/compressed/Makefile defines "static" to nothing when
>> compiling its copy of lib/fonts/font_acorn_8x8.c (via font.c), so that
>> the font array is available outside of the object file. I assume that
>> this causes various unused static inline functions in the headers it
>> includes (such as <linux/math.h>) to be included in the object file
>> because they become normal inline functions.
> Does this patch fix the issue then?
>
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index a159120d1e42..e3f550d62857 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -157,4 +157,4 @@ $(obj)/piggy_data: $(obj)/../Image FORCE
>
> $(obj)/piggy.o: $(obj)/piggy_data
>
> -CFLAGS_font.o := -Dstatic=
> +CFLAGS_font.o := -DBOOTLOADER
> diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c
> index 36c51016769d..3327aa6d161d 100644
> --- a/lib/fonts/font_acorn_8x8.c
> +++ b/lib/fonts/font_acorn_8x8.c
> @@ -5,7 +5,11 @@
>
> #define FONTDATAMAX 2048
>
> +#ifndef BOOTLOADER
> static const struct font_data acorndata_8x8 = {
> +#else
> +const struct font_data acorndata_8x8 = {
> +#endif
> { 0, 0, FONTDATAMAX, 0 }, {
> /* 00 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* ^@ */
> /* 01 */ 0x7e, 0x81, 0xa5, 0x81, 0xbd, 0x99, 0x81, 0x7e, /* ^A */
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
I do like this better than the other patch.
Best regards
Thomas
--
--
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] fbcon: correct CONFIG_FB_TILEBLITTING macro name in #endif comment
From: Thomas Zimmermann @ 2026-06-09 6:16 UTC (permalink / raw)
To: Ethan Nelson-Moore, linux-fbdev; +Cc: Helge Deller, Simona Vetter
In-Reply-To: <20260609033503.23428-1-enelsonmoore@gmail.com>
Am 09.06.26 um 05:35 schrieb Ethan Nelson-Moore:
> A comment in drivers/video/fbdev/core/fbcon.c incorrectly refers to
> CONFIG_MISC_TILEBLITTING instead of CONFIG_FB_TILEBLITTING. Correct it.
>
> Discovered while searching for CONFIG_* symbols referenced in code but
> not defined in any Kconfig file.
>
> Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/video/fbdev/core/fbcon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index b0e3e765360d..07eab2729895 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -769,7 +769,7 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount)
> return 0;
> }
>
> -#endif /* CONFIG_MISC_TILEBLITTING */
> +#endif /* CONFIG_FB_TILEBLITTING */
>
> static void fbcon_release(struct fb_info *info)
> {
--
--
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] staging: sm750fb: make g_fbmode array const
From: Ahmet Sezgin Duran @ 2026-06-09 6:12 UTC (permalink / raw)
To: Brock Haftner, Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman,
outreachy
Cc: linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20260609011736.17401-1-brockhaftner@gmail.com>
On 6/9/26 4:17 AM, Brock Haftner wrote:
> The g_fbmode array is a static array of constant strings, but the pointer
> array itself is not marked as const. Fix the checkpatch.pl warning by
> adding the const modifier to the array declaration.
>
> Signed-off-by: Brock Haftner <brockhaftner@gmail.com>
> ---
> drivers/staging/sm750fb/sm750.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 89c811e0806c..8f533f3b1b42 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -21,7 +21,7 @@
> static int g_hwcursor = 1;
> static int g_noaccel __ro_after_init;
> static int g_nomtrr __ro_after_init;
> -static const char *g_fbmode[] = {NULL, NULL};
> +static const char * const g_fbmode[] = {NULL, NULL};
> static const char *g_def_fbmode = "1024x768-32@60";
> static char *g_settings;
> static int g_dualview __ro_after_init;
Did you compile this patch while enabling sm750fb driver in the config?
Regards,
Ahmet Sezgin Duran
^ permalink raw reply
* Re: [PATCH] lib/fonts: Avoid unncessary 64-bit math in font code
From: Helge Deller @ 2026-06-09 9:03 UTC (permalink / raw)
To: Thomas Zimmermann, Helge Deller, Ethan Nelson-Moore
Cc: linux-fbdev, dri-devel
In-Reply-To: <aaf75c58-c9d5-464a-9651-0021e6784e09@suse.de>
On 6/9/26 08:14, Thomas Zimmermann wrote:
> Hi,
>
> thanks for the fix.
>
> Am 08.06.26 um 22:26 schrieb Helge Deller:
>> * Ethan Nelson-Moore <enelsonmoore@gmail.com>:
>>> Hi, Helge and Thomas,
>>>
>>> On Mon, Jun 8, 2026 at 12:58 PM Helge Deller <deller@gmx.de> wrote:
>>>> On 6/8/26 13:25, Thomas Zimmermann wrote:
>>>>> Why is there a 64-bit division at all?
>>>> Not sure. Might be platform specific.
>>>> Maybe, because you add two integers and divide by an integer, that the
>>>> compiler then chooses to use 64-bit integer division by 32-bit integer.
>>> Actually, I think the real issue is that
>>> arch/arm/boot/compressed/Makefile defines "static" to nothing when
>>> compiling its copy of lib/fonts/font_acorn_8x8.c (via font.c), so that
>>> the font array is available outside of the object file. I assume that
>>> this causes various unused static inline functions in the headers it
>>> includes (such as <linux/math.h>) to be included in the object file
>>> because they become normal inline functions.
>> Does this patch fix the issue then?
>>
>> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
>> index a159120d1e42..e3f550d62857 100644
>> --- a/arch/arm/boot/compressed/Makefile
>> +++ b/arch/arm/boot/compressed/Makefile
>> @@ -157,4 +157,4 @@ $(obj)/piggy_data: $(obj)/../Image FORCE
>> $(obj)/piggy.o: $(obj)/piggy_data
>> -CFLAGS_font.o := -Dstatic=
>> +CFLAGS_font.o := -DBOOTLOADER
>> diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c
>> index 36c51016769d..3327aa6d161d 100644
>> --- a/lib/fonts/font_acorn_8x8.c
>> +++ b/lib/fonts/font_acorn_8x8.c
>> @@ -5,7 +5,11 @@
>> #define FONTDATAMAX 2048
>> +#ifndef BOOTLOADER
>> static const struct font_data acorndata_8x8 = {
>> +#else
>> +const struct font_data acorndata_8x8 = {
>> +#endif
>> { 0, 0, FONTDATAMAX, 0 }, {
>> /* 00 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* ^@ */
>> /* 01 */ 0x7e, 0x81, 0xa5, 0x81, 0xbd, 0x99, 0x81, 0x7e, /* ^A */
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Thanks for review!
> I do like this better than the other patch.
Yes, I'll drop the other patch and re-send this one properly.
Helge
^ permalink raw reply
* [PATCH] fbdev/arm: Export acorndata_8x8 font symbol for bootloader
From: Helge Deller @ 2026-06-09 9:10 UTC (permalink / raw)
To: linux-fbdev, dri-devel
Cc: Ethan Nelson-Moore, Thomas Zimmermann, linux-arm-kernel,
Russell King
The text display code used in the Risc PC kernel image decompression
code uses arch/arm/boot/compressed/font.c, which includes
lib/fonts/font_acorn_8x8.c, which further includes <linux/font.h>.
Since commit 97df8960240a ("lib/fonts: Provide helpers for calculating
glyph pitch and size") <linux/font.h> contains inline functions that
require __do_div64, which is not linked into the ARM kernel
decompressor. This makes Risc PC zImages fail to build.
Resolve this issue by defining the BOOTLOADER symbol and use it to avoid
a static declaration of the acorndata_8x8 symbol. That way it can be
referenced by the arm bootloader, and other static math functions and
symbols (like __do_div64) stay static and don't get unneccesary included
in the ARM kernel bootloader decompressor object file.
Fixes: 97df8960240a ("lib/fonts: Provide helpers for calculating glyph pitch and size")
Reported-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Helge Deller <deller@gmx.de>
---
arch/arm/boot/compressed/Makefile | 2 +-
lib/fonts/font_acorn_8x8.c | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index a159120d1e42..e3f550d62857 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -157,4 +157,4 @@ $(obj)/piggy_data: $(obj)/../Image FORCE
$(obj)/piggy.o: $(obj)/piggy_data
-CFLAGS_font.o := -Dstatic=
+CFLAGS_font.o := -DBOOTLOADER
diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c
index 36c51016769d..4ff52c79f8c4 100644
--- a/lib/fonts/font_acorn_8x8.c
+++ b/lib/fonts/font_acorn_8x8.c
@@ -5,7 +5,12 @@
#define FONTDATAMAX 2048
+#ifdef BOOTLOADER
+/* The acorndata_8x8 symbol is needed by the ARM bootloader too. */
+const struct font_data acorndata_8x8 = {
+#else
static const struct font_data acorndata_8x8 = {
+#endif
{ 0, 0, FONTDATAMAX, 0 }, {
/* 00 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* ^@ */
/* 01 */ 0x7e, 0x81, 0xa5, 0x81, 0xbd, 0x99, 0x81, 0x7e, /* ^A */
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] fbcon: correct CONFIG_FB_TILEBLITTING macro name in #endif comment
From: Helge Deller @ 2026-06-09 9:13 UTC (permalink / raw)
To: Thomas Zimmermann, Ethan Nelson-Moore, linux-fbdev; +Cc: Simona Vetter
In-Reply-To: <64b807ea-649c-4ee6-9db4-631e310465ff@suse.de>
On 6/9/26 08:16, Thomas Zimmermann wrote:
> Am 09.06.26 um 05:35 schrieb Ethan Nelson-Moore:
>> A comment in drivers/video/fbdev/core/fbcon.c incorrectly refers to
>> CONFIG_MISC_TILEBLITTING instead of CONFIG_FB_TILEBLITTING. Correct it.
>>
>> Discovered while searching for CONFIG_* symbols referenced in code but
>> not defined in any Kconfig file.
>>
>> Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH] fbdev/arm: Export acorndata_8x8 font symbol for bootloader
From: Russell King (Oracle) @ 2026-06-09 9:46 UTC (permalink / raw)
To: Helge Deller
Cc: linux-fbdev, dri-devel, Ethan Nelson-Moore, Thomas Zimmermann,
linux-arm-kernel
In-Reply-To: <20260609091056.265794-1-deller@gmx.de>
On Tue, Jun 09, 2026 at 11:10:56AM +0200, Helge Deller wrote:
> The text display code used in the Risc PC kernel image decompression
> code uses arch/arm/boot/compressed/font.c, which includes
> lib/fonts/font_acorn_8x8.c, which further includes <linux/font.h>.
>
> Since commit 97df8960240a ("lib/fonts: Provide helpers for calculating
> glyph pitch and size") <linux/font.h> contains inline functions that
> require __do_div64, which is not linked into the ARM kernel
> decompressor. This makes Risc PC zImages fail to build.
>
> Resolve this issue by defining the BOOTLOADER symbol and use it to avoid
> a static declaration of the acorndata_8x8 symbol. That way it can be
> referenced by the arm bootloader, and other static math functions and
> symbols (like __do_div64) stay static and don't get unneccesary included
> in the ARM kernel bootloader decompressor object file.
The decompressor font support has actually been broken since:
commit 6735b4632def0640dbdf4eb9f99816aca18c4f16
Author: Peilin Ye <yepeilin.cs@gmail.com>
Date: Thu Sep 24 09:42:22 2020 -0400
Fonts: Support FONT_EXTRA_WORDS macros for built-in fonts
which added extra data to the beginning of the array of font
information:
ENTRY(ll_write_char)
stmfd sp!, {r4 - r7, lr}
...
/*
* calculate offset into character table
*/
mov r1, r1, lsl #3
r1 is the character, this multiplies the character value by 8.
adr ip, LC0
ldmia ip, {r3, r4, r5, r6, lr}
sub ip, ip, r3
add r6, r6, ip
in conjunction with the data table:
LC0: .word LC0
.word bytes_per_char_h
.word video_size_row
.word acorndata_8x8
.word con_charconvtable
results in r6 pointing at acorndata_8x8. We then index this using the
modified character value above:
orr r1, r1, #7
ldrb r7, [r6, r1]
This breaks if extra data is added to the start, and thus has been
broken since the above commit.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH 1/1] video, sm501: Fix buffer errors in OF binding code
From: Helge Deller @ 2026-06-09 14:00 UTC (permalink / raw)
To: David Laight, Heiko Schocher, linux-fbdev, devicetree-discuss,
Ben Dooks, Vincent Sanders, Samuel Ortiz, linux-kernel,
Randy Dunlap, Paul Mundt, Danila Chernetsov, Kees Cook
In-Reply-To: <20260608124242.13164-1-david.laight.linux@gmail.com>
On 6/8/26 14:42, David Laight wrote:
> The code that gets the frame buffer mode from OF has 'use after free',
> 'buffer overrun' and memory leaks.
>
> info->edid_data isn't free if the probe functions fail or if
> pd->def_mode is set.
>
> If both the CRT and PANEL are enabled info->edid_data is used after
> being freed and is freed twice.
>
> The string returned by of_get_property(np, "mode", &len) is just
> written over either the static "640x480-16@60" or the module parameter
> string without any regard for the length (which is most likely longer).
>
> Use kstrump() for the OF mode and free everything before freeing 'info.
>
> Fixes: 4295f9bf74a88 ("video, sm501: add OF binding to support SM501")
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> drivers/video/fbdev/sm501fb.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH next] drivers/video/fbdev/s3fb: Use strscpy() to copy strings into arrays
From: Helge Deller @ 2026-06-09 14:07 UTC (permalink / raw)
To: david.laight.linux, Kees Cook, linux-hardening, dri-devel,
linux-fbdev, linux-kernel
Cc: Arnd Bergmann
In-Reply-To: <20260608095523.2606-11-david.laight.linux@gmail.com>
On 6/8/26 11:54, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> Replacing strcpy() with strscpy() ensures that overflow of the target
> buffer cannot happen.
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
>
> drivers/video/fbdev/s3fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH] video: fbdev: remove skeletonfb example driver with no remaining purpose
From: Helge Deller @ 2026-06-09 14:19 UTC (permalink / raw)
To: Geert Uytterhoeven, Ethan Nelson-Moore; +Cc: linux-fbdev
In-Reply-To: <CAMuHMdXiToNLeQqW54+tOm6-eh9Xefxe3QaMC2Zg7r-3pBOx8A@mail.gmail.com>
On 6/8/26 10:03, Geert Uytterhoeven wrote:
> On Sun, 7 Jun 2026 at 03:58, Ethan Nelson-Moore <enelsonmoore@gmail.com> wrote:
>> The skeletonfb driver is intended to serve as an example for writing
>> new framebuffer drivers. However, new framebuffer drivers are no longer
>> accepted into the kernel because DRM has obsoleted fbdev, so it no
>> longer has a purpose. In spite of this, it continues to be updated to
>> reflect fbdev API changes, wasting maintainers' time.
I don't want to argue, but the last update was 3 years ago, so
I don't see that much wasted time.
> Remove it.
>>
>> Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
>
> Thanks for your patch!
>
> Makes sense, as we still have vfb.c, which can actually be built.
> Perhaps some of the comments and/or kerneldoc should be moved
> elsewhere, so it is preserved?
That's what I'm mostly concerned about.
It has so much (still) useful info, that it would be sad to loose that.
Looking up the info in an already deleted file isn't a good alternative.
So, either someone should move comments/info over, or I'd like to keep it
for some more time as it's not actually hurting anybody.
Helge
^ permalink raw reply
* [PATCH] fbdev: modedb: keep mode option buffer until parsing completes
From: Ruoyu Wang @ 2026-06-09 16:00 UTC (permalink / raw)
To: Simona Vetter, Helge Deller, Javier Martinez Canillas,
Thomas Zimmermann
Cc: linux-fbdev, dri-devel, linux-kernel, Ruoyu Wang
When fb_find_mode() obtains the mode option from fb_get_options(),
mode_option_buf owns the returned string and name points into that
buffer. The done label frees mode_option_buf before the database
fallback has finished using name in name_matches(), so the fallback can
read freed memory.
Move the free to a common exit path and convert the successful returns
that can use mode_option_buf into jumps to that exit path.
Fixes: 089d924d03d5 ("fbdev: Read video= option with fb_get_option() in modedb")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
drivers/video/fbdev/core/modedb.c | 41 ++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
index 703d0b7aec322..82f6ea38e1fb8 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -627,7 +627,7 @@ int fb_find_mode(struct fb_var_screeninfo *var,
unsigned int default_bpp)
{
char *mode_option_buf = NULL;
- int i;
+ int i, ret;
/* Set up defaults */
if (!db) {
@@ -724,10 +724,9 @@ int fb_find_mode(struct fb_var_screeninfo *var,
res_specified = 1;
}
done:
- kfree(mode_option_buf);
if (cvt) {
struct fb_videomode cvt_mode;
- int ret;
+ int cvt_ret;
DPRINTK("CVT mode %dx%d@%dHz%s%s%s\n", xres, yres,
(refresh) ? refresh : 60,
@@ -745,11 +744,12 @@ int fb_find_mode(struct fb_var_screeninfo *var,
else
cvt_mode.vmode &= ~FB_VMODE_INTERLACED;
- ret = fb_find_mode_cvt(&cvt_mode, margins, rb);
+ cvt_ret = fb_find_mode_cvt(&cvt_mode, margins, rb);
- if (!ret && !fb_try_mode(var, info, &cvt_mode, bpp)) {
+ if (!cvt_ret && !fb_try_mode(var, info, &cvt_mode, bpp)) {
DPRINTK("modedb CVT: CVT mode ok\n");
- return 1;
+ ret = 1;
+ goto out;
}
DPRINTK("CVT mode invalid, getting mode from database\n");
@@ -793,8 +793,10 @@ int fb_find_mode(struct fb_var_screeninfo *var,
if (!interlace_specified ||
db_interlace == interlace)
if (refresh_specified &&
- db[i].refresh == refresh)
- return 1;
+ db[i].refresh == refresh) {
+ ret = 1;
+ goto out;
+ }
if (score < diff) {
diff = score;
@@ -804,7 +806,8 @@ int fb_find_mode(struct fb_var_screeninfo *var,
}
if (best != -1) {
fb_try_mode(var, info, &db[best], bpp);
- return (refresh_specified) ? 2 : 1;
+ ret = (refresh_specified) ? 2 : 1;
+ goto out;
}
diff = 2 * (xres + yres);
@@ -831,21 +834,29 @@ int fb_find_mode(struct fb_var_screeninfo *var,
}
if (best != -1) {
fb_try_mode(var, info, &db[best], bpp);
- return 5;
+ ret = 5;
+ goto out;
}
}
DPRINTK("Trying default video mode\n");
- if (!fb_try_mode(var, info, default_mode, default_bpp))
- return 3;
+ if (!fb_try_mode(var, info, default_mode, default_bpp)) {
+ ret = 3;
+ goto out;
+ }
DPRINTK("Trying all modes\n");
for (i = 0; i < dbsize; i++)
- if (!fb_try_mode(var, info, &db[i], default_bpp))
- return 4;
+ if (!fb_try_mode(var, info, &db[i], default_bpp)) {
+ ret = 4;
+ goto out;
+ }
DPRINTK("No valid mode found\n");
- return 0;
+ ret = 0;
+out:
+ kfree(mode_option_buf);
+ return ret;
}
/**
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] fbdev: modedb: keep mode option buffer until parsing completes
From: Helge Deller @ 2026-06-09 16:07 UTC (permalink / raw)
To: Ruoyu Wang, Simona Vetter, Javier Martinez Canillas,
Thomas Zimmermann
Cc: linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20260609160028.5-1-ruoyuw560@gmail.com>
On 6/9/26 18:00, Ruoyu Wang wrote:
> When fb_find_mode() obtains the mode option from fb_get_options(),
> mode_option_buf owns the returned string and name points into that
> buffer. The done label frees mode_option_buf before the database
> fallback has finished using name in name_matches(), so the fallback can
> read freed memory.
>
> Move the free to a common exit path and convert the successful returns
> that can use mode_option_buf into jumps to that exit path.
There was another similiar patch already posted:
https://patchwork.kernel.org/project/linux-fbdev/patch/20260526091507.421730-1-islituo@gmail.com/
Do you want to check if the Scope-based kfree can be used here,
as suggested by me in that thread? It's at least much smaller than your patch...
Helge
>
> Fixes: 089d924d03d5 ("fbdev: Read video= option with fb_get_option() in modedb")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
> drivers/video/fbdev/core/modedb.c | 41 ++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
> index 703d0b7aec322..82f6ea38e1fb8 100644
> --- a/drivers/video/fbdev/core/modedb.c
> +++ b/drivers/video/fbdev/core/modedb.c
> @@ -627,7 +627,7 @@ int fb_find_mode(struct fb_var_screeninfo *var,
> unsigned int default_bpp)
> {
> char *mode_option_buf = NULL;
> - int i;
> + int i, ret;
>
> /* Set up defaults */
> if (!db) {
> @@ -724,10 +724,9 @@ int fb_find_mode(struct fb_var_screeninfo *var,
> res_specified = 1;
> }
> done:
> - kfree(mode_option_buf);
> if (cvt) {
> struct fb_videomode cvt_mode;
> - int ret;
> + int cvt_ret;
>
> DPRINTK("CVT mode %dx%d@%dHz%s%s%s\n", xres, yres,
> (refresh) ? refresh : 60,
> @@ -745,11 +744,12 @@ int fb_find_mode(struct fb_var_screeninfo *var,
> else
> cvt_mode.vmode &= ~FB_VMODE_INTERLACED;
>
> - ret = fb_find_mode_cvt(&cvt_mode, margins, rb);
> + cvt_ret = fb_find_mode_cvt(&cvt_mode, margins, rb);
>
> - if (!ret && !fb_try_mode(var, info, &cvt_mode, bpp)) {
> + if (!cvt_ret && !fb_try_mode(var, info, &cvt_mode, bpp)) {
> DPRINTK("modedb CVT: CVT mode ok\n");
> - return 1;
> + ret = 1;
> + goto out;
> }
>
> DPRINTK("CVT mode invalid, getting mode from database\n");
> @@ -793,8 +793,10 @@ int fb_find_mode(struct fb_var_screeninfo *var,
> if (!interlace_specified ||
> db_interlace == interlace)
> if (refresh_specified &&
> - db[i].refresh == refresh)
> - return 1;
> + db[i].refresh == refresh) {
> + ret = 1;
> + goto out;
> + }
>
> if (score < diff) {
> diff = score;
> @@ -804,7 +806,8 @@ int fb_find_mode(struct fb_var_screeninfo *var,
> }
> if (best != -1) {
> fb_try_mode(var, info, &db[best], bpp);
> - return (refresh_specified) ? 2 : 1;
> + ret = (refresh_specified) ? 2 : 1;
> + goto out;
> }
>
> diff = 2 * (xres + yres);
> @@ -831,21 +834,29 @@ int fb_find_mode(struct fb_var_screeninfo *var,
> }
> if (best != -1) {
> fb_try_mode(var, info, &db[best], bpp);
> - return 5;
> + ret = 5;
> + goto out;
> }
> }
>
> DPRINTK("Trying default video mode\n");
> - if (!fb_try_mode(var, info, default_mode, default_bpp))
> - return 3;
> + if (!fb_try_mode(var, info, default_mode, default_bpp)) {
> + ret = 3;
> + goto out;
> + }
>
> DPRINTK("Trying all modes\n");
> for (i = 0; i < dbsize; i++)
> - if (!fb_try_mode(var, info, &db[i], default_bpp))
> - return 4;
> + if (!fb_try_mode(var, info, &db[i], default_bpp)) {
> + ret = 4;
> + goto out;
> + }
>
> DPRINTK("No valid mode found\n");
> - return 0;
> + ret = 0;
> +out:
> + kfree(mode_option_buf);
> + return ret;
> }
>
> /**
^ permalink raw reply
* Re: [PATCH v4 02/14] mfd: lm3533: Remove driver specific regmap wrappers
From: Andy Shevchenko @ 2026-06-09 19:02 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <CAPVz0n2rdgw8Xr3uxVdQGwrHTNFqK4SKQDFU2FEB8LzLwPhQ_A@mail.gmail.com>
On Sat, Jun 06, 2026 at 10:22:43AM +0300, Svyatoslav Ryhel wrote:
> сб, 6 черв. 2026 р. о 09:53 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
> > On Sat, Jun 06, 2026 at 07:57:26AM +0300, Svyatoslav Ryhel wrote:
...
> > > + ret = regmap_assign_bits(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO,
> > > + LM3533_ALS_INT_ENABLE_MASK, enable);
> >
> > In cases like this perhaps leaving mask would be fine and together with
>
> I prefer to remove intermediate variables it the helper allows to
> directly pass needed value.
>
> > struct regmap *map = als->lm3533->regmap;
>
> next patch drops lm3533 so there will be als->regmap which IMHO is
> more logical instead of passing entire lm3533 to child devices.
Still it's longer than map. A local variable may help with making lines
shorter.
> > this be nice one-liner:
> >
> > ret = regmap_assign_bits(map, LM3533_REG_ALS_ZONE_INFO, mask, enable);
> >
> > > if (ret) {
> > > dev_err(&indio_dev->dev, "failed to set int mode %d\n",
> > > enable);
> >
> > In many cases it won't increase LoC count.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 04/14] mfd: lm3533: Pass only regmap and light sensor presence to child devices
From: Andy Shevchenko @ 2026-06-09 19:06 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <20260606045738.21050-5-clamor95@gmail.com>
On Sat, Jun 06, 2026 at 07:57:28AM +0300, Svyatoslav Ryhel wrote:
> Instead of passing the entire lm3533 core data structure, only pass the
> regmap and the light sensor presence flag to child devices.
...
> struct lm3533_als {
> - struct lm3533 *lm3533;
> + struct regmap *regmap;
> struct platform_device *pdev;
And this pdev is probably not needed. But I haven't checked the whole lot of
the patches yet.
> unsigned long flags;
...
> struct lm3533_ctrlbank {
> - struct lm3533 *lm3533;
> + struct regmap *regmap;
> struct device *dev;
Ditto.
> int id;
> };
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 05/14] iio: light: lm3533-als: Remove redundant pdata helpers
From: Andy Shevchenko @ 2026-06-09 19:10 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <20260606045738.21050-6-clamor95@gmail.com>
On Sat, Jun 06, 2026 at 07:57:29AM +0300, Svyatoslav Ryhel wrote:
> The lm3533_als_set_input_mode and lm3533_als_set_resistor functions are
> used only in lm3533_als_setup. Incorporate their code into
> lm3533_als_setup directly to simplify driver readability.
Use func() when referring to a function in the commit message.
...
> static int lm3533_als_setup(struct lm3533_als *als,
> const struct lm3533_als_platform_data *pdata)
> {
> + struct device *dev = &als->pdev->dev;
> int ret;
>
> - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> + ret = regmap_assign_bits(als->regmap, LM3533_REG_ALS_CONF,
> + LM3533_ALS_INPUT_MODE_MASK, pdata->pwm_mode);
> if (ret)
> - return ret;
> + return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> + pdata->pwm_mode);
>
> /* ALS input is always high impedance in PWM-mode. */
> if (!pdata->pwm_mode) {
> - ret = lm3533_als_set_resistor(als, pdata->r_select);
> + if (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
> + pdata->r_select > LM3533_ALS_RESISTOR_MAX)
> + return dev_err_probe(dev, -EINVAL,
> + "invalid resistor value\n");
> +
> + ret = regmap_write(als->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
> + pdata->r_select);
> if (ret)
> - return ret;
> + return dev_err_probe(dev, ret, "failed to set resistor\n");
> }
>
> return 0;
Wondering if it would be better to
/* Bail out when in PWM-mode */
if (pdata->pwm_mode)
return 0;
/* ALS input is always high impedance in PWM-mode. */
...
as the above changes almost every line in that conditional.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 07/14] mfd: lm3533: Switch sysfs_create_group() to device_add_group()
From: Andy Shevchenko @ 2026-06-09 19:13 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <20260606045738.21050-8-clamor95@gmail.com>
On Sat, Jun 06, 2026 at 07:57:31AM +0300, Svyatoslav Ryhel wrote:
> Switch from sysfs_create_group() to device_add_group() including device
> managed where appropriate.
This should use .dev_groups member of struct device_driver.
...
> + ret = devm_device_add_group(&bd->dev, &lm3533_bl_attribute_group);
This will make Greg KH very grumpy. (For the record, original code as well
but it already is in upstream. So, thanks for trying to address this, just
needs a bit more of work.)
> + if (ret < 0)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to create sysfs attributes\n");
--
With Best Regards,
Andy Shevchenko
^ 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