| Message ID | 20200728155223.95356-1-xypron.glpk@gmx.de |
|---|---|
| State | Accepted |
| Commit | 183f1e27123769c41b3a73b92839497d4812f790 |
| Delegated to: | Andes |
| Headers | show |
| Series | [1/1] clk: kendryte/pll.h: do not redefine nop() | expand |
On 7/28/20 11:52 AM, Heinrich Schuchardt 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. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@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 > > +#endif > + > extern const struct clk_ops k210_pll_ops; > > struct clk *k210_register_pll_struct(const char *name, const char *parent_name, > -- > 2.27.0 > Reviewed-by: Sean Anderson <seanga2@gmail.com>
On Tue, Jul 28, 2020 at 11:52 PM Heinrich Schuchardt <xypron.glpk@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? > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@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"); ?? Regards, Bin
On 8/2/20 11:29 PM, Bin Meng wrote: > On Tue, Jul 28, 2020 at 11:52 PM Heinrich Schuchardt <xypron.glpk@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@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? --Sean
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@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@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? Best regards Heinrich
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@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@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
On 03.08.20 16:08, Sean Anderson wrote: > 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@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@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 > This is what udelay(1) gets compiled to: 000000000000004a <.LBE106>: udelay(1); 4a: 4505 li a0,1 000000000000004c <.LVL21>: 4c: 00000097 auipc ra,0x0 50: 000080e7 jalr ra # 4c <.LVL21> Somehow udelay() seems not to be fully implemented and the platform. So I think we should stay with the nop() macro. @Bin: For sandbox testing it is ok to define nop() as nil here. On the real hardware nop gives us a small delay. Best regards Heinrich
On 8/3/20 2:20 PM, Heinrich Schuchardt wrote: > On 03.08.20 16:08, Sean Anderson wrote: >> 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 >> > > This is what udelay(1) gets compiled to: > > 000000000000004a <.LBE106>: > udelay(1); > 4a: 4505 li a0,1 > > 000000000000004c <.LVL21>: > 4c: 00000097 auipc ra,0x0 > 50: 000080e7 jalr ra # 4c <.LVL21> > > Somehow udelay() seems not to be fully implemented and the platform. So > I think we should stay with the nop() macro. Hm, I don't see this generated code. What function is that a disassembly of? $ riscv64-linux-gnu-objdump -S --disassemble=__udelay u-boot u-boot: file format elf64-littleriscv Disassembly of section .text: Disassembly of section .text_rest: 00000000800197f8 <__udelay>: do_div(tick, 1000000); return tick; } void __weak __udelay(unsigned long usec) { 800197f8: 1101 addi sp,sp,-32 800197fa: ec06 sd ra,24(sp) 800197fc: e822 sd s0,16(sp) 800197fe: e426 sd s1,8(sp) 80019800: 84aa mv s1,a0 uint64_t tmp; tmp = get_ticks() + usec_to_tick(usec); /* get current timestamp */ 80019802: f7bff0ef jal ra,8001977c <get_ticks> 80019806: 842a mv s0,a0 80019808: 8526 mv a0,s1 8001980a: fcbff0ef jal ra,800197d4 <usec_to_tick> 8001980e: 942a add s0,s0,a0 while (get_ticks() < tmp+1) /* loop till event */ 80019810: 0405 addi s0,s0,1 80019812: f6bff0ef jal ra,8001977c <get_ticks> 80019816: fe856ee3 bltu a0,s0,80019812 <__udelay+0x1a> /*NOP*/; } 8001981a: 60e2 ld ra,24(sp) 8001981c: 6442 ld s0,16(sp) 8001981e: 64a2 ld s1,8(sp) 80019820: 6105 addi sp,sp,32 80019822: 8082 ret --Sean
On 05.08.20 13:45, Sean Anderson wrote: > On 8/3/20 2:20 PM, Heinrich Schuchardt wrote: >> On 03.08.20 16:08, Sean Anderson wrote: >>> 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 >>> >> >> This is what udelay(1) gets compiled to: >> >> 000000000000004a <.LBE106>: >> udelay(1); >> 4a: 4505 li a0,1 >> >> 000000000000004c <.LVL21>: >> 4c: 00000097 auipc ra,0x0 >> 50: 000080e7 jalr ra # 4c <.LVL21> >> >> Somehow udelay() seems not to be fully implemented and the platform. So >> I think we should stay with the nop() macro. > > Hm, I don't see this generated code. What function is that a disassembly > of? I was disassembling riscv64-linux-gnu-objdump -D -S drivers/clk/kendryte/pll.o If I disassemble the file u-boot function k210_pll_enable seems to be ok. nop()s replaced by udelay(1): writel(reg, pll->reg); 80012524: 7518 ld a4,40(a0) __iowmb(); 80012526: 0550000f fence ow,ow reg |= K210_PLL_RESET; 8001252a: 003006b7 lui a3,0x300 8001252e: 8fd5 or a5,a5,a3 __arch_putl(val, addr); 80012530: c31c sw a5,0(a4) udelay(1); 80012532: 4505 li a0,1 80012534: 612200ef jal ra,80032b46 <udelay> writel(reg, pll->reg); 80012538: 741c ld a5,40(s0) __iowmb(); 8001253a: 0550000f fence ow,ow So it was simply the link step missing to show the call. But anyway which way do you want the patch? Best regards Heinrich > > $ riscv64-linux-gnu-objdump -S --disassemble=__udelay u-boot > > u-boot: file format elf64-littleriscv > > > Disassembly of section .text: > > Disassembly of section .text_rest: > > 00000000800197f8 <__udelay>: > do_div(tick, 1000000); > return tick; > } > > void __weak __udelay(unsigned long usec) > { > 800197f8: 1101 addi sp,sp,-32 > 800197fa: ec06 sd ra,24(sp) > 800197fc: e822 sd s0,16(sp) > 800197fe: e426 sd s1,8(sp) > 80019800: 84aa mv s1,a0 > uint64_t tmp; > > tmp = get_ticks() + usec_to_tick(usec); /* get current timestamp */ > 80019802: f7bff0ef jal ra,8001977c <get_ticks> > 80019806: 842a mv s0,a0 > 80019808: 8526 mv a0,s1 > 8001980a: fcbff0ef jal ra,800197d4 <usec_to_tick> > 8001980e: 942a add s0,s0,a0 > > while (get_ticks() < tmp+1) /* loop till event */ > 80019810: 0405 addi s0,s0,1 > 80019812: f6bff0ef jal ra,8001977c <get_ticks> > 80019816: fe856ee3 bltu a0,s0,80019812 <__udelay+0x1a> > /*NOP*/; > } > 8001981a: 60e2 ld ra,24(sp) > 8001981c: 6442 ld s0,16(sp) > 8001981e: 64a2 ld s1,8(sp) > 80019820: 6105 addi sp,sp,32 > 80019822: 8082 ret > > --Sean >
On 8/5/20 2:19 PM, Heinrich Schuchardt wrote: > On 05.08.20 13:45, Sean Anderson wrote: >> On 8/3/20 2:20 PM, Heinrich Schuchardt wrote: >>> On 03.08.20 16:08, Sean Anderson wrote: >>>> 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 >>>> >>> >>> This is what udelay(1) gets compiled to: >>> >>> 000000000000004a <.LBE106>: >>> udelay(1); >>> 4a: 4505 li a0,1 >>> >>> 000000000000004c <.LVL21>: >>> 4c: 00000097 auipc ra,0x0 >>> 50: 000080e7 jalr ra # 4c <.LVL21> >>> >>> Somehow udelay() seems not to be fully implemented and the platform. So >>> I think we should stay with the nop() macro. >> >> Hm, I don't see this generated code. What function is that a disassembly >> of? > > I was disassembling > > riscv64-linux-gnu-objdump -D -S drivers/clk/kendryte/pll.o > > If I disassemble the file u-boot function k210_pll_enable seems to be > ok. nop()s replaced by udelay(1): > > writel(reg, pll->reg); > 80012524: 7518 ld a4,40(a0) > __iowmb(); > 80012526: 0550000f fence ow,ow > reg |= K210_PLL_RESET; > 8001252a: 003006b7 lui a3,0x300 > 8001252e: 8fd5 or a5,a5,a3 > __arch_putl(val, addr); > 80012530: c31c sw a5,0(a4) > udelay(1); > 80012532: 4505 li a0,1 > 80012534: 612200ef jal ra,80032b46 <udelay> > writel(reg, pll->reg); > 80012538: 741c ld a5,40(s0) > __iowmb(); > 8001253a: 0550000f fence ow,ow > > So it was simply the link step missing to show the call. > > But anyway which way do you want the patch? Hello Sean, did I miss you reply? Best regards Heinrich > > Best regards > > Heinrich > >> >> $ riscv64-linux-gnu-objdump -S --disassemble=__udelay u-boot >> >> u-boot: file format elf64-littleriscv >> >> >> Disassembly of section .text: >> >> Disassembly of section .text_rest: >> >> 00000000800197f8 <__udelay>: >> do_div(tick, 1000000); >> return tick; >> } >> >> void __weak __udelay(unsigned long usec) >> { >> 800197f8: 1101 addi sp,sp,-32 >> 800197fa: ec06 sd ra,24(sp) >> 800197fc: e822 sd s0,16(sp) >> 800197fe: e426 sd s1,8(sp) >> 80019800: 84aa mv s1,a0 >> uint64_t tmp; >> >> tmp = get_ticks() + usec_to_tick(usec); /* get current timestamp */ >> 80019802: f7bff0ef jal ra,8001977c <get_ticks> >> 80019806: 842a mv s0,a0 >> 80019808: 8526 mv a0,s1 >> 8001980a: fcbff0ef jal ra,800197d4 <usec_to_tick> >> 8001980e: 942a add s0,s0,a0 >> >> while (get_ticks() < tmp+1) /* loop till event */ >> 80019810: 0405 addi s0,s0,1 >> 80019812: f6bff0ef jal ra,8001977c <get_ticks> >> 80019816: fe856ee3 bltu a0,s0,80019812 <__udelay+0x1a> >> /*NOP*/; >> } >> 8001981a: 60e2 ld ra,24(sp) >> 8001981c: 6442 ld s0,16(sp) >> 8001981e: 64a2 ld s1,8(sp) >> 80019820: 6105 addi sp,sp,32 >> 80019822: 8082 ret >> >> --Sean >> >
On 8/15/20 3:53 AM, Heinrich Schuchardt wrote: > On 8/5/20 2:19 PM, Heinrich Schuchardt wrote: >> On 05.08.20 13:45, Sean Anderson wrote: >>> On 8/3/20 2:20 PM, Heinrich Schuchardt wrote: >>>> On 03.08.20 16:08, Sean Anderson wrote: >>>>> 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 >>>>> >>>> >>>> This is what udelay(1) gets compiled to: >>>> >>>> 000000000000004a <.LBE106>: >>>> udelay(1); >>>> 4a: 4505 li a0,1 >>>> >>>> 000000000000004c <.LVL21>: >>>> 4c: 00000097 auipc ra,0x0 >>>> 50: 000080e7 jalr ra # 4c <.LVL21> >>>> >>>> Somehow udelay() seems not to be fully implemented and the platform. So >>>> I think we should stay with the nop() macro. >>> >>> Hm, I don't see this generated code. What function is that a disassembly >>> of? >> >> I was disassembling >> >> riscv64-linux-gnu-objdump -D -S drivers/clk/kendryte/pll.o >> >> If I disassemble the file u-boot function k210_pll_enable seems to be >> ok. nop()s replaced by udelay(1): >> >> writel(reg, pll->reg); >> 80012524: 7518 ld a4,40(a0) >> __iowmb(); >> 80012526: 0550000f fence ow,ow >> reg |= K210_PLL_RESET; >> 8001252a: 003006b7 lui a3,0x300 >> 8001252e: 8fd5 or a5,a5,a3 >> __arch_putl(val, addr); >> 80012530: c31c sw a5,0(a4) >> udelay(1); >> 80012532: 4505 li a0,1 >> 80012534: 612200ef jal ra,80032b46 <udelay> >> writel(reg, pll->reg); >> 80012538: 741c ld a5,40(s0) >> __iowmb(); >> 8001253a: 0550000f fence ow,ow >> >> So it was simply the link step missing to show the call. >> >> But anyway which way do you want the patch? I think the patch as-is is fine. --Sean > > Hello Sean, > > did I miss you reply?> > Best regards > > Heinrich > >> >> Best regards >> >> Heinrich >> >>> >>> $ riscv64-linux-gnu-objdump -S --disassemble=__udelay u-boot >>> >>> u-boot: file format elf64-littleriscv >>> >>> >>> Disassembly of section .text: >>> >>> Disassembly of section .text_rest: >>> >>> 00000000800197f8 <__udelay>: >>> do_div(tick, 1000000); >>> return tick; >>> } >>> >>> void __weak __udelay(unsigned long usec) >>> { >>> 800197f8: 1101 addi sp,sp,-32 >>> 800197fa: ec06 sd ra,24(sp) >>> 800197fc: e822 sd s0,16(sp) >>> 800197fe: e426 sd s1,8(sp) >>> 80019800: 84aa mv s1,a0 >>> uint64_t tmp; >>> >>> tmp = get_ticks() + usec_to_tick(usec); /* get current timestamp */ >>> 80019802: f7bff0ef jal ra,8001977c <get_ticks> >>> 80019806: 842a mv s0,a0 >>> 80019808: 8526 mv a0,s1 >>> 8001980a: fcbff0ef jal ra,800197d4 <usec_to_tick> >>> 8001980e: 942a add s0,s0,a0 >>> >>> while (get_ticks() < tmp+1) /* loop till event */ >>> 80019810: 0405 addi s0,s0,1 >>> 80019812: f6bff0ef jal ra,8001977c <get_ticks> >>> 80019816: fe856ee3 bltu a0,s0,80019812 <__udelay+0x1a> >>> /*NOP*/; >>> } >>> 8001981a: 60e2 ld ra,24(sp) >>> 8001981c: 6442 ld s0,16(sp) >>> 8001981e: 64a2 ld s1,8(sp) >>> 80019820: 6105 addi sp,sp,32 >>> 80019822: 8082 ret >>> >>> --Sean >>> >> >
On Tue, Jul 28, 2020 at 11:52 PM Heinrich Schuchardt <xypron.glpk@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. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > include/kendryte/pll.h | 5 +++++ > 1 file changed, 5 insertions(+) > Reviewed-by: Bin Meng <bin.meng@windriver.com>
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 +#endif + extern const struct clk_ops k210_pll_ops; struct clk *k210_register_pll_struct(const char *name, const char *parent_name,
The kendryte PLL code uses nop as barrier. The macro is not defined for the sandbox on x86 but is defined on RISC-V. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- include/kendryte/pll.h | 5 +++++ 1 file changed, 5 insertions(+) -- 2.27.0