On Sat, Jan 03, 2026 at 10:59:44AM +0100, Helge Deller wrote:

On 12/30/25 19:25, Chintan Patel wrote:

quoted

On 12/30/25 00:13, Helge Deller wrote:

...

quoted

quoted

quoted

-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

?

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... ?

I'm also fine with original code. But a suggested approach would work as well
(at least like it sounds from the above description). Ideally would be nice to
get rid of ifdeffery completely (that's why we have PTR_IF() for), although
it might be not so readable. TL;DR: the most readable solution is the winner.

-- 
With Best Regards,
Andy Shevchenko