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(-)