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

[Conor Dooley]

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@xxxxxxxxxxxxx>

Thanks,
Conor.

Attachment: signature.asc
Description: PGP signature