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

[Heinrich Schuchardt]

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
>