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

[Michael Ellerman <mpe@ellerman.id.au>]

Paul Mackerras <paulus@ozlabs.org> writes:
>

Little bit of change log never hurts :)

> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> ---
>  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