| Message ID | 20240315115810.15816-1-dober6023@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v4] hwmon:Add EC Chip driver for Lenovo ThinkStation motherboards | expand |
On Fri, Mar 15, 2024 at 07:58:10AM -0400, David Ober wrote: > This addition adds in the ability for the system to scan > the EC chip in the Lenovo ThinkStation systems to get the > current fan RPM speeds the Maximum speed value for each > fan also provides the CPU, DIMM other thermal statuses > > Signed-off-by: David Ober <dober@lenovo.com> > Signed-off-by: David Ober <dober6023@gmail.com> Applied to hwmon-next. Please note that I'll push the branch after the commit window closed. Thanks, Guenter
On Wed, Mar 20, 2024 at 10:59:34AM -0700, Guenter Roeck wrote: > On Fri, Mar 15, 2024 at 07:58:10AM -0400, David Ober wrote: > > This addition adds in the ability for the system to scan > > the EC chip in the Lenovo ThinkStation systems to get the > > current fan RPM speeds the Maximum speed value for each > > fan also provides the CPU, DIMM other thermal statuses > > > > Signed-off-by: David Ober <dober@lenovo.com> > > Signed-off-by: David Ober <dober6023@gmail.com> > > Applied to hwmon-next. > > Please note that I'll push the branch after the commit window closed. Hmm... Was it even compiled? lenovo-ec-sensors.c:154:31: error: expected '}' static int p8_fan_map[] = {0. 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 14}; ^ lenovo-ec-sensors.c:154:27: note: to match this '{' static int p8_fan_map[] = {0. 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 14}; ^
On Fri, Mar 15, 2024 at 07:58:10AM -0400, David Ober wrote: > This addition adds in the ability for the system to scan > the EC chip in the Lenovo ThinkStation systems to get the > current fan RPM speeds the Maximum speed value for each > fan also provides the CPU, DIMM other thermal statuses Besides the compilation error, see other remarks below. ... > Signed-off-by: David Ober <dober@lenovo.com> > Signed-off-by: David Ober <dober6023@gmail.com> Can you leave only one of these? > +config SENSORS_LENOVO_EC > + tristate "Sensor reader for Lenovo ThinkStations" > + depends on X86 > + help > + If you say yes here you get support for LENOVO > + EC Sensor data on newer ThinkStation systems > + > + This driver can also be built as a module. If so, the module > + will be called lenovo_ec_sensors. ... > + * Copyright (C) 2023 David Ober (Lenovo) <dober@lenovo.com> 2024? ... Use IWYU principle (include what you use). See below what I have noticed (may not be the full list of missing inclusions). > +#include <linux/acpi.h> > +#include <linux/delay.h> + device.h > +#include <linux/dmi.h> + err.h > +#include <linux/hwmon.h> > +#include <linux/io.h> > +#include <linux/ioport.h> > +#include <linux/kernel.h> How is this one being used, please? > +#include <linux/module.h> + mutex.h > +#include <linux/platform_device.h> + types.h > +#include <linux/units.h> ... > +#define io_write8(a, b) outb_p(b, a) > +#define io_read8(a) inb_p(a) First of all, these are too generic, second, what's the point? If you wish to make useful macros, put a pointer to the private data, for example, from which you can get the address. ... > +static inline uint8_t uint8_t... > +get_ec_reg(unsigned char page, unsigned char index) > +{ > + u8 onebyte; u8... Just in a few closer lines :-( Can you be consistent with a kernel types? > + unsigned short m_index; > + unsigned short phy_index = page * 256 + index; > + > + io_write8(MCHP_EMI0_APPLICATION_ID, 0x01); > + > + m_index = phy_index & 0x7FFC; GENMASK() (you will need bits.h for this) > + io_write8(MCHP_EMI0_EC_ADDRESS_LSB, m_index); > + io_write8(MCHP_EMI0_EC_ADDRESS_MSB, m_index >> 8); Can the 16-bit write suffice? > + onebyte = io_read8(MCHP_EMI0_EC_DATA_BYTE0 + (phy_index & 3)); GENMASK() > + io_write8(MCHP_EMI0_APPLICATION_ID, 0x01); /* write 0x01 again to clean */ > + return onebyte; > +} ... > +struct ec_sensors_data { > + struct mutex mec_mutex; /* lock for sensor data access */ > + /*int platform_id;*/ Huh?! Please remove, if no use for it. > + const char *const *fan_labels; > + const char *const *temp_labels; > + const int *fan_map; > + const int *temp_map; > +}; ... > +static int > +lenovo_ec_do_read_fan(struct ec_sensors_data *data, u32 attr, int channel, long *val) > +{ > + u8 lsb, msb; > + > + channel *= 2; > + switch (attr) { > + case hwmon_fan_input: > + mutex_lock(&data->mec_mutex); > + lsb = get_ec_reg(4, 0x20 + channel); > + msb = get_ec_reg(4, 0x21 + channel); > + mutex_unlock(&data->mec_mutex); > + *val = (msb << 8) + lsb; > + return 0; > + case hwmon_fan_max: > + mutex_lock(&data->mec_mutex); > + lsb = get_ec_reg(4, 0x40 + channel); > + msb = get_ec_reg(4, 0x41 + channel); > + mutex_unlock(&data->mec_mutex); > + *val = (msb << 8) + lsb; > + return 0; > + case hwmon_fan_min: > + case hwmon_fan_div: > + case hwmon_fan_alarm: > + break; > + default: > + break; > + } > + return -EOPNOTSUPP; Combine with the above and return only once this from the default case. > +} ... > +static int > +lenovo_ec_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, const char **str) > +{ > + struct ec_sensors_data *state = dev_get_drvdata(dev); > + > + switch (type) { > + case hwmon_temp: > + *str = state->temp_labels[channel]; > + break; In the other function you returned directly. Keep the style consistent, please. > + case hwmon_fan: > + *str = state->fan_labels[channel]; > + break; > + default: > + return -EOPNOTSUPP; > + } > + return 0; > +} ... > +static int > +lenovo_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct ec_sensors_data *data = dev_get_drvdata(dev); > + > + switch (type) { > + case hwmon_temp: > + return lenovo_ec_do_read_temp(data, attr, data->temp_map[channel], val); > + case hwmon_fan: > + return lenovo_ec_do_read_fan(data, attr, data->fan_map[channel], val); > + default: > + return -EOPNOTSUPP; > + } > + return 0; Dead code. > +} ... > +static umode_t > +lenovo_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + switch (type) { > + case hwmon_temp: > + if (attr == hwmon_temp_input || attr == hwmon_temp_label) > + return 0444; > + break; > + case hwmon_fan: > + if (attr == hwmon_fan_input || attr == hwmon_fan_max || attr == hwmon_fan_label) > + return 0444; > + break; > + default: > + return 0; > + } > + return 0; Same comment about the style. > +} ... > +static int lenovo_ec_probe(struct platform_device *pdev) > +{ > + struct device *hwdev; > + struct ec_sensors_data *ec_data; > + const struct hwmon_chip_info *chip_info; > + struct device *dev = &pdev->dev; > + const struct dmi_system_id *dmi_id; > + int app_id; > + > + ec_data = devm_kzalloc(dev, sizeof(struct ec_sensors_data), GFP_KERNEL); > + if (!ec_data) { > + release_region(IO_REGION_START, IO_REGION_LENGTH); This is weird. Please, either move request region to the probe, or drop these calls here. Obviously you haven't checked the bind-unbind-bind cycle with error injection. > + return -ENOMEM; > + } > + > + dev_set_drvdata(dev, ec_data); > + > + chip_info = &lenovo_ec_chip_info; > + > + mutex_init(&ec_data->mec_mutex); > + > + mutex_lock(&ec_data->mec_mutex); > + app_id = io_read8(MCHP_EMI0_APPLICATION_ID); > + if (app_id) /* check EMI Application ID Value */ > + io_write8(MCHP_EMI0_APPLICATION_ID, app_id); /* set EMI Application ID to 0 */ > + io_write8(MCHP_EMI0_EC_ADDRESS_LSB, MCHP_SING_IDX); > + io_write8(MCHP_EMI0_EC_ADDRESS_MSB, MCHP_SING_IDX >> 8); > + mutex_unlock(&ec_data->mec_mutex); > + > + if ((io_read8(MCHP_EMI0_EC_DATA_BYTE0) != 'M') && > + (io_read8(MCHP_EMI0_EC_DATA_BYTE1) != 'C') && > + (io_read8(MCHP_EMI0_EC_DATA_BYTE2) != 'H') && > + (io_read8(MCHP_EMI0_EC_DATA_BYTE3) != 'P')) { > + release_region(IO_REGION_START, IO_REGION_LENGTH); > + return -ENODEV; > + } > + > + dmi_id = dmi_first_match(thinkstation_dmi_table); > + > + switch ((long)dmi_id->driver_data) { > + case 0: > + ec_data->fan_labels = px_ec_fan_label; > + ec_data->temp_labels = lenovo_px_ec_temp_label; > + ec_data->fan_map = px_fan_map; > + ec_data->temp_map = px_temp_map; > + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_px; > + break; > + case 1: > + ec_data->fan_labels = p7_ec_fan_label; > + ec_data->temp_labels = lenovo_gen_ec_temp_label; > + ec_data->fan_map = p7_fan_map; > + ec_data->temp_map = gen_temp_map; > + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p7; > + break; > + case 2: > + ec_data->fan_labels = p5_ec_fan_label; > + ec_data->temp_labels = lenovo_gen_ec_temp_label; > + ec_data->fan_map = p5_fan_map; > + ec_data->temp_map = gen_temp_map; > + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p5; > + break; > + case 3: > + ec_data->fan_labels = p8_ec_fan_label; > + ec_data->temp_labels = lenovo_gen_ec_temp_label; > + ec_data->fan_map = p8_fan_map; > + ec_data->temp_map = gen_temp_map; > + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p8; > + break; > + default: > + release_region(IO_REGION_START, IO_REGION_LENGTH); > + return -ENODEV; > + } > + > + hwdev = devm_hwmon_device_register_with_info(dev, "lenovo_ec", > + ec_data, > + chip_info, NULL); > + > + return PTR_ERR_OR_ZERO(hwdev); > +} ... > + if (!request_region(IO_REGION_START, IO_REGION_LENGTH, "LNV-WKS")) { > + pr_err(":request fail\n"); I haven't noticed pr_fmt(). How can the user distinguish this from something similar from another place? > + return -EIO; > + } ... > +static void __exit lenovo_ec_exit(void) > +{ > + release_region(IO_REGION_START, IO_REGION_LENGTH); > + platform_device_unregister(lenovo_ec_sensors_platform_device); > + platform_driver_unregister(&lenovo_ec_sensors_platform_driver); > +} > + Unneeded blank line (see also below). > +module_init(lenovo_ec_init); > +module_exit(lenovo_ec_exit); Move each of them closer to the respective callback implementation.
On 3/25/24 05:29, Andy Shevchenko wrote: > On Fri, Mar 15, 2024 at 07:58:10AM -0400, David Ober wrote: >> This addition adds in the ability for the system to scan >> the EC chip in the Lenovo ThinkStation systems to get the >> current fan RPM speeds the Maximum speed value for each >> fan also provides the CPU, DIMM other thermal statuses > > Besides the compilation error, see other remarks below. > Thanks a lot for the detailed review. Dropped the driver for now. Guenter
Andy Sorry about the compile break comma got changed to period when I was adding in the spaces checkpatch forced me to put in between the values and since I was only adding spaces did not think to check compile Thanks for the comments I will fix these and resubmit soon. David -----Original Message----- From: Andy Shevchenko <andy@black.fi.intel.com> Sent: Monday, March 25, 2024 8:30 AM To: David Ober <dober6023@gmail.com> Cc: linux-hwmon@vger.kernel.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; jdelvare@suse.com; linux@roeck-us.net; corbet@lwn.net; David Ober <dober@lenovo.com>; Mark Pearson <mpearson@lenovo.com> Subject: [External] Re: [PATCH v4] hwmon:Add EC Chip driver for Lenovo ThinkStation motherboards On Fri, Mar 15, 2024 at 07:58:10AM -0400, David Ober wrote: > This addition adds in the ability for the system to scan the EC chip > in the Lenovo ThinkStation systems to get the current fan RPM speeds > the Maximum speed value for each fan also provides the CPU, DIMM other > thermal statuses Besides the compilation error, see other remarks below. ... > Signed-off-by: David Ober <dober@lenovo.com> > Signed-off-by: David Ober <dober6023@gmail.com> Can you leave only one of these? > +config SENSORS_LENOVO_EC > + tristate "Sensor reader for Lenovo ThinkStations" > + depends on X86 > + help > + If you say yes here you get support for LENOVO > + EC Sensor data on newer ThinkStation systems > + > + This driver can also be built as a module. If so, the module > + will be called lenovo_ec_sensors. ... > + * Copyright (C) 2023 David Ober (Lenovo) <dober@lenovo.com> 2024? ... Use IWYU principle (include what you use). See below what I have noticed (may not be the full list of missing inclusions). > +#include <linux/acpi.h> > +#include <linux/delay.h> + device.h > +#include <linux/dmi.h> + err.h > +#include <linux/hwmon.h> > +#include <linux/io.h> > +#include <linux/ioport.h> > +#include <linux/kernel.h> How is this one being used, please? > +#include <linux/module.h> + mutex.h > +#include <linux/platform_device.h> + types.h > +#include <linux/units.h> ... > +#define io_write8(a, b) outb_p(b, a) > +#define io_read8(a) inb_p(a) First of all, these are too generic, second, what's the point? If you wish to make useful macros, put a pointer to the private data, for example, from which you can get the address. ... > +static inline uint8_t uint8_t... > +get_ec_reg(unsigned char page, unsigned char index) { > + u8 onebyte; u8... Just in a few closer lines :-( Can you be consistent with a kernel types? > + unsigned short m_index; > + unsigned short phy_index = page * 256 + index; > + > + io_write8(MCHP_EMI0_APPLICATION_ID, 0x01); > + > + m_index = phy_index & 0x7FFC; GENMASK() (you will need bits.h for this) > + io_write8(MCHP_EMI0_EC_ADDRESS_LSB, m_index); > + io_write8(MCHP_EMI0_EC_ADDRESS_MSB, m_index >> 8); Can the 16-bit write suffice? > + onebyte = io_read8(MCHP_EMI0_EC_DATA_BYTE0 + (phy_index & 3)); GENMASK() > + io_write8(MCHP_EMI0_APPLICATION_ID, 0x01); /* write 0x01 again to clean */ > + return onebyte; > +} ... > +struct ec_sensors_data { > + struct mutex mec_mutex; /* lock for sensor data access */ > + /*int platform_id;*/ Huh?! Please remove, if no use for it. > + const char *const *fan_labels; > + const char *const *temp_labels; > + const int *fan_map; > + const int *temp_map; > +}; ... > +static int > +lenovo_ec_do_read_fan(struct ec_sensors_data *data, u32 attr, int > +channel, long *val) { > + u8 lsb, msb; > + > + channel *= 2; > + switch (attr) { > + case hwmon_fan_input: > + mutex_lock(&data->mec_mutex); > + lsb = get_ec_reg(4, 0x20 + channel); > + msb = get_ec_reg(4, 0x21 + channel); > + mutex_unlock(&data->mec_mutex); > + *val = (msb << 8) + lsb; > + return 0; > + case hwmon_fan_max: > + mutex_lock(&data->mec_mutex); > + lsb = get_ec_reg(4, 0x40 + channel); > + msb = get_ec_reg(4, 0x41 + channel); > + mutex_unlock(&data->mec_mutex); > + *val = (msb << 8) + lsb; > + return 0; > + case hwmon_fan_min: > + case hwmon_fan_div: > + case hwmon_fan_alarm: > + break; > + default: > + break; > + } > + return -EOPNOTSUPP; Combine with the above and return only once this from the default case. > +} ... > +static int > +lenovo_ec_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, const char **str) { > + struct ec_sensors_data *state = dev_get_drvdata(dev); > + > + switch (type) { > + case hwmon_temp: > + *str = state->temp_labels[channel]; > + break; In the other function you returned directly. Keep the style consistent, please. > + case hwmon_fan: > + *str = state->fan_labels[channel]; > + break; > + default: > + return -EOPNOTSUPP; > + } > + return 0; > +} ... > +static int > +lenovo_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) { > + struct ec_sensors_data *data = dev_get_drvdata(dev); > + > + switch (type) { > + case hwmon_temp: > + return lenovo_ec_do_read_temp(data, attr, data->temp_map[channel], val); > + case hwmon_fan: > + return lenovo_ec_do_read_fan(data, attr, data->fan_map[channel], val); > + default: > + return -EOPNOTSUPP; > + } > + return 0; Dead code. > +} ... > +static umode_t > +lenovo_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + switch (type) { > + case hwmon_temp: > + if (attr == hwmon_temp_input || attr == hwmon_temp_label) > + return 0444; > + break; > + case hwmon_fan: > + if (attr == hwmon_fan_input || attr == hwmon_fan_max || attr == hwmon_fan_label) > + return 0444; > + break; > + default: > + return 0; > + } > + return 0; Same comment about the style. > +} ... > +static int lenovo_ec_probe(struct platform_device *pdev) { > + struct device *hwdev; > + struct ec_sensors_data *ec_data; > + const struct hwmon_chip_info *chip_info; > + struct device *dev = &pdev->dev; > + const struct dmi_system_id *dmi_id; > + int app_id; > + > + ec_data = devm_kzalloc(dev, sizeof(struct ec_sensors_data), GFP_KERNEL); > + if (!ec_data) { > + release_region(IO_REGION_START, IO_REGION_LENGTH); This is weird. Please, either move request region to the probe, or drop these calls here. Obviously you haven't checked the bind-unbind-bind cycle with error injection. > + return -ENOMEM; > + } > + > + dev_set_drvdata(dev, ec_data); > + > + chip_info = &lenovo_ec_chip_info; > + > + mutex_init(&ec_data->mec_mutex); > + > + mutex_lock(&ec_data->mec_mutex); > + app_id = io_read8(MCHP_EMI0_APPLICATION_ID); > + if (app_id) /* check EMI Application ID Value */ > + io_write8(MCHP_EMI0_APPLICATION_ID, app_id); /* set EMI Application ID to 0 */ > + io_write8(MCHP_EMI0_EC_ADDRESS_LSB, MCHP_SING_IDX); > + io_write8(MCHP_EMI0_EC_ADDRESS_MSB, MCHP_SING_IDX >> 8); > + mutex_unlock(&ec_data->mec_mutex); > + > + if ((io_read8(MCHP_EMI0_EC_DATA_BYTE0) != 'M') && > + (io_read8(MCHP_EMI0_EC_DATA_BYTE1) != 'C') && > + (io_read8(MCHP_EMI0_EC_DATA_BYTE2) != 'H') && > + (io_read8(MCHP_EMI0_EC_DATA_BYTE3) != 'P')) { > + release_region(IO_REGION_START, IO_REGION_LENGTH); > + return -ENODEV; > + } > + > + dmi_id = dmi_first_match(thinkstation_dmi_table); > + > + switch ((long)dmi_id->driver_data) { > + case 0: > + ec_data->fan_labels = px_ec_fan_label; > + ec_data->temp_labels = lenovo_px_ec_temp_label; > + ec_data->fan_map = px_fan_map; > + ec_data->temp_map = px_temp_map; > + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_px; > + break; > + case 1: > + ec_data->fan_labels = p7_ec_fan_label; > + ec_data->temp_labels = lenovo_gen_ec_temp_label; > + ec_data->fan_map = p7_fan_map; > + ec_data->temp_map = gen_temp_map; > + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p7; > + break; > + case 2: > + ec_data->fan_labels = p5_ec_fan_label; > + ec_data->temp_labels = lenovo_gen_ec_temp_label; > + ec_data->fan_map = p5_fan_map; > + ec_data->temp_map = gen_temp_map; > + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p5; > + break; > + case 3: > + ec_data->fan_labels = p8_ec_fan_label; > + ec_data->temp_labels = lenovo_gen_ec_temp_label; > + ec_data->fan_map = p8_fan_map; > + ec_data->temp_map = gen_temp_map; > + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p8; > + break; > + default: > + release_region(IO_REGION_START, IO_REGION_LENGTH); > + return -ENODEV; > + } > + > + hwdev = devm_hwmon_device_register_with_info(dev, "lenovo_ec", > + ec_data, > + chip_info, NULL); > + > + return PTR_ERR_OR_ZERO(hwdev); > +} ... > + if (!request_region(IO_REGION_START, IO_REGION_LENGTH, "LNV-WKS")) { > + pr_err(":request fail\n"); I haven't noticed pr_fmt(). How can the user distinguish this from something similar from another place? > + return -EIO; > + } ... > +static void __exit lenovo_ec_exit(void) { > + release_region(IO_REGION_START, IO_REGION_LENGTH); > + platform_device_unregister(lenovo_ec_sensors_platform_device); > + platform_driver_unregister(&lenovo_ec_sensors_platform_driver); > +} > + Unneeded blank line (see also below). > +module_init(lenovo_ec_init); > +module_exit(lenovo_ec_exit); Move each of them closer to the respective callback implementation. -- With Best Regards, Andy Shevchenko
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ec38c8892158..a4bb403bd4ad 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -862,6 +862,16 @@ config SENSORS_LAN966X This driver can also be built as a module. If so, the module will be called lan966x-hwmon. +config SENSORS_LENOVO_EC + tristate "Sensor reader for Lenovo ThinkStations" + depends on X86 + help + If you say yes here you get support for LENOVO + EC Sensor data on newer ThinkStation systems + + This driver can also be built as a module. If so, the module + will be called lenovo_ec_sensors. + config SENSORS_LINEAGE tristate "Lineage Compact Power Line Power Entry Module" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 4ac9452b5430..aa3c2dc390ec 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -104,6 +104,7 @@ obj-$(CONFIG_SENSORS_JC42) += jc42.o obj-$(CONFIG_SENSORS_K8TEMP) += k8temp.o obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.o obj-$(CONFIG_SENSORS_LAN966X) += lan966x-hwmon.o +obj-$(CONFIG_SENSORS_LENOVO_EC) += lenovo-ec-sensors.o obj-$(CONFIG_SENSORS_LINEAGE) += lineage-pem.o obj-$(CONFIG_SENSORS_LOCHNAGAR) += lochnagar-hwmon.o obj-$(CONFIG_SENSORS_LM63) += lm63.o diff --git a/drivers/hwmon/lenovo-ec-sensors.c b/drivers/hwmon/lenovo-ec-sensors.c new file mode 100644 index 000000000000..0fc32bc11365 --- /dev/null +++ b/drivers/hwmon/lenovo-ec-sensors.c @@ -0,0 +1,615 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * HWMON driver for Lenovo ThinkStation based workstations + * via the embedded controller registers + * + * Copyright (C) 2023 David Ober (Lenovo) <dober@lenovo.com> + * + * EC provides: + * - CPU temperature + * - DIMM temperature + * - Chassis zone temperatures + * - CPU fan RPM + * - DIMM fan RPM + * - Chassis fans RPM + */ +#include <linux/acpi.h> +#include <linux/delay.h> +#include <linux/dmi.h> +#include <linux/hwmon.h> +#include <linux/io.h> +#include <linux/ioport.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/units.h> + +#define MCHP_SING_IDX 0x0000 +#define MCHP_EMI0_APPLICATION_ID 0x090C +#define MCHP_EMI0_EC_ADDRESS_LSB 0x0902 +#define MCHP_EMI0_EC_ADDRESS_MSB 0x0903 +#define MCHP_EMI0_EC_DATA_BYTE0 0x0904 +#define MCHP_EMI0_EC_DATA_BYTE1 0x0905 +#define MCHP_EMI0_EC_DATA_BYTE2 0x0906 +#define MCHP_EMI0_EC_DATA_BYTE3 0x0907 +#define IO_REGION_START 0x0900 +#define IO_REGION_LENGTH 0xD + +#define io_write8(a, b) outb_p(b, a) +#define io_read8(a) inb_p(a) + +static inline uint8_t +get_ec_reg(unsigned char page, unsigned char index) +{ + u8 onebyte; + unsigned short m_index; + unsigned short phy_index = page * 256 + index; + + io_write8(MCHP_EMI0_APPLICATION_ID, 0x01); + + m_index = phy_index & 0x7FFC; + io_write8(MCHP_EMI0_EC_ADDRESS_LSB, m_index); + io_write8(MCHP_EMI0_EC_ADDRESS_MSB, m_index >> 8); + + onebyte = io_read8(MCHP_EMI0_EC_DATA_BYTE0 + (phy_index & 3)); + + io_write8(MCHP_EMI0_APPLICATION_ID, 0x01); /* write 0x01 again to clean */ + return onebyte; +} + +enum systems { + LENOVO_PX, + LENOVO_P7, + LENOVO_P5, + LENOVO_P8, +}; + +static int px_temp_map[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}; + +static const char * const lenovo_px_ec_temp_label[] = { + "CPU1", + "CPU2", + "R_DIMM1", + "L_DIMM1", + "R_DIMM2", + "L_DIMM2", + "PCH", + "M2_R", + "M2_Z1R", + "M2_Z2R", + "PCI_Z1", + "PCI_Z2", + "PCI_Z3", + "PCI_Z4", + "AMB", +}; + +static int gen_temp_map[] = {0, 2, 3, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}; + +static const char * const lenovo_gen_ec_temp_label[] = { + "CPU1", + "R_DIMM", + "L_DIMM", + "PCH", + "M2_R", + "M2_Z1R", + "M2_Z2R", + "PCI_Z1", + "PCI_Z2", + "PCI_Z3", + "PCI_Z4", + "AMB", +}; + +static int px_fan_map[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}; + +static const char * const px_ec_fan_label[] = { + "CPU1_Fan", + "CPU2_Fan", + "Front_Fan1-1", + "Front_Fan1-2", + "Front_Fan2", + "Front_Fan3", + "MEM_Fan1", + "MEM_Fan2", + "Rear_Fan1", + "Rear_Fan2", + "Flex_Bay_Fan1", + "Flex_Bay_Fan2", + "Flex_Bay_Fan2", + "PSU_HDD_Fan", + "PSU1_Fan", + "PSU2_Fan", +}; + +static int p7_fan_map[] = {0, 2, 3, 4, 5, 6, 7, 8, 10, 11, 14}; + +static const char * const p7_ec_fan_label[] = { + "CPU1_Fan", + "HP_CPU_Fan1", + "HP_CPU_Fan2", + "PCIE1_4_Fan", + "PCIE5_7_Fan", + "MEM_Fan1", + "MEM_Fan2", + "Rear_Fan1", + "BCB_Fan", + "Flex_Bay_Fan", + "PSU_Fan", +}; + +static int p5_fan_map[] = {0, 5, 6, 7, 8, 10, 11, 14}; + +static const char * const p5_ec_fan_label[] = { + "CPU_Fan", + "HDD_Fan", + "Duct_Fan1", + "MEM_Fan", + "Rear_Fan", + "Front_Fan", + "Flex_Bay_Fan", + "PSU_Fan", +}; + +static int p8_fan_map[] = {0. 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 14}; + +static const char * const p8_ec_fan_label[] = { + "CPU1_Fan", + "CPU2_Fan", + "HP_CPU_Fan1", + "HP_CPU_Fan2", + "PCIE1_4_Fan", + "PCIE5_7_Fan", + "DIMM1_Fan1", + "DIMM1_Fan2", + "DIMM2_Fan1", + "DIMM2_Fan2", + "Rear_Fan", + "HDD_Bay_Fan", + "Flex_Bay_Fan", + "PSU_Fan", +}; + +struct ec_sensors_data { + struct mutex mec_mutex; /* lock for sensor data access */ + /*int platform_id;*/ + const char *const *fan_labels; + const char *const *temp_labels; + const int *fan_map; + const int *temp_map; +}; + +static int +lenovo_ec_do_read_temp(struct ec_sensors_data *data, u32 attr, int channel, long *val) +{ + u8 lsb; + + switch (attr) { + case hwmon_temp_input: + mutex_lock(&data->mec_mutex); + lsb = get_ec_reg(2, 0x81 + channel); + mutex_unlock(&data->mec_mutex); + if (lsb <= 0x40) + return -ENODATA; + *val = (lsb - 0x40) * 1000; + return 0; + default: + break; + } + return -EOPNOTSUPP; +} + +static int +lenovo_ec_do_read_fan(struct ec_sensors_data *data, u32 attr, int channel, long *val) +{ + u8 lsb, msb; + + channel *= 2; + switch (attr) { + case hwmon_fan_input: + mutex_lock(&data->mec_mutex); + lsb = get_ec_reg(4, 0x20 + channel); + msb = get_ec_reg(4, 0x21 + channel); + mutex_unlock(&data->mec_mutex); + *val = (msb << 8) + lsb; + return 0; + case hwmon_fan_max: + mutex_lock(&data->mec_mutex); + lsb = get_ec_reg(4, 0x40 + channel); + msb = get_ec_reg(4, 0x41 + channel); + mutex_unlock(&data->mec_mutex); + *val = (msb << 8) + lsb; + return 0; + case hwmon_fan_min: + case hwmon_fan_div: + case hwmon_fan_alarm: + break; + default: + break; + } + return -EOPNOTSUPP; +} + +static int +lenovo_ec_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, const char **str) +{ + struct ec_sensors_data *state = dev_get_drvdata(dev); + + switch (type) { + case hwmon_temp: + *str = state->temp_labels[channel]; + break; + case hwmon_fan: + *str = state->fan_labels[channel]; + break; + default: + return -EOPNOTSUPP; + } + return 0; +} + +static int +lenovo_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct ec_sensors_data *data = dev_get_drvdata(dev); + + switch (type) { + case hwmon_temp: + return lenovo_ec_do_read_temp(data, attr, data->temp_map[channel], val); + case hwmon_fan: + return lenovo_ec_do_read_fan(data, attr, data->fan_map[channel], val); + default: + return -EOPNOTSUPP; + } + + return 0; +} + +static umode_t +lenovo_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + switch (type) { + case hwmon_temp: + if (attr == hwmon_temp_input || attr == hwmon_temp_label) + return 0444; + break; + case hwmon_fan: + if (attr == hwmon_fan_input || attr == hwmon_fan_max || attr == hwmon_fan_label) + return 0444; + break; + default: + return 0; + } + return 0; +} + +static const struct hwmon_channel_info *lenovo_ec_hwmon_info_px[] = { + HWMON_CHANNEL_INFO(temp, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL), + HWMON_CHANNEL_INFO(fan, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX), + NULL +}; + +static const struct hwmon_channel_info *lenovo_ec_hwmon_info_p8[] = { + HWMON_CHANNEL_INFO(temp, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL), + HWMON_CHANNEL_INFO(fan, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX), + NULL +}; + +static const struct hwmon_channel_info *lenovo_ec_hwmon_info_p7[] = { + HWMON_CHANNEL_INFO(temp, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL), + HWMON_CHANNEL_INFO(fan, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX), + NULL +}; + +static const struct hwmon_channel_info *lenovo_ec_hwmon_info_p5[] = { + HWMON_CHANNEL_INFO(temp, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL), + HWMON_CHANNEL_INFO(fan, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX, + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MAX), + NULL +}; + +static const struct hwmon_ops lenovo_ec_hwmon_ops = { + .is_visible = lenovo_ec_hwmon_is_visible, + .read = lenovo_ec_hwmon_read, + .read_string = lenovo_ec_hwmon_read_string, +}; + +static struct hwmon_chip_info lenovo_ec_chip_info = { + .ops = &lenovo_ec_hwmon_ops, +}; + +static const struct dmi_system_id thinkstation_dmi_table[] = { + { + .ident = "LENOVO_PX", + .driver_data = (void *)(long)LENOVO_PX, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "30EU"), + }, + }, + { + .ident = "LENOVO_PX", + .driver_data = (void *)(long)LENOVO_PX, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "30EV"), + }, + }, + { + .ident = "LENOVO_P7", + .driver_data = (void *)(long)LENOVO_P7, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "30F2"), + }, + }, + { + .ident = "LENOVO_P7", + .driver_data = (void *)(long)LENOVO_P7, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "30F3"), + }, + }, + { + .ident = "LENOVO_P5", + .driver_data = (void *)(long)LENOVO_P5, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "30G9"), + }, + }, + { + .ident = "LENOVO_P5", + .driver_data = (void *)(long)LENOVO_P5, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "30GA"), + }, + }, + { + .ident = "LENOVO_P8", + .driver_data = (void *)(long)LENOVO_P8, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "30HH"), + }, + }, + { + .ident = "LENOVO_P8", + .driver_data = (void *)(long)LENOVO_P8, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "30HJ"), + }, + }, + {} +}; +MODULE_DEVICE_TABLE(dmi, thinkstation_dmi_table); + +static int lenovo_ec_probe(struct platform_device *pdev) +{ + struct device *hwdev; + struct ec_sensors_data *ec_data; + const struct hwmon_chip_info *chip_info; + struct device *dev = &pdev->dev; + const struct dmi_system_id *dmi_id; + int app_id; + + ec_data = devm_kzalloc(dev, sizeof(struct ec_sensors_data), GFP_KERNEL); + if (!ec_data) { + release_region(IO_REGION_START, IO_REGION_LENGTH); + return -ENOMEM; + } + + dev_set_drvdata(dev, ec_data); + + chip_info = &lenovo_ec_chip_info; + + mutex_init(&ec_data->mec_mutex); + + mutex_lock(&ec_data->mec_mutex); + app_id = io_read8(MCHP_EMI0_APPLICATION_ID); + if (app_id) /* check EMI Application ID Value */ + io_write8(MCHP_EMI0_APPLICATION_ID, app_id); /* set EMI Application ID to 0 */ + io_write8(MCHP_EMI0_EC_ADDRESS_LSB, MCHP_SING_IDX); + io_write8(MCHP_EMI0_EC_ADDRESS_MSB, MCHP_SING_IDX >> 8); + mutex_unlock(&ec_data->mec_mutex); + + if ((io_read8(MCHP_EMI0_EC_DATA_BYTE0) != 'M') && + (io_read8(MCHP_EMI0_EC_DATA_BYTE1) != 'C') && + (io_read8(MCHP_EMI0_EC_DATA_BYTE2) != 'H') && + (io_read8(MCHP_EMI0_EC_DATA_BYTE3) != 'P')) { + release_region(IO_REGION_START, IO_REGION_LENGTH); + return -ENODEV; + } + + dmi_id = dmi_first_match(thinkstation_dmi_table); + + switch ((long)dmi_id->driver_data) { + case 0: + ec_data->fan_labels = px_ec_fan_label; + ec_data->temp_labels = lenovo_px_ec_temp_label; + ec_data->fan_map = px_fan_map; + ec_data->temp_map = px_temp_map; + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_px; + break; + case 1: + ec_data->fan_labels = p7_ec_fan_label; + ec_data->temp_labels = lenovo_gen_ec_temp_label; + ec_data->fan_map = p7_fan_map; + ec_data->temp_map = gen_temp_map; + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p7; + break; + case 2: + ec_data->fan_labels = p5_ec_fan_label; + ec_data->temp_labels = lenovo_gen_ec_temp_label; + ec_data->fan_map = p5_fan_map; + ec_data->temp_map = gen_temp_map; + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p5; + break; + case 3: + ec_data->fan_labels = p8_ec_fan_label; + ec_data->temp_labels = lenovo_gen_ec_temp_label; + ec_data->fan_map = p8_fan_map; + ec_data->temp_map = gen_temp_map; + lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p8; + break; + default: + release_region(IO_REGION_START, IO_REGION_LENGTH); + return -ENODEV; + } + + hwdev = devm_hwmon_device_register_with_info(dev, "lenovo_ec", + ec_data, + chip_info, NULL); + + return PTR_ERR_OR_ZERO(hwdev); +} + +static struct platform_driver lenovo_ec_sensors_platform_driver = { + .driver = { + .name = "lenovo-ec-sensors", + }, + .probe = lenovo_ec_probe, +}; + +static struct platform_device *lenovo_ec_sensors_platform_device; + +static int __init lenovo_ec_init(void) +{ + if (!dmi_check_system(thinkstation_dmi_table)) + return -ENODEV; + + if (!request_region(IO_REGION_START, IO_REGION_LENGTH, "LNV-WKS")) { + pr_err(":request fail\n"); + return -EIO; + } + + lenovo_ec_sensors_platform_device = + platform_create_bundle(&lenovo_ec_sensors_platform_driver, + lenovo_ec_probe, NULL, 0, NULL, 0); + + if (IS_ERR(lenovo_ec_sensors_platform_device)) { + release_region(IO_REGION_START, IO_REGION_LENGTH); + return PTR_ERR(lenovo_ec_sensors_platform_device); + } + + return 0; +} + +static void __exit lenovo_ec_exit(void) +{ + release_region(IO_REGION_START, IO_REGION_LENGTH); + platform_device_unregister(lenovo_ec_sensors_platform_device); + platform_driver_unregister(&lenovo_ec_sensors_platform_driver); +} + +module_init(lenovo_ec_init); +module_exit(lenovo_ec_exit); + +MODULE_AUTHOR("David Ober <dober@lenovo.com>"); +MODULE_DESCRIPTION("HWMON driver for sensors accesssible via EC in LENOVO motherboards"); +MODULE_LICENSE("GPL");