Registering the cooling device from the probe can result in the
execution of get_property() function before it gets initialized.
To avoid this, register the cooling device from a workqueue
instead of registering in the probe.
Signed-off-by: Manaf Meethalavalappu Pallikunhi <[email protected]>
---
drivers/power/supply/power_supply_core.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 10a357a..c306b9d 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -132,6 +132,7 @@ void power_supply_changed(struct power_supply *psy)
}
EXPORT_SYMBOL_GPL(power_supply_changed);
+static int psy_register_cooler(struct device *dev, struct power_supply *psy);
/*
* Notify that power supply was registered after parent finished the probing.
*
@@ -139,6 +140,8 @@ EXPORT_SYMBOL_GPL(power_supply_changed);
* calling power_supply_changed() directly from power_supply_register()
* would lead to execution of get_property() function provided by the driver
* too early - before the probe ends.
+ * Also, registering cooling device from the probe will execute the
+ * get_property() function. So register the cooling device after the probe.
*
* Avoid that by waiting on parent's mutex.
*/
@@ -156,6 +159,7 @@ static void power_supply_deferred_register_work(struct work_struct *work)
}
power_supply_changed(psy);
+ psy_register_cooler(psy->dev.parent, psy);
if (psy->dev.parent)
mutex_unlock(&psy->dev.parent->mutex);
@@ -1257,10 +1261,6 @@ __power_supply_register(struct device *parent,
if (rc)
goto register_thermal_failed;
- rc = psy_register_cooler(psy);
- if (rc)
- goto register_cooler_failed;
-
rc = power_supply_create_triggers(psy);
if (rc)
goto create_triggers_failed;
@@ -1290,8 +1290,6 @@ __power_supply_register(struct device *parent,
add_hwmon_sysfs_failed:
power_supply_remove_triggers(psy);
create_triggers_failed:
- psy_unregister_cooler(psy);
-register_cooler_failed:
psy_unregister_thermal(psy);
register_thermal_failed:
device_del(dev);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Hi Manaf,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on sre-power-supply/for-next]
[also build test WARNING on v5.16-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Manaf-Meethalavalappu-Pallikunhi/power_supply-Register-cooling-device-outside-of-probe/20211213-191238
base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: i386-randconfig-s001-20211213 (https://download.01.org/0day-ci/archive/20211213/[email protected]/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/e5930a7ed9ae1b121e0dde177184ff74abbf0371
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Manaf-Meethalavalappu-Pallikunhi/power_supply-Register-cooling-device-outside-of-probe/20211213-191238
git checkout e5930a7ed9ae1b121e0dde177184ff74abbf0371
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/power/supply/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers/power/supply/power_supply_core.c:1086:12: error: conflicting types for 'psy_register_cooler'
1086 | static int psy_register_cooler(struct power_supply *psy)
| ^~~~~~~~~~~~~~~~~~~
drivers/power/supply/power_supply_core.c:129:12: note: previous declaration of 'psy_register_cooler' was here
129 | static int psy_register_cooler(struct device *dev, struct power_supply *psy);
| ^~~~~~~~~~~~~~~~~~~
>> drivers/power/supply/power_supply_core.c:129:12: warning: 'psy_register_cooler' used but never defined
drivers/power/supply/power_supply_core.c:1086:12: warning: 'psy_register_cooler' defined but not used [-Wunused-function]
1086 | static int psy_register_cooler(struct power_supply *psy)
| ^~~~~~~~~~~~~~~~~~~
vim +/psy_register_cooler +129 drivers/power/supply/power_supply_core.c
128
> 129 static int psy_register_cooler(struct device *dev, struct power_supply *psy);
130 /*
131 * Notify that power supply was registered after parent finished the probing.
132 *
133 * Often power supply is registered from driver's probe function. However
134 * calling power_supply_changed() directly from power_supply_register()
135 * would lead to execution of get_property() function provided by the driver
136 * too early - before the probe ends.
137 * Also, registering cooling device from the probe will execute the
138 * get_property() function. So register the cooling device after the probe.
139 *
140 * Avoid that by waiting on parent's mutex.
141 */
142 static void power_supply_deferred_register_work(struct work_struct *work)
143 {
144 struct power_supply *psy = container_of(work, struct power_supply,
145 deferred_register_work.work);
146
147 if (psy->dev.parent) {
148 while (!mutex_trylock(&psy->dev.parent->mutex)) {
149 if (psy->removing)
150 return;
151 msleep(10);
152 }
153 }
154
155 power_supply_changed(psy);
156 psy_register_cooler(psy->dev.parent, psy);
157
158 if (psy->dev.parent)
159 mutex_unlock(&psy->dev.parent->mutex);
160 }
161
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi Manaf,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on sre-power-supply/for-next]
[also build test ERROR on v5.16-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Manaf-Meethalavalappu-Pallikunhi/power_supply-Register-cooling-device-outside-of-probe/20211213-191238
base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: openrisc-randconfig-r033-20211213 (https://download.01.org/0day-ci/archive/20211214/[email protected]/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/e5930a7ed9ae1b121e0dde177184ff74abbf0371
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Manaf-Meethalavalappu-Pallikunhi/power_supply-Register-cooling-device-outside-of-probe/20211213-191238
git checkout e5930a7ed9ae1b121e0dde177184ff74abbf0371
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=openrisc SHELL=/bin/bash drivers/power/supply/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All error/warnings (new ones prefixed by >>):
>> drivers/power/supply/power_supply_core.c:1086:12: error: conflicting types for 'psy_register_cooler'; have 'int(struct power_supply *)'
1086 | static int psy_register_cooler(struct power_supply *psy)
| ^~~~~~~~~~~~~~~~~~~
drivers/power/supply/power_supply_core.c:129:12: note: previous declaration of 'psy_register_cooler' with type 'int(struct device *, struct power_supply *)'
129 | static int psy_register_cooler(struct device *dev, struct power_supply *psy);
| ^~~~~~~~~~~~~~~~~~~
>> drivers/power/supply/power_supply_core.c:129:12: warning: 'psy_register_cooler' used but never defined
drivers/power/supply/power_supply_core.c:1086:12: warning: 'psy_register_cooler' defined but not used [-Wunused-function]
1086 | static int psy_register_cooler(struct power_supply *psy)
| ^~~~~~~~~~~~~~~~~~~
vim +1086 drivers/power/supply/power_supply_core.c
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 1085
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 @1086 static int psy_register_cooler(struct power_supply *psy)
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 1087 {
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 1088 /* Register for cooling device if psy can control charging */
9ba533eb99bb2a drivers/power/supply/power_supply_core.c Matthias Kaehlcke 2021-09-01 1089 if (psy_has_property(psy->desc, POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT)) {
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 1090 psy->tcd = thermal_cooling_device_register(
297d716f6260cc drivers/power/power_supply_core.c Krzysztof Kozlowski 2015-03-12 1091 (char *)psy->desc->name,
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 1092 psy, &psy_tcd_ops);
9d2410c79b5b2d drivers/power/power_supply_core.c Viresh Kumar 2014-09-04 1093 return PTR_ERR_OR_ZERO(psy->tcd);
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 1094 }
9ba533eb99bb2a drivers/power/supply/power_supply_core.c Matthias Kaehlcke 2021-09-01 1095
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 1096 return 0;
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 1097 }
952aeeb3ee28bc drivers/power/power_supply_core.c Ramakrishna Pallala 2012-10-09 1098
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]