1. Patchew
  2. linux
  3. View series

[PATCH v2] hwmon: max34451: Work around lost page

William A. Kennington III posted 1 patch 1 month ago 
drivers/hwmon/pmbus/max34440.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
[PATCH v2] hwmon: max34451: Work around lost page
Posted by William A. Kennington III 1 month ago 
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 <william@wkennington.com>
---
V1 -> V2: Make all page changes delay the required 50us

 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 c9dda33831ff..0d9cb39a9cc6 100644
--- a/drivers/hwmon/pmbus/max34440.c
+++ b/drivers/hwmon/pmbus/max34440.c
@@ -12,10 +12,19 @@
 #include <linux/init.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
+#include <linux/delay.h>
 #include "pmbus.h"
 
 enum chips { max34440, max34441, max34446, max34451, max34460, 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
@@ -238,6 +247,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;
 
@@ -312,6 +322,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,
@@ -355,6 +366,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,
@@ -392,6 +404,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,
@@ -415,6 +428,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,
@@ -445,6 +459,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,
@@ -480,6 +495,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.49.0.504.g3bcea36a83-goog
Re: [PATCH v2] hwmon: max34451: Work around lost page
Posted by Guenter Roeck 1 month ago 
On Mon, Apr 07, 2025 at 06:10:06PM -0700, William A. Kennington III wrote:
> 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 <william@wkennington.com>

Applied to hwmon-next.

Alexis: Do you happen to know if the new revision of MAX34451 still
has this problem ? Also, it would be nice to get some feedback if
the patch covers all affected devices.

Thanks,
Guenter

> ---
> V1 -> V2: Make all page changes delay the required 50us
> 
>  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 c9dda33831ff..0d9cb39a9cc6 100644
> --- a/drivers/hwmon/pmbus/max34440.c
> +++ b/drivers/hwmon/pmbus/max34440.c
> @@ -12,10 +12,19 @@
>  #include <linux/init.h>
>  #include <linux/err.h>
>  #include <linux/i2c.h>
> +#include <linux/delay.h>
>  #include "pmbus.h"
>  
>  enum chips { max34440, max34441, max34446, max34451, max34460, 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
> @@ -238,6 +247,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;
>  
> @@ -312,6 +322,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,
> @@ -355,6 +366,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,
> @@ -392,6 +404,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,
> @@ -415,6 +428,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,
> @@ -445,6 +459,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,
> @@ -480,6 +495,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,
>  	},
>  };
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.