[PATCH] hwmon: max34451: Workaround for lost page

[Guenter Roeck]

On 4/1/25 15:55, William Kennington wrote:
> On Tue, Apr 1, 2025 at 3:52 PM Guenter Roeck <linux at roeck-us.net> wrote:
>>
>> On 4/1/25 15:08, William A. Kennington III wrote:
>>> When requesting new pages from the max34451 we sometimes see that the
>>> firmware doesn't update the page on the max34451 side fast enough. This
>>> results in the kernel receiving data for a different page than what it
>>> expects.
>>>
>>> To remedy this, the manufacturer recommends we wait 50-100us until
>>> the firmware should be ready with the new page.
>>>
>>> Signed-off-by: William A. Kennington III <william at wkennington.com>
>>> ---
>>>    drivers/hwmon/pmbus/max34440.c | 7 +++++++
>>>    1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c
>>> index c9dda33831ff..ac3a26f7cff3 100644
>>> --- a/drivers/hwmon/pmbus/max34440.c
>>> +++ b/drivers/hwmon/pmbus/max34440.c
>>> @@ -12,6 +12,7 @@
>>>    #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 };
>>> @@ -241,6 +242,12 @@ static int max34451_set_supported_funcs(struct i2c_client *client,
>>>                if (rv < 0)
>>>                        return rv;
>>>
>>> +             /* Firmware is sometimes not ready if we try and read the
>>
>> This is not the networking subsystem. Standard multi-line comments, please.
> 
> Okay, let me fix that.
> 
>>
>>> +              * data from the page immediately after setting. Maxim
>>> +              * recommends 50-100us delay.
>>> +              */
>>> +             fsleep(50);
>>
>> I would suggest to wait 100uS to be safe. The function is only called during probe,
>> so that should be ok.
> 
> Yeah, I don't think they did strenuous measurement of these values on
> their end. We have been using this patch for 4-5 years now with
> seemingly good robustness on the 50us value. I just pulled up an old
> email from the vendor that gives this context.
> 
>>
>> Is this a generic problem with this chip when changing pages ?
> 
> I believe that is the case, but this patch is pretty old at this
> point. Is there somewhere to add in quirks for such chips that would
> allow us to build in such a delay?
> 

So far we only have delays for all accesses and for write operations.
See access_delay and write_delay in struct pmbus_data. If the problem
only affects page changes, we might have to add page_change_delay or
something similar. Alternatively, maybe we could just set write_delay.
If the chip has trouble with page changes, it might well be that it has
trouble with write operations in general.

Thanks,
Guenter