Re: [PATCH] pxa3xx_nand: fix time calculation

[Eric Miao <eric.y.miao@gmail.com>]

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.