* [PATCH] fbdev: sh_mobile_lcdcfb: Restore the per-overlay sysfs attributes
@ 2026-09-07 2:18 Karl Mehltretter
2026-09-10 21:15 ` Helge Deller
0 siblings, 1 reply; 3+ messages in thread
From: Karl Mehltretter @ 2026-09-07 2:18 UTC (permalink / raw)
To: Helge Deller
Cc: Karl Mehltretter, Shixiong Ou, Chintan Patel, Laurent Pinchart,
Thomas Zimmermann, linux-fbdev, dri-devel, stable
The ovl_* files documented in
Documentation/ABI/testing/sysfs-devices-platform-sh_mobile_lcdc_fb no
longer exist.
Commit a979182a2453 ("fbdev: lcdcfb: Register sysfs groups through driver
core") moved the attributes from each overlay's framebuffer device to
the platform driver's dev_groups and renamed them overlay_*. There is
now only one set of attributes for the whole LCDC. The callbacks still
expect dev_get_drvdata() to return a struct fb_info, but the platform
device holds struct sh_mobile_lcdc_priv, so they access the wrong
structure.
Restore the documented names and register the group on each overlay's
framebuffer device with device_add_groups(). Remove it with
device_remove_groups() before unregistering the framebuffer. Skip
creation when info->dev is NULL, as the old device_create_file() calls did.
Drop the CONFIG_FB_DEVICE guard, since the driver already depends on it.
Fixes: a979182a2453 ("fbdev: lcdcfb: Register sysfs groups through driver core")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
Found while reviewing the sysfs ABI documentation.
Compile-tested only (allmodconfig, W=1), I have no SH-Mobile hardware.
drivers/video/fbdev/sh_mobile_lcdcfb.c | 37 +++++++++++++++++---------
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index e8324b01700f..18fc4e465283 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -1337,22 +1337,24 @@ overlay_rop3_store(struct device *dev, struct device_attribute *attr,
return count;
}
-static DEVICE_ATTR_RW(overlay_alpha);
-static DEVICE_ATTR_RW(overlay_mode);
-static DEVICE_ATTR_RW(overlay_position);
-static DEVICE_ATTR_RW(overlay_rop3);
-
-static struct attribute *overlay_sysfs_attrs[] __maybe_unused = {
- &dev_attr_overlay_alpha.attr,
- &dev_attr_overlay_mode.attr,
- &dev_attr_overlay_position.attr,
- &dev_attr_overlay_rop3.attr,
+static struct device_attribute dev_attr_ovl_alpha =
+ __ATTR(ovl_alpha, 0644, overlay_alpha_show, overlay_alpha_store);
+static struct device_attribute dev_attr_ovl_mode =
+ __ATTR(ovl_mode, 0644, overlay_mode_show, overlay_mode_store);
+static struct device_attribute dev_attr_ovl_position =
+ __ATTR(ovl_position, 0644, overlay_position_show, overlay_position_store);
+static struct device_attribute dev_attr_ovl_rop3 =
+ __ATTR(ovl_rop3, 0644, overlay_rop3_show, overlay_rop3_store);
+
+static struct attribute *overlay_sysfs_attrs[] = {
+ &dev_attr_ovl_alpha.attr,
+ &dev_attr_ovl_mode.attr,
+ &dev_attr_ovl_position.attr,
+ &dev_attr_ovl_rop3.attr,
NULL,
};
-#ifdef CONFIG_FB_DEVICE
ATTRIBUTE_GROUPS(overlay_sysfs);
-#endif
static const struct fb_fix_screeninfo sh_mobile_lcdc_overlay_fix = {
.id = "SH Mobile LCDC",
@@ -1513,6 +1515,7 @@ sh_mobile_lcdc_overlay_fb_unregister(struct sh_mobile_lcdc_overlay *ovl)
if (info == NULL || info->dev == NULL)
return;
+ device_remove_groups(info->dev, overlay_sysfs_groups);
unregister_framebuffer(ovl->info);
}
@@ -1530,6 +1533,15 @@ sh_mobile_lcdc_overlay_fb_register(struct sh_mobile_lcdc_overlay *ovl)
if (ret < 0)
return ret;
+ /* The framebuffer device is optional, see fb_device_create(). */
+ if (info->dev) {
+ ret = device_add_groups(info->dev, overlay_sysfs_groups);
+ if (ret < 0) {
+ unregister_framebuffer(info);
+ return ret;
+ }
+ }
+
dev_info(lcdc->dev, "registered %s/overlay %u as %dx%d %dbpp.\n",
dev_name(lcdc->dev), ovl->index, info->var.xres,
info->var.yres, info->var.bits_per_pixel);
@@ -2637,7 +2649,6 @@ static int sh_mobile_lcdc_probe(struct platform_device *pdev)
static struct platform_driver sh_mobile_lcdc_driver = {
.driver = {
.name = "sh_mobile_lcdc_fb",
- .dev_groups = overlay_sysfs_groups,
.pm = &sh_mobile_lcdc_dev_pm_ops,
},
.probe = sh_mobile_lcdc_probe,
base-commit: 986c24e0fe44f844b44d365b71ce831947f50298
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] fbdev: sh_mobile_lcdcfb: Restore the per-overlay sysfs attributes
2026-09-07 2:18 [PATCH] fbdev: sh_mobile_lcdcfb: Restore the per-overlay sysfs attributes Karl Mehltretter
@ 2026-09-10 21:15 ` Helge Deller
2026-09-11 1:41 ` Shixiong Ou
0 siblings, 1 reply; 3+ messages in thread
From: Helge Deller @ 2026-09-10 21:15 UTC (permalink / raw)
To: Karl Mehltretter
Cc: Shixiong Ou, Chintan Patel, Laurent Pinchart, Thomas Zimmermann,
linux-fbdev, dri-devel, stable, Arnd Bergmann
On 9/7/26 04:18, Karl Mehltretter wrote:
> The ovl_* files documented in
> Documentation/ABI/testing/sysfs-devices-platform-sh_mobile_lcdc_fb no
> longer exist.
>
> Commit a979182a2453 ("fbdev: lcdcfb: Register sysfs groups through driver
> core") moved the attributes from each overlay's framebuffer device to
> the platform driver's dev_groups and renamed them overlay_*. There is
> now only one set of attributes for the whole LCDC. The callbacks still
> expect dev_get_drvdata() to return a struct fb_info, but the platform
> device holds struct sh_mobile_lcdc_priv, so they access the wrong
> structure.
>
> Restore the documented names and register the group on each overlay's
> framebuffer device with device_add_groups(). Remove it with
> device_remove_groups() before unregistering the framebuffer. Skip
> creation when info->dev is NULL, as the old device_create_file() calls did.
>
> Drop the CONFIG_FB_DEVICE guard, since the driver already depends on it.
>
> Fixes: a979182a2453 ("fbdev: lcdcfb: Register sysfs groups through driver core")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> Found while reviewing the sysfs ABI documentation.
>
> Compile-tested only (allmodconfig, W=1), I have no SH-Mobile hardware.
This all seems correct, but since it reverts a few of the last patches
I'd prefer that some other person could also review it....
> drivers/video/fbdev/sh_mobile_lcdcfb.c | 37 +++++++++++++++++---------
> 1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> index e8324b01700f..18fc4e465283 100644
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> @@ -1337,22 +1337,24 @@ overlay_rop3_store(struct device *dev, struct device_attribute *attr,
> return count;
> }
>
> -static DEVICE_ATTR_RW(overlay_alpha);
> -static DEVICE_ATTR_RW(overlay_mode);
> -static DEVICE_ATTR_RW(overlay_position);
> -static DEVICE_ATTR_RW(overlay_rop3);
> -
> -static struct attribute *overlay_sysfs_attrs[] __maybe_unused = {
> - &dev_attr_overlay_alpha.attr,
> - &dev_attr_overlay_mode.attr,
> - &dev_attr_overlay_position.attr,
> - &dev_attr_overlay_rop3.attr,
> +static struct device_attribute dev_attr_ovl_alpha =
> + __ATTR(ovl_alpha, 0644, overlay_alpha_show, overlay_alpha_store);
> +static struct device_attribute dev_attr_ovl_mode =
> + __ATTR(ovl_mode, 0644, overlay_mode_show, overlay_mode_store);
> +static struct device_attribute dev_attr_ovl_position =
> + __ATTR(ovl_position, 0644, overlay_position_show, overlay_position_store);
> +static struct device_attribute dev_attr_ovl_rop3 =
> + __ATTR(ovl_rop3, 0644, overlay_rop3_show, overlay_rop3_store);
> +
> +static struct attribute *overlay_sysfs_attrs[] = {
> + &dev_attr_ovl_alpha.attr,
> + &dev_attr_ovl_mode.attr,
> + &dev_attr_ovl_position.attr,
> + &dev_attr_ovl_rop3.attr,
> NULL,
> };
>
> -#ifdef CONFIG_FB_DEVICE
> ATTRIBUTE_GROUPS(overlay_sysfs);
> -#endif
>
> static const struct fb_fix_screeninfo sh_mobile_lcdc_overlay_fix = {
> .id = "SH Mobile LCDC",
> @@ -1513,6 +1515,7 @@ sh_mobile_lcdc_overlay_fb_unregister(struct sh_mobile_lcdc_overlay *ovl)
> if (info == NULL || info->dev == NULL)
> return;
>
> + device_remove_groups(info->dev, overlay_sysfs_groups);
> unregister_framebuffer(ovl->info);
> }
>
> @@ -1530,6 +1533,15 @@ sh_mobile_lcdc_overlay_fb_register(struct sh_mobile_lcdc_overlay *ovl)
> if (ret < 0)
> return ret;
>
> + /* The framebuffer device is optional, see fb_device_create(). */
> + if (info->dev) {
I think this needs to be:
if (dev_of_fbinfo(info)) {
to avoid compile error when CONFIG_FB_DEVICE=n.
> + ret = device_add_groups(info->dev, overlay_sysfs_groups);
same here. dev_of_fbinfo(info).
> + if (ret < 0) {
> + unregister_framebuffer(info);
> + return ret;
> + }
> + }
> +
> dev_info(lcdc->dev, "registered %s/overlay %u as %dx%d %dbpp.\n",
> dev_name(lcdc->dev), ovl->index, info->var.xres,
> info->var.yres, info->var.bits_per_pixel);
> @@ -2637,7 +2649,6 @@ static int sh_mobile_lcdc_probe(struct platform_device *pdev)
> static struct platform_driver sh_mobile_lcdc_driver = {
> .driver = {
> .name = "sh_mobile_lcdc_fb",
> - .dev_groups = overlay_sysfs_groups,
> .pm = &sh_mobile_lcdc_dev_pm_ops,
> },
> .probe = sh_mobile_lcdc_probe,
>
> base-commit: 986c24e0fe44f844b44d365b71ce831947f50298
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] fbdev: sh_mobile_lcdcfb: Restore the per-overlay sysfs attributes
2026-09-10 21:15 ` Helge Deller
@ 2026-09-11 1:41 ` Shixiong Ou
0 siblings, 0 replies; 3+ messages in thread
From: Shixiong Ou @ 2026-09-11 1:41 UTC (permalink / raw)
To: Helge Deller, Karl Mehltretter
Cc: Chintan Patel, Laurent Pinchart, Thomas Zimmermann, linux-fbdev,
dri-devel, stable, Arnd Bergmann
On 2026/9/11 05:15, Helge Deller wrote:
> On 9/7/26 04:18, Karl Mehltretter wrote:
>> The ovl_* files documented in
>> Documentation/ABI/testing/sysfs-devices-platform-sh_mobile_lcdc_fb no
>> longer exist.
>>
>> Commit a979182a2453 ("fbdev: lcdcfb: Register sysfs groups through
>> driver
>> core") moved the attributes from each overlay's framebuffer device to
>> the platform driver's dev_groups and renamed them overlay_*. There is
>> now only one set of attributes for the whole LCDC. The callbacks still
>> expect dev_get_drvdata() to return a struct fb_info, but the platform
>> device holds struct sh_mobile_lcdc_priv, so they access the wrong
>> structure.
>>
>> Restore the documented names and register the group on each overlay's
>> framebuffer device with device_add_groups(). Remove it with
>> device_remove_groups() before unregistering the framebuffer. Skip
>> creation when info->dev is NULL, as the old device_create_file()
>> calls did.
>>
>> Drop the CONFIG_FB_DEVICE guard, since the driver already depends on it.
>>
>> Fixes: a979182a2453 ("fbdev: lcdcfb: Register sysfs groups through
>> driver core")
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
>> ---
>> Found while reviewing the sysfs ABI documentation.
>>
>> Compile-tested only (allmodconfig, W=1), I have no SH-Mobile hardware.
>
>
> This all seems correct, but since it reverts a few of the last patches
> I'd prefer that some other person could also review it....
>
Looks good to me, and I agree .dev_groups is wrong here.
Shixiong.
>
>> drivers/video/fbdev/sh_mobile_lcdcfb.c | 37 +++++++++++++++++---------
>> 1 file changed, 24 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c
>> b/drivers/video/fbdev/sh_mobile_lcdcfb.c
>> index e8324b01700f..18fc4e465283 100644
>> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
>> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
>> @@ -1337,22 +1337,24 @@ overlay_rop3_store(struct device *dev, struct
>> device_attribute *attr,
>> return count;
>> }
>> -static DEVICE_ATTR_RW(overlay_alpha);
>> -static DEVICE_ATTR_RW(overlay_mode);
>> -static DEVICE_ATTR_RW(overlay_position);
>> -static DEVICE_ATTR_RW(overlay_rop3);
>> -
>> -static struct attribute *overlay_sysfs_attrs[] __maybe_unused = {
>> - &dev_attr_overlay_alpha.attr,
>> - &dev_attr_overlay_mode.attr,
>> - &dev_attr_overlay_position.attr,
>> - &dev_attr_overlay_rop3.attr,
>> +static struct device_attribute dev_attr_ovl_alpha =
>> + __ATTR(ovl_alpha, 0644, overlay_alpha_show, overlay_alpha_store);
>> +static struct device_attribute dev_attr_ovl_mode =
>> + __ATTR(ovl_mode, 0644, overlay_mode_show, overlay_mode_store);
>> +static struct device_attribute dev_attr_ovl_position =
>> + __ATTR(ovl_position, 0644, overlay_position_show,
>> overlay_position_store);
>> +static struct device_attribute dev_attr_ovl_rop3 =
>> + __ATTR(ovl_rop3, 0644, overlay_rop3_show, overlay_rop3_store);
>> +
>> +static struct attribute *overlay_sysfs_attrs[] = {
>> + &dev_attr_ovl_alpha.attr,
>> + &dev_attr_ovl_mode.attr,
>> + &dev_attr_ovl_position.attr,
>> + &dev_attr_ovl_rop3.attr,
>> NULL,
>> };
>> -#ifdef CONFIG_FB_DEVICE
>> ATTRIBUTE_GROUPS(overlay_sysfs);
>> -#endif
>> static const struct fb_fix_screeninfo sh_mobile_lcdc_overlay_fix
>> = {
>> .id = "SH Mobile LCDC",
>> @@ -1513,6 +1515,7 @@ sh_mobile_lcdc_overlay_fb_unregister(struct
>> sh_mobile_lcdc_overlay *ovl)
>> if (info == NULL || info->dev == NULL)
>> return;
>> + device_remove_groups(info->dev, overlay_sysfs_groups);
>> unregister_framebuffer(ovl->info);
>> }
>> @@ -1530,6 +1533,15 @@ sh_mobile_lcdc_overlay_fb_register(struct
>> sh_mobile_lcdc_overlay *ovl)
>> if (ret < 0)
>> return ret;
>> + /* The framebuffer device is optional, see fb_device_create(). */
>> + if (info->dev) {
>
> I think this needs to be:
> if (dev_of_fbinfo(info)) {
> to avoid compile error when CONFIG_FB_DEVICE=n.
>
>
>> + ret = device_add_groups(info->dev, overlay_sysfs_groups);
>
> same here. dev_of_fbinfo(info).
>
>> + if (ret < 0) {
>> + unregister_framebuffer(info);
>> + return ret;
>> + }
>> + }
>> +
>> dev_info(lcdc->dev, "registered %s/overlay %u as %dx%d %dbpp.\n",
>> dev_name(lcdc->dev), ovl->index, info->var.xres,
>> info->var.yres, info->var.bits_per_pixel);
>> @@ -2637,7 +2649,6 @@ static int sh_mobile_lcdc_probe(struct
>> platform_device *pdev)
>> static struct platform_driver sh_mobile_lcdc_driver = {
>> .driver = {
>> .name = "sh_mobile_lcdc_fb",
>> - .dev_groups = overlay_sysfs_groups,
>> .pm = &sh_mobile_lcdc_dev_pm_ops,
>> },
>> .probe = sh_mobile_lcdc_probe,
>>
>> base-commit: 986c24e0fe44f844b44d365b71ce831947f50298
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-11 1:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07 2:18 [PATCH] fbdev: sh_mobile_lcdcfb: Restore the per-overlay sysfs attributes Karl Mehltretter
2026-09-10 21:15 ` Helge Deller
2026-09-11 1:41 ` Shixiong Ou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).