On 7/23/20 12:51 PM, Sagar Kadam wrote:
Hello Sean,
-----Original Message----- From: Sean Anderson seanga2@gmail.com Sent: Thursday, July 23, 2020 8:22 PM To: Bin Meng bmeng.cn@gmail.com Cc: Pragnesh Patel pragnesh.patel@sifive.com; Sagar Kadam sagar.kadam@sifive.com; U-Boot Mailing List u-boot@lists.denx.de; Rick Chen rickchen36@gmail.com Subject: Re: [PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver
[External Email] Do not click links or attachments unless you recognize the sender and know the content is safe
On 7/23/20 10:47 AM, Bin Meng wrote:
Hi Sean,
On Thu, Jul 23, 2020 at 9:57 PM Sean Anderson seanga2@gmail.com
wrote:
On 7/23/20 9:50 AM, Bin Meng wrote:
Hi Sean,
On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson seanga2@gmail.com
wrote:
We may need to add a clock-frequency binding like for the K210.
Signed-off-by: Sean Anderson seanga2@gmail.com
This patch builds but has NOT been tested.
arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi index afdb4f4402..e56bfc7595 100644 --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -55,8 +55,13 @@ }; clint@2000000 { compatible = "riscv,clint0";
interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7&cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3 &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7&cpu1_intc 3 &cpu1_intc 7&cpu2_intc 3 &cpu2_intc 7&cpu3_intc 3 &cpu3_intc 7&cpu4_intc 3- &cpu4_intc 7>; reg = <0x0 0x2000000 0x0 0xc0000>;
clocks = <&prci PRCI_CLK_COREPLL>;This looks wrong to me. The CLINT timer frequency should come from the
RTC node.
+Pragnesh Patel
+Sagar Kadam
On further review, I think you are right that this should be RTCCLK_FREQ.
Perhaps the clocks part should be moved into arch/riscv/dts/hifive-unleashed-a00-u-boot.dts and changed to something like
clocks = <&rtcclk>;
Yes, CLINT is driven by RTC clock.
How does the device tree look like in the upstream Linux to work with the new CLINT timer driver?
Regards, Bin
Anup's patch doesn't change the device tree at all so I think they are still using timebase-frequency.
In that case I was wondering what would be better:
- IMHO we can skip the property as in your patch [4/6] riscv: Rework Sifive CLINT as UCLASS_TIMER driver has a fallback to timebase-frequency, in case clock for clint is not specified and is falling back to timebase-frequency which is again set to RTCCLK_FREQ
- OR Shift the clock part to board -uboot dts file as you are suggesting in order to keep it synonymous with other TIMER changes done as a part of this series.
I prefer the second option for two reasons. First, properties which affect a device should be located near its binding in the device tree. Using timebase-frequency only really makes sense when the cpu itself is the timer device. This is the case when we read the time from a CSR, but not when there is a separate device. Second, it lets the device use the clock subsystem which adds flexibility. If the device is configured for a different clock speed, the timer can adjust itself.
--Sean