* Re: [PATCH v2 3/4] fbdev: au1100fb: Use %zu to printk a value of type size_t
From: Uwe Kleine-König @ 2026-02-07 21:32 UTC (permalink / raw)
To: Helge Deller; +Cc: Chen Ni, linux-fbdev, dri-devel
In-Reply-To: <615c1263-fbdc-40bd-aff6-b1411af012fe@gmx.de>
[-- Attachment #1: Type: text/plain, Size: 1704 bytes --]
On Sat, Feb 07, 2026 at 08:06:18PM +0100, Helge Deller wrote:
> On 2/7/26 18:12, Uwe Kleine-König wrote:
> > %zu is the dedicated type for size_t. %d only works on 32bit
> > architectures where size_t is typedef'd to be unsigned int. (And then
> > the signedness doesn't fit, but `gcc -Wformat` doesn't stumble over this.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> > ---
> > drivers/video/fbdev/au1100fb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
> > index beba8befaec9..a0e1aceaf9a6 100644
> > --- a/drivers/video/fbdev/au1100fb.c
> > +++ b/drivers/video/fbdev/au1100fb.c
> > @@ -456,7 +456,7 @@ static int au1100fb_drv_probe(struct platform_device *dev)
> > PAGE_ALIGN(fbdev->fb_len),
> > &fbdev->fb_phys, GFP_KERNEL);
> > if (!fbdev->fb_mem) {
> > - print_err("fail to allocate framebuffer (size: %dK))",
> > + print_err("fail to allocate framebuffer (size: %zuK))",
> > fbdev->fb_len / 1024);
> > return -ENOMEM;
> > }
>
> I think there is a second hunk missing?
>
> @@ -470,7 +470,7 @@ static int au1100fb_drv_probe(struct platform_device *dev)
> fbdev->info.fix.smem_len = fbdev->fb_len;
>
> print_dbg("Framebuffer memory map at %p", fbdev->fb_mem);
> - print_dbg("phys=0x%08x, size=%dK", fbdev->fb_phys, fbdev->fb_len / 1024);
> + print_dbg("phys=0x%08x, size=%zuK", fbdev->fb_phys, fbdev->fb_len / 1024);
Ah, the compiler didn't stumble about this one because print_dbg is a
stub without DEBUG defined.
I will prepare a v3.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 3/4] fbdev: au1100fb: Use %zu to printk a value of type size_t
From: Helge Deller @ 2026-02-07 19:06 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Chen Ni, linux-fbdev, dri-devel
In-Reply-To: <444f0cd1a39cc665a9e2d76454138c3e71c7747f.1770483674.git.u.kleine-koenig@baylibre.com>
On 2/7/26 18:12, Uwe Kleine-König wrote:
> %zu is the dedicated type for size_t. %d only works on 32bit
> architectures where size_t is typedef'd to be unsigned int. (And then
> the signedness doesn't fit, but `gcc -Wformat` doesn't stumble over this.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> drivers/video/fbdev/au1100fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
> index beba8befaec9..a0e1aceaf9a6 100644
> --- a/drivers/video/fbdev/au1100fb.c
> +++ b/drivers/video/fbdev/au1100fb.c
> @@ -456,7 +456,7 @@ static int au1100fb_drv_probe(struct platform_device *dev)
> PAGE_ALIGN(fbdev->fb_len),
> &fbdev->fb_phys, GFP_KERNEL);
> if (!fbdev->fb_mem) {
> - print_err("fail to allocate framebuffer (size: %dK))",
> + print_err("fail to allocate framebuffer (size: %zuK))",
> fbdev->fb_len / 1024);
> return -ENOMEM;
> }
I think there is a second hunk missing?
@@ -470,7 +470,7 @@ static int au1100fb_drv_probe(struct platform_device *dev)
fbdev->info.fix.smem_len = fbdev->fb_len;
print_dbg("Framebuffer memory map at %p", fbdev->fb_mem);
- print_dbg("phys=0x%08x, size=%dK", fbdev->fb_phys, fbdev->fb_len / 1024);
+ print_dbg("phys=0x%08x, size=%zuK", fbdev->fb_phys, fbdev->fb_len / 1024);
Helge
^ permalink raw reply
* Re: [PATCH v2 1/4] fbdev: au1100fb: Don't store device specific data in global variables
From: Helge Deller @ 2026-02-07 19:04 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Chen Ni, linux-fbdev, dri-devel
In-Reply-To: <0e1ecdde447dc32816f1f066812ec8e84b4aad0a.1770483674.git.u.kleine-koenig@baylibre.com>
Hi Uwe,
On 2/7/26 18:12, Uwe Kleine-König wrote:
> Using global data to store device specific data is a bad pattern that
> breaks if there is more than one device. So expand driver data and drop
> the global variables.
>
> While there is probably no machine that has two or more au1100fb
> devices, this makes the driver a better template for new drivers and
> saves some memory if there is no such bound device.
>
> bloat-o-meter reports (for ARCH=arm allmodconfig + CONFIG_FB_AU1100=y
> and ignoring the rename of the init function):
>
> add/remove: 1/4 grow/shrink: 2/2 up/down: 1360/-4800 (-3440)
> Function old new delta
> au1100fb_drv_probe 2648 3328 +680
> $a 12808 13484 +676
> au1100fb_drv_resume 404 400 -4
> au1100fb_fix 68 - -68
> au1100fb_var 160 - -160
> fbregs 2048 - -2048
> $d 9525 7009 -2516
> Total: Before=38664, After=35224, chg -8.90%
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> I think this doesn't need a Fixes line, but if you want, it would be:
>
> Fixes: 3b495f2bb749 ("Au1100 FB driver uplift for 2.6.")
> Fixes: f77f50ca1a23 ("[PATCH] au1100fb: add power management support")
> ---
> drivers/video/fbdev/au1100fb.c | 63 ++++++++++++++++------------------
> drivers/video/fbdev/au1100fb.h | 5 +++
> 2 files changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
> index feaa1061c436..75344ee080f3 100644
> --- a/drivers/video/fbdev/au1100fb.c
> +++ b/drivers/video/fbdev/au1100fb.c
> @@ -84,21 +84,6 @@ struct fb_bitfield rgb_bitfields[][4] =
> { { 8, 4, 0 }, { 4, 4, 0 }, { 0, 4, 0 }, { 0, 0, 0 } },
> };
>
> -static struct fb_fix_screeninfo au1100fb_fix = {
> - .id = "AU1100 FB",
> - .xpanstep = 1,
> - .ypanstep = 1,
> - .type = FB_TYPE_PACKED_PIXELS,
> - .accel = FB_ACCEL_NONE,
> -};
> -
> -static struct fb_var_screeninfo au1100fb_var = {
> - .activate = FB_ACTIVATE_NOW,
> - .height = -1,
> - .width = -1,
> - .vmode = FB_VMODE_NONINTERLACED,
> -};
> -
> /* fb_blank
> * Blank the screen. Depending on the mode, the screen will be
> * activated with the backlight color, or desactivated
> @@ -432,19 +417,26 @@ static int au1100fb_drv_probe(struct platform_device *dev)
> return -EFAULT;
> }
>
> - au1100fb_fix.mmio_start = regs_res->start;
> - au1100fb_fix.mmio_len = resource_size(regs_res);
> + fbdev->info.fix = (struct fb_fix_screeninfo) {
> + .mmio_start = regs_res->start,
> + .mmio_len = resource_size(regs_res),
> + .id = "AU1100 FB",
> + .xpanstep = 1,
> + .ypanstep = 1,
> + .type = FB_TYPE_PACKED_PIXELS,
> + .accel = FB_ACCEL_NONE,
> + };
>
> if (!devm_request_mem_region(&dev->dev,
> - au1100fb_fix.mmio_start,
> - au1100fb_fix.mmio_len,
> + fbdev->info.fix.mmio_start,
> + fbdev->info.fix.mmio_len,
> DRIVER_NAME)) {
> print_err("fail to lock memory region at 0x%08lx",
> - au1100fb_fix.mmio_start);
> + fbdev->info.fix.mmio_start);
> return -EBUSY;
> }
>
> - fbdev->regs = (struct au1100fb_regs*)KSEG1ADDR(au1100fb_fix.mmio_start);
> + fbdev->regs = (struct au1100fb_regs*)KSEG1ADDR(fbdev->info.fix.mmio_start);
>
> print_dbg("Register memory map at %p", fbdev->regs);
> print_dbg("phys=0x%08x, size=%d", fbdev->regs_phys, fbdev->regs_len);
> @@ -469,22 +461,27 @@ static int au1100fb_drv_probe(struct platform_device *dev)
> return -ENOMEM;
> }
>
> - au1100fb_fix.smem_start = fbdev->fb_phys;
> - au1100fb_fix.smem_len = fbdev->fb_len;
> + fbdev->info.fix.smem_start = fbdev->fb_phys;
> + fbdev->info.fix.smem_len = fbdev->fb_len;
>
> print_dbg("Framebuffer memory map at %p", fbdev->fb_mem);
> print_dbg("phys=0x%08x, size=%dK", fbdev->fb_phys, fbdev->fb_len / 1024);
>
> /* load the panel info into the var struct */
> - au1100fb_var.bits_per_pixel = fbdev->panel->bpp;
> - au1100fb_var.xres = fbdev->panel->xres;
> - au1100fb_var.xres_virtual = au1100fb_var.xres;
> - au1100fb_var.yres = fbdev->panel->yres;
> - au1100fb_var.yres_virtual = au1100fb_var.yres;
> + fbdev->info.var = (struct fb_var_screeninfo) {
> + .activate = FB_ACTIVATE_NOW,
> + .height = -1,
> + .width = -1,
> + .vmode = FB_VMODE_NONINTERLACED,
> + .bits_per_pixel = fbdev->panel->bpp,
> + .xres = fbdev->panel->xres,
> + .xres_virtual = fbdev->panel->xres,
> + .yres = fbdev->panel->yres,
> + .yres_virtual = fbdev->panel->yres,
> + };
>
> fbdev->info.screen_base = fbdev->fb_mem;
> fbdev->info.fbops = &au1100fb_ops;
> - fbdev->info.fix = au1100fb_fix;
>
> fbdev->info.pseudo_palette =
> devm_kcalloc(&dev->dev, 16, sizeof(u32), GFP_KERNEL);
> @@ -497,8 +494,6 @@ static int au1100fb_drv_probe(struct platform_device *dev)
> return -EFAULT;
> }
>
> - fbdev->info.var = au1100fb_var;
> -
> /* Set h/w registers */
> au1100fb_setmode(fbdev);
>
> @@ -547,7 +542,7 @@ void au1100fb_drv_remove(struct platform_device *dev)
> #ifdef CONFIG_PM
> static struct au1100fb_regs fbregs;
^ you missed to delete "fbregs" now.
Your previous series deleted it.
> -int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
> +static int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
> {
> struct au1100fb_device *fbdev = platform_get_drvdata(dev);
>
> @@ -559,7 +554,7 @@ int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
>
> clk_disable(fbdev->lcdclk);
>
> - memcpy(&fbregs, fbdev->regs, sizeof(struct au1100fb_regs));
> + memcpy(&fbdev->pm_regs, fbdev->regs, sizeof(struct au1100fb_regs));
Although memcpy() was used before, isn't this:
fbdev->pm_regs = *fbdev->regs;
sufficient and better?
> return 0;
> }
> @@ -572,7 +567,7 @@ int au1100fb_drv_resume(struct platform_device *dev)
> if (!fbdev)
> return 0;
>
> - memcpy(fbdev->regs, &fbregs, sizeof(struct au1100fb_regs));
> + memcpy(fbdev->regs, &fbdev->pm_regs, sizeof(struct au1100fb_regs));
same here.
> ret = clk_enable(fbdev->lcdclk);
> if (ret)
> diff --git a/drivers/video/fbdev/au1100fb.h b/drivers/video/fbdev/au1100fb.h
> index 79f4048726f1..dc53d063fcc3 100644
> --- a/drivers/video/fbdev/au1100fb.h
> +++ b/drivers/video/fbdev/au1100fb.h
> @@ -105,6 +105,11 @@ struct au1100fb_device {
> size_t regs_len;
> unsigned int regs_phys;
>
> +#ifdef CONFIG_PM
> + /* stores the register values during suspend */
> + struct au1100fb_regs pm_regs;
> +#endif
> +
> unsigned char* fb_mem; /* FrameBuffer memory map */
> size_t fb_len;
> dma_addr_t fb_phys;
^ permalink raw reply
* [PATCH v2 4/4] fbdev: au1100fb: Make driver compilable on non-mips platforms
From: Uwe Kleine-König @ 2026-02-07 17:12 UTC (permalink / raw)
To: Helge Deller; +Cc: Chen Ni, linux-fbdev, dri-devel
In-Reply-To: <cover.1770483674.git.u.kleine-koenig@baylibre.com>
The header asm/mach-au1x00/au1000.h is unused apart from pulling in
<linux/delay.h> (for mdelay()) and <linux/io.h> (for KSEG1ADDR()). Then
the only platform specific part in the driver is the usage of the KSEG1ADDR
macro, which for the non-mips case can be stubbed.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
drivers/video/fbdev/Kconfig | 3 ++-
drivers/video/fbdev/au1100fb.c | 12 ++++++++++--
drivers/video/fbdev/au1100fb.h | 2 --
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 45733522ff48..4514c42db9fa 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1345,7 +1345,8 @@ endchoice
config FB_AU1100
bool "Au1100 LCD Driver"
- depends on (FB = y) && MIPS_ALCHEMY
+ depends on FB
+ depends on MIPS_ALCHEMY || COMPILE_TEST
select FB_IOMEM_HELPERS
help
This is the framebuffer driver for the AMD Au1100 SOC. It can drive
diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index a0e1aceaf9a6..758002cb5ea8 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -42,6 +42,8 @@
* 675 Mass Ave, Cambridge, MA 02139, USA.
*/
#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/errno.h>
@@ -55,12 +57,15 @@
#include <linux/platform_device.h>
#include <linux/slab.h>
-#include <asm/mach-au1x00/au1000.h>
-
#define DEBUG 0
#include "au1100fb.h"
+#if defined(CONFIG_COMPILE_TEST) && !defined(CONFIG_MIPS)
+/* This is only defined to be able to compile this driver on non-mips platforms */
+#define KSEG1ADDR(x) (x)
+#endif
+
#define DRIVER_NAME "au1100fb"
#define DRIVER_DESC "LCD controller driver for AU1100 processors"
@@ -331,7 +336,10 @@ static int au1100fb_fb_mmap(struct fb_info *fbi, struct vm_area_struct *vma)
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+#ifndef CONFIG_S390
+ /* On s390 pgprot_val() is a function and thus not a lvalue */
pgprot_val(vma->vm_page_prot) |= (6 << 9); //CCA=6
+#endif
return dma_mmap_coherent(fbdev->dev, vma, fbdev->fb_mem, fbdev->fb_phys,
fbdev->fb_len);
diff --git a/drivers/video/fbdev/au1100fb.h b/drivers/video/fbdev/au1100fb.h
index dc53d063fcc3..998328cd16a2 100644
--- a/drivers/video/fbdev/au1100fb.h
+++ b/drivers/video/fbdev/au1100fb.h
@@ -30,8 +30,6 @@
#ifndef _AU1100LCD_H
#define _AU1100LCD_H
-#include <asm/mach-au1x00/au1000.h>
-
#define print_err(f, arg...) printk(KERN_ERR DRIVER_NAME ": " f "\n", ## arg)
#define print_warn(f, arg...) printk(KERN_WARNING DRIVER_NAME ": " f "\n", ## arg)
#define print_info(f, arg...) printk(KERN_INFO DRIVER_NAME ": " f "\n", ## arg)
--
2.47.3
^ permalink raw reply related
* [PATCH v2 3/4] fbdev: au1100fb: Use %zu to printk a value of type size_t
From: Uwe Kleine-König @ 2026-02-07 17:12 UTC (permalink / raw)
To: Helge Deller; +Cc: Chen Ni, linux-fbdev, dri-devel
In-Reply-To: <cover.1770483674.git.u.kleine-koenig@baylibre.com>
%zu is the dedicated type for size_t. %d only works on 32bit
architectures where size_t is typedef'd to be unsigned int. (And then
the signedness doesn't fit, but `gcc -Wformat` doesn't stumble over this.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
drivers/video/fbdev/au1100fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index beba8befaec9..a0e1aceaf9a6 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -456,7 +456,7 @@ static int au1100fb_drv_probe(struct platform_device *dev)
PAGE_ALIGN(fbdev->fb_len),
&fbdev->fb_phys, GFP_KERNEL);
if (!fbdev->fb_mem) {
- print_err("fail to allocate framebuffer (size: %dK))",
+ print_err("fail to allocate framebuffer (size: %zuK))",
fbdev->fb_len / 1024);
return -ENOMEM;
}
--
2.47.3
^ permalink raw reply related
* [PATCH v2 2/4] fbdev: au1100fb: Mark several local functions as static
From: Uwe Kleine-König @ 2026-02-07 17:12 UTC (permalink / raw)
To: Helge Deller; +Cc: Chen Ni, linux-fbdev, dri-devel
In-Reply-To: <cover.1770483674.git.u.kleine-koenig@baylibre.com>
This fixes several (fatal) compiler warnings à la
drivers/video/fbdev/au1100fb.c:530:6: error: no previous prototype for ‘au1100fb_drv_remove’ [-Werror=missing-prototypes]
523 | void au1100fb_drv_remove(struct platform_device *dev)
| ^~~~~~~~~~~~~~~~~~~
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
drivers/video/fbdev/au1100fb.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index 75344ee080f3..beba8befaec9 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -120,7 +120,7 @@ static int au1100fb_fb_blank(int blank_mode, struct fb_info *fbi)
* Set hardware with var settings. This will enable the controller with a specific
* mode, normally validated with the fb_check_var method
*/
-int au1100fb_setmode(struct au1100fb_device *fbdev)
+static int au1100fb_setmode(struct au1100fb_device *fbdev)
{
struct fb_info *info;
u32 words;
@@ -219,7 +219,7 @@ int au1100fb_setmode(struct au1100fb_device *fbdev)
/* fb_setcolreg
* Set color in LCD palette.
*/
-int au1100fb_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue, unsigned transp, struct fb_info *fbi)
+static int au1100fb_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue, unsigned transp, struct fb_info *fbi)
{
struct au1100fb_device *fbdev;
u32 *palette;
@@ -278,7 +278,7 @@ int au1100fb_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned
/* fb_pan_display
* Pan display in x and/or y as specified
*/
-int au1100fb_fb_pan_display(struct fb_var_screeninfo *var, struct fb_info *fbi)
+static int au1100fb_fb_pan_display(struct fb_var_screeninfo *var, struct fb_info *fbi)
{
struct au1100fb_device *fbdev;
int dy;
@@ -325,7 +325,7 @@ int au1100fb_fb_pan_display(struct fb_var_screeninfo *var, struct fb_info *fbi)
* Map video memory in user space. We don't use the generic fb_mmap method mainly
* to allow the use of the TLB streaming flag (CCA=6)
*/
-int au1100fb_fb_mmap(struct fb_info *fbi, struct vm_area_struct *vma)
+static int au1100fb_fb_mmap(struct fb_info *fbi, struct vm_area_struct *vma)
{
struct au1100fb_device *fbdev = to_au1100fb_device(fbi);
@@ -517,7 +517,7 @@ static int au1100fb_drv_probe(struct platform_device *dev)
return -ENODEV;
}
-void au1100fb_drv_remove(struct platform_device *dev)
+static void au1100fb_drv_remove(struct platform_device *dev)
{
struct au1100fb_device *fbdev = NULL;
@@ -559,7 +559,7 @@ static int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
return 0;
}
-int au1100fb_drv_resume(struct platform_device *dev)
+static int au1100fb_drv_resume(struct platform_device *dev)
{
struct au1100fb_device *fbdev = platform_get_drvdata(dev);
int ret;
--
2.47.3
^ permalink raw reply related
* [PATCH v2 1/4] fbdev: au1100fb: Don't store device specific data in global variables
From: Uwe Kleine-König @ 2026-02-07 17:12 UTC (permalink / raw)
To: Helge Deller; +Cc: Chen Ni, linux-fbdev, dri-devel
In-Reply-To: <cover.1770483674.git.u.kleine-koenig@baylibre.com>
Using global data to store device specific data is a bad pattern that
breaks if there is more than one device. So expand driver data and drop
the global variables.
While there is probably no machine that has two or more au1100fb
devices, this makes the driver a better template for new drivers and
saves some memory if there is no such bound device.
bloat-o-meter reports (for ARCH=arm allmodconfig + CONFIG_FB_AU1100=y
and ignoring the rename of the init function):
add/remove: 1/4 grow/shrink: 2/2 up/down: 1360/-4800 (-3440)
Function old new delta
au1100fb_drv_probe 2648 3328 +680
$a 12808 13484 +676
au1100fb_drv_resume 404 400 -4
au1100fb_fix 68 - -68
au1100fb_var 160 - -160
fbregs 2048 - -2048
$d 9525 7009 -2516
Total: Before=38664, After=35224, chg -8.90%
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
I think this doesn't need a Fixes line, but if you want, it would be:
Fixes: 3b495f2bb749 ("Au1100 FB driver uplift for 2.6.")
Fixes: f77f50ca1a23 ("[PATCH] au1100fb: add power management support")
---
drivers/video/fbdev/au1100fb.c | 63 ++++++++++++++++------------------
drivers/video/fbdev/au1100fb.h | 5 +++
2 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index feaa1061c436..75344ee080f3 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -84,21 +84,6 @@ struct fb_bitfield rgb_bitfields[][4] =
{ { 8, 4, 0 }, { 4, 4, 0 }, { 0, 4, 0 }, { 0, 0, 0 } },
};
-static struct fb_fix_screeninfo au1100fb_fix = {
- .id = "AU1100 FB",
- .xpanstep = 1,
- .ypanstep = 1,
- .type = FB_TYPE_PACKED_PIXELS,
- .accel = FB_ACCEL_NONE,
-};
-
-static struct fb_var_screeninfo au1100fb_var = {
- .activate = FB_ACTIVATE_NOW,
- .height = -1,
- .width = -1,
- .vmode = FB_VMODE_NONINTERLACED,
-};
-
/* fb_blank
* Blank the screen. Depending on the mode, the screen will be
* activated with the backlight color, or desactivated
@@ -432,19 +417,26 @@ static int au1100fb_drv_probe(struct platform_device *dev)
return -EFAULT;
}
- au1100fb_fix.mmio_start = regs_res->start;
- au1100fb_fix.mmio_len = resource_size(regs_res);
+ fbdev->info.fix = (struct fb_fix_screeninfo) {
+ .mmio_start = regs_res->start,
+ .mmio_len = resource_size(regs_res),
+ .id = "AU1100 FB",
+ .xpanstep = 1,
+ .ypanstep = 1,
+ .type = FB_TYPE_PACKED_PIXELS,
+ .accel = FB_ACCEL_NONE,
+ };
if (!devm_request_mem_region(&dev->dev,
- au1100fb_fix.mmio_start,
- au1100fb_fix.mmio_len,
+ fbdev->info.fix.mmio_start,
+ fbdev->info.fix.mmio_len,
DRIVER_NAME)) {
print_err("fail to lock memory region at 0x%08lx",
- au1100fb_fix.mmio_start);
+ fbdev->info.fix.mmio_start);
return -EBUSY;
}
- fbdev->regs = (struct au1100fb_regs*)KSEG1ADDR(au1100fb_fix.mmio_start);
+ fbdev->regs = (struct au1100fb_regs*)KSEG1ADDR(fbdev->info.fix.mmio_start);
print_dbg("Register memory map at %p", fbdev->regs);
print_dbg("phys=0x%08x, size=%d", fbdev->regs_phys, fbdev->regs_len);
@@ -469,22 +461,27 @@ static int au1100fb_drv_probe(struct platform_device *dev)
return -ENOMEM;
}
- au1100fb_fix.smem_start = fbdev->fb_phys;
- au1100fb_fix.smem_len = fbdev->fb_len;
+ fbdev->info.fix.smem_start = fbdev->fb_phys;
+ fbdev->info.fix.smem_len = fbdev->fb_len;
print_dbg("Framebuffer memory map at %p", fbdev->fb_mem);
print_dbg("phys=0x%08x, size=%dK", fbdev->fb_phys, fbdev->fb_len / 1024);
/* load the panel info into the var struct */
- au1100fb_var.bits_per_pixel = fbdev->panel->bpp;
- au1100fb_var.xres = fbdev->panel->xres;
- au1100fb_var.xres_virtual = au1100fb_var.xres;
- au1100fb_var.yres = fbdev->panel->yres;
- au1100fb_var.yres_virtual = au1100fb_var.yres;
+ fbdev->info.var = (struct fb_var_screeninfo) {
+ .activate = FB_ACTIVATE_NOW,
+ .height = -1,
+ .width = -1,
+ .vmode = FB_VMODE_NONINTERLACED,
+ .bits_per_pixel = fbdev->panel->bpp,
+ .xres = fbdev->panel->xres,
+ .xres_virtual = fbdev->panel->xres,
+ .yres = fbdev->panel->yres,
+ .yres_virtual = fbdev->panel->yres,
+ };
fbdev->info.screen_base = fbdev->fb_mem;
fbdev->info.fbops = &au1100fb_ops;
- fbdev->info.fix = au1100fb_fix;
fbdev->info.pseudo_palette =
devm_kcalloc(&dev->dev, 16, sizeof(u32), GFP_KERNEL);
@@ -497,8 +494,6 @@ static int au1100fb_drv_probe(struct platform_device *dev)
return -EFAULT;
}
- fbdev->info.var = au1100fb_var;
-
/* Set h/w registers */
au1100fb_setmode(fbdev);
@@ -547,7 +542,7 @@ void au1100fb_drv_remove(struct platform_device *dev)
#ifdef CONFIG_PM
static struct au1100fb_regs fbregs;
-int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
+static int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
{
struct au1100fb_device *fbdev = platform_get_drvdata(dev);
@@ -559,7 +554,7 @@ int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
clk_disable(fbdev->lcdclk);
- memcpy(&fbregs, fbdev->regs, sizeof(struct au1100fb_regs));
+ memcpy(&fbdev->pm_regs, fbdev->regs, sizeof(struct au1100fb_regs));
return 0;
}
@@ -572,7 +567,7 @@ int au1100fb_drv_resume(struct platform_device *dev)
if (!fbdev)
return 0;
- memcpy(fbdev->regs, &fbregs, sizeof(struct au1100fb_regs));
+ memcpy(fbdev->regs, &fbdev->pm_regs, sizeof(struct au1100fb_regs));
ret = clk_enable(fbdev->lcdclk);
if (ret)
diff --git a/drivers/video/fbdev/au1100fb.h b/drivers/video/fbdev/au1100fb.h
index 79f4048726f1..dc53d063fcc3 100644
--- a/drivers/video/fbdev/au1100fb.h
+++ b/drivers/video/fbdev/au1100fb.h
@@ -105,6 +105,11 @@ struct au1100fb_device {
size_t regs_len;
unsigned int regs_phys;
+#ifdef CONFIG_PM
+ /* stores the register values during suspend */
+ struct au1100fb_regs pm_regs;
+#endif
+
unsigned char* fb_mem; /* FrameBuffer memory map */
size_t fb_len;
dma_addr_t fb_phys;
--
2.47.3
^ permalink raw reply related
* [PATCH v2 0/4] fbdev: au1100fb: support COMPILE_TEST and fix multi-device support
From: Uwe Kleine-König @ 2026-02-07 17:12 UTC (permalink / raw)
To: Helge Deller; +Cc: Chen Ni, linux-fbdev, dri-devel
Hello,
v1 is available at
https://lore.kernel.org/linux-fbdev/cover.1770196161.git.u.kleine-koenig@baylibre.com
.
I reshuffled the series to have the only really relevant fix at the
start. Patch #3 is new in this revision and so is the s390 specific
#ifdef in patch #4. Patches #2 and #3 fix issues that pop up when
compiling the driver for arm and arm64 and s390. The last patch drops
the hard dependency on MIPS_ALCHEMY. For that the above mentioned s390
specfic #ifdef block has to be added (or alternatively s390 excluded
from build testing) because s390 is the only platform where pgprot_val
is not an l-value. The things that happen in that block are maybe very
mips-specific such that maybe this should be an #ifdef CONFIG_MIPS block
instead of an #ifndef CONFIG_S390 block?
Note that in the v1 thread Helge wrote to already having applied the
first two patches, they are not yet in next, so I included them here
again.
Best regards
Uwe
Uwe Kleine-König (4):
fbdev: au1100fb: Don't store device specific data in global variables
fbdev: au1100fb: Mark several local functions as static
fbdev: au1100fb: Use %zu to printk a value of type size_t
fbdev: au1100fb: Make driver compilable on non-mips platforms
drivers/video/fbdev/Kconfig | 3 +-
drivers/video/fbdev/au1100fb.c | 89 ++++++++++++++++++----------------
drivers/video/fbdev/au1100fb.h | 7 ++-
3 files changed, 53 insertions(+), 46 deletions(-)
base-commit: 0636e6205beed850d985276dc56fd73d785bea5c
--
2.47.3
^ permalink raw reply
* [PATCH v4 3/3] staging: sm750fb: convert logging to device-based in sm750_hw.c
From: Artem Lytkin @ 2026-02-07 15:37 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
Cc: linux-fbdev, linux-staging, linux-kernel, Artem Lytkin
In-Reply-To: <20260207153703.2049-1-iprintercanon@gmail.com>
Replace pr_err() calls with dev_err() using &pdev->dev or
&sm750_dev->pdev->dev to provide proper device context in log
messages. This makes it easier to identify which device generated
the message when multiple framebuffer devices are present.
Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
drivers/staging/sm750fb/sm750_hw.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 983b51164..22cbabd26 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -42,7 +42,7 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
*/
ret = pci_request_region(pdev, 1, "sm750fb");
if (ret) {
- pr_err("Can not request PCI regions.\n");
+ dev_err(&pdev->dev, "Can not request PCI regions.\n");
goto exit;
}
@@ -50,7 +50,7 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
sm750_dev->pvReg =
ioremap(sm750_dev->vidreg_start, sm750_dev->vidreg_size);
if (!sm750_dev->pvReg) {
- pr_err("mmio failed\n");
+ dev_err(&pdev->dev, "mmio failed\n");
ret = -EFAULT;
goto exit;
}
@@ -75,7 +75,7 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
ioremap_wc(sm750_dev->vidmem_start, sm750_dev->vidmem_size);
if (!sm750_dev->pvMem) {
iounmap(sm750_dev->pvReg);
- pr_err("Map video memory failed\n");
+ dev_err(&pdev->dev, "Map video memory failed\n");
ret = -EFAULT;
goto exit;
}
@@ -291,7 +291,7 @@ int hw_sm750_crtc_set_mode(struct lynxfb_crtc *crtc,
ret = ddk750_set_mode_timing(&modparm, clock);
if (ret) {
- pr_err("Set mode timing failed\n");
+ dev_err(&sm750_dev->pdev->dev, "Set mode timing failed\n");
goto exit;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v4 2/3] staging: sm750fb: convert logging to device-based in sm750.c
From: Artem Lytkin @ 2026-02-07 15:37 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
Cc: linux-fbdev, linux-staging, linux-kernel, Artem Lytkin
In-Reply-To: <20260207153703.2049-1-iprintercanon@gmail.com>
Replace pr_err() calls with dev_err() using info->device to provide
proper device context in log messages. This makes it easier to
identify which device generated the message when multiple framebuffer
devices are present.
Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 987ba8772..c5eb3e98f 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -388,7 +388,8 @@ static int lynxfb_ops_set_par(struct fb_info *info)
var->accel_flags = 0;/*FB_ACCELF_TEXT;*/
if (ret) {
- pr_err("bpp %d not supported\n", var->bits_per_pixel);
+ dev_err(info->device, "bpp %d not supported\n",
+ var->bits_per_pixel);
return ret;
}
ret = hw_sm750_crtc_set_mode(crtc, var, fix);
@@ -487,7 +488,8 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo *var,
ret = lynxfb_set_color_offsets(info);
if (ret) {
- pr_err("bpp %d not supported\n", var->bits_per_pixel);
+ dev_err(info->device, "bpp %d not supported\n",
+ var->bits_per_pixel);
return ret;
}
@@ -502,7 +504,7 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo *var,
request = ALIGN(request, crtc->line_pad);
request = request * var->yres_virtual;
if (crtc->vidmem_size < request) {
- pr_err("not enough video memory for mode\n");
+ dev_err(info->device, "not enough video memory for mode\n");
return -ENOMEM;
}
@@ -527,7 +529,7 @@ static int lynxfb_ops_setcolreg(unsigned int regno,
ret = 0;
if (regno > 256) {
- pr_err("regno = %d\n", regno);
+ dev_err(info->device, "regno = %d\n", regno);
return -EINVAL;
}
@@ -834,7 +836,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
ret = fb_alloc_cmap(&info->cmap, 256, 0);
if (ret < 0) {
- pr_err("Could not allocate memory for cmap.\n");
+ dev_err(info->device, "Could not allocate memory for cmap.\n");
goto exit;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v4 1/3] staging: sm750fb: remove debug and diagnostic prints
From: Artem Lytkin @ 2026-02-07 15:37 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
Cc: linux-fbdev, linux-staging, linux-kernel, Artem Lytkin
In-Reply-To: <20260207153703.2049-1-iprintercanon@gmail.com>
Remove all pr_info, pr_debug, and pr_warn calls that dump internal
variable values, pointer addresses, and structure contents not useful
for production use. This includes the complete fb_find_mode() result
logging in lynxfb_set_fbinfo(), the CH7301 DVI chip status messages
in hw_sm750_inithw(), and various debug prints throughout the driver.
Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 76 +-----------------------------
drivers/staging/sm750fb/sm750_hw.c | 18 +------
2 files changed, 3 insertions(+), 91 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index fecd7457e..987ba8772 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -375,7 +375,6 @@ static int lynxfb_ops_set_par(struct fb_info *info)
line_length = var->xres_virtual * var->bits_per_pixel / 8;
line_length = ALIGN(line_length, crtc->line_pad);
fix->line_length = line_length;
- pr_info("fix->line_length = %d\n", fix->line_length);
/*
* var->red,green,blue,transp are need to be set by driver
@@ -485,11 +484,6 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo *var,
par = info->par;
crtc = &par->crtc;
- pr_debug("check var:%dx%d-%d\n",
- var->xres,
- var->yres,
- var->bits_per_pixel);
-
ret = lynxfb_set_color_offsets(info);
if (ret) {
@@ -580,7 +574,6 @@ static int lynxfb_ops_blank(int blank, struct fb_info *info)
struct lynxfb_par *par;
struct lynxfb_output *output;
- pr_debug("blank = %d.\n", blank);
par = info->par;
output = &par->output;
sm750_dev = par->dev;
@@ -625,7 +618,6 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
crtc->channel = sm750_primary;
crtc->o_screen = 0;
crtc->v_screen = sm750_dev->pvMem;
- pr_info("use simul primary mode\n");
break;
case sm750_simul_sec:
output->paths = sm750_pnc;
@@ -734,12 +726,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
lynx750_ext, NULL, vesa_modes,
};
int cdb[] = {ARRAY_SIZE(lynx750_ext), 0, VESA_MODEDB_SIZE};
- static const char * const mdb_desc[] = {
- "driver prepared modes",
- "kernel prepared default modedb",
- "kernel HELPERS prepared vesa_modes",
- };
-
static const char *fixId[2] = {
"sm750_fb1", "sm750_fb2",
};
@@ -767,7 +753,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
crtc->cursor.mmio = sm750_dev->pvReg +
0x800f0 + (int)crtc->channel * 0x140;
- pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
crtc->cursor.max_h = 64;
crtc->cursor.max_w = 64;
crtc->cursor.size = crtc->cursor.max_h * crtc->cursor.max_w * 2 / 8;
@@ -801,47 +786,10 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
ret = fb_find_mode(var, info, g_fbmode[index],
pdb[i], cdb[i], NULL, 8);
- if (ret == 1) {
- pr_info("success! use specified mode:%s in %s\n",
- g_fbmode[index],
- mdb_desc[i]);
+ if (ret == 1 || ret == 2)
break;
- } else if (ret == 2) {
- pr_warn("use specified mode:%s in %s,with an ignored refresh rate\n",
- g_fbmode[index],
- mdb_desc[i]);
- break;
- } else if (ret == 3) {
- pr_warn("wanna use default mode\n");
- /*break;*/
- } else if (ret == 4) {
- pr_warn("fall back to any valid mode\n");
- } else {
- pr_warn("ret = %d,fb_find_mode failed,with %s\n",
- ret,
- mdb_desc[i]);
- }
}
- /* some member of info->var had been set by fb_find_mode */
-
- pr_info("Member of info->var is :\n"
- "xres=%d\n"
- "yres=%d\n"
- "xres_virtual=%d\n"
- "yres_virtual=%d\n"
- "xoffset=%d\n"
- "yoffset=%d\n"
- "bits_per_pixel=%d\n"
- " ...\n",
- var->xres,
- var->yres,
- var->xres_virtual,
- var->yres_virtual,
- var->xoffset,
- var->yoffset,
- var->bits_per_pixel);
-
/* set par */
par->info = info;
@@ -851,7 +799,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
info->pseudo_palette = &par->pseudo_palette[0];
info->screen_base = crtc->v_screen;
- pr_debug("screen_base vaddr = %p\n", info->screen_base);
info->screen_size = line_length * var->yres_virtual;
/* set info->fix */
@@ -865,7 +812,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
strscpy(fix->id, fixId[index], sizeof(fix->id));
fix->smem_start = crtc->o_screen + sm750_dev->vidmem_start;
- pr_info("fix->smem_start = %lx\n", fix->smem_start);
/*
* according to mmap experiment from user space application,
* fix->mmio_len should not larger than virtual size
@@ -874,13 +820,10 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
* data into the bound over virtual size
*/
fix->smem_len = crtc->vidmem_size;
- pr_info("fix->smem_len = %x\n", fix->smem_len);
info->screen_size = fix->smem_len;
fix->line_length = line_length;
fix->mmio_start = sm750_dev->vidreg_start;
- pr_info("fix->mmio_start = %lx\n", fix->mmio_start);
fix->mmio_len = sm750_dev->vidreg_size;
- pr_info("fix->mmio_len = %x\n", fix->mmio_len);
lynxfb_set_visual_mode(info);
@@ -889,22 +832,12 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
var->accel_flags = 0;
var->vmode = FB_VMODE_NONINTERLACED;
- pr_debug("#1 show info->cmap :\nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
- info->cmap.start, info->cmap.len,
- info->cmap.red, info->cmap.green, info->cmap.blue,
- info->cmap.transp);
-
ret = fb_alloc_cmap(&info->cmap, 256, 0);
if (ret < 0) {
pr_err("Could not allocate memory for cmap.\n");
goto exit;
}
- pr_debug("#2 show info->cmap :\nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
- info->cmap.start, info->cmap.len,
- info->cmap.red, info->cmap.green, info->cmap.blue,
- info->cmap.transp);
-
exit:
lynxfb_ops_check_var(var, info);
return ret;
@@ -1131,12 +1064,8 @@ static int __init lynxfb_setup(char *options)
int len;
char *opt, *tmp;
- if (!options || !*options) {
- pr_warn("no options.\n");
+ if (!options || !*options)
return 0;
- }
-
- pr_info("options:%s\n", options);
len = strlen(options) + 1;
g_settings = kzalloc(len, GFP_KERNEL);
@@ -1173,7 +1102,6 @@ static int __init lynxfb_setup(char *options)
}
/* misc g_settings are transport to chip specific routines */
- pr_info("parameter left for chip specific analysis:%s\n", g_settings);
return 0;
}
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index ce46f240c..983b51164 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -34,8 +34,6 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
sm750_dev->vidreg_start = pci_resource_start(pdev, 1);
sm750_dev->vidreg_size = SZ_2M;
- pr_info("mmio phyAddr = %lx\n", sm750_dev->vidreg_start);
-
/*
* reserve the vidreg space of smi adaptor
* if you do this, you need to add release region code
@@ -56,7 +54,6 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
ret = -EFAULT;
goto exit;
}
- pr_info("mmio virtual addr = %p\n", sm750_dev->pvReg);
sm750_dev->accel.dpr_base = sm750_dev->pvReg + DE_BASE_ADDR_TYPE1;
sm750_dev->accel.dp_port_base = sm750_dev->pvReg + DE_PORT_ADDR_TYPE1;
@@ -72,8 +69,6 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
* @ddk750_get_vm_size function can be safe.
*/
sm750_dev->vidmem_size = ddk750_get_vm_size();
- pr_info("video memory phyAddr = %lx, size = %u bytes\n",
- sm750_dev->vidmem_start, sm750_dev->vidmem_size);
/* reserve the vidmem space of smi adaptor */
sm750_dev->pvMem =
@@ -84,7 +79,6 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
ret = -EFAULT;
goto exit;
}
- pr_info("video memory vaddr = %p\n", sm750_dev->pvMem);
exit:
return ret;
}
@@ -163,11 +157,9 @@ int hw_sm750_inithw(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
* The following register values for CH7301 are from
* Chrontel app note and our experiment.
*/
- pr_info("yes,CH7301 DVI chip found\n");
sm750_sw_i2c_write_reg(0xec, 0x1d, 0x16);
sm750_sw_i2c_write_reg(0xec, 0x21, 0x9);
sm750_sw_i2c_write_reg(0xec, 0x49, 0xC0);
- pr_info("okay,CH7301 DVI chip setup done\n");
}
}
@@ -192,14 +184,12 @@ int hw_sm750_output_set_mode(struct lynxfb_output *output,
if (sm750_get_chip_type() != SM750LE) {
if (channel == sm750_primary) {
- pr_info("primary channel\n");
if (output->paths & sm750_panel)
disp_set |= do_LCD1_PRI;
if (output->paths & sm750_crt)
disp_set |= do_CRT_PRI;
} else {
- pr_info("secondary channel\n");
if (output->paths & sm750_panel)
disp_set |= do_LCD1_SEC;
if (output->paths & sm750_crt)
@@ -215,7 +205,6 @@ int hw_sm750_output_set_mode(struct lynxfb_output *output,
poke32(DISPLAY_CONTROL_750LE, reg);
}
- pr_info("ddk setlogicdispout done\n");
return ret;
}
@@ -232,10 +221,8 @@ int hw_sm750_crtc_check_mode(struct lynxfb_crtc *crtc,
case 16:
break;
case 32:
- if (sm750_dev->revid == SM750LE_REVISION_ID) {
- pr_debug("750le do not support 32bpp\n");
+ if (sm750_dev->revid == SM750LE_REVISION_ID)
return -EINVAL;
- }
break;
default:
return -EINVAL;
@@ -302,7 +289,6 @@ int hw_sm750_crtc_set_mode(struct lynxfb_crtc *crtc,
else
clock = SECONDARY_PLL;
- pr_debug("Request pixel clock = %lu\n", modparm.pixel_clock);
ret = ddk750_set_mode_timing(&modparm, clock);
if (ret) {
pr_err("Set mode timing failed\n");
@@ -431,12 +417,10 @@ int hw_sm750_set_blank(struct lynxfb_output *output, int blank)
switch (blank) {
case FB_BLANK_UNBLANK:
- pr_debug("flag = FB_BLANK_UNBLANK\n");
dpms = SYSTEM_CTRL_DPMS_VPHP;
pps = PANEL_DISPLAY_CTRL_DATA;
break;
case FB_BLANK_NORMAL:
- pr_debug("flag = FB_BLANK_NORMAL\n");
dpms = SYSTEM_CTRL_DPMS_VPHP;
crtdb = CRT_DISPLAY_CTRL_BLANK;
break;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 0/3] staging: sm750fb: clean up logging
From: Artem Lytkin @ 2026-02-07 15:37 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
Cc: linux-fbdev, linux-staging, linux-kernel, Artem Lytkin
This series cleans up the logging in the sm750fb driver.
Changes since v3:
- Dropped the strcat and strncmp patches as requested by Greg KH
- Consistently removed all pr_info/pr_debug/pr_warn diagnostic prints
in one patch (previously some fb_find_mode prints were kept)
- Split dev_err conversions into two patches: one for sm750.c
and one for sm750_hw.c
Patch 1 removes all pr_info/pr_debug/pr_warn diagnostic prints
from both sm750.c and sm750_hw.c. These include address dumps,
debug variable prints, fb_find_mode result logging, and CH7301
DVI chip status messages.
Patches 2-3 convert the remaining pr_err() calls to dev_err()
for proper device context.
Artem Lytkin (3):
staging: sm750fb: remove debug and diagnostic prints
staging: sm750fb: convert logging to device-based in sm750.c
staging: sm750fb: convert logging to device-based in sm750_hw.c
drivers/staging/sm750fb/sm750.c | 88 +++---------------------------
drivers/staging/sm750fb/sm750_hw.c | 26 ++-------
2 files changed, 14 insertions(+), 100 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: rename CamelCase variables in sm750_accel.c
From: Greg Kroah-Hartman @ 2026-02-07 13:34 UTC (permalink / raw)
To: Artem Lytkin
Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260205234808.2232-1-iprintercanon@gmail.com>
On Thu, Feb 05, 2026 at 11:48:08PM +0000, Artem Lytkin wrote:
> Rename CamelCase function parameters and local variables to
> snake_case to comply with kernel coding style:
>
> sBase -> src_base dBase -> dst_base
> sPitch -> src_pitch dPitch -> dst_pitch
> Bpp -> bpp nDirection -> direction
> pSrcbuf -> src_buf srcDelta -> src_delta
> startBit -> start_bit bytePerPixel -> bytes_per_pixel
> fColor -> fg_color bColor -> bg_color
> ulBytesPerScan -> bytes_per_scan
> ul4BytesPerScan -> bytes_per_scan_aligned
> ulBytesRemain -> bytes_remain
> ajRemain -> remain_buf
> write_dpPort -> write_dp_port
>
> Update the corresponding kernel-doc comments and function
> declarations in sm750_accel.h.
>
> Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
> ---
> drivers/staging/sm750fb/sm750_accel.c | 134 +++++++++++++-------------
> drivers/staging/sm750fb/sm750_accel.h | 46 ++++-----
> 2 files changed, 90 insertions(+), 90 deletions(-)
Does not apply to my tree due to others doing the same sort of changes
right at the same time as you were, but their changes came in first,
sorry.
greg k-h
^ permalink raw reply
* Re: [PATCH v3 3/5] staging: sm750fb: remove debug and diagnostic prints
From: Greg Kroah-Hartman @ 2026-02-07 13:32 UTC (permalink / raw)
To: Artem Lytkin
Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260204120602.6715-3-iprintercanon@gmail.com>
On Wed, Feb 04, 2026 at 12:06:00PM +0000, Artem Lytkin wrote:
> @@ -811,11 +802,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
> g_fbmode[index],
> mdb_desc[i]);
> break;
> - } else if (ret == 3) {
> - pr_warn("wanna use default mode\n");
> - /*break;*/
> - } else if (ret == 4) {
> - pr_warn("fall back to any valid mode\n");
> + } else if (ret == 3 || ret == 4) {
> + continue;
> } else {
> pr_warn("ret = %d,fb_find_mode failed,with %s\n",
> ret,
Why delete some of these but not all? Why delete any of them?
Consistancy matters :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 2/5] staging: sm750fb: use strcmp() for exact option matching
From: Greg Kroah-Hartman @ 2026-02-07 13:31 UTC (permalink / raw)
To: Artem Lytkin
Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260204120602.6715-2-iprintercanon@gmail.com>
On Wed, Feb 04, 2026 at 12:05:59PM +0000, Artem Lytkin wrote:
> Replace strncmp(opt, "...", strlen("...")) with strcmp() in option
> parsing functions. Options from strsep() are complete null-terminated
> tokens, so prefix matching via strncmp() could cause false positives
> for options like "noaccelXYZ" matching "noaccel".
>
> Fixes: 81dee67e215b ("staging: sm750fb: add sm750 to staging")
> Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
> ---
> drivers/staging/sm750fb/sm750.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 4c6e84c03..bd2d4a290 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -937,21 +937,21 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
> dev_info(&sm750_dev->pdev->dev, "opt=%s\n", opt);
> dev_info(&sm750_dev->pdev->dev, "src=%s\n", src);
>
> - if (!strncmp(opt, "swap", strlen("swap"))) {
> + if (!strcmp(opt, "swap")) {
While I understand the feeling, again, this really isn't doing anything
except cause other code checkers to go "Wait, we can't call strcmp() we
must replace that with strncmp()!"
Please don't replace one warning with another. Option parsing is a
pain, let's not make it any more of a pain than it is. Ideally all of
the framebuffer drivers could make some "simple" helper functions to
handle this crazy logic for them, instead of forcing them to all do it
manually :(
Yet another reason all of us want to just delete all of these drivers...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 1/5] staging: sm750fb: replace strcat() with memcpy() in lynxfb_setup()
From: Greg Kroah-Hartman @ 2026-02-07 13:27 UTC (permalink / raw)
To: Artem Lytkin
Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260204120602.6715-1-iprintercanon@gmail.com>
On Wed, Feb 04, 2026 at 12:05:58PM +0000, Artem Lytkin wrote:
> As part of kernel hardening, I am auditing calls to strcat(). This
> code works but it is a bit ugly.
>
> This function takes a string "options" and allocates "g_settings"
> which is large enough to hold a copy of "options". It copies all the
> options from "options" to "g_settings" except "noaccel", "nomtrr" and
> "dual". The new buffer is large enough to fit all the options so
> there is no buffer overflow in using strcat() here.
>
> However, using strcat() is misleading because "tmp" always points
> to the next unused character in the "g_settings" buffer and it's
> always the NUL character. Use memcpy() instead to make the code
> easier to read. This also removes an instance of strcat() which
> is a #NiceBonus.
>
> Signed-off-by: Artem Lytkin <iprintercanon@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 fecd7457e..4c6e84c03 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -1163,7 +1163,7 @@ static int __init lynxfb_setup(char *options)
> } else if (!strncmp(opt, "dual", strlen("dual"))) {
> g_dualview = 1;
> } else {
> - strcat(tmp, opt);
> + memcpy(tmp, opt, strlen(opt));
You are open-coding a call to strcat() here :(
Please don't replace one "warning" with another, this will just cause
code churn over time. If the original code is fine, just leave it
as-is, your change here did not actually do anything at all.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: rename pvMem to pv_mem
From: Greg Kroah-Hartman @ 2026-02-07 13:25 UTC (permalink / raw)
To: dhyaan19022009-hue
Cc: Sudip Mukherjee, Teddy Wang, linux-staging, linux-kernel,
linux-fbdev, dhyaan19022009-hue
In-Reply-To: <20260204062240.20293-1-dhyaan19022009@gmail.com>
On Wed, Feb 04, 2026 at 11:52:40AM +0530, dhyaan19022009-hue wrote:
> Rename the CamelCase variable pvMem to the snake_case pv_mem to
> comply with the Linux kernel coding style. This fixes multiple
> warnings reported by checkpatch.pl.
>
> Signed-off-by: dhyaan19022009-hue <dhyaan19022009@gmail.com>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- It looks like you did not use your "real" name for the patch on either
the Signed-off-by: line, or the From: line (both of which have to
match). Please read the kernel file,
Documentation/process/submitting-patches.rst for how to do this
correctly.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: fix CamelCase and Hungarian prefix in variable names
From: Greg KH @ 2026-02-07 12:43 UTC (permalink / raw)
To: Shreyas Ravi
Cc: sudipm.mukherjee, teddy.wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260204054753.3137479-1-shreyasravi320@gmail.com>
On Tue, Feb 03, 2026 at 09:47:53PM -0800, Shreyas Ravi wrote:
> Fix multiple coding style issues:
> - Rename CamelCase variables to snake_case
> - Drop Hungarian prefixes on variable names
>
> No functional changes.
>
> Signed-off-by: Shreyas Ravi <shreyasravi320@gmail.com>
> ---
> drivers/staging/sm750fb/sm750.c | 132 +++++++++++++-------------
> drivers/staging/sm750fb/sm750.h | 16 ++--
> drivers/staging/sm750fb/sm750_accel.c | 132 +++++++++++++-------------
> drivers/staging/sm750fb/sm750_accel.h | 44 ++++-----
> drivers/staging/sm750fb/sm750_hw.c | 26 ++---
> 5 files changed, 175 insertions(+), 175 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index dec1f6b88a7d..67cba3eb0422 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -30,14 +30,14 @@
> */
>
> /* common var for all device */
> -static int g_hwcursor = 1;
> -static int g_noaccel;
> -static int g_nomtrr;
> -static const char *g_fbmode[] = {NULL, NULL};
> -static const char *g_def_fbmode = "1024x768-32@60";
> -static char *g_settings;
> -static int g_dualview;
> -static char *g_option;
> +static int sm750_hwcursor = 1;
> +static int sm750_noaccel;
> +static int sm750_nomtrr;
> +static const char *sm750_fbmode[] = {NULL, NULL};
> +static const char *sm750_def_fbmode = "1024x768-32@60";
> +static char *sm750_settings;
> +static int sm750_dualview;
> +static char *sm750_option;
Why are static variables prefixed with the driver name? They previously
were not, why add the prefix here?
And you are doing multiple things here, changing Hungarian prefixes AND
CamelCase names, please only do one logical change per patch.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Daniel Almeida @ 2026-02-06 22:33 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Jonathan Corbet, Alex Deucher,
Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
Lucas De Marchi, Thomas Hellström, Helge Deller,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <983b7db3-b0e6-45d6-866e-f001b64abde1@nvidia.com>
>>
>>
>> Yeah, but my point being: is there a reason why the underlying list has to
>> remain read-only? Is this a safety requirement or an invariant that is established
>> by the code above?
> I'm not fully sure if it's an invariant or a safety requirement, but anyone
> creating a C list head on the rust side must guarantee that it is not modified.
> Since rust has no visibility on the C side, I believe it is a Rust invariant
> here that the existence of CListHead assumes that the list cannot be modified
> once Rust has access over it. That is up to the creator (user) of the CListHead
> to guarantee. In the DRM buddy case, once the list is allocated and accessible
> from Rust, C code will not modify it while the Rust object exists.
>
> Does that make sense, or is there a better way to document this?
>
> --
> Joel Fernandes
In which case, I recommend moving this to a safety requirement when
creating the list.
I assume the purpose of not modifying the list on the C side is to avoid
corrupting the list in Rust somehow?
— Daniel
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Daniel Almeida @ 2026-02-06 21:30 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Gary Guo, Joel Fernandes, linux-kernel, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
Matthew Auld, Matthew Brost, Lucas De Marchi,
Thomas Hellström, Helge Deller, Alice Ryhl, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DG820GM5YHJS.11E92OR824CWM@kernel.org>
> On 6 Feb 2026, at 14:27, Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Fri Feb 6, 2026 at 6:20 PM CET, Gary Guo wrote:
>> I asked for this to be changed to `pub(crate)` because I think this isn't
>> something that should be used by drivers.
>>
>> As you said, tt might be tricky to enforce that with new build system when
>> subsystems are inside different crates. But until then I think it's better to
>> limit visibility.
>
> It should *usually* not be used by drivers, but there are exceptions. For
> instance, it is perfectly valid to be used by Rust drivers that interact with C
> drivers.
I agree with what Danilo said here.
I don’t see a reason to forbid drivers from using this. If the reason is
the unsafe bits, then isn’t it the same pattern used by impl_has_work!()
anyways? i.e.: a macro that implements an unsafe trait so long as the driver
gives it the right Work field. Seems equivalent in spirit to the clist_create macro
introduced by this patch.
— Daniel
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-02-06 21:26 UTC (permalink / raw)
To: Daniel Almeida
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Jonathan Corbet, Alex Deucher,
Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
Lucas De Marchi, Thomas Hellström, Helge Deller,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <E846F3BB-DE64-4E6B-ACA3-00F965038478@collabora.com>
On 2/6/2026 4:21 PM, Daniel Almeida wrote:
>
>
>> On 6 Feb 2026, at 17:51, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>
>> Hi Daniel,
>> Hope you do not mind me replying piecemeal as I can reply more quickly. Thank
>> you for all the comments.
>>
>> On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>>>> +use crate::{
>>>> + bindings,
>>>> + types::Opaque, //
>>>> +};
>>>> +
>>>> +use pin_init::PinInit;
>>>> +
>>>> +/// Wraps a `list_head` object for use in intrusive linked lists.
>>>> +///
>>>> +/// # Invariants
>>>> +///
>>>> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
>>>> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
>>>> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
>>>
>>> Can you expand on the two points above?
>>
>> This is basically saying that a C `list_head` that is wrapped by a `CListHead`
>> is read-only for the lifetime of `ClistHead`. modifying the pointers anymore.
>> That is the invariant.
>>
>> Or did I miss something?
>>
>> --
>> Joel Fernandes
>>
>>
>
>
> Yeah, but my point being: is there a reason why the underlying list has to
> remain read-only? Is this a safety requirement or an invariant that is established
> by the code above?
I'm not fully sure if it's an invariant or a safety requirement, but anyone
creating a C list head on the rust side must guarantee that it is not modified.
Since rust has no visibility on the C side, I believe it is a Rust invariant
here that the existence of CListHead assumes that the list cannot be modified
once Rust has access over it. That is up to the creator (user) of the CListHead
to guarantee. In the DRM buddy case, once the list is allocated and accessible
from Rust, C code will not modify it while the Rust object exists.
Does that make sense, or is there a better way to document this?
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Daniel Almeida @ 2026-02-06 21:21 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Jonathan Corbet, Alex Deucher,
Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
Lucas De Marchi, Thomas Hellström, Helge Deller,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <7ed85eca-2a5e-4e8f-8356-e7fbbf7d3a8f@nvidia.com>
> On 6 Feb 2026, at 17:51, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> Hi Daniel,
> Hope you do not mind me replying piecemeal as I can reply more quickly. Thank
> you for all the comments.
>
> On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>>> +use crate::{
>>> + bindings,
>>> + types::Opaque, //
>>> +};
>>> +
>>> +use pin_init::PinInit;
>>> +
>>> +/// Wraps a `list_head` object for use in intrusive linked lists.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
>>> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
>>> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
>>
>> Can you expand on the two points above?
>
> This is basically saying that a C `list_head` that is wrapped by a `CListHead`
> is read-only for the lifetime of `ClistHead`. modifying the pointers anymore.
> That is the invariant.
>
> Or did I miss something?
>
> --
> Joel Fernandes
>
>
Yeah, but my point being: is there a reason why the underlying list has to
remain read-only? Is this a safety requirement or an invariant that is established
by the code above?
— Daniel
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-02-06 21:12 UTC (permalink / raw)
To: Gary Guo, Daniel Almeida
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Jonathan Corbet, Alex Deucher,
Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
Lucas De Marchi, Thomas Hellström, Helge Deller,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Balbir Singh, Philipp Stanner, Elle Rhumsaa, joel,
nouveau, dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <DG86CJXCM7DB.1A4F4JTMSS9ZZ@garyguo.net>
On 2/6/2026 3:51 PM, Gary Guo wrote:
> On Fri Feb 6, 2026 at 8:46 PM GMT, Joel Fernandes wrote:
>> On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>>>> +#[repr(transparent)]
>>>> +pub(crate) struct CList<T, const OFFSET: usize>(CListHead, PhantomData<T>);
>>>> +
>>>> +impl<T, const OFFSET: usize> CList<T, OFFSET> {
>>>> + /// Create a typed [`CList`] reference from a raw sentinel `list_head` pointer.
>>>> + ///
>>>> + /// # Safety
>>>> + ///
>>>> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>>>> + /// representing a list sentinel.
>>>> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>>>> + /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
>>>> + /// - `T` must be `#[repr(transparent)]` over the C struct.
>>>> + #[inline]
>>>> + pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
>>>> + // SAFETY:
>>>> + // - [`CList`] has same layout as [`CListHead`] due to repr(transparent).
>>>> + // - Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>>>> + unsafe { &*ptr.cast() }
>>>> + }
>>>> +
>>>> + /// Check if the list is empty.
>>>> + #[inline]
>>>> + #[expect(dead_code)]
>>>> + pub(crate) fn is_empty(&self) -> bool {
>>>
>>> Why can’t this be pub?
>>
>> I believe this was suggested by Gary. See the other thread where we are
>> discussing it (with Gary and Danilo) and let us discuss there.
>
> I suggested the module to be `pub(crate)`. For the individual item it is not
> necessary if the module itself already have limited visibility.
>
Sure, I can change it to module-level pub then, and drop the pub(crate) if
everyone agrees.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-02-06 20:51 UTC (permalink / raw)
To: Daniel Almeida
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Jonathan Corbet, Alex Deucher,
Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
Lucas De Marchi, Thomas Hellström, Helge Deller,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <73B64D35-6574-4776-962D-865465C40226@collabora.com>
Hi Daniel,
Hope you do not mind me replying piecemeal as I can reply more quickly. Thank
you for all the comments.
On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>> +use crate::{
>> + bindings,
>> + types::Opaque, //
>> +};
>> +
>> +use pin_init::PinInit;
>> +
>> +/// Wraps a `list_head` object for use in intrusive linked lists.
>> +///
>> +/// # Invariants
>> +///
>> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
>> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
>> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
>
> Can you expand on the two points above?
This is basically saying that a C `list_head` that is wrapped by a `CListHead`
is read-only for the lifetime of `ClistHead`. modifying the pointers anymore.
That is the invariant.
Or did I miss something?
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Gary Guo @ 2026-02-06 20:51 UTC (permalink / raw)
To: Joel Fernandes, Daniel Almeida
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Jonathan Corbet, Alex Deucher,
Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
Lucas De Marchi, Thomas Hellström, Helge Deller,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <8dde5b79-29d6-4840-be9a-31bc4af27cf9@nvidia.com>
On Fri Feb 6, 2026 at 8:46 PM GMT, Joel Fernandes wrote:
> On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>>> +#[repr(transparent)]
>>> +pub(crate) struct CList<T, const OFFSET: usize>(CListHead, PhantomData<T>);
>>> +
>>> +impl<T, const OFFSET: usize> CList<T, OFFSET> {
>>> + /// Create a typed [`CList`] reference from a raw sentinel `list_head` pointer.
>>> + ///
>>> + /// # Safety
>>> + ///
>>> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>>> + /// representing a list sentinel.
>>> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>>> + /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
>>> + /// - `T` must be `#[repr(transparent)]` over the C struct.
>>> + #[inline]
>>> + pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
>>> + // SAFETY:
>>> + // - [`CList`] has same layout as [`CListHead`] due to repr(transparent).
>>> + // - Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>>> + unsafe { &*ptr.cast() }
>>> + }
>>> +
>>> + /// Check if the list is empty.
>>> + #[inline]
>>> + #[expect(dead_code)]
>>> + pub(crate) fn is_empty(&self) -> bool {
>>
>> Why can’t this be pub?
>
> I believe this was suggested by Gary. See the other thread where we are
> discussing it (with Gary and Danilo) and let us discuss there.
I suggested the module to be `pub(crate)`. For the individual item it is not
necessary if the module itself already have limited visibility.
Best,
Gary
^ 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