* [PATCH] staging: fbtft: convert sysfs attributes to use attribute_group
@ 2026-05-12 9:28 Harshit Shaw
2026-05-12 9:44 ` Greg KH
2026-05-12 10:58 ` Andy Shevchenko
0 siblings, 2 replies; 3+ messages in thread
From: Harshit Shaw @ 2026-05-12 9:28 UTC (permalink / raw)
To: deller
Cc: gregkh, chintanlike, tzimmermann, andriy.shevchenko,
linux-staging, linux-kernel, Harshit Shaw
Replace direct device_create_file() and device_remove_file() calls
with the correct attribute_group API using sysfs_create_group() and
sysfs_remove_group(). This is the proper way to register sysfs
attributes in kernel drivers.
Signed-off-by: Harshit Shaw <shawharshit116@gmail.com>
---
drivers/staging/fbtft/fbtft-sysfs.c | 83 ++++++++++++++++++-----------
1 file changed, 52 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
index d05599d80011..8d3c4e49b68e 100644
--- a/drivers/staging/fbtft/fbtft-sysfs.c
+++ b/drivers/staging/fbtft/fbtft-sysfs.c
@@ -142,9 +142,30 @@ static ssize_t show_gamma_curve(struct device *device,
return sprintf_gamma(par, par->gamma.curves, buf);
}
-static struct device_attribute gamma_device_attrs[] = {
- __ATTR(gamma, 0660, show_gamma_curve, store_gamma_curve),
-};
+static ssize_t store_debug(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fb_info *fb_info = dev_get_drvdata(device);
+ struct fbtft_par *par = fb_info->par;
+ int ret;
+
+ ret = kstrtoul(buf, 10, &par->debug);
+ if (ret)
+ return ret;
+ fbtft_expand_debug_value(&par->debug);
+
+ return count;
+}
+
+static ssize_t show_debug(struct device *device,
+ struct device_attribute *attr, char *buf)
+{
+ struct fb_info *fb_info = dev_get_drvdata(device);
+ struct fbtft_par *par = fb_info->par;
+
+ return sysfs_emit(buf, "%lu\n", par->debug);
+}
void fbtft_expand_debug_value(unsigned long *debug)
{
@@ -173,45 +194,45 @@ void fbtft_expand_debug_value(unsigned long *debug)
}
}
-static ssize_t store_debug(struct device *device,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct fb_info *fb_info = dev_get_drvdata(device);
- struct fbtft_par *par = fb_info->par;
- int ret;
-
- ret = kstrtoul(buf, 10, &par->debug);
- if (ret)
- return ret;
- fbtft_expand_debug_value(&par->debug);
+static DEVICE_ATTR(gamma, 0644, show_gamma_curve, store_gamma_curve);
+static DEVICE_ATTR(debug, 0644, show_debug, store_debug);
- return count;
-}
+static struct attribute *fbtft_attrs[] = {
+ &dev_attr_debug.attr,
+ NULL,
+};
-static ssize_t show_debug(struct device *device,
- struct device_attribute *attr, char *buf)
-{
- struct fb_info *fb_info = dev_get_drvdata(device);
- struct fbtft_par *par = fb_info->par;
+static struct attribute *fbtft_gamma_attrs[] = {
+ &dev_attr_gamma.attr,
+ NULL,
+};
- return sysfs_emit(buf, "%lu\n", par->debug);
-}
+static const struct attribute_group fbtft_attr_group = {
+ .attrs = fbtft_attrs,
+};
-static struct device_attribute debug_device_attr =
- __ATTR(debug, 0660, show_debug, store_debug);
+static const struct attribute_group fbtft_gamma_attr_group = {
+ .attrs = fbtft_gamma_attrs,
+};
void fbtft_sysfs_init(struct fbtft_par *par)
{
struct device *dev;
+ int ret;
dev = dev_of_fbinfo(par->info);
if (!dev)
return;
- device_create_file(dev, &debug_device_attr);
- if (par->gamma.curves && par->fbtftops.set_gamma)
- device_create_file(dev, &gamma_device_attrs[0]);
+ ret = sysfs_create_group(&dev->kobj, &fbtft_attr_group);
+ if (ret)
+ dev_warn(dev, "failed to create debug sysfs entry\n");
+
+ if (par->gamma.curves && par->fbtftops.set_gamma) {
+ ret = sysfs_create_group(&dev->kobj, &fbtft_gamma_attr_group);
+ if (ret)
+ dev_warn(dev, "failed to create gamma sysfs entry\n");
+ }
}
void fbtft_sysfs_exit(struct fbtft_par *par)
@@ -222,7 +243,7 @@ void fbtft_sysfs_exit(struct fbtft_par *par)
if (!dev)
return;
- device_remove_file(dev, &debug_device_attr);
+ sysfs_remove_group(&dev->kobj, &fbtft_attr_group);
if (par->gamma.curves && par->fbtftops.set_gamma)
- device_remove_file(dev, &gamma_device_attrs[0]);
+ sysfs_remove_group(&dev->kobj, &fbtft_gamma_attr_group);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] staging: fbtft: convert sysfs attributes to use attribute_group
2026-05-12 9:28 [PATCH] staging: fbtft: convert sysfs attributes to use attribute_group Harshit Shaw
@ 2026-05-12 9:44 ` Greg KH
2026-05-12 10:58 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2026-05-12 9:44 UTC (permalink / raw)
To: Harshit Shaw
Cc: deller, chintanlike, tzimmermann, andriy.shevchenko,
linux-staging, linux-kernel
On Tue, May 12, 2026 at 09:28:17AM +0000, Harshit Shaw wrote:
> Replace direct device_create_file() and device_remove_file() calls
> with the correct attribute_group API using sysfs_create_group() and
> sysfs_remove_group(). This is the proper way to register sysfs
> attributes in kernel drivers.
>
> Signed-off-by: Harshit Shaw <shawharshit116@gmail.com>
> ---
> drivers/staging/fbtft/fbtft-sysfs.c | 83 ++++++++++++++++++-----------
> 1 file changed, 52 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
> index d05599d80011..8d3c4e49b68e 100644
> --- a/drivers/staging/fbtft/fbtft-sysfs.c
> +++ b/drivers/staging/fbtft/fbtft-sysfs.c
> @@ -142,9 +142,30 @@ static ssize_t show_gamma_curve(struct device *device,
> return sprintf_gamma(par, par->gamma.curves, buf);
> }
>
> -static struct device_attribute gamma_device_attrs[] = {
> - __ATTR(gamma, 0660, show_gamma_curve, store_gamma_curve),
> -};
> +static ssize_t store_debug(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct fb_info *fb_info = dev_get_drvdata(device);
> + struct fbtft_par *par = fb_info->par;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, &par->debug);
> + if (ret)
> + return ret;
> + fbtft_expand_debug_value(&par->debug);
> +
> + return count;
> +}
> +
> +static ssize_t show_debug(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + struct fb_info *fb_info = dev_get_drvdata(device);
> + struct fbtft_par *par = fb_info->par;
> +
> + return sysfs_emit(buf, "%lu\n", par->debug);
> +}
>
> void fbtft_expand_debug_value(unsigned long *debug)
> {
> @@ -173,45 +194,45 @@ void fbtft_expand_debug_value(unsigned long *debug)
> }
> }
>
> -static ssize_t store_debug(struct device *device,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> -{
> - struct fb_info *fb_info = dev_get_drvdata(device);
> - struct fbtft_par *par = fb_info->par;
> - int ret;
> -
> - ret = kstrtoul(buf, 10, &par->debug);
> - if (ret)
> - return ret;
> - fbtft_expand_debug_value(&par->debug);
> +static DEVICE_ATTR(gamma, 0644, show_gamma_curve, store_gamma_curve);
> +static DEVICE_ATTR(debug, 0644, show_debug, store_debug);
>
> - return count;
> -}
> +static struct attribute *fbtft_attrs[] = {
> + &dev_attr_debug.attr,
> + NULL,
> +};
>
> -static ssize_t show_debug(struct device *device,
> - struct device_attribute *attr, char *buf)
> -{
> - struct fb_info *fb_info = dev_get_drvdata(device);
> - struct fbtft_par *par = fb_info->par;
> +static struct attribute *fbtft_gamma_attrs[] = {
> + &dev_attr_gamma.attr,
> + NULL,
> +};
>
> - return sysfs_emit(buf, "%lu\n", par->debug);
> -}
> +static const struct attribute_group fbtft_attr_group = {
> + .attrs = fbtft_attrs,
> +};
>
> -static struct device_attribute debug_device_attr =
> - __ATTR(debug, 0660, show_debug, store_debug);
> +static const struct attribute_group fbtft_gamma_attr_group = {
> + .attrs = fbtft_gamma_attrs,
> +};
Why not use the ATTRIBUTE_GROUPS() macro instead? That's what it is
there for :)
> void fbtft_sysfs_init(struct fbtft_par *par)
> {
> struct device *dev;
> + int ret;
>
> dev = dev_of_fbinfo(par->info);
> if (!dev)
> return;
>
> - device_create_file(dev, &debug_device_attr);
> - if (par->gamma.curves && par->fbtftops.set_gamma)
> - device_create_file(dev, &gamma_device_attrs[0]);
> + ret = sysfs_create_group(&dev->kobj, &fbtft_attr_group);
Close, but if you ever find a driver calling a "raw" sysfs_* function,
something is really wrong.
This group needs to be added to the driver itself, as a default group
pointer, and then the driver core will add/remove the group
automatically for you when a device is bound/unbound from the driver.
No need to ever do this on its own.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] staging: fbtft: convert sysfs attributes to use attribute_group
2026-05-12 9:28 [PATCH] staging: fbtft: convert sysfs attributes to use attribute_group Harshit Shaw
2026-05-12 9:44 ` Greg KH
@ 2026-05-12 10:58 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-05-12 10:58 UTC (permalink / raw)
To: Harshit Shaw
Cc: deller, gregkh, chintanlike, tzimmermann, linux-staging,
linux-kernel
On Tue, May 12, 2026 at 09:28:17AM +0000, Harshit Shaw wrote:
> Replace direct device_create_file() and device_remove_file() calls
> with the correct attribute_group API using sysfs_create_group() and
> sysfs_remove_group(). This is the proper way to register sysfs
> attributes in kernel drivers.
...
> +static DEVICE_ATTR(gamma, 0644, show_gamma_curve, store_gamma_curve);
> +static DEVICE_ATTR(debug, 0644, show_debug, store_debug);
Move each of them closer to the used callbacks. Also there is DEVICE_ATTR_RW()
macro.
...
> +static struct attribute *fbtft_attrs[] = {
> + &dev_attr_debug.attr,
> + NULL,
Don't put trailing commas in terminator entries.
> +};
...
> +static struct attribute *fbtft_gamma_attrs[] = {
> + &dev_attr_gamma.attr,
> + NULL,
Ditto.
> +};
...
Also these definitions should be moved closer to the related initialisers.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-12 10:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 9:28 [PATCH] staging: fbtft: convert sysfs attributes to use attribute_group Harshit Shaw
2026-05-12 9:44 ` Greg KH
2026-05-12 10:58 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox