| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9aec4ef1-7428-4630-a4af-d7448a023a60@suse.de>
Date: Fri, 14 Mar 2025 09:16:58 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: oushixiong1025@....com, Simona Vetter <simona@...ll.ch>
Cc: Helge Deller <deller@....de>,
Samuel Thibault <samuel.thibault@...-lyon.org>,
Zsolt Kajtar <soci@....rulez.org>,
Thomas Weißschuh <thomas.weissschuh@...utronix.de>,
linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, Shixiong Ou <oushixiong@...inos.cn>
Subject: Re: [PATCH v2] fbcon: Use static attribute groups for sysfs entries
Hi
Am 14.03.25 um 07:09 schrieb oushixiong1025@....com:
> From: Shixiong Ou <oushixiong@...inos.cn>
>
> Using device_create_with_groups() to simplify creation and removal.
> Same as commit 1083a7be4504 ("tty: Use static attribute groups for
> sysfs entries").
>
> Signed-off-by: Shixiong Ou <oushixiong@...inos.cn>
Acked-by: Thomas Zimmermann <tzimmermann@...e.de>
with minor comments below
> ---
> drivers/video/fbdev/core/fbcon.c | 69 +++++++++-----------------------
> 1 file changed, 19 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 07d127110ca4..1d792bd11063 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;
> @@ -3159,7 +3158,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 +3180,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 +3202,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 +3221,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 +3246,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,35 +3280,18 @@ 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;
> +static DEVICE_ATTR_RW(rotate);
> +static DEVICE_ATTR_WO(rotate_all);
> +static DEVICE_ATTR_RW(cursor_blink);
Since you're changing it anyway, could you please sort these attributes
alphabetically. So cursor blink should go first. It's easier to read then.
>
> - 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 struct attribute *fbcon_device_attrs[] = {
> + &dev_attr_rotate.attr,
> + &dev_attr_rotate_all.attr,
> + &dev_attr_cursor_blink.attr,
> + NULL
Alphabetical sorting here as well.
Best regards
Thomas
> +};
>
> - return 0;
> -}
> +ATTRIBUTE_GROUPS(fbcon_device);
>
> #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> static void fbcon_register_existing_fbs(struct work_struct *work)
> @@ -3367,16 +3349,16 @@ void __init fb_console_init(void)
> 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,
> + fbcon_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;
> @@ -3387,18 +3369,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
> @@ -3411,7 +3381,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)
Powered by blists - more mailing lists