Re: [PATCH v1 2/2] riscv: dts: starfive: add the missing monitor core

[Icenowy Zheng]

在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
> From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> 
> The JH7100 has a 32 bit monitor core that is missing from the device
> tree. Add it (and its cpu-map entry) to more accurately reflect the
> actual topology of the SoC.
> 
> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> ---
>  arch/riscv/boot/dts/starfive/jh7100.dtsi | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> index c617a61e26e2..92fce5b66d3d 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> @@ -67,6 +67,23 @@ cpu1_intc: interrupt-controller {
>                         };
>                 };
>  
> +               E24: cpu@2 {
> +                       compatible = "sifive,e24", "riscv";
> +                       reg = <2>;
> +                       device_type = "cpu";
> +                       i-cache-block-size = <32>;
> +                       i-cache-sets = <256>;
> +                       i-cache-size = <16384>;
> +                       riscv,isa = "rv32imafc";
> +                       status = "disabled";
> +
> +                       cpu2_intc: interrupt-controller {
> +                               compatible = "riscv,cpu-intc";
> +                               interrupt-controller;
> +                               #interrupt-cells = <1>;
> +                       };
> +               };
> +
>                 cpu-map {
>                         cluster0 {
>                                 core0 {
> @@ -76,6 +93,10 @@ core0 {
>                                 core1 {
>                                         cpu = <&U74_1>;
>                                 };
> +
> +                               core2 {
> +                                       cpu = <&E24>;
> +                               };

Sorry but I think this change makes the topology more inaccurate.

The E24 core is very independent, just another CPU core connected the
same bus -- even no coherency (E24 takes AHB, which is not coherency-
sensible). Even the TAP of it is independent with the U74 TAP.

And by default it does not boot any proper code (if a debugger is
attached, it will discover that the E24 is in consistently fault at 0x0
(mtvec is 0x0 and when fault it jumps to 0x0 and fault again), until
its clock is just shutdown by Linux cleaning up unused clocks.)

Personally I think it should be implemented as a remoteproc instead.

>                         };
>                 };
>         };