Message ID | 20200626173755.26379-7-andrzej.p@collabora.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v5,01/11] acpi: thermal: Fix error handling in the register function | expand |
Hi Andrzej,
I love your patch! Perhaps something to improve:
[auto build test WARNING on 48778464bb7d346b47157d21ffde2af6b2d39110]
url: https://github.com/0day-ci/linux/commits/Andrzej-Pietrasiewicz/Stop-monitoring-disabled-devices/20200627-013944
base: 48778464bb7d346b47157d21ffde2af6b2d39110
config: x86_64-randconfig-s022-20200624 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-dirty
# save the attached .config to linux build tree
make W=1 C=1 ARCH=x86_64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/thermal/thermal_core.c:462:5: sparse: sparse: symbol 'thermal_zone_device_set_mode' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Andrzej, I love your patch! Perhaps something to improve: [auto build test WARNING on 48778464bb7d346b47157d21ffde2af6b2d39110] url: https://github.com/0day-ci/linux/commits/Andrzej-Pietrasiewicz/Stop-monitoring-disabled-devices/20200627-013944 base: 48778464bb7d346b47157d21ffde2af6b2d39110 config: arm-imx_v6_v7_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.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 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 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/thermal/thermal_core.c:462:5: warning: no previous prototype for 'thermal_zone_device_set_mode' [-Wmissing-prototypes] 462 | int thermal_zone_device_set_mode(struct thermal_zone_device *tz, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/thermal_zone_device_set_mode +462 drivers/thermal/thermal_core.c 461 > 462 int thermal_zone_device_set_mode(struct thermal_zone_device *tz, 463 enum thermal_device_mode mode) 464 { 465 int ret = 0; 466 467 mutex_lock(&tz->lock); 468 469 /* do nothing if mode isn't changing */ 470 if (mode == tz->mode) { 471 mutex_unlock(&tz->lock); 472 473 return ret; 474 } 475 476 if (tz->ops->set_mode) 477 ret = tz->ops->set_mode(tz, mode); 478 479 if (!ret) 480 tz->mode = mode; 481 482 mutex_unlock(&tz->lock); 483 484 thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); 485 486 return ret; 487 } 488 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Fri, Jun 26, 2020 at 11:08 PM Andrzej Pietrasiewicz <andrzej.p@collabora.com> wrote: > > Prepare for making the drivers not access tzd's private members. > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > [EXPORT_SYMBOL -> EXPORT_SYMBOL_GPL] > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > --- > drivers/thermal/thermal_core.c | 53 ++++++++++++++++++++++++++++++++++ > include/linux/thermal.h | 13 +++++++++ > 2 files changed, 66 insertions(+) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index 14d3b1b94c4f..3181295075b9 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -459,6 +459,59 @@ static void thermal_zone_device_reset(struct thermal_zone_device *tz) > thermal_zone_device_init(tz); > } > > +int thermal_zone_device_set_mode(struct thermal_zone_device *tz, > + enum thermal_device_mode mode) Should this be static? > +{ > + int ret = 0; > + > + mutex_lock(&tz->lock); > + > + /* do nothing if mode isn't changing */ > + if (mode == tz->mode) { > + mutex_unlock(&tz->lock); > + > + return ret; > + } > + > + if (tz->ops->set_mode) > + ret = tz->ops->set_mode(tz, mode); > + > + if (!ret) > + tz->mode = mode; > + > + mutex_unlock(&tz->lock); > + > + thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); > + > + return ret; > +} > + > +int thermal_zone_device_enable(struct thermal_zone_device *tz) > +{ > + return thermal_zone_device_set_mode(tz, THERMAL_DEVICE_ENABLED); > +} > +EXPORT_SYMBOL_GPL(thermal_zone_device_enable); > + > +int thermal_zone_device_disable(struct thermal_zone_device *tz) > +{ > + return thermal_zone_device_set_mode(tz, THERMAL_DEVICE_DISABLED); > +} > +EXPORT_SYMBOL_GPL(thermal_zone_device_disable); > + > +int thermal_zone_device_is_enabled(struct thermal_zone_device *tz) > +{ > + enum thermal_device_mode mode; > + > + mutex_lock(&tz->lock); > + > + mode = tz->mode; > + > + mutex_unlock(&tz->lock); > + > + return mode == THERMAL_DEVICE_ENABLED; > +} > +EXPORT_SYMBOL_GPL(thermal_zone_device_is_enabled); > + > void thermal_zone_device_update(struct thermal_zone_device *tz, > enum thermal_notify_event event) > { > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index a808f6fa2777..df013c39ba9b 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -416,6 +416,9 @@ int thermal_zone_get_offset(struct thermal_zone_device *tz); > > void thermal_cdev_update(struct thermal_cooling_device *); > void thermal_notify_framework(struct thermal_zone_device *, int); > +int thermal_zone_device_enable(struct thermal_zone_device *tz); > +int thermal_zone_device_disable(struct thermal_zone_device *tz); > +int thermal_zone_device_is_enabled(struct thermal_zone_device *tz); > #else > static inline struct thermal_zone_device *thermal_zone_device_register( > const char *type, int trips, int mask, void *devdata, > @@ -463,6 +466,16 @@ static inline void thermal_cdev_update(struct thermal_cooling_device *cdev) > static inline void thermal_notify_framework(struct thermal_zone_device *tz, > int trip) > { } > + > +static inline int thermal_zone_device_enable(struct thermal_zone_device *tz) > +{ return -ENODEV; } > + > +static inline int thermal_zone_device_disable(struct thermal_zone_device *tz) > +{ return -ENODEV; } > + > +static inline int > +thermal_zone_device_is_enabled(struct thermal_zone_device *tz) > +{ return -ENODEV; } > #endif /* CONFIG_THERMAL */ > > #endif /* __THERMAL_H__ */ > -- > 2.17.1 >
A respin of v5 with these changes: v5..v6: - staticized thermal_zone_device_set_mode() (kbuild test robot) v4..v5: - EXPORT_SYMBOL -> EXPORT_SYMBOL_GPL (Daniel) - dropped unnecessary thermal_zone_device_enable() in int3400_thermal.c and in thermal_of.c (Bartlomiej) Andrzej Pietrasiewicz (11): acpi: thermal: Fix error handling in the register function thermal: Store thermal mode in a dedicated enum thermal: Add current mode to thermal zone device thermal: Store device mode in struct thermal_zone_device thermal: remove get_mode() operation of drivers thermal: Add mode helpers thermal: Use mode helpers in drivers thermal: Explicitly enable non-changing thermal zone devices thermal: core: Stop polling DISABLED thermal devices thermal: Simplify or eliminate unnecessary set_mode() methods thermal: Rename set_mode() to change_mode() drivers/acpi/thermal.c | 75 +++++---------- .../ethernet/chelsio/cxgb4/cxgb4_thermal.c | 8 ++ .../ethernet/mellanox/mlxsw/core_thermal.c | 91 ++++--------------- drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 9 +- drivers/platform/x86/acerhdf.c | 33 +++---- drivers/platform/x86/intel_mid_thermal.c | 6 ++ drivers/power/supply/power_supply_core.c | 9 +- drivers/thermal/armada_thermal.c | 6 ++ drivers/thermal/da9062-thermal.c | 16 +--- drivers/thermal/dove_thermal.c | 6 ++ drivers/thermal/hisi_thermal.c | 6 +- drivers/thermal/imx_thermal.c | 57 ++++-------- .../intel/int340x_thermal/int3400_thermal.c | 38 ++------ .../int340x_thermal/int340x_thermal_zone.c | 5 + drivers/thermal/intel/intel_pch_thermal.c | 5 + .../thermal/intel/intel_quark_dts_thermal.c | 34 ++----- drivers/thermal/intel/intel_soc_dts_iosf.c | 3 + drivers/thermal/intel/x86_pkg_temp_thermal.c | 6 ++ drivers/thermal/kirkwood_thermal.c | 7 ++ drivers/thermal/rcar_thermal.c | 9 +- drivers/thermal/rockchip_thermal.c | 6 +- drivers/thermal/spear_thermal.c | 7 ++ drivers/thermal/sprd_thermal.c | 6 +- drivers/thermal/st/st_thermal.c | 5 + drivers/thermal/thermal_core.c | 76 ++++++++++++++-- drivers/thermal/thermal_of.c | 41 +-------- drivers/thermal/thermal_sysfs.c | 37 +------- include/linux/thermal.h | 19 +++- 28 files changed, 275 insertions(+), 351 deletions(-) base-commit: 9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 14d3b1b94c4f..3181295075b9 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -459,6 +459,59 @@ static void thermal_zone_device_reset(struct thermal_zone_device *tz) thermal_zone_device_init(tz); } +int thermal_zone_device_set_mode(struct thermal_zone_device *tz, + enum thermal_device_mode mode) +{ + int ret = 0; + + mutex_lock(&tz->lock); + + /* do nothing if mode isn't changing */ + if (mode == tz->mode) { + mutex_unlock(&tz->lock); + + return ret; + } + + if (tz->ops->set_mode) + ret = tz->ops->set_mode(tz, mode); + + if (!ret) + tz->mode = mode; + + mutex_unlock(&tz->lock); + + thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); + + return ret; +} + +int thermal_zone_device_enable(struct thermal_zone_device *tz) +{ + return thermal_zone_device_set_mode(tz, THERMAL_DEVICE_ENABLED); +} +EXPORT_SYMBOL_GPL(thermal_zone_device_enable); + +int thermal_zone_device_disable(struct thermal_zone_device *tz) +{ + return thermal_zone_device_set_mode(tz, THERMAL_DEVICE_DISABLED); +} +EXPORT_SYMBOL_GPL(thermal_zone_device_disable); + +int thermal_zone_device_is_enabled(struct thermal_zone_device *tz) +{ + enum thermal_device_mode mode; + + mutex_lock(&tz->lock); + + mode = tz->mode; + + mutex_unlock(&tz->lock); + + return mode == THERMAL_DEVICE_ENABLED; +} +EXPORT_SYMBOL_GPL(thermal_zone_device_is_enabled); + void thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event) { diff --git a/include/linux/thermal.h b/include/linux/thermal.h index a808f6fa2777..df013c39ba9b 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -416,6 +416,9 @@ int thermal_zone_get_offset(struct thermal_zone_device *tz); void thermal_cdev_update(struct thermal_cooling_device *); void thermal_notify_framework(struct thermal_zone_device *, int); +int thermal_zone_device_enable(struct thermal_zone_device *tz); +int thermal_zone_device_disable(struct thermal_zone_device *tz); +int thermal_zone_device_is_enabled(struct thermal_zone_device *tz); #else static inline struct thermal_zone_device *thermal_zone_device_register( const char *type, int trips, int mask, void *devdata, @@ -463,6 +466,16 @@ static inline void thermal_cdev_update(struct thermal_cooling_device *cdev) static inline void thermal_notify_framework(struct thermal_zone_device *tz, int trip) { } + +static inline int thermal_zone_device_enable(struct thermal_zone_device *tz) +{ return -ENODEV; } + +static inline int thermal_zone_device_disable(struct thermal_zone_device *tz) +{ return -ENODEV; } + +static inline int +thermal_zone_device_is_enabled(struct thermal_zone_device *tz) +{ return -ENODEV; } #endif /* CONFIG_THERMAL */ #endif /* __THERMAL_H__ */