* [PATCH v2] staging: sm750fb: style fixes: align call and split chained assignment
From: Cristian Del Gobbo @ 2025-10-29 2:20 UTC (permalink / raw)
To: sudip.mukherjee
Cc: teddy.wang, gregkh, linux-fbdev, linux-staging, linux-kernel,
Cristian Del Gobbo, Dan Carpenter
- Drop previous change that made g_fbmode[] elements const (broke build).
- Align the continued arguments of sm750_hw_cursor_set_size() with the
opening parenthesis.
- Replace a chained assignment of red/green/blue with a temporary
variable to satisfy checkpatch and improve readability.
No functional change intended.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Cristian Del Gobbo <cristiandelgobbo87@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 3659af7e519d..94a99af4320e 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -121,8 +121,8 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
sm750_hw_cursor_disable(cursor);
if (fbcursor->set & FB_CUR_SETSIZE)
sm750_hw_cursor_set_size(cursor,
- fbcursor->image.width,
- fbcursor->image.height);
+ fbcursor->image.width,
+ fbcursor->image.height);
if (fbcursor->set & FB_CUR_SETPOS)
sm750_hw_cursor_set_pos(cursor,
@@ -538,7 +538,11 @@ static int lynxfb_ops_setcolreg(unsigned int regno,
}
if (info->var.grayscale)
- red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+ int y = (red * 77 + green * 151 + blue * 28) >> 8;
+
+ red = y;
+ green = y;
+ blue = y;
if (var->bits_per_pixel == 8 &&
info->fix.visual == FB_VISUAL_PSEUDOCOLOR) {
--
2.34.1
^ permalink raw reply related
* [PATCH v2] staging: sm750fb: style fixes: align call and split chained assignment
From: Cristian Del Gobbo @ 2025-10-29 2:17 UTC (permalink / raw)
To: sudip.mukherjee
Cc: teddy.wang, gregkh, linux-fbdev, linux-staging, linux-kernel,
Cristian Del Gobbo, Dan Carpenter
- Drop previous change that made g_fbmode[] elements const (broke build).
- Align the continued arguments of sm750_hw_cursor_set_size() with the
opening parenthesis.
- Replace a chained assignment of red/green/blue with a temporary
variable to satisfy checkpatch and improve readability.
No functional change intended.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Cristian Del Gobbo <cristiandelgobbo87@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 3659af7e519d..94a99af4320e 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -121,8 +121,8 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
sm750_hw_cursor_disable(cursor);
if (fbcursor->set & FB_CUR_SETSIZE)
sm750_hw_cursor_set_size(cursor,
- fbcursor->image.width,
- fbcursor->image.height);
+ fbcursor->image.width,
+ fbcursor->image.height);
if (fbcursor->set & FB_CUR_SETPOS)
sm750_hw_cursor_set_pos(cursor,
@@ -538,7 +538,11 @@ static int lynxfb_ops_setcolreg(unsigned int regno,
}
if (info->var.grayscale)
- red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+ int y = (red * 77 + green * 151 + blue * 28) >> 8;
+
+ red = y;
+ green = y;
+ blue = y;
if (var->bits_per_pixel == 8 &&
info->fix.visual == FB_VISUAL_PSEUDOCOLOR) {
--
2.34.1
^ permalink raw reply related
* [PATCH v2] staging: sm750fb: style fixes: align call and split chained assignment
From: Cristian Del Gobbo @ 2025-10-29 2:14 UTC (permalink / raw)
To: sudip.mukherjee
Cc: teddy.wang, gregkh, linux-fbdev, linux-staging, linux-kernel,
Cristian Del Gobbo, Dan Carpenter
- Drop previous change that made g_fbmode[] elements const (broke build).
- Align the continued arguments of sm750_hw_cursor_set_size() with the
opening parenthesis.
- Replace a chained assignment of red/green/blue with a temporary
variable to satisfy checkpatch and improve readability.
No functional change intended.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Cristian Del Gobbo <cristiandelgobbo87@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 3659af7e519d..94a99af4320e 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -121,8 +121,8 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
sm750_hw_cursor_disable(cursor);
if (fbcursor->set & FB_CUR_SETSIZE)
sm750_hw_cursor_set_size(cursor,
- fbcursor->image.width,
- fbcursor->image.height);
+ fbcursor->image.width,
+ fbcursor->image.height);
if (fbcursor->set & FB_CUR_SETPOS)
sm750_hw_cursor_set_pos(cursor,
@@ -538,7 +538,11 @@ static int lynxfb_ops_setcolreg(unsigned int regno,
}
if (info->var.grayscale)
- red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+ int y = (red * 77 + green * 151 + blue * 28) >> 8;
+
+ red = y;
+ green = y;
+ blue = y;
if (var->bits_per_pixel == 8 &&
info->fix.visual == FB_VISUAL_PSEUDOCOLOR) {
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] fbdev: atyfb: Check if pll_ops->init_pll failed
From: Helge Deller @ 2025-10-28 21:29 UTC (permalink / raw)
To: Daniel Palmer, linux-fbdev; +Cc: dri-devel, linux-kernel
In-Reply-To: <20251024093715.4012119-1-daniel@0x0f.com>
On 10/24/25 11:37, Daniel Palmer wrote:
> Actually check the return value from pll_ops->init_pll()
> as it can return an error.
>
> If the card's BIOS didn't run because it's not the primary VGA card
> the fact that the xclk source is unsupported is printed as shown
> below but the driver continues on regardless and on my machine causes
> a hard lock up.
>
> [ 61.470088] atyfb 0000:03:05.0: enabling device (0080 -> 0083)
> [ 61.476191] atyfb: using auxiliary register aperture
> [ 61.481239] atyfb: 3D RAGE XL (Mach64 GR, PCI-33) [0x4752 rev 0x27]
> [ 61.487569] atyfb: 512K SGRAM (1:1), 14.31818 MHz XTAL, 230 MHz PLL, 83 Mhz MCLK, 63 MHz XCLK
> [ 61.496112] atyfb: Unsupported xclk source: 5.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
> drivers/video/fbdev/aty/atyfb_base.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v2] fbcon: Set fb_display[i]->mode to NULL when the mode is released
From: Helge Deller @ 2025-10-28 21:27 UTC (permalink / raw)
To: Quanmin Yan, tzimmermann
Cc: simona, linux-kernel, linux-fbdev, dri-devel, wangkefeng.wang,
zuoze1, sunnanyong
In-Reply-To: <20251010081659.3609082-1-yanquanmin1@huawei.com>
On 10/10/25 10:16, Quanmin Yan wrote:
> 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>
> ---
> drivers/video/fbdev/core/fbcon.c | 19 +++++++++++++++++++
> drivers/video/fbdev/core/fbmem.c | 1 +
> include/linux/fbcon.h | 2 ++
> 3 files changed, 22 insertions(+)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v2] fbdev: bitblit: bound-check glyph index in bit_putcs*
From: Helge Deller @ 2025-10-28 21:23 UTC (permalink / raw)
To: linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <cc1e8193-6588-4e75-896e-b807f451fb4c@suse.de>
On 10/20/25 16:29, Thomas Zimmermann wrote:
> Hi
>
> Am 20.10.25 um 15:47 schrieb Junjie Cao:
>> bit_putcs_aligned()/unaligned() derived the glyph pointer from the
>> character value masked by 0xff/0x1ff, which may exceed the actual font's
>> glyph count and read past the end of the built-in font array.
>> Clamp the index to the actual glyph count before computing the address.
>>
>> This fixes a global out-of-bounds read reported by syzbot.
>>
>> Reported-by: syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=793cf822d213be1a74f2
>> Tested-by: syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
...
>> v1: https://lore.kernel.org/linux-fbdev/5d237d1a-a528-4205-a4d8-71709134f1e1@suse.de/
>> v1 -> v2:
>> - Fix indentation and add blank line after declarations with the .pl helper
>> - No functional changes
>>
>> drivers/video/fbdev/core/bitblit.c | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH] fbdev/pvr2fb: Fix leftover reference to ONCHIP_NR_DMA_CHANNELS
From: Helge Deller @ 2025-10-28 21:16 UTC (permalink / raw)
To: Florian Fuchs, linux-fbdev; +Cc: linux-kernel, linux-sh, dri-devel
In-Reply-To: <20251025223850.1056175-1-fuchsfl@gmail.com>
On 10/26/25 00:38, Florian Fuchs wrote:
> Commit e24cca19babe ("sh: Kill off MAX_DMA_ADDRESS leftovers.") removed
> the define ONCHIP_NR_DMA_CHANNELS. So that the leftover reference needs
> to be replaced by CONFIG_NR_ONCHIP_DMA_CHANNELS to compile successfully
> with CONFIG_PVR2_DMA enabled.
>
> Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
> ---
> Note: The fix has been compiled, and tested on real Dreamcast hardware,
> with CONFIG_PVR2_DMA=y.
>
> drivers/video/fbdev/pvr2fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH] video: valkyriefb: Fix reference count leak in valkyriefb_init
From: Helge Deller @ 2025-10-28 21:15 UTC (permalink / raw)
To: Miaoqian Lin, Paul Mackerras, Benjamin Herrenschmidt, linux-fbdev,
dri-devel, linux-kernel
Cc: stable
In-Reply-To: <20251027084340.79419-1-linmq006@gmail.com>
On 10/27/25 09:43, Miaoqian Lin wrote:
> The of_find_node_by_name() function returns a device tree node with its
> reference count incremented. The caller is responsible for calling
> of_node_put() to release this reference when done.
>
> Found via static analysis.
>
> Fixes: cc5d0189b9ba ("[PATCH] powerpc: Remove device_node addrs/n_addr")
> Cc: stable@vger.kernel.org
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> drivers/video/fbdev/valkyriefb.c | 2 ++
> 1 file changed, 2 insertions(+)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH] video: fb: Fix typo in comment
From: Helge Deller @ 2025-10-28 21:12 UTC (permalink / raw)
To: PIYUSH CHOUDHARY; +Cc: linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20251019183508.20804-1-mercmerc961@gmail.com>
On 10/19/25 20:35, PIYUSH CHOUDHARY wrote:
> Fix typo: "verical" -> "vertical" in macro description
>
> Signed-off-by: PIYUSH CHOUDHARY <mercmerc961@gmail.com>
> ---
> include/uapi/linux/fb.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v2] fbdev: vga16fb: Request memory region.
From: Helge Deller @ 2025-10-28 21:09 UTC (permalink / raw)
To: Javier Garcia; +Cc: linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <20251028191615.2765711-1-rampxxxx@gmail.com>
On 10/28/25 20:16, Javier Garcia wrote:
> This patch reserve and release VGA memory region.
>
> This align with Documentation/drm/todo.rst
> "Request memory regions in all fbdev drivers"
>
> I've tested with 32bits kernel and qemu.
>
> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
> ---
> v1 -> v2:
> * Add release in vga16fb_remove , thanks Helge Deller.
> * v1 https://lore.kernel.org/lkml/20251016171845.1397153-1-rampxxxx@gmail.com/
>
> drivers/video/fbdev/vga16fb.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
applied.
Thanks!
Helge
^ permalink raw reply
* [PATCH v2] fbdev: vga16fb: Request memory region.
From: Javier Garcia @ 2025-10-28 19:16 UTC (permalink / raw)
To: deller; +Cc: linux-fbdev, dri-devel, linux-kernel, shuah, Javier Garcia
In-Reply-To: <6c565f4c-ef05-45f2-9a82-cbba4a11cc07@gmx.de>
This patch reserve and release VGA memory region.
This align with Documentation/drm/todo.rst
"Request memory regions in all fbdev drivers"
I've tested with 32bits kernel and qemu.
Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
---
v1 -> v2:
* Add release in vga16fb_remove , thanks Helge Deller.
* v1 https://lore.kernel.org/lkml/20251016171845.1397153-1-rampxxxx@gmail.com/
drivers/video/fbdev/vga16fb.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index eedab14c7d51..208e3eefb3ff 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -1319,6 +1319,11 @@ static int vga16fb_probe(struct platform_device *dev)
if (ret)
return ret;
+ if (!request_mem_region(vga16fb_fix.smem_start, vga16fb_fix.smem_len,
+ "vga16b")) {
+ dev_err(&dev->dev,"vga16b: cannot reserve video memory at 0x%lx\n",
+ vga16fb_fix.smem_start);
+ }
printk(KERN_DEBUG "vga16fb: initializing\n");
info = framebuffer_alloc(sizeof(struct vga16fb_par), &dev->dev);
@@ -1398,6 +1403,8 @@ static int vga16fb_probe(struct platform_device *dev)
err_ioremap:
framebuffer_release(info);
err_fb_alloc:
+ release_mem_region(vga16fb_fix.smem_start,
+ vga16fb_fix.smem_len);
return ret;
}
@@ -1407,6 +1414,8 @@ static void vga16fb_remove(struct platform_device *dev)
if (info)
unregister_framebuffer(info);
+ release_mem_region(vga16fb_fix.smem_start,
+ vga16fb_fix.smem_len);
}
static const struct platform_device_id vga16fb_driver_id_table[] = {
--
2.50.1
^ permalink raw reply related
* [PATCH] fbdev/vesafb: Use dev_* fn's instead printk.
From: Javier Garcia @ 2025-10-28 18:50 UTC (permalink / raw)
To: deller; +Cc: linux-fbdev, dri-devel, linux-kernel, shuah, Javier Garcia
- Family dev_* fn's will show device name, giving extra info to logs.
- Delete the prefix `vesafb:` from msg strings, not needed now.
[ 0.981825] vesa-framebuffer vesa-framebuffer.0: scrolling: redraw
Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
---
drivers/video/fbdev/vesafb.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index a81df8865143..36c1fc553883 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -314,8 +314,8 @@ static int vesafb_probe(struct platform_device *dev)
#endif
if (!request_mem_region(vesafb_fix.smem_start, size_total, "vesafb")) {
- printk(KERN_WARNING
- "vesafb: cannot reserve video memory at 0x%lx\n",
+ dev_warn(&dev->dev,
+ "cannot reserve video memory at 0x%lx\n",
vesafb_fix.smem_start);
/* We cannot make this fatal. Sometimes this comes from magic
spaces our resource handlers simply don't know about */
@@ -333,12 +333,12 @@ static int vesafb_probe(struct platform_device *dev)
par->base = si->lfb_base;
par->size = size_total;
- printk(KERN_INFO "vesafb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
+ dev_info(&dev->dev,"mode is %dx%dx%d, linelength=%d, pages=%d\n",
vesafb_defined.xres, vesafb_defined.yres, vesafb_defined.bits_per_pixel,
vesafb_fix.line_length, si->pages);
if (si->vesapm_seg) {
- printk(KERN_INFO "vesafb: protected mode interface info at %04x:%04x\n",
+ dev_info(&dev->dev, "protected mode interface info at %04x:%04x\n",
si->vesapm_seg, si->vesapm_off);
}
@@ -352,9 +352,9 @@ static int vesafb_probe(struct platform_device *dev)
pmi_base = (unsigned short *)phys_to_virt(pmi_phys);
pmi_start = (void*)((char*)pmi_base + pmi_base[1]);
pmi_pal = (void*)((char*)pmi_base + pmi_base[2]);
- printk(KERN_INFO "vesafb: pmi: set display start = %p, set palette = %p\n",pmi_start,pmi_pal);
+ dev_info(&dev->dev, "pmi: set display start = %p, set palette = %p\n",pmi_start,pmi_pal);
if (pmi_base[3]) {
- printk(KERN_INFO "vesafb: pmi: ports = ");
+ dev_info(&dev->dev, "pmi: ports = ");
for (i = pmi_base[3]/2; pmi_base[i] != 0xffff; i++)
printk("%x ", pmi_base[i]);
printk("\n");
@@ -365,14 +365,14 @@ static int vesafb_probe(struct platform_device *dev)
* Rules are: we have to set up a descriptor for the requested
* memory area and pass it in the ES register to the BIOS function.
*/
- printk(KERN_INFO "vesafb: can't handle memory requests, pmi disabled\n");
+ dev_info(&dev->dev, "can't handle memory requests, pmi disabled\n");
ypan = pmi_setpal = 0;
}
}
}
if (vesafb_defined.bits_per_pixel == 8 && !pmi_setpal && !vga_compat) {
- printk(KERN_WARNING "vesafb: hardware palette is unchangeable,\n"
+ dev_warn(&dev->dev, "hardware palette is unchangeable,\n"
" colors may be incorrect\n");
vesafb_fix.visual = FB_VISUAL_STATIC_PSEUDOCOLOR;
}
@@ -380,10 +380,10 @@ static int vesafb_probe(struct platform_device *dev)
vesafb_defined.xres_virtual = vesafb_defined.xres;
vesafb_defined.yres_virtual = vesafb_fix.smem_len / vesafb_fix.line_length;
if (ypan && vesafb_defined.yres_virtual > vesafb_defined.yres) {
- printk(KERN_INFO "vesafb: scrolling: %s using protected mode interface, yres_virtual=%d\n",
+ dev_info(&dev->dev, "scrolling: %s using protected mode interface, yres_virtual=%d\n",
(ypan > 1) ? "ywrap" : "ypan",vesafb_defined.yres_virtual);
} else {
- printk(KERN_INFO "vesafb: scrolling: redraw\n");
+ dev_info(&dev->dev, "scrolling: redraw\n");
vesafb_defined.yres_virtual = vesafb_defined.yres;
ypan = 0;
}
@@ -410,7 +410,7 @@ static int vesafb_probe(struct platform_device *dev)
vesafb_defined.bits_per_pixel;
}
- printk(KERN_INFO "vesafb: %s: "
+ dev_info(&dev->dev, "%s: "
"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
(vesafb_defined.bits_per_pixel > 8) ?
"Truecolor" : (vga_compat || pmi_setpal) ?
@@ -453,14 +453,14 @@ static int vesafb_probe(struct platform_device *dev)
}
if (!info->screen_base) {
- printk(KERN_ERR
- "vesafb: abort, cannot ioremap video memory 0x%x @ 0x%lx\n",
+ dev_err(&dev->dev,
+ "abort, cannot ioremap video memory 0x%x @ 0x%lx\n",
vesafb_fix.smem_len, vesafb_fix.smem_start);
err = -EIO;
goto err_release_region;
}
- printk(KERN_INFO "vesafb: framebuffer at 0x%lx, mapped to 0x%p, "
+ dev_info(&dev->dev, "framebuffer at 0x%lx, mapped to 0x%p, "
"using %dk, total %dk\n",
vesafb_fix.smem_start, info->screen_base,
size_remap/1024, size_total/1024);
--
2.50.1
^ permalink raw reply related
* Re: [PATCH 2/2] backlight: aw99706: Add support for Awinic AW99706 backlight
From: Daniel Thompson @ 2025-10-28 13:22 UTC (permalink / raw)
To: Junjie Cao
Cc: Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, dri-devel,
linux-leds, devicetree, linux-kernel, linux-fbdev, Pengyu Luo
In-Reply-To: <20251026123923.1531727-3-caojunjie650@gmail.com>
On Sun, Oct 26, 2025 at 08:39:23PM +0800, Junjie Cao wrote:
> Add support for Awinic AW99706 backlight, which can be found in
> tablet and notebook backlight, one case is the Lenovo Legion Y700
> Gen4. This driver refers to the official datasheets and android
> driver, they can be found in [1].
>
> [1] https://www.awinic.com/en/productDetail/AW99706QNR
>
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> Signed-off-by: Junjie Cao <caojunjie650@gmail.com>
> ---
> diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
> new file mode 100644
> index 000000000..8dafdea45
> --- /dev/null
> +++ b/drivers/video/backlight/aw99706.c
> @@ -0,0 +1,503 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * aw99706 - Backlight driver for the AWINIC AW99706
> + *
> + * Copyright (C) 2025 Junjie Cao <caojunjie650@gmail.com>
> + * Copyright (C) 2025 Pengyu Luo <mitltlatltl@gmail.com>
> + *
> + * Based on vendor driver:
> + * Copyright (c) 2023 AWINIC Technology CO., LTD
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#define AW99706_MAX_BRT_LVL 4095
> +#define AW99706_REG_MAX 0x1F
> +#define AW99706_ID 0x07
> +
> +/* registers list */
> +#define AW99706_CFG0_REG 0x00
> +#define AW99706_DIM_MODE_MASK GENMASK(1, 0)
> +
> +#define AW99706_CFG1_REG 0x01
> +#define AW99706_SW_FREQ_MASK GENMASK(3, 0)
> +#define AW99706_SW_ILMT_MASK GENMASK(5, 4)
> +
> +#define AW99706_CFG2_REG 0x02
> +#define AW99706_ILED_MAX_MASK GENMASK(6, 0)
> +#define AW99706_UVLOSEL_MASK BIT(7)
> +
> +#define AW99706_CFG3_REG 0x03
> +#define AW99706_CFG4_REG 0x04
> +#define AW99706_BRT_MSB_MASK GENMASK(3, 0)
> +
> +#define AW99706_CFG5_REG 0x05
> +#define AW99706_BRT_LSB_MASK GENMASK(7, 0)
> +
> +#define AW99706_CFG6_REG 0x06
> +#define AW99706_FADE_TIME_MASK GENMASK(2, 0)
> +#define AW99706_SLOPE_TIME_MASK GENMASK(5, 3)
> +#define AW99706_RAMP_CTL_MASK GENMASK(7, 6)
> +
> +#define AW99706_CFG7_REG 0x07
> +#define AW99706_BRT_MODE_MASK GENMASK(1, 0)
> +
> +#define AW99706_CFG8_REG 0x08
> +#define AW99706_ONOFF_TIME_MASK GENMASK(2, 0)
> +
> +#define AW99706_CFG9_REG 0x09
> +#define AW99706_CFGA_REG 0x0A
> +#define AW99706_CFGB_REG 0x0B
> +#define AW99706_CFGC_REG 0x0C
> +#define AW99706_CFGD_REG 0x0D
> +#define AW99706_FLAG_REG 0x10
> +#define AW99706_BACKLIGHT_EN_MASK BIT(7)
> +
> +#define AW99706_CHIPID_REG 0x11
> +#define AW99706_LED_OPEN_FLAG_REG 0x12
> +#define AW99706_LED_SHORT_FLAG_REG 0x13
> +#define AW99706_MTPLDOSEL_REG 0x1E
> +#define AW99706_MTPRUN_REG 0x1F
> +
> +#define RESV 0
> +
> +/* Boost switching frequency table, in kHz */
> +static const u32 aw99706_sw_freq_tbl[] = {
> + RESV, RESV, RESV, RESV, 300, 400, 500, 600,
> + 660, 750, 850, 1000, 1200, 1330, 1500, 1700
> +};
> +
> +/* Switching current limitation table, in mA */
> +static const u32 aw99706_sw_ilmt_tbl[] = {
> + 1500, 2000, 2500, 3000
> +};
> +
> +/* ULVO threshold table, in mV */
> +static const u32 aw99706_ulvo_thres_tbl[] = {
> + 2200, 5000
> +};
> +
> +/* Fade In/Out time table, in us */
> +static const u32 aw99706_fade_time_tbl[] = {
> + 8, 16, 32, 64, 128, 256, 512, 1024
> +};
> +
> +/* Slope time table, in ms */
> +static const u32 aw99706_slopetime_tbl[] = {
> + 8, 24, 48, 96, 200, 300, 400, 500
> +};
> +
> +/* Turn on/off time table, in ns */
> +static const u32 aw99706_onoff_time_tbl[] = {
> + RESV, 250, 500, 1000, 2000, 4000, 8000, 16000
> +};
> +
> +struct aw99706_device {
> + struct i2c_client *client;
> + struct device *dev;
> + struct regmap *regmap;
> + struct backlight_device *bl_dev;
> + struct gpio_desc *hwen_gpio;
> + bool bl_enable;
> +};
> +
> +enum reg_access {
> + REG_NONE_ACCESS = 0,
> + REG_RD_ACCESS = 1,
> + REG_WR_ACCESS = 2,
> +};
> +
> +struct aw99706_reg {
> + u8 defval;
> + u8 access;
> +};
> +
> +const struct aw99706_reg aw99706_regs[AW99706_REG_MAX + 1] = {
> + [AW99706_CFG0_REG] = {0x65, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFG1_REG] = {0x39, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFG2_REG] = {0x1e, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFG3_REG] = {0x04, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFG4_REG] = {0x00, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFG5_REG] = {0x00, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFG6_REG] = {0xa9, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFG7_REG] = {0x04, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFG8_REG] = {0x0c, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFG9_REG] = {0x4b, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFGA_REG] = {0x72, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFGB_REG] = {0x01, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFGC_REG] = {0x6c, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_CFGD_REG] = {0xfe, REG_RD_ACCESS | REG_WR_ACCESS},
> + [AW99706_FLAG_REG] = {0x00, REG_RD_ACCESS},
> + [AW99706_CHIPID_REG] = {AW99706_ID, REG_RD_ACCESS},
> + [AW99706_LED_OPEN_FLAG_REG] = {0x00, REG_RD_ACCESS},
> + [AW99706_LED_SHORT_FLAG_REG] = {0x00, REG_RD_ACCESS},
> +
> + /*
> + * Write bit is dropped here, writing BIT(0) to MTPLDOSEL will unlock
> + * Multi-time Programmable (MTP).
> + */
> + [AW99706_MTPLDOSEL_REG] = {0x00, REG_RD_ACCESS},
> + [AW99706_MTPRUN_REG] = {0x00, REG_NONE_ACCESS},
> +};
> +
> +static bool aw99706_readable_reg(struct device *dev, unsigned int reg)
> +{
> + return aw99706_regs[reg].access & REG_RD_ACCESS;
> +}
> +
> +static bool aw99706_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + return aw99706_regs[reg].access & REG_WR_ACCESS;
> +}
> +
> +static inline int aw99706_i2c_read(struct aw99706_device *aw, u8 reg,
> + unsigned int *val)
> +{
> + return regmap_read(aw->regmap, reg, val);
> +}
> +
> +static inline int aw99706_i2c_write(struct aw99706_device *aw, u8 reg, u8 val)
> +{
> + return regmap_write(aw->regmap, reg, val);
> +}
> +
> +static inline int aw99706_i2c_update_bits(struct aw99706_device *aw, u8 reg,
> + u8 mask, u8 val)
> +{
> + return regmap_update_bits(aw->regmap, reg, mask, val);
> +}
> +
> +struct aw99706_dt_prop {
> + const char * const name;
> + const u32 * const lookup_tbl;
> + u8 tbl_size;
> + u8 reg;
> + u8 mask;
> + u8 val;
> + u32 raw_val;
> +};
> +
> +static struct aw99706_dt_prop aw99706_dt_props[] = {
> + {
> + "awinic,dim-mode", NULL,
> + 0,
> + AW99706_CFG0_REG, AW99706_DIM_MODE_MASK
> + },
> + {
> + "awinic,sw-freq", aw99706_sw_freq_tbl,
> + ARRAY_SIZE(aw99706_sw_freq_tbl),
> + AW99706_CFG1_REG, AW99706_SW_FREQ_MASK
> + },
> + {
> + "awinic,sw-ilmt", aw99706_sw_ilmt_tbl,
> + ARRAY_SIZE(aw99706_sw_ilmt_tbl),
> + AW99706_CFG1_REG, AW99706_SW_ILMT_MASK
> + },
> + {
> + "awinic,iled-max", NULL,
> + 0,
> + AW99706_CFG2_REG, AW99706_ILED_MAX_MASK
> +
> + },
> + {
> + "awinic,uvlo-thres", aw99706_ulvo_thres_tbl,
> + ARRAY_SIZE(aw99706_ulvo_thres_tbl),
> + AW99706_CFG2_REG, AW99706_UVLOSEL_MASK
> + },
> + {
> + "awinic,fade-time", aw99706_fade_time_tbl,
> + ARRAY_SIZE(aw99706_fade_time_tbl),
> + AW99706_CFG6_REG, AW99706_FADE_TIME_MASK
> + },
> + {
> + "awinic,slope-time", aw99706_slopetime_tbl,
> + ARRAY_SIZE(aw99706_slopetime_tbl),
> + AW99706_CFG6_REG, AW99706_SLOPE_TIME_MASK
> + },
> + {
> + "awinic,ramp-ctl", NULL,
> + 0,
> + AW99706_CFG6_REG, AW99706_RAMP_CTL_MASK
> + },
> + {
> + "awinic,brt-mode", NULL,
> + 0,
> + AW99706_CFG7_REG, AW99706_BRT_MODE_MASK
> + },
> + {
> + "awinic,onoff-time", aw99706_onoff_time_tbl,
> + ARRAY_SIZE(aw99706_onoff_time_tbl),
> + AW99706_CFG8_REG, AW99706_ONOFF_TIME_MASK
> + },
> +};
> +
> +static int aw99706_lookup(const u32 * const tbl, int size, u32 val)
> +{
> + int i;
> +
> + for (i = 0; i < size; i++)
> + if (tbl[i] == val)
> + return i;
> +
> + return -1;
> +}
> +
> +static inline void aw99706_prop_set_default(struct aw99706_dt_prop *prop)
> +{
> + prop->val = prop->mask & aw99706_regs[prop->reg].defval;
Why included the default value in the register descriptions?
defval is only used to provide values for missing DT properties so using
the raw register values is cryptic and hard to read.
Including a default value in the aw99706_dt_props table instead would be
much more readable (because the defaults could use the same units at the
device tree).
> +}
> +
> +static void aw99706_dt_property_convert(struct aw99706_dt_prop *prop)
> +{
> + unsigned int val, shift;
> +
> + if (prop->lookup_tbl) {
> + val = aw99706_lookup(prop->lookup_tbl, prop->tbl_size,
> + prop->raw_val);
> + if (val < 0) {
> + aw99706_prop_set_default(prop);
This should not happen silently.
If the DT has provided an invalid value then we be issuing *at minimum*
a message at warning level or above. Many drivers will simply refuse to
probe when the DT is broken.
> + return;
> + }
> +
> + } else {
> + val = prop->raw_val;
> + }
> +
> + shift = ffs(prop->mask) - 1;
> + val <<= shift;
> + prop->val = prop->mask & val;
> +}
> +
> +static void aw99706_dt_parse(struct aw99706_device *aw)
> +{
> + struct aw99706_dt_prop *prop;
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(aw99706_dt_props); i++) {
> + prop = &aw99706_dt_props[i];
> + ret = device_property_read_u32(aw->dev, prop->name,
> + &prop->raw_val);
> + if (ret < 0) {
> + dev_warn(aw->dev, "Missing property %s: %d\n",
> + prop->name, ret);
Why is there a warning when an optional property is not present. A DT
not including an optional property needs no message at all.
> +
> + aw99706_prop_set_default(prop);
> + } else {
> + aw99706_dt_property_convert(prop);
> + }
> + }
> +
> + /* This property requires a long linear array, using formula for now */
> + aw99706_dt_props[3].val = (aw99706_dt_props[3].raw_val - 5000) / 500;
Using a formula is fine, but I don't like doing it retrospectively.
Hard coding the 3 makes maintenance difficult and we end up making the
whole of aw99706_dt_props writeable just so we can store raw_val once!
Much better, IMHO, to embed a function pointer into the table and make
the whole table const. The function pointer can be
aw99706_dt_property_convert() in most cases (although rename it
`aw99706_dt_property_lookup_from_table() ) and can implement any
formula you need.
> +}
> +
> +static int aw99706_hw_init(struct aw99706_device *aw)
> +{
> + int ret, i;
> +
> + gpiod_set_value_cansleep(aw->hwen_gpio, 1);
> +
> + for (i = 0; i < ARRAY_SIZE(aw99706_dt_props); i++) {
> + ret = aw99706_i2c_update_bits(aw, aw99706_dt_props[i].reg,
> + aw99706_dt_props[i].mask,
> + aw99706_dt_props[i].val);
> + if (ret < 0) {
> + dev_err(aw->dev, "Failed to write init data %d\n", ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int aw99706_bl_enable(struct aw99706_device *aw, bool en)
> +{
> + int ret;
> + u8 val;
> +
> + FIELD_MODIFY(AW99706_BACKLIGHT_EN_MASK, &val, en);
> + ret = aw99706_i2c_update_bits(aw, AW99706_CFGD_REG,
> + AW99706_BACKLIGHT_EN_MASK, val);
> + if (ret)
> + dev_err(aw->dev, "Failed to enable backlight!\n");
> +
> + return ret;
> +}
> +
> +static int aw99706_backlight_switch(struct aw99706_device *aw, u32 brt_lvl)
> +{
> + bool bl_enable_now = !!brt_lvl;
> + int ret = 0;
> +
> + if (aw->bl_enable != bl_enable_now) {
> + aw->bl_enable = bl_enable_now;
> + ret = aw99706_bl_enable(aw, bl_enable_now);
> + }
> +
> + return ret;
> +}
> +
> +static int aw99706_update_brightness(struct aw99706_device *aw, u32 brt_lvl)
> +{
> + int ret;
> +
> + ret = aw99706_i2c_write(aw, AW99706_CFG4_REG,
> + (brt_lvl >> 8) & AW99706_BRT_MSB_MASK);
> + if (ret < 0)
> + return ret;
> +
> + ret = aw99706_i2c_write(aw, AW99706_CFG5_REG,
> + brt_lvl & AW99706_BRT_LSB_MASK);
> + if (ret < 0)
> + return ret;
> +
> + return aw99706_backlight_switch(aw, brt_lvl);
I'm not sure there is much benefit pushing this out into a seperate
function. Merge this inline.
> +}
Everything below this function looked OK at first glance.
Daniel.
^ permalink raw reply
* Re: [PATCH] backlight: ktd2801: depend on GPIOLIB
From: Daniel Thompson @ 2025-10-28 12:52 UTC (permalink / raw)
To: Daniel Thompson
Cc: duje.mihanovic, Lee Jones, Jingoo Han, Helge Deller, Randy Dunlap,
dri-devel, linux-fbdev, linux-kernel
In-Reply-To: <aQC1iJlm2jS479_0@aspen.lan>
On Tue, Oct 28, 2025 at 12:22:37PM +0000, Daniel Thompson wrote:
> On Fri, Apr 11, 2025 at 07:22:18PM +0200, Duje Mihanović via B4 Relay wrote:
> > From: Duje Mihanović <duje.mihanovic@skole.hr>
> >
> > The ExpressWire library used by the driver depends on GPIOLIB, and by
> > extension the driver does as well. This is not reflected in the driver's
> > Kconfig entry, potentially causing Kconfig warnings. Fix this by adding
> > the dependency.
> >
> > Link: https://lore.kernel.org/all/5cf231e1-0bba-4595-9441-46acc5255512@infradead.org
> > Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
>
> Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Ignore this... I opened up my mailbox with the sort order reversed!
Daniel.
^ permalink raw reply
* Re: [PATCH v2] backlight: qcom-wled: fix unbalanced ovp irq enable
From: Daniel Thompson @ 2025-10-28 12:40 UTC (permalink / raw)
To: Konrad Dybcio
Cc: foss, Lee Jones, Jingoo Han, Helge Deller, linux-arm-msm,
dri-devel, linux-fbdev, linux-kernel
In-Reply-To: <280f1e92-36a1-450b-b6df-b36c3aed3c1c@oss.qualcomm.com>
On Wed, Oct 22, 2025 at 07:14:32PM +0200, Konrad Dybcio wrote:
> On 10/21/25 8:53 PM, Joel Selvaraj via B4 Relay wrote:
> > From: Joel Selvaraj <foss@joelselvaraj.com>
> >
> > In Xiaomi Poco F1 and at least few other devices, the qcom wled driver
> > triggers unbalanced ovp irq enable warning like the following during
> > boot up.
> >
> > [ 1.151677] ------------[ cut here ]------------
> > [ 1.151680] Unbalanced enable for IRQ 176
> > [ 1.151693] WARNING: CPU: 0 PID: 160 at kernel/irq/manage.c:774 __enable_irq+0x50/0x80
> > [ 1.151710] Modules linked in:
> > [ 1.151717] CPU: 0 PID: 160 Comm: kworker/0:11 Not tainted 5.17.0-sdm845 #4
> > [ 1.151724] Hardware name: Xiaomi Pocophone F1 (DT)
> > [ 1.151728] Workqueue: events wled_ovp_work
> > ...<snip>...
> > [ 1.151833] Call trace:
> > [ 1.151836] __enable_irq+0x50/0x80
> > [ 1.151841] enable_irq+0x48/0xa0
> > [ 1.151846] wled_ovp_work+0x18/0x24
> > [ 1.151850] process_one_work+0x1d0/0x350
> > [ 1.151858] worker_thread+0x13c/0x460
> > [ 1.151862] kthread+0x110/0x114
> > [ 1.151868] ret_from_fork+0x10/0x20
> > [ 1.151876] ---[ end trace 0000000000000000 ]---
> >
> > Fix it by storing and checking the state of ovp irq before enabling and
> > disabling it.
> >
> > Signed-off-by: Joel Selvaraj <foss@joelselvaraj.com>
> > ---
> > I was able to debug the issue a little further. This happens mainly because
> > devm_request_threaded_irq already enables the ovp irq during probe. Then ovp
> > work gets scheduled when update_status happens and in turn enables the irq again.
> > Tracking the status makes it easy to avoid the double irq enable. But I am
> > open to try a different approach if there is any suggestion.
>
> Would reverting this change and adding (| IRQF_NO_AUTOEN) to that call
> fix it?
I'd definitely favour trying an alternative approach.
wled_[disable|enable]_ovp_irq() do hide "unbalanced enable/disable"
warnings but they will not nest correctly. That put things are high risk
of bugs (even if there are no bugs now it makes maintaining this driver
"high risk" in the future).
Daniel.
^ permalink raw reply
* Re: [PATCH] backlight: ktd2801: depend on GPIOLIB
From: Daniel Thompson @ 2025-10-28 12:22 UTC (permalink / raw)
To: duje.mihanovic
Cc: Lee Jones, Jingoo Han, Helge Deller, Randy Dunlap, dri-devel,
linux-fbdev, linux-kernel
In-Reply-To: <20250411-ktd-fix-v1-1-e7425d273268@skole.hr>
On Fri, Apr 11, 2025 at 07:22:18PM +0200, Duje Mihanović via B4 Relay wrote:
> From: Duje Mihanović <duje.mihanovic@skole.hr>
>
> The ExpressWire library used by the driver depends on GPIOLIB, and by
> extension the driver does as well. This is not reflected in the driver's
> Kconfig entry, potentially causing Kconfig warnings. Fix this by adding
> the dependency.
>
> Link: https://lore.kernel.org/all/5cf231e1-0bba-4595-9441-46acc5255512@infradead.org
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Daniel.
^ permalink raw reply
* Re: [PATCH] fbdev: vga16fb: Request memory region.
From: Helge Deller @ 2025-10-27 20:07 UTC (permalink / raw)
To: Javier Garcia; +Cc: linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <CABPJ0vgtpjh2q605TifawiS36qAS+OO_dAnYVGsqd03GSXZp+g@mail.gmail.com>
On 10/27/25 19:02, Javier Garcia wrote:
> Hi,
>
> Helge Deller, any comment on this patch?
Well, it doesn't hurt, as it will not abort (as before) even if the region is occupied.
In that sense, I don't see a problem to add it.
But you are missing to release the memory when the driver is released
via vga16fb_remove.
If you send an updated/fixed patch I will apply it to the fbdev tree
to get some testing.
Helge
> ---
> Javier Garcia
>
>
> On Thu, 16 Oct 2025 at 19:18, Javier Garcia <rampxxxx@gmail.com> wrote:
>>
>> This patch reserve and release VGA memory region with `*_mem_region`
>> fn's.
>>
>> This align with Documentation/drm/todo.rst
>> "Request memory regions in all fbdev drivers"
>>
>> I've tested with kernel and qemu both 32bits.
>>
>> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
>> ---
>>
>> When I've run the code always return -EBUSY which makes sense as
>> mem is already requested,`/proc/iomem` shows `000a0000-000bffff : Video RAM area`.
>>
>> I've seen that `cirrusfb` has this kind of code wrapped up with `#if 0`, and I
>> wonder if it makes sense to also wrap up with `#if 0`, at least , in
>> that way the code gets commented about expected behavior.
>>
>>
>> drivers/video/fbdev/vga16fb.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
>> index eedab14c7d51..f506bf144a97 100644
>> --- a/drivers/video/fbdev/vga16fb.c
>> +++ b/drivers/video/fbdev/vga16fb.c
>> @@ -1319,6 +1319,11 @@ static int vga16fb_probe(struct platform_device *dev)
>> if (ret)
>> return ret;
>>
>> + if (!request_mem_region(vga16fb_fix.smem_start, vga16fb_fix.smem_len,
>> + "vga16b")) {
>> + dev_err(&dev->dev,"vga16b: cannot reserve video memory at 0x%lx\n",
>> + vga16fb_fix.smem_start);
>> + }
>> printk(KERN_DEBUG "vga16fb: initializing\n");
>> info = framebuffer_alloc(sizeof(struct vga16fb_par), &dev->dev);
>>
>> @@ -1398,6 +1403,8 @@ static int vga16fb_probe(struct platform_device *dev)
>> err_ioremap:
>> framebuffer_release(info);
>> err_fb_alloc:
>> + release_mem_region(vga16fb_fix.smem_start,
>> + vga16fb_fix.smem_len);
>> return ret;
>> }
>>
>> --
>> 2.50.1
>>
^ permalink raw reply
* Re: [PATCH 2/2] backlight: aw99706: Add support for Awinic AW99706 backlight
From: kernel test robot @ 2025-10-27 18:59 UTC (permalink / raw)
To: Junjie Cao, Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: oe-kbuild-all, dri-devel, linux-leds, devicetree, linux-kernel,
linux-fbdev, Junjie Cao, Pengyu Luo
In-Reply-To: <20251026123923.1531727-3-caojunjie650@gmail.com>
Hi Junjie,
kernel test robot noticed the following build warnings:
[auto build test WARNING on lee-backlight/for-backlight-next]
[also build test WARNING on lee-leds/for-leds-next linus/master v6.18-rc3 next-20251027]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Junjie-Cao/backlight-aw99706-Add-support-for-Awinic-AW99706-backlight/20251026-214135
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git for-backlight-next
patch link: https://lore.kernel.org/r/20251026123923.1531727-3-caojunjie650%40gmail.com
patch subject: [PATCH 2/2] backlight: aw99706: Add support for Awinic AW99706 backlight
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20251028/202510280208.NhQyE0y1-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251028/202510280208.NhQyE0y1-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510280208.NhQyE0y1-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/fortify-string.h:5,
from include/linux/string.h:382,
from arch/powerpc/include/asm/paca.h:16,
from arch/powerpc/include/asm/current.h:13,
from include/linux/sched.h:12,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/backlight.h:12,
from drivers/video/backlight/aw99706.c:12:
drivers/video/backlight/aw99706.c: In function 'aw99706_bl_enable':
>> include/linux/bitfield.h:172:27: warning: 'val' is used uninitialized [-Wuninitialized]
172 | *(_reg_p) &= ~(_mask); \
| ^~
drivers/video/backlight/aw99706.c:325:9: note: in expansion of macro 'FIELD_MODIFY'
325 | FIELD_MODIFY(AW99706_BACKLIGHT_EN_MASK, &val, en);
| ^~~~~~~~~~~~
drivers/video/backlight/aw99706.c:323:12: note: 'val' was declared here
323 | u8 val;
| ^~~
vim +/val +172 include/linux/bitfield.h
e2192de59e457a Johannes Berg 2023-01-18 120
e2192de59e457a Johannes Berg 2023-01-18 121 /**
e2192de59e457a Johannes Berg 2023-01-18 122 * FIELD_PREP_CONST() - prepare a constant bitfield element
e2192de59e457a Johannes Berg 2023-01-18 123 * @_mask: shifted mask defining the field's length and position
e2192de59e457a Johannes Berg 2023-01-18 124 * @_val: value to put in the field
e2192de59e457a Johannes Berg 2023-01-18 125 *
e2192de59e457a Johannes Berg 2023-01-18 126 * FIELD_PREP_CONST() masks and shifts up the value. The result should
e2192de59e457a Johannes Berg 2023-01-18 127 * be combined with other fields of the bitfield using logical OR.
e2192de59e457a Johannes Berg 2023-01-18 128 *
e2192de59e457a Johannes Berg 2023-01-18 129 * Unlike FIELD_PREP() this is a constant expression and can therefore
e2192de59e457a Johannes Berg 2023-01-18 130 * be used in initializers. Error checking is less comfortable for this
e2192de59e457a Johannes Berg 2023-01-18 131 * version, and non-constant masks cannot be used.
e2192de59e457a Johannes Berg 2023-01-18 132 */
e2192de59e457a Johannes Berg 2023-01-18 133 #define FIELD_PREP_CONST(_mask, _val) \
e2192de59e457a Johannes Berg 2023-01-18 134 ( \
e2192de59e457a Johannes Berg 2023-01-18 135 /* mask must be non-zero */ \
e2192de59e457a Johannes Berg 2023-01-18 136 BUILD_BUG_ON_ZERO((_mask) == 0) + \
e2192de59e457a Johannes Berg 2023-01-18 137 /* check if value fits */ \
e2192de59e457a Johannes Berg 2023-01-18 138 BUILD_BUG_ON_ZERO(~((_mask) >> __bf_shf(_mask)) & (_val)) + \
e2192de59e457a Johannes Berg 2023-01-18 139 /* check if mask is contiguous */ \
e2192de59e457a Johannes Berg 2023-01-18 140 __BF_CHECK_POW2((_mask) + (1ULL << __bf_shf(_mask))) + \
e2192de59e457a Johannes Berg 2023-01-18 141 /* and create the value */ \
e2192de59e457a Johannes Berg 2023-01-18 142 (((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask)) \
e2192de59e457a Johannes Berg 2023-01-18 143 )
e2192de59e457a Johannes Berg 2023-01-18 144
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 145 /**
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 146 * FIELD_GET() - extract a bitfield element
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 147 * @_mask: shifted mask defining the field's length and position
7240767450d6d8 Masahiro Yamada 2017-10-03 148 * @_reg: value of entire bitfield
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 149 *
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 150 * FIELD_GET() extracts the field specified by @_mask from the
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 151 * bitfield passed in as @_reg by masking and shifting it down.
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 152 */
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 153 #define FIELD_GET(_mask, _reg) \
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 154 ({ \
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 155 __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 156 (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 157 })
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 158
a256ae22570ee4 Luo Jie 2025-04-17 159 /**
a256ae22570ee4 Luo Jie 2025-04-17 160 * FIELD_MODIFY() - modify a bitfield element
a256ae22570ee4 Luo Jie 2025-04-17 161 * @_mask: shifted mask defining the field's length and position
a256ae22570ee4 Luo Jie 2025-04-17 162 * @_reg_p: pointer to the memory that should be updated
a256ae22570ee4 Luo Jie 2025-04-17 163 * @_val: value to store in the bitfield
a256ae22570ee4 Luo Jie 2025-04-17 164 *
a256ae22570ee4 Luo Jie 2025-04-17 165 * FIELD_MODIFY() modifies the set of bits in @_reg_p specified by @_mask,
a256ae22570ee4 Luo Jie 2025-04-17 166 * by replacing them with the bitfield value passed in as @_val.
a256ae22570ee4 Luo Jie 2025-04-17 167 */
a256ae22570ee4 Luo Jie 2025-04-17 168 #define FIELD_MODIFY(_mask, _reg_p, _val) \
a256ae22570ee4 Luo Jie 2025-04-17 169 ({ \
a256ae22570ee4 Luo Jie 2025-04-17 170 typecheck_pointer(_reg_p); \
a256ae22570ee4 Luo Jie 2025-04-17 171 __BF_FIELD_CHECK(_mask, *(_reg_p), _val, "FIELD_MODIFY: "); \
a256ae22570ee4 Luo Jie 2025-04-17 @172 *(_reg_p) &= ~(_mask); \
a256ae22570ee4 Luo Jie 2025-04-17 173 *(_reg_p) |= (((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask)); \
a256ae22570ee4 Luo Jie 2025-04-17 174 })
a256ae22570ee4 Luo Jie 2025-04-17 175
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] fbdev: vga16fb: Request memory region.
From: Javier Garcia @ 2025-10-27 18:02 UTC (permalink / raw)
To: deller; +Cc: linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <20251016171845.1397153-1-rampxxxx@gmail.com>
Hi,
Helge Deller, any comment on this patch?
---
Javier Garcia
On Thu, 16 Oct 2025 at 19:18, Javier Garcia <rampxxxx@gmail.com> wrote:
>
> This patch reserve and release VGA memory region with `*_mem_region`
> fn's.
>
> This align with Documentation/drm/todo.rst
> "Request memory regions in all fbdev drivers"
>
> I've tested with kernel and qemu both 32bits.
>
> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
> ---
>
> When I've run the code always return -EBUSY which makes sense as
> mem is already requested,`/proc/iomem` shows `000a0000-000bffff : Video RAM area`.
>
> I've seen that `cirrusfb` has this kind of code wrapped up with `#if 0`, and I
> wonder if it makes sense to also wrap up with `#if 0`, at least , in
> that way the code gets commented about expected behavior.
>
>
> drivers/video/fbdev/vga16fb.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
> index eedab14c7d51..f506bf144a97 100644
> --- a/drivers/video/fbdev/vga16fb.c
> +++ b/drivers/video/fbdev/vga16fb.c
> @@ -1319,6 +1319,11 @@ static int vga16fb_probe(struct platform_device *dev)
> if (ret)
> return ret;
>
> + if (!request_mem_region(vga16fb_fix.smem_start, vga16fb_fix.smem_len,
> + "vga16b")) {
> + dev_err(&dev->dev,"vga16b: cannot reserve video memory at 0x%lx\n",
> + vga16fb_fix.smem_start);
> + }
> printk(KERN_DEBUG "vga16fb: initializing\n");
> info = framebuffer_alloc(sizeof(struct vga16fb_par), &dev->dev);
>
> @@ -1398,6 +1403,8 @@ static int vga16fb_probe(struct platform_device *dev)
> err_ioremap:
> framebuffer_release(info);
> err_fb_alloc:
> + release_mem_region(vga16fb_fix.smem_start,
> + vga16fb_fix.smem_len);
> return ret;
> }
>
> --
> 2.50.1
>
^ permalink raw reply
* Re: [PATCH 2/2] backlight: aw99706: Add support for Awinic AW99706 backlight
From: kernel test robot @ 2025-10-27 12:06 UTC (permalink / raw)
To: Junjie Cao, Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: oe-kbuild-all, dri-devel, linux-leds, devicetree, linux-kernel,
linux-fbdev, Junjie Cao, Pengyu Luo
In-Reply-To: <20251026123923.1531727-3-caojunjie650@gmail.com>
Hi Junjie,
kernel test robot noticed the following build warnings:
[auto build test WARNING on lee-backlight/for-backlight-next]
[also build test WARNING on lee-leds/for-leds-next linus/master lee-backlight/for-backlight-fixes v6.18-rc3 next-20251027]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Junjie-Cao/backlight-aw99706-Add-support-for-Awinic-AW99706-backlight/20251026-214135
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git for-backlight-next
patch link: https://lore.kernel.org/r/20251026123923.1531727-3-caojunjie650%40gmail.com
patch subject: [PATCH 2/2] backlight: aw99706: Add support for Awinic AW99706 backlight
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20251027/202510271932.kN86aCge-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251027/202510271932.kN86aCge-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510271932.kN86aCge-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/video/backlight/aw99706.c:468:12: warning: 'aw99706_resume' defined but not used [-Wunused-function]
468 | static int aw99706_resume(struct device *dev)
| ^~~~~~~~~~~~~~
>> drivers/video/backlight/aw99706.c:461:12: warning: 'aw99706_suspend' defined but not used [-Wunused-function]
461 | static int aw99706_suspend(struct device *dev)
| ^~~~~~~~~~~~~~~
vim +/aw99706_resume +468 drivers/video/backlight/aw99706.c
460
> 461 static int aw99706_suspend(struct device *dev)
462 {
463 struct aw99706_device *aw = dev_get_drvdata(dev);
464
465 return aw99706_update_brightness(aw, 0);
466 }
467
> 468 static int aw99706_resume(struct device *dev)
469 {
470 struct aw99706_device *aw = dev_get_drvdata(dev);
471
472 return aw99706_hw_init(aw);
473 }
474
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: leds: backlight: Add Awinic AW99706 backlight
From: Junjie Cao @ 2025-10-27 10:29 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, dri-devel,
linux-leds, devicetree, linux-kernel, linux-fbdev
In-Reply-To: <c32970a8-c1d1-4130-839b-981bca5373f3@kernel.org>
On Mon, Oct 27, 2025 at 4:38 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 27/10/2025 07:58, Junjie Cao wrote:
> > On Sun, Oct 26, 2025 at 9:48 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 26/10/2025 13:39, Junjie Cao wrote:
> >>> +
> >>> + reg:
> >>> + maxItems: 1
> >>> +
> >>> + enable-gpios:
> >>> + description: GPIO to use to enable/disable the backlight (HWEN pin).
> >>> + maxItems: 1
> >>> +
> >>> + awinic,dim-mode:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: >
> >>> + Select dimming mode of the device.
> >>> + 0 = Bypass mode.
> >>> + 1 = DC mode.
> >>> + 2 = MIX mode.
> >>> + 3 = MIX-26k.
> >>> + enum: [0, 1, 2, 3]
> >>> + default: 1
> >>> +
> >>> + awinic,sw-freq:
> >>
> >> Please use proper units, see:
> >> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> >> and other examples
> >>
> >> Same everywhere else.
> >>
> >
> > ACK
> >
> >>
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: Boost switching frequency in kHz.
> >>> + enum: [300, 400, 500, 600, 660, 750, 850, 1000, 1200, 1330, 1500, 1700]
> >>> + default: 750
> >>> +
> >>> + awinic,sw-ilmt:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: Switching current limitation in mA.
> >>> + enum: [1500, 2000, 2500, 3000]
> >>> + default: 3000
> >>> +
> >>> + awinic,iled-max:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: Maximum LED current setting in uA.
> >>> + minimum: 5000
> >>> + maximum: 50000
> >>> + multipleOf: 500
> >>> + default: 20000
> >>> +
> >>> + awinic,uvlo-thres:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: UVLO(Under Voltage Lock Out) in mV.
> >>> + enum: [2200, 5000]
> >>> + default: 2200
> >>> +
> >>> + awinic,fade-time:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: Fade In/Out Time(per step) in us.
> >>> + enum: [8, 16, 32, 64, 128, 256, 512, 1024]
> >>
> >> Why would this be fixed setting? This really looks like runtime, drop.
> >>
> >
> > Yes, it is fixed. I am quoting this from the datasheet.
>
> Fixed per board.
>
>
> > AW99706B provides Fade in/out mode to transform backlight from one brightness
> > to another or turn on/off backlight with a fixed slope. Writing 0b00 into
> > RAMP_CTR (CFG 0x06) to enter Fade in/out mode, and the the slope of current
> > transition can be set in FADE_TIME (CFG 0x06).
> >
> >>> + default: 16
> >>> +
> >>> + awinic,slope-time:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: Slope time in ms.
> >>
> >> Slope of what?
> >>
> >
> > Ramp time in slope mode, it is retained from downstream drivers, it will
> > be more clear in the next version.
> >
> >>> + enum: [8, 24, 48, 96, 200, 300, 400, 500]
> >>> + default: 300
> >>> +
> >>> + awinic,ramp-ctl:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: >
> >>> + Select ramp control and filter of the device.
> >>> + 0 = Fade in/fade out.
> >>> + 1 = Light filter.
> >>> + 2 = Medium filter.
> >>> + 3 = Heavy filter.
> >>> + enum: [0, 1, 2, 3]
> >>> + default: 2
> >>> +
> >>> + awinic,brt-mode:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: >
> >>> + Select brightness control of the device.
> >>> + 0 = PWM.
> >>> + 1 = IIC.
> >>> + 2 = IIC x PWM.
> >>> + 3 = IIC x PWM(P-ramp).
> >>> + enum: [0, 1, 2, 3]
> >>> + default: 1
> >>> +
> >>> + awinic,onoff-time:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: Turn on/off time(per step) in ns.
> >>> + enum: [250, 500, 1000, 2000, 4000, 8000, 16000]
> >>
> >> Not a DT property.
> >>
> >
> > It is mandatory in the downstream driver, I keep it.
>
> Huh? I don't care about downstream driver. Again, not a DT property. You
> cannot add here runtime properties and when, we tell you that, you just
> ignore our review.
>
> NAK
>
My apologies for the misunderstanding and my poorly worded previous
comment. I absolutely did not intend to ignore your review.
I mentioned the "downstream driver" only to explain why I had originally
included the property.
I now understand your point clearly. I will remove them in the next
version.
Thanks for your fast reviews and for clarifying this principle for me.
>
> >
> > The following is the description about it,
> >
> > If the value in ONOFF_CTR(CFG 0x08 [4:3]) is 0b00, the turning on/off ramp of
> > AW99706B is soft start and fast end. In this mode, the ramp time can be
> > programmed by ONOFF_TIME (CFG 0x08 [2:0]).
> >
> >>> + default: 2000
> >>> +
> >>> +required:
> >>> + - compatible
> >>> + - reg
> >>> + - enable-gpios
> >>> +
> >>> +unevaluatedProperties: false
> >>> +
> >>> +examples:
> >>> + - |
> >>> + #include <dt-bindings/gpio/gpio.h>
> >>> +
> >>> + i2c {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + aw99706@76 {
> >>> + compatible = "awinic,aw99706";
> >>> + reg = <0x76>;
> >>> + enable-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>;
> >>
> >> Where are other properties from common.yaml? Looks like you re-invented
> >> some parts.
> >>
> >
> > Sorry, I forgot it, when writing the bindings, I used ktz8866.yaml as a
> > template. I should have dropped the common.yaml. This driver does
> > not require other properties in common.yaml.
>
>
> I don't care about driver much, but anyway it should use common.yaml.
> Please read the feedback very carefully.
>
ACK
Regards,
Junjie
^ permalink raw reply
* [PATCH] video: valkyriefb: Fix reference count leak in valkyriefb_init
From: Miaoqian Lin @ 2025-10-27 8:43 UTC (permalink / raw)
To: Helge Deller, Paul Mackerras, Benjamin Herrenschmidt, linux-fbdev,
dri-devel, linux-kernel
Cc: linmq006, stable
The of_find_node_by_name() function returns a device tree node with its
reference count incremented. The caller is responsible for calling
of_node_put() to release this reference when done.
Found via static analysis.
Fixes: cc5d0189b9ba ("[PATCH] powerpc: Remove device_node addrs/n_addr")
Cc: stable@vger.kernel.org
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
drivers/video/fbdev/valkyriefb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/fbdev/valkyriefb.c b/drivers/video/fbdev/valkyriefb.c
index 91d070ef6989..6ff059ee1694 100644
--- a/drivers/video/fbdev/valkyriefb.c
+++ b/drivers/video/fbdev/valkyriefb.c
@@ -329,11 +329,13 @@ static int __init valkyriefb_init(void)
if (of_address_to_resource(dp, 0, &r)) {
printk(KERN_ERR "can't find address for valkyrie\n");
+ of_node_put(dp);
return 0;
}
frame_buffer_phys = r.start;
cmap_regs_phys = r.start + 0x304000;
+ of_node_put(dp);
}
#endif /* ppc (!CONFIG_MAC) */
--
2.39.5 (Apple Git-154)
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: leds: backlight: Add Awinic AW99706 backlight
From: Krzysztof Kozlowski @ 2025-10-27 8:38 UTC (permalink / raw)
To: Junjie Cao
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, dri-devel,
linux-leds, devicetree, linux-kernel, linux-fbdev
In-Reply-To: <CAK6c68gqHMR-FpH3MY9E_9R+V0J75V9zOii=x81e+bRcnBYOig@mail.gmail.com>
On 27/10/2025 07:58, Junjie Cao wrote:
> On Sun, Oct 26, 2025 at 9:48 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 26/10/2025 13:39, Junjie Cao wrote:
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + enable-gpios:
>>> + description: GPIO to use to enable/disable the backlight (HWEN pin).
>>> + maxItems: 1
>>> +
>>> + awinic,dim-mode:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: >
>>> + Select dimming mode of the device.
>>> + 0 = Bypass mode.
>>> + 1 = DC mode.
>>> + 2 = MIX mode.
>>> + 3 = MIX-26k.
>>> + enum: [0, 1, 2, 3]
>>> + default: 1
>>> +
>>> + awinic,sw-freq:
>>
>> Please use proper units, see:
>> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
>> and other examples
>>
>> Same everywhere else.
>>
>
> ACK
>
>>
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: Boost switching frequency in kHz.
>>> + enum: [300, 400, 500, 600, 660, 750, 850, 1000, 1200, 1330, 1500, 1700]
>>> + default: 750
>>> +
>>> + awinic,sw-ilmt:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: Switching current limitation in mA.
>>> + enum: [1500, 2000, 2500, 3000]
>>> + default: 3000
>>> +
>>> + awinic,iled-max:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: Maximum LED current setting in uA.
>>> + minimum: 5000
>>> + maximum: 50000
>>> + multipleOf: 500
>>> + default: 20000
>>> +
>>> + awinic,uvlo-thres:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: UVLO(Under Voltage Lock Out) in mV.
>>> + enum: [2200, 5000]
>>> + default: 2200
>>> +
>>> + awinic,fade-time:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: Fade In/Out Time(per step) in us.
>>> + enum: [8, 16, 32, 64, 128, 256, 512, 1024]
>>
>> Why would this be fixed setting? This really looks like runtime, drop.
>>
>
> Yes, it is fixed. I am quoting this from the datasheet.
Fixed per board.
> AW99706B provides Fade in/out mode to transform backlight from one brightness
> to another or turn on/off backlight with a fixed slope. Writing 0b00 into
> RAMP_CTR (CFG 0x06) to enter Fade in/out mode, and the the slope of current
> transition can be set in FADE_TIME (CFG 0x06).
>
>>> + default: 16
>>> +
>>> + awinic,slope-time:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: Slope time in ms.
>>
>> Slope of what?
>>
>
> Ramp time in slope mode, it is retained from downstream drivers, it will
> be more clear in the next version.
>
>>> + enum: [8, 24, 48, 96, 200, 300, 400, 500]
>>> + default: 300
>>> +
>>> + awinic,ramp-ctl:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: >
>>> + Select ramp control and filter of the device.
>>> + 0 = Fade in/fade out.
>>> + 1 = Light filter.
>>> + 2 = Medium filter.
>>> + 3 = Heavy filter.
>>> + enum: [0, 1, 2, 3]
>>> + default: 2
>>> +
>>> + awinic,brt-mode:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: >
>>> + Select brightness control of the device.
>>> + 0 = PWM.
>>> + 1 = IIC.
>>> + 2 = IIC x PWM.
>>> + 3 = IIC x PWM(P-ramp).
>>> + enum: [0, 1, 2, 3]
>>> + default: 1
>>> +
>>> + awinic,onoff-time:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: Turn on/off time(per step) in ns.
>>> + enum: [250, 500, 1000, 2000, 4000, 8000, 16000]
>>
>> Not a DT property.
>>
>
> It is mandatory in the downstream driver, I keep it.
Huh? I don't care about downstream driver. Again, not a DT property. You
cannot add here runtime properties and when, we tell you that, you just
ignore our review.
NAK
>
> The following is the description about it,
>
> If the value in ONOFF_CTR(CFG 0x08 [4:3]) is 0b00, the turning on/off ramp of
> AW99706B is soft start and fast end. In this mode, the ramp time can be
> programmed by ONOFF_TIME (CFG 0x08 [2:0]).
>
>>> + default: 2000
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - enable-gpios
>>> +
>>> +unevaluatedProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/gpio/gpio.h>
>>> +
>>> + i2c {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + aw99706@76 {
>>> + compatible = "awinic,aw99706";
>>> + reg = <0x76>;
>>> + enable-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>;
>>
>> Where are other properties from common.yaml? Looks like you re-invented
>> some parts.
>>
>
> Sorry, I forgot it, when writing the bindings, I used ktz8866.yaml as a
> template. I should have dropped the common.yaml. This driver does
> not require other properties in common.yaml.
I don't care about driver much, but anyway it should use common.yaml.
Please read the feedback very carefully.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: make g_fbmode0 an array of const pointers
From: Dan Carpenter @ 2025-10-27 8:15 UTC (permalink / raw)
To: Cristian Del Gobbo
Cc: sudip.mukherjee, teddy.wang, gregkh, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20251026233432.1707-1-cristiandelgobbo87@gmail.com>
On Mon, Oct 27, 2025 at 12:34:32AM +0100, Cristian Del Gobbo wrote:
> Change g_fbmode0 from 'static const char *' to 'static const char * const'
> so that both the array and its elements are const. This addresses a
> checkpatch warning and matches intended usage.
>
> No functional change intended.
>
> Signed-off-by: Cristian Del Gobbo <cristiandelgobbo87@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 3659af7e519d..ceb89ee99ce0 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -33,7 +33,7 @@
> static int g_hwcursor = 1;
> static int g_noaccel;
> static int g_nomtrr;
> -static const char *g_fbmode[] = {NULL, NULL};
> +static const char * const g_fbmode[] = {NULL, NULL};
This breaks the build. Please, at least try compiling your patches.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: leds: backlight: Add Awinic AW99706 backlight
From: Junjie Cao @ 2025-10-27 6:58 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, dri-devel,
linux-leds, devicetree, linux-kernel, linux-fbdev
In-Reply-To: <c17c10d4-cc1f-46fd-8719-e7bb9ffa91ba@kernel.org>
On Sun, Oct 26, 2025 at 9:48 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 26/10/2025 13:39, Junjie Cao wrote:
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + enable-gpios:
> > + description: GPIO to use to enable/disable the backlight (HWEN pin).
> > + maxItems: 1
> > +
> > + awinic,dim-mode:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: >
> > + Select dimming mode of the device.
> > + 0 = Bypass mode.
> > + 1 = DC mode.
> > + 2 = MIX mode.
> > + 3 = MIX-26k.
> > + enum: [0, 1, 2, 3]
> > + default: 1
> > +
> > + awinic,sw-freq:
>
> Please use proper units, see:
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> and other examples
>
> Same everywhere else.
>
ACK
>
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Boost switching frequency in kHz.
> > + enum: [300, 400, 500, 600, 660, 750, 850, 1000, 1200, 1330, 1500, 1700]
> > + default: 750
> > +
> > + awinic,sw-ilmt:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Switching current limitation in mA.
> > + enum: [1500, 2000, 2500, 3000]
> > + default: 3000
> > +
> > + awinic,iled-max:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Maximum LED current setting in uA.
> > + minimum: 5000
> > + maximum: 50000
> > + multipleOf: 500
> > + default: 20000
> > +
> > + awinic,uvlo-thres:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: UVLO(Under Voltage Lock Out) in mV.
> > + enum: [2200, 5000]
> > + default: 2200
> > +
> > + awinic,fade-time:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Fade In/Out Time(per step) in us.
> > + enum: [8, 16, 32, 64, 128, 256, 512, 1024]
>
> Why would this be fixed setting? This really looks like runtime, drop.
>
Yes, it is fixed. I am quoting this from the datasheet.
AW99706B provides Fade in/out mode to transform backlight from one brightness
to another or turn on/off backlight with a fixed slope. Writing 0b00 into
RAMP_CTR (CFG 0x06) to enter Fade in/out mode, and the the slope of current
transition can be set in FADE_TIME (CFG 0x06).
> > + default: 16
> > +
> > + awinic,slope-time:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Slope time in ms.
>
> Slope of what?
>
Ramp time in slope mode, it is retained from downstream drivers, it will
be more clear in the next version.
> > + enum: [8, 24, 48, 96, 200, 300, 400, 500]
> > + default: 300
> > +
> > + awinic,ramp-ctl:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: >
> > + Select ramp control and filter of the device.
> > + 0 = Fade in/fade out.
> > + 1 = Light filter.
> > + 2 = Medium filter.
> > + 3 = Heavy filter.
> > + enum: [0, 1, 2, 3]
> > + default: 2
> > +
> > + awinic,brt-mode:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: >
> > + Select brightness control of the device.
> > + 0 = PWM.
> > + 1 = IIC.
> > + 2 = IIC x PWM.
> > + 3 = IIC x PWM(P-ramp).
> > + enum: [0, 1, 2, 3]
> > + default: 1
> > +
> > + awinic,onoff-time:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Turn on/off time(per step) in ns.
> > + enum: [250, 500, 1000, 2000, 4000, 8000, 16000]
>
> Not a DT property.
>
It is mandatory in the downstream driver, I keep it.
The following is the description about it,
If the value in ONOFF_CTR(CFG 0x08 [4:3]) is 0b00, the turning on/off ramp of
AW99706B is soft start and fast end. In this mode, the ramp time can be
programmed by ONOFF_TIME (CFG 0x08 [2:0]).
> > + default: 2000
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - enable-gpios
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > +
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + aw99706@76 {
> > + compatible = "awinic,aw99706";
> > + reg = <0x76>;
> > + enable-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>;
>
> Where are other properties from common.yaml? Looks like you re-invented
> some parts.
>
Sorry, I forgot it, when writing the bindings, I used ktz8866.yaml as a
template. I should have dropped the common.yaml. This driver does
not require other properties in common.yaml.
Regards,
Junjie
^ 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