From: Corey Minyard <minyard@acm.org>
To: Patrick Venture <venture@google.com>,
arnd@arndb.de, gregkh@linuxfoundation.org
Cc: openipmi-developer@lists.sourceforge.net,
linux-kernel@vger.kernel.org, openbmc@lists.ozlabs.org
Subject: Re: [PATCH v2] ipmi: looped device detection
Date: Wed, 12 Sep 2018 17:09:58 -0500 [thread overview]
Message-ID: <585d1c3a-6121-c20d-f6d6-7567595cd1af@acm.org> (raw)
In-Reply-To: <20180911225630.124502-1-venture@google.com>
On 09/11/2018 05:56 PM, Patrick Venture wrote:
> Try to get the device ID repeatedly during initialization before giving up.
> The BMC isn't always responsive, and this allows it to be slightly flaky
> during early boot.
>
> Tested: Installed on a system with the BMC software disabled
> such that it was non-responsive. The driver correctly detected this
> and gave up as expected. Then I re-enabled the BMC software unloaded
> and reloaded the driver and it was detected properly.
The patch looks fine, but I wonder if this is something that is really
valuable.
I have wondered about this before.
The question is: If the BMC is unavailable, what are the chances of it
becoming
available by the time you do 5 attempts? I would guess that is a pretty
small
chance, which is why I haven't done this already.
You could have something that re-tested periodically, but there are so many
systems with IPMI specified in ACPI or SMBIOS that is wrong, and it would
try forever. Also not really a good thing.
So I've left it to reload the driver or use the hotmod interface.
-corey
> Signed-off-by: Patrick Venture <venture@google.com>
> ---
> v2:
> - removed extra variable that was set but not used.
> ---
> drivers/char/ipmi/ipmi_si_intf.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 90ec010bffbd..5fed96897fe8 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -1918,11 +1918,13 @@ int ipmi_si_add_smi(struct si_sm_io *io)
> * held, primarily to keep smi_num consistent, we only one to do these
> * one at a time.
> */
> +#define GET_DEVICE_ID_ATTEMPTS 5
> static int try_smi_init(struct smi_info *new_smi)
> {
> int rv = 0;
> int i;
> char *init_name = NULL;
> + unsigned long sleep_rm;
>
> pr_info(PFX "Trying %s-specified %s state machine at %s address 0x%lx, slave address 0x%x, irq %d\n",
> ipmi_addr_src_to_str(new_smi->io.addr_source),
> @@ -2003,7 +2005,26 @@ static int try_smi_init(struct smi_info *new_smi)
> * Attempt a get device id command. If it fails, we probably
> * don't have a BMC here.
> */
> - rv = try_get_dev_id(new_smi);
> + for (i = 0; i < GET_DEVICE_ID_ATTEMPTS; i++) {
> + pr_info(PFX "Attempting to read BMC device ID\n");
> + rv = try_get_dev_id(new_smi);
> + /* If it succeeded, stop trying */
> + if (!rv)
> + break;
> +
> + /* Sleep for ~0.25s before trying again instead of hammering
> + * the BMC.
> + */
> + sleep_rm = msleep_interruptible(250);
> + if (sleep_rm != 0) {
> + pr_info(PFX "Find BMC interrupted\n");
> + rv = -EINTR;
> + goto out_err;
> + }
> + }
> +
> + /* If we exited the loop above and rv is non-zero we ran out of tries.
> + */
> if (rv) {
> if (new_smi->io.addr_source)
> dev_err(new_smi->io.dev,
next prev parent reply other threads:[~2018-09-12 22:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-11 22:56 [PATCH v2] ipmi: looped device detection Patrick Venture
2018-09-12 22:09 ` Corey Minyard [this message]
2018-09-12 22:54 ` Patrick Venture
2018-09-18 18:42 ` Patrick Venture
2018-09-18 21:37 ` Corey Minyard
2018-09-19 19:56 ` Patrick Venture
2018-09-19 20:20 ` Corey Minyard
2018-09-20 18:08 ` Patrick Venture
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=585d1c3a-6121-c20d-f6d6-7567595cd1af@acm.org \
--to=minyard@acm.org \
--cc=arnd@arndb.de \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=openbmc@lists.ozlabs.org \
--cc=openipmi-developer@lists.sourceforge.net \
--cc=venture@google.com \
/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).