* [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional @ 2025-10-05 17:38 Javier Garcia 2025-10-05 19:51 ` Helge Deller 2025-10-06 8:07 ` Uwe Kleine-König 0 siblings, 2 replies; 4+ messages in thread From: Javier Garcia @ 2025-10-05 17:38 UTC (permalink / raw) To: deller, u.kleine-koenig, tzimmermann Cc: linux-fbdev, dri-devel, linux-kernel, shuah, Javier Garcia 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional 2025-10-05 17:38 [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional Javier Garcia @ 2025-10-05 19:51 ` Helge Deller 2025-10-06 8:07 ` Uwe Kleine-König 1 sibling, 0 replies; 4+ messages in thread From: Helge Deller @ 2025-10-05 19:51 UTC (permalink / raw) To: Javier Garcia, u.kleine-koenig, tzimmermann Cc: linux-fbdev, dri-devel, linux-kernel, shuah 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional 2025-10-05 17:38 [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional Javier Garcia 2025-10-05 19:51 ` Helge Deller @ 2025-10-06 8:07 ` Uwe Kleine-König 2025-10-06 16:10 ` Javier Garcia 1 sibling, 1 reply; 4+ messages in thread From: Uwe Kleine-König @ 2025-10-06 8:07 UTC (permalink / raw) To: Javier Garcia Cc: deller, tzimmermann, linux-fbdev, dri-devel, linux-kernel, shuah 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional 2025-10-06 8:07 ` Uwe Kleine-König @ 2025-10-06 16:10 ` Javier Garcia 0 siblings, 0 replies; 4+ messages in thread From: Javier Garcia @ 2025-10-06 16:10 UTC (permalink / raw) To: Uwe Kleine-König Cc: deller, tzimmermann, linux-fbdev, dri-devel, linux-kernel, shuah 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-06 16:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-05 17:38 [PATCH] fbdev: mb862xxfbdrv: Make CONFIG_FB_DEVICE optional Javier Garcia 2025-10-05 19:51 ` Helge Deller 2025-10-06 8:07 ` Uwe Kleine-König 2025-10-06 16:10 ` Javier Garcia
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).