Re: [PATCH 5/7] RISC-V: arch/riscv/lib

[Palmer Dabbelt]

On Tue, 23 May 2017 04:19:42 PDT (-0700), Arnd Bergmann wrote:
> On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote:
>> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
>> new file mode 100644
>> index 000000000000..f644e582f4b8
>> --- /dev/null
>> +++ b/arch/riscv/lib/Makefile
>
>> +
>> +void __delay(unsigned long cycles)
>> +{
>> +       u64 t0 = get_cycles();
>> +
>> +       while ((unsigned long)(get_cycles() - t0) < cycles)
>> +               cpu_relax();
>> +}
>> +
>> +void udelay(unsigned long usecs)
>> +{
>> +       u64 ucycles = (u64)usecs * timebase;
>> +       do_div(ucycles, 1000000U);
>> +       __delay((unsigned long)ucycles);
>> +}
>> +EXPORT_SYMBOL(udelay);
>> +
>> +void ndelay(unsigned long nsecs)
>> +{
>> +       u64 ncycles = (u64)nsecs * timebase;
>> +       do_div(ncycles, 1000000000U);
>> +       __delay((unsigned long)ncycles);
>> +}
>
> I'd be slightly worried about a global 'timebase' identifier that
> might conflict with a variable in some random driver.

Makes sense.  I've renamed it to riscv_timebase

  https://github.com/riscv/riscv-linux/commit/ed7d769e2c14e8809c3c125e0bba2978cb6fd37b

> Also, it would be good to replace the multiply+div64
> with a single multiplication here, see how x86 and arm do it
> (for the tsc/__timer_delay case).

Makes sense.  I think this should do it

  https://github.com/riscv/riscv-linux/commit/d397332f6ebff42f3ecb385e9cf3284fdeda6776

but I'm finding this hard to test as this only works for 2ms sleeps.  It seems
at least in the right ballpark

  [    0.048000] before 1000x usleep 1000
  [    1.060000] before 1000x nsleep 1000000
  [    2.072000] done

>       Arnd

Thanks for the feedback.  I'll incorporate this along with all the other
feedback into a v2.