Re: [PATCH v2 3/5] ARM: dts: NSP: Add common bindings for MX64/MX65

On 11/06/2021 21:30, Vladimir Oltean wrote:

On Fri, Jun 11, 2021 at 12:27:15AM +0100, Matthew Hagan wrote:

quoted

These bindings are required for all Meraki MX64/MX65 devices.

Signed-off-by: Matthew Hagan <redacted>
---
 .../dts/bcm958625-meraki-mx6x-common.dtsi     | 148 ++++++++++++++++++
 1 file changed, 148 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm958625-meraki-mx6x-common.dtsi

diff --git a/arch/arm/boot/dts/bcm958625-meraki-mx6x-common.dtsi b/arch/arm/boot/dts/bcm958625-meraki-mx6x-common.dtsi
new file mode 100644
index 000000000000..47a30dedf7b3
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-meraki-mx6x-common.dtsi

@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Common Bindings for Cisco Meraki MX64 (Kingpin) and MX65 (Alamo) devices.
+ *
+ * Copyright (C) 2021 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+#include "bcm-nsp.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x60000000 0x80000000>;
+	};
+
+	pwm-leds {
+		compatible = "pwm-leds";
+
+		red {
+			label = "red:led";
+			pwms = <&pwm 1 50000>;
+		};
+
+		green {
+			label = "green:led";
+			pwms = <&pwm 2 50000>;
+		};
+
+		blue {
+			label = "blue:led";
+			pwms = <&pwm 3 50000>;
+		};
+	};
+};
+
+&L2 {
+	arm,io-coherent;
+	prefetch-data = <1>;
+	prefetch-instr = <1>;
+};

It is common practice to sort labels alphabetically and nodes by unit address.

This will be done. Thanks.

quoted

+
+&uart0 {
+	clock-frequency = <62500000>;
+	status = "okay";
+};
+
+&i2c0 {
+	status = "okay";
+	at24@50 {
+		compatible = "atmel,24c64";
+		pagesize = <32>;
+		reg = <0x50>;
+	};
+};
+
+&amac2 {
+	status = "okay";
+};
+
+&nand {
+	nandcs@0 {
+		compatible = "brcm,nandcs";
+		reg = <0>;
+		nand-on-flash-bbt;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		nand-ecc-strength = <24>;
+		nand-ecc-step-size = <1024>;
+
+		brcm,nand-oob-sector-size = <27>;
+
+		partition@0 {
+			label = "u-boot";
+			reg = <0x0 0x80000>;
+			read-only;
+		};
+
+		partition@80000 {
+			label = "shmoo";
+			reg = <0x80000 0x80000>;
+			read-only;
+		};
+
+		partition@100000 {
+			label = "bootkernel1";
+			reg = <0x100000 0x300000>;
+		};
+
+		partition@400000 {
+			label = "senao_nvram";
+			reg = <0x400000 0x100000>;
+		};
+
+		partition@500000 {
+			label = "bootkernel2";
+			reg = <0x500000 0x300000>;
+		};
+
+		partition@800000 {
+			label = "ubi";
+			reg = <0x800000 0x3f700000>;
+		};
+	};
+};
+
+&qspi {
+	status = "disabled";
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&ohci0 {
+	status = "okay";
+};
+
+&pwm {
+	status = "okay";
+	#pwm-cells = <2>;

What is the reason for overriding this to 2?

I can't provide an explanation other than that it only works at 2.

When set to 3 I get the following:

[    0.784051] OF: /pwm-leds/red: #pwm-cells = 3 found -1
[    0.789201] of_pwm_get(): can't parse "pwms" property
[    0.794295] leds_pwm pwm-leds: error -EINVAL: unable to request PWM for red:led
[    0.801628] leds_pwm: probe of pwm-leds failed with error -22

quoted

+	chan0 {
+		channel = <1>;
+		active_low = <1>;
+		};

Bad indentation for this bracket.

This will be removed in v3

quoted

+	chan1 {
+		channel = <2>;
+		active_low = <1>;
+	};
+	chan2 {
+		channel = <3>;
+		active_low = <1>;
+	};
+};
+
+&ccbtimer1 {
+	status = "disabled";
+};
+
+&sata_phy {
+	status = "disabled";
+};

It is common practice to disable these in the common SoC dtsi and let
individual boards enable them as necessary, instead of the opposite.

Will add patches in v3 to disable ccbtimers and qspi by default. The sata_phy one is not required.

quoted

-- 
2.26.3