| Message ID | 1639393841-17444-1-git-send-email-quic_manafm@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | power_supply: Register cooling device outside of probe | expand |
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/202112132307.mubGFyNy-lkp@intel.com/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 <lkp@intel.com>
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/kbuild-all@lists.01.org
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);
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 <quic_manafm@quicinc.com> --- drivers/power/supply/power_supply_core.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)