* [PATCH] NUC900/rtc: change the waiting for device ready implement
@ 2010-05-18 9:10 Wan ZongShun
2010-05-18 9:24 ` Wan ZongShun
0 siblings, 1 reply; 6+ messages in thread
From: Wan ZongShun @ 2010-05-18 9:10 UTC (permalink / raw)
To: linux-arm-kernel
Dear Alessandro,
This patch is to change the waiting for device ready implement
for winbond nuc900 platform.
Thanks!
Signed-off-by:Wan ZongShun<mcuos.com@gmail.com>
---
drivers/rtc/rtc-nuc900.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/rtc/rtc-nuc900.c b/drivers/rtc/rtc-nuc900.c
index a351bd5..8684051 100644
--- a/drivers/rtc/rtc-nuc900.c
+++ b/drivers/rtc/rtc-nuc900.c
@@ -85,22 +85,21 @@ static irqreturn_t nuc900_rtc_interrupt(int irq, void *_rtc)
static int *check_rtc_access_enable(struct nuc900_rtc *nuc900_rtc)
{
- unsigned int i;
+ unsigned int i, timeout = 0x1000, val;
__raw_writel(INIRRESET, nuc900_rtc->rtc_reg + REG_RTC_INIR);
mdelay(10);
__raw_writel(AERPOWERON, nuc900_rtc->rtc_reg + REG_RTC_AER);
- for (i = 0; i < 1000; i++) {
- if (__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
- return 0;
- }
+ while (!(__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
+ && timeout--)
+ mdelay(1);
- if ((__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB) == 0x0)
- return ERR_PTR(-ENODEV);
+ if (timeout == 0x1000)
+ return ERR_PTR(-EPERM);
- return ERR_PTR(-EPERM);
+ return 0;
}
static void nuc900_rtc_bcd2bin(unsigned int timereg,
--
1.6.3.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] NUC900/rtc: change the waiting for device ready implement
2010-05-18 9:10 [PATCH] NUC900/rtc: change the waiting for device ready implement Wan ZongShun
@ 2010-05-18 9:24 ` Wan ZongShun
0 siblings, 0 replies; 6+ messages in thread
From: Wan ZongShun @ 2010-05-18 9:24 UTC (permalink / raw)
To: linux-arm-kernel
Dear sir,
Re-submitting the patch to fix careless bug.
Signed-off-by:Wan ZongShun<mcuos.com@gmail.com>
---
drivers/rtc/rtc-nuc900.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/rtc/rtc-nuc900.c b/drivers/rtc/rtc-nuc900.c
index a351bd5..21d1330 100644
--- a/drivers/rtc/rtc-nuc900.c
+++ b/drivers/rtc/rtc-nuc900.c
@@ -85,22 +85,21 @@ static irqreturn_t nuc900_rtc_interrupt(int irq, void *_rtc)
static int *check_rtc_access_enable(struct nuc900_rtc *nuc900_rtc)
{
- unsigned int i;
+ unsigned int i, timeout = 0x1000;
__raw_writel(INIRRESET, nuc900_rtc->rtc_reg + REG_RTC_INIR);
mdelay(10);
__raw_writel(AERPOWERON, nuc900_rtc->rtc_reg + REG_RTC_AER);
- for (i = 0; i < 1000; i++) {
- if (__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
- return 0;
- }
+ while (!(__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
+ && timeout--)
+ mdelay(1);
- if ((__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB) == 0x0)
- return ERR_PTR(-ENODEV);
+ if (!timeout)
+ return ERR_PTR(-EPERM);
- return ERR_PTR(-EPERM);
+ return 0;
}
static void nuc900_rtc_bcd2bin(unsigned int timereg,
--
1.6.3.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] NUC900/rtc: change the waiting for device ready implement
@ 2010-05-27 6:59 Wan ZongShun
2010-06-01 5:03 ` Wan ZongShun
2010-06-01 22:43 ` Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Wan ZongShun @ 2010-05-27 6:59 UTC (permalink / raw)
To: linux-arm-kernel
Dear Andrew,
This patch is only to change the waiting for device ready implement
for winbond nuc900 platform.
Signed-off-by:Wan ZongShun<mcuos.com@gmail.com>
---
drivers/rtc/rtc-nuc900.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/rtc/rtc-nuc900.c b/drivers/rtc/rtc-nuc900.c
index a351bd5..21d1330 100644
--- a/drivers/rtc/rtc-nuc900.c
+++ b/drivers/rtc/rtc-nuc900.c
@@ -85,22 +85,21 @@ static irqreturn_t nuc900_rtc_interrupt(int irq, void *_rtc)
static int *check_rtc_access_enable(struct nuc900_rtc *nuc900_rtc)
{
- unsigned int i;
+ unsigned int i, timeout = 0x1000;
__raw_writel(INIRRESET, nuc900_rtc->rtc_reg + REG_RTC_INIR);
mdelay(10);
__raw_writel(AERPOWERON, nuc900_rtc->rtc_reg + REG_RTC_AER);
- for (i = 0; i < 1000; i++) {
- if (__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
- return 0;
- }
+ while (!(__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
+ && timeout--)
+ mdelay(1);
- if ((__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB) == 0x0)
- return ERR_PTR(-ENODEV);
+ if (!timeout)
+ return ERR_PTR(-EPERM);
- return ERR_PTR(-EPERM);
+ return 0;
}
static void nuc900_rtc_bcd2bin(unsigned int timereg,
--
1.6.3.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] NUC900/rtc: change the waiting for device ready implement
2010-05-27 6:59 Wan ZongShun
@ 2010-06-01 5:03 ` Wan ZongShun
2010-06-01 22:43 ` Andrew Morton
1 sibling, 0 replies; 6+ messages in thread
From: Wan ZongShun @ 2010-06-01 5:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi Andrew,
How about this patch?
Do you think this change is necessary?
? 2010?5?27? ??2:59?Wan ZongShun <mcuos.com@gmail.com> ???
> Dear Andrew,
>
> This patch is only to change the waiting for device ready implement
> for winbond nuc900 platform.
>
> Signed-off-by:Wan ZongShun<mcuos.com@gmail.com>
>
> ---
> ?drivers/rtc/rtc-nuc900.c | ? 15 +++++++--------
> ?1 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/rtc/rtc-nuc900.c b/drivers/rtc/rtc-nuc900.c
> index a351bd5..21d1330 100644
> --- a/drivers/rtc/rtc-nuc900.c
> +++ b/drivers/rtc/rtc-nuc900.c
> @@ -85,22 +85,21 @@ static irqreturn_t nuc900_rtc_interrupt(int irq, void *_rtc)
>
> ?static int *check_rtc_access_enable(struct nuc900_rtc *nuc900_rtc)
> ?{
> - ? ? ? unsigned int i;
> + ? ? ? unsigned int i, timeout = 0x1000;
> ? ? ? ?__raw_writel(INIRRESET, nuc900_rtc->rtc_reg + REG_RTC_INIR);
>
> ? ? ? ?mdelay(10);
>
> ? ? ? ?__raw_writel(AERPOWERON, nuc900_rtc->rtc_reg + REG_RTC_AER);
>
> - ? ? ? for (i = 0; i < 1000; i++) {
> - ? ? ? ? ? ? ? if (__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
> - ? ? ? ? ? ? ? ? ? ? ? return 0;
> - ? ? ? }
> + ? ? ? while (!(__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? && timeout--)
> + ? ? ? ? ? ? ? mdelay(1);
>
> - ? ? ? if ((__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB) == 0x0)
> - ? ? ? ? ? ? ? return ERR_PTR(-ENODEV);
> + ? ? ? if (!timeout)
> + ? ? ? ? ? ? ? return ERR_PTR(-EPERM);
>
> - ? ? ? return ERR_PTR(-EPERM);
> + ? ? ? return 0;
> ?}
>
> ?static void nuc900_rtc_bcd2bin(unsigned int timereg,
> --
> 1.6.3.3
>
--
*linux-arm-kernel mailing list
mail addr:linux-arm-kernel at lists.infradead.org
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
* linux-arm-NUC900 mailing list
mail addr:NUC900 at googlegroups.com
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
mcuos.com at gmail.com
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] NUC900/rtc: change the waiting for device ready implement
2010-05-27 6:59 Wan ZongShun
2010-06-01 5:03 ` Wan ZongShun
@ 2010-06-01 22:43 ` Andrew Morton
2010-06-02 7:06 ` Wan ZongShun
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2010-06-01 22:43 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 27 May 2010 14:59:31 +0800
Wan ZongShun <mcuos.com@gmail.com> wrote:
> Dear Andrew,
>
> This patch is only to change the waiting for device ready implement
> for winbond nuc900 platform.
>
It's not very helpful to only say "I changed it". _why_ did you change
it? What change did you make? What was wrong with the old code and
what's better about the new code?
> --- a/drivers/rtc/rtc-nuc900.c
> +++ b/drivers/rtc/rtc-nuc900.c
> @@ -85,22 +85,21 @@ static irqreturn_t nuc900_rtc_interrupt(int irq, void *_rtc)
>
> static int *check_rtc_access_enable(struct nuc900_rtc *nuc900_rtc)
> {
> - unsigned int i;
> + unsigned int i, timeout = 0x1000;
> __raw_writel(INIRRESET, nuc900_rtc->rtc_reg + REG_RTC_INIR);
>
> mdelay(10);
>
> __raw_writel(AERPOWERON, nuc900_rtc->rtc_reg + REG_RTC_AER);
>
> - for (i = 0; i < 1000; i++) {
> - if (__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
> - return 0;
> - }
> + while (!(__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
> + && timeout--)
> + mdelay(1);
>
> - if ((__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB) == 0x0)
> - return ERR_PTR(-ENODEV);
> + if (!timeout)
> + return ERR_PTR(-EPERM);
>
> - return ERR_PTR(-EPERM);
> + return 0;
> }
I can see that the patch makes two changes: it adds an mdelay(1) to the
polling loop and it changes the return value from ENODEV to EPERM if
the loop timed out.
But I don't have the faintest idea _why_ the changes were made!
So. I merged the patch without a changelog. Please send a changelog
for this patch which permits others to understand the change.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] NUC900/rtc: change the waiting for device ready implement
2010-06-01 22:43 ` Andrew Morton
@ 2010-06-02 7:06 ` Wan ZongShun
0 siblings, 0 replies; 6+ messages in thread
From: Wan ZongShun @ 2010-06-02 7:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi Andrew ,
The patch makes two changes:
(1) it adds an mdelay(1) to the polling loop
Here I change the 'for' loop to 'while' loop and and add an mdelay(1),
which can make less access to the hardware register.
(2) it changes the return value from ENODEV to EPERM if the loop timed out.
I think the 'Operation not permitted' description is more suitable for
the meaning
of 'check_rtc_access_enable()' function, it just be used to judge rtc
access operation is permitted or not.
2010/6/2 Andrew Morton <akpm@linux-foundation.org>:
> On Thu, 27 May 2010 14:59:31 +0800
> Wan ZongShun <mcuos.com@gmail.com> wrote:
>
>> Dear Andrew,
>>
>> This patch is only to change the waiting for device ready implement
>> for winbond nuc900 platform.
>>
>
> It's not very helpful to only say "I changed it". ?_why_ did you change
> it? ?What change did you make? ?What was wrong with the old code and
> what's better about the new code?
>
>> --- a/drivers/rtc/rtc-nuc900.c
>> +++ b/drivers/rtc/rtc-nuc900.c
>> @@ -85,22 +85,21 @@ static irqreturn_t nuc900_rtc_interrupt(int irq, void *_rtc)
>>
>> ?static int *check_rtc_access_enable(struct nuc900_rtc *nuc900_rtc)
>> ?{
>> - ? ? unsigned int i;
>> + ? ? unsigned int i, timeout = 0x1000;
>> ? ? ? __raw_writel(INIRRESET, nuc900_rtc->rtc_reg + REG_RTC_INIR);
>>
>> ? ? ? mdelay(10);
>>
>> ? ? ? __raw_writel(AERPOWERON, nuc900_rtc->rtc_reg + REG_RTC_AER);
>>
>> - ? ? for (i = 0; i < 1000; i++) {
>> - ? ? ? ? ? ? if (__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
>> - ? ? ? ? ? ? ? ? ? ? return 0;
>> - ? ? }
>> + ? ? while (!(__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? && timeout--)
>> + ? ? ? ? ? ? mdelay(1);
>>
>> - ? ? if ((__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB) == 0x0)
>> - ? ? ? ? ? ? return ERR_PTR(-ENODEV);
>> + ? ? if (!timeout)
>> + ? ? ? ? ? ? return ERR_PTR(-EPERM);
>>
>> - ? ? return ERR_PTR(-EPERM);
>> + ? ? return 0;
>> }
>
> I can see that the patch makes two changes: it adds an mdelay(1) to the
> polling loop and it changes the return value from ENODEV to EPERM if
> the loop timed out.
>
> But I don't have the faintest idea _why_ the changes were made!
>
>
> So. ?I merged the patch without a changelog. ?Please send a changelog
> for this patch which permits others to understand the change.
>
>
--
*linux-arm-kernel mailing list
mail addr:linux-arm-kernel at lists.infradead.org
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
* linux-arm-NUC900 mailing list
mail addr:NUC900 at googlegroups.com
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
mcuos.com at gmail.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-06-02 7:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-18 9:10 [PATCH] NUC900/rtc: change the waiting for device ready implement Wan ZongShun
2010-05-18 9:24 ` Wan ZongShun
-- strict thread matches above, loose matches on Subject: below --
2010-05-27 6:59 Wan ZongShun
2010-06-01 5:03 ` Wan ZongShun
2010-06-01 22:43 ` Andrew Morton
2010-06-02 7:06 ` Wan ZongShun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).