On Fri, Feb 05, 2021 at 05:39:51AM +0900, Hector Martin wrote:

apple

Don't make things different for this one platform (comparing to all
other platforms). Apple is not that special. :)

A lot here might be difficult to reverse-egineer or figure out by
ourself, so usually people rely on vendor sources (the open source
compliance package). Didn't you receive such for the iOS (or whatever
was on your Mac)?

+ */
+
+/dts-v1/;
+#include <dt-bindings/interrupt-controller/apple-aic.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+	model = "Apple Mac Mini M1 2020";
+	compatible = "AAPL,j274", "AAPL,m1", "AAPL,arm-platform";

I guess Rob will comment on the dt-bindings more... but for me a generic
"arm-platform" is too generic. What's the point of it? I didn't see any
of such generic compatibles in other platforms.

+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	chosen {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		bootargs = "earlycon";

This should not be hard-coded in DTS. Pass it from bootloader.

+		stdout-path = "serial0:1500000";

Use aliases.

+
+		framebuffer0: framebuffer@0 {
+			compatible = "AAPL,simple-framebuffer", "simple-framebuffer";
+			reg = <0 0 0 0>; // To be filled by loader
+			// Format properties will be added by loader

Use /* style of comments

+			status = "disabled";
+		};
+	};
+
+	memory@800000000 {
+		device_type = "memory";
+		reg = <0 0 0 0>; // To be filled by loader
+	};
+
+	aliases {
+		serial0 = &serial0;
+	};
+
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			compatible = "AAPL,icestorm";
+			device_type = "cpu";
+			reg = <0x0 0x0>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0 0>; // To be filled by loader
+		};
+		cpu1: cpu@1 {
+			compatible = "AAPL,icestorm";
+			device_type = "cpu";
+			reg = <0x0 0x1>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0 0>; // To be filled by loader
+		};
+		cpu2: cpu@2 {
+			compatible = "AAPL,icestorm";
+			device_type = "cpu";
+			reg = <0x0 0x2>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0 0>; // To be filled by loader
+		};
+		cpu3: cpu@3 {
+			compatible = "AAPL,icestorm";
+			device_type = "cpu";
+			reg = <0x0 0x3>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0 0>; // To be filled by loader
+		};
+		cpu4: cpu@10100 {
+			compatible = "AAPL,firestorm";
+			device_type = "cpu";
+			reg = <0x0 0x10100>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0 0>; // To be filled by loader
+		};
+		cpu5: cpu@10101 {
+			compatible = "AAPL,firestorm";
+			device_type = "cpu";
+			reg = <0x0 0x10101>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0 0>; // To be filled by loader
+		};
+		cpu6: cpu@10102 {
+			compatible = "AAPL,firestorm";
+			device_type = "cpu";
+			reg = <0x0 0x10102>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0 0>; // To be filled by loader
+		};
+		cpu7: cpu@10103 {
+			compatible = "AAPL,firestorm";
+			device_type = "cpu";
+			reg = <0x0 0x10103>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0 0>; // To be filled by loader
+		};
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupt-parent = <&aic>;
+		interrupts = <AIC_FIQ 0 IRQ_TYPE_LEVEL_HIGH>,
+				<AIC_FIQ 0 IRQ_TYPE_LEVEL_HIGH>,
+				<AIC_FIQ 1 IRQ_TYPE_LEVEL_HIGH>,
+				<AIC_FIQ 0 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	clk24: clk24 {

Just "clock". Node names should be generic.

+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <24000000>;
+		clock-output-names = "clk24";

What clock is it? Part of board or SoC? Isn't it a work-around for
missing clock drivers?

+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		aic: interrupt-controller@23b100000 {
+			compatible = "AAPL,m1-aic", "AAPL,aic";
+			#interrupt-cells = <3>;
+			interrupt-controller;
+			reg = <0x2 0x3b100000 0x0 0x8000>;
+		};
+
+		serial0: serial@235200000 {
+			compatible = "AAPL,s5l-uart";
+			reg = <0x2 0x35200000 0x0 0x1000>;
+			reg-io-width = <4>;
+			interrupt-parent = <&aic>;
+			interrupts = <AIC_IRQ 605 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk24>, <&clk24>;
+			clock-names = "uart", "clk_uart_baud0";
+		};
+

No blank lines at end of blocks.

Best regards,
Krzysztof