1. Patchew
  2. linux
  3. View series

[PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional

Javier Garcia posted 1 patch 2 months, 1 week ago
There is a newer version of this series
drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 10 ++++++++++
1 file changed, 10 insertions(+)
[PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
Posted by Javier Garcia 2 months, 1 week ago 
This patch wraps the relevant code blocks with #ifdef CONFIG_FB_DEVICE,
allowing the driver to be built and used even if CONFIG_FB_DEVICE is not selected.

The sysfs only give access to show some controller and cursor registers so
it's not needed to allow driver works correctly.

Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
---
 drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index ade88e7bc760..a691a5fefb25 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -530,6 +530,7 @@ static int mb862xxfb_init_fbinfo(struct fb_info *fbi)
 	return 0;
 }
 
+#ifdef CONFIG_FB_DEVICE
 /*
  * show some display controller and cursor registers
  */
@@ -569,6 +570,7 @@ static ssize_t dispregs_show(struct device *dev,
 }
 
 static DEVICE_ATTR_RO(dispregs);
+#endif
 
 static irqreturn_t mb862xx_intr(int irq, void *dev_id)
 {
@@ -759,9 +761,11 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
 
 	dev_set_drvdata(dev, info);
 
+#ifdef CONFIG_FB_DEVICE
 	if (device_create_file(dev, &dev_attr_dispregs))
 		dev_err(dev, "Can't create sysfs regdump file\n");
 	return 0;
+#endif
 
 rel_cmap:
 	fb_dealloc_cmap(&info->cmap);
@@ -801,7 +805,9 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
 	free_irq(par->irq, (void *)par);
 	irq_dispose_mapping(par->irq);
 
+#ifdef CONFIG_FB_DEVICE
 	device_remove_file(&ofdev->dev, &dev_attr_dispregs);
+#endif
 
 	unregister_framebuffer(fbi);
 	fb_dealloc_cmap(&fbi->cmap);
@@ -1101,8 +1107,10 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
 
 	pci_set_drvdata(pdev, info);
 
+#ifdef CONFIG_FB_DEVICE
 	if (device_create_file(dev, &dev_attr_dispregs))
 		dev_err(dev, "Can't create sysfs regdump file\n");
+#endif
 
 	if (par->type == BT_CARMINE)
 		outreg(ctrl, GC_CTRL_INT_MASK, GC_CARMINE_INT_EN);
@@ -1151,7 +1159,9 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
 
 	mb862xx_i2c_exit(par);
 
+#ifdef CONFIG_FB_DEVICE
 	device_remove_file(&pdev->dev, &dev_attr_dispregs);
+#endif
 
 	unregister_framebuffer(fbi);
 	fb_dealloc_cmap(&fbi->cmap);
-- 
2.50.1
Re: [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
Posted by Uwe Kleine-König 2 months, 1 week ago 
Hello,

On Sun, Oct 05, 2025 at 07:38:12PM +0200, Javier Garcia wrote:
> This patch wraps the relevant code blocks with #ifdef CONFIG_FB_DEVICE,
> allowing the driver to be built and used even if CONFIG_FB_DEVICE is not selected.
> 
> The sysfs only give access to show some controller and cursor registers so
> it's not needed to allow driver works correctly.
> 
> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>

I don't understand this patch. CONFIG_FB_DEVICE is about legacy /dev/fb*
stuff, and this patch make the creation of a sysfs file dependent on
that.

> diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> index ade88e7bc760..a691a5fefb25 100644
> --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> @@ -530,6 +530,7 @@ static int mb862xxfb_init_fbinfo(struct fb_info *fbi)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_FB_DEVICE
>  /*
>   * show some display controller and cursor registers
>   */
> @@ -569,6 +570,7 @@ static ssize_t dispregs_show(struct device *dev,
>  }
>  
>  static DEVICE_ATTR_RO(dispregs);
> +#endif
>  
>  static irqreturn_t mb862xx_intr(int irq, void *dev_id)
>  {
> @@ -759,9 +761,11 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
>  
>  	dev_set_drvdata(dev, info);
>  
> +#ifdef CONFIG_FB_DEVICE
>  	if (device_create_file(dev, &dev_attr_dispregs))
>  		dev_err(dev, "Can't create sysfs regdump file\n");
>  	return 0;
> +#endif

That looks wrong. Without CONFIG_FB_DEVICE set the success return is
removed and all resources are freed essentially making the
CONFIG_FB_MB862XX_LIME part of the driver unusable.

>  rel_cmap:
>  	fb_dealloc_cmap(&info->cmap);
> @@ -801,7 +805,9 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
>  	free_irq(par->irq, (void *)par);
>  	irq_dispose_mapping(par->irq);
>  
> +#ifdef CONFIG_FB_DEVICE
>  	device_remove_file(&ofdev->dev, &dev_attr_dispregs);
> +#endif
>  
>  	unregister_framebuffer(fbi);
>  	fb_dealloc_cmap(&fbi->cmap);
> @@ -1101,8 +1107,10 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
>  
>  	pci_set_drvdata(pdev, info);
>  
> +#ifdef CONFIG_FB_DEVICE
>  	if (device_create_file(dev, &dev_attr_dispregs))
>  		dev_err(dev, "Can't create sysfs regdump file\n");
> +#endif
>  
>  	if (par->type == BT_CARMINE)
>  		outreg(ctrl, GC_CTRL_INT_MASK, GC_CARMINE_INT_EN);
> @@ -1151,7 +1159,9 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
>  
>  	mb862xx_i2c_exit(par);
>  
> +#ifdef CONFIG_FB_DEVICE
>  	device_remove_file(&pdev->dev, &dev_attr_dispregs);
> +#endif
>  
>  	unregister_framebuffer(fbi);
>  	fb_dealloc_cmap(&fbi->cmap);

The amount of ifdefs could be greatly reduced using the following patch:

diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index a691a5fefb25..3f79dfc27a53 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -530,7 +530,6 @@ static int mb862xxfb_init_fbinfo(struct fb_info *fbi)
 	return 0;
 }
 
-#ifdef CONFIG_FB_DEVICE
 /*
  * show some display controller and cursor registers
  */
@@ -570,7 +569,6 @@ static ssize_t dispregs_show(struct device *dev,
 }
 
 static DEVICE_ATTR_RO(dispregs);
-#endif
 
 static irqreturn_t mb862xx_intr(int irq, void *dev_id)
 {
@@ -761,11 +759,9 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
 
 	dev_set_drvdata(dev, info);
 
-#ifdef CONFIG_FB_DEVICE
-	if (device_create_file(dev, &dev_attr_dispregs))
+	if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
 		dev_err(dev, "Can't create sysfs regdump file\n");
 	return 0;
-#endif
 
 rel_cmap:
 	fb_dealloc_cmap(&info->cmap);
@@ -805,9 +801,8 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
 	free_irq(par->irq, (void *)par);
 	irq_dispose_mapping(par->irq);
 
-#ifdef CONFIG_FB_DEVICE
-	device_remove_file(&ofdev->dev, &dev_attr_dispregs);
-#endif
+	if (IS_ENABLED(CONFIG_FB_DEVICE))
+		device_remove_file(&ofdev->dev, &dev_attr_dispregs);
 
 	unregister_framebuffer(fbi);
 	fb_dealloc_cmap(&fbi->cmap);
@@ -1107,10 +1102,8 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
 
 	pci_set_drvdata(pdev, info);
 
-#ifdef CONFIG_FB_DEVICE
-	if (device_create_file(dev, &dev_attr_dispregs))
+	if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
 		dev_err(dev, "Can't create sysfs regdump file\n");
-#endif
 
 	if (par->type == BT_CARMINE)
 		outreg(ctrl, GC_CTRL_INT_MASK, GC_CARMINE_INT_EN);
@@ -1159,9 +1152,8 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
 
 	mb862xx_i2c_exit(par);
 
-#ifdef CONFIG_FB_DEVICE
-	device_remove_file(&pdev->dev, &dev_attr_dispregs);
-#endif
+	if (IS_ENABLED(CONFIG_FB_DEVICE))
+		device_remove_file(&pdev->dev, &dev_attr_dispregs);
 
 	unregister_framebuffer(fbi);
 	fb_dealloc_cmap(&fbi->cmap);

(It would still be questionable however to make the device file creation
dependent on FB_DEVICE.)

Best regards
Uwe
Re: [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
Posted by Javier Garcia 2 months, 1 week ago 
Hello.

On Mon, 6 Oct 2025 at 10:07, Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello,
>
> On Sun, Oct 05, 2025 at 07:38:12PM +0200, Javier Garcia wrote:
> > This patch wraps the relevant code blocks with #ifdef CONFIG_FB_DEVICE,
> > allowing the driver to be built and used even if CONFIG_FB_DEVICE is not selected.
> >
> > The sysfs only give access to show some controller and cursor registers so
> > it's not needed to allow driver works correctly.
> >
> > Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
>
> I don't understand this patch. CONFIG_FB_DEVICE is about legacy /dev/fb*
> stuff, and this patch make the creation of a sysfs file dependent on
> that.

This is part of the TODO
https://www.kernel.org/doc/html/latest/gpu/todo.html#remove-driver-dependencies-on-fb-device

I think the idea is to make the driver independent of CONFIG_FB_DEVICE .

>
> > diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> > index ade88e7bc760..a691a5fefb25 100644
> > --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> > +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> > @@ -530,6 +530,7 @@ static int mb862xxfb_init_fbinfo(struct fb_info *fbi)
> >       return 0;
> >  }
> >
> > +#ifdef CONFIG_FB_DEVICE
> >  /*
> >   * show some display controller and cursor registers
> >   */
> > @@ -569,6 +570,7 @@ static ssize_t dispregs_show(struct device *dev,
> >  }
> >
> >  static DEVICE_ATTR_RO(dispregs);
> > +#endif
> >
> >  static irqreturn_t mb862xx_intr(int irq, void *dev_id)
> >  {
> > @@ -759,9 +761,11 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
> >
> >       dev_set_drvdata(dev, info);
> >
> > +#ifdef CONFIG_FB_DEVICE
> >       if (device_create_file(dev, &dev_attr_dispregs))
> >               dev_err(dev, "Can't create sysfs regdump file\n");
> >       return 0;
> > +#endif
>
> That looks wrong. Without CONFIG_FB_DEVICE set the success return is
> removed and all resources are freed essentially making the
> CONFIG_FB_MB862XX_LIME part of the driver unusable.

Thanks, I'll fix that.

And add your suggestion instead of the `ifdef`.

>
> >  rel_cmap:
> >       fb_dealloc_cmap(&info->cmap);
> > @@ -801,7 +805,9 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
> >       free_irq(par->irq, (void *)par);
> >       irq_dispose_mapping(par->irq);
> >
> > +#ifdef CONFIG_FB_DEVICE
> >       device_remove_file(&ofdev->dev, &dev_attr_dispregs);
> > +#endif
> >
> >       unregister_framebuffer(fbi);
> >       fb_dealloc_cmap(&fbi->cmap);
> > @@ -1101,8 +1107,10 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
> >
> >       pci_set_drvdata(pdev, info);
> >
> > +#ifdef CONFIG_FB_DEVICE
> >       if (device_create_file(dev, &dev_attr_dispregs))
> >               dev_err(dev, "Can't create sysfs regdump file\n");
> > +#endif
> >
> >       if (par->type == BT_CARMINE)
> >               outreg(ctrl, GC_CTRL_INT_MASK, GC_CARMINE_INT_EN);
> > @@ -1151,7 +1159,9 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
> >
> >       mb862xx_i2c_exit(par);
> >
> > +#ifdef CONFIG_FB_DEVICE
> >       device_remove_file(&pdev->dev, &dev_attr_dispregs);
> > +#endif
> >
> >       unregister_framebuffer(fbi);
> >       fb_dealloc_cmap(&fbi->cmap);
>
> The amount of ifdefs could be greatly reduced using the following patch:
>
> diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> index a691a5fefb25..3f79dfc27a53 100644
> --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> @@ -530,7 +530,6 @@ static int mb862xxfb_init_fbinfo(struct fb_info *fbi)
>         return 0;
>  }
>
> -#ifdef CONFIG_FB_DEVICE
>  /*
>   * show some display controller and cursor registers
>   */
> @@ -570,7 +569,6 @@ static ssize_t dispregs_show(struct device *dev,
>  }
>
>  static DEVICE_ATTR_RO(dispregs);
> -#endif
>
>  static irqreturn_t mb862xx_intr(int irq, void *dev_id)
>  {
> @@ -761,11 +759,9 @@ static int of_platform_mb862xx_probe(struct platform_device *ofdev)
>
>         dev_set_drvdata(dev, info);
>
> -#ifdef CONFIG_FB_DEVICE
> -       if (device_create_file(dev, &dev_attr_dispregs))
> +       if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
>                 dev_err(dev, "Can't create sysfs regdump file\n");
>         return 0;
> -#endif
>
>  rel_cmap:
>         fb_dealloc_cmap(&info->cmap);
> @@ -805,9 +801,8 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
>         free_irq(par->irq, (void *)par);
>         irq_dispose_mapping(par->irq);
>
> -#ifdef CONFIG_FB_DEVICE
> -       device_remove_file(&ofdev->dev, &dev_attr_dispregs);
> -#endif
> +       if (IS_ENABLED(CONFIG_FB_DEVICE))
> +               device_remove_file(&ofdev->dev, &dev_attr_dispregs);
>
>         unregister_framebuffer(fbi);
>         fb_dealloc_cmap(&fbi->cmap);
> @@ -1107,10 +1102,8 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
>
>         pci_set_drvdata(pdev, info);
>
> -#ifdef CONFIG_FB_DEVICE
> -       if (device_create_file(dev, &dev_attr_dispregs))
> +       if (IS_ENABLED(CONFIG_FB_DEVICE) && device_create_file(dev, &dev_attr_dispregs))
>                 dev_err(dev, "Can't create sysfs regdump file\n");
> -#endif
>
>         if (par->type == BT_CARMINE)
>                 outreg(ctrl, GC_CTRL_INT_MASK, GC_CARMINE_INT_EN);
> @@ -1159,9 +1152,8 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
>
>         mb862xx_i2c_exit(par);
>
> -#ifdef CONFIG_FB_DEVICE
> -       device_remove_file(&pdev->dev, &dev_attr_dispregs);
> -#endif
> +       if (IS_ENABLED(CONFIG_FB_DEVICE))
> +               device_remove_file(&pdev->dev, &dev_attr_dispregs);
>
>         unregister_framebuffer(fbi);
>         fb_dealloc_cmap(&fbi->cmap);
>
> (It would still be questionable however to make the device file creation
> dependent on FB_DEVICE.)

https://www.kernel.org/doc/html/latest/gpu/todo.html#remove-driver-dependencies-on-fb-device

>
> Best regards
> Uwe
Re: [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional
Posted by Helge Deller 2 months, 1 week ago 
On 10/5/25 19:38, Javier Garcia wrote:
> This patch wraps the relevant code blocks with #ifdef CONFIG_FB_DEVICE,
> allowing the driver to be built and used even if CONFIG_FB_DEVICE is not selected.
> 
> The sysfs only give access to show some controller and cursor registers so
> it's not needed to allow driver works correctly.
> 
> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
> ---
>   drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)

applied.

Thanks!
Helge

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.