* [PATCH 0/2] Add driver for MAX42500 @ 2024-12-20 1:20 Kent Libetario 2024-12-20 1:20 ` [PATCH 1/2] dt-bindings: hwmon: add adi,max42500.yaml Kent Libetario 2024-12-20 1:20 ` [PATCH 2/2] drivers: hwmon: add driver for max42500 Kent Libetario 0 siblings, 2 replies; 8+ messages in thread From: Kent Libetario @ 2024-12-20 1:20 UTC (permalink / raw) To: linux-hwmon, devicetree, linux-kernel Cc: Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley This patch series introduces a new hardware monitoring (hwmon) driver for the MAX42500 hardware. The driver is a SoC power-system monitor with up to seven voltage monitor inputs. Each input has programmable OV/UV thresholds over the full temperature range and support DVS through the I2C interface. The MAX42500 contains a programmable flexible power sequence recorder (FPSR) and a programmable challenge/response watchdog with a configurable RESET output. These are also accessible through the I2C interface. The patches are organized as follows: 1. Patch 1/2: Add documentation for devicetree bindings of MAX42500 driver. 2. Patch 2/2: Add the core implementation of the MAX42500 hwmon driver. The driver has been tested on the raspberry pi-4 platform. Kent Libetario (2): dt-bindings: hwmon: add adi,max42500.yaml drivers: hwmon: add driver for max42500 .../bindings/hwmon/adi,max42500.yaml | 42 + MAINTAINERS | 7 + drivers/hwmon/Kconfig | 13 + drivers/hwmon/Makefile | 1 + drivers/hwmon/max42500.c | 1049 +++++++++++++++++ 5 files changed, 1112 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,max42500.yaml create mode 100644 drivers/hwmon/max42500.c base-commit: de076198d1e4934c5fc17aa52d5f1884f469ce1a -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] dt-bindings: hwmon: add adi,max42500.yaml 2024-12-20 1:20 [PATCH 0/2] Add driver for MAX42500 Kent Libetario @ 2024-12-20 1:20 ` Kent Libetario 2024-12-27 8:50 ` Krzysztof Kozlowski 2024-12-20 1:20 ` [PATCH 2/2] drivers: hwmon: add driver for max42500 Kent Libetario 1 sibling, 1 reply; 8+ messages in thread From: Kent Libetario @ 2024-12-20 1:20 UTC (permalink / raw) To: linux-hwmon, devicetree, linux-kernel Cc: Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley Add documentation for devicetree bindings for MAX42500 Signed-off-by: Kent Libetario <Kent.Libetario@analog.com> --- .../bindings/hwmon/adi,max42500.yaml | 42 +++++++++++++++++++ MAINTAINERS | 6 +++ 2 files changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,max42500.yaml diff --git a/Documentation/devicetree/bindings/hwmon/adi,max42500.yaml b/Documentation/devicetree/bindings/hwmon/adi,max42500.yaml new file mode 100644 index 000000000000..19446f571d9c --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/adi,max42500.yaml @@ -0,0 +1,42 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/adi,max42500.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices MAX42500 Industrial Power System Monitor + +maintainers: + - Kent Libetario <Kent.Libetario@analog.com> + +description: | + Analog Devices MAX42500 Industrial Power System Monitor Family + https://www.analog.com/media/en/technical-documentation/data-sheets/max42500.pdf + +properties: + compatible: + enum: + - adi,max42500 + + reg: + description: I2C address of slave device. + minimum: 0x28 + maximum: 0x2B + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + hwmon@28 { + reg = <0x28>; + compatible = "adi,max42500"; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 379289369505..d1703b4834c8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14046,6 +14046,12 @@ W: https://ez.analog.com/linux-software-drivers F: Documentation/devicetree/bindings/rtc/adi,max31335.yaml F: drivers/rtc/rtc-max31335.c +MAX42500 POWER-SYSTEM MONITOR DRIVER +M: Kent Libetario <kent.libetario@analog.com> +L: linux-hwmon@vger.kernel.org +S: Supported +F: Documentation/devicetree/bindings/hwmon/adi,max42500.yaml + MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER L: linux-hwmon@vger.kernel.org S: Orphan -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: add adi,max42500.yaml 2024-12-20 1:20 ` [PATCH 1/2] dt-bindings: hwmon: add adi,max42500.yaml Kent Libetario @ 2024-12-27 8:50 ` Krzysztof Kozlowski 2025-03-10 9:26 ` Libetario, Kent 0 siblings, 1 reply; 8+ messages in thread From: Krzysztof Kozlowski @ 2024-12-27 8:50 UTC (permalink / raw) To: Kent Libetario Cc: linux-hwmon, devicetree, linux-kernel, Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley On Fri, Dec 20, 2024 at 09:20:02AM +0800, Kent Libetario wrote: > Add documentation for devicetree bindings for MAX42500 > > Signed-off-by: Kent Libetario <Kent.Libetario@analog.com> Why not adding it to trivial devices? Aren't you missing some properties, like VDD, EN and RESET? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/2] dt-bindings: hwmon: add adi,max42500.yaml 2024-12-27 8:50 ` Krzysztof Kozlowski @ 2025-03-10 9:26 ` Libetario, Kent 0 siblings, 0 replies; 8+ messages in thread From: Libetario, Kent @ 2025-03-10 9:26 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley > -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: Friday, December 27, 2024 4:51 PM > To: Libetario, Kent <Kent.Libetario@analog.com> > Cc: linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Jean Delvare <jdelvare@suse.com>; Guenter Roeck > <linux@roeck-us.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org> > Subject: Re: [PATCH 1/2] dt-bindings: hwmon: add adi,max42500.yaml > > [External] > > On Fri, Dec 20, 2024 at 09:20:02AM +0800, Kent Libetario wrote: > > Add documentation for devicetree bindings for MAX42500 > > > > Signed-off-by: Kent Libetario <Kent.Libetario@analog.com> > > Why not adding it to trivial devices? Aren't you missing some properties, like > VDD, EN and RESET? > > Best regards, > Krzysztof Yes, I will update the EN properties as output-only in the host to control and trigger events to the device. But the VDD and RESET properties are fixed in the device itself and they cannot be configured by software or device tree. The RESET is commonly input to the NMI or RESET pin of the host MCU while the VDD depends only in the configuration of the hardware circuit connected to the system supply. Regards, Kent ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] drivers: hwmon: add driver for max42500 2024-12-20 1:20 [PATCH 0/2] Add driver for MAX42500 Kent Libetario 2024-12-20 1:20 ` [PATCH 1/2] dt-bindings: hwmon: add adi,max42500.yaml Kent Libetario @ 2024-12-20 1:20 ` Kent Libetario 2024-12-21 5:01 ` kernel test robot 2024-12-21 18:08 ` Guenter Roeck 1 sibling, 2 replies; 8+ messages in thread From: Kent Libetario @ 2024-12-20 1:20 UTC (permalink / raw) To: linux-hwmon, devicetree, linux-kernel Cc: Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley Add the core implementation of the MAX42500 hwmon driver. Signed-off-by: Kent Libetario <Kent.Libetario@analog.com> --- MAINTAINERS | 1 + drivers/hwmon/Kconfig | 13 + drivers/hwmon/Makefile | 1 + drivers/hwmon/max42500.c | 1049 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 1064 insertions(+) create mode 100644 drivers/hwmon/max42500.c diff --git a/MAINTAINERS b/MAINTAINERS index d1703b4834c8..434191e16dd5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14051,6 +14051,7 @@ M: Kent Libetario <kent.libetario@analog.com> L: linux-hwmon@vger.kernel.org S: Supported F: Documentation/devicetree/bindings/hwmon/adi,max42500.yaml +F: drivers/hwmon/max42500.c MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER L: linux-hwmon@vger.kernel.org diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index dd376602f3f1..ec0d7aad7789 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1220,6 +1220,19 @@ config MAX31827 This driver can also be built as a module. If so, the module will be called max31827. +config SENSORS_MAX42500 + tristate "MAX42500 Industrial Power System Monitor Family" + depends on I2C + select REGMAP_I2C + help + If you say yes here you get support for MAX42500 SoC power-system monitor + with up to seven voltage monitor. The driver also contains a programmable + challenge/response watchdog, which is accessible through the I2C interface, + along with a configurable RESET output. + + This driver can also be built as a module. If so, the module + will be called max42500. + config SENSORS_MAX6620 tristate "Maxim MAX6620 fan controller" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index b827b92f2a78..d27d8fc01141 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -152,6 +152,7 @@ obj-$(CONFIG_SENSORS_MAX197) += max197.o obj-$(CONFIG_SENSORS_MAX31722) += max31722.o obj-$(CONFIG_SENSORS_MAX31730) += max31730.o obj-$(CONFIG_SENSORS_MAX31760) += max31760.o +obj-$(CONFIG_SENSORS_MAX42500) += max42500.o obj-$(CONFIG_SENSORS_MAX6620) += max6620.o obj-$(CONFIG_SENSORS_MAX6621) += max6621.o obj-$(CONFIG_SENSORS_MAX6639) += max6639.o diff --git a/drivers/hwmon/max42500.c b/drivers/hwmon/max42500.c new file mode 100644 index 000000000000..23b90c766767 --- /dev/null +++ b/drivers/hwmon/max42500.c @@ -0,0 +1,1049 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * MAX42500 - Industrial Power System Monitor + * + * Copyright 2024 Analog Devices Inc. + */ + +#include <linux/init.h> +#include <linux/jiffies.h> +#include <linux/slab.h> +#include <linux/crc8.h> +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/err.h> +#include <linux/gpio.h> +#include <linux/gpio/driver.h> +#include <linux/hwmon.h> +#include <linux/i2c.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/property.h> +#include <linux/regmap.h> + +/* Implements Polynomial X^8 + X^2 + X^1 +1 */ +#define CRC8_PEC 0x07 + +DECLARE_CRC8_TABLE(max42500_crc8); + +#define MAX42500_REG_ID 0x00 +#define MAX42500_REG_CONFIG1 0x01 +#define MAX42500_REG_CONFIG2 0x02 +#define MAX42500_REG_VMON 0x03 +#define MAX42500_REG_RSTMAP 0x04 +#define MAX42500_REG_STATOV 0x05 +#define MAX42500_REG_STATUV 0x06 +#define MAX42500_REG_STATOFF 0x07 +#define MAX42500_REG_VIN1 0x08 +#define MAX42500_REG_VIN2 0x09 +#define MAX42500_REG_VIN3 0x0A +#define MAX42500_REG_VIN4 0x0B +#define MAX42500_REG_VIN5 0x0C +#define MAX42500_REG_VINO6 0x0D +#define MAX42500_REG_VINU6 0x0E +#define MAX42500_REG_VINO7 0x0F +#define MAX42500_REG_VINU7 0x10 +#define MAX42500_REG_OVUV1 0x11 +#define MAX42500_REG_OVUV2 0x12 +#define MAX42500_REG_OVUV3 0x13 +#define MAX42500_REG_OVUV4 0x14 +#define MAX42500_REG_OVUV5 0x15 +#define MAX42500_REG_FPSSTAT1 0x16 +#define MAX42500_REG_FPSCFG1 0x17 +#define MAX42500_REG_UTIME1 0x18 +#define MAX42500_REG_UTIME2 0x19 +#define MAX42500_REG_UTIME3 0x1A +#define MAX42500_REG_UTIME4 0x1B +#define MAX42500_REG_UTIME5 0x1C +#define MAX42500_REG_UTIME6 0x1D +#define MAX42500_REG_UTIME7 0x1E +#define MAX42500_REG_DTIME1 0x1F +#define MAX42500_REG_DTIME2 0x20 +#define MAX42500_REG_DTIME3 0x21 +#define MAX42500_REG_DTIME4 0x22 +#define MAX42500_REG_DTIME5 0x23 +#define MAX42500_REG_DTIME6 0x24 +#define MAX42500_REG_DTIME7 0x25 +#define MAX42500_REG_WDSTAT 0x26 +#define MAX42500_REG_WDCDIV 0x27 +#define MAX42500_REG_WDCFG1 0x28 +#define MAX42500_REG_WDCFG2 0x29 +#define MAX42500_REG_WDKEY 0x2A +#define MAX42500_REG_WDLOCK 0x2B +#define MAX42500_REG_RSTCTRL 0x2C +#define MAX42500_REG_CID 0x2D + +/** X is set based on the pull configuration of the ADDR pin */ +#define MAX42500_ADDR(x) (0x28 + (x)) +#define MAX42500_SILICON_ID (0x30) +#define MAX42500_I2C_WR_FRAME_SIZE (4) +#define MAX42500_I2C_RD_FRAME_SIZE (5) + +/** MAX42500 Nominal voltage computation */ +#define MAX42500_VNOM_MAX_VM1_VM4 0xFF // 3.6875v +#define MAX42500_VNOM_MAX_VM5 0xFF // 5.6v +#define MAX42500_MIN_VNOM 0x00 // 0.5v +#define MAX42500_VNOM_STEP_VM1_VM4 0x01 // 0.0125v +#define MAX42500_VNOM_STEP_VM5 0x01 // 0.02v + +/** MAX42500 Undervoltage/Overvoltage maximum and minimum thresholds*/ +#define MAX42500_MAX_THRESH_VM1_VM5 0x0F // OV = 110.0%, UV = 90.0% +#define MAX42500_MIN_THRESH_VM1_VM5 0x00 // OV = 102.5%, UV = 97.5% +#define MAX42500_MAX_THRESH_VM6_V7 0xFF // 1.775v +#define MAX42500_MIN_THRESH_VM6_V7 0x00 // 0.5v + +/* CONFIG1 bit masks */ +#define MAX42500_CONFIG1_PECE_MASK BIT(0) +#define MAX42500_CONFIG1_MBST_MASK BIT(1) +#define MAX42500_CONFIG1_RR_MASK BIT(2) + +/* VMON bit masks */ +#define MAX42500_VMON_IN_MASK(bit) BIT(bit) +#define MAX42500_VMON_VMPD_MASK BIT(7) + +/* RSTMAP bit masks */ +#define MAX42500_RSTMAP_IN_MASK(bit) BIT(bit) +#define MAX42500_RSTMAP_PARM_MASK BIT(7) + +/* WDCDIV bit masks */ +#define MAX42500_WDCDIV_SWW_MASK BIT(6) +#define MAX42500_WDCDIV_WDIC_MASK (0x3F) + +/* WDCFG2 bit masks */ +#define MAX42500_WDCFG2_WDEN_MASK BIT(3) +#define MAX42500_WDCFG2_1UP_MASK (0x7) + +/* WDLOCK bit masks */ +#define MAX42500_WDLOCK_LOCK_MASK BIT(0) + +/* RSTCTRL bit masks */ +#define MAX42500_RSTCTRL_MR1_MASK BIT(2) +#define MAX42500_RSTCTRL_RHLD_MASK (0x3) + +/* MAX42500 device status */ +enum max42500_status { + MAX42500_STATUS_OFF, + MAX42500_STATUS_SLEEP, + MAX42500_STATUS_ON, + MAX42500_STATUS_MAX +}; + +/* MAX42500 voltage monitor input */ +enum max42500_vm_input { + MAX42500_VM1, + MAX42500_VM2, + MAX42500_VM3, + MAX42500_VM4, + MAX42500_VM5, + MAX42500_VM6, + MAX42500_VM7, + MAX42500_VM_MAX +}; + +/* MAX42500 comparator status */ +enum max42500_comp_stat { + MAX42500_COMP_STAT_OFF, + MAX42500_COMP_STAT_UV, + MAX42500_COMP_STAT_OV, + MAX42500_COMP_STAT_MAX +}; + +/* MAX42500 watchdog mode */ +enum max42500_wd_mode { + MAX42500_WD_MODE_CH_RESP, + MAX42500_WD_MODE_SIMPLE, + MAX42500_WD_MODE_MAX +}; + +/* MAX42500 reset hold/active timeout time. */ +enum max42500_wd_rhld { + MAX42500_WD_RHOLD_0_MS, + MAX42500_WD_RHOLD_8_MS, + MAX42500_WD_RHOLD_16_MS, + MAX42500_WD_RHOLD_32_MS, + MAX42500_WD_RHOLD_MAX +}; + +struct max42500_config { + /* Packet error checking enable */ + u8 pece; + /* Enabled voltage monitor inputs */ + u8 vmon_en; + /* Voltage monitor power down enable */ + u8 vmon_vmpd; + /* Enabled voltage monitor reset mapping */ + u8 reset_map; + /* Watchdog mode */ + enum max42500_wd_mode wd_mode; + /* Watchdog clock div */ + u8 wd_cdiv; + /* Watchdog close window */ + u8 wd_close; + /* Watchdog open window */ + u8 wd_open; + /* Watchdog first update window */ + u8 wd_1ud; + /* Watchdog enable */ + u8 wd_en; +}; + +struct max42500_state { + struct i2c_client *client; + struct regmap *regmap; + struct max42500_config *config; + long pwrup_stamp[MAX42500_VM_MAX]; + long pwrdn_stamp[MAX42500_VM_MAX]; + u8 ov_thresh1[MAX42500_VM_MAX - 2]; + u8 ov_thresh2[MAX42500_VM_MAX - 5]; + u8 uv_thresh1[MAX42500_VM_MAX - 2]; + u8 uv_thresh2[MAX42500_VM_MAX - 5]; + u8 nominal_volt[MAX42500_VM_MAX - 2]; + u8 comp_status[MAX42500_VM_MAX * MAX42500_COMP_STAT_MAX]; +}; + +/************************ Functions Definitions **************************/ +/** + * @brief Read a raw value from a register. + * @return 0 in case of success, error code otherwise. + */ +static int max42500_reg_read(struct max42500_state *st, + u8 reg_addr, u8 *reg_data) +{ + int ret; + u8 i2c_data[MAX42500_I2C_RD_FRAME_SIZE] = {0}; + u8 bytes_num; + u8 pece_value; + + /* PEC is computed over entire I2C frame from first START condition */ + i2c_data[0] = (st->client->addr << 1); + i2c_data[1] = reg_addr; + i2c_data[2] = (st->client->addr << 1) | 0x1; + + /* I2C write target address */ + bytes_num = 1; + + ret = regmap_bulk_write(st->regmap, reg_addr, &i2c_data[1], bytes_num); + if (ret) + return ret; + + /* Change byte count if PECE is enabled (1-byte data. 1-byte PEC) */ + bytes_num = (st->config->pece) ? 2 : bytes_num; + + ret = regmap_bulk_read(st->regmap, reg_addr, &i2c_data[3], bytes_num); + if (ret) + return ret; + + if (st->config->pece) { + /* Compute CRC over entire I2C frame */ + pece_value = crc8(max42500_crc8, i2c_data, + (MAX42500_I2C_RD_FRAME_SIZE - 1), 0); + + if (i2c_data[4] != pece_value) + return -EIO; + } + + *reg_data = i2c_data[3]; + + return 0; +} + +/** + * @brief Write a raw value to a register. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_reg_write(struct max42500_state *st, + u8 reg_addr, u8 data) +{ + u8 i2c_data[MAX42500_I2C_WR_FRAME_SIZE] = {0}; + u8 bytes_num; + u8 pece_value; + + bytes_num = (st->config->pece) ? (MAX42500_I2C_WR_FRAME_SIZE - 1) : 2; + i2c_data[0] = (st->client->addr << 1); + i2c_data[1] = reg_addr; + i2c_data[2] = (u8)(data & 0xFF); + + pece_value = 0; + if (st->config->pece) + pece_value = crc8(max42500_crc8, i2c_data, bytes_num, 0); + + i2c_data[0] = i2c_data[1]; + i2c_data[1] = i2c_data[2]; + i2c_data[2] = pece_value; + + return regmap_bulk_write(st->regmap, reg_addr, i2c_data, bytes_num); +} + +/** + * @brief Update a register's value based on a mask. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_reg_update(struct max42500_state *st, + u8 reg_addr, u8 mask, u8 data) +{ + int ret; + u8 reg_data; + + ret = max42500_reg_read(st, reg_addr, ®_data); + if (ret) + return ret; + + reg_data &= ~mask; + reg_data |= mask & data; + + return max42500_reg_write(st, reg_addr, reg_data); +} + +/** + * @brief Set nominal voltage for VM1 to VM5. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_set_nominal_voltage(struct max42500_state *st, + enum max42500_vm_input vm_in, u8 voltage) +{ + u8 reg_addr; + + switch (vm_in) { + case MAX42500_VM1: + case MAX42500_VM2: + case MAX42500_VM3: + case MAX42500_VM4: + if (voltage < MAX42500_MIN_VNOM || + voltage > MAX42500_VNOM_MAX_VM1_VM4) + return -EINVAL; + reg_addr = MAX42500_REG_VIN1 + vm_in; + break; + case MAX42500_VM5: + if (voltage < MAX42500_MIN_VNOM || + voltage > MAX42500_VNOM_MAX_VM5) + return -EINVAL; + reg_addr = MAX42500_REG_VIN5; + break; + default: + return -EINVAL; + } + + st->nominal_volt[vm_in] = voltage; + return max42500_reg_write(st, reg_addr, voltage); +} + +/** + * @brief Get the status of the voltage monitor input. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_get_comp_status(struct max42500_state *st, + u8 vm_in, u8 *status) +{ + int ret; + u8 reg_addr; + u8 vm_in_status; + + switch (vm_in % MAX42500_COMP_STAT_MAX) { + case MAX42500_COMP_STAT_OFF: + reg_addr = MAX42500_REG_STATOFF; + break; + case MAX42500_COMP_STAT_UV: + reg_addr = MAX42500_REG_STATUV; + break; + case MAX42500_COMP_STAT_OV: + reg_addr = MAX42500_REG_STATOV; + break; + default: + return -EINVAL; + } + + ret = max42500_reg_read(st, reg_addr, &vm_in_status); + if (ret) + return ret; + + switch (vm_in % MAX42500_VM_MAX) { + case MAX42500_VM1: + *status = (u8)FIELD_GET(BIT(MAX42500_VM1), vm_in_status); + break; + case MAX42500_VM2: + *status = (u8)FIELD_GET(BIT(MAX42500_VM2), vm_in_status); + break; + case MAX42500_VM3: + *status = (u8)FIELD_GET(BIT(MAX42500_VM3), vm_in_status); + break; + case MAX42500_VM4: + *status = (u8)FIELD_GET(BIT(MAX42500_VM4), vm_in_status); + break; + case MAX42500_VM5: + *status = (u8)FIELD_GET(BIT(MAX42500_VM5), vm_in_status); + break; + case MAX42500_VM6: + *status = (u8)FIELD_GET(BIT(MAX42500_VM6), vm_in_status); + break; + case MAX42500_VM7: + *status = (u8)FIELD_GET(BIT(MAX42500_VM7), vm_in_status); + break; + default: + return -EINVAL; + } + st->comp_status[vm_in] = *status; + + return 0; +} + +/** + * @brief Set the overvoltage threshold of VM1 to VM5. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_set_ov_thresh1(struct max42500_state *st, + enum max42500_vm_input vm_in, u8 thresh) +{ + if (thresh < MAX42500_MIN_THRESH_VM1_VM5 || + thresh > MAX42500_MAX_THRESH_VM1_VM5) + return -EINVAL; + + switch (vm_in) { + case MAX42500_VM1: + case MAX42500_VM2: + case MAX42500_VM3: + case MAX42500_VM4: + case MAX42500_VM5: + st->ov_thresh1[vm_in] = thresh; + return max42500_reg_update(st, + MAX42500_REG_OVUV1 + vm_in, + GENMASK(7, 4), + FIELD_PREP(GENMASK(7, 4), + thresh)); + default: + return -EINVAL; + } +} + +/** + * @brief Set the overvoltage threshold of VM6 and VM7. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_set_ov_thresh2(struct max42500_state *st, + enum max42500_vm_input vm_in, u8 thresh) +{ + u8 reg_addr; + + if (thresh < MAX42500_MIN_THRESH_VM6_V7 || + thresh > MAX42500_MAX_THRESH_VM6_V7) + return -EINVAL; + + switch (vm_in) { + case MAX42500_VM6: + reg_addr = MAX42500_REG_VINO6; + break; + case MAX42500_VM7: + reg_addr = MAX42500_REG_VINO7; + break; + default: + return -EINVAL; + } + + st->ov_thresh2[vm_in] = thresh; + return max42500_reg_write(st, reg_addr, thresh); +} + +/** + * @brief Set the undervoltage threshold of VM1 to VM5. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_set_uv_thresh1(struct max42500_state *st, + enum max42500_vm_input vm_in, u8 thresh) +{ + if (thresh < MAX42500_MIN_THRESH_VM1_VM5 || + thresh > MAX42500_MAX_THRESH_VM1_VM5) + return -EINVAL; + + switch (vm_in) { + case MAX42500_VM1: + case MAX42500_VM2: + case MAX42500_VM3: + case MAX42500_VM4: + case MAX42500_VM5: + st->uv_thresh1[vm_in] = thresh; + return max42500_reg_update(st, + MAX42500_REG_OVUV1 + vm_in, + GENMASK(3, 0), + thresh); + default: + return -EINVAL; + } +} + +/** + * @brief Set the undervoltage threshold of VM6 and VM7. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_set_uv_thresh2(struct max42500_state *st, + enum max42500_vm_input vm_in, u8 thresh) +{ + u8 reg_addr; + + if (thresh < MAX42500_MIN_THRESH_VM6_V7 || + thresh > MAX42500_MAX_THRESH_VM6_V7) + return -EINVAL; + + switch (vm_in) { + case MAX42500_VM6: + reg_addr = MAX42500_REG_VINU6; + break; + case MAX42500_VM7: + reg_addr = MAX42500_REG_VINU7; + break; + default: + return -EINVAL; + } + + st->uv_thresh2[vm_in] = thresh; + return max42500_reg_write(st, reg_addr, thresh); +} + +/** + * @brief Get the FPS clock divider value. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_get_fps_clk_div(struct max42500_state *st, + u8 *fps_clk_div) +{ + int ret; + u8 reg_val; + + ret = max42500_reg_read(st, MAX42500_REG_FPSCFG1, ®_val); + if (ret) + return ret; + + *fps_clk_div = (u8)FIELD_GET(GENMASK(2, 0), reg_val); + + return 0; +} + +/** + * @brief Get power-up timestamp for a specified voltage monitor input. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_get_power_up_timestamp(struct max42500_state *st, + enum max42500_vm_input vm_in, long *timestamp) +{ + int ret; + u8 reg_val; + u8 fps_clk_div; + + ret = max42500_reg_read(st, MAX42500_REG_UTIME1 + vm_in, ®_val); + if (ret) + return ret; + + /* Check if the input voltage rose above the UV threshold */ + if (reg_val == 0) { + /* Input voltage never rose above UV threshold*/ + *timestamp = 0; + return 0; + } + + ret = max42500_get_fps_clk_div(st, &fps_clk_div); + if (ret) + return ret; + + *timestamp = (reg_val - 1) * 25 * (1 << fps_clk_div); + st->pwrup_stamp[vm_in] = *timestamp; + return 0; +} + +/** + * @brief Get power-down timestamp for a specified voltage monitor input. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_get_power_down_timestamp(struct max42500_state *st, + enum max42500_vm_input vm_in, long *timestamp) +{ + int ret; + u8 reg_val; + u8 fps_clk_div; + + ret = max42500_reg_read(st, MAX42500_REG_DTIME1 + vm_in, ®_val); + if (ret) + return ret; + + /* Check if the input voltage fell below the OFF threshold */ + if (reg_val == 0) { + /* Input voltage never fell below OFF threshold */ + *timestamp = 0; + return 0; + } + + ret = max42500_get_fps_clk_div(st, &fps_clk_div); + if (ret) + return ret; + + *timestamp = (reg_val - 1) * 25 * (1 << fps_clk_div); + st->pwrdn_stamp[vm_in] = *timestamp; + return 0; +} + +/** + * @brief Enable/Disable watchdog + * @return 0 in case of success, error code otherwise + */ +static int max42500_set_watchdog_enable(struct max42500_state *st, + bool wd_enable) +{ + int ret; + u8 reg_val; + + ret = max42500_reg_read(st, MAX42500_REG_WDCFG2, ®_val); + if (ret) + return ret; + + if (wd_enable) + reg_val |= BIT(3); + else + reg_val &= ~BIT(3); + + return max42500_reg_write(st, MAX42500_REG_WDCFG2, reg_val); +} + +/** + * @brief 8-bit watchdog key computation. + * @return 0 in case of success, negative error code otherwise. + */ +static int max42500_get_watchdog_key(struct max42500_state *st, + u8 *new_wd_key) +{ + int ret; + u8 curr_wd_key; + + ret = max42500_reg_read(st, MAX42500_REG_WDKEY, &curr_wd_key); + if (ret) + return ret; + + /* Calculate the new bit using the LFSR polynomial */ + u8 new_bit = ((curr_wd_key >> 7) ^ + (curr_wd_key >> 5) ^ + (curr_wd_key >> 4) ^ + (curr_wd_key >> 3)) & 0x01; + + /* Shift existing bits upwards to MSb and insert the new bit as LSb */ + *new_wd_key = (curr_wd_key << 1) | new_bit; + + return 0; +} + +/** + * @brief Update the watchdog key based on the mode and current value. + * @return 0 in case of success, error code otherwise. + */ +static int max42500_set_watchdog_key(struct max42500_state *st) +{ + int ret; + u8 reg_val; + u8 wd_key; + u8 wd_mode; + + ret = max42500_reg_read(st, MAX42500_REG_WDKEY, &wd_key); + if (ret) + return ret; + + ret = max42500_reg_read(st, MAX42500_REG_WDCDIV, ®_val); + if (ret) + return ret; + + wd_mode = (u8)FIELD_GET(BIT(6), reg_val); + + /* Compute new watchdog key for challenge/response mode */ + if (wd_mode == MAX42500_WD_MODE_CH_RESP) + max42500_get_watchdog_key(st, &wd_key); + + return max42500_reg_write(st, MAX42500_REG_WDKEY, wd_key); +} + +/** @brief Set watchdog reset hold time + * @return 0 in case of success, error code otherwise + */ +static int max42500_set_watchdog_rhld(struct max42500_state *st, u8 rhld) +{ + return max42500_reg_update(st, + MAX42500_REG_RSTCTRL, + GENMASK(1, 0), + rhld); +} + +static umode_t max42500_is_visible(const void *data, + enum hwmon_sensor_types type, u32 attr, int channel) +{ + const struct max42500_state *st = data; + + switch (type) { + case hwmon_chip: + switch (attr) { + case hwmon_chip_in_reset_history: + if (st->pwrup_stamp[channel]) + return 0444; + break; + case hwmon_chip_curr_reset_history: + if (st->pwrdn_stamp[channel]) + return 0444; + break; + case hwmon_chip_register_tz: + return 0200; + case hwmon_chip_alarms: + case hwmon_chip_update_interval: + case hwmon_chip_temp_reset_history: + case hwmon_chip_power_reset_history: + return 0644; + } + break; + case hwmon_in: + switch (attr) { + case hwmon_in_input: + if (st->nominal_volt[channel]) + return 0644; + break; + case hwmon_in_lowest: + if (st->ov_thresh1[channel]) + return 0644; + break; + case hwmon_in_highest: + if (st->ov_thresh2[channel]) + return 0644; + break; + case hwmon_in_min: + if (st->uv_thresh1[channel]) + return 0644; + break; + case hwmon_in_max: + if (st->uv_thresh2[channel]) + return 0644; + break; + case hwmon_in_label: + if (st->comp_status[channel]) + return 0444; + break; + } + break; + default: + break; + } + + return 0; +} + +static int max42500_read_in(struct device *dev, u32 attr, int channel, + long *value) +{ + struct max42500_state *st = dev_get_drvdata(dev); + + switch (attr) { + case hwmon_in_label: + return max42500_get_comp_status(st, channel, (u8 *)value); + default: + return -EOPNOTSUPP; + } +} + +static int max42500_read_chip(struct device *dev, u32 attr, int channel, + long *value) +{ + struct max42500_state *st = dev_get_drvdata(dev); + + switch (attr) { + case hwmon_chip_in_reset_history: + return max42500_get_power_up_timestamp(st, channel, value); + case hwmon_chip_curr_reset_history: + return max42500_get_power_down_timestamp(st, channel, value); + case hwmon_chip_power_reset_history: + return max42500_get_watchdog_key(st, (u8 *)value); + case hwmon_chip_update_interval: + return max42500_get_fps_clk_div(st, (u8 *)value); + default: + return -EOPNOTSUPP; + } +} + +static int max42500_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *value) +{ + switch (type) { + case hwmon_chip: + return max42500_read_chip(dev, attr, channel, value); + case hwmon_in: + return max42500_read_in(dev, attr, channel, value); + default: + return -EOPNOTSUPP; + } +} + +static int max42500_write_in(struct device *dev, u32 attr, int channel, + long value) +{ + struct max42500_state *st = dev_get_drvdata(dev); + + switch (attr) { + case hwmon_in_input: + return max42500_set_nominal_voltage(st, channel, (u8)value); + case hwmon_in_min: + return max42500_set_uv_thresh1(st, channel, (u8)value); + case hwmon_in_max: + return max42500_set_uv_thresh2(st, channel, (u8)value); + case hwmon_in_lowest: + return max42500_set_ov_thresh1(st, channel, (u8)value); + case hwmon_in_highest: + return max42500_set_ov_thresh2(st, channel, (u8)value); + default: + return -EOPNOTSUPP; + } +} + +static int max42500_write_chip(struct device *dev, u32 attr, int channel, + long value) +{ + struct max42500_state *st = dev_get_drvdata(dev); + + switch (attr) { + case hwmon_chip_temp_reset_history: + return max42500_set_watchdog_rhld(st, (u8)value); + case hwmon_chip_register_tz: + return max42500_set_watchdog_key(st); + case hwmon_chip_alarms: + return max42500_set_watchdog_enable(st, (bool)value); + default: + return -EOPNOTSUPP; + } +} + +static int max42500_write(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long value) +{ + switch (type) { + case hwmon_chip: + return max42500_write_chip(dev, attr, channel, value); + case hwmon_in: + return max42500_write_in(dev, attr, channel, value); + default: + return -EOPNOTSUPP; + } +} + +static const struct max42500_config max42500_init = { + .pece = true, + .vmon_en = true, + .vmon_vmpd = true, + .reset_map = (MAX42500_RSTMAP_IN_MASK(MAX42500_VM2) | + MAX42500_RSTMAP_IN_MASK(MAX42500_VM3) | + MAX42500_RSTMAP_IN_MASK(MAX42500_VM4) | + MAX42500_RSTMAP_PARM_MASK), + .wd_mode = MAX42500_WD_MODE_SIMPLE, + .wd_cdiv = 0x0, + .wd_close = 0x0, + .wd_open = 0x0, + .wd_1ud = 0x0, + .wd_en = true, +}; + +static int max42500_init_client(struct max42500_state *st, + struct device *dev) +{ + int err; + u8 device_id; + + if (!st || !dev) + return -EINVAL; + + crc8_populate_msb(max42500_crc8, CRC8_PEC); + + /* Check device silicon ID */ + err = max42500_reg_read(st, MAX42500_REG_ID, &device_id); + if (err) + return err; + + if (device_id != MAX42500_SILICON_ID) + return -ENODEV; + + /* Configure PEC */ + err = max42500_reg_update(st, + MAX42500_REG_CONFIG1, + BIT(0), + max42500_init.pece); + if (err) + return err; + + /* Set PEC enable/disable for subsequent register access */ + st->config->pece = max42500_init.pece; + + /* Enable voltage monitor inputs */ + err = max42500_reg_update(st, + MAX42500_REG_VMON, + GENMASK(6, 0), + max42500_init.vmon_en); + if (err) + return err; + + /* Enable voltage monitor power-down */ + err = max42500_reg_update(st, + MAX42500_REG_VMON, + BIT(7), + max42500_init.vmon_vmpd); + if (err) + return err; + + /* Enable input OV/UV mapping to reset pin */ + err = max42500_reg_update(st, + MAX42500_REG_RSTMAP, + GENMASK(6, 0), + max42500_init.reset_map); + if (err) + return err; + + /* Set watchdog mode */ + err = max42500_reg_update(st, + MAX42500_REG_WDCDIV, + BIT(6), + max42500_init.wd_mode << 6); + if (err) + return err; + + /* Set watchdog clock div */ + err = max42500_reg_update(st, + MAX42500_REG_WDCDIV, + GENMASK(5, 0), + max42500_init.wd_cdiv); + if (err) + return err; + + /* Set watchdog open window */ + err = max42500_reg_update(st, + MAX42500_REG_WDCFG1, + GENMASK(3, 0), + max42500_init.wd_open); + if (err) + return err; + + /* Set watchdog close window */ + err = max42500_reg_update(st, + MAX42500_REG_WDCFG1, + GENMASK(7, 4), + max42500_init.wd_close); + if (err) + return err; + + /* Set watchdog first update window */ + err = max42500_reg_update(st, + MAX42500_REG_WDCFG2, + GENMASK(2, 0), + max42500_init.wd_1ud); + if (err) + return err; + + /* Set watchdog enable */ + err = max42500_reg_update(st, + MAX42500_REG_WDCFG2, + BIT(3), + max42500_init.wd_en); + if (err) + return err; + + /* Update parameters */ + st->config->vmon_en = max42500_init.vmon_en; + st->config->vmon_vmpd = max42500_init.vmon_vmpd; + st->config->reset_map = max42500_init.reset_map; + st->config->wd_mode = max42500_init.wd_mode; + st->config->wd_cdiv = max42500_init.wd_cdiv; + st->config->wd_open = max42500_init.wd_open; + st->config->wd_close = max42500_init.wd_close; + st->config->wd_1ud = max42500_init.wd_1ud; + st->config->wd_en = max42500_init.wd_en; + + return 0; +} + +static const struct hwmon_ops max42500_hwmon_ops = { + .is_visible = max42500_is_visible, + .read = max42500_read, + .write = max42500_write, +}; + +static const struct hwmon_channel_info *max42500_info[] = { + HWMON_CHANNEL_INFO(chip, + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY | + HWMON_C_TEMP_RESET_HISTORY | HWMON_C_POWER_RESET_HISTORY | + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL | HWMON_C_ALARMS, + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY), + HWMON_CHANNEL_INFO(in, + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN | + HWMON_I_HIGHEST | HWMON_I_MAX, + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN | + HWMON_I_HIGHEST | HWMON_I_MAX, + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN, + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN, + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN, + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL), + NULL +}; + +static const struct hwmon_chip_info max42500_chip_info = { + .ops = &max42500_hwmon_ops, + .info = max42500_info, +}; + +static const struct regmap_config max42500_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = 0x2D +}; + +static int max42500_probe(struct i2c_client *client) +{ + struct device *hwmon_dev; + struct max42500_state *st; + int err; + + st = devm_kzalloc(&client->dev, sizeof(*st), GFP_KERNEL); + if (!st) + return -ENOMEM; + + st->client = client; + st->regmap = devm_regmap_init_i2c(client, &max42500_regmap_config); + if (IS_ERR(st->regmap)) + return PTR_ERR(st->regmap); + + err = max42500_init_client(st, &client->dev); + if (err) + return err; + + hwmon_dev = devm_hwmon_device_register_with_info(&client->dev, + client->name, st, &max42500_chip_info, NULL); + + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct of_device_id max42500_of_match[] = { + { .compatible = "adi,max42500" }, + { } +}; +MODULE_DEVICE_TABLE(of, max42500_of_match); + +static const struct i2c_device_id max42500_id[] = { + { "max42500", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, max42500_id); + +static struct i2c_driver max42500_driver = { + .driver = { + .name = "max42500", + .of_match_table = max42500_of_match, + }, + .probe = max42500_probe, + .id_table = max42500_id, +}; + +module_i2c_driver(max42500_driver); + +MODULE_AUTHOR("Kent Libetario <kent.libetario@analog.com>"); +MODULE_DESCRIPTION("Hwmon driver for MAX42500"); +MODULE_LICENSE("GPL"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drivers: hwmon: add driver for max42500 2024-12-20 1:20 ` [PATCH 2/2] drivers: hwmon: add driver for max42500 Kent Libetario @ 2024-12-21 5:01 ` kernel test robot 2024-12-21 18:08 ` Guenter Roeck 1 sibling, 0 replies; 8+ messages in thread From: kernel test robot @ 2024-12-21 5:01 UTC (permalink / raw) To: Kent Libetario, linux-hwmon, devicetree, linux-kernel Cc: oe-kbuild-all, Jean Delvare, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley Hi Kent, kernel test robot noticed the following build warnings: [auto build test WARNING on de076198d1e4934c5fc17aa52d5f1884f469ce1a] url: https://github.com/intel-lab-lkp/linux/commits/Kent-Libetario/dt-bindings-hwmon-add-adi-max42500-yaml/20241220-092728 base: de076198d1e4934c5fc17aa52d5f1884f469ce1a patch link: https://lore.kernel.org/r/20241220012003.9568-3-Kent.Libetario%40analog.com patch subject: [PATCH 2/2] drivers: hwmon: add driver for max42500 config: x86_64-randconfig-r072-20241221 (https://download.01.org/0day-ci/archive/20241221/202412211209.zqypWgoc-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241221/202412211209.zqypWgoc-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202412211209.zqypWgoc-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/hwmon/max42500.c:206: warning: Cannot understand * @brief Read a raw value from a register. on line 206 - I thought it was a doc line >> drivers/hwmon/max42500.c:251: warning: Cannot understand * @brief Write a raw value to a register. on line 251 - I thought it was a doc line >> drivers/hwmon/max42500.c:278: warning: Cannot understand * @brief Update a register's value based on a mask. on line 278 - I thought it was a doc line >> drivers/hwmon/max42500.c:298: warning: Cannot understand * @brief Set nominal voltage for VM1 to VM5. on line 298 - I thought it was a doc line >> drivers/hwmon/max42500.c:331: warning: Cannot understand * @brief Get the status of the voltage monitor input. on line 331 - I thought it was a doc line >> drivers/hwmon/max42500.c:390: warning: Cannot understand * @brief Set the overvoltage threshold of VM1 to VM5. on line 390 - I thought it was a doc line >> drivers/hwmon/max42500.c:418: warning: Cannot understand * @brief Set the overvoltage threshold of VM6 and VM7. on line 418 - I thought it was a doc line >> drivers/hwmon/max42500.c:446: warning: Cannot understand * @brief Set the undervoltage threshold of VM1 to VM5. on line 446 - I thought it was a doc line >> drivers/hwmon/max42500.c:473: warning: Cannot understand * @brief Set the undervoltage threshold of VM6 and VM7. on line 473 - I thought it was a doc line >> drivers/hwmon/max42500.c:501: warning: Cannot understand * @brief Get the FPS clock divider value. on line 501 - I thought it was a doc line >> drivers/hwmon/max42500.c:520: warning: Cannot understand * @brief Get power-up timestamp for a specified voltage monitor input. on line 520 - I thought it was a doc line >> drivers/hwmon/max42500.c:551: warning: Cannot understand * @brief Get power-down timestamp for a specified voltage monitor input. on line 551 - I thought it was a doc line >> drivers/hwmon/max42500.c:582: warning: Cannot understand * @brief Enable/Disable watchdog on line 582 - I thought it was a doc line >> drivers/hwmon/max42500.c:604: warning: Cannot understand * @brief 8-bit watchdog key computation. on line 604 - I thought it was a doc line >> drivers/hwmon/max42500.c:630: warning: Cannot understand * @brief Update the watchdog key based on the mode and current value. on line 630 - I thought it was a doc line vim +206 drivers/hwmon/max42500.c 203 204 /************************ Functions Definitions **************************/ 205 /** > 206 * @brief Read a raw value from a register. 207 * @return 0 in case of success, error code otherwise. 208 */ 209 static int max42500_reg_read(struct max42500_state *st, 210 u8 reg_addr, u8 *reg_data) 211 { 212 int ret; 213 u8 i2c_data[MAX42500_I2C_RD_FRAME_SIZE] = {0}; 214 u8 bytes_num; 215 u8 pece_value; 216 217 /* PEC is computed over entire I2C frame from first START condition */ 218 i2c_data[0] = (st->client->addr << 1); 219 i2c_data[1] = reg_addr; 220 i2c_data[2] = (st->client->addr << 1) | 0x1; 221 222 /* I2C write target address */ 223 bytes_num = 1; 224 225 ret = regmap_bulk_write(st->regmap, reg_addr, &i2c_data[1], bytes_num); 226 if (ret) 227 return ret; 228 229 /* Change byte count if PECE is enabled (1-byte data. 1-byte PEC) */ 230 bytes_num = (st->config->pece) ? 2 : bytes_num; 231 232 ret = regmap_bulk_read(st->regmap, reg_addr, &i2c_data[3], bytes_num); 233 if (ret) 234 return ret; 235 236 if (st->config->pece) { 237 /* Compute CRC over entire I2C frame */ 238 pece_value = crc8(max42500_crc8, i2c_data, 239 (MAX42500_I2C_RD_FRAME_SIZE - 1), 0); 240 241 if (i2c_data[4] != pece_value) 242 return -EIO; 243 } 244 245 *reg_data = i2c_data[3]; 246 247 return 0; 248 } 249 250 /** > 251 * @brief Write a raw value to a register. 252 * @return 0 in case of success, negative error code otherwise. 253 */ 254 static int max42500_reg_write(struct max42500_state *st, 255 u8 reg_addr, u8 data) 256 { 257 u8 i2c_data[MAX42500_I2C_WR_FRAME_SIZE] = {0}; 258 u8 bytes_num; 259 u8 pece_value; 260 261 bytes_num = (st->config->pece) ? (MAX42500_I2C_WR_FRAME_SIZE - 1) : 2; 262 i2c_data[0] = (st->client->addr << 1); 263 i2c_data[1] = reg_addr; 264 i2c_data[2] = (u8)(data & 0xFF); 265 266 pece_value = 0; 267 if (st->config->pece) 268 pece_value = crc8(max42500_crc8, i2c_data, bytes_num, 0); 269 270 i2c_data[0] = i2c_data[1]; 271 i2c_data[1] = i2c_data[2]; 272 i2c_data[2] = pece_value; 273 274 return regmap_bulk_write(st->regmap, reg_addr, i2c_data, bytes_num); 275 } 276 277 /** > 278 * @brief Update a register's value based on a mask. 279 * @return 0 in case of success, negative error code otherwise. 280 */ 281 static int max42500_reg_update(struct max42500_state *st, 282 u8 reg_addr, u8 mask, u8 data) 283 { 284 int ret; 285 u8 reg_data; 286 287 ret = max42500_reg_read(st, reg_addr, ®_data); 288 if (ret) 289 return ret; 290 291 reg_data &= ~mask; 292 reg_data |= mask & data; 293 294 return max42500_reg_write(st, reg_addr, reg_data); 295 } 296 297 /** > 298 * @brief Set nominal voltage for VM1 to VM5. 299 * @return 0 in case of success, negative error code otherwise. 300 */ 301 static int max42500_set_nominal_voltage(struct max42500_state *st, 302 enum max42500_vm_input vm_in, u8 voltage) 303 { 304 u8 reg_addr; 305 306 switch (vm_in) { 307 case MAX42500_VM1: 308 case MAX42500_VM2: 309 case MAX42500_VM3: 310 case MAX42500_VM4: 311 if (voltage < MAX42500_MIN_VNOM || 312 voltage > MAX42500_VNOM_MAX_VM1_VM4) 313 return -EINVAL; 314 reg_addr = MAX42500_REG_VIN1 + vm_in; 315 break; 316 case MAX42500_VM5: 317 if (voltage < MAX42500_MIN_VNOM || 318 voltage > MAX42500_VNOM_MAX_VM5) 319 return -EINVAL; 320 reg_addr = MAX42500_REG_VIN5; 321 break; 322 default: 323 return -EINVAL; 324 } 325 326 st->nominal_volt[vm_in] = voltage; 327 return max42500_reg_write(st, reg_addr, voltage); 328 } 329 330 /** > 331 * @brief Get the status of the voltage monitor input. 332 * @return 0 in case of success, negative error code otherwise. 333 */ 334 static int max42500_get_comp_status(struct max42500_state *st, 335 u8 vm_in, u8 *status) 336 { 337 int ret; 338 u8 reg_addr; 339 u8 vm_in_status; 340 341 switch (vm_in % MAX42500_COMP_STAT_MAX) { 342 case MAX42500_COMP_STAT_OFF: 343 reg_addr = MAX42500_REG_STATOFF; 344 break; 345 case MAX42500_COMP_STAT_UV: 346 reg_addr = MAX42500_REG_STATUV; 347 break; 348 case MAX42500_COMP_STAT_OV: 349 reg_addr = MAX42500_REG_STATOV; 350 break; 351 default: 352 return -EINVAL; 353 } 354 355 ret = max42500_reg_read(st, reg_addr, &vm_in_status); 356 if (ret) 357 return ret; 358 359 switch (vm_in % MAX42500_VM_MAX) { 360 case MAX42500_VM1: 361 *status = (u8)FIELD_GET(BIT(MAX42500_VM1), vm_in_status); 362 break; 363 case MAX42500_VM2: 364 *status = (u8)FIELD_GET(BIT(MAX42500_VM2), vm_in_status); 365 break; 366 case MAX42500_VM3: 367 *status = (u8)FIELD_GET(BIT(MAX42500_VM3), vm_in_status); 368 break; 369 case MAX42500_VM4: 370 *status = (u8)FIELD_GET(BIT(MAX42500_VM4), vm_in_status); 371 break; 372 case MAX42500_VM5: 373 *status = (u8)FIELD_GET(BIT(MAX42500_VM5), vm_in_status); 374 break; 375 case MAX42500_VM6: 376 *status = (u8)FIELD_GET(BIT(MAX42500_VM6), vm_in_status); 377 break; 378 case MAX42500_VM7: 379 *status = (u8)FIELD_GET(BIT(MAX42500_VM7), vm_in_status); 380 break; 381 default: 382 return -EINVAL; 383 } 384 st->comp_status[vm_in] = *status; 385 386 return 0; 387 } 388 389 /** > 390 * @brief Set the overvoltage threshold of VM1 to VM5. 391 * @return 0 in case of success, negative error code otherwise. 392 */ 393 static int max42500_set_ov_thresh1(struct max42500_state *st, 394 enum max42500_vm_input vm_in, u8 thresh) 395 { 396 if (thresh < MAX42500_MIN_THRESH_VM1_VM5 || 397 thresh > MAX42500_MAX_THRESH_VM1_VM5) 398 return -EINVAL; 399 400 switch (vm_in) { 401 case MAX42500_VM1: 402 case MAX42500_VM2: 403 case MAX42500_VM3: 404 case MAX42500_VM4: 405 case MAX42500_VM5: 406 st->ov_thresh1[vm_in] = thresh; 407 return max42500_reg_update(st, 408 MAX42500_REG_OVUV1 + vm_in, 409 GENMASK(7, 4), 410 FIELD_PREP(GENMASK(7, 4), 411 thresh)); 412 default: 413 return -EINVAL; 414 } 415 } 416 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drivers: hwmon: add driver for max42500 2024-12-20 1:20 ` [PATCH 2/2] drivers: hwmon: add driver for max42500 Kent Libetario 2024-12-21 5:01 ` kernel test robot @ 2024-12-21 18:08 ` Guenter Roeck 2025-03-10 9:05 ` Libetario, Kent 1 sibling, 1 reply; 8+ messages in thread From: Guenter Roeck @ 2024-12-21 18:08 UTC (permalink / raw) To: Kent Libetario Cc: linux-hwmon, devicetree, linux-kernel, Jean Delvare, Rob Herring, Krzysztof Kozlowski, Conor Dooley On Fri, Dec 20, 2024 at 09:20:03AM +0800, Kent Libetario wrote: > Add the core implementation of the MAX42500 hwmon driver. > Please have a look into Documentation/process/submitting-patches.rst and follow the guidance about describing patches. > Signed-off-by: Kent Libetario <Kent.Libetario@analog.com> > --- > MAINTAINERS | 1 + > drivers/hwmon/Kconfig | 13 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/max42500.c | 1049 ++++++++++++++++++++++++++++++++++++++ Documentation missing. There is a large number of checkpatch CHECK messages, almost all about multi-line alignment. Indeed, when looking through the code, multi-line aligment seems arbitrary. Please fix. PEC support should be implemented through the i2c controller driver. Please see other drivers such as lm90.c or max31827.c for examples how to do that. Some more comments inline. The review is partial. The major problem I see is that there are not just a few but _lots_ of ABI violations. This is unacceptable. Please rework the driver and follow the hwmon ABI for all sysfs attributes. Guenter > 4 files changed, 1064 insertions(+) > create mode 100644 drivers/hwmon/max42500.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d1703b4834c8..434191e16dd5 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14051,6 +14051,7 @@ M: Kent Libetario <kent.libetario@analog.com> > L: linux-hwmon@vger.kernel.org > S: Supported > F: Documentation/devicetree/bindings/hwmon/adi,max42500.yaml > +F: drivers/hwmon/max42500.c > > MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER > L: linux-hwmon@vger.kernel.org > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index dd376602f3f1..ec0d7aad7789 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1220,6 +1220,19 @@ config MAX31827 > This driver can also be built as a module. If so, the module > will be called max31827. > > +config SENSORS_MAX42500 > + tristate "MAX42500 Industrial Power System Monitor Family" > + depends on I2C > + select REGMAP_I2C > + help > + If you say yes here you get support for MAX42500 SoC power-system monitor > + with up to seven voltage monitor. The driver also contains a programmable > + challenge/response watchdog, which is accessible through the I2C interface, > + along with a configurable RESET output. > + > + This driver can also be built as a module. If so, the module > + will be called max42500. > + > config SENSORS_MAX6620 > tristate "Maxim MAX6620 fan controller" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index b827b92f2a78..d27d8fc01141 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -152,6 +152,7 @@ obj-$(CONFIG_SENSORS_MAX197) += max197.o > obj-$(CONFIG_SENSORS_MAX31722) += max31722.o > obj-$(CONFIG_SENSORS_MAX31730) += max31730.o > obj-$(CONFIG_SENSORS_MAX31760) += max31760.o > +obj-$(CONFIG_SENSORS_MAX42500) += max42500.o > obj-$(CONFIG_SENSORS_MAX6620) += max6620.o > obj-$(CONFIG_SENSORS_MAX6621) += max6621.o > obj-$(CONFIG_SENSORS_MAX6639) += max6639.o > diff --git a/drivers/hwmon/max42500.c b/drivers/hwmon/max42500.c > new file mode 100644 > index 000000000000..23b90c766767 > --- /dev/null > +++ b/drivers/hwmon/max42500.c > @@ -0,0 +1,1049 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * MAX42500 - Industrial Power System Monitor > + * > + * Copyright 2024 Analog Devices Inc. > + */ > + > +#include <linux/init.h> > +#include <linux/jiffies.h> > +#include <linux/slab.h> > +#include <linux/crc8.h> > +#include <linux/bitfield.h> > +#include <linux/bitops.h> > +#include <linux/err.h> > +#include <linux/gpio.h> > +#include <linux/gpio/driver.h> > +#include <linux/hwmon.h> > +#include <linux/i2c.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/property.h> > +#include <linux/regmap.h> > + > +/* Implements Polynomial X^8 + X^2 + X^1 +1 */ > +#define CRC8_PEC 0x07 > + > +DECLARE_CRC8_TABLE(max42500_crc8); > + > +#define MAX42500_REG_ID 0x00 > +#define MAX42500_REG_CONFIG1 0x01 > +#define MAX42500_REG_CONFIG2 0x02 > +#define MAX42500_REG_VMON 0x03 > +#define MAX42500_REG_RSTMAP 0x04 > +#define MAX42500_REG_STATOV 0x05 > +#define MAX42500_REG_STATUV 0x06 > +#define MAX42500_REG_STATOFF 0x07 > +#define MAX42500_REG_VIN1 0x08 > +#define MAX42500_REG_VIN2 0x09 > +#define MAX42500_REG_VIN3 0x0A > +#define MAX42500_REG_VIN4 0x0B > +#define MAX42500_REG_VIN5 0x0C > +#define MAX42500_REG_VINO6 0x0D > +#define MAX42500_REG_VINU6 0x0E > +#define MAX42500_REG_VINO7 0x0F > +#define MAX42500_REG_VINU7 0x10 > +#define MAX42500_REG_OVUV1 0x11 > +#define MAX42500_REG_OVUV2 0x12 > +#define MAX42500_REG_OVUV3 0x13 > +#define MAX42500_REG_OVUV4 0x14 > +#define MAX42500_REG_OVUV5 0x15 > +#define MAX42500_REG_FPSSTAT1 0x16 > +#define MAX42500_REG_FPSCFG1 0x17 > +#define MAX42500_REG_UTIME1 0x18 > +#define MAX42500_REG_UTIME2 0x19 > +#define MAX42500_REG_UTIME3 0x1A > +#define MAX42500_REG_UTIME4 0x1B > +#define MAX42500_REG_UTIME5 0x1C > +#define MAX42500_REG_UTIME6 0x1D > +#define MAX42500_REG_UTIME7 0x1E > +#define MAX42500_REG_DTIME1 0x1F > +#define MAX42500_REG_DTIME2 0x20 > +#define MAX42500_REG_DTIME3 0x21 > +#define MAX42500_REG_DTIME4 0x22 > +#define MAX42500_REG_DTIME5 0x23 > +#define MAX42500_REG_DTIME6 0x24 > +#define MAX42500_REG_DTIME7 0x25 > +#define MAX42500_REG_WDSTAT 0x26 > +#define MAX42500_REG_WDCDIV 0x27 > +#define MAX42500_REG_WDCFG1 0x28 > +#define MAX42500_REG_WDCFG2 0x29 > +#define MAX42500_REG_WDKEY 0x2A > +#define MAX42500_REG_WDLOCK 0x2B > +#define MAX42500_REG_RSTCTRL 0x2C > +#define MAX42500_REG_CID 0x2D > + > +/** X is set based on the pull configuration of the ADDR pin */ > +#define MAX42500_ADDR(x) (0x28 + (x)) > +#define MAX42500_SILICON_ID (0x30) > +#define MAX42500_I2C_WR_FRAME_SIZE (4) > +#define MAX42500_I2C_RD_FRAME_SIZE (5) > + > +/** MAX42500 Nominal voltage computation */ > +#define MAX42500_VNOM_MAX_VM1_VM4 0xFF // 3.6875v > +#define MAX42500_VNOM_MAX_VM5 0xFF // 5.6v > +#define MAX42500_MIN_VNOM 0x00 // 0.5v > +#define MAX42500_VNOM_STEP_VM1_VM4 0x01 // 0.0125v > +#define MAX42500_VNOM_STEP_VM5 0x01 // 0.02v > + > +/** MAX42500 Undervoltage/Overvoltage maximum and minimum thresholds*/ > +#define MAX42500_MAX_THRESH_VM1_VM5 0x0F // OV = 110.0%, UV = 90.0% > +#define MAX42500_MIN_THRESH_VM1_VM5 0x00 // OV = 102.5%, UV = 97.5% > +#define MAX42500_MAX_THRESH_VM6_V7 0xFF // 1.775v > +#define MAX42500_MIN_THRESH_VM6_V7 0x00 // 0.5v > + > +/* CONFIG1 bit masks */ > +#define MAX42500_CONFIG1_PECE_MASK BIT(0) > +#define MAX42500_CONFIG1_MBST_MASK BIT(1) > +#define MAX42500_CONFIG1_RR_MASK BIT(2) > + > +/* VMON bit masks */ > +#define MAX42500_VMON_IN_MASK(bit) BIT(bit) > +#define MAX42500_VMON_VMPD_MASK BIT(7) > + > +/* RSTMAP bit masks */ > +#define MAX42500_RSTMAP_IN_MASK(bit) BIT(bit) > +#define MAX42500_RSTMAP_PARM_MASK BIT(7) > + > +/* WDCDIV bit masks */ > +#define MAX42500_WDCDIV_SWW_MASK BIT(6) > +#define MAX42500_WDCDIV_WDIC_MASK (0x3F) > + > +/* WDCFG2 bit masks */ > +#define MAX42500_WDCFG2_WDEN_MASK BIT(3) > +#define MAX42500_WDCFG2_1UP_MASK (0x7) > + > +/* WDLOCK bit masks */ > +#define MAX42500_WDLOCK_LOCK_MASK BIT(0) > + > +/* RSTCTRL bit masks */ > +#define MAX42500_RSTCTRL_MR1_MASK BIT(2) > +#define MAX42500_RSTCTRL_RHLD_MASK (0x3) > + > +/* MAX42500 device status */ > +enum max42500_status { > + MAX42500_STATUS_OFF, > + MAX42500_STATUS_SLEEP, > + MAX42500_STATUS_ON, > + MAX42500_STATUS_MAX > +}; > + > +/* MAX42500 voltage monitor input */ > +enum max42500_vm_input { > + MAX42500_VM1, > + MAX42500_VM2, > + MAX42500_VM3, > + MAX42500_VM4, > + MAX42500_VM5, > + MAX42500_VM6, > + MAX42500_VM7, > + MAX42500_VM_MAX > +}; > + > +/* MAX42500 comparator status */ > +enum max42500_comp_stat { > + MAX42500_COMP_STAT_OFF, > + MAX42500_COMP_STAT_UV, > + MAX42500_COMP_STAT_OV, > + MAX42500_COMP_STAT_MAX > +}; > + > +/* MAX42500 watchdog mode */ > +enum max42500_wd_mode { > + MAX42500_WD_MODE_CH_RESP, > + MAX42500_WD_MODE_SIMPLE, > + MAX42500_WD_MODE_MAX > +}; > + > +/* MAX42500 reset hold/active timeout time. */ > +enum max42500_wd_rhld { > + MAX42500_WD_RHOLD_0_MS, > + MAX42500_WD_RHOLD_8_MS, > + MAX42500_WD_RHOLD_16_MS, > + MAX42500_WD_RHOLD_32_MS, > + MAX42500_WD_RHOLD_MAX > +}; > + > +struct max42500_config { > + /* Packet error checking enable */ > + u8 pece; > + /* Enabled voltage monitor inputs */ > + u8 vmon_en; > + /* Voltage monitor power down enable */ > + u8 vmon_vmpd; > + /* Enabled voltage monitor reset mapping */ > + u8 reset_map; > + /* Watchdog mode */ > + enum max42500_wd_mode wd_mode; > + /* Watchdog clock div */ > + u8 wd_cdiv; > + /* Watchdog close window */ > + u8 wd_close; > + /* Watchdog open window */ > + u8 wd_open; > + /* Watchdog first update window */ > + u8 wd_1ud; > + /* Watchdog enable */ > + u8 wd_en; > +}; > + > +struct max42500_state { > + struct i2c_client *client; > + struct regmap *regmap; > + struct max42500_config *config; > + long pwrup_stamp[MAX42500_VM_MAX]; > + long pwrdn_stamp[MAX42500_VM_MAX]; > + u8 ov_thresh1[MAX42500_VM_MAX - 2]; > + u8 ov_thresh2[MAX42500_VM_MAX - 5]; > + u8 uv_thresh1[MAX42500_VM_MAX - 2]; > + u8 uv_thresh2[MAX42500_VM_MAX - 5]; > + u8 nominal_volt[MAX42500_VM_MAX - 2]; > + u8 comp_status[MAX42500_VM_MAX * MAX42500_COMP_STAT_MAX]; > +}; > + > +/************************ Functions Definitions **************************/ > +/** > + * @brief Read a raw value from a register. > + * @return 0 in case of success, error code otherwise. > + */ > +static int max42500_reg_read(struct max42500_state *st, > + u8 reg_addr, u8 *reg_data) > +{ > + int ret; > + u8 i2c_data[MAX42500_I2C_RD_FRAME_SIZE] = {0}; > + u8 bytes_num; > + u8 pece_value; > + > + /* PEC is computed over entire I2C frame from first START condition */ > + i2c_data[0] = (st->client->addr << 1); > + i2c_data[1] = reg_addr; > + i2c_data[2] = (st->client->addr << 1) | 0x1; > + > + /* I2C write target address */ > + bytes_num = 1; > + > + ret = regmap_bulk_write(st->regmap, reg_addr, &i2c_data[1], bytes_num); > + if (ret) > + return ret; > + > + /* Change byte count if PECE is enabled (1-byte data. 1-byte PEC) */ > + bytes_num = (st->config->pece) ? 2 : bytes_num; > + > + ret = regmap_bulk_read(st->regmap, reg_addr, &i2c_data[3], bytes_num); > + if (ret) > + return ret; > + > + if (st->config->pece) { > + /* Compute CRC over entire I2C frame */ > + pece_value = crc8(max42500_crc8, i2c_data, > + (MAX42500_I2C_RD_FRAME_SIZE - 1), 0); > + > + if (i2c_data[4] != pece_value) > + return -EIO; > + } > + > + *reg_data = i2c_data[3]; > + > + return 0; This seems to re-implement PEC support in the driver. As mentioned, PEC support should be handled in the i2c controller driver. Also, _if_ it is for some reasons necessary to implement chip access functions, those should be implemented as regmap bus. The functional part of the driver should just use regmap functions to access the chip. > +} > + > +/** > + * @brief Write a raw value to a register. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_reg_write(struct max42500_state *st, > + u8 reg_addr, u8 data) > +{ > + u8 i2c_data[MAX42500_I2C_WR_FRAME_SIZE] = {0}; > + u8 bytes_num; > + u8 pece_value; > + > + bytes_num = (st->config->pece) ? (MAX42500_I2C_WR_FRAME_SIZE - 1) : 2; > + i2c_data[0] = (st->client->addr << 1); > + i2c_data[1] = reg_addr; > + i2c_data[2] = (u8)(data & 0xFF); > + > + pece_value = 0; > + if (st->config->pece) > + pece_value = crc8(max42500_crc8, i2c_data, bytes_num, 0); > + > + i2c_data[0] = i2c_data[1]; > + i2c_data[1] = i2c_data[2]; > + i2c_data[2] = pece_value; > + > + return regmap_bulk_write(st->regmap, reg_addr, i2c_data, bytes_num); > +} > + > +/** > + * @brief Update a register's value based on a mask. > + * @return 0 in case of success, negative error code otherwise. Please fix. > + */ > +static int max42500_reg_update(struct max42500_state *st, > + u8 reg_addr, u8 mask, u8 data) > +{ > + int ret; > + u8 reg_data; > + > + ret = max42500_reg_read(st, reg_addr, ®_data); > + if (ret) > + return ret; > + > + reg_data &= ~mask; > + reg_data |= mask & data; > + > + return max42500_reg_write(st, reg_addr, reg_data); > +} > + > +/** > + * @brief Set nominal voltage for VM1 to VM5. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_set_nominal_voltage(struct max42500_state *st, > + enum max42500_vm_input vm_in, u8 voltage) > +{ > + u8 reg_addr; > + > + switch (vm_in) { > + case MAX42500_VM1: > + case MAX42500_VM2: > + case MAX42500_VM3: > + case MAX42500_VM4: > + if (voltage < MAX42500_MIN_VNOM || > + voltage > MAX42500_VNOM_MAX_VM1_VM4) > + return -EINVAL; The driver should use clamp_val() for value boundary control. > + reg_addr = MAX42500_REG_VIN1 + vm_in; > + break; > + case MAX42500_VM5: > + if (voltage < MAX42500_MIN_VNOM || > + voltage > MAX42500_VNOM_MAX_VM5) > + return -EINVAL; > + reg_addr = MAX42500_REG_VIN5; > + break; > + default: > + return -EINVAL; > + } > + > + st->nominal_volt[vm_in] = voltage; I fail to understand how this is supposed to work. The locally cached values are set (only) here, with a sysfs attribute. During probe they are not set, meaning the input attribute won't be available because it is only created if nominal_volt is set which is not the case at that time. That doesn't make sense. There needs to be some default. Also, the whole point of using regmap is that regmap provides caching. The code should use that facility instead of caching locally. > + return max42500_reg_write(st, reg_addr, voltage); > +} > + > +/** > + * @brief Get the status of the voltage monitor input. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_get_comp_status(struct max42500_state *st, > + u8 vm_in, u8 *status) > +{ > + int ret; > + u8 reg_addr; > + u8 vm_in_status; > + > + switch (vm_in % MAX42500_COMP_STAT_MAX) { > + case MAX42500_COMP_STAT_OFF: > + reg_addr = MAX42500_REG_STATOFF; > + break; > + case MAX42500_COMP_STAT_UV: > + reg_addr = MAX42500_REG_STATUV; > + break; > + case MAX42500_COMP_STAT_OV: > + reg_addr = MAX42500_REG_STATOV; > + break; > + default: > + return -EINVAL; > + } > + > + ret = max42500_reg_read(st, reg_addr, &vm_in_status); > + if (ret) > + return ret; > + > + switch (vm_in % MAX42500_VM_MAX) { > + case MAX42500_VM1: > + *status = (u8)FIELD_GET(BIT(MAX42500_VM1), vm_in_status); > + break; > + case MAX42500_VM2: > + *status = (u8)FIELD_GET(BIT(MAX42500_VM2), vm_in_status); > + break; > + case MAX42500_VM3: > + *status = (u8)FIELD_GET(BIT(MAX42500_VM3), vm_in_status); > + break; > + case MAX42500_VM4: > + *status = (u8)FIELD_GET(BIT(MAX42500_VM4), vm_in_status); > + break; > + case MAX42500_VM5: > + *status = (u8)FIELD_GET(BIT(MAX42500_VM5), vm_in_status); > + break; > + case MAX42500_VM6: > + *status = (u8)FIELD_GET(BIT(MAX42500_VM6), vm_in_status); > + break; > + case MAX42500_VM7: > + *status = (u8)FIELD_GET(BIT(MAX42500_VM7), vm_in_status); > + break; > + default: > + return -EINVAL; > + } > + st->comp_status[vm_in] = *status; > + > + return 0; > +} > + > +/** > + * @brief Set the overvoltage threshold of VM1 to VM5. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_set_ov_thresh1(struct max42500_state *st, > + enum max42500_vm_input vm_in, u8 thresh) > +{ > + if (thresh < MAX42500_MIN_THRESH_VM1_VM5 || > + thresh > MAX42500_MAX_THRESH_VM1_VM5) > + return -EINVAL; > + > + switch (vm_in) { > + case MAX42500_VM1: > + case MAX42500_VM2: > + case MAX42500_VM3: > + case MAX42500_VM4: > + case MAX42500_VM5: > + st->ov_thresh1[vm_in] = thresh; > + return max42500_reg_update(st, > + MAX42500_REG_OVUV1 + vm_in, > + GENMASK(7, 4), > + FIELD_PREP(GENMASK(7, 4), > + thresh)); > + default: > + return -EINVAL; > + } > +} > + > +/** > + * @brief Set the overvoltage threshold of VM6 and VM7. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_set_ov_thresh2(struct max42500_state *st, > + enum max42500_vm_input vm_in, u8 thresh) > +{ > + u8 reg_addr; > + > + if (thresh < MAX42500_MIN_THRESH_VM6_V7 || > + thresh > MAX42500_MAX_THRESH_VM6_V7) > + return -EINVAL; > + > + switch (vm_in) { > + case MAX42500_VM6: > + reg_addr = MAX42500_REG_VINO6; > + break; > + case MAX42500_VM7: > + reg_addr = MAX42500_REG_VINO7; > + break; > + default: > + return -EINVAL; > + } > + > + st->ov_thresh2[vm_in] = thresh; > + return max42500_reg_write(st, reg_addr, thresh); > +} > + > +/** > + * @brief Set the undervoltage threshold of VM1 to VM5. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_set_uv_thresh1(struct max42500_state *st, > + enum max42500_vm_input vm_in, u8 thresh) > +{ > + if (thresh < MAX42500_MIN_THRESH_VM1_VM5 || > + thresh > MAX42500_MAX_THRESH_VM1_VM5) > + return -EINVAL; > + > + switch (vm_in) { > + case MAX42500_VM1: > + case MAX42500_VM2: > + case MAX42500_VM3: > + case MAX42500_VM4: > + case MAX42500_VM5: > + st->uv_thresh1[vm_in] = thresh; > + return max42500_reg_update(st, > + MAX42500_REG_OVUV1 + vm_in, > + GENMASK(3, 0), > + thresh); > + default: > + return -EINVAL; > + } > +} > + > +/** > + * @brief Set the undervoltage threshold of VM6 and VM7. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_set_uv_thresh2(struct max42500_state *st, > + enum max42500_vm_input vm_in, u8 thresh) > +{ > + u8 reg_addr; > + > + if (thresh < MAX42500_MIN_THRESH_VM6_V7 || > + thresh > MAX42500_MAX_THRESH_VM6_V7) > + return -EINVAL; > + > + switch (vm_in) { > + case MAX42500_VM6: > + reg_addr = MAX42500_REG_VINU6; > + break; > + case MAX42500_VM7: > + reg_addr = MAX42500_REG_VINU7; > + break; > + default: > + return -EINVAL; > + } > + > + st->uv_thresh2[vm_in] = thresh; > + return max42500_reg_write(st, reg_addr, thresh); > +} > + > +/** > + * @brief Get the FPS clock divider value. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_get_fps_clk_div(struct max42500_state *st, > + u8 *fps_clk_div) > +{ > + int ret; > + u8 reg_val; > + > + ret = max42500_reg_read(st, MAX42500_REG_FPSCFG1, ®_val); > + if (ret) > + return ret; > + > + *fps_clk_div = (u8)FIELD_GET(GENMASK(2, 0), reg_val); > + > + return 0; > +} > + > +/** > + * @brief Get power-up timestamp for a specified voltage monitor input. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_get_power_up_timestamp(struct max42500_state *st, > + enum max42500_vm_input vm_in, long *timestamp) > +{ > + int ret; > + u8 reg_val; > + u8 fps_clk_div; > + > + ret = max42500_reg_read(st, MAX42500_REG_UTIME1 + vm_in, ®_val); > + if (ret) > + return ret; > + > + /* Check if the input voltage rose above the UV threshold */ > + if (reg_val == 0) { > + /* Input voltage never rose above UV threshold*/ > + *timestamp = 0; > + return 0; > + } > + > + ret = max42500_get_fps_clk_div(st, &fps_clk_div); > + if (ret) > + return ret; > + > + *timestamp = (reg_val - 1) * 25 * (1 << fps_clk_div); > + st->pwrup_stamp[vm_in] = *timestamp; > + return 0; > +} > + > +/** > + * @brief Get power-down timestamp for a specified voltage monitor input. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_get_power_down_timestamp(struct max42500_state *st, > + enum max42500_vm_input vm_in, long *timestamp) > +{ > + int ret; > + u8 reg_val; > + u8 fps_clk_div; > + > + ret = max42500_reg_read(st, MAX42500_REG_DTIME1 + vm_in, ®_val); > + if (ret) > + return ret; > + > + /* Check if the input voltage fell below the OFF threshold */ > + if (reg_val == 0) { > + /* Input voltage never fell below OFF threshold */ > + *timestamp = 0; > + return 0; > + } > + > + ret = max42500_get_fps_clk_div(st, &fps_clk_div); > + if (ret) > + return ret; > + > + *timestamp = (reg_val - 1) * 25 * (1 << fps_clk_div); > + st->pwrdn_stamp[vm_in] = *timestamp; > + return 0; > +} > + > +/** > + * @brief Enable/Disable watchdog > + * @return 0 in case of success, error code otherwise > + */ > +static int max42500_set_watchdog_enable(struct max42500_state *st, > + bool wd_enable) > +{ > + int ret; > + u8 reg_val; > + > + ret = max42500_reg_read(st, MAX42500_REG_WDCFG2, ®_val); > + if (ret) > + return ret; > + > + if (wd_enable) > + reg_val |= BIT(3); > + else > + reg_val &= ~BIT(3); > + > + return max42500_reg_write(st, MAX42500_REG_WDCFG2, reg_val); > +} > + > +/** > + * @brief 8-bit watchdog key computation. > + * @return 0 in case of success, negative error code otherwise. > + */ > +static int max42500_get_watchdog_key(struct max42500_state *st, > + u8 *new_wd_key) > +{ > + int ret; > + u8 curr_wd_key; > + > + ret = max42500_reg_read(st, MAX42500_REG_WDKEY, &curr_wd_key); > + if (ret) > + return ret; > + > + /* Calculate the new bit using the LFSR polynomial */ > + u8 new_bit = ((curr_wd_key >> 7) ^ > + (curr_wd_key >> 5) ^ > + (curr_wd_key >> 4) ^ > + (curr_wd_key >> 3)) & 0x01; > + > + /* Shift existing bits upwards to MSb and insert the new bit as LSb */ > + *new_wd_key = (curr_wd_key << 1) | new_bit; > + > + return 0; > +} > + > +/** > + * @brief Update the watchdog key based on the mode and current value. > + * @return 0 in case of success, error code otherwise. > + */ > +static int max42500_set_watchdog_key(struct max42500_state *st) > +{ > + int ret; > + u8 reg_val; > + u8 wd_key; > + u8 wd_mode; > + > + ret = max42500_reg_read(st, MAX42500_REG_WDKEY, &wd_key); > + if (ret) > + return ret; > + > + ret = max42500_reg_read(st, MAX42500_REG_WDCDIV, ®_val); > + if (ret) > + return ret; > + > + wd_mode = (u8)FIELD_GET(BIT(6), reg_val); > + > + /* Compute new watchdog key for challenge/response mode */ > + if (wd_mode == MAX42500_WD_MODE_CH_RESP) > + max42500_get_watchdog_key(st, &wd_key); > + > + return max42500_reg_write(st, MAX42500_REG_WDKEY, wd_key); > +} > + > +/** @brief Set watchdog reset hold time > + * @return 0 in case of success, error code otherwise > + */ > +static int max42500_set_watchdog_rhld(struct max42500_state *st, u8 rhld) > +{ > + return max42500_reg_update(st, > + MAX42500_REG_RSTCTRL, > + GENMASK(1, 0), > + rhld); > +} > + > +static umode_t max42500_is_visible(const void *data, > + enum hwmon_sensor_types type, u32 attr, int channel) > +{ > + const struct max42500_state *st = data; > + > + switch (type) { > + case hwmon_chip: > + switch (attr) { > + case hwmon_chip_in_reset_history: > + if (st->pwrup_stamp[channel]) > + return 0444; > + break; > + case hwmon_chip_curr_reset_history: > + if (st->pwrdn_stamp[channel]) > + return 0444; > + break; > + case hwmon_chip_register_tz: > + return 0200; > + case hwmon_chip_alarms: > + case hwmon_chip_update_interval: > + case hwmon_chip_temp_reset_history: > + case hwmon_chip_power_reset_history: > + return 0644; > + } > + break; > + case hwmon_in: > + switch (attr) { > + case hwmon_in_input: > + if (st->nominal_volt[channel]) > + return 0644; > + break; > + case hwmon_in_lowest: > + if (st->ov_thresh1[channel]) > + return 0644; > + break; > + case hwmon_in_highest: > + if (st->ov_thresh2[channel]) > + return 0644; > + break; > + case hwmon_in_min: > + if (st->uv_thresh1[channel]) > + return 0644; > + break; > + case hwmon_in_max: > + if (st->uv_thresh2[channel]) > + return 0644; > + break; > + case hwmon_in_label: > + if (st->comp_status[channel]) > + return 0444; > + break; > + } > + break; > + default: > + break; > + } > + > + return 0; > +} > + > +static int max42500_read_in(struct device *dev, u32 attr, int channel, > + long *value) > +{ > + struct max42500_state *st = dev_get_drvdata(dev); > + > + switch (attr) { > + case hwmon_in_label: > + return max42500_get_comp_status(st, channel, (u8 *)value); The label atribute is supposed to return a text. > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int max42500_read_chip(struct device *dev, u32 attr, int channel, > + long *value) > +{ > + struct max42500_state *st = dev_get_drvdata(dev); > + > + switch (attr) { > + case hwmon_chip_in_reset_history: > + return max42500_get_power_up_timestamp(st, channel, value); > + case hwmon_chip_curr_reset_history: > + return max42500_get_power_down_timestamp(st, channel, value); > + case hwmon_chip_power_reset_history: > + return max42500_get_watchdog_key(st, (u8 *)value); > + case hwmon_chip_update_interval: > + return max42500_get_fps_clk_div(st, (u8 *)value); More ABI violations. > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int max42500_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *value) > +{ > + switch (type) { > + case hwmon_chip: > + return max42500_read_chip(dev, attr, channel, value); > + case hwmon_in: > + return max42500_read_in(dev, attr, channel, value); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int max42500_write_in(struct device *dev, u32 attr, int channel, > + long value) > +{ > + struct max42500_state *st = dev_get_drvdata(dev); > + > + switch (attr) { > + case hwmon_in_input: > + return max42500_set_nominal_voltage(st, channel, (u8)value); This is an ABI violation: inX_input attributes return voltages provided by the chip, and are not writeable. Looking into the datasheet, this has to be set using the min/max attributes. > + case hwmon_in_min: > + return max42500_set_uv_thresh1(st, channel, (u8)value); This is a severe ABI violation. First, again, "(u8)value" is unacceptable. Second, the ABI states that this is a value in mV, not a percentage. I don't know why the chip uses a concept of "nominal voltage" and percentages of that for thresholds. I undersatand that this makes working with the chip difficult, but that does not mean that ABI violations are acceptable. AFAICS the chip doesn't even report the current voltages, so any inX_input attributes must not be provided. The limit attributes must set limits in mV. > + case hwmon_in_max: > + return max42500_set_uv_thresh2(st, channel, (u8)value); > + case hwmon_in_lowest: > + return max42500_set_ov_thresh1(st, channel, (u8)value); > + case hwmon_in_highest: > + return max42500_set_ov_thresh2(st, channel, (u8)value); This looks like another ABI violation. _lowest and _highest_ attributes report historic values. > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int max42500_write_chip(struct device *dev, u32 attr, int channel, > + long value) > +{ > + struct max42500_state *st = dev_get_drvdata(dev); > + > + switch (attr) { > + case hwmon_chip_temp_reset_history: > + return max42500_set_watchdog_rhld(st, (u8)value); > + case hwmon_chip_register_tz: > + return max42500_set_watchdog_key(st); > + case hwmon_chip_alarms: > + return max42500_set_watchdog_enable(st, (bool)value); > + default: The above all seem to abouse the ABI and need documentation and explanation to even be able to evaluate. On top of that, cutting off values such as by "(u8)value" is unacceptable; writing 256 would have the same result as writing 0. > + return -EOPNOTSUPP; > + } > +} > + > +static int max42500_write(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long value) > +{ > + switch (type) { > + case hwmon_chip: > + return max42500_write_chip(dev, attr, channel, value); > + case hwmon_in: > + return max42500_write_in(dev, attr, channel, value); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static const struct max42500_config max42500_init = { > + .pece = true, > + .vmon_en = true, > + .vmon_vmpd = true, > + .reset_map = (MAX42500_RSTMAP_IN_MASK(MAX42500_VM2) | > + MAX42500_RSTMAP_IN_MASK(MAX42500_VM3) | > + MAX42500_RSTMAP_IN_MASK(MAX42500_VM4) | > + MAX42500_RSTMAP_PARM_MASK), > + .wd_mode = MAX42500_WD_MODE_SIMPLE, > + .wd_cdiv = 0x0, > + .wd_close = 0x0, > + .wd_open = 0x0, > + .wd_1ud = 0x0, > + .wd_en = true, > +}; > + > +static int max42500_init_client(struct max42500_state *st, > + struct device *dev) > +{ > + int err; > + u8 device_id; > + > + if (!st || !dev) > + return -EINVAL; How would any of those ever be NULL ? Please drop such unnecessary checks. > + > + crc8_populate_msb(max42500_crc8, CRC8_PEC); > + > + /* Check device silicon ID */ > + err = max42500_reg_read(st, MAX42500_REG_ID, &device_id); > + if (err) > + return err; > + > + if (device_id != MAX42500_SILICON_ID) > + return -ENODEV; > + > + /* Configure PEC */ > + err = max42500_reg_update(st, > + MAX42500_REG_CONFIG1, > + BIT(0), > + max42500_init.pece); > + if (err) > + return err; > + > + /* Set PEC enable/disable for subsequent register access */ > + st->config->pece = max42500_init.pece; > + > + /* Enable voltage monitor inputs */ > + err = max42500_reg_update(st, > + MAX42500_REG_VMON, > + GENMASK(6, 0), > + max42500_init.vmon_en); > + if (err) > + return err; > + > + /* Enable voltage monitor power-down */ > + err = max42500_reg_update(st, > + MAX42500_REG_VMON, > + BIT(7), > + max42500_init.vmon_vmpd); > + if (err) > + return err; > + > + /* Enable input OV/UV mapping to reset pin */ > + err = max42500_reg_update(st, > + MAX42500_REG_RSTMAP, > + GENMASK(6, 0), > + max42500_init.reset_map); > + if (err) > + return err; > + > + /* Set watchdog mode */ > + err = max42500_reg_update(st, > + MAX42500_REG_WDCDIV, > + BIT(6), > + max42500_init.wd_mode << 6); > + if (err) > + return err; > + > + /* Set watchdog clock div */ > + err = max42500_reg_update(st, > + MAX42500_REG_WDCDIV, > + GENMASK(5, 0), > + max42500_init.wd_cdiv); > + if (err) > + return err; > + > + /* Set watchdog open window */ > + err = max42500_reg_update(st, > + MAX42500_REG_WDCFG1, > + GENMASK(3, 0), > + max42500_init.wd_open); > + if (err) > + return err; > + > + /* Set watchdog close window */ > + err = max42500_reg_update(st, > + MAX42500_REG_WDCFG1, > + GENMASK(7, 4), > + max42500_init.wd_close); > + if (err) > + return err; > + > + /* Set watchdog first update window */ > + err = max42500_reg_update(st, > + MAX42500_REG_WDCFG2, > + GENMASK(2, 0), > + max42500_init.wd_1ud); > + if (err) > + return err; > + > + /* Set watchdog enable */ > + err = max42500_reg_update(st, > + MAX42500_REG_WDCFG2, > + BIT(3), > + max42500_init.wd_en); > + if (err) > + return err; > + > + /* Update parameters */ > + st->config->vmon_en = max42500_init.vmon_en; > + st->config->vmon_vmpd = max42500_init.vmon_vmpd; > + st->config->reset_map = max42500_init.reset_map; > + st->config->wd_mode = max42500_init.wd_mode; > + st->config->wd_cdiv = max42500_init.wd_cdiv; > + st->config->wd_open = max42500_init.wd_open; > + st->config->wd_close = max42500_init.wd_close; > + st->config->wd_1ud = max42500_init.wd_1ud; > + st->config->wd_en = max42500_init.wd_en; Why not just setting a pointer to max42500_init ? > + > + return 0; > +} > + > +static const struct hwmon_ops max42500_hwmon_ops = { > + .is_visible = max42500_is_visible, > + .read = max42500_read, > + .write = max42500_write, > +}; > + > +static const struct hwmon_channel_info *max42500_info[] = { > + HWMON_CHANNEL_INFO(chip, > + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY | > + HWMON_C_TEMP_RESET_HISTORY | HWMON_C_POWER_RESET_HISTORY | > + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL | HWMON_C_ALARMS, > + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, > + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, > + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, > + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, > + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY, > + HWMON_C_IN_RESET_HISTORY | HWMON_C_CURR_RESET_HISTORY), > + HWMON_CHANNEL_INFO(in, > + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN | > + HWMON_I_HIGHEST | HWMON_I_MAX, > + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN | > + HWMON_I_HIGHEST | HWMON_I_MAX, > + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN, > + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN, > + HWMON_I_LABEL | HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_MIN, > + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, > + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, > + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, > + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL), > + NULL > +}; > + > +static const struct hwmon_chip_info max42500_chip_info = { > + .ops = &max42500_hwmon_ops, > + .info = max42500_info, > +}; > + > +static const struct regmap_config max42500_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = 0x2D > +}; > + > +static int max42500_probe(struct i2c_client *client) > +{ > + struct device *hwmon_dev; > + struct max42500_state *st; > + int err; > + > + st = devm_kzalloc(&client->dev, sizeof(*st), GFP_KERNEL); > + if (!st) > + return -ENOMEM; > + > + st->client = client; > + st->regmap = devm_regmap_init_i2c(client, &max42500_regmap_config); > + if (IS_ERR(st->regmap)) > + return PTR_ERR(st->regmap); > + > + err = max42500_init_client(st, &client->dev); > + if (err) > + return err; > + > + hwmon_dev = devm_hwmon_device_register_with_info(&client->dev, > + client->name, st, &max42500_chip_info, NULL); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct of_device_id max42500_of_match[] = { > + { .compatible = "adi,max42500" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, max42500_of_match); > + > +static const struct i2c_device_id max42500_id[] = { > + { "max42500", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, max42500_id); > + > +static struct i2c_driver max42500_driver = { > + .driver = { > + .name = "max42500", > + .of_match_table = max42500_of_match, > + }, > + .probe = max42500_probe, > + .id_table = max42500_id, > +}; > + > +module_i2c_driver(max42500_driver); > + > +MODULE_AUTHOR("Kent Libetario <kent.libetario@analog.com>"); > +MODULE_DESCRIPTION("Hwmon driver for MAX42500"); > +MODULE_LICENSE("GPL"); ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] drivers: hwmon: add driver for max42500 2024-12-21 18:08 ` Guenter Roeck @ 2025-03-10 9:05 ` Libetario, Kent 0 siblings, 0 replies; 8+ messages in thread From: Libetario, Kent @ 2025-03-10 9:05 UTC (permalink / raw) To: Guenter Roeck Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Jean Delvare, Rob Herring, Krzysztof Kozlowski, Conor Dooley > -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Sunday, December 22, 2024 2:09 AM > To: Libetario, Kent <Kent.Libetario@analog.com> > Cc: linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Jean Delvare <jdelvare@suse.com>; Rob Herring > <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley > <conor+dt@kernel.org> > Subject: Re: [PATCH 2/2] drivers: hwmon: add driver for max42500 > > [External] > > On Fri, Dec 20, 2024 at 09:20:03AM +0800, Kent Libetario wrote: > > Add the core implementation of the MAX42500 hwmon driver. > > > > Please have a look into Documentation/process/submitting-patches.rst > and follow the guidance about describing patches. > Yes, I will look into the documentation and update the patch descriptions. > > Signed-off-by: Kent Libetario <Kent.Libetario@analog.com> > > --- > > MAINTAINERS | 1 + > > drivers/hwmon/Kconfig | 13 + > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/max42500.c | 1049 > > ++++++++++++++++++++++++++++++++++++++ > > Documentation missing. > > There is a large number of checkpatch CHECK messages, almost all about multi- > line alignment. Indeed, when looking through the code, multi-line aligment > seems arbitrary. Please fix. > > PEC support should be implemented through the i2c controller driver. > Please see other drivers such as lm90.c or max31827.c for examples how to do > that. > > Some more comments inline. The review is partial. The major problem I see is > that there are not just a few but _lots_ of ABI violations. > This is unacceptable. Please rework the driver and follow the hwmon ABI for all > sysfs attributes. > > Guenter > Yes, I have reworked the driver and that is why it took me sometime to reply from your review comments due to a large revision. I have fixed the alignment format issues updated the code and added the documentation as you instructed and from the driver references you provided. I have initiated also to divide the drivers into three, including hwmon driver. The other 2 drivers are the mfd and watchdog drivers using their own ABI sysfs attributes standards. Also, I have used debugfs for the non-standard attributes in all of three drivers that are not defined in their ABI sysfs document. Thanks, Kent > > 4 files changed, 1064 insertions(+) > > create mode 100644 drivers/hwmon/max42500.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS index > > d1703b4834c8..434191e16dd5 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -14051,6 +14051,7 @@ M: Kent Libetario <kent.libetario@analog.com> > > L: linux-hwmon@vger.kernel.org > > S: Supported > > F: Documentation/devicetree/bindings/hwmon/adi,max42500.yaml > > +F: drivers/hwmon/max42500.c > > > > MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER > > L: linux-hwmon@vger.kernel.org > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index > > dd376602f3f1..ec0d7aad7789 100644 > > --- a/drivers/hwmon/Kconfig > > +++ b/drivers/hwmon/Kconfig > > @@ -1220,6 +1220,19 @@ config MAX31827 > > This driver can also be built as a module. If so, the module > > will be called max31827. > > > > +config SENSORS_MAX42500 > > + tristate "MAX42500 Industrial Power System Monitor Family" > > + depends on I2C > > + select REGMAP_I2C > > + help > > + If you say yes here you get support for MAX42500 SoC power-system > monitor > > + with up to seven voltage monitor. The driver also contains a > programmable > > + challenge/response watchdog, which is accessible through the I2C > interface, > > + along with a configurable RESET output. > > + > > + This driver can also be built as a module. If so, the module > > + will be called max42500. > > + > > config SENSORS_MAX6620 > > tristate "Maxim MAX6620 fan controller" > > depends on I2C > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index > > b827b92f2a78..d27d8fc01141 100644 > > --- a/drivers/hwmon/Makefile > > +++ b/drivers/hwmon/Makefile > > @@ -152,6 +152,7 @@ obj-$(CONFIG_SENSORS_MAX197) += max197.o > > obj-$(CONFIG_SENSORS_MAX31722) += max31722.o > > obj-$(CONFIG_SENSORS_MAX31730) += max31730.o > > obj-$(CONFIG_SENSORS_MAX31760) += max31760.o > > +obj-$(CONFIG_SENSORS_MAX42500) += max42500.o > > obj-$(CONFIG_SENSORS_MAX6620) += max6620.o > > obj-$(CONFIG_SENSORS_MAX6621) += max6621.o > > obj-$(CONFIG_SENSORS_MAX6639) += max6639.o > > diff --git a/drivers/hwmon/max42500.c b/drivers/hwmon/max42500.c new > > file mode 100644 index 000000000000..23b90c766767 > > --- /dev/null > > +++ b/drivers/hwmon/max42500.c > > @@ -0,0 +1,1049 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * MAX42500 - Industrial Power System Monitor > > + * > > + * Copyright 2024 Analog Devices Inc. > > + */ > > + > > +#include <linux/init.h> > > +#include <linux/jiffies.h> > > +#include <linux/slab.h> > > +#include <linux/crc8.h> > > +#include <linux/bitfield.h> > > +#include <linux/bitops.h> > > +#include <linux/err.h> > > +#include <linux/gpio.h> > > +#include <linux/gpio/driver.h> > > +#include <linux/hwmon.h> > > +#include <linux/i2c.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/property.h> > > +#include <linux/regmap.h> > > + > > +/* Implements Polynomial X^8 + X^2 + X^1 +1 */ > > +#define CRC8_PEC 0x07 > > + > > +DECLARE_CRC8_TABLE(max42500_crc8); > > + > > +#define MAX42500_REG_ID 0x00 > > +#define MAX42500_REG_CONFIG1 0x01 > > +#define MAX42500_REG_CONFIG2 0x02 > > +#define MAX42500_REG_VMON 0x03 > > +#define MAX42500_REG_RSTMAP 0x04 > > +#define MAX42500_REG_STATOV 0x05 > > +#define MAX42500_REG_STATUV 0x06 > > +#define MAX42500_REG_STATOFF 0x07 > > +#define MAX42500_REG_VIN1 0x08 > > +#define MAX42500_REG_VIN2 0x09 > > +#define MAX42500_REG_VIN3 0x0A > > +#define MAX42500_REG_VIN4 0x0B > > +#define MAX42500_REG_VIN5 0x0C > > +#define MAX42500_REG_VINO6 0x0D > > +#define MAX42500_REG_VINU6 0x0E > > +#define MAX42500_REG_VINO7 0x0F > > +#define MAX42500_REG_VINU7 0x10 > > +#define MAX42500_REG_OVUV1 0x11 > > +#define MAX42500_REG_OVUV2 0x12 > > +#define MAX42500_REG_OVUV3 0x13 > > +#define MAX42500_REG_OVUV4 0x14 > > +#define MAX42500_REG_OVUV5 0x15 > > +#define MAX42500_REG_FPSSTAT1 0x16 > > +#define MAX42500_REG_FPSCFG1 0x17 > > +#define MAX42500_REG_UTIME1 0x18 > > +#define MAX42500_REG_UTIME2 0x19 > > +#define MAX42500_REG_UTIME3 0x1A > > +#define MAX42500_REG_UTIME4 0x1B > > +#define MAX42500_REG_UTIME5 0x1C > > +#define MAX42500_REG_UTIME6 0x1D > > +#define MAX42500_REG_UTIME7 0x1E > > +#define MAX42500_REG_DTIME1 0x1F > > +#define MAX42500_REG_DTIME2 0x20 > > +#define MAX42500_REG_DTIME3 0x21 > > +#define MAX42500_REG_DTIME4 0x22 > > +#define MAX42500_REG_DTIME5 0x23 > > +#define MAX42500_REG_DTIME6 0x24 > > +#define MAX42500_REG_DTIME7 0x25 > > +#define MAX42500_REG_WDSTAT 0x26 > > +#define MAX42500_REG_WDCDIV 0x27 > > +#define MAX42500_REG_WDCFG1 0x28 > > +#define MAX42500_REG_WDCFG2 0x29 > > +#define MAX42500_REG_WDKEY 0x2A > > +#define MAX42500_REG_WDLOCK 0x2B > > +#define MAX42500_REG_RSTCTRL 0x2C > > +#define MAX42500_REG_CID 0x2D > > + > > +/** X is set based on the pull configuration of the ADDR pin */ > > +#define MAX42500_ADDR(x) (0x28 + (x)) > > +#define MAX42500_SILICON_ID (0x30) > > +#define MAX42500_I2C_WR_FRAME_SIZE (4) > > +#define MAX42500_I2C_RD_FRAME_SIZE (5) > > + > > +/** MAX42500 Nominal voltage computation */ > > +#define MAX42500_VNOM_MAX_VM1_VM4 0xFF // 3.6875v > > +#define MAX42500_VNOM_MAX_VM5 0xFF // 5.6v > > +#define MAX42500_MIN_VNOM 0x00 // 0.5v > > +#define MAX42500_VNOM_STEP_VM1_VM4 0x01 // 0.0125v > > +#define MAX42500_VNOM_STEP_VM5 0x01 // 0.02v > > + > > +/** MAX42500 Undervoltage/Overvoltage maximum and minimum > thresholds*/ > > +#define MAX42500_MAX_THRESH_VM1_VM5 0x0F // OV = 110.0%, > UV = 90.0% > > +#define MAX42500_MIN_THRESH_VM1_VM5 0x00 // OV = 102.5%, > UV = 97.5% > > +#define MAX42500_MAX_THRESH_VM6_V7 0xFF // 1.775v > > +#define MAX42500_MIN_THRESH_VM6_V7 0x00 // 0.5v > > + > > +/* CONFIG1 bit masks */ > > +#define MAX42500_CONFIG1_PECE_MASK BIT(0) > > +#define MAX42500_CONFIG1_MBST_MASK BIT(1) > > +#define MAX42500_CONFIG1_RR_MASK BIT(2) > > + > > +/* VMON bit masks */ > > +#define MAX42500_VMON_IN_MASK(bit) BIT(bit) > > +#define MAX42500_VMON_VMPD_MASK BIT(7) > > + > > +/* RSTMAP bit masks */ > > +#define MAX42500_RSTMAP_IN_MASK(bit) BIT(bit) > > +#define MAX42500_RSTMAP_PARM_MASK BIT(7) > > + > > +/* WDCDIV bit masks */ > > +#define MAX42500_WDCDIV_SWW_MASK BIT(6) > > +#define MAX42500_WDCDIV_WDIC_MASK (0x3F) > > + > > +/* WDCFG2 bit masks */ > > +#define MAX42500_WDCFG2_WDEN_MASK BIT(3) > > +#define MAX42500_WDCFG2_1UP_MASK (0x7) > > + > > +/* WDLOCK bit masks */ > > +#define MAX42500_WDLOCK_LOCK_MASK BIT(0) > > + > > +/* RSTCTRL bit masks */ > > +#define MAX42500_RSTCTRL_MR1_MASK BIT(2) > > +#define MAX42500_RSTCTRL_RHLD_MASK (0x3) > > + > > +/* MAX42500 device status */ > > +enum max42500_status { > > + MAX42500_STATUS_OFF, > > + MAX42500_STATUS_SLEEP, > > + MAX42500_STATUS_ON, > > + MAX42500_STATUS_MAX > > +}; > > + > > +/* MAX42500 voltage monitor input */ > > +enum max42500_vm_input { > > + MAX42500_VM1, > > + MAX42500_VM2, > > + MAX42500_VM3, > > + MAX42500_VM4, > > + MAX42500_VM5, > > + MAX42500_VM6, > > + MAX42500_VM7, > > + MAX42500_VM_MAX > > +}; > > + > > +/* MAX42500 comparator status */ > > +enum max42500_comp_stat { > > + MAX42500_COMP_STAT_OFF, > > + MAX42500_COMP_STAT_UV, > > + MAX42500_COMP_STAT_OV, > > + MAX42500_COMP_STAT_MAX > > +}; > > + > > +/* MAX42500 watchdog mode */ > > +enum max42500_wd_mode { > > + MAX42500_WD_MODE_CH_RESP, > > + MAX42500_WD_MODE_SIMPLE, > > + MAX42500_WD_MODE_MAX > > +}; > > + > > +/* MAX42500 reset hold/active timeout time. */ enum max42500_wd_rhld > > +{ > > + MAX42500_WD_RHOLD_0_MS, > > + MAX42500_WD_RHOLD_8_MS, > > + MAX42500_WD_RHOLD_16_MS, > > + MAX42500_WD_RHOLD_32_MS, > > + MAX42500_WD_RHOLD_MAX > > +}; > > + > > +struct max42500_config { > > + /* Packet error checking enable */ > > + u8 pece; > > + /* Enabled voltage monitor inputs */ > > + u8 vmon_en; > > + /* Voltage monitor power down enable */ > > + u8 vmon_vmpd; > > + /* Enabled voltage monitor reset mapping */ > > + u8 reset_map; > > + /* Watchdog mode */ > > + enum max42500_wd_mode wd_mode; > > + /* Watchdog clock div */ > > + u8 wd_cdiv; > > + /* Watchdog close window */ > > + u8 wd_close; > > + /* Watchdog open window */ > > + u8 wd_open; > > + /* Watchdog first update window */ > > + u8 wd_1ud; > > + /* Watchdog enable */ > > + u8 wd_en; > > +}; > > + > > +struct max42500_state { > > + struct i2c_client *client; > > + struct regmap *regmap; > > + struct max42500_config *config; > > + long pwrup_stamp[MAX42500_VM_MAX]; > > + long pwrdn_stamp[MAX42500_VM_MAX]; > > + u8 ov_thresh1[MAX42500_VM_MAX - 2]; > > + u8 ov_thresh2[MAX42500_VM_MAX - 5]; > > + u8 uv_thresh1[MAX42500_VM_MAX - 2]; > > + u8 uv_thresh2[MAX42500_VM_MAX - 5]; > > + u8 nominal_volt[MAX42500_VM_MAX - 2]; > > + u8 comp_status[MAX42500_VM_MAX * MAX42500_COMP_STAT_MAX]; }; > > + > > +/************************ Functions Definitions > > +**************************/ > > +/** > > + * @brief Read a raw value from a register. > > + * @return 0 in case of success, error code otherwise. > > + */ > > +static int max42500_reg_read(struct max42500_state *st, > > + u8 reg_addr, u8 > *reg_data) > > +{ > > + int ret; > > + u8 i2c_data[MAX42500_I2C_RD_FRAME_SIZE] = {0}; > > + u8 bytes_num; > > + u8 pece_value; > > + > > + /* PEC is computed over entire I2C frame from first START condition */ > > + i2c_data[0] = (st->client->addr << 1); > > + i2c_data[1] = reg_addr; > > + i2c_data[2] = (st->client->addr << 1) | 0x1; > > + > > + /* I2C write target address */ > > + bytes_num = 1; > > + > > + ret = regmap_bulk_write(st->regmap, reg_addr, &i2c_data[1], > bytes_num); > > + if (ret) > > + return ret; > > + > > + /* Change byte count if PECE is enabled (1-byte data. 1-byte PEC) */ > > + bytes_num = (st->config->pece) ? 2 : bytes_num; > > + > > + ret = regmap_bulk_read(st->regmap, reg_addr, &i2c_data[3], > bytes_num); > > + if (ret) > > + return ret; > > + > > + if (st->config->pece) { > > + /* Compute CRC over entire I2C frame */ > > + pece_value = crc8(max42500_crc8, i2c_data, > > + > (MAX42500_I2C_RD_FRAME_SIZE - 1), 0); > > + > > + if (i2c_data[4] != pece_value) > > + return -EIO; > > + } > > + > > + *reg_data = i2c_data[3]; > > + > > + return 0; > > This seems to re-implement PEC support in the driver. As mentioned, PEC > support should be handled in the i2c controller driver. > Yes, I will update the code to reflect you instructions to use the i2c driver's PEC implementation. > Also, _if_ it is for some reasons necessary to implement chip access functions, > those should be implemented as regmap bus. The functional part of the driver > should just use regmap functions to access the chip. > Yes, I agree and will update the code to use regmap functions as you instructed. > > +} > > + > > +/** > > + * @brief Write a raw value to a register. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_reg_write(struct max42500_state *st, > > + u8 reg_addr, u8 data) > > +{ > > + u8 i2c_data[MAX42500_I2C_WR_FRAME_SIZE] = {0}; > > + u8 bytes_num; > > + u8 pece_value; > > + > > + bytes_num = (st->config->pece) ? (MAX42500_I2C_WR_FRAME_SIZE - 1) : > 2; > > + i2c_data[0] = (st->client->addr << 1); > > + i2c_data[1] = reg_addr; > > + i2c_data[2] = (u8)(data & 0xFF); > > + > > + pece_value = 0; > > + if (st->config->pece) > > + pece_value = crc8(max42500_crc8, i2c_data, bytes_num, 0); > > + > > + i2c_data[0] = i2c_data[1]; > > + i2c_data[1] = i2c_data[2]; > > + i2c_data[2] = pece_value; > > + > > + return regmap_bulk_write(st->regmap, reg_addr, i2c_data, bytes_num); > > +} > > + > > +/** > > + * @brief Update a register's value based on a mask. > > + * @return 0 in case of success, negative error code otherwise. > > Please fix. > Yes, will do. > > + */ > > +static int max42500_reg_update(struct max42500_state *st, > > + u8 reg_addr, u8 mask, > u8 data) { > > + int ret; > > + u8 reg_data; > > + > > + ret = max42500_reg_read(st, reg_addr, ®_data); > > + if (ret) > > + return ret; > > + > > + reg_data &= ~mask; > > + reg_data |= mask & data; > > + > > + return max42500_reg_write(st, reg_addr, reg_data); } > > + > > +/** > > + * @brief Set nominal voltage for VM1 to VM5. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_set_nominal_voltage(struct max42500_state *st, > > + enum max42500_vm_input vm_in, u8 voltage) { > > + u8 reg_addr; > > + > > + switch (vm_in) { > > + case MAX42500_VM1: > > + case MAX42500_VM2: > > + case MAX42500_VM3: > > + case MAX42500_VM4: > > + if (voltage < MAX42500_MIN_VNOM || > > + voltage > MAX42500_VNOM_MAX_VM1_VM4) > > + return -EINVAL; > > The driver should use clamp_val() for value boundary control. > Yes, I will update the code according to your instructions. > > + reg_addr = MAX42500_REG_VIN1 + vm_in; > > + break; > > + case MAX42500_VM5: > > + if (voltage < MAX42500_MIN_VNOM || > > + voltage > MAX42500_VNOM_MAX_VM5) > > + return -EINVAL; > > + reg_addr = MAX42500_REG_VIN5; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + st->nominal_volt[vm_in] = voltage; > > I fail to understand how this is supposed to work. The locally cached values are > set (only) here, with a sysfs attribute. During probe they are not set, meaning > the input attribute won't be available because it is only created if nominal_volt > is set which is not the case at that time. That doesn't make sense. There needs > to be some default. > I will update the code to get first the latest value from the userspace by the sysfs attributes or from the device registers, since the device itself already has default OTP initial values setting. > Also, the whole point of using regmap is that regmap provides caching. The code > should use that facility instead of caching locally. > Yes, I will update the code to reflect the proper usage of regmap according to your instructions. > > + return max42500_reg_write(st, reg_addr, voltage); } > > + > > +/** > > + * @brief Get the status of the voltage monitor input. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_get_comp_status(struct max42500_state *st, > > + u8 vm_in, u8 > *status) > > +{ > > + int ret; > > + u8 reg_addr; > > + u8 vm_in_status; > > + > > + switch (vm_in % MAX42500_COMP_STAT_MAX) { > > + case MAX42500_COMP_STAT_OFF: > > + reg_addr = MAX42500_REG_STATOFF; > > + break; > > + case MAX42500_COMP_STAT_UV: > > + reg_addr = MAX42500_REG_STATUV; > > + break; > > + case MAX42500_COMP_STAT_OV: > > + reg_addr = MAX42500_REG_STATOV; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + ret = max42500_reg_read(st, reg_addr, &vm_in_status); > > + if (ret) > > + return ret; > > + > > + switch (vm_in % MAX42500_VM_MAX) { > > + case MAX42500_VM1: > > + *status = (u8)FIELD_GET(BIT(MAX42500_VM1), vm_in_status); > > + break; > > + case MAX42500_VM2: > > + *status = (u8)FIELD_GET(BIT(MAX42500_VM2), vm_in_status); > > + break; > > + case MAX42500_VM3: > > + *status = (u8)FIELD_GET(BIT(MAX42500_VM3), vm_in_status); > > + break; > > + case MAX42500_VM4: > > + *status = (u8)FIELD_GET(BIT(MAX42500_VM4), vm_in_status); > > + break; > > + case MAX42500_VM5: > > + *status = (u8)FIELD_GET(BIT(MAX42500_VM5), vm_in_status); > > + break; > > + case MAX42500_VM6: > > + *status = (u8)FIELD_GET(BIT(MAX42500_VM6), vm_in_status); > > + break; > > + case MAX42500_VM7: > > + *status = (u8)FIELD_GET(BIT(MAX42500_VM7), vm_in_status); > > + break; > > + default: > > + return -EINVAL; > > + } > > + st->comp_status[vm_in] = *status; > > + > > + return 0; > > +} > > + > > +/** > > + * @brief Set the overvoltage threshold of VM1 to VM5. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_set_ov_thresh1(struct max42500_state *st, > > + enum max42500_vm_input vm_in, u8 thresh) { > > + if (thresh < MAX42500_MIN_THRESH_VM1_VM5 || > > + thresh > MAX42500_MAX_THRESH_VM1_VM5) > > + return -EINVAL; > > + > > + switch (vm_in) { > > + case MAX42500_VM1: > > + case MAX42500_VM2: > > + case MAX42500_VM3: > > + case MAX42500_VM4: > > + case MAX42500_VM5: > > + st->ov_thresh1[vm_in] = thresh; > > + return max42500_reg_update(st, > > + > MAX42500_REG_OVUV1 + vm_in, > > + GENMASK(7, 4), > > + > FIELD_PREP(GENMASK(7, 4), > > + thresh)); > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +/** > > + * @brief Set the overvoltage threshold of VM6 and VM7. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_set_ov_thresh2(struct max42500_state *st, > > + enum max42500_vm_input vm_in, u8 thresh) { > > + u8 reg_addr; > > + > > + if (thresh < MAX42500_MIN_THRESH_VM6_V7 || > > + thresh > MAX42500_MAX_THRESH_VM6_V7) > > + return -EINVAL; > > + > > + switch (vm_in) { > > + case MAX42500_VM6: > > + reg_addr = MAX42500_REG_VINO6; > > + break; > > + case MAX42500_VM7: > > + reg_addr = MAX42500_REG_VINO7; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + st->ov_thresh2[vm_in] = thresh; > > + return max42500_reg_write(st, reg_addr, thresh); } > > + > > +/** > > + * @brief Set the undervoltage threshold of VM1 to VM5. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_set_uv_thresh1(struct max42500_state *st, > > + enum max42500_vm_input vm_in, u8 thresh) { > > + if (thresh < MAX42500_MIN_THRESH_VM1_VM5 || > > + thresh > MAX42500_MAX_THRESH_VM1_VM5) > > + return -EINVAL; > > + > > + switch (vm_in) { > > + case MAX42500_VM1: > > + case MAX42500_VM2: > > + case MAX42500_VM3: > > + case MAX42500_VM4: > > + case MAX42500_VM5: > > + st->uv_thresh1[vm_in] = thresh; > > + return max42500_reg_update(st, > > + MAX42500_REG_OVUV1 + vm_in, > > + GENMASK(3, 0), > > + thresh); > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +/** > > + * @brief Set the undervoltage threshold of VM6 and VM7. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_set_uv_thresh2(struct max42500_state *st, > > + enum max42500_vm_input vm_in, u8 thresh) { > > + u8 reg_addr; > > + > > + if (thresh < MAX42500_MIN_THRESH_VM6_V7 || > > + thresh > MAX42500_MAX_THRESH_VM6_V7) > > + return -EINVAL; > > + > > + switch (vm_in) { > > + case MAX42500_VM6: > > + reg_addr = MAX42500_REG_VINU6; > > + break; > > + case MAX42500_VM7: > > + reg_addr = MAX42500_REG_VINU7; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + st->uv_thresh2[vm_in] = thresh; > > + return max42500_reg_write(st, reg_addr, thresh); } > > + > > +/** > > + * @brief Get the FPS clock divider value. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_get_fps_clk_div(struct max42500_state *st, > > + u8 *fps_clk_div) > > +{ > > + int ret; > > + u8 reg_val; > > + > > + ret = max42500_reg_read(st, MAX42500_REG_FPSCFG1, ®_val); > > + if (ret) > > + return ret; > > + > > + *fps_clk_div = (u8)FIELD_GET(GENMASK(2, 0), reg_val); > > + > > + return 0; > > +} > > + > > +/** > > + * @brief Get power-up timestamp for a specified voltage monitor input. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_get_power_up_timestamp(struct max42500_state *st, > > + enum max42500_vm_input vm_in, long *timestamp) { > > + int ret; > > + u8 reg_val; > > + u8 fps_clk_div; > > + > > + ret = max42500_reg_read(st, MAX42500_REG_UTIME1 + vm_in, > ®_val); > > + if (ret) > > + return ret; > > + > > + /* Check if the input voltage rose above the UV threshold */ > > + if (reg_val == 0) { > > + /* Input voltage never rose above UV threshold*/ > > + *timestamp = 0; > > + return 0; > > + } > > + > > + ret = max42500_get_fps_clk_div(st, &fps_clk_div); > > + if (ret) > > + return ret; > > + > > + *timestamp = (reg_val - 1) * 25 * (1 << fps_clk_div); > > + st->pwrup_stamp[vm_in] = *timestamp; > > + return 0; > > +} > > + > > +/** > > + * @brief Get power-down timestamp for a specified voltage monitor input. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_get_power_down_timestamp(struct max42500_state *st, > > + enum max42500_vm_input vm_in, long *timestamp) { > > + int ret; > > + u8 reg_val; > > + u8 fps_clk_div; > > + > > + ret = max42500_reg_read(st, MAX42500_REG_DTIME1 + vm_in, > ®_val); > > + if (ret) > > + return ret; > > + > > + /* Check if the input voltage fell below the OFF threshold */ > > + if (reg_val == 0) { > > + /* Input voltage never fell below OFF threshold */ > > + *timestamp = 0; > > + return 0; > > + } > > + > > + ret = max42500_get_fps_clk_div(st, &fps_clk_div); > > + if (ret) > > + return ret; > > + > > + *timestamp = (reg_val - 1) * 25 * (1 << fps_clk_div); > > + st->pwrdn_stamp[vm_in] = *timestamp; > > + return 0; > > +} > > + > > +/** > > + * @brief Enable/Disable watchdog > > + * @return 0 in case of success, error code otherwise */ static int > > +max42500_set_watchdog_enable(struct max42500_state *st, > > + bool > wd_enable) > > +{ > > + int ret; > > + u8 reg_val; > > + > > + ret = max42500_reg_read(st, MAX42500_REG_WDCFG2, ®_val); > > + if (ret) > > + return ret; > > + > > + if (wd_enable) > > + reg_val |= BIT(3); > > + else > > + reg_val &= ~BIT(3); > > + > > + return max42500_reg_write(st, MAX42500_REG_WDCFG2, reg_val); } > > + > > +/** > > + * @brief 8-bit watchdog key computation. > > + * @return 0 in case of success, negative error code otherwise. > > + */ > > +static int max42500_get_watchdog_key(struct max42500_state *st, > > + u8 > *new_wd_key) > > +{ > > + int ret; > > + u8 curr_wd_key; > > + > > + ret = max42500_reg_read(st, MAX42500_REG_WDKEY, &curr_wd_key); > > + if (ret) > > + return ret; > > + > > + /* Calculate the new bit using the LFSR polynomial */ > > + u8 new_bit = ((curr_wd_key >> 7) ^ > > + (curr_wd_key >> 5) ^ > > + (curr_wd_key >> 4) ^ > > + (curr_wd_key >> 3)) & 0x01; > > + > > + /* Shift existing bits upwards to MSb and insert the new bit as LSb */ > > + *new_wd_key = (curr_wd_key << 1) | new_bit; > > + > > + return 0; > > +} > > + > > +/** > > + * @brief Update the watchdog key based on the mode and current value. > > + * @return 0 in case of success, error code otherwise. > > + */ > > +static int max42500_set_watchdog_key(struct max42500_state *st) { > > + int ret; > > + u8 reg_val; > > + u8 wd_key; > > + u8 wd_mode; > > + > > + ret = max42500_reg_read(st, MAX42500_REG_WDKEY, &wd_key); > > + if (ret) > > + return ret; > > + > > + ret = max42500_reg_read(st, MAX42500_REG_WDCDIV, ®_val); > > + if (ret) > > + return ret; > > + > > + wd_mode = (u8)FIELD_GET(BIT(6), reg_val); > > + > > + /* Compute new watchdog key for challenge/response mode */ > > + if (wd_mode == MAX42500_WD_MODE_CH_RESP) > > + max42500_get_watchdog_key(st, &wd_key); > > + > > + return max42500_reg_write(st, MAX42500_REG_WDKEY, wd_key); } > > + > > +/** @brief Set watchdog reset hold time > > + * @return 0 in case of success, error code otherwise */ static int > > +max42500_set_watchdog_rhld(struct max42500_state *st, u8 rhld) { > > + return max42500_reg_update(st, > > + > MAX42500_REG_RSTCTRL, > > + GENMASK(1, 0), > > + rhld); > > +} > > + > > +static umode_t max42500_is_visible(const void *data, > > + enum hwmon_sensor_types type, u32 attr, int channel) { > > + const struct max42500_state *st = data; > > + > > + switch (type) { > > + case hwmon_chip: > > + switch (attr) { > > + case hwmon_chip_in_reset_history: > > + if (st->pwrup_stamp[channel]) > > + return 0444; > > + break; > > + case hwmon_chip_curr_reset_history: > > + if (st->pwrdn_stamp[channel]) > > + return 0444; > > + break; > > + case hwmon_chip_register_tz: > > + return 0200; > > + case hwmon_chip_alarms: > > + case hwmon_chip_update_interval: > > + case hwmon_chip_temp_reset_history: > > + case hwmon_chip_power_reset_history: > > + return 0644; > > + } > > + break; > > + case hwmon_in: > > + switch (attr) { > > + case hwmon_in_input: > > + if (st->nominal_volt[channel]) > > + return 0644; > > + break; > > + case hwmon_in_lowest: > > + if (st->ov_thresh1[channel]) > > + return 0644; > > + break; > > + case hwmon_in_highest: > > + if (st->ov_thresh2[channel]) > > + return 0644; > > + break; > > + case hwmon_in_min: > > + if (st->uv_thresh1[channel]) > > + return 0644; > > + break; > > + case hwmon_in_max: > > + if (st->uv_thresh2[channel]) > > + return 0644; > > + break; > > + case hwmon_in_label: > > + if (st->comp_status[channel]) > > + return 0444; > > + break; > > + } > > + break; > > + default: > > + break; > > + } > > + > > + return 0; > > +} > > + > > +static int max42500_read_in(struct device *dev, u32 attr, int channel, > > + long *value) > > +{ > > + struct max42500_state *st = dev_get_drvdata(dev); > > + > > + switch (attr) { > > + case hwmon_in_label: > > + return max42500_get_comp_status(st, channel, (u8 *)value); > > The label atribute is supposed to return a text. > Yes, I agree and will update the code according to the ABI sysfs standards. > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static int max42500_read_chip(struct device *dev, u32 attr, int channel, > > + long *value) > > +{ > > + struct max42500_state *st = dev_get_drvdata(dev); > > + > > + switch (attr) { > > + case hwmon_chip_in_reset_history: > > + return max42500_get_power_up_timestamp(st, channel, value); > > + case hwmon_chip_curr_reset_history: > > + return max42500_get_power_down_timestamp(st, channel, > value); > > + case hwmon_chip_power_reset_history: > > + return max42500_get_watchdog_key(st, (u8 *)value); > > + case hwmon_chip_update_interval: > > + return max42500_get_fps_clk_div(st, (u8 *)value); > > More ABI violations. > Yes, I agree and will update the code according to the ABI sysfs standards. > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static int max42500_read(struct device *dev, enum hwmon_sensor_types > type, > > + u32 attr, int channel, long > *value) { > > + switch (type) { > > + case hwmon_chip: > > + return max42500_read_chip(dev, attr, channel, value); > > + case hwmon_in: > > + return max42500_read_in(dev, attr, channel, value); > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static int max42500_write_in(struct device *dev, u32 attr, int channel, > > + long value) > > +{ > > + struct max42500_state *st = dev_get_drvdata(dev); > > + > > + switch (attr) { > > + case hwmon_in_input: > > + return max42500_set_nominal_voltage(st, channel, (u8)value); > > This is an ABI violation: inX_input attributes return voltages provided by the > chip, and are not writeable. Looking into the datasheet, this has to be set using > the min/max attributes. > Yes, I agree and will update the code to reflect your instructions and add the ABI documentation to address the violations. > > + case hwmon_in_min: > > + return max42500_set_uv_thresh1(st, channel, (u8)value); > > This is a severe ABI violation. First, again, "(u8)value" is unacceptable. > Second, the ABI states that this is a value in mV, not a percentage. > I don't know why the chip uses a concept of "nominal voltage" and percentages > of that for thresholds. I undersatand that this makes working with the chip > difficult, but that does not mean that ABI violations are acceptable. > > AFAICS the chip doesn't even report the current voltages, so any inX_input > attributes must not be provided. The limit attributes must set limits in mV. > Yes, I agree and have a discussion with the hardware application engineers. I will update the code to reflect the appropriate attributes that can be used for the "nominal voltage" and the percentages for the OV/UV thresholds. For the limits, can we use uV to address the decimal part of mV, e.g. 12.5mV? > > + case hwmon_in_max: > > + return max42500_set_uv_thresh2(st, channel, (u8)value); > > + case hwmon_in_lowest: > > + return max42500_set_ov_thresh1(st, channel, (u8)value); > > + case hwmon_in_highest: > > + return max42500_set_ov_thresh2(st, channel, (u8)value); > > This looks like another ABI violation. _lowest and _highest_ attributes report > historic values. > Yes, I agree and will update the code and add ABI documentation to address the violations. > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static int max42500_write_chip(struct device *dev, u32 attr, int channel, > > + long value) > > +{ > > + struct max42500_state *st = dev_get_drvdata(dev); > > + > > + switch (attr) { > > + case hwmon_chip_temp_reset_history: > > + return max42500_set_watchdog_rhld(st, (u8)value); > > + case hwmon_chip_register_tz: > > + return max42500_set_watchdog_key(st); > > + case hwmon_chip_alarms: > > + return max42500_set_watchdog_enable(st, (bool)value); > > + default: > > The above all seem to abouse the ABI and need documentation and explanation > to even be able to evaluate. > Yes, I will add an ABI documentation to explain the attributes. > On top of that, cutting off values such as by "(u8)value" is unacceptable; writing > 256 would have the same result as writing 0. > Yes, I agree and will update these using bit ops macros to handle the values. > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static int max42500_write(struct device *dev, enum hwmon_sensor_types > type, > > + u32 attr, int channel, long > value) { > > + switch (type) { > > + case hwmon_chip: > > + return max42500_write_chip(dev, attr, channel, value); > > + case hwmon_in: > > + return max42500_write_in(dev, attr, channel, value); > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static const struct max42500_config max42500_init = { > > + .pece = true, > > + .vmon_en = true, > > + .vmon_vmpd = true, > > + .reset_map = (MAX42500_RSTMAP_IN_MASK(MAX42500_VM2) | > > + > MAX42500_RSTMAP_IN_MASK(MAX42500_VM3) | > > + > MAX42500_RSTMAP_IN_MASK(MAX42500_VM4) | > > + MAX42500_RSTMAP_PARM_MASK), > > + .wd_mode = MAX42500_WD_MODE_SIMPLE, > > + .wd_cdiv = 0x0, > > + .wd_close = 0x0, > > + .wd_open = 0x0, > > + .wd_1ud = 0x0, > > + .wd_en = true, > > +}; > > + > > +static int max42500_init_client(struct max42500_state *st, > > + struct device *dev) > > +{ > > + int err; > > + u8 device_id; > > + > > + if (!st || !dev) > > + return -EINVAL; > > How would any of those ever be NULL ? Please drop such unnecessary checks. Yes, I will update the code to reflect these changes. > > + > > + crc8_populate_msb(max42500_crc8, CRC8_PEC); > > + > > + /* Check device silicon ID */ > > + err = max42500_reg_read(st, MAX42500_REG_ID, &device_id); > > + if (err) > > + return err; > > + > > + if (device_id != MAX42500_SILICON_ID) > > + return -ENODEV; > > + > > + /* Configure PEC */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_CONFIG1, > > + BIT(0), > > + max42500_init.pece); > > + if (err) > > + return err; > > + > > + /* Set PEC enable/disable for subsequent register access */ > > + st->config->pece = max42500_init.pece; > > + > > + /* Enable voltage monitor inputs */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_VMON, > > + GENMASK(6, 0), > > + max42500_init.vmon_en); > > + if (err) > > + return err; > > + > > + /* Enable voltage monitor power-down */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_VMON, > > + BIT(7), > > + max42500_init.vmon_vmpd); > > + if (err) > > + return err; > > + > > + /* Enable input OV/UV mapping to reset pin */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_RSTMAP, > > + GENMASK(6, 0), > > + max42500_init.reset_map); > > + if (err) > > + return err; > > + > > + /* Set watchdog mode */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_WDCDIV, > > + BIT(6), > > + max42500_init.wd_mode << 6); > > + if (err) > > + return err; > > + > > + /* Set watchdog clock div */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_WDCDIV, > > + GENMASK(5, 0), > > + max42500_init.wd_cdiv); > > + if (err) > > + return err; > > + > > + /* Set watchdog open window */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_WDCFG1, > > + GENMASK(3, 0), > > + max42500_init.wd_open); > > + if (err) > > + return err; > > + > > + /* Set watchdog close window */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_WDCFG1, > > + GENMASK(7, 4), > > + max42500_init.wd_close); > > + if (err) > > + return err; > > + > > + /* Set watchdog first update window */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_WDCFG2, > > + GENMASK(2, 0), > > + max42500_init.wd_1ud); > > + if (err) > > + return err; > > + > > + /* Set watchdog enable */ > > + err = max42500_reg_update(st, > > + MAX42500_REG_WDCFG2, > > + BIT(3), > > + max42500_init.wd_en); > > + if (err) > > + return err; > > + > > + /* Update parameters */ > > + st->config->vmon_en = max42500_init.vmon_en; > > + st->config->vmon_vmpd = max42500_init.vmon_vmpd; > > + st->config->reset_map = max42500_init.reset_map; > > + st->config->wd_mode = max42500_init.wd_mode; > > + st->config->wd_cdiv = max42500_init.wd_cdiv; > > + st->config->wd_open = max42500_init.wd_open; > > + st->config->wd_close = max42500_init.wd_close; > > + st->config->wd_1ud = max42500_init.wd_1ud; > > + st->config->wd_en = max42500_init.wd_en; > > Why not just setting a pointer to max42500_init ? > This is a relic from the bare metal driver code. I will remove this since the device itself already has default OTP initial values setting. > > + > > + return 0; > > +} > > + > > +static const struct hwmon_ops max42500_hwmon_ops = { > > + .is_visible = max42500_is_visible, > > + .read = max42500_read, > > + .write = max42500_write, > > +}; > > + > > +static const struct hwmon_channel_info *max42500_info[] = { > > + HWMON_CHANNEL_INFO(chip, > > + HWMON_C_IN_RESET_HISTORY | > HWMON_C_CURR_RESET_HISTORY | > > + HWMON_C_TEMP_RESET_HISTORY | > HWMON_C_POWER_RESET_HISTORY | > > + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL > | HWMON_C_ALARMS, > > + HWMON_C_IN_RESET_HISTORY | > HWMON_C_CURR_RESET_HISTORY, > > + HWMON_C_IN_RESET_HISTORY | > HWMON_C_CURR_RESET_HISTORY, > > + HWMON_C_IN_RESET_HISTORY | > HWMON_C_CURR_RESET_HISTORY, > > + HWMON_C_IN_RESET_HISTORY | > HWMON_C_CURR_RESET_HISTORY, > > + HWMON_C_IN_RESET_HISTORY | > HWMON_C_CURR_RESET_HISTORY, > > + HWMON_C_IN_RESET_HISTORY | > HWMON_C_CURR_RESET_HISTORY), > > + HWMON_CHANNEL_INFO(in, > > + HWMON_I_LABEL | HWMON_I_INPUT | > HWMON_I_LOWEST | HWMON_I_MIN | > > + HWMON_I_HIGHEST | HWMON_I_MAX, > > + HWMON_I_LABEL | HWMON_I_INPUT | > HWMON_I_LOWEST | HWMON_I_MIN | > > + HWMON_I_HIGHEST | HWMON_I_MAX, > > + HWMON_I_LABEL | HWMON_I_INPUT | > HWMON_I_LOWEST | HWMON_I_MIN, > > + HWMON_I_LABEL | HWMON_I_INPUT | > HWMON_I_LOWEST | HWMON_I_MIN, > > + HWMON_I_LABEL | HWMON_I_INPUT | > HWMON_I_LOWEST | HWMON_I_MIN, > > + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, > HWMON_I_LABEL, > > + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, > HWMON_I_LABEL, > > + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, > HWMON_I_LABEL, > > + HWMON_I_LABEL, HWMON_I_LABEL, HWMON_I_LABEL, > HWMON_I_LABEL), > > + NULL > > +}; > > + > > +static const struct hwmon_chip_info max42500_chip_info = { > > + .ops = &max42500_hwmon_ops, > > + .info = max42500_info, > > +}; > > + > > +static const struct regmap_config max42500_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .max_register = 0x2D > > +}; > > + > > +static int max42500_probe(struct i2c_client *client) { > > + struct device *hwmon_dev; > > + struct max42500_state *st; > > + int err; > > + > > + st = devm_kzalloc(&client->dev, sizeof(*st), GFP_KERNEL); > > + if (!st) > > + return -ENOMEM; > > + > > + st->client = client; > > + st->regmap = devm_regmap_init_i2c(client, &max42500_regmap_config); > > + if (IS_ERR(st->regmap)) > > + return PTR_ERR(st->regmap); > > + > > + err = max42500_init_client(st, &client->dev); > > + if (err) > > + return err; > > + > > + hwmon_dev = devm_hwmon_device_register_with_info(&client->dev, > > + client->name, st, &max42500_chip_info, > NULL); > > + > > + return PTR_ERR_OR_ZERO(hwmon_dev); > > +} > > + > > +static const struct of_device_id max42500_of_match[] = { > > + { .compatible = "adi,max42500" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, max42500_of_match); > > + > > +static const struct i2c_device_id max42500_id[] = { > > + { "max42500", 0 }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(i2c, max42500_id); > > + > > +static struct i2c_driver max42500_driver = { > > + .driver = { > > + .name = "max42500", > > + .of_match_table = max42500_of_match, > > + }, > > + .probe = max42500_probe, > > + .id_table = max42500_id, > > +}; > > + > > +module_i2c_driver(max42500_driver); > > + > > +MODULE_AUTHOR("Kent Libetario <kent.libetario@analog.com>"); > > +MODULE_DESCRIPTION("Hwmon driver for MAX42500"); > > +MODULE_LICENSE("GPL"); ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-03-10 9:31 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-20 1:20 [PATCH 0/2] Add driver for MAX42500 Kent Libetario 2024-12-20 1:20 ` [PATCH 1/2] dt-bindings: hwmon: add adi,max42500.yaml Kent Libetario 2024-12-27 8:50 ` Krzysztof Kozlowski 2025-03-10 9:26 ` Libetario, Kent 2024-12-20 1:20 ` [PATCH 2/2] drivers: hwmon: add driver for max42500 Kent Libetario 2024-12-21 5:01 ` kernel test robot 2024-12-21 18:08 ` Guenter Roeck 2025-03-10 9:05 ` Libetario, Kent
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox