power: supply: max8997_charger: make EXTCON dependency unconditional
PATCH 1/1 power: supply: max8997_charger: make EXTCON dependency unconditional
From: Arnd Bergmann <[email protected]>
Some of the extcon interfaces have a fallback implementation that can be used when EXTCON is disabled, but some others do not, causing a build failure:
drivers/power/supply/max8997_charger.c:261:9: error: implicit declaration of function 'devm_extcon_register_notifier_all' [-Werror,-Wimplicit-function-declaration]
ret = devm_extcon_register_notifier_all(&pdev->dev, charger->edev,
^drivers/power/supply/max8997_charger.c:261:9: note: did you mean 'devm_extcon_register_notifier'? include/linux/extcon.h:263:19: note: 'devm_extcon_register_notifier' declared here static inline int devm_extcon_register_notifier(struct device *dev,
I assume there is no reason to actually build this driver without extcon support, so a hard dependency is the easiest fix. Alternatively the header file could be extended to provide additional inline stubs.
--- drivers/power/supply/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/power/supply/Kconfig +1 -1
@@ -555,7 +555,7 @@ config CHARGER_MAX77693 config CHARGER_MAX8997 tristate "Maxim MAX8997/MAX8966 PMIC battery charger driver" depends on MFD_MAX8997 && REGULATOR_MAX8997 - depends on EXTCON || !EXTCON + depends on EXTCON help Say Y to enable support for the battery charger control sysfs and platform data of MAX8997/LP3974 PMICs.
Signature · 1 line
2.29.2
Quoted 9 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
Some of the extcon interfaces have a fallback implementation that can be used when EXTCON is disabled, but some others do not, causing a build failure:
drivers/power/supply/max8997_charger.c:261:9: error: implicit declaration of function 'devm_extcon_register_notifier_all' [-Werror,-Wimplicit-function-declaration]ret = devm_extcon_register_notifier_all(&pdev->dev, charger->edev, ^
Quoted 7 lines
drivers/power/supply/max8997_charger.c:261:9: note: did you mean 'devm_extcon_register_notifier'?include/linux/extcon.h:263:19: note: 'devm_extcon_register_notifier' declared here static inline int devm_extcon_register_notifier(struct device *dev,I assume there is no reason to actually build this driver without extcon support, so a hard dependency is the easiest fix. Alternatively the header file could be extended to provide additional inline stubs.
Hi Arnd,
Thanks for the patch but I think I got it covered with:
https://lore.kernel.org/lkml/[email protected]/
(sent via extcon tree).
Did you experience a new/different issue?
Best regards,
Krzysztof
Quoted 19 lines · Krzysztof Kozlowski
Quoted 9 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
Some of the extcon interfaces have a fallback implementation that can be used when EXTCON is disabled, but some others do not, causing a build failure:
drivers/power/supply/max8997_charger.c:261:9: error: implicit declaration of function 'devm_extcon_register_notifier_all' [-Werror,-Wimplicit-function-declaration]ret = devm_extcon_register_notifier_all(&pdev->dev, charger->edev, ^Quoted 7 lines
drivers/power/supply/max8997_charger.c:261:9: note: did you mean 'devm_extcon_register_notifier'?include/linux/extcon.h:263:19: note: 'devm_extcon_register_notifier' declared here static inline int devm_extcon_register_notifier(struct device *dev,I assume there is no reason to actually build this driver without extcon support, so a hard dependency is the easiest fix. Alternatively the header file could be extended to provide additional inline stubs.
Quoted 7 lines
Hi Arnd,
Thanks for the patch but I think I got it covered with:
https://lore.kernel.org/lkml/[email protected]/
(sent via extcon tree).Did you experience a new/different issue?
The patch should be fine and address the problem, I just didn't see it was already fixed in linux-next as I'm still testing on mainline (rc2 at the moment).
I assume the fix will make it into a future -rc then.
Arnd
Quoted 27 lines · Arnd Bergmann
Quoted 25 lines · Krzysztof Kozlowski
Quoted 9 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
Some of the extcon interfaces have a fallback implementation that can be used when EXTCON is disabled, but some others do not, causing a build failure:
drivers/power/supply/max8997_charger.c:261:9: error: implicit declaration of function 'devm_extcon_register_notifier_all' [-Werror,-Wimplicit-function-declaration]ret = devm_extcon_register_notifier_all(&pdev->dev, charger->edev, ^Quoted 7 lines
drivers/power/supply/max8997_charger.c:261:9: note: did you mean 'devm_extcon_register_notifier'?include/linux/extcon.h:263:19: note: 'devm_extcon_register_notifier' declared here static inline int devm_extcon_register_notifier(struct device *dev,I assume there is no reason to actually build this driver without extcon support, so a hard dependency is the easiest fix. Alternatively the header file could be extended to provide additional inline stubs.
Hi Arnd,
Thanks for the patch but I think I got it covered with:
https://lore.kernel.org/lkml/[email protected]/
(sent via extcon tree).
Quoted 7 lines
Quoted 1 line
Did you experience a new/different issue?
The patch should be fine and address the problem, I just didn't see it was already fixed in linux-next as I'm still testing on mainline (rc2 at the moment).
I assume the fix will make it into a future -rc then.
It's still only in linux-next via extcon tree, so it seems Greg did not take it yet.
Chanwoo,
You might need to follow up on this, so your pull request won't get lost.
Best regards,
Krzysztof
Hi Krzysztof,
Quoted 34 lines · Krzysztof Kozlowski
Quoted 33 lines · Arnd Bergmann
Quoted 19 lines · Krzysztof Kozlowski
Quoted 9 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
Some of the extcon interfaces have a fallback implementation that can be used when EXTCON is disabled, but some others do not, causing a build failure:
drivers/power/supply/max8997_charger.c:261:9: error: implicit declaration of function 'devm_extcon_register_notifier_all' [-Werror,-Wimplicit-function-declaration]ret = devm_extcon_register_notifier_all(&pdev->dev, charger->edev, ^Quoted 7 lines
drivers/power/supply/max8997_charger.c:261:9: note: did you mean 'devm_extcon_register_notifier'?include/linux/extcon.h:263:19: note: 'devm_extcon_register_notifier' declared here static inline int devm_extcon_register_notifier(struct device *dev,I assume there is no reason to actually build this driver without extcon support, so a hard dependency is the easiest fix. Alternatively the header file could be extended to provide additional inline stubs.
Quoted 7 lines
Hi Arnd,
Thanks for the patch but I think I got it covered with:
https://lore.kernel.org/lkml/[email protected]/
(sent via extcon tree).Did you experience a new/different issue?
The patch should be fine and address the problem, I just didn't see it was already fixed in linux-next as I'm still testing on mainline (rc2 at the moment).
Quoted 7 lines
Quoted 1 line
I assume the fix will make it into a future -rc then.
It's still only in linux-next via extcon tree, so it seems Greg did not take it yet.
Chanwoo,
You might need to follow up on this, so your pull request won't get lost.
I'm sorry. Because of my fault, the previous pull request was not merged to v5.12-rc1. To fix this issue, I'll send the pull request for rc3 to Greg.
Best Regards,
Chanwoo Choi
Samsung Electronics
Hello Arnd,
Quoted 6 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
I assume there is no reason to actually build this driver without extcon
support, so a hard dependency is the easiest fix. Alternatively the header file could be extended to provide additional inline stubs.
I am absolutely not insisting this but it would be better if there was no hard dependency. I've tried couple of times to do changes to bunch of drivers (added some devm-functionality or generic definitions or - you name it) and I always end up at least compile-testing changes to multiple drivers. I always repeat following:
1. Manually hack the Makefiles to compile changed drivers as modules
2. Try CONFIG_COMPLILE_TEST
- unfortunately not too widely supported
3. Manually hack more to get drivers with 'hard dependencies' compiled
- occasionally ending up to commenting out the calls with dependencies.
So, if adding the stub is straightforward I'd vote for it.
But I guess you know this quite well so I am just giving my 10 cents - decision can be yours :)
Best Regards
Matti Vaittinen
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland
SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
"non cogito me" dixit Rene Descarte, deinde evanescavit
(Thanks for the translation Simon)
Quoted 22 lines · Matti Vaittinen
Hello Arnd,
Quoted 6 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
I assume there is no reason to actually build this driver without extcon
support, so a hard dependency is the easiest fix. Alternatively the header file could be extended to provide additional inline stubs.I am absolutely not insisting this but it would be better if there was no hard dependency. I've tried couple of times to do changes to bunch of drivers (added some devm-functionality or generic definitions or - you name it) and I always end up at least compile-testing changes to multiple drivers. I always repeat following:
1. Manually hack the Makefiles to compile changed drivers as modules
2. Try
CONFIG_COMPLILE_TEST
- unfortunately not too widely supported
Quoted 7 lines
3. Manually hack more to get drivers with 'hard dependencies' compiled
- occasionally ending up to commenting out the calls with dependencies.So, if adding the stub is straightforward I'd vote for it.
But I guess you know this quite well so I am just giving my 10 cents - decision can be yours :)
As Krzysztof said, he's already fixed this in linux-next the way you prefer. Both approaches are common, and subsystem maintainers have different preferences. I more or less picked one at random here.
The main downside of the 'depends on FOO || !FOO' dependency is that new developers tend to be confused by the syntax. It also means you don't easily catch the problem with building the driver as built-in if the dependency is missing, these can go unnoticed for a long time until someone runs into the wrong randconfig build.
Arnd
Quoted 1 line · Arnd Bergmann
- depends on EXTCON || !EXTCON
I stumbled over this.
What is the point of having this line at all?
What magic trick does it serve for?
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 7 lines · Andy Shevchenko
Quoted 1 line · Arnd Bergmann
- depends on EXTCON || !EXTCON
I stumbled over this.
What is the point of having this line at all?
What magic trick does it serve for?
Okay, it seems I can answer my question: it requires extcon to be built-in when the driver is built-in. I often saw similar written slightly differently.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 7 lines · Andy Shevchenko
Quoted 1 line · Arnd Bergmann
- depends on EXTCON || !EXTCON
I stumbled over this.
What is the point of having this line at all?
What magic trick does it serve for?
The logic was somewhat hairy. I used it once for BD70528 watchdog which provides lock functions for RTC - or stubs if WDG is not used. Problem which that solved was that when RTC was built-in and WDG was a module - these functions were missing. It might've been Guenter or A. Belloni who suggested it.
I don't remember 100% sure but I think it might require EXTCON to be build as a module. I guess the discussion I had regarding BD70528 can be found from lore.kernel.org - but it is probably buried deep... Maybe someone will give better and more definite answer.
Best Regards
Matti Vaittinen