Patchwork [3/3] powerpc/fsl: add MPIC timer wakeup support

login
register
mail settings
Submitter Dongsheng Wang
Date March 8, 2013, 7:38 a.m.
Message ID <1362728327-21013-3-git-send-email-dongsheng.wang@freescale.com>
Download mbox | patch
Permalink /patch/226036/
State Superseded
Delegated to: Kumar Gala
Headers show

Comments

Dongsheng Wang - March 8, 2013, 7:38 a.m.
The driver provides a way to wake up the system by the MPIC timer.

For example,
echo 5 > /sys/devices/system/mpic/timer_wakeup
echo standby > /sys/power/state

After 5 seconds the MPIC timer will generate an interrupt to wake up
the system.

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>
---
 arch/powerpc/platforms/Kconfig              |    9 ++
 arch/powerpc/sysdev/Makefile                |    1 +
 arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c |  185 +++++++++++++++++++++++++++
 3 files changed, 195 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
Wang Dongsheng-B40534 - March 19, 2013, 6:25 a.m.
Hi Scoot,

Thanks for reviewing.

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, March 19, 2013 8:31 AM
> To: Wang Dongsheng-B40534
> Cc: Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-
> B40534; Zhao Chenhui-B35336; Li Yang-R58472
> Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> 
> 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.
> >
> > For example,
> > echo 5 > /sys/devices/system/mpic/timer_wakeup
> > echo standby > /sys/power/state
> >
> > After 5 seconds the MPIC timer will generate an interrupt to wake up
> > the system.
> >
> > 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,
> p1022, etc) or just regular sleep?
> 
The deep sleep can also work.

> > ---
> >  arch/powerpc/platforms/Kconfig              |    9 ++
> >  arch/powerpc/sysdev/Makefile                |    1 +
> >  arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c |  185
> > +++++++++++++++++++++++++++
> >  3 files changed, 195 insertions(+), 0 deletions(-)  create mode
> > 100644 arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
> >
> > diff --git a/arch/powerpc/platforms/Kconfig
> > 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.
> >
> > +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
> name and depends on "FSL_SOC && MPIC_TIMER".
> 
Um...yes, I will remove it.
 
> > +static irqreturn_t fsl_mpic_timer_irq(int irq, void *dev_id) {
> > +	struct fsl_mpic_timer_wakeup *wakeup = dev_id;
> > +
> > +	schedule_work(&wakeup->free_work);
> > +	return IRQ_HANDLED;
> > +}
> 
> return wakeup->timer ? IRQ_HANDLED : IRQ_NONE;
> 
Looks better, thanks.

> > +
> > +static ssize_t fsl_timer_wakeup_show(struct device *dev,
> > +				struct device_attribute *attr,
> > +				char *buf)
> > +{
> > +	struct timeval interval;
> > +	int val = 0;
> > +
> > +	mutex_lock(&sysfs_lock);
> > +	if (fsl_wakeup->timer) {
> > +		mpic_get_remain_time(fsl_wakeup->timer, &interval);
> > +		val = 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 = 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 attributes
> seem to, though...).
> 
I think we don't need to care about LF terminator.
The kstrtol--> _kstrtoull has been done.

> > +	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 = 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 = NULL;
> 	}
> 
> 	if (!interval.tv_sec) {
> 		mutex_unlock(&sysfs_lock);
> 		return count;
> 	}
> 
You can't break up the it.
if echo zero the code will cancel the timer that is currently running.
Not echo non-zero value just zero to cancel.

> > +	ret = subsys_system_register(&mpic_subsys, NULL);
> > +	if (ret)
> > +		goto err;
> 
> Maybe arch/powerpc/sysdev/mpic.c should be doing this?
> 
This can be registered by mpic. Maybe better than here.

> > +
> > +	for (i = 0; mpic_attributes[i]; i++) {
> > +		ret = device_create_file(mpic_subsys.dev_root,
> > +					mpic_attributes[i]);
> > +		if (ret)
> > +			goto err2;
> > +	}
> 
> Is this code ever going to register more than one?
> 
No, just one. I only keep the style here.
If you don't think it's necessary I can remove this loop.
Wang Dongsheng-B40534 - March 20, 2013, 3:48 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, March 20, 2013 6:55 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org;
> Zhao Chenhui-B35336; Li Yang-R58472
> Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> 
> On 03/19/2013 01:25:42 AM, Wang Dongsheng-B40534 wrote:
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Tuesday, March 19, 2013 8:31 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org; Wang
> > Dongsheng-
> > > B40534; Zhao Chenhui-B35336; Li Yang-R58472
> > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> > >
> > > On 03/08/2013 01:38:47 AM, Wang Dongsheng wrote:
> > > > +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 = 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
> > attributes
> > > seem to, though...).
> > >
> > I think we don't need to care about LF terminator.
> > The kstrtol--> _kstrtoull has been done.
> 
> My point is, what happens if userspace passes in a buffer that has no
> terminator of any sort?  kstrtol will continue reading beyond the end of
> the buffer.
> 
Do not care about terminator.

kstrtol--> _kstrtoull--> _parse_integer

_kstrtoull(...) {
	...
	rv = _parse_integer(s, base, &_res);
	if (rv & KSTRTOX_OVERFLOW)
		return -ERANGE;
	rv &= ~KSTRTOX_OVERFLOW;
	if (rv == 0)
		return -EINVAL;
	s += rv;

	if (*s == '\n')
		s++;
	if (*s)
		return -EINVAL;
	...
}

_parse_integer(...) {
	...
	while (*s) {
		if ('0' <= *s && *s <= '9')
			val = *s - '0';
		else if ('a' <= _tolower(*s) && _tolower(*s) <= 'f')
			val = _tolower(*s) - 'a' + 10;
		else
			break;	//this will break out to convert.

		if (val >= base)
			break;
	}
	...
}

> > > > +	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 = 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 = NULL;
> > > 	}
> > >
> > > 	if (!interval.tv_sec) {
> > > 		mutex_unlock(&sysfs_lock);
> > > 		return count;
> > > 	}
> > >
> > You can't break up the it.
> > if echo zero the code will cancel the timer that is currently running.
> > Not echo non-zero value just zero to cancel.
> 
> Echoing a nonzero value wouldn't just be to cancel, it would be to set a
> new timer after cancelling the old.
> 
If you think this way is better, I can change.
But why should do it? 
Explicitly stop the timer (echo 0) before reuse it is more reasonable for me. 

> > > > +	for (i = 0; mpic_attributes[i]; i++) {
> > > > +		ret = device_create_file(mpic_subsys.dev_root,
> > > > +					mpic_attributes[i]);
> > > > +		if (ret)
> > > > +			goto err2;
> > > > +	}
> > >
> > > Is this code ever going to register more than one?
> > >
> > No, just one. I only keep the style here.
> > If you don't think it's necessary I can remove this loop.
> 
> I don't think it's necessary.
> 
Ok, I will remove this loop.
Wang Dongsheng-B40534 - March 22, 2013, 5:46 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, March 21, 2013 5:49 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org;
> Zhao Chenhui-B35336; Li Yang-R58472
> Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> 
> On 03/19/2013 10:48:53 PM, Wang Dongsheng-B40534 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, March 20, 2013 6:55 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Wood Scott-B07421; Gala Kumar-B11780;
> > linuxppc-dev@lists.ozlabs.org;
> > > Zhao Chenhui-B35336; Li Yang-R58472
> > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> > >
> > > On 03/19/2013 01:25:42 AM, Wang Dongsheng-B40534 wrote:
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Tuesday, March 19, 2013 8:31 AM
> > > > > To: Wang Dongsheng-B40534
> > > > > Cc: Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org; Wang
> > > > Dongsheng-
> > > > > B40534; Zhao Chenhui-B35336; Li Yang-R58472
> > > > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup
> > support
> > > > >
> > > > > On 03/08/2013 01:38:47 AM, Wang Dongsheng wrote:
> > > > > > +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 = 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
> > > > attributes
> > > > > seem to, though...).
> > > > >
> > > > I think we don't need to care about LF terminator.
> > > > The kstrtol--> _kstrtoull has been done.
> > >
> > > My point is, what happens if userspace passes in a buffer that has
> > no
> > > terminator of any sort?  kstrtol will continue reading beyond the
> > end of
> > > the buffer.
> > >
> > Do not care about terminator.
> 
> kstrtol() obviously *does* because it doesn't take the buffer length as
> a parameter.
> 
> > kstrtol--> _kstrtoull--> _parse_integer
> >
> > _kstrtoull(...) {
> > 	...
> > 	rv = _parse_integer(s, base, &_res);
> > 	if (rv & KSTRTOX_OVERFLOW)
> > 		return -ERANGE;
> > 	rv &= ~KSTRTOX_OVERFLOW;
> > 	if (rv == 0)
> > 		return -EINVAL;
> > 	s += rv;
> >
> > 	if (*s == '\n')
> > 		s++;
> > 	if (*s)
> > 		return -EINVAL;
> > 	...
> > }
> >
> > _parse_integer(...) {
> > 	...
> > 	while (*s) {
> > 		if ('0' <= *s && *s <= '9')
> > 			val = *s - '0';
> > 		else if ('a' <= _tolower(*s) && _tolower(*s) <= 'f')
> > 			val = _tolower(*s) - 'a' + 10;
> > 		else
> > 			break;	//this will break out to convert.
> 
> Really?  How do you know that the next byte after the buffer isn't a
> valid hex digit?  How do you even know that we won't take a fault
> accessing it?
> 
Under what case is unsafe, please make sense.

"kstrtol" is used in almost of sysfs interface, I think it should be accepted in defaule :).
Wang Dongsheng-B40534 - March 26, 2013, 3:27 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Saturday, March 23, 2013 6:11 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org;
> Zhao Chenhui-B35336; Li Yang-R58472
> Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> 
> On 03/22/2013 12:46:24 AM, Wang Dongsheng-B40534 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Thursday, March 21, 2013 5:49 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Wood Scott-B07421; Gala Kumar-B11780;
> > linuxppc-dev@lists.ozlabs.org;
> > > Zhao Chenhui-B35336; Li Yang-R58472
> > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> > >
> > > On 03/19/2013 10:48:53 PM, Wang Dongsheng-B40534 wrote:
> > > > 	while (*s) {
> > > > 		if ('0' <= *s && *s <= '9')
> > > > 			val = *s - '0';
> > > > 		else if ('a' <= _tolower(*s) && _tolower(*s) <= 'f')
> > > > 			val = _tolower(*s) - 'a' + 10;
> > > > 		else
> > > > 			break;	//this will break out to convert.
> > >
> > > Really?  How do you know that the next byte after the buffer isn't a
> > > valid hex digit?  How do you even know that we won't take a fault
> > > accessing it?
> > >
> > Under what case is unsafe, please make sense.
> 
> char buffer[1] = { '5' };
> write(fd, &buffer, 1);
> 
> What comes after that '5' byte in the pointer you pass to kstrtol?
> 
The buffer is userspace. It will fall in the kernel space.
Kernel will get a free page, and copy the buffer to page.
This page has been cleared before copy to page.
The page has already have null-terminated.

> > "kstrtol" is used in almost of sysfs interface, I think it should be
> > accepted in defaule :).
> 
> Just because a lot of other people copy blindly doesn't make it right.
> Most of the examples I found use sscanf instead, though that has the same
> problem.
> 
> I do see a few instances of the "strings from sysfs write are not 0
> terminated!" in the comments, though (kernel/time/clocksource.c and
> kernel/rtmutex-tester.c).
> 
> Also "words written to sysfs files may, or may not, be \n terminated"
> in drivers/md/md.c.
> 
It's not "kstrtol" doesn't work as well, They do not belong to this kind
of scenarios.
Scott Wood - March 26, 2013, 5:35 p.m.
On 03/25/2013 10:27:24 PM, Wang Dongsheng-B40534 wrote:
> 
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Saturday, March 23, 2013 6:11 AM
> > To: Wang Dongsheng-B40534
> > Cc: Wood Scott-B07421; Gala Kumar-B11780;  
> linuxppc-dev@lists.ozlabs.org;
> > Zhao Chenhui-B35336; Li Yang-R58472
> > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> >
> > On 03/22/2013 12:46:24 AM, Wang Dongsheng-B40534 wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Thursday, March 21, 2013 5:49 AM
> > > > To: Wang Dongsheng-B40534
> > > > Cc: Wood Scott-B07421; Gala Kumar-B11780;
> > > linuxppc-dev@lists.ozlabs.org;
> > > > Zhao Chenhui-B35336; Li Yang-R58472
> > > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup  
> support
> > > >
> > > > On 03/19/2013 10:48:53 PM, Wang Dongsheng-B40534 wrote:
> > > > > 	while (*s) {
> > > > > 		if ('0' <= *s && *s <= '9')
> > > > > 			val = *s - '0';
> > > > > 		else if ('a' <= _tolower(*s) && _tolower(*s) <=  
> 'f')
> > > > > 			val = _tolower(*s) - 'a' + 10;
> > > > > 		else
> > > > > 			break;	//this will break out to  
> convert.
> > > >
> > > > Really?  How do you know that the next byte after the buffer  
> isn't a
> > > > valid hex digit?  How do you even know that we won't take a  
> fault
> > > > accessing it?
> > > >
> > > Under what case is unsafe, please make sense.
> >
> > char buffer[1] = { '5' };
> > write(fd, &buffer, 1);
> >
> > What comes after that '5' byte in the pointer you pass to kstrtol?
> >
> The buffer is userspace. It will fall in the kernel space.
> Kernel will get a free page, and copy the buffer to page.
> This page has been cleared before copy to page.
> The page has already have null-terminated.

It doesn't allocate a whole page, it uses kmalloc (not kzalloc!).  Even  
if kzalloc were used, a larger user buffer could be the exact size of  
the region that was allocated.

See memdup_user() in mm/util.c

-Scott
Wang Dongsheng-B40534 - March 27, 2013, 3:21 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, March 27, 2013 1:36 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org;
> Zhao Chenhui-B35336; Li Yang-R58472
> Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> 
> On 03/25/2013 10:27:24 PM, Wang Dongsheng-B40534 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Saturday, March 23, 2013 6:11 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Wood Scott-B07421; Gala Kumar-B11780;
> > linuxppc-dev@lists.ozlabs.org;
> > > Zhao Chenhui-B35336; Li Yang-R58472
> > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> > >
> > > On 03/22/2013 12:46:24 AM, Wang Dongsheng-B40534 wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Thursday, March 21, 2013 5:49 AM
> > > > > To: Wang Dongsheng-B40534
> > > > > Cc: Wood Scott-B07421; Gala Kumar-B11780;
> > > > linuxppc-dev@lists.ozlabs.org;
> > > > > Zhao Chenhui-B35336; Li Yang-R58472
> > > > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup
> > support
> > > > >
> > > > > On 03/19/2013 10:48:53 PM, Wang Dongsheng-B40534 wrote:
> > > > > > 	while (*s) {
> > > > > > 		if ('0' <= *s && *s <= '9')
> > > > > > 			val = *s - '0';
> > > > > > 		else if ('a' <= _tolower(*s) && _tolower(*s) <=
> > 'f')
> > > > > > 			val = _tolower(*s) - 'a' + 10;
> > > > > > 		else
> > > > > > 			break;	//this will break out to
> > convert.
> > > > >
> > > > > Really?  How do you know that the next byte after the buffer
> > isn't a
> > > > > valid hex digit?  How do you even know that we won't take a
> > fault
> > > > > accessing it?
> > > > >
> > > > Under what case is unsafe, please make sense.
> > >
> > > char buffer[1] = { '5' };
> > > write(fd, &buffer, 1);
> > >
> > > What comes after that '5' byte in the pointer you pass to kstrtol?
> > >
> > The buffer is userspace. It will fall in the kernel space.
> > Kernel will get a free page, and copy the buffer to page.
> > This page has been cleared before copy to page.
> > The page has already have null-terminated.
> 
> It doesn't allocate a whole page, it uses kmalloc (not kzalloc!).  Even
> if kzalloc were used, a larger user buffer could be the exact size of the
> region that was allocated.
> 
> See memdup_user() in mm/util.c
> 
Did you miss something?

See fill_write_buffer() in fs/sysfs/file.c. It's used get_zeroed_page()...

See SYSCALL_DEFINE3(write,...) in fs/read_write.c

[c0000000f1ff3a60] [c000000000008224] .show_stack+0x74/0x1b0 (unreliable)
[c0000000f1ff3b10] [c00000000002f370] .fsl_timer_wakeup_store+0x30/0x200
[c0000000f1ff3bc0] [c00000000030accc] .dev_attr_store+0x3c/0x50
[c0000000f1ff3c30] [c00000000018c47c] .sysfs_write_file+0xec/0x1f0
[c0000000f1ff3ce0] [c00000000010dfb4] .vfs_write+0xf4/0x1b0
[c0000000f1ff3d80] [c00000000010e360] .SyS_write+0x60/0xe0
[c0000000f1ff3e30] [c000000000000590] syscall_exit+0x0/0x80
Wang Dongsheng-B40534 - March 28, 2013, 3:09 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, March 28, 2013 4:26 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org;
> Zhao Chenhui-B35336; Li Yang-R58472
> Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> 
> On 03/26/2013 10:21:04 PM, Wang Dongsheng-B40534 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, March 27, 2013 1:36 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Wood Scott-B07421; Gala Kumar-B11780;
> > linuxppc-dev@lists.ozlabs.org;
> > > Zhao Chenhui-B35336; Li Yang-R58472
> > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> > >
> > > On 03/25/2013 10:27:24 PM, Wang Dongsheng-B40534 wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Saturday, March 23, 2013 6:11 AM
> > > > > To: Wang Dongsheng-B40534
> > > > > Cc: Wood Scott-B07421; Gala Kumar-B11780;
> > > > linuxppc-dev@lists.ozlabs.org;
> > > > > Zhao Chenhui-B35336; Li Yang-R58472
> > > > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup
> > support
> > > > >
> > > > > On 03/22/2013 12:46:24 AM, Wang Dongsheng-B40534 wrote:
> > > > > > Under what case is unsafe, please make sense.
> > > > >
> > > > > char buffer[1] = { '5' };
> > > > > write(fd, &buffer, 1);
> > > > >
> > > > > What comes after that '5' byte in the pointer you pass to
> > kstrtol?
> > > > >
> > > > The buffer is userspace. It will fall in the kernel space.
> > > > Kernel will get a free page, and copy the buffer to page.
> > > > This page has been cleared before copy to page.
> > > > The page has already have null-terminated.
> > >
> > > It doesn't allocate a whole page, it uses kmalloc (not kzalloc!).
> > Even
> > > if kzalloc were used, a larger user buffer could be the exact size
> > of the
> > > region that was allocated.
> > >
> > > See memdup_user() in mm/util.c
> > >
> > Did you miss something?
> > See fill_write_buffer() in fs/sysfs/file.c. It's used
> > get_zeroed_page()...
> 
> OK, I was looking at fs/sysfs/bin.c which is something slightly different.
> 
> fill_write_buffer() forces the size to be no more than "PAGE_SIZE - 1"
> so we know there's a terminator.
> 
> Perhaps kernel/rtmutex-tester.c and kernel/time/clocksource.c are
> similarly confused?
> 
Yes. But its depends on file->f_op.
See vfs_write in fs/read_write.c.

Patch

diff --git a/arch/powerpc/platforms/Kconfig 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.
 
+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. The driver
+	  provides a way to wake up the system by MPIC timer,
+	  e.g. "echo 5 > /sys/devices/system/mpic/timer_wakeup"
+
 config PPC_EPAPR_HV_PIC
 	bool
 	default n
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index ff6184a..e1b8a80 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -5,6 +5,7 @@  ccflags-$(CONFIG_PPC64)		:= -mno-minimal-toc
 mpic-msi-obj-$(CONFIG_PCI_MSI)	+= mpic_msi.o mpic_u3msi.o mpic_pasemi_msi.o
 obj-$(CONFIG_MPIC)		+= mpic.o $(mpic-msi-obj-y)
 obj-$(CONFIG_MPIC_TIMER)        += mpic_timer.o
+obj-$(CONFIG_FSL_MPIC_TIMER_WAKEUP)	+= fsl_mpic_timer_wakeup.o
 mpic-msgr-obj-$(CONFIG_MPIC_MSGR)	+= mpic_msgr.o
 obj-$(CONFIG_MPIC)		+= mpic.o $(mpic-msi-obj-y) $(mpic-msgr-obj-y)
 obj-$(CONFIG_PPC_EPAPR_HV_PIC)	+= ehv_pic.o
diff --git a/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c b/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
new file mode 100644
index 0000000..e94ba65
--- /dev/null
+++ b/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
@@ -0,0 +1,185 @@ 
+/*
+ * MPIC timer wakeup driver
+ *
+ * Copyright 2013 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+
+#include <asm/mpic_timer.h>
+
+struct fsl_mpic_timer_wakeup {
+	struct mpic_timer *timer;
+	struct work_struct free_work;
+};
+
+static struct fsl_mpic_timer_wakeup *fsl_wakeup;
+static DEFINE_MUTEX(sysfs_lock);
+
+static void fsl_free_resource(struct work_struct *ws)
+{
+	struct fsl_mpic_timer_wakeup *wakeup =
+		container_of(ws, struct fsl_mpic_timer_wakeup, free_work);
+
+	mutex_lock(&sysfs_lock);
+
+	if (wakeup->timer) {
+		disable_irq_wake(wakeup->timer->irq);
+		mpic_free_timer(wakeup->timer);
+	}
+
+	wakeup->timer = NULL;
+	mutex_unlock(&sysfs_lock);
+}
+
+static irqreturn_t fsl_mpic_timer_irq(int irq, void *dev_id)
+{
+	struct fsl_mpic_timer_wakeup *wakeup = dev_id;
+
+	schedule_work(&wakeup->free_work);
+	return IRQ_HANDLED;
+}
+
+static ssize_t fsl_timer_wakeup_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct timeval interval;
+	int val = 0;
+
+	mutex_lock(&sysfs_lock);
+	if (fsl_wakeup->timer) {
+		mpic_get_remain_time(fsl_wakeup->timer, &interval);
+		val = 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 = 0;
+	if (kstrtol(buf, 0, &interval.tv_sec))
+		return -EINVAL;
+
+	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 = NULL;
+		mutex_unlock(&sysfs_lock);
+
+		return count;
+	}
+
+	if (fsl_wakeup->timer) {
+		mutex_unlock(&sysfs_lock);
+		return -EBUSY;
+	}
+
+	fsl_wakeup->timer = mpic_request_timer(fsl_mpic_timer_irq,
+						fsl_wakeup, &interval);
+	if (!fsl_wakeup->timer) {
+		mutex_unlock(&sysfs_lock);
+		return -EINVAL;
+	}
+
+	ret = enable_irq_wake(fsl_wakeup->timer->irq);
+	if (ret) {
+		mpic_free_timer(fsl_wakeup->timer);
+		fsl_wakeup->timer = NULL;
+		mutex_unlock(&sysfs_lock);
+
+		return ret;
+	}
+	mpic_start_timer(fsl_wakeup->timer);
+
+	mutex_unlock(&sysfs_lock);
+
+	return count;
+}
+
+static struct bus_type mpic_subsys = {
+	.name = "mpic",
+	.dev_name = "mpic",
+};
+
+static DEVICE_ATTR(timer_wakeup, 0644,
+			fsl_timer_wakeup_show, fsl_timer_wakeup_store);
+
+static struct device_attribute *mpic_attributes[] = {
+	&dev_attr_timer_wakeup,
+	NULL
+};
+
+static int __init fsl_wakeup_sys_init(void)
+{
+	int ret;
+	int i;
+
+	fsl_wakeup = kzalloc(sizeof(struct fsl_mpic_timer_wakeup), GFP_KERNEL);
+	if (!fsl_wakeup)
+		return -ENOMEM;
+
+	INIT_WORK(&fsl_wakeup->free_work, fsl_free_resource);
+
+	ret = subsys_system_register(&mpic_subsys, NULL);
+	if (ret)
+		goto err;
+
+	for (i = 0; mpic_attributes[i]; i++) {
+		ret = device_create_file(mpic_subsys.dev_root,
+					mpic_attributes[i]);
+		if (ret)
+			goto err2;
+	}
+
+	return ret;
+
+err2:
+	while (--i >= 0)
+		device_remove_file(mpic_subsys.dev_root, mpic_attributes[i]);
+
+	bus_unregister(&mpic_subsys);
+
+err:
+	kfree(fsl_wakeup);
+
+	return ret;
+}
+
+static void __exit fsl_wakeup_sys_exit(void)
+{
+	int i;
+
+	for (i = 0; mpic_attributes[i]; i++)
+		device_remove_file(mpic_subsys.dev_root,
+				mpic_attributes[i]);
+	bus_unregister(&mpic_subsys);
+	kfree(fsl_wakeup);
+}
+
+module_init(fsl_wakeup_sys_init);
+module_exit(fsl_wakeup_sys_exit);
+
+MODULE_DESCRIPTION("Freescale MPIC global timer wakeup driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Wang Dongsheng <dongsheng.wang@freescale.com>");