Message ID | 1254907903-11797-1-git-send-email-enrico.scholz@sigma-chemnitz.de |
---|---|
State | New, archived |
Headers | show |
On Wed, 2009-10-07 at 11:31 +0200, Enrico Scholz wrote: > Use DIV_ROUND_UP to calculate number of clocks. Else, calculated clocks > are nearly always to low and for times < 10ns, they will be negative on > PXA320 (which has a nand clock of 104 MHz). > > Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> > --- > drivers/mtd/nand/pxa3xx_nand.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > index 6ea520a..32cb4d1 100644 > --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -316,7 +316,7 @@ static struct pxa3xx_nand_flash *builtin_flash_types[] = { > #define tAR_NDTR1(r) (((r) >> 0) & 0xf) > > /* convert nano-seconds to nand flash controller clock cycles */ > -#define ns2cycle(ns, clk) (int)(((ns) * (clk / 1000000) / 1000) - 1) > +#define ns2cycle(ns, clk) (int)(DIV_ROUND_UP((ns) * (clk / 1000000), 1000) - 1) Why there is -1 at the end?
On Sun, Oct 11, 2009 at 8:51 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote: > On Wed, 2009-10-07 at 11:31 +0200, Enrico Scholz wrote: >> Use DIV_ROUND_UP to calculate number of clocks. Else, calculated clocks >> are nearly always to low and for times < 10ns, they will be negative on >> PXA320 (which has a nand clock of 104 MHz). >> >> Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> >> --- >> drivers/mtd/nand/pxa3xx_nand.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c >> index 6ea520a..32cb4d1 100644 >> --- a/drivers/mtd/nand/pxa3xx_nand.c >> +++ b/drivers/mtd/nand/pxa3xx_nand.c >> @@ -316,7 +316,7 @@ static struct pxa3xx_nand_flash *builtin_flash_types[] = { >> #define tAR_NDTR1(r) (((r) >> 0) & 0xf) >> >> /* convert nano-seconds to nand flash controller clock cycles */ >> -#define ns2cycle(ns, clk) (int)(((ns) * (clk / 1000000) / 1000) - 1) >> +#define ns2cycle(ns, clk) (int)(DIV_ROUND_UP((ns) * (clk / 1000000), 1000) - 1) > > Why there is -1 at the end? > This is because the actual clock cycles are what in the register + 1. Enrico, I'd recommend we remove this '-1' to always make the final result a little bit larger and avoid the negative case. DIV_ROUND_UP() is kind of confusing here, since the original equation is: cycles = ns / (1/clk * 10^9) = ns * clk / 10^9 And since clk is normally N * MHz, thus clk/1000,000 will most likely be an integer, and simplified to (ns * clk/1000,000) / 1000. My rough guess is that removing '-1' would suffice for most cases, e.g. with 104MHz NAND controller frequency: 9ns = 9 * 104 / 1000 = 0, which is fine (as 0+1 = 1cycle ~ 1000/104 ns), and 10ns = 10 * 104/1000 = 1, which is acceptable as 1 + 1 = 2cycle ~ 2000/104ns Could you please verify that removing '-1' will solve all corner cases? Thanks.
Eric Miao <eric.y.miao@gmail.com> writes: >>> Use DIV_ROUND_UP to calculate number of clocks. Else, calculated clocks >>> are nearly always to low and for times < 10ns, they will be negative on >>> PXA320 (which has a nand clock of 104 MHz). >>> >>> -#define ns2cycle(ns, clk) (int)(((ns) * (clk / 1000000) / 1000) - 1) >>> +#define ns2cycle(ns, clk) (int)(DIV_ROUND_UP((ns) * (clk / 1000000), 1000) - 1) >> >> Why there is -1 at the end? > ... > I'd recommend we remove this '-1' to always make the final result > a little bit larger and avoid the negative case. DIV_ROUND_UP() is > kind of confusing here, since the original equation is: > > cycles = ns / (1/clk * 10^9) = ns * clk / 10^9 Hi Eric, for me, the DIV_ROUND_UP() is more clean, because 'ns' are minimum values given on the datasheet and you have to round up fractional cycle counts hence. But I agree that | #define ns2cycle(ns, clk) (int)(DIV_ROUND_UP((ns) * (clk / 1000000), 1000) - 1) and | #define ns2cycle(ns, clk) (int)(((ns) * (clk / 1000000) / 1000)) give same results for most values. I do not have a strong preference for one of these two variants. > Could you please verify that removing '-1' will solve all corner > cases? Except for overflows (which are uninteresting here), timings which are a multiple of 1000ns are the only corner cases. And these will result into cycle counts which are increased by 1 compared to their ideal value. Enrico
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index 6ea520a..32cb4d1 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -316,7 +316,7 @@ static struct pxa3xx_nand_flash *builtin_flash_types[] = { #define tAR_NDTR1(r) (((r) >> 0) & 0xf) /* convert nano-seconds to nand flash controller clock cycles */ -#define ns2cycle(ns, clk) (int)(((ns) * (clk / 1000000) / 1000) - 1) +#define ns2cycle(ns, clk) (int)(DIV_ROUND_UP((ns) * (clk / 1000000), 1000) - 1) /* convert nand flash controller clock cycles to nano-seconds */ #define cycle2ns(c, clk) ((((c) + 1) * 1000000 + clk / 500) / (clk / 1000))
Use DIV_ROUND_UP to calculate number of clocks. Else, calculated clocks are nearly always to low and for times < 10ns, they will be negative on PXA320 (which has a nand clock of 104 MHz). Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> --- drivers/mtd/nand/pxa3xx_nand.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)