Re: [PATCH 2/6] fbcon: Add sysfs attributes before registering the device

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Am 23.09.24 um 17:57 schrieb Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Currently fbcon adds the attributes after registering the device,
> which means the attributes may not be there by the time udev
> gets the ADD uevent. Fix the race by switching over to
> device_create_with_groups().
>
> With this one can reliably turn off the power wasting cursor
> blink with a udev rule such as:
> ACTION=="add", SUBSYSTEM=="graphics", TEST=="cursor_blink", ATTR{cursor_blink}="0"
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx>

> ---
>   drivers/video/fbdev/core/fbcon.c | 73 +++++++++-----------------------
>   1 file changed, 19 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 8936fa79b9e0..bbe332572ca7 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -160,7 +160,6 @@ static int info_idx = -1;
>   
>   /* console rotation */
>   static int initial_rotation = -1;
> -static int fbcon_has_sysfs;
>   static int margin_color;
>   
>   static const struct consw fb_con;
> @@ -188,8 +187,6 @@ static void fbcon_redraw_move(struct vc_data *vc, struct fbcon_display *p,
>   static void fbcon_modechanged(struct fb_info *info);
>   static void fbcon_set_all_vcs(struct fb_info *info);
>   
> -static struct device *fbcon_device;
> -
>   #ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
>   static inline void fbcon_set_rotation(struct fb_info *info)
>   {
> @@ -3151,7 +3148,7 @@ static const struct consw fb_con = {
>   	.con_debug_leave	= fbcon_debug_leave,
>   };
>   
> -static ssize_t store_rotate(struct device *device,
> +static ssize_t rotate_store(struct device *device,
>   			    struct device_attribute *attr, const char *buf,
>   			    size_t count)
>   {
> @@ -3173,7 +3170,7 @@ static ssize_t store_rotate(struct device *device,
>   	return count;
>   }
>   
> -static ssize_t store_rotate_all(struct device *device,
> +static ssize_t rotate_all_store(struct device *device,
>   				struct device_attribute *attr,const char *buf,
>   				size_t count)
>   {
> @@ -3195,7 +3192,7 @@ static ssize_t store_rotate_all(struct device *device,
>   	return count;
>   }
>   
> -static ssize_t show_rotate(struct device *device,
> +static ssize_t rotate_show(struct device *device,
>   			   struct device_attribute *attr,char *buf)
>   {
>   	struct fb_info *info;
> @@ -3214,13 +3211,13 @@ static ssize_t show_rotate(struct device *device,
>   	return sysfs_emit(buf, "%d\n", rotate);
>   }
>   
> -static ssize_t show_cursor_blink(struct device *device,
> +static ssize_t cursor_blink_show(struct device *device,
>   				 struct device_attribute *attr, char *buf)
>   {
>   	return sysfs_emit(buf, "%d\n", !fbcon_cursor_noblink);
>   }
>   
> -static ssize_t store_cursor_blink(struct device *device,
> +static ssize_t cursor_blink_store(struct device *device,
>   				  struct device_attribute *attr,
>   				  const char *buf, size_t count)
>   {
> @@ -3253,35 +3250,17 @@ static ssize_t store_cursor_blink(struct device *device,
>   	return count;
>   }
>   
> -static struct device_attribute device_attrs[] = {
> -	__ATTR(rotate, S_IRUGO|S_IWUSR, show_rotate, store_rotate),
> -	__ATTR(rotate_all, S_IWUSR, NULL, store_rotate_all),
> -	__ATTR(cursor_blink, S_IRUGO|S_IWUSR, show_cursor_blink,
> -	       store_cursor_blink),
> -};
> -
> -static int fbcon_init_device(void)
> -{
> -	int i, error = 0;
> -
> -	fbcon_has_sysfs = 1;
> -
> -	for (i = 0; i < ARRAY_SIZE(device_attrs); i++) {
> -		error = device_create_file(fbcon_device, &device_attrs[i]);
> -
> -		if (error)
> -			break;
> -	}
> -
> -	if (error) {
> -		while (--i >= 0)
> -			device_remove_file(fbcon_device, &device_attrs[i]);
> -
> -		fbcon_has_sysfs = 0;
> -	}
> +static DEVICE_ATTR_RW(rotate);
> +static DEVICE_ATTR_WO(rotate_all);
> +static DEVICE_ATTR_RW(cursor_blink);
>   
> -	return 0;
> -}
> +static struct attribute *device_attrs[] = {
> +	&dev_attr_rotate.attr,
> +	&dev_attr_rotate_all.attr,
> +	&dev_attr_cursor_blink.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(device);
>   
>   #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>   static void fbcon_register_existing_fbs(struct work_struct *work)
> @@ -3336,19 +3315,18 @@ static void fbcon_start(void)
>   
>   void __init fb_console_init(void)
>   {
> +	struct device *fbcon_device;
>   	int i;
>   
>   	console_lock();
> -	fbcon_device = device_create(fb_class, NULL, MKDEV(0, 0), NULL,
> -				     "fbcon");
>   
> +	fbcon_device = device_create_with_groups(fb_class, NULL, MKDEV(0, 0),
> +						 NULL, device_groups, "fbcon");
>   	if (IS_ERR(fbcon_device)) {
>   		printk(KERN_WARNING "Unable to create device "
>   		       "for fbcon; errno = %ld\n",
>   		       PTR_ERR(fbcon_device));
> -		fbcon_device = NULL;
> -	} else
> -		fbcon_init_device();
> +	}
>   
>   	for (i = 0; i < MAX_NR_CONSOLES; i++)
>   		con2fb_map[i] = -1;
> @@ -3359,18 +3337,6 @@ void __init fb_console_init(void)
>   
>   #ifdef MODULE
>   
> -static void __exit fbcon_deinit_device(void)
> -{
> -	int i;
> -
> -	if (fbcon_has_sysfs) {
> -		for (i = 0; i < ARRAY_SIZE(device_attrs); i++)
> -			device_remove_file(fbcon_device, &device_attrs[i]);
> -
> -		fbcon_has_sysfs = 0;
> -	}
> -}
> -
>   void __exit fb_console_exit(void)
>   {
>   #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> @@ -3383,7 +3349,6 @@ void __exit fb_console_exit(void)
>   #endif
>   
>   	console_lock();
> -	fbcon_deinit_device();
>   	device_destroy(fb_class, MKDEV(0, 0));
>   
>   	do_unregister_con_driver(&fb_con);

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)