All of lore.kernel.org
 help / color / mirror / Atom feed
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] arm: ls1: add CPU hotplug platform support
Date: Fri, 26 Sep 2014 13:46:14 +0100	[thread overview]
Message-ID: <20140926124614.GD7422@leverpostej> (raw)
In-Reply-To: <20140926122003.GP5182@n2100.arm.linux.org.uk>

On Fri, Sep 26, 2014 at 01:20:04PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 26, 2014 at 07:25:01PM +0800, Chenhui Zhao wrote:
> > +static inline void ls1_do_lowpower(unsigned int cpu, int *spurious)
> > +{
> > +	/*
> > +	 * there is no power-control hardware on this platform, so all
> > +	 * we can do is put the core into WFI; this is safe as the calling
> > +	 * code will have already disabled interrupts
> > +	 */
> > +	for (;;) {
> > +		wfi();
> > +
> > +		if (pen_release == cpu_logical_map(cpu)) {
> > +			/*OK, proper wakeup, we're done*/
> > +			break;
> > +		}
> > +
> > +		/*
> > +		 * Getting here, means that we have come out of WFI without
> > +		 * having been woken up - this shouldn't happen
> > +		 *
> > +		 * Just note it happening - when we're woken, we can report
> > +		 * its occurrence.
> > +		 */
> > +		(*spurious)++;
> > +	}
> > +}
> 
> This is pretty much unacceptable - this breaks kexec(), and suspend
> support because your secondary CPUs aren't really sleeping, they're
> sitting in a loop doing nothing.

Agreed.

This looks to be a carbon copy of the vexpress pseudo-hotplug in
arch/arm/mach-vexpress/hotplug.c, which is obviously broken in the way
you describe above. Perhaps we should go about ripping that out?

Mark.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Chenhui Zhao <chenhui.zhao@freescale.com>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Zhuoyu.Zhang@freescale.com" <Zhuoyu.Zhang@freescale.com>,
	"Jason.Jin@freescale.com" <Jason.Jin@freescale.com>,
	"leoli@freescale.com" <leoli@freescale.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/3] arm: ls1: add CPU hotplug platform support
Date: Fri, 26 Sep 2014 13:46:14 +0100	[thread overview]
Message-ID: <20140926124614.GD7422@leverpostej> (raw)
In-Reply-To: <20140926122003.GP5182@n2100.arm.linux.org.uk>

On Fri, Sep 26, 2014 at 01:20:04PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 26, 2014 at 07:25:01PM +0800, Chenhui Zhao wrote:
> > +static inline void ls1_do_lowpower(unsigned int cpu, int *spurious)
> > +{
> > +	/*
> > +	 * there is no power-control hardware on this platform, so all
> > +	 * we can do is put the core into WFI; this is safe as the calling
> > +	 * code will have already disabled interrupts
> > +	 */
> > +	for (;;) {
> > +		wfi();
> > +
> > +		if (pen_release == cpu_logical_map(cpu)) {
> > +			/*OK, proper wakeup, we're done*/
> > +			break;
> > +		}
> > +
> > +		/*
> > +		 * Getting here, means that we have come out of WFI without
> > +		 * having been woken up - this shouldn't happen
> > +		 *
> > +		 * Just note it happening - when we're woken, we can report
> > +		 * its occurrence.
> > +		 */
> > +		(*spurious)++;
> > +	}
> > +}
> 
> This is pretty much unacceptable - this breaks kexec(), and suspend
> support because your secondary CPUs aren't really sleeping, they're
> sitting in a loop doing nothing.

Agreed.

This looks to be a carbon copy of the vexpress pseudo-hotplug in
arch/arm/mach-vexpress/hotplug.c, which is obviously broken in the way
you describe above. Perhaps we should go about ripping that out?

Mark.

  reply	other threads:[~2014-09-26 12:46 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-26 11:25 [PATCH 0/3] arm: ls1: add deep sleep support Chenhui Zhao
2014-09-26 11:25 ` Chenhui Zhao
2014-09-26 11:25 ` [PATCH 1/3] arm: ls1: add CPU hotplug platform support Chenhui Zhao
2014-09-26 11:25   ` Chenhui Zhao
2014-09-26 12:20   ` Russell King - ARM Linux
2014-09-26 12:20     ` Russell King - ARM Linux
2014-09-26 12:46     ` Mark Rutland [this message]
2014-09-26 12:46       ` Mark Rutland
2014-09-26 13:03       ` Russell King - ARM Linux
2014-09-26 13:03         ` Russell King - ARM Linux
2014-09-26 13:20         ` Mark Rutland
2014-09-26 13:20           ` Mark Rutland
2014-09-28 10:57           ` Li Yang
2014-09-28 10:57             ` Li Yang
2014-09-28 14:27             ` Russell King - ARM Linux
2014-09-28 14:27               ` Russell King - ARM Linux
2014-09-26 11:25 ` [PATCH 2/3] pm: add FSM configuration for deep sleep Chenhui Zhao
2014-09-26 11:25   ` Chenhui Zhao
2014-09-26 12:02   ` Russell King - ARM Linux
2014-09-26 12:02     ` Russell King - ARM Linux
2014-09-26 20:51     ` Russell King - ARM Linux
2014-09-26 20:51       ` Russell King - ARM Linux
2014-09-28  9:53       ` Chenhui Zhao
2014-09-28  9:53         ` Chenhui Zhao
2014-09-26 11:25 ` [PATCH 3/3] arm: pm: add deep sleep support for LS1 Chenhui Zhao
2014-09-26 11:25   ` Chenhui Zhao
2014-09-26 12:14   ` Russell King - ARM Linux
2014-09-26 12:14     ` Russell King - ARM Linux
2014-09-28 11:06     ` Chenhui Zhao
2014-09-28 11:06       ` Chenhui Zhao
2014-09-28 14:26       ` Russell King - ARM Linux
2014-09-28 14:26         ` Russell King - ARM Linux
2014-09-29  9:42         ` Chenhui Zhao
2014-09-29  9:42           ` Chenhui Zhao

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=20140926124614.GD7422@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=linux-arm-kernel@lists.infradead.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.