> -----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...@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 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.w...@freescale.com>
> > > Signed-off-by: Li Yang <le...@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.
>
> 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.
>
/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
>
> 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).
>
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 = 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 the new time is zero, consider that a cancellation of the timer 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.
if (!time.tv_sec) {...} this is check zero.
The "mpic_request_timer" before this code.
- Dongsheng
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
