| Message ID | 20211220215828.189814-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | thermal: Kconfig: simplify dependency of THERMAL_HWMON | expand |
Hi "Uwe,
I love your patch! Yet something to improve:
[auto build test ERROR on rafael-pm/thermal]
[also build test ERROR on v5.16-rc6 next-20211220]
[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/Uwe-Kleine-K-nig/thermal-Kconfig-simplify-dependency-of-THERMAL_HWMON/20211221-060020
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal
config: openrisc-randconfig-r011-20211220 (https://download.01.org/0day-ci/archive/20211221/202112211604.Vb1MeFLx-lkp@intel.com/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/440241d26b4fa7c82b2bce16874fbc3e4c26edba
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Uwe-Kleine-K-nig/thermal-Kconfig-simplify-dependency-of-THERMAL_HWMON/20211221-060020
git checkout 440241d26b4fa7c82b2bce16874fbc3e4c26edba
# 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
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
or1k-linux-ld: drivers/thermal/thermal_hwmon.o: in function `thermal_add_hwmon_sysfs':
>> (.text+0x490): undefined reference to `hwmon_device_register_with_info'
(.text+0x490): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `hwmon_device_register_with_info'
>> or1k-linux-ld: (.text+0x508): undefined reference to `hwmon_device_unregister'
(.text+0x508): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `hwmon_device_unregister'
or1k-linux-ld: drivers/thermal/thermal_hwmon.o: in function `thermal_remove_hwmon_sysfs':
>> (.text+0x814): undefined reference to `hwmon_device_unregister'
(.text+0x814): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `hwmon_device_unregister'
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi "Uwe,
I love your patch! Yet something to improve:
[auto build test ERROR on rafael-pm/thermal]
[also build test ERROR on v5.16-rc6 next-20211220]
[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/Uwe-Kleine-K-nig/thermal-Kconfig-simplify-dependency-of-THERMAL_HWMON/20211221-060020
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal
config: h8300-buildonly-randconfig-r003-20211221 (https://download.01.org/0day-ci/archive/20211221/202112211653.bGCOVr1T-lkp@intel.com/config)
compiler: h8300-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/440241d26b4fa7c82b2bce16874fbc3e4c26edba
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Uwe-Kleine-K-nig/thermal-Kconfig-simplify-dependency-of-THERMAL_HWMON/20211221-060020
git checkout 440241d26b4fa7c82b2bce16874fbc3e4c26edba
# 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=h8300 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
h8300-linux-ld: drivers/thermal/thermal_hwmon.o: in function `thermal_add_hwmon_sysfs':
>> thermal_hwmon.c:(.text+0x1df): undefined reference to `hwmon_device_register_with_info'
>> h8300-linux-ld: thermal_hwmon.c:(.text+0x3a5): undefined reference to `hwmon_device_unregister'
h8300-linux-ld: drivers/thermal/thermal_hwmon.o: in function `.L52':
>> thermal_hwmon.c:(.text+0x6c1): undefined reference to `hwmon_device_unregister'
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Thu, Dec 30, 2021 at 04:37:20PM +0100, Rafael J. Wysocki wrote: > On Mon, Dec 20, 2021 at 10:58 PM Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > > > THERMAL is bool (since commit 554b3529fe01 ("thermal/drivers/core: > > Remove the module Kconfig's option") in v5.2-rc1) and so cannot be > > configured as a module. As THERMAL_HWMON also depends on THERMAL (via a > > big if block in drivers/thermal/Kconfig) the condition > > > > depends on HWMON=y || HWMON=THERMAL > > > > is equivalent to the simpler > > > > depends on HWMON=y > > > > . Use the latter. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > The change looks reasonable to me, but 0-day is evidently unhappy with > it. Can you have a look at its replies to this message, please? 0-day isn't happy because I only wrote HWMON=y in the commit log, but not in the Kconfig. The bot should be improved to better understand the intention of a patch :-) > > --- > > Hello, > > > > I want to use this patch as an opportunity to question if THERMAL being > > a bool and not a tristate is still considered the right thing to do. > > IIRC there are correctness concerns regarding modular THERMAL. In the commit log of 554b3529fe018 this sounds more like modular being a burden to maintain because most systems use =y anyhow. > > This dependency attracted attention in the context of the armel Debian > > kernel which has HWMON=m for binary size reason. When THERMAL was > > changed to bool this resulted in THERMAL_HWMON not being available any > > more. So on these systems THERMAL_HWMON is forced to be off, which I guess is still less correct than THERMAL=m?! Best regards Uwe
On Thu, Dec 30, 2021 at 8:04 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > On Thu, Dec 30, 2021 at 04:37:20PM +0100, Rafael J. Wysocki wrote: > > On Mon, Dec 20, 2021 at 10:58 PM Uwe Kleine-König > > <u.kleine-koenig@pengutronix.de> wrote: > > > > > > THERMAL is bool (since commit 554b3529fe01 ("thermal/drivers/core: > > > Remove the module Kconfig's option") in v5.2-rc1) and so cannot be > > > configured as a module. As THERMAL_HWMON also depends on THERMAL (via a > > > big if block in drivers/thermal/Kconfig) the condition > > > > > > depends on HWMON=y || HWMON=THERMAL > > > > > > is equivalent to the simpler > > > > > > depends on HWMON=y > > > > > > . Use the latter. > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > The change looks reasonable to me, but 0-day is evidently unhappy with > > it. Can you have a look at its replies to this message, please? > > 0-day isn't happy because I only wrote HWMON=y in the commit log, but > not in the Kconfig. The bot should be improved to better understand > the intention of a patch :-) > > > > --- > > > Hello, > > > > > > I want to use this patch as an opportunity to question if THERMAL being > > > a bool and not a tristate is still considered the right thing to do. > > > > IIRC there are correctness concerns regarding modular THERMAL. > > In the commit log of 554b3529fe018 this sounds more like modular being a > burden to maintain because most systems use =y anyhow. > > > > This dependency attracted attention in the context of the armel Debian > > > kernel which has HWMON=m for binary size reason. When THERMAL was > > > changed to bool this resulted in THERMAL_HWMON not being available any > > > more. > > So on these systems THERMAL_HWMON is forced to be off, which I guess is > still less correct than THERMAL=m?! If the problem ISTR is real, it is, because unloading thermal could cause a use-after-free to occur in some cases. That said, it might be better to address this particular problem instead of making the whole thing non-modular.
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index d7f44deab5b1..726bf396c8a9 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -52,7 +52,7 @@ config THERMAL_EMERGENCY_POWEROFF_DELAY_MS config THERMAL_HWMON bool prompt "Expose thermal sensors as hwmon device" - depends on HWMON=y || HWMON=THERMAL + depends on HWMON default y help In case a sensor is registered with the thermal
THERMAL is bool (since commit 554b3529fe01 ("thermal/drivers/core: Remove the module Kconfig's option") in v5.2-rc1) and so cannot be configured as a module. As THERMAL_HWMON also depends on THERMAL (via a big if block in drivers/thermal/Kconfig) the condition depends on HWMON=y || HWMON=THERMAL is equivalent to the simpler depends on HWMON=y . Use the latter. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Hello, I want to use this patch as an opportunity to question if THERMAL being a bool and not a tristate is still considered the right thing to do. This dependency attracted attention in the context of the armel Debian kernel which has HWMON=m for binary size reason. When THERMAL was changed to bool this resulted in THERMAL_HWMON not being available any more. Best regards Uwe drivers/thermal/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)