* Re: [PATCH v3] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
From: Helge Deller @ 2025-10-10 12:08 UTC (permalink / raw)
To: Uwe Kleine-König, Javier Garcia
Cc: tzimmermann, linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <dis2jb72ejrbmv26jdj3rwawrdmhmde5fahrkdn6y3elsgg4p7@wsjopejnmz5f>
On 10/9/25 10:50, Uwe Kleine-König wrote:
> Hello Javier,
>
> On Wed, Oct 08, 2025 at 08:36:27PM +0200, Javier Garcia wrote:
>> This patch wraps the relevant code blocks with `IS_ENABLED(CONFIG_FB_DEVICE)`.
>>
>> Allows the driver to be used for framebuffer text console, even if
>> support for the /dev/fb device isn't compiled-in (CONFIG_FB_DEVICE=n).
>>
>> This align with Documentation/drm/todo.rst
This seems to be Documentation/gpu/todo.rst now...
>> "Remove driver dependencies on FB_DEVICE">>> I've not the card so I was not able to test it.
>
> I still don't understand why the creation of the dispregs sysfs file
> should be conditional on FB_DEVICE.
I think this is because people simply believe it, as it's documented like this
in the todo file. I think this is wrong.
I think the problem was, that device_create_file() has a "struct device *"
pointer as first parameter. Some device drivers probably referenced
the "struct device" pointer of the "/dev/fb" device, which does not exist
when FB_DEVICE isn't enabled. As such, the device_create_file() would fail
during initialization (since the devide ptr is NULL) of the driver and
prevent the driver from working.
That's not the case for this driver here, and probably not for the other
remaining drivers.
> Either they have nothing to do with each other, or I'm missing
> something.
Right now you are right... it has nothing to do with each other.
> The former makes this patch wrong, the latter would be an
> indication that the commit log is still non-optimal.
Either way, I've dropped the patch from the git repo for now.
I don't think the patch is wrong, but it's not deemed necessary either.
If someone has that device I'd happy to apply it after some feedback.
In addition, maybe the section from the todo file should be dropped?
Helge
^ permalink raw reply
* Re: [PATCH v2] fbcon: Set fb_display[i]->mode to NULL when the mode is released
From: Thomas Zimmermann @ 2025-10-10 8:49 UTC (permalink / raw)
To: Quanmin Yan
Cc: simona, deller, linux-kernel, linux-fbdev, dri-devel,
wangkefeng.wang, zuoze1, sunnanyong
In-Reply-To: <20251010081659.3609082-1-yanquanmin1@huawei.com>
Am 10.10.25 um 10:16 schrieb Quanmin Yan:
> Recently, we discovered the following issue through syzkaller:
>
> BUG: KASAN: slab-use-after-free in fb_mode_is_equal+0x285/0x2f0
> Read of size 4 at addr ff11000001b3c69c by task syz.xxx
> ...
> Call Trace:
> <TASK>
> dump_stack_lvl+0xab/0xe0
> print_address_description.constprop.0+0x2c/0x390
> print_report+0xb9/0x280
> kasan_report+0xb8/0xf0
> fb_mode_is_equal+0x285/0x2f0
> fbcon_mode_deleted+0x129/0x180
> fb_set_var+0xe7f/0x11d0
> do_fb_ioctl+0x6a0/0x750
> fb_ioctl+0xe0/0x140
> __x64_sys_ioctl+0x193/0x210
> do_syscall_64+0x5f/0x9c0
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Based on experimentation and analysis, during framebuffer unregistration,
> only the memory of fb_info->modelist is freed, without setting the
> corresponding fb_display[i]->mode to NULL for the freed modes. This leads
> to UAF issues during subsequent accesses. Here's an example of reproduction
> steps:
> 1. With /dev/fb0 already registered in the system, load a kernel module
> to register a new device /dev/fb1;
> 2. Set fb1's mode to the global fb_display[] array (via FBIOPUT_CON2FBMAP);
> 3. Switch console from fb to VGA (to allow normal rmmod of the ko);
> 4. Unload the kernel module, at this point fb1's modelist is freed, leaving
> a wild pointer in fb_display[];
> 5. Trigger the bug via system calls through fb0 attempting to delete a mode
> from fb0.
>
> Add a check in do_unregister_framebuffer(): if the mode to be freed exists
> in fb_display[], set the corresponding mode pointer to NULL.
>
> Signed-off-by: Quanmin Yan <yanquanmin1@huawei.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Thanks for fixing the bug.
> ---
>
> Changes from v1
> (https://lore.kernel.org/all/20250923110608.3385083-1-yanquanmin1@huawei.com/)
> - Focus on fixing the issue specifically in the framebuffer unregistration
> path, as other paths do not encounter this problem.
> - Adjusted according to Thomas's suggestions.
>
> drivers/video/fbdev/core/fbcon.c | 19 +++++++++++++++++++
> drivers/video/fbdev/core/fbmem.c | 1 +
> include/linux/fbcon.h | 2 ++
> 3 files changed, 22 insertions(+)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 96cc9b389246..9bd3c3814b5c 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -2810,6 +2810,25 @@ int fbcon_mode_deleted(struct fb_info *info,
> return found;
> }
>
> +static void fbcon_delete_mode(struct fb_videomode *m)
> +{
> + struct fbcon_display *p;
> +
> + for (int i = first_fb_vc; i <= last_fb_vc; i++) {
> + p = &fb_display[i];
> + if (p->mode == m)
> + p->mode = NULL;
> + }
> +}
> +
> +void fbcon_delete_modelist(struct list_head *head)
> +{
> + struct fb_modelist *modelist;
> +
> + list_for_each_entry(modelist, head, list)
> + fbcon_delete_mode(&modelist->mode);
> +}
> +
> #ifdef CONFIG_VT_HW_CONSOLE_BINDING
> static void fbcon_unbind(void)
> {
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 53f1719b1ae1..eff757ebbed1 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -544,6 +544,7 @@ static void do_unregister_framebuffer(struct fb_info *fb_info)
> fb_info->pixmap.addr = NULL;
> }
>
> + fbcon_delete_modelist(&fb_info->modelist);
> fb_destroy_modelist(&fb_info->modelist);
> registered_fb[fb_info->node] = NULL;
> num_registered_fb--;
> diff --git a/include/linux/fbcon.h b/include/linux/fbcon.h
> index 81f0e698acbf..f206370060e1 100644
> --- a/include/linux/fbcon.h
> +++ b/include/linux/fbcon.h
> @@ -18,6 +18,7 @@ void fbcon_suspended(struct fb_info *info);
> void fbcon_resumed(struct fb_info *info);
> int fbcon_mode_deleted(struct fb_info *info,
> struct fb_videomode *mode);
> +void fbcon_delete_modelist(struct list_head *head);
> void fbcon_new_modelist(struct fb_info *info);
> void fbcon_get_requirement(struct fb_info *info,
> struct fb_blit_caps *caps);
> @@ -38,6 +39,7 @@ static inline void fbcon_suspended(struct fb_info *info) {}
> static inline void fbcon_resumed(struct fb_info *info) {}
> static inline int fbcon_mode_deleted(struct fb_info *info,
> struct fb_videomode *mode) { return 0; }
> +static inline void fbcon_delete_modelist(struct list_head *head) {}
> static inline void fbcon_new_modelist(struct fb_info *info) {}
> static inline void fbcon_get_requirement(struct fb_info *info,
> struct fb_blit_caps *caps) {}
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* [PATCH v2] fbcon: Set fb_display[i]->mode to NULL when the mode is released
From: Quanmin Yan @ 2025-10-10 8:16 UTC (permalink / raw)
To: tzimmermann
Cc: simona, deller, linux-kernel, linux-fbdev, dri-devel, yanquanmin1,
wangkefeng.wang, zuoze1, sunnanyong
Recently, we discovered the following issue through syzkaller:
BUG: KASAN: slab-use-after-free in fb_mode_is_equal+0x285/0x2f0
Read of size 4 at addr ff11000001b3c69c by task syz.xxx
...
Call Trace:
<TASK>
dump_stack_lvl+0xab/0xe0
print_address_description.constprop.0+0x2c/0x390
print_report+0xb9/0x280
kasan_report+0xb8/0xf0
fb_mode_is_equal+0x285/0x2f0
fbcon_mode_deleted+0x129/0x180
fb_set_var+0xe7f/0x11d0
do_fb_ioctl+0x6a0/0x750
fb_ioctl+0xe0/0x140
__x64_sys_ioctl+0x193/0x210
do_syscall_64+0x5f/0x9c0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
Based on experimentation and analysis, during framebuffer unregistration,
only the memory of fb_info->modelist is freed, without setting the
corresponding fb_display[i]->mode to NULL for the freed modes. This leads
to UAF issues during subsequent accesses. Here's an example of reproduction
steps:
1. With /dev/fb0 already registered in the system, load a kernel module
to register a new device /dev/fb1;
2. Set fb1's mode to the global fb_display[] array (via FBIOPUT_CON2FBMAP);
3. Switch console from fb to VGA (to allow normal rmmod of the ko);
4. Unload the kernel module, at this point fb1's modelist is freed, leaving
a wild pointer in fb_display[];
5. Trigger the bug via system calls through fb0 attempting to delete a mode
from fb0.
Add a check in do_unregister_framebuffer(): if the mode to be freed exists
in fb_display[], set the corresponding mode pointer to NULL.
Signed-off-by: Quanmin Yan <yanquanmin1@huawei.com>
---
Changes from v1
(https://lore.kernel.org/all/20250923110608.3385083-1-yanquanmin1@huawei.com/)
- Focus on fixing the issue specifically in the framebuffer unregistration
path, as other paths do not encounter this problem.
- Adjusted according to Thomas's suggestions.
drivers/video/fbdev/core/fbcon.c | 19 +++++++++++++++++++
drivers/video/fbdev/core/fbmem.c | 1 +
include/linux/fbcon.h | 2 ++
3 files changed, 22 insertions(+)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 96cc9b389246..9bd3c3814b5c 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2810,6 +2810,25 @@ int fbcon_mode_deleted(struct fb_info *info,
return found;
}
+static void fbcon_delete_mode(struct fb_videomode *m)
+{
+ struct fbcon_display *p;
+
+ for (int i = first_fb_vc; i <= last_fb_vc; i++) {
+ p = &fb_display[i];
+ if (p->mode == m)
+ p->mode = NULL;
+ }
+}
+
+void fbcon_delete_modelist(struct list_head *head)
+{
+ struct fb_modelist *modelist;
+
+ list_for_each_entry(modelist, head, list)
+ fbcon_delete_mode(&modelist->mode);
+}
+
#ifdef CONFIG_VT_HW_CONSOLE_BINDING
static void fbcon_unbind(void)
{
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 53f1719b1ae1..eff757ebbed1 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -544,6 +544,7 @@ static void do_unregister_framebuffer(struct fb_info *fb_info)
fb_info->pixmap.addr = NULL;
}
+ fbcon_delete_modelist(&fb_info->modelist);
fb_destroy_modelist(&fb_info->modelist);
registered_fb[fb_info->node] = NULL;
num_registered_fb--;
diff --git a/include/linux/fbcon.h b/include/linux/fbcon.h
index 81f0e698acbf..f206370060e1 100644
--- a/include/linux/fbcon.h
+++ b/include/linux/fbcon.h
@@ -18,6 +18,7 @@ void fbcon_suspended(struct fb_info *info);
void fbcon_resumed(struct fb_info *info);
int fbcon_mode_deleted(struct fb_info *info,
struct fb_videomode *mode);
+void fbcon_delete_modelist(struct list_head *head);
void fbcon_new_modelist(struct fb_info *info);
void fbcon_get_requirement(struct fb_info *info,
struct fb_blit_caps *caps);
@@ -38,6 +39,7 @@ static inline void fbcon_suspended(struct fb_info *info) {}
static inline void fbcon_resumed(struct fb_info *info) {}
static inline int fbcon_mode_deleted(struct fb_info *info,
struct fb_videomode *mode) { return 0; }
+static inline void fbcon_delete_modelist(struct list_head *head) {}
static inline void fbcon_new_modelist(struct fb_info *info) {}
static inline void fbcon_get_requirement(struct fb_info *info,
struct fb_blit_caps *caps) {}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] fbcon: Set fb_display[i]->mode to NULL when the mode is released
From: Quanmin Yan @ 2025-10-10 8:10 UTC (permalink / raw)
To: Thomas Zimmermann, simona
Cc: deller, linux-kernel, linux-fbdev, dri-devel, wangkefeng.wang,
zuoze1, sunnanyong
In-Reply-To: <234d94ac-3984-4ff9-9743-9178f68370de@suse.de>
Hi Thomas,
在 2025/9/24 17:17, Thomas Zimmermann 写道:
> Hi,
>
> thanks for the report.
>
> Am 23.09.25 um 13:06 schrieb Quanmin Yan:
>> Recently, we discovered the following issue through syzkaller:
>>
>> BUG: KASAN: slab-use-after-free in fb_mode_is_equal+0x285/0x2f0
>> Read of size 4 at addr ff11000001b3c69c by task syz.xxx
>> ...
>> Call Trace:
>> <TASK>
>> dump_stack_lvl+0xab/0xe0
>> print_address_description.constprop.0+0x2c/0x390
>> print_report+0xb9/0x280
>> kasan_report+0xb8/0xf0
>> fb_mode_is_equal+0x285/0x2f0
>> fbcon_mode_deleted+0x129/0x180
>> fb_set_var+0xe7f/0x11d0
>> do_fb_ioctl+0x6a0/0x750
>> fb_ioctl+0xe0/0x140
>> __x64_sys_ioctl+0x193/0x210
>> do_syscall_64+0x5f/0x9c0
>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>
>> The issue occurs in the function fb_mode_is_equal(p->mode, mode), I also
>> noticed that when freeing the memory related to fb_info->modelist,
>> there's
>> no attempt to set the corresponding fb_display[i]->mode to NULL after
>> freeing. Based on analysis, the root cause of this bug appears to be
>> that
>> a certain p->mode has become a wild pointer.
>>
>> I've identified two code paths for freeing modelist->mode:
>> 1. fb_delete_videomode - removes videomode entry from modelist.
>> 2. fb_destroy_modelist - destroys the entire modelist.
>
> What about fb_new_modelist()? [1] It's called from store_modes() and
> the whole logic in the caller seems fragile to me. This could leave
> p->mode set when it should be cleared; and vice versa.
>
> [1]
> https://elixir.bootlin.com/linux/v6.16.8/source/drivers/video/fbdev/core/fbmem.c#L712
> [2]
> https://elixir.bootlin.com/linux/v6.16.8/source/drivers/video/fbdev/core/fbsysfs.c#L113
>
>
After testing, this path does not exhibit the issue because in
fbcon_new_modelist [1], the
modes in fb_display[] get refreshed and won't leave wild pointers. Even
if fb_new_modelist()
fails, the original modelist will be restored [2].
[1]
https://elixir.bootlin.com/linux/v6.16.8/source/drivers/video/fbdev/core/fbmem.c#L736
[2]
https://elixir.bootlin.com/linux/v6.16.8/source/drivers/video/fbdev/core/fbsysfs.c#L115
>>
>> Analysis shows that fb_delete_videomode path should have been fixed in
>> a previous patch[1]. Therefore, the current bug is likely triggered
>> through the fb_destroy_modelist path. I've found a reproducible test
>> case:
>> 1. With /dev/fb0 already registered in the system, load a kernel module
>> to register a new device /dev/fb1;
>> 2. Set fb1's mode to the global fb_display[] array (via
>> FBIOPUT_CON2FBMAP);
>> 3. Switch console from fb to VGA (to allow normal rmmod of the ko);
>> 4. Unload the kernel module - at this point fb1's modelist is freed,
>> leaving
>> a wild pointer in fb_display[];
>> 5. Trigger the bug via system calls through fb0 attempting to delete
>> a mode
>> from fb0.
>>
>> To prevent similar issues from recurring, consider traversing
>> fb_display[]
>> whenever releasing a mode from fb_info. If the corresponding mode exists
>> in fb_display[], set its pointer to NULL.
>>
>> [1]
>> https://lore.kernel.org/all/20210712085544.2828-1-thunder.leizhen@huawei.com/
>>
>> Signed-off-by: Quanmin Yan <yanquanmin1@huawei.com>
>> ---
>> This is my first time working on fb issues. If there are any
>> misunderstandings
>> in my analysis, I would appreciate corrections from the community.
>>
>> drivers/video/fbdev/core/fbcon.c | 11 +++++++++++
>> drivers/video/fbdev/core/modedb.c | 7 +++++++
>> include/linux/fbcon.h | 2 ++
>> 3 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/core/fbcon.c
>> b/drivers/video/fbdev/core/fbcon.c
>> index b062b05f4128..bfbf79d6cd05 100644
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>> @@ -2803,6 +2803,17 @@ int fbcon_mode_deleted(struct fb_info *info,
>> return found;
>> }
>> +void fb_display_clean_videomode(struct fb_videomode *m)
>
> Rather fbcon_delete_mode
>
>> +{
>> + struct fbcon_display *p;
>> +
>> + for (int i = first_fb_vc; i <= last_fb_vc; i++) {
>
> I think this code also needs to test the fb_info, or it might clear
> another display's mode. See [3] for an example
>
> [3]
> https://elixir.bootlin.com/linux/v6.16.8/source/drivers/video/fbdev/core/fbcon.c#L2786
>
>> + p = &fb_display[i];
>> + if (p->mode == m)
>> + p->mode = NULL;
>> + }
>> +}
>> +
>> #ifdef CONFIG_VT_HW_CONSOLE_BINDING
>> static void fbcon_unbind(void)
>> {
>> diff --git a/drivers/video/fbdev/core/modedb.c
>> b/drivers/video/fbdev/core/modedb.c
>> index 53a610948c4a..5a0ee96ebefa 100644
>> --- a/drivers/video/fbdev/core/modedb.c
>> +++ b/drivers/video/fbdev/core/modedb.c
>> @@ -16,6 +16,7 @@
>> #include <linux/slab.h>
>> #include <linux/fb.h>
>> #include <linux/kernel.h>
>> +#include <linux/fbcon.h>
>> #undef DEBUG
>> @@ -1100,6 +1101,7 @@ void fb_delete_videomode(const struct
>> fb_videomode *mode,
>> modelist = list_entry(pos, struct fb_modelist, list);
>> m = &modelist->mode;
>> if (fb_mode_is_equal(m, mode)) {
>> + fb_display_clean_videomode(m);
>
> There's only one caller of fb_delete_videomode(). I think this call
> should be right before fb_delete_videomode().
This path has already been fixed [3], so this modification is no longer
necessary and will be
removed in the next version of the patch.
[3]
https://lore.kernel.org/all/20210712085544.2828-1-thunder.leizhen@huawei.com/
Additionally, in the next version of the patch, I have incorporated your
other suggestions
and made corresponding adjustments. I will send the updated patch set
shortly.
Best regards,
Quanmin Yan
>
>> list_del(pos);
>> kfree(pos);
>> }
>> @@ -1113,8 +1115,13 @@ void fb_delete_videomode(const struct
>> fb_videomode *mode,
>> void fb_destroy_modelist(struct list_head *head)
>> {
>> struct list_head *pos, *n;
>> + struct fb_modelist *modelist;
>> + struct fb_videomode *m;
>> list_for_each_safe(pos, n, head) {
>> + modelist = list_entry(pos, struct fb_modelist, list);
>> + m = &modelist->mode;
>> + fb_display_clean_videomode(m);
>
> Same here: I think fb_destroy_modelist() should only release the mode.
> Clearing the fbcon mode should be done by the caller.
>
> Best regards
> Thomas
>
>> list_del(pos);
>> kfree(pos);
>> }
>> diff --git a/include/linux/fbcon.h b/include/linux/fbcon.h
>> index 81f0e698acbf..2b5e93aeaaff 100644
>> --- a/include/linux/fbcon.h
>> +++ b/include/linux/fbcon.h
>> @@ -18,6 +18,7 @@ void fbcon_suspended(struct fb_info *info);
>> void fbcon_resumed(struct fb_info *info);
>> int fbcon_mode_deleted(struct fb_info *info,
>> struct fb_videomode *mode);
>> +void fb_display_clean_videomode(struct fb_videomode *m);
>> void fbcon_new_modelist(struct fb_info *info);
>> void fbcon_get_requirement(struct fb_info *info,
>> struct fb_blit_caps *caps);
>> @@ -38,6 +39,7 @@ static inline void fbcon_suspended(struct fb_info
>> *info) {}
>> static inline void fbcon_resumed(struct fb_info *info) {}
>> static inline int fbcon_mode_deleted(struct fb_info *info,
>> struct fb_videomode *mode) { return 0; }
>> +static inline void fb_display_clean_videomode(struct fb_videomode
>> *m) {}
>> static inline void fbcon_new_modelist(struct fb_info *info) {}
>> static inline void fbcon_get_requirement(struct fb_info *info,
>> struct fb_blit_caps *caps) {}
>
^ permalink raw reply
* Re: [PATCH v4 1/4] dt-bindings: backlight: Add max25014 bindings
From: Conor Dooley @ 2025-10-09 18:21 UTC (permalink / raw)
To: Frank Li
Cc: maudspierings, Lee Jones, Daniel Thompson, Jingoo Han,
Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds,
devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel
In-Reply-To: <aOfM7jnlPO77YSu1@lizhi-Precision-Tower-5810>
[-- Attachment #1: Type: text/plain, Size: 6354 bytes --]
On Thu, Oct 09, 2025 at 10:55:42AM -0400, Frank Li wrote:
> On Thu, Oct 09, 2025 at 08:48:25AM +0200, Maud Spierings via B4 Relay wrote:
> > From: Maud Spierings <maudspierings@gocontroll.com>
>
> Subject needn't double bindings.
>
> dt-bindings: backlight: Add max25014 support
>
> >
> > The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
> > with integrated boost controller.
> >
> > In the current implementation the control registers for channel 1,
> > control all channels. So only one led subnode with led-sources is
> > supported right now. If at some point the driver functionality is
> > expanded the bindings can be easily extended with it.
>
> Need descript hardware, not driver. Need descript full functions even though
> driver have not implement yet.
>
> >
> > Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
> > ---
> > .../bindings/leds/backlight/maxim,max25014.yaml | 109 +++++++++++++++++++++
> > MAINTAINERS | 5 +
> > 2 files changed, 114 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
> > new file mode 100644
> > index 0000000000000..496520e1374e5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Maxim max25014 backlight controller
> > +
> > +maintainers:
> > + - Maud Spierings <maudspierings@gocontroll.com>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - maxim,max25014
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + enable-gpios:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + power-supply:
> > + description: Regulator which controls the boost converter input rail.
> > +
> > + pwms:
> > + maxItems: 1
> > +
> > + maxim,iset:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + maximum: 15
> > + default: 11
> > + description:
> > + Value of the ISET register field. This controls the current scale of the
> > + outputs, a higher number means more current.
>
> Can't use register value directly, need use standard unit. or percentage
>
> 100: means max, 0: min.
From that datasheet, it seems like the values here don't neatly map to
currents, because it depends on the value of the iref register. I don't
love percentages here either, too much of a force-fit for me.
If current is used, a property for the reference resistor will be
needed, to compute the register values. That only makes sense to me if
Maxim/Analog provide a formula that can be used to calculate the
appropriate register value, and I did not find one in the datasheet from
my quick skim, only two example current tables.
Sure, those two examples can be reverse-engineered to give a way to
compute it, but can we be sure that the numbers apply across the whole
range of permitted values for the resistor?
I don't like using register values for stuff, but I think it is the most
accurate and least likely to cause problems way of representing this.
Maud, on the language used - its the ISET field in the ISET register, I
think the property should make that clear.
> > +patternProperties:
> > + "^led@[01]$":
Why does this have [01] if reg has to be zero?
> > + type: object
> > + description: Properties for a string of connected LEDs.
> > + $ref: common.yaml#
> > +
> > + properties:
> > + reg:
> > + const: 0
> > +
> > + led-sources:
> > + allOf:
> > + - minItems: 1
> > + maxItems: 4
> > + items:
> > + minimum: 0
> > + maximum: 3
> > + default: [0, 1, 2, 3]
> > +
> > + default-brightness:
> > + minimum: 0
> > + maximum: 100
> > + default: 50
> > +
> > + required:
> > + - reg
> > +
> > + additionalProperties: false
>
> there are $ref, should use unevaluatedProperties: false
Not always, looks like they've listed some properties from the file,
which would make addtionalPropeties: false correct if they dont want
the other properties in the file to be used.
>
> Frank
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + backlight@6f {
> > + compatible = "maxim,max25014";
> > + reg = <0x6f>;
> > + enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> > + power-supply = <®_backlight>;
> > + pwms = <&pwm1>;
> > + maxim,iset = <7>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + led@0 {
> > + reg = <0>;
> > + led-sources = <0 1 2 3>;
> > + default-brightness = <50>;
> > + };
> > + };
> > + };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 47fbc5e06808f..be5e2515900ce 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -15171,6 +15171,11 @@ F: Documentation/userspace-api/media/drivers/max2175.rst
> > F: drivers/media/i2c/max2175*
> > F: include/uapi/linux/max2175.h
> >
> > +MAX25014 BACKLIGHT DRIVER
> > +M: Maud Spierings <maudspierings@gocontroll.com>
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
> > +
> > MAX31335 RTC DRIVER
> > M: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > L: linux-rtc@vger.kernel.org
> >
> > --
> > 2.51.0
> >
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/4] dt-bindings: backlight: Add max25014 bindings
From: Frank Li @ 2025-10-09 14:55 UTC (permalink / raw)
To: maudspierings
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree,
linux-kernel, linux-fbdev, imx, linux-arm-kernel
In-Reply-To: <20251009-max25014-v4-1-6adb2a0aa35f@gocontroll.com>
On Thu, Oct 09, 2025 at 08:48:25AM +0200, Maud Spierings via B4 Relay wrote:
> From: Maud Spierings <maudspierings@gocontroll.com>
Subject needn't double bindings.
dt-bindings: backlight: Add max25014 support
>
> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
> with integrated boost controller.
>
> In the current implementation the control registers for channel 1,
> control all channels. So only one led subnode with led-sources is
> supported right now. If at some point the driver functionality is
> expanded the bindings can be easily extended with it.
Need descript hardware, not driver. Need descript full functions even though
driver have not implement yet.
>
> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
> ---
> .../bindings/leds/backlight/maxim,max25014.yaml | 109 +++++++++++++++++++++
> MAINTAINERS | 5 +
> 2 files changed, 114 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
> new file mode 100644
> index 0000000000000..496520e1374e5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim max25014 backlight controller
> +
> +maintainers:
> + - Maud Spierings <maudspierings@gocontroll.com>
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max25014
> +
> + reg:
> + maxItems: 1
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + enable-gpios:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + power-supply:
> + description: Regulator which controls the boost converter input rail.
> +
> + pwms:
> + maxItems: 1
> +
> + maxim,iset:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + maximum: 15
> + default: 11
> + description:
> + Value of the ISET register field. This controls the current scale of the
> + outputs, a higher number means more current.
Can't use register value directly, need use standard unit. or percentage
100: means max, 0: min.
> +
> +patternProperties:
> + "^led@[01]$":
> + type: object
> + description: Properties for a string of connected LEDs.
> + $ref: common.yaml#
> +
> + properties:
> + reg:
> + const: 0
> +
> + led-sources:
> + allOf:
> + - minItems: 1
> + maxItems: 4
> + items:
> + minimum: 0
> + maximum: 3
> + default: [0, 1, 2, 3]
> +
> + default-brightness:
> + minimum: 0
> + maximum: 100
> + default: 50
> +
> + required:
> + - reg
> +
> + additionalProperties: false
there are $ref, should use unevaluatedProperties: false
Frank
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + backlight@6f {
> + compatible = "maxim,max25014";
> + reg = <0x6f>;
> + enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> + power-supply = <®_backlight>;
> + pwms = <&pwm1>;
> + maxim,iset = <7>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led@0 {
> + reg = <0>;
> + led-sources = <0 1 2 3>;
> + default-brightness = <50>;
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 47fbc5e06808f..be5e2515900ce 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15171,6 +15171,11 @@ F: Documentation/userspace-api/media/drivers/max2175.rst
> F: drivers/media/i2c/max2175*
> F: include/uapi/linux/max2175.h
>
> +MAX25014 BACKLIGHT DRIVER
> +M: Maud Spierings <maudspierings@gocontroll.com>
> +S: Maintained
> +F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
> +
> MAX31335 RTC DRIVER
> M: Antoniu Miclaus <antoniu.miclaus@analog.com>
> L: linux-rtc@vger.kernel.org
>
> --
> 2.51.0
>
>
^ permalink raw reply
* Re: [PATCH] fbdev: Fix logic error in "offb" name match
From: Thomas Zimmermann @ 2025-10-09 11:43 UTC (permalink / raw)
To: Finn Thain, Simona Vetter, Helge Deller, Javier Martinez Canillas
Cc: stable, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <f91c6079ef9764c7e23abd80ceab39a35f39417f.1759964186.git.fthain@linux-m68k.org>
Am 09.10.25 um 00:56 schrieb Finn Thain:
> A regression was reported to me recently whereby /dev/fb0 had disappeared
> from a PowerBook G3 Series "Wallstreet". The problem shows up when the
> "video=ofonly" parameter is passed to the kernel, which is what the
> bootloader does when "no video driver" is selected. The cause of the
> problem is the "offb" string comparison, which got mangled when it got
> refactored. Fix it.
>
> Cc: stable@vger.kernel.org
> Fixes: 93604a5ade3a ("fbdev: Handle video= parameter in video/cmdline.c")
> Reported-and-tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/video/fbdev/core/fb_cmdline.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
> index 4d1634c492ec..594b60424d1c 100644
> --- a/drivers/video/fbdev/core/fb_cmdline.c
> +++ b/drivers/video/fbdev/core/fb_cmdline.c
> @@ -40,7 +40,7 @@ int fb_get_options(const char *name, char **option)
> bool enabled;
>
> if (name)
> - is_of = strncmp(name, "offb", 4);
> + is_of = !strncmp(name, "offb", 4);
>
> enabled = __video_get_options(name, &options, is_of);
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* Re: [PATCH v3] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
From: Uwe Kleine-König @ 2025-10-09 8:50 UTC (permalink / raw)
To: Javier Garcia
Cc: deller, tzimmermann, linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <20251008183627.1279115-1-rampxxxx@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 779 bytes --]
Hello Javier,
On Wed, Oct 08, 2025 at 08:36:27PM +0200, Javier Garcia wrote:
> This patch wraps the relevant code blocks with `IS_ENABLED(CONFIG_FB_DEVICE)`.
>
> Allows the driver to be used for framebuffer text console, even if
> support for the /dev/fb device isn't compiled-in (CONFIG_FB_DEVICE=n).
>
> This align with Documentation/drm/todo.rst
> "Remove driver dependencies on FB_DEVICE"
>
> I've not the card so I was not able to test it.
I still don't understand why the creation of the dispregs sysfs file
should be conditional on FB_DEVICE. Either they have nothing to do with
each other, or I'm missing something. The former makes this patch wrong,
the latter would be an indication that the commit log is still
non-optimal.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v4 4/4] arm64: dts: freescale: moduline-display-av123z7m-n17: add backlight
From: Maud Spierings via B4 Relay @ 2025-10-09 6:48 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
In-Reply-To: <20251009-max25014-v4-0-6adb2a0aa35f@gocontroll.com>
From: Maud Spierings <maudspierings@gocontroll.com>
Add the missing backlight.
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
...x8p-ml81-moduline-display-106-av123z7m-n17.dtso | 27 +++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso
index 3eb665ce9d5d2..c320e0f563af9 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso
+++ b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso
@@ -16,6 +16,7 @@
panel {
compatible = "boe,av123z7m-n17";
+ backlight = <&backlight>;
enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
pinctrl-0 = <&pinctrl_panel>;
pinctrl-names = "default";
@@ -91,10 +92,34 @@ lvds1_out: endpoint {
};
};
- /* max25014 @ 0x6f */
+ backlight: backlight@6f {
+ compatible = "maxim,max25014";
+ reg = <0x6f>;
+ default-brightness = <50>;
+ enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_backlight>;
+ maxim,iset = <7>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+ led-sources = <0 1 2 3>;
+ default-brightness = <50>;
+ };
+ };
};
&iomuxc {
+ pinctrl_backlight: backlightgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_GPIO1_IO04__GPIO1_IO04
+ (MX8MP_PULL_UP | MX8MP_PULL_ENABLE)
+ >;
+ };
+
pinctrl_lvds_bridge: lvdsbridgegrp {
fsl,pins = <
MX8MP_IOMUXC_SAI1_TXD2__GPIO4_IO14
--
2.51.0
^ permalink raw reply related
* [PATCH v4 2/4] backlight: add max25014atg backlight
From: Maud Spierings via B4 Relay @ 2025-10-09 6:48 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
In-Reply-To: <20251009-max25014-v4-0-6adb2a0aa35f@gocontroll.com>
From: Maud Spierings <maudspierings@gocontroll.com>
The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
with integrated boost controller.
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
MAINTAINERS | 1 +
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/max25014.c | 409 +++++++++++++++++++++++++++++++++++++
4 files changed, 418 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index be5e2515900ce..77bb4fd3474e1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15175,6 +15175,7 @@ MAX25014 BACKLIGHT DRIVER
M: Maud Spierings <maudspierings@gocontroll.com>
S: Maintained
F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
+F: drivers/video/backlight/max25014.c
MAX31335 RTC DRIVER
M: Antoniu Miclaus <antoniu.miclaus@analog.com>
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index d9374d208ceeb..d3bb6ccd41853 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -262,6 +262,13 @@ config BACKLIGHT_DA9052
help
Enable the Backlight Driver for DA9052-BC and DA9053-AA/Bx PMICs.
+config BACKLIGHT_MAX25014
+ tristate "Backlight driver for the Maxim MAX25014 chip"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ If you are using a MAX25014 chip as a backlight driver say Y to enable it.
+
config BACKLIGHT_MAX8925
tristate "Backlight driver for MAX8925"
depends on MFD_MAX8925
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index dfbb169bf6ea2..1170d9ec40b8d 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o
obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o
obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o
+obj-$(CONFIG_BACKLIGHT_MAX25014) += max25014.o
obj-$(CONFIG_BACKLIGHT_MAX8925) += max8925_bl.o
obj-$(CONFIG_BACKLIGHT_MP3309C) += mp3309c.o
obj-$(CONFIG_BACKLIGHT_MT6370) += mt6370-backlight.o
diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c
new file mode 100644
index 0000000000000..36bae508697e8
--- /dev/null
+++ b/drivers/video/backlight/max25014.c
@@ -0,0 +1,409 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Backlight driver for Maxim MAX25014
+ *
+ * Copyright (C) 2025 GOcontroll B.V.
+ * Author: Maud Spierings <maudspierings@gocontroll.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define MAX25014_ISET_DEFAULT_100 11
+#define MAX_BRIGHTNESS 100
+#define MIN_BRIGHTNESS 0
+#define TON_MAX 130720 /* @153Hz */
+#define TON_STEP 1307 /* @153Hz */
+#define TON_MIN 0
+
+#define MAX25014_DEV_ID 0x00
+#define MAX25014_REV_ID 0x01
+#define MAX25014_ISET 0x02
+#define MAX25014_IMODE 0x03
+#define MAX25014_TON1H 0x04
+#define MAX25014_TON1L 0x05
+#define MAX25014_TON2H 0x06
+#define MAX25014_TON2L 0x07
+#define MAX25014_TON3H 0x08
+#define MAX25014_TON3L 0x09
+#define MAX25014_TON4H 0x0A
+#define MAX25014_TON4L 0x0B
+#define MAX25014_TON_1_4_LSB 0x0C
+#define MAX25014_SETTING 0x12
+#define MAX25014_DISABLE 0x13
+#define MAX25014_BSTMON 0x14
+#define MAX25014_IOUT1 0x15
+#define MAX25014_IOUT2 0x16
+#define MAX25014_IOUT3 0x17
+#define MAX25014_IOUT4 0x18
+#define MAX25014_OPEN 0x1B
+#define MAX25014_SHORT_GND 0x1C
+#define MAX25014_SHORT_LED 0x1D
+#define MAX25014_MASK 0x1E
+#define MAX25014_DIAG 0x1F
+
+#define MAX25014_IMODE_HDIM BIT(2)
+#define MAX25014_ISET_ENABLE BIT(5)
+#define MAX25014_ISET_PSEN BIT(4)
+#define MAX25014_DIAG_HW_RST BIT(2)
+#define MAX25014_SETTING_FPWM GENMASK(6, 4)
+
+struct max25014 {
+ struct i2c_client *client;
+ struct backlight_device *bl;
+ struct regmap *regmap;
+ struct gpio_desc *enable;
+ struct regulator *vin; /* regulator for boost converter Vin rail */
+ uint32_t iset;
+ uint8_t strings_mask;
+};
+
+static const struct regmap_config max25014_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX25014_DIAG,
+};
+
+/**
+ * @brief control the brightness with i2c registers
+ *
+ * @param regmap trivial
+ * @param brt brightness
+ * @return int
+ */
+static int max25014_register_control(struct regmap *regmap, uint32_t brt)
+{
+ uint32_t reg = TON_STEP * brt;
+ int ret;
+ /*
+ * 18 bit number lowest, 2 bits in first register,
+ * next lowest 8 in the L register, next 8 in the H register
+ * Seemingly setting the strength of only one string controls all of
+ * them, individual settings don't affect the outcome.
+ */
+
+ ret = regmap_write(regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011);
+ if (ret != 0)
+ return ret;
+ ret = regmap_write(regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111);
+ if (ret != 0)
+ return ret;
+ return regmap_write(regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111);
+}
+
+static int max25014_check_errors(struct max25014 *maxim)
+{
+ uint8_t i;
+ int ret;
+ uint32_t val;
+
+ ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val);
+ if (ret != 0)
+ return ret;
+ if (val > 0) {
+ dev_err(&maxim->client->dev, "Open led strings detected on:\n");
+ for (i = 0; i < 4; i++) {
+ if (val & 1 << i)
+ dev_err(&maxim->client->dev, "string %d\n", i + 1);
+ }
+ return -EIO;
+ }
+
+ ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val);
+ if (ret != 0)
+ return ret;
+ if (val > 0) {
+ dev_err(&maxim->client->dev, "Short to ground detected on:\n");
+ for (i = 0; i < 4; i++) {
+ if (val & 1 << i)
+ dev_err(&maxim->client->dev, "string %d\n", i + 1);
+ }
+ return -EIO;
+ }
+
+ ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val);
+ if (ret != 0)
+ return ret;
+ if (val > 0) {
+ dev_err(&maxim->client->dev, "Shorted led detected on:\n");
+ for (i = 0; i < 4; i++) {
+ if (val & 1 << i)
+ dev_err(&maxim->client->dev, "string %d\n", i + 1);
+ }
+ return -EIO;
+ }
+
+ ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val);
+ if (ret != 0)
+ return ret;
+ /*
+ * The HW_RST bit always starts at 1 after power up.
+ * It is reset on first read, does not indicate an error.
+ */
+ if (val > 0 && val != MAX25014_DIAG_HW_RST) {
+ if (val & 0b1)
+ dev_err(&maxim->client->dev,
+ "Overtemperature shutdown\n");
+ if (val & 0b10)
+ dev_err(&maxim->client->dev,
+ "Chip is getting too hot (>125C)\n");
+ if (val & 0b1000)
+ dev_err(&maxim->client->dev,
+ "Boost converter overvoltage\n");
+ if (val & 0b10000)
+ dev_err(&maxim->client->dev,
+ "Boost converter undervoltage\n");
+ if (val & 0b100000)
+ dev_err(&maxim->client->dev, "IREF out of range\n");
+ return -EIO;
+ }
+ return 0;
+}
+
+/*
+ * 1. disable unused strings
+ * 2. set dim mode
+ * 3. set initial brightness
+ * 4. set setting register
+ * 5. enable the backlight
+ */
+static int max25014_configure(struct max25014 *maxim)
+{
+ int ret;
+ uint32_t val;
+
+ ret = regmap_write(maxim->regmap, MAX25014_DISABLE,
+ maxim->strings_mask);
+ if (ret != 0)
+ return ret;
+
+ ret = regmap_write(maxim->regmap, MAX25014_IMODE, MAX25014_IMODE_HDIM);
+ if (ret != 0)
+ return ret;
+
+ ret = regmap_read(maxim->regmap, MAX25014_SETTING, &val);
+ if (ret != 0)
+ return ret;
+
+ ret = regmap_write(maxim->regmap, MAX25014_SETTING,
+ val & ~MAX25014_SETTING_FPWM);
+ if (ret != 0)
+ return ret;
+
+ ret = regmap_write(maxim->regmap, MAX25014_ISET,
+ maxim->iset | MAX25014_ISET_ENABLE |
+ MAX25014_ISET_PSEN);
+ return ret;
+}
+
+static int max25014_update_status(struct backlight_device *bl_dev)
+{
+ struct max25014 *maxim = bl_get_data(bl_dev);
+
+ if (backlight_is_blank(maxim->bl))
+ bl_dev->props.brightness = 0;
+
+ return max25014_register_control(maxim->regmap,
+ bl_dev->props.brightness);
+}
+
+static const struct backlight_ops max25014_bl_ops = {
+ .options = BL_CORE_SUSPENDRESUME,
+ .update_status = max25014_update_status,
+};
+
+static int max25014_parse_dt(struct max25014 *maxim,
+ uint32_t *initial_brightness)
+{
+ struct device *dev = &maxim->client->dev;
+ struct device_node *node = dev->of_node;
+ struct fwnode_handle *child;
+ uint32_t strings[4];
+ int res, i;
+
+ if (!node) {
+ dev_err(dev, "no platform data\n");
+ return -EINVAL;
+ }
+
+ child = device_get_next_child_node(dev, NULL);
+ if (child) {
+ res = fwnode_property_count_u32(child, "led-sources");
+ if (res > 0) {
+ fwnode_property_read_u32_array(child, "led-sources",
+ strings, res);
+
+ /* set all strings as disabled, then enable those in led-sources*/
+ maxim->strings_mask = 0xf;
+ for (i = 0; i < res; i++) {
+ if (strings[i] <= 4)
+ maxim->strings_mask &= ~BIT(strings[i]);
+ }
+ }
+
+ fwnode_property_read_u32(child, "default-brightness",
+ initial_brightness);
+
+ fwnode_handle_put(child);
+ }
+
+ maxim->iset = MAX25014_ISET_DEFAULT_100;
+ of_property_read_u32(node, "maxim,iset", &maxim->iset);
+
+ if (maxim->iset < 0 || maxim->iset > 15) {
+ dev_err(dev,
+ "Invalid iset, should be a value from 0-15, entered was %d\n",
+ maxim->iset);
+ return -EINVAL;
+ }
+
+ if (*initial_brightness < 0 || *initial_brightness > 100) {
+ dev_err(dev,
+ "Invalid initial brightness, should be a value from 0-100, entered was %d\n",
+ *initial_brightness);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int max25014_probe(struct i2c_client *cl)
+{
+ struct backlight_device *bl;
+ const struct i2c_device_id *id = i2c_client_get_device_id(cl);
+ struct max25014 *maxim;
+ struct backlight_properties props;
+ int ret;
+ uint32_t initial_brightness = 50;
+
+ maxim = devm_kzalloc(&cl->dev, sizeof(struct max25014), GFP_KERNEL);
+ if (!maxim)
+ return -ENOMEM;
+
+ maxim->client = cl;
+
+ ret = max25014_parse_dt(maxim, &initial_brightness);
+ if (ret < 0)
+ return ret;
+
+ maxim->vin = devm_regulator_get_optional(&maxim->client->dev, "power");
+ if (IS_ERR(maxim->vin)) {
+ if (PTR_ERR(maxim->vin) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ maxim->vin = NULL;
+ }
+
+ if (maxim->vin) {
+ ret = regulator_enable(maxim->vin);
+ if (ret < 0) {
+ dev_err(&maxim->client->dev,
+ "failed to enable Vin: %d\n", ret);
+ return ret;
+ }
+ }
+
+ maxim->enable = devm_gpiod_get_optional(&maxim->client->dev, "enable",
+ GPIOD_ASIS);
+ if (IS_ERR(maxim->enable)) {
+ ret = PTR_ERR(maxim->enable);
+ dev_err(&maxim->client->dev, "failed to get enable gpio: %d\n",
+ ret);
+ goto disable_vin;
+ }
+
+ if (maxim->enable)
+ gpiod_set_value_cansleep(maxim->enable, 1);
+
+ /* Enable can be tied to vin rail wait if either is available */
+ if (maxim->enable || maxim->vin) {
+ /* Datasheet Electrical Characteristics tSTARTUP 2ms */
+ usleep_range(2000, 2500);
+ }
+
+ maxim->regmap = devm_regmap_init_i2c(cl, &max25014_regmap_config);
+ if (IS_ERR(maxim->regmap)) {
+ ret = PTR_ERR(maxim->regmap);
+ dev_err(&maxim->client->dev,
+ "failed to initialize the i2c regmap: %d\n", ret);
+ goto disable_full;
+ }
+
+ i2c_set_clientdata(cl, maxim);
+
+ ret = max25014_check_errors(maxim);
+ if (ret) { /* error is already reported in the above function */
+ goto disable_full;
+ }
+
+ ret = max25014_configure(maxim);
+ if (ret) {
+ dev_err(&maxim->client->dev, "device config err: %d", ret);
+ goto disable_full;
+ }
+
+ memset(&props, 0, sizeof(props));
+ props.type = BACKLIGHT_PLATFORM;
+ props.max_brightness = MAX_BRIGHTNESS;
+ props.brightness = initial_brightness;
+ props.scale = BACKLIGHT_SCALE_LINEAR;
+
+ bl = devm_backlight_device_register(&maxim->client->dev, id->name,
+ &maxim->client->dev, maxim,
+ &max25014_bl_ops, &props);
+ if (IS_ERR(bl))
+ return PTR_ERR(bl);
+
+ maxim->bl = bl;
+
+ return 0;
+
+disable_full:
+ if (maxim->enable)
+ gpiod_set_value_cansleep(maxim->enable, 0);
+disable_vin:
+ if (maxim->vin)
+ regulator_disable(maxim->vin);
+ return ret;
+}
+
+static void max25014_remove(struct i2c_client *cl)
+{
+ struct max25014 *maxim = i2c_get_clientdata(cl);
+
+ maxim->bl->props.brightness = 0;
+ max25014_update_status(maxim->bl);
+ if (maxim->enable)
+ gpiod_set_value_cansleep(maxim->enable, 0);
+ if (maxim->vin)
+ regulator_disable(maxim->vin);
+}
+
+static const struct of_device_id max25014_dt_ids[] = {
+ { .compatible = "maxim,max25014", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max25014_dt_ids);
+
+static const struct i2c_device_id max25014_ids[] = {
+ { "max25014" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max25014_ids);
+
+static struct i2c_driver max25014_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = of_match_ptr(max25014_dt_ids),
+ },
+ .probe = max25014_probe,
+ .remove = max25014_remove,
+ .id_table = max25014_ids,
+};
+module_i2c_driver(max25014_driver);
+
+MODULE_DESCRIPTION("Maxim MAX25014 backlight driver");
+MODULE_AUTHOR("Maud Spierings <maudspierings@gocontroll.com>");
+MODULE_LICENSE("GPL");
--
2.51.0
^ permalink raw reply related
* [PATCH v4 3/4] arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight
From: Maud Spierings via B4 Relay @ 2025-10-09 6:48 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
In-Reply-To: <20251009-max25014-v4-0-6adb2a0aa35f@gocontroll.com>
From: Maud Spierings <maudspierings@gocontroll.com>
Add the missing backlight driver.
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
...x8p-ml81-moduline-display-106-av101hdt-a10.dtso | 32 ++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso
index e3965caca6be4..e8107145a7f3b 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso
+++ b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso
@@ -17,6 +17,7 @@
panel {
compatible = "boe,av101hdt-a10";
+ backlight = <&backlight>;
enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
pinctrl-0 = <&pinctrl_panel>;
pinctrl-names = "default";
@@ -40,7 +41,38 @@ reg_vbus: regulator-vbus {
};
};
+&i2c4 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ backlight: backlight@6f {
+ compatible = "maxim,max25014";
+ reg = <0x6f>;
+ default-brightness = <50>;
+ enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_backlight>;
+ maxim,iset = <7>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+ led-sources = <0 1 2>;
+ default-brightness = <50>;
+ };
+ };
+};
+
&iomuxc {
+ pinctrl_backlight: backlightgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_GPIO1_IO04__GPIO1_IO04
+ (MX8MP_PULL_UP | MX8MP_PULL_ENABLE)
+ >;
+ };
+
pinctrl_panel: panelgrp {
fsl,pins = <
MX8MP_IOMUXC_GPIO1_IO07__GPIO1_IO07
--
2.51.0
^ permalink raw reply related
* [PATCH v4 0/4] backlight: add new max25014 backlight driver
From: Maud Spierings via B4 Relay @ 2025-10-09 6:48 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
The Maxim MAX25014 is an automotive grade backlight driver IC. Its
datasheet can be found at [1].
With its integrated boost controller, it can power 4 channels (led
strings) and has a number of different modes using pwm and or i2c.
Currently implemented is only i2c control.
link: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX25014.pdf [1]
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
Changes in v4:
- use a led node to describe the backlight
- use led-sources to enable specific channels
- also wait 2ms when there is a supply but no enable
- change dev_warn() to dev_err() in error path in max25014_check_errors()
- set backlight_properties.scale to BACKLIGHT_SCALE_LINEAR
- rebase latest next
- add address-cells and size-cells to i2c4 in av101hdt-a10.dtso
- Link to v3: https://lore.kernel.org/r/20250911-max25014-v3-0-d03f4eba375e@gocontroll.com
Changes in v3:
- fixed commit message type intgrated -> integrated
- added maximum and description to maxim,iset-property
- dropped unused labels and pinctrl in bindings example
- put the compatible first in the bindings example and dts
- removed brackets around defines
- removed the leftover pdata struct field
- removed the initial_brightness struct field
- Link to v2: https://lore.kernel.org/r/20250819-max25014-v2-0-5fd7aeb141ea@gocontroll.com
Changes in v2:
- Remove leftover unused property from the bindings example
- Complete the bindings example with all properties
- Remove some double info from the maxim,iset property
- Remove platform_data header, fold its data into the max25014 struct
- Don't force defines to be unsigned
- Remove stray struct max25014 declaration
- Remove chipname and device from the max25014 struct
- Inline the max25014_backlight_register() and strings_mask() functions
- Remove CONFIG_OF ifdef
- Link to v1: https://lore.kernel.org/r/20250725-max25014-v1-0-0e8cce92078e@gocontroll.com
---
Maud Spierings (4):
dt-bindings: backlight: Add max25014 bindings
backlight: add max25014atg backlight
arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight
arm64: dts: freescale: moduline-display-av123z7m-n17: add backlight
.../bindings/leds/backlight/maxim,max25014.yaml | 109 ++++++
MAINTAINERS | 6 +
...x8p-ml81-moduline-display-106-av101hdt-a10.dtso | 32 ++
...x8p-ml81-moduline-display-106-av123z7m-n17.dtso | 27 +-
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/max25014.c | 409 +++++++++++++++++++++
7 files changed, 590 insertions(+), 1 deletion(-)
---
base-commit: 7c3ba4249a3604477ea9c077e10089ba7ddcaa03
change-id: 20250626-max25014-4207591e1af5
Best regards,
--
Maud Spierings <maudspierings@gocontroll.com>
^ permalink raw reply
* [PATCH v4 1/4] dt-bindings: backlight: Add max25014 bindings
From: Maud Spierings via B4 Relay @ 2025-10-09 6:48 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
In-Reply-To: <20251009-max25014-v4-0-6adb2a0aa35f@gocontroll.com>
From: Maud Spierings <maudspierings@gocontroll.com>
The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
with integrated boost controller.
In the current implementation the control registers for channel 1,
control all channels. So only one led subnode with led-sources is
supported right now. If at some point the driver functionality is
expanded the bindings can be easily extended with it.
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
.../bindings/leds/backlight/maxim,max25014.yaml | 109 +++++++++++++++++++++
MAINTAINERS | 5 +
2 files changed, 114 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
new file mode 100644
index 0000000000000..496520e1374e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim max25014 backlight controller
+
+maintainers:
+ - Maud Spierings <maudspierings@gocontroll.com>
+
+properties:
+ compatible:
+ enum:
+ - maxim,max25014
+
+ reg:
+ maxItems: 1
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ enable-gpios:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ power-supply:
+ description: Regulator which controls the boost converter input rail.
+
+ pwms:
+ maxItems: 1
+
+ maxim,iset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 15
+ default: 11
+ description:
+ Value of the ISET register field. This controls the current scale of the
+ outputs, a higher number means more current.
+
+patternProperties:
+ "^led@[01]$":
+ type: object
+ description: Properties for a string of connected LEDs.
+ $ref: common.yaml#
+
+ properties:
+ reg:
+ const: 0
+
+ led-sources:
+ allOf:
+ - minItems: 1
+ maxItems: 4
+ items:
+ minimum: 0
+ maximum: 3
+ default: [0, 1, 2, 3]
+
+ default-brightness:
+ minimum: 0
+ maximum: 100
+ default: 50
+
+ required:
+ - reg
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ backlight@6f {
+ compatible = "maxim,max25014";
+ reg = <0x6f>;
+ enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+ power-supply = <®_backlight>;
+ pwms = <&pwm1>;
+ maxim,iset = <7>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+ led-sources = <0 1 2 3>;
+ default-brightness = <50>;
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 47fbc5e06808f..be5e2515900ce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15171,6 +15171,11 @@ F: Documentation/userspace-api/media/drivers/max2175.rst
F: drivers/media/i2c/max2175*
F: include/uapi/linux/max2175.h
+MAX25014 BACKLIGHT DRIVER
+M: Maud Spierings <maudspierings@gocontroll.com>
+S: Maintained
+F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
+
MAX31335 RTC DRIVER
M: Antoniu Miclaus <antoniu.miclaus@analog.com>
L: linux-rtc@vger.kernel.org
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] fbdev: Fix logic error in "offb" name match
From: Helge Deller @ 2025-10-08 23:02 UTC (permalink / raw)
To: Finn Thain, Simona Vetter, Thomas Zimmermann,
Javier Martinez Canillas
Cc: stable, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <f91c6079ef9764c7e23abd80ceab39a35f39417f.1759964186.git.fthain@linux-m68k.org>
On 10/9/25 00:56, Finn Thain wrote:
> A regression was reported to me recently whereby /dev/fb0 had disappeared
> from a PowerBook G3 Series "Wallstreet". The problem shows up when the
> "video=ofonly" parameter is passed to the kernel, which is what the
> bootloader does when "no video driver" is selected. The cause of the
> problem is the "offb" string comparison, which got mangled when it got
> refactored. Fix it.
>
> Cc: stable@vger.kernel.org
> Fixes: 93604a5ade3a ("fbdev: Handle video= parameter in video/cmdline.c")
> Reported-and-tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
> ---
> drivers/video/fbdev/core/fb_cmdline.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v3] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
From: Helge Deller @ 2025-10-08 23:00 UTC (permalink / raw)
To: Javier Garcia, u.kleine-koenig, tzimmermann
Cc: linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <20251008183627.1279115-1-rampxxxx@gmail.com>
On 10/8/25 20:36, Javier Garcia wrote:
> This patch wraps the relevant code blocks with `IS_ENABLED(CONFIG_FB_DEVICE)`.
>
> Allows the driver to be used for framebuffer text console, even if
> support for the /dev/fb device isn't compiled-in (CONFIG_FB_DEVICE=n).
>
> This align with Documentation/drm/todo.rst
> "Remove driver dependencies on FB_DEVICE"
>
> I've not the card so I was not able to test it.
>
> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
applied (with minor changes to the commit message).
Thanks!
Helge
^ permalink raw reply
* [PATCH] fbdev: Fix logic error in "offb" name match
From: Finn Thain @ 2025-10-08 22:56 UTC (permalink / raw)
To: Simona Vetter, Helge Deller, Thomas Zimmermann,
Javier Martinez Canillas
Cc: stable, linux-fbdev, dri-devel, linux-kernel
A regression was reported to me recently whereby /dev/fb0 had disappeared
from a PowerBook G3 Series "Wallstreet". The problem shows up when the
"video=ofonly" parameter is passed to the kernel, which is what the
bootloader does when "no video driver" is selected. The cause of the
problem is the "offb" string comparison, which got mangled when it got
refactored. Fix it.
Cc: stable@vger.kernel.org
Fixes: 93604a5ade3a ("fbdev: Handle video= parameter in video/cmdline.c")
Reported-and-tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/video/fbdev/core/fb_cmdline.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
index 4d1634c492ec..594b60424d1c 100644
--- a/drivers/video/fbdev/core/fb_cmdline.c
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -40,7 +40,7 @@ int fb_get_options(const char *name, char **option)
bool enabled;
if (name)
- is_of = strncmp(name, "offb", 4);
+ is_of = !strncmp(name, "offb", 4);
enabled = __video_get_options(name, &options, is_of);
--
2.49.1
^ permalink raw reply related
* [PATCH v3] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
From: Javier Garcia @ 2025-10-08 18:36 UTC (permalink / raw)
To: deller, u.kleine-koenig, tzimmermann
Cc: linux-fbdev, dri-devel, linux-kernel, shuah, Javier Garcia
In-Reply-To: <20251006164143.1187434-1-rampxxxx@gmail.com>
This patch wraps the relevant code blocks with `IS_ENABLED(CONFIG_FB_DEVICE)`.
Allows the driver to be used for framebuffer text console, even if
support for the /dev/fb device isn't compiled-in (CONFIG_FB_DEVICE=n).
This align with Documentation/drm/todo.rst
"Remove driver dependencies on FB_DEVICE"
I've not the card so I was not able to test it.
Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
---
v2 -> v3:
* Change commit msg , thanks Helge Deller.
* Delete not used include , thanks Uwe Kleine-Koenig.
* v1 https://lore.kernel.org/lkml/20251006164143.1187434-1-rampxxxx@gmail.com/
v1 -> v2:
* Fix error and improvement , thanks Uwe Kleine-Koenig.
* v1 https://lore.kernel.org/lkml/20251005173812.1169436-1-rampxxxx@gmail.com
drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index ade88e7bc760..3f79dfc27a53 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -759,7 +759,7 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
dev_set_drvdata(dev, info);
- if (device_create_file(dev, &dev_attr_dispregs))
+ if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
dev_err(dev, "Can't create sysfs regdump file\n");
return 0;
@@ -801,7 +801,8 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
free_irq(par->irq, (void *)par);
irq_dispose_mapping(par->irq);
- device_remove_file(&ofdev->dev, &dev_attr_dispregs);
+ if (IS_ENABLED(CONFIG_FB_DEVICE))
+ device_remove_file(&ofdev->dev, &dev_attr_dispregs);
unregister_framebuffer(fbi);
fb_dealloc_cmap(&fbi->cmap);
@@ -1101,7 +1102,7 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
pci_set_drvdata(pdev, info);
- if (device_create_file(dev, &dev_attr_dispregs))
+ if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
dev_err(dev, "Can't create sysfs regdump file\n");
if (par->type == BT_CARMINE)
@@ -1151,7 +1152,8 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
mb862xx_i2c_exit(par);
- device_remove_file(&pdev->dev, &dev_attr_dispregs);
+ if (IS_ENABLED(CONFIG_FB_DEVICE))
+ device_remove_file(&pdev->dev, &dev_attr_dispregs);
unregister_framebuffer(fbi);
fb_dealloc_cmap(&fbi->cmap);
--
2.50.1
^ permalink raw reply related
* Re: [PATCH] fbdev: udlfb: make CONFIG_FB_DEVICE optional
From: Thomas Zimmermann @ 2025-10-07 7:35 UTC (permalink / raw)
To: Helge Deller, sukrut heroorkar, Mikulas Patocka
Cc: David Hunter, kernel test robot, Bernie Thompson, Arnd Bergmann,
Randy Dunlap, Bartosz Golaszewski, Zsolt Kajtar,
Gonzalo Silvalde Blanco, linux-fbdev, dri-devel, linux-kernel,
llvm, oe-kbuild-all, skhan
In-Reply-To: <c1d86274-44e2-4ceb-b887-5c4af45d8b37@gmx.de>
Hi
Am 03.10.25 um 21:50 schrieb Helge Deller:
> On 10/3/25 20:43, sukrut heroorkar wrote:
>> On Thu, Oct 2, 2025 at 8:52 AM Thomas Zimmermann
>> <tzimmermann@suse.de> wrote:
>>> Am 02.10.25 um 08:41 schrieb Helge Deller:
>>>>>>> kernel test robot noticed the following build errors:
>>>>>>
>>>>>> Did you compile and test this code before submitting this patch?
>>>>>
>>>>> Yes, I had compiled & loaded the udlfb module with no errors. Please
>>>>> let me know how to proceed in this case.
>>>>
>>>> Look at the reported build error, which seems to happen in dev_dbg().
>>>> So, maybe in your testing you did not have debugging enabled?
>>>> The report contains the .config file with which you can test.
>>>
>>> Can we rather make an effort to remove the udlfb driver entirely? A few
>>> years back, there was one user who was still using it because of some
>>> problems with the DRM udl driver. But I think we've addressed them. The
>>> discussion is at [1].
>
> Would be good to know if they issues/crashes really have been solved.
> In [1] it seems the crashes still happened with DRM.
The thread at [1] was the original removal attempt. And that was 5 years
ago. I think we could retry and take the reporter (Mikulas) into cc.
Best regards
Thomas
>
>> Should I send a patch series to completely remove udlfb,
>
> No. (at least not yet)
>
>> since [1] echoed that DRM udl driver is good enough?
>>> [1]
>>> https://lore.kernel.org/dri-devel/20201130125200.10416-1-tzimmermann@suse.de/
>
> Well, some people who do *NOT* actively use fbdev with the old
> cards say the DRM replacements are "good enough".
> For tThose people who really depend on fbdev and the speed it has
> over DRM, the DRM "basic-drivers" are simply a
> nice-to-have-but-not-really-useable
> type of drivers.
> So, unless the really affected people say the DRM replacement
> is fully usable, we need to keep the fbdev driver.
>
> Helge
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* Re: [PATCH v2] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
From: Uwe Kleine-König @ 2025-10-07 6:57 UTC (permalink / raw)
To: Javier Garcia
Cc: deller, tzimmermann, linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <20251006164143.1187434-1-rampxxxx@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1634 bytes --]
On Mon, Oct 06, 2025 at 06:41:43PM +0200, Javier Garcia wrote:
> This patch wraps the relevant code blocks with `IS_ENABLED(ifdef CONFIG_FB_DEVICE)`,
stray "ifdef "
> allowing the driver to be built and used even if CONFIG_FB_DEVICE is not selected.
The driver built fine without FB_DEVICE already before, doesn't it?
> The sysfs only give access to show some controller and cursor registers so
> it's not needed to allow driver works correctly.
>
> This align with Documentation/drm/todo.rst
> "Remove driver dependencies on FB_DEVICE"
Given the above, I still don't understand that. (But maybe this can be
fixed by Helge's request to improve the commit message.)
> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
> ---
> v1 -> v2:
> * Fix error and improvement , thanks Uwe Kleine-Koenig.
> * v1 https://lore.kernel.org/lkml/20251005173812.1169436-1-rampxxxx@gmail.com
>
>
> drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> index ade88e7bc760..dc99b8c9ff0f 100644
> --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> +#include "linux/kconfig.h"
I didn't need that during my build tests, also don't use "" here, but
<>.
> #include <linux/pci.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> [...]
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
From: Helge Deller @ 2025-10-06 23:52 UTC (permalink / raw)
To: Javier Garcia, u.kleine-koenig, tzimmermann
Cc: linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <20251006164143.1187434-1-rampxxxx@gmail.com>
Hi Javier,
thanks for the updated patch!
Some notes:
On 10/6/25 18:41, Javier Garcia wrote:
> This patch wraps the relevant code blocks with `IS_ENABLED(ifdef CONFIG_FB_DEVICE)`,
> allowing the driver to be built and used even if CONFIG_FB_DEVICE is not selected.
>
> The sysfs only give access to show some controller and cursor registers so
> it's not needed to allow driver works correctly.
>
> This align with Documentation/drm/todo.rst
> "Remove driver dependencies on FB_DEVICE"
Can you make this commit message more crisp?
E.g. what is the benefit? Something new possible or fixed?
Also, the driver can be built without CONFIG_FB_DEVICE, but it will
then probably not work.
Do you have that card and tested it?
Maybe something like:
Allow the driver to be used for framebuffer text console, even if
support for the /dev/fb device isn't compiled-in (CONFIG_FB_DEVICE=n).
> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
> ---
> v1 -> v2:
> * Fix error and improvement , thanks Uwe Kleine-Koenig.
> * v1 https://lore.kernel.org/lkml/20251005173812.1169436-1-rampxxxx@gmail.com
>
>
> drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> index ade88e7bc760..dc99b8c9ff0f 100644
> --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> +#include "linux/kconfig.h"
> #include <linux/pci.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> @@ -759,7 +760,7 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
>
> dev_set_drvdata(dev, info);
>
> - if (device_create_file(dev, &dev_attr_dispregs))
> + if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
> dev_err(dev, "Can't create sysfs regdump file\n");
> return 0;
>
> @@ -801,7 +802,8 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
> free_irq(par->irq, (void *)par);
> irq_dispose_mapping(par->irq);
>
> - device_remove_file(&ofdev->dev, &dev_attr_dispregs);
> + if(IS_ENABLED(CONFIG_FB_DEVICE))
> + device_remove_file(&ofdev->dev, &dev_attr_dispregs);
>
> unregister_framebuffer(fbi);
> fb_dealloc_cmap(&fbi->cmap);
> @@ -1101,7 +1103,7 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
>
> pci_set_drvdata(pdev, info);
>
> - if (device_create_file(dev, &dev_attr_dispregs))
> + if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
> dev_err(dev, "Can't create sysfs regdump file\n");
>
> if (par->type == BT_CARMINE)
> @@ -1151,7 +1153,8 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
>
> mb862xx_i2c_exit(par);
>
> - device_remove_file(&pdev->dev, &dev_attr_dispregs);
> + if(IS_ENABLED(CONFIG_FB_DEVICE))
a space is missign after "if".
Helge
^ permalink raw reply
* [PATCH v2] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
From: Javier Garcia @ 2025-10-06 16:41 UTC (permalink / raw)
To: deller, u.kleine-koenig, tzimmermann
Cc: linux-fbdev, dri-devel, linux-kernel, shuah, Javier Garcia
This patch wraps the relevant code blocks with `IS_ENABLED(ifdef CONFIG_FB_DEVICE)`,
allowing the driver to be built and used even if CONFIG_FB_DEVICE is not selected.
The sysfs only give access to show some controller and cursor registers so
it's not needed to allow driver works correctly.
This align with Documentation/drm/todo.rst
"Remove driver dependencies on FB_DEVICE"
Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
---
v1 -> v2:
* Fix error and improvement , thanks Uwe Kleine-Koenig.
* v1 https://lore.kernel.org/lkml/20251005173812.1169436-1-rampxxxx@gmail.com
drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index ade88e7bc760..dc99b8c9ff0f 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -17,6 +17,7 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/interrupt.h>
+#include "linux/kconfig.h"
#include <linux/pci.h>
#include <linux/of.h>
#include <linux/of_address.h>
@@ -759,7 +760,7 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
dev_set_drvdata(dev, info);
- if (device_create_file(dev, &dev_attr_dispregs))
+ if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
dev_err(dev, "Can't create sysfs regdump file\n");
return 0;
@@ -801,7 +802,8 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
free_irq(par->irq, (void *)par);
irq_dispose_mapping(par->irq);
- device_remove_file(&ofdev->dev, &dev_attr_dispregs);
+ if(IS_ENABLED(CONFIG_FB_DEVICE))
+ device_remove_file(&ofdev->dev, &dev_attr_dispregs);
unregister_framebuffer(fbi);
fb_dealloc_cmap(&fbi->cmap);
@@ -1101,7 +1103,7 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
pci_set_drvdata(pdev, info);
- if (device_create_file(dev, &dev_attr_dispregs))
+ if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
dev_err(dev, "Can't create sysfs regdump file\n");
if (par->type == BT_CARMINE)
@@ -1151,7 +1153,8 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
mb862xx_i2c_exit(par);
- device_remove_file(&pdev->dev, &dev_attr_dispregs);
+ if(IS_ENABLED(CONFIG_FB_DEVICE))
+ device_remove_file(&pdev->dev, &dev_attr_dispregs);
unregister_framebuffer(fbi);
fb_dealloc_cmap(&fbi->cmap);
--
2.50.1
^ permalink raw reply related
* Re: [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
From: Javier Garcia @ 2025-10-06 16:10 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: deller, tzimmermann, linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <ewca4jzmahwdl47rbojxtynbizu2vuompjxrprsz7aelovnvao@kzpxjjbjj6px>
Hello.
On Mon, 6 Oct 2025 at 10:07, Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello,
>
> On Sun, Oct 05, 2025 at 07:38:12PM +0200, Javier Garcia wrote:
> > This patch wraps the relevant code blocks with #ifdef CONFIG_FB_DEVICE,
> > allowing the driver to be built and used even if CONFIG_FB_DEVICE is not selected.
> >
> > The sysfs only give access to show some controller and cursor registers so
> > it's not needed to allow driver works correctly.
> >
> > Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
>
> I don't understand this patch. CONFIG_FB_DEVICE is about legacy /dev/fb*
> stuff, and this patch make the creation of a sysfs file dependent on
> that.
This is part of the TODO
https://www.kernel.org/doc/html/latest/gpu/todo.html#remove-driver-dependencies-on-fb-device
I think the idea is to make the driver independent of CONFIG_FB_DEVICE .
>
> > diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> > index ade88e7bc760..a691a5fefb25 100644
> > --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> > +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> > @@ -530,6 +530,7 @@ static int mb862xxfb_init_fbinfo(struct fb_info *fbi)
> > return 0;
> > }
> >
> > +#ifdef CONFIG_FB_DEVICE
> > /*
> > * show some display controller and cursor registers
> > */
> > @@ -569,6 +570,7 @@ static ssize_t dispregs_show(struct device *dev,
> > }
> >
> > static DEVICE_ATTR_RO(dispregs);
> > +#endif
> >
> > static irqreturn_t mb862xx_intr(int irq, void *dev_id)
> > {
> > @@ -759,9 +761,11 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
> >
> > dev_set_drvdata(dev, info);
> >
> > +#ifdef CONFIG_FB_DEVICE
> > if (device_create_file(dev, &dev_attr_dispregs))
> > dev_err(dev, "Can't create sysfs regdump file\n");
> > return 0;
> > +#endif
>
> That looks wrong. Without CONFIG_FB_DEVICE set the success return is
> removed and all resources are freed essentially making the
> CONFIG_FB_MB862XX_LIME part of the driver unusable.
Thanks, I'll fix that.
And add your suggestion instead of the `ifdef`.
>
> > rel_cmap:
> > fb_dealloc_cmap(&info->cmap);
> > @@ -801,7 +805,9 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
> > free_irq(par->irq, (void *)par);
> > irq_dispose_mapping(par->irq);
> >
> > +#ifdef CONFIG_FB_DEVICE
> > device_remove_file(&ofdev->dev, &dev_attr_dispregs);
> > +#endif
> >
> > unregister_framebuffer(fbi);
> > fb_dealloc_cmap(&fbi->cmap);
> > @@ -1101,8 +1107,10 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
> >
> > pci_set_drvdata(pdev, info);
> >
> > +#ifdef CONFIG_FB_DEVICE
> > if (device_create_file(dev, &dev_attr_dispregs))
> > dev_err(dev, "Can't create sysfs regdump file\n");
> > +#endif
> >
> > if (par->type == BT_CARMINE)
> > outreg(ctrl, GC_CTRL_INT_MASK, GC_CARMINE_INT_EN);
> > @@ -1151,7 +1159,9 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
> >
> > mb862xx_i2c_exit(par);
> >
> > +#ifdef CONFIG_FB_DEVICE
> > device_remove_file(&pdev->dev, &dev_attr_dispregs);
> > +#endif
> >
> > unregister_framebuffer(fbi);
> > fb_dealloc_cmap(&fbi->cmap);
>
> The amount of ifdefs could be greatly reduced using the following patch:
>
> diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> index a691a5fefb25..3f79dfc27a53 100644
> --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> @@ -530,7 +530,6 @@ static int mb862xxfb_init_fbinfo(struct fb_info *fbi)
> return 0;
> }
>
> -#ifdef CONFIG_FB_DEVICE
> /*
> * show some display controller and cursor registers
> */
> @@ -570,7 +569,6 @@ static ssize_t dispregs_show(struct device *dev,
> }
>
> static DEVICE_ATTR_RO(dispregs);
> -#endif
>
> static irqreturn_t mb862xx_intr(int irq, void *dev_id)
> {
> @@ -761,11 +759,9 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
>
> dev_set_drvdata(dev, info);
>
> -#ifdef CONFIG_FB_DEVICE
> - if (device_create_file(dev, &dev_attr_dispregs))
> + if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
> dev_err(dev, "Can't create sysfs regdump file\n");
> return 0;
> -#endif
>
> rel_cmap:
> fb_dealloc_cmap(&info->cmap);
> @@ -805,9 +801,8 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
> free_irq(par->irq, (void *)par);
> irq_dispose_mapping(par->irq);
>
> -#ifdef CONFIG_FB_DEVICE
> - device_remove_file(&ofdev->dev, &dev_attr_dispregs);
> -#endif
> + if (IS_ENABLED(CONFIG_FB_DEVICE))
> + device_remove_file(&ofdev->dev, &dev_attr_dispregs);
>
> unregister_framebuffer(fbi);
> fb_dealloc_cmap(&fbi->cmap);
> @@ -1107,10 +1102,8 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
>
> pci_set_drvdata(pdev, info);
>
> -#ifdef CONFIG_FB_DEVICE
> - if (device_create_file(dev, &dev_attr_dispregs))
> + if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
> dev_err(dev, "Can't create sysfs regdump file\n");
> -#endif
>
> if (par->type == BT_CARMINE)
> outreg(ctrl, GC_CTRL_INT_MASK, GC_CARMINE_INT_EN);
> @@ -1159,9 +1152,8 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
>
> mb862xx_i2c_exit(par);
>
> -#ifdef CONFIG_FB_DEVICE
> - device_remove_file(&pdev->dev, &dev_attr_dispregs);
> -#endif
> + if (IS_ENABLED(CONFIG_FB_DEVICE))
> + device_remove_file(&pdev->dev, &dev_attr_dispregs);
>
> unregister_framebuffer(fbi);
> fb_dealloc_cmap(&fbi->cmap);
>
> (It would still be questionable however to make the device file creation
> dependent on FB_DEVICE.)
https://www.kernel.org/doc/html/latest/gpu/todo.html#remove-driver-dependencies-on-fb-device
>
> Best regards
> Uwe
^ permalink raw reply
* Re: [PATCH] fbdev: udlfb: make CONFIG_FB_DEVICE optional
From: David Hunter @ 2025-10-06 13:49 UTC (permalink / raw)
To: sukrut heroorkar
Cc: kernel test robot, Helge Deller, Bernie Thompson,
Thomas Zimmermann, Arnd Bergmann, Randy Dunlap,
Bartosz Golaszewski, Zsolt Kajtar, Gonzalo Silvalde Blanco,
linux-fbdev, dri-devel, linux-kernel, llvm, oe-kbuild-all, skhan
In-Reply-To: <CAHCkknrZ-ieNKeg-aj3-NVqgGSk770jJpUpCvn_SuffkPu+ZrQ@mail.gmail.com>
On 10/2/25 02:35, sukrut heroorkar wrote:
>>> kernel test robot noticed the following build errors:
>>>
>> Did you compile and test this code before submitting this patch?
>>
>>
> Yes, I had compiled & loaded the udlfb module with no errors. Please
> let me know how to proceed
> in this case.
Hey Sukrut,
When you make code that deletes something from the kernel, you need to
make sure that all other code that references that code will still
function properly.
When you surround things in the #ifdev, depending on the config file,
the compiler strips those things out and compiles without them. That
means that you actually need to compile and test under two conditions.
Once when CONFIG_FB_DEVICE=y and another time when CONFIG_FB_DEVICE=n.
The test robot gave you a sample config file that you can use. Please
ensure that you have the proper version of Clang on your Host machine
for that particular config file, if you choose to use it.
Also, I strongly recommend that you run the code on a sufficient
hardware or emulator after you test it so that you can verify that the
code does what you think it does.
Thanks,
David Hunter
^ permalink raw reply
* Re: [PATCH v3 1/4] dt-bindings: backlight: Add max25014 bindingsy
From: Maud Spierings @ 2025-10-06 10:02 UTC (permalink / raw)
To: Frank Li
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, dri-devel,
linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel
In-Reply-To: <aMQ6rZJzbMeBrrFv@lizhi-Precision-Tower-5810>
On 9/12/25 17:22, Frank Li wrote:
> On Fri, Sep 12, 2025 at 08:17:09AM +0200, Maud Spierings wrote:
>> Hi Frank,
>> Thanks for the review.
>>
>> On 9/11/25 17:33, Frank Li wrote:
>>> On Thu, Sep 11, 2025 at 09:53:18AM +0200, Maud Spierings via B4 Relay wrote:
>>>> From: Maud Spierings <maudspierings@gocontroll.com>
>>>>
>>>> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
>>>> with integrated boost controller.
>>>>
>>>> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
>>>> ---
>>>> .../bindings/leds/backlight/maxim,max25014.yaml | 81 ++++++++++++++++++++++
>>>> MAINTAINERS | 5 ++
>>>> 2 files changed, 86 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
>>>> new file mode 100644
>>>> index 0000000000000000000000000000000000000000..e113a2ad16aa74f982b9c2ea80578aed2d9424fe
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
>>>> @@ -0,0 +1,81 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Maxim max25014 backlight controller
>>>> +
>>>> +maintainers:
>>>> + - Maud Spierings <maudspierings@gocontroll.com>
>>>> +
>>>> +allOf:
>>>> + - $ref: common.yaml#
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + enum:
>>>> + - maxim,max25014
>>>> +
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> + enable-gpios:
>>>> + maxItems: 1
>>>> +
>>>> + interrupts:
>>>> + maxItems: 1
>>>> +
>>>> + power-supply:
>>>> + description: Regulator which controls the boost converter input rail.
>>>> +
>>>> + pwms:
>>>> + maxItems: 1
>>>> +
>>>> + maxim,iset:
>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>> + maximum: 15
>>>> + default: 11
>>>> + description:
>>>> + Value of the ISET register field. This controls the current scale of the
>>>> + outputs, a higher number means more current.
>>>
>>> Use standard unit. Do not use register value directly.
>>
>> It is unfortunately not just a value in Amps, it depends on the hardware
>> design. There is a kind of "default" table with a 49.9K resistor, but
>> depending on that resistor the current is different.
>
> You should calculate in your driver. if 49.9K is dependence, you should
> add xxx_ohm at dts.
I've tried to find the logic behind the Riref resistor and its
values/effects, but there is no formula for it, there are example values
for 49.9k and 40.2k, besides that all that is stated that the minimum
allowed value is 27.5k and the maximum value is 83.5k.
Not sure how to continue after that, I cannot verify/approximate any
relation with only two data points.
Kind regards,
Maud
^ permalink raw reply
* Re: [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
From: Uwe Kleine-König @ 2025-10-06 8:07 UTC (permalink / raw)
To: Javier Garcia
Cc: deller, tzimmermann, linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <20251005173812.1169436-1-rampxxxx@gmail.com>
Hello,
On Sun, Oct 05, 2025 at 07:38:12PM +0200, Javier Garcia wrote:
> This patch wraps the relevant code blocks with #ifdef CONFIG_FB_DEVICE,
> allowing the driver to be built and used even if CONFIG_FB_DEVICE is not selected.
>
> The sysfs only give access to show some controller and cursor registers so
> it's not needed to allow driver works correctly.
>
> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
I don't understand this patch. CONFIG_FB_DEVICE is about legacy /dev/fb*
stuff, and this patch make the creation of a sysfs file dependent on
that.
> diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> index ade88e7bc760..a691a5fefb25 100644
> --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> @@ -530,6 +530,7 @@ static int mb862xxfb_init_fbinfo(struct fb_info *fbi)
> return 0;
> }
>
> +#ifdef CONFIG_FB_DEVICE
> /*
> * show some display controller and cursor registers
> */
> @@ -569,6 +570,7 @@ static ssize_t dispregs_show(struct device *dev,
> }
>
> static DEVICE_ATTR_RO(dispregs);
> +#endif
>
> static irqreturn_t mb862xx_intr(int irq, void *dev_id)
> {
> @@ -759,9 +761,11 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
>
> dev_set_drvdata(dev, info);
>
> +#ifdef CONFIG_FB_DEVICE
> if (device_create_file(dev, &dev_attr_dispregs))
> dev_err(dev, "Can't create sysfs regdump file\n");
> return 0;
> +#endif
That looks wrong. Without CONFIG_FB_DEVICE set the success return is
removed and all resources are freed essentially making the
CONFIG_FB_MB862XX_LIME part of the driver unusable.
> rel_cmap:
> fb_dealloc_cmap(&info->cmap);
> @@ -801,7 +805,9 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
> free_irq(par->irq, (void *)par);
> irq_dispose_mapping(par->irq);
>
> +#ifdef CONFIG_FB_DEVICE
> device_remove_file(&ofdev->dev, &dev_attr_dispregs);
> +#endif
>
> unregister_framebuffer(fbi);
> fb_dealloc_cmap(&fbi->cmap);
> @@ -1101,8 +1107,10 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
>
> pci_set_drvdata(pdev, info);
>
> +#ifdef CONFIG_FB_DEVICE
> if (device_create_file(dev, &dev_attr_dispregs))
> dev_err(dev, "Can't create sysfs regdump file\n");
> +#endif
>
> if (par->type == BT_CARMINE)
> outreg(ctrl, GC_CTRL_INT_MASK, GC_CARMINE_INT_EN);
> @@ -1151,7 +1159,9 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
>
> mb862xx_i2c_exit(par);
>
> +#ifdef CONFIG_FB_DEVICE
> device_remove_file(&pdev->dev, &dev_attr_dispregs);
> +#endif
>
> unregister_framebuffer(fbi);
> fb_dealloc_cmap(&fbi->cmap);
The amount of ifdefs could be greatly reduced using the following patch:
diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index a691a5fefb25..3f79dfc27a53 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -530,7 +530,6 @@ static int mb862xxfb_init_fbinfo(struct fb_info *fbi)
return 0;
}
-#ifdef CONFIG_FB_DEVICE
/*
* show some display controller and cursor registers
*/
@@ -570,7 +569,6 @@ static ssize_t dispregs_show(struct device *dev,
}
static DEVICE_ATTR_RO(dispregs);
-#endif
static irqreturn_t mb862xx_intr(int irq, void *dev_id)
{
@@ -761,11 +759,9 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
dev_set_drvdata(dev, info);
-#ifdef CONFIG_FB_DEVICE
- if (device_create_file(dev, &dev_attr_dispregs))
+ if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
dev_err(dev, "Can't create sysfs regdump file\n");
return 0;
-#endif
rel_cmap:
fb_dealloc_cmap(&info->cmap);
@@ -805,9 +801,8 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
free_irq(par->irq, (void *)par);
irq_dispose_mapping(par->irq);
-#ifdef CONFIG_FB_DEVICE
- device_remove_file(&ofdev->dev, &dev_attr_dispregs);
-#endif
+ if (IS_ENABLED(CONFIG_FB_DEVICE))
+ device_remove_file(&ofdev->dev, &dev_attr_dispregs);
unregister_framebuffer(fbi);
fb_dealloc_cmap(&fbi->cmap);
@@ -1107,10 +1102,8 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
pci_set_drvdata(pdev, info);
-#ifdef CONFIG_FB_DEVICE
- if (device_create_file(dev, &dev_attr_dispregs))
+ if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
dev_err(dev, "Can't create sysfs regdump file\n");
-#endif
if (par->type == BT_CARMINE)
outreg(ctrl, GC_CTRL_INT_MASK, GC_CARMINE_INT_EN);
@@ -1159,9 +1152,8 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
mb862xx_i2c_exit(par);
-#ifdef CONFIG_FB_DEVICE
- device_remove_file(&pdev->dev, &dev_attr_dispregs);
-#endif
+ if (IS_ENABLED(CONFIG_FB_DEVICE))
+ device_remove_file(&pdev->dev, &dev_attr_dispregs);
unregister_framebuffer(fbi);
fb_dealloc_cmap(&fbi->cmap);
(It would still be questionable however to make the device file creation
dependent on FB_DEVICE.)
Best regards
Uwe
^ 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