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: <aa2b08f8-e6c3-4768-bce9-e36ad9d1e74b@gmail.com>
Date: Tue, 30 Dec 2025 10:25:47 -0800
From: Chintan Patel <chintanlike@...il.com>
To: Helge Deller <deller@...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 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.


Thanks!