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. :) Best regards, Hal