Hi Thomas, On Mon, Jun 05, 2023 at 04:48:08PM +0200, Thomas Zimmermann wrote: > Move the logic to create and destroy fbdev devices into the new > helpers fb_device_create() and fb_device_destroy(). > > There was a call to fb_cleanup_device() in do_unregister_framebuffer() > that was too late. The device had already been removed at this point. > Move the call into fb_device_destroy(). > > Declare the helpers in the new internal header file fb_internal.h, as s/ / / > they are only used within the fbdev core module. > > Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de> > --- > drivers/video/fbdev/core/fb_internal.h | 12 ++++++++ > drivers/video/fbdev/core/fbmem.c | 21 +++----------- > drivers/video/fbdev/core/fbsysfs.c | 38 ++++++++++++++++++++++++-- > include/linux/fb.h | 3 -- > 4 files changed, 52 insertions(+), 22 deletions(-) > create mode 100644 drivers/video/fbdev/core/fb_internal.h > > diff --git a/drivers/video/fbdev/core/fb_internal.h > b/drivers/video/fbdev/core/fb_internal.h > new file mode 100644 > index 000000000000..0b9640ae7a3d > --- /dev/null > +++ b/drivers/video/fbdev/core/fb_internal.h > @@ -0,0 +1,12 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#ifndef _FB_INTERNAL_H > +#define _FB_INTERNAL_H > + > +struct fb_info; > + > +/* fbsysfs.c */ > +int fb_device_create(struct fb_info *fb_info); > +void fb_device_destroy(struct fb_info *fb_info); > + > +#endif > diff --git a/drivers/video/fbdev/core/fbmem.c > b/drivers/video/fbdev/core/fbmem.c > index f91ae7d4c94d..66532774d351 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -40,6 +40,8 @@ > #include <video/nomodeset.h> > #include <video/vga.h> > > +#include "fb_internal.h" > + > /* > * Frame buffer device initialization and setup routines > */ > @@ -1447,14 +1449,7 @@ static int do_register_framebuffer(struct fb_info > *fb_info) > mutex_init(&fb_info->lock); > mutex_init(&fb_info->mm_lock); > > - fb_info->dev = device_create(fb_class, fb_info->device, > - MKDEV(FB_MAJOR, i), NULL, "fb%d", i); > - if (IS_ERR(fb_info->dev)) { > - /* Not fatal */ > - printk(KERN_WARNING "Unable to create device for framebuffer > %d; errno = %ld\n", i, PTR_ERR(fb_info->dev)); > - fb_info->dev = NULL; > - } else > - fb_init_device(fb_info); > + fb_device_create(fb_info); The return result is ignored here. If we do not need it in later patches then drop the return result.
> > if (fb_info->pixmap.addr == NULL) { > fb_info->pixmap.addr = kmalloc(FBPIXMAPSIZE, GFP_KERNEL); > @@ -1515,16 +1510,9 @@ static void unlink_framebuffer(struct fb_info *fb_info) > if (WARN_ON(i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)) > return; > > - if (!fb_info->dev) > - return; > - > - device_destroy(fb_class, MKDEV(FB_MAJOR, i)); > - > + fb_device_destroy(fb_info); > pm_vt_switch_unregister(fb_info->device); > - > unbind_console(fb_info); > - > - fb_info->dev = NULL; > } > > static void do_unregister_framebuffer(struct fb_info *fb_info) > @@ -1539,7 +1527,6 @@ static void do_unregister_framebuffer(struct fb_info > *fb_info) > fb_destroy_modelist(&fb_info->modelist); > registered_fb[fb_info->node] = NULL; > num_registered_fb--; > - fb_cleanup_device(fb_info); > #ifdef CONFIG_GUMSTIX_AM200EPD > { > struct fb_event event; > diff --git a/drivers/video/fbdev/core/fbsysfs.c > b/drivers/video/fbdev/core/fbsysfs.c > index 849073f1ca06..fafe574398b0 100644 > --- a/drivers/video/fbdev/core/fbsysfs.c > +++ b/drivers/video/fbdev/core/fbsysfs.c > @@ -8,6 +8,9 @@ > #include <linux/console.h> > #include <linux/fb.h> > #include <linux/fbcon.h> > +#include <linux/major.h> > + > +#include "fb_internal.h" > > #define FB_SYSFS_FLAG_ATTR 1 > > @@ -435,7 +438,7 @@ static struct device_attribute device_attrs[] = { > #endif > }; > > -int fb_init_device(struct fb_info *fb_info) > +static int fb_init_device(struct fb_info *fb_info) > { > int i, error = 0; > > @@ -459,7 +462,7 @@ int fb_init_device(struct fb_info *fb_info) > return 0; > } > > -void fb_cleanup_device(struct fb_info *fb_info) > +static void fb_cleanup_device(struct fb_info *fb_info) > { > unsigned int i; > > @@ -470,3 +473,34 @@ void fb_cleanup_device(struct fb_info *fb_info) > fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR; > } > } > + > +int fb_device_create(struct fb_info *fb_info) > +{ > + int node = fb_info->node; > + dev_t devt = MKDEV(FB_MAJOR, node); > + int ret; > + > + fb_info->dev = device_create(fb_class, fb_info->device, devt, NULL, > "fb%d", node); > + if (IS_ERR(fb_info->dev)) { > + /* Not fatal */ > + ret = PTR_ERR(fb_info->dev); This error code is lost as we always return 0. > + pr_warn("Unable to create device for framebuffer %d; error > %d\n", node, ret); > + fb_info->dev = NULL; > + } else { > + fb_init_device(fb_info); > + } > + > + return 0; > +} > + > +void fb_device_destroy(struct fb_info *fb_info) > +{ > + dev_t devt = MKDEV(FB_MAJOR, fb_info->node); > + > + if (!fb_info->dev) > + return; > + > + fb_cleanup_device(fb_info); > + device_destroy(fb_class, devt); > + fb_info->dev = NULL; > +} > diff --git a/include/linux/fb.h b/include/linux/fb.h > index ce6823e157e6..1988d11f78bc 100644 > --- a/include/linux/fb.h > +++ b/include/linux/fb.h > @@ -735,11 +735,8 @@ static inline bool fb_be_math(struct fb_info *info) > #endif /* CONFIG_FB_FOREIGN_ENDIAN */ > } > > -/* drivers/video/fbsysfs.c */ > extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev); > extern void framebuffer_release(struct fb_info *info); > -extern int fb_init_device(struct fb_info *fb_info); > -extern void fb_cleanup_device(struct fb_info *head); > extern void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 > max); > > /* drivers/video/fbmon.c */ > -- > 2.40.1