Re: [PATCH] riscv: Add support for AMD/Xilinx MicroBlaze V

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

Yo,

Since you sent it to me, I may as well comment...

On Fri, Nov 03, 2023 at 05:03:25PM +0100, Michal Simek wrote:
> MicroBlaze V is new AMD/Xilinx soft-core 32bit RISC-V processor IP.
> It is hardware compatible with classic MicroBlaze processor.
> 
> The patch contains initial wiring and configuration for initial HW design
> with memory, cpu, interrupt controller, timers and uartlite console.
> 
> Provided DT is just describing one configuration and should be taken only
> as example.


> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * dts file for Xilinx MicroBlaze V
> + *
> + * (C) Copyright 2023, Advanced Micro Devices, Inc.
> + *
> + * Michal Simek <michal.simek@amd.com>
> + */
> +
> +/dts-v1/;
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	model = "Xilinx MicroBlaze V 32bit";
> +	compatible = "xlnx,mbv32";
> +
> +	cpus: cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		timebase-frequency = <102000000>;
> +		cpu_0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "riscv";

You're missing a cpu specific compatible here.
"riscv" in isolation is only for {emu,simu}lators.

> +			reg = <0>;
> +			status = "okay";
> +			riscv,isa = "rv32imafdc";
> +			clock-frequency = <100000000>;
> +			i-cache-size = <32768>;
> +			d-cache-size = <32768>;

Missing an interrupt-controller child node for the cpu-intc, no?

> +		};
> +	};
> +
> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		bootargs = "earlycon";
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	memory@20000000 {
> +		device_type = "memory";
> +		reg = <0x20000000 0x20000000>;
> +	};
> +
> +	axi: axi {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		ranges;
> +		bootph-all;
> +
> +		axi_intc: interrupt-controller@41200000 {
> +			compatible = "xlnx.xps-intc";

This is some non-standard interrupt controller, rather than a plic,
right?

Also, should you not also have a riscv,timer node?

Cheers,
Conor.

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