On 3/19/25 20:55, Alexis Czezar Torreno wrote:
The max344** family has an issue with some PMBUS address being switched.
This includes max34451 however version MAX34451-NA6 and later has this
issue fixed and this commit supports that update.
Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@xxxxxxxxxx>
---
Documentation/hwmon/max34440.rst | 8 ++-
drivers/hwmon/pmbus/max34440.c | 102 ++++++++++++++++++++++++++-------------
2 files changed, 74 insertions(+), 36 deletions(-)
diff --git a/Documentation/hwmon/max34440.rst b/Documentation/hwmon/max34440.rst
index 162d289f08140341e8e76ab7033834ba07a8b935..b1f3f75091bb8e233e766c24913194dd62b0cd90 100644
--- a/Documentation/hwmon/max34440.rst
+++ b/Documentation/hwmon/max34440.rst
@@ -35,7 +35,7 @@ Supported chips:
PMBus 16-Channel V/I Monitor and 12-Channel Sequencer/Marginer
- Prefixes: 'max34451'
+ Prefixes: 'max34451', 'max34451_na6'
Addresses scanned: -
@@ -93,6 +93,10 @@ attribute is set to a positive value. Power measurement is only enabled if
channel 1 (3) is configured for voltage measurement, and channel 2 (4) is
configured for current measurement.
+For MAX34451, version MAX34451ETNA6+ and later are denoted with prefix
+'max34451_na6'. The previous versions contains some errors on the PMBUS
+addresses and these are fixed on the later versions.
+
Platform data support
---------------------
@@ -192,4 +196,4 @@ temp[1-8]_reset_history Write any value to reset history.
- MAX34451 supports attribute groups in[1-16] (or curr[1-16] based on
input pins) and temp[1-5].
- MAX34460 supports attribute groups in[1-12] and temp[1-5].
- - MAX34461 supports attribute groups in[1-16] and temp[1-5].
+ - MAX34461 supports attribute groups in[1-16] and temp[1-5].
\ No newline at end of file
diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c
index c9dda33831ff24e7b5e2fd1956a65e6bd2bfcbb9..d483c01f256c96f048c9da5981f10f52402d981c 100644
--- a/drivers/hwmon/pmbus/max34440.c
+++ b/drivers/hwmon/pmbus/max34440.c
@@ -14,7 +14,15 @@
#include <linux/i2c.h>
#include "pmbus.h"
-enum chips { max34440, max34441, max34446, max34451, max34460, max34461 };
+enum chips {
+ max34440,
+ max34441,
+ max34446,
+ max34451,
+ max34451_na6,
+ max34460,
+ max34461,
+};
#define MAX34440_MFR_VOUT_PEAK 0xd4
#define MAX34440_MFR_IOUT_PEAK 0xd5
@@ -34,6 +42,7 @@ enum chips { max34440, max34441, max34446, max34451, max34460, max34461 };
/*
* The whole max344* family have IOUT_OC_WARN_LIMIT and IOUT_OC_FAULT_LIMIT
* swapped from the standard pmbus spec addresses.
+ * For max34451, version MAX34451ETNA6+ and later has this issue fixed.
*/
#define MAX34440_IOUT_OC_WARN_LIMIT 0x46
#define MAX34440_IOUT_OC_FAULT_LIMIT 0x4A
@@ -59,12 +68,20 @@ static int max34440_read_word_data(struct i2c_client *client, int page,
switch (reg) {
case PMBUS_IOUT_OC_FAULT_LIMIT:
- ret = pmbus_read_word_data(client, page, phase,
- MAX34440_IOUT_OC_FAULT_LIMIT);
+ if (data->id == max34451_na6)
Use a flag instead of a chip ID, or even better store the register addresses
in max34440_data to avoid the runtime checks.
+ ret = pmbus_read_word_data(client, page, phase,
+ PMBUS_IOUT_OC_FAULT_LIMIT);
+ else
+ ret = pmbus_read_word_data(client, page, phase,
+ MAX34440_IOUT_OC_FAULT_LIMIT);
break;
case PMBUS_IOUT_OC_WARN_LIMIT:
- ret = pmbus_read_word_data(client, page, phase,
- MAX34440_IOUT_OC_WARN_LIMIT);
+ if (data->id == max34451_na6)
+ ret = pmbus_read_word_data(client, page, phase,
+ PMBUS_IOUT_OC_WARN_LIMIT);
+ else
+ ret = pmbus_read_word_data(client, page, phase,
+ MAX34440_IOUT_OC_WARN_LIMIT);
break;
case PMBUS_VIRT_READ_VOUT_MIN:
ret = pmbus_read_word_data(client, page, phase,
@@ -75,7 +92,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 != max34451_na6)
return -ENXIO;
ret = pmbus_read_word_data(client, page, phase,
MAX34446_MFR_IOUT_AVG);
@@ -133,12 +151,20 @@ static int max34440_write_word_data(struct i2c_client *client, int page,
switch (reg) {
case PMBUS_IOUT_OC_FAULT_LIMIT:
- ret = pmbus_write_word_data(client, page, MAX34440_IOUT_OC_FAULT_LIMIT,
- word);
+ if (data->id == max34451_na6)
+ ret = pmbus_write_word_data(client, page, PMBUS_IOUT_OC_FAULT_LIMIT,
+ word);
+ else
+ ret = pmbus_write_word_data(client, page, MAX34440_IOUT_OC_FAULT_LIMIT,
+ word);
break;
case PMBUS_IOUT_OC_WARN_LIMIT:
- ret = pmbus_write_word_data(client, page, MAX34440_IOUT_OC_WARN_LIMIT,
- word);
+ if (data->id == max34451_na6)
+ ret = pmbus_write_word_data(client, page, PMBUS_IOUT_OC_WARN_LIMIT,
+ word);
+ else
+ ret = pmbus_write_word_data(client, page, MAX34440_IOUT_OC_WARN_LIMIT,
+ word);
break;
case PMBUS_VIRT_RESET_POUT_HISTORY:
ret = pmbus_write_word_data(client, page,
@@ -159,7 +185,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 == max34451_na6))
ret = pmbus_write_word_data(client, page,
MAX34446_MFR_IOUT_AVG, 0);
@@ -270,6 +297,29 @@ static int max34451_set_supported_funcs(struct i2c_client *client,
return 0;
}
+#define MAX34451_COMMON_INFO \
+ .pages = 21, \
+ .format[PSC_VOLTAGE_OUT] = direct, \
+ .format[PSC_TEMPERATURE] = direct, \
+ .format[PSC_CURRENT_OUT] = direct, \
+ .m[PSC_VOLTAGE_OUT] = 1, \
+ .b[PSC_VOLTAGE_OUT] = 0, \
+ .R[PSC_VOLTAGE_OUT] = 3, \
+ .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, \
+ /* func 0-15 is set dynamically before probing */ \
+ .func[16] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP, \
+ .func[17] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP, \
+ .func[18] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP, \
+ .func[19] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP, \
+ .func[20] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP, \
+ .read_word_data = max34440_read_word_data, \
+ .write_word_data = max34440_write_word_data,
+
static struct pmbus_driver_info max34440_info[] = {
[max34440] = {
.pages = 14,
@@ -394,27 +444,10 @@ static struct pmbus_driver_info max34440_info[] = {
.write_word_data = max34440_write_word_data,
},
[max34451] = {
- .pages = 21,
- .format[PSC_VOLTAGE_OUT] = direct,
- .format[PSC_TEMPERATURE] = direct,
- .format[PSC_CURRENT_OUT] = direct,
- .m[PSC_VOLTAGE_OUT] = 1,
- .b[PSC_VOLTAGE_OUT] = 0,
- .R[PSC_VOLTAGE_OUT] = 3,
- .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,
- /* func 0-15 is set dynamically before probing */
- .func[16] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
- .func[17] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
- .func[18] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
- .func[19] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
- .func[20] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
- .read_word_data = max34440_read_word_data,
- .write_word_data = max34440_write_word_data,
+ MAX34451_COMMON_INFO,
+ },
+ [max34451_na6] = {
+ MAX34451_COMMON_INFO,
},
This is way too complicated. Use a flag or set the register addresses in struct
max34440_data instead.
[max34460] = {
.pages = 18,
@@ -495,7 +528,7 @@ static int max34440_probe(struct i2c_client *client)
data->id = i2c_match_id(max34440_id, client)->driver_data;
data->info = max34440_info[data->id];
- if (data->id == max34451) {
+ if (data->id == max34451 || data->id == max34451_na6) {
rv = max34451_set_supported_funcs(client, data);
if (rv)
return rv;
@@ -509,6 +542,7 @@ static const struct i2c_device_id max34440_id[] = {
{"max34441", max34441},
{"max34446", max34446},
{"max34451", max34451},
+ {"max34451_na6", max34451_na6},
Relying on this is way too fragile. This must be detectable from the chip;
maybe using MFR_REVISION.
{"max34460", max34460},
{"max34461", max34461},
{}
@@ -529,4 +563,4 @@ module_i2c_driver(max34440_driver);
MODULE_AUTHOR("Guenter Roeck");
MODULE_DESCRIPTION("PMBus driver for Maxim MAX34440/MAX34441");
MODULE_LICENSE("GPL");
-MODULE_IMPORT_NS("PMBUS");
+MODULE_IMPORT_NS(PMBUS);
Looks like your code is based on an older kernel branch. Please
rebase on top of mainline.
Guenter
