[PATCH] thermal: Kconfig: simplify dependency of THERMAL_HWMON

[PATCH] thermal: Kconfig: simplify dependency of THERMAL_HWMON

[Uwe Kleine-König]

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

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
-- 
2.33.0

Re: [PATCH] thermal: Kconfig: simplify dependency of THERMAL_HWMON

[kernel test robot]

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

Re: [PATCH] thermal: Kconfig: simplify dependency of THERMAL_HWMON

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2517 bytes --]

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(a)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(a)lists.01.org

Re: [PATCH] thermal: Kconfig: simplify dependency of THERMAL_HWMON

[kernel test robot]

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

Re: [PATCH] thermal: Kconfig: simplify dependency of THERMAL_HWMON

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2189 bytes --]

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(a)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(a)lists.01.org

Re: [PATCH] thermal: Kconfig: simplify dependency of THERMAL_HWMON

[Rafael J. Wysocki]

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?

> ---
> 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.

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

Re: [PATCH] thermal: Kconfig: simplify dependency of THERMAL_HWMON

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] thermal: Kconfig: simplify dependency of THERMAL_HWMON

[Rafael J. Wysocki]

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.