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.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
