Re: [PATCH v2 5/5] ARM: dts: NSP: Add DT files for Meraki MX65 series

From: Vladimir Oltean
Date: Fri Jun 11 2021 - 16:49:11 EST

On Fri, Jun 11, 2021 at 12:27:17AM +0100, Matthew Hagan wrote:
> MX65 Hardware info:
> - CPU: Broadcom BCM58625 Cortex A9 @ 1200Mhz
> - RAM: 2 GB (4 x 4Gb SK Hynix H5TC4G83CFR)
> - Storage: 1 GB (Micron MT29F8G08ABACA)
> - Networking: BCM58625 switch (2x 1GbE ports, used for WAN ports 1 & 2)
> 2x Qualcomm QCA8337 switches (10x 1GbE ports, used for LAN ports 3-12)
> - PSE: Broadcom BCM59111KMLG connected to LAN ports 11 & 12
> - USB: 1x USB2.0
> - Serial: Internal header
>
> As with the MX64, the MX65 has 2x Broadcom BCM43520KMLG on the PCI bus.
>
> Signed-off-by: Matthew Hagan <mnhagan88@xxxxxxxxx>
> ---
> arch/arm/boot/dts/bcm958625-meraki-alamo.dtsi | 298 ++++++++++++++++++
> arch/arm/boot/dts/bcm958625-meraki-mx65.dts | 15 +
> arch/arm/boot/dts/bcm958625-meraki-mx65w.dts | 23 ++
> 3 files changed, 336 insertions(+)
> create mode 100644 arch/arm/boot/dts/bcm958625-meraki-alamo.dtsi
> create mode 100644 arch/arm/boot/dts/bcm958625-meraki-mx65.dts
> create mode 100644 arch/arm/boot/dts/bcm958625-meraki-mx65w.dts
>
> diff --git a/arch/arm/boot/dts/bcm958625-meraki-alamo.dtsi b/arch/arm/boot/dts/bcm958625-meraki-alamo.dtsi
> new file mode 100644
> index 000000000000..0ff78fb89b6d
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm958625-meraki-alamo.dtsi
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Device Tree Bindings for Cisco Meraki MX65 series (Alamo).
> + *
> + * Copyright (C) 2021 Matthew Hagan <mnhagan88@xxxxxxxxx>
> + */
> +
> +#include "bcm958625-meraki-mx6x-common.dtsi"
> +
> +#include <dt-bindings/input/input.h>
> +
> +/ {
> + aliases {
> + mdio-mux-mmio = &mdiomux0;
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + orange_power {
> + label = "orange:power";
> + gpios = <&gpioa 3 GPIO_ACTIVE_HIGH>;
> + default-state = "on";
> + };
> +
> + wan1_right {
> + label = "green:wan1-right";
> + gpios = <&gpioa 24 GPIO_ACTIVE_LOW>;
> + };
> +
> + wan1_left {
> + label = "green:wan1-left";
> + gpios = <&gpioa 25 GPIO_ACTIVE_LOW>;
> + };
> +
> + wan2_right {
> + label = "green:wan2-right";
> + gpios = <&gpioa 26 GPIO_ACTIVE_LOW>;
> + };
> +
> + wan2_left {
> + label = "green:wan2-left";
> + gpios = <&gpioa 27 GPIO_ACTIVE_LOW>;
> + };
> +
> + white_status {
> + label = "white:status";
> + gpios = <&gpioa 31 GPIO_ACTIVE_HIGH>;
> + };
> + };
> +
> + keys {
> + compatible = "gpio-keys-polled";
> + autorepeat;
> + poll-interval = <20>;
> +
> + reset {
> + label = "reset";
> + linux,code = <KEY_RESTART>;
> + gpios = <&gpioa 8 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> + mdio: mdio@18032000 {
> + compatible = "brcm,iproc-mdio";
> + reg = <0x18032000 0x8>;
> + #size-cells = <0>;
> + #address-cells = <1>;
> + };

Odd that mdio@18032000 is under the / node and not under /soc? I don't
think you can combine nodes with a unit address with nodes without in
the same level of the hierarchy.

Also, it is odd that you define something SoC-specific like this in a
board file. Isn't this actually stray and not used?

> +
> + mdiomux0: mdio-mux {
> + compatible = "mdio-mux-mmioreg";
> + reg = <0x18032000 0x4>;
> + mux-mask = <0x200>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + mdio-parent-bus = <&mdio>;
> +
> + mdio_ext: mdio@200 {
> + reg = <0x200>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +
> + mdio-mii-mux {
> + compatible = "mdio-mux-mmioreg";
> + reg = <0x1803f1c0 0x4>;
> + mux-mask = <0x2000>;
> + mdio-parent-bus = <&mdio_ext>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mdio@0 {
> + reg = <0x0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy_port6: phy@0 {
> + reg = <0>;
> + };
> +
> + phy_port7: phy@1 {
> + reg = <1>;
> + };
> +
> + phy_port8: phy@2 {
> + reg = <2>;
> + };
> +
> + phy_port9: phy@3 {
> + reg = <3>;
> + };
> +
> + phy_port10: phy@4 {
> + reg = <4>;
> + };
> +
> + switch@10 {
> + compatible = "qca,qca8337";
> + #address-cells = <1>;
> + #size-cells = <0>;

The "ports" node is adimensional, so address-cells and size-cells are
not needed.

> + reg = <0x10>;
> + dsa,member = <1 0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + label = "cpu";

'label' property for the CPU port is not needed/used. We know it is a
CPU port because it has the 'ethernet' property.

> + ethernet = <&sgmii1>;
> + phy-mode = "sgmii";
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + label = "lan8";
> + phy-handle = <&phy_port6>;
> + };
> +
> + port@2 {
> + reg = <2>;
> + label = "lan9";
> + phy-handle = <&phy_port7>;
> + };
> +
> + port@3 {
> + reg = <3>;
> + label = "lan10";
> + phy-handle = <&phy_port8>;
> + };
> +
> + port@4 {
> + reg = <4>;
> + label = "lan11";
> + phy-handle = <&phy_port9>;
> + };
> +
> + port@5 {
> + reg = <5>;
> + label = "lan12";
> + phy-handle = <&phy_port10>;
> + };
> + };
> + };
> + };
> +
> + mdio-mii@2000 {
> + reg = <0x2000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy_port1: phy@0 {
> + reg = <0>;
> + };
> +
> + phy_port2: phy@1 {
> + reg = <1>;
> + };
> +
> + phy_port3: phy@2 {
> + reg = <2>;
> + };
> +
> + phy_port4: phy@3 {
> + reg = <3>;
> + };
> +
> + phy_port5: phy@4 {
> + reg = <4>;
> + };
> +
> + switch@10 {
> + compatible = "qca,qca8337";
> + #address-cells = <1>;
> + #size-cells = <0>;

Similar comment.

> + reg = <0x10>;
> + dsa,member = <2 0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + label = "cpu";
> + ethernet = <&sgmii0>;
> + phy-mode = "sgmii";
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + label = "lan3";
> + phy-handle = <&phy_port1>;
> + };
> +
> + port@2 {
> + reg = <2>;
> + label = "lan4";
> + phy-handle = <&phy_port2>;
> + };
> +
> + port@3 {
> + reg = <3>;
> + label = "lan5";
> + phy-handle = <&phy_port3>;
> + };
> +
> + port@4 {
> + reg = <4>;
> + label = "lan6";
> + phy-handle = <&phy_port4>;
> + };
> +
> + port@5 {
> + reg = <5>;
> + label = "lan7";
> + phy-handle = <&phy_port5>;
> + };
> + };
> + };
> + };
> + };
> +};
> +
> +&srab {
> + compatible = "brcm,bcm58625-srab", "brcm,nsp-srab";
> + status = "okay";
> + dsa,member = <0 0>;
> +
> + ports {
> + port@0 {
> + label = "wan1";
> + reg = <0>;
> + };
> +
> + port@1 {
> + label = "wan2";
> + reg = <1>;
> + };
> +
> + sgmii0: port@4 {
> + label = "sw0";
> + reg = <4>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> + sgmii1: port@5 {
> + label = "sw1";
> + reg = <5>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> + port@8 {
> + ethernet = <&amac2>;
> + label = "cpu";
> + reg = <8>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> +};
> diff --git a/arch/arm/boot/dts/bcm958625-meraki-mx65.dts b/arch/arm/boot/dts/bcm958625-meraki-mx65.dts
> new file mode 100644
> index 000000000000..aec86c3a4d05
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm958625-meraki-mx65.dts
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Device Tree Bindings for Cisco Meraki MX65.
> + *
> + * Copyright (C) 2021 Matthew Hagan <mnhagan88@xxxxxxxxx>
> + */
> +
> +/dts-v1/;
> +
> +#include "bcm958625-meraki-alamo.dtsi"
> +
> +/ {
> + model = "Cisco Meraki MX65";
> + compatible = "meraki,mx65", "brcm,bcm58625", "brcm,nsp";
> +};
> diff --git a/arch/arm/boot/dts/bcm958625-meraki-mx65w.dts b/arch/arm/boot/dts/bcm958625-meraki-mx65w.dts
> new file mode 100644
> index 000000000000..0045a33055c1
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm958625-meraki-mx65w.dts
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Device Tree Bindings for Cisco Meraki MX65W.
> + *
> + * Copyright (C) 2021 Matthew Hagan <mnhagan88@xxxxxxxxx>
> + */
> +
> +/dts-v1/;
> +
> +#include "bcm958625-meraki-mx65x.dtsi"
> +
> +/ {
> + model = "Cisco Meraki MX65W";
> + compatible = "meraki,mx65w", "brcm,bcm58625", "brcm,nsp";
> +};
> +
> +&pcie0 {
> + status = "okay";
> +};
> +
> +&pcie1 {
> + status = "okay";
> +};
> --
> 2.26.3
>