[GIT PULL] fbdev fixes and cleanups for v6.10-rc1
Hi Linus, please pull some fixes and cleanups for the fbdev drivers for kernel 6.10-rc1. Code cleanups for offb, shmobile, sisfb, savage, au1200fb, uvesafb, omap2 and sh7760fb, as well as the addition of some HAS_IOPORT dependencies and adjustment of generated logo file to make build reproducible. Thanks! Helge The following changes since commit 4cece764965020c22cff7665b18a012006359095: Linux 6.9-rc1 (2024-03-24 14:10:05 -0700) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.10-rc1 for you to fetch changes up to ce4a7ae84a58b9f33aae8d6c769b3c94f3d5ce76: fbdev: offb: replace of_node_put with __free(device_node) (2024-04-25 12:13:06 +0200) fbdev fixes and cleanups for 6.10-rc1: Code cleanups for offb, shmobile, sisfb, savage, au1200fb, uvesafb, omap2 and sh7760fb, as well as the addition of some HAS_IOPORT dependencies and adjustment of generated logo file to make build reproducible. Abdulrasaq Lawani (1): fbdev: offb: replace of_node_put with __free(device_node) Arnd Bergmann (2): fbdev: shmobile: fix snprintf truncation fbdev: sisfb: hide unused variables Cai Xinchen (1): fbdev: savage: Handle err return when savagefb_check_var failed Justin Stitt (4): fbdev: fsl-diu-fb: replace deprecated strncpy with strscpy_pad fbdev: au1200fb: replace deprecated strncpy with strscpy fbdev: uvesafb: replace deprecated strncpy with strscpy_pad video: hdmi: prefer length specifier in format over string copying Kuninori Morimoto (1): fbdev: omap2: replace of_graph_get_next_endpoint() Lucas Stach (1): video: logo: Drop full path of the input filename in generated file Niklas Schnelle (1): fbdev: add HAS_IOPORT dependencies Randy Dunlap (1): fbdev: sh7760fb: allow modular build drivers/video/fbdev/Kconfig | 26 +- drivers/video/fbdev/au1200fb.c| 2 +- drivers/video/fbdev/fsl-diu-fb.c | 2 +- drivers/video/fbdev/offb.c| 3 +-- drivers/video/fbdev/omap2/omapfb/dss/dsi.c| 3 ++- drivers/video/fbdev/omap2/omapfb/dss/dss-of.c | 20 +--- drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c | 3 ++- drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c | 3 ++- drivers/video/fbdev/omap2/omapfb/dss/venc.c | 3 ++- drivers/video/fbdev/pxafb.c | 2 +- drivers/video/fbdev/savage/savagefb_driver.c | 5 - drivers/video/fbdev/sh_mobile_lcdcfb.c| 2 +- drivers/video/fbdev/sis/init301.c | 3 +-- drivers/video/fbdev/uvesafb.c | 2 +- drivers/video/hdmi.c | 10 ++ drivers/video/logo/pnmtologo.c| 2 -- include/video/omapfb_dss.h| 3 --- 17 files changed, 35 insertions(+), 59 deletions(-)
Re: [PATCH] video: fbdev: replacing of_node_put with __free(device_node)
On 4/23/24 03:20, Abdulrasaq Lawani wrote:
Replaced instance of of_node_put with __free(device_node)
to simplify code and protect against any memory leaks
due to future changes in the control flow.
Suggested-by: Julia Lawall
Signed-off-by: Abdulrasaq Lawani
applied.
Thanks!
Helge
---
drivers/video/fbdev/offb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index b421b46d88ef..ea38a260774b 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -357,7 +357,7 @@ static void offb_init_palette_hacks(struct fb_info *info,
struct device_node *dp
par->cmap_type = cmap_gxt2000;
} else if (of_node_name_prefix(dp, "vga,Display-")) {
/* Look for AVIVO initialized by SLOF */
- struct device_node *pciparent = of_get_parent(dp);
+ struct device_node *pciparent __free(device_node) =
of_get_parent(dp);
const u32 *vid, *did;
vid = of_get_property(pciparent, "vendor-id", NULL);
did = of_get_property(pciparent, "device-id", NULL);
@@ -369,7 +369,6 @@ static void offb_init_palette_hacks(struct fb_info *info,
struct device_node *dp
if (par->cmap_adr)
par->cmap_type = cmap_avivo;
}
- of_node_put(pciparent);
} else if (dp && of_device_is_compatible(dp, "qemu,std-vga")) {
#ifdef __BIG_ENDIAN
const __be32 io_of_addr[3] = { 0x0100, 0x0, 0x0 };
Re: [PATCH -next] fbdev: savage: Handle err return when savagefb_check_var failed
On 4/16/24 08:51, Cai Xinchen wrote:
The commit 04e5eac8f3ab("fbdev: savage: Error out if pixclock equals zero")
checks the value of pixclock to avoid divide-by-zero error. However
the function savagefb_probe doesn't handle the error return of
savagefb_check_var. When pixclock is 0, it will cause divide-by-zero error.
Fixes: 04e5eac8f3ab ("fbdev: savage: Error out if pixclock equals zero")
Signed-off-by: Cai Xinchen
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/savage/savagefb_driver.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
applied.
Thanks!
Helge
diff --git a/drivers/video/fbdev/savage/savagefb_driver.c
b/drivers/video/fbdev/savage/savagefb_driver.c
index ebc9aeffdde7..ac41f8f37589 100644
--- a/drivers/video/fbdev/savage/savagefb_driver.c
+++ b/drivers/video/fbdev/savage/savagefb_driver.c
@@ -2276,7 +2276,10 @@ static int savagefb_probe(struct pci_dev *dev, const
struct pci_device_id *id)
if (info->var.xres_virtual > 0x1000)
info->var.xres_virtual = 0x1000;
#endif
- savagefb_check_var(>var, info);
+ err = savagefb_check_var(>var, info);
+ if (err)
+ goto failed;
+
savagefb_set_fix(info);
/*
Re: [PATCH] video: fbdev: au1200fb: replace deprecated strncpy with strscpy
On 4/25/24 01:49, Kees Cook wrote:
On Wed, Mar 20, 2024 at 11:48:52PM +0100, Helge Deller wrote:
On 3/20/24 23:35, Justin Stitt wrote:
Hi,
On Wed, Mar 20, 2024 at 12:56 AM Helge Deller wrote:
On 3/19/24 00:46, Justin Stitt wrote:
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
Let's use the new 2-argument strscpy() which guarantees NUL-termination
on the destination buffer while also simplifying the syntax. Note that
strscpy() will not NUL-pad the destination buffer like strncpy() does.
However, the NUL-padding behavior of strncpy() is not required since
fbdev is already NUL-allocated from au1200fb_drv_probe() ->
frameuffer_alloc(), rendering any additional NUL-padding redundant.
| p = kzalloc(fb_info_size + size, GFP_KERNEL);
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
Found with: $ rg "strncpy\("
---
drivers/video/fbdev/au1200fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c
index 6f20efc663d7..e718fea63662 100644
--- a/drivers/video/fbdev/au1200fb.c
+++ b/drivers/video/fbdev/au1200fb.c
@@ -1557,7 +1557,7 @@ static int au1200fb_init_fbinfo(struct au1200fb_device
*fbdev)
return ret;
}
- strncpy(fbi->fix.id, "AU1200", sizeof(fbi->fix.id));
+ strscpy(fbi->fix.id, "AU1200");
I wonder if you really build-tested this, as this driver is for the mips
architecture...
And I don't see a strscpy() function which takes just 2 arguments.
But I might be wrong
I did build successfully :thumbs_up:
Commit e6584c3964f2f ("string: Allow 2-argument strscpy()") introduced
this new strscpy() form; it is present in string.h on Linus' tree.
Interesting patch.
Might give compile problems if patches like yours gets automatically
picked up to stable series as long as Kees patch hasn't been backported yet...
Anyway, thanks for the pointer!
I'll apply your patch in the next round for fbdev.
Hi! I haven't seen this show up in -next yet. Have you had a chance to
pick it up?
There are also these too:
https://lore.kernel.org/all/20240320-strncpy-drivers-video-fbdev-fsl-diu-fb-c-v1-1-3cd3c012f...@google.com/
https://patchwork.kernel.org/project/linux-hardening/patch/20240320-strncpy-drivers-video-fbdev-uvesafb-c-v1-1-fd6af3766...@google.com/
https://patchwork.kernel.org/project/linux-hardening/patch/20240320-strncpy-drivers-video-hdmi-c-v1-1-f9a08168c...@google.com/
All 4 patches picked up into fbdev for-next git tree now.
Thanks!
Helge
Re: [PATCH 1/1] video: Handle HAS_IOPORT dependencies
On 4/22/24 21:28, Arnd Bergmann wrote:
On Mon, Apr 22, 2024, at 10:34, Niklas Schnelle wrote:
On Thu, 2024-04-11 at 16:00 +0200, Helge Deller wrote:
* Niklas Schnelle :
In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to #ifdef functions and their callsites which
unconditionally use these I/O accessors. In the include/video/vga.h
these are conveniently all those functions with the vga_io_* prefix.
Why don't you code it like in the patch below?
inb_p(), outb_p() and outw() would then need to be defined externally
without an implementation so that they would generate link time errors
(instead of compile time errors).
This may be personal preference but I feel like link time errors would
be very late to catch a configuration that can't work. Also this would
bypass the __compiletime_error("inb()) requires CONFIG_HAS_IOPORT");
added instead of the in*()/out*() helpers to make it easy to spot the
problem.
I'm not a fan of #ifdeffery either but I think in this case it is
simple, well enough contained and overall there aren't that many spots
where we need to exclude just some sections of code vs entire drivers
with vga.h probably being the worst of them all.
Agreed. I also tried to see if we can move stuff out of vga.h
to have it included in fewer places, as almost everything that
uses this header already has a HAS_IOPORT dependency, but that
would be a lot more work.
The other one that gains a few ugly #ifdefs is the 8250 driver,
everything else is already merged in linux-next or needs a simple
Kconfig dependency.
I think we can make the vga.h file a little more readable
by duplicating the functions and still keep the __compiletime_error()
version in asm/io.h, see below.
Ok.
I assume you or Niklas will then send an updated patch?
Helge
diff --git a/include/video/vga.h b/include/video/vga.h
index 947c0abd04ef..7e1d8252b732 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -197,6 +197,23 @@ struct vgastate {
extern int save_vga(struct vgastate *state);
extern int restore_vga(struct vgastate *state);
+static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short
port)
+{
+ return readb (regbase + port);
+}
+
+static inline void vga_mm_w (void __iomem *regbase, unsigned short port,
unsigned char val)
+{
+ writeb (val, regbase + port);
+}
+
+static inline void vga_mm_w_fast (void __iomem *regbase, unsigned short port,
+ unsigned char reg, unsigned char val)
+{
+ writew (VGA_OUT16VAL (val, reg), regbase + port);
+}
+
+#ifdef CONFIG_HAS_IOPORT
/*
* generic VGA port read/write
*/
@@ -217,22 +234,6 @@ static inline void vga_io_w_fast (unsigned short port,
unsigned char reg,
outw(VGA_OUT16VAL (val, reg), port);
}
-static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short
port)
-{
- return readb (regbase + port);
-}
-
-static inline void vga_mm_w (void __iomem *regbase, unsigned short port,
unsigned char val)
-{
- writeb (val, regbase + port);
-}
-
-static inline void vga_mm_w_fast (void __iomem *regbase, unsigned short port,
- unsigned char reg, unsigned char val)
-{
- writew (VGA_OUT16VAL (val, reg), regbase + port);
-}
-
static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
{
if (regbase)
@@ -258,7 +259,25 @@ static inline void vga_w_fast (void __iomem *regbase,
unsigned short port,
else
vga_io_w_fast (port, reg, val);
}
+#else
+static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
+{
+ return vga_mm_r(regbase, port);
+}
+
+static inline void vga_w(void __iomem *regbase, unsigned short port, unsigned
char val)
+{
+ vga_mm_w (regbase, port, val);
+}
+
+static inline void vga_w_fast (void __iomem *regbase, unsigned short port,
+ unsigned char reg, unsigned char val)
+{
+ vga_mm_w_fast(regbase, port, reg, val);
+}
+
+#endif
/*
* VGA CRTC register read/write
Re: [bug-report] task info hung problem in fb_deferred_io_work()
On 4/18/24 16:26, Takashi Iwai wrote: On Thu, 18 Apr 2024 16:06:52 +0200, Nam Cao wrote: On 2024-04-18 Harshit Mogalapalli wrote: While fuzzing 5.15.y kernel with Syzkaller, we noticed a INFO: task hung bug in fb_deferred_io_work() Which framebuffer device are you using exactly? It is possible that the problem is with the device driver, not core framebuffer. Note that it was already known that using flush_delayed_work() caused a problem. See the thread of the fix patch: https://lore.kernel.org/all/20230129082856.22113-1-ti...@suse.de/ Harshit reported the hung tasks with kernel v5.15-stable, and can even reproduce that issue with kernel v6.9-rc4 although it has all of your patches from that referenced mail thread applied. So, what does your statement that "it was already known that it causes problems" exactly mean? Can it be fixed? Is someone looking into fixing it? BTW, the problem is seen with bochs drm. Helge
Re: [PATCH 1/1] video: Handle HAS_IOPORT dependencies
* Niklas Schnelle :
> In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
> compile time. We thus need to #ifdef functions and their callsites which
> unconditionally use these I/O accessors. In the include/video/vga.h
> these are conveniently all those functions with the vga_io_* prefix.
Why don't you code it like in the patch below?
inb_p(), outb_p() and outw() would then need to be defined externally
without an implementation so that they would generate link time errors
(instead of compile time errors).
diff --git a/include/video/vga.h b/include/video/vga.h
index 947c0abd04ef..32c915e109fa 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -203,18 +203,20 @@ extern int restore_vga(struct vgastate *state);
static inline unsigned char vga_io_r (unsigned short port)
{
- return inb_p(port);
+ return IS_ENABLED(CONFIG_HAS_IOPORT) ? inb_p(port) : 0;
}
static inline void vga_io_w (unsigned short port, unsigned char val)
{
- outb_p(val, port);
+ if (IS_ENABLED(CONFIG_HAS_IOPORT))
+ outb_p(val, port);
}
static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
unsigned char val)
{
- outw(VGA_OUT16VAL (val, reg), port);
+ if (IS_ENABLED(CONFIG_HAS_IOPORT))
+ outw(VGA_OUT16VAL (val, reg), port);
}
static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short
port)
>
> Co-developed-by: Arnd Bergmann
> Signed-off-by: Arnd Bergmann
> Signed-off-by: Niklas Schnelle
> ---
> Note: This patch does not depend any not-yet-mainline HAS_IOPORT changes
> and may be merged via subsystem specific trees at your earliest
> convenience.
>
> include/video/vga.h | 35 +--
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/include/video/vga.h b/include/video/vga.h
> index 947c0abd04ef..ed89295941c4 100644
> --- a/include/video/vga.h
> +++ b/include/video/vga.h
> @@ -201,6 +201,7 @@ extern int restore_vga(struct vgastate *state);
> * generic VGA port read/write
> */
>
> +#ifdef CONFIG_HAS_IOPORT
> static inline unsigned char vga_io_r (unsigned short port)
> {
> return inb_p(port);
> @@ -210,12 +211,12 @@ static inline void vga_io_w (unsigned short port,
> unsigned char val)
> {
> outb_p(val, port);
> }
> -
> static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
> unsigned char val)
> {
> outw(VGA_OUT16VAL (val, reg), port);
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short
> port)
> {
> @@ -235,28 +236,34 @@ static inline void vga_mm_w_fast (void __iomem
> *regbase, unsigned short port,
>
> static inline unsigned char vga_r (void __iomem *regbase, unsigned short
> port)
> {
> - if (regbase)
> - return vga_mm_r (regbase, port);
> - else
> +#ifdef CONFIG_HAS_IOPORT
> + if (!regbase)
> return vga_io_r (port);
> + else
> +#endif /* CONFIG_HAS_IOPORT */
> + return vga_mm_r (regbase, port);
> }
>
> static inline void vga_w (void __iomem *regbase, unsigned short port,
> unsigned char val)
> {
> - if (regbase)
> - vga_mm_w (regbase, port, val);
> - else
> +#ifdef CONFIG_HAS_IOPORT
> + if (!regbase)
> vga_io_w (port, val);
> + else
> +#endif /* CONFIG_HAS_IOPORT */
> + vga_mm_w (regbase, port, val);
> }
>
>
> static inline void vga_w_fast (void __iomem *regbase, unsigned short port,
> unsigned char reg, unsigned char val)
> {
> - if (regbase)
> - vga_mm_w_fast (regbase, port, reg, val);
> - else
> +#ifdef CONFIG_HAS_IOPORT
> + if (!regbase)
> vga_io_w_fast (port, reg, val);
> + else
> +#endif /* CONFIG_HAS_IOPORT */
> + vga_mm_w_fast (regbase, port, reg, val);
> }
>
>
> @@ -280,6 +287,7 @@ static inline void vga_wcrt (void __iomem *regbase,
> unsigned char reg, unsigned
> #endif /* VGA_OUTW_WRITE */
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> static inline unsigned char vga_io_rcrt (unsigned char reg)
> {
> vga_io_w (VGA_CRT_IC, reg);
> @@ -295,6 +303,7 @@ static inline void vga_io_wcrt (unsigned char reg,
> unsigned char val)
> vga_io_w (VGA_CRT_DC, val);
> #endif /* VGA_OUTW_WRITE */
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static inline unsigned char vga_mm_rcrt (void __iomem *regbase, unsigned
> char reg)
> {
> @@ -333,6 +342,7 @@ static inline void vga_wseq (void __iomem *regbase,
> unsigned char reg, unsigned
> #endif /* VGA_OUTW_WRITE */
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> static inline unsigned char vga_io_rseq (unsigned char reg)
> {
> vga_io_w (VGA_SEQ_I, reg);
> @@ -348,6 +358,7 @@ static inline void vga_io_wseq (unsigned char reg,
> unsigned char val)
> vga_io_w (VGA_SEQ_D, val);
Re: [PATCH v2 1/1] fbdev: add HAS_IOPORT dependencies
On 4/10/24 12:56, Niklas Schnelle wrote: In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at compile time. We thus need to add HAS_IOPORT as dependency for those drivers using them. Co-developed-by: Arnd Bergmann Signed-off-by: Arnd Bergmann Signed-off-by: Niklas Schnelle added to fbdev git tree. Thanks! Helge --- Note: This patch does not depend any not-yet-mainline HAS_IOPORT changes and may be merged via subsystem specific trees at your earliest convenience. v1 -> v2: - Add dependency for FB_ARC drivers/video/fbdev/Kconfig | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 197b6d5268e9..76bbfd3767da 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -157,7 +157,7 @@ config FB_IMX config FB_CYBER2000 tristate "CyberPro 2000/2010/5000 support" - depends on FB && PCI && (BROKEN || !SPARC64) + depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64) select FB_IOMEM_HELPERS help This enables support for the Integraphics CyberPro 20x0 and 5000 @@ -245,7 +245,7 @@ config FB_FM2 config FB_ARC tristate "Arc Monochrome LCD board support" - depends on FB && (X86 || COMPILE_TEST) + depends on FB && HAS_IOPORT && (X86 || COMPILE_TEST) select FB_SYSMEM_HELPERS_DEFERRED help This enables support for the Arc Monochrome LCD board. The board @@ -1046,7 +1046,7 @@ config FB_ATY_BACKLIGHT config FB_S3 tristate "S3 Trio/Virge support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1107,7 +1107,7 @@ config FB_SAVAGE_ACCEL config FB_SIS tristate "SiS/XGI display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select BOOT_VESA_SUPPORT if FB_SIS = y select FB_CFB_FILLRECT select FB_CFB_COPYAREA @@ -1138,7 +1138,7 @@ config FB_SIS_315 config FB_VIA tristate "VIA UniChrome (Pro) and Chrome9 display support" - depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST) + depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || COMPILE_TEST) select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1177,7 +1177,7 @@ endif config FB_NEOMAGIC tristate "NeoMagic display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1204,7 +1204,7 @@ config FB_KYRO config FB_3DFX tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1252,7 +1252,7 @@ config FB_VOODOO1 config FB_VT8623 tristate "VIA VT8623 support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1267,7 +1267,7 @@ config FB_VT8623 config FB_TRIDENT tristate "Trident/CyberXXX/CyberBlade support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1290,7 +1290,7 @@ config FB_TRIDENT config FB_ARK tristate "ARK 2000PV support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1814,7 +1814,7 @@ config FB_SSD1307 config FB_SM712 tristate "Silicon Motion SM712 framebuffer support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_IOMEM_HELPERS help Frame buffer driver for the Silicon Motion SM710, SM712, SM721
Re: [PATCH] fbdev/sh7760fb: allow modular build
On 4/10/24 06:54, Randy Dunlap wrote:
Hi,
Will someone be merging this patch?
I've just added it to the fbdev git tree.
Thanks!
Helge
thanks.
On 2/10/24 1:31 AM, John Paul Adrian Glaubitz wrote:
On Fri, 2024-02-09 at 21:39 -0800, Randy Dunlap wrote:
There is no reason to prohibit sh7760fb from being built as a
loadable module as suggested by Geert, so change the config symbol
from bool to tristate to allow that and change the FB dependency as
needed.
Fixes: f75f71b2c418 ("fbdev/sh7760fb: Depend on FB=y")
Suggested-by: Geert Uytterhoeven
Signed-off-by: Randy Dunlap
Cc: Thomas Zimmermann
Cc: Javier Martinez Canillas
Cc: John Paul Adrian Glaubitz
Cc: Sam Ravnborg
Cc: Helge Deller
Cc: linux-fb...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
---
drivers/video/fbdev/Kconfig |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -- a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1645,8 +1645,8 @@ config FB_COBALT
select FB_IOMEM_HELPERS
config FB_SH7760
- bool "SH7760/SH7763/SH7720/SH7721 LCDC support"
- depends on FB=y && (CPU_SUBTYPE_SH7760 || CPU_SUBTYPE_SH7763 \
+ tristate "SH7760/SH7763/SH7720/SH7721 LCDC support"
+ depends on FB && (CPU_SUBTYPE_SH7760 || CPU_SUBTYPE_SH7763 \
|| CPU_SUBTYPE_SH7720 || CPU_SUBTYPE_SH7721)
select FB_IOMEM_HELPERS
help
Acked-by: John Paul Adrian Glaubitz
Adrian
Re: [PATCH] drivers: video: logo: Don't mention the full path of the input in output
On 4/4/24 18:44, Lucas Stach wrote:
Am Donnerstag, dem 04.04.2024 um 15:15 +0200 schrieb Helge Deller:
On 4/4/24 14:18, Lucas Stach wrote:
This change strips $abs_srctree of the input file containing the
PNM data in the generated output. The motivation for this change
is Yocto emitting a build warning
WARNING: linux-foo-6.8-r0 do_package_qa: QA Issue:
File
/usr/src/debug/linux-foo/6.8-r0/drivers/video/logo/logo_linux_clut224.c
in package linux-foo-src contains reference to TMPDIR
So this change brings us one step closer to make the build result
reproducible independent of the build path.
Signed-off-by: Lucas Stach
---
drivers/video/logo/pnmtologo.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/video/logo/pnmtologo.c b/drivers/video/logo/pnmtologo.c
index 2434a25afb64..59ccd721e8af 100644
--- a/drivers/video/logo/pnmtologo.c
+++ b/drivers/video/logo/pnmtologo.c
@@ -223,6 +223,18 @@ static inline int is_equal(struct color c1, struct color
c2)
static void write_header(void)
{
+ const char *abs_srctree = getenv("abs_srctree");
+ const char *rel_filename;
+
+ if (abs_srctree &&
+ !strncmp(abs_srctree, filename, strlen(abs_srctree))) {
+ rel_filename = filename + strlen(abs_srctree);
+ while (*rel_filename == '/')
+ ++rel_filename;
+ } else {
+ rel_filename = filename;
+ }
+
/* open logo file */
if (outputname) {
out = fopen(outputname, "w");
@@ -235,7 +247,7 @@ static void write_header(void)
fputs("/*\n", out);
fputs(" * DO NOT EDIT THIS FILE!\n", out);
fputs(" *\n", out);
- fprintf(out, " * It was automatically generated from %s\n", filename);
+ fprintf(out, " * It was automatically generated from %s\n",
rel_filename);
can't you use instead: ?
+ fprintf(out, " * It was automatically generated from %s\n",
basename(filename));
The difference to basename is that this keeps the path in the source
tree intact, e.g. it shortens the absolute path to
"drivers/video/logo/logo_linux_clut224.c", so the comment in the
generated file still has a full reference to the file location in the
source tree. It only strips out the part of the path that is host
dependent.
That's true, but
a) it's just a comment which is generated, and
b) all source and generated logo files are in the
[src|build]/drivers/video/logo/ directory anyway, and
c) the file name already suggests where it is generated from.
So, IMHO basically we could simply drop the whole comment line alltogether as
well.
Helge
Re: [PATCH] drivers: video: logo: Don't mention the full path of the input in output
On 4/4/24 14:18, Lucas Stach wrote:
This change strips $abs_srctree of the input file containing the
PNM data in the generated output. The motivation for this change
is Yocto emitting a build warning
WARNING: linux-foo-6.8-r0 do_package_qa: QA Issue:
File
/usr/src/debug/linux-foo/6.8-r0/drivers/video/logo/logo_linux_clut224.c
in package linux-foo-src contains reference to TMPDIR
So this change brings us one step closer to make the build result
reproducible independent of the build path.
Signed-off-by: Lucas Stach
---
drivers/video/logo/pnmtologo.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/video/logo/pnmtologo.c b/drivers/video/logo/pnmtologo.c
index 2434a25afb64..59ccd721e8af 100644
--- a/drivers/video/logo/pnmtologo.c
+++ b/drivers/video/logo/pnmtologo.c
@@ -223,6 +223,18 @@ static inline int is_equal(struct color c1, struct color
c2)
static void write_header(void)
{
+ const char *abs_srctree = getenv("abs_srctree");
+ const char *rel_filename;
+
+ if (abs_srctree &&
+ !strncmp(abs_srctree, filename, strlen(abs_srctree))) {
+ rel_filename = filename + strlen(abs_srctree);
+ while (*rel_filename == '/')
+ ++rel_filename;
+ } else {
+ rel_filename = filename;
+ }
+
/* open logo file */
if (outputname) {
out = fopen(outputname, "w");
@@ -235,7 +247,7 @@ static void write_header(void)
fputs("/*\n", out);
fputs(" * DO NOT EDIT THIS FILE!\n", out);
fputs(" *\n", out);
- fprintf(out, " * It was automatically generated from %s\n", filename);
+ fprintf(out, " * It was automatically generated from %s\n",
rel_filename);
can't you use instead: ?
+ fprintf(out, " * It was automatically generated from %s\n",
basename(filename));
Helge
fputs(" *\n", out);
fprintf(out, " * Linux logo %s\n", logoname);
fputs(" */\n\n", out);
Re: [PATCH 13/34] sisfb: hide unused variables
On 4/3/24 10:06, Arnd Bergmann wrote:
From: Arnd Bergmann
Building with W=1 shows that a couple of variables in this driver are only
used in certain configurations:
drivers/video/fbdev/sis/init301.c:239:28: error: 'SiS_Part2CLVX_6' defined but
not used [-Werror=unused-const-variable=]
...
Fixes: 5908986ef348 ("video: fbdev: sis: avoid mismatched prototypes")
Signed-off-by: Arnd Bergmann
---
drivers/video/fbdev/sis/init301.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
applied to fbdev git tree.
Thanks!
Helge
Re: [PATCH v2 2/3] arch: Remove struct fb_info from video helpers
On 3/28/24 14:33, Thomas Zimmermann wrote: Am 28.03.24 um 12:04 schrieb Helge Deller: On 3/27/24 21:41, Thomas Zimmermann wrote: The per-architecture video helpers do not depend on struct fb_info or anything else from fbdev. Remove it from the interface and replace fb_is_primary_device() with video_is_primary_device(). The new helper Since you rename this function, wouldn't something similar to device_is_primary_display() or device_is_primary_console() or is_primary_graphics_device() or is_primary_display_device() be a better name? The video_ prefix is there to signal that the code is part of the video subsystem. But there's too much code that tried to figure out a default video device. So I actually have different plans for this function. I'd like to replace it with a helper that returns the default device instead of just testing for it. Sample code for x86 is already in vgaarb.c. [1] The function's name would then be video_default_device() and return the appropriate struct device*. video_is_primary device() will be removed. This rename here is the easiest step towards the new helper. Ok? Sounds ok. Helge
Re: [PATCH v2 3/3] arch: Rename fbdev header and source files
On 3/27/24 21:41, Thomas Zimmermann wrote:
The per-architecture fbdev code has no dependencies on fbdev and can
be used for any video-related subsystem. Rename the files to 'video'.
Use video-sti.c on parisc as the source file depends on CONFIG_STI_CORE.
Further update all includes statements, includ guards, and Makefiles.
Also update a few strings and comments to refer to video instead of
fbdev.
Signed-off-by: Thomas Zimmermann
Cc: Vineet Gupta
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Huacai Chen
Cc: WANG Xuerui
Cc: Geert Uytterhoeven
Cc: Thomas Bogendoerfer
Cc: "James E.J. Bottomley"
Cc: Helge Deller
Cc: Michael Ellerman
Cc: Nicholas Piggin
Cc: Yoshinori Sato
Cc: Rich Felker
Cc: John Paul Adrian Glaubitz
Cc: "David S. Miller"
Cc: Andreas Larsson
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: x...@kernel.org
Cc: "H. Peter Anvin"
---
arch/arc/include/asm/fb.h| 8
arch/arc/include/asm/video.h | 8
arch/arm/include/asm/fb.h| 6 --
arch/arm/include/asm/video.h | 6 ++
arch/arm64/include/asm/fb.h | 10 --
arch/arm64/include/asm/video.h | 10 ++
arch/loongarch/include/asm/{fb.h => video.h} | 8
arch/m68k/include/asm/{fb.h => video.h} | 8
arch/mips/include/asm/{fb.h => video.h} | 12 ++--
arch/parisc/include/asm/{fb.h => video.h}| 8
arch/parisc/video/Makefile | 2 +-
arch/parisc/video/{fbdev.c => video-sti.c} | 2 +-
arch/powerpc/include/asm/{fb.h => video.h} | 8
arch/powerpc/kernel/pci-common.c | 2 +-
arch/sh/include/asm/fb.h | 7 ---
arch/sh/include/asm/video.h | 7 +++
arch/sparc/include/asm/{fb.h => video.h} | 8
arch/sparc/video/Makefile| 2 +-
arch/sparc/video/{fbdev.c => video.c}| 4 ++--
arch/x86/include/asm/{fb.h => video.h} | 8
arch/x86/video/Makefile | 2 +-
arch/x86/video/{fbdev.c => video.c} | 3 ++-
include/asm-generic/Kbuild | 2 +-
include/asm-generic/{fb.h => video.h}| 6 +++---
include/linux/fb.h | 2 +-
25 files changed, 75 insertions(+), 74 deletions(-)
delete mode 100644 arch/arc/include/asm/fb.h
create mode 100644 arch/arc/include/asm/video.h
delete mode 100644 arch/arm/include/asm/fb.h
create mode 100644 arch/arm/include/asm/video.h
delete mode 100644 arch/arm64/include/asm/fb.h
create mode 100644 arch/arm64/include/asm/video.h
rename arch/loongarch/include/asm/{fb.h => video.h} (86%)
rename arch/m68k/include/asm/{fb.h => video.h} (86%)
rename arch/mips/include/asm/{fb.h => video.h} (76%)
rename arch/parisc/include/asm/{fb.h => video.h} (68%)
rename arch/parisc/video/{fbdev.c => video-sti.c} (96%)
rename arch/powerpc/include/asm/{fb.h => video.h} (76%)
delete mode 100644 arch/sh/include/asm/fb.h
create mode 100644 arch/sh/include/asm/video.h
rename arch/sparc/include/asm/{fb.h => video.h} (89%)
rename arch/sparc/video/{fbdev.c => video.c} (86%)
rename arch/x86/include/asm/{fb.h => video.h} (77%)
rename arch/x86/video/{fbdev.c => video.c} (97%)
rename include/asm-generic/{fb.h => video.h} (96%)
diff --git a/arch/arc/include/asm/fb.h b/arch/arc/include/asm/fb.h
deleted file mode 100644
index 9c2383d29cbb9..0
--- a/arch/arc/include/asm/fb.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-#ifndef _ASM_FB_H_
-#define _ASM_FB_H_
-
-#include
-
-#endif /* _ASM_FB_H_ */
diff --git a/arch/arc/include/asm/video.h b/arch/arc/include/asm/video.h
new file mode 100644
index 0..8ff7263727fe7
--- /dev/null
+++ b/arch/arc/include/asm/video.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_VIDEO_H_
+#define _ASM_VIDEO_H_
+
+#include
+
+#endif /* _ASM_VIDEO_H_ */
I wonder, since that file simply #includes the generic version,
wasn't there a possibility that kbuild could symlink
the generic version for us?
Does it need to be mandatory in include/asm-generic/Kbuild ?
Same applies to a few other files below.
Helge
diff --git a/arch/arm/include/asm/fb.h b/arch/arm/include/asm/fb.h
deleted file mode 100644
index ce20a43c30339..0
--- a/arch/arm/include/asm/fb.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifndef _ASM_FB_H_
-#define _ASM_FB_H_
-
-#include
-
-#endif /* _ASM_FB_H_ */
diff --git a/arch/arm/include/asm/video.h b/arch/arm/include/asm/video.h
new file mode 100644
index 0..f570565366e67
--- /dev/null
+++ b/arch/arm/include/asm/video.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_VIDEO_H_
+#define _ASM_VIDEO_H_
+
+#include
+
+#endif /* _ASM_VIDEO_H_ */
diff --git a/arch/a
Re: [PATCH v2 1/3] arch: Select fbdev helpers with CONFIG_VIDEO
On 3/27/24 21:41, Thomas Zimmermann wrote: Various Kconfig options selected the per-architecture helpers for fbdev. But none of the contained code depends on fbdev. Standardize on CONFIG_VIDEO, which will allow to add more general helpers for video functionality. CONFIG_VIDEO protects each architecture's video/ directory. Your patch in general looks good. But is renaming the config option from CONFIG_FB_CORE to CONFIG_VIDEO the best choice? CONFIG_VIDEO might be mixed up with multimedia/video-streaming. Why not e.g. CONFIG_GRAPHICS_CORE? I'm fine with CONFIG_VIDEO as well, but if someone has a better idea we maybe should go with that instead now? Helge This allows for the use of more fine-grained control for each directory's files, such as the use of CONFIG_STI_CORE on parisc. v2: - sparc: rebased onto Makefile changes Signed-off-by: Thomas Zimmermann Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: "David S. Miller" Cc: Andreas Larsson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: x...@kernel.org Cc: "H. Peter Anvin" --- arch/parisc/Makefile | 2 +- arch/sparc/Makefile | 4 ++-- arch/sparc/video/Makefile | 2 +- arch/x86/Makefile | 2 +- arch/x86/video/Makefile | 3 ++- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile index 316f84f1d15c8..21b8166a68839 100644 --- a/arch/parisc/Makefile +++ b/arch/parisc/Makefile @@ -119,7 +119,7 @@ export LIBGCC libs-y+= arch/parisc/lib/ $(LIBGCC) -drivers-y += arch/parisc/video/ +drivers-$(CONFIG_VIDEO) += arch/parisc/video/ boot := arch/parisc/boot diff --git a/arch/sparc/Makefile b/arch/sparc/Makefile index 2a03daa68f285..757451c3ea1df 100644 --- a/arch/sparc/Makefile +++ b/arch/sparc/Makefile @@ -59,8 +59,8 @@ endif libs-y += arch/sparc/prom/ libs-y += arch/sparc/lib/ -drivers-$(CONFIG_PM) += arch/sparc/power/ -drivers-$(CONFIG_FB_CORE) += arch/sparc/video/ +drivers-$(CONFIG_PM)+= arch/sparc/power/ +drivers-$(CONFIG_VIDEO) += arch/sparc/video/ boot := arch/sparc/boot diff --git a/arch/sparc/video/Makefile b/arch/sparc/video/Makefile index d4d83f1702c61..9dd82880a027a 100644 --- a/arch/sparc/video/Makefile +++ b/arch/sparc/video/Makefile @@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_FB_CORE) += fbdev.o +obj-y += fbdev.o diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 15a5f4f2ff0aa..c0ea612c62ebe 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -265,7 +265,7 @@ drivers-$(CONFIG_PCI)+= arch/x86/pci/ # suspend and hibernation support drivers-$(CONFIG_PM) += arch/x86/power/ -drivers-$(CONFIG_FB_CORE) += arch/x86/video/ +drivers-$(CONFIG_VIDEO) += arch/x86/video/ # boot loader support. Several targets are kept for legacy purposes diff --git a/arch/x86/video/Makefile b/arch/x86/video/Makefile index 5ebe48752ffc4..9dd82880a027a 100644 --- a/arch/x86/video/Makefile +++ b/arch/x86/video/Makefile @@ -1,2 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_FB_CORE) += fbdev.o + +obj-y += fbdev.o
Re: [PATCH v2 2/3] arch: Remove struct fb_info from video helpers
On 3/27/24 21:41, Thomas Zimmermann wrote:
The per-architecture video helpers do not depend on struct fb_info
or anything else from fbdev. Remove it from the interface and replace
fb_is_primary_device() with video_is_primary_device(). The new helper
Since you rename this function, wouldn't something similar to
device_is_primary_display()
or
device_is_primary_console()
or
is_primary_graphics_device()
or
is_primary_display_device()
be a better name?
Helge
is similar in functionality, but can operate on non-fbdev devices.
Signed-off-by: Thomas Zimmermann
Cc: "James E.J. Bottomley"
Cc: Helge Deller
Cc: "David S. Miller"
Cc: Andreas Larsson
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: x...@kernel.org
Cc: "H. Peter Anvin"
---
arch/parisc/include/asm/fb.h | 8 +---
arch/parisc/video/fbdev.c| 9 +
arch/sparc/include/asm/fb.h | 7 ---
arch/sparc/video/fbdev.c | 17 -
arch/x86/include/asm/fb.h| 8 +---
arch/x86/video/fbdev.c | 18 +++---
drivers/video/fbdev/core/fbcon.c | 2 +-
include/asm-generic/fb.h | 11 ++-
8 files changed, 41 insertions(+), 39 deletions(-)
diff --git a/arch/parisc/include/asm/fb.h b/arch/parisc/include/asm/fb.h
index 658a8a7dc5312..ed2a195a3e762 100644
--- a/arch/parisc/include/asm/fb.h
+++ b/arch/parisc/include/asm/fb.h
@@ -2,11 +2,13 @@
#ifndef _ASM_FB_H_
#define _ASM_FB_H_
-struct fb_info;
+#include
+
+struct device;
#if defined(CONFIG_STI_CORE)
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
#endif
#include
diff --git a/arch/parisc/video/fbdev.c b/arch/parisc/video/fbdev.c
index e4f8ac99fc9e0..540fa0c919d59 100644
--- a/arch/parisc/video/fbdev.c
+++ b/arch/parisc/video/fbdev.c
@@ -5,12 +5,13 @@
* Copyright (C) 2001-2002 Thomas Bogendoerfer
*/
-#include
#include
#include
-int fb_is_primary_device(struct fb_info *info)
+#include
+
+bool video_is_primary_device(struct device *dev)
{
struct sti_struct *sti;
@@ -21,6 +22,6 @@ int fb_is_primary_device(struct fb_info *info)
return true;
/* return true if it's the default built-in framebuffer driver */
- return (sti->dev == info->device);
+ return (sti->dev == dev);
}
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);
diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h
index 24440c0fda490..07f0325d6921c 100644
--- a/arch/sparc/include/asm/fb.h
+++ b/arch/sparc/include/asm/fb.h
@@ -3,10 +3,11 @@
#define _SPARC_FB_H_
#include
+#include
#include
-struct fb_info;
+struct device;
#ifdef CONFIG_SPARC32
static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
@@ -18,8 +19,8 @@ static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
#define pgprot_framebuffer pgprot_framebuffer
#endif
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
static inline void fb_memcpy_fromio(void *to, const volatile void __iomem
*from, size_t n)
{
diff --git a/arch/sparc/video/fbdev.c b/arch/sparc/video/fbdev.c
index bff66dd1909a4..e46f0499c2774 100644
--- a/arch/sparc/video/fbdev.c
+++ b/arch/sparc/video/fbdev.c
@@ -1,26 +1,25 @@
// SPDX-License-Identifier: GPL-2.0
#include
-#include
+#include
#include
+#include
#include
-int fb_is_primary_device(struct fb_info *info)
+bool video_is_primary_device(struct device *dev)
{
- struct device *dev = info->device;
- struct device_node *node;
+ struct device_node *node = dev->of_node;
if (console_set_on_cmdline)
- return 0;
+ return false;
- node = dev->of_node;
if (node && node == of_console_device)
- return 1;
+ return true;
- return 0;
+ return false;
}
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);
MODULE_DESCRIPTION("Sparc fbdev helpers");
MODULE_LICENSE("GPL");
diff --git a/arch/x86/include/asm/fb.h b/arch/x86/include/asm/fb.h
index c3b9582de7efd..999db33792869 100644
--- a/arch/x86/include/asm/fb.h
+++ b/arch/x86/include/asm/fb.h
@@ -2,17 +2,19 @@
#ifndef _ASM_X86_FB_H
#define _ASM_X86_FB_H
+#include
+
#include
-struct fb_info;
+struct device;
pgprot_t pgprot_framebuffer(pgprot_t prot,
unsigned long vm_start, unsigned long vm_end,
unsigned long offset);
#define pgprot_framebuffer pgprot_framebuffer
-int fb_is_primary_device(struct fb_info *info);
-#de
Re: [PATCH 1/9] fbdev: shmobile: fix snprintf truncation
On 3/26/24 23:38, Arnd Bergmann wrote:
From: Arnd Bergmann
The name of the overlay does not fit into the fixed-length field:
drivers/video/fbdev/sh_mobile_lcdcfb.c:1577:2: error: 'snprintf' will always be
truncated; specified size is 16, but format string expands to at least 25
Make it short enough by changing the string.
Fixes: c5deac3c9b22 ("fbdev: sh_mobile_lcdc: Implement overlays support")
Signed-off-by: Arnd Bergmann
applied to fbdev git tree.
Thanks!
Helge
---
drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c
b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index eb2297b37504..d35d2cf8 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -1575,7 +1575,7 @@ sh_mobile_lcdc_overlay_fb_init(struct
sh_mobile_lcdc_overlay *ovl)
*/
info->fix = sh_mobile_lcdc_overlay_fix;
snprintf(info->fix.id, sizeof(info->fix.id),
-"SH Mobile LCDC Overlay %u", ovl->index);
+"SHMobile ovl %u", ovl->index);
info->fix.smem_start = ovl->dma_handle;
info->fix.smem_len = ovl->fb_size;
info->fix.line_length = ovl->pitch;
Re: [PATCH 2/2] video: fbdev: replace of_graph_get_next_endpoint()
On 3/25/24 03:57, Kuninori Morimoto wrote:
From DT point of view, in general, drivers should be asking for a
specific port number because their function is fixed in the binding.
of_graph_get_next_endpoint() doesn't match to this concept.
Simply replace
- of_graph_get_next_endpoint(xxx, NULL);
+ of_graph_get_endpoint_by_regs(xxx, 0, -1);
Link: https://lore.kernel.org/r/20240202174941.ga310089-r...@kernel.org
Signed-off-by: Kuninori Morimoto
Reviewed-by: Laurent Pinchart
applied to fbdev git tree.
Thanks!
Helge
---
drivers/video/fbdev/omap2/omapfb/dss/dsi.c| 3 ++-
drivers/video/fbdev/omap2/omapfb/dss/dss-of.c | 20 +--
drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c | 3 ++-
drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c | 3 ++-
drivers/video/fbdev/omap2/omapfb/dss/venc.c | 3 ++-
drivers/video/fbdev/pxafb.c | 2 +-
include/video/omapfb_dss.h| 3 ---
7 files changed, 10 insertions(+), 27 deletions(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
index b7eb17a16ec4..1f13bcf73da5 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
@@ -5079,7 +5080,7 @@ static int dsi_probe_of(struct platform_device *pdev)
struct device_node *ep;
struct omap_dsi_pin_config pin_cfg;
- ep = omapdss_of_get_first_endpoint(node);
+ ep = of_graph_get_endpoint_by_regs(node, 0, -1);
if (!ep)
return 0;
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
b/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
index 0282d4eef139..14965a3fd05b 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
@@ -130,24 +130,6 @@ static struct device_node
*omapdss_of_get_remote_port(const struct device_node *
return np;
}
-struct device_node *
-omapdss_of_get_first_endpoint(const struct device_node *parent)
-{
- struct device_node *port, *ep;
-
- port = omapdss_of_get_next_port(parent, NULL);
-
- if (!port)
- return NULL;
-
- ep = omapdss_of_get_next_endpoint(port, NULL);
-
- of_node_put(port);
-
- return ep;
-}
-EXPORT_SYMBOL_GPL(omapdss_of_get_first_endpoint);
-
struct omap_dss_device *
omapdss_of_find_source_for_first_ep(struct device_node *node)
{
@@ -155,7 +137,7 @@ omapdss_of_find_source_for_first_ep(struct device_node
*node)
struct device_node *src_port;
struct omap_dss_device *src;
- ep = omapdss_of_get_first_endpoint(node);
+ ep = of_graph_get_endpoint_by_regs(node, 0, -1);
if (!ep)
return ERR_PTR(-EINVAL);
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
index f05b4e35a842..8f407ec134dc 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -529,7 +530,7 @@ static int hdmi_probe_of(struct platform_device *pdev)
struct device_node *ep;
int r;
- ep = omapdss_of_get_first_endpoint(node);
+ ep = of_graph_get_endpoint_by_regs(node, 0, -1);
if (!ep)
return 0;
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
index 03292945b1d4..4ad219f522b9 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -561,7 +562,7 @@ static int hdmi_probe_of(struct platform_device *pdev)
struct device_node *ep;
int r;
- ep = omapdss_of_get_first_endpoint(node);
+ ep = of_graph_get_endpoint_by_regs(node, 0, -1);
if (!ep)
return 0;
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/venc.c
b/drivers/video/fbdev/omap2/omapfb/dss/venc.c
index c9d40e28a06f..0bd80d3b8f1b 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/venc.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/venc.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
@@ -764,7 +765,7 @@ static int venc_probe_of(struct platform_device *pdev)
u32 channels;
int r;
- ep = omapdss_of_get_first_endpoint(node);
+ ep = of_graph_get_endpoint_by_regs(node, 0, -1);
if (!ep)
return 0;
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index fa943612c4e2..2ef56fa28aff 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -2171,7 +2171,7 @@ static int of_get_pxafb_mode_info(struct device *dev,
u32 bus_width;
int ret, i;
-
[GIT PULL] fbdev fixes and cleanups for v6.9-rc1 (v2)
Hi Linus, Yesterday's fbdev tree showed one small compiler warning which has been fixed in this pull request. So, please pull the latest fixes and cleanups for the fbdev drivers for kernel 6.9-rc1. Beside the typical bunch of smaller fixes, the Linux console now allows fonts up to a size of 64 x 128 pixels. Thanks! Helge The following changes since commit 41bccc98fb7931d63d03f326a746ac4d429c1dd3: Linux 6.8-rc2 (2024-01-28 17:01:12 -0800) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.9-rc1 for you to fetch changes up to 0688d3b1d882dd1dcf73305306e71ebf1653f595: fbdev: panel-tpo-td043mtea1: Convert sprintf() to sysfs_emit() (2024-03-20 09:02:32 +0100) fbdev fixes and cleanups for 6.9-rc1: - Allow console fonts up to 64x128 pixels (Samuel Thibault) - Prevent division-by-zero in fb monitor code (Roman Smirnov) - Drop Renesas ARM platforms from Mobile LCDC framebuffer driver (Geert Uytterhoeven) - Various code cleanups in viafb, uveafb and mb862xxfb drivers by Aleksandr Burakov, Li Zhijian and Michael Ellerman Aleksandr Burakov (1): fbdev: viafb: fix typo in hw_bitblt_1 and hw_bitblt_2 Geert Uytterhoeven (1): fbdev: Restrict FB_SH_MOBILE_LCDC to SuperH Li Zhijian (2): fbdev: uvesafb: Convert sprintf/snprintf to sysfs_emit fbdev: panel-tpo-td043mtea1: Convert sprintf() to sysfs_emit() Michael Ellerman (1): fbdev: mb862xxfb: Fix defined but not used error Roman Smirnov (1): fbmon: prevent division by zero in fb_videomode_from_videomode() Samuel Thibault (1): fbcon: Increase maximum font width x height to 64 x 128 drivers/firmware/efi/earlycon.c| 2 +- drivers/video/fbdev/Kconfig| 2 +- drivers/video/fbdev/arkfb.c| 15 +++ drivers/video/fbdev/core/fbcon.c | 16 +--- drivers/video/fbdev/core/fbmem.c | 12 ++-- drivers/video/fbdev/core/fbmon.c | 7 --- drivers/video/fbdev/core/svgalib.c | 15 +++ drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 18 +- .../fbdev/omap2/omapfb/displays/panel-tpo-td043mtea1.c | 12 drivers/video/fbdev/s3fb.c | 15 +++ drivers/video/fbdev/uvesafb.c | 2 +- drivers/video/fbdev/vga16fb.c | 6 +- drivers/video/fbdev/via/accel.c| 4 ++-- drivers/video/fbdev/vt8623fb.c | 15 +++ drivers/video/sticore.c| 2 +- include/linux/fb.h | 18 -- include/linux/font.h | 3 ++- lib/fonts/fonts.c | 15 +-- 18 files changed, 110 insertions(+), 69 deletions(-)
Re: [GIT PULL] fbdev fixes and cleanups for v6.9-rc1
On 3/21/24 19:29, Helge Deller wrote: Hi Linus, please pull fixes and cleanups for the fbdev subsystem for kernel 6.9-rc1. Linus, please wait There is one new "unused variable warning" introduced with those patches. I'll fix it up and send a new pull request tomorrow... Helge Beside the typical bunch of smaller fixes, the Linux console now allows fonts up to a size of 64 x 128 pixels. Thanks! Helge The following changes since commit 41bccc98fb7931d63d03f326a746ac4d429c1dd3: Linux 6.8-rc2 (2024-01-28 17:01:12 -0800) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.9-rc1 for you to fetch changes up to 0688d3b1d882dd1dcf73305306e71ebf1653f595: fbdev: panel-tpo-td043mtea1: Convert sprintf() to sysfs_emit() (2024-03-20 09:02:32 +0100) fbdev fixes and cleanups for 6.9-rc1: - Allow console fonts up to 64x128 pixels (Samuel Thibault) - Prevent division-by-zero in fb monitor code (Roman Smirnov) - Drop Renesas ARM platforms from Mobile LCDC framebuffer driver (Geert Uytterhoeven) - Various code cleanups in viafb, uveafb and mb862xxfb drivers by Aleksandr Burakov, Li Zhijian and Michael Ellerman Aleksandr Burakov (1): fbdev: viafb: fix typo in hw_bitblt_1 and hw_bitblt_2 Geert Uytterhoeven (1): fbdev: Restrict FB_SH_MOBILE_LCDC to SuperH Li Zhijian (2): fbdev: uvesafb: Convert sprintf/snprintf to sysfs_emit fbdev: panel-tpo-td043mtea1: Convert sprintf() to sysfs_emit() Michael Ellerman (1): fbdev: mb862xxfb: Fix defined but not used error Roman Smirnov (1): fbmon: prevent division by zero in fb_videomode_from_videomode() Samuel Thibault (1): fbcon: Increase maximum font width x height to 64 x 128 drivers/firmware/efi/earlycon.c| 2 +- drivers/video/fbdev/Kconfig| 2 +- drivers/video/fbdev/arkfb.c| 15 +++ drivers/video/fbdev/core/fbcon.c | 16 +--- drivers/video/fbdev/core/fbmem.c | 12 ++-- drivers/video/fbdev/core/fbmon.c | 7 --- drivers/video/fbdev/core/svgalib.c | 15 +++ drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 18 +- .../fbdev/omap2/omapfb/displays/panel-tpo-td043mtea1.c | 12 drivers/video/fbdev/s3fb.c | 15 +++ drivers/video/fbdev/uvesafb.c | 2 +- drivers/video/fbdev/vga16fb.c | 6 +- drivers/video/fbdev/via/accel.c| 4 ++-- drivers/video/fbdev/vt8623fb.c | 15 +++ drivers/video/sticore.c| 2 +- include/linux/fb.h | 18 -- include/linux/font.h | 3 ++- lib/fonts/fonts.c | 15 +-- 18 files changed, 110 insertions(+), 69 deletions(-)
[GIT PULL] fbdev fixes and cleanups for v6.9-rc1
Hi Linus, please pull fixes and cleanups for the fbdev subsystem for kernel 6.9-rc1. Beside the typical bunch of smaller fixes, the Linux console now allows fonts up to a size of 64 x 128 pixels. Thanks! Helge The following changes since commit 41bccc98fb7931d63d03f326a746ac4d429c1dd3: Linux 6.8-rc2 (2024-01-28 17:01:12 -0800) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.9-rc1 for you to fetch changes up to 0688d3b1d882dd1dcf73305306e71ebf1653f595: fbdev: panel-tpo-td043mtea1: Convert sprintf() to sysfs_emit() (2024-03-20 09:02:32 +0100) fbdev fixes and cleanups for 6.9-rc1: - Allow console fonts up to 64x128 pixels (Samuel Thibault) - Prevent division-by-zero in fb monitor code (Roman Smirnov) - Drop Renesas ARM platforms from Mobile LCDC framebuffer driver (Geert Uytterhoeven) - Various code cleanups in viafb, uveafb and mb862xxfb drivers by Aleksandr Burakov, Li Zhijian and Michael Ellerman Aleksandr Burakov (1): fbdev: viafb: fix typo in hw_bitblt_1 and hw_bitblt_2 Geert Uytterhoeven (1): fbdev: Restrict FB_SH_MOBILE_LCDC to SuperH Li Zhijian (2): fbdev: uvesafb: Convert sprintf/snprintf to sysfs_emit fbdev: panel-tpo-td043mtea1: Convert sprintf() to sysfs_emit() Michael Ellerman (1): fbdev: mb862xxfb: Fix defined but not used error Roman Smirnov (1): fbmon: prevent division by zero in fb_videomode_from_videomode() Samuel Thibault (1): fbcon: Increase maximum font width x height to 64 x 128 drivers/firmware/efi/earlycon.c| 2 +- drivers/video/fbdev/Kconfig| 2 +- drivers/video/fbdev/arkfb.c| 15 +++ drivers/video/fbdev/core/fbcon.c | 16 +--- drivers/video/fbdev/core/fbmem.c | 12 ++-- drivers/video/fbdev/core/fbmon.c | 7 --- drivers/video/fbdev/core/svgalib.c | 15 +++ drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 18 +- .../fbdev/omap2/omapfb/displays/panel-tpo-td043mtea1.c | 12 drivers/video/fbdev/s3fb.c | 15 +++ drivers/video/fbdev/uvesafb.c | 2 +- drivers/video/fbdev/vga16fb.c | 6 +- drivers/video/fbdev/via/accel.c| 4 ++-- drivers/video/fbdev/vt8623fb.c | 15 +++ drivers/video/sticore.c| 2 +- include/linux/fb.h | 18 -- include/linux/font.h | 3 ++- lib/fonts/fonts.c | 15 +-- 18 files changed, 110 insertions(+), 69 deletions(-)
Re: [PATCH] video: fbdev: au1200fb: replace deprecated strncpy with strscpy
On 3/20/24 23:35, Justin Stitt wrote:
Hi,
On Wed, Mar 20, 2024 at 12:56 AM Helge Deller wrote:
On 3/19/24 00:46, Justin Stitt wrote:
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
Let's use the new 2-argument strscpy() which guarantees NUL-termination
on the destination buffer while also simplifying the syntax. Note that
strscpy() will not NUL-pad the destination buffer like strncpy() does.
However, the NUL-padding behavior of strncpy() is not required since
fbdev is already NUL-allocated from au1200fb_drv_probe() ->
frameuffer_alloc(), rendering any additional NUL-padding redundant.
| p = kzalloc(fb_info_size + size, GFP_KERNEL);
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
Found with: $ rg "strncpy\("
---
drivers/video/fbdev/au1200fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c
index 6f20efc663d7..e718fea63662 100644
--- a/drivers/video/fbdev/au1200fb.c
+++ b/drivers/video/fbdev/au1200fb.c
@@ -1557,7 +1557,7 @@ static int au1200fb_init_fbinfo(struct au1200fb_device
*fbdev)
return ret;
}
- strncpy(fbi->fix.id, "AU1200", sizeof(fbi->fix.id));
+ strscpy(fbi->fix.id, "AU1200");
I wonder if you really build-tested this, as this driver is for the mips
architecture...
And I don't see a strscpy() function which takes just 2 arguments.
But I might be wrong
I did build successfully :thumbs_up:
Commit e6584c3964f2f ("string: Allow 2-argument strscpy()") introduced
this new strscpy() form; it is present in string.h on Linus' tree.
Interesting patch.
Might give compile problems if patches like yours gets automatically
picked up to stable series as long as Kees patch hasn't been backported yet...
Anyway, thanks for the pointer!
I'll apply your patch in the next round for fbdev.
Helge
Re: [PATCH v3] video: fbdev: panel-tpo-td043mtea1: Convert sprintf() family to sysfs_emit() family
On 3/19/24 10:23, Li Zhijian wrote:
Per filesystems/sysfs.rst, show() should only use sysfs_emit()
or sysfs_emit_at() when formatting the value to be returned to user space.
coccinelle complains that there are still a couple of functions that use
snprintf(). Convert them to sysfs_emit().
sprintf() and scnprintf() will be converted as well if they have.
Generally, this patch is generated by
make coccicheck M= MODE=patch \
COCCI=scripts/coccinelle/api/device_attr_show.cocci
No functional change intended
CC: Helge Deller
CC: linux-o...@vger.kernel.org
CC: linux-fb...@vger.kernel.org
CC: dri-devel@lists.freedesktop.org
Signed-off-by: Li Zhijian
applied to fbdev git tree.
Thanks!
Helge
---
V3:
split it from a mess of drm,fbdev, becuase they are not the same subsystem.
V2:
Fix missing '+' before '=' in
drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td043mtea1.c
This is a part of the work "Fix coccicheck device_attr_show warnings"[1]
Split them per subsystem so that the maintainer can review it easily
[1] https://lore.kernel.org/lkml/20240116041129.3937800-1-lizhij...@fujitsu.com/
---
.../omap2/omapfb/displays/panel-tpo-td043mtea1.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td043mtea1.c
b/drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td043mtea1.c
index 477789cff8e0..3624452e1dd0 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td043mtea1.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td043mtea1.c
@@ -228,14 +228,10 @@ static ssize_t tpo_td043_gamma_show(struct device *dev,
int ret;
int i;
- for (i = 0; i < ARRAY_SIZE(ddata->gamma); i++) {
- ret = snprintf(buf + len, PAGE_SIZE - len, "%u ",
- ddata->gamma[i]);
- if (ret < 0)
- return ret;
- len += ret;
- }
- buf[len - 1] = '\n';
+ for (i = 0; i < ARRAY_SIZE(ddata->gamma); i++)
+ len += sysfs_emit_at(buf, len, "%u ", ddata->gamma[i]);
+ if (len)
+ buf[len - 1] = '\n';
return len;
}
Re: [PATCH] video: fbdev: au1200fb: replace deprecated strncpy with strscpy
On 3/19/24 00:46, Justin Stitt wrote:
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
Let's use the new 2-argument strscpy() which guarantees NUL-termination
on the destination buffer while also simplifying the syntax. Note that
strscpy() will not NUL-pad the destination buffer like strncpy() does.
However, the NUL-padding behavior of strncpy() is not required since
fbdev is already NUL-allocated from au1200fb_drv_probe() ->
frameuffer_alloc(), rendering any additional NUL-padding redundant.
| p = kzalloc(fb_info_size + size, GFP_KERNEL);
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
Found with: $ rg "strncpy\("
---
drivers/video/fbdev/au1200fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c
index 6f20efc663d7..e718fea63662 100644
--- a/drivers/video/fbdev/au1200fb.c
+++ b/drivers/video/fbdev/au1200fb.c
@@ -1557,7 +1557,7 @@ static int au1200fb_init_fbinfo(struct au1200fb_device
*fbdev)
return ret;
}
- strncpy(fbi->fix.id, "AU1200", sizeof(fbi->fix.id));
+ strscpy(fbi->fix.id, "AU1200");
I wonder if you really build-tested this, as this driver is for the mips
architecture...
And I don't see a strscpy() function which takes just 2 arguments.
But I might be wrong
Helge
Re: [PATCH v3] fbmon: prevent division by zero in fb_videomode_from_videomode()
On 3/19/24 09:13, Roman Smirnov wrote:
The expression htotal * vtotal can have a zero value on
overflow. It is necessary to prevent division by zero like in
fb_var_to_videomode().
Found by Linux Verification Center (linuxtesting.org) with Svace.
Signed-off-by: Roman Smirnov
Reviewed-by: Sergey Shtylyov
---
V1 -> V2: Replaced the code of the first version with a check.
V2 -> V3: Replaced the code of the second version with a zero check
drivers/video/fbdev/core/fbmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 79e5bfbdd34c..ca946919d962 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -1344,7 +1344,7 @@ int fb_videomode_from_videomode(const struct videomode
*vm,
vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
vm->vsync_len;
/* prevent division by zero */
- if (htotal && vtotal) {
+ if (htotal && vtotal && (htotal * vtotal)) {
fbmode->refresh = vm->pixelclock / (htotal * vtotal);
I modified your patch like this:
...
- if (htotal && vtotal) {
- fbmode->refresh = vm->pixelclock / (htotal * vtotal);
+ total = htotal * vtotal;
+ if (total) {
+ fbmode->refresh = vm->pixelclock / total;
and added it to the fbdev git tree.
https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next
Thanks,
Helge
Re: [PATCH v2] fbmon: prevent division by zero in fb_videomode_from_videomode()
On 3/18/24 09:11, Roman Smirnov wrote:
On Fri, 15 Mar 2024 09:44:08 +0100 Helge Deller wrote:
On 3/5/24 14:51, Roman Smirnov wrote:
The expression htotal * vtotal can have a zero value on
overflow.
I'm not sure if those always results in zero in kernel on overflow.
Might be architecture-depended too, but let's assume it
can become zero,
It is necessary to prevent division by zero like in
fb_var_to_videomode().
Found by Linux Verification Center (linuxtesting.org) with Svace.
Signed-off-by: Roman Smirnov
Reviewed-by: Sergey Shtylyov
---
V1 -> V2: Replaced the code of the first version with a check.
drivers/video/fbdev/core/fbmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 79e5bfbdd34c..b137590386da 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -1344,7 +1344,7 @@ int fb_videomode_from_videomode(const struct videomode
*vm,
vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
vm->vsync_len;
/* prevent division by zero */
- if (htotal && vtotal) {
+ if (htotal && vtotal && (vm->pixelclock / htotal >= vtotal)) {
why don't you then simply check for
if .. ((htotal * vtotal) == 0) ...
instead?
Helge
Thomas Zimmermann from the previous discussion said:
On Tue, 5 Mar 2024 11:18:05 +0100 Thomas Zimmerman wrote:
Maybe use
if (htotal && vtotal && (vm->pixelclock / htotal >= vtotal))
for the test. That rules out overflowing multiplication and sets
refresh to 0 in such cases.
This prevents overflow, which is also a problematic case.
I don't like adding another division here and I doubt we have
a problem with possible overflow.
So, I suggest to keep it simple, something like:
...
total = htotal * vtotal;
if (total)
fbmode->refresh = vm->pixelclock / total;
else...
Helge
Re: [PATCHv2] fbcon: Increase maximum font width x height to 64 x 128
On 3/16/24 01:10, Samuel Thibault wrote:
By using bitmaps we actually support whatever size we would want, but the
console currently limits fonts to 64x128 (which gives 60x16 text on 4k
screens), so we don't need more for now, and we can easily increase later.
Signed-off-by: Samuel Thibault
I cleaned up the tabs & spaces in your patch, fixed the missing
get_default_font() change in sticore.c and applied it to the fbdev git tree.
Let's keep it for a few days in for-next...
Thanks!
Helge
---
Difference from v1:
- use a bitmap rather than just extending the integer size
- add missing updates in drivers
---
drivers/firmware/efi/earlycon.c| 2 +-
drivers/video/fbdev/arkfb.c| 15 +++
drivers/video/fbdev/core/fbcon.c | 16 +---
drivers/video/fbdev/core/fbmem.c | 12 ++--
drivers/video/fbdev/core/svgalib.c | 15 +++
drivers/video/fbdev/s3fb.c | 15 +++
drivers/video/fbdev/vga16fb.c | 6 +-
drivers/video/fbdev/vt8623fb.c | 15 +++
include/linux/fb.h | 18 --
include/linux/font.h | 3 ++-
lib/fonts/fonts.c | 15 +--
11 files changed, 88 insertions(+), 44 deletions(-)
diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
index f80a9af3d16e..d18a1a5de144 100644
--- a/drivers/firmware/efi/earlycon.c
+++ b/drivers/firmware/efi/earlycon.c
@@ -252,7 +252,7 @@ static int __init efi_earlycon_setup(struct earlycon_device
*device,
if (si->lfb_depth != 32)
return -ENODEV;
- font = get_default_font(xres, yres, -1, -1);
+ font = get_default_font(xres, yres, NULL, NULL);
if (!font)
return -ENODEV;
diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index dca9c0325b3f..082501feceb9 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -622,8 +622,13 @@ static int arkfb_set_par(struct fb_info *info)
info->tileops = NULL;
/* in 4bpp supports 8p wide tiles only, any tiles otherwise */
- info->pixmap.blit_x = (bpp == 4) ? (1 << (8 - 1)) : (~(u32)0);
- info->pixmap.blit_y = ~(u32)0;
+ if (bpp == 4) {
+ bitmap_zero(info->pixmap.blit_x, FB_MAX_BLIT_WIDTH);
+ set_bit(8 - 1, info->pixmap.blit_x);
+ } else {
+ bitmap_fill(info->pixmap.blit_x, FB_MAX_BLIT_WIDTH);
+ }
+ bitmap_fill(info->pixmap.blit_y, FB_MAX_BLIT_HEIGHT);
offset_value = (info->var.xres_virtual * bpp) / 64;
screen_size = info->var.yres_virtual * info->fix.line_length;
@@ -635,8 +640,10 @@ static int arkfb_set_par(struct fb_info *info)
info->tileops = _tile_ops;
/* supports 8x16 tiles only */
- info->pixmap.blit_x = 1 << (8 - 1);
- info->pixmap.blit_y = 1 << (16 - 1);
+ bitmap_zero(info->pixmap.blit_x, FB_MAX_BLIT_WIDTH);
+ set_bit(8 - 1, info->pixmap.blit_x);
+ bitmap_zero(info->pixmap.blit_y, FB_MAX_BLIT_HEIGHT);
+ set_bit(16 - 1, info->pixmap.blit_y);
offset_value = info->var.xres_virtual / 16;
screen_size = (info->var.xres_virtual * info->var.yres_virtual)
/ 64;
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 46823c2e2ba1..72ff3147a3bf 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2483,12 +2483,12 @@ static int fbcon_set_font(struct vc_data *vc, struct
console_font *font,
h > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
return -EINVAL;
- if (font->width > 32 || font->height > 32)
+ if (font->width > FB_MAX_BLIT_WIDTH || font->height >
FB_MAX_BLIT_HEIGHT)
return -EINVAL;
/* Make sure drawing engine can handle the font */
- if (!(info->pixmap.blit_x & BIT(font->width - 1)) ||
- !(info->pixmap.blit_y & BIT(font->height - 1)))
+ if (!test_bit(font->width - 1, info->pixmap.blit_x) ||
+ !test_bit(font->height - 1, info->pixmap.blit_y))
return -EINVAL;
/* Make sure driver can handle the font length */
@@ -3082,8 +3082,8 @@ void fbcon_get_requirement(struct fb_info *info,
vc = vc_cons[i].d;
if (vc && vc->vc_mode == KD_TEXT &&
info->node == con2fb_map[i]) {
- caps->x |= 1 << (vc->vc_font.width - 1);
- caps->y |= 1 << (vc->vc_font.height - 1);
+ set_bit(vc->vc_font.width - 1, caps->x);
+ set_bit(vc->vc_font.height - 1, caps->y);
charcnt =
Re: [PATCH 10/14] parisc: Add support for suppressing warning backtraces
On 3/12/24 18:03, Guenter Roeck wrote:
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT and CONFIG_DEBUG_BUGVERBOSE
are enabled. Otherwise, the __func__ assembly parameter is replaced with a
(dummy) NULL parameter to avoid an image size increase due to unused
__func__ entries (this is necessary because __func__ is not a define but a
virtual variable).
While at it, declare assembler parameters as constants where possible.
Refine .blockz instructions to calculate the necessary padding instead
of using fixed values.
Signed-off-by: Guenter Roeck
Acked-by: Helge Deller
Helge
---
arch/parisc/include/asm/bug.h | 29 +
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index 833555f74ffa..792dacc2a653 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -23,8 +23,17 @@
# define __BUG_REL(val) ".word " __stringify(val)
#endif
-
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* IS_ENABLED(CONFIG_KUNIT) */
+
#define BUG() \
do {\
asm volatile("\n" \
@@ -33,10 +42,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n" \
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %c3\n"\
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection" \
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__), \
"i" (0), "i" (sizeof(struct bug_entry)) ); \
unreachable(); \
} while(0)
@@ -58,10 +69,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n" \
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %3\n" \
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection" \
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__), \
"i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) );\
} while(0)
@@ -74,7 +87,7 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t.short %0\n" \
-"\t.blockz %1-4-2\n" \
+"\t.blockz %c1-(.-2b)\n" \
"\t.popsection" \
: : "i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) );\
Re: [PATCH] fbcon: Increase maximum font width x height to 64 x 64
You should have marked this patch with "v2"...
On 3/13/24 17:59, Samuel Thibault wrote:
This remains relatively simple by just enlarging integers.
I like the patch, but I still see some u32...
drivers/video/fbdev/vt8623fb.c: info->pixmap.blit_x = (bpp == 4) ? (1
<< (8 - 1)) : (~(u32)0);
drivers/video/fbdev/arkfb.c:info->pixmap.blit_x = (bpp == 4) ? (1
<< (8 - 1)) : (~(u32)0);
drivers/video/fbdev/core/fbmem.c: fb_info->pixmap.blit_x =
~(u32)0;
drivers/video/fbdev/s3fb.c: info->pixmap.blit_x = (bpp == 4) ? (1
<< (8 - 1)) : (~(u32)0);
And please check blit_y too.
It wouldn't be that simple to get to the console's 64x128 maximum, as it would
require 128b integers.
How realistic are fonts > 64x64 pixels ?
If they are, using the bitmap_xx functions (include/linux/bitmap.h)
now instead would be better.
Helge
Signed-off-by: Samuel Thibault
---
drivers/video/fbdev/core/fbcon.c | 17 ++---
include/linux/fb.h | 10 +-
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 46823c2e2ba1..849562f92bd5 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -101,6 +101,9 @@ enum {
FBCON_LOGO_DONTSHOW = -3/* do not show the logo */
};
+#define FBCON_MAX_FONT_WIDTH (sizeof(((struct fb_pixmap *) 0)->blit_x) * 8)
+#define FBCON_MAX_FONT_HEIGHT (sizeof(((struct fb_pixmap *) 0)->blit_y) * 8)
+
static struct fbcon_display fb_display[MAX_NR_CONSOLES];
static struct fb_info *fbcon_registered_fb[FB_MAX];
@@ -2483,12 +2486,12 @@ static int fbcon_set_font(struct vc_data *vc, struct
console_font *font,
h > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
return -EINVAL;
- if (font->width > 32 || font->height > 32)
+ if (font->width > FBCON_MAX_FONT_WIDTH || font->height >
FBCON_MAX_FONT_HEIGHT)
return -EINVAL;
/* Make sure drawing engine can handle the font */
- if (!(info->pixmap.blit_x & BIT(font->width - 1)) ||
- !(info->pixmap.blit_y & BIT(font->height - 1)))
+ if (!(info->pixmap.blit_x & BIT_ULL(font->width - 1)) ||
+ !(info->pixmap.blit_y & BIT_ULL(font->height - 1)))
return -EINVAL;
/* Make sure driver can handle the font length */
@@ -3082,8 +3085,8 @@ void fbcon_get_requirement(struct fb_info *info,
vc = vc_cons[i].d;
if (vc && vc->vc_mode == KD_TEXT &&
info->node == con2fb_map[i]) {
- caps->x |= 1 << (vc->vc_font.width - 1);
- caps->y |= 1 << (vc->vc_font.height - 1);
+ caps->x |= 1ULL << (vc->vc_font.width - 1);
+ caps->y |= 1ULL << (vc->vc_font.height - 1);
charcnt = vc->vc_font.charcount;
if (caps->len < charcnt)
caps->len = charcnt;
@@ -3094,8 +3097,8 @@ void fbcon_get_requirement(struct fb_info *info,
if (vc && vc->vc_mode == KD_TEXT &&
info->node == con2fb_map[fg_console]) {
- caps->x = 1 << (vc->vc_font.width - 1);
- caps->y = 1 << (vc->vc_font.height - 1);
+ caps->x = 1ULL << (vc->vc_font.width - 1);
+ caps->y = 1ULL << (vc->vc_font.height - 1);
caps->len = vc->vc_font.charcount;
}
}
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 05dc9624897d..2bac166cd3f2 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -144,8 +144,8 @@ struct fb_event {
};
struct fb_blit_caps {
- u32 x;
- u32 y;
+ u64 x;
+ u64 y;
u32 len;
u32 flags;
};
@@ -192,10 +192,10 @@ struct fb_pixmap {
u32 scan_align; /* alignment per scanline */
u32 access_align; /* alignment per read/write (bits) */
u32 flags; /* see FB_PIXMAP_* */
- u32 blit_x; /* supported bit block dimensions (1-32)*/
- u32 blit_y; /* Format: blit_x = 1 << (width - 1)*/
+ u64 blit_x; /* supported bit block dimensions (1-64)*/
+ u64 blit_y; /* Format: blit_x = 1 << (width - 1)*/
/* blit_y = 1 << (height - 1) */
- /* if 0, will be set to 0x (all)*/
+ /* if 0, will be set to ~0ull (all) */
/* access methods */
void (*writeio)(struct fb_info *info, void __iomem *dst, void *src,
unsigned int size);
void (*readio) (struct fb_info *info, void *dst, void __iomem *src,
unsigned
Re: [PATCH] fbdev/mb862xxfb: Fix defined but not used error
On 2/29/24 12:50, Michael Ellerman wrote:
socrates_gc_mode is defined at the top-level but then only used inside
an #ifdef CONFIG_FB_MB862XX_LIME, leading to an error with some configs:
drivers/video/fbdev/mb862xx/mb862xxfbdrv.c:36:31: error: ‘socrates_gc_mode’
defined but not used
36 | static struct mb862xx_gc_mode socrates_gc_mode = {
Fix it by moving socrates_gc_mode inside that ifdef, immediately prior
to the only function where it's used.
Signed-off-by: Michael Ellerman
applied.
Thanks!
Helge
---
drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index 7c402e9fd7a9..baec312d7b33 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -32,15 +32,6 @@
#define CARMINE_MEM_SIZE 0x800
#define DRV_NAME "mb862xxfb"
-#if defined(CONFIG_SOCRATES)
-static struct mb862xx_gc_mode socrates_gc_mode = {
- /* Mode for Prime View PM070WL4 TFT LCD Panel */
- { "800x480", 45, 800, 480, 4, 86, 42, 33, 10, 128, 2, 0, 0, 0 },
- /* 16 bits/pixel, 16MB, 133MHz, SDRAM memory mode value */
- 16, 0x100, GC_CCF_COT_133, 0x4157ba63
-};
-#endif
-
/* Helpers */
static inline int h_total(struct fb_var_screeninfo *var)
{
@@ -666,6 +657,15 @@ static int mb862xx_gdc_init(struct mb862xxfb_par *par)
return 0;
}
+#if defined(CONFIG_SOCRATES)
+static struct mb862xx_gc_mode socrates_gc_mode = {
+ /* Mode for Prime View PM070WL4 TFT LCD Panel */
+ { "800x480", 45, 800, 480, 4, 86, 42, 33, 10, 128, 2, 0, 0, 0 },
+ /* 16 bits/pixel, 16MB, 133MHz, SDRAM memory mode value */
+ 16, 0x100, GC_CCF_COT_133, 0x4157ba63
+};
+#endif
+
static int of_platform_mb862xx_probe(struct platform_device *ofdev)
{
struct device_node *np = ofdev->dev.of_node;
Re: [PATCH v2] fbmon: prevent division by zero in fb_videomode_from_videomode()
On 3/5/24 14:51, Roman Smirnov wrote:
The expression htotal * vtotal can have a zero value on
overflow.
I'm not sure if thos always results in zero in kernel on overflow.
Might be architecture-depended too, but let's assume it
can become zero,
It is necessary to prevent division by zero like in
fb_var_to_videomode().
Found by Linux Verification Center (linuxtesting.org) with Svace.
Signed-off-by: Roman Smirnov
Reviewed-by: Sergey Shtylyov
---
V1 -> V2: Replaced the code of the first version with a check.
drivers/video/fbdev/core/fbmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 79e5bfbdd34c..b137590386da 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -1344,7 +1344,7 @@ int fb_videomode_from_videomode(const struct videomode
*vm,
vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
vm->vsync_len;
/* prevent division by zero */
- if (htotal && vtotal) {
+ if (htotal && vtotal && (vm->pixelclock / htotal >= vtotal)) {
why don't you then simply check for
if .. ((htotal * vtotal) == 0) ...
instead?
Helge
fbmode->refresh = vm->pixelclock / (htotal * vtotal);
/* a mode must have htotal and vtotal != 0 or it is invalid */
} else {
Re: [PATCH v2] fbdev: uvesafb: Convert sprintf/snprintf to sysfs_emit
On 3/14/24 10:58, Li Zhijian wrote: Per filesystems/sysfs.rst, show() should only use sysfs_emit() or sysfs_emit_at() when formatting the value to be returned to user space. coccinelle complains that there are still a couple of functions that use snprintf(). Convert them to sysfs_emit(). sprintf() will be converted as weel if they have. Generally, this patch is generated by make coccicheck M= MODE=patch \ COCCI=scripts/coccinelle/api/device_attr_show.cocci No functional change intended CC: Helge Deller CC: linux-fb...@vger.kernel.org CC: dri-devel@lists.freedesktop.org Signed-off-by: Li Zhijian applied. Thanks! Helge --- This is a part of the work "Fix coccicheck device_attr_show warnings"[1] Split them per subsystem so that the maintainer can review it easily [1] https://lore.kernel.org/lkml/20240116041129.3937800-1-lizhij...@fujitsu.com/ --- drivers/video/fbdev/uvesafb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/uvesafb.c b/drivers/video/fbdev/uvesafb.c index e1f421e91b4f..73f00c079a94 100644 --- a/drivers/video/fbdev/uvesafb.c +++ b/drivers/video/fbdev/uvesafb.c @@ -1546,7 +1546,7 @@ static ssize_t uvesafb_show_vbe_ver(struct device *dev, struct fb_info *info = dev_get_drvdata(dev); struct uvesafb_par *par = info->par; - return snprintf(buf, PAGE_SIZE, "%.4x\n", par->vbe_ib.vbe_version); + return sysfs_emit(buf, "%.4x\n", par->vbe_ib.vbe_version); } static DEVICE_ATTR(vbe_version, S_IRUGO, uvesafb_show_vbe_ver, NULL);
Re: [PATCH 1/1] fbdev/hyperv_fb: Fix logic error for Gen2 VMs in hvfb_getmem()
On 2/9/24 16:23, Michael Kelley wrote:
From: Thomas Zimmermann Sent: Thursday, February 1, 2024
12:17 AM
Hi
Am 01.02.24 um 07:00 schrieb mhkelle...@gmail.com:
From: Michael Kelley
A recent commit removing the use of screen_info introduced a logic
error. The error causes hvfb_getmem() to always return -ENOMEM
for Generation 2 VMs. As a result, the Hyper-V frame buffer
device fails to initialize. The error was introduced by removing
an "else if" clause, leaving Gen2 VMs to always take the -ENOMEM
error path.
Fix the problem by removing the error path "else" clause. Gen 2
VMs now always proceed through the MMIO memory allocation code,
but with "base" and "size" defaulting to 0.
Indeed, that's how it was supposed to work. IDK how I didn't notice this
bug. Thanks a lot for the fix.
Fixes: 0aa0838c84da ("fbdev/hyperv_fb: Remove firmware framebufferswith aperture
helpers")
Signed-off-by: Michael Kelley
Reviewed-by: Thomas Zimmermann
Wei Liu and Helge Deller --
Should this fix go through the Hyper-V tree or the fbdev tree? I'm not
aware of a reason that it really matters, but it needs to be one or the
other, and sooner rather than later, because the Hyper-V driver is broken
starting in 6.8-rc1.
I'm fine with either.
If there is an upcoming hyper-v pull request, I'm fine if this is included
there. If not, let me know and I can take it via fbdev.
Helge
Michael
---
drivers/video/fbdev/hyperv_fb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/video/fbdev/hyperv_fb.c
b/drivers/video/fbdev/hyperv_fb.c
index c26ee6fd73c9..8fdccf033b2d 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -1010,8 +1010,6 @@ static int hvfb_getmem(struct hv_device *hdev,
struct fb_info *info)
goto getmem_done;
}
pr_info("Unable to allocate enough contiguous physical memory on Gen
1 VM. Using MMIO instead.\n");
- } else {
- goto err1;
}
/*
Re: [PATCH] fbdev: Restrict FB_SH_MOBILE_LCDC to SuperH
On 1/31/24 17:08, Geert Uytterhoeven wrote:
Since commit f402f7a02af6956d ("staging: board: Remove Armadillo-800-EVA
board staging code"), there are no more users of the legacy SuperH
Mobile LCDC framebuffer driver on Renesas ARM platforms. All former
users on these platforms have been converted to the SH-Mobile DRM
driver, using DT.
Suggested-by: Arnd Bergmann
Signed-off-by: Geert Uytterhoeven
---
Commit f402f7a02af6956d is in staging-next (next-20240129 and later).
applied to fbdev git tree.
Thanks!
Helge
---
drivers/video/fbdev/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 2d0bcc1d786e50bb..b688900bb67eed55 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1554,7 +1554,7 @@ config FB_FSL_DIU
config FB_SH_MOBILE_LCDC
tristate "SuperH Mobile LCDC framebuffer support"
depends on FB && HAVE_CLK && HAS_IOMEM
- depends on SUPERH || ARCH_RENESAS || COMPILE_TEST
+ depends on SUPERH || COMPILE_TEST
depends on FB_DEVICE
select FB_BACKLIGHT
select FB_DEFERRED_IO
[GIT PULL] fbdev fix for v6.8-rc2
Hi Linus, please pull a small patch set with 3 fixes and 2 small cleanups for fbdev for kernel 6.8-rc2: A crash fix in stifb which was missed to be included in the drm-misc tree, two checks to prevent wrong userspace input in sisfb and savagefb and two trivial printk cleanups. Thanks! Helge The following changes since commit 7a396820222d6d4c02057f41658b162bdcdadd0e: Merge tag 'v6.8-rc-part2-smb-client' of git://git.samba.org/sfrench/cifs-2.6 (2024-01-20 16:48:07 -0800) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.8-rc2 for you to fetch changes up to 4b088005c897a62fe98f70ab69687706cb2fad3b: fbdev: stifb: Fix crash in stifb_blank() (2024-01-23 09:13:24 +0100) fbdev fixes and cleanups for 6.8-rc2: - stifb: Fix crash in stifb_blank() - savage/sis: Error out if pixclock equals zero - minor trivial cleanups Fullway Wang (2): fbdev: savage: Error out if pixclock equals zero fbdev: sis: Error out if pixclock equals zero Geert Uytterhoeven (1): fbcon: Fix incorrect printed function name in fbcon_prepare_logo() Helge Deller (1): fbdev: stifb: Fix crash in stifb_blank() Jiapeng Chong (1): fbdev: vt8500lcdfb: Remove unnecessary print function dev_err() drivers/video/fbdev/core/fbcon.c | 3 +-- drivers/video/fbdev/savage/savagefb_driver.c | 3 +++ drivers/video/fbdev/sis/sis_main.c | 2 ++ drivers/video/fbdev/stifb.c | 2 +- drivers/video/fbdev/vt8500lcdfb.c| 1 - 5 files changed, 7 insertions(+), 4 deletions(-)
Re: [PATCH] fbcon: Fix incorrect printed function name in fbcon_prepare_logo()
On 1/22/24 16:04, Geert Uytterhoeven wrote:
If the boot logo does not fit, a message is printed, including a wrong
function name prefix. Instead of correcting the function name (or using
__func__), just use "fbcon", like is done in several other messages.
While at it, modernize the call by switching to pr_info().
Signed-off-by: Geert Uytterhoeven
applied to fbdev git tree.
Thanks!
Helge
---
drivers/video/fbdev/core/fbcon.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 63af6ab034b5f1bb..1183e7a871f8b270 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -631,8 +631,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct
fb_info *info,
if (logo_lines > vc->vc_bottom) {
logo_shown = FBCON_LOGO_CANSHOW;
- printk(KERN_INFO
- "fbcon_init: disable boot-logo (boot-logo bigger than
screen).\n");
+ pr_info("fbcon: disable boot-logo (boot-logo bigger than
screen).\n");
} else {
logo_shown = FBCON_LOGO_DRAW;
vc->vc_top = logo_lines;
Re: [PATCH v2 00/47] tty: vt: cleanup and documentation
On 1/22/24 12:03, Jiri Slaby (SUSE) wrote: Push the console code (vt.c, vt.h, console.h, ...) into a bit more maintainable state. Especially all around consw structure and document it. CSI parser is also a bit cleaned up. More to follow some time in the next round. I've not yet looked through all of those patches, but I tried to boot up a machine with the STI console driver and I've not seen any issues yet. So far: Tested-by: Helge Deller # parisc STI console Helge [v2] See respective patches for changes. The major changes: * vesa.h introduced * parameters of csi*() simplified Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Cc: Helge Deller Cc: "James E.J. Bottomley" Cc: Jonathan Corbet Cc: linux-...@vger.kernel.org Cc: linux-fb...@vger.kernel.org Cc: linux-par...@vger.kernel.org Cc: Martin Hostettler Cc: Thomas Zimmermann Jiri Slaby (SUSE) (47): vgacon: inline vc_scrolldelta_helper() into vgacon_scrolldelta() fbcon: make display_desc a static array in fbcon_startup() tty: vt: fix 20 vs 0x20 typo in EScsiignore tty: vt: expect valid vc when in tty ops tty: vt: pass proper pointers from tioclinux() tty: vt: push console lock from tioclinux() down to 2 functions tty: vt: pass vc_resize_user as a parameter tty: vt: make vc_is_sel()'s vc const tty: vt: define an enum for CSI+m codes tty: vt: use case ranges for CSI+m fg/bg colors tty: vt: define an enum for CSI+J codes tty: vt: reflow csi_J() use clamp() for counts in csi_?() handlers don't pass vc->vc_par[0] to csi_?() handlers tty: vt: define an enum for CSI+K codes tty: vt: reflow csi_K() tty: vt: define an enum for ascii characters tty: vt: remove extern from functions in selection.h tty: vt: make consw::con_debug_*() return void tty: vt: make init parameter of consw::con_init() a bool tty: vt: sanitize arguments of consw::con_clear() tty: vt: remove checks for count in consw::con_clear() implementations tty: vt: add con_putc() helper tty: vt: eliminate unneeded consw::con_putc() implementations tty: vt: sanitize consw::con_putc() parameters tty: vt: sanitize consw::con_putcs() parameters consoles: use if instead of switch-case in consw::con_cursor() fbdev/core: simplify cursor_state setting in fbcon_ops::cursor() tty: vt: remove CM_* constants tty: vt: make consw::con_switch() return a bool tty: vt: stop using -1 for blank mode in consw::con_blank() tty: vt: define a common enum for VESA blanking constants tty: vt: use VESA blanking constants tty: vt: use enum constants for VESA blanking modes tty: vt: make types around consw::con_blank() bool tty: vt: make font of consw::con_font_set() const tty: vt: make consw::con_font_default()'s name const tty: vt: change consw::con_set_origin() return type fbcon: remove consw::con_screen_pos() tty: vt: remove consw::con_screen_pos() tty: vt: make types of screenpos() more consistent fbcon: remove fbcon_getxy() tty: vt: remove consw::con_getxy() tty: vt: remove unused consw::con_flush_scrollback() tty: vt: document the rest of struct consw tty: vt: fix up kernel-doc Documentation: add console.rst Documentation/driver-api/tty/console.rst | 45 ++ Documentation/driver-api/tty/index.rst | 1 + drivers/tty/vt/selection.c | 43 +- drivers/tty/vt/vt.c | 645 +++ drivers/tty/vt/vt_ioctl.c| 6 +- drivers/video/console/dummycon.c | 38 +- drivers/video/console/mdacon.c | 43 +- drivers/video/console/newport_con.c | 69 +-- drivers/video/console/sticon.c | 79 ++- drivers/video/console/vgacon.c | 152 +++--- drivers/video/fbdev/core/bitblit.c | 13 +- drivers/video/fbdev/core/fbcon.c | 123 ++--- drivers/video/fbdev/core/fbcon.h | 4 +- drivers/video/fbdev/core/fbcon_ccw.c | 13 +- drivers/video/fbdev/core/fbcon_cw.c | 13 +- drivers/video/fbdev/core/fbcon_ud.c | 13 +- drivers/video/fbdev/core/tileblit.c | 4 +- include/linux/console.h | 124 +++-- include/linux/console_struct.h | 1 - include/linux/selection.h| 56 +- include/linux/vt_kern.h | 12 +- include/uapi/linux/fb.h | 8 +- include/uapi/linux/vesa.h| 18 + 23 files changed, 755 insertions(+), 768 deletions(-) create mode 100644 Documentation/driver-api/tty/console.rst create mode 100644 include/uapi/linux/vesa.h
Re: REGRESSION: no console on current -git
On 1/19/24 22:22, Jens Axboe wrote: On 1/19/24 2:14 PM, Helge Deller wrote: On 1/19/24 22:01, Jens Axboe wrote: On 1/19/24 1:55 PM, Helge Deller wrote: Adding Mirsad Todorovac (who reported a similar issue). On 1/19/24 19:39, Jens Axboe wrote: My trusty R7525 test box is failing to show a console, or in fact anything, on current -git. There's no output after: Loading Linux 6.7.0+ ... Loading initial ramdisk ... and I don't get a console up. I went through the bisection pain and found this was the culprit: commit df67699c9cb0ceb70f6cc60630ca938c06773eda Author: Thomas Zimmermann Date: Wed Jan 3 11:15:11 2024 +0100 firmware/sysfb: Clear screen_info state after consuming it Reverting this commit, and everything is fine. Looking at dmesg with a buggy kernel, I get no frame or fb messages. On a good kernel, it looks ilke this: [1.416486] efifb: probing for efifb [1.416602] efifb: framebuffer at 0xde00, using 3072k, total 3072k [1.416605] efifb: mode is 1024x768x32, linelength=4096, pages=1 [1.416607] efifb: scrolling: redraw [1.416608] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 [1.449746] fb0: EFI VGA frame buffer device Happy to test a fix, or barring that, can someone just revert this commit please? I've temporarily added a revert patch into the fbdev for-next tree for now, so people should not face the issue in the for-next series: https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next I'd like to wait for Thomas to return on monday to check the issue as there are some other upcoming patches in this area from him. Given the issue (and that I'm not the only one reporting it), can we please just get that pushed so it'll make -rc1? It can always get re-introduced in a fixed fashion. I don't run -next here, I rely on mainline working for my testing. I agree, it would be good to get it fixed for -rc1. So, it's ok for me, but I won't be able to test the revert short time right now. If you can assure that the revert fixes it, and builds in git-head, I can now prepare the pull request for Linus now (or he just reverts commit df67699c9cb0 manually). I already tested a revert on top of the current tree, and it builds just fine and boots with a working console. So reverting it does work and solves the issue. I sent a pull request with the revert. Thanks! Helge
[GIT PULL] One fbdev regression fix for v6.8-rc1
Hi Linus, there were various reports from people without any graphics output on the screen and it turns out one commit triggers the problem. Please pull a single revert for now for -rc1 to fix this regression. We will work out how to fix it correctly later. Thanks! Helge The following changes since commit 556e2d17cae620d549c5474b1ece053430cd50bc: Merge tag 'ceph-for-6.8-rc1' of https://github.com/ceph/ceph-client (2024-01-19 09:58:55 -0800) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.8-rc1-2 for you to fetch changes up to 2bebc3cd48701607e38e8258ab9692de9b1a718b: Revert "firmware/sysfb: Clear screen_info state after consuming it" (2024-01-19 22:22:26 +0100) fbdev fix for 6.8-rc1: - Revert "firmware/sysfb: Clear screen_info state after consuming it" -------- Helge Deller (1): Revert "firmware/sysfb: Clear screen_info state after consuming it" drivers/firmware/sysfb.c | 14 +- 1 file changed, 1 insertion(+), 13 deletions(-)
Re: REGRESSION: no console on current -git
On 1/19/24 22:01, Jens Axboe wrote: On 1/19/24 1:55 PM, Helge Deller wrote: Adding Mirsad Todorovac (who reported a similar issue). On 1/19/24 19:39, Jens Axboe wrote: My trusty R7525 test box is failing to show a console, or in fact anything, on current -git. There's no output after: Loading Linux 6.7.0+ ... Loading initial ramdisk ... and I don't get a console up. I went through the bisection pain and found this was the culprit: commit df67699c9cb0ceb70f6cc60630ca938c06773eda Author: Thomas Zimmermann Date: Wed Jan 3 11:15:11 2024 +0100 firmware/sysfb: Clear screen_info state after consuming it Reverting this commit, and everything is fine. Looking at dmesg with a buggy kernel, I get no frame or fb messages. On a good kernel, it looks ilke this: [1.416486] efifb: probing for efifb [1.416602] efifb: framebuffer at 0xde00, using 3072k, total 3072k [1.416605] efifb: mode is 1024x768x32, linelength=4096, pages=1 [1.416607] efifb: scrolling: redraw [1.416608] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 [1.449746] fb0: EFI VGA frame buffer device Happy to test a fix, or barring that, can someone just revert this commit please? I've temporarily added a revert patch into the fbdev for-next tree for now, so people should not face the issue in the for-next series: https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next I'd like to wait for Thomas to return on monday to check the issue as there are some other upcoming patches in this area from him. Given the issue (and that I'm not the only one reporting it), can we please just get that pushed so it'll make -rc1? It can always get re-introduced in a fixed fashion. I don't run -next here, I rely on mainline working for my testing. I agree, it would be good to get it fixed for -rc1. So, it's ok for me, but I won't be able to test the revert short time right now. If you can assure that the revert fixes it, and builds in git-head, I can now prepare the pull request for Linus now (or he just reverts commit df67699c9cb0 manually). Helge
Re: REGRESSION: no console on current -git
Adding Mirsad Todorovac (who reported a similar issue). On 1/19/24 19:39, Jens Axboe wrote: My trusty R7525 test box is failing to show a console, or in fact anything, on current -git. There's no output after: Loading Linux 6.7.0+ ... Loading initial ramdisk ... and I don't get a console up. I went through the bisection pain and found this was the culprit: commit df67699c9cb0ceb70f6cc60630ca938c06773eda Author: Thomas Zimmermann Date: Wed Jan 3 11:15:11 2024 +0100 firmware/sysfb: Clear screen_info state after consuming it Reverting this commit, and everything is fine. Looking at dmesg with a buggy kernel, I get no frame or fb messages. On a good kernel, it looks ilke this: [1.416486] efifb: probing for efifb [1.416602] efifb: framebuffer at 0xde00, using 3072k, total 3072k [1.416605] efifb: mode is 1024x768x32, linelength=4096, pages=1 [1.416607] efifb: scrolling: redraw [1.416608] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 [1.449746] fb0: EFI VGA frame buffer device Happy to test a fix, or barring that, can someone just revert this commit please? I've temporarily added a revert patch into the fbdev for-next tree for now, so people should not face the issue in the for-next series: https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next I'd like to wait for Thomas to return on monday to check the issue as there are some other upcoming patches in this area from him. Helge
[GIT PULL] fbdev fixes and updates for v6.8-rc1
Hi Linus,
please pull the fbdev changes for kernel 6.8-rc1.
Three fbdev drivers (~8500 lines of code) will be dropped:
The Carillo Ranch fbdev driver is for an Intel product which was never shipped,
and for the intelfb and the amba-clcd drivers the drm drivers can be used
instead.
The other code changes are minor:
Some fb_deferred_io flushing fixes, imxfb margin fixes and stifb cleanups.
Please note that there is a merge conflict with the drm tree for those files:
- drivers/video/fbdev/amba-clcd.c
- drivers/video/fbdev/vermilion/vermilion.c
They were modified in the drm tree, but deleted in the fbdev tree.
In case you don't want to deal with this upcoming merge conflict I offer
to resend a rebased pull request after you pulled drm.
Thanks!
Helge
The following changes since commit de927f6c0b07d9e698416c5b287c521b07694cac:
Merge tag 's390-6.8-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux (2024-01-10 18:18:20
-0800)
are available in the Git repository at:
http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git
tags/fbdev-for-6.8-rc1
for you to fetch changes up to 689237ab37c59b9909bc9371d7fece3081683fba:
fbdev/intelfb: Remove driver (2024-01-12 12:38:37 +0100)
fbdev fixes and cleanups for 6.8-rc1:
- Remove intelfb fbdev driver (Thomas Zimmermann)
- Remove amba-clcd fbdev driver (Linus Walleij)
- Remove vmlfb Carillo Ranch fbdev driver (Matthew Wilcox)
- fb_deferred_io flushing fixes (Nam Cao)
- imxfb code fixes and cleanups (Dario Binacchi)
- stifb primary screen detection cleanups (Thomas Zimmermann)
Colin Ian King (1):
video/logo: use %u format specifier for unsigned int values
Dario Binacchi (11):
fbdev: imxfb: fix left margin setting
fbdev: imxfb: move PCR bitfields near their offset
fbdev: imxfb: use BIT, FIELD_{GET,PREP} and GENMASK macros
fbdev: imxfb: replace some magic numbers with constants
fbdev: imxfb: add missing SPDX tag
fbdev: imxfb: drop ftrace-like logging
fbdev: imxfb: add missing spaces after ','
fbdev: imxfb: Fix style warnings relating to printk()
fbdev: imxfb: use __func__ for function name
fbdev: imxfb: add '*/' on a separate line in block comment
fbdev: mmp: Fix typo and wording in code comment
Linus Walleij (1):
fbdev: amba-clcd: Delete the old CLCD driver
Matthew Wilcox (Oracle) (1):
fbdev: Remove support for Carillo Ranch driver
Nam Cao (2):
fbdev: flush deferred work in fb_deferred_io_fsync()
fbdev: flush deferred IO before closing
Randy Dunlap (1):
fbdev: hgafb: fix kernel-doc comments
Stanislav Kinsburskii (1):
fbdev: fsl-diu-fb: Fix sparse warning due to virt_to_phys() prototype
change
Thomas Zimmermann (10):
video/sticore: Store ROM device in STI struct
fbdev/stifb: Allocate fb_info instance with framebuffer_alloc()
arch/parisc: Detect primary video device from device instance
video/sticore: Remove info field from STI struct
fbdev/sis: Remove dependency on screen_info
drm/hyperv: Remove firmware framebuffers with aperture helper
fbdev/hyperv_fb: Remove firmware framebuffers with aperture helpers
firmware/sysfb: Clear screen_info state after consuming it
fbdev/hyperv_fb: Do not clear global screen_info
fbdev/intelfb: Remove driver
Documentation/fb/index.rst |1 -
Documentation/fb/intelfb.rst | 155 --
Documentation/userspace-api/ioctl/ioctl-number.rst |1 -
MAINTAINERS| 12 -
arch/parisc/video/fbdev.c |2 +-
drivers/Makefile |3 +-
drivers/firmware/sysfb.c | 14 +-
drivers/gpu/drm/hyperv/hyperv_drm_drv.c|8 +-
drivers/video/backlight/Kconfig|7 -
drivers/video/backlight/Makefile |1 -
drivers/video/backlight/cr_bllcd.c | 264 ---
drivers/video/fbdev/Kconfig| 72 -
drivers/video/fbdev/Makefile |2 -
drivers/video/fbdev/amba-clcd.c| 984 -
drivers/video/fbdev/core/fb_defio.c|8 +-
drivers/video/fbdev/fsl-diu-fb.c |2 +-
drivers/video/fbdev/hgafb.c| 13 +-
drivers/video/fbdev/hyperv_fb.c| 20 +-
drivers/video/fbdev/imxfb.c| 179 +-
drivers/video/fbdev/intelfb/Makefile |8 -
drivers/video/fbdev/intelfb/intelfb.h | 382
drivers/video/fbdev/intelfb/intelfb_i2c.c | 209 --
drivers/video/fbdev/intelfb/intelfbdrv.c | 1680
Re: [PATCH 1/4] drm/hyperv: Remove firmware framebuffers with aperture helper
On 1/8/24 10:26, Javier Martinez Canillas wrote: Thomas Zimmermann writes: Hello Thomas, Replace use of screen_info state with the correct interface from the aperture helpers. The state is only for architecture and firmware code. It is not guaranteed to contain valid data. Drivers are thus not allowed to use it. For removing conflicting firmware framebuffers, there are aperture helpers. Hence replace screen_info with the correct function that will remove conflicting framebuffers for the hyperv-drm driver. Also move the call to the correct place within the driver. Signed-off-by: Thomas Zimmermann --- Reviewed-by: Javier Martinez Canillas Series applied to fbdev git tree. Thanks, Helge
Re: [PATCH] fbdev/intelfb: Remove driver
On 1/5/24 10:10, Thomas Zimmermann wrote: From looking at the PCI IDs, every device supported by intelfb is also supported by i915. Anyone still using intelfb should please move on to i915, which does everything intelfb does but better. Removing intelfb is motivated by the driver's excessive use of the global screen_info state. The state belongs to architecture and firmware code; device drivers should not attempt to access it. But fixing intelfb would require a significant change in the driver's probing logic. As intelfb has been obsolete for nearly 2 decades, it is probably not worth the effort. Let's just remove it. Also remove the related documentation. Signed-off-by: Thomas Zimmermann --- Documentation/fb/index.rst|1 - Documentation/fb/intelfb.rst | 155 -- .../userspace-api/ioctl/ioctl-number.rst |1 - MAINTAINERS |7 - drivers/Makefile |3 +- drivers/video/fbdev/Kconfig | 39 - drivers/video/fbdev/intelfb/Makefile |8 - drivers/video/fbdev/intelfb/intelfb.h | 382 --- drivers/video/fbdev/intelfb/intelfb_i2c.c | 209 -- drivers/video/fbdev/intelfb/intelfbdrv.c | 1680 - drivers/video/fbdev/intelfb/intelfbhw.c | 2115 - drivers/video/fbdev/intelfb/intelfbhw.h | 609 - 12 files changed, 1 insertion(+), 5208 deletions(-) delete mode 100644 Documentation/fb/intelfb.rst delete mode 100644 drivers/video/fbdev/intelfb/Makefile delete mode 100644 drivers/video/fbdev/intelfb/intelfb.h delete mode 100644 drivers/video/fbdev/intelfb/intelfb_i2c.c delete mode 100644 drivers/video/fbdev/intelfb/intelfbdrv.c delete mode 100644 drivers/video/fbdev/intelfb/intelfbhw.c delete mode 100644 drivers/video/fbdev/intelfb/intelfbhw.h applied to fbdev git tree. Thanks! Helge
Re: [PATCH] fbdev/sis: Remove dependency on screen_info
On 1/3/24 13:13, Thomas Zimmermann wrote: When built-in, the sis driver tries to detect the current display mode from the global screen_info state. That state is only for architecture and firmware code. Drivers should not use it directly as it's not guaranteed to contain valid information. Remove the mode-detection code from sis. Drivers that want to detect a pre-set mode on probe should read the hardware registers directly. Signed-off-by: Thomas Zimmermann --- drivers/video/fbdev/sis/sis_main.c | 37 -- 1 file changed, 37 deletions(-) applied to fbdev git tree. Thanks! Helge
Re: [PATCH 1/4] video/sticore: Store ROM device in STI struct
On 12/20/23 14:22, Thomas Zimmermann wrote: Store the ROM's parent device in each STI struct, so we can associate the STI framebuffer with a device. The new field will eventually replace the fbdev subsystem's info field, which the function fb_is_primary_device() currently requires to detect the firmware's output. By using the device instead of the framebuffer info, a later patch can generalize the helper for use in non-fbdev code. Signed-off-by: Thomas Zimmermann --- drivers/video/sticore.c | 5 + include/video/sticore.h | 4 2 files changed, 9 insertions(+) Series applied to fbdev git tree. Thanks! Helge
Re: [PATCH 2/4] fbdev/stifb: Allocate fb_info instance with framebuffer_alloc()
Hi Thomas,
On 12/20/23 14:22, Thomas Zimmermann wrote:
Allocate stifb's instance of fb_info with framebuffer_alloc(). This
is the preferred way of creating fb_info with associated driver data
stored in struct fb_info.par. Requires several, but minor, changes
through out the driver's code.
The intended side effect of this patch is that the new instance of
struct fb_info now has its device field correctly set to the parent
device of the STI ROM. A later patch can detect if the device is the
firmware's primary output. It is also now correctly located within
the Linux device hierarchy.
Thanks for this cleanup series!
Sadly stifb then crashes at bootup.
Please include this copy hunk in this patch, which then fixes the crash:
diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
index 2de0e675fd15..f8bbd8d6f5b2 100644
--- a/drivers/video/fbdev/stifb.c
+++ b/drivers/video/fbdev/stifb.c
@@ -1158,7 +1158,7 @@ stifb_init_display(struct stifb_info *fb)
}
break;
}
- stifb_blank(0, (struct fb_info *)fb); /* 0=enable screen */
+ stifb_blank(0, fb->info); /* 0=enable screen */
With this applied, you may add to the series:
Tested-by: Helge Deller
Reviewed-by: Helge Deller
Thanks!
Helge
Signed-off-by: Thomas Zimmermann
---
drivers/video/fbdev/stifb.c | 106 +++-
1 file changed, 56 insertions(+), 50 deletions(-)
diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
index 548d992f8cb1..36e6bcab83aa 100644
--- a/drivers/video/fbdev/stifb.c
+++ b/drivers/video/fbdev/stifb.c
@@ -103,7 +103,7 @@ typedef struct {
} ngle_rom_t;
struct stifb_info {
- struct fb_info info;
+ struct fb_info *info;
unsigned int id;
ngle_rom_t ngle_rom;
struct sti_struct *sti;
@@ -153,15 +153,15 @@ static int __initdata stifb_bpp_pref[MAX_STI_ROMS];
#define REG_440x210030
#define REG_450x210034
-#define READ_BYTE(fb,reg) gsc_readb((fb)->info.fix.mmio_start +
(reg))
-#define READ_WORD(fb,reg) gsc_readl((fb)->info.fix.mmio_start +
(reg))
+#define READ_BYTE(fb, reg) gsc_readb((fb)->info->fix.mmio_start +
(reg))
+#define READ_WORD(fb, reg) gsc_readl((fb)->info->fix.mmio_start +
(reg))
#ifndef DEBUG_STIFB_REGS
# define DEBUG_OFF()
# define DEBUG_ON()
-# define WRITE_BYTE(value,fb,reg)
gsc_writeb((value),(fb)->info.fix.mmio_start + (reg))
-# define WRITE_WORD(value,fb,reg)
gsc_writel((value),(fb)->info.fix.mmio_start + (reg))
+# define WRITE_BYTE(value, fb, reg)gsc_writeb((value),
(fb)->info->fix.mmio_start + (reg))
+# define WRITE_WORD(value, fb, reg)gsc_writel((value),
(fb)->info->fix.mmio_start + (reg))
#else
static int debug_on = 1;
# define DEBUG_OFF() debug_on=0
@@ -169,11 +169,11 @@ static int __initdata stifb_bpp_pref[MAX_STI_ROMS];
# define WRITE_BYTE(value,fb,reg) do { if (debug_on) \
printk(KERN_DEBUG "%30s:
WRITE_BYTE(0x%06x) = 0x%02x (old=0x%02x)\n", \
__func__, reg, value,
READ_BYTE(fb,reg)); \
-
gsc_writeb((value),(fb)->info.fix.mmio_start + (reg)); } while (0)
+ gsc_writeb((value),
(fb)->info->fix.mmio_start + (reg)); } while (0)
# define WRITE_WORD(value,fb,reg) do { if (debug_on) \
printk(KERN_DEBUG "%30s:
WRITE_WORD(0x%06x) = 0x%08x (old=0x%08x)\n", \
__func__, reg, value,
READ_WORD(fb,reg)); \
-
gsc_writel((value),(fb)->info.fix.mmio_start + (reg)); } while (0)
+ gsc_writel((value),
(fb)->info->fix.mmio_start + (reg)); } while (0)
#endif /* DEBUG_STIFB_REGS */
@@ -210,13 +210,13 @@ SETUP_FB(struct stifb_info *fb)
reg10_value = 0x13601000;
break;
case S9000_ID_A1439A:
- if (fb->info.var.bits_per_pixel == 32)
+ if (fb->info->var.bits_per_pixel == 32)
reg10_value = 0xBBA0A000;
else
reg10_value = 0x13601000;
break;
case S9000_ID_HCRX:
- if (fb->info.var.bits_per_pixel == 32)
+ if (fb->info->var.bits_per_pixel == 32)
reg10_value = 0xBBA0A000;
else
reg10_value = 0x13602000;
@@ -254,7 +254,7 @@ static void
FINISH_IMAGE_COLORMAP_ACCESS(struct stifb_info *fb)
{
WRIT
Re: [PATCH 1/2] fb: flush deferred work in fb_deferred_io_fsync()
On 12/18/23 10:57, Nam Cao wrote:
The driver's fsync() is supposed to flush any pending operation to
hardware. It is implemented in this driver by cancelling the queued
deferred IO first, then schedule it for "immediate execution" by calling
schedule_delayed_work() again with delay=0. However, setting delay=0
only means the work is scheduled immediately, it does not mean the work
is executed immediately. There is no guarantee that the work is finished
after schedule_delayed_work() returns. After this driver's fsync()
returns, there can still be pending work. Furthermore, if close() is
called by users immediately after fsync(), the pending work gets
cancelled and fsync() may do nothing.
To ensure that the deferred IO completes, use flush_delayed_work()
instead. Write operations to this driver either write to the device
directly, or invoke schedule_delayed_work(); so by flushing the
workqueue, it can be guaranteed that all previous writes make it to the
device.
Fixes: 5e841b88d23d ("fb: fsync() method for deferred I/O flush.")
Cc: sta...@vger.kernel.org
Signed-off-by: Nam Cao
Reviewed-by: Sebastian Andrzej Siewior
---
drivers/video/fbdev/core/fb_defio.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
both patches applied to fbdev for-next git tree.
Thanks!
Helge
Re: [PATCH] fbdev/clcd: Delete the old CLCD driver
On 12/12/23 15:18, Linus Walleij wrote:
We have managed to ascertain that all users of the old FBDEV
code that are out of tree are now gone.
The new DRM driver can be found in drivers/gpu/drm/pl111/.
The remaining out of tree user was the ARM FVP emulation
platform, running Android. Thanks to changes in Android
versions 13 and 14, Android can now use the DRM driver when
being emulated under FVP. Some final patches are being put
in place to make it fully featured.
This is essentially a revert of the partial revert in
commit 112c35237c72 ("Partially revert "video: fbdev: amba-clcd: Retire elder CLCD
driver"")
Signed-off-by: Linus Walleij
applied.
Thanks!
Helge
---
MAINTAINERS | 5 -
drivers/video/fbdev/Kconfig | 18 -
drivers/video/fbdev/Makefile| 1 -
drivers/video/fbdev/amba-clcd.c | 984
include/linux/amba/clcd-regs.h | 87
include/linux/amba/clcd.h | 290
6 files changed, 1385 deletions(-)
Re: [PATCH 1/2] fbdev: Remove support for Carillo Ranch driver
On 12/8/23 23:47, Matthew Wilcox (Oracle) wrote: As far as anybody can tell, this product never shipped. If it did, it shipped in 2007 and nobody has access to one any more. Remove the fbdev driver and the backlight driver. Signed-off-by: Matthew Wilcox (Oracle) --- drivers/video/backlight/Kconfig |7 - drivers/video/backlight/Makefile |1 - drivers/video/backlight/cr_bllcd.c| 264 - drivers/video/fbdev/Kconfig | 15 - drivers/video/fbdev/Makefile |1 - drivers/video/fbdev/vermilion/Makefile|6 - drivers/video/fbdev/vermilion/cr_pll.c| 195 drivers/video/fbdev/vermilion/vermilion.c | 1175 - drivers/video/fbdev/vermilion/vermilion.h | 245 - 9 files changed, 1909 deletions(-) delete mode 100644 drivers/video/backlight/cr_bllcd.c delete mode 100644 drivers/video/fbdev/vermilion/Makefile delete mode 100644 drivers/video/fbdev/vermilion/cr_pll.c delete mode 100644 drivers/video/fbdev/vermilion/vermilion.c delete mode 100644 drivers/video/fbdev/vermilion/vermilion.h I've applied this patch to the fbdev git tree. It touches the backlight tree, so I hope it's ok for backlight people? Helge
Re: [PATCH] fbdev: hgafb: fix kernel-doc comments
On 12/6/23 00:14, Randy Dunlap wrote: Fix kernel-doc warnings found when using "W=1". hgafb.c:370: warning: No description found for return value of 'hgafb_open' hgafb.c:384: warning: No description found for return value of 'hgafb_release' hgafb.c:406: warning: No description found for return value of 'hgafb_setcolreg' hgafb.c:425: warning: No description found for return value of 'hgafb_pan_display' hgafb.c:425: warning: expecting prototype for hga_pan_display(). Prototype was for hgafb_pan_display() instead hgafb.c:455: warning: No description found for return value of 'hgafb_blank' Signed-off-by: Randy Dunlap Cc: Ferenc Bakonyi Cc: Helge Deller Cc: linux-nvi...@lists.surfsouth.com Cc: linux-fb...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org --- drivers/video/fbdev/hgafb.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) applied. Thanks! Helge
Re: [GIT PULL] fbdev fixes and updates for v6.7-rc3
On 11/26/23 17:29, Linus Torvalds wrote: On Sat, 25 Nov 2023 at 22:58, Helge Deller wrote: please pull some small fbdev fixes for 6.7-rc3. These all seem to be pure cleanups, not bug fixes. Well, at least: - fbdev: imxfb: fix left margin setting is a bug fix, and: - fbdev: fsl-diu-fb: Fix sparse warning due to virt_to_phys() prototype change pops up in in multiple sparse reports. Please resend during the merge window. I'll do. Thanks! Helge
[GIT PULL] fbdev fixes and updates for v6.7-rc3
Hi Linus,
please pull some small fbdev fixes for 6.7-rc3.
A left margin fix and code cleanups in imxfb, and one
sparse warning fix in fsl-diu-fb.
Thanks,
Helge
---
The following changes since commit ffc253263a1375a65fa6c9f62a893e9767fbebfa:
Linux 6.6 (2023-10-29 16:31:08 -1000)
are available in the Git repository at:
http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git
tags/fbdev-for-6.7-rc3
for you to fetch changes up to 64a1aed0aa07698d12deca9b7821ea77762ff328:
fbdev: mmp: Fix typo and wording in code comment (2023-11-25 09:54:41 +0100)
fbdev fixes and cleanups for 6.7-rc3:
- fix left margin settings in imxfb
- sparse warning fix in fsl-diu-fb
- lots of code cleanups in imxfb
Dario Binacchi (11):
fbdev: imxfb: fix left margin setting
fbdev: imxfb: move PCR bitfields near their offset
fbdev: imxfb: use BIT, FIELD_{GET,PREP} and GENMASK macros
fbdev: imxfb: replace some magic numbers with constants
fbdev: imxfb: add missing SPDX tag
fbdev: imxfb: drop ftrace-like logging
fbdev: imxfb: add missing spaces after ','
fbdev: imxfb: Fix style warnings relating to printk()
fbdev: imxfb: use __func__ for function name
fbdev: imxfb: add '*/' on a separate line in block comment
fbdev: mmp: Fix typo and wording in code comment
Stanislav Kinsburskii (1):
fbdev: fsl-diu-fb: Fix sparse warning due to virt_to_phys() prototype
change
drivers/video/fbdev/fsl-diu-fb.c | 2 +-
drivers/video/fbdev/imxfb.c | 179 ---
drivers/video/fbdev/mmp/hw/mmp_spi.c | 2 +-
3 files changed, 104 insertions(+), 79 deletions(-)
Re: [PATCH v2] fbdev/fsl-diu-fb: Fix srapse warning due to virt_to_phys() prototype change
On 11/21/23 01:01, Stanislav Kinsburskii wrote: Explicitly cast __iomem pointer to const void* with __force to fix the following warning: incorrect type in argument 1 (different address spaces) expected void const volatile *address got char [noderef] __iomem *screen_base Signed-off-by: Stanislav Kinsburskii Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202311161120.bgyxtbmq-...@intel.com/ --- drivers/video/fbdev/fsl-diu-fb.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/fsl-diu-fb.c b/drivers/video/fbdev/fsl-diu-fb.c index 7fbd9f069ac2..b0fda5bd1964 100644 --- a/drivers/video/fbdev/fsl-diu-fb.c +++ b/drivers/video/fbdev/fsl-diu-fb.c @@ -877,7 +877,7 @@ static int map_video_memory(struct fb_info *info) } mutex_lock(>mm_lock); info->screen_base = p; - info->fix.smem_start = virt_to_phys(info->screen_base); + info->fix.smem_start = virt_to_phys((__force const void *)info->screen_base); info->fix.smem_len = smem_len; mutex_unlock(>mm_lock); info->screen_size = info->fix.smem_len; applied to fbdev git tree (with some typos corrected). Helge
Re: [PATCH] video: fbdev: mmp: Fix typo in code comment
On 11/24/23 16:34, Randy Dunlap wrote:
Hi,
On 11/24/23 01:52, Dario Binacchi wrote:
s/singals/signals/
Fixes: 641b4b1b6a7c ("video: mmpdisp: add spi port in display controller")
Signed-off-by: Dario Binacchi
---
drivers/video/fbdev/mmp/hw/mmp_spi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/mmp/hw/mmp_spi.c
b/drivers/video/fbdev/mmp/hw/mmp_spi.c
index 16401eb95c6c..64e34b7e739e 100644
--- a/drivers/video/fbdev/mmp/hw/mmp_spi.c
+++ b/drivers/video/fbdev/mmp/hw/mmp_spi.c
@@ -91,7 +91,7 @@ static int lcd_spi_setup(struct spi_device *spi)
writel(tmp, reg_base + LCD_SPU_SPI_CTRL);
/*
-* After set mode it need a time to pull up the spi singals,
+* After set mode it need a time to pull up the spi signals,
Also:
it needs time
or
it needs some time
I've fixed it up and applied that patch to fbdev git tree.
Thanks!
Helge
Re: [PATCH v3 RESEND 00/20] remove I2C_CLASS_DDC support
On 11/19/23 21:51, Heiner Kallweit wrote: On 19.11.2023 21:48, Heiner Kallweit wrote: On 19.11.2023 21:28, Helge Deller wrote: On 11/19/23 12:28, Heiner Kallweit wrote: After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. The fbdev changes look at least ok so far, so: Acked-by: Helge Deller #fbdev I think this refers to patch 5 of the series. Could you please reply to patch 5 instead of the cover letter with your acked-by so that patchwork gets it right? Thanks! Sorry, just looked at where you are in To, not Cc. So your ack includes patches 6, 9, 10, 13? Yes. Helge v2: - change tag in commit subject of patch 03 - add ack tags v3: - fix a compile error in patch 5 Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c | 1 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 - drivers/gpu/drm/ast/ast_i2c.c | 1 - drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 - drivers/gpu/drm/display/drm_dp_helper.c | 1 - drivers/gpu/drm/display/drm_dp_mst_topology.c | 1 - drivers/gpu/drm/gma500/cdv_intel_dp.c | 1 - drivers/gpu/drm/gma500/intel_gmbus.c | 1 - drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c | 1 - drivers/gpu/drm/gma500/psb_intel_sdvo.c | 1 - drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c | 1 - drivers/gpu/drm/i915/display/intel_gmbus.c | 1 - drivers/gpu/drm/i915/display/intel_sdvo.c | 1 - drivers/gpu/drm/loongson/lsdc_i2c.c | 1 - drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c | 1 - drivers/gpu/drm/mgag200/mgag200_i2c.c | 1 - drivers/gpu/drm/msm/hdmi/hdmi_i2c.c | 1 - drivers/gpu/drm/radeon/radeon_i2c.c | 1 - drivers/gpu/drm/rockchip/inno_hdmi.c | 1 - drivers/gpu/drm/rockchip/rk3066_hdmi.c | 1 - drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c | 1 - drivers/video/fbdev/core/fb_ddc.c | 1 - drivers/video/fbdev/cyber2000fb.c | 1 - drivers/video/fbdev/i740fb.c | 1 - drivers/video/fbdev/intelfb/intelfb_i2c.c | 15 +-- drivers/video/fbdev/matrox/i2c-matroxfb.c | 12 drivers/video/fbdev/s3fb.c | 1 - drivers/video/fbdev/tdfxfb.c | 1 - drivers/video/fbdev/tridentfb.c | 1 - drivers/video/fbdev/via/via_i2c.c | 1 - include/linux/i2c.h | 1 - 31 files changed, 9 insertions(+), 47 deletions(-)
Re: [PATCH v3 RESEND 00/20] remove I2C_CLASS_DDC support
On 11/19/23 12:28, Heiner Kallweit wrote: After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. The fbdev changes look at least ok so far, so: Acked-by: Helge Deller#fbdev v2: - change tag in commit subject of patch 03 - add ack tags v3: - fix a compile error in patch 5 Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c |1 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |1 - drivers/gpu/drm/ast/ast_i2c.c |1 - drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |1 - drivers/gpu/drm/display/drm_dp_helper.c |1 - drivers/gpu/drm/display/drm_dp_mst_topology.c |1 - drivers/gpu/drm/gma500/cdv_intel_dp.c |1 - drivers/gpu/drm/gma500/intel_gmbus.c |1 - drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c|1 - drivers/gpu/drm/gma500/psb_intel_sdvo.c |1 - drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c |1 - drivers/gpu/drm/i915/display/intel_gmbus.c|1 - drivers/gpu/drm/i915/display/intel_sdvo.c |1 - drivers/gpu/drm/loongson/lsdc_i2c.c |1 - drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c |1 - drivers/gpu/drm/mgag200/mgag200_i2c.c |1 - drivers/gpu/drm/msm/hdmi/hdmi_i2c.c |1 - drivers/gpu/drm/radeon/radeon_i2c.c |1 - drivers/gpu/drm/rockchip/inno_hdmi.c |1 - drivers/gpu/drm/rockchip/rk3066_hdmi.c|1 - drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c|1 - drivers/video/fbdev/core/fb_ddc.c |1 - drivers/video/fbdev/cyber2000fb.c |1 - drivers/video/fbdev/i740fb.c |1 - drivers/video/fbdev/intelfb/intelfb_i2c.c | 15 +-- drivers/video/fbdev/matrox/i2c-matroxfb.c | 12 drivers/video/fbdev/s3fb.c|1 - drivers/video/fbdev/tdfxfb.c |1 - drivers/video/fbdev/tridentfb.c |1 - drivers/video/fbdev/via/via_i2c.c |1 - include/linux/i2c.h |1 - 31 files changed, 9 insertions(+), 47 deletions(-)
Re: [PATCH 00/10] Fix left margin setup and code cleanup
On 11/11/23 11:41, Dario Binacchi wrote:
This series was created with the intention of fixing the left margin
setting. At the same time, I made some changes with the aim of making
the code more readable, as well as removing the errors/warnings
reported by checkpatch.
applied to fbdev for-next git tree.
Thanks!
Helge
Dario Binacchi (10):
fbdev: imxfb: fix left margin setting
fbdev: imxfb: move PCR bitfields near their offset
fbdev: imxfb: use BIT, FIELD_{GET,PREP} and GENMASK macros
fbdev: imxfb: replace some magic numbers with constants
fbdev: imxfb: add missing SPDX tag
fbdev: imxfb: drop ftrace-like logging
fbdev: imxfb: add missing spaces after ','
fbdev: imxfb: Fix style warnings relating to printk()
fbdev: imxfb: use __func__ for function name
fbdev: imxfb: add '*/' on a separate line in block comment
drivers/video/fbdev/imxfb.c | 179
1 file changed, 102 insertions(+), 77 deletions(-)
[GIT PULL] fbdev fixes and updates for v6.7-rc1
The following changes since commit be3ca57cfb777ad820c6659d52e60bbdd36bf5ff: Merge tag 'media/v6.7-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media (2023-11-06 15:06:06 -0800) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.7-rc1 for you to fetch changes up to a5035c81847430dfa3482807b07325f29e9e8c09: fbdev: fsl-diu-fb: mark wr_reg_wa() static (2023-11-10 09:16:02 +0100) fbdev fixes and cleanups for 6.7-rc1: - fix double free and resource leaks in imsttfb - lots of remove callback cleanups and section mismatch fixes in omapfb, amifb and atmel_lcdfb - error code fix and memparse simplification in omapfb Andy Shevchenko (2): fbdev: omapfb: Do not shadow error code from platform_get_irq() fbdev: omapfb: Replace custom memparse() implementation Arnd Bergmann (2): fbdev: hyperv_fb: fix uninitialized local variable use fbdev: fsl-diu-fb: mark wr_reg_wa() static Christophe JAILLET (1): fbdev: offb: Simplify offb_init_fb() Dan Carpenter (2): fbdev: imsttfb: fix double free in probe() fbdev: imsttfb: fix a resource leak in probe Philipp Stanner (1): fbdev: viafb: use new array-copying-wrapper Uwe Kleine-König (23): fbdev: omapfb: Drop unused remove function fbdev: atmel_lcdfb: Stop using platform_driver_probe() fbdev: omapfb/analog-tv: Don't put .remove() in .exit.text and drop suppress_bind_attrs fbdev: omapfb/dpi: Don't put .remove() in .exit.text and drop suppress_bind_attrs fbdev: omapfb/dsi-cm: Don't put .remove() in .exit.text and drop suppress_bind_attrs fbdev: omapfb/dvi: Don't put .remove() in .exit.text and drop suppress_bind_attrs fbdev: omapfb/hdmi: Don't put .remove() in .exit.text and drop suppress_bind_attrs fbdev: omapfb/opa362: Don't put .remove() in .exit.text and drop suppress_bind_attrs fbdev: omapfb/sharp-ls037v7dw01: Don't put .remove() in .exit.text and drop suppress_bind_attrs fbdev: omapfb/tfp410: Don't put .remove() in .exit.text and drop suppress_bind_attrs fbdev: omapfb/tpd12s015: Don't put .remove() in .exit.text and drop suppress_bind_attrs fbdev: atmel_lcdfb: Convert to platform remove callback returning void fbdev: omapfb/analog-tv: Convert to platform remove callback returning void fbdev: omapfb/dpi: Convert to platform remove callback returning void fbdev: omapfb/dsi-cm: Convert to platform remove callback returning void fbdev: omapfb/dvi: Convert to platform remove callback returning void fbdev: omapfb/hdmi: Convert to platform remove callback returning void fbdev: omapfb/opa362: Convert to platform remove callback returning void fbdev: omapfb/sharp-ls037v7dw01: Convert to platform remove callback returning void fbdev: omapfb/tfp410: Convert to platform remove callback returning void fbdev: omapfb/tpd12s015: Convert to platform remove callback returning void fbdev: amifb: Mark driver struct with __refdata to prevent section mismatch warning fbdev: amifb: Convert to platform remove callback returning void drivers/video/fbdev/amifb.c| 13 +--- drivers/video/fbdev/atmel_lcdfb.c | 18 +-- drivers/video/fbdev/fsl-diu-fb.c | 2 +- drivers/video/fbdev/hyperv_fb.c| 2 ++ drivers/video/fbdev/imsttfb.c | 35 +++--- drivers/video/fbdev/offb.c | 8 ++--- drivers/video/fbdev/omap/omapfb_main.c | 28 +++-- .../omap2/omapfb/displays/connector-analog-tv.c| 7 ++--- .../fbdev/omap2/omapfb/displays/connector-dvi.c| 7 ++--- .../fbdev/omap2/omapfb/displays/connector-hdmi.c | 7 ++--- .../fbdev/omap2/omapfb/displays/encoder-opa362.c | 7 ++--- .../fbdev/omap2/omapfb/displays/encoder-tfp410.c | 7 ++--- .../omap2/omapfb/displays/encoder-tpd12s015.c | 7 ++--- .../video/fbdev/omap2/omapfb/displays/panel-dpi.c | 7 ++--- .../fbdev/omap2/omapfb/displays/panel-dsi-cm.c | 7 ++--- .../omapfb/displays/panel-sharp-ls037v7dw01.c | 7 ++--- drivers/video/fbdev/omap2/omapfb/vrfb.c| 9 +- drivers/video/fbdev/via/viafbdev.c | 2 +- 18 files changed, 70 insertions(+), 110 deletions(-)
Re: [PATCH 0/2] fb: amifb: Convert to platform remove callback returning void
On 11/9/23 23:01, Uwe Kleine-König wrote: in the series https://lore.kernel.org/20231107091740.3924258-1-u.kleine-koe...@pengutronix.de I suggested to replace module_platform_driver_probe() in the amifb driver. Geert Uytterhoeven objected because the amiga platform has very little RAM. This series implements the alternative: Mark the driver struct with __refdata to suppress the section warning. Patch #2 is just a rebase from the above series. Thanks for the follow-up! I've applied both patches. Helge
Re: [PATCH 01/22] fb: amifb: Stop using platform_driver_probe()
On 11/8/23 22:34, Geert Uytterhoeven wrote: Hi Helge, On Wed, Nov 8, 2023 at 10:32 PM Helge Deller wrote: On 11/8/23 22:06, Geert Uytterhoeven wrote: On Tue, Nov 7, 2023 at 10:20 AM Uwe Kleine-König wrote: On today's platforms the benefit of platform_driver_probe() isn't that relevant any more. It allows to drop some code after booting (or module loading) for .probe() and discard the .remove() function completely if the driver is built-in. This typically saves a few 100k. Which is a lot on platforms with only a few MiBs of RAM... True. Given the warnings below, what is your suggestion? Better to drop the amifb patch ? I think so. There is a reason these drivers use platform_driver_probe(). Ok, I've dropped both amifb patches. Helge
Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()
On 11/9/23 07:24, Uwe Kleine-König wrote:
Hello,
On Wed, Nov 08, 2023 at 10:57:00PM +0100, Helge Deller wrote:
On 11/8/23 22:52, Uwe Kleine-König wrote:
But info and so info->fix live longer than the probe function, don't
they?
Yes, they do.
But AFAICS info->fix contains a *copy* of the initial atmel_lcdfb_fix struct
(and not a pointer to it). So that should be ok.
If you say so that's good. I grepped a bit around and didn't find a
place where a copy is made. But that's probably me and I'll consider the
case closed.
It's not directly obvious, but the copy happens in the line you pointed
out previously.
In include/linux/fb.h:
struct fb_info {
...
struct fb_var_screeninfo var; /* Current var */
struct fb_fix_screeninfo fix; /* Current fix */
so, "fb_info.fix" is a struct, and not a pointer.
In drivers/video/fbdev/atmel_lcdfb.c:
static int atmel_lcdfb_probe(struct platform_device *pdev)
{
...
info->fix = atmel_lcdfb_fix; // (line 1065)
this becomes effectively a:
memcpy(>fix, _lcdfb_fix, sizeof(struct fb_fix_screeninfo));
so, the compiler copies the "__initconst" data over to the info->fix struct.
Helge
Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()
On 11/8/23 22:52, Uwe Kleine-König wrote:
On Wed, Nov 08, 2023 at 10:24:09PM +0100, Helge Deller wrote:
On 11/8/23 22:00, Uwe Kleine-König wrote:
On Wed, Nov 08, 2023 at 11:48:05AM -0700, Nathan Chancellor wrote:
diff --git a/drivers/video/fbdev/atmel_lcdfb.c
b/drivers/video/fbdev/atmel_lcdfb.c
index 88c75ae7d315..9e391e5eaf9d 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct
atmel_lcdfb_info *sinfo, int
}
}
-static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = {
+static const struct fb_fix_screeninfo atmel_lcdfb_fix = {
.type = FB_TYPE_PACKED_PIXELS,
.visual = FB_VISUAL_TRUECOLOR,
.xpanstep = 0,
I wonder if this was broken already before my patch. atmel_lcdfb_probe()
does
info->fix = atmel_lcdfb_fix;
and unless I miss something (this is well possible) that is used e.g. in
atmel_lcdfb_set_par() -> atmel_lcdfb_update_dma(). So atmel_lcdfb_fix
should better not live in .init memory?! Someone with more knowledge
about fbdev might want to take a look and decide if this justifies a
separate fix that should then be backported to stable, too?!
I don't think a backport this is necessary.
The "__initconst" atmel_lcdfb_fix struct was only copied in the
"__init" atmel_lcdfb_probe() function.
So, both were dropped at the same time in older kernels.
But info and so info->fix live longer than the probe function, don't
they?
Yes, they do.
But AFAICS info->fix contains a *copy* of the initial atmel_lcdfb_fix struct
(and not a pointer to it). So that should be ok.
Helge
Re: [PATCH 01/22] fb: amifb: Stop using platform_driver_probe()
On 11/8/23 22:06, Geert Uytterhoeven wrote: On Tue, Nov 7, 2023 at 10:20 AM Uwe Kleine-König wrote: On today's platforms the benefit of platform_driver_probe() isn't that relevant any more. It allows to drop some code after booting (or module loading) for .probe() and discard the .remove() function completely if the driver is built-in. This typically saves a few 100k. Which is a lot on platforms with only a few MiBs of RAM... True. Given the warnings below, what is your suggestion? Better to drop the amifb patch ? Helge ... WARNING: modpost: vmlinux: section mismatch in reference: amifb_probe+0x15c (section: .text) -> ami_modedb (section: .init.data) ...
Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()
On 11/8/23 22:00, Uwe Kleine-König wrote:
On Wed, Nov 08, 2023 at 11:48:05AM -0700, Nathan Chancellor wrote:
diff --git a/drivers/video/fbdev/atmel_lcdfb.c
b/drivers/video/fbdev/atmel_lcdfb.c
index 88c75ae7d315..9e391e5eaf9d 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct
atmel_lcdfb_info *sinfo, int
}
}
-static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = {
+static const struct fb_fix_screeninfo atmel_lcdfb_fix = {
.type = FB_TYPE_PACKED_PIXELS,
.visual = FB_VISUAL_TRUECOLOR,
.xpanstep = 0,
I wonder if this was broken already before my patch. atmel_lcdfb_probe()
does
info->fix = atmel_lcdfb_fix;
and unless I miss something (this is well possible) that is used e.g. in
atmel_lcdfb_set_par() -> atmel_lcdfb_update_dma(). So atmel_lcdfb_fix
should better not live in .init memory?! Someone with more knowledge
about fbdev might want to take a look and decide if this justifies a
separate fix that should then be backported to stable, too?!
I don't think a backport this is necessary.
The "__initconst" atmel_lcdfb_fix struct was only copied in the
"__init" atmel_lcdfb_probe() function.
So, both were dropped at the same time in older kernels.
Since your patch dropped the "__init" from atmel_lcdfb_probe(),
the __initconst from atmel_lcdfb_fix has to be removed too.
So, I believe folding in Nathan's patch is OK and we don't need
a seperate (or backport) patch.
Helge
Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()
On 11/8/23 19:48, Nathan Chancellor wrote:
On Tue, Nov 07, 2023 at 10:17:43AM +0100, Uwe Kleine-König wrote:
On today's platforms the benefit of platform_driver_probe() isn't that
relevant any more. It allows to drop some code after booting (or module
loading) for .probe() and discard the .remove() function completely if
the driver is built-in. This typically saves a few 100k.
The downside of platform_driver_probe() is that the driver cannot be
bound and unbound at runtime which is ancient and also slightly
complicates testing. There are also thoughts to deprecate
platform_driver_probe() because it adds some complexity in the driver
core for little gain. Also many drivers don't use it correctly. This
driver for example misses to mark the driver struct with __refdata which
is needed to suppress a (W=1) modpost warning:
WARNING: modpost: drivers/video/fbdev/atmel_lcdfb: section mismatch in
reference: atmel_lcdfb_driver+0x4 (section: .data) -> atmel_lcdfb_remove
(section: .exit.text)
Signed-off-by: Uwe Kleine-König
---
drivers/video/fbdev/atmel_lcdfb.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/atmel_lcdfb.c
b/drivers/video/fbdev/atmel_lcdfb.c
index a908db233409..b218731ef732 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1017,7 +1017,7 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info
*sinfo)
return ret;
}
-static int __init atmel_lcdfb_probe(struct platform_device *pdev)
+static int atmel_lcdfb_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct fb_info *info;
@@ -1223,7 +1223,7 @@ static int __init atmel_lcdfb_probe(struct
platform_device *pdev)
return ret;
}
-static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
+static int atmel_lcdfb_remove(struct platform_device *pdev)
{
struct device *dev = >dev;
struct fb_info *info = dev_get_drvdata(dev);
@@ -1301,7 +1301,8 @@ static int atmel_lcdfb_resume(struct platform_device
*pdev)
#endif
static struct platform_driver atmel_lcdfb_driver = {
- .remove = __exit_p(atmel_lcdfb_remove),
+ .probe = atmel_lcdfb_probe,
+ .remove = atmel_lcdfb_remove,
.suspend= atmel_lcdfb_suspend,
.resume = atmel_lcdfb_resume,
.driver = {
@@ -1310,7 +1311,7 @@ static struct platform_driver atmel_lcdfb_driver = {
},
};
-module_platform_driver_probe(atmel_lcdfb_driver, atmel_lcdfb_probe);
+module_platform_driver(atmel_lcdfb_driver, );
MODULE_DESCRIPTION("AT91 LCD Controller framebuffer driver");
MODULE_AUTHOR("Nicolas Ferre ");
--
2.42.0
For what it's worth, this introduces a warning when building certain
configurations (such as ARCH=arm multi_v5_defconfig) with clang:
WARNING: modpost: vmlinux: section mismatch in reference:
atmel_lcdfb_probe+0x6c4 (section: .text) -> atmel_lcdfb_init_fbinfo (section:
.init.text)
WARNING: modpost: vmlinux: section mismatch in reference:
atmel_lcdfb_probe+0x858 (section: .text) -> atmel_lcdfb_fix (section:
.init.rodata)
This appears to be legitimate to me? GCC did not warn but I assume that
is due to differences in inlining. The following clears it up for me,
should I send a standalone patch or should this be squashed in?
I've squashed it into the original patch.
Thank you!
Helge
Cheers,
Nathan
diff --git a/drivers/video/fbdev/atmel_lcdfb.c
b/drivers/video/fbdev/atmel_lcdfb.c
index 88c75ae7d315..9e391e5eaf9d 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct
atmel_lcdfb_info *sinfo, int
}
}
-static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = {
+static const struct fb_fix_screeninfo atmel_lcdfb_fix = {
.type = FB_TYPE_PACKED_PIXELS,
.visual = FB_VISUAL_TRUECOLOR,
.xpanstep = 0,
@@ -841,7 +841,7 @@ static void atmel_lcdfb_task(struct work_struct *work)
atmel_lcdfb_reset(sinfo);
}
-static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
+static int atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
{
struct fb_info *info = sinfo->info;
int ret = 0;
Re: [PATCH] fbdev: hyperv_fb: fix uninitialized local variable use
On 11/8/23 15:58, Arnd Bergmann wrote:
From: Arnd Bergmann
When CONFIG_SYSFB is disabled, the hyperv_fb driver can now run into
undefined behavior on a gen2 VM, as indicated by this smatch warning:
drivers/video/fbdev/hyperv_fb.c:1077 hvfb_getmem() error: uninitialized symbol
'base'.
drivers/video/fbdev/hyperv_fb.c:1077 hvfb_getmem() error: uninitialized symbol
'size'.
Since there is no way to know the actual framebuffer in this configuration,
just return an allocation failure here, which should avoid the build
warning and the undefined behavior.
Reported-by: kernel test robot
Reported-by: Dan Carpenter
Closes: https://lore.kernel.org/r/202311070802.ycpvehaz-...@intel.com/
Fixes: a07b50d80ab6 ("hyperv: avoid dependency on screen_info")
Signed-off-by: Arnd Bergmann
applied.
Thanks!
Helge
---
drivers/video/fbdev/hyperv_fb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index bf59daf862fc..a80939fe2ee6 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -1013,6 +1013,8 @@ static int hvfb_getmem(struct hv_device *hdev, struct
fb_info *info)
} else if (IS_ENABLED(CONFIG_SYSFB)) {
base = screen_info.lfb_base;
size = screen_info.lfb_size;
+ } else {
+ goto err1;
}
/*
Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()
On 11/7/23 21:01, Uwe Kleine-König wrote:
On Tue, Nov 07, 2023 at 10:17:43AM +0100, Uwe Kleine-König wrote:
On today's platforms the benefit of platform_driver_probe() isn't that
relevant any more. It allows to drop some code after booting (or module
loading) for .probe() and discard the .remove() function completely if
the driver is built-in. This typically saves a few 100k.
The downside of platform_driver_probe() is that the driver cannot be
bound and unbound at runtime which is ancient and also slightly
complicates testing. There are also thoughts to deprecate
platform_driver_probe() because it adds some complexity in the driver
core for little gain. Also many drivers don't use it correctly. This
driver for example misses to mark the driver struct with __refdata which
is needed to suppress a (W=1) modpost warning:
WARNING: modpost: drivers/video/fbdev/atmel_lcdfb: section mismatch in
reference: atmel_lcdfb_driver+0x4 (section: .data) -> atmel_lcdfb_remove
(section: .exit.text)
Signed-off-by: Uwe Kleine-König
---
drivers/video/fbdev/atmel_lcdfb.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/atmel_lcdfb.c
b/drivers/video/fbdev/atmel_lcdfb.c
index a908db233409..b218731ef732 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1017,7 +1017,7 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info
*sinfo)
return ret;
}
-static int __init atmel_lcdfb_probe(struct platform_device *pdev)
+static int atmel_lcdfb_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct fb_info *info;
@@ -1223,7 +1223,7 @@ static int __init atmel_lcdfb_probe(struct
platform_device *pdev)
return ret;
}
-static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
+static int atmel_lcdfb_remove(struct platform_device *pdev)
{
struct device *dev = >dev;
struct fb_info *info = dev_get_drvdata(dev);
@@ -1301,7 +1301,8 @@ static int atmel_lcdfb_resume(struct platform_device
*pdev)
#endif
static struct platform_driver atmel_lcdfb_driver = {
- .remove = __exit_p(atmel_lcdfb_remove),
+ .probe = atmel_lcdfb_probe,
+ .remove = atmel_lcdfb_remove,
.suspend= atmel_lcdfb_suspend,
.resume = atmel_lcdfb_resume,
.driver = {
@@ -1310,7 +1311,7 @@ static struct platform_driver atmel_lcdfb_driver = {
},
};
-module_platform_driver_probe(atmel_lcdfb_driver, atmel_lcdfb_probe);
+module_platform_driver(atmel_lcdfb_driver, );
Argh, the , must be removed. I had this in my working copy but forgot to
squash it into this commit. Sorry!
Can you squash in the following please?:
Sure.
I fixed it up in the git tree.
Thanks!
Helge
diff --git a/drivers/video/fbdev/atmel_lcdfb.c
b/drivers/video/fbdev/atmel_lcdfb.c
index 0531d6f6dcc5..88c75ae7d315 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1308,8 +1308,7 @@ static struct platform_driver atmel_lcdfb_driver = {
.of_match_table = atmel_lcdfb_dt_ids,
},
};
-
-module_platform_driver(atmel_lcdfb_driver, );
+module_platform_driver(atmel_lcdfb_driver);
MODULE_DESCRIPTION("AT91 LCD Controller framebuffer driver");
MODULE_AUTHOR("Nicolas Ferre ");
Best regards
Uwe
Re: [PATCH 00/22] fb: handle remove callbacks in .exit.text and convert to .remove_new
On 11/7/23 10:17, Uwe Kleine-König wrote:
there are currently several platform drivers that have their .remove
callback defined in .exit.text. While this works fine, it comes with a
few downsides: Since commit f177cd0c15fc ("modpost: Don't let "driver"s
reference .exit.*") it triggers a modpost warning unless the driver
struct is marked with __refdata. None of the drivers in
drivers/video/fbdev get that right (which is understandable the warning
was added only recently). While it would be possible to add that marker,
that's also a bit ugly as this bypasses all other section checks that
modpost does. Having the remove callback in .exit.text also means that
the corresponding devices cannot be unbound at runtime which is
sometimes usefull for debugging purposes.
To fix the modpost warning I picked the progressive option and moved the
.remove() callbacks (and for two drivers also .probe()) into .text (i.e.
the default code section) and dropped .suppress_bind_attrs = true (which
is implicitly set for drivers using platform_driver_probe()). Note even
though these patches fix a warning, it currently only happens with W=1,
so this isn't urgent and there is no need to apply these before v6.7.
The next merge window is fine (although I wouldn't object an earlier
application of course :-) The alternative is to add the __refdata
marker, ideally with a comment describing the need. (See e.g. commit
141626dbc2e6 ("rtc: sh: Mark driver struct with __refdata to prevent
section mismatch warning") .)
As a follow-up I converted the affected drivers to .remove_new(). There
was already a series doing this for the other drivers in
drivers/video/fb, but my coccinelle script missed these drivers as it
didn't handle
.remove = __exit_p(removefunc),
. See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal. I
considered creating a second series for this conversion, but as the
patches conflict I put all patches in a single series to make it easier
to apply it.
Thanks Uwe!
I've applied the series as-is to the fbdev for-next git tree.
The patches look ok, and if they survive the next few days they will go
upstream soon.
Helge
Re: [PATCH 1/2] fbdev/imsttfb: fix double free in probe()
On 10/27/23 14:04, Dan Carpenter wrote:
The init_imstt() function calls framebuffer_release() on error and then
the probe() function calls it again. It should only be done in probe.
Fixes: 518ecb6a209f ("fbdev: imsttfb: Fix error path of imsttfb_probe()")
Signed-off-by: Dan Carpenter
Both patches applied.
Thanks!
Helge
Re: [PATCH] drivers/video/fbdev: use new array-copying-wrapper
On 11/2/23 20:24, Philipp Stanner wrote: viafbdev.c utilizes memdup_user() to copy an array from userspace. There is a new wrapper, specifically designed for copying arrays. Use this one instead. Suggested-by: Dave Airlie Signed-off-by: Philipp Stanner applied. Thanks! Helge --- drivers/video/fbdev/via/viafbdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/via/viafbdev.c b/drivers/video/fbdev/via/viafbdev.c index 58868f8880d6..a52b1ba43a48 100644 --- a/drivers/video/fbdev/via/viafbdev.c +++ b/drivers/video/fbdev/via/viafbdev.c @@ -574,7 +574,7 @@ static int viafb_ioctl(struct fb_info *info, u_int cmd, u_long arg) break; case VIAFB_SET_GAMMA_LUT: - viafb_gamma_table = memdup_user(argp, 256 * sizeof(u32)); + viafb_gamma_table = memdup_array_user(argp, 256, sizeof(u32)); if (IS_ERR(viafb_gamma_table)) return PTR_ERR(viafb_gamma_table); viafb_set_gamma_table(viafb_bpp, viafb_gamma_table);
Re: [PATCH] fbdev: omapfb: Drop unused remove function
On 11/3/23 18:35, Uwe Kleine-König wrote:
OMAP2_VRFB is a bool, so the vrfb driver can never be compiled as a
module. With that __exit_p(vrfb_remove) always evaluates to NULL and
vrfb_remove() is unused.
If the driver was compilable as a module, it would fail to build because
the type of vrfb_remove() isn't compatible with struct
platform_driver::remove(). (The former returns void, the latter int.)
Fixes: aa1e49a3752f ("OMAPDSS: VRFB: add omap_vrfb_supported()")
Signed-off-by: Uwe Kleine-König
applied.
Thanks!
Helge
---
drivers/video/fbdev/omap2/omapfb/vrfb.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/vrfb.c
b/drivers/video/fbdev/omap2/omapfb/vrfb.c
index ee0dd4c6a646..568e6e1eca62 100644
--- a/drivers/video/fbdev/omap2/omapfb/vrfb.c
+++ b/drivers/video/fbdev/omap2/omapfb/vrfb.c
@@ -368,17 +368,10 @@ static int __init vrfb_probe(struct platform_device *pdev)
return 0;
}
-static void __exit vrfb_remove(struct platform_device *pdev)
-{
- vrfb_loaded = false;
-}
-
static struct platform_driver vrfb_driver = {
.driver.name= "omapvrfb",
- .remove = __exit_p(vrfb_remove),
};
-
-module_platform_driver_probe(vrfb_driver, vrfb_probe);
+builtin_platform_driver_probe(vrfb_driver, vrfb_probe);
MODULE_AUTHOR("Tomi Valkeinen ");
MODULE_DESCRIPTION("OMAP VRFB");
base-commit: e27090b1413ff236ca1aec26d6b022149115de2c
Re: [PATCH] fbdev/offb: Simplify offb_init_fb()
On 10/21/23 08:53, Christophe JAILLET wrote: Turn a strcpy()+strncat()+'\0' into an equivalent snprintf(). Signed-off-by: Christophe JAILLET applied. Thanks! Helge
Re: [PATCH v1 1/1] fbdev: omapfb: Replace custom memparse() implementation
On 10/18/23 05:47, Andy Shevchenko wrote: Our library has memparse() for parsing numbers with respective suffixes suitable for memory sizes. Use it instead of custom implementation. Signed-off-by: Andy Shevchenko --- drivers/video/fbdev/omap/omapfb_main.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) both patches applied. Thanks! Helge
[GIT PULL] fbdev fixes and updates for v6.6-rc7
Hi Linus, please pull a few unexiting small fbdev fixes & cleanups for 6.6-rc7. Thanks, Helge --- The following changes since commit ce9ecca0238b140b88f43859b211c9fdfd8e5b70: Linux 6.6-rc2 (2023-09-17 14:40:24 -0700) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.6-rc7 for you to fetch changes up to e8e4a470b677511f9d1ad4f3cef32adc1d9a60ca: fbdev: core: syscopyarea: fix sloppy typing (2023-10-16 23:22:53 +0200) fbdev fixes and cleanups for 6.6-rc7: various minor fixes, cleanups and annotations for atyfb, sa1100fb, omapfb, uvesafb and mmp. Arnd Bergmann (2): fbdev: atyfb: only use ioremap_uc() on i386 and ia64 fbdev: sa1100fb: mark sa1100fb_init() static Dan Carpenter (1): fbdev: omapfb: fix some error codes Jorge Maidana (2): fbdev: uvesafb: Remove uvesafb_exec() prototype from include/video/uvesafb.h fbdev: uvesafb: Call cn_del_callback() at the end of uvesafb_exit() Kees Cook (2): fbdev: mmp: Annotate struct mmphw_ctrl with __counted_by fbdev: mmp: Annotate struct mmp_path with __counted_by Sergey Shtylyov (2): fbdev: core: cfbcopyarea: fix sloppy typing fbdev: core: syscopyarea: fix sloppy typing drivers/video/fbdev/aty/atyfb_base.c | 4 drivers/video/fbdev/core/cfbcopyarea.c | 2 +- drivers/video/fbdev/core/syscopyarea.c | 2 +- drivers/video/fbdev/mmp/hw/mmp_ctrl.h | 2 +- drivers/video/fbdev/omap/omapfb_main.c | 4 ++-- drivers/video/fbdev/sa1100fb.c | 2 +- drivers/video/fbdev/uvesafb.c | 2 +- include/video/mmp_disp.h | 2 +- include/video/uvesafb.h| 2 -- 9 files changed, 12 insertions(+), 10 deletions(-)
Re: [PATCH v2 1/2] video: fbdev: core: cfbcopyarea: fix sloppy typing
On 10/13/23 22:50, Sergey Shtylyov wrote:
In cfb_copyarea(), the local variable bits_per_line is needlessly typed as
*unsigned long* -- which is a 32-bit type on the 32-bit arches and a 64-bit
type on the 64-bit arches; that variable's value is derived from the __u32
typed fb_fix_screeninfo::line_length field (multiplied by 8u) and a 32-bit
*unsigned int* type should still be enough to store the # of bits per line.
Found by Linux Verification Center (linuxtesting.org) with the Svace static
analysis tool.
Signed-off-by: Sergey Shtylyov
applied both patches.
Thanks!
Helge
---
drivers/video/fbdev/core/cfbcopyarea.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/cfbcopyarea.c
b/drivers/video/fbdev/core/cfbcopyarea.c
index 6d4bfeecee35..5b80bf3dae50 100644
--- a/drivers/video/fbdev/core/cfbcopyarea.c
+++ b/drivers/video/fbdev/core/cfbcopyarea.c
@@ -382,7 +382,7 @@ void cfb_copyarea(struct fb_info *p, const struct
fb_copyarea *area)
{
u32 dx = area->dx, dy = area->dy, sx = area->sx, sy = area->sy;
u32 height = area->height, width = area->width;
- unsigned long const bits_per_line = p->fix.line_length*8u;
+ unsigned int const bits_per_line = p->fix.line_length * 8u;
unsigned long __iomem *base = NULL;
int bits = BITS_PER_LONG, bytes = bits >> 3;
unsigned dst_idx = 0, src_idx = 0, rev_copy = 0;
Re: [PATCH 1/2] fbdev/uvesafb: Remove uvesafb_exec() prototype from include/video/uvesafb.h
On 10/6/23 22:43, Jorge Maidana wrote:
uvesafb_exec() is a static function defined and called only in
drivers/video/fbdev/uvesafb.c, remove the prototype from
include/video/uvesafb.h.
Fixes the warning:
./include/video/uvesafb.h:112:12: warning: 'uvesafb_exec' declared 'static' but
never defined [-Wunused-function]
when including '' in an external program.
Signed-off-by: Jorge Maidana
applied both patches.
Thanks!
Helge
---
include/video/uvesafb.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/video/uvesafb.h b/include/video/uvesafb.h
index 8d2a3bfc8..47d96e75e 100644
--- a/include/video/uvesafb.h
+++ b/include/video/uvesafb.h
@@ -109,8 +109,6 @@ struct uvesafb_ktask {
u32 ack;
};
-static int uvesafb_exec(struct uvesafb_ktask *tsk);
-
#define UVESAFB_EXACT_RES 1
#define UVESAFB_EXACT_DEPTH 2
Re: [PATCH] fbdev: fbmon: fix potential divide error in fb_validate_mode
On 10/14/23 14:14, Zhang Shurong wrote: We can easily use FBIOPUT_VSCREENINFO set fb_var_screeninfo, so it's possible for a divide by zero error to occur. Fix this by making sure the divisor is non-zero before the computation. Signed-off-by: Zhang Shurong --- drivers/video/fbdev/core/fbmon.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c index 79e5bfbdd34c..bdd15b8e3a71 100644 --- a/drivers/video/fbdev/core/fbmon.c +++ b/drivers/video/fbdev/core/fbmon.c @@ -1470,6 +1470,9 @@ int fb_validate_mode(const struct fb_var_screeninfo *var, struct fb_info *info) if (var->vmode & FB_VMODE_DOUBLE) vtotal *= 2; + if (!htotal || !vtotal) + return -EINVAL; This is above here: htotal = var->xres + var->right_margin + var->hsync_len + var->left_margin; vtotal = var->yres + var->lower_margin + var->vsync_len + var->upper_margin; I don't see how htotal and vtotal can become zero... Helge
Re: [PATCH] fbdev: omapfb: fix some error codes
On 10/16/23 13:19, Dan Carpenter wrote:
Return negative -ENXIO instead of positive ENXIO.
Signed-off-by: Dan Carpenter
applied.
Thanks!
Helge
---
No fixes tag because the caller doesn't check for errors.
drivers/video/fbdev/omap/omapfb_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/omap/omapfb_main.c
b/drivers/video/fbdev/omap/omapfb_main.c
index f28cb90947a3..42c96f1cfc93 100644
--- a/drivers/video/fbdev/omap/omapfb_main.c
+++ b/drivers/video/fbdev/omap/omapfb_main.c
@@ -1645,13 +1645,13 @@ static int omapfb_do_probe(struct platform_device *pdev,
}
fbdev->int_irq = platform_get_irq(pdev, 0);
if (fbdev->int_irq < 0) {
- r = ENXIO;
+ r = -ENXIO;
goto cleanup;
}
fbdev->ext_irq = platform_get_irq(pdev, 1);
if (fbdev->ext_irq < 0) {
- r = ENXIO;
+ r = -ENXIO;
goto cleanup;
}
Re: [PATCH] fbdev: sa1100fb: mark sa1100fb_init() static
On 10/16/23 22:04, Arnd Bergmann wrote:
From: Arnd Bergmann
This is a global function that is only referenced as an initcall. This causes
a warning:
drivers/video/fbdev/sa1100fb.c:1218:12: error: no previous prototype for
'sa1100fb_init' [-Werror=missing-prototypes]
Make it static instead.
Signed-off-by: Arnd Bergmann
applied.
Thanks!
Helge
---
drivers/video/fbdev/sa1100fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/sa1100fb.c b/drivers/video/fbdev/sa1100fb.c
index 63e12a7c1100e..e14eb634cb944 100644
--- a/drivers/video/fbdev/sa1100fb.c
+++ b/drivers/video/fbdev/sa1100fb.c
@@ -1215,7 +1215,7 @@ static struct platform_driver sa1100fb_driver = {
},
};
-int __init sa1100fb_init(void)
+static int __init sa1100fb_init(void)
{
if (fb_get_options("sa1100fb", NULL))
return -ENODEV;
Re: [PATCH] video: fbdev: arkfb: fix possible object reference leak
On 10/5/23 09:01, Zhang Shurong wrote: Add missing pci_disable_device() in error path in ark_pci_probe(). Do you have this hardware and tested your patch? I'm sure there is a reason, why "pci_disable_device()" was commented out in the original submission in commit 681e14730c73c... Additionally I'm wondering why your patch doesn't show up in the fbdev patchwork, although you added linux-fbdev mailing list. Probably a vger issue. Helge Signed-off-by: Zhang Shurong --- drivers/video/fbdev/arkfb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c index 60a96fdb5dd8..6c4e5065646f 100644 --- a/drivers/video/fbdev/arkfb.c +++ b/drivers/video/fbdev/arkfb.c @@ -1064,7 +1064,7 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) err_dac: pci_release_regions(dev); err_request_regions: -/* pci_disable_device(dev); */ + pci_disable_device(dev); err_enable_device: framebuffer_release(info); return rc; @@ -1085,7 +1085,7 @@ static void ark_pci_remove(struct pci_dev *dev) pci_iounmap(dev, info->screen_base); pci_release_regions(dev); -/* pci_disable_device(dev); */ + pci_disable_device(dev); framebuffer_release(info); }
Re: [PATCH v5 1/4] video: fbdev: atyfb: only use ioremap_uc() on i386 and ia64
On 9/21/23 13:04, Baoquan He wrote: From: Arnd Bergmann ioremap_uc() is only meaningful on old x86-32 systems with the PAT extension, and on ia64 with its slightly unconventional ioremap() behavior, everywhere else this is the same as ioremap() anyway. Change the only driver that still references ioremap_uc() to only do so on x86-32/ia64 in order to allow removing that interface at some point in the future for the other architectures. On some architectures, ioremap_uc() just returns NULL, changing the driver to call ioremap() means that they now have a chance of working correctly. Signed-off-by: Arnd Bergmann Signed-off-by: Baoquan He Reviewed-by: Luis Chamberlain Cc: Helge Deller Cc: Thomas Zimmermann Cc: Christophe Leroy Cc: linux-fb...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org applied to fbdev git tree. Thanks! Helge --- drivers/video/fbdev/aty/atyfb_base.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c index 5c87817a4f4c..3dcf83f5e7b4 100644 --- a/drivers/video/fbdev/aty/atyfb_base.c +++ b/drivers/video/fbdev/aty/atyfb_base.c @@ -3440,11 +3440,15 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info, } info->fix.mmio_start = raddr; +#if defined(__i386__) || defined(__ia64__) /* * By using strong UC we force the MTRR to never have an * effect on the MMIO region on both non-PAT and PAT systems. */ par->ati_regbase = ioremap_uc(info->fix.mmio_start, 0x1000); +#else + par->ati_regbase = ioremap(info->fix.mmio_start, 0x1000); +#endif if (par->ati_regbase == NULL) return -ENOMEM;
Re: [PATCH] video: fbdev: mmp: Annotate struct mmphw_ctrl with __counted_by
On 9/23/23 18:33, Gustavo A. R. Silva wrote: On 9/22/23 11:51, Kees Cook wrote: Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct mmphw_ctrl. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Helge Deller Cc: linux-fb...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Both fbdev patches applied to fbdev git tree. Thank you Kees! Helge
Re: [PATCH 1/2] video: fbdev: core: cfbcopyarea: fix sloppy typing
On 9/19/23 20:55, Sergey Shtylyov wrote:
On 9/19/23 10:05 AM, Helge Deller wrote:
In cfb_copyarea(), when initializing *unsigned long const* bits_per_line
__u32 typed fb_fix_screeninfo::line_length gets multiplied by 8u -- which
might overflow __u32; multiplying by 8UL instead should fix that...
Also, that bits_per_line constant is used to advance *unsigned* src_idx
and dst_idx variables -- which might be overflowed as well; declaring
them as *unsigned long* should fix that too...
Found by Linux Verification Center (linuxtesting.org) with the Svace static
analysis tool.
Signed-off-by: Sergey Shtylyov
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/core/cfbcopyarea.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/core/cfbcopyarea.c
b/drivers/video/fbdev/core/cfbcopyarea.c
index 6d4bfeecee35..b67ba69ea2fb 100644
--- a/drivers/video/fbdev/core/cfbcopyarea.c
+++ b/drivers/video/fbdev/core/cfbcopyarea.c
@@ -382,10 +382,11 @@ void cfb_copyarea(struct fb_info *p, const struct
fb_copyarea *area)
{
u32 dx = area->dx, dy = area->dy, sx = area->sx, sy = area->sy;
u32 height = area->height, width = area->width;
- unsigned long const bits_per_line = p->fix.line_length*8u;
+ unsigned long const bits_per_line = p->fix.line_length * 8UL;
you wrote:
__u32 typed fb_fix_screeninfo::line_length gets multiplied by 8u -- which
might overflow __u32; multiplying by 8UL instead should fix that...
This would only be true on 64-bit CPUs, where unsigned long is 64 bits,
Right, Svace was run with the arm64 and x86_64 configs -- and I forgot
to make the emphasis on the 64-bit specifics here...
while on 32-bit CPUs, it's still 32 bits (same as _u32).
Yes, indeed. That *unsigned long const* doesn't seem justified
at all then...
Instead we could make bits_per_line __u32 (or unsigned int) too.
Yes. Will you accept such a patch? :-)
Sure.
Helge
Re: [PATCH 2/2] video: fbdev: core: syscopyarea: fix sloppy typing
On 9/18/23 22:52, Sergey Shtylyov wrote:
In sys_copyarea(), when initializing *unsigned long const* bits_per_line
__u32 typed fb_fix_screeninfo::line_length gets multiplied by 8u -- which
might overflow __u32; multiplying by 8UL instead should fix that...
Also, that bits_per_line constant is used to advance *unsigned* src_idx
and dst_idx variables -- which might be overflowed as well; declaring
them as *unsigned long* should fix that too...
Found by Linux Verification Center (linuxtesting.org) with the Svace static
analysis tool.
Signed-off-by: Sergey Shtylyov
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/core/syscopyarea.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/core/syscopyarea.c
b/drivers/video/fbdev/core/syscopyarea.c
index c1eda3190968..1035131383a6 100644
--- a/drivers/video/fbdev/core/syscopyarea.c
+++ b/drivers/video/fbdev/core/syscopyarea.c
@@ -316,10 +316,11 @@ void sys_copyarea(struct fb_info *p, const struct
fb_copyarea *area)
{
u32 dx = area->dx, dy = area->dy, sx = area->sx, sy = area->sy;
u32 height = area->height, width = area->width;
- unsigned long const bits_per_line = p->fix.line_length*8u;
+ unsigned long const bits_per_line = p->fix.line_length * 8UL;
unsigned long *base = NULL;
int bits = BITS_PER_LONG, bytes = bits >> 3;
- unsigned dst_idx = 0, src_idx = 0, rev_copy = 0;
+ unsigned long dst_idx = 0, src_idx = 0;
+ unsigned int rev_copy = 0;
As mentioned in the other mail, both patches are not needed.
Helge
if (p->state != FBINFO_STATE_RUNNING)
return;
Re: [PATCH 1/2] video: fbdev: core: cfbcopyarea: fix sloppy typing
On 9/18/23 22:52, Sergey Shtylyov wrote:
In cfb_copyarea(), when initializing *unsigned long const* bits_per_line
__u32 typed fb_fix_screeninfo::line_length gets multiplied by 8u -- which
might overflow __u32; multiplying by 8UL instead should fix that...
Also, that bits_per_line constant is used to advance *unsigned* src_idx
and dst_idx variables -- which might be overflowed as well; declaring
them as *unsigned long* should fix that too...
Found by Linux Verification Center (linuxtesting.org) with the Svace static
analysis tool.
Signed-off-by: Sergey Shtylyov
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/core/cfbcopyarea.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/core/cfbcopyarea.c
b/drivers/video/fbdev/core/cfbcopyarea.c
index 6d4bfeecee35..b67ba69ea2fb 100644
--- a/drivers/video/fbdev/core/cfbcopyarea.c
+++ b/drivers/video/fbdev/core/cfbcopyarea.c
@@ -382,10 +382,11 @@ void cfb_copyarea(struct fb_info *p, const struct
fb_copyarea *area)
{
u32 dx = area->dx, dy = area->dy, sx = area->sx, sy = area->sy;
u32 height = area->height, width = area->width;
- unsigned long const bits_per_line = p->fix.line_length*8u;
+ unsigned long const bits_per_line = p->fix.line_length * 8UL;
you wrote:
__u32 typed fb_fix_screeninfo::line_length gets multiplied by 8u -- which
might overflow __u32; multiplying by 8UL instead should fix that...
This would only be true on 64-bit CPUs, where unsigned long is 64 bits,
while on 32-bit CPUs, it's still 32 bits (same as _u32).
Instead we could make bits_per_line __u32 (or unsigned int) too.
unsigned long __iomem *base = NULL;
int bits = BITS_PER_LONG, bytes = bits >> 3;
- unsigned dst_idx = 0, src_idx = 0, rev_copy = 0;
+ unsigned long dst_idx = 0, src_idx = 0;
An "unsigned int" can address at least up to 4GB, which is fully sufficent here.
So, both patches don't have any real effect.
NAK.
Helge
Re: [PATCH] lockdep: Fix static memory detection even more
* Guenter Roeck :
> On 9/3/23 14:11, Helge Deller wrote:
> > * Guenter Roeck :
> > > Hi,
> > >
> > > On Sat, Aug 12, 2023 at 05:48:52PM +0200, Helge Deller wrote:
> > > > On the parisc architecture, lockdep reports for all static objects which
> > > > are in the __initdata section (e.g. "setup_done" in devtmpfs,
> > > > "kthreadd_done" in init/main.c) this warning:
> > > >
> > > > INFO: trying to register non-static key.
> > > >
> > > > The warning itself is wrong, because those objects are in the __initdata
> > > > section, but the section itself is on parisc outside of range from
> > > > _stext to _end, which is why the static_obj() functions returns a wrong
> > > > answer.
> > > >
> > > > While fixing this issue, I noticed that the whole existing check can
> > > > be simplified a lot.
> > > > Instead of checking against the _stext and _end symbols (which include
> > > > code areas too) just check for the .data and .bss segments (since we
> > > > check a
> > > > data object). This can be done with the existing is_kernel_core_data()
> > > > macro.
> > > >
> > > > In addition objects in the __initdata section can be checked with
> > > > init_section_contains().
> > > >
> > > > This partly reverts and simplifies commit bac59d18c701 ("x86/setup: Fix
> > > > static
> > > > memory detection").
> > > >
> > > > Tested on x86-64 and parisc.
> > > >
> > > > Signed-off-by: Helge Deller
> > > > Fixes: bac59d18c701 ("x86/setup: Fix static memory detection")
> > >
> > > On loongarch, this patch results in the following backtrace.
> > >
> > > EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
> > > EFI stub: Exiting boot services
> > > [0.00] INFO: trying to register non-static key.
> > > [0.00] The code is fine but needs lockdep annotation, or maybe
> > > [0.00] you didn't initialize this object before use?
> > > [0.00] turning off the locking correctness validator.
> > > [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 6.5.0+ #1
> > > [0.00] Stack : 90223d6c
> > > 91df
> > > [0.00] 91df39a0 91df39a8
> > >
> > > [0.00] 91df39a8 0001
> > > 9154b910
> > > [0.00] fffe 91df39a8
> > >
> > > [0.00] 0001 0003 0010
> > > 0030
> > > [0.00] 0063 0001
> > >
> > > [0.00] 91c60650
> > > 91e12000
> > > [0.00] 91560bc0
> > > 92ee6000
> > > [0.00] 90223d84
> > >
> > > [0.00] 00b0 0004
> > > 0800
> > > [0.00] ...
> > > [0.00] Call Trace:
> > > [0.00] [<90223d84>] show_stack+0x5c/0x180
> > > [0.00] [<9153e0b4>] dump_stack_lvl+0x88/0xd0
> > > [0.00] [<902bc548>] register_lock_class+0x768/0x770
> > > [0.00] [<902bc710>] __lock_acquire+0xb0/0x2a18
> > > [0.00] [<902bba1c>] lock_acquire+0x11c/0x328
> > > [0.00] [<90b34a60>] __debug_object_init+0x60/0x244
> > > [0.00] [<90337f94>] init_cgroup_housekeeping+0xe8/0x144
> > > [0.00] [<9033e364>] init_cgroup_root+0x38/0xa0
> > > [0.00] [<917801ac>] cgroup_init_early+0x44/0x16c
> > > [0.00] [<91770758>] start_kernel+0x50/0x624
> > > [0.00] [<915410b4>] kernel_entry+0xb4/0xc4
> > >
> > > Reverting it fixes the problem. Bisect log attached.
> > >
> > > This is also seen in v6.5.y and v6.4.y since the patch has been applied
> > > to
Re: [PATCH] lockdep: Fix static memory detection even more
* Guenter Roeck :
> Hi,
>
> On Sat, Aug 12, 2023 at 05:48:52PM +0200, Helge Deller wrote:
> > On the parisc architecture, lockdep reports for all static objects which
> > are in the __initdata section (e.g. "setup_done" in devtmpfs,
> > "kthreadd_done" in init/main.c) this warning:
> >
> > INFO: trying to register non-static key.
> >
> > The warning itself is wrong, because those objects are in the __initdata
> > section, but the section itself is on parisc outside of range from
> > _stext to _end, which is why the static_obj() functions returns a wrong
> > answer.
> >
> > While fixing this issue, I noticed that the whole existing check can
> > be simplified a lot.
> > Instead of checking against the _stext and _end symbols (which include
> > code areas too) just check for the .data and .bss segments (since we check a
> > data object). This can be done with the existing is_kernel_core_data()
> > macro.
> >
> > In addition objects in the __initdata section can be checked with
> > init_section_contains().
> >
> > This partly reverts and simplifies commit bac59d18c701 ("x86/setup: Fix
> > static
> > memory detection").
> >
> > Tested on x86-64 and parisc.
> >
> > Signed-off-by: Helge Deller
> > Fixes: bac59d18c701 ("x86/setup: Fix static memory detection")
>
> On loongarch, this patch results in the following backtrace.
>
> EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
> EFI stub: Exiting boot services
> [0.00] INFO: trying to register non-static key.
> [0.00] The code is fine but needs lockdep annotation, or maybe
> [0.00] you didn't initialize this object before use?
> [0.00] turning off the locking correctness validator.
> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 6.5.0+ #1
> [0.00] Stack : 90223d6c
> 91df
> [0.00] 91df39a0 91df39a8
>
> [0.00] 91df39a8 0001
> 9154b910
> [0.00] fffe 91df39a8
>
> [0.00] 0001 0003 0010
> 0030
> [0.00] 0063 0001
>
> [0.00] 91c60650
> 91e12000
> [0.00] 91560bc0
> 92ee6000
> [0.00] 90223d84
>
> [0.00] 00b0 0004
> 0800
> [0.00] ...
> [0.00] Call Trace:
> [0.00] [<90223d84>] show_stack+0x5c/0x180
> [0.00] [<9153e0b4>] dump_stack_lvl+0x88/0xd0
> [0.00] [<902bc548>] register_lock_class+0x768/0x770
> [0.00] [<902bc710>] __lock_acquire+0xb0/0x2a18
> [0.00] [<902bba1c>] lock_acquire+0x11c/0x328
> [0.00] [<90b34a60>] __debug_object_init+0x60/0x244
> [0.00] [<90337f94>] init_cgroup_housekeeping+0xe8/0x144
> [0.00] [<9033e364>] init_cgroup_root+0x38/0xa0
> [0.00] [<917801ac>] cgroup_init_early+0x44/0x16c
> [0.00] [<91770758>] start_kernel+0x50/0x624
> [0.00] [<915410b4>] kernel_entry+0xb4/0xc4
>
> Reverting it fixes the problem. Bisect log attached.
>
> This is also seen in v6.5.y and v6.4.y since the patch has been applied
> to those branches.
Does this happens with CONFIG_SMP=n ?
If so, I think the untested patch below might fix the issue.
Helge
---
[PATCH] loogarch: Keep PERCPU section in init section even for !CONFIG_SMP
Signed-off-by: Helge Deller
diff --git a/arch/loongarch/kernel/vmlinux.lds.S
b/arch/loongarch/kernel/vmlinux.lds.S
index b1686afcf876..32d61e931cdc 100644
--- a/arch/loongarch/kernel/vmlinux.lds.S
+++ b/arch/loongarch/kernel/vmlinux.lds.S
@@ -99,9 +99,7 @@ SECTIONS
EXIT_DATA
}
-#ifdef CONFIG_SMP
PERCPU_SECTION(1 << CONFIG_L1_CACHE_SHIFT)
-#endif
.init.bss : {
*(.init.bss)
Re: [PATCH 5/7] fbdev/core: Build fb_logo iff CONFIG_LOGO has been selected
On 8/29/23 16:15, Thomas Zimmermann wrote:
Only build fb_logo.c if CONFIG_LOGO has been selected. Otherwise
provide empty implementations of the contained interfaces and avoid
using the exported variables.
Signed-off-by: Thomas Zimmermann
...
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index f157a5a1dffc..24b038510a71 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -474,15 +474,19 @@ static int __init fb_console_setup(char *this_opt)
if (!strncmp(options, "logo-pos:", 9)) {
options += 9;
+#ifdef CONFIG_LOGO
if (!strcmp(options, "center"))
fb_center_logo = true;
+#endif
IMHO, *sometimes* it makes sense to not use #ifdef and code it instead like
this:
if (IS_ENABLED(CONFIG_LOGO) &&
!strcmp(options, "center"))
...
That way the compiler will optimize that code away as well, but in
addition it will compile-check that you have correct coding independend
if CONFIG_LOGO is set or not.
continue;
}
if (!strncmp(options, "logo-count:", 11)) {
options += 11;
+#ifdef CONFIG_LOGO
if (*options)
fb_logo_count = simple_strtol(options,
, 0);
+#endif
same here.
Helge
[GIT PULL] fbdev fixes and updates for v6.6-rc1
Hi Linus, please pull some fbdev fixes & updates for kernel 6.6-rc1. Remove the mx3fb driver and various smaller cleanups. Thanks! Helge -- The following changes since commit 87dfd85c38923acd9517e8df4afc908565df0961: Merge tag 'for-linus' of git://git.armlinux.org.uk/~rmk/linux-arm (2023-08-31 12:49:10 -0700) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.6-rc1 for you to fetch changes up to 4a9762aa358ee0e3deb6e759959f092a3cea86be: fbdev: Update fbdev source file paths (2023-09-01 00:29:34 +0200) fbdev fixes and cleanups for 6.6-rc1: - Drop the mx3fb driver - Use list_for_each_entry() helper in fbcore code - Shorten neofb product names for fb-fix id field - reduce memory usage in ssd1307fb Fabio Estevam (1): fbdev: mx3fb: Remove the driver Geert Uytterhoeven (1): fbdev: ssd1307fb: Use bool for ssd1307fb_deviceinfo flags Helge Deller (1): fbdev: neofb: Shorten Neomagic product name in info struct Jinjie Ruan (1): fbdev/core: Use list_for_each_entry() helper Jonathan Neuschäfer (1): fbdev: Update fbdev source file paths drivers/video/fbdev/Kconfig | 11 - drivers/video/fbdev/Makefile |1 - drivers/video/fbdev/core/fbsysfs.c|8 +- drivers/video/fbdev/core/modedb.c | 22 +- drivers/video/fbdev/mx3fb.c | 1695 - drivers/video/fbdev/neofb.c | 27 +- drivers/video/fbdev/ssd1307fb.c |4 +- include/linux/fb.h| 10 +- include/linux/platform_data/video-mx3fb.h | 50 - 9 files changed, 23 insertions(+), 1805 deletions(-) delete mode 100644 drivers/video/fbdev/mx3fb.c delete mode 100644 include/linux/platform_data/video-mx3fb.h
Re: truncation in drivers/video/fbdev/neofb.c
* Nick Desaulniers :
> On Thu, Aug 31, 2023 at 2:23 PM Helge Deller wrote:
> >
> > * Helge Deller :
> > > On 8/29/23 18:45, Nick Desaulniers wrote:
> > > > A recent change in clang made it better about spotting snprintf that
> > > > will result in truncation. Nathan reported the following instances:
> > > >
> > > > drivers/video/fbdev/neofb.c:1959:3: warning: 'snprintf' will always be
> > > > truncated; specified size is 16, but format string expands to at least
> > > > 17 [-Wfortify-source]
> >
> > FYI, I've added the patch below to the fbdev for-next git tree.
> > [...]
>
> This indeed makes the warning go away, but that's more so due to the
> use of strscpy now rather than snprintf. That alone is a good change
> but we still have definite truncation. See below:
> [...]
Nick, thanks for your review and findings!
Now every string should be max. 15 chars (which fits with the trailing
NUL into the char[16] array).
Helge
Subject: [PATCH] fbdev: neofb: Shorten Neomagic product name in info struct
Avoid those compiler warnings:
neofb.c:1959:3: warning: 'snprintf' will always be truncated;
specified size is 16, but format string expands to at least 17
[-Wfortify-source]
Signed-off-by: Helge Deller
Reported-by: Nathan Chancellor
Reported-by: Nick Desaulniers
Link:
https://lore.kernel.org/all/cakwvodn0xovwjq6ufm_rojtkb0f1i1hw-j_xygfkdnfdhwa...@mail.gmail.com/
Link: https://github.com/ClangBuiltLinux/linux/issues/1923
diff --git a/drivers/video/fbdev/neofb.c b/drivers/video/fbdev/neofb.c
index d2f622b4c372..b58b11015c0c 100644
--- a/drivers/video/fbdev/neofb.c
+++ b/drivers/video/fbdev/neofb.c
@@ -1948,49 +1948,40 @@ static struct fb_info *neo_alloc_fb_info(struct pci_dev
*dev,
switch (info->fix.accel) {
case FB_ACCEL_NEOMAGIC_NM2070:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128");
+ strscpy(info->fix.id, "MagicGraph128", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2090:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128V");
+ strscpy(info->fix.id, "MagicGraph128V", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2093:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128ZV");
+ strscpy(info->fix.id, "MagicGraph128ZV", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2097:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128ZV+");
+ strscpy(info->fix.id, "Mag.Graph128ZV+", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2160:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128XD");
+ strscpy(info->fix.id, "MagicGraph128XD", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2200:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 256AV");
+ strscpy(info->fix.id, "MagicGraph256AV", sizeof(info->fix.id));
info->flags |= FBINFO_HWACCEL_IMAGEBLIT |
FBINFO_HWACCEL_COPYAREA |
FBINFO_HWACCEL_FILLRECT;
break;
case FB_ACCEL_NEOMAGIC_NM2230:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 256AV+");
+ strscpy(info->fix.id, "Mag.Graph256AV+", sizeof(info->fix.id));
info->flags |= FBINFO_HWACCEL_IMAGEBLIT |
FBINFO_HWACCEL_COPYAREA |
FBINFO_HWACCEL_FILLRECT;
break;
case FB_ACCEL_NEOMAGIC_NM2360:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 256ZX");
+ strscpy(info->fix.id, "MagicGraph256ZX", sizeof(info->fix.id));
info->flags |= FBINFO_HWACCEL_IMAGEBLIT |
FBINFO_HWACCEL_COPYAREA |
FBINFO_HWACCEL_FILLRECT;
break;
case FB_ACCEL_NEOMAGIC_NM2380:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 256XL+");
+ strscpy(info->fix.id, "Mag.Graph256XL+", sizeof(info->fix.id));
info->flags |= FBINFO_HWACCEL_IMAGEBLIT |
FBINFO_HWACCEL_COPYAREA |
FBINFO_HWACCEL_FILLRECT;
Re: [PATCH] fbdev: Update fbdev source file paths
On 8/31/23 11:02, Helge Deller wrote:
On 8/31/23 10:51, Daniel Vetter wrote:
On Thu, Aug 31, 2023 at 08:44:59AM +0200, Jonathan Neuschäfer wrote:
On Wed, Aug 30, 2023 at 09:10:26AM +0200, Thomas Zimmermann wrote:
Hi
Am 29.08.23 um 22:02 schrieb Jonathan Neuschäfer:
The files fbmem.c, fb_defio.c, fbsysfs.c, fbmon.c, modedb.c, and
fbcmap.c were moved to drivers/video/fbdev, and subsequently to
drivers/video/fbdev/core, in the commits listed below.
Reported by kalekale in #kernel (Libera IRC).
Fixes: f7018c213502 ("video: move fbdev to drivers/video/fbdev")
Fixes: 19757fc8432a ("fbdev: move fbdev core files to separate directory")
Signed-off-by: Jonathan Neuschäfer
IMHO these comments might just be removed.
I think it's nice to have some sort of visual separation between groups
of functions in fb.h, which these comments provide at the moment.
Therefore I'm currently leaning towards my patch as it is, but I'm
willing to have my mind changed and do a v2 which just removes the
comments.
Just the filename without the full path maybe?
Yes, I'd prefer that as well.
I've manually changed it and applied the patch to the fbdev git tree.
Thanks!
Helge
Re: [PATCH] fbdev: ssd1307fb: Use bool for ssd1307fb_deviceinfo flags
On 8/31/23 14:05, Geert Uytterhoeven wrote:
The .need_pwm and .need_chargepump fields in struct ssd1307fb_deviceinfo
are flags that can have only two possible values: 0 and 1.
Reduce kernel size by changing their types from int to bool.
Signed-off-by: Geert Uytterhoeven
applied to fbdev git tree.
Thanks!
Helge
---
drivers/video/fbdev/ssd1307fb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 86dd24022871a843..5ae48e36fccb4e8d 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -52,8 +52,8 @@ struct ssd1307fb_deviceinfo {
u32 default_vcomh;
u32 default_dclk_div;
u32 default_dclk_frq;
- int need_pwm;
- int need_chargepump;
+ bool need_pwm;
+ bool need_chargepump;
};
struct ssd1307fb_par {
Re: truncation in drivers/video/fbdev/neofb.c
* Helge Deller :
> On 8/29/23 18:45, Nick Desaulniers wrote:
> > A recent change in clang made it better about spotting snprintf that
> > will result in truncation. Nathan reported the following instances:
> >
> > drivers/video/fbdev/neofb.c:1959:3: warning: 'snprintf' will always be
> > truncated; specified size is 16, but format string expands to at least
> > 17 [-Wfortify-source]
FYI, I've added the patch below to the fbdev for-next git tree.
Helge
From: Helge Deller
Subject: [PATCH] fbdev: neofb: Shorten Neomagic product name in info struct
Avoid those compiler warnings:
neofb.c:1959:3: warning: 'snprintf' will always be truncated;
specified size is 16, but format string expands to at least 17
[-Wfortify-source]
Signed-off-by: Helge Deller
Reported-by: Nathan Chancellor
Reported-by: Nick Desaulniers
Link:
https://lore.kernel.org/all/cakwvodn0xovwjq6ufm_rojtkb0f1i1hw-j_xygfkdnfdhwa...@mail.gmail.com/
Link: https://github.com/ClangBuiltLinux/linux/issues/1923
diff --git a/drivers/video/fbdev/neofb.c b/drivers/video/fbdev/neofb.c
index d2f622b4c372..b905fe93b525 100644
--- a/drivers/video/fbdev/neofb.c
+++ b/drivers/video/fbdev/neofb.c
@@ -1948,49 +1948,40 @@ static struct fb_info *neo_alloc_fb_info(struct pci_dev
*dev,
switch (info->fix.accel) {
case FB_ACCEL_NEOMAGIC_NM2070:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128");
+ strscpy(info->fix.id, "MagicGraph128", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2090:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128V");
+ strscpy(info->fix.id, "MagicGraph128V", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2093:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128ZV");
+ strscpy(info->fix.id, "MagicGraph128ZV", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2097:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128ZV+");
+ strscpy(info->fix.id, "Mag.Graph128ZV+", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2160:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 128XD");
+ strscpy(info->fix.id, "MagicGraph128XD", sizeof(info->fix.id));
break;
case FB_ACCEL_NEOMAGIC_NM2200:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 256AV");
+ strscpy(info->fix.id, "MagicGraph256AV", sizeof(info->fix.id));
info->flags |= FBINFO_HWACCEL_IMAGEBLIT |
FBINFO_HWACCEL_COPYAREA |
FBINFO_HWACCEL_FILLRECT;
break;
case FB_ACCEL_NEOMAGIC_NM2230:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 256AV+");
+ strscpy(info->fix.id, "MagicGraph256AV+", sizeof(info->fix.id));
info->flags |= FBINFO_HWACCEL_IMAGEBLIT |
FBINFO_HWACCEL_COPYAREA |
FBINFO_HWACCEL_FILLRECT;
break;
case FB_ACCEL_NEOMAGIC_NM2360:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 256ZX");
+ strscpy(info->fix.id, "MagicGraph256ZX", sizeof(info->fix.id));
info->flags |= FBINFO_HWACCEL_IMAGEBLIT |
FBINFO_HWACCEL_COPYAREA |
FBINFO_HWACCEL_FILLRECT;
break;
case FB_ACCEL_NEOMAGIC_NM2380:
- snprintf(info->fix.id, sizeof(info->fix.id),
- "MagicGraph 256XL+");
+ strscpy(info->fix.id, "MagicGraph256XL+", sizeof(info->fix.id));
info->flags |= FBINFO_HWACCEL_IMAGEBLIT |
FBINFO_HWACCEL_COPYAREA |
FBINFO_HWACCEL_FILLRECT;
Re: [PATCH] fbdev: Update fbdev source file paths
On 8/31/23 10:51, Daniel Vetter wrote:
On Thu, Aug 31, 2023 at 08:44:59AM +0200, Jonathan Neuschäfer wrote:
On Wed, Aug 30, 2023 at 09:10:26AM +0200, Thomas Zimmermann wrote:
Hi
Am 29.08.23 um 22:02 schrieb Jonathan Neuschäfer:
The files fbmem.c, fb_defio.c, fbsysfs.c, fbmon.c, modedb.c, and
fbcmap.c were moved to drivers/video/fbdev, and subsequently to
drivers/video/fbdev/core, in the commits listed below.
Reported by kalekale in #kernel (Libera IRC).
Fixes: f7018c213502 ("video: move fbdev to drivers/video/fbdev")
Fixes: 19757fc8432a ("fbdev: move fbdev core files to separate directory")
Signed-off-by: Jonathan Neuschäfer
IMHO these comments might just be removed.
I think it's nice to have some sort of visual separation between groups
of functions in fb.h, which these comments provide at the moment.
Therefore I'm currently leaning towards my patch as it is, but I'm
willing to have my mind changed and do a v2 which just removes the
comments.
Just the filename without the full path maybe?
Yes, I'd prefer that as well.
Helge
That's enough to find the
right file, and it's also better at highlighting the actual important part
of the comment since the path is very redundant.
-Sima
Thanks
Best regards
Thomas
---
include/linux/fb.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index ce7d588edc3e6..3cda5b9f2469b 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -592,7 +592,7 @@ extern ssize_t fb_sys_write(struct fb_info *info, const
char __user *buf,
__FB_DEFAULT_SYS_OPS_DRAW, \
__FB_DEFAULT_SYS_OPS_MMAP
-/* drivers/video/fbmem.c */
+/* drivers/video/fbdev/core/fbmem.c */
extern int register_framebuffer(struct fb_info *fb_info);
extern void unregister_framebuffer(struct fb_info *fb_info);
extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
Re: truncation in drivers/video/fbdev/neofb.c
On 8/29/23 18:45, Nick Desaulniers wrote:
Helge,
A recent change in clang made it better about spotting snprintf that
will result in truncation. Nathan reported the following instances:
drivers/video/fbdev/neofb.c:1959:3: warning: 'snprintf' will always be
truncated; specified size is 16, but format string expands to at least
17 [-Wfortify-source]
drivers/video/fbdev/neofb.c:1963:3: warning: 'snprintf' will always be
truncated; specified size is 16, but format string expands to at least
18 [-Wfortify-source]
drivers/video/fbdev/neofb.c:1967:3: warning: 'snprintf' will always be
truncated; specified size is 16, but format string expands to at least
17 [-Wfortify-source]
drivers/video/fbdev/neofb.c:1971:3: warning: 'snprintf' will always be
truncated; specified size is 16, but format string expands to at least
17 [-Wfortify-source]
drivers/video/fbdev/neofb.c:1978:3: warning: 'snprintf' will always be
truncated; specified size is 16, but format string expands to at least
18 [-Wfortify-source]
drivers/video/fbdev/neofb.c:1985:3: warning: 'snprintf' will always be
truncated; specified size is 16, but format string expands to at least
17 [-Wfortify-source]
drivers/video/fbdev/neofb.c:1992:3: warning: 'snprintf' will always be
truncated; specified size is 16, but format string expands to at least
18 [-Wfortify-source]
https://github.com/ClangBuiltLinux/linux/issues/1923
Clang is right here. `info->fix.id` is declared as `char id[16];` so
indeed string literals like "MagicGraph 256AV+" indeed lead to
truncation. But this is declared in include/uapi/linux/fb.h; I assume
those headers cant be changed? Can the strings be shortened then? Is
it perhaps time to delete this driver?
I see AKPM mentioned alluded to this in
commit 0e90454 ("neofb: avoid overwriting fb_info fields")
(Also, snprintf probably isn't necessary for string literals that
don't contain format strings)
It's just an ID field, so I don't think we need the full name of the card.
So using strscpy() and shorten the name, e.g. "MagicGr. 256XL+"
instead of "MagicGraph 256XL+" is probably the most simple solution?
Anyone want to send a patch?
Helge
Re: [PATCH -next] fbdev/core: Use list_for_each_entry() helper
On 8/23/23 09:21, Jinjie Ruan wrote: Convert list_for_each() to list_for_each_entry() so that the pos list_head pointer and list_entry() call are no longer needed, which can reduce a few lines of code. No functional changed. Signed-off-by: Jinjie Ruan applied. Thanks! Helge
[GIT PULL] fbdev fixes and cleanups for v6.5-rc7
Hi Linus, please pull some small fbdev fixes & cleanups for kernel 6.5-rc7. Various code cleanups in amifb, atmel_lcdfb, ssd1307fb, kyro and goldfishfb. Thanks! Helge -- The following changes since commit 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4: Linux 6.5-rc4 (2023-07-30 13:23:47 -0700) are available in the Git repository at: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.5-rc7 for you to fetch changes up to 0650d5098f8b6b232cd5ea0e15437fc38f7d63ba: fbdev: goldfishfb: Do not check 0 for platform_get_irq() (2023-08-15 23:39:33 +0200) fbdev fixes and cleanups for 6.5-rc7: - various code cleanups in amifb, atmel_lcdfb, ssd1307fb, kyro and goldfishfb Atul Raut (1): fbdev: amifb: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper Ruan Jinjie (1): fbdev: atmel_lcdfb: Remove redundant of_match_ptr() Uwe Kleine-König (1): fbdev: ssd1307fb: Print the PWM's label instead of its number Yuanjun Gong (1): fbdev: mmp: fix value check in mmphw_probe() Yue Haibing (1): fbdev: kyro: Remove unused declarations Zhu Wang (1): fbdev: goldfishfb: Do not check 0 for platform_get_irq() drivers/video/fbdev/amifb.c | 2 +- drivers/video/fbdev/atmel_lcdfb.c | 2 +- drivers/video/fbdev/goldfishfb.c | 4 ++-- drivers/video/fbdev/mmp/hw/mmp_ctrl.c | 4 +++- drivers/video/fbdev/ssd1307fb.c | 4 ++-- include/video/kyro.h | 12 6 files changed, 9 insertions(+), 19 deletions(-)
Re: [PATCH v2] fbcon: mark fbcon_registered_fb static
On 8/15/23 23:34, Helge Deller wrote:
On 8/4/23 13:32, Min-Hua Chen wrote:
Mark fbcon_registered_fb, and fbcon_num_registered_fb static
to fix the following sparse warnings:
drivers/video/fbdev/core/fbcon.c:105:16: sparse: warning: symbol
'fbcon_registered_fb' was not declared. Should it be static?
drivers/video/fbdev/core/fbcon.c:106:5: sparse: warning: symbol
'fbcon_num_registered_fb' was not declared. Should it be static?
No functional change intended.
Cc: Helge Deller
Signed-off-by: Min-Hua Chen
applied to the for-next-6.6 branch.
Just dropped it again, since it was already applied by Daniel to the drm-misc
tree ...
Helge
Thanks,
Helge
---
change since v1:
1. registered_fb and num_registered_fb are still used in
drivers/staging/olpc_dcon,
so do keep them as-is.
2. change the subject to fbcon: mark fbcon_registered_fb static
---
drivers/video/fbdev/core/fbcon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 887fad44e7ec..976900d6893c 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -102,8 +102,8 @@ enum {
static struct fbcon_display fb_display[MAX_NR_CONSOLES];
-struct fb_info *fbcon_registered_fb[FB_MAX];
-int fbcon_num_registered_fb;
+static struct fb_info *fbcon_registered_fb[FB_MAX];
+static int fbcon_num_registered_fb;
#define fbcon_for_each_registered_fb(i) \
for (i = 0; WARN_CONSOLE_UNLOCKED(), i < FB_MAX; i++) \
Re: [PATCH v2] fbcon: mark fbcon_registered_fb static
On 8/4/23 13:32, Min-Hua Chen wrote:
Mark fbcon_registered_fb, and fbcon_num_registered_fb static
to fix the following sparse warnings:
drivers/video/fbdev/core/fbcon.c:105:16: sparse: warning: symbol
'fbcon_registered_fb' was not declared. Should it be static?
drivers/video/fbdev/core/fbcon.c:106:5: sparse: warning: symbol
'fbcon_num_registered_fb' was not declared. Should it be static?
No functional change intended.
Cc: Helge Deller
Signed-off-by: Min-Hua Chen
applied to the for-next-6.6 branch.
Thanks,
Helge
---
change since v1:
1. registered_fb and num_registered_fb are still used in
drivers/staging/olpc_dcon,
so do keep them as-is.
2. change the subject to fbcon: mark fbcon_registered_fb static
---
drivers/video/fbdev/core/fbcon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 887fad44e7ec..976900d6893c 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -102,8 +102,8 @@ enum {
static struct fbcon_display fb_display[MAX_NR_CONSOLES];
-struct fb_info *fbcon_registered_fb[FB_MAX];
-int fbcon_num_registered_fb;
+static struct fb_info *fbcon_registered_fb[FB_MAX];
+static int fbcon_num_registered_fb;
#define fbcon_for_each_registered_fb(i) \
for (i = 0; WARN_CONSOLE_UNLOCKED(), i < FB_MAX; i++)\
