| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<PH0PR03MB63516D1BCA80A50B0C3E029EF1B72@PH0PR03MB6351.namprd03.prod.outlook.com>
Date: Thu, 10 Apr 2025 23:59:38 +0000
From: "Torreno, Alexis Czezar" <AlexisCzezar.Torreno@...log.com>
To: Guenter Roeck <linux@...ck-us.net>,
"William A. Kennington III"
<william@...nnington.com>
CC: Jean Delvare <jdelvare@...e.com>,
"linux-hwmon@...r.kernel.org"
<linux-hwmon@...r.kernel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] hwmon: max34451: Work around lost page
> > [External]
> >
> > 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@...nnington.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.
> >
>
> I unfortunately am not aware of this. Let me see if can get an answer from the
> engr of the part.
To follow up, here's the exact response I got.
- This is not an issue with the part. As the fix mentions, "the manufacturer
recommends we wait 50us until the firmware should be ready with the new page".
Compliant with I2C/SMBus spec, some parts require a longer idle time for the
bus to be free before receiving the next command, in this situation it is normal to
recommend a longer idle time in between commands. It's more related to the
part's inner timing and architecture.
So I guess no specific changes related to this was made for later versions so the
patch here should cover all.
Thanks,
>
> >
> > > ---
> > > 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,
> > > },
> > > };
> > >
Powered by blists - more mailing lists