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

[Heinrich Schuchardt]

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
>>
>