public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale•com>
To: Wang Dongsheng <dongsheng.wang@freescale•com>
Cc: kumar.gala@freescale•com, linuxppc-dev@lists•ozlabs.org,
	Wang Dongsheng <dongsheng.wang@freescale•com>,
	Zhao Chenhui <chenhui.zhao@freescale•com>
Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
Date: Mon, 18 Mar 2013 19:30:58 -0500	[thread overview]
Message-ID: <1363653058.27435.24@snotra> (raw)
In-Reply-To: <1362728327-21013-3-git-send-email-dongsheng.wang@freescale.com> (from dongsheng.wang@freescale.com on Fri Mar  8 01:38:47 2013)

On 03/08/2013 01:38:47 AM, Wang Dongsheng wrote:
> The driver provides a way to wake up the system by the MPIC timer.
>=20
> For example,
> echo 5 > /sys/devices/system/mpic/timer_wakeup
> echo standby > /sys/power/state
>=20
> After 5 seconds the MPIC timer will generate an interrupt to wake up
> the system.
>=20
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale•com>
> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale•com>
> Signed-off-by: Li Yang <leoli@freescale•com>

Does this work with deep sleep (echo mem > /sys/power/state on mpc8536, =20
p1022, etc) or just regular sleep?

> ---
>  arch/powerpc/platforms/Kconfig              |    9 ++
>  arch/powerpc/sysdev/Makefile                |    1 +
>  arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c |  185 =20
> +++++++++++++++++++++++++++
>  3 files changed, 195 insertions(+), 0 deletions(-)
>  create mode 100644 arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
>=20
> diff --git a/arch/powerpc/platforms/Kconfig =20
> b/arch/powerpc/platforms/Kconfig
> index 5af04fa..487c37f 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -99,6 +99,15 @@ config MPIC_TIMER
>  	  only tested on fsl chip, but it can potentially support
>  	  other global timers complying to Open-PIC standard.
>=20
> +config FSL_MPIC_TIMER_WAKEUP
> +	tristate "Freescale MPIC global timer wakeup driver"
> +	depends on FSL_SOC &&  MPIC_TIMER
> +	default n
> +	help
> +	  This is only for freescale powerpc platform.

This sentence is redundant... It already says "Freescale MPIC" in the =20
name and depends on "FSL_SOC && MPIC_TIMER".

> +static irqreturn_t fsl_mpic_timer_irq(int irq, void *dev_id)
> +{
> +	struct fsl_mpic_timer_wakeup *wakeup =3D dev_id;
> +
> +	schedule_work(&wakeup->free_work);
> +	return IRQ_HANDLED;
> +}

return wakeup->timer ? IRQ_HANDLED : IRQ_NONE;

> +
> +static ssize_t fsl_timer_wakeup_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct timeval interval;
> +	int val =3D 0;
> +
> +	mutex_lock(&sysfs_lock);
> +	if (fsl_wakeup->timer) {
> +		mpic_get_remain_time(fsl_wakeup->timer, &interval);
> +		val =3D interval.tv_sec + 1;
> +	}
> +	mutex_unlock(&sysfs_lock);
> +
> +	return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t fsl_timer_wakeup_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf,
> +				size_t count)
> +{
> +	struct timeval interval;
> +	int ret;
> +
> +	interval.tv_usec =3D 0;
> +	if (kstrtol(buf, 0, &interval.tv_sec))
> +		return -EINVAL;

I don't think the buffer will NUL-terminated...  Ordinarily there'll be
an LF terminator, but you can't rely on that (many other sysfs =20
attributes
seem to, though...).

> +	mutex_lock(&sysfs_lock);
> +
> +	if (fsl_wakeup->timer && !interval.tv_sec) {
> +		disable_irq_wake(fsl_wakeup->timer->irq);
> +		mpic_free_timer(fsl_wakeup->timer);
> +		fsl_wakeup->timer =3D NULL;
> +		mutex_unlock(&sysfs_lock);
> +
> +		return count;
> +	}
> +
> +	if (fsl_wakeup->timer) {
> +		mutex_unlock(&sysfs_lock);
> +		return -EBUSY;
> +	}

So to change an already-set timer you have to set it to zero and then to
what you want?  Why not just do:

	if (fsl_wakeup->timer) {
		disable_irq_wake(...);
		mpic_free_timer(...);
		fsl_wakeup_timer =3D NULL;
	}

	if (!interval.tv_sec) {
		mutex_unlock(&sysfs_lock);
		return count;
	}

> +	ret =3D subsys_system_register(&mpic_subsys, NULL);
> +	if (ret)
> +		goto err;

Maybe arch/powerpc/sysdev/mpic.c should be doing this?

> +
> +	for (i =3D 0; mpic_attributes[i]; i++) {
> +		ret =3D device_create_file(mpic_subsys.dev_root,
> +					mpic_attributes[i]);
> +		if (ret)
> +			goto err2;
> +	}

Is this code ever going to register more than one?

-Scott=

  reply	other threads:[~2013-03-19  0:31 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-08  7:38 [PATCH 1/3] powerpc/mpic: add irq_set_wake support Wang Dongsheng
2013-03-08  7:38 ` [PATCH 2/3] powerpc/mpic: add global timer support Wang Dongsheng
2013-03-18 23:46   ` Scott Wood
2013-03-19  7:55     ` Wang Dongsheng-B40534
2013-03-19 22:59       ` Scott Wood
2013-03-20  6:45         ` Wang Dongsheng-B40534
2013-03-20 22:59           ` Scott Wood
2013-03-22  6:14             ` Wang Dongsheng-B40534
2013-03-22 22:29               ` Scott Wood
2013-03-26  3:29                 ` Wang Dongsheng-B40534
2013-03-26 17:31                   ` Scott Wood
2013-03-27  3:23                     ` Wang Dongsheng-B40534
2013-03-27 17:11                       ` Scott Wood
2013-03-28  2:29                         ` Wang Dongsheng-B40534
2013-03-28 19:47                           ` Scott Wood
2013-03-29  1:58                             ` Wang Dongsheng-B40534
2013-03-08  7:38 ` [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support Wang Dongsheng
2013-03-19  0:30   ` Scott Wood [this message]
2013-03-19  6:25     ` Wang Dongsheng-B40534
2013-03-19 22:54       ` Scott Wood
2013-03-20  3:48         ` Wang Dongsheng-B40534
2013-03-20 21:48           ` Scott Wood
2013-03-22  5:46             ` Wang Dongsheng-B40534
2013-03-22 22:11               ` Scott Wood
2013-03-26  3:27                 ` Wang Dongsheng-B40534
2013-03-26 17:35                   ` Scott Wood
2013-03-27  3:21                     ` Wang Dongsheng-B40534
2013-03-27 20:25                       ` Scott Wood
2013-03-28  3:09                         ` Wang Dongsheng-B40534
2013-03-18  9:28 ` [PATCH 1/3] powerpc/mpic: add irq_set_wake support Wang Dongsheng-B40534
2013-03-18 14:41   ` Benjamin Herrenschmidt
2013-03-18 14:44     ` Gala Kumar-B11780

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=1363653058.27435.24@snotra \
    --to=scottwood@freescale$(echo .)com \
    --cc=chenhui.zhao@freescale$(echo .)com \
    --cc=dongsheng.wang@freescale$(echo .)com \
    --cc=kumar.gala@freescale$(echo .)com \
    --cc=linuxppc-dev@lists$(echo .)ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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