diff mbox series

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

Message ID 20240923155749.30846-3-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series fbcon: Fix 'cursor_blink' sysfs attribute | expand

Commit Message

Ville Syrjälä Sept. 23, 2024, 3:57 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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@linux.intel.com>
---
 drivers/video/fbdev/core/fbcon.c | 73 +++++++++-----------------------
 1 file changed, 19 insertions(+), 54 deletions(-)

Comments

Thomas Zimmermann Sept. 24, 2024, 7:34 a.m. UTC | #1 
Am 23.09.24 um 17:57 schrieb Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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@linux.intel.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   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);
diff mbox series

Patch

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);