Re: [PATCH v3 4/4] fbdev: sh_mobile_lcdc: Make FB_DEVICE dependency optional

[Helge Deller <deller@gmx.de>]

On 12/30/25 19:25, Chintan Patel wrote:
> 
> 
> On 12/30/25 00:13, Helge Deller wrote:
>> * Chintan Patel <chintanlike@gmail.com>:
>>> The sh_mobile_lcdc driver exposes overlay configuration via sysfs, but the
>>> core driver does not require CONFIG_FB_DEVICE.
>>>
>>> Make sysfs support optional by defining overlay_sysfs_groups conditionally
>>> using PTR_IF(). The driver always sets .dev_groups, and the kernel
>>> naturally skips NULL attribute groups while the code remains buildable
>>> and type-checked.
>>>
>>> Suggested-by: Helge Deller <deller@gmx.de>
>>> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
>>> ---
>>>   drivers/video/fbdev/sh_mobile_lcdcfb.c | 12 +++++++++++-
>>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
>>> index dd950e4ab5ce..cb7ed1ff9165 100644
>>> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
>>> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
>>> @@ -1350,7 +1350,17 @@ static struct attribute *overlay_sysfs_attrs[] = {
>>>       &dev_attr_overlay_rop3.attr,
>>>       NULL,
>>>   };
>>> -ATTRIBUTE_GROUPS(overlay_sysfs);
>>
>> Instead of replacing the ^ ATTRIBUTE_GROUPS() by the code below,
>> isn't it possible to just mark the overlay_sysfs_attrs[] array
>> _maybe_unused, and just do:
>> + #ifdef CONFIG_FB_DEVICE
>> + ATTRIBUTE_GROUPS(overlay_sysfs);
>> + #endif
>>
>> ?
> Hi Helge,
> 
> Yes, the __maybe_unused + #ifdef ATTRIBUTE_GROUPS() approach would work.
> 
> I went with the PTR_IF(IS_ENABLED()) pattern because Andy suggested
> using PTR_IF() to conditionally include overlay_sysfs_group in
> overlay_sysfs_groups, and to keep .dev_groups always populated while
> letting the device core skip NULL groups. This avoids conditional
> wiring via #ifdef and keeps the code type-checked without
> CONFIG_FB_DEVICE.
> If you still prefer the simpler #ifdef ATTRIBUTE_GROUPS() approach
> for this driver, I can switch to that, but I wanted to follow Andy’s
> guidance here.

I assume Andy will agree to my suggested approach, as it's cleaner
and avoids code bloat/duplication. Maybe you send out a v4 with my
suggested approach, then it's easier to judge... ?

Helge