Re: [PATCH v6 4/4] power: supply: core: add UAPI to discover currently used extensions

[Thomas Weißschuh]

On 2024-12-18 23:11:35+0100, Sebastian Reichel wrote:
> On Wed, Dec 18, 2024 at 09:29:31PM +0100, Thomas Weißschuh wrote:
> > On 2024-12-18 12:52:29-0700, Nathan Chancellor wrote:
> > > I am seeing a build failure in certain configurations because
> > > power_supply_extension_group is only declared under a CONFIG_SYSFS ifdef
> > > but this code can be built without CONFIG_SYSFS.
> > 
> > Thanks for the report.
> > 
> > >   $ echo 'CONFIG_EXPERT=y
> > >   CONFIG_SYSFS=n' >allno.config
> > > 
> > >   $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux- KCONFIG_ALLCONFIG=1 mrproper allnoconfig drivers/power/supply/power_supply_core.o
> > >   drivers/power/supply/power_supply_core.c: In function 'power_supply_register_extension':
> > >   drivers/power/supply/power_supply_core.c:1389:55: error: 'power_supply_extension_group' undeclared (first use in this function); did you mean 'power_supply_attr_groups'?
> > >    1389 |         ret = sysfs_add_link_to_group(&psy->dev.kobj, power_supply_extension_group.name,
> > >         |                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >         |                                                       power_supply_attr_groups
> > >   drivers/power/supply/power_supply_core.c:1389:55: note: each undeclared identifier is reported only once for each function it appears in
> > >   drivers/power/supply/power_supply_core.c: In function 'power_supply_unregister_extension':
> > >   drivers/power/supply/power_supply_core.c:1419:54: error: 'power_supply_extension_group' undeclared (first use in this function); did you mean 'power_supply_attr_groups'?
> > >    1419 |                                                      power_supply_extension_group.name,
> > >         |                                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >         |                                                      power_supply_attr_groups
> > 
> > The reproducer doesn't actually enable CONFIG_POWER_SUPPLY, when I use it
> > I get a whole array of errors.
> > 
> > > Should the declaration be moved out from the ifdef or is there some
> > > other solution I am not seeing?
> > 
> > This, inline constants or a #define.
> > 
> > Sebastian, do you want me to send a patch?
> 
> Yes, please send a patch. I suppose a define next to the NULL define
> for power_supply_attr_groups should be good enough and consistent
> with existing handling of this problem in the subsystem. 

That doesn't work because of the usage of the member "name" of the
symbol power_supply_extension_group.