[RFC PATCH] powerpc/fsl: add timer wakeup source

This is only for freescale powerpc platform. The driver provides a way
to wake up system. Proc interface(/proc/powerpc/wakeup_timer_seconds).

eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds", 5 seconds
after the system will be woken up. echo another time into proc interface
to update the time.

Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Li Yang <redacted>
---
 arch/powerpc/platforms/Kconfig            |   23 +++
 arch/powerpc/platforms/Makefile           |    1 +
 arch/powerpc/platforms/fsl_timer_wakeup.c |  217 +++++++++++++++++++++++++++++
 3 files changed, 241 insertions(+)
 create mode 100644 arch/powerpc/platforms/fsl_timer_wakeup.c

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index b190a6e..7b9232a 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig

@@ -99,6 +99,29 @@ config MPIC_TIMER
 	  only tested on fsl chip, but it can potentially support
 	  other global timers complying to Open-PIC standard.
 
+menuconfig FSL_WAKEUP_SOURCE
+	bool "Freescale wakeup source"
+	depends on FSL_SOC && SUSPEND
+	default n
+	help
+	  This option enables wakeup source for wake up system
+	  features. This is only for freescale powerpc platform.
+
+if FSL_WAKEUP_SOURCE
+
+config FSL_TIMER_WAKEUP
+	tristate "Freescale mpic global timer wakeup event"
+	default n
+	help
+	  This is only for freescale powerpc platform. The driver
+	  provides a way to wake up system.
+	  Proc interface(/proc/powerpc/wakeup_timer_seconds).
+	  eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds",
+	  5 seconds after the system will be woken up. echo another
+	  time into proc interface to update the time.
+
+endif
+
 config PPC_EPAPR_HV_PIC
 	bool
 	default n

diff --git a/arch/powerpc/platforms/Makefile b/arch/powerpc/platforms/Makefile
index 879b4a4..8e9a04f 100644
--- a/arch/powerpc/platforms/Makefile
+++ b/arch/powerpc/platforms/Makefile

@@ -2,6 +2,7 @@
 subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
 
 obj-$(CONFIG_FSL_ULI1575)	+= fsl_uli1575.o
+obj-$(CONFIG_FSL_TIMER_WAKEUP)	+= fsl_timer_wakeup.o
 
 obj-$(CONFIG_PPC_PMAC)		+= powermac/
 obj-$(CONFIG_PPC_CHRP)		+= chrp/

diff --git a/arch/powerpc/platforms/fsl_timer_wakeup.c b/arch/powerpc/platforms/fsl_timer_wakeup.c
new file mode 100644
index 0000000..f20199f
--- /dev/null
+++ b/arch/powerpc/platforms/fsl_timer_wakeup.c

@@ -0,0 +1,217 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ * Wang Dongsheng <Dongsheng.Wang@freescale.com>
+ * Li Yang <leoli@freescale.com>
+ *
+ * This is only for freescale powerpc platform. The driver provides a way
+ * to wake up system. Proc interface(/proc/powerpc/wakeup_timer_seconds).
+ *
+ * eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds", 5 seconds
+ * after the system will be woken up. echo another time into proc interface
+ * to update the time.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/sysfs.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/proc_fs.h>
+#include <asm/uaccess.h>
+#include <asm/mpic_timer.h>
+
+struct fsl_timer_wakeup {
+	struct mpic_timer *timer;
+	struct work_struct free_work;
+	struct mutex mutex;
+	struct proc_dir_entry *proc_timer_wakeup;
+	struct timeval time;
+};
+
+static struct fsl_timer_wakeup *priv;
+
+static void timer_event_wakeup_free_work(struct work_struct *ws)
+{
+	struct fsl_timer_wakeup *priv =
+		container_of(ws, struct fsl_timer_wakeup, free_work);
+
+	mutex_lock(&priv->mutex);
+	mpic_free_timer(priv->timer);
+	priv->timer = NULL;
+	mutex_unlock(&priv->mutex);
+}
+
+static irqreturn_t timer_event_interrupt(int irq, void *dev_id)
+{
+	struct fsl_timer_wakeup *priv = dev_id;
+
+	schedule_work(&priv->free_work);
+
+	return IRQ_HANDLED;
+}
+
+static ssize_t timer_wakeup_read(struct file *file, char __user *buf,
+		size_t count, loff_t *offp)
+{
+	struct fsl_timer_wakeup *priv;
+	struct inode *inode = file->f_path.dentry->d_inode;
+	struct proc_dir_entry *dp;
+
+	int ret;
+	char *kbuf;
+
+	dp = PDE(inode);
+	priv = dp->data;
+
+	mutex_lock(&priv->mutex);
+
+	if (!priv->timer ||
+			(priv->time.tv_sec >= 0 && priv->time.tv_usec >= 0)) {
+		priv->time.tv_sec = -1;
+		priv->time.tv_usec = -1;
+
+		mutex_unlock(&priv->mutex);
+
+		return 0;
+	}
+
+	mpic_get_remain_time(priv->timer, &priv->time);
+
+	mutex_unlock(&priv->mutex);
+
+	kbuf = kzalloc(count, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	sprintf(kbuf, "%ld\n%c", priv->time.tv_sec + 1, '\0');
+
+	ret = strlen(kbuf);
+
+	copy_to_user(buf, kbuf, count);
+
+	kfree(kbuf);
+
+	return ret;
+}
+
+static ssize_t timer_wakeup_write(struct file *file, const char __user *buf,
+		size_t count, loff_t *off)
+{
+	struct fsl_timer_wakeup *priv;
+	struct inode *inode = file->f_path.dentry->d_inode;
+	struct proc_dir_entry *dp;
+	struct timeval time;
+	char *kbuf;
+
+	dp = PDE(inode);
+	priv = dp->data;
+
+	kbuf = kzalloc(count + 1, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	if (copy_from_user(kbuf, buf, count)) {
+		kfree(kbuf);
+		return -EFAULT;
+	}
+
+	kbuf[count] = '\0';
+
+	if (kstrtol(kbuf, 0, &time.tv_sec)) {
+		kfree(kbuf);
+		return -EINVAL;
+	}
+
+	kfree(kbuf);
+
+	time.tv_usec = 0;
+
+	mutex_lock(&priv->mutex);
+
+	if (!time.tv_sec) {
+		if (priv->timer) {
+			mpic_free_timer(priv->timer);
+			priv->timer = NULL;
+		}
+		mutex_unlock(&priv->mutex);
+
+		return count;
+	}
+
+	if (priv->timer) {
+		mpic_free_timer(priv->timer);
+		priv->timer = NULL;
+	}
+
+	priv->timer = mpic_request_timer(timer_event_interrupt, priv, &time);
+	if (!priv->timer) {
+		mutex_unlock(&priv->mutex);
+
+		return -EINVAL;
+	}
+
+	mpic_start_timer(priv->timer);
+
+	mutex_unlock(&priv->mutex);
+
+	return count;
+}
+
+static const struct file_operations timer_wakeup_fops = {
+	.owner		= THIS_MODULE,
+	.read		= timer_wakeup_read,
+	.write		= timer_wakeup_write,
+	.llseek		= no_llseek,
+};
+
+static int timer_wakeup_init(void)
+{
+	struct proc_dir_entry *ent;
+
+	priv = kzalloc(sizeof(struct fsl_timer_wakeup), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_init(&priv->mutex);
+	INIT_WORK(&priv->free_work, timer_event_wakeup_free_work);
+	priv->time.tv_sec = -1;
+	priv->time.tv_usec = -1;
+
+	ent = proc_create_data("powerpc/wakeup_timer_seconds",
+			S_IRUSR | S_IWUSR, NULL, &timer_wakeup_fops, priv);
+	if (!ent) {
+		kfree(priv);
+		return -ENOMEM;
+	}
+
+	priv->proc_timer_wakeup = ent;
+
+	return 0;
+}
+
+static void timer_wakeup_exit(void)
+{
+	if (priv->timer)
+		mpic_free_timer(priv->timer);
+
+	remove_proc_entry("wakeup_timer_seconds",
+			priv->proc_timer_wakeup->parent);
+
+	kfree(priv);
+}
+
+module_init(timer_wakeup_init);
+module_exit(timer_wakeup_exit);
+
+MODULE_DESCRIPTION("Freescale mpic global timer event wake-up driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Wang Dongsheng <dongsheng.wang@freescale.com>");

-- 
1.7.9.5

On Oct 3, 2012, at 5:42 AM, Wang Dongsheng wrote:

This is only for freescale powerpc platform. The driver provides a way
to wake up system. Proc interface(/proc/powerpc/wakeup_timer_seconds).
=20
eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds", 5 seconds
after the system will be woken up. echo another time into proc =

interface

to update the time.
=20
Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Li Yang <redacted>
---
arch/powerpc/platforms/Kconfig            |   23 +++
arch/powerpc/platforms/Makefile           |    1 +
arch/powerpc/platforms/fsl_timer_wakeup.c |  217 =

+++++++++++++++++++++++++++++

3 files changed, 241 insertions(+)
create mode 100644 arch/powerpc/platforms/fsl_timer_wakeup.c

Adding the Linux PM list to see if there is some existing support for =
this on other arch's in kernel.

I'm pretty sure /proc/ is NOT where we want this exposed.

- k

b/arch/powerpc/platforms/Kconfig

b/arch/powerpc/platforms/Makefile

b/arch/powerpc/platforms/fsl_timer_wakeup.c

way

+ * to wake up system. Proc =

interface(/proc/powerpc/wakeup_timer_seconds).

+ *
+ * eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds", 5 seconds
+ * after the system will be woken up. echo another time into proc =

interface

+ * to update the time.
+ *
+ * This program is free software; you can redistribute it and/or =

modify it

+ * under the terms of the GNU General Public License version 2 as =

published

+ * by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/sysfs.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/proc_fs.h>
+#include <asm/uaccess.h>
+#include <asm/mpic_timer.h>
+
+struct fsl_timer_wakeup {
+	struct mpic_timer *timer;
+	struct work_struct free_work;
+	struct mutex mutex;
+	struct proc_dir_entry *proc_timer_wakeup;
+	struct timeval time;
+};
+
+static struct fsl_timer_wakeup *priv;
+
+static void timer_event_wakeup_free_work(struct work_struct *ws)
+{
+	struct fsl_timer_wakeup *priv =3D
+		container_of(ws, struct fsl_timer_wakeup, free_work);
+
+	mutex_lock(&priv->mutex);
+	mpic_free_timer(priv->timer);
+	priv->timer =3D NULL;
+	mutex_unlock(&priv->mutex);
+}
+
+static irqreturn_t timer_event_interrupt(int irq, void *dev_id)
+{
+	struct fsl_timer_wakeup *priv =3D dev_id;
+
+	schedule_work(&priv->free_work);
+
+	return IRQ_HANDLED;
+}
+
+static ssize_t timer_wakeup_read(struct file *file, char __user *buf,
+		size_t count, loff_t *offp)
+{
+	struct fsl_timer_wakeup *priv;
+	struct inode *inode =3D file->f_path.dentry->d_inode;
+	struct proc_dir_entry *dp;
+
+	int ret;
+	char *kbuf;
+
+	dp =3D PDE(inode);
+	priv =3D dp->data;
+
+	mutex_lock(&priv->mutex);
+
+	if (!priv->timer ||
+			(priv->time.tv_sec >=3D 0 && priv->time.tv_usec =
=3D 0)) {
+		priv->time.tv_sec =3D -1;
+		priv->time.tv_usec =3D -1;
+
+		mutex_unlock(&priv->mutex);
+
+		return 0;
+	}
+
+	mpic_get_remain_time(priv->timer, &priv->time);
+
+	mutex_unlock(&priv->mutex);
+
+	kbuf =3D kzalloc(count, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	sprintf(kbuf, "%ld\n%c", priv->time.tv_sec + 1, '\0');
+
+	ret =3D strlen(kbuf);
+
+	copy_to_user(buf, kbuf, count);
+
+	kfree(kbuf);
+
+	return ret;
+}
+
+static ssize_t timer_wakeup_write(struct file *file, const char =

__user *buf,

+		size_t count, loff_t *off)
+{
+	struct fsl_timer_wakeup *priv;
+	struct inode *inode =3D file->f_path.dentry->d_inode;
+	struct proc_dir_entry *dp;
+	struct timeval time;
+	char *kbuf;
+
+	dp =3D PDE(inode);
+	priv =3D dp->data;
+
+	kbuf =3D kzalloc(count + 1, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	if (copy_from_user(kbuf, buf, count)) {
+		kfree(kbuf);
+		return -EFAULT;
+	}
+
+	kbuf[count] =3D '\0';
+
+	if (kstrtol(kbuf, 0, &time.tv_sec)) {
+		kfree(kbuf);
+		return -EINVAL;
+	}
+
+	kfree(kbuf);
+
+	time.tv_usec =3D 0;
+
+	mutex_lock(&priv->mutex);
+
+	if (!time.tv_sec) {
+		if (priv->timer) {
+			mpic_free_timer(priv->timer);
+			priv->timer =3D NULL;
+		}
+		mutex_unlock(&priv->mutex);
+
+		return count;
+	}
+
+	if (priv->timer) {
+		mpic_free_timer(priv->timer);
+		priv->timer =3D NULL;
+	}
+
+	priv->timer =3D mpic_request_timer(timer_event_interrupt, priv, =

&time);

+	if (!priv->timer) {
+		mutex_unlock(&priv->mutex);
+
+		return -EINVAL;
+	}
+
+	mpic_start_timer(priv->timer);
+
+	mutex_unlock(&priv->mutex);
+
+	return count;
+}
+
+static const struct file_operations timer_wakeup_fops =3D {
+	.owner		=3D THIS_MODULE,
+	.read		=3D timer_wakeup_read,
+	.write		=3D timer_wakeup_write,
+	.llseek		=3D no_llseek,
+};
+
+static int timer_wakeup_init(void)
+{
+	struct proc_dir_entry *ent;
+
+	priv =3D kzalloc(sizeof(struct fsl_timer_wakeup), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_init(&priv->mutex);
+	INIT_WORK(&priv->free_work, timer_event_wakeup_free_work);
+	priv->time.tv_sec =3D -1;
+	priv->time.tv_usec =3D -1;
+
+	ent =3D proc_create_data("powerpc/wakeup_timer_seconds",
+			S_IRUSR | S_IWUSR, NULL, &timer_wakeup_fops, =

priv);

+	if (!ent) {
+		kfree(priv);
+		return -ENOMEM;
+	}
+
+	priv->proc_timer_wakeup =3D ent;
+
+	return 0;
+}
+
+static void timer_wakeup_exit(void)
+{
+	if (priv->timer)
+		mpic_free_timer(priv->timer);
+
+	remove_proc_entry("wakeup_timer_seconds",
+			priv->proc_timer_wakeup->parent);
+
+	kfree(priv);
+}
+
+module_init(timer_wakeup_init);
+module_exit(timer_wakeup_exit);
+
+MODULE_DESCRIPTION("Freescale mpic global timer event wake-up =

driver");

+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Wang Dongsheng [off-list ref]");
--=20
1.7.9.5

On 10/03/2012 08:35:58 AM, Kumar Gala wrote:

=20
On Oct 3, 2012, at 5:42 AM, Wang Dongsheng wrote:
=20

quoted

This is only for freescale powerpc platform. The driver provides a =20

way

quoted

to wake up system. Proc =20

interface(/proc/powerpc/wakeup_timer_seconds).

quoted

eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds", 5 seconds
after the system will be woken up. echo another time into proc =20

interface

quoted

to update the time.

Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Li Yang <redacted>
---
arch/powerpc/platforms/Kconfig            |   23 +++
arch/powerpc/platforms/Makefile           |    1 +
arch/powerpc/platforms/fsl_timer_wakeup.c |  217 =20

+++++++++++++++++++++++++++++

quoted

3 files changed, 241 insertions(+)
create mode 100644 arch/powerpc/platforms/fsl_timer_wakeup.c

=20
Adding the Linux PM list to see if there is some existing support for =20
this on other arch's in kernel.
=20
I'm pretty sure /proc/ is NOT where we want this exposed.

Should probably go under the sysfs directory of the mpic device.  Or =20
better, make a generic interface for timer-based suspend wakeup (if =20
there isn't one already).  This current approach sits in an unpleasant =20
middle ground between generic and device-specific.

quoted

diff --git a/arch/powerpc/platforms/Kconfig =20

b/arch/powerpc/platforms/Kconfig

quoted

index b190a6e..7b9232a 100644

--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig

@@ -99,6 +99,29 @@ config MPIC_TIMER

	  only tested on fsl chip, but it can potentially support
	  other global timers complying to Open-PIC standard.

+menuconfig FSL_WAKEUP_SOURCE
+	bool "Freescale wakeup source"
+	depends on FSL_SOC && SUSPEND
+	default n
+	help
+	  This option enables wakeup source for wake up system
+	  features. This is only for freescale powerpc platform.
+
+if FSL_WAKEUP_SOURCE
+
+config FSL_TIMER_WAKEUP
+	tristate "Freescale mpic global timer wakeup event"
+	default n
+	help
+	  This is only for freescale powerpc platform. The driver
+	  provides a way to wake up system.
+	  Proc interface(/proc/powerpc/wakeup_timer_seconds).
+	  eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds",
+	  5 seconds after the system will be woken up. echo another
+	  time into proc interface to update the time.
+
+endif

Use depends rather than if/else.  Why do you need FSL_WAKEUP_SOURCE?

These names are overly broad -- this is only for FSL MPIC, not for =20
other FSL chips (e.g. mpc83xx has a different global timer =20
implementation, and there's FSL ARM chips, etc).

quoted

+static ssize_t timer_wakeup_write(struct file *file, const char =20

__user *buf,

quoted

+		size_t count, loff_t *off)
+{
+	struct fsl_timer_wakeup *priv;
+	struct inode *inode =3D file->f_path.dentry->d_inode;
+	struct proc_dir_entry *dp;
+	struct timeval time;
+	char *kbuf;
+
+	dp =3D PDE(inode);
+	priv =3D dp->data;
+
+	kbuf =3D kzalloc(count + 1, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	if (copy_from_user(kbuf, buf, count)) {
+		kfree(kbuf);
+		return -EFAULT;
+	}
+
+	kbuf[count] =3D '\0';
+
+	if (kstrtol(kbuf, 0, &time.tv_sec)) {
+		kfree(kbuf);
+		return -EINVAL;
+	}
+
+	kfree(kbuf);
+
+	time.tv_usec =3D 0;
+
+	mutex_lock(&priv->mutex);
+
+	if (!time.tv_sec) {
+		if (priv->timer) {
+			mpic_free_timer(priv->timer);
+			priv->timer =3D NULL;
+		}
+		mutex_unlock(&priv->mutex);
+
+		return count;
+	}
+
+	if (priv->timer) {
+		mpic_free_timer(priv->timer);
+		priv->timer =3D NULL;
+	}
+
+	priv->timer =3D mpic_request_timer(timer_event_interrupt, priv, =20

&time);

If the new time is zero, consider that a cancellation of the timer and =20
don't request a new one or return -EINVAL.

-Scott=

-----Original Message-----
From: Wood Scott-B07421
Sent: Thursday, October 04, 2012 6:20 AM
To: Kumar Gala
Cc: Wang Dongsheng; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org list=

;

Wang Dongsheng-B40534; Li Yang-R58472; linux-pm@vger.kernel.org
Subject: Re: [RFC PATCH] powerpc/fsl: add timer wakeup source
=20
On 10/03/2012 08:35:58 AM, Kumar Gala wrote:

quoted

On Oct 3, 2012, at 5:42 AM, Wang Dongsheng wrote:

quoted

This is only for freescale powerpc platform. The driver provides a

way

quoted

to wake up system. Proc

interface(/proc/powerpc/wakeup_timer_seconds).

quoted

eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds", 5 seconds after
the system will be woken up. echo another time into proc

interface

quoted

to update the time.

Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Li Yang <redacted>
---
arch/powerpc/platforms/Kconfig            |   23 +++
arch/powerpc/platforms/Makefile           |    1 +
arch/powerpc/platforms/fsl_timer_wakeup.c |  217

+++++++++++++++++++++++++++++

quoted

3 files changed, 241 insertions(+)
create mode 100644 arch/powerpc/platforms/fsl_timer_wakeup.c

Adding the Linux PM list to see if there is some existing support for
this on other arch's in kernel.

I'm pretty sure /proc/ is NOT where we want this exposed.

=20
Should probably go under the sysfs directory of the mpic device.  Or
better, make a generic interface for timer-based suspend wakeup (if there
isn't one already).  This current approach sits in an unpleasant middle
ground between generic and device-specific.
=09

/sys/power/wakeup_timer_seconds how about this?
I think it is a freescale generic interface, this interface control by
FSL_SOC && SUSPEND.

quoted

quoted

diff --git a/arch/powerpc/platforms/Kconfig

b/arch/powerpc/platforms/Kconfig

quoted

index b190a6e..7b9232a 100644

--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig

@@ -99,6 +99,29 @@ config MPIC_TIMER

	  only tested on fsl chip, but it can potentially support
	  other global timers complying to Open-PIC standard.

+menuconfig FSL_WAKEUP_SOURCE
+	bool "Freescale wakeup source"
+	depends on FSL_SOC && SUSPEND
+	default n
+	help
+	  This option enables wakeup source for wake up system
+	  features. This is only for freescale powerpc platform.
+
+if FSL_WAKEUP_SOURCE
+
+config FSL_TIMER_WAKEUP
+	tristate "Freescale mpic global timer wakeup event"
+	default n
+	help
+	  This is only for freescale powerpc platform. The driver
+	  provides a way to wake up system.
+	  Proc interface(/proc/powerpc/wakeup_timer_seconds).
+	  eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds",
+	  5 seconds after the system will be woken up. echo another
+	  time into proc interface to update the time.
+
+endif

=20
Use depends rather than if/else.  Why do you need FSL_WAKEUP_SOURCE?

It lists all wake up source. If later have wakeup source can be improved by
it to control. Buttons event wakeup source will be added after the timer.

These names are overly broad -- this is only for FSL MPIC, not for other
FSL chips (e.g. mpc83xx has a different global timer implementation, and
there's FSL ARM chips, etc).
=20

Yes, thanks. Change to FSL_MPIC_TIMER_WAKEUP.

quoted

quoted

+static ssize_t timer_wakeup_write(struct file *file, const char

__user *buf,

quoted

+		size_t count, loff_t *off)
+{
+	struct fsl_timer_wakeup *priv;
+	struct inode *inode =3D file->f_path.dentry->d_inode;
+	struct proc_dir_entry *dp;
+	struct timeval time;
+	char *kbuf;
+
+	dp =3D PDE(inode);
+	priv =3D dp->data;
+
+	kbuf =3D kzalloc(count + 1, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	if (copy_from_user(kbuf, buf, count)) {
+		kfree(kbuf);
+		return -EFAULT;
+	}
+
+	kbuf[count] =3D '\0';
+
+	if (kstrtol(kbuf, 0, &time.tv_sec)) {
+		kfree(kbuf);
+		return -EINVAL;
+	}
+
+	kfree(kbuf);
+
+	time.tv_usec =3D 0;
+
+	mutex_lock(&priv->mutex);
+
+	if (!time.tv_sec) {
+		if (priv->timer) {
+			mpic_free_timer(priv->timer);
+			priv->timer =3D NULL;
+		}
+		mutex_unlock(&priv->mutex);
+
+		return count;
+	}
+
+	if (priv->timer) {
+		mpic_free_timer(priv->timer);
+		priv->timer =3D NULL;
+	}
+
+	priv->timer =3D mpic_request_timer(timer_event_interrupt, priv,

&time);

=20
If the new time is zero, consider that a cancellation of the timer and
don't request a new one or return -EINVAL.
=20

Thanks, I think i should add comments. Let this patch easy to read.
Here is get a new timer.
If the new time is zero, consider that has been checked.

if (!time.tv_sec) {...} this is check zero.
The "mpic_request_timer" before this code.

 - Dongsheng

On 10/08/2012 02:13:26 AM, Wang Dongsheng-B40534 wrote:

=20
=20

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Thursday, October 04, 2012 6:20 AM
To: Kumar Gala
Cc: Wang Dongsheng; Wood Scott-B07421; =20

linuxppc-dev@lists.ozlabs.org list;

quoted

Wang Dongsheng-B40534; Li Yang-R58472; linux-pm@vger.kernel.org
Subject: Re: [RFC PATCH] powerpc/fsl: add timer wakeup source

On 10/03/2012 08:35:58 AM, Kumar Gala wrote:

quoted

On Oct 3, 2012, at 5:42 AM, Wang Dongsheng wrote:

quoted

This is only for freescale powerpc platform. The driver =20

provides a

quoted

quoted

way

quoted

to wake up system. Proc

interface(/proc/powerpc/wakeup_timer_seconds).

quoted

eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds", 5 seconds =20

after

quoted

quoted

quoted

the system will be woken up. echo another time into proc

interface

quoted

to update the time.

Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Li Yang <redacted>
---
arch/powerpc/platforms/Kconfig            |   23 +++
arch/powerpc/platforms/Makefile           |    1 +
arch/powerpc/platforms/fsl_timer_wakeup.c |  217

+++++++++++++++++++++++++++++

quoted

3 files changed, 241 insertions(+)
create mode 100644 arch/powerpc/platforms/fsl_timer_wakeup.c

Adding the Linux PM list to see if there is some existing support =20

for

quoted

quoted

this on other arch's in kernel.

I'm pretty sure /proc/ is NOT where we want this exposed.

Should probably go under the sysfs directory of the mpic device.  Or
better, make a generic interface for timer-based suspend wakeup (if =20

there

quoted

isn't one already).  This current approach sits in an unpleasant =20

middle

quoted

ground between generic and device-specific.
=09

/sys/power/wakeup_timer_seconds how about this?
I think it is a freescale generic interface, this interface control by
FSL_SOC && SUSPEND.

There's no such thing as a "Freescale generic interface".  Linux APIs =20
are not organized by hardware vendor.  Either make a truly generic =20
interface, reuse an existing one, or do something that is attached to =20
the specific driver.

quoted

quoted

quoted

diff --git a/arch/powerpc/platforms/Kconfig

b/arch/powerpc/platforms/Kconfig

quoted

index b190a6e..7b9232a 100644

--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig

@@ -99,6 +99,29 @@ config MPIC_TIMER

	  only tested on fsl chip, but it can potentially =20

support

quoted

quoted

quoted

	  other global timers complying to Open-PIC standard.

+menuconfig FSL_WAKEUP_SOURCE
+	bool "Freescale wakeup source"
+	depends on FSL_SOC && SUSPEND
+	default n
+	help
+	  This option enables wakeup source for wake up system
+	  features. This is only for freescale powerpc platform.
+
+if FSL_WAKEUP_SOURCE
+
+config FSL_TIMER_WAKEUP
+	tristate "Freescale mpic global timer wakeup event"
+	default n
+	help
+	  This is only for freescale powerpc platform. The =20

driver

quoted

quoted

quoted

+	  provides a way to wake up system.
+	  Proc interface(/proc/powerpc/wakeup_timer_seconds).
+	  eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds",
+	  5 seconds after the system will be woken up. echo =20

another

quoted

quoted

quoted

+	  time into proc interface to update the time.
+
+endif

Use depends rather than if/else.  Why do you need FSL_WAKEUP_SOURCE?

It lists all wake up source. If later have wakeup source can be =20
improved by
it to control. Buttons event wakeup source will be added after the =20
timer.

It does not list all wake up sources -- there's also ethernet, USB, etc.

quoted

These names are overly broad -- this is only for FSL MPIC, not for =20

other

quoted

FSL chips (e.g. mpc83xx has a different global timer =20

implementation, and

quoted

there's FSL ARM chips, etc).

Yes, thanks. Change to FSL_MPIC_TIMER_WAKEUP.
=20

quoted

quoted

quoted

+static ssize_t timer_wakeup_write(struct file *file, const char

__user *buf,

quoted

+		size_t count, loff_t *off)
+{
+	struct fsl_timer_wakeup *priv;
+	struct inode *inode =3D file->f_path.dentry->d_inode;
+	struct proc_dir_entry *dp;
+	struct timeval time;
+	char *kbuf;
+
+	dp =3D PDE(inode);
+	priv =3D dp->data;
+
+	kbuf =3D kzalloc(count + 1, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	if (copy_from_user(kbuf, buf, count)) {
+		kfree(kbuf);
+		return -EFAULT;
+	}
+
+	kbuf[count] =3D '\0';
+
+	if (kstrtol(kbuf, 0, &time.tv_sec)) {
+		kfree(kbuf);
+		return -EINVAL;
+	}
+
+	kfree(kbuf);
+
+	time.tv_usec =3D 0;
+
+	mutex_lock(&priv->mutex);
+
+	if (!time.tv_sec) {
+		if (priv->timer) {
+			mpic_free_timer(priv->timer);
+			priv->timer =3D NULL;
+		}
+		mutex_unlock(&priv->mutex);
+
+		return count;
+	}
+
+	if (priv->timer) {
+		mpic_free_timer(priv->timer);
+		priv->timer =3D NULL;
+	}
+
+	priv->timer =3D mpic_request_timer(timer_event_interrupt, =20

priv,

quoted

quoted

&time);

If the new time is zero, consider that a cancellation of the timer =20

and

quoted

don't request a new one or return -EINVAL.

Thanks, I think i should add comments. Let this patch easy to read.
Here is get a new timer.
If the new time is zero, consider that has been checked.
=20
if (!time.tv_sec) {...} this is check zero.
The "mpic_request_timer" before this code.

Ah, I see.  Wouldn't it be simpler to remove that block and just test =20
time.tv_sec when requesting the new timer?

-Scott=

DQo+ID4gPiA+IEknbSBwcmV0dHkgc3VyZSAvcHJvYy8gaXMgTk9UIHdoZXJlIHdlIHdhbnQgdGhp
cyBleHBvc2VkLg0KPiA+ID4NCj4gPiA+IFNob3VsZCBwcm9iYWJseSBnbyB1bmRlciB0aGUgc3lz
ZnMgZGlyZWN0b3J5IG9mIHRoZSBtcGljIGRldmljZS4gIE9yDQo+ID4gPiBiZXR0ZXIsIG1ha2Ug
YSBnZW5lcmljIGludGVyZmFjZSBmb3IgdGltZXItYmFzZWQgc3VzcGVuZCB3YWtldXAgKGlmDQo+
ID4gdGhlcmUNCj4gPiA+IGlzbid0IG9uZSBhbHJlYWR5KS4gIFRoaXMgY3VycmVudCBhcHByb2Fj
aCBzaXRzIGluIGFuIHVucGxlYXNhbnQNCj4gbWlkZGxlDQo+ID4gPiBncm91bmQgYmV0d2VlbiBn
ZW5lcmljIGFuZCBkZXZpY2Utc3BlY2lmaWMuDQo+ID4gPg0KPiA+IC9zeXMvcG93ZXIvd2FrZXVw
X3RpbWVyX3NlY29uZHMgaG93IGFib3V0IHRoaXM/DQo+ID4gSSB0aGluayBpdCBpcyBhIGZyZWVz
Y2FsZSBnZW5lcmljIGludGVyZmFjZSwgdGhpcyBpbnRlcmZhY2UgY29udHJvbCBieQ0KPiA+IEZT
TF9TT0MgJiYgU1VTUEVORC4NCg0KPiBUaGVyZSdzIG5vIHN1Y2ggdGhpbmcgYXMgYSAiRnJlZXNj
YWxlIGdlbmVyaWMgaW50ZXJmYWNlIi4gIExpbnV4IEFQSXMNCj4gYXJlIG5vdCBvcmdhbml6ZWQg
YnkgaGFyZHdhcmUgdmVuZG9yLiAgRWl0aGVyIG1ha2UgYSB0cnVseSBnZW5lcmljDQo+IGludGVy
ZmFjZSwgcmV1c2UgYW4gZXhpc3Rpbmcgb25lLCBvciBkbyBzb21ldGhpbmcgdGhhdCBpcyBhdHRh
Y2hlZCB0bw0KPiB0aGUgc3BlY2lmaWMgZHJpdmVyLg0KVGhhbmtzLCBJIHRoaW5rIGkgY2FuIGNo
YW5nZSBtcGljIHRpbWVyIHJlZ2lzdHJhdGlvbiwgaSB3aWxsIHVzZSBwbGF0Zm9ybV9kcml2ZXJf
cmVnaXN0ZXIuDQpJIHdpbGwgbWVyZ2UgIm1waWMgdGltZXIiIGFuZCAidGltZXIgd2FrZXVwIi4g
SW4gIm1waWMgdGltZXIiIHRoZSB3YWtldXAgZnVuY3Rpb24NCmNvbnRyb2xzIGJ5IFNVU1BFTkQu
IFRoZSBzeXMgcGF0aCBpcyAiL3N5cy9kZXZpY2VzL3NvYzg1NzIuNC9mZmU0MTEwMC50aW1lci93
YWtldXBfc2Vjb25kcyIuDQpkbyB5b3UgaGF2ZSBhbnkgc3VnZ2VzdGlvbnMgZm9yIHRoaXM/IG9y
IGhhdmUgYSBiZXR0ZXIgaWRlYSBhYm91dCB0aGUgc3lzIHBhdGg/DQoNCj4gPiA+IFVzZSBkZXBl
bmRzIHJhdGhlciB0aGFuIGlmL2Vsc2UuICBXaHkgZG8geW91IG5lZWQgRlNMX1dBS0VVUF9TT1VS
Q0U/DQo+ID4gPg0KPiA+IEl0IGxpc3RzIGFsbCB3YWtlIHVwIHNvdXJjZS4gSWYgbGF0ZXIgaGF2
ZSB3YWtldXAgc291cmNlIGNhbiBiZQ0KPiA+IGltcHJvdmVkIGJ5DQo+ID4gaXQgdG8gY29udHJv
bC4gQnV0dG9ucyBldmVudCB3YWtldXAgc291cmNlIHdpbGwgYmUgYWRkZWQgYWZ0ZXIgdGhlDQo+
ID4gdGltZXIuDQoNCj4gSXQgZG9lcyBub3QgbGlzdCBhbGwgd2FrZSB1cCBzb3VyY2VzIC0tIHRo
ZXJlJ3MgYWxzbyBldGhlcm5ldCwgVVNCLCBldGMuDQpmaW5lLCBjaGFuZ2UgdG8gZGVwZW5kcy4g
dGhhbmtzLg0KDQo+ID4gPg0KPiA+ID4gSWYgdGhlIG5ldyB0aW1lIGlzIHplcm8sIGNvbnNpZGVy
IHRoYXQgYSBjYW5jZWxsYXRpb24gb2YgdGhlIHRpbWVyDQo+ID4gYW5kDQo+ID4gPiBkb24ndCBy
ZXF1ZXN0IGEgbmV3IG9uZSBvciByZXR1cm4gLUVJTlZBTC4NCj4gPiA+DQo+ID4gVGhhbmtzLCBJ
IHRoaW5rIGkgc2hvdWxkIGFkZCBjb21tZW50cy4gTGV0IHRoaXMgcGF0Y2ggZWFzeSB0byByZWFk
Lg0KPiA+IEhlcmUgaXMgZ2V0IGEgbmV3IHRpbWVyLg0KPiA+IElmIHRoZSBuZXcgdGltZSBpcyB6
ZXJvLCBjb25zaWRlciB0aGF0IGhhcyBiZWVuIGNoZWNrZWQuDQo+ID4NCj4gPiBpZiAoIXRpbWUu
dHZfc2VjKSB7Li4ufSB0aGlzIGlzIGNoZWNrIHplcm8uDQo+ID4gVGhlICJtcGljX3JlcXVlc3Rf
dGltZXIiIGJlZm9yZSB0aGlzIGNvZGUuDQoNCj4gQWgsIEkgc2VlLiAgV291bGRuJ3QgaXQgYmUg
c2ltcGxlciB0byByZW1vdmUgdGhhdCBibG9jayBhbmQganVzdCB0ZXN0DQo+IHRpbWUudHZfc2Vj
IHdoZW4gcmVxdWVzdGluZyB0aGUgbmV3IHRpbWVyPw0KWWVzLCB3YWtlIHVwIGZ1bmN0aW9uIG9u
bHkgbmVlZCBzZWNvbmRzIGxldmVsLCBhbmQgaXQncyBqdXN0IGhvbGQgYSB0aW1lci4NCmVjaG8g
emVybyB0byBjYW5jZWwsIGFuZCBlY2hvIGEgbmV3IHRpbWUgdG8gdXBkYXRlIHRoZSB0aW1lciB0
aW1lLg==

On 10/09/2012 08:56:53 AM, Wang Dongsheng-B40534 wrote:

=20

quoted

quoted

quoted

quoted

I'm pretty sure /proc/ is NOT where we want this exposed.

Should probably go under the sysfs directory of the mpic =20

device.  Or

quoted

quoted

quoted

better, make a generic interface for timer-based suspend wakeup =20

(if

quoted

quoted

there

quoted

isn't one already).  This current approach sits in an unpleasant

middle

quoted

quoted

ground between generic and device-specific.

/sys/power/wakeup_timer_seconds how about this?
I think it is a freescale generic interface, this interface =20

control by

quoted

quoted

FSL_SOC && SUSPEND.

=20

quoted

There's no such thing as a "Freescale generic interface".  Linux =20

APIs

quoted

are not organized by hardware vendor.  Either make a truly generic
interface, reuse an existing one, or do something that is attached =20

to

quoted

the specific driver.

Thanks, I think i can change mpic timer registration, i will use =20
platform_driver_register.
I will merge "mpic timer" and "timer wakeup". In "mpic timer" the =20
wakeup function
controls by SUSPEND. The sys path is =20
"/sys/devices/soc8572.4/ffe41100.timer/wakeup_seconds".
do you have any suggestions for this? or have a better idea about the =20
sys path?

I'm not sure what you mean by the merging, but the path looks OK (I'd =20
prefer a dash rather than underscore, but I'm not sure what the usual =20
practice is in sysfs).

That said, this seems like something that could use a truly generic =20
interface.

-Scott=

On Wed, Oct 3, 2012 at 5:42 AM, Wang Dongsheng [off-list ref] wr=
ote:

Signed-off-by: Wang Dongsheng <redacted>

Wang,

Your patches must always have a From: line of your Freescale email
address.  Do not use your gmail.com address to send patches.

This needs to be fixed when this patch is applied.

--=20
Timur Tabi
Linux kernel developer at Freescale=