Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-24 Thread Jae Hyun Yoo 

Hi Andy,

Thanks a lot for your review. Please check my inline answers.

On 4/24/2018 8:56 AM, Andy Shevchenko wrote:

On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote:


  drivers/hwmon/peci-cputemp.c  | 783
++
  drivers/hwmon/peci-dimmtemp.c | 432 +++


Does it make sense one driver per patch?



Yes, I'll separate it into two patches.


+#define CLIENT_CPU_ID_MASK0xf0ff0  /* Mask for Family / Model
info */



+struct cpu_gen_info {
+   u32 type;
+   u32 cpu_id;
+   u32 core_max;
+};




+static const struct cpu_gen_info cpu_gen_info_table[] = {
+   { .type = CPU_GEN_HSX,
+ .cpu_id = 0x306f0, /* Family code: 6, Model number: 63
(0x3f) */
+ .core_max = CORE_MAX_ON_HSX },
+   { .type = CPU_GEN_BRX,
+ .cpu_id = 0x406f0, /* Family code: 6, Model number: 79
(0x4f) */
+ .core_max = CORE_MAX_ON_BDX },
+   { .type = CPU_GEN_SKX,
+ .cpu_id = 0x50650, /* Family code: 6, Model number: 85
(0x55) */
+ .core_max = CORE_MAX_ON_SKX },
+};


Are we talking about x86 CPU IDs here?
If so, why x86 corresponding headers, including intel-family.h are not
used?



Yes, that would make more sense. I'll include the intel-family.h and 
will use these defines instead:

INTEL_FAM6_HASWELL_X
INTEL_FAM6_BROADWELL_X
INTEL_FAM6_SKYLAKE_X

Thanks,

Jae

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-24 Thread Jae Hyun Yoo 

Hi Andy,

Thanks a lot for your review. Please check my inline answers.

On 4/24/2018 8:56 AM, Andy Shevchenko wrote:

On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote:


  drivers/hwmon/peci-cputemp.c  | 783
++
  drivers/hwmon/peci-dimmtemp.c | 432 +++


Does it make sense one driver per patch?



Yes, I'll separate it into two patches.


+#define CLIENT_CPU_ID_MASK0xf0ff0  /* Mask for Family / Model
info */



+struct cpu_gen_info {
+   u32 type;
+   u32 cpu_id;
+   u32 core_max;
+};




+static const struct cpu_gen_info cpu_gen_info_table[] = {
+   { .type = CPU_GEN_HSX,
+ .cpu_id = 0x306f0, /* Family code: 6, Model number: 63
(0x3f) */
+ .core_max = CORE_MAX_ON_HSX },
+   { .type = CPU_GEN_BRX,
+ .cpu_id = 0x406f0, /* Family code: 6, Model number: 79
(0x4f) */
+ .core_max = CORE_MAX_ON_BDX },
+   { .type = CPU_GEN_SKX,
+ .cpu_id = 0x50650, /* Family code: 6, Model number: 85
(0x55) */
+ .core_max = CORE_MAX_ON_SKX },
+};


Are we talking about x86 CPU IDs here?
If so, why x86 corresponding headers, including intel-family.h are not
used?



Yes, that would make more sense. I'll include the intel-family.h and 
will use these defines instead:

INTEL_FAM6_HASWELL_X
INTEL_FAM6_BROADWELL_X
INTEL_FAM6_SKYLAKE_X

Thanks,

Jae

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-24 Thread Andy Shevchenko 
On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote:

>  drivers/hwmon/peci-cputemp.c  | 783
> ++
>  drivers/hwmon/peci-dimmtemp.c | 432 +++

Does it make sense one driver per patch?

> +#define CLIENT_CPU_ID_MASK0xf0ff0  /* Mask for Family / Model
> info */

> +struct cpu_gen_info {
> + u32 type;
> + u32 cpu_id;
> + u32 core_max;
> +};
> 

> +static const struct cpu_gen_info cpu_gen_info_table[] = {
> + { .type = CPU_GEN_HSX,
> +   .cpu_id = 0x306f0, /* Family code: 6, Model number: 63
> (0x3f) */
> +   .core_max = CORE_MAX_ON_HSX },
> + { .type = CPU_GEN_BRX,
> +   .cpu_id = 0x406f0, /* Family code: 6, Model number: 79
> (0x4f) */
> +   .core_max = CORE_MAX_ON_BDX },
> + { .type = CPU_GEN_SKX,
> +   .cpu_id = 0x50650, /* Family code: 6, Model number: 85
> (0x55) */
> +   .core_max = CORE_MAX_ON_SKX },
> +};

Are we talking about x86 CPU IDs here?
If so, why x86 corresponding headers, including intel-family.h are not
used?

-- 
Andy Shevchenko 
Intel Finland Oy

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-24 Thread Andy Shevchenko 
On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote:

>  drivers/hwmon/peci-cputemp.c  | 783
> ++
>  drivers/hwmon/peci-dimmtemp.c | 432 +++

Does it make sense one driver per patch?

> +#define CLIENT_CPU_ID_MASK0xf0ff0  /* Mask for Family / Model
> info */

> +struct cpu_gen_info {
> + u32 type;
> + u32 cpu_id;
> + u32 core_max;
> +};
> 

> +static const struct cpu_gen_info cpu_gen_info_table[] = {
> + { .type = CPU_GEN_HSX,
> +   .cpu_id = 0x306f0, /* Family code: 6, Model number: 63
> (0x3f) */
> +   .core_max = CORE_MAX_ON_HSX },
> + { .type = CPU_GEN_BRX,
> +   .cpu_id = 0x406f0, /* Family code: 6, Model number: 79
> (0x4f) */
> +   .core_max = CORE_MAX_ON_BDX },
> + { .type = CPU_GEN_SKX,
> +   .cpu_id = 0x50650, /* Family code: 6, Model number: 85
> (0x55) */
> +   .core_max = CORE_MAX_ON_SKX },
> +};

Are we talking about x86 CPU IDs here?
If so, why x86 corresponding headers, including intel-family.h are not
used?

-- 
Andy Shevchenko 
Intel Finland Oy

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-12 Thread Jae Hyun Yoo 

On 4/12/2018 10:37 AM, Guenter Roeck wrote:

On Thu, Apr 12, 2018 at 10:09:51AM -0700, Jae Hyun Yoo wrote:
[ ... ]

+static int find_core_index(struct peci_cputemp *priv, int channel)
+{
+    int core_channel = channel - DEFAULT_CHANNEL_NUMS;
+    int idx, found = 0;
+
+    for (idx = 0; idx < priv->gen_info->core_max; idx++) {
+    if (priv->core_mask & BIT(idx)) {
+    if (core_channel == found)
+    break;
+
+    found++;
+    }
+    }
+
+    return idx;


What if nothing is found ?



Core temperature group will be registered only when it detects at
least one core checked by check_resolved_cores(), so
find_core_index() can be called only when priv->core_mask has a
non-zero value. The 'nothing is found' case will not happen.


That doesn't guarantee a match. If what you are saying is correct
there should always be
a well defined match of channel -> idx, and the search should be
unnecessary.



There could be some disabled cores in the resolved core mask bit
sequence also it should remove indexing gap in channel numbering so it
is the reason why this search function is needed. Well defined match of
channel -> idx would not be always satisfied.


Are you saying that each call to the function, with the same parameters,
can return a different result ?



No, the result will be consistent. After reading the priv->core_mask once in
check_resolved_cores(), the value will not be changed. I'm saying about this
case, for example if core number 2 is unresolved in total 4 cores, then the
idx order will be '0, 1, 3' but channel order will be '5, 6, 7' without
making any indexing gap.



And you yet you claim that this is not well defined ? Or are you concerned
about the amount of memory consumed by providing an array for the mapping ?

Note that an indexing gap is acceptable and, in many cases, preferred.



If the indexing gap is acceptable, the index search function isn't 
needed anymore. I'll fix all relating code to make that use direct 
mapping of channel -> idx then. Thanks!



[ ... ]


+
+    dev_dbg(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev),
priv->name);
+


Why does this message display the device name twice ?



For an example, dev_name(hwmon_dev) shows 'hwmon5' and priv->name shows
'peci-cputemp0'.


And dev_dbg() shows another device name. So you'll have something like

peci-cputemp0: hwmon5: sensor 'peci-cputemp0'



Practically it shows like

peci-cputemp 0-30:00: hwmon10: sensor 'peci_cputemp.cpu0'

where 0-30:00 is assigned by peci core.



And what message would you see for cpu1 ?



It shows like

peci-cputemp 0-31:00: hwmon10: sensor 'peci_cputemp.cpu1'

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-12 Thread Jae Hyun Yoo 

On 4/12/2018 10:37 AM, Guenter Roeck wrote:

On Thu, Apr 12, 2018 at 10:09:51AM -0700, Jae Hyun Yoo wrote:
[ ... ]

+static int find_core_index(struct peci_cputemp *priv, int channel)
+{
+    int core_channel = channel - DEFAULT_CHANNEL_NUMS;
+    int idx, found = 0;
+
+    for (idx = 0; idx < priv->gen_info->core_max; idx++) {
+    if (priv->core_mask & BIT(idx)) {
+    if (core_channel == found)
+    break;
+
+    found++;
+    }
+    }
+
+    return idx;


What if nothing is found ?



Core temperature group will be registered only when it detects at
least one core checked by check_resolved_cores(), so
find_core_index() can be called only when priv->core_mask has a
non-zero value. The 'nothing is found' case will not happen.


That doesn't guarantee a match. If what you are saying is correct
there should always be
a well defined match of channel -> idx, and the search should be
unnecessary.



There could be some disabled cores in the resolved core mask bit
sequence also it should remove indexing gap in channel numbering so it
is the reason why this search function is needed. Well defined match of
channel -> idx would not be always satisfied.


Are you saying that each call to the function, with the same parameters,
can return a different result ?



No, the result will be consistent. After reading the priv->core_mask once in
check_resolved_cores(), the value will not be changed. I'm saying about this
case, for example if core number 2 is unresolved in total 4 cores, then the
idx order will be '0, 1, 3' but channel order will be '5, 6, 7' without
making any indexing gap.



And you yet you claim that this is not well defined ? Or are you concerned
about the amount of memory consumed by providing an array for the mapping ?

Note that an indexing gap is acceptable and, in many cases, preferred.



If the indexing gap is acceptable, the index search function isn't 
needed anymore. I'll fix all relating code to make that use direct 
mapping of channel -> idx then. Thanks!



[ ... ]


+
+    dev_dbg(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev),
priv->name);
+


Why does this message display the device name twice ?



For an example, dev_name(hwmon_dev) shows 'hwmon5' and priv->name shows
'peci-cputemp0'.


And dev_dbg() shows another device name. So you'll have something like

peci-cputemp0: hwmon5: sensor 'peci-cputemp0'



Practically it shows like

peci-cputemp 0-30:00: hwmon10: sensor 'peci_cputemp.cpu0'

where 0-30:00 is assigned by peci core.



And what message would you see for cpu1 ?



It shows like

peci-cputemp 0-31:00: hwmon10: sensor 'peci_cputemp.cpu1'

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-12 Thread Guenter Roeck 
On Thu, Apr 12, 2018 at 10:09:51AM -0700, Jae Hyun Yoo wrote:
[ ... ]
> >>+static int find_core_index(struct peci_cputemp *priv, int channel)
> >>+{
> >>+    int core_channel = channel - DEFAULT_CHANNEL_NUMS;
> >>+    int idx, found = 0;
> >>+
> >>+    for (idx = 0; idx < priv->gen_info->core_max; idx++) {
> >>+    if (priv->core_mask & BIT(idx)) {
> >>+    if (core_channel == found)
> >>+    break;
> >>+
> >>+    found++;
> >>+    }
> >>+    }
> >>+
> >>+    return idx;
> >
> >What if nothing is found ?
> >
> 
> Core temperature group will be registered only when it detects at
> least one core checked by check_resolved_cores(), so
> find_core_index() can be called only when priv->core_mask has a
> non-zero value. The 'nothing is found' case will not happen.
> 
> >>>That doesn't guarantee a match. If what you are saying is correct
> >>>there should always be
> >>>a well defined match of channel -> idx, and the search should be
> >>>unnecessary.
> >>>
> >>
> >>There could be some disabled cores in the resolved core mask bit
> >>sequence also it should remove indexing gap in channel numbering so it
> >>is the reason why this search function is needed. Well defined match of
> >>channel -> idx would not be always satisfied.
> >>
> >Are you saying that each call to the function, with the same parameters,
> >can return a different result ?
> >
> 
> No, the result will be consistent. After reading the priv->core_mask once in
> check_resolved_cores(), the value will not be changed. I'm saying about this
> case, for example if core number 2 is unresolved in total 4 cores, then the
> idx order will be '0, 1, 3' but channel order will be '5, 6, 7' without
> making any indexing gap.
> 

And you yet you claim that this is not well defined ? Or are you concerned
about the amount of memory consumed by providing an array for the mapping ?

Note that an indexing gap is acceptable and, in many cases, preferred.

[ ... ]

> >>+
> >>+    dev_dbg(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev),
> >>priv->name);
> >>+
> >>>
> >>>Why does this message display the device name twice ?
> >>>
> >>
> >>For an example, dev_name(hwmon_dev) shows 'hwmon5' and priv->name shows
> >>'peci-cputemp0'.
> >>
> >And dev_dbg() shows another device name. So you'll have something like
> >
> >peci-cputemp0: hwmon5: sensor 'peci-cputemp0'
> >
> 
> Practically it shows like
> 
> peci-cputemp 0-30:00: hwmon10: sensor 'peci_cputemp.cpu0'
> 
> where 0-30:00 is assigned by peci core.
> 

And what message would you see for cpu1 ?

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-12 Thread Guenter Roeck 
On Thu, Apr 12, 2018 at 10:09:51AM -0700, Jae Hyun Yoo wrote:
[ ... ]
> >>+static int find_core_index(struct peci_cputemp *priv, int channel)
> >>+{
> >>+    int core_channel = channel - DEFAULT_CHANNEL_NUMS;
> >>+    int idx, found = 0;
> >>+
> >>+    for (idx = 0; idx < priv->gen_info->core_max; idx++) {
> >>+    if (priv->core_mask & BIT(idx)) {
> >>+    if (core_channel == found)
> >>+    break;
> >>+
> >>+    found++;
> >>+    }
> >>+    }
> >>+
> >>+    return idx;
> >
> >What if nothing is found ?
> >
> 
> Core temperature group will be registered only when it detects at
> least one core checked by check_resolved_cores(), so
> find_core_index() can be called only when priv->core_mask has a
> non-zero value. The 'nothing is found' case will not happen.
> 
> >>>That doesn't guarantee a match. If what you are saying is correct
> >>>there should always be
> >>>a well defined match of channel -> idx, and the search should be
> >>>unnecessary.
> >>>
> >>
> >>There could be some disabled cores in the resolved core mask bit
> >>sequence also it should remove indexing gap in channel numbering so it
> >>is the reason why this search function is needed. Well defined match of
> >>channel -> idx would not be always satisfied.
> >>
> >Are you saying that each call to the function, with the same parameters,
> >can return a different result ?
> >
> 
> No, the result will be consistent. After reading the priv->core_mask once in
> check_resolved_cores(), the value will not be changed. I'm saying about this
> case, for example if core number 2 is unresolved in total 4 cores, then the
> idx order will be '0, 1, 3' but channel order will be '5, 6, 7' without
> making any indexing gap.
> 

And you yet you claim that this is not well defined ? Or are you concerned
about the amount of memory consumed by providing an array for the mapping ?

Note that an indexing gap is acceptable and, in many cases, preferred.

[ ... ]

> >>+
> >>+    dev_dbg(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev),
> >>priv->name);
> >>+
> >>>
> >>>Why does this message display the device name twice ?
> >>>
> >>
> >>For an example, dev_name(hwmon_dev) shows 'hwmon5' and priv->name shows
> >>'peci-cputemp0'.
> >>
> >And dev_dbg() shows another device name. So you'll have something like
> >
> >peci-cputemp0: hwmon5: sensor 'peci-cputemp0'
> >
> 
> Practically it shows like
> 
> peci-cputemp 0-30:00: hwmon10: sensor 'peci_cputemp.cpu0'
> 
> where 0-30:00 is assigned by peci core.
> 

And what message would you see for cpu1 ?

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-12 Thread Jae Hyun Yoo 

On 4/11/2018 8:40 PM, Guenter Roeck wrote:

On 04/11/2018 07:51 PM, Jae Hyun Yoo wrote:

On 4/11/2018 5:34 PM, Guenter Roeck wrote:

On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote:

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile    |   2 +
  drivers/hwmon/peci-cputemp.c  | 783 
++

  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
    This driver can also be built as a module.  If so, the module
    will be called nct7904.
+config SENSORS_PECI_CPUTEMP
+    tristate "PECI CPU temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI
+  cputemp driver which provides Digital Thermal Sensor (DTS) 
thermal
+  readings of the CPU package and CPU cores that are 
accessible using

+  the PECI Client Command Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-cputemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+    tristate "PECI DIMM temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel 
PECI hwmon
+  driver which provides Digital Thermal Sensor (DTS) thermal 
readings of
+  DIMM components that are accessible using the PECI Client 
Command

+  Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-dimmtemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-dimmtemp.
+
  config SENSORS_NSA320
  tristate "ZyXEL NSA320 and compatible fan speed and 
temperature sensors"

  depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)    += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904)    += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)    += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)    += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP)    += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)    += peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360)    += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427)    += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591)    += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c 
b/drivers/hwmon/peci-cputemp.c

new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI    6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on 
Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on 
Broadwell */
+#define CORE_MAX_ON_SKX   28 /* Max number of cores on 
Skylake */

+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + 
CORETEMP_CHANNEL_NUMS)

+
+#define CLIENT_CPU_ID_MASK    0xf0ff0 

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-12 Thread Jae Hyun Yoo 

On 4/11/2018 8:40 PM, Guenter Roeck wrote:

On 04/11/2018 07:51 PM, Jae Hyun Yoo wrote:

On 4/11/2018 5:34 PM, Guenter Roeck wrote:

On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote:

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile    |   2 +
  drivers/hwmon/peci-cputemp.c  | 783 
++

  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
    This driver can also be built as a module.  If so, the module
    will be called nct7904.
+config SENSORS_PECI_CPUTEMP
+    tristate "PECI CPU temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI
+  cputemp driver which provides Digital Thermal Sensor (DTS) 
thermal
+  readings of the CPU package and CPU cores that are 
accessible using

+  the PECI Client Command Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-cputemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+    tristate "PECI DIMM temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel 
PECI hwmon
+  driver which provides Digital Thermal Sensor (DTS) thermal 
readings of
+  DIMM components that are accessible using the PECI Client 
Command

+  Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-dimmtemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-dimmtemp.
+
  config SENSORS_NSA320
  tristate "ZyXEL NSA320 and compatible fan speed and 
temperature sensors"

  depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)    += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904)    += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)    += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)    += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP)    += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)    += peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360)    += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427)    += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591)    += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c 
b/drivers/hwmon/peci-cputemp.c

new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI    6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on 
Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on 
Broadwell */
+#define CORE_MAX_ON_SKX   28 /* Max number of cores on 
Skylake */

+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + 
CORETEMP_CHANNEL_NUMS)

+
+#define CLIENT_CPU_ID_MASK    0xf0ff0  /* Mask for Family / Model 
info */

+
+#define UPDATE_INTERVAL_MIN   HZ
+
+enum cpu_gens {
+    CPU_GEN_HSX, /* Haswell Xeon */
+    CPU_GEN_BRX, /* Broadwell Xeon */
+    CPU_GEN_SKX, /* Skylake Xeon */
+    CPU_GEN_MAX
+};
+
+struct cpu_gen_info {
+    u32 type;
+    u32 cpu_id;
+    u32 core_max;
+};
+
+struct temp_data {
+    bool valid;
+    s32  value;
+    unsigned long last_updated;
+};
+
+struct temp_group {
+    struct temp_data die;
+    struct temp_data dts_margin;
+    struct temp_data tcontrol;
+    struct temp_data tthrottle;
+    struct

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-11 Thread Guenter Roeck 

On 04/11/2018 07:51 PM, Jae Hyun Yoo wrote:

On 4/11/2018 5:34 PM, Guenter Roeck wrote:

On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote:

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile    |   2 +
  drivers/hwmon/peci-cputemp.c  | 783 ++
  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
    This driver can also be built as a module.  If so, the module
    will be called nct7904.
+config SENSORS_PECI_CPUTEMP
+    tristate "PECI CPU temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI
+  cputemp driver which provides Digital Thermal Sensor (DTS) thermal
+  readings of the CPU package and CPU cores that are accessible using
+  the PECI Client Command Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-cputemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+    tristate "PECI DIMM temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI hwmon
+  driver which provides Digital Thermal Sensor (DTS) thermal readings of
+  DIMM components that are accessible using the PECI Client Command
+  Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-dimmtemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-dimmtemp.
+
  config SENSORS_NSA320
  tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
  depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)    += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904)    += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)    += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)    += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP)    += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)    += peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360)    += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427)    += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591)    += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c
new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI    6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on Broadwell */
+#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNEL_NUMS)
+
+#define CLIENT_CPU_ID_MASK    0xf0ff0  /* Mask for Family / Model info */
+
+#define

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-11 Thread Guenter Roeck 

On 04/11/2018 07:51 PM, Jae Hyun Yoo wrote:

On 4/11/2018 5:34 PM, Guenter Roeck wrote:

On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote:

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile    |   2 +
  drivers/hwmon/peci-cputemp.c  | 783 ++
  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
    This driver can also be built as a module.  If so, the module
    will be called nct7904.
+config SENSORS_PECI_CPUTEMP
+    tristate "PECI CPU temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI
+  cputemp driver which provides Digital Thermal Sensor (DTS) thermal
+  readings of the CPU package and CPU cores that are accessible using
+  the PECI Client Command Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-cputemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+    tristate "PECI DIMM temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI hwmon
+  driver which provides Digital Thermal Sensor (DTS) thermal readings of
+  DIMM components that are accessible using the PECI Client Command
+  Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-dimmtemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-dimmtemp.
+
  config SENSORS_NSA320
  tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
  depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)    += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904)    += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)    += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)    += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP)    += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)    += peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360)    += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427)    += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591)    += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c
new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI    6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on Broadwell */
+#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNEL_NUMS)
+
+#define CLIENT_CPU_ID_MASK    0xf0ff0  /* Mask for Family / Model info */
+
+#define UPDATE_INTERVAL_MIN   HZ
+
+enum cpu_gens {
+    CPU_GEN_HSX, /* Haswell Xeon */
+    CPU_GEN_BRX, /* Broadwell Xeon */
+    CPU_GEN_SKX, /* Skylake Xeon */
+    CPU_GEN_MAX
+};
+
+struct cpu_gen_info {
+    u32 type;
+    u32 cpu_id;
+    u32 core_max;
+};
+
+struct temp_data {
+    bool valid;
+    s32  value;
+    unsigned long last_updated;
+};
+
+struct temp_group {
+    struct temp_data die;
+    struct temp_data dts_margin;
+    struct temp_data tcontrol;
+    struct temp_data tthrottle;
+    struct temp_data tjmax;
+    struct temp_data

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-11 Thread Jae Hyun Yoo 

On 4/11/2018 5:34 PM, Guenter Roeck wrote:

On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote:

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile    |   2 +
  drivers/hwmon/peci-cputemp.c  | 783 
++

  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
    This driver can also be built as a module.  If so, the module
    will be called nct7904.
+config SENSORS_PECI_CPUTEMP
+    tristate "PECI CPU temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI
+  cputemp driver which provides Digital Thermal Sensor (DTS) 
thermal
+  readings of the CPU package and CPU cores that are accessible 
using

+  the PECI Client Command Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-cputemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+    tristate "PECI DIMM temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel 
PECI hwmon
+  driver which provides Digital Thermal Sensor (DTS) thermal 
readings of
+  DIMM components that are accessible using the PECI Client 
Command

+  Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-dimmtemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-dimmtemp.
+
  config SENSORS_NSA320
  tristate "ZyXEL NSA320 and compatible fan speed and 
temperature sensors"

  depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)    += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904)    += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)    += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)    += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP)    += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)    += peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360)    += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427)    += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591)    += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c 
b/drivers/hwmon/peci-cputemp.c

new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI    6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on 
Broadwell */

+#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + 
CORETEMP_CHANNEL_NUMS)

+
+#define CLIENT_CPU_ID_MASK    0xf0ff0  /* Mask for Family / Model 
info */

+
+#define UPDATE_INTERVAL_MIN   HZ
+
+enum cpu_gens

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-11 Thread Jae Hyun Yoo 

On 4/11/2018 5:34 PM, Guenter Roeck wrote:

On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote:

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile    |   2 +
  drivers/hwmon/peci-cputemp.c  | 783 
++

  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
    This driver can also be built as a module.  If so, the module
    will be called nct7904.
+config SENSORS_PECI_CPUTEMP
+    tristate "PECI CPU temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI
+  cputemp driver which provides Digital Thermal Sensor (DTS) 
thermal
+  readings of the CPU package and CPU cores that are accessible 
using

+  the PECI Client Command Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-cputemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+    tristate "PECI DIMM temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel 
PECI hwmon
+  driver which provides Digital Thermal Sensor (DTS) thermal 
readings of
+  DIMM components that are accessible using the PECI Client 
Command

+  Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-dimmtemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-dimmtemp.
+
  config SENSORS_NSA320
  tristate "ZyXEL NSA320 and compatible fan speed and 
temperature sensors"

  depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)    += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904)    += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)    += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)    += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP)    += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)    += peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360)    += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427)    += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591)    += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c 
b/drivers/hwmon/peci-cputemp.c

new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI    6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on 
Broadwell */

+#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + 
CORETEMP_CHANNEL_NUMS)

+
+#define CLIENT_CPU_ID_MASK    0xf0ff0  /* Mask for Family / Model 
info */

+
+#define UPDATE_INTERVAL_MIN   HZ
+
+enum cpu_gens {
+    CPU_GEN_HSX, /* Haswell Xeon */
+    CPU_GEN_BRX, /* Broadwell Xeon */
+    CPU_GEN_SKX, /* Skylake Xeon */
+    CPU_GEN_MAX
+};
+
+struct cpu_gen_info {
+    u32 type;
+    u32 cpu_id;
+    u32 core_max;
+};
+
+struct temp_data {
+    bool valid;
+    s32  value;
+    unsigned long last_updated;
+};
+
+struct temp_group {
+    struct temp_data die;
+    struct temp_data dts_margin;
+    struct temp_data tcontrol;
+    struct temp_data tthrottle;
+    struct temp_data tjmax;
+    struct temp_data core[CORETEMP_CHANNEL_NUMS];
+};
+
+struct

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-11 Thread Guenter Roeck 

On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote:

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile    |   2 +
  drivers/hwmon/peci-cputemp.c  | 783 ++
  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
    This driver can also be built as a module.  If so, the module
    will be called nct7904.
+config SENSORS_PECI_CPUTEMP
+    tristate "PECI CPU temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI
+  cputemp driver which provides Digital Thermal Sensor (DTS) thermal
+  readings of the CPU package and CPU cores that are accessible using
+  the PECI Client Command Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-cputemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+    tristate "PECI DIMM temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI hwmon
+  driver which provides Digital Thermal Sensor (DTS) thermal readings of
+  DIMM components that are accessible using the PECI Client Command
+  Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-dimmtemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-dimmtemp.
+
  config SENSORS_NSA320
  tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
  depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)    += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904)    += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)    += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)    += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP)    += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)    += peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360)    += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427)    += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591)    += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c
new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI    6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on Broadwell */
+#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNEL_NUMS)
+
+#define CLIENT_CPU_ID_MASK    0xf0ff0  /* Mask for Family / Model info */
+
+#define UPDATE_INTERVAL_MIN   HZ
+
+enum cpu_gens {
+    CPU_GEN_HSX, /* Haswell Xeon */
+    CPU_GEN_BRX, /*

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-11 Thread Guenter Roeck 

On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote:

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile    |   2 +
  drivers/hwmon/peci-cputemp.c  | 783 ++
  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
    This driver can also be built as a module.  If so, the module
    will be called nct7904.
+config SENSORS_PECI_CPUTEMP
+    tristate "PECI CPU temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI
+  cputemp driver which provides Digital Thermal Sensor (DTS) thermal
+  readings of the CPU package and CPU cores that are accessible using
+  the PECI Client Command Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-cputemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+    tristate "PECI DIMM temperature monitoring support"
+    depends on OF
+    depends on PECI
+    help
+  If you say yes here you get support for the generic Intel PECI hwmon
+  driver which provides Digital Thermal Sensor (DTS) thermal readings of
+  DIMM components that are accessible using the PECI Client Command
+  Suite via the processor PECI client.
+  Check Documentation/hwmon/peci-dimmtemp for details.
+
+  This driver can also be built as a module.  If so, the module
+  will be called peci-dimmtemp.
+
  config SENSORS_NSA320
  tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
  depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)    += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904)    += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)    += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)    += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP)    += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)    += peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360)    += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427)    += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591)    += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c
new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI    6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on Broadwell */
+#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNEL_NUMS)
+
+#define CLIENT_CPU_ID_MASK    0xf0ff0  /* Mask for Family / Model info */
+
+#define UPDATE_INTERVAL_MIN   HZ
+
+enum cpu_gens {
+    CPU_GEN_HSX, /* Haswell Xeon */
+    CPU_GEN_BRX, /* Broadwell Xeon */
+    CPU_GEN_SKX, /* Skylake Xeon */
+    CPU_GEN_MAX
+};
+
+struct cpu_gen_info {
+    u32 type;
+    u32 cpu_id;
+    u32 core_max;
+};
+
+struct temp_data {
+    bool valid;
+    s32  value;
+    unsigned long last_updated;
+};
+
+struct temp_group {
+    struct temp_data die;
+    struct temp_data dts_margin;
+    struct temp_data tcontrol;
+    struct temp_data tthrottle;
+    struct temp_data tjmax;
+    struct temp_data core[CORETEMP_CHANNEL_NUMS];
+};
+
+struct peci_cputemp {
+    struct peci_client *client;
+    struct device *dev;

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-11 Thread Jae Hyun Yoo 

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile|   2 +
  drivers/hwmon/peci-cputemp.c  | 783 ++
  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
  This driver can also be built as a module.  If so, the module
  will be called nct7904.
  
+config SENSORS_PECI_CPUTEMP

+   tristate "PECI CPU temperature monitoring support"
+   depends on OF
+   depends on PECI
+   help
+ If you say yes here you get support for the generic Intel PECI
+ cputemp driver which provides Digital Thermal Sensor (DTS) thermal
+ readings of the CPU package and CPU cores that are accessible using
+ the PECI Client Command Suite via the processor PECI client.
+ Check Documentation/hwmon/peci-cputemp for details.
+
+ This driver can also be built as a module.  If so, the module
+ will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+   tristate "PECI DIMM temperature monitoring support"
+   depends on OF
+   depends on PECI
+   help
+ If you say yes here you get support for the generic Intel PECI hwmon
+ driver which provides Digital Thermal Sensor (DTS) thermal readings of
+ DIMM components that are accessible using the PECI Client Command
+ Suite via the processor PECI client.
+ Check Documentation/hwmon/peci-dimmtemp for details.
+
+ This driver can also be built as a module.  If so, the module
+ will be called peci-dimmtemp.
+
  config SENSORS_NSA320
tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)   += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)  += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)  += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP) += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)+= peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c
new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on Broadwell */
+#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNEL_NUMS)
+
+#define CLIENT_CPU_ID_MASK0xf0ff0  /* Mask for Family / Model info */
+
+#define UPDATE_INTERVAL_MIN   HZ
+
+enum cpu_gens {
+   CPU_GEN_HSX, /* Haswell Xeon */

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-11 Thread Jae Hyun Yoo 

Hi Guenter,

Thanks a lot for sharing your time. Please see my inline answers.

On 4/10/2018 3:28 PM, Guenter Roeck wrote:

On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:

This commit adds PECI cputemp and dimmtemp hwmon drivers.

Signed-off-by: Jae Hyun Yoo 
Reviewed-by: Haiyue Wang 
Reviewed-by: James Feist 
Reviewed-by: Vernon Mauery 
Cc: Alan Cox 
Cc: Andrew Jeffery 
Cc: Andrew Lunn 
Cc: Andy Shevchenko 
Cc: Arnd Bergmann 
Cc: Benjamin Herrenschmidt 
Cc: Fengguang Wu 
Cc: Greg KH 
Cc: Guenter Roeck 
Cc: Jason M Biils 
Cc: Jean Delvare 
Cc: Joel Stanley 
Cc: Julia Cartwright 
Cc: Miguel Ojeda 
Cc: Milton Miller II 
Cc: Pavel Machek 
Cc: Randy Dunlap 
Cc: Stef van Os 
Cc: Sumeet R Pawnikar 
---
  drivers/hwmon/Kconfig |  28 ++
  drivers/hwmon/Makefile|   2 +
  drivers/hwmon/peci-cputemp.c  | 783 ++
  drivers/hwmon/peci-dimmtemp.c | 432 +++
  4 files changed, 1245 insertions(+)
  create mode 100644 drivers/hwmon/peci-cputemp.c
  create mode 100644 drivers/hwmon/peci-dimmtemp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f249a4428458..c52f610f81d0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
  This driver can also be built as a module.  If so, the module
  will be called nct7904.
  
+config SENSORS_PECI_CPUTEMP

+   tristate "PECI CPU temperature monitoring support"
+   depends on OF
+   depends on PECI
+   help
+ If you say yes here you get support for the generic Intel PECI
+ cputemp driver which provides Digital Thermal Sensor (DTS) thermal
+ readings of the CPU package and CPU cores that are accessible using
+ the PECI Client Command Suite via the processor PECI client.
+ Check Documentation/hwmon/peci-cputemp for details.
+
+ This driver can also be built as a module.  If so, the module
+ will be called peci-cputemp.
+
+config SENSORS_PECI_DIMMTEMP
+   tristate "PECI DIMM temperature monitoring support"
+   depends on OF
+   depends on PECI
+   help
+ If you say yes here you get support for the generic Intel PECI hwmon
+ driver which provides Digital Thermal Sensor (DTS) thermal readings of
+ DIMM components that are accessible using the PECI Client Command
+ Suite via the processor PECI client.
+ Check Documentation/hwmon/peci-dimmtemp for details.
+
+ This driver can also be built as a module.  If so, the module
+ will be called peci-dimmtemp.
+
  config SENSORS_NSA320
tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..48d9598fcd3a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802)   += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)  += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)  += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_CPUTEMP) += peci-cputemp.o
+obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)+= peci-dimmtemp.o
  obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c
new file mode 100644
index ..f0bc92687512
--- /dev/null
+++ b/drivers/hwmon/peci-cputemp.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 


Is this include needed ?



No it isn't. Will drop the line.


+#include 
+#include 
+#include 
+#include 
+
+#define TEMP_TYPE_PECI6  /* Sensor type 6: Intel PECI */
+
+#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
+#define CORE_MAX_ON_BDX   24 /* Max number of cores on Broadwell */
+#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
+
+#define DEFAULT_CHANNEL_NUMS  5
+#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
+#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNEL_NUMS)
+
+#define CLIENT_CPU_ID_MASK0xf0ff0  /* Mask for Family / Model info */
+
+#define UPDATE_INTERVAL_MIN   HZ
+
+enum cpu_gens {
+   CPU_GEN_HSX, /* Haswell Xeon */
+   CPU_GEN_BRX, /* Broadwell Xeon */
+   CPU_GEN_SKX, /* Skylake Xeon */
+   CPU_GEN_MAX
+};
+
+struct cpu_gen_info {
+   u32 type;
+   u32 cpu_id;
+   u32 core_max;
+};
+
+struct temp_data {
+   bool valid;
+   s32  value;
+   unsigned long last_updated;
+};
+
+struct temp_group {
+   struct temp_data die;
+   struct temp_data dts_margin;
+   struct temp_data tcontrol;
+   struct temp_data tthrottle;
+   struct temp_data tjmax;
+   struct temp_data core[CORETEMP_CHANNEL_NUMS];
+};
+
+struct

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-10 Thread Guenter Roeck 
On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:
> This commit adds PECI cputemp and dimmtemp hwmon drivers.
> 
> Signed-off-by: Jae Hyun Yoo 
> Reviewed-by: Haiyue Wang 
> Reviewed-by: James Feist 
> Reviewed-by: Vernon Mauery 
> Cc: Alan Cox 
> Cc: Andrew Jeffery 
> Cc: Andrew Lunn 
> Cc: Andy Shevchenko 
> Cc: Arnd Bergmann 
> Cc: Benjamin Herrenschmidt 
> Cc: Fengguang Wu 
> Cc: Greg KH 
> Cc: Guenter Roeck 
> Cc: Jason M Biils 
> Cc: Jean Delvare 
> Cc: Joel Stanley 
> Cc: Julia Cartwright 
> Cc: Miguel Ojeda 
> Cc: Milton Miller II 
> Cc: Pavel Machek 
> Cc: Randy Dunlap 
> Cc: Stef van Os 
> Cc: Sumeet R Pawnikar 
> ---
>  drivers/hwmon/Kconfig |  28 ++
>  drivers/hwmon/Makefile|   2 +
>  drivers/hwmon/peci-cputemp.c  | 783 
> ++
>  drivers/hwmon/peci-dimmtemp.c | 432 +++
>  4 files changed, 1245 insertions(+)
>  create mode 100644 drivers/hwmon/peci-cputemp.c
>  create mode 100644 drivers/hwmon/peci-dimmtemp.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index f249a4428458..c52f610f81d0 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
> This driver can also be built as a module.  If so, the module
> will be called nct7904.
>  
> +config SENSORS_PECI_CPUTEMP
> + tristate "PECI CPU temperature monitoring support"
> + depends on OF
> + depends on PECI
> + help
> +   If you say yes here you get support for the generic Intel PECI
> +   cputemp driver which provides Digital Thermal Sensor (DTS) thermal
> +   readings of the CPU package and CPU cores that are accessible using
> +   the PECI Client Command Suite via the processor PECI client.
> +   Check Documentation/hwmon/peci-cputemp for details.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called peci-cputemp.
> +
> +config SENSORS_PECI_DIMMTEMP
> + tristate "PECI DIMM temperature monitoring support"
> + depends on OF
> + depends on PECI
> + help
> +   If you say yes here you get support for the generic Intel PECI hwmon
> +   driver which provides Digital Thermal Sensor (DTS) thermal readings of
> +   DIMM components that are accessible using the PECI Client Command
> +   Suite via the processor PECI client.
> +   Check Documentation/hwmon/peci-dimmtemp for details.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called peci-dimmtemp.
> +
>  config SENSORS_NSA320
>   tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
>   depends on GPIOLIB && OF
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index e7d52a36e6c4..48d9598fcd3a 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o
>  obj-$(CONFIG_SENSORS_NCT7904)+= nct7904.o
>  obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o
>  obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
> +obj-$(CONFIG_SENSORS_PECI_CPUTEMP)   += peci-cputemp.o
> +obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)  += peci-dimmtemp.o
>  obj-$(CONFIG_SENSORS_PC87360)+= pc87360.o
>  obj-$(CONFIG_SENSORS_PC87427)+= pc87427.o
>  obj-$(CONFIG_SENSORS_PCF8591)+= pcf8591.o
> diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c
> new file mode 100644
> index ..f0bc92687512
> --- /dev/null
> +++ b/drivers/hwmon/peci-cputemp.c
> @@ -0,0 +1,783 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 Intel Corporation
> +
> +#include 
> +#include 
> +#include 

Is this include needed ?

> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define TEMP_TYPE_PECI6  /* Sensor type 6: Intel PECI */
> +
> +#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
> +#define CORE_MAX_ON_BDX   24 /* Max number of cores on Broadwell */
> +#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
> +
> +#define DEFAULT_CHANNEL_NUMS  5
> +#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
> +#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNEL_NUMS)
> +
> +#define CLIENT_CPU_ID_MASK0xf0ff0  /* Mask for Family / Model info */
> +
> +#define UPDATE_INTERVAL_MIN   HZ
> +
> +enum cpu_gens {
> +

Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers

2018-04-10 Thread Guenter Roeck 
On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote:
> This commit adds PECI cputemp and dimmtemp hwmon drivers.
> 
> Signed-off-by: Jae Hyun Yoo 
> Reviewed-by: Haiyue Wang 
> Reviewed-by: James Feist 
> Reviewed-by: Vernon Mauery 
> Cc: Alan Cox 
> Cc: Andrew Jeffery 
> Cc: Andrew Lunn 
> Cc: Andy Shevchenko 
> Cc: Arnd Bergmann 
> Cc: Benjamin Herrenschmidt 
> Cc: Fengguang Wu 
> Cc: Greg KH 
> Cc: Guenter Roeck 
> Cc: Jason M Biils 
> Cc: Jean Delvare 
> Cc: Joel Stanley 
> Cc: Julia Cartwright 
> Cc: Miguel Ojeda 
> Cc: Milton Miller II 
> Cc: Pavel Machek 
> Cc: Randy Dunlap 
> Cc: Stef van Os 
> Cc: Sumeet R Pawnikar 
> ---
>  drivers/hwmon/Kconfig |  28 ++
>  drivers/hwmon/Makefile|   2 +
>  drivers/hwmon/peci-cputemp.c  | 783 
> ++
>  drivers/hwmon/peci-dimmtemp.c | 432 +++
>  4 files changed, 1245 insertions(+)
>  create mode 100644 drivers/hwmon/peci-cputemp.c
>  create mode 100644 drivers/hwmon/peci-dimmtemp.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index f249a4428458..c52f610f81d0 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1259,6 +1259,34 @@ config SENSORS_NCT7904
> This driver can also be built as a module.  If so, the module
> will be called nct7904.
>  
> +config SENSORS_PECI_CPUTEMP
> + tristate "PECI CPU temperature monitoring support"
> + depends on OF
> + depends on PECI
> + help
> +   If you say yes here you get support for the generic Intel PECI
> +   cputemp driver which provides Digital Thermal Sensor (DTS) thermal
> +   readings of the CPU package and CPU cores that are accessible using
> +   the PECI Client Command Suite via the processor PECI client.
> +   Check Documentation/hwmon/peci-cputemp for details.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called peci-cputemp.
> +
> +config SENSORS_PECI_DIMMTEMP
> + tristate "PECI DIMM temperature monitoring support"
> + depends on OF
> + depends on PECI
> + help
> +   If you say yes here you get support for the generic Intel PECI hwmon
> +   driver which provides Digital Thermal Sensor (DTS) thermal readings of
> +   DIMM components that are accessible using the PECI Client Command
> +   Suite via the processor PECI client.
> +   Check Documentation/hwmon/peci-dimmtemp for details.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called peci-dimmtemp.
> +
>  config SENSORS_NSA320
>   tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
>   depends on GPIOLIB && OF
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index e7d52a36e6c4..48d9598fcd3a 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o
>  obj-$(CONFIG_SENSORS_NCT7904)+= nct7904.o
>  obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o
>  obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
> +obj-$(CONFIG_SENSORS_PECI_CPUTEMP)   += peci-cputemp.o
> +obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)  += peci-dimmtemp.o
>  obj-$(CONFIG_SENSORS_PC87360)+= pc87360.o
>  obj-$(CONFIG_SENSORS_PC87427)+= pc87427.o
>  obj-$(CONFIG_SENSORS_PCF8591)+= pcf8591.o
> diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c
> new file mode 100644
> index ..f0bc92687512
> --- /dev/null
> +++ b/drivers/hwmon/peci-cputemp.c
> @@ -0,0 +1,783 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 Intel Corporation
> +
> +#include 
> +#include 
> +#include 

Is this include needed ?

> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define TEMP_TYPE_PECI6  /* Sensor type 6: Intel PECI */
> +
> +#define CORE_MAX_ON_HSX   18 /* Max number of cores on Haswell */
> +#define CORE_MAX_ON_BDX   24 /* Max number of cores on Broadwell */
> +#define CORE_MAX_ON_SKX   28 /* Max number of cores on Skylake */
> +
> +#define DEFAULT_CHANNEL_NUMS  5
> +#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX
> +#define CPUTEMP_CHANNEL_NUMS  (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNEL_NUMS)
> +
> +#define CLIENT_CPU_ID_MASK0xf0ff0  /* Mask for Family / Model info */
> +
> +#define UPDATE_INTERVAL_MIN   HZ
> +
> +enum cpu_gens {
> + CPU_GEN_HSX, /* Haswell Xeon */
> + CPU_GEN_BRX, /* Broadwell Xeon */
> + CPU_GEN_SKX, /* Skylake Xeon */
> + CPU_GEN_MAX
> +};
> +
> +struct cpu_gen_info {
> + u32 type;
> + u32 cpu_id;
> + u32 core_max;
> +};
> +
> +struct temp_data {
> + bool valid;
> + s32  value;
> + unsigned long last_updated;
> +};
> +
> +struct temp_group {
> + struct temp_data die;
> + struct temp_data dts_margin;
> + struct temp_data tcontrol;
> + struct temp_data tthrottle;
> + struct temp_data tjmax;
> + struct temp_data