| Message ID | 1362728327-21013-3-git-send-email-dongsheng.wang@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kumar Gala |
| Headers | show |
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.
> -----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.
> -----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 :).
> -----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.
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
> -----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
> -----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.
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>");