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

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

[Wang Dongsheng]

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 <dongsheng.wang@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 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

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

[Kumar Gala]

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 <dongsheng.wang@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> 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

>=20
> 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.
>=20
> +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) :=3D -Werror
>=20
> obj-$(CONFIG_FSL_ULI1575)	+=3D fsl_uli1575.o
> +obj-$(CONFIG_FSL_TIMER_WAKEUP)	+=3D fsl_timer_wakeup.o
>=20
> obj-$(CONFIG_PPC_PMAC)		+=3D powermac/
> obj-$(CONFIG_PPC_CHRP)		+=3D 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 =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 <dongsheng.wang@freescale.com>");
> --=20
> 1.7.9.5

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

[Scott Wood]

On 10/03/2012 08:35:58 AM, Kumar Gala wrote:
>=20
> On Oct 3, 2012, at 5:42 AM, Wang Dongsheng wrote:
>=20
> > This is only for freescale powerpc platform. The driver provides a =20
> way
> > to wake up system. Proc =20
> 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 =20
> interface
> > to update the time.
> >
> > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> > arch/powerpc/platforms/Kconfig            |   23 +++
> > arch/powerpc/platforms/Makefile           |    1 +
> > arch/powerpc/platforms/fsl_timer_wakeup.c |  217 =20
> +++++++++++++++++++++++++++++
> > 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.

> > diff --git a/arch/powerpc/platforms/Kconfig =20
> 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

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).

> > +static ssize_t timer_wakeup_write(struct file *file, const char =20
> __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, =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=

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

[Wang Dongsheng-B40534]

> -----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:
> >
> > 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).
> > >
> > > 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 <dongsheng.wang@freescale.com>
> > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > ---
> > > 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.
>=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.

> > > 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
>=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.

> > > +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);
>=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

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

[Scott Wood]

On 10/08/2012 02:13:26 AM, Wang Dongsheng-B40534 wrote:
>=20
>=20
> > -----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;
> > 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:
> > >
> > > On Oct 3, 2012, at 5:42 AM, Wang Dongsheng wrote:
> > >
> > > > This is only for freescale powerpc platform. The driver =20
> provides a
> > > way
> > > > to wake up system. Proc
> > > interface(/proc/powerpc/wakeup_timer_seconds).
> > > >
> > > > eg: "echo 5 > /proc/powerpc/wakeup_timer_seconds", 5 seconds =20
> after
> > > > the system will be woken up. echo another time into proc
> > > interface
> > > > to update the time.
> > > >
> > > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > > ---
> > > > 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 =20
> for
> > > 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
> > isn't one already).  This current approach sits in an unpleasant =20
> 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.

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.

> > > > 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 =20
> 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 =20
> 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 =20
> another
> > > > +	  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.

> > 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).
> >
> Yes, thanks. Change to FSL_MPIC_TIMER_WAKEUP.
>=20
> > > > +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, =20
> priv,
> > > &time);
> >
> > If the new time is zero, consider that a cancellation of the timer =20
> and
> > 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=

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

[Wang Dongsheng-B40534]

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==

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

[Scott Wood]

On 10/09/2012 08:56:53 AM, Wang Dongsheng-B40534 wrote:
>=20
> > > > > 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
> > > > better, make a generic interface for timer-based suspend wakeup =20
> (if
> > > there
> > > > isn't one already).  This current approach sits in an unpleasant
> > middle
> > > > 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
> > > FSL_SOC && SUSPEND.
>=20
> > There's no such thing as a "Freescale generic interface".  Linux =20
> APIs
> > are not organized by hardware vendor.  Either make a truly generic
> > interface, reuse an existing one, or do something that is attached =20
> to
> > 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=

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

[Tabi Timur-B04825]

On Wed, Oct 3, 2012 at 5:42 AM, Wang Dongsheng <dongsheng.wds@gmail.com> wr=
ote:
>
>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>

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=