Re: [PATCH 29/30] fbdev/core: Rework fb init code

Sam Ravnborg
2023-06-07T20:51:12Z

Hi Thomas.

On Mon, Jun 05, 2023 at 04:48:11PM +0200, Thomas Zimmermann wrote:
> Init the class "graphics" before the rest of fbdev. Later steps, such
> as the sysfs code, depend on the class. Also arrange the module's exit
> code in reverse order.
>
> Unexport the global variable fb_class, which is only shared internally
> within the fbdev core module.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> drivers/video/fbdev/core/fb_internal.h | 1 +
> drivers/video/fbdev/core/fbcon.c | 1 +
> drivers/video/fbdev/core/fbmem.c | 52 ++++++++++----------------
> include/linux/fb.h | 1 -
> 4 files changed, 22 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fb_internal.h b/drivers/video/fbdev/core/fb_internal.h
> index abe06c9da36e..0b43c0cd5096 100644
> --- a/drivers/video/fbdev/core/fb_internal.h
> +++ b/drivers/video/fbdev/core/fb_internal.h
> @@ -11,6 +11,7 @@ int fb_register_chrdev(void);
> void fb_unregister_chrdev(void);
>
> /* fbmem.c */
> +extern struct class *fb_class;
> extern struct mutex registration_lock;
> extern struct fb_info *registered_fb[FB_MAX];
> extern int num_registered_fb;
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index c6c9d040bdec..8e76bc246b38 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -78,6 +78,7 @@
> #include <asm/irq.h>
>
> #include "fbcon.h"
> +#include "fb_internal.h"
>
> /*
> * FIXME: Locking
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 2d26ac46337b..b21b66017c01 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -45,6 +45,8 @@
>
> #define FBPIXMAPSIZE (1024 * 8)
>
> +struct class *fb_class;
> +
> DEFINE_MUTEX(registration_lock);
> struct fb_info *registered_fb[FB_MAX] __read_mostly;
> int num_registered_fb __read_mostly;
> @@ -897,9 +899,6 @@ fb_blank(struct fb_info *info, int blank)
> }
> EXPORT_SYMBOL(fb_blank);
>
> -struct class *fb_class;
> -EXPORT_SYMBOL(fb_class);
> -
> static int fb_check_foreignness(struct fb_info *fi)
> {
> const bool foreign_endian = fi->flags & FBINFO_FOREIGN_ENDIAN;
> @@ -1106,59 +1105,48 @@ void fb_set_suspend(struct fb_info *info, int state)
> }
> EXPORT_SYMBOL(fb_set_suspend);
>
> -/**
> - * fbmem_init - init frame buffer subsystem
> - *
> - * Initialize the frame buffer subsystem.
> - *
> - * NOTE: This function is _only_ to be called by drivers/char/mem.c.
> - *
> - */
> -
> -static int __init
> -fbmem_init(void)
> +static int __init fbmem_init(void)
> {
> int ret;
>
> + fb_class = class_create("graphics");
> + if (IS_ERR(fb_class)) {
> + ret = PTR_ERR(fb_class);
> + pr_err("Unable to create fb class; errno = %d\n", ret);
> + goto err_fb_class;
> + }
> +
> ret = fb_init_procfs();
> if (ret)
> - return ret;
> + goto err_class_destroy;
>
> ret = fb_register_chrdev();
> if (ret)
> - goto err_chrdev;
> -
> - fb_class = class_create("graphics");
> - if (IS_ERR(fb_class)) {
> - ret = PTR_ERR(fb_class);
> - pr_warn("Unable to create fb class; errno = %d\n", ret);
> - fb_class = NULL;
> - goto err_class;
> - }
> + goto err_fb_cleanup_procfs;
>
> fb_console_init();
>
> return 0;
>
> -err_class:
> - fb_unregister_chrdev();
> -err_chrdev:
> +err_fb_cleanup_procfs:
> fb_cleanup_procfs();
> +err_class_destroy:
> + class_destroy(fb_class);
> +err_fb_class:
> + fb_class = NULL;
> return ret;
> }
>
> #ifdef MODULE
> -module_init(fbmem_init);
> -static void __exit
> -fbmem_exit(void)
> +static void __exit fbmem_exit(void)
> {
> fb_console_exit();
> -
> + fb_unregister_chrdev();
> fb_cleanup_procfs();
> class_destroy(fb_class);
> - fb_unregister_chrdev();
> }
>
> +module_init(fbmem_init);
> module_exit(fbmem_exit);
> MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("Framebuffer base");
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 1988d11f78bc..541a0e3ce21f 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -609,7 +609,6 @@ extern int fb_new_modelist(struct fb_info *info);
>
> extern bool fb_center_logo;
> extern int fb_logo_count;
> -extern struct class *fb_class;
>
> static inline void lock_fb_info(struct fb_info *info)
> {
> --
> 2.40.1