openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Eddie James <eajames@linux.ibm.com>
To: openbmc@lists.ozlabs.org
Cc: Andrew Jeffery <andrew@aj.id.au>
Subject: [PATCH linux dev-5.10 07/14] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG
Date: Wed, 11 Aug 2021 10:42:25 -0500	[thread overview]
Message-ID: <20210811154232.12649-8-eajames@linux.ibm.com> (raw)
In-Reply-To: <20210811154232.12649-1-eajames@linux.ibm.com>

From: Andrew Jeffery <andrew@aj.id.au>

It has been observed across large fleets of systems that a small subset
of those systems occasionally loose control of some number of fans
across a BMC reboot (their hwmon fan attributes are missing from sysfs).

>From extensive testing and tracing it was discovered that writes
enabling a fan in FAN_CONFIG_1_2 failed to stick on the system under
test with a frequency of about 1 in 1000 re-binds of the driver.

The MAX31785 datasheet recommends in the documentation for
MFR_FAN_CONFIG that the asssociated fan(s) be disabled before updating
the register. The sequence in question implements this suggestion, and
the observed loss-of-fans symptom occurs when the write to re-enable the
fan in FAN_CONFIG_1_2 fails to stick.

The trace data suggests a one-shot retry is enough to successfully
update FAN_CONFIG_1_2. With the workaround, no loss of fans was observed
in over 20,000 consecutive rebinds of the driver.

OpenBMC-Staging-Count: 1
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/hwmon/pmbus/max31785.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
index 7518fff356f9..b37da2ec1ce4 100644
--- a/drivers/hwmon/pmbus/max31785.c
+++ b/drivers/hwmon/pmbus/max31785.c
@@ -398,6 +398,7 @@ static int max31785_of_fan_config(struct i2c_client *client,
 	u32 page;
 	u32 uval;
 	int ret;
+	int i;
 
 	if (!of_device_is_compatible(child, "pmbus-fan"))
 		return 0;
@@ -574,10 +575,24 @@ static int max31785_of_fan_config(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
-	ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12,
-						 pb_cfg);
-	if (ret < 0)
-		return ret;
+	for (i = 0; i < 2; i++) {
+		ret = max31785_i2c_smbus_write_byte_data(client,
+							 PMBUS_FAN_CONFIG_12,
+							 pb_cfg);
+		if (ret < 0)
+			continue;
+
+		ret = max31785_i2c_smbus_read_byte_data(client,
+							PMBUS_FAN_CONFIG_12);
+		if (ret < 0)
+			continue;
+
+		if (ret == pb_cfg)
+			break;
+	}
+
+	if (i == 2)
+		return -EIO;
 
 	/*
 	 * Fans are on pages 0 - 5. If the page property of a fan node is
-- 
2.27.0


  parent reply	other threads:[~2021-08-11 15:48 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 01/14] i2c: Allow throttling of transfers to client devices Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 02/14] pmbus: (ucd9000) Throttle SMBus transfers to avoid poor behaviour Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 03/14] ucd9000: Add a throttle delay attribute in debugfs Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 04/14] fsi: run clock at 100MHz Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 05/14] pmbus: (core) Add a one-shot retry in pmbus_set_page() Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 06/14] pmbus: (max31785) Add a local pmbus_set_page() implementation Eddie James
2021-08-11 16:13   ` Matthew Barth
2021-08-11 15:42 ` Eddie James [this message]
2021-08-11 16:14   ` [PATCH linux dev-5.10 07/14] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG Matthew Barth
2021-08-11 15:42 ` [PATCH linux dev-5.10 08/14] ARM: dts: aspeed: Rainier: Add fan controller properties Eddie James
2021-08-11 16:14   ` Matthew Barth
2021-08-11 15:42 ` [PATCH linux dev-5.10 09/14] ARM: dts: aspeed: Everest: " Eddie James
2021-08-11 16:15   ` Matthew Barth
2021-08-11 15:42 ` [PATCH linux dev-5.10 10/14] ARM: dts: aspeed: Rainier 4U: Delete fan dual-tach properties Eddie James
2021-08-11 16:17   ` Matthew Barth
2021-08-11 15:42 ` [PATCH linux dev-5.10 11/14] ARM: dts: aspeed: Add Rainier 2U and 4U device trees for pass 1 hardware Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 12/14] fsi: sbefifo: Increase command timeouts to 30 seconds Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 13/14] fsi: occ: Add dynamic debug to dump command and response Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 14/14] fsi: sbefifo: Use interruptible mutex locking Eddie James

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210811154232.12649-8-eajames@linux.ibm.com \
    --to=eajames@linux.ibm.com \
    --cc=andrew@aj.id.au \
    --cc=openbmc@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).