[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2234425e-b676-4564-96c6-57c0a635292c@roeck-us.net>
Date: Thu, 3 Apr 2025 09:32:44 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Alexis Czezar Torreno <alexisczezar.torreno@...log.com>
Cc: Jean Delvare <jdelvare@...e.com>, Jonathan Corbet <corbet@....net>,
Delphine CC Chiu <Delphine_CC_Chiu@...ynn.com>,
linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org
Subject: Re: [PATCH v2 1/2] hwmon: (pmbus/max34440): Fix support for max34451
On Thu, Apr 03, 2025 at 01:16:18PM +0800, 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@...log.com>
> ---
> drivers/hwmon/pmbus/max34440.c | 55 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c
> index c9dda33831ff24e7b5e2fd1956a65e6bd2bfcbb9..585746806663409bc97042647f6c0aba4c6f520a 100644
> --- a/drivers/hwmon/pmbus/max34440.c
> +++ b/drivers/hwmon/pmbus/max34440.c
> @@ -34,16 +34,22 @@ 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
>
> +#define MAX34451ETNA6_MFR_REV 0x0012
> +
> #define MAX34451_MFR_CHANNEL_CONFIG 0xe4
> #define MAX34451_MFR_CHANNEL_CONFIG_SEL_MASK 0x3f
>
> struct max34440_data {
> int id;
> struct pmbus_driver_info info;
> + bool pmbus_addr_fixed;
Unnecessary. See below.
> + u32 iout_oc_warn_limit;
> + u32 iout_oc_fault_limit;
u8 would be sufficient.
> };
>
> #define to_max34440_data(x) container_of(x, struct max34440_data, info)
> @@ -60,11 +66,11 @@ 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);
> + data->iout_oc_fault_limit);
> break;
> case PMBUS_IOUT_OC_WARN_LIMIT:
> ret = pmbus_read_word_data(client, page, phase,
> - MAX34440_IOUT_OC_WARN_LIMIT);
> + data->iout_oc_warn_limit);
> break;
> case PMBUS_VIRT_READ_VOUT_MIN:
> ret = pmbus_read_word_data(client, page, phase,
> @@ -133,11 +139,11 @@ 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,
> + ret = pmbus_write_word_data(client, page, data->iout_oc_fault_limit,
> word);
> break;
> case PMBUS_IOUT_OC_WARN_LIMIT:
> - ret = pmbus_write_word_data(client, page, MAX34440_IOUT_OC_WARN_LIMIT,
> + ret = pmbus_write_word_data(client, page, data->iout_oc_warn_limit,
> word);
> break;
> case PMBUS_VIRT_RESET_POUT_HISTORY:
> @@ -235,6 +241,24 @@ static int max34451_set_supported_funcs(struct i2c_client *client,
> */
>
> int page, rv;
> + bool max34451_na6 = false;
> +
> + rv = i2c_smbus_read_word_data(client, PMBUS_MFR_REVISION);
> + if (rv < 0)
> + return rv;
> +
> + if (rv == MAX34451ETNA6_MFR_REV) {
Sure that this is only one revision ?
Would it be better to use ">=" instead of "==" ?
> + max34451_na6 = true;
> + data->pmbus_addr_fixed = true;
> + data->info.format[PSC_VOLTAGE_IN] = direct;
> + data->info.format[PSC_CURRENT_IN] = direct;
> + data->info.m[PSC_VOLTAGE_IN] = 1;
> + data->info.b[PSC_VOLTAGE_IN] = 0;
> + data->info.R[PSC_VOLTAGE_IN] = 3;
> + data->info.m[PSC_CURRENT_IN] = 1;
> + data->info.b[PSC_CURRENT_IN] = 0;
> + data->info.R[PSC_CURRENT_IN] = 2;
Assign register addresses directly here.
data->iout_oc_fault_limit = PMBUS_IOUT_OC_FAULT_LIMIT;
data->iout_oc_warn_limit = PMBUS_IOUT_OC_WARN_LIMIT;
} else {
data->iout_oc_fault_limit = MAX34440_IOUT_OC_FAULT_LIMIT;
data->iout_oc_warn_limit = MAX34440_IOUT_OC_WARN_LIMIT;
> + }
Thanks,
Guenter
Powered by blists - more mailing lists