From 629cf8f6c23a987201558ffcca5590a60ae3959d Mon Sep 17 00:00:00 2001 From: Alexis Czezar Torreno Date: Mon, 7 Apr 2025 11:47:25 +0800 Subject: [PATCH 01/16] hwmon: (pmbus/max34440) Add support for ADPM12160 ADPM12160 is a quarter brick DC/DC Power Module. It is a high power non-isolated converter capable of delivering a fully regulated 12V, with continuous power level of 1600W with peak power at 2400W for a limited time. Uses PMBus Configuration. Signed-off-by: Alexis Czezar Torreno Link: https://lore.kernel.org/r/20250407-dev_adpm12160-v3-2-9cd3095445c8@analog.com [groeck: The chip is "ADPM12160"] Signed-off-by: Guenter Roeck --- Documentation/hwmon/max34440.rst | 30 ++++++++++++----- drivers/hwmon/pmbus/max34440.c | 55 ++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 11 deletions(-) diff --git a/Documentation/hwmon/max34440.rst b/Documentation/hwmon/max34440.rst index 162d289f0814..8591a7152ce5 100644 --- a/Documentation/hwmon/max34440.rst +++ b/Documentation/hwmon/max34440.rst @@ -3,6 +3,14 @@ Kernel driver max34440 Supported chips: + * ADI ADPM12160 + + Prefixes: 'adpm12160' + + Addresses scanned: - + + Datasheet: - + * Maxim MAX34440 Prefixes: 'max34440' @@ -67,13 +75,14 @@ Author: Guenter Roeck Description ----------- -This driver supports hardware monitoring for Maxim MAX34440 PMBus 6-Channel -Power-Supply Manager, MAX34441 PMBus 5-Channel Power-Supply Manager -and Intelligent Fan Controller, and MAX34446 PMBus Power-Supply Data Logger. -It also supports the MAX34451, MAX34460, and MAX34461 PMBus Voltage Monitor & -Sequencers. The MAX34451 supports monitoring voltage or current of 12 channels -based on GIN pins. The MAX34460 supports 12 voltage channels, and the MAX34461 -supports 16 voltage channels. +This driver supports multiple devices: hardware monitoring for Maxim MAX34440 +PMBus 6-Channel Power-Supply Manager, MAX34441 PMBus 5-Channel Power-Supply +Manager and Intelligent Fan Controller, and MAX34446 PMBus Power-Supply Data +Logger; PMBus Voltage Monitor and Sequencers for MAX34451, MAX34460, and +MAX34461; PMBus DC/DC Power Module ADPM12160. The MAX34451 supports monitoring +voltage or current of 12 channels based on GIN pins. The MAX34460 supports 12 +voltage channels, and the MAX34461 supports 16 voltage channels. The ADPM1260 +also monitors both input and output of voltage and current. The driver is a client driver to the core PMBus driver. Please see Documentation/hwmon/pmbus.rst for details on PMBus client drivers. @@ -128,7 +137,10 @@ in[1-6]_highest Historical maximum voltage. in[1-6]_reset_history Write any value to reset history. ======================= ======================================================= -.. note:: MAX34446 only supports in[1-4]. +.. note:: + + - MAX34446 only supports in[1-4]. + - ADPM12160 only supports in[1-2]. Label is "vin1" and "vout1" respectively. Curr ~~~~ @@ -150,6 +162,7 @@ curr[1-6]_reset_history Write any value to reset history. - in6 and curr6 attributes only exist for MAX34440. - MAX34446 only supports curr[1-4]. + - For ADPM12160, curr[1] is "iin1" and curr[2-6] are "iout[1-5]. Power ~~~~~ @@ -185,6 +198,7 @@ temp[1-8]_reset_history Write any value to reset history. .. note:: - temp7 and temp8 attributes only exist for MAX34440. - MAX34446 only supports temp[1-3]. + - ADPM12160 only supports temp[1]. .. note:: diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c index d6d556b01385..3d70c454efb5 100644 --- a/drivers/hwmon/pmbus/max34440.c +++ b/drivers/hwmon/pmbus/max34440.c @@ -14,7 +14,15 @@ #include #include "pmbus.h" -enum chips { max34440, max34441, max34446, max34451, max34460, max34461 }; +enum chips { + adpm12160, + max34440, + max34441, + max34446, + max34451, + max34460, + max34461, +}; #define MAX34440_MFR_VOUT_PEAK 0xd4 #define MAX34440_MFR_IOUT_PEAK 0xd5 @@ -80,7 +88,8 @@ static int max34440_read_word_data(struct i2c_client *client, int page, MAX34440_MFR_VOUT_PEAK); break; case PMBUS_VIRT_READ_IOUT_AVG: - if (data->id != max34446 && data->id != max34451) + if (data->id != max34446 && data->id != max34451 && + data->id != adpm12160) return -ENXIO; ret = pmbus_read_word_data(client, page, phase, MAX34446_MFR_IOUT_AVG); @@ -164,7 +173,8 @@ static int max34440_write_word_data(struct i2c_client *client, int page, case PMBUS_VIRT_RESET_IOUT_HISTORY: ret = pmbus_write_word_data(client, page, MAX34440_MFR_IOUT_PEAK, 0); - if (!ret && (data->id == max34446 || data->id == max34451)) + if (!ret && (data->id == max34446 || data->id == max34451 || + data->id == adpm12160)) ret = pmbus_write_word_data(client, page, MAX34446_MFR_IOUT_AVG, 0); @@ -309,6 +319,41 @@ static int max34451_set_supported_funcs(struct i2c_client *client, } static struct pmbus_driver_info max34440_info[] = { + [adpm12160] = { + .pages = 19, + .format[PSC_VOLTAGE_IN] = direct, + .format[PSC_VOLTAGE_OUT] = direct, + .format[PSC_CURRENT_IN] = direct, + .format[PSC_CURRENT_OUT] = direct, + .format[PSC_TEMPERATURE] = direct, + .m[PSC_VOLTAGE_IN] = 1, + .b[PSC_VOLTAGE_IN] = 0, + .R[PSC_VOLTAGE_IN] = 0, + .m[PSC_VOLTAGE_OUT] = 1, + .b[PSC_VOLTAGE_OUT] = 0, + .R[PSC_VOLTAGE_OUT] = 0, + .m[PSC_CURRENT_IN] = 1, + .b[PSC_CURRENT_IN] = 0, + .R[PSC_CURRENT_IN] = 2, + .m[PSC_CURRENT_OUT] = 1, + .b[PSC_CURRENT_OUT] = 0, + .R[PSC_CURRENT_OUT] = 2, + .m[PSC_TEMPERATURE] = 1, + .b[PSC_TEMPERATURE] = 0, + .R[PSC_TEMPERATURE] = 2, + /* absent func below [18] are not for monitoring */ + .func[2] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT, + .func[4] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT, + .func[5] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT, + .func[6] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT, + .func[7] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT, + .func[8] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT, + .func[9] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT, + .func[10] = PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT, + .func[18] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP, + .read_word_data = max34440_read_word_data, + .write_word_data = max34440_write_word_data, + }, [max34440] = { .pages = 14, .format[PSC_VOLTAGE_IN] = direct, @@ -539,12 +584,16 @@ static int max34440_probe(struct i2c_client *client) rv = max34451_set_supported_funcs(client, data); if (rv) return rv; + } else if (data->id == adpm12160) { + data->iout_oc_fault_limit = PMBUS_IOUT_OC_FAULT_LIMIT; + data->iout_oc_warn_limit = PMBUS_IOUT_OC_WARN_LIMIT; } return pmbus_do_probe(client, &data->info); } static const struct i2c_device_id max34440_id[] = { + {"adpm12160", adpm12160}, {"max34440", max34440}, {"max34441", max34441}, {"max34446", max34446}, -- 2.50.1 From 0b3c04c81804197bf0025f3281e4463152f04bf1 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Sat, 22 Mar 2025 07:26:02 -0700 Subject: [PATCH 02/16] hwmon: (pmbus) Do not set regulators_node for single-channel chips Single-channel regulators do not need and should not have a "regulators" node. We can not entirely remove it due to existing bindings. To solve the problem for new drivers, provide additional macros PMBUS_REGULATOR_ONE_NODE and PMBUS_REGULATOR_STEP_ONE_NODE and convert existing drivers to use those macros. The exception is the ir38064 driver because its devicetree files and its description do not require or use the nested regulators node. Modify PMBUS_REGULATOR_STEP_ONE and PMBUS_REGULATOR_ONE to set the regulators_node pointer to NULL. Cc: Cedricjustine.Encarnacion@analog.com Signed-off-by: Guenter Roeck Reviewed-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20250322142602.560042-1-linux@roeck-us.net Signed-off-by: Guenter Roeck --- drivers/hwmon/pmbus/lm25066.c | 2 +- drivers/hwmon/pmbus/mpq7932.c | 4 ++-- drivers/hwmon/pmbus/pmbus.h | 18 +++++++++++++++--- drivers/hwmon/pmbus/tda38640.c | 2 +- drivers/hwmon/pmbus/tps25990.c | 2 +- 5 files changed, 20 insertions(+), 8 deletions(-) diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c index 40b0dda32ea6..dd7275a67a0a 100644 --- a/drivers/hwmon/pmbus/lm25066.c +++ b/drivers/hwmon/pmbus/lm25066.c @@ -437,7 +437,7 @@ static int lm25066_write_word_data(struct i2c_client *client, int page, int reg, #if IS_ENABLED(CONFIG_SENSORS_LM25066_REGULATOR) static const struct regulator_desc lm25066_reg_desc[] = { - PMBUS_REGULATOR_ONE("vout"), + PMBUS_REGULATOR_ONE_NODE("vout"), }; #endif diff --git a/drivers/hwmon/pmbus/mpq7932.c b/drivers/hwmon/pmbus/mpq7932.c index c1e2d0cb2fd0..8f10e37a7a76 100644 --- a/drivers/hwmon/pmbus/mpq7932.c +++ b/drivers/hwmon/pmbus/mpq7932.c @@ -51,8 +51,8 @@ static const struct regulator_desc mpq7932_regulators_desc[] = { }; static const struct regulator_desc mpq7932_regulators_desc_one[] = { - PMBUS_REGULATOR_STEP_ONE("buck", MPQ7932_N_VOLTAGES, - MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN), + PMBUS_REGULATOR_STEP_ONE_NODE("buck", MPQ7932_N_VOLTAGES, + MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN), }; #endif diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h index 742dafc44390..d2e9bfb5320f 100644 --- a/drivers/hwmon/pmbus/pmbus.h +++ b/drivers/hwmon/pmbus/pmbus.h @@ -509,11 +509,11 @@ int pmbus_regulator_init_cb(struct regulator_dev *rdev, #define PMBUS_REGULATOR(_name, _id) PMBUS_REGULATOR_STEP(_name, _id, 0, 0, 0) -#define PMBUS_REGULATOR_STEP_ONE(_name, _voltages, _step, _min_uV) \ +#define __PMBUS_REGULATOR_STEP_ONE(_name, _node, _voltages, _step, _min_uV) \ { \ .name = (_name), \ .of_match = of_match_ptr(_name), \ - .regulators_node = of_match_ptr("regulators"), \ + .regulators_node = of_match_ptr(_node), \ .ops = &pmbus_regulator_ops, \ .type = REGULATOR_VOLTAGE, \ .owner = THIS_MODULE, \ @@ -523,7 +523,19 @@ int pmbus_regulator_init_cb(struct regulator_dev *rdev, .init_cb = pmbus_regulator_init_cb, \ } -#define PMBUS_REGULATOR_ONE(_name) PMBUS_REGULATOR_STEP_ONE(_name, 0, 0, 0) +/* + * _NODE macros are defined for historic reasons and MUST NOT be used in new + * drivers. + */ +#define PMBUS_REGULATOR_STEP_ONE_NODE(_name, _voltages, _step, _min_uV) \ + __PMBUS_REGULATOR_STEP_ONE(_name, "regulators", _voltages, _step, _min_uV) + +#define PMBUS_REGULATOR_ONE_NODE(_name) PMBUS_REGULATOR_STEP_ONE_NODE(_name, 0, 0, 0) + +#define PMBUS_REGULATOR_STEP_ONE(_name, _voltages, _step, _min_uV) \ + __PMBUS_REGULATOR_STEP_ONE(_name, NULL, _voltages, _step, _min_uV) + +#define PMBUS_REGULATOR_ONE(_name) PMBUS_REGULATOR_STEP_ONE(_name, 0, 0, 0) /* Function declarations */ diff --git a/drivers/hwmon/pmbus/tda38640.c b/drivers/hwmon/pmbus/tda38640.c index 07fe58c24485..d902d39f49f4 100644 --- a/drivers/hwmon/pmbus/tda38640.c +++ b/drivers/hwmon/pmbus/tda38640.c @@ -15,7 +15,7 @@ #include "pmbus.h" static const struct regulator_desc __maybe_unused tda38640_reg_desc[] = { - PMBUS_REGULATOR_ONE("vout"), + PMBUS_REGULATOR_ONE_NODE("vout"), }; struct tda38640_data { diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c index 0d2655e69549..c13edd7e1abf 100644 --- a/drivers/hwmon/pmbus/tps25990.c +++ b/drivers/hwmon/pmbus/tps25990.c @@ -333,7 +333,7 @@ static int tps25990_write_byte_data(struct i2c_client *client, #if IS_ENABLED(CONFIG_SENSORS_TPS25990_REGULATOR) static const struct regulator_desc tps25990_reg_desc[] = { - PMBUS_REGULATOR_ONE("vout"), + PMBUS_REGULATOR_ONE_NODE("vout"), }; #endif -- 2.50.1 From 6de6868df18728790eb4ffe764b49f356fea7397 Mon Sep 17 00:00:00 2001 From: Naresh Solanki Date: Fri, 4 Apr 2025 17:26:45 +0530 Subject: [PATCH 03/16] hwmon: (max6639) Allow setting target RPM Currently, during startup, the fan is set to its maximum RPM by default, which may not be suitable for all use cases. This patch introduces support for specifying a target RPM via the Device Tree property "target-rpm". Changes: - Added `target_rpm` field to `max6639_data` structure to store the target RPM for each fan channel. - Modified `max6639_probe_child_from_dt()` to read the `"target-rpm"` property from the Device Tree and set `target_rpm` accordingly. - Updated `max6639_init_client()` to use `target_rpm` to compute the initial PWM duty cycle instead of defaulting to full speed (120/120). Behavior: - If `"target-rpm"` is specified, the fan speed is set accordingly. - If `"target-rpm"` is not specified, the previous behavior (full speed at startup) is retained. This allows better control over fan speed during system initialization. Signed-off-by: Naresh Solanki Link: https://lore.kernel.org/r/20250404115646.2000563-1-you@example.com Signed-off-by: Guenter Roeck --- drivers/hwmon/max6639.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c index 32b4d54b2076..a06346496e1d 100644 --- a/drivers/hwmon/max6639.c +++ b/drivers/hwmon/max6639.c @@ -80,6 +80,7 @@ struct max6639_data { /* Register values initialized only once */ u8 ppr[MAX6639_NUM_CHANNELS]; /* Pulses per rotation 0..3 for 1..4 ppr */ u8 rpm_range[MAX6639_NUM_CHANNELS]; /* Index in above rpm_ranges table */ + u32 target_rpm[MAX6639_NUM_CHANNELS]; /* Optional regulator for FAN supply */ struct regulator *reg; @@ -563,6 +564,10 @@ static int max6639_probe_child_from_dt(struct i2c_client *client, if (!err) data->rpm_range[i] = rpm_range_to_reg(val); + err = of_property_read_u32(child, "target-rpm", &val); + if (!err) + data->target_rpm[i] = val; + return 0; } @@ -573,6 +578,7 @@ static int max6639_init_client(struct i2c_client *client, const struct device_node *np = dev->of_node; struct device_node *child; int i, err; + u8 target_duty; /* Reset chip to default values, see below for GCONFIG setup */ err = regmap_write(data->regmap, MAX6639_REG_GCONFIG, MAX6639_GCONFIG_POR); @@ -586,6 +592,8 @@ static int max6639_init_client(struct i2c_client *client, /* default: 4000 RPM */ data->rpm_range[0] = 1; data->rpm_range[1] = 1; + data->target_rpm[0] = 4000; + data->target_rpm[1] = 4000; for_each_child_of_node(np, child) { if (strcmp(child->name, "fan")) @@ -639,8 +647,12 @@ static int max6639_init_client(struct i2c_client *client, if (err) return err; - /* PWM 120/120 (i.e. 100%) */ - err = regmap_write(data->regmap, MAX6639_REG_TARGTDUTY(i), 120); + /* Set PWM based on target RPM if specified */ + if (data->target_rpm[i] > rpm_ranges[data->rpm_range[i]]) + data->target_rpm[i] = rpm_ranges[data->rpm_range[i]]; + + target_duty = 120 * data->target_rpm[i] / rpm_ranges[data->rpm_range[i]]; + err = regmap_write(data->regmap, MAX6639_REG_TARGTDUTY(i), target_duty); if (err) return err; } -- 2.50.1 From ab2f6bffe7311327d5f6432e413c704f395b93c3 Mon Sep 17 00:00:00 2001 From: "William A. Kennington III" Date: Mon, 7 Apr 2025 18:10:06 -0700 Subject: [PATCH 04/16] hwmon: (max34451) Work around lost page When requesting new pages from the max34451 we sometimes see that the firmware responds with stale or bad data to reads that happen immediately after a page change. This is due to a lack of clock stretching after page changing on the device side when it needs more time to complete the operation. To remedy this, the manufacturer recommends we wait 50us until the firmware should be ready with the new page. Signed-off-by: William A. Kennington III Link: https://lore.kernel.org/r/20250408011006.1314622-1-william@wkennington.com Signed-off-by: Guenter Roeck --- drivers/hwmon/pmbus/max34440.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c index 3d70c454efb5..56834d26f8ef 100644 --- a/drivers/hwmon/pmbus/max34440.c +++ b/drivers/hwmon/pmbus/max34440.c @@ -12,6 +12,7 @@ #include #include #include +#include #include "pmbus.h" enum chips { @@ -24,6 +25,14 @@ enum chips { max34461, }; +/* + * Firmware is sometimes not ready if we try and read the + * data from the page immediately after setting. Maxim + * recommends 50us delay due to the chip failing to clock + * stretch long enough here. + */ +#define MAX34440_PAGE_CHANGE_DELAY 50 + #define MAX34440_MFR_VOUT_PEAK 0xd4 #define MAX34440_MFR_IOUT_PEAK 0xd5 #define MAX34440_MFR_TEMPERATURE_PEAK 0xd6 @@ -272,6 +281,7 @@ static int max34451_set_supported_funcs(struct i2c_client *client, for (page = 0; page < 16; page++) { rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page); + fsleep(MAX34440_PAGE_CHANGE_DELAY); if (rv < 0) return rv; @@ -395,6 +405,7 @@ static struct pmbus_driver_info max34440_info[] = { .read_byte_data = max34440_read_byte_data, .read_word_data = max34440_read_word_data, .write_word_data = max34440_write_word_data, + .page_change_delay = MAX34440_PAGE_CHANGE_DELAY, }, [max34441] = { .pages = 12, @@ -438,6 +449,7 @@ static struct pmbus_driver_info max34440_info[] = { .read_byte_data = max34440_read_byte_data, .read_word_data = max34440_read_word_data, .write_word_data = max34440_write_word_data, + .page_change_delay = MAX34440_PAGE_CHANGE_DELAY, }, [max34446] = { .pages = 7, @@ -475,6 +487,7 @@ static struct pmbus_driver_info max34440_info[] = { .read_byte_data = max34440_read_byte_data, .read_word_data = max34440_read_word_data, .write_word_data = max34440_write_word_data, + .page_change_delay = MAX34440_PAGE_CHANGE_DELAY, }, [max34451] = { .pages = 21, @@ -498,6 +511,7 @@ static struct pmbus_driver_info max34440_info[] = { .func[20] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP, .read_word_data = max34440_read_word_data, .write_word_data = max34440_write_word_data, + .page_change_delay = MAX34440_PAGE_CHANGE_DELAY, }, [max34460] = { .pages = 18, @@ -528,6 +542,7 @@ static struct pmbus_driver_info max34440_info[] = { .func[17] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP, .read_word_data = max34440_read_word_data, .write_word_data = max34440_write_word_data, + .page_change_delay = MAX34440_PAGE_CHANGE_DELAY, }, [max34461] = { .pages = 23, @@ -563,6 +578,7 @@ static struct pmbus_driver_info max34440_info[] = { .func[21] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP, .read_word_data = max34440_read_word_data, .write_word_data = max34440_write_word_data, + .page_change_delay = MAX34440_PAGE_CHANGE_DELAY, }, }; -- 2.50.1 From 0bf08f9e358d33a8972cdb3d698079f5d768d7ed Mon Sep 17 00:00:00 2001 From: Eugene Shalygin Date: Tue, 8 Apr 2025 22:44:57 +0200 Subject: [PATCH 05/16] hwmon: (asus-ec-sensors) sort sensor definition arrays The arrays have to be sorted by the sensor register bank and index because this is what the sensor reading function expects. So sort them and leave a comment for future contributors. Signed-off-by: Eugene Shalygin Link: https://lore.kernel.org/r/20250408204505.11412-1-eugene.shalygin@gmail.com [groeck: Fixed alignment of new multi-line comment] Signed-off-by: Guenter Roeck --- drivers/hwmon/asus-ec-sensors.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c index 006ced5ab6e6..7f2389db8923 100644 --- a/drivers/hwmon/asus-ec-sensors.c +++ b/drivers/hwmon/asus-ec-sensors.c @@ -169,7 +169,11 @@ enum board_family { family_intel_600_series }; -/* All the known sensors for ASUS EC controllers */ +/* + * All the known sensors for ASUS EC controllers. These arrays have to be sorted + * by the full ((bank << 8) + index) register index (see asus_ec_block_read() as + * to why). + */ static const struct ec_sensor_info sensors_family_amd_400[] = { [ec_sensor_temp_chipset] = EC_SENSOR("Chipset", hwmon_temp, 1, 0x00, 0x3a), @@ -183,10 +187,10 @@ static const struct ec_sensor_info sensors_family_amd_400[] = { EC_SENSOR("VRM", hwmon_temp, 1, 0x00, 0x3e), [ec_sensor_in_cpu_core] = EC_SENSOR("CPU Core", hwmon_in, 2, 0x00, 0xa2), - [ec_sensor_fan_cpu_opt] = - EC_SENSOR("CPU_Opt", hwmon_fan, 2, 0x00, 0xbc), [ec_sensor_fan_vrm_hs] = EC_SENSOR("VRM HS", hwmon_fan, 2, 0x00, 0xb2), + [ec_sensor_fan_cpu_opt] = + EC_SENSOR("CPU_Opt", hwmon_fan, 2, 0x00, 0xbc), [ec_sensor_fan_chipset] = /* no chipset fans in this generation */ EC_SENSOR("Chipset", hwmon_fan, 0, 0x00, 0x00), @@ -194,10 +198,10 @@ static const struct ec_sensor_info sensors_family_amd_400[] = { EC_SENSOR("Water_Flow", hwmon_fan, 2, 0x00, 0xb4), [ec_sensor_curr_cpu] = EC_SENSOR("CPU", hwmon_curr, 1, 0x00, 0xf4), - [ec_sensor_temp_water_in] = - EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x0d), [ec_sensor_temp_water_out] = EC_SENSOR("Water_Out", hwmon_temp, 1, 0x01, 0x0b), + [ec_sensor_temp_water_in] = + EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x0d), }; static const struct ec_sensor_info sensors_family_amd_500[] = { @@ -239,19 +243,20 @@ static const struct ec_sensor_info sensors_family_amd_500[] = { static const struct ec_sensor_info sensors_family_amd_600[] = { [ec_sensor_temp_cpu] = EC_SENSOR("CPU", hwmon_temp, 1, 0x00, 0x30), - [ec_sensor_temp_cpu_package] = EC_SENSOR("CPU Package", hwmon_temp, 1, 0x00, 0x31), + [ec_sensor_temp_cpu_package] = + EC_SENSOR("CPU Package", hwmon_temp, 1, 0x00, 0x31), [ec_sensor_temp_mb] = EC_SENSOR("Motherboard", hwmon_temp, 1, 0x00, 0x32), [ec_sensor_temp_vrm] = EC_SENSOR("VRM", hwmon_temp, 1, 0x00, 0x33), [ec_sensor_temp_t_sensor] = EC_SENSOR("T_Sensor", hwmon_temp, 1, 0x00, 0x36), + [ec_sensor_fan_cpu_opt] = + EC_SENSOR("CPU_Opt", hwmon_fan, 2, 0x00, 0xb0), [ec_sensor_temp_water_in] = EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x00), [ec_sensor_temp_water_out] = EC_SENSOR("Water_Out", hwmon_temp, 1, 0x01, 0x01), - [ec_sensor_fan_cpu_opt] = - EC_SENSOR("CPU_Opt", hwmon_fan, 2, 0x00, 0xb0), }; static const struct ec_sensor_info sensors_family_intel_300[] = { -- 2.50.1 From 9b116ba6c9eb9d3b33de0363fafdc5941116f029 Mon Sep 17 00:00:00 2001 From: Ciprian Marian Costea Date: Wed, 9 Apr 2025 10:45:29 +0300 Subject: [PATCH 06/16] hwmon: (ina2xx) make regulator 'vs' support optional According to the 'ti,ina2xx' binding, the 'vs-supply' property is optional. Use devm_regulator_get_enable_optional() to avoid a kernel warning message if the property is not provided. Co-developed-by: Florin Buica Tested-by: Enric Balletbo i Serra Signed-off-by: Florin Buica Signed-off-by: Ciprian Marian Costea Link: https://lore.kernel.org/r/20250409074529.2233733-1-ciprianmarian.costea@oss.nxp.com [groeck: Use standard multi-line comment] Signed-off-by: Guenter Roeck --- drivers/hwmon/ina2xx.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c index 345fe7db9de9..bc3c1f7314b3 100644 --- a/drivers/hwmon/ina2xx.c +++ b/drivers/hwmon/ina2xx.c @@ -959,8 +959,12 @@ static int ina2xx_probe(struct i2c_client *client) return PTR_ERR(data->regmap); } - ret = devm_regulator_get_enable(dev, "vs"); - if (ret) + /* + * Regulator core returns -ENODEV if the 'vs' is not available. + * Hence the check for -ENODEV return code is necessary. + */ + ret = devm_regulator_get_enable_optional(dev, "vs"); + if (ret < 0 && ret != -ENODEV) return dev_err_probe(dev, ret, "failed to enable vs regulator\n"); ret = ina2xx_init(dev, data); -- 2.50.1 From 0d01110e6356e95320091f36e3d7ce92fa597d1f Mon Sep 17 00:00:00 2001 From: Alexander Stein Date: Wed, 9 Apr 2025 08:54:26 +0200 Subject: [PATCH 07/16] hwmon: (gpio-fan) Add regulator support FANs might be supplied by a regulator which needs to be enabled as well. This is implemented using runtime PM. Every time speed_index changes from 0 to non-zero and vise versa RPM is resumed or suspended. Intitial RPM state is determined by initial value of speed_index. Signed-off-by: Alexander Stein Link: https://lore.kernel.org/r/20250409065430.1413439-1-alexander.stein@ew.tq-group.com Signed-off-by: Guenter Roeck --- drivers/hwmon/gpio-fan.c | 103 +++++++++++++++++++++++++++++++++------ 1 file changed, 87 insertions(+), 16 deletions(-) diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c index b779240328d5..516c34bb61c9 100644 --- a/drivers/hwmon/gpio-fan.c +++ b/drivers/hwmon/gpio-fan.c @@ -20,6 +20,9 @@ #include #include #include +#include +#include +#include #include struct gpio_fan_speed { @@ -42,6 +45,7 @@ struct gpio_fan_data { bool pwm_enable; struct gpio_desc *alarm_gpio; struct work_struct alarm_work; + struct regulator *supply; }; /* @@ -125,13 +129,32 @@ static int __get_fan_ctrl(struct gpio_fan_data *fan_data) } /* Must be called with fan_data->lock held, except during initialization. */ -static void set_fan_speed(struct gpio_fan_data *fan_data, int speed_index) +static int set_fan_speed(struct gpio_fan_data *fan_data, int speed_index) { if (fan_data->speed_index == speed_index) - return; + return 0; + + if (fan_data->speed_index == 0 && speed_index > 0) { + int ret; + + ret = pm_runtime_resume_and_get(fan_data->dev); + if (ret < 0) + return ret; + } __set_fan_ctrl(fan_data, fan_data->speed[speed_index].ctrl_val); + + if (fan_data->speed_index > 0 && speed_index == 0) { + int ret; + + ret = pm_runtime_put_sync(fan_data->dev); + if (ret < 0) + return ret; + } + fan_data->speed_index = speed_index; + + return 0; } static int get_fan_speed_index(struct gpio_fan_data *fan_data) @@ -176,7 +199,7 @@ static ssize_t pwm1_store(struct device *dev, struct device_attribute *attr, struct gpio_fan_data *fan_data = dev_get_drvdata(dev); unsigned long pwm; int speed_index; - int ret = count; + int ret; if (kstrtoul(buf, 10, &pwm) || pwm > 255) return -EINVAL; @@ -189,12 +212,12 @@ static ssize_t pwm1_store(struct device *dev, struct device_attribute *attr, } speed_index = DIV_ROUND_UP(pwm * (fan_data->num_speed - 1), 255); - set_fan_speed(fan_data, speed_index); + ret = set_fan_speed(fan_data, speed_index); exit_unlock: mutex_unlock(&fan_data->lock); - return ret; + return ret ? ret : count; } static ssize_t pwm1_enable_show(struct device *dev, @@ -211,6 +234,7 @@ static ssize_t pwm1_enable_store(struct device *dev, { struct gpio_fan_data *fan_data = dev_get_drvdata(dev); unsigned long val; + int ret = 0; if (kstrtoul(buf, 10, &val) || val > 1) return -EINVAL; @@ -224,11 +248,11 @@ static ssize_t pwm1_enable_store(struct device *dev, /* Disable manual control mode: set fan at full speed. */ if (val == 0) - set_fan_speed(fan_data, fan_data->num_speed - 1); + ret = set_fan_speed(fan_data, fan_data->num_speed - 1); mutex_unlock(&fan_data->lock); - return count; + return ret ? ret : count; } static ssize_t pwm1_mode_show(struct device *dev, @@ -279,7 +303,7 @@ static ssize_t set_rpm(struct device *dev, struct device_attribute *attr, goto exit_unlock; } - set_fan_speed(fan_data, rpm_to_speed_index(fan_data, rpm)); + ret = set_fan_speed(fan_data, rpm_to_speed_index(fan_data, rpm)); exit_unlock: mutex_unlock(&fan_data->lock); @@ -386,6 +410,7 @@ static int gpio_fan_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state) { struct gpio_fan_data *fan_data = cdev->devdata; + int ret; if (!fan_data) return -EINVAL; @@ -395,11 +420,11 @@ static int gpio_fan_set_cur_state(struct thermal_cooling_device *cdev, mutex_lock(&fan_data->lock); - set_fan_speed(fan_data, state); + ret = set_fan_speed(fan_data, state); mutex_unlock(&fan_data->lock); - return 0; + return ret; } static const struct thermal_cooling_device_ops gpio_fan_cool_ops = { @@ -499,6 +524,8 @@ static void gpio_fan_stop(void *data) mutex_lock(&fan_data->lock); set_fan_speed(data, 0); mutex_unlock(&fan_data->lock); + + pm_runtime_disable(fan_data->dev); } static int gpio_fan_probe(struct platform_device *pdev) @@ -521,6 +548,11 @@ static int gpio_fan_probe(struct platform_device *pdev) platform_set_drvdata(pdev, fan_data); mutex_init(&fan_data->lock); + fan_data->supply = devm_regulator_get(dev, "fan"); + if (IS_ERR(fan_data->supply)) + return dev_err_probe(dev, PTR_ERR(fan_data->supply), + "Failed to get fan-supply"); + /* Configure control GPIOs if available. */ if (fan_data->gpios && fan_data->num_gpios > 0) { if (!fan_data->speed || fan_data->num_speed <= 1) @@ -548,6 +580,17 @@ static int gpio_fan_probe(struct platform_device *pdev) return err; } + pm_runtime_set_suspended(&pdev->dev); + pm_runtime_enable(&pdev->dev); + /* If current GPIO state is active, mark RPM as active as well */ + if (fan_data->speed_index > 0) { + int ret; + + ret = pm_runtime_resume_and_get(&pdev->dev); + if (ret) + return ret; + } + /* Optional cooling device register for Device tree platforms */ fan_data->cdev = devm_thermal_of_cooling_device_register(dev, np, "gpio-fan", fan_data, &gpio_fan_cool_ops); @@ -565,41 +608,69 @@ static void gpio_fan_shutdown(struct platform_device *pdev) set_fan_speed(fan_data, 0); } +static int gpio_fan_runtime_suspend(struct device *dev) +{ + struct gpio_fan_data *fan_data = dev_get_drvdata(dev); + int ret = 0; + + if (fan_data->supply) + ret = regulator_disable(fan_data->supply); + + return ret; +} + +static int gpio_fan_runtime_resume(struct device *dev) +{ + struct gpio_fan_data *fan_data = dev_get_drvdata(dev); + int ret = 0; + + if (fan_data->supply) + ret = regulator_enable(fan_data->supply); + + return ret; +} + static int gpio_fan_suspend(struct device *dev) { struct gpio_fan_data *fan_data = dev_get_drvdata(dev); + int ret = 0; if (fan_data->gpios) { fan_data->resume_speed = fan_data->speed_index; mutex_lock(&fan_data->lock); - set_fan_speed(fan_data, 0); + ret = set_fan_speed(fan_data, 0); mutex_unlock(&fan_data->lock); } - return 0; + return ret; } static int gpio_fan_resume(struct device *dev) { struct gpio_fan_data *fan_data = dev_get_drvdata(dev); + int ret = 0; if (fan_data->gpios) { mutex_lock(&fan_data->lock); - set_fan_speed(fan_data, fan_data->resume_speed); + ret = set_fan_speed(fan_data, fan_data->resume_speed); mutex_unlock(&fan_data->lock); } - return 0; + return ret; } -static DEFINE_SIMPLE_DEV_PM_OPS(gpio_fan_pm, gpio_fan_suspend, gpio_fan_resume); +static const struct dev_pm_ops gpio_fan_pm = { + RUNTIME_PM_OPS(gpio_fan_runtime_suspend, + gpio_fan_runtime_resume, NULL) + SYSTEM_SLEEP_PM_OPS(gpio_fan_suspend, gpio_fan_resume) +}; static struct platform_driver gpio_fan_driver = { .probe = gpio_fan_probe, .shutdown = gpio_fan_shutdown, .driver = { .name = "gpio-fan", - .pm = pm_sleep_ptr(&gpio_fan_pm), + .pm = pm_ptr(&gpio_fan_pm), .of_match_table = of_gpio_fan_match, }, }; -- 2.50.1 From 7e581c193bde7d5ac49587d9a182e5d13e05547c Mon Sep 17 00:00:00 2001 From: Gerhard Engleder Date: Wed, 9 Apr 2025 21:08:30 +0200 Subject: [PATCH 08/16] hwmon: Add KEBA battery monitoring controller support The KEBA battery monitoring controller is found in the system FPGA of KEBA PLC devices. It puts a load on the coin cell battery to check the state of the battery. If the coin cell battery is nearly empty, then the user space is signaled with a hwmon alarm. The auxiliary device for this driver is instantiated by the cp500 misc driver. Signed-off-by: Gerhard Engleder Link: https://lore.kernel.org/r/20250409190830.60489-1-gerhard@engleder-embedded.com Signed-off-by: Guenter Roeck --- Documentation/hwmon/index.rst | 1 + Documentation/hwmon/kbatt.rst | 60 ++++++++++++++ drivers/hwmon/Kconfig | 10 +++ drivers/hwmon/Makefile | 1 + drivers/hwmon/kbatt.c | 147 ++++++++++++++++++++++++++++++++++ 5 files changed, 219 insertions(+) create mode 100644 Documentation/hwmon/kbatt.rst create mode 100644 drivers/hwmon/kbatt.c diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index f0ddf6222c44..da5895115724 100644 --- a/Documentation/hwmon/index.rst +++ b/Documentation/hwmon/index.rst @@ -106,6 +106,7 @@ Hardware Monitoring Kernel Drivers jc42 k10temp k8temp + kbatt lan966x lineage-pem lm25066 diff --git a/Documentation/hwmon/kbatt.rst b/Documentation/hwmon/kbatt.rst new file mode 100644 index 000000000000..b72718c5ede3 --- /dev/null +++ b/Documentation/hwmon/kbatt.rst @@ -0,0 +1,60 @@ +.. SPDX-License-Identifier: GPL-2.0 + +Kernel driver kbatt +=================== + +Supported chips: + + * KEBA battery monitoring controller (IP core in FPGA) + + Prefix: 'kbatt' + +Authors: + + Gerhard Engleder + Petar Bojanic + +Description +----------- + +The KEBA battery monitoring controller is an IP core for FPGAs, which +monitors the health of a coin cell battery. The coin cell battery is +typically used to supply the RTC during power off to keep the current +time. E.g., the CP500 FPGA includes this IP core to monitor the coin cell +battery of PLCs and the corresponding cp500 driver creates an auxiliary +device for the kbatt driver. + +This driver provides information about the coin cell battery health to +user space. Actually the user space shall be informed that the coin cell +battery is nearly empty and needs to be replaced. + +The coin cell battery must be tested actively to get to know if its nearly +empty or not. Therefore, a load is put on the coin cell battery and the +resulting voltage is evaluated. This evaluation is done by some hard wired +analog logic, which compares the voltage to a defined limit. If the +voltage is above the limit, then the coin cell battery is assumed to be +ok. If the voltage is below the limit, then the coin cell battery is +nearly empty (or broken, removed, ...) and shall be replaced by a new one. +The KEBA battery monitoring controller allows to start the test of the +coin cell battery and to get the result if the voltage is above or below +the limit. The actual voltage is not available. Only the information if +the voltage is below a limit is available. + +The test load, which is put on the coin cell battery for the health check, +is similar to the load during power off. Therefore, the lifetime of the +coin cell battery is reduced directly by the duration of each test. To +limit the negative impact to the lifetime the test is limited to at most +once every 10 seconds. The test load is put on the coin cell battery for +100ms. Thus, in worst case the coin cell battery lifetime is reduced by +1% of the uptime or 3.65 days per year. As the coin cell battery lasts +multiple years, this lifetime reduction negligible. + +This driver only provides a single alarm attribute, which is raised when +the coin cell battery is nearly empty. + +====================== ==== =================================================== +Attribute R/W Contents +====================== ==== =================================================== +in0_min_alarm R voltage of coin cell battery under load is below + limit +====================== ==== =================================================== diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index f91f713b0105..832d7e5f9f7b 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -335,6 +335,16 @@ config SENSORS_K10TEMP This driver can also be built as a module. If so, the module will be called k10temp. +config SENSORS_KBATT + tristate "KEBA battery controller support" + depends on KEBA_CP500 + help + This driver supports the battery monitoring controller found in + KEBA system FPGA devices. + + This driver can also be built as a module. If so, the module + will be called kbatt. + config SENSORS_FAM15H_POWER tristate "AMD Family 15h processor power" depends on X86 && PCI && CPU_SUP_AMD diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 766c652ef22b..af18deb0422e 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -110,6 +110,7 @@ obj-$(CONFIG_SENSORS_IT87) += it87.o obj-$(CONFIG_SENSORS_JC42) += jc42.o obj-$(CONFIG_SENSORS_K8TEMP) += k8temp.o obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.o +obj-$(CONFIG_SENSORS_KBATT) += kbatt.o obj-$(CONFIG_SENSORS_LAN966X) += lan966x-hwmon.o obj-$(CONFIG_SENSORS_LENOVO_EC) += lenovo-ec-sensors.o obj-$(CONFIG_SENSORS_LINEAGE) += lineage-pem.o diff --git a/drivers/hwmon/kbatt.c b/drivers/hwmon/kbatt.c new file mode 100644 index 000000000000..501b8f4ded33 --- /dev/null +++ b/drivers/hwmon/kbatt.c @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2025 KEBA Industrial Automation GmbH + * + * Driver for KEBA battery monitoring controller FPGA IP core + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define KBATT "kbatt" + +#define KBATT_CONTROL_REG 0x4 +#define KBATT_CONTROL_BAT_TEST 0x01 + +#define KBATT_STATUS_REG 0x8 +#define KBATT_STATUS_BAT_OK 0x01 + +#define KBATT_MAX_UPD_INTERVAL (10 * HZ) +#define KBATT_SETTLE_TIME_US (100 * USEC_PER_MSEC) + +struct kbatt { + /* update lock */ + struct mutex lock; + void __iomem *base; + + unsigned long next_update; /* in jiffies */ + bool alarm; +}; + +static bool kbatt_alarm(struct kbatt *kbatt) +{ + mutex_lock(&kbatt->lock); + + if (!kbatt->next_update || time_after(jiffies, kbatt->next_update)) { + /* switch load on */ + iowrite8(KBATT_CONTROL_BAT_TEST, + kbatt->base + KBATT_CONTROL_REG); + + /* wait some time to let things settle */ + fsleep(KBATT_SETTLE_TIME_US); + + /* check battery state */ + if (ioread8(kbatt->base + KBATT_STATUS_REG) & + KBATT_STATUS_BAT_OK) + kbatt->alarm = false; + else + kbatt->alarm = true; + + /* switch load off */ + iowrite8(0, kbatt->base + KBATT_CONTROL_REG); + + kbatt->next_update = jiffies + KBATT_MAX_UPD_INTERVAL; + } + + mutex_unlock(&kbatt->lock); + + return kbatt->alarm; +} + +static int kbatt_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct kbatt *kbatt = dev_get_drvdata(dev); + + *val = kbatt_alarm(kbatt) ? 1 : 0; + + return 0; +} + +static umode_t kbatt_is_visible(const void *data, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + if (channel == 0 && attr == hwmon_in_min_alarm) + return 0444; + + return 0; +} + +static const struct hwmon_channel_info *kbatt_info[] = { + HWMON_CHANNEL_INFO(in, + /* 0: input minimum alarm channel */ + HWMON_I_MIN_ALARM), + NULL +}; + +static const struct hwmon_ops kbatt_hwmon_ops = { + .is_visible = kbatt_is_visible, + .read = kbatt_read, +}; + +static const struct hwmon_chip_info kbatt_chip_info = { + .ops = &kbatt_hwmon_ops, + .info = kbatt_info, +}; + +static int kbatt_probe(struct auxiliary_device *auxdev, + const struct auxiliary_device_id *id) +{ + struct keba_batt_auxdev *kbatt_auxdev = + container_of(auxdev, struct keba_batt_auxdev, auxdev); + struct device *dev = &auxdev->dev; + struct device *hwmon_dev; + struct kbatt *kbatt; + int retval; + + kbatt = devm_kzalloc(dev, sizeof(*kbatt), GFP_KERNEL); + if (!kbatt) + return -ENOMEM; + + retval = devm_mutex_init(dev, &kbatt->lock); + if (retval) + return retval; + + kbatt->base = devm_ioremap_resource(dev, &kbatt_auxdev->io); + if (IS_ERR(kbatt->base)) + return PTR_ERR(kbatt->base); + + hwmon_dev = devm_hwmon_device_register_with_info(dev, KBATT, kbatt, + &kbatt_chip_info, + NULL); + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct auxiliary_device_id kbatt_devtype_aux[] = { + { .name = "keba.batt" }, + {} +}; +MODULE_DEVICE_TABLE(auxiliary, kbatt_devtype_aux); + +static struct auxiliary_driver kbatt_driver_aux = { + .name = KBATT, + .id_table = kbatt_devtype_aux, + .probe = kbatt_probe, +}; +module_auxiliary_driver(kbatt_driver_aux); + +MODULE_AUTHOR("Petar Bojanic "); +MODULE_AUTHOR("Gerhard Engleder "); +MODULE_DESCRIPTION("KEBA battery monitoring controller driver"); +MODULE_LICENSE("GPL"); -- 2.50.1 From 56591083846b8f4203234faf52de7a89f038ceeb Mon Sep 17 00:00:00 2001 From: John Keeping Date: Thu, 10 Apr 2025 19:03:57 +0100 Subject: [PATCH 09/16] hwmon: (pwm-fan) disable threaded interrupts The interrupt handler here just increments an atomic counter, jumping to a threaded handler risks missing tachometer pulses and is likely to be more expensive than the simple atomic increment. Signed-off-by: John Keeping Link: https://lore.kernel.org/r/20250410180357.2258822-1-jkeeping@inmusicbrands.com Signed-off-by: Guenter Roeck --- drivers/hwmon/pwm-fan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index d506a5e7e033..2df294793f6e 100644 --- a/drivers/hwmon/pwm-fan.c +++ b/drivers/hwmon/pwm-fan.c @@ -620,8 +620,8 @@ static int pwm_fan_probe(struct platform_device *pdev) if (tach->irq == -EPROBE_DEFER) return tach->irq; if (tach->irq > 0) { - ret = devm_request_irq(dev, tach->irq, pulse_handler, 0, - pdev->name, tach); + ret = devm_request_irq(dev, tach->irq, pulse_handler, + IRQF_NO_THREAD, pdev->name, tach); if (ret) { dev_err(dev, "Failed to request interrupt: %d\n", -- 2.50.1 From 80fcd1e7f5c7009fa1c64737df100cc304c19c1f Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Fri, 11 Apr 2025 12:20:53 +0100 Subject: [PATCH 10/16] hwmon: (xgene-hwmon) Simplify PCC shared memory region handling The PCC driver now handles mapping and unmapping of shared memory areas as part of pcc_mbox_{request,free}_channel(). Without these before, this xgene hwmon driver did handling of those mappings like several other PCC mailbox client drivers. There were redundant operations, leading to unnecessary code. Maintaining the consistency across these driver was harder due to scattered handling of shmem. Just use the mapped shmem and remove all redundant operations from this driver. Cc: Jean Delvare Cc: Guenter Roeck Cc: linux-hwmon@vger.kernel.org Signed-off-by: Sudeep Holla Link: https://lore.kernel.org/r/20250411112053.1148624-1-sudeep.holla@arm.com Signed-off-by: Guenter Roeck --- drivers/hwmon/xgene-hwmon.c | 39 ++++--------------------------------- 1 file changed, 4 insertions(+), 35 deletions(-) diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c index 2cdbd5f107a2..11c5d80428cd 100644 --- a/drivers/hwmon/xgene-hwmon.c +++ b/drivers/hwmon/xgene-hwmon.c @@ -103,8 +103,6 @@ struct xgene_hwmon_dev { struct device *hwmon_dev; bool temp_critical_alarm; - phys_addr_t comm_base_addr; - void *pcc_comm_addr; unsigned int usecs_lat; }; @@ -125,7 +123,8 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask) static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg) { - struct acpi_pcct_shared_memory *generic_comm_base = ctx->pcc_comm_addr; + struct acpi_pcct_shared_memory __iomem *generic_comm_base = + ctx->pcc_chan->shmem; u32 *ptr = (void *)(generic_comm_base + 1); int rc, i; u16 val; @@ -523,7 +522,8 @@ static void xgene_hwmon_rx_cb(struct mbox_client *cl, void *msg) static void xgene_hwmon_pcc_rx_cb(struct mbox_client *cl, void *msg) { struct xgene_hwmon_dev *ctx = to_xgene_hwmon_dev(cl); - struct acpi_pcct_shared_memory *generic_comm_base = ctx->pcc_comm_addr; + struct acpi_pcct_shared_memory __iomem *generic_comm_base = + ctx->pcc_chan->shmem; struct slimpro_resp_msg amsg; /* @@ -649,7 +649,6 @@ static int xgene_hwmon_probe(struct platform_device *pdev) } else { struct pcc_mbox_chan *pcc_chan; const struct acpi_device_id *acpi_id; - int version; acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev); @@ -658,8 +657,6 @@ static int xgene_hwmon_probe(struct platform_device *pdev) goto out_mbox_free; } - version = (int)acpi_id->driver_data; - if (device_property_read_u32(&pdev->dev, "pcc-channel", &ctx->mbox_idx)) { dev_err(&pdev->dev, "no pcc-channel property\n"); @@ -685,34 +682,6 @@ static int xgene_hwmon_probe(struct platform_device *pdev) goto out; } - /* - * This is the shared communication region - * for the OS and Platform to communicate over. - */ - ctx->comm_base_addr = pcc_chan->shmem_base_addr; - if (ctx->comm_base_addr) { - if (version == XGENE_HWMON_V2) - ctx->pcc_comm_addr = (void __force *)devm_ioremap(&pdev->dev, - ctx->comm_base_addr, - pcc_chan->shmem_size); - else - ctx->pcc_comm_addr = devm_memremap(&pdev->dev, - ctx->comm_base_addr, - pcc_chan->shmem_size, - MEMREMAP_WB); - } else { - dev_err(&pdev->dev, "Failed to get PCC comm region\n"); - rc = -ENODEV; - goto out; - } - - if (IS_ERR_OR_NULL(ctx->pcc_comm_addr)) { - dev_err(&pdev->dev, - "Failed to ioremap PCC comm region\n"); - rc = -ENOMEM; - goto out; - } - /* * pcc_chan->latency is just a Nominal value. In reality * the remote processor could be much slower to reply. -- 2.50.1 From 38b5a5acabb639a2e3d40bffb92bd42f613dcd71 Mon Sep 17 00:00:00 2001 From: Chen Ni Date: Mon, 14 Apr 2025 15:47:39 +0800 Subject: [PATCH 11/16] hwmon: (lm90) Use to_delayed_work() Use to_delayed_work() instead of open-coding it. Signed-off-by: Chen Ni Link: https://lore.kernel.org/r/20250414074739.3954203-1-nichen@iscas.ac.cn Signed-off-by: Guenter Roeck --- drivers/hwmon/lm90.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c index 75f09553fd67..c1f528e292f3 100644 --- a/drivers/hwmon/lm90.c +++ b/drivers/hwmon/lm90.c @@ -1235,7 +1235,7 @@ static int lm90_update_alarms(struct lm90_data *data, bool force) static void lm90_alert_work(struct work_struct *__work) { - struct delayed_work *delayed_work = container_of(__work, struct delayed_work, work); + struct delayed_work *delayed_work = to_delayed_work(__work); struct lm90_data *data = container_of(delayed_work, struct lm90_data, alert_work); /* Nothing to do if alerts are enabled */ -- 2.50.1 From 41e743881e85b4cda80c53e11075e7b19809b3ce Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Wed, 16 Apr 2025 15:59:19 -0700 Subject: [PATCH 12/16] hwmon: (aht10) Drop doctype annotations from static functions doctype annotations of static functions have little if any value. Drop them to silence 0-day complaints. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202504161919.duDL1s2X-lkp@intel.com/ Cc: Johannes Cornelis Draaijer Reviewed-by: Tzung-Bi Shih Signed-off-by: Guenter Roeck --- drivers/hwmon/aht10.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/hwmon/aht10.c b/drivers/hwmon/aht10.c index 312ef3e98754..d1c55e2eb479 100644 --- a/drivers/hwmon/aht10.c +++ b/drivers/hwmon/aht10.c @@ -94,7 +94,7 @@ struct aht10_data { unsigned int meas_size; }; -/** +/* * aht10_init() - Initialize an AHT10/AHT20 chip * @data: the data associated with this AHT10/AHT20 chip * Return: 0 if successful, 1 if not @@ -124,7 +124,7 @@ static int aht10_init(struct aht10_data *data) return 0; } -/** +/* * aht10_polltime_expired() - check if the minimum poll interval has * expired * @data: the data containing the time to compare @@ -140,7 +140,7 @@ static int aht10_polltime_expired(struct aht10_data *data) DECLARE_CRC8_TABLE(crc8_table); -/** +/* * crc8_check() - check crc of the sensor's measurements * @raw_data: data frame received from sensor(including crc as the last byte) * @count: size of the data frame @@ -155,7 +155,7 @@ static int crc8_check(u8 *raw_data, int count) return crc8(crc8_table, raw_data, count, CRC8_INIT_VALUE); } -/** +/* * aht10_read_values() - read and parse the raw data from the AHT10/AHT20 * @data: the struct aht10_data to use for the lock * Return: 0 if successful, 1 if not @@ -214,7 +214,7 @@ static int aht10_read_values(struct aht10_data *data) return 0; } -/** +/* * aht10_interval_write() - store the given minimum poll interval. * Return: 0 on success, -EINVAL if a value lower than the * AHT10_MIN_POLL_INTERVAL is given @@ -226,7 +226,7 @@ static ssize_t aht10_interval_write(struct aht10_data *data, return 0; } -/** +/* * aht10_interval_read() - read the minimum poll interval * in milliseconds */ @@ -237,7 +237,7 @@ static ssize_t aht10_interval_read(struct aht10_data *data, return 0; } -/** +/* * aht10_temperature1_read() - read the temperature in millidegrees */ static int aht10_temperature1_read(struct aht10_data *data, long *val) @@ -252,7 +252,7 @@ static int aht10_temperature1_read(struct aht10_data *data, long *val) return 0; } -/** +/* * aht10_humidity1_read() - read the relative humidity in millipercent */ static int aht10_humidity1_read(struct aht10_data *data, long *val) -- 2.50.1 From e799657a8aac1d974c90a295dd88b1d95697d18d Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Mon, 21 Apr 2025 00:33:34 +0200 Subject: [PATCH 13/16] hwmon: (dell-smm) Add the Dell OptiPlex 7050 to the DMI whitelist MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit A user reported that the driver works on the OptiPlex 7050. Add this machine to the DMI whitelist. Closes: https://github.com/Wer-Wolf/i8kutils/issues/12 Signed-off-by: Armin Wolf Acked-by: Pali Rohár Link: https://lore.kernel.org/r/20250420223334.12920-1-W_Armin@gmx.de Signed-off-by: Guenter Roeck --- drivers/hwmon/dell-smm-hwmon.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c index 79e5606e6d2f..1e2c8e284001 100644 --- a/drivers/hwmon/dell-smm-hwmon.c +++ b/drivers/hwmon/dell-smm-hwmon.c @@ -1273,6 +1273,13 @@ static const struct dmi_system_id i8k_dmi_table[] __initconst = { DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "OptiPlex 7060"), }, }, + { + .ident = "Dell OptiPlex 7050", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "OptiPlex 7050"), + }, + }, { .ident = "Dell Precision", .matches = { -- 2.50.1 From 4cf1aab45cc56e9276924c21b79da42e2eac01df Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Wed, 19 Jun 2024 08:53:56 -0700 Subject: [PATCH 14/16] hwmon: (spd5118) Split into common and I2C specific code Split spd5118 driver into common and I2C specific code to enable adding support for I3C. Signed-off-by: Guenter Roeck --- drivers/hwmon/spd5118.c | 277 +++++++++++++++++++++------------------- 1 file changed, 143 insertions(+), 134 deletions(-) diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c index 358152868d96..02eb21684c3a 100644 --- a/drivers/hwmon/spd5118.c +++ b/drivers/hwmon/spd5118.c @@ -305,51 +305,6 @@ static bool spd5118_vendor_valid(u8 bank, u8 id) return id && id != 0x7f; } -/* Return 0 if detection is successful, -ENODEV otherwise */ -static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info) -{ - struct i2c_adapter *adapter = client->adapter; - int regval; - - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | - I2C_FUNC_SMBUS_WORD_DATA)) - return -ENODEV; - - regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE); - if (regval != 0x5118) - return -ENODEV; - - regval = i2c_smbus_read_word_data(client, SPD5118_REG_VENDOR); - if (regval < 0 || !spd5118_vendor_valid(regval & 0xff, regval >> 8)) - return -ENODEV; - - regval = i2c_smbus_read_byte_data(client, SPD5118_REG_CAPABILITY); - if (regval < 0) - return -ENODEV; - if (!(regval & SPD5118_CAP_TS_SUPPORT) || (regval & 0xfc)) - return -ENODEV; - - regval = i2c_smbus_read_byte_data(client, SPD5118_REG_TEMP_CLR); - if (regval) - return -ENODEV; - regval = i2c_smbus_read_byte_data(client, SPD5118_REG_ERROR_CLR); - if (regval) - return -ENODEV; - - regval = i2c_smbus_read_byte_data(client, SPD5118_REG_REVISION); - if (regval < 0 || (regval & 0xc1)) - return -ENODEV; - - regval = i2c_smbus_read_byte_data(client, SPD5118_REG_TEMP_CONFIG); - if (regval < 0) - return -ENODEV; - if (regval & ~SPD5118_TS_DISABLE) - return -ENODEV; - - strscpy(info->type, "spd5118", I2C_NAME_SIZE); - return 0; -} - static const struct hwmon_channel_info *spd5118_info[] = { HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), @@ -483,7 +438,7 @@ static bool spd5118_volatile_reg(struct device *dev, unsigned int reg) } } -static const struct regmap_range_cfg spd5118_regmap_range_cfg[] = { +static const struct regmap_range_cfg spd5118_i2c_regmap_range_cfg[] = { { .selector_reg = SPD5118_REG_I2C_LEGACY_MODE, .selector_mask = SPD5118_LEGACY_PAGE_MASK, @@ -495,7 +450,7 @@ static const struct regmap_range_cfg spd5118_regmap_range_cfg[] = { }, }; -static const struct regmap_config spd5118_regmap_config = { +static const struct regmap_config spd5118_i2c_regmap_config = { .reg_bits = 8, .val_bits = 8, .max_register = 0x7ff, @@ -503,87 +458,62 @@ static const struct regmap_config spd5118_regmap_config = { .volatile_reg = spd5118_volatile_reg, .cache_type = REGCACHE_MAPLE, - .ranges = spd5118_regmap_range_cfg, - .num_ranges = ARRAY_SIZE(spd5118_regmap_range_cfg), + .ranges = spd5118_i2c_regmap_range_cfg, + .num_ranges = ARRAY_SIZE(spd5118_i2c_regmap_range_cfg), }; -static int spd5118_init(struct i2c_client *client) +static int spd5118_suspend(struct device *dev) { - struct i2c_adapter *adapter = client->adapter; - int err, regval, mode; - - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | - I2C_FUNC_SMBUS_WORD_DATA)) - return -ENODEV; - - regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE); - if (regval < 0 || (regval && regval != 0x5118)) - return -ENODEV; + struct spd5118_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + u32 regval; + int err; /* - * If the device type registers return 0, it is possible that the chip - * has a non-zero page selected and takes the specification literally, - * i.e. disables access to volatile registers besides the page register - * if the page is not 0. Try to identify such chips. + * Make sure the configuration register in the regmap cache is current + * before bypassing it. */ - if (!regval) { - /* Vendor ID registers must also be 0 */ - regval = i2c_smbus_read_word_data(client, SPD5118_REG_VENDOR); - if (regval) - return -ENODEV; - - /* The selected page in MR11 must not be 0 */ - mode = i2c_smbus_read_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE); - if (mode < 0 || (mode & ~SPD5118_LEGACY_MODE_MASK) || - !(mode & SPD5118_LEGACY_PAGE_MASK)) - return -ENODEV; + err = regmap_read(regmap, SPD5118_REG_TEMP_CONFIG, ®val); + if (err < 0) + return err; - err = i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE, - mode & SPD5118_LEGACY_MODE_ADDR); - if (err) - return -ENODEV; + regcache_cache_bypass(regmap, true); + regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG, SPD5118_TS_DISABLE, + SPD5118_TS_DISABLE); + regcache_cache_bypass(regmap, false); - /* - * If the device type registers are still bad after selecting - * page 0, this is not a SPD5118 device. Restore original - * legacy mode register value and abort. - */ - regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE); - if (regval != 0x5118) { - i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE, mode); - return -ENODEV; - } - } + regcache_cache_only(regmap, true); + regcache_mark_dirty(regmap); - /* We are reasonably sure that this is really a SPD5118 hub controller */ return 0; } -static int spd5118_probe(struct i2c_client *client) +static int spd5118_resume(struct device *dev) { - struct device *dev = &client->dev; - unsigned int regval, revision, vendor, bank; + struct spd5118_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + + regcache_cache_only(regmap, false); + return regcache_sync(regmap); +} + +static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume); + +static int spd5118_common_probe(struct device *dev, struct regmap *regmap) +{ + unsigned int capability, revision, vendor, bank; struct spd5118_data *data; struct device *hwmon_dev; - struct regmap *regmap; int err; - err = spd5118_init(client); - if (err) - return err; - data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); if (!data) return -ENOMEM; - regmap = devm_regmap_init_i2c(client, &spd5118_regmap_config); - if (IS_ERR(regmap)) - return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n"); - - err = regmap_read(regmap, SPD5118_REG_CAPABILITY, ®val); + err = regmap_read(regmap, SPD5118_REG_CAPABILITY, &capability); if (err) return err; - if (!(regval & SPD5118_CAP_TS_SUPPORT)) + if (!(capability & SPD5118_CAP_TS_SUPPORT)) return -ENODEV; err = regmap_read(regmap, SPD5118_REG_REVISION, &revision); @@ -627,48 +557,127 @@ static int spd5118_probe(struct i2c_client *client) return 0; } -static int spd5118_suspend(struct device *dev) +/* I2C */ + +/* Return 0 if detection is successful, -ENODEV otherwise */ +static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info) { - struct spd5118_data *data = dev_get_drvdata(dev); - struct regmap *regmap = data->regmap; - u32 regval; - int err; + struct i2c_adapter *adapter = client->adapter; + int regval; + + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | + I2C_FUNC_SMBUS_WORD_DATA)) + return -ENODEV; + + regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE); + if (regval != 0x5118) + return -ENODEV; + + regval = i2c_smbus_read_word_data(client, SPD5118_REG_VENDOR); + if (regval < 0 || !spd5118_vendor_valid(regval & 0xff, regval >> 8)) + return -ENODEV; + + regval = i2c_smbus_read_byte_data(client, SPD5118_REG_CAPABILITY); + if (regval < 0) + return -ENODEV; + if (!(regval & SPD5118_CAP_TS_SUPPORT) || (regval & 0xfc)) + return -ENODEV; + + regval = i2c_smbus_read_byte_data(client, SPD5118_REG_TEMP_CLR); + if (regval) + return -ENODEV; + regval = i2c_smbus_read_byte_data(client, SPD5118_REG_ERROR_CLR); + if (regval) + return -ENODEV; + + regval = i2c_smbus_read_byte_data(client, SPD5118_REG_REVISION); + if (regval < 0 || (regval & 0xc1)) + return -ENODEV; + + regval = i2c_smbus_read_byte_data(client, SPD5118_REG_TEMP_CONFIG); + if (regval < 0) + return -ENODEV; + if (regval & ~SPD5118_TS_DISABLE) + return -ENODEV; + + strscpy(info->type, "spd5118", I2C_NAME_SIZE); + return 0; +} + +static int spd5118_i2c_init(struct i2c_client *client) +{ + struct i2c_adapter *adapter = client->adapter; + int err, regval, mode; + + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | + I2C_FUNC_SMBUS_WORD_DATA)) + return -ENODEV; + + regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE); + if (regval < 0 || (regval && regval != 0x5118)) + return -ENODEV; /* - * Make sure the configuration register in the regmap cache is current - * before bypassing it. + * If the device type registers return 0, it is possible that the chip + * has a non-zero page selected and takes the specification literally, + * i.e. disables access to volatile registers besides the page register + * if the page is not 0. Try to identify such chips. */ - err = regmap_read(regmap, SPD5118_REG_TEMP_CONFIG, ®val); - if (err < 0) - return err; + if (!regval) { + /* Vendor ID registers must also be 0 */ + regval = i2c_smbus_read_word_data(client, SPD5118_REG_VENDOR); + if (regval) + return -ENODEV; - regcache_cache_bypass(regmap, true); - regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG, SPD5118_TS_DISABLE, - SPD5118_TS_DISABLE); - regcache_cache_bypass(regmap, false); + /* The selected page in MR11 must not be 0 */ + mode = i2c_smbus_read_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE); + if (mode < 0 || (mode & ~SPD5118_LEGACY_MODE_MASK) || + !(mode & SPD5118_LEGACY_PAGE_MASK)) + return -ENODEV; - regcache_cache_only(regmap, true); - regcache_mark_dirty(regmap); + err = i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE, + mode & SPD5118_LEGACY_MODE_ADDR); + if (err) + return -ENODEV; + /* + * If the device type registers are still bad after selecting + * page 0, this is not a SPD5118 device. Restore original + * legacy mode register value and abort. + */ + regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE); + if (regval != 0x5118) { + i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE, mode); + return -ENODEV; + } + } + + /* We are reasonably sure that this is really a SPD5118 hub controller */ return 0; } -static int spd5118_resume(struct device *dev) +static int spd5118_i2c_probe(struct i2c_client *client) { - struct spd5118_data *data = dev_get_drvdata(dev); - struct regmap *regmap = data->regmap; + struct device *dev = &client->dev; + struct regmap *regmap; + int err; - regcache_cache_only(regmap, false); - return regcache_sync(regmap); -} + err = spd5118_i2c_init(client); + if (err) + return err; -static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume); + regmap = devm_regmap_init_i2c(client, &spd5118_i2c_regmap_config); + if (IS_ERR(regmap)) + return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n"); + + return spd5118_common_probe(dev, regmap); +} -static const struct i2c_device_id spd5118_id[] = { +static const struct i2c_device_id spd5118_i2c_id[] = { { "spd5118" }, { } }; -MODULE_DEVICE_TABLE(i2c, spd5118_id); +MODULE_DEVICE_TABLE(i2c, spd5118_i2c_id); static const struct of_device_id spd5118_of_ids[] = { { .compatible = "jedec,spd5118", }, @@ -676,20 +685,20 @@ static const struct of_device_id spd5118_of_ids[] = { }; MODULE_DEVICE_TABLE(of, spd5118_of_ids); -static struct i2c_driver spd5118_driver = { +static struct i2c_driver spd5118_i2c_driver = { .class = I2C_CLASS_HWMON, .driver = { .name = "spd5118", .of_match_table = spd5118_of_ids, .pm = pm_sleep_ptr(&spd5118_pm_ops), }, - .probe = spd5118_probe, - .id_table = spd5118_id, + .probe = spd5118_i2c_probe, + .id_table = spd5118_i2c_id, .detect = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? spd5118_detect : NULL, .address_list = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL, }; -module_i2c_driver(spd5118_driver); +module_i2c_driver(spd5118_i2c_driver); MODULE_AUTHOR("René Rebe "); MODULE_AUTHOR("Guenter Roeck "); -- 2.50.1 From ae28532aff1f7912c1d118b4257c095ce62f1cb0 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Sun, 17 Nov 2024 07:55:32 -0800 Subject: [PATCH 15/16] hwmon: (spd5118) Name chips taking the specification literally The Renesas/IDT SPD5118 Hub Controller is known to take the specification literally and does not permit access to volatile registers except for the page register if the selected page is non-zero. Explicitly name the chip to ensure that the information does not get lost. Signed-off-by: Guenter Roeck --- drivers/hwmon/spd5118.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c index 02eb21684c3a..c5ad06b90cbf 100644 --- a/drivers/hwmon/spd5118.c +++ b/drivers/hwmon/spd5118.c @@ -621,7 +621,8 @@ static int spd5118_i2c_init(struct i2c_client *client) * If the device type registers return 0, it is possible that the chip * has a non-zero page selected and takes the specification literally, * i.e. disables access to volatile registers besides the page register - * if the page is not 0. Try to identify such chips. + * if the page is not 0. The Renesas/ITD SPD5118 Hub Controller is known + * to show this behavior. Try to identify such chips. */ if (!regval) { /* Vendor ID registers must also be 0 */ -- 2.50.1 From be82d39c537e884e490a79bcc38a3be8d9e8b0a9 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 14 Jun 2024 15:13:46 -0700 Subject: [PATCH 16/16] hwmon: (spd5118) Support 16-bit addressing for NVMEM accesses I3C uses 16-bit register addresses. Setting the page through the legacy mode access pointer in the legacy mode device configuration register (MR11) is not needed. This is similar to 16-bit addressing in legacy mode. Signed-off-by: Guenter Roeck --- drivers/hwmon/spd5118.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c index c5ad06b90cbf..92a3cb0bb515 100644 --- a/drivers/hwmon/spd5118.c +++ b/drivers/hwmon/spd5118.c @@ -66,6 +66,9 @@ static const unsigned short normal_i2c[] = { #define SPD5118_EEPROM_BASE 0x80 #define SPD5118_EEPROM_SIZE (SPD5118_PAGE_SIZE * SPD5118_NUM_PAGES) +#define PAGE_ADDR0(page) (((page) & BIT(0)) << 6) +#define PAGE_ADDR1_4(page) (((page) & GENMASK(4, 1)) >> 1) + /* Temperature unit in millicelsius */ #define SPD5118_TEMP_UNIT (MILLIDEGREE_PER_DEGREE / 4) /* Representable temperature range in millicelsius */ @@ -75,6 +78,7 @@ static const unsigned short normal_i2c[] = { struct spd5118_data { struct regmap *regmap; struct mutex nvmem_lock; + bool is_16bit; }; /* hwmon */ @@ -331,11 +335,12 @@ static const struct hwmon_chip_info spd5118_chip_info = { /* nvmem */ -static ssize_t spd5118_nvmem_read_page(struct regmap *regmap, char *buf, +static ssize_t spd5118_nvmem_read_page(struct spd5118_data *data, char *buf, unsigned int offset, size_t count) { - int addr = (offset >> SPD5118_PAGE_SHIFT) * 0x100 + SPD5118_EEPROM_BASE; - int err; + int page = offset >> SPD5118_PAGE_SHIFT; + struct regmap *regmap = data->regmap; + int err, addr; offset &= SPD5118_PAGE_MASK; @@ -343,6 +348,12 @@ static ssize_t spd5118_nvmem_read_page(struct regmap *regmap, char *buf, if (offset + count > SPD5118_PAGE_SIZE) count = SPD5118_PAGE_SIZE - offset; + if (data->is_16bit) { + addr = SPD5118_EEPROM_BASE | PAGE_ADDR0(page) | + (PAGE_ADDR1_4(page) << 8); + } else { + addr = page * 0x100 + SPD5118_EEPROM_BASE; + } err = regmap_bulk_read(regmap, addr + offset, buf, count); if (err) return err; @@ -365,7 +376,7 @@ static int spd5118_nvmem_read(void *priv, unsigned int off, void *val, size_t co mutex_lock(&data->nvmem_lock); while (count) { - ret = spd5118_nvmem_read_page(data->regmap, buf, off, count); + ret = spd5118_nvmem_read_page(data, buf, off, count); if (ret < 0) { mutex_unlock(&data->nvmem_lock); return ret; @@ -516,6 +527,13 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap) if (!(capability & SPD5118_CAP_TS_SUPPORT)) return -ENODEV; + /* + * 16-bit register addresses are not (yet) supported with I2C. + * Therefore, if this is an I2C device, register addresses must be + * 8 bit wide. + */ + data->is_16bit = !!i2c_verify_adapter(dev); + err = regmap_read(regmap, SPD5118_REG_REVISION, &revision); if (err) return err; -- 2.50.1