linux-kernel - Re: [PATCH v3 4/4] fbdev: sh_mobile_lcdc: Make FB

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <73007a0a-dd96-43eb-be2e-ccbf8b19cd79@gmx.de>
Date: Sat, 3 Jan 2026 10:59:44 +0100
From: Helge Deller <deller@....de>
To: Chintan Patel <chintanlike@...il.com>, Helge Deller <deller@...nel.org>,
 andy <andy@...nel.org>
Cc: linux-fbdev@...r.kernel.org, linux-omap@...r.kernel.org,
 linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v3 4/4] fbdev: sh_mobile_lcdc: Make FB_DEVICE dependency
 optional

On 12/30/25 19:25, Chintan Patel wrote:
> 
> 
> On 12/30/25 00:13, Helge Deller wrote:
>> * Chintan Patel <chintanlike@...il.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@....de>
>>> Signed-off-by: Chintan Patel <chintanlike@...il.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