The Kendryte K210 SoC CLINT is compatible with Sifive clint v0 (sifive,clint0). Fix the Kendryte K210 device tree clint entry to be inline with the sifive timer definition documented in Documentation/devicetree/bindings/timer/sifive,clint.yaml. Rename the clint0 entry to "timer", fix the register mapping and fix the interrupt extended entry. This fixes boot failures with Kendryte K210 SoC boards. Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> --- arch/riscv/boot/dts/kendryte/k210.dtsi | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/riscv/boot/dts/kendryte/k210.dtsi b/arch/riscv/boot/dts/kendryte/k210.dtsi index c1df56ccb8d5..48d65fea45e9 100644 --- a/arch/riscv/boot/dts/kendryte/k210.dtsi +++ b/arch/riscv/boot/dts/kendryte/k210.dtsi @@ -95,10 +95,11 @@ sysctl: sysctl@50440000 { #clock-cells = <1>; }; - clint0: interrupt-controller@2000000 { + timer@2000000 { compatible = "riscv,clint0"; - reg = <0x2000000 0xC000>; - interrupts-extended = <&cpu0_intc 3>, <&cpu1_intc 3>; + reg = <0x2000000 0x10000>; + interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 + &cpu1_intc 3 &cpu1_intc 7>; clocks = <&sysctl K210_CLK_ACLK>; }; -- 2.26.2 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
On 9/15/20 8:55 AM, Damien Le Moal wrote: > The Kendryte K210 SoC CLINT is compatible with Sifive clint v0 > (sifive,clint0). Fix the Kendryte K210 device tree clint entry to be > inline with the sifive timer definition documented in > Documentation/devicetree/bindings/timer/sifive,clint.yaml. Rename the > clint0 entry to "timer", fix the register mapping and fix the interrupt > extended entry. > > This fixes boot failures with Kendryte K210 SoC boards. > > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > --- > arch/riscv/boot/dts/kendryte/k210.dtsi | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/boot/dts/kendryte/k210.dtsi b/arch/riscv/boot/dts/kendryte/k210.dtsi > index c1df56ccb8d5..48d65fea45e9 100644 > --- a/arch/riscv/boot/dts/kendryte/k210.dtsi > +++ b/arch/riscv/boot/dts/kendryte/k210.dtsi > @@ -95,10 +95,11 @@ sysctl: sysctl@50440000 { > #clock-cells = <1>; > }; > > - clint0: interrupt-controller@2000000 { > + timer@2000000 { > compatible = "riscv,clint0"; > - reg = <0x2000000 0xC000>; > - interrupts-extended = <&cpu0_intc 3>, <&cpu1_intc 3>; > + reg = <0x2000000 0x10000>; Why the change in size? The docs from the SDK [1] (which I think were copied from SiFive documentation around the time of their release) state that the CLINT is 0xC000 bytes. In addition, accessing memory past 0x0200C000 yields a repeating memory pattern: 0200c000: 69823183 02020120 6083c006 00008002 .1.i ......`.... 0200c010: 69823183 02020120 6083c006 00008002 .1.i ......`.... 0200c020: 69823183 02020120 6083c006 00008002 .1.i ......`.... 0200c030: 69823183 02020120 6083c006 00008002 .1.i ......`.... (continues until 0x02010000) To me, it is clear that the CLINT is not 0x10000 in size, even though the legal address space extends to that size. I am more curious how this fixes booting. What was the issue before? > + interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 > + &cpu1_intc 3 &cpu1_intc 7>; > clocks = <&sysctl K210_CLK_ACLK>; This clock is 50 times too fast(!) See [2] and [3] for a fix. I don't know if adding a "virtual" clock is the best solution for Linux. I would like to keep the device trees relatively in-sync, so please let me know if you think there is a better solution. > }; > > [1] https://github.com/kendryte/kendryte-standalone-sdk/blob/develop/lib/drivers/include/clint.h [2] https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929-8-seanga2@gmail.com/ [3] https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929-9-seanga2@gmail.com/ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2020/09/16 11:15, Sean Anderson wrote: > On 9/15/20 8:55 AM, Damien Le Moal wrote: >> The Kendryte K210 SoC CLINT is compatible with Sifive clint v0 >> (sifive,clint0). Fix the Kendryte K210 device tree clint entry to be >> inline with the sifive timer definition documented in >> Documentation/devicetree/bindings/timer/sifive,clint.yaml. Rename the >> clint0 entry to "timer", fix the register mapping and fix the interrupt >> extended entry. >> >> This fixes boot failures with Kendryte K210 SoC boards. >> >> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> >> --- >> arch/riscv/boot/dts/kendryte/k210.dtsi | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/arch/riscv/boot/dts/kendryte/k210.dtsi b/arch/riscv/boot/dts/kendryte/k210.dtsi >> index c1df56ccb8d5..48d65fea45e9 100644 >> --- a/arch/riscv/boot/dts/kendryte/k210.dtsi >> +++ b/arch/riscv/boot/dts/kendryte/k210.dtsi >> @@ -95,10 +95,11 @@ sysctl: sysctl@50440000 { >> #clock-cells = <1>; >> }; >> >> - clint0: interrupt-controller@2000000 { >> + timer@2000000 { >> compatible = "riscv,clint0"; >> - reg = <0x2000000 0xC000>; >> - interrupts-extended = <&cpu0_intc 3>, <&cpu1_intc 3>; >> + reg = <0x2000000 0x10000>; > > Why the change in size? The docs from the SDK [1] (which I think were copied > from SiFive documentation around the time of their release) state that > the CLINT is 0xC000 bytes. In addition, accessing memory past 0x0200C000 > yields a repeating memory pattern: Oops. Copy-paste error. I will change that back to 0x0000C000. > > > 0200c000: 69823183 02020120 6083c006 00008002 .1.i ......`.... > 0200c010: 69823183 02020120 6083c006 00008002 .1.i ......`.... > 0200c020: 69823183 02020120 6083c006 00008002 .1.i ......`.... > 0200c030: 69823183 02020120 6083c006 00008002 .1.i ......`.... > (continues until 0x02010000) > > To me, it is clear that the CLINT is not 0x10000 in size, even though > the legal address space extends to that size. > > I am more curious how this fixes booting. What was the issue before? There were 2 problems: 1) The old declaration of the entry as clint0: interrupt-controller@2000000 led to a failure to find the driver. 2) The interrupt extended entry was wrong > >> + interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 >> + &cpu1_intc 3 &cpu1_intc 7>; >> clocks = <&sysctl K210_CLK_ACLK>; > > This clock is 50 times too fast(!) See [2] and [3] for a fix. I don't > know if adding a "virtual" clock is the best solution for Linux. I would > like to keep the device trees relatively in-sync, so please let me know > if you think there is a better solution. Huu... Let me check that. Once booted, using commands such as date or time does not give me weird results, well at least, before this patch and clint changes. I forgot to check yesterday when I tested (I only boot tested). Will check. And yes, I will make effort to keep the DT in sync. Is the Kendryte support upstream in U-Boot now ? I have not checked recently. > >> }; >> >> > > [1] https://github.com/kendryte/kendryte-standalone-sdk/blob/develop/lib/drivers/include/clint.h > [2] https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929-8-seanga2@gmail.com/ > [3] https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929-9-seanga2@gmail.com/ > -- Damien Le Moal Western Digital Research _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
> -----Original Message----- > From: Sean Anderson <seanga2@gmail.com> > Sent: 16 September 2020 07:46 > To: Damien Le Moal <Damien.LeMoal@wdc.com>; linux- > riscv@lists.infradead.org; Palmer Dabbelt <palmer@dabbelt.com> > Cc: Anup Patel <Anup.Patel@wdc.com> > Subject: Re: [PATCH] riscv: Fix Kendryte K210 device tree > > On 9/15/20 8:55 AM, Damien Le Moal wrote: > > The Kendryte K210 SoC CLINT is compatible with Sifive clint v0 > > (sifive,clint0). Fix the Kendryte K210 device tree clint entry to be > > inline with the sifive timer definition documented in > > Documentation/devicetree/bindings/timer/sifive,clint.yaml. Rename the > > clint0 entry to "timer", fix the register mapping and fix the > > interrupt extended entry. > > > > This fixes boot failures with Kendryte K210 SoC boards. > > > > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > > --- > > arch/riscv/boot/dts/kendryte/k210.dtsi | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/arch/riscv/boot/dts/kendryte/k210.dtsi > > b/arch/riscv/boot/dts/kendryte/k210.dtsi > > index c1df56ccb8d5..48d65fea45e9 100644 > > --- a/arch/riscv/boot/dts/kendryte/k210.dtsi > > +++ b/arch/riscv/boot/dts/kendryte/k210.dtsi > > @@ -95,10 +95,11 @@ sysctl: sysctl@50440000 { > > #clock-cells = <1>; > > }; > > > > - clint0: interrupt-controller@2000000 { > > + timer@2000000 { > > compatible = "riscv,clint0"; > > - reg = <0x2000000 0xC000>; > > - interrupts-extended = <&cpu0_intc 3>, <&cpu1_intc > 3>; > > + reg = <0x2000000 0x10000>; > > Why the change in size? The docs from the SDK [1] (which I think were > copied from SiFive documentation around the time of their release) state > that the CLINT is 0xC000 bytes. In addition, accessing memory past > 0x0200C000 yields a repeating memory pattern: > > > 0200c000: 69823183 02020120 6083c006 00008002 .1.i ......`.... > 0200c010: 69823183 02020120 6083c006 00008002 .1.i ......`.... > 0200c020: 69823183 02020120 6083c006 00008002 .1.i ......`.... > 0200c030: 69823183 02020120 6083c006 00008002 .1.i ......`.... > (continues until 0x02010000) > > To me, it is clear that the CLINT is not 0x10000 in size, even though the legal > address space extends to that size. > > I am more curious how this fixes booting. What was the issue before? I agree. Only change in interrupts-extended is required and change in reg property can be dropped. Regards, Anup > > > + interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 > > + &cpu1_intc 3 &cpu1_intc 7>; > > clocks = <&sysctl K210_CLK_ACLK>; > > This clock is 50 times too fast(!) See [2] and [3] for a fix. I don't know if adding > a "virtual" clock is the best solution for Linux. I would like to keep the device > trees relatively in-sync, so please let me know if you think there is a better > solution. > > > }; > > > > > > [1] https://github.com/kendryte/kendryte-standalone- > sdk/blob/develop/lib/drivers/include/clint.h > [2] > https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929- > 8-seanga2@gmail.com/ > [3] > https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929- > 9-seanga2@gmail.com/ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
On 9/15/20 11:08 PM, Damien Le Moal wrote: > On 2020/09/16 11:15, Sean Anderson wrote: >> On 9/15/20 8:55 AM, Damien Le Moal wrote: >>> The Kendryte K210 SoC CLINT is compatible with Sifive clint v0 >>> (sifive,clint0). Fix the Kendryte K210 device tree clint entry to be >>> inline with the sifive timer definition documented in >>> Documentation/devicetree/bindings/timer/sifive,clint.yaml. Rename the >>> clint0 entry to "timer", fix the register mapping and fix the interrupt >>> extended entry. >>> >>> This fixes boot failures with Kendryte K210 SoC boards. >>> >>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> >>> --- >>> arch/riscv/boot/dts/kendryte/k210.dtsi | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/riscv/boot/dts/kendryte/k210.dtsi b/arch/riscv/boot/dts/kendryte/k210.dtsi >>> index c1df56ccb8d5..48d65fea45e9 100644 >>> --- a/arch/riscv/boot/dts/kendryte/k210.dtsi >>> +++ b/arch/riscv/boot/dts/kendryte/k210.dtsi >>> @@ -95,10 +95,11 @@ sysctl: sysctl@50440000 { >>> #clock-cells = <1>; >>> }; >>> >>> - clint0: interrupt-controller@2000000 { >>> + timer@2000000 { >>> compatible = "riscv,clint0"; >>> - reg = <0x2000000 0xC000>; >>> - interrupts-extended = <&cpu0_intc 3>, <&cpu1_intc 3>; >>> + reg = <0x2000000 0x10000>; >> >> Why the change in size? The docs from the SDK [1] (which I think were copied >> from SiFive documentation around the time of their release) state that >> the CLINT is 0xC000 bytes. In addition, accessing memory past 0x0200C000 >> yields a repeating memory pattern: > > Oops. Copy-paste error. I will change that back to 0x0000C000. >> >> >> 0200c000: 69823183 02020120 6083c006 00008002 .1.i ......`.... >> 0200c010: 69823183 02020120 6083c006 00008002 .1.i ......`.... >> 0200c020: 69823183 02020120 6083c006 00008002 .1.i ......`.... >> 0200c030: 69823183 02020120 6083c006 00008002 .1.i ......`.... >> (continues until 0x02010000) >> >> To me, it is clear that the CLINT is not 0x10000 in size, even though >> the legal address space extends to that size. >> >> I am more curious how this fixes booting. What was the issue before? > > There were 2 problems: > 1) The old declaration of the entry as clint0: interrupt-controller@2000000 led > to a failure to find the driver. > 2) The interrupt extended entry was wrong > >> >>> + interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 >>> + &cpu1_intc 3 &cpu1_intc 7>; >>> clocks = <&sysctl K210_CLK_ACLK>; >> >> This clock is 50 times too fast(!) See [2] and [3] for a fix. I don't >> know if adding a "virtual" clock is the best solution for Linux. I would >> like to keep the device trees relatively in-sync, so please let me know >> if you think there is a better solution. > > Huu... Let me check that. Once booted, using commands such as date or time does > not give me weird results, well at least, before this patch and clint changes. I > forgot to check yesterday when I tested (I only boot tested). Will check. > > And yes, I will make effort to keep the DT in sync. Is the Kendryte support > upstream in U-Boot now ? I have not checked recently. Yes, it was included with release v2020.07. --Sean > >> >>> }; >>> >>> >> >> [1] https://github.com/kendryte/kendryte-standalone-sdk/blob/develop/lib/drivers/include/clint.h >> [2] https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929-8-seanga2@gmail.com/ >> [3] https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929-9-seanga2@gmail.com/ >> > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, 2020-09-16 at 07:30 -0400, Sean Anderson wrote: > On 9/15/20 11:08 PM, Damien Le Moal wrote: > > On 2020/09/16 11:15, Sean Anderson wrote: > > > On 9/15/20 8:55 AM, Damien Le Moal wrote: > > > > The Kendryte K210 SoC CLINT is compatible with Sifive clint v0 > > > > (sifive,clint0). Fix the Kendryte K210 device tree clint entry to be > > > > inline with the sifive timer definition documented in > > > > Documentation/devicetree/bindings/timer/sifive,clint.yaml. Rename the > > > > clint0 entry to "timer", fix the register mapping and fix the interrupt > > > > extended entry. > > > > > > > > This fixes boot failures with Kendryte K210 SoC boards. > > > > > > > > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > > > > --- > > > > arch/riscv/boot/dts/kendryte/k210.dtsi | 7 ++++--- > > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/arch/riscv/boot/dts/kendryte/k210.dtsi b/arch/riscv/boot/dts/kendryte/k210.dtsi > > > > index c1df56ccb8d5..48d65fea45e9 100644 > > > > --- a/arch/riscv/boot/dts/kendryte/k210.dtsi > > > > +++ b/arch/riscv/boot/dts/kendryte/k210.dtsi > > > > @@ -95,10 +95,11 @@ sysctl: sysctl@50440000 { > > > > #clock-cells = <1>; > > > > }; > > > > > > > > - clint0: interrupt-controller@2000000 { > > > > + timer@2000000 { > > > > compatible = "riscv,clint0"; > > > > - reg = <0x2000000 0xC000>; > > > > - interrupts-extended = <&cpu0_intc 3>, <&cpu1_intc 3>; > > > > + reg = <0x2000000 0x10000>; > > > > > > Why the change in size? The docs from the SDK [1] (which I think were copied > > > from SiFive documentation around the time of their release) state that > > > the CLINT is 0xC000 bytes. In addition, accessing memory past 0x0200C000 > > > yields a repeating memory pattern: > > > > Oops. Copy-paste error. I will change that back to 0x0000C000. > > > > > > 0200c000: 69823183 02020120 6083c006 00008002 .1.i ......`.... > > > 0200c010: 69823183 02020120 6083c006 00008002 .1.i ......`.... > > > 0200c020: 69823183 02020120 6083c006 00008002 .1.i ......`.... > > > 0200c030: 69823183 02020120 6083c006 00008002 .1.i ......`.... > > > (continues until 0x02010000) > > > > > > To me, it is clear that the CLINT is not 0x10000 in size, even though > > > the legal address space extends to that size. > > > > > > I am more curious how this fixes booting. What was the issue before? > > > > There were 2 problems: > > 1) The old declaration of the entry as clint0: interrupt-controller@2000000 led > > to a failure to find the driver. > > 2) The interrupt extended entry was wrong > > > > > > + interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 > > > > + &cpu1_intc 3 &cpu1_intc 7>; > > > > clocks = <&sysctl K210_CLK_ACLK>; > > > > > > This clock is 50 times too fast(!) See [2] and [3] for a fix. I don't > > > know if adding a "virtual" clock is the best solution for Linux. I would > > > like to keep the device trees relatively in-sync, so please let me know > > > if you think there is a better solution. > > > > Huu... Let me check that. Once booted, using commands such as date or time does > > not give me weird results, well at least, before this patch and clint changes. I > > forgot to check yesterday when I tested (I only boot tested). Will check. > > > > And yes, I will make effort to keep the DT in sync. Is the Kendryte support > > upstream in U-Boot now ? I have not checked recently. > > Yes, it was included with release v2020.07. Looking at it right now. Trying to fix the clock mess. I think major surgery of the sysctl code is needed so it will take some time. The device tree fix v2 I sent still point to the "fake" ACLK clock for clint MTIME, all wrong. But at run-time, time seems to be OK, so leaving it as is for now. Will send a series fixing the clocks & PLL setup, which will allow enabling more peripherals too :) Thanks ! > > --Sean > > > > > }; > > > > > > > > > > > > > > [1] https://github.com/kendryte/kendryte-standalone-sdk/blob/develop/lib/drivers/include/clint.h > > > [2] https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929-8-seanga2@gmail.com/ > > > [3] https://patchwork.ozlabs.org/project/uboot/patch/20200915141724.503929-9-seanga2@gmail.com/ > > > -- Damien Le Moal Western Digital Research _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv