[PATCH] pxa3xx_nand: fix time calculation

[PATCH] pxa3xx_nand: fix time calculation

[Enrico Scholz]

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)
 
 /* convert nand flash controller clock cycles to nano-seconds */
 #define cycle2ns(c, clk)	((((c) + 1) * 1000000 + clk / 500) / (clk / 1000))
-- 
1.6.2.5

Re: [PATCH] pxa3xx_nand: fix time calculation

[Artem Bityutskiy]

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?

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH] pxa3xx_nand: fix time calculation

[Eric Miao]

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.

Re: [PATCH] pxa3xx_nand: fix time calculation

[Enrico Scholz]

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