Thread (26 messages) flat view 26 messages, 4 authors, 2021-06-18

Re: [PATCH 02/11] powerpc: Add Microwatt device tree

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2021-06-17 04:41:59

Paul Mackerras [off-list ref] writes:
Little bit of change log never hurts :)
quoted hunk ↗ jump to hunk
Signed-off-by: Paul Mackerras <redacted>
---
 arch/powerpc/boot/dts/microwatt.dts | 105 ++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 arch/powerpc/boot/dts/microwatt.dts
diff --git a/arch/powerpc/boot/dts/microwatt.dts b/arch/powerpc/boot/dts/microwatt.dts
new file mode 100644
index 000000000000..9b2e64da9432
--- /dev/null
+++ b/arch/powerpc/boot/dts/microwatt.dts
@@ -0,0 +1,105 @@
+/dts-v1/;
+
+/ {
+	#size-cells = <0x02>;
+	#address-cells = <0x02>;
+	model-name = "microwatt";
+	compatible = "microwatt-soc";
+
+	reserved-memory {
+		#size-cells = <0x02>;
+		#address-cells = <0x02>;
+		ranges;
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x00000000 0x00000000 0x00000000 0x10000000>;
+	};
+
+	cpus {
+		#size-cells = <0x00>;
+		#address-cells = <0x01>;
+
+		ibm,powerpc-cpu-features {
+			display-name = "Microwatt";
+			isa = <3000>;
+			device_type = "cpu-features";
+			compatible = "ibm,powerpc-cpu-features";
+
+			mmu-radix {
+				isa = <3000>;
+				usable-privilege = <2>;
skiboot says 6?
+				os-support = <0x00>;
+			};
+
+			little-endian {
+				isa = <0>;
I guess you just copied that from skiboot.

The binding says it's required, but AFAICS the kernel doesn't use it.

And isa = 0 mean ISA_BASE, according to the skiboot source.
+				usable-privilege = <3>;
+				os-support = <0x00>;
+			};
+
+			cache-inhibited-large-page {
+				isa = <0x00>;
+				usable-privilege = <2>;
skiboot says 6, ie. HV and OS.
Don't think it actually matters because you say os-support = 0.
+				os-support = <0x00>;
+			};
+
+			fixed-point-v3 {
+				isa = <3000>;
+				usable-privilege = <3>;
skiboot says 7.
+			};
+
+			no-execute {
+				isa = <0x00>;
+				usable-privilege = <2>;
skiboot says 6.
+				os-support = <0x00>;
+			};
+
+			floating-point {
+				hfscr-bit-nr = <0x00>;
+				hwcap-bit-nr = <0x1b>;
Looks right, bit 27:

#define PPC_FEATURE_HAS_FPU		0x08000000

+				isa = <0x00>;
+				usable-privilege = <0x07>;
+				hv-support = <0x00>;
+				os-support = <0x00>;
+			};
+		};
+
+		PowerPC,Microwatt@0 {
+			i-cache-sets = <2>;
+			ibm,dec-bits = <64>;
+			reservation-granule-size = <64>;
Never seen that one before.
+			clock-frequency = <100000000>;
+			timebase-frequency = <100000000>;
Those seem quite high?
+			i-tlb-sets = <1>;
+			ibm,ppc-interrupt-server#s = <0>;
+			i-cache-block-size = <64>;
+			d-cache-block-size = <64>;
The kernel reads those, but also hard codes 128 in places.
See L1_CACHE_BYTES.
+			ibm,pa-features = [40 00 c2 27 00 00 00 80 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 80 00 80 00 80 00 00 00 80 00 80 00 00 00 80 00 80 00 80 00 80 00 80 00 80 00 80 00 80 00 80 00 80 00 80 00 80 00 80 00];
Do you need that?

You shouldn't, if we've done things right with the cpu-features support.
+			d-cache-sets = <2>;
+			ibm,pir = <0x3c>;
Needed?
+			i-tlb-size = <64>;
+			cpu-version = <0x990000>;
+			status = "okay";
+			i-cache-size = <0x1000>;
+			ibm,processor-radix-AP-encodings = <0x0c 0xa0000010 0x20000015 0x4000001e>;
+			tlb-size = <0>;
+			tlb-sets = <0>;
Does the kernel use those? I can't find it.
+			device_type = "cpu";
+			d-tlb-size = <128>;
+			d-tlb-sets = <2>;
+			reg = <0>;
+			general-purpose;
+			64-bit;
+			d-cache-size = <0x1000>;
+			ibm,chip-id = <0x00>;
+		};
+	};
+
+	chosen {
+		bootargs = "";
+		ibm,architecture-vec-5 = [19 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 40 00 40];
Do you need that?

I assume you run with MSR[HV] = 1 (you don't say anywhere), in which
case we never look at that property.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help