linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE
@ 2025-12-09  4:27 Chintan Patel
  2025-12-09  4:27 ` [PATCH 1/3] fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE Chintan Patel
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Chintan Patel @ 2025-12-09  4:27 UTC (permalink / raw)
  To: linux-fbdev, linux-staging, linux-omap
  Cc: linux-kernel, dri-devel, tzimmermann, andy, deller, gregkh,
	Chintan Patel

Hi all,

This small series makes several legacy fbdev drivers buildable with
CONFIG_FB_DEVICE=n. Currently, multiple fbdev drivers rely on fb_info->dev
and sysfs attribute registration unconditionally, which leads to build
failures whenever FB_DEVICE is disabled.

Thomas previously noted that FB_DEVICE should eventually become optional
and that drivers should not depend on sysfs or fb_info->dev being present
unless the Kconfig explicitly selects it. This series pushes in that
direction by tightening the FB_DEVICE dependency boundary without changing
any runtime behaviour when FB_DEVICE=y.

What this series does *not* change

- No functional behaviour changes when FB_DEVICE=y.
- No removal of sysfs interfaces.
- No changes to fbops, memory allocation, or display update paths.

Build & test coverage

Tested with the following combinations:

1. **FB=y, FB_DEVICE=y**  
   - Baseline configuration; no regressions expected.

2. **FB=y, FB_DEVICE=n**  
   - Drivers build successfully.
   - No sysfs attributes are created.
   - fbdev devices operate normally (where applicable).

3. **FB=n**  
   - Drivers depend on FB, so they properly do not build, unchanged.

Motivation

This moves fbdev closer to supporting FB_DEVICE as truly optional, helps
reduce Kconfig entanglement, and clears several long-standing TODO items
as suggested by Thomas Zimmermann around legacy sysfs usage inside fbdev 
drivers.

Feedback is welcome, especially on whether the guard boundaries around
sysfs are placed correctly or whether more logic should be pulled under
CONFIG_FB_DEVICE.

Thanks,
Chintan

Chintan Patel (3):
  fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE
  omapfb: Guard sysfs code under CONFIG_FB_DEVICE
  sh_mobile_lcdc: Guard overlay sysfs interfaces under CONFIG_FB_DEVICE

 drivers/staging/fbtft/fbtft-core.c            | 20 +++++++++++++++++--
 drivers/staging/fbtft/fbtft-sysfs.c           |  8 ++++++++
 drivers/video/fbdev/omap2/omapfb/Kconfig      |  2 +-
 .../video/fbdev/omap2/omapfb/omapfb-sysfs.c   | 11 ++++++++++
 drivers/video/fbdev/sh_mobile_lcdcfb.c        |  4 ++++
 5 files changed, 42 insertions(+), 3 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE
  2025-12-09  4:27 [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE Chintan Patel
@ 2025-12-09  4:27 ` Chintan Patel
  2025-12-09  7:25   ` Thomas Zimmermann
  2025-12-09  4:27 ` [PATCH 2/3] omapfb: Guard sysfs code under CONFIG_FB_DEVICE Chintan Patel
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Chintan Patel @ 2025-12-09  4:27 UTC (permalink / raw)
  To: linux-fbdev, linux-staging, linux-omap
  Cc: linux-kernel, dri-devel, tzimmermann, andy, deller, gregkh,
	Chintan Patel

The fbtft core and sysfs implementation unconditionally dereference
fb_info->dev and register sysfs attributes. When FB_DEVICE=n, these
fields are unavailable, leading to build failures.

This patch wraps all sysfs attribute creation/removal and dev_dbg/dev_info
logging in #ifdef CONFIG_FB_DEVICE, with pr_*() fallbacks for the
non-FB_DEVICE case. This makes fbtft fully buildable when FB_DEVICE is
disabled.

Signed-off-by: Chintan Patel <chintanlike@gmail.com>
---
 drivers/staging/fbtft/fbtft-core.c  | 20 ++++++++++++++++++--
 drivers/staging/fbtft/fbtft-sysfs.c |  8 ++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 9e7b84071174..dc967bdeabe8 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -365,9 +365,14 @@ static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
 	unsigned int val;
 	int ret = 1;
 
+#ifdef CONFIG_FB_DEVICE
 	dev_dbg(info->dev,
 		"%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
 		__func__, regno, red, green, blue, transp);
+#else
+	pr_debug("%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
+		 __func__, regno, red, green, blue, transp);
+#endif
 
 	switch (info->fix.visual) {
 	case FB_VISUAL_TRUECOLOR:
@@ -391,8 +396,11 @@ static int fbtft_fb_blank(int blank, struct fb_info *info)
 	struct fbtft_par *par = info->par;
 	int ret = -EINVAL;
 
-	dev_dbg(info->dev, "%s(blank=%d)\n",
-		__func__, blank);
+#ifdef CONFIG_FB_DEVICE
+	dev_dbg(info->dev, "%s(blank=%d)\n", __func__, blank);
+#else
+	pr_debug("%s(blank=%d)\n", __func__, blank);
+#endif
 
 	if (!par->fbtftops.blank)
 		return ret;
@@ -793,6 +801,8 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
 	if (spi)
 		sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,
 			spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
+
+#ifdef CONFIG_FB_DEVICE
 	dev_info(fb_info->dev,
 		 "%s frame buffer, %dx%d, %d KiB video memory%s, fps=%lu%s\n",
 		 fb_info->fix.id, fb_info->var.xres, fb_info->var.yres,
@@ -804,6 +814,12 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
 		fb_info->bl_dev->props.power = BACKLIGHT_POWER_ON;
 		fb_info->bl_dev->ops->update_status(fb_info->bl_dev);
 	}
+#else
+	pr_info("%s frame buffer, %dx%d, %d KiB video memory%s, fps=%lu%s\n",
+		fb_info->fix.id, fb_info->var.xres, fb_info->var.yres,
+		fb_info->fix.smem_len >> 10, text1,
+		HZ / fb_info->fbdefio->delay, text2);
+#endif
 
 	return 0;
 
diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
index e45c90a03a90..944f74f592d0 100644
--- a/drivers/staging/fbtft/fbtft-sysfs.c
+++ b/drivers/staging/fbtft/fbtft-sysfs.c
@@ -89,6 +89,7 @@ int fbtft_gamma_parse_str(struct fbtft_par *par, u32 *curves,
 	return ret;
 }
 
+#ifdef CONFIG_FB_DEVICE
 static ssize_t
 sprintf_gamma(struct fbtft_par *par, u32 *curves, char *buf)
 {
@@ -145,6 +146,7 @@ static ssize_t show_gamma_curve(struct device *device,
 static struct device_attribute gamma_device_attrs[] = {
 	__ATTR(gamma, 0660, show_gamma_curve, store_gamma_curve),
 };
+#endif
 
 void fbtft_expand_debug_value(unsigned long *debug)
 {
@@ -173,6 +175,7 @@ void fbtft_expand_debug_value(unsigned long *debug)
 	}
 }
 
+#ifdef CONFIG_FB_DEVICE
 static ssize_t store_debug(struct device *device,
 			   struct device_attribute *attr,
 			   const char *buf, size_t count)
@@ -200,17 +203,22 @@ static ssize_t show_debug(struct device *device,
 
 static struct device_attribute debug_device_attr =
 	__ATTR(debug, 0660, show_debug, store_debug);
+#endif
 
 void fbtft_sysfs_init(struct fbtft_par *par)
 {
+#ifdef CONFIG_FB_DEVICE
 	device_create_file(par->info->dev, &debug_device_attr);
 	if (par->gamma.curves && par->fbtftops.set_gamma)
 		device_create_file(par->info->dev, &gamma_device_attrs[0]);
+#endif
 }
 
 void fbtft_sysfs_exit(struct fbtft_par *par)
 {
+#ifdef CONFIG_FB_DEVICE
 	device_remove_file(par->info->dev, &debug_device_attr);
 	if (par->gamma.curves && par->fbtftops.set_gamma)
 		device_remove_file(par->info->dev, &gamma_device_attrs[0]);
+#endif
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] omapfb: Guard sysfs code under CONFIG_FB_DEVICE
  2025-12-09  4:27 [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE Chintan Patel
  2025-12-09  4:27 ` [PATCH 1/3] fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE Chintan Patel
@ 2025-12-09  4:27 ` Chintan Patel
  2025-12-09  4:27 ` [PATCH 3/3] sh_mobile_lcdc: Guard overlay sysfs interfaces " Chintan Patel
  2025-12-09  7:27 ` [PATCH 0/3] fbdev: Guard " Thomas Zimmermann
  3 siblings, 0 replies; 11+ messages in thread
From: Chintan Patel @ 2025-12-09  4:27 UTC (permalink / raw)
  To: linux-fbdev, linux-staging, linux-omap
  Cc: linux-kernel, dri-devel, tzimmermann, andy, deller, gregkh,
	Chintan Patel

omapfb implements multiple sysfs attributes for framebuffer rotation,
overlays, and debug information. These interfaces depend on FB_DEVICE
being enabled.

This patch wraps all sysfs attribute definitions, registration, and
removal in #ifdef CONFIG_FB_DEVICE. For FB_DEVICE=n, lightweight stub
functions are provided so that the driver builds and runs without
exposing sysfs interfaces.

Signed-off-by: Chintan Patel <chintanlike@gmail.com>
---
 drivers/video/fbdev/omap2/omapfb/Kconfig        |  2 +-
 drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/Kconfig b/drivers/video/fbdev/omap2/omapfb/Kconfig
index f4cdf999a080..ee664decbb64 100644
--- a/drivers/video/fbdev/omap2/omapfb/Kconfig
+++ b/drivers/video/fbdev/omap2/omapfb/Kconfig
@@ -5,7 +5,6 @@ config OMAP2_VRFB
 menuconfig FB_OMAP2
 	tristate "OMAP2+ frame buffer support"
 	depends on FB
-	depends on FB_DEVICE
 	depends on DRM_OMAP = n
 	depends on GPIOLIB
 	select FB_OMAP2_DSS
@@ -13,6 +12,7 @@ menuconfig FB_OMAP2
 	select FB_IOMEM_HELPERS
 	help
 	  Frame buffer driver for OMAP2+ based boards.
+	  Selecting FB_DEVICE enables additional sysfs interfaces.
 
 if FB_OMAP2
 
diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c b/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c
index 831b2c2fbdf9..0a340f69484f 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c
@@ -24,6 +24,7 @@
 
 #include "omapfb.h"
 
+#ifdef CONFIG_FB_DEVICE
 static ssize_t show_rotate_type(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -585,4 +586,14 @@ void omapfb_remove_sysfs(struct omapfb2_device *fbdev)
 					&omapfb_attrs[t]);
 	}
 }
+#else
+int omapfb_create_sysfs(struct omapfb2_device *fbdev)
+{
+	return 0;
+}
+
+void omapfb_remove_sysfs(struct omapfb2_device *fbdev)
+{
+}
+#endif
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] sh_mobile_lcdc: Guard overlay sysfs interfaces under CONFIG_FB_DEVICE
  2025-12-09  4:27 [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE Chintan Patel
  2025-12-09  4:27 ` [PATCH 1/3] fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE Chintan Patel
  2025-12-09  4:27 ` [PATCH 2/3] omapfb: Guard sysfs code under CONFIG_FB_DEVICE Chintan Patel
@ 2025-12-09  4:27 ` Chintan Patel
  2025-12-09  7:27 ` [PATCH 0/3] fbdev: Guard " Thomas Zimmermann
  3 siblings, 0 replies; 11+ messages in thread
From: Chintan Patel @ 2025-12-09  4:27 UTC (permalink / raw)
  To: linux-fbdev, linux-staging, linux-omap
  Cc: linux-kernel, dri-devel, tzimmermann, andy, deller, gregkh,
	Chintan Patel

The SH Mobile LCDC driver exposes overlay configuration via sysfs.
These attributes depend on FB_DEVICE and cause build failures when
FB_DEVICE=n.

Wrap all overlay sysfs attribute definitions and group registrations
within CONFIG_FB_DEVICE. When FB_DEVICE is disabled, the driver still
loads but without sysfs entries.

Signed-off-by: Chintan Patel <chintanlike@gmail.com>
---
 drivers/video/fbdev/sh_mobile_lcdcfb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index dd950e4ab5ce..a46da10789c3 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -1182,6 +1182,7 @@ static int __sh_mobile_lcdc_check_var(struct fb_var_screeninfo *var,
  * Frame buffer operations - Overlays
  */
 
+#ifdef CONFIG_FB_DEVICE
 static ssize_t
 overlay_alpha_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
@@ -1351,6 +1352,7 @@ static struct attribute *overlay_sysfs_attrs[] = {
 	NULL,
 };
 ATTRIBUTE_GROUPS(overlay_sysfs);
+#endif
 
 static const struct fb_fix_screeninfo sh_mobile_lcdc_overlay_fix  = {
 	.id =		"SH Mobile LCDC",
@@ -2637,7 +2639,9 @@ static int sh_mobile_lcdc_probe(struct platform_device *pdev)
 static struct platform_driver sh_mobile_lcdc_driver = {
 	.driver		= {
 		.name		= "sh_mobile_lcdc_fb",
+#ifdef CONFIG_FB_DEVICE
 		.dev_groups	= overlay_sysfs_groups,
+#endif
 		.pm		= &sh_mobile_lcdc_dev_pm_ops,
 	},
 	.probe		= sh_mobile_lcdc_probe,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE
  2025-12-09  4:27 ` [PATCH 1/3] fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE Chintan Patel
@ 2025-12-09  7:25   ` Thomas Zimmermann
  2025-12-10  4:24     ` Chintan Patel
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Zimmermann @ 2025-12-09  7:25 UTC (permalink / raw)
  To: Chintan Patel, linux-fbdev, linux-staging, linux-omap
  Cc: linux-kernel, dri-devel, andy, deller, gregkh

Hi

Am 09.12.25 um 05:27 schrieb Chintan Patel:
> The fbtft core and sysfs implementation unconditionally dereference
> fb_info->dev and register sysfs attributes. When FB_DEVICE=n, these
> fields are unavailable, leading to build failures.
>
> This patch wraps all sysfs attribute creation/removal and dev_dbg/dev_info
> logging in #ifdef CONFIG_FB_DEVICE, with pr_*() fallbacks for the
> non-FB_DEVICE case. This makes fbtft fully buildable when FB_DEVICE is
> disabled.
>
> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
> ---
>   drivers/staging/fbtft/fbtft-core.c  | 20 ++++++++++++++++++--
>   drivers/staging/fbtft/fbtft-sysfs.c |  8 ++++++++
>   2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> index 9e7b84071174..dc967bdeabe8 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -365,9 +365,14 @@ static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
>   	unsigned int val;
>   	int ret = 1;
>   
> +#ifdef CONFIG_FB_DEVICE
>   	dev_dbg(info->dev,

Rather use fb_dbg() [1] and similar helpers for logging. They only need 
the info pointer and do the correct output by themselves.

[1] https://elixir.bootlin.com/linux/v6.18/source/include/linux/fb.h#L895

>   		"%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
>   		__func__, regno, red, green, blue, transp);
> +#else
> +	pr_debug("%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
> +		 __func__, regno, red, green, blue, transp);
> +#endif
>   
>   	switch (info->fix.visual) {
>   	case FB_VISUAL_TRUECOLOR:
> @@ -391,8 +396,11 @@ static int fbtft_fb_blank(int blank, struct fb_info *info)
>   	struct fbtft_par *par = info->par;
>   	int ret = -EINVAL;
>   
> -	dev_dbg(info->dev, "%s(blank=%d)\n",
> -		__func__, blank);
> +#ifdef CONFIG_FB_DEVICE
> +	dev_dbg(info->dev, "%s(blank=%d)\n", __func__, blank);
> +#else
> +	pr_debug("%s(blank=%d)\n", __func__, blank);
> +#endif
>   
>   	if (!par->fbtftops.blank)
>   		return ret;
> @@ -793,6 +801,8 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
>   	if (spi)
>   		sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,
>   			spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
> +
> +#ifdef CONFIG_FB_DEVICE
>   	dev_info(fb_info->dev,

Same here with fb_info().

Best regards
Thomas

>   		 "%s frame buffer, %dx%d, %d KiB video memory%s, fps=%lu%s\n",
>   		 fb_info->fix.id, fb_info->var.xres, fb_info->var.yres,
> @@ -804,6 +814,12 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
>   		fb_info->bl_dev->props.power = BACKLIGHT_POWER_ON;
>   		fb_info->bl_dev->ops->update_status(fb_info->bl_dev);
>   	}
> +#else
> +	pr_info("%s frame buffer, %dx%d, %d KiB video memory%s, fps=%lu%s\n",
> +		fb_info->fix.id, fb_info->var.xres, fb_info->var.yres,
> +		fb_info->fix.smem_len >> 10, text1,
> +		HZ / fb_info->fbdefio->delay, text2);
> +#endif
>   
>   	return 0;
>   
> diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
> index e45c90a03a90..944f74f592d0 100644
> --- a/drivers/staging/fbtft/fbtft-sysfs.c
> +++ b/drivers/staging/fbtft/fbtft-sysfs.c
> @@ -89,6 +89,7 @@ int fbtft_gamma_parse_str(struct fbtft_par *par, u32 *curves,
>   	return ret;
>   }
>   
> +#ifdef CONFIG_FB_DEVICE
>   static ssize_t
>   sprintf_gamma(struct fbtft_par *par, u32 *curves, char *buf)
>   {
> @@ -145,6 +146,7 @@ static ssize_t show_gamma_curve(struct device *device,
>   static struct device_attribute gamma_device_attrs[] = {
>   	__ATTR(gamma, 0660, show_gamma_curve, store_gamma_curve),
>   };
> +#endif
>   
>   void fbtft_expand_debug_value(unsigned long *debug)
>   {
> @@ -173,6 +175,7 @@ void fbtft_expand_debug_value(unsigned long *debug)
>   	}
>   }
>   
> +#ifdef CONFIG_FB_DEVICE
>   static ssize_t store_debug(struct device *device,
>   			   struct device_attribute *attr,
>   			   const char *buf, size_t count)
> @@ -200,17 +203,22 @@ static ssize_t show_debug(struct device *device,
>   
>   static struct device_attribute debug_device_attr =
>   	__ATTR(debug, 0660, show_debug, store_debug);
> +#endif
>   
>   void fbtft_sysfs_init(struct fbtft_par *par)
>   {
> +#ifdef CONFIG_FB_DEVICE
>   	device_create_file(par->info->dev, &debug_device_attr);
>   	if (par->gamma.curves && par->fbtftops.set_gamma)
>   		device_create_file(par->info->dev, &gamma_device_attrs[0]);
> +#endif
>   }
>   
>   void fbtft_sysfs_exit(struct fbtft_par *par)
>   {
> +#ifdef CONFIG_FB_DEVICE
>   	device_remove_file(par->info->dev, &debug_device_attr);
>   	if (par->gamma.curves && par->fbtftops.set_gamma)
>   		device_remove_file(par->info->dev, &gamma_device_attrs[0]);
> +#endif
>   }

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE
  2025-12-09  4:27 [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE Chintan Patel
                   ` (2 preceding siblings ...)
  2025-12-09  4:27 ` [PATCH 3/3] sh_mobile_lcdc: Guard overlay sysfs interfaces " Chintan Patel
@ 2025-12-09  7:27 ` Thomas Zimmermann
  2025-12-09  8:22   ` Helge Deller
  3 siblings, 1 reply; 11+ messages in thread
From: Thomas Zimmermann @ 2025-12-09  7:27 UTC (permalink / raw)
  To: Chintan Patel, linux-fbdev, linux-staging, linux-omap
  Cc: linux-kernel, dri-devel, andy, deller, gregkh

Hi

Am 09.12.25 um 05:27 schrieb Chintan Patel:
> Hi all,
>
> This small series makes several legacy fbdev drivers buildable with
> CONFIG_FB_DEVICE=n. Currently, multiple fbdev drivers rely on fb_info->dev
> and sysfs attribute registration unconditionally, which leads to build
> failures whenever FB_DEVICE is disabled.
>
> Thomas previously noted that FB_DEVICE should eventually become optional
> and that drivers should not depend on sysfs or fb_info->dev being present
> unless the Kconfig explicitly selects it. This series pushes in that
> direction by tightening the FB_DEVICE dependency boundary without changing
> any runtime behaviour when FB_DEVICE=y.
>
> What this series does *not* change
>
> - No functional behaviour changes when FB_DEVICE=y.
> - No removal of sysfs interfaces.
> - No changes to fbops, memory allocation, or display update paths.
>
> Build & test coverage
>
> Tested with the following combinations:
>
> 1. **FB=y, FB_DEVICE=y**
>     - Baseline configuration; no regressions expected.
>
> 2. **FB=y, FB_DEVICE=n**
>     - Drivers build successfully.
>     - No sysfs attributes are created.
>     - fbdev devices operate normally (where applicable).
>
> 3. **FB=n**
>     - Drivers depend on FB, so they properly do not build, unchanged.
>
> Motivation
>
> This moves fbdev closer to supporting FB_DEVICE as truly optional, helps
> reduce Kconfig entanglement, and clears several long-standing TODO items
> as suggested by Thomas Zimmermann around legacy sysfs usage inside fbdev
> drivers.
>
> Feedback is welcome, especially on whether the guard boundaries around
> sysfs are placed correctly or whether more logic should be pulled under
> CONFIG_FB_DEVICE.

I left a comment on the first patch. If things still build nicely, then

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

for the series.

Best regards
Thomas

>
> Thanks,
> Chintan
>
> Chintan Patel (3):
>    fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE
>    omapfb: Guard sysfs code under CONFIG_FB_DEVICE
>    sh_mobile_lcdc: Guard overlay sysfs interfaces under CONFIG_FB_DEVICE
>
>   drivers/staging/fbtft/fbtft-core.c            | 20 +++++++++++++++++--
>   drivers/staging/fbtft/fbtft-sysfs.c           |  8 ++++++++
>   drivers/video/fbdev/omap2/omapfb/Kconfig      |  2 +-
>   .../video/fbdev/omap2/omapfb/omapfb-sysfs.c   | 11 ++++++++++
>   drivers/video/fbdev/sh_mobile_lcdcfb.c        |  4 ++++
>   5 files changed, 42 insertions(+), 3 deletions(-)
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE
  2025-12-09  7:27 ` [PATCH 0/3] fbdev: Guard " Thomas Zimmermann
@ 2025-12-09  8:22   ` Helge Deller
  2025-12-09  8:42     ` Thomas Zimmermann
  2025-12-09 14:25     ` Andy Shevchenko
  0 siblings, 2 replies; 11+ messages in thread
From: Helge Deller @ 2025-12-09  8:22 UTC (permalink / raw)
  To: Thomas Zimmermann, Chintan Patel, linux-fbdev, linux-staging,
	linux-omap
  Cc: linux-kernel, dri-devel, andy, gregkh

On 12/9/25 08:27, Thomas Zimmermann wrote:
> Hi
> 
> Am 09.12.25 um 05:27 schrieb Chintan Patel:
>> Hi all,
>>
>> This small series makes several legacy fbdev drivers buildable with
>> CONFIG_FB_DEVICE=n. Currently, multiple fbdev drivers rely on fb_info->dev
>> and sysfs attribute registration unconditionally, which leads to build
>> failures whenever FB_DEVICE is disabled.
>>
>> Thomas previously noted that FB_DEVICE should eventually become optional
>> and that drivers should not depend on sysfs or fb_info->dev being present
>> unless the Kconfig explicitly selects it. This series pushes in that
>> direction by tightening the FB_DEVICE dependency boundary without changing
>> any runtime behaviour when FB_DEVICE=y.
>>
>> What this series does *not* change
>>
>> - No functional behaviour changes when FB_DEVICE=y.
>> - No removal of sysfs interfaces.
>> - No changes to fbops, memory allocation, or display update paths.
>>
>> Build & test coverage
>>
>> Tested with the following combinations:
>>
>> 1. **FB=y, FB_DEVICE=y**
>>     - Baseline configuration; no regressions expected.
>>
>> 2. **FB=y, FB_DEVICE=n**
>>     - Drivers build successfully.
>>     - No sysfs attributes are created.
>>     - fbdev devices operate normally (where applicable).
>>
>> 3. **FB=n**
>>     - Drivers depend on FB, so they properly do not build, unchanged.
>>
>> Motivation
>>
>> This moves fbdev closer to supporting FB_DEVICE as truly optional, helps
>> reduce Kconfig entanglement, and clears several long-standing TODO items
>> as suggested by Thomas Zimmermann around legacy sysfs usage inside fbdev
>> drivers.
>>
>> Feedback is welcome, especially on whether the guard boundaries around
>> sysfs are placed correctly or whether more logic should be pulled under
>> CONFIG_FB_DEVICE.
> 
> I left a comment on the first patch. If things still build nicely, then
> 
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> for the series.

This whole series adds a whole lot of ifdef'ery, which I think is the
worst approach. It makes the code less readable and leads to two code
paths, which may trigger different build errors depending on the config.

I'm sure it must be possible to do the same without adding more #ifdefs,
e.g. by introducing a function like   dev_of_fbinfo(fbinfo)  which
simply returns NULL for the FB_DEVICE=n case.  Then, that value can be tested
like
	if (dev_of_fbinfo(fbinfo))
		{...do-the-things...}
For the FB_DEVICE=n case this will then be optimized out by the compiler,
while you still have full compiler syntax checking.

Thoughts?

Helge

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE
  2025-12-09  8:22   ` Helge Deller
@ 2025-12-09  8:42     ` Thomas Zimmermann
  2025-12-09 14:25     ` Andy Shevchenko
  1 sibling, 0 replies; 11+ messages in thread
From: Thomas Zimmermann @ 2025-12-09  8:42 UTC (permalink / raw)
  To: Helge Deller, Chintan Patel, linux-fbdev, linux-staging,
	linux-omap
  Cc: linux-kernel, dri-devel, andy, gregkh

Hi

Am 09.12.25 um 09:22 schrieb Helge Deller:
> On 12/9/25 08:27, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 09.12.25 um 05:27 schrieb Chintan Patel:
>>> Hi all,
>>>
>>> This small series makes several legacy fbdev drivers buildable with
>>> CONFIG_FB_DEVICE=n. Currently, multiple fbdev drivers rely on 
>>> fb_info->dev
>>> and sysfs attribute registration unconditionally, which leads to build
>>> failures whenever FB_DEVICE is disabled.
>>>
>>> Thomas previously noted that FB_DEVICE should eventually become 
>>> optional
>>> and that drivers should not depend on sysfs or fb_info->dev being 
>>> present
>>> unless the Kconfig explicitly selects it. This series pushes in that
>>> direction by tightening the FB_DEVICE dependency boundary without 
>>> changing
>>> any runtime behaviour when FB_DEVICE=y.
>>>
>>> What this series does *not* change
>>>
>>> - No functional behaviour changes when FB_DEVICE=y.
>>> - No removal of sysfs interfaces.
>>> - No changes to fbops, memory allocation, or display update paths.
>>>
>>> Build & test coverage
>>>
>>> Tested with the following combinations:
>>>
>>> 1. **FB=y, FB_DEVICE=y**
>>>     - Baseline configuration; no regressions expected.
>>>
>>> 2. **FB=y, FB_DEVICE=n**
>>>     - Drivers build successfully.
>>>     - No sysfs attributes are created.
>>>     - fbdev devices operate normally (where applicable).
>>>
>>> 3. **FB=n**
>>>     - Drivers depend on FB, so they properly do not build, unchanged.
>>>
>>> Motivation
>>>
>>> This moves fbdev closer to supporting FB_DEVICE as truly optional, 
>>> helps
>>> reduce Kconfig entanglement, and clears several long-standing TODO 
>>> items
>>> as suggested by Thomas Zimmermann around legacy sysfs usage inside 
>>> fbdev
>>> drivers.
>>>
>>> Feedback is welcome, especially on whether the guard boundaries around
>>> sysfs are placed correctly or whether more logic should be pulled under
>>> CONFIG_FB_DEVICE.
>>
>> I left a comment on the first patch. If things still build nicely, then
>>
>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>>
>> for the series.
>
> This whole series adds a whole lot of ifdef'ery, which I think is the
> worst approach. It makes the code less readable and leads to two code
> paths, which may trigger different build errors depending on the config.
>
> I'm sure it must be possible to do the same without adding more #ifdefs,
> e.g. by introducing a function like   dev_of_fbinfo(fbinfo)  which
> simply returns NULL for the FB_DEVICE=n case.  Then, that value can be 
> tested
> like
>     if (dev_of_fbinfo(fbinfo))
>         {...do-the-things...}
> For the FB_DEVICE=n case this will then be optimized out by the compiler,
> while you still have full compiler syntax checking.
>
> Thoughts?

Your choice. I don't see this as an important fix. The FB_DEVICE=n case 
is mostly useful for DRM-based systems that do not want to expose fbdev 
interfaces to user space. Those are the vast majority today. The very 
few special builds with fbdev drivers would likely use FB_DEVICE=y anyway.

Best regards
Thomas

>
> Helge
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE
  2025-12-09  8:22   ` Helge Deller
  2025-12-09  8:42     ` Thomas Zimmermann
@ 2025-12-09 14:25     ` Andy Shevchenko
  2025-12-10  4:26       ` Chintan Patel
  1 sibling, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2025-12-09 14:25 UTC (permalink / raw)
  To: Helge Deller
  Cc: Thomas Zimmermann, Chintan Patel, linux-fbdev, linux-staging,
	linux-omap, linux-kernel, dri-devel, andy, gregkh

On Tue, Dec 9, 2025 at 10:23 AM Helge Deller <deller@gmx.de> wrote:
> On 12/9/25 08:27, Thomas Zimmermann wrote:

...

> This whole series adds a whole lot of ifdef'ery, which I think is the
> worst approach. It makes the code less readable and leads to two code
> paths, which may trigger different build errors depending on the config.
>
> I'm sure it must be possible to do the same without adding more #ifdefs,
> e.g. by introducing a function like   dev_of_fbinfo(fbinfo)  which
> simply returns NULL for the FB_DEVICE=n case.  Then, that value can be tested
> like
>         if (dev_of_fbinfo(fbinfo))
>                 {...do-the-things...}
> For the FB_DEVICE=n case this will then be optimized out by the compiler,
> while you still have full compiler syntax checking.
>
> Thoughts?

I second you. I am also not a fan of ifdeffery when it can be avoided.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE
  2025-12-09  7:25   ` Thomas Zimmermann
@ 2025-12-10  4:24     ` Chintan Patel
  0 siblings, 0 replies; 11+ messages in thread
From: Chintan Patel @ 2025-12-10  4:24 UTC (permalink / raw)
  To: Thomas Zimmermann, linux-fbdev, linux-staging, linux-omap
  Cc: linux-kernel, dri-devel, andy, deller, gregkh

Hi Thomas,

On 12/8/25 23:25, Thomas Zimmermann wrote:
> Hi
> 
> Am 09.12.25 um 05:27 schrieb Chintan Patel:
>> The fbtft core and sysfs implementation unconditionally dereference
>> fb_info->dev and register sysfs attributes. When FB_DEVICE=n, these
>> fields are unavailable, leading to build failures.
>>
>> This patch wraps all sysfs attribute creation/removal and dev_dbg/ 
>> dev_info
>> logging in #ifdef CONFIG_FB_DEVICE, with pr_*() fallbacks for the
>> non-FB_DEVICE case. This makes fbtft fully buildable when FB_DEVICE is
>> disabled.
>>
>> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
>> ---
>>   drivers/staging/fbtft/fbtft-core.c  | 20 ++++++++++++++++++--
>>   drivers/staging/fbtft/fbtft-sysfs.c |  8 ++++++++
>>   2 files changed, 26 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/ 
>> fbtft/fbtft-core.c
>> index 9e7b84071174..dc967bdeabe8 100644
>> --- a/drivers/staging/fbtft/fbtft-core.c
>> +++ b/drivers/staging/fbtft/fbtft-core.c
>> @@ -365,9 +365,14 @@ static int fbtft_fb_setcolreg(unsigned int regno, 
>> unsigned int red,
>>       unsigned int val;
>>       int ret = 1;
>> +#ifdef CONFIG_FB_DEVICE
>>       dev_dbg(info->dev,
> 
> Rather use fb_dbg() [1] and similar helpers for logging. They only need 
> the info pointer and do the correct output by themselves.
> 
> [1] https://elixir.bootlin.com/linux/v6.18/source/include/linux/fb.h#L895

Thank you for this pointer - I actually didnt now this existed. Will do v2.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE
  2025-12-09 14:25     ` Andy Shevchenko
@ 2025-12-10  4:26       ` Chintan Patel
  0 siblings, 0 replies; 11+ messages in thread
From: Chintan Patel @ 2025-12-10  4:26 UTC (permalink / raw)
  To: Andy Shevchenko, Helge Deller
  Cc: Thomas Zimmermann, linux-fbdev, linux-staging, linux-omap,
	linux-kernel, dri-devel, andy, gregkh



On 12/9/25 06:25, Andy Shevchenko wrote:
> On Tue, Dec 9, 2025 at 10:23 AM Helge Deller <deller@gmx.de> wrote:
>> On 12/9/25 08:27, Thomas Zimmermann wrote:
> 
> ...
> 
>> This whole series adds a whole lot of ifdef'ery, which I think is the
>> worst approach. It makes the code less readable and leads to two code
>> paths, which may trigger different build errors depending on the config.
>>
>> I'm sure it must be possible to do the same without adding more #ifdefs,
>> e.g. by introducing a function like   dev_of_fbinfo(fbinfo)  which
>> simply returns NULL for the FB_DEVICE=n case.  Then, that value can be tested
>> like
>>          if (dev_of_fbinfo(fbinfo))
>>                  {...do-the-things...}
>> For the FB_DEVICE=n case this will then be optimized out by the compiler,
>> while you still have full compiler syntax checking.
>>
>> Thoughts?
> 
> I second you. I am also not a fan of ifdeffery when it can be avoided.
> 

Thank you for the review! Will do the change.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-12-10  4:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-09  4:27 [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE Chintan Patel
2025-12-09  4:27 ` [PATCH 1/3] fbtft: Make sysfs and dev_*() logging conditional on FB_DEVICE Chintan Patel
2025-12-09  7:25   ` Thomas Zimmermann
2025-12-10  4:24     ` Chintan Patel
2025-12-09  4:27 ` [PATCH 2/3] omapfb: Guard sysfs code under CONFIG_FB_DEVICE Chintan Patel
2025-12-09  4:27 ` [PATCH 3/3] sh_mobile_lcdc: Guard overlay sysfs interfaces " Chintan Patel
2025-12-09  7:27 ` [PATCH 0/3] fbdev: Guard " Thomas Zimmermann
2025-12-09  8:22   ` Helge Deller
2025-12-09  8:42     ` Thomas Zimmermann
2025-12-09 14:25     ` Andy Shevchenko
2025-12-10  4:26       ` Chintan Patel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).