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

[Sean Anderson]

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