[PATCH 1/1] clk: kendryte/pll.h: do not redefine nop()

[Sean Anderson]

On 8/3/20 9:59 AM, Heinrich Schuchardt wrote:
> On 03.08.20 12:10, Sean Anderson wrote:
>> On 8/2/20 11:29 PM, Bin Meng wrote:
>>> On Tue, Jul 28, 2020 at 11:52 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>
>>>> The kendryte PLL code uses nop as barrier. The macro is not defined for
>>>> the sandbox on x86 but is defined on RISC-V.
>>>
>>> Is this kendryte PLL driver built for Sandbox?
>>
>> Yes. I added a unit test for the parameter calculation function.
>>
>>>
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>> ---
>>>>  include/kendryte/pll.h | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/include/kendryte/pll.h b/include/kendryte/pll.h
>>>> index c8e3200799..55a40b9c97 100644
>>>> --- a/include/kendryte/pll.h
>>>> +++ b/include/kendryte/pll.h
>>>> @@ -7,6 +7,7 @@
>>>>
>>>>  #include <clk.h>
>>>>  #include <test/export.h>
>>>> +#include <asm/io.h>
>>>>
>>>>  #define K210_PLL_CLKR GENMASK(3, 0)
>>>>  #define K210_PLL_CLKF GENMASK(9, 4)
>>>> @@ -43,9 +44,13 @@ struct k210_pll_config {
>>>>  #ifdef CONFIG_UNIT_TEST
>>>>  TEST_STATIC int k210_pll_calc_config(u32 rate, u32 rate_in,
>>>>                                      struct k210_pll_config *best);
>>>> +
>>>> +#ifndef nop
>>>>  #define nop()
>>>>  #endif
>>>
>>> Maybe we can fix the kendryte PLL driver to use:
>>>
>>> asm volatile ("nop");
>>
>> Is that preferred over the nop macro?
> 
> On RISC-V nop is just a synonym for addi x0, x0, 0.
> 
> The only use of the nop statements is to guarantee a minimum time that
> the reset signal is set in k210_pll_enable(). Would udelay(1); be a
> valid replacement here?

Maybe. Because we are configuring the PLL, the CPU clock is temporarily
set to the in0 oscillator, so the timer would give an incorrect delay.
However, it would probably be fine even if incorrect.  The original
reason I used nops is because that is what the SDK does in its PLL
configuration function [1]. I believe I tried using udelay at one point,
but I don't remember whether I had any issues with it.

--Sean

[1] https://github.com/kendryte/kendryte-standalone-sdk/blob/develop/lib/drivers/sysctl.c#L844