All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Jia-Ju Bai <baijiaju1990@gmail.com>
Cc: tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
	paulo.r.zanoni@intel.com, kstewart@linuxfoundation.org,
	gregkh@linuxfoundation.org, x86@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kernel: x86: early-quirks: Replace mdelay with usleep_range in apple_airport_reset
Date: Wed, 24 Jan 2018 06:33:01 +0100	[thread overview]
Message-ID: <20180124053301.GA30540@wunner.de> (raw)
In-Reply-To: <1516761502-18360-1-git-send-email-baijiaju1990@gmail.com>

On Wed, Jan 24, 2018 at 10:38:22AM +0800, Jia-Ju Bai wrote:
> The function apple_airport_reset is not called in atomic context.
> Thus mdelay can be replaced with usleep_range, to avoid busy wait.
> 
> This is reported by a static analysis tool named DCNS written by myself.

No, usleep_range() is built on hrtimers and at this point in the
boot sequence we haven't called hrtimers_init() yet.

Look at init/main.c:start_kernel() and note that setup_arch()
(which calls early_quirks()) is called way before hrtimers_init().

Please amend your static checker to consider every call to mdelay()
that occurs before hrtimers_init() a false positive, or manually
verify each patch for this constraint before submission.

I'll test your patch later today on my MacBook Pro but I suspect
it'll cause a boot crash or hang.  In any case the choice of mdelay()
here was deliberate.

Thanks,

Lukas

> 
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
>  arch/x86/kernel/early-quirks.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index 1e82f78..559e81a 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -604,7 +604,7 @@ static void __init apple_airport_reset(int bus, int slot, int func)
>  	if ((pmcsr & PCI_PM_CTRL_STATE_MASK) != PCI_D0) {
>  		pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
>  		write_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL, pmcsr);
> -		mdelay(10);
> +		usleep_range(10000, 11000);
>  
>  		pmcsr = read_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL);
>  		if ((pmcsr & PCI_PM_CTRL_STATE_MASK) != PCI_D0) {
> -- 
> 1.7.9.5
> 

  reply	other threads:[~2018-01-24  5:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-24  2:38 [PATCH] kernel: x86: early-quirks: Replace mdelay with usleep_range in apple_airport_reset Jia-Ju Bai
2018-01-24  5:33 ` Lukas Wunner [this message]
2018-01-24 16:13   ` Andy Shevchenko

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=20180124053301.GA30540@wunner.de \
    --to=lukas@wunner.de \
    --cc=baijiaju1990@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=kstewart@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulo.r.zanoni@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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.