[U-Boot] [PATCH 1/1] Changes for fixing the timer to 1 msec and
The cmd_misc.c was changed to fix sleep. nand_base.c was fixed to fix the wait.
Signed-off-by: Manikandan Pillai mani.pillai@ti.com --- common/cmd_misc.c | 1 + cpu/arm_cortexa8/omap3/interrupts.c | 13 ++++--------- drivers/mtd/nand/nand_base.c | 2 ++ include/configs/omap3_evm.h | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/common/cmd_misc.c b/common/cmd_misc.c index 024299a..e3e64c0 100644 --- a/common/cmd_misc.c +++ b/common/cmd_misc.c @@ -38,6 +38,7 @@ int do_sleep (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) }
delay = simple_strtoul(argv[1], NULL, 10) * CONFIG_SYS_HZ; + delay += get_timer(0);
while (get_timer(start) < delay) { if (ctrlc ()) { diff --git a/cpu/arm_cortexa8/omap3/interrupts.c b/cpu/arm_cortexa8/omap3/interrupts.c index 9e9817d..5bfe6fc 100644 --- a/cpu/arm_cortexa8/omap3/interrupts.c +++ b/cpu/arm_cortexa8/omap3/interrupts.c @@ -193,7 +193,7 @@ void reset_timer(void)
ulong get_timer(ulong base) { - return get_timer_masked() - base; + return get_timer_masked(); }
void set_timer(ulong t) @@ -238,14 +238,9 @@ void reset_timer_masked(void)
ulong get_timer_masked(void) { - ulong now = readl(&timer_base->tcrr); /* current tick value */ - - if (now >= lastinc) /* normal mode (non roll) */ - /* move stamp fordward with absoulte diff ticks */ - timestamp += (now - lastinc); - else /* we have rollover of incrementer */ - timestamp += (0xFFFFFFFF - lastinc) + now; - lastinc = now; + timestamp = readl(&timer_base->tcrr); /* current tick value */ + /* get_timer() has to return time in milliseconds */ + timestamp >>= 6; return timestamp; }
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index d33fee2..283b456 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -509,6 +509,7 @@ void nand_wait_ready(struct mtd_info *mtd) struct nand_chip *chip = mtd->priv; u32 timeo = (CONFIG_SYS_HZ * 20) / 1000;
+ timeo += get_timer(0); reset_timer();
/* wait until command is processed or timeout occures */ @@ -854,6 +855,7 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *this) else timeo = (CONFIG_SYS_HZ * 20) / 1000;
+ timeo += get_timer(0); if ((state == FL_ERASING) && (this->options & NAND_IS_AND)) this->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1); else diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h index bf5614e..8d6e5fe 100644 --- a/include/configs/omap3_evm.h +++ b/include/configs/omap3_evm.h @@ -230,7 +230,7 @@
#define CONFIG_SYS_TIMERBASE OMAP34XX_GPT2 #define CONFIG_SYS_PVT V_PVT /* 2^(pvt+1) */ -#define CONFIG_SYS_HZ ((V_SCLK) / (2 << CONFIG_SYS_PVT)) +#define CONFIG_SYS_HZ 1000
/*----------------------------------------------------------------------- * Stack sizes
Hi Mani,
Manikandan Pillai wrote:
The cmd_misc.c was changed to fix sleep. nand_base.c was fixed to fix the wait.
Should this be the fix for
http://lists.denx.de/pipermail/u-boot/2009-February/048107.html
?
I'm not familiar with U-Boot timer handling, so just some questions. Maybe the U-Boot experts like to comment, too.
Signed-off-by: Manikandan Pillai mani.pillai@ti.com
common/cmd_misc.c | 1 + cpu/arm_cortexa8/omap3/interrupts.c | 13 ++++--------- drivers/mtd/nand/nand_base.c | 2 ++ include/configs/omap3_evm.h | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/common/cmd_misc.c b/common/cmd_misc.c index 024299a..e3e64c0 100644 --- a/common/cmd_misc.c +++ b/common/cmd_misc.c @@ -38,6 +38,7 @@ int do_sleep (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) }
delay = simple_strtoul(argv[1], NULL, 10) * CONFIG_SYS_HZ;
- delay += get_timer(0);
As I understand above error report there is an issue with OMAP3 timer not returning ms. Why do we have to touch cmd_misc.c for *all* architectures then?
while (get_timer(start) < delay) { if (ctrlc ()) { diff --git a/cpu/arm_cortexa8/omap3/interrupts.c b/cpu/arm_cortexa8/omap3/interrupts.c index 9e9817d..5bfe6fc 100644 --- a/cpu/arm_cortexa8/omap3/interrupts.c +++ b/cpu/arm_cortexa8/omap3/interrupts.c @@ -193,7 +193,7 @@ void reset_timer(void)
ulong get_timer(ulong base) {
- return get_timer_masked() - base;
- return get_timer_masked();
Why don't we have to deal with base here any more?
}
void set_timer(ulong t) @@ -238,14 +238,9 @@ void reset_timer_masked(void)
ulong get_timer_masked(void) {
- ulong now = readl(&timer_base->tcrr); /* current tick value */
- if (now >= lastinc) /* normal mode (non roll) */
/* move stamp fordward with absoulte diff ticks */timestamp += (now - lastinc);- else /* we have rollover of incrementer */
timestamp += (0xFFFFFFFF - lastinc) + now;- lastinc = now;
- timestamp = readl(&timer_base->tcrr); /* current tick value */
- /* get_timer() has to return time in milliseconds */
- timestamp >>= 6;
Why don't we have to deal with timer overflow here any more?
As mentioned I haven't looked into the details, but why wouldn't it be sufficient to just change first line of get_timer_masked() to something like
ulong now = readl(&timer_base->tcrr) >> 6; /* current tick value */
to get ms?
return timestamp; }
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index d33fee2..283b456 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -509,6 +509,7 @@ void nand_wait_ready(struct mtd_info *mtd) struct nand_chip *chip = mtd->priv; u32 timeo = (CONFIG_SYS_HZ * 20) / 1000;
- timeo += get_timer(0); reset_timer();
Same as above here and below: Why do we have to touch global code for *all* architectures?
/* wait until command is processed or timeout occures */ @@ -854,6 +855,7 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *this) else timeo = (CONFIG_SYS_HZ * 20) / 1000;
- timeo += get_timer(0); if ((state == FL_ERASING) && (this->options & NAND_IS_AND)) this->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1); else
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h index bf5614e..8d6e5fe 100644 --- a/include/configs/omap3_evm.h +++ b/include/configs/omap3_evm.h @@ -230,7 +230,7 @@
#define CONFIG_SYS_TIMERBASE OMAP34XX_GPT2 #define CONFIG_SYS_PVT V_PVT /* 2^(pvt+1) */ -#define CONFIG_SYS_HZ ((V_SCLK) / (2 << CONFIG_SYS_PVT)) +#define CONFIG_SYS_HZ 1000
Would be nice if you could change this for all OMAP3 boards.
Best regards
Dirk
Hi,
I was kind of concerned that I am changing some of the common files.
Got into a loop while tring to fix some issue for OMAP3.
I have got some queries on timer.
1. Is the timer interrupt enabled and tick incremented after every expiry. In the patch, I have tried to return the ms count of the free running timer. The timer 0 is a free running timer and it overflows at 0xffffffff. The count read from the timer is right shifted by 6 to give the count in msec.
2. Since the delay is given in msec, I add the delay with the get_timer(0) count So that the approximately correct delay can be counted
3. I would like to know the constraints for the get_timer()
Regards Mani
-----Original Message----- From: Dirk Behme [mailto:dirk.behme@googlemail.com] Sent: Tuesday, March 10, 2009 11:27 PM To: Pillai, Manikandan Cc: u-boot@lists.denx.de Subject: Re: [PATCH 1/1] Changes for fixing the timer to 1 msec and
Hi Mani,
Manikandan Pillai wrote:
The cmd_misc.c was changed to fix sleep. nand_base.c was fixed to fix the wait.
Should this be the fix for
http://lists.denx.de/pipermail/u-boot/2009-February/048107.html
?
I'm not familiar with U-Boot timer handling, so just some questions. Maybe the U-Boot experts like to comment, too.
Signed-off-by: Manikandan Pillai mani.pillai@ti.com
common/cmd_misc.c | 1 + cpu/arm_cortexa8/omap3/interrupts.c | 13 ++++--------- drivers/mtd/nand/nand_base.c | 2 ++ include/configs/omap3_evm.h | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/common/cmd_misc.c b/common/cmd_misc.c index 024299a..e3e64c0 100644 --- a/common/cmd_misc.c +++ b/common/cmd_misc.c @@ -38,6 +38,7 @@ int do_sleep (cmd_tbl_t *cmdtp, int flag, int argc, char
*argv[])
}
delay = simple_strtoul(argv[1], NULL, 10) * CONFIG_SYS_HZ;
- delay += get_timer(0);
As I understand above error report there is an issue with OMAP3 timer not returning ms. Why do we have to touch cmd_misc.c for *all* architectures then?
while (get_timer(start) < delay) { if (ctrlc ()) { diff --git a/cpu/arm_cortexa8/omap3/interrupts.c
b/cpu/arm_cortexa8/omap3/interrupts.c
index 9e9817d..5bfe6fc 100644 --- a/cpu/arm_cortexa8/omap3/interrupts.c +++ b/cpu/arm_cortexa8/omap3/interrupts.c @@ -193,7 +193,7 @@ void reset_timer(void)
ulong get_timer(ulong base) {
- return get_timer_masked() - base;
- return get_timer_masked();
Why don't we have to deal with base here any more?
}
void set_timer(ulong t) @@ -238,14 +238,9 @@ void reset_timer_masked(void)
ulong get_timer_masked(void) {
- ulong now = readl(&timer_base->tcrr); /* current tick value */
- if (now >= lastinc) /* normal mode (non roll) */
/* move stamp fordward with absoulte diff ticks */timestamp += (now - lastinc);- else /* we have rollover of incrementer */
timestamp += (0xFFFFFFFF - lastinc) + now;- lastinc = now;
- timestamp = readl(&timer_base->tcrr); /* current tick value */
- /* get_timer() has to return time in milliseconds */
- timestamp >>= 6;
Why don't we have to deal with timer overflow here any more?
As mentioned I haven't looked into the details, but why wouldn't it be sufficient to just change first line of get_timer_masked() to something like
ulong now = readl(&timer_base->tcrr) >> 6; /* current tick value */
to get ms?
return timestamp; }
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index d33fee2..283b456 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -509,6 +509,7 @@ void nand_wait_ready(struct mtd_info *mtd) struct nand_chip *chip = mtd->priv; u32 timeo = (CONFIG_SYS_HZ * 20) / 1000;
- timeo += get_timer(0); reset_timer();
Same as above here and below: Why do we have to touch global code for *all* architectures?
/* wait until command is processed or timeout occures */ @@ -854,6 +855,7 @@ static int nand_wait(struct mtd_info *mtd, struct
nand_chip *this)
else timeo = (CONFIG_SYS_HZ * 20) / 1000;
- timeo += get_timer(0); if ((state == FL_ERASING) && (this->options & NAND_IS_AND)) this->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1); else
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h index bf5614e..8d6e5fe 100644 --- a/include/configs/omap3_evm.h +++ b/include/configs/omap3_evm.h @@ -230,7 +230,7 @@
#define CONFIG_SYS_TIMERBASE OMAP34XX_GPT2 #define CONFIG_SYS_PVT V_PVT /* 2^(pvt+1) */ -#define CONFIG_SYS_HZ ((V_SCLK) / (2 << CONFIG_SYS_PVT)) +#define CONFIG_SYS_HZ 1000
Would be nice if you could change this for all OMAP3 boards.
Best regards
Dirk
Dear Mani,
Pillai, Manikandan wrote:
Hi,
I was kind of concerned that I am changing some of the common files.
Got into a loop while tring to fix some issue for OMAP3.
I have got some queries on timer.
Is the timer interrupt enabled and tick incremented after every expiry. In the patch, I have tried to return the ms count of the free running timer. The timer 0 is a free running timer and it overflows at 0xffffffff. The count read from the timer is right shifted by 6 to give the count in msec.
Since the delay is given in msec, I add the delay with the get_timer(0) count So that the approximately correct delay can be counted
I would like to know the constraints for the get_timer()
I'm not sure if there is any documentation how U-Boot's timer code should work. If not, and if nobody else likes to give some hints, the best way would be to look how other boards do it. I don't know what the best example would be, but looking at OMAP1, OMAP2 or DaVinci could help. Sorry, but as already mentioned I'm not familiar with U-Boot's timer code, too. Looks like the timer code actually used is the more or less the initial port from TI.
Best regards
Dirk
-----Original Message----- From: Dirk Behme [mailto:dirk.behme@googlemail.com] Sent: Tuesday, March 10, 2009 11:27 PM To: Pillai, Manikandan Cc: u-boot@lists.denx.de Subject: Re: [PATCH 1/1] Changes for fixing the timer to 1 msec and
Hi Mani,
Manikandan Pillai wrote:
The cmd_misc.c was changed to fix sleep. nand_base.c was fixed to fix the wait.
Should this be the fix for
http://lists.denx.de/pipermail/u-boot/2009-February/048107.html
?
I'm not familiar with U-Boot timer handling, so just some questions. Maybe the U-Boot experts like to comment, too.
Signed-off-by: Manikandan Pillai mani.pillai@ti.com
common/cmd_misc.c | 1 + cpu/arm_cortexa8/omap3/interrupts.c | 13 ++++--------- drivers/mtd/nand/nand_base.c | 2 ++ include/configs/omap3_evm.h | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/common/cmd_misc.c b/common/cmd_misc.c index 024299a..e3e64c0 100644 --- a/common/cmd_misc.c +++ b/common/cmd_misc.c @@ -38,6 +38,7 @@ int do_sleep (cmd_tbl_t *cmdtp, int flag, int argc, char
*argv[])
}
delay = simple_strtoul(argv[1], NULL, 10) * CONFIG_SYS_HZ;
- delay += get_timer(0);
As I understand above error report there is an issue with OMAP3 timer not returning ms. Why do we have to touch cmd_misc.c for *all* architectures then?
while (get_timer(start) < delay) { if (ctrlc ()) { diff --git a/cpu/arm_cortexa8/omap3/interrupts.c
b/cpu/arm_cortexa8/omap3/interrupts.c
index 9e9817d..5bfe6fc 100644 --- a/cpu/arm_cortexa8/omap3/interrupts.c +++ b/cpu/arm_cortexa8/omap3/interrupts.c @@ -193,7 +193,7 @@ void reset_timer(void)
ulong get_timer(ulong base) {
- return get_timer_masked() - base;
- return get_timer_masked();
Why don't we have to deal with base here any more?
}
void set_timer(ulong t) @@ -238,14 +238,9 @@ void reset_timer_masked(void)
ulong get_timer_masked(void) {
- ulong now = readl(&timer_base->tcrr); /* current tick value */
- if (now >= lastinc) /* normal mode (non roll) */
/* move stamp fordward with absoulte diff ticks */timestamp += (now - lastinc);- else /* we have rollover of incrementer */
timestamp += (0xFFFFFFFF - lastinc) + now;- lastinc = now;
- timestamp = readl(&timer_base->tcrr); /* current tick value */
- /* get_timer() has to return time in milliseconds */
- timestamp >>= 6;
Why don't we have to deal with timer overflow here any more?
As mentioned I haven't looked into the details, but why wouldn't it be sufficient to just change first line of get_timer_masked() to something like
ulong now = readl(&timer_base->tcrr) >> 6; /* current tick value */
to get ms?
return timestamp; }
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index d33fee2..283b456 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -509,6 +509,7 @@ void nand_wait_ready(struct mtd_info *mtd) struct nand_chip *chip = mtd->priv; u32 timeo = (CONFIG_SYS_HZ * 20) / 1000;
- timeo += get_timer(0); reset_timer();
Same as above here and below: Why do we have to touch global code for *all* architectures?
/* wait until command is processed or timeout occures */ @@ -854,6 +855,7 @@ static int nand_wait(struct mtd_info *mtd, struct
nand_chip *this)
else timeo = (CONFIG_SYS_HZ * 20) / 1000;
- timeo += get_timer(0); if ((state == FL_ERASING) && (this->options & NAND_IS_AND)) this->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1); else
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h index bf5614e..8d6e5fe 100644 --- a/include/configs/omap3_evm.h +++ b/include/configs/omap3_evm.h @@ -230,7 +230,7 @@
#define CONFIG_SYS_TIMERBASE OMAP34XX_GPT2 #define CONFIG_SYS_PVT V_PVT /* 2^(pvt+1) */ -#define CONFIG_SYS_HZ ((V_SCLK) / (2 << CONFIG_SYS_PVT)) +#define CONFIG_SYS_HZ 1000
Would be nice if you could change this for all OMAP3 boards.
Best regards
Dirk
On Tue, 2009-03-10 at 14:06 +0530, Manikandan Pillai wrote:
The cmd_misc.c was changed to fix sleep. nand_base.c was fixed to fix the wait.
Signed-off-by: Manikandan Pillai mani.pillai@ti.com
common/cmd_misc.c | 1 + cpu/arm_cortexa8/omap3/interrupts.c | 13 ++++--------- drivers/mtd/nand/nand_base.c | 2 ++ include/configs/omap3_evm.h | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-)
I can't comment on the patch contents, but it would be nice if you split this out into multiple patches. As is, you're fixing multiple bugs in 1 patch.
Something along the lines of: cmd_misc: Fix sleep command delay nand_base: Fix nand_wait() and nand_wait_ready() timeouts omap3: Change timer to 1 msec
Best, Peter
Dear Manikandan Pillai,
In message 1236674178-15624-1-git-send-email-mani.pillai@ti.com you wrote:
The cmd_misc.c was changed to fix sleep. nand_base.c was fixed to fix the wait.
Signed-off-by: Manikandan Pillai mani.pillai@ti.com
common/cmd_misc.c | 1 + cpu/arm_cortexa8/omap3/interrupts.c | 13 ++++--------- drivers/mtd/nand/nand_base.c | 2 ++ include/configs/omap3_evm.h | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/common/cmd_misc.c b/common/cmd_misc.c index 024299a..e3e64c0 100644 --- a/common/cmd_misc.c +++ b/common/cmd_misc.c @@ -38,6 +38,7 @@ int do_sleep (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) }
delay = simple_strtoul(argv[1], NULL, 10) * CONFIG_SYS_HZ;
- delay += get_timer(0);
This makes no sense to me. There are no problems known for this code (at least not on correctly working ports of U-Boot).
while (get_timer(start) < delay) { if (ctrlc ()) { diff --git a/cpu/arm_cortexa8/omap3/interrupts.c b/cpu/arm_cortexa8/omap3/interrupts.c index 9e9817d..5bfe6fc 100644 --- a/cpu/arm_cortexa8/omap3/interrupts.c +++ b/cpu/arm_cortexa8/omap3/interrupts.c @@ -193,7 +193,7 @@ void reset_timer(void)
ulong get_timer(ulong base) {
- return get_timer_masked() - base;
- return get_timer_masked();
This patch is definitely wrong.
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index d33fee2..283b456 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -509,6 +509,7 @@ void nand_wait_ready(struct mtd_info *mtd) struct nand_chip *chip = mtd->priv; u32 timeo = (CONFIG_SYS_HZ * 20) / 1000;
- timeo += get_timer(0); reset_timer();
This makes no sense to me. There are no problems known for this code (at least not on correctly working ports of U-Boot).
/* wait until command is processed or timeout occures */ @@ -854,6 +855,7 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *this) else timeo = (CONFIG_SYS_HZ * 20) / 1000;
- timeo += get_timer(0);
Ditto.
NAK.
Best regards,
Wolfgang Denk
participants (5)
-
Dirk Behme -
Manikandan Pillai -
Peter Tyser -
Pillai, Manikandan
-
Wolfgang Denk