| Message ID | 20200729095636.1077054-8-seanga2@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | riscv: Clean up timer drivers | expand |
+Anup Patel On Wed, Jul 29, 2020 at 5:57 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. > > Changes in v2: > - Fix SiFive CLINT not getting tick-rate from rtcclk > > arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- > arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi > index afdb4f4402..f126d3e0b3 100644 > --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi > +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > @@ -53,9 +53,13 @@ > reg = <0x0 0x10070000 0x0 0x1000>; > fuse-count = <0x1000>; > }; > - clint@2000000 { > + clint: 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>; > u-boot,dm-spl; > }; > diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > index e037150520..3275bb1f12 100644 > --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > @@ -26,6 +26,10 @@ > > }; > > +&clint { > + clocks = <&rtcclk>; > +}; Can we consider making this property a standard property by the kernel upstream? How does the kernel CLINT timer driver determine its running frequency? > + > &qspi2 { > mmc@0 { > u-boot,dm-spl; Regards, Bin
On 8/18/20 5:22 AM, Bin Meng wrote: > +Anup Patel > > On Wed, Jul 29, 2020 at 5:57 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. >> >> Changes in v2: >> - Fix SiFive CLINT not getting tick-rate from rtcclk >> >> arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- >> arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi >> index afdb4f4402..f126d3e0b3 100644 >> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi >> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi >> @@ -53,9 +53,13 @@ >> reg = <0x0 0x10070000 0x0 0x1000>; >> fuse-count = <0x1000>; >> }; >> - clint@2000000 { >> + clint: 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>; >> u-boot,dm-spl; >> }; >> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi >> index e037150520..3275bb1f12 100644 >> --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi >> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi >> @@ -26,6 +26,10 @@ >> >> }; >> >> +&clint { >> + clocks = <&rtcclk>; >> +}; > > Can we consider making this property a standard property by the kernel > upstream? How does the kernel CLINT timer driver determine its running > frequency? Currently it gets it from /cpus/timebase-frequency. --Sean
Hi Anup, On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson <seanga2@gmail.com> wrote: > > On 8/18/20 5:22 AM, Bin Meng wrote: > > +Anup Patel > > > > On Wed, Jul 29, 2020 at 5:57 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. > >> > >> Changes in v2: > >> - Fix SiFive CLINT not getting tick-rate from rtcclk > >> > >> arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- > >> arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ > >> 2 files changed, 10 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi > >> index afdb4f4402..f126d3e0b3 100644 > >> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi > >> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > >> @@ -53,9 +53,13 @@ > >> reg = <0x0 0x10070000 0x0 0x1000>; > >> fuse-count = <0x1000>; > >> }; > >> - clint@2000000 { > >> + clint: 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>; > >> u-boot,dm-spl; > >> }; > >> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > >> index e037150520..3275bb1f12 100644 > >> --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > >> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > >> @@ -26,6 +26,10 @@ > >> > >> }; > >> > >> +&clint { > >> + clocks = <&rtcclk>; > >> +}; > > > > Can we consider making this property a standard property by the kernel > > upstream? How does the kernel CLINT timer driver determine its running > > frequency? > > Currently it gets it from /cpus/timebase-frequency. What's your comment on this? I think this should go upstream to the standard bindings. Regards, Bin
On Thu, Sep 3, 2020 at 7:32 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson <seanga2@gmail.com> wrote: > > > > On 8/18/20 5:22 AM, Bin Meng wrote: > > > +Anup Patel > > > > > > On Wed, Jul 29, 2020 at 5:57 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. > > >> > > >> Changes in v2: > > >> - Fix SiFive CLINT not getting tick-rate from rtcclk > > >> > > >> arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- > > >> arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ > > >> 2 files changed, 10 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi > > >> index afdb4f4402..f126d3e0b3 100644 > > >> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi > > >> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > > >> @@ -53,9 +53,13 @@ > > >> reg = <0x0 0x10070000 0x0 0x1000>; > > >> fuse-count = <0x1000>; > > >> }; > > >> - clint@2000000 { > > >> + clint: 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>; > > >> u-boot,dm-spl; > > >> }; > > >> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > >> index e037150520..3275bb1f12 100644 > > >> --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > >> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > >> @@ -26,6 +26,10 @@ > > >> > > >> }; > > >> > > >> +&clint { > > >> + clocks = <&rtcclk>; > > >> +}; > > > > > > Can we consider making this property a standard property by the kernel > > > upstream? How does the kernel CLINT timer driver determine its running > > > frequency? > > > > Currently it gets it from /cpus/timebase-frequency. > > What's your comment on this? I think this should go upstream to the > standard bindings. My apologies for delay. Too many mailing list to track. I think "clocks" should be an optional DT property for CLINT driver (both Linux and U-Boot). If "clocks" is not available then we should fallback to "/cpus/timebase-frequency" Please note in any case the "/cpus/timebase-frequency" will remain mandatory for generic RISC-V timer (both Linux and U-Boot). Regards, Anup
Hi Anup, On Thu, Sep 3, 2020 at 10:46 AM Anup Patel <anup@brainfault.org> wrote: > > On Thu, Sep 3, 2020 at 7:32 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Anup, > > > > On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson <seanga2@gmail.com> wrote: > > > > > > On 8/18/20 5:22 AM, Bin Meng wrote: > > > > +Anup Patel > > > > > > > > On Wed, Jul 29, 2020 at 5:57 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. > > > >> > > > >> Changes in v2: > > > >> - Fix SiFive CLINT not getting tick-rate from rtcclk > > > >> > > > >> arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- > > > >> arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ > > > >> 2 files changed, 10 insertions(+), 2 deletions(-) > > > >> > > > >> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi > > > >> index afdb4f4402..f126d3e0b3 100644 > > > >> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi > > > >> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > > > >> @@ -53,9 +53,13 @@ > > > >> reg = <0x0 0x10070000 0x0 0x1000>; > > > >> fuse-count = <0x1000>; > > > >> }; > > > >> - clint@2000000 { > > > >> + clint: 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>; > > > >> u-boot,dm-spl; > > > >> }; > > > >> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > >> index e037150520..3275bb1f12 100644 > > > >> --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > >> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > >> @@ -26,6 +26,10 @@ > > > >> > > > >> }; > > > >> > > > >> +&clint { > > > >> + clocks = <&rtcclk>; > > > >> +}; > > > > > > > > Can we consider making this property a standard property by the kernel > > > > upstream? How does the kernel CLINT timer driver determine its running > > > > frequency? > > > > > > Currently it gets it from /cpus/timebase-frequency. > > > > What's your comment on this? I think this should go upstream to the > > standard bindings. > > My apologies for delay. Too many mailing list to track. > > I think "clocks" should be an optional DT property for CLINT driver (both > Linux and U-Boot). If "clocks" is not available then we should fallback to > "/cpus/timebase-frequency" > Agreed. The question is whether this should be mentioned in the DT bindings doc in the kernel tree? > Please note in any case the "/cpus/timebase-frequency" will remain > mandatory for generic RISC-V timer (both Linux and U-Boot). Regards, Bin
On Thu, Sep 3, 2020 at 8:19 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Thu, Sep 3, 2020 at 10:46 AM Anup Patel <anup@brainfault.org> wrote: > > > > On Thu, Sep 3, 2020 at 7:32 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Anup, > > > > > > On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson <seanga2@gmail.com> wrote: > > > > > > > > On 8/18/20 5:22 AM, Bin Meng wrote: > > > > > +Anup Patel > > > > > > > > > > On Wed, Jul 29, 2020 at 5:57 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. > > > > >> > > > > >> Changes in v2: > > > > >> - Fix SiFive CLINT not getting tick-rate from rtcclk > > > > >> > > > > >> arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- > > > > >> arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ > > > > >> 2 files changed, 10 insertions(+), 2 deletions(-) > > > > >> > > > > >> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi > > > > >> index afdb4f4402..f126d3e0b3 100644 > > > > >> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi > > > > >> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > > > > >> @@ -53,9 +53,13 @@ > > > > >> reg = <0x0 0x10070000 0x0 0x1000>; > > > > >> fuse-count = <0x1000>; > > > > >> }; > > > > >> - clint@2000000 { > > > > >> + clint: 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>; > > > > >> u-boot,dm-spl; > > > > >> }; > > > > >> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > > >> index e037150520..3275bb1f12 100644 > > > > >> --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > > >> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > > >> @@ -26,6 +26,10 @@ > > > > >> > > > > >> }; > > > > >> > > > > >> +&clint { > > > > >> + clocks = <&rtcclk>; > > > > >> +}; > > > > > > > > > > Can we consider making this property a standard property by the kernel > > > > > upstream? How does the kernel CLINT timer driver determine its running > > > > > frequency? > > > > > > > > Currently it gets it from /cpus/timebase-frequency. > > > > > > What's your comment on this? I think this should go upstream to the > > > standard bindings. > > > > My apologies for delay. Too many mailing list to track. > > > > I think "clocks" should be an optional DT property for CLINT driver (both > > Linux and U-Boot). If "clocks" is not available then we should fallback to > > "/cpus/timebase-frequency" > > > > Agreed. The question is whether this should be mentioned in the DT > bindings doc in the kernel tree? Yes, the "clocks" DT property should be updated in kernel DT bindings doc and kernel CLINT driver should also prefer "clocks" DT property when available. Regards, Anup > > > Please note in any case the "/cpus/timebase-frequency" will remain > > mandatory for generic RISC-V timer (both Linux and U-Boot). > > Regards, > Bin
On 9/3/20 1:01 AM, Anup Patel wrote: > On Thu, Sep 3, 2020 at 8:19 AM Bin Meng <bmeng.cn@gmail.com> wrote: >> >> Hi Anup, >> >> On Thu, Sep 3, 2020 at 10:46 AM Anup Patel <anup@brainfault.org> wrote: >>> >>> On Thu, Sep 3, 2020 at 7:32 AM Bin Meng <bmeng.cn@gmail.com> wrote: >>>> >>>> Hi Anup, >>>> >>>> On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson <seanga2@gmail.com> wrote: >>>>> >>>>> On 8/18/20 5:22 AM, Bin Meng wrote: >>>>>> +Anup Patel >>>>>> >>>>>> On Wed, Jul 29, 2020 at 5:57 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. >>>>>>> >>>>>>> Changes in v2: >>>>>>> - Fix SiFive CLINT not getting tick-rate from rtcclk >>>>>>> >>>>>>> arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- >>>>>>> arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ >>>>>>> 2 files changed, 10 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi >>>>>>> index afdb4f4402..f126d3e0b3 100644 >>>>>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi >>>>>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi >>>>>>> @@ -53,9 +53,13 @@ >>>>>>> reg = <0x0 0x10070000 0x0 0x1000>; >>>>>>> fuse-count = <0x1000>; >>>>>>> }; >>>>>>> - clint@2000000 { >>>>>>> + clint: 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>; >>>>>>> u-boot,dm-spl; >>>>>>> }; >>>>>>> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi >>>>>>> index e037150520..3275bb1f12 100644 >>>>>>> --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi >>>>>>> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi >>>>>>> @@ -26,6 +26,10 @@ >>>>>>> >>>>>>> }; >>>>>>> >>>>>>> +&clint { >>>>>>> + clocks = <&rtcclk>; >>>>>>> +}; >>>>>> >>>>>> Can we consider making this property a standard property by the kernel >>>>>> upstream? How does the kernel CLINT timer driver determine its running >>>>>> frequency? >>>>> >>>>> Currently it gets it from /cpus/timebase-frequency. >>>> >>>> What's your comment on this? I think this should go upstream to the >>>> standard bindings. >>> >>> My apologies for delay. Too many mailing list to track. >>> >>> I think "clocks" should be an optional DT property for CLINT driver (both >>> Linux and U-Boot). If "clocks" is not available then we should fallback to >>> "/cpus/timebase-frequency" >>> >> >> Agreed. The question is whether this should be mentioned in the DT >> bindings doc in the kernel tree? > > Yes, the "clocks" DT property should be updated in kernel DT bindings doc > and kernel CLINT driver should also prefer "clocks" DT property when available. > > Regards, > Anup Should clock-frequency be used as well, or should that be left for timebase-frequency? --Sean
On Thu, Sep 3, 2020 at 4:23 PM Sean Anderson <seanga2@gmail.com> wrote: > > On 9/3/20 1:01 AM, Anup Patel wrote: > > On Thu, Sep 3, 2020 at 8:19 AM Bin Meng <bmeng.cn@gmail.com> wrote: > >> > >> Hi Anup, > >> > >> On Thu, Sep 3, 2020 at 10:46 AM Anup Patel <anup@brainfault.org> wrote: > >>> > >>> On Thu, Sep 3, 2020 at 7:32 AM Bin Meng <bmeng.cn@gmail.com> wrote: > >>>> > >>>> Hi Anup, > >>>> > >>>> On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson <seanga2@gmail.com> wrote: > >>>>> > >>>>> On 8/18/20 5:22 AM, Bin Meng wrote: > >>>>>> +Anup Patel > >>>>>> > >>>>>> On Wed, Jul 29, 2020 at 5:57 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. > >>>>>>> > >>>>>>> Changes in v2: > >>>>>>> - Fix SiFive CLINT not getting tick-rate from rtcclk > >>>>>>> > >>>>>>> arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- > >>>>>>> arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ > >>>>>>> 2 files changed, 10 insertions(+), 2 deletions(-) > >>>>>>> > >>>>>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>>>>>> index afdb4f4402..f126d3e0b3 100644 > >>>>>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>>>>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>>>>>> @@ -53,9 +53,13 @@ > >>>>>>> reg = <0x0 0x10070000 0x0 0x1000>; > >>>>>>> fuse-count = <0x1000>; > >>>>>>> }; > >>>>>>> - clint@2000000 { > >>>>>>> + clint: 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>; > >>>>>>> u-boot,dm-spl; > >>>>>>> }; > >>>>>>> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > >>>>>>> index e037150520..3275bb1f12 100644 > >>>>>>> --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > >>>>>>> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > >>>>>>> @@ -26,6 +26,10 @@ > >>>>>>> > >>>>>>> }; > >>>>>>> > >>>>>>> +&clint { > >>>>>>> + clocks = <&rtcclk>; > >>>>>>> +}; > >>>>>> > >>>>>> Can we consider making this property a standard property by the kernel > >>>>>> upstream? How does the kernel CLINT timer driver determine its running > >>>>>> frequency? > >>>>> > >>>>> Currently it gets it from /cpus/timebase-frequency. > >>>> > >>>> What's your comment on this? I think this should go upstream to the > >>>> standard bindings. > >>> > >>> My apologies for delay. Too many mailing list to track. > >>> > >>> I think "clocks" should be an optional DT property for CLINT driver (both > >>> Linux and U-Boot). If "clocks" is not available then we should fallback to > >>> "/cpus/timebase-frequency" > >>> > >> > >> Agreed. The question is whether this should be mentioned in the DT > >> bindings doc in the kernel tree? > > > > Yes, the "clocks" DT property should be updated in kernel DT bindings doc > > and kernel CLINT driver should also prefer "clocks" DT property when available. > > > > Regards, > > Anup > > Should clock-frequency be used as well, or should that be left for timebase-frequency? Not sure about clock-frequency DT property. I think very few linux clocksource drivers use "clock-frequency" DT property. Probably it's fine to first check for "clocks" and "clock-frequency" before falling back to "timebase-frequency". Regards, Anup > > --Sean
diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi index afdb4f4402..f126d3e0b3 100644 --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -53,9 +53,13 @@ reg = <0x0 0x10070000 0x0 0x1000>; fuse-count = <0x1000>; }; - clint@2000000 { + clint: 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>; u-boot,dm-spl; }; diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index e037150520..3275bb1f12 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -26,6 +26,10 @@ }; +&clint { + clocks = <&rtcclk>; +}; + &qspi2 { mmc@0 { u-boot,dm-spl;
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. Changes in v2: - Fix SiFive CLINT not getting tick-rate from rtcclk arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)