* [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases
@ 2021-01-25 18:53 Guenter Roeck
2021-01-25 18:53 ` [PATCH 2/2] RFT: hwmon: (pmbus/max16601) Add support for MAX16508 Guenter Roeck
2021-01-25 22:49 ` [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases Alex Qiu
0 siblings, 2 replies; 6+ messages in thread
From: Guenter Roeck @ 2021-01-25 18:53 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Jean Delvare, Guenter Roeck, Alex Qiu, Ugur Usug
The MAX16601 can report the number of populated phases. Use this
information to only create sysfs attributes for populated phases.
Cc: Alex Qiu <xqiu@google.com>
Cc: Ugur Usug <Ugur.Usug@maximintegrated.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Documentation/hwmon/max16601.rst | 98 ++++++++++----------------------
drivers/hwmon/pmbus/max16601.c | 17 +++++-
2 files changed, 45 insertions(+), 70 deletions(-)
diff --git a/Documentation/hwmon/max16601.rst b/Documentation/hwmon/max16601.rst
index 346e74674c51..93d25dfa028e 100644
--- a/Documentation/hwmon/max16601.rst
+++ b/Documentation/hwmon/max16601.rst
@@ -60,75 +60,35 @@ curr1_input VCORE input current, derived from duty cycle and output
curr1_max Maximum input current.
curr1_max_alarm Current high alarm.
-curr2_label "iin1.0"
-curr2_input VCORE phase 0 input current.
-
-curr3_label "iin1.1"
-curr3_input VCORE phase 1 input current.
-
-curr4_label "iin1.2"
-curr4_input VCORE phase 2 input current.
-
-curr5_label "iin1.3"
-curr5_input VCORE phase 3 input current.
-
-curr6_label "iin1.4"
-curr6_input VCORE phase 4 input current.
-
-curr7_label "iin1.5"
-curr7_input VCORE phase 5 input current.
-
-curr8_label "iin1.6"
-curr8_input VCORE phase 6 input current.
-
-curr9_label "iin1.7"
-curr9_input VCORE phase 7 input current.
-
-curr10_label "iin2"
-curr10_input VCORE input current, derived from sensor element.
-
-curr11_label "iin3"
-curr11_input VSA input current.
-
-curr12_label "iout1"
-curr12_input VCORE output current.
-curr12_crit Critical output current.
-curr12_crit_alarm Output current critical alarm.
-curr12_max Maximum output current.
-curr12_max_alarm Output current high alarm.
-
-curr13_label "iout1.0"
-curr13_input VCORE phase 0 output current.
-
-curr14_label "iout1.1"
-curr14_input VCORE phase 1 output current.
-
-curr15_label "iout1.2"
-curr15_input VCORE phase 2 output current.
-
-curr16_label "iout1.3"
-curr16_input VCORE phase 3 output current.
-
-curr17_label "iout1.4"
-curr17_input VCORE phase 4 output current.
-
-curr18_label "iout1.5"
-curr18_input VCORE phase 5 output current.
-
-curr19_label "iout1.6"
-curr19_input VCORE phase 6 output current.
-
-curr20_label "iout1.7"
-curr20_input VCORE phase 7 output current.
-
-curr21_label "iout3"
-curr21_input VSA output current.
-curr21_highest Historical maximum VSA output current.
-curr21_reset_history Write any value to reset curr21_highest.
-curr21_crit Critical output current.
-curr21_crit_alarm Output current critical alarm.
-curr21_max Maximum output current.
-curr21_max_alarm Output current high alarm.
+curr[P+2]_label "iin1.P"
+curr[P+2]_input VCORE phase P input current.
+
+curr[N+2]_label "iin2"
+curr[N+2]_input VCORE input current, derived from sensor element.
+ 'N' is the number of enabled/populated phases.
+
+curr[N+3]_label "iin3"
+curr[N+3]_input VSA input current.
+
+curr[N+4]_label "iout1"
+curr[N+4]_input VCORE output current.
+curr[N+4]_crit Critical output current.
+curr[N+4]_crit_alarm Output current critical alarm.
+curr[N+4]_max Maximum output current.
+curr[N+4]_max_alarm Output current high alarm.
+
+curr[N+P+5]_label "iout1.P"
+curr[N+P+5]_input VCORE phase P output current.
+
+curr[2*N+5]_label "iout3"
+curr[2*N+5]_input VSA output current.
+curr[2*N+5]_highest Historical maximum VSA output current.
+curr[2*N+5]_reset_history
+ Write any value to reset curr21_highest.
+curr[2*N+5]_crit Critical output current.
+curr[2*N+5]_crit_alarm Output current critical alarm.
+curr[2*N+5]_max Maximum output current.
+curr[2*N+5]_max_alarm Output current high alarm.
power1_label "pin1"
power1_input Input power, derived from duty cycle and output current.
diff --git a/drivers/hwmon/pmbus/max16601.c b/drivers/hwmon/pmbus/max16601.c
index a960b86e72d2..efe6da3bc8d0 100644
--- a/drivers/hwmon/pmbus/max16601.c
+++ b/drivers/hwmon/pmbus/max16601.c
@@ -31,6 +31,7 @@
#include "pmbus.h"
+#define REG_DEFAULT_NUM_POP 0xc4
#define REG_SETPT_DVID 0xd1
#define DAC_10MV_MODE BIT(4)
#define REG_IOUT_AVG_PK 0xee
@@ -40,6 +41,8 @@
#define CORE_RAIL_INDICATOR BIT(7)
#define REG_PHASE_REPORTING 0xf4
+#define MAX16601_NUM_PHASES 8
+
struct max16601_data {
struct pmbus_driver_info info;
struct i2c_client *vsa;
@@ -195,6 +198,18 @@ static int max16601_identify(struct i2c_client *client,
else
info->vrm_version[0] = vr12;
+ reg = i2c_smbus_read_byte_data(client, REG_DEFAULT_NUM_POP);
+ if (reg < 0)
+ return reg;
+
+ /*
+ * If REG_DEFAULT_NUM_POP returns 0, we don't know how many phases
+ * are populated. Stick with the default in that case.
+ */
+ reg &= 0x0f;
+ if (reg && reg <= MAX16601_NUM_PHASES)
+ info->phases[0] = reg;
+
return 0;
}
@@ -216,7 +231,7 @@ static struct pmbus_driver_info max16601_info = {
.func[2] = PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT |
PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_PAGE_VIRTUAL,
- .phases[0] = 8,
+ .phases[0] = MAX16601_NUM_PHASES,
.pfunc[0] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP,
.pfunc[1] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT,
.pfunc[2] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP,
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] RFT: hwmon: (pmbus/max16601) Add support for MAX16508
2021-01-25 18:53 [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases Guenter Roeck
@ 2021-01-25 18:53 ` Guenter Roeck
2021-01-25 22:51 ` Alex Qiu
2021-01-25 22:49 ` [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases Alex Qiu
1 sibling, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2021-01-25 18:53 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Jean Delvare, Guenter Roeck, Alex Qiu, Ugur Usug
MAX16508 is quite similar to MAX16601, except that it does not support
the DEFAULT_NUM_POP register and we thus can not dynamically determine
the number of populated phases.
Cc: Alex Qiu <xqiu@google.com>
Cc: Ugur Usug <Ugur.Usug@maximintegrated.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Tested with MAX16601 to ensure that it doesn't break existing support,
but untested on MAX16508.
The most likely cause of failure is the chip detection string: The
datasheet suggests that it is "MAX16500" for all chips in the MAX165xx
series, but without hardware that is all but impossible to confirm.
It should also be confirmed that REG_DEFAULT_NUM_POP (the expected
number of populated phases) is indeed not supported on MAX16508.
Documentation/hwmon/max16601.rst | 12 +++++-
drivers/hwmon/pmbus/Kconfig | 4 +-
drivers/hwmon/pmbus/max16601.c | 74 +++++++++++++++++++++++---------
3 files changed, 66 insertions(+), 24 deletions(-)
diff --git a/Documentation/hwmon/max16601.rst b/Documentation/hwmon/max16601.rst
index 93d25dfa028e..d16792be7533 100644
--- a/Documentation/hwmon/max16601.rst
+++ b/Documentation/hwmon/max16601.rst
@@ -5,6 +5,14 @@ Kernel driver max16601
Supported chips:
+ * Maxim MAX16508
+
+ Prefix: 'max16508'
+
+ Addresses scanned: -
+
+ Datasheet: Not published
+
* Maxim MAX16601
Prefix: 'max16601'
@@ -19,8 +27,8 @@ Author: Guenter Roeck <linux@roeck-us.net>
Description
-----------
-This driver supports the MAX16601 VR13.HC Dual-Output Voltage Regulator
-Chipset.
+This driver supports the MAX16508 VR13 Dual-Output Voltage Regulator
+as well as the MAX16601 VR13.HC Dual-Output Voltage Regulator chipsets.
The driver is a client driver to the core PMBus driver.
Please see Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 03606d4298a4..32d2fc850621 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -158,10 +158,10 @@ config SENSORS_MAX16064
be called max16064.
config SENSORS_MAX16601
- tristate "Maxim MAX16601"
+ tristate "Maxim MAX16508, MAX16601"
help
If you say yes here you get hardware monitoring support for Maxim
- MAX16601.
+ MAX16508 and MAX16601.
This driver can also be built as a module. If so, the module will
be called max16601.
diff --git a/drivers/hwmon/pmbus/max16601.c b/drivers/hwmon/pmbus/max16601.c
index efe6da3bc8d0..0d1204c2dd54 100644
--- a/drivers/hwmon/pmbus/max16601.c
+++ b/drivers/hwmon/pmbus/max16601.c
@@ -1,11 +1,11 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Hardware monitoring driver for Maxim MAX16601
+ * Hardware monitoring driver for Maxim MAX16508 and MAX16601.
*
* Implementation notes:
*
- * Ths chip supports two rails, VCORE and VSA. Telemetry information for the
- * two rails is reported in two subsequent I2C addresses. The driver
+ * This chip series supports two rails, VCORE and VSA. Telemetry information
+ * for the two rails is reported in two subsequent I2C addresses. The driver
* instantiates a dummy I2C client at the second I2C address to report
* information for the VSA rail in a single instance of the driver.
* Telemetry for the VSA rail is reported to the PMBus core in PMBus page 2.
@@ -31,6 +31,8 @@
#include "pmbus.h"
+enum chips { max16508, max16601 };
+
#define REG_DEFAULT_NUM_POP 0xc4
#define REG_SETPT_DVID 0xd1
#define DAC_10MV_MODE BIT(4)
@@ -44,6 +46,7 @@
#define MAX16601_NUM_PHASES 8
struct max16601_data {
+ enum chips id;
struct pmbus_driver_info info;
struct i2c_client *vsa;
int iout_avg_pkg;
@@ -188,6 +191,7 @@ static int max16601_write_word(struct i2c_client *client, int page, int reg,
static int max16601_identify(struct i2c_client *client,
struct pmbus_driver_info *info)
{
+ struct max16601_data *data = to_max16601_data(info);
int reg;
reg = i2c_smbus_read_byte_data(client, REG_SETPT_DVID);
@@ -198,6 +202,9 @@ static int max16601_identify(struct i2c_client *client,
else
info->vrm_version[0] = vr12;
+ if (data->id != max16601)
+ return 0;
+
reg = i2c_smbus_read_byte_data(client, REG_DEFAULT_NUM_POP);
if (reg < 0)
return reg;
@@ -254,28 +261,61 @@ static void max16601_remove(void *_data)
i2c_unregister_device(data->vsa);
}
-static int max16601_probe(struct i2c_client *client)
+static const struct i2c_device_id max16601_id[] = {
+ {"max16508", max16508},
+ {"max16601", max16601},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, max16601_id);
+
+static int max16601_get_id(struct i2c_client *client)
{
struct device *dev = &client->dev;
u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
- struct max16601_data *data;
+ enum chips id;
int ret;
- if (!i2c_check_functionality(client->adapter,
- I2C_FUNC_SMBUS_READ_BYTE_DATA |
- I2C_FUNC_SMBUS_READ_BLOCK_DATA))
- return -ENODEV;
-
ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
- if (ret < 0)
+ if (ret < 0 || ret < 11)
return -ENODEV;
- /* PMBUS_IC_DEVICE_ID is expected to return "MAX16601y.xx" */
- if (ret < 11 || strncmp(buf, "MAX16601", 8)) {
+ /*
+ * PMBUS_IC_DEVICE_ID is expected to return "MAX16601y.xx"
+ * or "MAX16500y.xx".
+ */
+ if (!strncmp(buf, "MAX16500", 8)) {
+ id = max16508;
+ } else if (!strncmp(buf, "MAX16601", 8)) {
+ id = max16601;
+ } else {
buf[ret] = '\0';
dev_err(dev, "Unsupported chip '%s'\n", buf);
return -ENODEV;
}
+ return id;
+}
+
+static int max16601_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ const struct i2c_device_id *id;
+ struct max16601_data *data;
+ int ret, chip_id;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA |
+ I2C_FUNC_SMBUS_READ_BLOCK_DATA))
+ return -ENODEV;
+
+ chip_id = max16601_get_id(client);
+ if (chip_id < 0)
+ return chip_id;
+
+ id = i2c_match_id(max16601_id, client);
+ if (chip_id != id->driver_data)
+ dev_warn(&client->dev,
+ "Device mismatch: Configured %s (%d), detected %d\n",
+ id->name, (int) id->driver_data, chip_id);
ret = i2c_smbus_read_byte_data(client, REG_PHASE_ID);
if (ret < 0)
@@ -290,6 +330,7 @@ static int max16601_probe(struct i2c_client *client)
if (!data)
return -ENOMEM;
+ data->id = chip_id;
data->iout_avg_pkg = 0xfc00;
data->vsa = i2c_new_dummy_device(client->adapter, client->addr + 1);
if (IS_ERR(data->vsa)) {
@@ -305,13 +346,6 @@ static int max16601_probe(struct i2c_client *client)
return pmbus_do_probe(client, &data->info);
}
-static const struct i2c_device_id max16601_id[] = {
- {"max16601", 0},
- {}
-};
-
-MODULE_DEVICE_TABLE(i2c, max16601_id);
-
static struct i2c_driver max16601_driver = {
.driver = {
.name = "max16601",
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases
2021-01-25 18:53 [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases Guenter Roeck
2021-01-25 18:53 ` [PATCH 2/2] RFT: hwmon: (pmbus/max16601) Add support for MAX16508 Guenter Roeck
@ 2021-01-25 22:49 ` Alex Qiu
2021-01-28 2:03 ` Guenter Roeck
1 sibling, 1 reply; 6+ messages in thread
From: Alex Qiu @ 2021-01-25 22:49 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hardware Monitoring, Jean Delvare, Ugur Usug
Nit: Would it be better to specify what P and N mean upfront in the doc?
- Alex Qiu
On Mon, Jan 25, 2021 at 10:53 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> The MAX16601 can report the number of populated phases. Use this
> information to only create sysfs attributes for populated phases.
>
> Cc: Alex Qiu <xqiu@google.com>
> Cc: Ugur Usug <Ugur.Usug@maximintegrated.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Alex Qiu <xqiu@google.com>
> ---
> Documentation/hwmon/max16601.rst | 98 ++++++++++----------------------
> drivers/hwmon/pmbus/max16601.c | 17 +++++-
> 2 files changed, 45 insertions(+), 70 deletions(-)
>
> diff --git a/Documentation/hwmon/max16601.rst b/Documentation/hwmon/max16601.rst
> index 346e74674c51..93d25dfa028e 100644
> --- a/Documentation/hwmon/max16601.rst
> +++ b/Documentation/hwmon/max16601.rst
> @@ -60,75 +60,35 @@ curr1_input VCORE input current, derived from duty cycle and output
> curr1_max Maximum input current.
> curr1_max_alarm Current high alarm.
>
> -curr2_label "iin1.0"
> -curr2_input VCORE phase 0 input current.
> -
> -curr3_label "iin1.1"
> -curr3_input VCORE phase 1 input current.
> -
> -curr4_label "iin1.2"
> -curr4_input VCORE phase 2 input current.
> -
> -curr5_label "iin1.3"
> -curr5_input VCORE phase 3 input current.
> -
> -curr6_label "iin1.4"
> -curr6_input VCORE phase 4 input current.
> -
> -curr7_label "iin1.5"
> -curr7_input VCORE phase 5 input current.
> -
> -curr8_label "iin1.6"
> -curr8_input VCORE phase 6 input current.
> -
> -curr9_label "iin1.7"
> -curr9_input VCORE phase 7 input current.
> -
> -curr10_label "iin2"
> -curr10_input VCORE input current, derived from sensor element.
> -
> -curr11_label "iin3"
> -curr11_input VSA input current.
> -
> -curr12_label "iout1"
> -curr12_input VCORE output current.
> -curr12_crit Critical output current.
> -curr12_crit_alarm Output current critical alarm.
> -curr12_max Maximum output current.
> -curr12_max_alarm Output current high alarm.
> -
> -curr13_label "iout1.0"
> -curr13_input VCORE phase 0 output current.
> -
> -curr14_label "iout1.1"
> -curr14_input VCORE phase 1 output current.
> -
> -curr15_label "iout1.2"
> -curr15_input VCORE phase 2 output current.
> -
> -curr16_label "iout1.3"
> -curr16_input VCORE phase 3 output current.
> -
> -curr17_label "iout1.4"
> -curr17_input VCORE phase 4 output current.
> -
> -curr18_label "iout1.5"
> -curr18_input VCORE phase 5 output current.
> -
> -curr19_label "iout1.6"
> -curr19_input VCORE phase 6 output current.
> -
> -curr20_label "iout1.7"
> -curr20_input VCORE phase 7 output current.
> -
> -curr21_label "iout3"
> -curr21_input VSA output current.
> -curr21_highest Historical maximum VSA output current.
> -curr21_reset_history Write any value to reset curr21_highest.
> -curr21_crit Critical output current.
> -curr21_crit_alarm Output current critical alarm.
> -curr21_max Maximum output current.
> -curr21_max_alarm Output current high alarm.
> +curr[P+2]_label "iin1.P"
> +curr[P+2]_input VCORE phase P input current.
> +
> +curr[N+2]_label "iin2"
> +curr[N+2]_input VCORE input current, derived from sensor element.
> + 'N' is the number of enabled/populated phases.
> +
> +curr[N+3]_label "iin3"
> +curr[N+3]_input VSA input current.
> +
> +curr[N+4]_label "iout1"
> +curr[N+4]_input VCORE output current.
> +curr[N+4]_crit Critical output current.
> +curr[N+4]_crit_alarm Output current critical alarm.
> +curr[N+4]_max Maximum output current.
> +curr[N+4]_max_alarm Output current high alarm.
> +
> +curr[N+P+5]_label "iout1.P"
> +curr[N+P+5]_input VCORE phase P output current.
> +
> +curr[2*N+5]_label "iout3"
> +curr[2*N+5]_input VSA output current.
> +curr[2*N+5]_highest Historical maximum VSA output current.
> +curr[2*N+5]_reset_history
> + Write any value to reset curr21_highest.
> +curr[2*N+5]_crit Critical output current.
> +curr[2*N+5]_crit_alarm Output current critical alarm.
> +curr[2*N+5]_max Maximum output current.
> +curr[2*N+5]_max_alarm Output current high alarm.
>
> power1_label "pin1"
> power1_input Input power, derived from duty cycle and output current.
> diff --git a/drivers/hwmon/pmbus/max16601.c b/drivers/hwmon/pmbus/max16601.c
> index a960b86e72d2..efe6da3bc8d0 100644
> --- a/drivers/hwmon/pmbus/max16601.c
> +++ b/drivers/hwmon/pmbus/max16601.c
> @@ -31,6 +31,7 @@
>
> #include "pmbus.h"
>
> +#define REG_DEFAULT_NUM_POP 0xc4
> #define REG_SETPT_DVID 0xd1
> #define DAC_10MV_MODE BIT(4)
> #define REG_IOUT_AVG_PK 0xee
> @@ -40,6 +41,8 @@
> #define CORE_RAIL_INDICATOR BIT(7)
> #define REG_PHASE_REPORTING 0xf4
>
> +#define MAX16601_NUM_PHASES 8
> +
> struct max16601_data {
> struct pmbus_driver_info info;
> struct i2c_client *vsa;
> @@ -195,6 +198,18 @@ static int max16601_identify(struct i2c_client *client,
> else
> info->vrm_version[0] = vr12;
>
> + reg = i2c_smbus_read_byte_data(client, REG_DEFAULT_NUM_POP);
> + if (reg < 0)
> + return reg;
> +
> + /*
> + * If REG_DEFAULT_NUM_POP returns 0, we don't know how many phases
> + * are populated. Stick with the default in that case.
> + */
> + reg &= 0x0f;
> + if (reg && reg <= MAX16601_NUM_PHASES)
> + info->phases[0] = reg;
> +
> return 0;
> }
>
> @@ -216,7 +231,7 @@ static struct pmbus_driver_info max16601_info = {
> .func[2] = PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT |
> PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
> PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_PAGE_VIRTUAL,
> - .phases[0] = 8,
> + .phases[0] = MAX16601_NUM_PHASES,
> .pfunc[0] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP,
> .pfunc[1] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT,
> .pfunc[2] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP,
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] RFT: hwmon: (pmbus/max16601) Add support for MAX16508
2021-01-25 18:53 ` [PATCH 2/2] RFT: hwmon: (pmbus/max16601) Add support for MAX16508 Guenter Roeck
@ 2021-01-25 22:51 ` Alex Qiu
2021-01-28 1:59 ` Guenter Roeck
0 siblings, 1 reply; 6+ messages in thread
From: Alex Qiu @ 2021-01-25 22:51 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hardware Monitoring, Jean Delvare, Ugur Usug
Tested: No obvious hwmon directory entry or value reading changes
after applying the patch 2 on MAX16601. Had to work around the
pmbus_do_probe() in order to apply to the 5.1 kernel on my hand
though.
Thx!
- Alex Qiu
On Mon, Jan 25, 2021 at 10:53 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> MAX16508 is quite similar to MAX16601, except that it does not support
> the DEFAULT_NUM_POP register and we thus can not dynamically determine
> the number of populated phases.
>
> Cc: Alex Qiu <xqiu@google.com>
> Cc: Ugur Usug <Ugur.Usug@maximintegrated.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Alex Qiu <xqiu@google.com>
Tested-by: Alex Qiu <xqiu@google.com>
> ---
> Tested with MAX16601 to ensure that it doesn't break existing support,
> but untested on MAX16508.
>
> The most likely cause of failure is the chip detection string: The
> datasheet suggests that it is "MAX16500" for all chips in the MAX165xx
> series, but without hardware that is all but impossible to confirm.
>
> It should also be confirmed that REG_DEFAULT_NUM_POP (the expected
> number of populated phases) is indeed not supported on MAX16508.
>
> Documentation/hwmon/max16601.rst | 12 +++++-
> drivers/hwmon/pmbus/Kconfig | 4 +-
> drivers/hwmon/pmbus/max16601.c | 74 +++++++++++++++++++++++---------
> 3 files changed, 66 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/hwmon/max16601.rst b/Documentation/hwmon/max16601.rst
> index 93d25dfa028e..d16792be7533 100644
> --- a/Documentation/hwmon/max16601.rst
> +++ b/Documentation/hwmon/max16601.rst
> @@ -5,6 +5,14 @@ Kernel driver max16601
>
> Supported chips:
>
> + * Maxim MAX16508
> +
> + Prefix: 'max16508'
> +
> + Addresses scanned: -
> +
> + Datasheet: Not published
> +
> * Maxim MAX16601
>
> Prefix: 'max16601'
> @@ -19,8 +27,8 @@ Author: Guenter Roeck <linux@roeck-us.net>
> Description
> -----------
>
> -This driver supports the MAX16601 VR13.HC Dual-Output Voltage Regulator
> -Chipset.
> +This driver supports the MAX16508 VR13 Dual-Output Voltage Regulator
> +as well as the MAX16601 VR13.HC Dual-Output Voltage Regulator chipsets.
>
> The driver is a client driver to the core PMBus driver.
> Please see Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 03606d4298a4..32d2fc850621 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -158,10 +158,10 @@ config SENSORS_MAX16064
> be called max16064.
>
> config SENSORS_MAX16601
> - tristate "Maxim MAX16601"
> + tristate "Maxim MAX16508, MAX16601"
> help
> If you say yes here you get hardware monitoring support for Maxim
> - MAX16601.
> + MAX16508 and MAX16601.
>
> This driver can also be built as a module. If so, the module will
> be called max16601.
> diff --git a/drivers/hwmon/pmbus/max16601.c b/drivers/hwmon/pmbus/max16601.c
> index efe6da3bc8d0..0d1204c2dd54 100644
> --- a/drivers/hwmon/pmbus/max16601.c
> +++ b/drivers/hwmon/pmbus/max16601.c
> @@ -1,11 +1,11 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Hardware monitoring driver for Maxim MAX16601
> + * Hardware monitoring driver for Maxim MAX16508 and MAX16601.
> *
> * Implementation notes:
> *
> - * Ths chip supports two rails, VCORE and VSA. Telemetry information for the
> - * two rails is reported in two subsequent I2C addresses. The driver
> + * This chip series supports two rails, VCORE and VSA. Telemetry information
> + * for the two rails is reported in two subsequent I2C addresses. The driver
> * instantiates a dummy I2C client at the second I2C address to report
> * information for the VSA rail in a single instance of the driver.
> * Telemetry for the VSA rail is reported to the PMBus core in PMBus page 2.
> @@ -31,6 +31,8 @@
>
> #include "pmbus.h"
>
> +enum chips { max16508, max16601 };
> +
> #define REG_DEFAULT_NUM_POP 0xc4
> #define REG_SETPT_DVID 0xd1
> #define DAC_10MV_MODE BIT(4)
> @@ -44,6 +46,7 @@
> #define MAX16601_NUM_PHASES 8
>
> struct max16601_data {
> + enum chips id;
> struct pmbus_driver_info info;
> struct i2c_client *vsa;
> int iout_avg_pkg;
> @@ -188,6 +191,7 @@ static int max16601_write_word(struct i2c_client *client, int page, int reg,
> static int max16601_identify(struct i2c_client *client,
> struct pmbus_driver_info *info)
> {
> + struct max16601_data *data = to_max16601_data(info);
> int reg;
>
> reg = i2c_smbus_read_byte_data(client, REG_SETPT_DVID);
> @@ -198,6 +202,9 @@ static int max16601_identify(struct i2c_client *client,
> else
> info->vrm_version[0] = vr12;
>
> + if (data->id != max16601)
> + return 0;
> +
> reg = i2c_smbus_read_byte_data(client, REG_DEFAULT_NUM_POP);
> if (reg < 0)
> return reg;
> @@ -254,28 +261,61 @@ static void max16601_remove(void *_data)
> i2c_unregister_device(data->vsa);
> }
>
> -static int max16601_probe(struct i2c_client *client)
> +static const struct i2c_device_id max16601_id[] = {
> + {"max16508", max16508},
> + {"max16601", max16601},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, max16601_id);
> +
> +static int max16601_get_id(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
> - struct max16601_data *data;
> + enum chips id;
> int ret;
>
> - if (!i2c_check_functionality(client->adapter,
> - I2C_FUNC_SMBUS_READ_BYTE_DATA |
> - I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> - return -ENODEV;
> -
> ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
> - if (ret < 0)
> + if (ret < 0 || ret < 11)
> return -ENODEV;
>
> - /* PMBUS_IC_DEVICE_ID is expected to return "MAX16601y.xx" */
> - if (ret < 11 || strncmp(buf, "MAX16601", 8)) {
> + /*
> + * PMBUS_IC_DEVICE_ID is expected to return "MAX16601y.xx"
> + * or "MAX16500y.xx".
> + */
> + if (!strncmp(buf, "MAX16500", 8)) {
> + id = max16508;
> + } else if (!strncmp(buf, "MAX16601", 8)) {
> + id = max16601;
> + } else {
> buf[ret] = '\0';
> dev_err(dev, "Unsupported chip '%s'\n", buf);
> return -ENODEV;
> }
> + return id;
> +}
> +
> +static int max16601_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + const struct i2c_device_id *id;
> + struct max16601_data *data;
> + int ret, chip_id;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BYTE_DATA |
> + I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> + return -ENODEV;
> +
> + chip_id = max16601_get_id(client);
> + if (chip_id < 0)
> + return chip_id;
> +
> + id = i2c_match_id(max16601_id, client);
> + if (chip_id != id->driver_data)
> + dev_warn(&client->dev,
> + "Device mismatch: Configured %s (%d), detected %d\n",
> + id->name, (int) id->driver_data, chip_id);
>
> ret = i2c_smbus_read_byte_data(client, REG_PHASE_ID);
> if (ret < 0)
> @@ -290,6 +330,7 @@ static int max16601_probe(struct i2c_client *client)
> if (!data)
> return -ENOMEM;
>
> + data->id = chip_id;
> data->iout_avg_pkg = 0xfc00;
> data->vsa = i2c_new_dummy_device(client->adapter, client->addr + 1);
> if (IS_ERR(data->vsa)) {
> @@ -305,13 +346,6 @@ static int max16601_probe(struct i2c_client *client)
> return pmbus_do_probe(client, &data->info);
> }
>
> -static const struct i2c_device_id max16601_id[] = {
> - {"max16601", 0},
> - {}
> -};
> -
> -MODULE_DEVICE_TABLE(i2c, max16601_id);
> -
> static struct i2c_driver max16601_driver = {
> .driver = {
> .name = "max16601",
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] RFT: hwmon: (pmbus/max16601) Add support for MAX16508
2021-01-25 22:51 ` Alex Qiu
@ 2021-01-28 1:59 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2021-01-28 1:59 UTC (permalink / raw)
To: Alex Qiu; +Cc: Hardware Monitoring, Jean Delvare, Ugur Usug
On Mon, Jan 25, 2021 at 02:51:57PM -0800, Alex Qiu wrote:
> Tested: No obvious hwmon directory entry or value reading changes
> after applying the patch 2 on MAX16601. Had to work around the
> pmbus_do_probe() in order to apply to the 5.1 kernel on my hand
> though.
>
> Thx!
>
Thanks a lot for testing!
Guenter
> - Alex Qiu
>
> On Mon, Jan 25, 2021 at 10:53 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > MAX16508 is quite similar to MAX16601, except that it does not support
> > the DEFAULT_NUM_POP register and we thus can not dynamically determine
> > the number of populated phases.
> >
> > Cc: Alex Qiu <xqiu@google.com>
> > Cc: Ugur Usug <Ugur.Usug@maximintegrated.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Alex Qiu <xqiu@google.com>
> Tested-by: Alex Qiu <xqiu@google.com>
> > ---
> > Tested with MAX16601 to ensure that it doesn't break existing support,
> > but untested on MAX16508.
> >
> > The most likely cause of failure is the chip detection string: The
> > datasheet suggests that it is "MAX16500" for all chips in the MAX165xx
> > series, but without hardware that is all but impossible to confirm.
> >
> > It should also be confirmed that REG_DEFAULT_NUM_POP (the expected
> > number of populated phases) is indeed not supported on MAX16508.
> >
> > Documentation/hwmon/max16601.rst | 12 +++++-
> > drivers/hwmon/pmbus/Kconfig | 4 +-
> > drivers/hwmon/pmbus/max16601.c | 74 +++++++++++++++++++++++---------
> > 3 files changed, 66 insertions(+), 24 deletions(-)
> >
> > diff --git a/Documentation/hwmon/max16601.rst b/Documentation/hwmon/max16601.rst
> > index 93d25dfa028e..d16792be7533 100644
> > --- a/Documentation/hwmon/max16601.rst
> > +++ b/Documentation/hwmon/max16601.rst
> > @@ -5,6 +5,14 @@ Kernel driver max16601
> >
> > Supported chips:
> >
> > + * Maxim MAX16508
> > +
> > + Prefix: 'max16508'
> > +
> > + Addresses scanned: -
> > +
> > + Datasheet: Not published
> > +
> > * Maxim MAX16601
> >
> > Prefix: 'max16601'
> > @@ -19,8 +27,8 @@ Author: Guenter Roeck <linux@roeck-us.net>
> > Description
> > -----------
> >
> > -This driver supports the MAX16601 VR13.HC Dual-Output Voltage Regulator
> > -Chipset.
> > +This driver supports the MAX16508 VR13 Dual-Output Voltage Regulator
> > +as well as the MAX16601 VR13.HC Dual-Output Voltage Regulator chipsets.
> >
> > The driver is a client driver to the core PMBus driver.
> > Please see Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
> > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> > index 03606d4298a4..32d2fc850621 100644
> > --- a/drivers/hwmon/pmbus/Kconfig
> > +++ b/drivers/hwmon/pmbus/Kconfig
> > @@ -158,10 +158,10 @@ config SENSORS_MAX16064
> > be called max16064.
> >
> > config SENSORS_MAX16601
> > - tristate "Maxim MAX16601"
> > + tristate "Maxim MAX16508, MAX16601"
> > help
> > If you say yes here you get hardware monitoring support for Maxim
> > - MAX16601.
> > + MAX16508 and MAX16601.
> >
> > This driver can also be built as a module. If so, the module will
> > be called max16601.
> > diff --git a/drivers/hwmon/pmbus/max16601.c b/drivers/hwmon/pmbus/max16601.c
> > index efe6da3bc8d0..0d1204c2dd54 100644
> > --- a/drivers/hwmon/pmbus/max16601.c
> > +++ b/drivers/hwmon/pmbus/max16601.c
> > @@ -1,11 +1,11 @@
> > // SPDX-License-Identifier: GPL-2.0
> > /*
> > - * Hardware monitoring driver for Maxim MAX16601
> > + * Hardware monitoring driver for Maxim MAX16508 and MAX16601.
> > *
> > * Implementation notes:
> > *
> > - * Ths chip supports two rails, VCORE and VSA. Telemetry information for the
> > - * two rails is reported in two subsequent I2C addresses. The driver
> > + * This chip series supports two rails, VCORE and VSA. Telemetry information
> > + * for the two rails is reported in two subsequent I2C addresses. The driver
> > * instantiates a dummy I2C client at the second I2C address to report
> > * information for the VSA rail in a single instance of the driver.
> > * Telemetry for the VSA rail is reported to the PMBus core in PMBus page 2.
> > @@ -31,6 +31,8 @@
> >
> > #include "pmbus.h"
> >
> > +enum chips { max16508, max16601 };
> > +
> > #define REG_DEFAULT_NUM_POP 0xc4
> > #define REG_SETPT_DVID 0xd1
> > #define DAC_10MV_MODE BIT(4)
> > @@ -44,6 +46,7 @@
> > #define MAX16601_NUM_PHASES 8
> >
> > struct max16601_data {
> > + enum chips id;
> > struct pmbus_driver_info info;
> > struct i2c_client *vsa;
> > int iout_avg_pkg;
> > @@ -188,6 +191,7 @@ static int max16601_write_word(struct i2c_client *client, int page, int reg,
> > static int max16601_identify(struct i2c_client *client,
> > struct pmbus_driver_info *info)
> > {
> > + struct max16601_data *data = to_max16601_data(info);
> > int reg;
> >
> > reg = i2c_smbus_read_byte_data(client, REG_SETPT_DVID);
> > @@ -198,6 +202,9 @@ static int max16601_identify(struct i2c_client *client,
> > else
> > info->vrm_version[0] = vr12;
> >
> > + if (data->id != max16601)
> > + return 0;
> > +
> > reg = i2c_smbus_read_byte_data(client, REG_DEFAULT_NUM_POP);
> > if (reg < 0)
> > return reg;
> > @@ -254,28 +261,61 @@ static void max16601_remove(void *_data)
> > i2c_unregister_device(data->vsa);
> > }
> >
> > -static int max16601_probe(struct i2c_client *client)
> > +static const struct i2c_device_id max16601_id[] = {
> > + {"max16508", max16508},
> > + {"max16601", max16601},
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, max16601_id);
> > +
> > +static int max16601_get_id(struct i2c_client *client)
> > {
> > struct device *dev = &client->dev;
> > u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
> > - struct max16601_data *data;
> > + enum chips id;
> > int ret;
> >
> > - if (!i2c_check_functionality(client->adapter,
> > - I2C_FUNC_SMBUS_READ_BYTE_DATA |
> > - I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> > - return -ENODEV;
> > -
> > ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
> > - if (ret < 0)
> > + if (ret < 0 || ret < 11)
> > return -ENODEV;
> >
> > - /* PMBUS_IC_DEVICE_ID is expected to return "MAX16601y.xx" */
> > - if (ret < 11 || strncmp(buf, "MAX16601", 8)) {
> > + /*
> > + * PMBUS_IC_DEVICE_ID is expected to return "MAX16601y.xx"
> > + * or "MAX16500y.xx".
> > + */
> > + if (!strncmp(buf, "MAX16500", 8)) {
> > + id = max16508;
> > + } else if (!strncmp(buf, "MAX16601", 8)) {
> > + id = max16601;
> > + } else {
> > buf[ret] = '\0';
> > dev_err(dev, "Unsupported chip '%s'\n", buf);
> > return -ENODEV;
> > }
> > + return id;
> > +}
> > +
> > +static int max16601_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + const struct i2c_device_id *id;
> > + struct max16601_data *data;
> > + int ret, chip_id;
> > +
> > + if (!i2c_check_functionality(client->adapter,
> > + I2C_FUNC_SMBUS_READ_BYTE_DATA |
> > + I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> > + return -ENODEV;
> > +
> > + chip_id = max16601_get_id(client);
> > + if (chip_id < 0)
> > + return chip_id;
> > +
> > + id = i2c_match_id(max16601_id, client);
> > + if (chip_id != id->driver_data)
> > + dev_warn(&client->dev,
> > + "Device mismatch: Configured %s (%d), detected %d\n",
> > + id->name, (int) id->driver_data, chip_id);
> >
> > ret = i2c_smbus_read_byte_data(client, REG_PHASE_ID);
> > if (ret < 0)
> > @@ -290,6 +330,7 @@ static int max16601_probe(struct i2c_client *client)
> > if (!data)
> > return -ENOMEM;
> >
> > + data->id = chip_id;
> > data->iout_avg_pkg = 0xfc00;
> > data->vsa = i2c_new_dummy_device(client->adapter, client->addr + 1);
> > if (IS_ERR(data->vsa)) {
> > @@ -305,13 +346,6 @@ static int max16601_probe(struct i2c_client *client)
> > return pmbus_do_probe(client, &data->info);
> > }
> >
> > -static const struct i2c_device_id max16601_id[] = {
> > - {"max16601", 0},
> > - {}
> > -};
> > -
> > -MODULE_DEVICE_TABLE(i2c, max16601_id);
> > -
> > static struct i2c_driver max16601_driver = {
> > .driver = {
> > .name = "max16601",
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases
2021-01-25 22:49 ` [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases Alex Qiu
@ 2021-01-28 2:03 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2021-01-28 2:03 UTC (permalink / raw)
To: Alex Qiu; +Cc: Hardware Monitoring, Jean Delvare, Ugur Usug
On Mon, Jan 25, 2021 at 02:49:35PM -0800, Alex Qiu wrote:
> Nit: Would it be better to specify what P and N mean upfront in the doc?
>
I thought about it, but figured that the inline definition
is more useful. But of course that is a matter of PoV.
Let's see if people complain ...
Thanks a lot for the review!
Guenter
> - Alex Qiu
>
> On Mon, Jan 25, 2021 at 10:53 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > The MAX16601 can report the number of populated phases. Use this
> > information to only create sysfs attributes for populated phases.
> >
> > Cc: Alex Qiu <xqiu@google.com>
> > Cc: Ugur Usug <Ugur.Usug@maximintegrated.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Alex Qiu <xqiu@google.com>
> > ---
> > Documentation/hwmon/max16601.rst | 98 ++++++++++----------------------
> > drivers/hwmon/pmbus/max16601.c | 17 +++++-
> > 2 files changed, 45 insertions(+), 70 deletions(-)
> >
> > diff --git a/Documentation/hwmon/max16601.rst b/Documentation/hwmon/max16601.rst
> > index 346e74674c51..93d25dfa028e 100644
> > --- a/Documentation/hwmon/max16601.rst
> > +++ b/Documentation/hwmon/max16601.rst
> > @@ -60,75 +60,35 @@ curr1_input VCORE input current, derived from duty cycle and output
> > curr1_max Maximum input current.
> > curr1_max_alarm Current high alarm.
> >
> > -curr2_label "iin1.0"
> > -curr2_input VCORE phase 0 input current.
> > -
> > -curr3_label "iin1.1"
> > -curr3_input VCORE phase 1 input current.
> > -
> > -curr4_label "iin1.2"
> > -curr4_input VCORE phase 2 input current.
> > -
> > -curr5_label "iin1.3"
> > -curr5_input VCORE phase 3 input current.
> > -
> > -curr6_label "iin1.4"
> > -curr6_input VCORE phase 4 input current.
> > -
> > -curr7_label "iin1.5"
> > -curr7_input VCORE phase 5 input current.
> > -
> > -curr8_label "iin1.6"
> > -curr8_input VCORE phase 6 input current.
> > -
> > -curr9_label "iin1.7"
> > -curr9_input VCORE phase 7 input current.
> > -
> > -curr10_label "iin2"
> > -curr10_input VCORE input current, derived from sensor element.
> > -
> > -curr11_label "iin3"
> > -curr11_input VSA input current.
> > -
> > -curr12_label "iout1"
> > -curr12_input VCORE output current.
> > -curr12_crit Critical output current.
> > -curr12_crit_alarm Output current critical alarm.
> > -curr12_max Maximum output current.
> > -curr12_max_alarm Output current high alarm.
> > -
> > -curr13_label "iout1.0"
> > -curr13_input VCORE phase 0 output current.
> > -
> > -curr14_label "iout1.1"
> > -curr14_input VCORE phase 1 output current.
> > -
> > -curr15_label "iout1.2"
> > -curr15_input VCORE phase 2 output current.
> > -
> > -curr16_label "iout1.3"
> > -curr16_input VCORE phase 3 output current.
> > -
> > -curr17_label "iout1.4"
> > -curr17_input VCORE phase 4 output current.
> > -
> > -curr18_label "iout1.5"
> > -curr18_input VCORE phase 5 output current.
> > -
> > -curr19_label "iout1.6"
> > -curr19_input VCORE phase 6 output current.
> > -
> > -curr20_label "iout1.7"
> > -curr20_input VCORE phase 7 output current.
> > -
> > -curr21_label "iout3"
> > -curr21_input VSA output current.
> > -curr21_highest Historical maximum VSA output current.
> > -curr21_reset_history Write any value to reset curr21_highest.
> > -curr21_crit Critical output current.
> > -curr21_crit_alarm Output current critical alarm.
> > -curr21_max Maximum output current.
> > -curr21_max_alarm Output current high alarm.
> > +curr[P+2]_label "iin1.P"
> > +curr[P+2]_input VCORE phase P input current.
> > +
> > +curr[N+2]_label "iin2"
> > +curr[N+2]_input VCORE input current, derived from sensor element.
> > + 'N' is the number of enabled/populated phases.
> > +
> > +curr[N+3]_label "iin3"
> > +curr[N+3]_input VSA input current.
> > +
> > +curr[N+4]_label "iout1"
> > +curr[N+4]_input VCORE output current.
> > +curr[N+4]_crit Critical output current.
> > +curr[N+4]_crit_alarm Output current critical alarm.
> > +curr[N+4]_max Maximum output current.
> > +curr[N+4]_max_alarm Output current high alarm.
> > +
> > +curr[N+P+5]_label "iout1.P"
> > +curr[N+P+5]_input VCORE phase P output current.
> > +
> > +curr[2*N+5]_label "iout3"
> > +curr[2*N+5]_input VSA output current.
> > +curr[2*N+5]_highest Historical maximum VSA output current.
> > +curr[2*N+5]_reset_history
> > + Write any value to reset curr21_highest.
> > +curr[2*N+5]_crit Critical output current.
> > +curr[2*N+5]_crit_alarm Output current critical alarm.
> > +curr[2*N+5]_max Maximum output current.
> > +curr[2*N+5]_max_alarm Output current high alarm.
> >
> > power1_label "pin1"
> > power1_input Input power, derived from duty cycle and output current.
> > diff --git a/drivers/hwmon/pmbus/max16601.c b/drivers/hwmon/pmbus/max16601.c
> > index a960b86e72d2..efe6da3bc8d0 100644
> > --- a/drivers/hwmon/pmbus/max16601.c
> > +++ b/drivers/hwmon/pmbus/max16601.c
> > @@ -31,6 +31,7 @@
> >
> > #include "pmbus.h"
> >
> > +#define REG_DEFAULT_NUM_POP 0xc4
> > #define REG_SETPT_DVID 0xd1
> > #define DAC_10MV_MODE BIT(4)
> > #define REG_IOUT_AVG_PK 0xee
> > @@ -40,6 +41,8 @@
> > #define CORE_RAIL_INDICATOR BIT(7)
> > #define REG_PHASE_REPORTING 0xf4
> >
> > +#define MAX16601_NUM_PHASES 8
> > +
> > struct max16601_data {
> > struct pmbus_driver_info info;
> > struct i2c_client *vsa;
> > @@ -195,6 +198,18 @@ static int max16601_identify(struct i2c_client *client,
> > else
> > info->vrm_version[0] = vr12;
> >
> > + reg = i2c_smbus_read_byte_data(client, REG_DEFAULT_NUM_POP);
> > + if (reg < 0)
> > + return reg;
> > +
> > + /*
> > + * If REG_DEFAULT_NUM_POP returns 0, we don't know how many phases
> > + * are populated. Stick with the default in that case.
> > + */
> > + reg &= 0x0f;
> > + if (reg && reg <= MAX16601_NUM_PHASES)
> > + info->phases[0] = reg;
> > +
> > return 0;
> > }
> >
> > @@ -216,7 +231,7 @@ static struct pmbus_driver_info max16601_info = {
> > .func[2] = PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT |
> > PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
> > PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_PAGE_VIRTUAL,
> > - .phases[0] = 8,
> > + .phases[0] = MAX16601_NUM_PHASES,
> > .pfunc[0] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP,
> > .pfunc[1] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT,
> > .pfunc[2] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP,
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-28 2:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-25 18:53 [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases Guenter Roeck
2021-01-25 18:53 ` [PATCH 2/2] RFT: hwmon: (pmbus/max16601) Add support for MAX16508 Guenter Roeck
2021-01-25 22:51 ` Alex Qiu
2021-01-28 1:59 ` Guenter Roeck
2021-01-25 22:49 ` [PATCH 1/2] hwmon: (pmbus/max16601) Determine and use number of populated phases Alex Qiu
2021-01-28 2:03 ` Guenter Roeck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.