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

[Sam Ravnborg <sam@ravnborg.org>]

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