Re: [RFT PATCH v3 27/27] arm64: apple: Add initial Apple Mac mini (M1, 2020) devicetree
From: Konrad Dybcio <hidden>
Date: 2021-03-13 20:24:32
Hi!
quoted hunk ↗ jump to hunk
+++ b/arch/arm64/boot/dts/apple/t8103-j274.dts@@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0+ OR MIT
It seems to be "// SPDX-License-Identifier: (GPL-2.0+ OR MIT)" in other DTs (notice the brackets).
+/* + * Apple Mac mini (M1, 2020)
Not sure if it's necessary, you already state it in model=""
+ * + * target-type: J174
Isn't that a typo? Compatible states J*2*74, but I'm curious if that's intentional.
+ * + * Copyright The Asahi Linux Contributors
* Copyright (C) 2021 The Asahi Linux Contributors
+ */
+
+/dts-v1/;
+
+#include "t8103.dtsi"
+
+/ {
+ compatible = "apple,j274", "apple,t8103", "apple,arm-platform";
+ model = "Apple Mac mini (M1, 2020)";
+
+ aliases {
+ serial0 = &serial0;Isn't this M1-common?
+ };
+
+ chosen {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ stdout-path = "serial0";Doesn't serial work without this?
+
+ framebuffer0: framebuffer@0 {Not sure if we want the @0 (unless it's also overwritten by the loader).
+ compatible = "apple,simple-framebuffer", "simple-framebuffer"; + reg = <0 0 0 0>; /* To be filled by loader */ + /* Format properties will be added by loader */ + status = "disabled";
Does it get enabled by the loader? Also, isn't the framebuffer common for all M1 devices?
+ };
+ };
+
+ memory@800000000 {
+ device_type = "memory";
+ reg = <0x8 0 0x2 0>; /* To be filled by loader */
+ };Shouldn't this be in the SoC DTSI?
quoted hunk ↗ jump to hunk
+}; + +&serial0 { + status = "okay"; +};diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi new file mode 100644 index 000000000000..aac9e4e6abc5 --- /dev/null +++ b/arch/arm64/boot/dts/apple/t8103.dtsi@@ -0,0 +1,135 @@ +// SPDX-License-Identifier: GPL-2.0+ OR MIT +/* + * Apple T8103 "M1" SoC + * + * Other names: H13G, "Tonga" + * + * Copyright The Asahi Linux Contributors
Ditto
+ */
+
+#include <dt-bindings/interrupt-controller/apple-aic.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+ compatible = "apple,t8103", "apple,arm-platform";Please remove this line, it's getting overwritten anyway.
+ };
+
+ soc {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ ranges;
+ nonposted-mmio;
+I think " compatible = "simple-bus"; #address-cells = <2>; #size-cells = <2>; nonposted-mmio; ranges; " would look more coherent to the human eye, but that's rather a nit. Other than this, node order seems to be entirely random. Please sort them by address (where applicable) and by name where not, so that this doesn't become a huge mess as we go forward. Konrad _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel