<span style=" font-size:10pt;font-family:sans-serif">Test and verified
working.</span><br><br><span style=" font-size:10pt;font-family:sans-serif">Tested-by: George
Keishing <gkeishin@in.ibm.com></span><br><br><span style=" font-size:10pt;color:blue;font-family:sans-serif"><b>Thanks
and Regards,</b></span><br><span style=" font-size:10pt;color:blue;font-family:sans-serif">
George Keishing</span><br><span style=" font-size:10pt;color:blue;font-family:sans-serif">
IBM Systems &Technology Lab, Firmware Development,</span><br><span style=" font-size:10pt;font-family:sans-serif"><i>“</i></span><span style=" font-size:12pt">There isn't enough time in a day to be lazy!!! </span><span style=" font-size:9pt;font-family:sans-serif">.”</span><br><img src=cid:_2_0AFFF6780AFFF40C001609F465258265 style="border:0px solid;"><br><br><br><br><br><span style=" font-size:9pt;color:#5f5f5f;font-family:sans-serif">From:
</span><span style=" font-size:9pt;font-family:sans-serif">Andrew
Jeffery <andrew@aj.id.au></span><br><span style=" font-size:9pt;color:#5f5f5f;font-family:sans-serif">To:
</span><span style=" font-size:9pt;font-family:sans-serif">gkeishin@in.ibm.com</span><br><span style=" font-size:9pt;color:#5f5f5f;font-family:sans-serif">Date:
</span><span style=" font-size:9pt;font-family:sans-serif">04/04/2018
09:24 AM</span><br><span style=" font-size:9pt;color:#5f5f5f;font-family:sans-serif">Subject:
</span><span style=" font-size:9pt;font-family:sans-serif">Fwd:
[PATCH linux dev-4.13 5/5] pmbus (max31785): Wrap all I2C accessors in
one-shot failure handlers</span><br><hr noshade><br><br><br><tt><span style=" font-size:10pt"><br><br>----- Original message -----<br>From: Andrew Jeffery <andrew@aj.id.au><br>To: joel@jms.id.au<br>Cc: Andrew Jeffery <andrew@aj.id.au>, openbmc@lists.ozlabs.org<br>Subject: [PATCH linux dev-4.13 5/5] pmbus (max31785): Wrap all I2C accessors
in one-shot failure handlers<br>Date: Tue, 3 Apr 2018 23:56:55 +0930<br><br>The MAX31785(A) has shown erratic behaviour across multiple system<br>designs, unexpectedly clock stretching and NAKing transactions. Perform<br>a one-shot retry if necessary for all access attempts.<br><br>Signed-off-by: Andrew Jeffery <andrew@aj.id.au><br>---<br> drivers/hwmon/pmbus/max31785.c | 207 ++++++++++++++++++++++++++++++++---------<br> 1 file changed, 165 insertions(+), 42 deletions(-)<br><br>diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c<br>index 9a7e745b6b31..3fb111c6d528 100644<br>--- a/drivers/hwmon/pmbus/max31785.c<br>+++ b/drivers/hwmon/pmbus/max31785.c<br>@@ -37,29 +37,105 @@ enum max31785_regs {<br> #define MAX31785_NR_PAGES
23<br> #define MAX31785_NR_FAN_PAGES
6<br> <br>+/*<br>+ * MAX31785 dragons ahead<br>+ *<br>+ * We see weird issues where some transfers fail. There doesn't appear
to be<br>+ * any pattern to the problem, so below we wrap all the read/write calls
with a<br>+ * retry. The device provides no indication of this besides NACK'ing master<br>+ * Txs; no bits are set in STATUS_BYTE to suggest anything has gone wrong.<br>+ */<br>+<br>+#define max31785_retry(_func, ...) ({
\<br>+
/* All relevant functions return int, sue me */
\<br>+
int _ret = _func(__VA_ARGS__);
\<br>+
if (_ret == -EIO)
\<br>+
_ret
= _func(__VA_ARGS__);
\<br>+
_ret;
\<br>+})<br>+<br>+static int max31785_i2c_smbus_read_byte_data(struct i2c_client *client,<br>+
int command)<br>+{<br>+
return max31785_retry(i2c_smbus_read_byte_data, client, command);<br>+}<br>+<br>+<br>+static int max31785_i2c_smbus_write_byte_data(struct i2c_client *client,<br>+
int command, u16 data)<br>+{<br>+
return max31785_retry(i2c_smbus_write_byte_data, client, command, data);<br>+}<br>+<br>+static int max31785_i2c_smbus_read_word_data(struct i2c_client *client,<br>+
int command)<br>+{<br>+
return max31785_retry(i2c_smbus_read_word_data, client, command);<br>+}<br>+<br>+static int max31785_i2c_smbus_write_word_data(struct i2c_client *client,<br>+
int command, u16 data)<br>+{<br>+
return max31785_retry(i2c_smbus_write_word_data, client, command, data);<br>+}<br>+<br>+static int max31785_pmbus_write_byte(struct i2c_client *client, int page,<br>+
u8 value)<br>+{<br>+
return max31785_retry(pmbus_write_byte, client, page, value);<br>+}<br>+<br>+static int max31785_pmbus_read_byte_data(struct i2c_client *client, int
page,<br>+
int command)<br>+{<br>+
return max31785_retry(pmbus_read_byte_data, client, page, command);<br>+}<br>+<br>+static int max31785_pmbus_write_byte_data(struct i2c_client *client, int
page,<br>+
int command, u16 data)<br>+{<br>+
return max31785_retry(pmbus_write_byte_data, client, page, command,<br>+
data);<br>+}<br>+<br>+static int max31785_pmbus_read_word_data(struct i2c_client *client, int
page,<br>+
int command)<br>+{<br>+
return max31785_retry(pmbus_read_word_data, client, page, command);<br>+}<br>+<br>+static int max31785_pmbus_write_word_data(struct i2c_client *client, int
page,<br>+
int command, u16 data)<br>+{<br>+
return max31785_retry(pmbus_write_word_data, client, page, command,<br>+
data);<br>+}<br>+<br> static int max31785_read_byte_data(struct i2c_client *client, int page,<br>
int reg)<br> {<br>-
if (page < MAX31785_NR_PAGES)<br>-
return
-ENODATA;<br>-<br> switch
(reg) {<br> case
PMBUS_VOUT_MODE:<br>-
return
-ENOTSUPP;<br>+
if
(page >= MAX31785_NR_PAGES)<br>+
return -ENOTSUPP;<br>+
break;<br> case
PMBUS_FAN_CONFIG_12:<br>-
return
pmbus_read_byte_data(client, page - MAX31785_NR_PAGES,<br>-
reg);<br>+
if
(page >= MAX31785_NR_PAGES)<br>+
return max31785_pmbus_read_byte_data(client,<br>+
page - MAX31785_NR_PAGES,<br>+
reg);<br>+
break;<br> }<br> <br>-
return -ENODATA;<br>+
return max31785_pmbus_read_byte_data(client, page, reg);<br> }<br> <br> static int max31785_write_byte(struct i2c_client *client, int page, u8
value)<br> {<br>-
if (page < MAX31785_NR_PAGES)<br>-
return
-ENODATA;<br>+
if (page >= MAX31785_NR_PAGES)<br>+
return
-ENOTSUPP;<br> <br>-
return -ENOTSUPP;<br>+
return max31785_pmbus_write_byte(client, page, value);<br> }<br> <br> static int max31785_read_long_data(struct i2c_client *client, int page,<br>@@ -120,11 +196,13 @@ static int max31785_get_pwm_mode(struct i2c_client
*client, int page)<br> int
config;<br> int
command;<br> <br>-
config = pmbus_read_byte_data(client, page, PMBUS_FAN_CONFIG_12);<br>+
config = max31785_pmbus_read_byte_data(client, page,<br>+
PMBUS_FAN_CONFIG_12);<br> if
(config < 0)<br>
return config;<br> <br>-
command = pmbus_read_word_data(client, page, PMBUS_FAN_COMMAND_1);<br>+
command = max31785_pmbus_read_word_data(client, page,<br>+
PMBUS_FAN_COMMAND_1);<br> if
(command < 0)<br>
return command;<br> <br>@@ -148,15 +226,14 @@ static int max31785_read_word_data(struct i2c_client
*client, int page,<br> switch
(reg) {<br> case
PMBUS_READ_FAN_SPEED_1:<br>
if (page < MAX31785_NR_PAGES)<br>-
return -ENODATA;<br>+
return max31785_pmbus_read_word_data(client, page, reg);<br> <br>
rv = max31785_read_long_data(client, page - MAX31785_NR_PAGES,<br>
reg, &val);<br>
if (rv < 0)<br>
return
rv;<br> <br>-
rv
= (val >> 16) & 0xffff;<br>-
break;<br>+
return
(val >> 16) & 0xffff;<br> case
PMBUS_FAN_COMMAND_1:<br>
/*<br>
* PMBUS_FAN_COMMAND_x is probed to judge whether or not to<br>@@ -164,20 +241,28 @@ static int max31785_read_word_data(struct i2c_client
*client, int page,<br>
*<br>
* Don't expose fan_target attribute for virtual pages.<br>
*/<br>-
rv
= (page >= MAX31785_NR_PAGES) ? -ENOTSUPP : -ENODATA;<br>+
if
(page >= MAX31785_NR_PAGES)<br>+
return -ENOTSUPP;<br>
break;<br>+
case PMBUS_VIRT_FAN_TARGET_1:<br>+
if
(page >= MAX31785_NR_PAGES)<br>+
return -ENOTSUPP;<br>+<br>+
return
-ENODATA;<br> case
PMBUS_VIRT_PWM_1:<br>-
rv
= max31785_get_pwm(client, page);<br>-
break;<br>+
return
max31785_get_pwm(client, page);<br> case
PMBUS_VIRT_PWM_ENABLE_1:<br>-
rv
= max31785_get_pwm_mode(client, page);<br>-
break;<br>+
return
max31785_get_pwm_mode(client, page);<br> default:<br>-
rv
= -ENODATA;<br>+
if
(page >= MAX31785_NR_PAGES)<br>+
return -ENXIO;<br>
break;<br> }<br> <br>-
return rv;<br>+
if (reg >= PMBUS_VIRT_BASE)<br>+
return
-ENXIO;<br>+<br>+
return max31785_pmbus_read_word_data(client, page, reg);<br> }<br> <br> static inline u32 max31785_scale_pwm(u32 sensor_val)<br>@@ -201,6 +286,31 @@ static inline u32 max31785_scale_pwm(u32 sensor_val)<br> return
(sensor_val * 100) / 255;<br> }<br> <br>+static int max31785_update_fan(struct i2c_client *client, int page,<br>+
u8 config, u8 mask, u16 command)<br>+{<br>+
int from, rv;<br>+
u8 to;<br>+<br>+
from = max31785_pmbus_read_byte_data(client, page, PMBUS_FAN_CONFIG_12);<br>+
if (from < 0)<br>+
return
from;<br>+<br>+
to = (from & ~mask) | (config & mask);<br>+<br>+
if (to != from) {<br>+
rv
= max31785_pmbus_write_byte_data(client, page,<br>+
PMBUS_FAN_CONFIG_12, to);<br>+
if
(rv < 0)<br>+
return rv;<br>+
}<br>+<br>+
rv = max31785_pmbus_write_word_data(client, page, PMBUS_FAN_COMMAND_1,<br>+
command);<br>+<br>+
return rv;<br>+}<br>+<br> static int max31785_pwm_enable(struct i2c_client *client, int page,<br>
u16 word)<br> {<br>@@ -230,15 +340,18 @@ static int max31785_pwm_enable(struct i2c_client
*client, int page,<br>
return -EINVAL;<br> }<br> <br>-
return pmbus_update_fan(client, page, 0, config, PB_FAN_1_RPM, rate);<br>+
return max31785_update_fan(client, page, config, PB_FAN_1_RPM, rate);<br> }<br> <br> static int max31785_write_word_data(struct i2c_client *client, int page,<br>
int reg, u16 word)<br> {<br> switch
(reg) {<br>+
case PMBUS_VIRT_FAN_TARGET_1:<br>+
return
max31785_update_fan(client, page, PB_FAN_1_RPM,<br>+
PB_FAN_1_RPM, word);<br> case
PMBUS_VIRT_PWM_1:<br>-
return
pmbus_update_fan(client, page, 0, 0, PB_FAN_1_RPM,<br>+
return
max31785_update_fan(client, page, 0, PB_FAN_1_RPM,<br>
max31785_scale_pwm(word));<br> case
PMBUS_VIRT_PWM_ENABLE_1:<br>
return max31785_pwm_enable(client, page, word);<br>@@ -246,7 +359,10 @@ static int max31785_write_word_data(struct i2c_client
*client, int page,<br>
break;<br> }<br> <br>-
return -ENODATA;<br>+
if (reg < PMBUS_VIRT_BASE)<br>+
return
max31785_pmbus_write_word_data(client, page, reg, word);<br>+<br>+
return -ENXIO;<br> }<br> <br> /*<br>@@ -279,11 +395,11 @@ static int max31785_of_fan_config(struct i2c_client
*client,<br>
return -ENXIO;<br> }<br> <br>-
ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);<br>+
ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);<br> if
(ret < 0)<br>
return ret;<br> <br>-
pb_cfg = i2c_smbus_read_byte_data(client, PMBUS_FAN_CONFIG_12);<br>+
pb_cfg = max31785_i2c_smbus_read_byte_data(client, PMBUS_FAN_CONFIG_12);<br> if
(pb_cfg < 0)<br>
return pb_cfg;<br> <br>@@ -309,7 +425,9 @@ static int max31785_of_fan_config(struct i2c_client
*client,<br> }
else if (!strcmp("lock", sval)) {<br>
mfr_cfg |= MFR_FAN_CONFIG_ROTOR;<br> <br>-
ret
= i2c_smbus_write_word_data(client, MFR_FAN_FAULT_LIMIT, 1);<br>+
ret
= max31785_i2c_smbus_write_word_data(client,<br>+
MFR_FAN_FAULT_LIMIT,<br>+
1);<br>
if (ret < 0)<br>
return
ret;<br> <br>@@ -423,21 +541,23 @@ static int max31785_of_fan_config(struct i2c_client
*client,<br> if
(of_property_read_bool(child, "maxim,fan-fault-pin-mon"))<br>
mfr_fault_resp |= MFR_FAULT_RESPONSE_MONITOR;<br> <br>-
ret = i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12,<br>+
ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12,<br>
pb_cfg
& ~PB_FAN_1_INSTALLED);<br> if
(ret < 0)<br>
return ret;<br> <br>-
ret = i2c_smbus_write_word_data(client, MFR_FAN_CONFIG, mfr_cfg);<br>+
ret = max31785_i2c_smbus_write_word_data(client, MFR_FAN_CONFIG,<br>+
mfr_cfg);<br> if
(ret < 0)<br>
return ret;<br> <br>-
ret = i2c_smbus_write_byte_data(client, MFR_FAULT_RESPONSE,<br>-
mfr_fault_resp);<br>+
ret = max31785_i2c_smbus_write_byte_data(client, MFR_FAULT_RESPONSE,<br>+
mfr_fault_resp);<br> if
(ret < 0)<br>
return ret;<br> <br>-
ret = i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12, pb_cfg);<br>+
ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12,<br>+
pb_cfg);<br> if
(ret < 0)<br>
return ret;<br> <br>@@ -482,7 +602,7 @@ static int max31785_of_tmp_config(struct i2c_client
*client,<br>
return -ENXIO;<br> }<br> <br>-
ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);<br>+
ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);<br> if
(ret < 0)<br>
return ret;<br> <br>@@ -506,7 +626,7 @@ static int max31785_of_tmp_config(struct i2c_client
*client,<br>
i++;<br> }<br> <br>-
ret = i2c_smbus_write_word_data(client, MFR_TEMP_SENSOR_CONFIG,<br>+
ret = max31785_i2c_smbus_write_word_data(client, MFR_TEMP_SENSOR_CONFIG,<br>
mfr_tmp_cfg);<br> if
(ret < 0)<br>
return ret;<br>@@ -585,11 +705,11 @@ static int max31785_configure_dual_tach(struct i2c_client
*client,<br> int
i;<br> <br> for
(i = 0; i < MAX31785_NR_FAN_PAGES; i++) {<br>-
ret
= i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);<br>+
ret
= max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);<br>
if (ret < 0)<br>
return
ret;<br> <br>-
ret
= i2c_smbus_read_word_data(client, MFR_FAN_CONFIG);<br>+
ret
= max31785_i2c_smbus_read_word_data(client, MFR_FAN_CONFIG);<br>
if (ret < 0)<br>
return
ret;<br> <br>@@ -627,7 +747,7 @@ static int max31785_probe(struct i2c_client *client,<br> <br> *info
= max31785_info;<br> <br>-
ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 255);<br>+
ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, 255);<br> if
(ret < 0)<br>
return ret;<br> <br>@@ -669,17 +789,20 @@ static int max31785_probe(struct i2c_client *client,<br>
if (!have_fan || fan_configured)<br>
continue;<br> <br>-
ret
= i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);<br>+
ret
= max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE,<br>+
i);<br>
if (ret < 0)<br>
return
ret;<br> <br>-
ret
= i2c_smbus_read_byte_data(client, PMBUS_FAN_CONFIG_12);<br>+
ret
= max31785_i2c_smbus_read_byte_data(client,<br>+
PMBUS_FAN_CONFIG_12);<br>
if (ret < 0)<br>
return
ret;<br> <br>
ret &= ~PB_FAN_1_INSTALLED;<br>-
ret
= i2c_smbus_write_word_data(client, PMBUS_FAN_CONFIG_12,<br>-
ret);<br>+
ret
= max31785_i2c_smbus_write_word_data(client,<br>+
PMBUS_FAN_CONFIG_12,<br>+
ret);<br>
if (ret < 0)<br>
return
ret;<br> }<br>-- <br>2.14.1<br><br></span></tt><br><br><BR>