Re: [PATCH v3 14/17] riscv: dts: jh7110: Add initial StarFive JH7110 device tree
On Tue, Mar 07, 2023 at 06:30:19AM +, Conor Dooley wrote:
>
>
> On 7 March 2023 01:59:31 GMT, yanhong wang
> wrote:
> >
> >
> >On 2023/3/4 5:16, Conor Dooley wrote:
> >> On Fri, Mar 03, 2023 at 11:24:29AM +0800, Yanhong Wang wrote:
> >>> Add initial device tree for the JH7110 RISC-V SoC.
> >>>
> >>> Signed-off-by: Yanhong Wang
> >>> ---
> >>> arch/riscv/dts/jh7110.dtsi | 582 +
> >>> 1 file changed, 582 insertions(+)
> >>> create mode 100644 arch/riscv/dts/jh7110.dtsi
> >>>
> >>> diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
> >>> new file mode 100644
> >>> index 00..d3e9f92987
> >>> --- /dev/null
> >>> +++ b/arch/riscv/dts/jh7110.dtsi
> >>> @@ -0,0 +1,582 @@
> >>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> >>> +/*
> >>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +#include
> >>> +#include
> >>> +
> >>> +/ {
> >>> + compatible = "starfive,jh7110";
> >>> + #address-cells = <2>;
> >>> + #size-cells = <2>;
> >>> +
> >>> + cpus {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + S7_0: cpu@0 {
> >>> + compatible = "sifive,s7", "riscv";
> >>> + reg = <0>;
> >>> + d-cache-block-size = <64>;
> >>> + d-cache-sets = <64>;
> >>> + d-cache-size = <8192>;
> >>> + d-tlb-sets = <1>;
> >>> + d-tlb-size = <40>;
> >>> + device_type = "cpu";
> >>> + i-cache-block-size = <64>;
> >>> + i-cache-sets = <64>;
> >>> + i-cache-size = <16384>;
> >>> + i-tlb-sets = <1>;
> >>> + i-tlb-size = <40>;
> >>> + mmu-type = "riscv,sv39";
> >>> + next-level-cache = <>;
> >>> + riscv,isa = "rv64imac_zba_zbb";
> >>
> >> Hmm, based on what Sean said on the previous version, "We use strchr on
> >> it; so something like Zicsr is parsed as 5 extensions", are you sure that
> >> adding this here behaves correctly?
> >>
> >
> > As you said, u-boot does not parse the content after '_', zba/zbb has
> > no practical meaning in u-boot.
>
> That's not what Sean's comment on the previous version said.
> If it is actually ignored, this is fine, but Sean's comment read like
> it would be misinterpreted by U-Boot.
> I'll have to go read the code.
Having gone and found the code in question, it does indeed look like it
stops at an _, supports_extension() in arch/riscv/cpu/cpu.c, so having
Zba and Zbb in the riscv,isa string is fine.
Apologies for the noise here, I must've misunderstood the comments on
the previous version.
Cheers,
Conor.
signature.asc
Description: PGP signature
Re: [PATCH v3 14/17] riscv: dts: jh7110: Add initial StarFive JH7110 device tree
On 7 March 2023 01:59:31 GMT, yanhong wang
wrote:
>
>
>On 2023/3/4 5:16, Conor Dooley wrote:
>> On Fri, Mar 03, 2023 at 11:24:29AM +0800, Yanhong Wang wrote:
>>> Add initial device tree for the JH7110 RISC-V SoC.
>>>
>>> Signed-off-by: Yanhong Wang
>>> ---
>>> arch/riscv/dts/jh7110.dtsi | 582 +
>>> 1 file changed, 582 insertions(+)
>>> create mode 100644 arch/riscv/dts/jh7110.dtsi
>>>
>>> diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
>>> new file mode 100644
>>> index 00..d3e9f92987
>>> --- /dev/null
>>> +++ b/arch/riscv/dts/jh7110.dtsi
>>> @@ -0,0 +1,582 @@
>>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>>> +/*
>>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include
>>> +#include
>>> +
>>> +/ {
>>> + compatible = "starfive,jh7110";
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> +
>>> + cpus {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + S7_0: cpu@0 {
>>> + compatible = "sifive,s7", "riscv";
>>> + reg = <0>;
>>> + d-cache-block-size = <64>;
>>> + d-cache-sets = <64>;
>>> + d-cache-size = <8192>;
>>> + d-tlb-sets = <1>;
>>> + d-tlb-size = <40>;
>>> + device_type = "cpu";
>>> + i-cache-block-size = <64>;
>>> + i-cache-sets = <64>;
>>> + i-cache-size = <16384>;
>>> + i-tlb-sets = <1>;
>>> + i-tlb-size = <40>;
>>> + mmu-type = "riscv,sv39";
>>> + next-level-cache = <>;
>>> + riscv,isa = "rv64imac_zba_zbb";
>>
>> Hmm, based on what Sean said on the previous version, "We use strchr on
>> it; so something like Zicsr is parsed as 5 extensions", are you sure that
>> adding this here behaves correctly?
>>
>
>As you said, u-boot does not parse the content after '_', zba/zbb has no
>practical meaning in u-boot.
That's not what Sean's comment on the previous version said.
If it is actually ignored, this is fine, but Sean's comment read like it would
be misinterpreted by U-Boot.
I'll have to go read the code.
>The definitions of 'riscv,isa' refer to linux and is consistent with this.
>
>https://patchwork.kernel.org/project/linux-riscv/patch/20230221024645.127922-18-hal.f...@starfivetech.com/
>
>
>Do you have any better suggestion? Referring to the definition of Sifive
>fu740, remove '_zba_zbb'?
The fu740 doesn't have those extensions.
>
>> https://lore.kernel.org/u-boot/22e805d4-f823-975c-a970-a4a19bb13...@gmail.com/
>>
>> I know that having zba/zbb is *correct*, but if U-Boot doesn't parse it
>> correctly...
>>
>> Cheers,
>> Conor.
Re: [PATCH v3 14/17] riscv: dts: jh7110: Add initial StarFive JH7110 device tree
On 2023/3/4 5:16, Conor Dooley wrote:
> On Fri, Mar 03, 2023 at 11:24:29AM +0800, Yanhong Wang wrote:
>> Add initial device tree for the JH7110 RISC-V SoC.
>>
>> Signed-off-by: Yanhong Wang
>> ---
>> arch/riscv/dts/jh7110.dtsi | 582 +
>> 1 file changed, 582 insertions(+)
>> create mode 100644 arch/riscv/dts/jh7110.dtsi
>>
>> diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
>> new file mode 100644
>> index 00..d3e9f92987
>> --- /dev/null
>> +++ b/arch/riscv/dts/jh7110.dtsi
>> @@ -0,0 +1,582 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>> +/*
>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>> + */
>> +
>> +/dts-v1/;
>> +#include
>> +#include
>> +
>> +/ {
>> +compatible = "starfive,jh7110";
>> +#address-cells = <2>;
>> +#size-cells = <2>;
>> +
>> +cpus {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +
>> +S7_0: cpu@0 {
>> +compatible = "sifive,s7", "riscv";
>> +reg = <0>;
>> +d-cache-block-size = <64>;
>> +d-cache-sets = <64>;
>> +d-cache-size = <8192>;
>> +d-tlb-sets = <1>;
>> +d-tlb-size = <40>;
>> +device_type = "cpu";
>> +i-cache-block-size = <64>;
>> +i-cache-sets = <64>;
>> +i-cache-size = <16384>;
>> +i-tlb-sets = <1>;
>> +i-tlb-size = <40>;
>> +mmu-type = "riscv,sv39";
>> +next-level-cache = <>;
>> +riscv,isa = "rv64imac_zba_zbb";
>
> Hmm, based on what Sean said on the previous version, "We use strchr on
> it; so something like Zicsr is parsed as 5 extensions", are you sure that
> adding this here behaves correctly?
>
As you said, u-boot does not parse the content after '_', zba/zbb has no
practical meaning in u-boot.
The definitions of 'riscv,isa' refer to linux and is consistent with this.
https://patchwork.kernel.org/project/linux-riscv/patch/20230221024645.127922-18-hal.f...@starfivetech.com/
Do you have any better suggestion? Referring to the definition of Sifive
fu740, remove '_zba_zbb'?
> https://lore.kernel.org/u-boot/22e805d4-f823-975c-a970-a4a19bb13...@gmail.com/
>
> I know that having zba/zbb is *correct*, but if U-Boot doesn't parse it
> correctly...
>
> Cheers,
> Conor.
Re: [PATCH v3 14/17] riscv: dts: jh7110: Add initial StarFive JH7110 device tree
On Fri, Mar 03, 2023 at 11:24:29AM +0800, Yanhong Wang wrote:
> Add initial device tree for the JH7110 RISC-V SoC.
>
> Signed-off-by: Yanhong Wang
> ---
> arch/riscv/dts/jh7110.dtsi | 582 +
> 1 file changed, 582 insertions(+)
> create mode 100644 arch/riscv/dts/jh7110.dtsi
>
> diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
> new file mode 100644
> index 00..d3e9f92987
> --- /dev/null
> +++ b/arch/riscv/dts/jh7110.dtsi
> @@ -0,0 +1,582 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + */
> +
> +/dts-v1/;
> +#include
> +#include
> +
> +/ {
> + compatible = "starfive,jh7110";
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + S7_0: cpu@0 {
> + compatible = "sifive,s7", "riscv";
> + reg = <0>;
> + d-cache-block-size = <64>;
> + d-cache-sets = <64>;
> + d-cache-size = <8192>;
> + d-tlb-sets = <1>;
> + d-tlb-size = <40>;
> + device_type = "cpu";
> + i-cache-block-size = <64>;
> + i-cache-sets = <64>;
> + i-cache-size = <16384>;
> + i-tlb-sets = <1>;
> + i-tlb-size = <40>;
> + mmu-type = "riscv,sv39";
> + next-level-cache = <>;
> + riscv,isa = "rv64imac_zba_zbb";
Hmm, based on what Sean said on the previous version, "We use strchr on
it; so something like Zicsr is parsed as 5 extensions", are you sure that
adding this here behaves correctly?
https://lore.kernel.org/u-boot/22e805d4-f823-975c-a970-a4a19bb13...@gmail.com/
I know that having zba/zbb is *correct*, but if U-Boot doesn't parse it
correctly...
Cheers,
Conor.
signature.asc
Description: PGP signature
[PATCH v3 14/17] riscv: dts: jh7110: Add initial StarFive JH7110 device tree
Add initial device tree for the JH7110 RISC-V SoC.
Signed-off-by: Yanhong Wang
---
arch/riscv/dts/jh7110.dtsi | 582 +
1 file changed, 582 insertions(+)
create mode 100644 arch/riscv/dts/jh7110.dtsi
diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
new file mode 100644
index 00..d3e9f92987
--- /dev/null
+++ b/arch/riscv/dts/jh7110.dtsi
@@ -0,0 +1,582 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ */
+
+/dts-v1/;
+#include
+#include
+
+/ {
+ compatible = "starfive,jh7110";
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ S7_0: cpu@0 {
+ compatible = "sifive,s7", "riscv";
+ reg = <0>;
+ d-cache-block-size = <64>;
+ d-cache-sets = <64>;
+ d-cache-size = <8192>;
+ d-tlb-sets = <1>;
+ d-tlb-size = <40>;
+ device_type = "cpu";
+ i-cache-block-size = <64>;
+ i-cache-sets = <64>;
+ i-cache-size = <16384>;
+ i-tlb-sets = <1>;
+ i-tlb-size = <40>;
+ mmu-type = "riscv,sv39";
+ next-level-cache = <>;
+ riscv,isa = "rv64imac_zba_zbb";
+ tlb-split;
+ status = "disabled";
+
+ cpu0_intc: interrupt-controller {
+ compatible = "riscv,cpu-intc";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
+ };
+
+ U74_1: cpu@1 {
+ compatible = "sifive,u74-mc", "riscv";
+ reg = <1>;
+ d-cache-block-size = <64>;
+ d-cache-sets = <64>;
+ d-cache-size = <32768>;
+ d-tlb-sets = <1>;
+ d-tlb-size = <40>;
+ device_type = "cpu";
+ i-cache-block-size = <64>;
+ i-cache-sets = <64>;
+ i-cache-size = <32768>;
+ i-tlb-sets = <1>;
+ i-tlb-size = <40>;
+ mmu-type = "riscv,sv39";
+ next-level-cache = <>;
+ riscv,isa = "rv64imafdc_zba_zbb";
+ tlb-split;
+
+ cpu1_intc: interrupt-controller {
+ compatible = "riscv,cpu-intc";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
+ };
+
+ U74_2: cpu@2 {
+ compatible = "sifive,u74-mc", "riscv";
+ reg = <2>;
+ d-cache-block-size = <64>;
+ d-cache-sets = <64>;
+ d-cache-size = <32768>;
+ d-tlb-sets = <1>;
+ d-tlb-size = <40>;
+ device_type = "cpu";
+ i-cache-block-size = <64>;
+ i-cache-sets = <64>;
+ i-cache-size = <32768>;
+ i-tlb-sets = <1>;
+ i-tlb-size = <40>;
+ mmu-type = "riscv,sv39";
+ next-level-cache = <>;
+ riscv,isa = "rv64imafdc_zba_zbb";
+ tlb-split;
+
+ cpu2_intc: interrupt-controller {
+ compatible = "riscv,cpu-intc";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
+ };
+
+ U74_3: cpu@3 {
+ compatible = "sifive,u74-mc", "riscv";
+ reg = <3>;
+ d-cache-block-size = <64>;
+ d-cache-sets = <64>;
+ d-cache-size = <32768>;
+ d-tlb-sets = <1>;
+ d-tlb-size = <40>;
+ device_type = "cpu";
+ i-cache-block-size = <64>;
+ i-cache-sets = <64>;
+ i-cache-size = <32768>;
+ i-tlb-sets = <1>;
+ i-tlb-size = <40>;
+ mmu-type = "riscv,sv39";
+ next-level-cache = <>;
+ riscv,isa = "rv64imafdc_zba_zbb";
+ tlb-split;
+
+ cpu3_intc: interrupt-controller {
+
