Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
Hi Stef, Thanks for sharing your time to test it. That is expected result in v2. Previously in v1, it used delayed creation on core temperature group so it was okay if hwmon driver is registered when client CPU is powered down, but in v2, the driver should check resolved cores at probing time to prevent the delayed creation on core temperature group and indexing gap which breaks hwmon subsystem's common rule, so I added peci_detect() into peci_new_device() in PECI core driver to check online status of the client CPU when registering a new device. You may need to use dynamic dtoverlay for loading/unloading a PECI hwmon driver according to the current client CPU power state. It means a PECI hwmon driver can be registered only when the client CPU is powered on. This design will be kept in v3 as well. Thanks, Jae On 3/13/2018 2:32 AM, Stef van Os wrote: Hi Jae, I tried version 1 and 2 of your PECI patch on our (AST2500 / Xeon E5 v4) system. The V1 patchset works as expected (reading back temperature 0 until PECI is up), but the hwmon driver probe fails with version 2. It communicates with the Xeon and assumes during kernel boot of the Aspeed that PECI to the Xeon's is already up and running, but our system enables the main Xeon supplies from AST2500 userspace. If I load the hwmon driver as a module to load later on, the driver does not call probe like e.g. a I2C driver on the I2C bus does. Am I using V2 wrongly? BR, Stef On 02/21/2018 05:16 PM, Jae Hyun Yoo wrote: This commit adds a generic PECI hwmon client driver implementation. Signed-off-by: Jae Hyun Yoo --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 928 + 3 files changed, 939 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ef23553ff5cb..f22e0c31f597 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on PECI + help + If you say yes here you get support for the generic PECI hwmon + driver. + + This driver can also be built as a module. If so, the module + will be called peci-hwmon. + 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 f814b4ace138..946f54b168e5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -135,6 +135,7 @@ 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_HWMON) += peci-hwmon.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-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index ..edd27744adcb --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,928 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DIMM_SLOT_NUMS_MAX 12 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX Platinum) */ +#define TEMP_TYPE_PECI 6 /* Sensor type 6: Intel PECI */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN 24 + +#define DEFAULT_ATTR_GRP_NUMS 5 + +#define UPDATE_INTERVAL_MIN HZ +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) + +enum sign { + POS, + NEG +}; + +struct temp_data { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group { + struct temp_data tjmax; + struct temp_data tcontrol; + struct temp_data tthrottle; + struct temp_data dts_margin; + struct temp_data die; + struct temp_data core[CORE_NUMS_MAX]; + struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; +}; + +struct core_temp_group { + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct dimm_temp_group { + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct peci_hwmon { + struct peci_client *client; + struct device *dev; + struct device *hwmon_dev; + struct workqueue
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
Hi Jae, I tried version 1 and 2 of your PECI patch on our (AST2500 / Xeon E5 v4) system. The V1 patchset works as expected (reading back temperature 0 until PECI is up), but the hwmon driver probe fails with version 2. It communicates with the Xeon and assumes during kernel boot of the Aspeed that PECI to the Xeon's is already up and running, but our system enables the main Xeon supplies from AST2500 userspace. If I load the hwmon driver as a module to load later on, the driver does not call probe like e.g. a I2C driver on the I2C bus does. Am I using V2 wrongly? BR, Stef On 02/21/2018 05:16 PM, Jae Hyun Yoo wrote: This commit adds a generic PECI hwmon client driver implementation. Signed-off-by: Jae Hyun Yoo --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 928 + 3 files changed, 939 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ef23553ff5cb..f22e0c31f597 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on PECI + help + If you say yes here you get support for the generic PECI hwmon + driver. + + This driver can also be built as a module. If so, the module + will be called peci-hwmon. + 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 f814b4ace138..946f54b168e5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -135,6 +135,7 @@ 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_HWMON) += peci-hwmon.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-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index ..edd27744adcb --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,928 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DIMM_SLOT_NUMS_MAX12 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX Platinum) */ +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN 24 + +#define DEFAULT_ATTR_GRP_NUMS 5 + +#define UPDATE_INTERVAL_MIN HZ +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) + +enum sign { + POS, + NEG +}; + +struct temp_data { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group { + struct temp_data tjmax; + struct temp_data tcontrol; + struct temp_data tthrottle; + struct temp_data dts_margin; + struct temp_data die; + struct temp_data core[CORE_NUMS_MAX]; + struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; +}; + +struct core_temp_group { + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct dimm_temp_group { + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct peci_hwmon { + struct peci_client *client; + struct device *dev; + struct device *hwmon_dev; + struct workqueue_struct *work_queue; + struct delayed_work work_handler; + char name[PECI_NAME_SIZE]; + struct temp_group temp; + u8 addr; + uint cpu_no; + u32 core_mask; + u32 dimm_mask; + const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1]; + const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1]; + uint global_idx; + uint core_idx; + uint dimm_idx; +}; + +enum label { + L_DIE, + L_DTS, + L_TCONTROL, + L_TTHROTTLE, + L_TJMAX, + L_MAX +}; + +static const char *peci_label[L_MAX] = { + "Die\n", + "DTS margin to Tcontrol\n", + "Tcontrol\n", + "Tthrottle\n", + "Tjmax\n", +}; + +static int send_peci_cmd(struct peci_hwmon *priv, enum pec
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
On 2/23/2018 4:00 PM, Miguel Ojeda wrote: On Thu, Feb 22, 2018 at 2:29 AM, Jae Hyun Yoo wrote: On 2/21/2018 4:37 PM, Andrew Lunn wrote: But even with this change, it still needs to use delayed creation because BMC side kernel doesn't know how many DIMMs are populated on a remote server before the remote server completes its memory training and testing in BIOS, but it needs to check the remote server's CPU temperature as immediate as possible to make appropriate thermal control based on the remote CPU's temperature to avoid any critical thermal issue. What would be a better solution in this case? You could change this driver so that it supports one DIMM. Move the 'hotplug' part into another driver which creates and destroys instances of the hwmon DIMM device as the DIMMS come and go. Also, do you need to handle CPU hotplug? You could split the CPU temperature part into a separate hwmon driver? And again create and destroy devices as CPUs come and go? Andrew That seems like a possible option. I'll rewrite the hwmon driver again like that. Thanks for the good idea. :) By the way, in the rewrite, please try to avoid the create*workqueue() functions (they are deprecated :). Cheers, Miguel Hi Miguel, Thanks for letting me know that. I'll replace that with alloc_workqueue(). :) Regards, Jae Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
On Thu, Feb 22, 2018 at 2:29 AM, Jae Hyun Yoo wrote: > On 2/21/2018 4:37 PM, Andrew Lunn wrote: >>> >>> But even with this change, it still needs to use delayed creation >>> because BMC side kernel doesn't know how many DIMMs are populated on >>> a remote server before the remote server completes its memory >>> training and testing in BIOS, but it needs to check the remote >>> server's CPU temperature as immediate as possible to make >>> appropriate thermal control based on the remote CPU's temperature to >>> avoid any critical thermal issue. What would be a better solution in >>> this case? >> >> >> You could change this driver so that it supports one DIMM. Move the >> 'hotplug' part into another driver which creates and destroys >> instances of the hwmon DIMM device as the DIMMS come and go. >> >> Also, do you need to handle CPU hotplug? You could split the CPU >> temperature part into a separate hwmon driver? And again create and >> destroy devices as CPUs come and go? >> >> Andrew >> > > That seems like a possible option. I'll rewrite the hwmon driver again like > that. > > Thanks for the good idea. :) By the way, in the rewrite, please try to avoid the create*workqueue() functions (they are deprecated :). Cheers, Miguel > > Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
On 2/21/2018 4:37 PM, Andrew Lunn wrote: But even with this change, it still needs to use delayed creation because BMC side kernel doesn't know how many DIMMs are populated on a remote server before the remote server completes its memory training and testing in BIOS, but it needs to check the remote server's CPU temperature as immediate as possible to make appropriate thermal control based on the remote CPU's temperature to avoid any critical thermal issue. What would be a better solution in this case? You could change this driver so that it supports one DIMM. Move the 'hotplug' part into another driver which creates and destroys instances of the hwmon DIMM device as the DIMMS come and go. Also, do you need to handle CPU hotplug? You could split the CPU temperature part into a separate hwmon driver? And again create and destroy devices as CPUs come and go? Andrew That seems like a possible option. I'll rewrite the hwmon driver again like that. Thanks for the good idea. :) Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
> But even with this change, it still needs to use delayed creation > because BMC side kernel doesn't know how many DIMMs are populated on > a remote server before the remote server completes its memory > training and testing in BIOS, but it needs to check the remote > server's CPU temperature as immediate as possible to make > appropriate thermal control based on the remote CPU's temperature to > avoid any critical thermal issue. What would be a better solution in > this case? You could change this driver so that it supports one DIMM. Move the 'hotplug' part into another driver which creates and destroys instances of the hwmon DIMM device as the DIMMS come and go. Also, do you need to handle CPU hotplug? You could split the CPU temperature part into a separate hwmon driver? And again create and destroy devices as CPUs come and go? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
On 2/21/2018 1:48 PM, Guenter Roeck wrote: On Wed, Feb 21, 2018 at 01:24:48PM -0800, Jae Hyun Yoo wrote: Hi Guenter, Thanks for sharing your time to review this code. Please check my answers inline. On 2/21/2018 10:26 AM, Guenter Roeck wrote: On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote: This commit adds a generic PECI hwmon client driver implementation. Signed-off-by: Jae Hyun Yoo --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 928 + 3 files changed, 939 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ef23553ff5cb..f22e0c31f597 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on PECI + help + If you say yes here you get support for the generic PECI hwmon + driver. + + This driver can also be built as a module. If so, the module + will be called peci-hwmon. + 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 f814b4ace138..946f54b168e5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -135,6 +135,7 @@ 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_HWMON) += peci-hwmon.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-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index ..edd27744adcb --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,928 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DIMM_SLOT_NUMS_MAX12 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX Platinum) */ +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN 24 + +#define DEFAULT_ATTR_GRP_NUMS 5 + +#define UPDATE_INTERVAL_MIN HZ +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) + +enum sign { + POS, + NEG +}; + +struct temp_data { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group { + struct temp_data tjmax; + struct temp_data tcontrol; + struct temp_data tthrottle; + struct temp_data dts_margin; + struct temp_data die; + struct temp_data core[CORE_NUMS_MAX]; + struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; +}; + +struct core_temp_group { + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct dimm_temp_group { + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct peci_hwmon { + struct peci_client *client; + struct device *dev; + struct device *hwmon_dev; + struct workqueue_struct *work_queue; + struct delayed_work work_handler; + char name[PECI_NAME_SIZE]; + struct temp_group temp; + u8 addr; + uint cpu_no; + u32 core_mask; + u32 dimm_mask; + const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1]; + const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1]; + uint global_idx; + uint core_idx; + uint dimm_idx; +}; + +enum label { + L_DIE, + L_DTS, + L_TCONTROL, + L_TTHROTTLE, + L_TJMAX, + L_MAX +}; + +static const char *peci_label[L_MAX] = { + "Die\n", + "DTS margin to Tcontrol\n", + "Tcontrol\n", + "Tthrottle\n", + "Tjmax\n", +}; + +static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void *msg) +{ + return peci_command(priv->client->adapter, cmd, msg); +} + +static int need_update(struct temp_data *temp) +{ + if (temp->valid && + time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN)) + return 0; + + return 1; +} + +static s32 ten_dot_six_to_mill
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote: > This commit adds a generic PECI hwmon client driver implementation. > > Signed-off-by: Jae Hyun Yoo > --- > drivers/hwmon/Kconfig | 10 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/peci-hwmon.c | 928 > + > 3 files changed, 939 insertions(+) > create mode 100644 drivers/hwmon/peci-hwmon.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index ef23553ff5cb..f22e0c31f597 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 > This driver can also be built as a module. If so, the module > will be called nct7904. > > +config SENSORS_PECI_HWMON > + tristate "PECI hwmon support" > + depends on PECI > + help > + If you say yes here you get support for the generic PECI hwmon > + driver. > + > + This driver can also be built as a module. If so, the module > + will be called peci-hwmon. > + > 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 f814b4ace138..946f54b168e5 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -135,6 +135,7 @@ 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_HWMON) += peci-hwmon.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-hwmon.c b/drivers/hwmon/peci-hwmon.c > new file mode 100644 > index ..edd27744adcb > --- /dev/null > +++ b/drivers/hwmon/peci-hwmon.c > @@ -0,0 +1,928 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2018 Intel Corporation > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define DIMM_SLOT_NUMS_MAX12 /* Max DIMM numbers (channel ranks x 2) */ > +#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX Platinum) > */ > +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ > + > +#define CORE_TEMP_ATTRS 5 > +#define DIMM_TEMP_ATTRS 2 > +#define ATTR_NAME_LEN 24 > + > +#define DEFAULT_ATTR_GRP_NUMS 5 > + > +#define UPDATE_INTERVAL_MIN HZ > +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) > + > +enum sign { > + POS, > + NEG > +}; > + > +struct temp_data { > + bool valid; > + s32 value; > + unsigned long last_updated; > +}; > + > +struct temp_group { > + struct temp_data tjmax; > + struct temp_data tcontrol; > + struct temp_data tthrottle; > + struct temp_data dts_margin; > + struct temp_data die; > + struct temp_data core[CORE_NUMS_MAX]; > + struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; > +}; > + > +struct core_temp_group { > + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; > + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; > + struct attribute *attrs[CORE_TEMP_ATTRS + 1]; > + struct attribute_group attr_group; > +}; > + > +struct dimm_temp_group { > + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; > + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; > + struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; > + struct attribute_group attr_group; > +}; > + > +struct peci_hwmon { > + struct peci_client *client; > + struct device *dev; > + struct device *hwmon_dev; > + struct workqueue_struct *work_queue; > + struct delayed_work work_handler; > + char name[PECI_NAME_SIZE]; > + struct temp_group temp; > + u8 addr; > + uint cpu_no; > + u32 core_mask; > + u32 dimm_mask; > + const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1]; > + const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1]; > + uint global_idx; > + uint core_idx; > + uint dimm_idx; > +}; > + > +enum label { > + L_DIE, > + L_DTS, > + L_TCONTROL, > + L_TTHROTTLE, > + L_TJMAX, > + L_MAX > +}; > + > +static const char *peci_label[L_MAX] = { > + "Die\n", > + "DTS margin to Tcontrol\n", > + "Tcontrol\n", > + "Tthrottle\n", > + "Tjmax\n", > +}; > + > +static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void > *msg) > +{ > + return peci_command(priv->client->adapter, cmd, msg); > +} > + > +static int need_update(struct temp_data *temp) > +{ > + if (temp->valid && > + time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN)) > + return 0; > + > + return 1; > +} > + > +static s32 ten_dot_six_to_millidegree(s32 x) > +{ > + return ((
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
On Wed, Feb 21, 2018 at 01:24:48PM -0800, Jae Hyun Yoo wrote: > Hi Guenter, > > Thanks for sharing your time to review this code. Please check my answers > inline. > > On 2/21/2018 10:26 AM, Guenter Roeck wrote: > >On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote: > >>This commit adds a generic PECI hwmon client driver implementation. > >> > >>Signed-off-by: Jae Hyun Yoo > >>--- > >> drivers/hwmon/Kconfig | 10 + > >> drivers/hwmon/Makefile | 1 + > >> drivers/hwmon/peci-hwmon.c | 928 > >> + > >> 3 files changed, 939 insertions(+) > >> create mode 100644 drivers/hwmon/peci-hwmon.c > >> > >>diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > >>index ef23553ff5cb..f22e0c31f597 100644 > >>--- a/drivers/hwmon/Kconfig > >>+++ b/drivers/hwmon/Kconfig > >>@@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 > >> This driver can also be built as a module. If so, the module > >> will be called nct7904. > >>+config SENSORS_PECI_HWMON > >>+ tristate "PECI hwmon support" > >>+ depends on PECI > >>+ help > >>+ If you say yes here you get support for the generic PECI hwmon > >>+ driver. > >>+ > >>+ This driver can also be built as a module. If so, the module > >>+ will be called peci-hwmon. > >>+ > >> 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 f814b4ace138..946f54b168e5 100644 > >>--- a/drivers/hwmon/Makefile > >>+++ b/drivers/hwmon/Makefile > >>@@ -135,6 +135,7 @@ 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_HWMON) += peci-hwmon.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-hwmon.c b/drivers/hwmon/peci-hwmon.c > >>new file mode 100644 > >>index ..edd27744adcb > >>--- /dev/null > >>+++ b/drivers/hwmon/peci-hwmon.c > >>@@ -0,0 +1,928 @@ > >>+// SPDX-License-Identifier: GPL-2.0 > >>+// Copyright (c) 2018 Intel Corporation > >>+ > >>+#include > >>+#include > >>+#include > >>+#include > >>+#include > >>+#include > >>+#include > >>+#include > >>+ > >>+#define DIMM_SLOT_NUMS_MAX12 /* Max DIMM numbers (channel ranks x 2) > >>*/ > >>+#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX > >>Platinum) */ > >>+#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ > >>+ > >>+#define CORE_TEMP_ATTRS 5 > >>+#define DIMM_TEMP_ATTRS 2 > >>+#define ATTR_NAME_LEN 24 > >>+ > >>+#define DEFAULT_ATTR_GRP_NUMS 5 > >>+ > >>+#define UPDATE_INTERVAL_MIN HZ > >>+#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) > >>+ > >>+enum sign { > >>+ POS, > >>+ NEG > >>+}; > >>+ > >>+struct temp_data { > >>+ bool valid; > >>+ s32 value; > >>+ unsigned long last_updated; > >>+}; > >>+ > >>+struct temp_group { > >>+ struct temp_data tjmax; > >>+ struct temp_data tcontrol; > >>+ struct temp_data tthrottle; > >>+ struct temp_data dts_margin; > >>+ struct temp_data die; > >>+ struct temp_data core[CORE_NUMS_MAX]; > >>+ struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; > >>+}; > >>+ > >>+struct core_temp_group { > >>+ struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; > >>+ char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; > >>+ struct attribute *attrs[CORE_TEMP_ATTRS + 1]; > >>+ struct attribute_group attr_group; > >>+}; > >>+ > >>+struct dimm_temp_group { > >>+ struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; > >>+ char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; > >>+ struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; > >>+ struct attribute_group attr_group; > >>+}; > >>+ > >>+struct peci_hwmon { > >>+ struct peci_client *client; > >>+ struct device *dev; > >>+ struct device *hwmon_dev; > >>+ struct workqueue_struct *work_queue; > >>+ struct delayed_work work_handler; > >>+ char name[PECI_NAME_SIZE]; > >>+ struct temp_group temp; > >>+ u8 addr; > >>+ uint cpu_no; > >>+ u32 core_mask; > >>+ u32 dimm_mask; > >>+ const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1]; > >>+ const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1]; > >>+ uint global_idx; > >>+ uint core_idx; > >>+ uint dimm_idx; > >>+}; > >>+ > >>+enum label { > >>+ L_DIE, > >>+ L_DTS, > >>+ L_TCONTROL, > >>+ L_TTHROTTLE, > >>+ L_TJMAX, > >>+ L_MAX > >>+}; > >>+ > >>+static const char *peci_label[L_MAX] = { > >>+ "Die\n", > >>+ "DTS margin to Tcontrol\n", > >>+ "Tcontrol\n", > >>+ "Tthrottle\n", > >>+ "Tjmax\n", > >>+}; > >>+ > >>+static int send_peci_cmd(struct peci_hwmo
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
Hi Guenter, Thanks for sharing your time to review this code. Please check my answers inline. On 2/21/2018 10:26 AM, Guenter Roeck wrote: On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote: This commit adds a generic PECI hwmon client driver implementation. Signed-off-by: Jae Hyun Yoo --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 928 + 3 files changed, 939 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ef23553ff5cb..f22e0c31f597 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on PECI + help + If you say yes here you get support for the generic PECI hwmon + driver. + + This driver can also be built as a module. If so, the module + will be called peci-hwmon. + 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 f814b4ace138..946f54b168e5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -135,6 +135,7 @@ 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_HWMON) += peci-hwmon.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-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index ..edd27744adcb --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,928 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DIMM_SLOT_NUMS_MAX12 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX Platinum) */ +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN 24 + +#define DEFAULT_ATTR_GRP_NUMS 5 + +#define UPDATE_INTERVAL_MIN HZ +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) + +enum sign { + POS, + NEG +}; + +struct temp_data { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group { + struct temp_data tjmax; + struct temp_data tcontrol; + struct temp_data tthrottle; + struct temp_data dts_margin; + struct temp_data die; + struct temp_data core[CORE_NUMS_MAX]; + struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; +}; + +struct core_temp_group { + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct dimm_temp_group { + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct peci_hwmon { + struct peci_client *client; + struct device *dev; + struct device *hwmon_dev; + struct workqueue_struct *work_queue; + struct delayed_work work_handler; + char name[PECI_NAME_SIZE]; + struct temp_group temp; + u8 addr; + uint cpu_no; + u32 core_mask; + u32 dimm_mask; + const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1]; + const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1]; + uint global_idx; + uint core_idx; + uint dimm_idx; +}; + +enum label { + L_DIE, + L_DTS, + L_TCONTROL, + L_TTHROTTLE, + L_TJMAX, + L_MAX +}; + +static const char *peci_label[L_MAX] = { + "Die\n", + "DTS margin to Tcontrol\n", + "Tcontrol\n", + "Tthrottle\n", + "Tjmax\n", +}; + +static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void *msg) +{ + return peci_command(priv->client->adapter, cmd, msg); +} + +static int need_update(struct temp_data *temp) +{ + if (temp->valid && + time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN)) + return 0; + + return 1; +} + +static s32 ten_dot_six_to_millidegree(s32 x) +{ + return x) ^ 0x8000) - 0x8000) * 1000 / 64); +} + +static int get_tjmax(st
[PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
This commit adds a generic PECI hwmon client driver implementation. Signed-off-by: Jae Hyun Yoo --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 928 + 3 files changed, 939 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ef23553ff5cb..f22e0c31f597 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on PECI + help + If you say yes here you get support for the generic PECI hwmon + driver. + + This driver can also be built as a module. If so, the module + will be called peci-hwmon. + 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 f814b4ace138..946f54b168e5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -135,6 +135,7 @@ 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_HWMON) += peci-hwmon.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-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index ..edd27744adcb --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,928 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DIMM_SLOT_NUMS_MAX12 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX Platinum) */ +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN 24 + +#define DEFAULT_ATTR_GRP_NUMS 5 + +#define UPDATE_INTERVAL_MIN HZ +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) + +enum sign { + POS, + NEG +}; + +struct temp_data { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group { + struct temp_data tjmax; + struct temp_data tcontrol; + struct temp_data tthrottle; + struct temp_data dts_margin; + struct temp_data die; + struct temp_data core[CORE_NUMS_MAX]; + struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; +}; + +struct core_temp_group { + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct dimm_temp_group { + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct peci_hwmon { + struct peci_client *client; + struct device *dev; + struct device *hwmon_dev; + struct workqueue_struct *work_queue; + struct delayed_work work_handler; + char name[PECI_NAME_SIZE]; + struct temp_group temp; + u8 addr; + uint cpu_no; + u32 core_mask; + u32 dimm_mask; + const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1]; + const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1]; + uint global_idx; + uint core_idx; + uint dimm_idx; +}; + +enum label { + L_DIE, + L_DTS, + L_TCONTROL, + L_TTHROTTLE, + L_TJMAX, + L_MAX +}; + +static const char *peci_label[L_MAX] = { + "Die\n", + "DTS margin to Tcontrol\n", + "Tcontrol\n", + "Tthrottle\n", + "Tjmax\n", +}; + +static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void *msg) +{ + return peci_command(priv->client->adapter, cmd, msg); +} + +static int need_update(struct temp_data *temp) +{ + if (temp->valid && + time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN)) + return 0; + + return 1; +} + +static s32 ten_dot_six_to_millidegree(s32 x) +{ + return x) ^ 0x8000) - 0x8000) * 1000 / 64); +} + +static int get_tjmax(struct peci_hwmon *priv) +{ + struct peci_rd_pkg_cfg_msg msg; + int rc; + + if (!priv->temp.tjmax.valid) { + msg.addr = priv->addr; + msg.index = MBX_INDEX_TEMP_TARGET; +