[OpenWrt-Devel,1/9] hwmon-coretemp: add thermal monitor for Core/Core2/Atom

Message ID 1508703687-25650-2-git-send-email-wigyori@uid0.hu
State New
Headers show
Series
  • merge: add missing devices / commits into LEDE
Related show

Commit Message

Zoltan HERPAI Oct. 22, 2017, 8:21 p.m.
From: Philip Prindeville <philipp@redfish-solutions.com>

Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com>
---
 package/kernel/linux/modules/hwmon.mk | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Yousong Zhou Oct. 23, 2017, 3:50 a.m. | #1
On 23 October 2017 at 04:21, Zoltan HERPAI <wigyori@uid0.hu> wrote:
> From: Philip Prindeville <philipp@redfish-solutions.com>
>
> Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com>
> ---
>  package/kernel/linux/modules/hwmon.mk | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/package/kernel/linux/modules/hwmon.mk b/package/kernel/linux/modules/hwmon.mk
> index ed05cae..ae1a004 100644
> --- a/package/kernel/linux/modules/hwmon.mk
> +++ b/package/kernel/linux/modules/hwmon.mk
> @@ -108,6 +108,21 @@ endef
>  $(eval $(call KernelPackage,hwmon-nct6775))
>
>
> +define KernelPackage/hwmon-coretemp
> +  TITLE:=Intel Core/Core2/Atom thermal monitoring support
> +  KCONFIG:=CONFIG_SENSORS_CORETEMP
> +  FILES:=$(LINUX_DIR)/drivers/hwmon/coretemp.ko
> +  AUTOLOAD:=$(call AutoProbe,coretemp)
> +  $(call AddDepends/hwmon,@TARGET_x86)
> +endef
> +
> +define KernelPackage/hwmon-coretemp/description
> + Kernel module for Intel Core/Core2/Atom thermal monitor chip
> +endef
> +
> +$(eval $(call KernelPackage,hwmon-coretemp))
> +
> +

This module is already builtin for x86/64 subtarget.  And since it's a
target-specific module, maybe we should move this to x86/modules.mk

                yousong

>  define KernelPackage/hwmon-ina2xx
>    TITLE:=INA2XX monitoring support
>    KCONFIG:=CONFIG_SENSORS_INA2XX
> --
> 1.9.1
>
>
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
Felix Fietkau Oct. 23, 2017, 5:57 a.m. | #2
On 2017-10-23 05:50, Yousong Zhou wrote:
> On 23 October 2017 at 04:21, Zoltan HERPAI <wigyori@uid0.hu> wrote:
>> From: Philip Prindeville <philipp@redfish-solutions.com>
>>
>> Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com>
>> ---
>>  package/kernel/linux/modules/hwmon.mk | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/package/kernel/linux/modules/hwmon.mk b/package/kernel/linux/modules/hwmon.mk
>> index ed05cae..ae1a004 100644
>> --- a/package/kernel/linux/modules/hwmon.mk
>> +++ b/package/kernel/linux/modules/hwmon.mk
>> @@ -108,6 +108,21 @@ endef
>>  $(eval $(call KernelPackage,hwmon-nct6775))
>>
>>
>> +define KernelPackage/hwmon-coretemp
>> +  TITLE:=Intel Core/Core2/Atom thermal monitoring support
>> +  KCONFIG:=CONFIG_SENSORS_CORETEMP
>> +  FILES:=$(LINUX_DIR)/drivers/hwmon/coretemp.ko
>> +  AUTOLOAD:=$(call AutoProbe,coretemp)
>> +  $(call AddDepends/hwmon,@TARGET_x86)
>> +endef
>> +
>> +define KernelPackage/hwmon-coretemp/description
>> + Kernel module for Intel Core/Core2/Atom thermal monitor chip
>> +endef
>> +
>> +$(eval $(call KernelPackage,hwmon-coretemp))
>> +
>> +
> 
> This module is already builtin for x86/64 subtarget.  And since it's a
> target-specific module, maybe we should move this to x86/modules.mk
I don't think we should have this as a module at all. If this is needed
for x86/generic as well, simply enable it in the kernel config there.

- Felix
Philip Prindeville Oct. 31, 2017, 5:46 p.m. | #3
> On Oct 22, 2017, at 11:57 PM, Felix Fietkau <nbd@nbd.name> wrote:
> 
> On 2017-10-23 05:50, Yousong Zhou wrote:
>> On 23 October 2017 at 04:21, Zoltan HERPAI <wigyori@uid0.hu> wrote:
>>> From: Philip Prindeville <philipp@redfish-solutions.com>
>>> 
>>> Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com>
>>> ---
>>> package/kernel/linux/modules/hwmon.mk | 15 +++++++++++++++
>>> 1 file changed, 15 insertions(+)
>>> 
>>> diff --git a/package/kernel/linux/modules/hwmon.mk b/package/kernel/linux/modules/hwmon.mk
>>> index ed05cae..ae1a004 100644
>>> --- a/package/kernel/linux/modules/hwmon.mk
>>> +++ b/package/kernel/linux/modules/hwmon.mk
>>> @@ -108,6 +108,21 @@ endef
>>> $(eval $(call KernelPackage,hwmon-nct6775))
>>> 
>>> 
>>> +define KernelPackage/hwmon-coretemp
>>> +  TITLE:=Intel Core/Core2/Atom thermal monitoring support
>>> +  KCONFIG:=CONFIG_SENSORS_CORETEMP
>>> +  FILES:=$(LINUX_DIR)/drivers/hwmon/coretemp.ko
>>> +  AUTOLOAD:=$(call AutoProbe,coretemp)
>>> +  $(call AddDepends/hwmon,@TARGET_x86)
>>> +endef
>>> +
>>> +define KernelPackage/hwmon-coretemp/description
>>> + Kernel module for Intel Core/Core2/Atom thermal monitor chip
>>> +endef
>>> +
>>> +$(eval $(call KernelPackage,hwmon-coretemp))
>>> +
>>> +
>> 
>> This module is already builtin for x86/64 subtarget.  And since it's a
>> target-specific module, maybe we should move this to x86/modules.mk
> I don't think we should have this as a module at all. If this is needed
> for x86/generic as well, simply enable it in the kernel config there.
> 
> - Felix


And indeed, that’s why I closed PR #1470 and opened #1471 instead:

https://github.com/lede-project/source/pull/1471

if we’re all agreed this is the way to go, can someone please merge it?

Thanks,

-Philip

Patch

diff --git a/package/kernel/linux/modules/hwmon.mk b/package/kernel/linux/modules/hwmon.mk
index ed05cae..ae1a004 100644
--- a/package/kernel/linux/modules/hwmon.mk
+++ b/package/kernel/linux/modules/hwmon.mk
@@ -108,6 +108,21 @@  endef
 $(eval $(call KernelPackage,hwmon-nct6775))
 
 
+define KernelPackage/hwmon-coretemp
+  TITLE:=Intel Core/Core2/Atom thermal monitoring support
+  KCONFIG:=CONFIG_SENSORS_CORETEMP
+  FILES:=$(LINUX_DIR)/drivers/hwmon/coretemp.ko
+  AUTOLOAD:=$(call AutoProbe,coretemp)
+  $(call AddDepends/hwmon,@TARGET_x86)
+endef
+
+define KernelPackage/hwmon-coretemp/description
+ Kernel module for Intel Core/Core2/Atom thermal monitor chip
+endef
+
+$(eval $(call KernelPackage,hwmon-coretemp))
+
+
 define KernelPackage/hwmon-ina2xx
   TITLE:=INA2XX monitoring support
   KCONFIG:=CONFIG_SENSORS_INA2XX