Re: [PATCH v4 17/19] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Conor Dooley
2023-02-27T18:11:17Z

[-- Attachment #1.1: Type: text/plain, Size: 1477 bytes --]

On Thu, Feb 23, 2023 at 03:16:51PM +0800, Hal Feng wrote:
> On Tue, 21 Feb 2023 17:03:52 +0000, Conor Dooley wrote:
> > On Tue, Feb 21, 2023 at 10:46:43AM +0800, Hal Feng wrote:
> >
> >> + 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 = <&ccache>;
> >> + riscv,isa = "rv64imac_zicsr_zba_zbb";
> >
> > I still think that adding just zicsr here is pointless. If you're going
> > to be specific, why not also mention that you have zifencei too?
>
> I would like to remove "_zicsr" in the next version. Thanks.
>
> >
> >> + tlb-split;
> >> + status = "disabled";
> >> +
> >> + cpu0_intc: interrupt-controller {
> >> + compatible = "riscv,cpu-intc";
> >> + interrupt-controller;
> >> + #interrupt-cells = <1>;
> >> + };
> >> + };
> >
> > Rest of this looks fine to me though, thanks for adding the s7
> > compatible and zba/zbb :)
>
> Thanks for your review. :)

I hadn't actually given you one yet ;)

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]