| Message ID | 20210811154232.12649-7-eajames@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Rainier and Everest system fixes | expand |
On 8/11/21 10:42 AM, Eddie James wrote: > From: Andrew Jeffery <andrew@aj.id.au> > > Extensive testing and tracing has shown that the MAX31785 is unreliable > in the face of PAGE write commands, ACK'ing the PAGE request but > reporting a value of 0 on some subsequent PAGE reads. The trace data > suggests that a one-shot retry of the PAGE write is enough to get the > requested value to stick. > > As we configure the device before registering with the PMBus core, > centralise PAGE handling inside the driver and implement the one-shot > retry semantics there. > > OpenBMC-Staging-Count: 1 > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > Signed-off-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Matthew Barth <msbarth@linux.ibm.com> > --- > drivers/hwmon/pmbus/max31785.c | 32 ++++++++++++++++++++++++++------ > 1 file changed, 26 insertions(+), 6 deletions(-) > > diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c > index d097f72d4d47..7518fff356f9 100644 > --- a/drivers/hwmon/pmbus/max31785.c > +++ b/drivers/hwmon/pmbus/max31785.c > @@ -362,6 +362,27 @@ static int max31785_write_word_data(struct i2c_client *client, int page, > return -ENXIO; > } > > +static int max31785_pmbus_set_page(struct i2c_client *client, int page) > +{ > + int ret; > + int i; > + > + for (i = 0; i < 2; i++) { > + ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page); > + if (ret < 0) > + return ret; > + > + ret = max31785_i2c_smbus_read_byte_data(client, PMBUS_PAGE); > + if (ret < 0) > + return ret; > + > + if (ret == page) > + return 0; > + } > + > + return -EIO; > +} > + > /* > * Returns negative error codes if an unrecoverable problem is detected, 0 if a > * recoverable problem is detected, or a positive value on success. > @@ -392,7 +413,7 @@ static int max31785_of_fan_config(struct i2c_client *client, > return -ENXIO; > } > > - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page); > + ret = max31785_pmbus_set_page(client, page); > if (ret < 0) > return ret; > > @@ -599,7 +620,7 @@ static int max31785_of_tmp_config(struct i2c_client *client, > return -ENXIO; > } > > - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page); > + ret = max31785_pmbus_set_page(client, page); > if (ret < 0) > return ret; > > @@ -700,7 +721,7 @@ static int max31785_configure_dual_tach(struct i2c_client *client, > int i; > > for (i = 0; i < MAX31785_NR_FAN_PAGES; i++) { > - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, i); > + ret = max31785_pmbus_set_page(client, i); > if (ret < 0) > return ret; > > @@ -741,7 +762,7 @@ static int max31785_probe(struct i2c_client *client) > > *info = max31785_info; > > - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, 255); > + ret = max31785_pmbus_set_page(client, 255); > if (ret < 0) > return ret; > > @@ -785,8 +806,7 @@ static int max31785_probe(struct i2c_client *client) > if (!have_fan || fan_configured) > continue; > > - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, > - i); > + ret = max31785_pmbus_set_page(client, i); > if (ret < 0) > return ret; >
diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c index d097f72d4d47..7518fff356f9 100644 --- a/drivers/hwmon/pmbus/max31785.c +++ b/drivers/hwmon/pmbus/max31785.c @@ -362,6 +362,27 @@ static int max31785_write_word_data(struct i2c_client *client, int page, return -ENXIO; } +static int max31785_pmbus_set_page(struct i2c_client *client, int page) +{ + int ret; + int i; + + for (i = 0; i < 2; i++) { + ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page); + if (ret < 0) + return ret; + + ret = max31785_i2c_smbus_read_byte_data(client, PMBUS_PAGE); + if (ret < 0) + return ret; + + if (ret == page) + return 0; + } + + return -EIO; +} + /* * Returns negative error codes if an unrecoverable problem is detected, 0 if a * recoverable problem is detected, or a positive value on success. @@ -392,7 +413,7 @@ static int max31785_of_fan_config(struct i2c_client *client, return -ENXIO; } - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page); + ret = max31785_pmbus_set_page(client, page); if (ret < 0) return ret; @@ -599,7 +620,7 @@ static int max31785_of_tmp_config(struct i2c_client *client, return -ENXIO; } - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page); + ret = max31785_pmbus_set_page(client, page); if (ret < 0) return ret; @@ -700,7 +721,7 @@ static int max31785_configure_dual_tach(struct i2c_client *client, int i; for (i = 0; i < MAX31785_NR_FAN_PAGES; i++) { - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, i); + ret = max31785_pmbus_set_page(client, i); if (ret < 0) return ret; @@ -741,7 +762,7 @@ static int max31785_probe(struct i2c_client *client) *info = max31785_info; - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, 255); + ret = max31785_pmbus_set_page(client, 255); if (ret < 0) return ret; @@ -785,8 +806,7 @@ static int max31785_probe(struct i2c_client *client) if (!have_fan || fan_configured) continue; - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, - i); + ret = max31785_pmbus_set_page(client, i); if (ret < 0) return ret;