[PATCH 1/2] fbcon: Register sysfs groups through device_add_group

From: Shixiong Ou <redacted>

Use device_add_group() to simplify creation and removal.

Signed-off-by: Shixiong Ou <redacted>
---
 drivers/video/fbdev/core/fbcon.c | 48 +++++++++++++++-----------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 07d127110ca4..51c3e8a5a092 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c

@@ -3159,7 +3159,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)
 {

@@ -3181,7 +3181,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)
 {

@@ -3203,7 +3203,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;

@@ -3222,7 +3222,7 @@ 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)
 {
 	struct fb_info *info;

@@ -3247,7 +3247,7 @@ static ssize_t show_cursor_blink(struct device *device,
 	return sysfs_emit(buf, "%d\n", blink);
 }
 
-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)
 {

@@ -3281,32 +3281,30 @@ 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 DEVICE_ATTR_RW(rotate);
+static DEVICE_ATTR_WO(rotate_all);
+static DEVICE_ATTR_RW(cursor_blink);
+
+static struct attribute *fbcon_device_attrs[] = {
+	&dev_attr_rotate.attr,
+	&dev_attr_rotate_all.attr,
+	&dev_attr_cursor_blink.attr,
+	NULL,
+};
+
+static const struct attribute_group fbcon_device_attr_group = {
+	.attrs		= fbcon_device_attrs,
 };
 
 static int fbcon_init_device(void)
 {
-	int i, error = 0;
+	int ret;
 
 	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]);
-
+	ret = device_add_group(fbcon_device, &fbcon_device_attr_group);
+	if (ret)
 		fbcon_has_sysfs = 0;
-	}
 
 	return 0;
 }

@@ -3389,11 +3387,9 @@ void __init fb_console_init(void)
 
 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]);
+		device_remove_group(fb_info->dev, &fbcon_device_attr_group);
 
 		fbcon_has_sysfs = 0;
 	}

-- 
2.25.1

From: Shixiong Ou <redacted>

fbcon_init_device() doesn't need to return a value.

Signed-off-by: Shixiong Ou <redacted>
---
 drivers/video/fbdev/core/fbcon.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 51c3e8a5a092..c1692b68f69e 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c

@@ -3296,7 +3296,7 @@ static const struct attribute_group fbcon_device_attr_group = {
 	.attrs		= fbcon_device_attrs,
 };
 
-static int fbcon_init_device(void)
+static void fbcon_init_device(void)
 {
 	int ret;
 

@@ -3305,8 +3305,6 @@ static int fbcon_init_device(void)
 	ret = device_add_group(fbcon_device, &fbcon_device_attr_group);
 	if (ret)
 		fbcon_has_sysfs = 0;
-
-	return 0;
 }
 
 #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER

-- 
2.25.1

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.14-rc6 next-20250311]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/oushixiong1025-163-com/fbcon-Change-return-value-type-to-void/20250311-193230
base:   linus/master
patch link:    https://lore.kernel.org/r/20250311112856.1020095-1-oushixiong1025%40163.com
patch subject: [PATCH 1/2] fbcon: Register sysfs groups through device_add_group
config: sparc-randconfig-001-20250312 (https://download.01.org/0day-ci/archive/20250312/202503121852.0x6J0c7a-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250312/202503121852.0x6J0c7a-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot [off-list ref]
| Closes: https://lore.kernel.org/oe-kbuild-all/202503121852.0x6J0c7a-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/video/fbdev/core/fbcon.c: In function 'fbcon_deinit_device':

quoted

drivers/video/fbdev/core/fbcon.c:3390:37: error: 'fb_info' undeclared (first use in this function)

    3390 |                 device_remove_group(fb_info->dev, &fbcon_device_attr_group);
         |                                     ^~~~~~~
   drivers/video/fbdev/core/fbcon.c:3390:37: note: each undeclared identifier is reported only once for each function it appears in


vim +/fb_info +3390 drivers/video/fbdev/core/fbcon.c

  3388	
  3389		if (fbcon_has_sysfs) {

3390			device_remove_group(fb_info->dev, &fbcon_device_attr_group);

  3391	
  3392			fbcon_has_sysfs = 0;
  3393		}
  3394	}
  3395	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.14-rc6 next-20250312]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/oushixiong1025-163-com/fbcon-Change-return-value-type-to-void/20250311-193230
base:   linus/master
patch link:    https://lore.kernel.org/r/20250311112856.1020095-1-oushixiong1025%40163.com
patch subject: [PATCH 1/2] fbcon: Register sysfs groups through device_add_group
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20250312/202503122323.h75SQfrd-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project e15545cad8297ec7555f26e5ae74a9f0511203e7)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250312/202503122323.h75SQfrd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot [off-list ref]
| Closes: https://lore.kernel.org/oe-kbuild-all/202503122323.h75SQfrd-lkp@intel.com/

All errors (new ones prefixed by >>):

quoted

drivers/video/fbdev/core/fbcon.c:3390:23: error: use of undeclared identifier 'fb_info'

    3390 |                 device_remove_group(fb_info->dev, &fbcon_device_attr_group);
         |                                     ^
   1 error generated.


vim +/fb_info +3390 drivers/video/fbdev/core/fbcon.c

  3388	
  3389		if (fbcon_has_sysfs) {

3390			device_remove_group(fb_info->dev, &fbcon_device_attr_group);

  3391	
  3392			fbcon_has_sysfs = 0;
  3393		}
  3394	}
  3395	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Hi,

On Tue, Mar 11, 2025 at 07:28:55PM +0800, oushixiong1025@163.com wrote:

No trailing commas after sentinel values.

Please at least compile-test your changes before submission.

 
 		fbcon_has_sysfs = 0;
 	}

All of this can be simplified even more:

* fbcon_deinit_device() can be removed easily, as the attributes are
  automatically removed when the device is destroyed.
* Using device_create_with_groups() the device core will take complete care of
  the attribute lifecycle, also allowing to remove fbcon_init_device()


Thomas

在 2025/3/13 00:47, Thomas Weißschuh 写道:

Hi,

On Tue, Mar 11, 2025 at 07:28:55PM +0800, oushixiong1025@163.com wrote:

quoted

From: Shixiong Ou <redacted>

Use device_add_group() to simplify creation and removal.

Signed-off-by: Shixiong Ou <redacted>
---
  drivers/video/fbdev/core/fbcon.c | 48 +++++++++++++++-----------------
  1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 07d127110ca4..51c3e8a5a092 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c

@@ -3159,7 +3159,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)
  {

@@ -3181,7 +3181,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)
  {

@@ -3203,7 +3203,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;

@@ -3222,7 +3222,7 @@ 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)
  {
  	struct fb_info *info;

@@ -3247,7 +3247,7 @@ static ssize_t show_cursor_blink(struct device *device,
  	return sysfs_emit(buf, "%d\n", blink);
  }
  
-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)
  {

@@ -3281,32 +3281,30 @@ 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 DEVICE_ATTR_RW(rotate);
+static DEVICE_ATTR_WO(rotate_all);
+static DEVICE_ATTR_RW(cursor_blink);
+
+static struct attribute *fbcon_device_attrs[] = {
+	&dev_attr_rotate.attr,
+	&dev_attr_rotate_all.attr,
+	&dev_attr_cursor_blink.attr,
+	NULL,

No trailing commas after sentinel values.

quoted

+};
+
+static const struct attribute_group fbcon_device_attr_group = {
+	.attrs		= fbcon_device_attrs,
  };
  
  static int fbcon_init_device(void)
  {
-	int i, error = 0;
+	int ret;
  
  	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]);
-
+	ret = device_add_group(fbcon_device, &fbcon_device_attr_group);
+	if (ret)
  		fbcon_has_sysfs = 0;
-	}
  
  	return 0;
  }

@@ -3389,11 +3387,9 @@ void __init fb_console_init(void)
  
  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]);
+		device_remove_group(fb_info->dev, &fbcon_device_attr_group);

Please at least compile-test your changes before submission.

quoted

  
  		fbcon_has_sysfs = 0;
  	}

All of this can be simplified even more:

* fbcon_deinit_device() can be removed easily, as the attributes are
   automatically removed when the device is destroyed.
* Using device_create_with_groups() the device core will take complete care of
   the attribute lifecycle, also allowing to remove fbcon_init_device()

Thanks for your advice, I will fix and refactor this code in the next patch.

Thanks and Regards,
Shixiong Ou.

Thomas