From: Matthew Barth <msbarth@linux.ibm.com>
To: Eddie James <eajames@linux.ibm.com>, openbmc@lists.ozlabs.org
Cc: Andrew Jeffery <andrew@aj.id.au>
Subject: Re: [PATCH linux dev-5.10 07/14] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG
Date: Wed, 11 Aug 2021 11:14:06 -0500 [thread overview]
Message-ID: <05567b62-89f2-b743-1f6a-500a99275cfb@linux.ibm.com> (raw)
In-Reply-To: <20210811154232.12649-8-eajames@linux.ibm.com>
On 8/11/21 10:42 AM, Eddie James wrote:
> 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>
Reviewed-by: Matthew Barth <msbarth@linux.ibm.com>
> ---
> 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
next prev parent reply other threads:[~2021-08-11 16:43 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 ` [PATCH linux dev-5.10 07/14] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG Eddie James
2021-08-11 16:14 ` Matthew Barth [this message]
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=05567b62-89f2-b743-1f6a-500a99275cfb@linux.ibm.com \
--to=msbarth@linux.ibm.com \
--cc=andrew@aj.id.au \
--cc=eajames@linux.ibm.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.