From b9bb92e1d1be921e91d8b469dc1261ea5ac71991 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 13:19:12 -0700 Subject: hwmon: (gpio-fan) Check return value from devm_add_action_or_reset devm_add_action_or_reset() can fail due to a memory allocation failure. Check for it and return the error if that happens. Fixes: 9534784550ab ("hwmon: (gpio-fan) Use devm_thermal_of_cooling_device_register") Signed-off-by: Guenter Roeck --- drivers/hwmon/gpio-fan.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c index 84753680a4e8..54c27e683ee1 100644 --- a/drivers/hwmon/gpio-fan.c +++ b/drivers/hwmon/gpio-fan.c @@ -524,7 +524,9 @@ static int gpio_fan_probe(struct platform_device *pdev) err = fan_ctrl_init(fan_data); if (err) return err; - devm_add_action_or_reset(dev, gpio_fan_stop, fan_data); + err = devm_add_action_or_reset(dev, gpio_fan_stop, fan_data); + if (err) + return err; } /* Make this driver part of hwmon class. */ -- cgit v1.2.3 From 5696e4aaabf2a6bae0fe8b4abf17fe17c2b03beb Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 13:27:06 -0700 Subject: hwmon: (pwm-fan) Check return value from devm_add_action_or_reset devm_add_action_or_reset() can fail due to a memory allocation failure. Check for it and return the error if that happens. Fixes: 37bcec5d9f71 ("hwmon: (pwm-fan) Use devm_thermal_of_cooling_device_register") Signed-off-by: Guenter Roeck --- drivers/hwmon/pwm-fan.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index 08c9b9f1c16e..54c0ff00d67f 100644 --- a/drivers/hwmon/pwm-fan.c +++ b/drivers/hwmon/pwm-fan.c @@ -320,8 +320,10 @@ static int pwm_fan_probe(struct platform_device *pdev) dev_err(dev, "Failed to enable fan supply: %d\n", ret); return ret; } - devm_add_action_or_reset(dev, pwm_fan_regulator_disable, - ctx->reg_en); + ret = devm_add_action_or_reset(dev, pwm_fan_regulator_disable, + ctx->reg_en); + if (ret) + return ret; } ctx->pwm_value = MAX_PWM; @@ -337,7 +339,9 @@ static int pwm_fan_probe(struct platform_device *pdev) return ret; } timer_setup(&ctx->rpm_timer, sample_timer, 0); - devm_add_action_or_reset(dev, pwm_fan_pwm_disable, ctx); + ret = devm_add_action_or_reset(dev, pwm_fan_pwm_disable, ctx); + if (ret) + return ret; of_property_read_u32(dev->of_node, "pulses-per-revolution", &ppr); ctx->pulses_per_revolution = ppr; -- cgit v1.2.3 From c83529c17e12046ac4a8d8fbbc49c51255c7b4da Mon Sep 17 00:00:00 2001 From: "Adamski, Krzysztof (Nokia - PL/Wroclaw)" Date: Wed, 29 May 2019 14:36:21 +0000 Subject: hwmon: (pmbus/adm1275) support PMBUS_VIRT_*_SAMPLES The device supports setting the number of samples for averaging the measurements. There are two separate settings - PWR_AVG for averaging PIN and VI_AVG for averaging VIN/VAUX/IOUT, both being part of PMON_CONFIG register. The values are stored as exponent of base 2 of the actual number of samples that will be taken. Signed-off-by: Krzysztof Adamski Reviewed-by: Alexander Sverdlin [groeck: Dropped unused variables] Signed-off-by: Guenter Roeck --- drivers/hwmon/pmbus/adm1275.c | 59 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c index 82052b6611c9..a477ce0474bb 100644 --- a/drivers/hwmon/pmbus/adm1275.c +++ b/drivers/hwmon/pmbus/adm1275.c @@ -14,6 +14,8 @@ #include #include #include +#include +#include #include "pmbus.h" enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1293, adm1294 }; @@ -69,6 +71,10 @@ enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1293, adm1294 }; #define ADM1075_VAUX_OV_WARN BIT(7) #define ADM1075_VAUX_UV_WARN BIT(6) +#define ADM1275_PWR_AVG_MASK GENMASK(13, 11) +#define ADM1275_VI_AVG_MASK GENMASK(10, 8) +#define ADM1275_SAMPLES_AVG_MAX 128 + struct adm1275_data { int id; bool have_oc_fault; @@ -155,6 +161,32 @@ static const struct coefficients adm1293_coefficients[] = { [18] = { 7658, 0, -3 }, /* power, 21V, irange200 */ }; +static inline int adm1275_read_pmon_config(struct i2c_client *client, u16 mask) +{ + int ret; + + ret = i2c_smbus_read_word_data(client, ADM1275_PMON_CONFIG); + if (ret < 0) + return ret; + + return FIELD_GET(mask, (u16)ret); +} + +static inline int adm1275_write_pmon_config(struct i2c_client *client, u16 mask, + u16 word) +{ + int ret; + + ret = i2c_smbus_read_word_data(client, ADM1275_PMON_CONFIG); + if (ret < 0) + return ret; + + word = FIELD_PREP(mask, word) | (ret & ~mask); + ret = i2c_smbus_write_word_data(client, ADM1275_PMON_CONFIG, word); + + return ret; +} + static int adm1275_read_word_data(struct i2c_client *client, int page, int reg) { const struct pmbus_driver_info *info = pmbus_get_driver_info(client); @@ -233,6 +265,19 @@ static int adm1275_read_word_data(struct i2c_client *client, int page, int reg) if (!data->have_temp_max) return -ENXIO; break; + case PMBUS_VIRT_POWER_SAMPLES: + ret = adm1275_read_pmon_config(client, ADM1275_PWR_AVG_MASK); + if (ret < 0) + break; + ret = BIT(ret); + break; + case PMBUS_VIRT_IN_SAMPLES: + case PMBUS_VIRT_CURR_SAMPLES: + ret = adm1275_read_pmon_config(client, ADM1275_VI_AVG_MASK); + if (ret < 0) + break; + ret = BIT(ret); + break; default: ret = -ENODATA; break; @@ -277,6 +322,17 @@ static int adm1275_write_word_data(struct i2c_client *client, int page, int reg, case PMBUS_VIRT_RESET_TEMP_HISTORY: ret = pmbus_write_word_data(client, 0, ADM1278_PEAK_TEMP, 0); break; + case PMBUS_VIRT_POWER_SAMPLES: + word = clamp_val(word, 1, ADM1275_SAMPLES_AVG_MAX); + ret = adm1275_write_pmon_config(client, ADM1275_PWR_AVG_MASK, + ilog2(word)); + break; + case PMBUS_VIRT_IN_SAMPLES: + case PMBUS_VIRT_CURR_SAMPLES: + word = clamp_val(word, 1, ADM1275_SAMPLES_AVG_MAX); + ret = adm1275_write_pmon_config(client, ADM1275_VI_AVG_MASK, + ilog2(word)); + break; default: ret = -ENODATA; break; @@ -430,7 +486,8 @@ static int adm1275_probe(struct i2c_client *client, info->format[PSC_CURRENT_OUT] = direct; info->format[PSC_POWER] = direct; info->format[PSC_TEMPERATURE] = direct; - info->func[0] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT; + info->func[0] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | + PMBUS_HAVE_SAMPLES; info->read_word_data = adm1275_read_word_data; info->read_byte_data = adm1275_read_byte_data; -- cgit v1.2.3 From 8083034251f1e21271d3849feb0fd74380819f58 Mon Sep 17 00:00:00 2001 From: Alexander Soldatov Date: Wed, 17 Apr 2019 21:03:29 +0300 Subject: hwmon: (occ) Add temp sensor value check The occ driver supports two formats for the temp sensor value. The OCC firmware for P8 supports only the first format, for which no range checking or error processing is performed in the driver. Inspecting the OCC sources for P8 reveals that OCC may send a special value 0xFFFF to indicate that a sensor read timeout has occurred, see https://github.com/open-power/occ/blob/master_p8/src/occ/cmdh/cmdh_fsp_cmds.c#L395 That situation wasn't handled in the driver. This patch adds invalid temp value check for the sensor data format 1 and handles it the same way as it is done for the format 2, where EREMOTEIO is reported for this case. Signed-off-by: Alexander Soldatov Signed-off-by: Alexander Amelkin Reviewed-by: Alexander Amelkin Cc: Edward A. James Cc: Joel Stanley Reviewed-by: Eddie James Signed-off-by: Guenter Roeck --- drivers/hwmon/occ/common.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c index 13a6290c8d25..d593517af5c2 100644 --- a/drivers/hwmon/occ/common.c +++ b/drivers/hwmon/occ/common.c @@ -241,6 +241,12 @@ static ssize_t occ_show_temp_1(struct device *dev, val = get_unaligned_be16(&temp->sensor_id); break; case 1: + /* + * If a sensor reading has expired and couldn't be refreshed, + * OCC returns 0xFFFF for that sensor. + */ + if (temp->value == 0xFFFF) + return -EREMOTEIO; val = get_unaligned_be16(&temp->value) * 1000; break; default: -- cgit v1.2.3 From 344757bac526726ec08f8c710770cc2488569343 Mon Sep 17 00:00:00 2001 From: Vijay Khemka Date: Thu, 30 May 2019 16:11:56 -0700 Subject: hwmon: (pmbus) Add Infineon PXE1610 VR driver Added pmbus driver for the new device Infineon pxe1610 voltage regulator. It also supports similar family device PXE1110 and PXM1310. Signed-off-by: Vijay Khemka Signed-off-by: Guenter Roeck --- drivers/hwmon/pmbus/Kconfig | 9 +++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/pxe1610.c | 139 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+) create mode 100644 drivers/hwmon/pmbus/pxe1610.c diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig index 30751eb9550a..338ef9b5a395 100644 --- a/drivers/hwmon/pmbus/Kconfig +++ b/drivers/hwmon/pmbus/Kconfig @@ -154,6 +154,15 @@ config SENSORS_MAX8688 This driver can also be built as a module. If so, the module will be called max8688. +config SENSORS_PXE1610 + tristate "Infineon PXE1610" + help + If you say yes here you get hardware monitoring support for Infineon + PXE1610. + + This driver can also be built as a module. If so, the module will + be called pxe1610. + config SENSORS_TPS40422 tristate "TI TPS40422" help diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile index 2219b9300316..b0fbd017a91a 100644 --- a/drivers/hwmon/pmbus/Makefile +++ b/drivers/hwmon/pmbus/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_SENSORS_MAX20751) += max20751.o obj-$(CONFIG_SENSORS_MAX31785) += max31785.o obj-$(CONFIG_SENSORS_MAX34440) += max34440.o obj-$(CONFIG_SENSORS_MAX8688) += max8688.o +obj-$(CONFIG_SENSORS_PXE1610) += pxe1610.o obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c new file mode 100644 index 000000000000..ebe3f023f840 --- /dev/null +++ b/drivers/hwmon/pmbus/pxe1610.c @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Hardware monitoring driver for Infineon PXE1610 + * + * Copyright (c) 2019 Facebook Inc + * + */ + +#include +#include +#include +#include +#include +#include "pmbus.h" + +#define PXE1610_NUM_PAGES 3 + +/* Identify chip parameters. */ +static int pxe1610_identify(struct i2c_client *client, + struct pmbus_driver_info *info) +{ + if (pmbus_check_byte_register(client, 0, PMBUS_VOUT_MODE)) { + u8 vout_mode; + int ret; + + /* Read the register with VOUT scaling value.*/ + ret = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE); + if (ret < 0) + return ret; + + vout_mode = ret & GENMASK(4, 0); + + switch (vout_mode) { + case 1: + info->vrm_version = vr12; + break; + case 2: + info->vrm_version = vr13; + break; + default: + return -ENODEV; + } + } + + return 0; +} + +static struct pmbus_driver_info pxe1610_info = { + .pages = PXE1610_NUM_PAGES, + .format[PSC_VOLTAGE_IN] = linear, + .format[PSC_VOLTAGE_OUT] = vid, + .format[PSC_CURRENT_IN] = linear, + .format[PSC_CURRENT_OUT] = linear, + .format[PSC_TEMPERATURE] = linear, + .format[PSC_POWER] = linear, + .func[0] = PMBUS_HAVE_VIN + | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN + | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN + | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP + | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT + | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP, + .func[1] = PMBUS_HAVE_VIN + | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN + | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN + | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP + | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT + | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP, + .func[2] = PMBUS_HAVE_VIN + | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN + | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN + | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP + | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT + | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP, + .identify = pxe1610_identify, +}; + +static int pxe1610_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct pmbus_driver_info *info; + u8 buf[I2C_SMBUS_BLOCK_MAX]; + int ret; + + if (!i2c_check_functionality( + client->adapter, + I2C_FUNC_SMBUS_READ_BYTE_DATA + | I2C_FUNC_SMBUS_READ_WORD_DATA + | I2C_FUNC_SMBUS_READ_BLOCK_DATA)) + return -ENODEV; + + /* + * By default this device doesn't boot to page 0, so set page 0 + * to access all pmbus registers. + */ + i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0); + + /* Read Manufacturer id */ + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf); + if (ret < 0) { + dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n"); + return ret; + } + if (ret != 2 || strncmp(buf, "XP", 2)) { + dev_err(&client->dev, "MFR_ID unrecognized\n"); + return -ENODEV; + } + + info = devm_kmemdup(&client->dev, &pxe1610_info, + sizeof(struct pmbus_driver_info), + GFP_KERNEL); + if (!info) + return -ENOMEM; + + return pmbus_do_probe(client, id, info); +} + +static const struct i2c_device_id pxe1610_id[] = { + {"pxe1610", 0}, + {"pxe1110", 0}, + {"pxm1310", 0}, + {} +}; + +MODULE_DEVICE_TABLE(i2c, pxe1610_id); + +static struct i2c_driver pxe1610_driver = { + .driver = { + .name = "pxe1610", + }, + .probe = pxe1610_probe, + .remove = pmbus_do_remove, + .id_table = pxe1610_id, +}; + +module_i2c_driver(pxe1610_driver); + +MODULE_AUTHOR("Vijay Khemka "); +MODULE_DESCRIPTION("PMBus driver for Infineon PXE1610, PXE1110 and PXM1310"); +MODULE_LICENSE("GPL"); -- cgit v1.2.3 From 37ab356417950bcefce49aa18a2c9d68dfcaab4b Mon Sep 17 00:00:00 2001 From: Vijay Khemka Date: Thu, 30 May 2019 16:11:57 -0700 Subject: hwmon: (pmbus) Document Infineon PXE1610 driver Added documentation for Infineon PXE1610 driver Signed-off-by: Vijay Khemka Signed-off-by: Guenter Roeck --- Documentation/hwmon/pxe1610 | 90 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 Documentation/hwmon/pxe1610 diff --git a/Documentation/hwmon/pxe1610 b/Documentation/hwmon/pxe1610 new file mode 100644 index 000000000000..211cedeefb44 --- /dev/null +++ b/Documentation/hwmon/pxe1610 @@ -0,0 +1,90 @@ +Kernel driver pxe1610 +===================== + +Supported chips: + * Infineon PXE1610 + Prefix: 'pxe1610' + Addresses scanned: - + Datasheet: Datasheet is not publicly available. + + * Infineon PXE1110 + Prefix: 'pxe1110' + Addresses scanned: - + Datasheet: Datasheet is not publicly available. + + * Infineon PXM1310 + Prefix: 'pxm1310' + Addresses scanned: - + Datasheet: Datasheet is not publicly available. + +Author: Vijay Khemka + + +Description +----------- + +PXE1610/PXE1110 are Multi-rail/Multiphase Digital Controllers +and compliant to + -- Intel VR13 DC-DC converter specifications. + -- Intel SVID protocol. +Used for Vcore power regulation for Intel VR13 based microprocessors + -- Servers, Workstations, and High-end desktops + +PXM1310 is a Multi-rail Controller and it is compliant to + -- Intel VR13 DC-DC converter specifications. + -- Intel SVID protocol. +Used for DDR3/DDR4 Memory power regulation for Intel VR13 and +IMVP8 based systems + + +Usage Notes +----------- + +This driver does not probe for PMBus devices. You will have +to instantiate devices explicitly. + +Example: the following commands will load the driver for an PXE1610 +at address 0x70 on I2C bus #4: + +# modprobe pxe1610 +# echo pxe1610 0x70 > /sys/bus/i2c/devices/i2c-4/new_device + +It can also be instantiated by declaring in device tree + + +Sysfs attributes +---------------- + +curr1_label "iin" +curr1_input Measured input current +curr1_alarm Current high alarm + +curr[2-4]_label "iout[1-3]" +curr[2-4]_input Measured output current +curr[2-4]_crit Critical maximum current +curr[2-4]_crit_alarm Current critical high alarm + +in1_label "vin" +in1_input Measured input voltage +in1_crit Critical maximum input voltage +in1_crit_alarm Input voltage critical high alarm + +in[2-4]_label "vout[1-3]" +in[2-4]_input Measured output voltage +in[2-4]_lcrit Critical minimum output voltage +in[2-4]_lcrit_alarm Output voltage critical low alarm +in[2-4]_crit Critical maximum output voltage +in[2-4]_crit_alarm Output voltage critical high alarm + +power1_label "pin" +power1_input Measured input power +power1_alarm Input power high alarm + +power[2-4]_label "pout[1-3]" +power[2-4]_input Measured output power + +temp[1-3]_input Measured temperature +temp[1-3]_crit Critical high temperature +temp[1-3]_crit_alarm Chip temperature critical high alarm +temp[1-3]_max Maximum temperature +temp[1-3]_max_alarm Chip temperature high alarm -- cgit v1.2.3 From b67b7356135a4f969a33cde46359ab1068a75117 Mon Sep 17 00:00:00 2001 From: "amy.shih" Date: Fri, 31 May 2019 10:20:47 +0000 Subject: hwmon: (nct7904) Fix the incorrect value of tcpu_mask in nct7904_data struct. Detect the multi-function of voltage, thermal diode and thermistor from register VT_ADC_MD_REG to set value of tcpu_mask in nct7904_data struct, set temp[1-5]_input the input values TEMP_CH1~4 and LTD of temperature. Set temp[6~13]_input the input values of DTS temperature that correspond to sensors TCPU1~8. Signed-off-by: amy.shih Signed-off-by: Guenter Roeck --- drivers/hwmon/nct7904.c | 72 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 9 deletions(-) diff --git a/drivers/hwmon/nct7904.c b/drivers/hwmon/nct7904.c index 58a957445484..5708171197e7 100644 --- a/drivers/hwmon/nct7904.c +++ b/drivers/hwmon/nct7904.c @@ -4,6 +4,9 @@ * * Copyright (c) 2015 Kontron * Author: Vadim V. Vlasov + * + * Copyright (c) 2019 Advantech + * Author: Amy.Shih */ #include @@ -50,6 +53,8 @@ #define T_CPU1_HV_REG 0xA0 /* Bank 0; 2 regs (HV/LV) per sensor */ #define PRTS_REG 0x03 /* Bank 2 */ +#define PFE_REG 0x00 /* Bank 2; PECI Function Enable */ +#define TSI_CTRL_REG 0x50 /* Bank 2; TSI Control Register */ #define FANCTL1_FMR_REG 0x00 /* Bank 3; 1 reg per channel */ #define FANCTL1_OUT_REG 0x10 /* Bank 3; 1 reg per channel */ @@ -65,6 +70,8 @@ struct nct7904_data { u32 vsen_mask; u32 tcpu_mask; u8 fan_mode[FANCTL_MAX]; + u8 enable_dts; + u8 has_dts; }; /* Access functions */ @@ -229,11 +236,15 @@ static int nct7904_read_temp(struct device *dev, u32 attr, int channel, switch (attr) { case hwmon_temp_input: - if (channel == 0) + if (channel == 4) ret = nct7904_read_reg16(data, BANK_0, LTD_HV_REG); + else if (channel < 5) + ret = nct7904_read_reg16(data, BANK_0, + TEMP_CH1_HV_REG + channel * 4); else ret = nct7904_read_reg16(data, BANK_0, - T_CPU1_HV_REG + (channel - 1) * 2); + T_CPU1_HV_REG + (channel - 5) + * 2); if (ret < 0) return ret; temp = ((ret & 0xff00) >> 5) | (ret & 0x7); @@ -249,11 +260,11 @@ static umode_t nct7904_temp_is_visible(const void *_data, u32 attr, int channel) const struct nct7904_data *data = _data; if (attr == hwmon_temp_input) { - if (channel == 0) { - if (data->vsen_mask & BIT(17)) + if (channel < 5) { + if (data->tcpu_mask & BIT(channel)) return 0444; } else { - if (data->tcpu_mask & BIT(channel - 1)) + if (data->has_dts & BIT(channel - 5)) return 0444; } } @@ -460,6 +471,7 @@ static int nct7904_probe(struct i2c_client *client, struct device *dev = &client->dev; int ret, i; u32 mask; + u8 val, bit; data = devm_kzalloc(dev, sizeof(struct nct7904_data), GFP_KERNEL); if (!data) @@ -493,10 +505,52 @@ static int nct7904_probe(struct i2c_client *client, data->vsen_mask = mask; /* CPU_TEMP attributes */ - ret = nct7904_read_reg16(data, BANK_0, DTS_T_CTRL0_REG); - if (ret < 0) - return ret; - data->tcpu_mask = ((ret >> 8) & 0xf) | ((ret & 0xf) << 4); + ret = nct7904_read_reg(data, BANK_0, VT_ADC_CTRL0_REG); + + if ((ret & 0x6) == 0x6) + data->tcpu_mask |= 1; /* TR1 */ + if ((ret & 0x18) == 0x18) + data->tcpu_mask |= 2; /* TR2 */ + if ((ret & 0x20) == 0x20) + data->tcpu_mask |= 4; /* TR3 */ + if ((ret & 0x80) == 0x80) + data->tcpu_mask |= 8; /* TR4 */ + + /* LTD */ + ret = nct7904_read_reg(data, BANK_0, VT_ADC_CTRL2_REG); + if ((ret & 0x02) == 0x02) + data->tcpu_mask |= 0x10; + + /* Multi-Function detecting for Volt and TR/TD */ + ret = nct7904_read_reg(data, BANK_0, VT_ADC_MD_REG); + + for (i = 0; i < 4; i++) { + val = (ret & (0x03 << i)) >> (i * 2); + bit = (1 << i); + if (val == 0) + data->tcpu_mask &= ~bit; + } + + /* PECI */ + ret = nct7904_read_reg(data, BANK_2, PFE_REG); + if (ret & 0x80) { + data->enable_dts = 1; //Enable DTS & PECI + } else { + ret = nct7904_read_reg(data, BANK_2, TSI_CTRL_REG); + if (ret & 0x80) + data->enable_dts = 0x3; //Enable DTS & TSI + } + + /* Check DTS enable status */ + if (data->enable_dts) { + data->has_dts = + nct7904_read_reg(data, BANK_0, DTS_T_CTRL0_REG) & 0xF; + if (data->enable_dts & 0x2) { + data->has_dts |= + (nct7904_read_reg(data, BANK_0, DTS_T_CTRL1_REG) & 0xF) + << 4; + } + } for (i = 0; i < FANCTL_MAX; i++) { ret = nct7904_read_reg(data, BANK_3, FANCTL1_FMR_REG + i); -- cgit v1.2.3 From 9158411b96b14d9d448e978b0fc8a1cffcb8a1c6 Mon Sep 17 00:00:00 2001 From: Robert Hancock Date: Wed, 5 Jun 2019 13:49:01 -0600 Subject: hwmon: (pmbus) Add Infineon IRPS5401 driver Add a driver to support the Infineon IRPS5401 PMIC. This chip has 5 pages corresponding to 4 switching outputs and one linear (LDO) output. The switching and LDO outputs have slightly different supported parameters. Signed-off-by: Robert Hancock Signed-off-by: Guenter Roeck --- drivers/hwmon/pmbus/Kconfig | 9 ++++++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/irps5401.c | 67 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) create mode 100644 drivers/hwmon/pmbus/irps5401.c diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig index 338ef9b5a395..b6588483fae1 100644 --- a/drivers/hwmon/pmbus/Kconfig +++ b/drivers/hwmon/pmbus/Kconfig @@ -64,6 +64,15 @@ config SENSORS_IR38064 This driver can also be built as a module. If so, the module will be called ir38064. +config SENSORS_IRPS5401 + tristate "Infineon IRPS5401" + help + If you say yes here you get hardware monitoring support for the + Infineon IRPS5401 controller. + + This driver can also be built as a module. If so, the module will + be called irps5401. + config SENSORS_ISL68137 tristate "Intersil ISL68137" help diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile index b0fbd017a91a..c950ea9a5d00 100644 --- a/drivers/hwmon/pmbus/Makefile +++ b/drivers/hwmon/pmbus/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o obj-$(CONFIG_SENSORS_IR35221) += ir35221.o obj-$(CONFIG_SENSORS_IR38064) += ir38064.o +obj-$(CONFIG_SENSORS_IRPS5401) += irps5401.o obj-$(CONFIG_SENSORS_ISL68137) += isl68137.o obj-$(CONFIG_SENSORS_LM25066) += lm25066.o obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o diff --git a/drivers/hwmon/pmbus/irps5401.c b/drivers/hwmon/pmbus/irps5401.c new file mode 100644 index 000000000000..d37daa001fb3 --- /dev/null +++ b/drivers/hwmon/pmbus/irps5401.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Hardware monitoring driver for the Infineon IRPS5401M PMIC. + * + * Copyright (c) 2019 SED Systems, a division of Calian Ltd. + * + * The device supports VOUT_PEAK, IOUT_PEAK, and TEMPERATURE_PEAK, however + * this driver does not currently support them. + */ + +#include +#include +#include +#include +#include +#include "pmbus.h" + +#define IRPS5401_SW_FUNC (PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | \ + PMBUS_HAVE_STATUS_INPUT | \ + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | \ + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | \ + PMBUS_HAVE_PIN | PMBUS_HAVE_POUT | \ + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP) + +#define IRPS5401_LDO_FUNC (PMBUS_HAVE_VIN | \ + PMBUS_HAVE_STATUS_INPUT | \ + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | \ + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | \ + PMBUS_HAVE_PIN | PMBUS_HAVE_POUT | \ + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP) + +static struct pmbus_driver_info irps5401_info = { + .pages = 5, + .func[0] = IRPS5401_SW_FUNC, + .func[1] = IRPS5401_SW_FUNC, + .func[2] = IRPS5401_SW_FUNC, + .func[3] = IRPS5401_SW_FUNC, + .func[4] = IRPS5401_LDO_FUNC, +}; + +static int irps5401_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + return pmbus_do_probe(client, id, &irps5401_info); +} + +static const struct i2c_device_id irps5401_id[] = { + {"irps5401", 0}, + {} +}; + +MODULE_DEVICE_TABLE(i2c, irps5401_id); + +static struct i2c_driver irps5401_driver = { + .driver = { + .name = "irps5401", + }, + .probe = irps5401_probe, + .remove = pmbus_do_remove, + .id_table = irps5401_id, +}; + +module_i2c_driver(irps5401_driver); + +MODULE_AUTHOR("Robert Hancock"); +MODULE_DESCRIPTION("PMBus driver for Infineon IRPS5401"); +MODULE_LICENSE("GPL"); -- cgit v1.2.3 From 5fe625c136367fc7283d021d3ae574fce060d716 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 6 Jun 2019 17:52:42 +0900 Subject: hwmon: (smsc47m1) fix (suspicious) outside array bounds warnings Kbuild test robot reports outside array bounds warnings. This is reproducible for ARCH=sh allmodconfig with the kernel.org toolchains available at: https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/8.1.0/x86_64-gcc-8.1.0-nolibc-sh4-linux.tar.xz CC [M] drivers/hwmon/smsc47m1.o drivers/hwmon/smsc47m1.c: In function 'fan_div_store': drivers/hwmon/smsc47m1.c:370:49: warning: array subscript [0, 2] is outside array bounds of 'u8[3]' {aka 'unsigned char[3]'} [-Warray-bounds] tmp = 192 - (old_div * (192 - data->fan_preload[nr]) ~~~~~~~~~~~~~~~~~^~~~ drivers/hwmon/smsc47m1.c:372:19: warning: array subscript [0, 2] is outside array bounds of 'u8[3]' {aka 'unsigned char[3]'} [-Warray-bounds] data->fan_preload[nr] = clamp_val(tmp, 0, 191); ~~~~~~~~~~~~~~~~~^~~~ drivers/hwmon/smsc47m1.c:373:53: warning: array subscript [0, 2] is outside array bounds of 'const u8[3]' {aka 'const unsigned char[3]'} [-Warray-bounds] smsc47m1_write_value(data, SMSC47M1_REG_FAN_PRELOAD[nr], ~~~~~~~~~~~~~~~~~~~~~~~~^~~~ Looking at the code, I believe these are false positives. While it is ridiculous to patch our driver to make the insane compiler happy, clarifying the unreachable path will be helpful not only for compilers but also for humans. Reported-by: kbuild test robot Signed-off-by: Masahiro Yamada [groeck: Use BUG() instead of unreachable() to make objtool happy] Signed-off-by: Guenter Roeck --- drivers/hwmon/smsc47m1.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c index cc6aca6e436c..b637836b58a1 100644 --- a/drivers/hwmon/smsc47m1.c +++ b/drivers/hwmon/smsc47m1.c @@ -351,6 +351,8 @@ static ssize_t fan_div_store(struct device *dev, tmp |= data->fan_div[2] << 4; smsc47m1_write_value(data, SMSC47M2_REG_FANDIV3, tmp); break; + default: + BUG(); } /* Preserve fan min */ -- cgit v1.2.3 From 792eac1843196708e6f72e73b8f50e273721757e Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Thu, 6 Jun 2019 09:43:14 -0700 Subject: hwmon: (core) Add comment describing how hwdev is freed in error path The hwmon core registers the hwmon device before adding sensors to the thermal core. If that fails, the hwmon device is released and an error is returned to the caller. From the code flow, it appears to be necessary to free struct hwmon_device *, allocated with kzalloc(), in that situation. This is incorrect, since the data structure will be freed automatically in hwmon_dev_release() when device_unregister() is called. This used to result in a double free, which was found and fixed with commit 74e3512731bd ("hwmon: (core) Fix double-free in __hwmon_device_register()"). This is, however, not obvious; any reader may erroneously conclude that the data structure is not freed. Add comment explaining why kfree() is not necessary in this situation. Reported-by: Eduardo Valentin Cc: Eduardo Valentin Signed-off-by: Guenter Roeck --- drivers/hwmon/hwmon.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c index 05e120e01cb4..1f3b30b085b9 100644 --- a/drivers/hwmon/hwmon.c +++ b/drivers/hwmon/hwmon.c @@ -651,6 +651,12 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata, hwdev, j); if (err) { device_unregister(hdev); + /* + * Don't worry about hwdev; + * hwmon_dev_release(), called + * from device_unregister(), + * will free it. + */ goto ida_remove; } } -- cgit v1.2.3 From 8e5e7ddd38b69ef48a2104cd663fab0e3dd03e14 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:16 -0700 Subject: hwmon: (max6650) Use devm function to register thermal device Use devm_thermal_of_cooling_device_register to register the thermal cooling device. This lets us drop the remove function. At the same time, use 'dev' variable in probe function consistently. Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index 6b9056f9483f..e540d0b0145e 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -101,7 +101,6 @@ module_param(clock, int, 0444); struct max6650_data { struct i2c_client *client; const struct attribute_group *groups[3]; - struct thermal_cooling_device *cooling_dev; struct mutex update_lock; int nr_fans; char valid; /* zero until following fields are valid */ @@ -744,6 +743,7 @@ static const struct thermal_cooling_device_ops max6650_cooling_ops = { static int max6650_probe(struct i2c_client *client, const struct i2c_device_id *id) { + struct thermal_cooling_device *cooling_dev; struct device *dev = &client->dev; const struct of_device_id *of_id = of_match_device(of_match_ptr(max6650_dt_match), dev); @@ -780,28 +780,16 @@ static int max6650_probe(struct i2c_client *client, return err; #if IS_ENABLED(CONFIG_THERMAL) - data->cooling_dev = - thermal_of_cooling_device_register(client->dev.of_node, - client->name, data, - &max6650_cooling_ops); - if (IS_ERR(data->cooling_dev)) - dev_warn(&client->dev, - "thermal cooling device register failed: %ld\n", - PTR_ERR(data->cooling_dev)); + cooling_dev = devm_thermal_of_cooling_device_register(dev, dev->of_node, + client->name, data, &max6650_cooling_ops); + if (IS_ERR(cooling_dev)) { + dev_warn(dev, "thermal cooling device register failed: %ld\n", + PTR_ERR(cooling_dev)); + } #endif return 0; } -static int max6650_remove(struct i2c_client *client) -{ - struct max6650_data *data = i2c_get_clientdata(client); - - if (!IS_ERR(data->cooling_dev)) - thermal_cooling_device_unregister(data->cooling_dev); - - return 0; -} - static const struct i2c_device_id max6650_id[] = { { "max6650", 1 }, { "max6651", 4 }, @@ -815,7 +803,6 @@ static struct i2c_driver max6650_driver = { .of_match_table = of_match_ptr(max6650_dt_match), }, .probe = max6650_probe, - .remove = max6650_remove, .id_table = max6650_id, }; -- cgit v1.2.3 From b2905bb8e1f3764c21e449931f3fb75405f7d41e Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:17 -0700 Subject: hwmon: (max6650) Introduce pwm_to_dac and dac_to_pwm Consolidate conversion from pwm value to dac value and from dac value to pwm value into helper functions. While doing this, only update the cached dac value if writing it to the chip was successful after an update. Also, put macro argument of DIV_FROM_REG() into (), and simplify return statement of max6650_set_cur_state(). Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 55 ++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index e540d0b0145e..461484e7828a 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -92,7 +92,8 @@ module_param(clock, int, 0444); #define FAN_RPM_MIN 240 #define FAN_RPM_MAX 30000 -#define DIV_FROM_REG(reg) (1 << (reg & 7)) +#define DIV_FROM_REG(reg) (1 << ((reg) & 7)) +#define DAC_LIMIT(v12) ((v12) ? 180 : 76) /* * Client data (each client gets its own) @@ -136,6 +137,22 @@ static const struct of_device_id __maybe_unused max6650_dt_match[] = { }; MODULE_DEVICE_TABLE(of, max6650_dt_match); +static int dac_to_pwm(int dac, bool v12) +{ + /* + * Useful range for dac is 0-180 for 12V fans and 0-76 for 5V fans. + * Lower DAC values mean higher speeds. + */ + return clamp_val(255 - (255 * dac) / DAC_LIMIT(v12), 0, 255); +} + +static u8 pwm_to_dac(unsigned int pwm, bool v12) +{ + int limit = DAC_LIMIT(v12); + + return limit - (limit * pwm) / 255; +} + static struct max6650_data *max6650_update_device(struct device *dev) { struct max6650_data *data = dev_get_drvdata(dev); @@ -343,22 +360,10 @@ static ssize_t fan1_target_store(struct device *dev, static ssize_t pwm1_show(struct device *dev, struct device_attribute *devattr, char *buf) { - int pwm; struct max6650_data *data = max6650_update_device(dev); - /* - * Useful range for dac is 0-180 for 12V fans and 0-76 for 5V fans. - * Lower DAC values mean higher speeds. - */ - if (data->config & MAX6650_CFG_V12) - pwm = 255 - (255 * (int)data->dac)/180; - else - pwm = 255 - (255 * (int)data->dac)/76; - - if (pwm < 0) - pwm = 0; - - return sprintf(buf, "%d\n", pwm); + return sprintf(buf, "%d\n", dac_to_pwm(data->dac, + data->config & MAX6650_CFG_V12)); } static ssize_t pwm1_store(struct device *dev, @@ -369,6 +374,7 @@ static ssize_t pwm1_store(struct device *dev, struct i2c_client *client = data->client; unsigned long pwm; int err; + u8 dac; err = kstrtoul(buf, 10, &pwm); if (err) @@ -377,13 +383,10 @@ static ssize_t pwm1_store(struct device *dev, pwm = clamp_val(pwm, 0, 255); mutex_lock(&data->update_lock); - - if (data->config & MAX6650_CFG_V12) - data->dac = 180 - (180 * pwm)/255; - else - data->dac = 76 - (76 * pwm)/255; - err = i2c_smbus_write_byte_data(client, MAX6650_REG_DAC, data->dac); - + dac = pwm_to_dac(pwm, data->config & MAX6650_CFG_V12); + err = i2c_smbus_write_byte_data(client, MAX6650_REG_DAC, dac); + if (!err) + data->dac = dac; mutex_unlock(&data->update_lock); return err < 0 ? err : count; @@ -714,11 +717,7 @@ static int max6650_set_cur_state(struct thermal_cooling_device *cdev, mutex_lock(&data->update_lock); - if (data->config & MAX6650_CFG_V12) - data->dac = 180 - (180 * state)/255; - else - data->dac = 76 - (76 * state)/255; - + data->dac = pwm_to_dac(state, data->config & MAX6650_CFG_V12); err = i2c_smbus_write_byte_data(client, MAX6650_REG_DAC, data->dac); if (!err) { @@ -730,7 +729,7 @@ static int max6650_set_cur_state(struct thermal_cooling_device *cdev, mutex_unlock(&data->update_lock); - return err < 0 ? err : 0; + return err; } static const struct thermal_cooling_device_ops max6650_cooling_ops = { -- cgit v1.2.3 From b9d8de4a173dad844f3a3e08dac1364f13ace1e2 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:18 -0700 Subject: hwmon: (max6650) Improve error handling in max6650_init_client Do not overwrite errors reported from i2c functions, and don't ignore any errors. Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index 461484e7828a..caede4d3e21a 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -604,8 +604,8 @@ static int max6650_init_client(struct max6650_data *data, struct i2c_client *client) { struct device *dev = &client->dev; - int config; - int err = -EIO; + int reg; + int err; u32 voltage; u32 prescale; u32 target_rpm; @@ -619,21 +619,20 @@ static int max6650_init_client(struct max6650_data *data, &prescale)) prescale = prescaler; - config = i2c_smbus_read_byte_data(client, MAX6650_REG_CONFIG); - - if (config < 0) { - dev_err(dev, "Error reading config, aborting.\n"); - return err; + reg = i2c_smbus_read_byte_data(client, MAX6650_REG_CONFIG); + if (reg < 0) { + dev_err(dev, "Error reading config register, aborting.\n"); + return reg; } switch (voltage) { case 0: break; case 5: - config &= ~MAX6650_CFG_V12; + reg &= ~MAX6650_CFG_V12; break; case 12: - config |= MAX6650_CFG_V12; + reg |= MAX6650_CFG_V12; break; default: dev_err(dev, "illegal value for fan_voltage (%d)\n", voltage); @@ -643,22 +642,22 @@ static int max6650_init_client(struct max6650_data *data, case 0: break; case 1: - config &= ~MAX6650_CFG_PRESCALER_MASK; + reg &= ~MAX6650_CFG_PRESCALER_MASK; break; case 2: - config = (config & ~MAX6650_CFG_PRESCALER_MASK) + reg = (reg & ~MAX6650_CFG_PRESCALER_MASK) | MAX6650_CFG_PRESCALER_2; break; case 4: - config = (config & ~MAX6650_CFG_PRESCALER_MASK) + reg = (reg & ~MAX6650_CFG_PRESCALER_MASK) | MAX6650_CFG_PRESCALER_4; break; case 8: - config = (config & ~MAX6650_CFG_PRESCALER_MASK) + reg = (reg & ~MAX6650_CFG_PRESCALER_MASK) | MAX6650_CFG_PRESCALER_8; break; case 16: - config = (config & ~MAX6650_CFG_PRESCALER_MASK) + reg = (reg & ~MAX6650_CFG_PRESCALER_MASK) | MAX6650_CFG_PRESCALER_16; break; default: @@ -666,16 +665,22 @@ static int max6650_init_client(struct max6650_data *data, } dev_info(dev, "Fan voltage: %dV, prescaler: %d.\n", - (config & MAX6650_CFG_V12) ? 12 : 5, - 1 << (config & MAX6650_CFG_PRESCALER_MASK)); + (reg & MAX6650_CFG_V12) ? 12 : 5, + 1 << (reg & MAX6650_CFG_PRESCALER_MASK)); - if (i2c_smbus_write_byte_data(client, MAX6650_REG_CONFIG, config)) { + err = i2c_smbus_write_byte_data(client, MAX6650_REG_CONFIG, reg); + if (err) { dev_err(dev, "Config write error, aborting.\n"); return err; } - data->config = config; - data->count = i2c_smbus_read_byte_data(client, MAX6650_REG_COUNT); + data->config = reg; + reg = i2c_smbus_read_byte_data(client, MAX6650_REG_COUNT); + if (reg < 0) { + dev_err(dev, "Failed to read count register, aborting.\n"); + return reg; + } + data->count = reg; if (!of_property_read_u32(client->dev.of_node, "maxim,fan-target-rpm", &target_rpm)) { -- cgit v1.2.3 From bf8c9edaa5c61999ed06c001d87c9a0c86cd0bf8 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:19 -0700 Subject: hwmon: (max6650) Declare valid as boolean Declare valid as boolean to match its use case. Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index caede4d3e21a..90565318aafb 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -104,7 +104,7 @@ struct max6650_data { const struct attribute_group *groups[3]; struct mutex update_lock; int nr_fans; - char valid; /* zero until following fields are valid */ + bool valid; /* false until following fields are valid */ unsigned long last_updated; /* in jiffies */ /* register values */ @@ -183,7 +183,7 @@ static struct max6650_data *max6650_update_device(struct device *dev) MAX6650_REG_ALARM); data->last_updated = jiffies; - data->valid = 1; + data->valid = true; } mutex_unlock(&data->update_lock); -- cgit v1.2.3 From f5b20b11bbc230f64c099b12c37121aacf6f0b65 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:20 -0700 Subject: hwmon: (max6650) Cache alarm_en register The alarm_en register is read each time the is_visible function is called. Since it is a configuration register, this is completely unnecessary. Read it once and cache its value. Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index 90565318aafb..2edee4ca5cae 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -114,6 +114,7 @@ struct max6650_data { u8 count; u8 dac; u8 alarm; + u8 alarm_en; unsigned long cooling_dev_state; }; @@ -545,8 +546,6 @@ static umode_t max6650_attrs_visible(struct kobject *kobj, struct attribute *a, { struct device *dev = container_of(kobj, struct device, kobj); struct max6650_data *data = dev_get_drvdata(dev); - struct i2c_client *client = data->client; - u8 alarm_en = i2c_smbus_read_byte_data(client, MAX6650_REG_ALARM_EN); struct device_attribute *devattr; /* @@ -559,7 +558,7 @@ static umode_t max6650_attrs_visible(struct kobject *kobj, struct attribute *a, || devattr == &sensor_dev_attr_fan1_fault.dev_attr || devattr == &sensor_dev_attr_gpio1_alarm.dev_attr || devattr == &sensor_dev_attr_gpio2_alarm.dev_attr) { - if (!(alarm_en & to_sensor_dev_attr(devattr)->index)) + if (!(data->alarm_en & to_sensor_dev_attr(devattr)->index)) return 0; } @@ -682,6 +681,13 @@ static int max6650_init_client(struct max6650_data *data, } data->count = reg; + reg = i2c_smbus_read_byte_data(client, MAX6650_REG_ALARM_EN); + if (reg < 0) { + dev_err(dev, "Failed to read alarm configuration, aborting.\n"); + return reg; + } + data->alarm_en = reg; + if (!of_property_read_u32(client->dev.of_node, "maxim,fan-target-rpm", &target_rpm)) { max6650_set_target(data, target_rpm); -- cgit v1.2.3 From 0d5cc9383eea0835621cd458c34f1209d483e2ef Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:21 -0700 Subject: hwmon: (max6650) Simplify alarm handling Instead of re-reading the alarm register after reporting an alarm, mark cached values as invalid. While this results in always reading all data on subsequent reads, it is quite unlikely that such reads will actually happen before the cache times out. The upside is avoiding unnecessary unconditional i2c read operations. Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index 2edee4ca5cae..045e67f73846 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -512,15 +512,12 @@ static ssize_t alarm_show(struct device *dev, { struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct max6650_data *data = max6650_update_device(dev); - struct i2c_client *client = data->client; - int alarm = 0; + bool alarm = data->alarm & attr->index; - if (data->alarm & attr->index) { + if (alarm) { mutex_lock(&data->update_lock); - alarm = 1; data->alarm &= ~attr->index; - data->alarm |= i2c_smbus_read_byte_data(client, - MAX6650_REG_ALARM); + data->valid = false; mutex_unlock(&data->update_lock); } -- cgit v1.2.3 From e193acb3d681ba28355ab8bafc49768ac609b94e Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:22 -0700 Subject: hwmon: (max6650) Convert to use devm_hwmon_device_register_with_info Convert driver to use devm_hwmon_device_register_with_info to simplify the code and to reduce its size. Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 508 +++++++++++++++++++++++------------------------- 1 file changed, 247 insertions(+), 261 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index 045e67f73846..bcd50307d963 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -101,7 +101,6 @@ module_param(clock, int, 0444); struct max6650_data { struct i2c_client *client; - const struct attribute_group *groups[3]; struct mutex update_lock; int nr_fans; bool valid; /* false until following fields are valid */ @@ -216,26 +215,6 @@ static int max6650_set_operating_mode(struct max6650_data *data, u8 mode) return 0; } -static ssize_t fan_show(struct device *dev, struct device_attribute *devattr, - char *buf) -{ - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); - struct max6650_data *data = max6650_update_device(dev); - int rpm; - - /* - * Calculation details: - * - * Each tachometer counts over an interval given by the "count" - * register (0.25, 0.5, 1 or 2 seconds). This module assumes - * that the fans produce two pulses per revolution (this seems - * to be the most common). - */ - - rpm = ((data->tach[attr->index] * 120) / DIV_FROM_REG(data->count)); - return sprintf(buf, "%d\n", rpm); -} - /* * Set the fan speed to the specified RPM (or read back the RPM setting). * This works in closed loop mode only. Use pwm1 for open loop speed setting. @@ -277,26 +256,6 @@ static ssize_t fan_show(struct device *dev, struct device_attribute *devattr, * controlled. */ -static ssize_t fan1_target_show(struct device *dev, - struct device_attribute *devattr, char *buf) -{ - struct max6650_data *data = max6650_update_device(dev); - int kscale, ktach, rpm; - - /* - * Use the datasheet equation: - * - * FanSpeed = KSCALE x fCLK / [256 x (KTACH + 1)] - * - * then multiply by 60 to give rpm. - */ - - kscale = DIV_FROM_REG(data->config); - ktach = data->speed; - rpm = 60 * kscale * clock / (256 * (ktach + 1)); - return sprintf(buf, "%d\n", rpm); -} - static int max6650_set_target(struct max6650_data *data, unsigned long rpm) { int kscale, ktach; @@ -325,183 +284,8 @@ static int max6650_set_target(struct max6650_data *data, unsigned long rpm) data->speed); } -static ssize_t fan1_target_store(struct device *dev, - struct device_attribute *devattr, - const char *buf, size_t count) -{ - struct max6650_data *data = dev_get_drvdata(dev); - unsigned long rpm; - int err; - - err = kstrtoul(buf, 10, &rpm); - if (err) - return err; - - mutex_lock(&data->update_lock); - - err = max6650_set_target(data, rpm); - - mutex_unlock(&data->update_lock); - - if (err < 0) - return err; - - return count; -} - -/* - * Get/set the fan speed in open loop mode using pwm1 sysfs file. - * Speed is given as a relative value from 0 to 255, where 255 is maximum - * speed. Note that this is done by writing directly to the chip's DAC, - * it won't change the closed loop speed set by fan1_target. - * Also note that due to rounding errors it is possible that you don't read - * back exactly the value you have set. - */ - -static ssize_t pwm1_show(struct device *dev, struct device_attribute *devattr, - char *buf) -{ - struct max6650_data *data = max6650_update_device(dev); - - return sprintf(buf, "%d\n", dac_to_pwm(data->dac, - data->config & MAX6650_CFG_V12)); -} - -static ssize_t pwm1_store(struct device *dev, - struct device_attribute *devattr, const char *buf, - size_t count) -{ - struct max6650_data *data = dev_get_drvdata(dev); - struct i2c_client *client = data->client; - unsigned long pwm; - int err; - u8 dac; - - err = kstrtoul(buf, 10, &pwm); - if (err) - return err; - - pwm = clamp_val(pwm, 0, 255); - - mutex_lock(&data->update_lock); - dac = pwm_to_dac(pwm, data->config & MAX6650_CFG_V12); - err = i2c_smbus_write_byte_data(client, MAX6650_REG_DAC, dac); - if (!err) - data->dac = dac; - mutex_unlock(&data->update_lock); - - return err < 0 ? err : count; -} - /* - * Get/Set controller mode: - * Possible values: - * 0 = Fan always on - * 1 = Open loop, Voltage is set according to speed, not regulated. - * 2 = Closed loop, RPM for all fans regulated by fan1 tachometer - * 3 = Fan off - */ -static ssize_t pwm1_enable_show(struct device *dev, - struct device_attribute *devattr, char *buf) -{ - struct max6650_data *data = max6650_update_device(dev); - int mode = (data->config & MAX6650_CFG_MODE_MASK) >> 4; - int sysfs_modes[4] = {0, 3, 2, 1}; - - return sprintf(buf, "%d\n", sysfs_modes[mode]); -} - -static ssize_t pwm1_enable_store(struct device *dev, - struct device_attribute *devattr, - const char *buf, size_t count) -{ - struct max6650_data *data = dev_get_drvdata(dev); - unsigned long mode; - int err; - const u8 max6650_modes[] = { - MAX6650_CFG_MODE_ON, - MAX6650_CFG_MODE_OPEN_LOOP, - MAX6650_CFG_MODE_CLOSED_LOOP, - MAX6650_CFG_MODE_OFF, - }; - - err = kstrtoul(buf, 10, &mode); - if (err) - return err; - - if (mode >= ARRAY_SIZE(max6650_modes)) - return -EINVAL; - - mutex_lock(&data->update_lock); - - max6650_set_operating_mode(data, max6650_modes[mode]); - - mutex_unlock(&data->update_lock); - - return count; -} - -/* - * Read/write functions for fan1_div sysfs file. The MAX6650 has no such - * divider. We handle this by converting between divider and counttime: - * - * (counttime == k) <==> (divider == 2^k), k = 0, 1, 2, or 3 - * - * Lower values of k allow to connect a faster fan without the risk of - * counter overflow. The price is lower resolution. You can also set counttime - * using the module parameter. Note that the module parameter "prescaler" also - * influences the behaviour. Unfortunately, there's no sysfs attribute - * defined for that. See the data sheet for details. - */ - -static ssize_t fan1_div_show(struct device *dev, - struct device_attribute *devattr, char *buf) -{ - struct max6650_data *data = max6650_update_device(dev); - - return sprintf(buf, "%d\n", DIV_FROM_REG(data->count)); -} - -static ssize_t fan1_div_store(struct device *dev, - struct device_attribute *devattr, - const char *buf, size_t count) -{ - struct max6650_data *data = dev_get_drvdata(dev); - struct i2c_client *client = data->client; - unsigned long div; - int err; - - err = kstrtoul(buf, 10, &div); - if (err) - return err; - - mutex_lock(&data->update_lock); - switch (div) { - case 1: - data->count = 0; - break; - case 2: - data->count = 1; - break; - case 4: - data->count = 2; - break; - case 8: - data->count = 3; - break; - default: - mutex_unlock(&data->update_lock); - return -EINVAL; - } - - i2c_smbus_write_byte_data(client, MAX6650_REG_COUNT, data->count); - mutex_unlock(&data->update_lock); - - return count; -} - -/* - * Get alarm stati: + * Get gpio alarm status: * Possible values: * 0 = no alarm * 1 = alarm @@ -524,17 +308,6 @@ static ssize_t alarm_show(struct device *dev, return sprintf(buf, "%d\n", alarm); } -static SENSOR_DEVICE_ATTR_RO(fan1_input, fan, 0); -static SENSOR_DEVICE_ATTR_RO(fan2_input, fan, 1); -static SENSOR_DEVICE_ATTR_RO(fan3_input, fan, 2); -static SENSOR_DEVICE_ATTR_RO(fan4_input, fan, 3); -static DEVICE_ATTR_RW(fan1_target); -static DEVICE_ATTR_RW(fan1_div); -static DEVICE_ATTR_RW(pwm1_enable); -static DEVICE_ATTR_RW(pwm1); -static SENSOR_DEVICE_ATTR_RO(fan1_max_alarm, alarm, MAX6650_ALRM_MAX); -static SENSOR_DEVICE_ATTR_RO(fan1_min_alarm, alarm, MAX6650_ALRM_MIN); -static SENSOR_DEVICE_ATTR_RO(fan1_fault, alarm, MAX6650_ALRM_TACH); static SENSOR_DEVICE_ATTR_RO(gpio1_alarm, alarm, MAX6650_ALRM_GPIO1); static SENSOR_DEVICE_ATTR_RO(gpio2_alarm, alarm, MAX6650_ALRM_GPIO2); @@ -550,11 +323,8 @@ static umode_t max6650_attrs_visible(struct kobject *kobj, struct attribute *a, */ devattr = container_of(a, struct device_attribute, attr); - if (devattr == &sensor_dev_attr_fan1_max_alarm.dev_attr - || devattr == &sensor_dev_attr_fan1_min_alarm.dev_attr - || devattr == &sensor_dev_attr_fan1_fault.dev_attr - || devattr == &sensor_dev_attr_gpio1_alarm.dev_attr - || devattr == &sensor_dev_attr_gpio2_alarm.dev_attr) { + if (devattr == &sensor_dev_attr_gpio1_alarm.dev_attr || + devattr == &sensor_dev_attr_gpio2_alarm.dev_attr) { if (!(data->alarm_en & to_sensor_dev_attr(devattr)->index)) return 0; } @@ -563,14 +333,6 @@ static umode_t max6650_attrs_visible(struct kobject *kobj, struct attribute *a, } static struct attribute *max6650_attrs[] = { - &sensor_dev_attr_fan1_input.dev_attr.attr, - &dev_attr_fan1_target.attr, - &dev_attr_fan1_div.attr, - &dev_attr_pwm1_enable.attr, - &dev_attr_pwm1.attr, - &sensor_dev_attr_fan1_max_alarm.dev_attr.attr, - &sensor_dev_attr_fan1_min_alarm.dev_attr.attr, - &sensor_dev_attr_fan1_fault.dev_attr.attr, &sensor_dev_attr_gpio1_alarm.dev_attr.attr, &sensor_dev_attr_gpio2_alarm.dev_attr.attr, NULL @@ -581,21 +343,11 @@ static const struct attribute_group max6650_group = { .is_visible = max6650_attrs_visible, }; -static struct attribute *max6651_attrs[] = { - &sensor_dev_attr_fan2_input.dev_attr.attr, - &sensor_dev_attr_fan3_input.dev_attr.attr, - &sensor_dev_attr_fan4_input.dev_attr.attr, +static const struct attribute_group *max6650_groups[] = { + &max6650_group, NULL }; -static const struct attribute_group max6651_group = { - .attrs = max6651_attrs, -}; - -/* - * Real code - */ - static int max6650_init_client(struct max6650_data *data, struct i2c_client *client) { @@ -747,6 +499,244 @@ static const struct thermal_cooling_device_ops max6650_cooling_ops = { }; #endif +static int max6650_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct max6650_data *data = max6650_update_device(dev); + int mode; + + switch (type) { + case hwmon_pwm: + switch (attr) { + case hwmon_pwm_input: + *val = dac_to_pwm(data->dac, + data->config & MAX6650_CFG_V12); + break; + case hwmon_pwm_enable: + /* + * Possible values: + * 0 = Fan always on + * 1 = Open loop, Voltage is set according to speed, + * not regulated. + * 2 = Closed loop, RPM for all fans regulated by fan1 + * tachometer + * 3 = Fan off + */ + mode = (data->config & MAX6650_CFG_MODE_MASK) >> 4; + *val = (4 - mode) & 3; /* {0 1 2 3} -> {0 3 2 1} */ + break; + default: + return -EOPNOTSUPP; + } + break; + case hwmon_fan: + switch (attr) { + case hwmon_fan_input: + /* + * Calculation details: + * + * Each tachometer counts over an interval given by the + * "count" register (0.25, 0.5, 1 or 2 seconds). + * The driver assumes that the fans produce two pulses + * per revolution (this seems to be the most common). + */ + *val = DIV_ROUND_CLOSEST(data->tach[channel] * 120, + DIV_FROM_REG(data->count)); + break; + case hwmon_fan_div: + *val = DIV_FROM_REG(data->count); + break; + case hwmon_fan_target: + /* + * Use the datasheet equation: + * FanSpeed = KSCALE x fCLK / [256 x (KTACH + 1)] + * then multiply by 60 to give rpm. + */ + *val = 60 * DIV_FROM_REG(data->config) * clock / + (256 * (data->speed + 1)); + break; + case hwmon_fan_min_alarm: + *val = !!(data->alarm & MAX6650_ALRM_MIN); + data->alarm &= ~MAX6650_ALRM_MIN; + data->valid = false; + break; + case hwmon_fan_max_alarm: + *val = !!(data->alarm & MAX6650_ALRM_MAX); + data->alarm &= ~MAX6650_ALRM_MAX; + data->valid = false; + break; + case hwmon_fan_fault: + *val = !!(data->alarm & MAX6650_ALRM_TACH); + data->alarm &= ~MAX6650_ALRM_TACH; + data->valid = false; + break; + default: + return -EOPNOTSUPP; + } + break; + default: + return -EOPNOTSUPP; + } + return 0; +} + +static const u8 max6650_pwm_modes[] = { + MAX6650_CFG_MODE_ON, + MAX6650_CFG_MODE_OPEN_LOOP, + MAX6650_CFG_MODE_CLOSED_LOOP, + MAX6650_CFG_MODE_OFF, +}; + +static int max6650_write(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long val) +{ + struct max6650_data *data = dev_get_drvdata(dev); + int ret = 0; + u8 reg; + + mutex_lock(&data->update_lock); + + switch (type) { + case hwmon_pwm: + switch (attr) { + case hwmon_pwm_input: + reg = pwm_to_dac(clamp_val(val, 0, 255), + data->config & MAX6650_CFG_V12); + ret = i2c_smbus_write_byte_data(data->client, + MAX6650_REG_DAC, reg); + if (ret) + break; + data->dac = reg; + break; + case hwmon_pwm_enable: + if (val < 0 || val >= ARRAY_SIZE(max6650_pwm_modes)) { + ret = -EINVAL; + break; + } + ret = max6650_set_operating_mode(data, + max6650_pwm_modes[val]); + break; + default: + ret = -EOPNOTSUPP; + break; + } + break; + case hwmon_fan: + switch (attr) { + case hwmon_fan_div: + switch (val) { + case 1: + reg = 0; + break; + case 2: + reg = 1; + break; + case 4: + reg = 2; + break; + case 8: + reg = 3; + break; + default: + ret = -EINVAL; + goto error; + } + ret = i2c_smbus_write_byte_data(data->client, + MAX6650_REG_COUNT, reg); + if (ret) + break; + data->count = reg; + break; + case hwmon_fan_target: + if (val < 0) { + ret = -EINVAL; + break; + } + ret = max6650_set_target(data, val); + break; + default: + ret = -EOPNOTSUPP; + break; + } + break; + default: + ret = -EOPNOTSUPP; + break; + } + +error: + mutex_unlock(&data->update_lock); + return ret; +} + +static umode_t max6650_is_visible(const void *_data, + enum hwmon_sensor_types type, u32 attr, + int channel) +{ + const struct max6650_data *data = _data; + + if (channel && (channel >= data->nr_fans || type != hwmon_fan)) + return 0; + + switch (type) { + case hwmon_fan: + switch (attr) { + case hwmon_fan_input: + return 0444; + case hwmon_fan_target: + case hwmon_fan_div: + return 0644; + case hwmon_fan_min_alarm: + if (data->alarm_en & MAX6650_ALRM_MIN) + return 0444; + break; + case hwmon_fan_max_alarm: + if (data->alarm_en & MAX6650_ALRM_MAX) + return 0444; + break; + case hwmon_fan_fault: + if (data->alarm_en & MAX6650_ALRM_TACH) + return 0444; + break; + default: + break; + } + break; + case hwmon_pwm: + switch (attr) { + case hwmon_pwm_input: + case hwmon_pwm_enable: + return 0644; + default: + break; + } + break; + default: + break; + } + return 0; +} + +static const struct hwmon_channel_info *max6650_info[] = { + HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_DIV | + HWMON_F_MIN_ALARM | HWMON_F_MAX_ALARM | + HWMON_F_FAULT, + HWMON_F_INPUT, HWMON_F_INPUT, HWMON_F_INPUT), + HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT | HWMON_PWM_ENABLE), + NULL +}; + +static const struct hwmon_ops max6650_hwmon_ops = { + .read = max6650_read, + .write = max6650_write, + .is_visible = max6650_is_visible, +}; + +static const struct hwmon_chip_info max6650_chip_info = { + .ops = &max6650_hwmon_ops, + .info = max6650_info, +}; + static int max6650_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -774,14 +764,10 @@ static int max6650_probe(struct i2c_client *client, if (err) return err; - data->groups[0] = &max6650_group; - /* 3 additional fan inputs for the MAX6651 */ - if (data->nr_fans == 4) - data->groups[1] = &max6651_group; - - hwmon_dev = devm_hwmon_device_register_with_groups(dev, - client->name, data, - data->groups); + hwmon_dev = devm_hwmon_device_register_with_info(dev, + client->name, data, + &max6650_chip_info, + max6650_groups); err = PTR_ERR_OR_ZERO(hwmon_dev); if (err) return err; -- cgit v1.2.3 From 62dbe50591769f6135b3c8a93c8168b3d177a823 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:23 -0700 Subject: hwmon: (max6650) Read non-volatile registers only once Only tachometer and alarm status registers are modified by the chip. All other registers only need to be read only once, and reading them repeatedly does not add any value. Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index bcd50307d963..6f1a1a6eae46 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -162,17 +162,10 @@ static struct max6650_data *max6650_update_device(struct device *dev) mutex_lock(&data->update_lock); if (time_after(jiffies, data->last_updated + HZ) || !data->valid) { - data->speed = i2c_smbus_read_byte_data(client, - MAX6650_REG_SPEED); - data->config = i2c_smbus_read_byte_data(client, - MAX6650_REG_CONFIG); for (i = 0; i < data->nr_fans; i++) { data->tach[i] = i2c_smbus_read_byte_data(client, tach_reg[i]); } - data->count = i2c_smbus_read_byte_data(client, - MAX6650_REG_COUNT); - data->dac = i2c_smbus_read_byte_data(client, MAX6650_REG_DAC); /* * Alarms are cleared on read in case the condition that @@ -421,8 +414,22 @@ static int max6650_init_client(struct max6650_data *data, dev_err(dev, "Config write error, aborting.\n"); return err; } - data->config = reg; + + reg = i2c_smbus_read_byte_data(client, MAX6650_REG_SPEED); + if (reg < 0) { + dev_err(dev, "Failed to read speed register, aborting.\n"); + return reg; + } + data->speed = reg; + + reg = i2c_smbus_read_byte_data(client, MAX6650_REG_DAC); + if (reg < 0) { + dev_err(dev, "Failed to read DAC register, aborting.\n"); + return reg; + } + data->dac = reg; + reg = i2c_smbus_read_byte_data(client, MAX6650_REG_COUNT); if (reg < 0) { dev_err(dev, "Failed to read count register, aborting.\n"); -- cgit v1.2.3 From 0c4a71d36566a783d795025260a7648447453966 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:24 -0700 Subject: hwmon: (max6650) Improve error handling in max6650_update_device Pass errors from i2c_smbus_read_byte_data() back to the caller of max6650_update_device(). Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index 6f1a1a6eae46..e65792020ca1 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -157,14 +157,19 @@ static struct max6650_data *max6650_update_device(struct device *dev) { struct max6650_data *data = dev_get_drvdata(dev); struct i2c_client *client = data->client; + int reg, err = 0; int i; mutex_lock(&data->update_lock); if (time_after(jiffies, data->last_updated + HZ) || !data->valid) { for (i = 0; i < data->nr_fans; i++) { - data->tach[i] = i2c_smbus_read_byte_data(client, - tach_reg[i]); + reg = i2c_smbus_read_byte_data(client, tach_reg[i]); + if (reg < 0) { + err = reg; + goto error; + } + data->tach[i] = reg; } /* @@ -172,15 +177,20 @@ static struct max6650_data *max6650_update_device(struct device *dev) * caused the alarm is removed. Keep the value latched here * for providing the register through different alarm files. */ - data->alarm |= i2c_smbus_read_byte_data(client, - MAX6650_REG_ALARM); - + reg = i2c_smbus_read_byte_data(client, MAX6650_REG_ALARM); + if (reg < 0) { + err = reg; + goto error; + } + data->alarm |= reg; data->last_updated = jiffies; data->valid = true; } +error: mutex_unlock(&data->update_lock); - + if (err) + data = ERR_PTR(err); return data; } @@ -289,8 +299,12 @@ static ssize_t alarm_show(struct device *dev, { struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct max6650_data *data = max6650_update_device(dev); - bool alarm = data->alarm & attr->index; + bool alarm; + if (IS_ERR(data)) + return PTR_ERR(data); + + alarm = data->alarm & attr->index; if (alarm) { mutex_lock(&data->update_lock); data->alarm &= ~attr->index; @@ -512,6 +526,9 @@ static int max6650_read(struct device *dev, enum hwmon_sensor_types type, struct max6650_data *data = max6650_update_device(dev); int mode; + if (IS_ERR(data)) + return PTR_ERR(data); + switch (type) { case hwmon_pwm: switch (attr) { -- cgit v1.2.3 From 228b9e196a6db701a80e80251ff521688865e6f9 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 7 Jun 2019 10:23:25 -0700 Subject: hwmon: (max6650) Fix minor formatting issues CHECK: struct mutex definition without comment CHECK: Alignment should match open parenthesis Cc: Jean-Francois Dagenais Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index e65792020ca1..5fdad4645cca 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -101,7 +101,7 @@ module_param(clock, int, 0444); struct max6650_data { struct i2c_client *client; - struct mutex update_lock; + struct mutex update_lock; /* protect alarm register updates */ int nr_fans; bool valid; /* false until following fields are valid */ unsigned long last_updated; /* in jiffies */ @@ -319,7 +319,7 @@ static SENSOR_DEVICE_ATTR_RO(gpio1_alarm, alarm, MAX6650_ALRM_GPIO1); static SENSOR_DEVICE_ATTR_RO(gpio2_alarm, alarm, MAX6650_ALRM_GPIO2); static umode_t max6650_attrs_visible(struct kobject *kobj, struct attribute *a, - int n) + int n) { struct device *dev = container_of(kobj, struct device, kobj); struct max6650_data *data = dev_get_drvdata(dev); @@ -500,11 +500,10 @@ static int max6650_set_cur_state(struct thermal_cooling_device *cdev, data->dac = pwm_to_dac(state, data->config & MAX6650_CFG_V12); err = i2c_smbus_write_byte_data(client, MAX6650_REG_DAC, data->dac); - if (!err) { max6650_set_operating_mode(data, state ? - MAX6650_CFG_MODE_OPEN_LOOP : - MAX6650_CFG_MODE_OFF); + MAX6650_CFG_MODE_OPEN_LOOP : + MAX6650_CFG_MODE_OFF); data->cooling_dev_state = state; } -- cgit v1.2.3 From 08d09d8099a7556d49ba0e770c32345cc79566b3 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 11 Jun 2019 19:58:58 +0200 Subject: hwmon: (asus_atk0110) no need to check return value of debugfs_create functions When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Luca Tettamanti Cc: Jean Delvare Cc: Guenter Roeck Cc: linux-hwmon@vger.kernel.org Signed-off-by: Greg Kroah-Hartman Signed-off-by: Guenter Roeck --- drivers/hwmon/asus_atk0110.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/drivers/hwmon/asus_atk0110.c b/drivers/hwmon/asus_atk0110.c index 8dd5b1b8db60..ff64a39d56de 100644 --- a/drivers/hwmon/asus_atk0110.c +++ b/drivers/hwmon/asus_atk0110.c @@ -789,33 +789,16 @@ static const struct file_operations atk_debugfs_ggrp_fops = { static void atk_debugfs_init(struct atk_data *data) { struct dentry *d; - struct dentry *f; data->debugfs.id = 0; d = debugfs_create_dir("asus_atk0110", NULL); - if (!d || IS_ERR(d)) - return; - f = debugfs_create_x32("id", 0600, d, &data->debugfs.id); - if (!f || IS_ERR(f)) - goto cleanup; - - f = debugfs_create_file_unsafe("gitm", 0400, d, data, - &atk_debugfs_gitm); - if (!f || IS_ERR(f)) - goto cleanup; - - f = debugfs_create_file("ggrp", 0400, d, data, - &atk_debugfs_ggrp_fops); - if (!f || IS_ERR(f)) - goto cleanup; + debugfs_create_x32("id", 0600, d, &data->debugfs.id); + debugfs_create_file_unsafe("gitm", 0400, d, data, &atk_debugfs_gitm); + debugfs_create_file("ggrp", 0400, d, data, &atk_debugfs_ggrp_fops); data->debugfs.root = d; - - return; -cleanup: - debugfs_remove_recursive(d); } static void atk_debugfs_cleanup(struct atk_data *data) -- cgit v1.2.3 From e67776cc30894d2840fc0b93fc3b3647677ef0b3 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Mon, 10 Jun 2019 11:51:54 +0200 Subject: hwmon: (lm90) simplify getting the adapter of a client We have a dedicated pointer for that, so use it. Much easier to read and less computation involved. Reported-by: Peter Rosin Signed-off-by: Wolfram Sang Signed-off-by: Guenter Roeck --- drivers/hwmon/lm90.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c index e562a578f20e..2ebcab8b0a9b 100644 --- a/drivers/hwmon/lm90.c +++ b/drivers/hwmon/lm90.c @@ -1718,7 +1718,7 @@ static int lm90_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct device *dev = &client->dev; - struct i2c_adapter *adapter = to_i2c_adapter(dev->parent); + struct i2c_adapter *adapter = client->adapter; struct hwmon_channel_info *info; struct regulator *regulator; struct device *hwmon_dev; -- cgit v1.2.3 From 7d45deb31bec3992cd92a240946fbf872661fa2c Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Wed, 12 Jun 2019 07:39:33 -0700 Subject: hwmon: (pmbus/adm1275) Fix power sampling support Not every chip supported by this driver supports setting the number of samples for power averaging. Also, the power monitoring register is not always a 16-bit register, and the configuration bits used for voltage sampling are different depending on the register width. Some conditional code is needed to fix the problem. On top of all that, the compiler complains about problems with FIELD_GET and FIELD_PREP macros if the file is built with W=1. Avoid using those macros to silence the warning. Cc: Krzysztof Adamski Cc: Alexander Sverdlin Reviewed-by: Krzysztof Adamski Signed-off-by: Guenter Roeck --- drivers/hwmon/pmbus/adm1275.c | 84 +++++++++++++++++++++++++++++++++---------- 1 file changed, 65 insertions(+), 19 deletions(-) diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c index a477ce0474bb..5caa37fbfc18 100644 --- a/drivers/hwmon/pmbus/adm1275.c +++ b/drivers/hwmon/pmbus/adm1275.c @@ -71,9 +71,17 @@ enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1293, adm1294 }; #define ADM1075_VAUX_OV_WARN BIT(7) #define ADM1075_VAUX_UV_WARN BIT(6) -#define ADM1275_PWR_AVG_MASK GENMASK(13, 11) -#define ADM1275_VI_AVG_MASK GENMASK(10, 8) -#define ADM1275_SAMPLES_AVG_MAX 128 +#define ADM1275_VI_AVG_SHIFT 0 +#define ADM1275_VI_AVG_MASK GENMASK(ADM1275_VI_AVG_SHIFT + 2, \ + ADM1275_VI_AVG_SHIFT) +#define ADM1275_SAMPLES_AVG_MAX 128 + +#define ADM1278_PWR_AVG_SHIFT 11 +#define ADM1278_PWR_AVG_MASK GENMASK(ADM1278_PWR_AVG_SHIFT + 2, \ + ADM1278_PWR_AVG_SHIFT) +#define ADM1278_VI_AVG_SHIFT 8 +#define ADM1278_VI_AVG_MASK GENMASK(ADM1278_VI_AVG_SHIFT + 2, \ + ADM1278_VI_AVG_SHIFT) struct adm1275_data { int id; @@ -86,6 +94,7 @@ struct adm1275_data { bool have_pin_min; bool have_pin_max; bool have_temp_max; + bool have_power_sampling; struct pmbus_driver_info info; }; @@ -161,28 +170,58 @@ static const struct coefficients adm1293_coefficients[] = { [18] = { 7658, 0, -3 }, /* power, 21V, irange200 */ }; -static inline int adm1275_read_pmon_config(struct i2c_client *client, u16 mask) +static int adm1275_read_pmon_config(const struct adm1275_data *data, + struct i2c_client *client, bool is_power) { - int ret; - - ret = i2c_smbus_read_word_data(client, ADM1275_PMON_CONFIG); + int shift, ret; + u16 mask; + + /* + * The PMON configuration register is a 16-bit register only on chips + * supporting power average sampling. On other chips it is an 8-bit + * register. + */ + if (data->have_power_sampling) { + ret = i2c_smbus_read_word_data(client, ADM1275_PMON_CONFIG); + mask = is_power ? ADM1278_PWR_AVG_MASK : ADM1278_VI_AVG_MASK; + shift = is_power ? ADM1278_PWR_AVG_SHIFT : ADM1278_VI_AVG_SHIFT; + } else { + ret = i2c_smbus_read_byte_data(client, ADM1275_PMON_CONFIG); + mask = ADM1275_VI_AVG_MASK; + shift = ADM1275_VI_AVG_SHIFT; + } if (ret < 0) return ret; - return FIELD_GET(mask, (u16)ret); + return (ret & mask) >> shift; } -static inline int adm1275_write_pmon_config(struct i2c_client *client, u16 mask, - u16 word) +static int adm1275_write_pmon_config(const struct adm1275_data *data, + struct i2c_client *client, + bool is_power, u16 word) { - int ret; - - ret = i2c_smbus_read_word_data(client, ADM1275_PMON_CONFIG); + int shift, ret; + u16 mask; + + if (data->have_power_sampling) { + ret = i2c_smbus_read_word_data(client, ADM1275_PMON_CONFIG); + mask = is_power ? ADM1278_PWR_AVG_MASK : ADM1278_VI_AVG_MASK; + shift = is_power ? ADM1278_PWR_AVG_SHIFT : ADM1278_VI_AVG_SHIFT; + } else { + ret = i2c_smbus_read_byte_data(client, ADM1275_PMON_CONFIG); + mask = ADM1275_VI_AVG_MASK; + shift = ADM1275_VI_AVG_SHIFT; + } if (ret < 0) return ret; - word = FIELD_PREP(mask, word) | (ret & ~mask); - ret = i2c_smbus_write_word_data(client, ADM1275_PMON_CONFIG, word); + word = (ret & ~mask) | ((word << shift) & mask); + if (data->have_power_sampling) + ret = i2c_smbus_write_word_data(client, ADM1275_PMON_CONFIG, + word); + else + ret = i2c_smbus_write_byte_data(client, ADM1275_PMON_CONFIG, + word); return ret; } @@ -266,14 +305,16 @@ static int adm1275_read_word_data(struct i2c_client *client, int page, int reg) return -ENXIO; break; case PMBUS_VIRT_POWER_SAMPLES: - ret = adm1275_read_pmon_config(client, ADM1275_PWR_AVG_MASK); + if (!data->have_power_sampling) + return -ENXIO; + ret = adm1275_read_pmon_config(data, client, true); if (ret < 0) break; ret = BIT(ret); break; case PMBUS_VIRT_IN_SAMPLES: case PMBUS_VIRT_CURR_SAMPLES: - ret = adm1275_read_pmon_config(client, ADM1275_VI_AVG_MASK); + ret = adm1275_read_pmon_config(data, client, false); if (ret < 0) break; ret = BIT(ret); @@ -323,14 +364,16 @@ static int adm1275_write_word_data(struct i2c_client *client, int page, int reg, ret = pmbus_write_word_data(client, 0, ADM1278_PEAK_TEMP, 0); break; case PMBUS_VIRT_POWER_SAMPLES: + if (!data->have_power_sampling) + return -ENXIO; word = clamp_val(word, 1, ADM1275_SAMPLES_AVG_MAX); - ret = adm1275_write_pmon_config(client, ADM1275_PWR_AVG_MASK, + ret = adm1275_write_pmon_config(data, client, true, ilog2(word)); break; case PMBUS_VIRT_IN_SAMPLES: case PMBUS_VIRT_CURR_SAMPLES: word = clamp_val(word, 1, ADM1275_SAMPLES_AVG_MAX); - ret = adm1275_write_pmon_config(client, ADM1275_VI_AVG_MASK, + ret = adm1275_write_pmon_config(data, client, false, ilog2(word)); break; default: @@ -528,6 +571,7 @@ static int adm1275_probe(struct i2c_client *client, data->have_vout = true; data->have_pin_max = true; data->have_temp_max = true; + data->have_power_sampling = true; coefficients = adm1272_coefficients; vindex = (config & ADM1275_VRANGE) ? 1 : 0; @@ -613,6 +657,7 @@ static int adm1275_probe(struct i2c_client *client, data->have_vout = true; data->have_pin_max = true; data->have_temp_max = true; + data->have_power_sampling = true; coefficients = adm1278_coefficients; vindex = 0; @@ -648,6 +693,7 @@ static int adm1275_probe(struct i2c_client *client, data->have_pin_min = true; data->have_pin_max = true; data->have_mfr_vaux_status = true; + data->have_power_sampling = true; coefficients = adm1293_coefficients; -- cgit v1.2.3 From 3253854dc19f1610b8d01fb9265bbd98ce18abd7 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Mon, 17 Jun 2019 14:34:30 +0200 Subject: hwmon: (max6650) Fix unused variable warning The newly added variable is only used in an #if block: drivers/hwmon/max6650.c: In function 'max6650_probe': drivers/hwmon/max6650.c:766:33: error: unused variable 'cooling_dev' [-Werror=unused-variable] Change the #if to if() so the compiler can see what is actually going on. Fixes: a8463754a5a9 ("hwmon: (max6650) Use devm function to register thermal device") Signed-off-by: Arnd Bergmann Signed-off-by: Guenter Roeck --- drivers/hwmon/max6650.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index 5fdad4645cca..3d9d371c35b5 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -467,8 +467,6 @@ static int max6650_init_client(struct max6650_data *data, return 0; } -#if IS_ENABLED(CONFIG_THERMAL) - static int max6650_get_max_state(struct thermal_cooling_device *cdev, unsigned long *state) { @@ -517,7 +515,6 @@ static const struct thermal_cooling_device_ops max6650_cooling_ops = { .get_cur_state = max6650_get_cur_state, .set_cur_state = max6650_set_cur_state, }; -#endif static int max6650_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel, long *val) @@ -795,14 +792,16 @@ static int max6650_probe(struct i2c_client *client, if (err) return err; -#if IS_ENABLED(CONFIG_THERMAL) - cooling_dev = devm_thermal_of_cooling_device_register(dev, dev->of_node, - client->name, data, &max6650_cooling_ops); - if (IS_ERR(cooling_dev)) { - dev_warn(dev, "thermal cooling device register failed: %ld\n", - PTR_ERR(cooling_dev)); + if (IS_ENABLED(CONFIG_THERMAL)) { + cooling_dev = devm_thermal_of_cooling_device_register(dev, + dev->of_node, client->name, + data, &max6650_cooling_ops); + if (IS_ERR(cooling_dev)) { + dev_warn(dev, "thermal cooling device register failed: %ld\n", + PTR_ERR(cooling_dev)); + } } -#endif + return 0; } -- cgit v1.2.3 From 23297edbc15ae19917253064c81f91c4df240f93 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Thu, 20 Jun 2019 06:14:42 -0700 Subject: hwmon: Convert remaining drivers to use SPDX identifier This gets rid of the unnecessary license boilerplate, and avoids having to deal with individual patches one by one. No functional changes intended. Reviewed-by: Corentin Labbe Reviewed-by: Jean Delvare Signed-off-by: Guenter Roeck --- drivers/hwmon/adm1029.c | 10 ---------- drivers/hwmon/scpi-hwmon.c | 10 +--------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/drivers/hwmon/adm1029.c b/drivers/hwmon/adm1029.c index 388060ff85e7..f7752a5bef31 100644 --- a/drivers/hwmon/adm1029.c +++ b/drivers/hwmon/adm1029.c @@ -10,16 +10,6 @@ * Very rare chip please let me know if you use it * * http://www.analog.com/UploadedFiles/Data_Sheets/ADM1029.pdf - * - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation version 2 of the License - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #include diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c index 9bfa228d0eb0..25aac40f2764 100644 --- a/drivers/hwmon/scpi-hwmon.c +++ b/drivers/hwmon/scpi-hwmon.c @@ -1,17 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /* * System Control and Power Interface(SCPI) based hwmon sensor driver * * Copyright (C) 2015 ARM Ltd. * Punit Agrawal - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - * - * This program is distributed "as is" WITHOUT ANY WARRANTY of any - * kind, whether express or implied; without even the implied warranty - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #include -- cgit v1.2.3 From b3e26067874700fb38aeddf2da9844afb36f1a94 Mon Sep 17 00:00:00 2001 From: "amy.shih" Date: Mon, 17 Jun 2019 08:08:50 +0000 Subject: hwmon: (nct7904) Add error handling in probe function. When register read and write operations return errors, needs to add error handling. Signed-off-by: amy.shih Signed-off-by: Guenter Roeck --- drivers/hwmon/nct7904.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/hwmon/nct7904.c b/drivers/hwmon/nct7904.c index 5708171197e7..401ed4a4a576 100644 --- a/drivers/hwmon/nct7904.c +++ b/drivers/hwmon/nct7904.c @@ -506,6 +506,8 @@ static int nct7904_probe(struct i2c_client *client, /* CPU_TEMP attributes */ ret = nct7904_read_reg(data, BANK_0, VT_ADC_CTRL0_REG); + if (ret < 0) + return ret; if ((ret & 0x6) == 0x6) data->tcpu_mask |= 1; /* TR1 */ @@ -518,11 +520,15 @@ static int nct7904_probe(struct i2c_client *client, /* LTD */ ret = nct7904_read_reg(data, BANK_0, VT_ADC_CTRL2_REG); + if (ret < 0) + return ret; if ((ret & 0x02) == 0x02) data->tcpu_mask |= 0x10; /* Multi-Function detecting for Volt and TR/TD */ ret = nct7904_read_reg(data, BANK_0, VT_ADC_MD_REG); + if (ret < 0) + return ret; for (i = 0; i < 4; i++) { val = (ret & (0x03 << i)) >> (i * 2); @@ -533,22 +539,29 @@ static int nct7904_probe(struct i2c_client *client, /* PECI */ ret = nct7904_read_reg(data, BANK_2, PFE_REG); + if (ret < 0) + return ret; if (ret & 0x80) { data->enable_dts = 1; //Enable DTS & PECI } else { ret = nct7904_read_reg(data, BANK_2, TSI_CTRL_REG); + if (ret < 0) + return ret; if (ret & 0x80) data->enable_dts = 0x3; //Enable DTS & TSI } /* Check DTS enable status */ if (data->enable_dts) { - data->has_dts = - nct7904_read_reg(data, BANK_0, DTS_T_CTRL0_REG) & 0xF; + ret = nct7904_read_reg(data, BANK_0, DTS_T_CTRL0_REG); + if (ret < 0) + return ret; + data->has_dts = ret & 0xF; if (data->enable_dts & 0x2) { - data->has_dts |= - (nct7904_read_reg(data, BANK_0, DTS_T_CTRL1_REG) & 0xF) - << 4; + ret = nct7904_read_reg(data, BANK_0, DTS_T_CTRL1_REG); + if (ret < 0) + return ret; + data->has_dts |= (ret & 0xF) << 4; } } -- cgit v1.2.3 From a653acf00d07430bbb0af06a4f5cc2073bbefde7 Mon Sep 17 00:00:00 2001 From: "amy.shih" Date: Mon, 17 Jun 2019 08:10:00 +0000 Subject: hwmon: (nct7904) Changes comments in probe function. Linux style for comments is the C89 "/* ... */" style, changes the comments to Linux style. Signed-off-by: amy.shih Signed-off-by: Guenter Roeck --- drivers/hwmon/nct7904.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/nct7904.c b/drivers/hwmon/nct7904.c index 401ed4a4a576..710c30562fc1 100644 --- a/drivers/hwmon/nct7904.c +++ b/drivers/hwmon/nct7904.c @@ -542,13 +542,13 @@ static int nct7904_probe(struct i2c_client *client, if (ret < 0) return ret; if (ret & 0x80) { - data->enable_dts = 1; //Enable DTS & PECI + data->enable_dts = 1; /* Enable DTS & PECI */ } else { ret = nct7904_read_reg(data, BANK_2, TSI_CTRL_REG); if (ret < 0) return ret; if (ret & 0x80) - data->enable_dts = 0x3; //Enable DTS & TSI + data->enable_dts = 0x3; /* Enable DTS & TSI */ } /* Check DTS enable status */ -- cgit v1.2.3 From 62456189f3292c62f87aef363f204886dc1d4b48 Mon Sep 17 00:00:00 2001 From: Boyang Yu Date: Fri, 28 Jun 2019 19:06:36 +0000 Subject: hwmon: (lm90) Fix max6658 sporadic wrong temperature reading max6658 may report unrealistically high temperature during the driver initialization, for which, its overtemp alarm pin also gets asserted. For certain devices implementing overtemp protection based on that pin, it may further trigger a reset to the device. By reproducing the problem, the wrong reading is found to be coincident with changing the conversion rate. To mitigate this issue, set the stop bit before changing the conversion rate and unset it thereafter. After such change, the wrong reading is not reproduced. Apply this change only to the max6657 kind for now, controlled by flag LM90_PAUSE_ON_CONFIG. Signed-off-by: Boyang Yu Signed-off-by: Guenter Roeck --- drivers/hwmon/lm90.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c index 2ebcab8b0a9b..40bb308d8dd7 100644 --- a/drivers/hwmon/lm90.c +++ b/drivers/hwmon/lm90.c @@ -174,6 +174,7 @@ enum chips { lm90, adm1032, lm99, lm86, max6657, max6659, adt7461, max6680, #define LM90_HAVE_EMERGENCY_ALARM (1 << 5)/* emergency alarm */ #define LM90_HAVE_TEMP3 (1 << 6) /* 3rd temperature sensor */ #define LM90_HAVE_BROKEN_ALERT (1 << 7) /* Broken alert */ +#define LM90_PAUSE_FOR_CONFIG (1 << 8) /* Pause conversion for config */ /* LM90 status */ #define LM90_STATUS_LTHRM (1 << 0) /* local THERM limit tripped */ @@ -367,6 +368,7 @@ static const struct lm90_params lm90_params[] = { .reg_local_ext = MAX6657_REG_R_LOCAL_TEMPL, }, [max6657] = { + .flags = LM90_PAUSE_FOR_CONFIG, .alert_alarms = 0x7c, .max_convrate = 8, .reg_local_ext = MAX6657_REG_R_LOCAL_TEMPL, @@ -567,6 +569,38 @@ static inline int lm90_select_remote_channel(struct i2c_client *client, return 0; } +static int lm90_write_convrate(struct i2c_client *client, + struct lm90_data *data, int val) +{ + int err; + int config_orig, config_stop; + + /* Save config and pause conversion */ + if (data->flags & LM90_PAUSE_FOR_CONFIG) { + config_orig = lm90_read_reg(client, LM90_REG_R_CONFIG1); + if (config_orig < 0) + return config_orig; + config_stop = config_orig | 0x40; + if (config_orig != config_stop) { + err = i2c_smbus_write_byte_data(client, + LM90_REG_W_CONFIG1, + config_stop); + if (err < 0) + return err; + } + } + + /* Set conv rate */ + err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONVRATE, val); + + /* Revert change to config */ + if (data->flags & LM90_PAUSE_FOR_CONFIG && config_orig != config_stop) + i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, + config_orig); + + return err; +} + /* * Set conversion rate. * client->update_lock must be held when calling this function (unless we are @@ -587,7 +621,7 @@ static int lm90_set_convrate(struct i2c_client *client, struct lm90_data *data, if (interval >= update_interval * 3 / 4) break; - err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONVRATE, i); + err = lm90_write_convrate(client, data, i); data->update_interval = DIV_ROUND_CLOSEST(update_interval, 64); return err; } @@ -1593,8 +1627,7 @@ static void lm90_restore_conf(void *_data) struct i2c_client *client = data->client; /* Restore initial configuration */ - i2c_smbus_write_byte_data(client, LM90_REG_W_CONVRATE, - data->convrate_orig); + lm90_write_convrate(client, data, data->convrate_orig); i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, data->config_orig); } @@ -1611,12 +1644,13 @@ static int lm90_init_client(struct i2c_client *client, struct lm90_data *data) /* * Start the conversions. */ - lm90_set_convrate(client, data, 500); /* 500ms; 2Hz conversion rate */ config = lm90_read_reg(client, LM90_REG_R_CONFIG1); if (config < 0) return config; data->config_orig = config; + lm90_set_convrate(client, data, 500); /* 500ms; 2Hz conversion rate */ + /* Check Temperature Range Select */ if (data->kind == adt7461 || data->kind == tmp451) { if (config & 0x04) -- cgit v1.2.3 From b849e5d18c366eae8185837bd7c4adbd27213fe2 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Sun, 30 Jun 2019 15:14:19 -0700 Subject: hwmon: (lm90) Cache configuration register value The configuration register does not change on its own. Yet, it is read in various locations, modified, and written back. Simplify and optimize the code by caching its value and by only writing it back when needed. Cc: Boyang Yu Signed-off-by: Guenter Roeck --- drivers/hwmon/lm90.c | 59 +++++++++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c index 40bb308d8dd7..7f35ea0044fd 100644 --- a/drivers/hwmon/lm90.c +++ b/drivers/hwmon/lm90.c @@ -459,6 +459,7 @@ struct lm90_data { unsigned int update_interval; /* in milliseconds */ + u8 config; /* Current configuration register value */ u8 config_orig; /* Original configuration register value */ u8 convrate_orig; /* Original conversion rate register value */ u16 alert_alarms; /* Which alarm bits trigger ALERT# */ @@ -554,17 +555,20 @@ static inline int lm90_select_remote_channel(struct i2c_client *client, struct lm90_data *data, int channel) { - int config; - if (data->kind == max6696) { - config = lm90_read_reg(client, LM90_REG_R_CONFIG1); - if (config < 0) - return config; - config &= ~0x08; + u8 config = data->config & ~0x08; + int err; + if (channel) config |= 0x08; - i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, - config); + if (data->config != config) { + err = i2c_smbus_write_byte_data(client, + LM90_REG_W_CONFIG1, + config); + if (err) + return err; + data->config = config; + } } return 0; } @@ -572,19 +576,16 @@ static inline int lm90_select_remote_channel(struct i2c_client *client, static int lm90_write_convrate(struct i2c_client *client, struct lm90_data *data, int val) { + u8 config = data->config; int err; - int config_orig, config_stop; /* Save config and pause conversion */ if (data->flags & LM90_PAUSE_FOR_CONFIG) { - config_orig = lm90_read_reg(client, LM90_REG_R_CONFIG1); - if (config_orig < 0) - return config_orig; - config_stop = config_orig | 0x40; - if (config_orig != config_stop) { + config |= 0x40; + if (data->config != config) { err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, - config_stop); + config); if (err < 0) return err; } @@ -594,9 +595,9 @@ static int lm90_write_convrate(struct i2c_client *client, err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONVRATE, val); /* Revert change to config */ - if (data->flags & LM90_PAUSE_FOR_CONFIG && config_orig != config_stop) + if (data->config != config) i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, - config_orig); + data->config); return err; } @@ -802,15 +803,12 @@ static int lm90_update_device(struct device *dev) */ if (!(data->config_orig & 0x80) && !(data->alarms & data->alert_alarms)) { - val = lm90_read_reg(client, LM90_REG_R_CONFIG1); - if (val < 0) - return val; - - if (val & 0x80) { + if (data->config & 0x80) { dev_dbg(&client->dev, "Re-enabling ALERT#\n"); + data->config &= ~0x80; i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, - val & ~0x80); + data->config); } } @@ -1648,6 +1646,7 @@ static int lm90_init_client(struct i2c_client *client, struct lm90_data *data) if (config < 0) return config; data->config_orig = config; + data->config = config; lm90_set_convrate(client, data, 500); /* 500ms; 2Hz conversion rate */ @@ -1672,8 +1671,10 @@ static int lm90_init_client(struct i2c_client *client, struct lm90_data *data) config &= ~0x08; config &= 0xBF; /* run */ - if (config != data->config_orig) /* Only write if changed */ + if (config != data->config) { /* Only write if changed */ i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, config); + data->config = config; + } return devm_add_action_or_reset(&client->dev, lm90_restore_conf, data); } @@ -1907,14 +1908,10 @@ static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type, if ((data->flags & LM90_HAVE_BROKEN_ALERT) && (alarms & data->alert_alarms)) { - int config; - dev_dbg(&client->dev, "Disabling ALERT#\n"); - config = lm90_read_reg(client, LM90_REG_R_CONFIG1); - if (config >= 0) - i2c_smbus_write_byte_data(client, - LM90_REG_W_CONFIG1, - config | 0x80); + data->config |= 0x80; + i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, + data->config); } } else { dev_info(&client->dev, "Everything OK\n"); -- cgit v1.2.3 From 7a1d220ccb0cc2b808eb176fb05bf55a38179f3f Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Sun, 30 Jun 2019 15:40:34 -0700 Subject: hwmon: (lm90) Introduce function to update configuration register The code to update the configuration register is repeated several times. Move it into a separate function. At the same time, un-inline lm90_select_remote_channel() and leave it up to the compiler to decide what to do with it. Also remove the 'client' argument from lm90_select_remote_channel() and from lm90_write_convrate() and take it from struct lm90_data instead where needed. This patch reduces code size by more than 800 bytes on x86_64. Cc: Boyang Yu Signed-off-by: Guenter Roeck --- drivers/hwmon/lm90.c | 89 +++++++++++++++++++++++----------------------------- 1 file changed, 40 insertions(+), 49 deletions(-) diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c index 7f35ea0044fd..9b3c9f390ef8 100644 --- a/drivers/hwmon/lm90.c +++ b/drivers/hwmon/lm90.c @@ -543,6 +543,21 @@ static int lm90_read16(struct i2c_client *client, u8 regh, u8 regl) return (newh << 8) | l; } +static int lm90_update_confreg(struct lm90_data *data, u8 config) +{ + if (data->config != config) { + int err; + + err = i2c_smbus_write_byte_data(data->client, + LM90_REG_W_CONFIG1, + config); + if (err) + return err; + data->config = config; + } + return 0; +} + /* * client->update_lock must be held when calling this function (unless we are * in detection or initialization steps), and while a remote channel other @@ -551,53 +566,37 @@ static int lm90_read16(struct i2c_client *client, u8 regh, u8 regl) * various registers have different meanings as a result of selecting a * non-default remote channel. */ -static inline int lm90_select_remote_channel(struct i2c_client *client, - struct lm90_data *data, - int channel) +static int lm90_select_remote_channel(struct lm90_data *data, int channel) { + int err = 0; + if (data->kind == max6696) { u8 config = data->config & ~0x08; - int err; if (channel) config |= 0x08; - if (data->config != config) { - err = i2c_smbus_write_byte_data(client, - LM90_REG_W_CONFIG1, - config); - if (err) - return err; - data->config = config; - } + err = lm90_update_confreg(data, config); } - return 0; + return err; } -static int lm90_write_convrate(struct i2c_client *client, - struct lm90_data *data, int val) +static int lm90_write_convrate(struct lm90_data *data, int val) { u8 config = data->config; int err; /* Save config and pause conversion */ if (data->flags & LM90_PAUSE_FOR_CONFIG) { - config |= 0x40; - if (data->config != config) { - err = i2c_smbus_write_byte_data(client, - LM90_REG_W_CONFIG1, - config); - if (err < 0) - return err; - } + err = lm90_update_confreg(data, config | 0x40); + if (err < 0) + return err; } /* Set conv rate */ - err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONVRATE, val); + err = i2c_smbus_write_byte_data(data->client, LM90_REG_W_CONVRATE, val); /* Revert change to config */ - if (data->config != config) - i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, - data->config); + lm90_update_confreg(data, config); return err; } @@ -622,7 +621,7 @@ static int lm90_set_convrate(struct i2c_client *client, struct lm90_data *data, if (interval >= update_interval * 3 / 4) break; - err = lm90_write_convrate(client, data, i); + err = lm90_write_convrate(data, i); data->update_interval = DIV_ROUND_CLOSEST(update_interval, 64); return err; } @@ -693,7 +692,7 @@ static int lm90_update_limits(struct device *dev) } if (data->kind == max6696) { - val = lm90_select_remote_channel(client, data, 1); + val = lm90_select_remote_channel(data, 1); if (val < 0) return val; @@ -717,7 +716,7 @@ static int lm90_update_limits(struct device *dev) return val; data->temp11[REMOTE2_HIGH] = val << 8; - lm90_select_remote_channel(client, data, 0); + lm90_select_remote_channel(data, 0); } return 0; @@ -777,19 +776,19 @@ static int lm90_update_device(struct device *dev) data->alarms = val; /* lower 8 bit of alarms */ if (data->kind == max6696) { - val = lm90_select_remote_channel(client, data, 1); + val = lm90_select_remote_channel(data, 1); if (val < 0) return val; val = lm90_read16(client, LM90_REG_R_REMOTE_TEMPH, LM90_REG_R_REMOTE_TEMPL); if (val < 0) { - lm90_select_remote_channel(client, data, 0); + lm90_select_remote_channel(data, 0); return val; } data->temp11[REMOTE2_TEMP] = val; - lm90_select_remote_channel(client, data, 0); + lm90_select_remote_channel(data, 0); val = lm90_read_reg(client, MAX6696_REG_R_STATUS2); if (val < 0) @@ -805,10 +804,7 @@ static int lm90_update_device(struct device *dev) !(data->alarms & data->alert_alarms)) { if (data->config & 0x80) { dev_dbg(&client->dev, "Re-enabling ALERT#\n"); - data->config &= ~0x80; - i2c_smbus_write_byte_data(client, - LM90_REG_W_CONFIG1, - data->config); + lm90_update_confreg(data, data->config & ~0x80); } } @@ -1026,7 +1022,7 @@ static int lm90_set_temp11(struct lm90_data *data, int index, long val) else data->temp11[index] = temp_to_s8(val) << 8; - lm90_select_remote_channel(client, data, index >= 3); + lm90_select_remote_channel(data, index >= 3); err = i2c_smbus_write_byte_data(client, regp->high, data->temp11[index] >> 8); if (err < 0) @@ -1035,7 +1031,7 @@ static int lm90_set_temp11(struct lm90_data *data, int index, long val) err = i2c_smbus_write_byte_data(client, regp->low, data->temp11[index] & 0xff); - lm90_select_remote_channel(client, data, 0); + lm90_select_remote_channel(data, 0); return err; } @@ -1084,9 +1080,9 @@ static int lm90_set_temp8(struct lm90_data *data, int index, long val) else data->temp8[index] = temp_to_s8(val); - lm90_select_remote_channel(client, data, index >= 6); + lm90_select_remote_channel(data, index >= 6); err = i2c_smbus_write_byte_data(client, reg[index], data->temp8[index]); - lm90_select_remote_channel(client, data, 0); + lm90_select_remote_channel(data, 0); return err; } @@ -1625,7 +1621,7 @@ static void lm90_restore_conf(void *_data) struct i2c_client *client = data->client; /* Restore initial configuration */ - lm90_write_convrate(client, data, data->convrate_orig); + lm90_write_convrate(data, data->convrate_orig); i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, data->config_orig); } @@ -1671,10 +1667,7 @@ static int lm90_init_client(struct i2c_client *client, struct lm90_data *data) config &= ~0x08; config &= 0xBF; /* run */ - if (config != data->config) { /* Only write if changed */ - i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, config); - data->config = config; - } + lm90_update_confreg(data, config); return devm_add_action_or_reset(&client->dev, lm90_restore_conf, data); } @@ -1909,9 +1902,7 @@ static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type, if ((data->flags & LM90_HAVE_BROKEN_ALERT) && (alarms & data->alert_alarms)) { dev_dbg(&client->dev, "Disabling ALERT#\n"); - data->config |= 0x80; - i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, - data->config); + lm90_update_confreg(data, data->config | 0x80); } } else { dev_info(&client->dev, "Everything OK\n"); -- cgit v1.2.3 From f2173fa2246e906602c6286c7dade68109d68976 Mon Sep 17 00:00:00 2001 From: Christian Schneider Date: Tue, 2 Jul 2019 15:23:37 +0200 Subject: hwmon: (gpio-fan) move fan_alarm_init after devm_hwmon_device_register_with_groups This makes it possible to use the hwmon_dev in fan_alarm_notify(). Otherwise it would be possible, that a interupt arrives and fan_alarm_notify() is executed, before hwmon_dev is initialized. Signed-off-by: Christian Schneider Signed-off-by: Guenter Roeck --- drivers/hwmon/gpio-fan.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c index 54c27e683ee1..5f9b406134b5 100644 --- a/drivers/hwmon/gpio-fan.c +++ b/drivers/hwmon/gpio-fan.c @@ -510,13 +510,6 @@ static int gpio_fan_probe(struct platform_device *pdev) platform_set_drvdata(pdev, fan_data); mutex_init(&fan_data->lock); - /* Configure alarm GPIO if available. */ - if (fan_data->alarm_gpio) { - err = fan_alarm_init(fan_data); - if (err) - return err; - } - /* Configure control GPIOs if available. */ if (fan_data->gpios && fan_data->num_gpios > 0) { if (!fan_data->speed || fan_data->num_speed <= 1) @@ -537,6 +530,13 @@ static int gpio_fan_probe(struct platform_device *pdev) if (IS_ERR(fan_data->hwmon_dev)) return PTR_ERR(fan_data->hwmon_dev); + /* Configure alarm GPIO if available. */ + if (fan_data->alarm_gpio) { + err = fan_alarm_init(fan_data); + if (err) + return err; + } + /* Optional cooling device register for Device tree platforms */ fan_data->cdev = devm_thermal_of_cooling_device_register(dev, np, "gpio-fan", fan_data, &gpio_fan_cool_ops); -- cgit v1.2.3 From 277c628fa5acf363b73cdd3793700dc575a988c0 Mon Sep 17 00:00:00 2001 From: Christian Schneider Date: Tue, 2 Jul 2019 15:23:38 +0200 Subject: hwmon: (gpio-fan) fix sysfs notifications and udev events for gpio-fan alarms sysfs_notify() and kobject_uevent() are passed the wrong device. fan_data->hwmon_dev needs to be passed, so that sysfs notification goes to right place and generated uevent has the right information Signed-off-by: Christian Schneider Signed-off-by: Guenter Roeck --- drivers/hwmon/gpio-fan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c index 5f9b406134b5..3ea4021f267c 100644 --- a/drivers/hwmon/gpio-fan.c +++ b/drivers/hwmon/gpio-fan.c @@ -54,8 +54,8 @@ static void fan_alarm_notify(struct work_struct *ws) struct gpio_fan_data *fan_data = container_of(ws, struct gpio_fan_data, alarm_work); - sysfs_notify(&fan_data->dev->kobj, NULL, "fan1_alarm"); - kobject_uevent(&fan_data->dev->kobj, KOBJ_CHANGE); + sysfs_notify(&fan_data->hwmon_dev->kobj, NULL, "fan1_alarm"); + kobject_uevent(&fan_data->hwmon_dev->kobj, KOBJ_CHANGE); } static irqreturn_t fan_alarm_irq_handler(int irq, void *dev_id) -- cgit v1.2.3 From 9f7546570bcb20debfaa97bcf720fa0fcb8fc05a Mon Sep 17 00:00:00 2001 From: Nishka Dasgupta Date: Sat, 6 Jul 2019 18:51:30 +0530 Subject: hwmon: (ina3221) Add of_node_put() before return Each iteration of for_each_child_of_node puts the previous node, but in the case of a return from the middle of the loop, there is no put, thus causing a memory leak. Hence add an of_node_put before the return. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta Link: https://lore.kernel.org/r/20190706132130.3129-1-nishkadg.linux@gmail.com Signed-off-by: Guenter Roeck --- drivers/hwmon/ina3221.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c index 55943b4dcc7b..0037e2bdacd6 100644 --- a/drivers/hwmon/ina3221.c +++ b/drivers/hwmon/ina3221.c @@ -713,8 +713,10 @@ static int ina3221_probe_from_dt(struct device *dev, struct ina3221_data *ina) for_each_child_of_node(np, child) { ret = ina3221_probe_child_from_dt(dev, child, ina); - if (ret) + if (ret) { + of_node_put(child); return ret; + } } return 0; -- cgit v1.2.3