diff mbox series

power: supply: core: Fix power_supply_init_attrs() stub

Message ID 20240227-fix-power_supply_init_attrs-stub-v1-1-43365e68d4b3@kernel.org (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series power: supply: core: Fix power_supply_init_attrs() stub | expand

Commit Message

Nathan Chancellor Feb. 27, 2024, 8:34 p.m. UTC 
When building without CONFIG_SYSFS, there is an error because of a
recent refactoring that failed to update the stub of
power_supply_init_attrs():

  drivers/power/supply/power_supply_core.c: In function 'power_supply_class_init':
  drivers/power/supply/power_supply_core.c:1630:9: error: too few arguments to function 'power_supply_init_attrs'
   1630 |         power_supply_init_attrs();
        |         ^~~~~~~~~~~~~~~~~~~~~~~
  In file included from drivers/power/supply/power_supply_core.c:25:
  drivers/power/supply/power_supply.h:25:20: note: declared here
     25 | static inline void power_supply_init_attrs(struct device_type *dev_type) {}
        |                    ^~~~~~~~~~~~~~~~~~~~~~~

Update the stub function to take no parameters like the rest of the
refactoring, which resolves the build error.

Fixes: 7b46b60944d7 ("power: supply: core: constify the struct device_type usage")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/power/supply/power_supply.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: 3da8d71754d3c1aa0b72d74c8a324a4bc7fab473
change-id: 20240227-fix-power_supply_init_attrs-stub-7be5328b4e72

Best regards,

Comments

Ricardo B. Marliere Feb. 27, 2024, 8:39 p.m. UTC | #1 
Hi Nathan,

On 27 Feb 13:34, Nathan Chancellor wrote:
> When building without CONFIG_SYSFS, there is an error because of a
> recent refactoring that failed to update the stub of
> power_supply_init_attrs():
> 
>   drivers/power/supply/power_supply_core.c: In function 'power_supply_class_init':
>   drivers/power/supply/power_supply_core.c:1630:9: error: too few arguments to function 'power_supply_init_attrs'
>    1630 |         power_supply_init_attrs();
>         |         ^~~~~~~~~~~~~~~~~~~~~~~
>   In file included from drivers/power/supply/power_supply_core.c:25:
>   drivers/power/supply/power_supply.h:25:20: note: declared here
>      25 | static inline void power_supply_init_attrs(struct device_type *dev_type) {}
>         |                    ^~~~~~~~~~~~~~~~~~~~~~~
> 
> Update the stub function to take no parameters like the rest of the
> refactoring, which resolves the build error.
> 
> Fixes: 7b46b60944d7 ("power: supply: core: constify the struct device_type usage")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  drivers/power/supply/power_supply.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> ────────────────────────────────────────────────────────────────────────────────
> modified: drivers/power/supply/power_supply.h
> ────────────────────────────────────────────────────────────────────────────────
> @ drivers/power/supply/power_supply.h:25 @ extern int power_supply_uevent(const struct device *dev, struct kobj_uevent_env
> 
> #else
> 
> static inline void power_supply_init_attrs(struct device_type *dev_type) {}
> static inline void power_supply_init_attrs(void) {}

I've missed that #else in my building test. Thanks for catching it.

Reviewed-by: Ricardo B. Marliere <ricardo@marliere.net>

> #define power_supply_uevent NULL
> 
> #endif /* CONFIG_SYSFS */
> 
> --
> base-commit: 3da8d71754d3c1aa0b72d74c8a324a4bc7fab473
> change-id: 20240227-fix-power_supply_init_attrs-stub-7be5328b4e72
> 
> Best regards,
> - 
> Nathan Chancellor <nathan@kernel.org>
>
Nathan Chancellor Feb. 27, 2024, 9:49 p.m. UTC | #2 
On Tue, Feb 27, 2024 at 05:39:55PM -0300, Ricardo B. Marliere wrote:
> Hi Nathan,
> 
> On 27 Feb 13:34, Nathan Chancellor wrote:
> > When building without CONFIG_SYSFS, there is an error because of a
> > recent refactoring that failed to update the stub of
> > power_supply_init_attrs():
> > 
> >   drivers/power/supply/power_supply_core.c: In function 'power_supply_class_init':
> >   drivers/power/supply/power_supply_core.c:1630:9: error: too few arguments to function 'power_supply_init_attrs'
> >    1630 |         power_supply_init_attrs();
> >         |         ^~~~~~~~~~~~~~~~~~~~~~~
> >   In file included from drivers/power/supply/power_supply_core.c:25:
> >   drivers/power/supply/power_supply.h:25:20: note: declared here
> >      25 | static inline void power_supply_init_attrs(struct device_type *dev_type) {}
> >         |                    ^~~~~~~~~~~~~~~~~~~~~~~
> > 
> > Update the stub function to take no parameters like the rest of the
> > refactoring, which resolves the build error.
> > 
> > Fixes: 7b46b60944d7 ("power: supply: core: constify the struct device_type usage")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >  drivers/power/supply/power_supply.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > 
> > ────────────────────────────────────────────────────────────────────────────────
> > modified: drivers/power/supply/power_supply.h
> > ────────────────────────────────────────────────────────────────────────────────
> > @ drivers/power/supply/power_supply.h:25 @ extern int power_supply_uevent(const struct device *dev, struct kobj_uevent_env
> > 
> > #else
> > 
> > static inline void power_supply_init_attrs(struct device_type *dev_type) {}
> > static inline void power_supply_init_attrs(void) {}
> 
> I've missed that #else in my building test. Thanks for catching it.
> 
> Reviewed-by: Ricardo B. Marliere <ricardo@marliere.net>

Thanks a lot for the quick feedback and no worries, it is hard to test
to catch these without doing a lot of build tests.

Unfortunately, I caught another problem with that change that is
independent of this one:

  ld.lld: error: undefined symbol: power_supply_attr_group
  >>> referenced by power_supply_core.c
  >>>               drivers/power/supply/power_supply_core.o:(power_supply_attr_groups) in archive vmlinux.a
  >>> did you mean: power_supply_attr_groups
  >>> defined in: vmlinux.a(drivers/power/supply/power_supply_core.o)

It looks like power_supply_attr_groups refers to power_supply_attr_group
but power_supply_attr_group is declared extern without a definition with
CONFIG_SYSFS=n. It is not immediately obvious to me what the fix is.

Cheers,
Nathan

> > #define power_supply_uevent NULL
> > 
> > #endif /* CONFIG_SYSFS */
> > 
> > --
> > base-commit: 3da8d71754d3c1aa0b72d74c8a324a4bc7fab473
> > change-id: 20240227-fix-power_supply_init_attrs-stub-7be5328b4e72
> > 
> > Best regards,
> > - 
> > Nathan Chancellor <nathan@kernel.org>
> >
Ricardo B. Marliere Feb. 28, 2024, 1:31 a.m. UTC | #3 
On 27 Feb 14:49, Nathan Chancellor wrote:
> On Tue, Feb 27, 2024 at 05:39:55PM -0300, Ricardo B. Marliere wrote:
> > Hi Nathan,
> > 
> > On 27 Feb 13:34, Nathan Chancellor wrote:
> > > When building without CONFIG_SYSFS, there is an error because of a
> > > recent refactoring that failed to update the stub of
> > > power_supply_init_attrs():
> > > 
> > >   drivers/power/supply/power_supply_core.c: In function 'power_supply_class_init':
> > >   drivers/power/supply/power_supply_core.c:1630:9: error: too few arguments to function 'power_supply_init_attrs'
> > >    1630 |         power_supply_init_attrs();
> > >         |         ^~~~~~~~~~~~~~~~~~~~~~~
> > >   In file included from drivers/power/supply/power_supply_core.c:25:
> > >   drivers/power/supply/power_supply.h:25:20: note: declared here
> > >      25 | static inline void power_supply_init_attrs(struct device_type *dev_type) {}
> > >         |                    ^~~~~~~~~~~~~~~~~~~~~~~
> > > 
> > > Update the stub function to take no parameters like the rest of the
> > > refactoring, which resolves the build error.
> > > 
> > > Fixes: 7b46b60944d7 ("power: supply: core: constify the struct device_type usage")
> > > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > > ---
> > >  drivers/power/supply/power_supply.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > 
> > > ────────────────────────────────────────────────────────────────────────────────
> > > modified: drivers/power/supply/power_supply.h
> > > ────────────────────────────────────────────────────────────────────────────────
> > > @ drivers/power/supply/power_supply.h:25 @ extern int power_supply_uevent(const struct device *dev, struct kobj_uevent_env
> > > 
> > > #else
> > > 
> > > static inline void power_supply_init_attrs(struct device_type *dev_type) {}
> > > static inline void power_supply_init_attrs(void) {}
> > 
> > I've missed that #else in my building test. Thanks for catching it.
> > 
> > Reviewed-by: Ricardo B. Marliere <ricardo@marliere.net>
> 
> Thanks a lot for the quick feedback and no worries, it is hard to test
> to catch these without doing a lot of build tests.
> 
> Unfortunately, I caught another problem with that change that is
> independent of this one:
> 
>   ld.lld: error: undefined symbol: power_supply_attr_group
>   >>> referenced by power_supply_core.c
>   >>>               drivers/power/supply/power_supply_core.o:(power_supply_attr_groups) in archive vmlinux.a
>   >>> did you mean: power_supply_attr_groups
>   >>> defined in: vmlinux.a(drivers/power/supply/power_supply_core.o)
> 
> It looks like power_supply_attr_groups refers to power_supply_attr_group
> but power_supply_attr_group is declared extern without a definition with
> CONFIG_SYSFS=n. It is not immediately obvious to me what the fix is.

Ah, indeed. I was able to build with the patch below. The problem is
that power_supply_attr_group is needed in power_supply_core.c but
defined in power_supply_sysfs.c, which is only targeted with
CONFIG_SYSFS=y. This was needed in order to make power_supply_dev_type
constant [1]. I will see if there is a better way of fixing it and send
a proper patch tomorrow.

Best regards,
-	Ricardo.

---
[1] https://lore.kernel.org/all/20240224-device_cleanup-power-v2-1-465ff94b896c@marliere.net/


diff --git a/drivers/power/supply/power_supply.h b/drivers/power/supply/power_supply.h
index 232fdd8c1212..ef9f1b2e87d5 100644
--- a/drivers/power/supply/power_supply.h
+++ b/drivers/power/supply/power_supply.h
@@ -13,16 +13,16 @@ struct device;
 struct device_type;
 struct power_supply;
 
-extern const struct attribute_group power_supply_attr_group;
-
 #ifdef CONFIG_SYSFS
 
 extern void power_supply_init_attrs(void);
 extern int power_supply_uevent(const struct device *dev, struct kobj_uevent_env *env);
+extern const struct attribute_group power_supply_attr_group;
 
 #else
 
-static inline void power_supply_init_attrs(struct device_type *dev_type) {}
+static inline void power_supply_init_attrs(void) {}
+static struct attribute_group power_supply_attr_group;
 #define power_supply_uevent NULL
 
 #endif /* CONFIG_SYSFS */



> 
> Cheers,
> Nathan
> 
> > > #define power_supply_uevent NULL
> > > 
> > > #endif /* CONFIG_SYSFS */
> > > 
> > > --
> > > base-commit: 3da8d71754d3c1aa0b72d74c8a324a4bc7fab473
> > > change-id: 20240227-fix-power_supply_init_attrs-stub-7be5328b4e72
> > > 
> > > Best regards,
> > > - 
> > > Nathan Chancellor <nathan@kernel.org>
> > >
Sebastian Reichel Feb. 28, 2024, 10:08 p.m. UTC | #4 
On Tue, 27 Feb 2024 13:34:42 -0700, Nathan Chancellor wrote:
> When building without CONFIG_SYSFS, there is an error because of a
> recent refactoring that failed to update the stub of
> power_supply_init_attrs():
> 
>   drivers/power/supply/power_supply_core.c: In function 'power_supply_class_init':
>   drivers/power/supply/power_supply_core.c:1630:9: error: too few arguments to function 'power_supply_init_attrs'
>    1630 |         power_supply_init_attrs();
>         |         ^~~~~~~~~~~~~~~~~~~~~~~
>   In file included from drivers/power/supply/power_supply_core.c:25:
>   drivers/power/supply/power_supply.h:25:20: note: declared here
>      25 | static inline void power_supply_init_attrs(struct device_type *dev_type) {}
>         |                    ^~~~~~~~~~~~~~~~~~~~~~~
> 
> [...]

Applied, thanks!

[1/1] power: supply: core: Fix power_supply_init_attrs() stub
      commit: b683d738c0a1c4c8fcf83fdf5eb4c6ce3d5130c6

Best regards,
diff mbox series

Patch

diff --git a/drivers/power/supply/power_supply.h b/drivers/power/supply/power_supply.h
index 232fdd8c1212..7d05756398b9 100644
--- a/drivers/power/supply/power_supply.h
+++ b/drivers/power/supply/power_supply.h
@@ -22,7 +22,7 @@  extern int power_supply_uevent(const struct device *dev, struct kobj_uevent_env
 
 #else
 
-static inline void power_supply_init_attrs(struct device_type *dev_type) {}
+static inline void power_supply_init_attrs(void) {}
 #define power_supply_uevent NULL
 
 #endif /* CONFIG_SYSFS */