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. > > 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()
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.be...@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.pil...@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 > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
