linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Debix Model A board devicetree
@ 2022-10-17 15:10 Daniel Scally
  2022-10-17 15:10 ` [PATCH v4 1/3] dt-bindings: vendor-prefixes: Add Polyhex Technology Co Daniel Scally
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Daniel Scally @ 2022-10-17 15:10 UTC (permalink / raw)
  To: krzysztof.kozlowski, shawnguo, robh, marcel.ziswiler, leoyang.li,
	devicetree, linux-arm-kernel
  Cc: s.hauer, kernel, festevam, linux-imx, laurent.pinchart,
	kieran.bingham, debix-tech, Daniel Scally

Hello

This series adds a .dts file for the Polyhex Debix Model A board [1]
A binding for the vendor is also added.

[1] http://www.polyhex.net/product/embedded-motherboard/board/nxp.html?id=483

Thanks
Dan

Daniel Scally (3):
  dt-bindings: vendor-prefixes: Add Polyhex Technology Co.
  dt-bindings: arm: fsl: Enumerate Debix Model A Board
  arm64: dts: Add device tree for the Debix Model A Board

 .../devicetree/bindings/arm/fsl.yaml          |   1 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 arch/arm64/boot/dts/freescale/Makefile        |   1 +
 .../dts/freescale/imx8mp-debix-model-a.dts    | 529 ++++++++++++++++++
 4 files changed, 533 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/3] dt-bindings: vendor-prefixes: Add Polyhex Technology Co.
  2022-10-17 15:10 [PATCH v4 0/3] Debix Model A board devicetree Daniel Scally
@ 2022-10-17 15:10 ` Daniel Scally
  2022-10-17 15:10 ` [PATCH v4 2/3] dt-bindings: arm: fsl: Enumerate Debix Model A Board Daniel Scally
  2022-10-17 15:10 ` [PATCH v4 3/3] arm64: dts: Add device tree for the " Daniel Scally
  2 siblings, 0 replies; 11+ messages in thread
From: Daniel Scally @ 2022-10-17 15:10 UTC (permalink / raw)
  To: krzysztof.kozlowski, shawnguo, robh, marcel.ziswiler, leoyang.li,
	devicetree, linux-arm-kernel
  Cc: s.hauer, kernel, festevam, linux-imx, laurent.pinchart,
	kieran.bingham, debix-tech, Daniel Scally

Add an entry for Polyhex Technology Co. to vendor-prefixes.yaml

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v4:

  - None

Changes in v3:

  - None

Changes in v2:

  - None

 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 2f0151e9f6be..7ba1d2c11aa9 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -995,6 +995,8 @@ patternProperties:
     description: PocketBook International SA
   "^polaroid,.*":
     description: Polaroid Corporation
+  "^polyhex,.*":
+    description: Polyhex Technology Co. Ltd.
   "^portwell,.*":
     description: Portwell Inc.
   "^poslab,.*":
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/3] dt-bindings: arm: fsl: Enumerate Debix Model A Board
  2022-10-17 15:10 [PATCH v4 0/3] Debix Model A board devicetree Daniel Scally
  2022-10-17 15:10 ` [PATCH v4 1/3] dt-bindings: vendor-prefixes: Add Polyhex Technology Co Daniel Scally
@ 2022-10-17 15:10 ` Daniel Scally
  2022-10-17 15:10 ` [PATCH v4 3/3] arm64: dts: Add device tree for the " Daniel Scally
  2 siblings, 0 replies; 11+ messages in thread
From: Daniel Scally @ 2022-10-17 15:10 UTC (permalink / raw)
  To: krzysztof.kozlowski, shawnguo, robh, marcel.ziswiler, leoyang.li,
	devicetree, linux-arm-kernel
  Cc: s.hauer, kernel, festevam, linux-imx, laurent.pinchart,
	kieran.bingham, debix-tech, Daniel Scally

Add an entry to the list of imx8mp boards denoting the Debix Model A
board from Polyhex Technology Co.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v4:

        - None

Changes in v3:

    - Dropped (2GB) from the comment too (Kieran)

Changes in v2:

    - Dropped 2gb suffix, added hyphens for readability (Laurent)

 Documentation/devicetree/bindings/arm/fsl.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 7431579ab0e8..1cb72eda8652 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -931,6 +931,7 @@ properties:
               - dh,imx8mp-dhcom-pdk2      # i.MX8MP DHCOM SoM on PDK2 board
               - fsl,imx8mp-evk            # i.MX8MP EVK Board
               - gateworks,imx8mp-gw74xx   # i.MX8MP Gateworks Board
+              - polyhex,imx8mp-debix-model-a # Polyhex Debix Model A Board
               - toradex,verdin-imx8mp     # Verdin iMX8M Plus Modules
               - toradex,verdin-imx8mp-nonwifi  # Verdin iMX8M Plus Modules without Wi-Fi / BT
               - toradex,verdin-imx8mp-wifi  # Verdin iMX8M Plus Wi-Fi / BT Modules
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/3] arm64: dts: Add device tree for the Debix Model A Board
  2022-10-17 15:10 [PATCH v4 0/3] Debix Model A board devicetree Daniel Scally
  2022-10-17 15:10 ` [PATCH v4 1/3] dt-bindings: vendor-prefixes: Add Polyhex Technology Co Daniel Scally
  2022-10-17 15:10 ` [PATCH v4 2/3] dt-bindings: arm: fsl: Enumerate Debix Model A Board Daniel Scally
@ 2022-10-17 15:10 ` Daniel Scally
  2022-10-29  3:50   ` Shawn Guo
  2022-12-01 17:10   ` Ahmad Fatoum
  2 siblings, 2 replies; 11+ messages in thread
From: Daniel Scally @ 2022-10-17 15:10 UTC (permalink / raw)
  To: krzysztof.kozlowski, shawnguo, robh, marcel.ziswiler, leoyang.li,
	devicetree, linux-arm-kernel
  Cc: s.hauer, kernel, festevam, linux-imx, laurent.pinchart,
	kieran.bingham, debix-tech, Daniel Scally

Add a device tree file describing the Debix Model A board from
Polyhex Technology Co.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v4 (Laurent):

        - Aligned the pinctrl group values with each other

Changes in v3 (Laurent):

    - Added IOB copyright notice
    - Removed the eth node for the connector that's on the separate I/O
    board
    - Re-ordered some pinctrl groups so they're alphabetical

Changes in v2:

    - Fixed the interrupt flag for i2c1/pmic@25
    - Fixed the node name for i2c4/rtc@51 (was "hym8563@51")
    - Fixed a group control name that didn't match the bindings pattern
    - Re-compared the rest of the DT with the EVK's .dts file to try to
    make sure it complies with the way things should be, hopefully without
    missing anything...

 arch/arm64/boot/dts/freescale/Makefile        |   1 +
 .../dts/freescale/imx8mp-debix-model-a.dts    | 529 ++++++++++++++++++
 2 files changed, 530 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 8bf7f7ecebaa..6a33a08946ac 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -80,6 +80,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mn-venice-gw7902.dtb
+dtb-$(CONFIG_ARCH_MXC) += imx8mp-debix-model-a.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk2.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts
new file mode 100644
index 000000000000..92cbd6d2b73f
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts
@@ -0,0 +1,529 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2019 NXP
+ * Copyright 2022 Ideas on Board Oy
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/usb/pd.h>
+
+#include "imx8mp.dtsi"
+
+/ {
+	model = "Polyhex Debix Model A i.MX8MPlus board";
+	compatible = "polyhex,imx8mp-debix-model-a", "fsl,imx8mp";
+
+	chosen {
+		stdout-path = &uart2;
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_gpio_led>;
+
+		status-led {
+			function = LED_FUNCTION_POWER;
+			color = <LED_COLOR_ID_RED>;
+			gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+	};
+
+	reg_usdhc2_vmmc: regulator-usdhc2 {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
+		regulator-name = "VSD_3V3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+};
+
+&A53_0 {
+	cpu-supply = <&buck2>;
+};
+
+&A53_1 {
+	cpu-supply = <&buck2>;
+};
+
+&A53_2 {
+	cpu-supply = <&buck2>;
+};
+
+&A53_3 {
+	cpu-supply = <&buck2>;
+};
+
+&eqos {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_eqos>;
+	phy-connection-type = "rgmii-id";
+	phy-handle = <&ethphy0>;
+	status = "okay";
+
+	mdio {
+		compatible = "snps,dwmac-mdio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ethphy0: ethernet-phy@0 {
+			compatible = "ethernet-phy-ieee802.3-c22";
+			reg = <0>;
+			reset-gpios = <&gpio4 18 GPIO_ACTIVE_LOW>;
+			reset-assert-us = <20>;
+			reset-deassert-us = <200000>;
+		};
+	};
+};
+
+&i2c1 {
+	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c1>;
+	status = "okay";
+
+	pmic@25 {
+		reg = <0x25>;
+		compatible = "nxp,pca9450c";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pmic>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <3 IRQ_TYPE_EDGE_RISING>;
+
+		regulators {
+			buck1: BUCK1 {
+				regulator-name = "BUCK1";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <2187500>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <3125>;
+			};
+
+			buck2: BUCK2 {
+				regulator-name = "BUCK2";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <2187500>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <3125>;
+				nxp,dvs-run-voltage = <950000>;
+				nxp,dvs-standby-voltage = <850000>;
+			};
+
+			buck4: BUCK4{
+				regulator-name = "BUCK4";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <3400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			buck5: BUCK5{
+				regulator-name = "BUCK5";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <3400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			buck6: BUCK6 {
+				regulator-name = "BUCK6";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <3400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo1: LDO1 {
+				regulator-name = "LDO1";
+				regulator-min-microvolt = <1600000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo2: LDO2 {
+				regulator-name = "LDO2";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1150000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo3: LDO3 {
+				regulator-name = "LDO3";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo4: LDO4 {
+				regulator-name = "LDO4";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo5: LDO5 {
+				regulator-name = "LDO5";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
+};
+
+&i2c2 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c2>;
+	status = "okay";
+};
+
+&i2c3 {
+	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c3>;
+	status = "okay";
+};
+
+&i2c4 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c4>;
+	status = "okay";
+
+	eeprom@50 {
+		compatible = "atmel,24c02";
+		reg = <0x50>;
+		pagesize = <16>;
+	};
+
+	rtc@51 {
+		compatible = "haoyu,hym8563";
+		reg = <0x51>;
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+		clock-output-names = "xin32k";
+		interrupt-parent = <&gpio2>;
+		interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_rtc_int>;
+	};
+};
+
+&i2c6 {
+	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c6>;
+	status = "okay";
+};
+
+&iomuxc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hog>;
+
+	pinctrl_hog: hoggrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL			0x400001c3
+			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA			0x400001c3
+			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD				0x40000019
+			MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC				0x40000019
+		>;
+	};
+
+	pinctrl_eqos: eqosgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_ENET_MDC__ENET_QOS_MDC				0x3
+			MX8MP_IOMUXC_ENET_MDIO__ENET_QOS_MDIO				0x3
+			MX8MP_IOMUXC_ENET_RD0__ENET_QOS_RGMII_RD0			0x91
+			MX8MP_IOMUXC_ENET_RD1__ENET_QOS_RGMII_RD1			0x91
+			MX8MP_IOMUXC_ENET_RD2__ENET_QOS_RGMII_RD2			0x91
+			MX8MP_IOMUXC_ENET_RD3__ENET_QOS_RGMII_RD3			0x91
+			MX8MP_IOMUXC_ENET_RXC__CCM_ENET_QOS_CLOCK_GENERATE_RX_CLK	0x91
+			MX8MP_IOMUXC_ENET_RX_CTL__ENET_QOS_RGMII_RX_CTL			0x91
+			MX8MP_IOMUXC_ENET_TD0__ENET_QOS_RGMII_TD0			0x1f
+			MX8MP_IOMUXC_ENET_TD1__ENET_QOS_RGMII_TD1			0x1f
+			MX8MP_IOMUXC_ENET_TD2__ENET_QOS_RGMII_TD2			0x1f
+			MX8MP_IOMUXC_ENET_TD3__ENET_QOS_RGMII_TD3			0x1f
+			MX8MP_IOMUXC_ENET_TX_CTL__ENET_QOS_RGMII_TX_CTL			0x1f
+			MX8MP_IOMUXC_ENET_TXC__CCM_ENET_QOS_CLOCK_GENERATE_TX_CLK	0x1f
+			MX8MP_IOMUXC_SAI1_RXFS__ENET1_1588_EVENT0_IN			0x1f
+			MX8MP_IOMUXC_SAI1_RXC__ENET1_1588_EVENT0_OUT			0x1f
+			MX8MP_IOMUXC_SAI1_TXD6__GPIO4_IO18				0x19
+		>;
+	};
+
+	pinctrl_fec: fecgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI1_RXD2__ENET1_MDC				0x3
+			MX8MP_IOMUXC_SAI1_RXD3__ENET1_MDIO				0x3
+			MX8MP_IOMUXC_SAI1_RXD4__ENET1_RGMII_RD0				0x91
+			MX8MP_IOMUXC_SAI1_RXD5__ENET1_RGMII_RD1				0x91
+			MX8MP_IOMUXC_SAI1_RXD6__ENET1_RGMII_RD2				0x91
+			MX8MP_IOMUXC_SAI1_RXD7__ENET1_RGMII_RD3				0x91
+			MX8MP_IOMUXC_SAI1_TXC__ENET1_RGMII_RXC				0x91
+			MX8MP_IOMUXC_SAI1_TXFS__ENET1_RGMII_RX_CTL			0x91
+			MX8MP_IOMUXC_SAI1_TXD0__ENET1_RGMII_TD0				0x1f
+			MX8MP_IOMUXC_SAI1_TXD1__ENET1_RGMII_TD1				0x1f
+			MX8MP_IOMUXC_SAI1_TXD2__ENET1_RGMII_TD2				0x1f
+			MX8MP_IOMUXC_SAI1_TXD3__ENET1_RGMII_TD3				0x1f
+			MX8MP_IOMUXC_SAI1_TXD4__ENET1_RGMII_TX_CTL			0x1f
+			MX8MP_IOMUXC_SAI1_TXD5__ENET1_RGMII_TXC				0x1f
+			MX8MP_IOMUXC_SAI1_RXD1__ENET1_1588_EVENT1_OUT			0x1f
+			MX8MP_IOMUXC_SAI1_RXD0__ENET1_1588_EVENT1_IN			0x1f
+			MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19				0x19
+		>;
+	};
+
+	pinctrl_gpio_led: gpioledgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_READY_B__GPIO3_IO16				0x19
+		>;
+	};
+
+	pinctrl_i2c1: i2c1grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL					0x400001c2
+			MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA					0x400001c2
+		>;
+	};
+
+	pinctrl_i2c2: i2c2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL					0x400001c2
+			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA					0x400001c2
+		>;
+	};
+
+	pinctrl_i2c3: i2c3grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL					0x400001c2
+			MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA					0x400001c2
+		>;
+	};
+	pinctrl_i2c4: i2c4grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C4_SCL__I2C4_SCL					0x400001c3
+			MX8MP_IOMUXC_I2C4_SDA__I2C4_SDA					0x400001c3
+		>;
+	};
+
+	pinctrl_i2c6: i2c6grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI5_RXFS__I2C6_SCL				0x400001c3
+			MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA					0x400001c3
+		>;
+	};
+
+	pinctrl_pmic: pmicirqgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03				0x41
+		>;
+	};
+
+	pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19				0x41
+		>;
+	};
+
+	pinctrl_rtc_int: rtcintgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD1_STROBE__GPIO2_IO11				0x140
+		>;
+	};
+
+	pinctrl_uart2: uart2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX				0x14f
+			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX				0x14f
+		>;
+	};
+
+	pinctrl_uart3: uart3grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_UART3_RXD__UART3_DCE_RX				0x49
+			MX8MP_IOMUXC_UART3_TXD__UART3_DCE_TX				0x49
+		>;
+	};
+
+	pinctrl_uart4: uart4grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_UART4_RXD__UART4_DCE_RX				0x49
+			MX8MP_IOMUXC_UART4_TXD__UART4_DCE_TX				0x49
+		>;
+	};
+
+	pinctrl_usb1_vbus: usb1grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI2_TXC__GPIO4_IO25				0x19
+			MX8MP_IOMUXC_SAI2_TXD0__GPIO4_IO26				0x19
+		>;
+	};
+
+	pinctrl_usdhc2: usdhc2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x190
+			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d0
+			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d0
+			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d0
+			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d0
+			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d0
+			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
+		>;
+	};
+
+	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x194
+			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d4
+			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d4
+			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d4
+			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d4
+			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d4
+			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
+		>;
+	};
+
+	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x196
+			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d6
+			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d6
+			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d6
+			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d6
+			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d6
+			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
+		>;
+	};
+
+	pinctrl_usdhc2_gpio: usdhc2gpiogrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12				0x1c4
+		>;
+	};
+
+	pinctrl_usdhc3: usdhc3grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK				0x190
+			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD				0x1d0
+			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0				0x1d0
+			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1				0x1d0
+			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2				0x1d0
+			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3				0x1d0
+			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4				0x1d0
+			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5				0x1d0
+			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6				0x1d0
+			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7				0x1d0
+			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE				0x190
+		>;
+	};
+
+	pinctrl_usdhc3_100mhz: usdhc3-100mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK				0x194
+			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD				0x1d4
+			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0				0x1d4
+			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1				0x1d4
+			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2				0x1d4
+			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3				0x1d4
+			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4				0x1d4
+			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5				0x1d4
+			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6				0x1d4
+			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7				0x1d4
+			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE				0x194
+		>;
+	};
+
+	pinctrl_usdhc3_200mhz: usdhc3-200mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK				0x196
+			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD				0x1d6
+			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0				0x1d6
+			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1				0x1d6
+			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2				0x1d6
+			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3				0x1d6
+			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4				0x1d6
+			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5				0x1d6
+			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6				0x1d6
+			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7				0x1d6
+			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE				0x196
+		>;
+	};
+
+	pinctrl_wdog: wdoggrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_GPIO1_IO02__WDOG1_WDOG_B				0xc6
+		>;
+	};
+};
+
+&snvs_pwrkey {
+	status = "okay";
+};
+
+&uart2 {
+	/* console */
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart2>;
+	status = "okay";
+};
+
+&uart3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart3>;
+	status = "okay";
+};
+
+&uart4 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart4>;
+	status = "okay";
+};
+
+/* SD Card */
+&usdhc2 {
+	assigned-clocks = <&clk IMX8MP_CLK_USDHC2>;
+	assigned-clock-rates = <400000000>;
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
+	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
+	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
+	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
+	vmmc-supply = <&reg_usdhc2_vmmc>;
+	bus-width = <4>;
+	status = "okay";
+};
+
+/* eMMc */
+&usdhc3 {
+	assigned-clocks = <&clk IMX8MP_CLK_USDHC3>;
+	assigned-clock-rates = <400000000>;
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc3>;
+	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+	bus-width = <8>;
+	non-removable;
+	status = "okay";
+};
+
+&wdog1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_wdog>;
+	fsl,ext-reset-output;
+	status = "okay";
+};
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: Add device tree for the Debix Model A Board
  2022-10-17 15:10 ` [PATCH v4 3/3] arm64: dts: Add device tree for the " Daniel Scally
@ 2022-10-29  3:50   ` Shawn Guo
  2022-12-06  8:05     ` Dan Scally
  2022-12-01 17:10   ` Ahmad Fatoum
  1 sibling, 1 reply; 11+ messages in thread
From: Shawn Guo @ 2022-10-29  3:50 UTC (permalink / raw)
  To: Daniel Scally
  Cc: krzysztof.kozlowski, robh, marcel.ziswiler, leoyang.li,
	devicetree, linux-arm-kernel, s.hauer, kernel, festevam,
	linux-imx, laurent.pinchart, kieran.bingham, debix-tech

On Mon, Oct 17, 2022 at 04:10:50PM +0100, Daniel Scally wrote:
> Add a device tree file describing the Debix Model A board from
> Polyhex Technology Co.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v4 (Laurent):
> 
>         - Aligned the pinctrl group values with each other
> 
> Changes in v3 (Laurent):
> 
>     - Added IOB copyright notice
>     - Removed the eth node for the connector that's on the separate I/O
>     board
>     - Re-ordered some pinctrl groups so they're alphabetical
> 
> Changes in v2:
> 
>     - Fixed the interrupt flag for i2c1/pmic@25
>     - Fixed the node name for i2c4/rtc@51 (was "hym8563@51")
>     - Fixed a group control name that didn't match the bindings pattern
>     - Re-compared the rest of the DT with the EVK's .dts file to try to
>     make sure it complies with the way things should be, hopefully without
>     missing anything...
> 
>  arch/arm64/boot/dts/freescale/Makefile        |   1 +
>  .../dts/freescale/imx8mp-debix-model-a.dts    | 529 ++++++++++++++++++
>  2 files changed, 530 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts
> 
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 8bf7f7ecebaa..6a33a08946ac 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -80,6 +80,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mn-venice-gw7902.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-debix-model-a.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk2.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts
> new file mode 100644
> index 000000000000..92cbd6d2b73f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts
> @@ -0,0 +1,529 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2019 NXP
> + * Copyright 2022 Ideas on Board Oy
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/usb/pd.h>
> +
> +#include "imx8mp.dtsi"
> +
> +/ {
> +	model = "Polyhex Debix Model A i.MX8MPlus board";
> +	compatible = "polyhex,imx8mp-debix-model-a", "fsl,imx8mp";
> +
> +	chosen {
> +		stdout-path = &uart2;
> +	};
> +
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpio_led>;
> +
> +		status-led {

Did you run 'make dtbs_check'? The node name doesn't seem to match
pattern defined by leds-gpio.yaml.

> +			function = LED_FUNCTION_POWER;
> +			color = <LED_COLOR_ID_RED>;
> +			gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
> +			default-state = "on";
> +		};
> +	};
> +
> +	reg_usdhc2_vmmc: regulator-usdhc2 {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
> +		regulator-name = "VSD_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +};
> +
> +&A53_0 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_1 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_2 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_3 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&eqos {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_eqos>;
> +	phy-connection-type = "rgmii-id";
> +	phy-handle = <&ethphy0>;
> +	status = "okay";
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethphy0: ethernet-phy@0 {
> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <0>;
> +			reset-gpios = <&gpio4 18 GPIO_ACTIVE_LOW>;
> +			reset-assert-us = <20>;
> +			reset-deassert-us = <200000>;
> +		};
> +	};
> +};
> +
> +&i2c1 {
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	status = "okay";
> +
> +	pmic@25 {
> +		reg = <0x25>;
> +		compatible = "nxp,pca9450c";

We usually start properties with `compatible`.

> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pmic>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <3 IRQ_TYPE_EDGE_RISING>;
> +
> +		regulators {
> +			buck1: BUCK1 {
> +				regulator-name = "BUCK1";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <2187500>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <3125>;
> +			};
> +
> +			buck2: BUCK2 {
> +				regulator-name = "BUCK2";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <2187500>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <3125>;
> +				nxp,dvs-run-voltage = <950000>;
> +				nxp,dvs-standby-voltage = <850000>;
> +			};
> +
> +			buck4: BUCK4{
> +				regulator-name = "BUCK4";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <3400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			buck5: BUCK5{
> +				regulator-name = "BUCK5";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <3400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			buck6: BUCK6 {
> +				regulator-name = "BUCK6";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <3400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo1: LDO1 {
> +				regulator-name = "LDO1";
> +				regulator-min-microvolt = <1600000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo2: LDO2 {
> +				regulator-name = "LDO2";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1150000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo3: LDO3 {
> +				regulator-name = "LDO3";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo4: LDO4 {
> +				regulator-name = "LDO4";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo5: LDO5 {
> +				regulator-name = "LDO5";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +		};
> +	};
> +};
> +
> +&i2c2 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	status = "okay";
> +};
> +
> +&i2c3 {
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c3>;
> +	status = "okay";
> +};
> +
> +&i2c4 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c4>;
> +	status = "okay";
> +
> +	eeprom@50 {
> +		compatible = "atmel,24c02";
> +		reg = <0x50>;
> +		pagesize = <16>;
> +	};
> +
> +	rtc@51 {
> +		compatible = "haoyu,hym8563";
> +		reg = <0x51>;
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +		clock-output-names = "xin32k";
> +		interrupt-parent = <&gpio2>;
> +		interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_rtc_int>;
> +	};
> +};
> +
> +&i2c6 {
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c6>;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_hog>;
> +
> +	pinctrl_hog: hoggrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL			0x400001c3
> +			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA			0x400001c3
> +			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD				0x40000019
> +			MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC				0x40000019
> +		>;
> +	};
> +
> +	pinctrl_eqos: eqosgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_ENET_MDC__ENET_QOS_MDC				0x3
> +			MX8MP_IOMUXC_ENET_MDIO__ENET_QOS_MDIO				0x3
> +			MX8MP_IOMUXC_ENET_RD0__ENET_QOS_RGMII_RD0			0x91
> +			MX8MP_IOMUXC_ENET_RD1__ENET_QOS_RGMII_RD1			0x91
> +			MX8MP_IOMUXC_ENET_RD2__ENET_QOS_RGMII_RD2			0x91
> +			MX8MP_IOMUXC_ENET_RD3__ENET_QOS_RGMII_RD3			0x91
> +			MX8MP_IOMUXC_ENET_RXC__CCM_ENET_QOS_CLOCK_GENERATE_RX_CLK	0x91
> +			MX8MP_IOMUXC_ENET_RX_CTL__ENET_QOS_RGMII_RX_CTL			0x91
> +			MX8MP_IOMUXC_ENET_TD0__ENET_QOS_RGMII_TD0			0x1f
> +			MX8MP_IOMUXC_ENET_TD1__ENET_QOS_RGMII_TD1			0x1f
> +			MX8MP_IOMUXC_ENET_TD2__ENET_QOS_RGMII_TD2			0x1f
> +			MX8MP_IOMUXC_ENET_TD3__ENET_QOS_RGMII_TD3			0x1f
> +			MX8MP_IOMUXC_ENET_TX_CTL__ENET_QOS_RGMII_TX_CTL			0x1f
> +			MX8MP_IOMUXC_ENET_TXC__CCM_ENET_QOS_CLOCK_GENERATE_TX_CLK	0x1f
> +			MX8MP_IOMUXC_SAI1_RXFS__ENET1_1588_EVENT0_IN			0x1f
> +			MX8MP_IOMUXC_SAI1_RXC__ENET1_1588_EVENT0_OUT			0x1f
> +			MX8MP_IOMUXC_SAI1_TXD6__GPIO4_IO18				0x19
> +		>;
> +	};
> +
> +	pinctrl_fec: fecgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SAI1_RXD2__ENET1_MDC				0x3
> +			MX8MP_IOMUXC_SAI1_RXD3__ENET1_MDIO				0x3
> +			MX8MP_IOMUXC_SAI1_RXD4__ENET1_RGMII_RD0				0x91
> +			MX8MP_IOMUXC_SAI1_RXD5__ENET1_RGMII_RD1				0x91
> +			MX8MP_IOMUXC_SAI1_RXD6__ENET1_RGMII_RD2				0x91
> +			MX8MP_IOMUXC_SAI1_RXD7__ENET1_RGMII_RD3				0x91
> +			MX8MP_IOMUXC_SAI1_TXC__ENET1_RGMII_RXC				0x91
> +			MX8MP_IOMUXC_SAI1_TXFS__ENET1_RGMII_RX_CTL			0x91
> +			MX8MP_IOMUXC_SAI1_TXD0__ENET1_RGMII_TD0				0x1f
> +			MX8MP_IOMUXC_SAI1_TXD1__ENET1_RGMII_TD1				0x1f
> +			MX8MP_IOMUXC_SAI1_TXD2__ENET1_RGMII_TD2				0x1f
> +			MX8MP_IOMUXC_SAI1_TXD3__ENET1_RGMII_TD3				0x1f
> +			MX8MP_IOMUXC_SAI1_TXD4__ENET1_RGMII_TX_CTL			0x1f
> +			MX8MP_IOMUXC_SAI1_TXD5__ENET1_RGMII_TXC				0x1f
> +			MX8MP_IOMUXC_SAI1_RXD1__ENET1_1588_EVENT1_OUT			0x1f
> +			MX8MP_IOMUXC_SAI1_RXD0__ENET1_1588_EVENT1_IN			0x1f
> +			MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19				0x19
> +		>;
> +	};
> +
> +	pinctrl_gpio_led: gpioledgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_NAND_READY_B__GPIO3_IO16				0x19
> +		>;
> +	};
> +
> +	pinctrl_i2c1: i2c1grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL					0x400001c2
> +			MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA					0x400001c2
> +		>;
> +	};
> +
> +	pinctrl_i2c2: i2c2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL					0x400001c2
> +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA					0x400001c2
> +		>;
> +	};
> +
> +	pinctrl_i2c3: i2c3grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL					0x400001c2
> +			MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA					0x400001c2
> +		>;
> +	};

Missing newline.

Shawn

> +	pinctrl_i2c4: i2c4grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C4_SCL__I2C4_SCL					0x400001c3
> +			MX8MP_IOMUXC_I2C4_SDA__I2C4_SDA					0x400001c3
> +		>;
> +	};
> +
> +	pinctrl_i2c6: i2c6grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SAI5_RXFS__I2C6_SCL				0x400001c3
> +			MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA					0x400001c3
> +		>;
> +	};
> +
> +	pinctrl_pmic: pmicirqgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03				0x41
> +		>;
> +	};
> +
> +	pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19				0x41
> +		>;
> +	};
> +
> +	pinctrl_rtc_int: rtcintgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD1_STROBE__GPIO2_IO11				0x140
> +		>;
> +	};
> +
> +	pinctrl_uart2: uart2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX				0x14f
> +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX				0x14f
> +		>;
> +	};
> +
> +	pinctrl_uart3: uart3grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_UART3_RXD__UART3_DCE_RX				0x49
> +			MX8MP_IOMUXC_UART3_TXD__UART3_DCE_TX				0x49
> +		>;
> +	};
> +
> +	pinctrl_uart4: uart4grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_UART4_RXD__UART4_DCE_RX				0x49
> +			MX8MP_IOMUXC_UART4_TXD__UART4_DCE_TX				0x49
> +		>;
> +	};
> +
> +	pinctrl_usb1_vbus: usb1grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SAI2_TXC__GPIO4_IO25				0x19
> +			MX8MP_IOMUXC_SAI2_TXD0__GPIO4_IO26				0x19
> +		>;
> +	};
> +
> +	pinctrl_usdhc2: usdhc2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x190
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d0
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x194
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d4
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d4
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d4
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d4
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d4
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x196
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d6
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d6
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d6
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d6
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d6
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_gpio: usdhc2gpiogrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12				0x1c4
> +		>;
> +	};
> +
> +	pinctrl_usdhc3: usdhc3grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK				0x190
> +			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD				0x1d0
> +			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0				0x1d0
> +			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1				0x1d0
> +			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2				0x1d0
> +			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3				0x1d0
> +			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4				0x1d0
> +			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5				0x1d0
> +			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6				0x1d0
> +			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7				0x1d0
> +			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE				0x190
> +		>;
> +	};
> +
> +	pinctrl_usdhc3_100mhz: usdhc3-100mhzgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK				0x194
> +			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD				0x1d4
> +			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0				0x1d4
> +			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1				0x1d4
> +			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2				0x1d4
> +			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3				0x1d4
> +			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4				0x1d4
> +			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5				0x1d4
> +			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6				0x1d4
> +			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7				0x1d4
> +			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE				0x194
> +		>;
> +	};
> +
> +	pinctrl_usdhc3_200mhz: usdhc3-200mhzgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK				0x196
> +			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD				0x1d6
> +			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0				0x1d6
> +			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1				0x1d6
> +			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2				0x1d6
> +			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3				0x1d6
> +			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4				0x1d6
> +			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5				0x1d6
> +			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6				0x1d6
> +			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7				0x1d6
> +			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE				0x196
> +		>;
> +	};
> +
> +	pinctrl_wdog: wdoggrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_GPIO1_IO02__WDOG1_WDOG_B				0xc6
> +		>;
> +	};
> +};
> +
> +&snvs_pwrkey {
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	/* console */
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart2>;
> +	status = "okay";
> +};
> +
> +&uart3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart3>;
> +	status = "okay";
> +};
> +
> +&uart4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart4>;
> +	status = "okay";
> +};
> +
> +/* SD Card */
> +&usdhc2 {
> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC2>;
> +	assigned-clock-rates = <400000000>;
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
> +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
> +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> +	vmmc-supply = <&reg_usdhc2_vmmc>;
> +	bus-width = <4>;
> +	status = "okay";
> +};
> +
> +/* eMMc */
> +&usdhc3 {
> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC3>;
> +	assigned-clock-rates = <400000000>;
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> +	bus-width = <8>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&wdog1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_wdog>;
> +	fsl,ext-reset-output;
> +	status = "okay";
> +};
> -- 
> 2.34.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: Add device tree for the Debix Model A Board
  2022-10-17 15:10 ` [PATCH v4 3/3] arm64: dts: Add device tree for the " Daniel Scally
  2022-10-29  3:50   ` Shawn Guo
@ 2022-12-01 17:10   ` Ahmad Fatoum
  2022-12-01 21:22     ` Kieran Bingham
  2022-12-06 10:25     ` Dan Scally
  1 sibling, 2 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2022-12-01 17:10 UTC (permalink / raw)
  To: Daniel Scally, krzysztof.kozlowski, shawnguo, robh,
	marcel.ziswiler, leoyang.li, devicetree, linux-arm-kernel
  Cc: s.hauer, kieran.bingham, debix-tech, linux-imx, kernel, festevam,
	laurent.pinchart

Hello Daniel,

On 17.10.22 17:10, Daniel Scally wrote:
> Add a device tree file describing the Debix Model A board from
> Polyhex Technology Co.

Thanks for your patch. Some minor comments below.

> Changes in v3 (Laurent):
> 
>     - Added IOB copyright notice
>     - Removed the eth node for the connector that's on the separate I/O
>     board

I'd have left the FEC node in and described the PHY, but left the FEC disabled.
Only the magnetics are on the expansion board, while the PHY is on the
base board.

> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2019 NXP
> + * Copyright 2022 Ideas on Board Oy
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/usb/pd.h>
> +
> +#include "imx8mp.dtsi"
> +
> +/ {
> +	model = "Polyhex Debix Model A i.MX8MPlus board";
> +	compatible = "polyhex,imx8mp-debix-model-a", "fsl,imx8mp";

I see that Model A and Model B share the same SoC and PCB. Could you
add polyhex,imx8mp-debix as a second compatible? That way, bootloader
may match against that compatible when they support both.
You'll need to adjust the binding accordingly.

> +
> +	chosen {
> +		stdout-path = &uart2;
> +	};
> +
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpio_led>;
> +
> +		status-led {
> +			function = LED_FUNCTION_POWER;
> +			color = <LED_COLOR_ID_RED>;
> +			gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
> +			default-state = "on";
> +		};
> +	};
> +
> +	reg_usdhc2_vmmc: regulator-usdhc2 {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
> +		regulator-name = "VSD_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +};
> +
> +&A53_0 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_1 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_2 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_3 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&eqos {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_eqos>;
> +	phy-connection-type = "rgmii-id";
> +	phy-handle = <&ethphy0>;
> +	status = "okay";
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethphy0: ethernet-phy@0 {

Could you append a /* RTL8211E */ comment here? This can be very useful for others
who need to bring up the same chip in the future.

> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <0>;

Is the PHY really at address 0 or does it just answer at this address
because it's the broadcast address?


> +&iomuxc {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_hog>;
> +
> +	pinctrl_hog: hoggrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL			0x400001c3
> +			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA			0x400001c3
> +			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD				0x40000019
> +			MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC				0x40000019

Why do you hog these?

> +	pinctrl_usb1_vbus: usb1grp {

This is unused.

> +	pinctrl_usdhc2: usdhc2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x190
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d0
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1

Just to make sure this doesn't fry SD-Cards by mistake: VSELECT is indeed
connected to a 1.8V/3.3V switch powering vqmmc?

> +/* SD Card */
> +&usdhc2 {
> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC2>;
> +	assigned-clock-rates = <400000000>;

I wonder why this is necessary. Do you see a difference
in /sys/kernel/debug/mmcX/ios between having this and leaving
it out?

> +	status = "okay";
> +};
> +
> +/* eMMc */

eMMC

> +&usdhc3 {
> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC3>;
> +	assigned-clock-rates = <400000000>;
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> +	bus-width = <8>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&wdog1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_wdog>;
> +	fsl,ext-reset-output;
> +	status = "okay";
> +};


Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: Add device tree for the Debix Model A Board
  2022-12-01 17:10   ` Ahmad Fatoum
@ 2022-12-01 21:22     ` Kieran Bingham
  2022-12-06 10:25     ` Dan Scally
  1 sibling, 0 replies; 11+ messages in thread
From: Kieran Bingham @ 2022-12-01 21:22 UTC (permalink / raw)
  To: Ahmad Fatoum, Daniel Scally, devicetree, krzysztof.kozlowski,
	leoyang.li, linux-arm-kernel, marcel.ziswiler, robh, shawnguo
  Cc: s.hauer, debix-tech, linux-imx, kernel, festevam, laurent.pinchart

Quoting Ahmad Fatoum (2022-12-01 17:10:20)
> Hello Daniel,
> 
> On 17.10.22 17:10, Daniel Scally wrote:
> > Add a device tree file describing the Debix Model A board from
> > Polyhex Technology Co.
> 
> Thanks for your patch. Some minor comments below.
> 
> > Changes in v3 (Laurent):
> > 
> >     - Added IOB copyright notice
> >     - Removed the eth node for the connector that's on the separate I/O
> >     board
> 
> I'd have left the FEC node in and described the PHY, but left the FEC disabled.
> Only the magnetics are on the expansion board, while the PHY is on the
> base board.
> 
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright 2019 NXP
> > + * Copyright 2022 Ideas on Board Oy
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/usb/pd.h>
> > +
> > +#include "imx8mp.dtsi"
> > +
> > +/ {
> > +     model = "Polyhex Debix Model A i.MX8MPlus board";
> > +     compatible = "polyhex,imx8mp-debix-model-a", "fsl,imx8mp";
> 
> I see that Model A and Model B share the same SoC and PCB. Could you
> add polyhex,imx8mp-debix as a second compatible? That way, bootloader
> may match against that compatible when they support both.
> You'll need to adjust the binding accordingly.

Polyhex also make a SOM with the brand DEBIX.
- https://debix.io/hardware/debix-som-a-io-board.html

But perhaps it will be fine, as that will be
"polyhex,imx8mp-debix-som" (and perhaps they'll do an A/B variant too?)

--
Kieran


> 
> > +
> > +     chosen {
> > +             stdout-path = &uart2;
> > +     };
> > +
> > +     gpio-leds {
> > +             compatible = "gpio-leds";
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&pinctrl_gpio_led>;
> > +
> > +             status-led {
> > +                     function = LED_FUNCTION_POWER;
> > +                     color = <LED_COLOR_ID_RED>;
> > +                     gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
> > +                     default-state = "on";
> > +             };
> > +     };
> > +
> > +     reg_usdhc2_vmmc: regulator-usdhc2 {
> > +             compatible = "regulator-fixed";
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
> > +             regulator-name = "VSD_3V3";
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +             gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> > +             enable-active-high;
> > +     };
> > +};
> > +
> > +&A53_0 {
> > +     cpu-supply = <&buck2>;
> > +};
> > +
> > +&A53_1 {
> > +     cpu-supply = <&buck2>;
> > +};
> > +
> > +&A53_2 {
> > +     cpu-supply = <&buck2>;
> > +};
> > +
> > +&A53_3 {
> > +     cpu-supply = <&buck2>;
> > +};
> > +
> > +&eqos {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&pinctrl_eqos>;
> > +     phy-connection-type = "rgmii-id";
> > +     phy-handle = <&ethphy0>;
> > +     status = "okay";
> > +
> > +     mdio {
> > +             compatible = "snps,dwmac-mdio";
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
> > +
> > +             ethphy0: ethernet-phy@0 {
> 
> Could you append a /* RTL8211E */ comment here? This can be very useful for others
> who need to bring up the same chip in the future.
> 
> > +                     compatible = "ethernet-phy-ieee802.3-c22";
> > +                     reg = <0>;
> 
> Is the PHY really at address 0 or does it just answer at this address
> because it's the broadcast address?
> 
> 
> > +&iomuxc {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&pinctrl_hog>;
> > +
> > +     pinctrl_hog: hoggrp {
> > +             fsl,pins = <
> > +                     MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL                     0x400001c3
> > +                     MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA                     0x400001c3
> > +                     MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD                         0x40000019
> > +                     MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC                         0x40000019
> 
> Why do you hog these?
> 
> > +     pinctrl_usb1_vbus: usb1grp {
> 
> This is unused.
> 
> > +     pinctrl_usdhc2: usdhc2grp {
> > +             fsl,pins = <
> > +                     MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK                                0x190
> > +                     MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD                                0x1d0
> > +                     MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0                            0x1d0
> > +                     MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1                            0x1d0
> > +                     MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2                            0x1d0
> > +                     MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3                            0x1d0
> > +                     MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT                         0xc1
> 
> Just to make sure this doesn't fry SD-Cards by mistake: VSELECT is indeed
> connected to a 1.8V/3.3V switch powering vqmmc?
> 
> > +/* SD Card */
> > +&usdhc2 {
> > +     assigned-clocks = <&clk IMX8MP_CLK_USDHC2>;
> > +     assigned-clock-rates = <400000000>;
> 
> I wonder why this is necessary. Do you see a difference
> in /sys/kernel/debug/mmcX/ios between having this and leaving
> it out?
> 
> > +     status = "okay";
> > +};
> > +
> > +/* eMMc */
> 
> eMMC
> 
> > +&usdhc3 {
> > +     assigned-clocks = <&clk IMX8MP_CLK_USDHC3>;
> > +     assigned-clock-rates = <400000000>;
> > +     pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > +     pinctrl-0 = <&pinctrl_usdhc3>;
> > +     pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> > +     pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> > +     bus-width = <8>;
> > +     non-removable;
> > +     status = "okay";
> > +};
> > +
> > +&wdog1 {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&pinctrl_wdog>;
> > +     fsl,ext-reset-output;
> > +     status = "okay";
> > +};
> 
> 
> Cheers,
> Ahmad
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: Add device tree for the Debix Model A Board
  2022-10-29  3:50   ` Shawn Guo
@ 2022-12-06  8:05     ` Dan Scally
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Scally @ 2022-12-06  8:05 UTC (permalink / raw)
  To: Shawn Guo
  Cc: krzysztof.kozlowski, robh, marcel.ziswiler, leoyang.li,
	devicetree, linux-arm-kernel, s.hauer, kernel, festevam,
	linux-imx, laurent.pinchart, kieran.bingham, debix-tech

Hi Shawn - sorry for the delay on this one

On 29/10/2022 04:50, Shawn Guo wrote:
> On Mon, Oct 17, 2022 at 04:10:50PM +0100, Daniel Scally wrote:
>> Add a device tree file describing the Debix Model A board from
>> Polyhex Technology Co.
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>> ---
>> Changes in v4 (Laurent):
>>
>>          - Aligned the pinctrl group values with each other
>>
>> Changes in v3 (Laurent):
>>
>>      - Added IOB copyright notice
>>      - Removed the eth node for the connector that's on the separate I/O
>>      board
>>      - Re-ordered some pinctrl groups so they're alphabetical
>>
>> Changes in v2:
>>
>>      - Fixed the interrupt flag for i2c1/pmic@25
>>      - Fixed the node name for i2c4/rtc@51 (was "hym8563@51")
>>      - Fixed a group control name that didn't match the bindings pattern
>>      - Re-compared the rest of the DT with the EVK's .dts file to try to
>>      make sure it complies with the way things should be, hopefully without
>>      missing anything...
>>
>>   arch/arm64/boot/dts/freescale/Makefile        |   1 +
>>   .../dts/freescale/imx8mp-debix-model-a.dts    | 529 ++++++++++++++++++
>>   2 files changed, 530 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts
>>
>> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
>> index 8bf7f7ecebaa..6a33a08946ac 100644
>> --- a/arch/arm64/boot/dts/freescale/Makefile
>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>> @@ -80,6 +80,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx8mn-venice-gw7902.dtb
>> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-debix-model-a.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk2.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts
>> new file mode 100644
>> index 000000000000..92cbd6d2b73f
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts
>> @@ -0,0 +1,529 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright 2019 NXP
>> + * Copyright 2022 Ideas on Board Oy
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/leds/common.h>
>> +#include <dt-bindings/usb/pd.h>
>> +
>> +#include "imx8mp.dtsi"
>> +
>> +/ {
>> +	model = "Polyhex Debix Model A i.MX8MPlus board";
>> +	compatible = "polyhex,imx8mp-debix-model-a", "fsl,imx8mp";
>> +
>> +	chosen {
>> +		stdout-path = &uart2;
>> +	};
>> +
>> +	gpio-leds {
>> +		compatible = "gpio-leds";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_gpio_led>;
>> +
>> +		status-led {
> Did you run 'make dtbs_check'? The node name doesn't seem to match
> pattern defined by leds-gpio.yaml.

I did yes, it doesn't complain about this because the pattern matching 
for leds-gpio has an "|led" in it, which is pretty broad. I'll switch to 
the "led-0" form though.


>
>> +			function = LED_FUNCTION_POWER;
>> +			color = <LED_COLOR_ID_RED>;
>> +			gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
>> +			default-state = "on";
>> +		};
>> +	};
>> +
>> +	reg_usdhc2_vmmc: regulator-usdhc2 {
>> +		compatible = "regulator-fixed";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
>> +		regulator-name = "VSD_3V3";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
>> +		enable-active-high;
>> +	};
>> +};
>> +
>> +&A53_0 {
>> +	cpu-supply = <&buck2>;
>> +};
>> +
>> +&A53_1 {
>> +	cpu-supply = <&buck2>;
>> +};
>> +
>> +&A53_2 {
>> +	cpu-supply = <&buck2>;
>> +};
>> +
>> +&A53_3 {
>> +	cpu-supply = <&buck2>;
>> +};
>> +
>> +&eqos {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_eqos>;
>> +	phy-connection-type = "rgmii-id";
>> +	phy-handle = <&ethphy0>;
>> +	status = "okay";
>> +
>> +	mdio {
>> +		compatible = "snps,dwmac-mdio";
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		ethphy0: ethernet-phy@0 {
>> +			compatible = "ethernet-phy-ieee802.3-c22";
>> +			reg = <0>;
>> +			reset-gpios = <&gpio4 18 GPIO_ACTIVE_LOW>;
>> +			reset-assert-us = <20>;
>> +			reset-deassert-us = <200000>;
>> +		};
>> +	};
>> +};
>> +
>> +&i2c1 {
>> +	clock-frequency = <400000>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_i2c1>;
>> +	status = "okay";
>> +
>> +	pmic@25 {
>> +		reg = <0x25>;
>> +		compatible = "nxp,pca9450c";
> We usually start properties with `compatible`.


Ack

>
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_pmic>;
>> +		interrupt-parent = <&gpio1>;
>> +		interrupts = <3 IRQ_TYPE_EDGE_RISING>;
>> +
>> +		regulators {
>> +			buck1: BUCK1 {
>> +				regulator-name = "BUCK1";
>> +				regulator-min-microvolt = <600000>;
>> +				regulator-max-microvolt = <2187500>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +				regulator-ramp-delay = <3125>;
>> +			};
>> +
>> +			buck2: BUCK2 {
>> +				regulator-name = "BUCK2";
>> +				regulator-min-microvolt = <600000>;
>> +				regulator-max-microvolt = <2187500>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +				regulator-ramp-delay = <3125>;
>> +				nxp,dvs-run-voltage = <950000>;
>> +				nxp,dvs-standby-voltage = <850000>;
>> +			};
>> +
>> +			buck4: BUCK4{
>> +				regulator-name = "BUCK4";
>> +				regulator-min-microvolt = <600000>;
>> +				regulator-max-microvolt = <3400000>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +			};
>> +
>> +			buck5: BUCK5{
>> +				regulator-name = "BUCK5";
>> +				regulator-min-microvolt = <600000>;
>> +				regulator-max-microvolt = <3400000>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +			};
>> +
>> +			buck6: BUCK6 {
>> +				regulator-name = "BUCK6";
>> +				regulator-min-microvolt = <600000>;
>> +				regulator-max-microvolt = <3400000>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo1: LDO1 {
>> +				regulator-name = "LDO1";
>> +				regulator-min-microvolt = <1600000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo2: LDO2 {
>> +				regulator-name = "LDO2";
>> +				regulator-min-microvolt = <800000>;
>> +				regulator-max-microvolt = <1150000>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo3: LDO3 {
>> +				regulator-name = "LDO3";
>> +				regulator-min-microvolt = <800000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo4: LDO4 {
>> +				regulator-name = "LDO4";
>> +				regulator-min-microvolt = <800000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo5: LDO5 {
>> +				regulator-name = "LDO5";
>> +				regulator-min-microvolt = <1800000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +&i2c2 {
>> +	clock-frequency = <100000>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_i2c2>;
>> +	status = "okay";
>> +};
>> +
>> +&i2c3 {
>> +	clock-frequency = <400000>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_i2c3>;
>> +	status = "okay";
>> +};
>> +
>> +&i2c4 {
>> +	clock-frequency = <100000>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_i2c4>;
>> +	status = "okay";
>> +
>> +	eeprom@50 {
>> +		compatible = "atmel,24c02";
>> +		reg = <0x50>;
>> +		pagesize = <16>;
>> +	};
>> +
>> +	rtc@51 {
>> +		compatible = "haoyu,hym8563";
>> +		reg = <0x51>;
>> +		#clock-cells = <0>;
>> +		clock-frequency = <32768>;
>> +		clock-output-names = "xin32k";
>> +		interrupt-parent = <&gpio2>;
>> +		interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_rtc_int>;
>> +	};
>> +};
>> +
>> +&i2c6 {
>> +	clock-frequency = <400000>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_i2c6>;
>> +	status = "okay";
>> +};
>> +
>> +&iomuxc {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_hog>;
>> +
>> +	pinctrl_hog: hoggrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL			0x400001c3
>> +			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA			0x400001c3
>> +			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD				0x40000019
>> +			MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC				0x40000019
>> +		>;
>> +	};
>> +
>> +	pinctrl_eqos: eqosgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_ENET_MDC__ENET_QOS_MDC				0x3
>> +			MX8MP_IOMUXC_ENET_MDIO__ENET_QOS_MDIO				0x3
>> +			MX8MP_IOMUXC_ENET_RD0__ENET_QOS_RGMII_RD0			0x91
>> +			MX8MP_IOMUXC_ENET_RD1__ENET_QOS_RGMII_RD1			0x91
>> +			MX8MP_IOMUXC_ENET_RD2__ENET_QOS_RGMII_RD2			0x91
>> +			MX8MP_IOMUXC_ENET_RD3__ENET_QOS_RGMII_RD3			0x91
>> +			MX8MP_IOMUXC_ENET_RXC__CCM_ENET_QOS_CLOCK_GENERATE_RX_CLK	0x91
>> +			MX8MP_IOMUXC_ENET_RX_CTL__ENET_QOS_RGMII_RX_CTL			0x91
>> +			MX8MP_IOMUXC_ENET_TD0__ENET_QOS_RGMII_TD0			0x1f
>> +			MX8MP_IOMUXC_ENET_TD1__ENET_QOS_RGMII_TD1			0x1f
>> +			MX8MP_IOMUXC_ENET_TD2__ENET_QOS_RGMII_TD2			0x1f
>> +			MX8MP_IOMUXC_ENET_TD3__ENET_QOS_RGMII_TD3			0x1f
>> +			MX8MP_IOMUXC_ENET_TX_CTL__ENET_QOS_RGMII_TX_CTL			0x1f
>> +			MX8MP_IOMUXC_ENET_TXC__CCM_ENET_QOS_CLOCK_GENERATE_TX_CLK	0x1f
>> +			MX8MP_IOMUXC_SAI1_RXFS__ENET1_1588_EVENT0_IN			0x1f
>> +			MX8MP_IOMUXC_SAI1_RXC__ENET1_1588_EVENT0_OUT			0x1f
>> +			MX8MP_IOMUXC_SAI1_TXD6__GPIO4_IO18				0x19
>> +		>;
>> +	};
>> +
>> +	pinctrl_fec: fecgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SAI1_RXD2__ENET1_MDC				0x3
>> +			MX8MP_IOMUXC_SAI1_RXD3__ENET1_MDIO				0x3
>> +			MX8MP_IOMUXC_SAI1_RXD4__ENET1_RGMII_RD0				0x91
>> +			MX8MP_IOMUXC_SAI1_RXD5__ENET1_RGMII_RD1				0x91
>> +			MX8MP_IOMUXC_SAI1_RXD6__ENET1_RGMII_RD2				0x91
>> +			MX8MP_IOMUXC_SAI1_RXD7__ENET1_RGMII_RD3				0x91
>> +			MX8MP_IOMUXC_SAI1_TXC__ENET1_RGMII_RXC				0x91
>> +			MX8MP_IOMUXC_SAI1_TXFS__ENET1_RGMII_RX_CTL			0x91
>> +			MX8MP_IOMUXC_SAI1_TXD0__ENET1_RGMII_TD0				0x1f
>> +			MX8MP_IOMUXC_SAI1_TXD1__ENET1_RGMII_TD1				0x1f
>> +			MX8MP_IOMUXC_SAI1_TXD2__ENET1_RGMII_TD2				0x1f
>> +			MX8MP_IOMUXC_SAI1_TXD3__ENET1_RGMII_TD3				0x1f
>> +			MX8MP_IOMUXC_SAI1_TXD4__ENET1_RGMII_TX_CTL			0x1f
>> +			MX8MP_IOMUXC_SAI1_TXD5__ENET1_RGMII_TXC				0x1f
>> +			MX8MP_IOMUXC_SAI1_RXD1__ENET1_1588_EVENT1_OUT			0x1f
>> +			MX8MP_IOMUXC_SAI1_RXD0__ENET1_1588_EVENT1_IN			0x1f
>> +			MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19				0x19
>> +		>;
>> +	};
>> +
>> +	pinctrl_gpio_led: gpioledgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_NAND_READY_B__GPIO3_IO16				0x19
>> +		>;
>> +	};
>> +
>> +	pinctrl_i2c1: i2c1grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL					0x400001c2
>> +			MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA					0x400001c2
>> +		>;
>> +	};
>> +
>> +	pinctrl_i2c2: i2c2grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL					0x400001c2
>> +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA					0x400001c2
>> +		>;
>> +	};
>> +
>> +	pinctrl_i2c3: i2c3grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL					0x400001c2
>> +			MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA					0x400001c2
>> +		>;
>> +	};
> Missing newline.


Thanks!

>
> Shawn
>
>> +	pinctrl_i2c4: i2c4grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_I2C4_SCL__I2C4_SCL					0x400001c3
>> +			MX8MP_IOMUXC_I2C4_SDA__I2C4_SDA					0x400001c3
>> +		>;
>> +	};
>> +
>> +	pinctrl_i2c6: i2c6grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SAI5_RXFS__I2C6_SCL				0x400001c3
>> +			MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA					0x400001c3
>> +		>;
>> +	};
>> +
>> +	pinctrl_pmic: pmicirqgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03				0x41
>> +		>;
>> +	};
>> +
>> +	pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19				0x41
>> +		>;
>> +	};
>> +
>> +	pinctrl_rtc_int: rtcintgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SD1_STROBE__GPIO2_IO11				0x140
>> +		>;
>> +	};
>> +
>> +	pinctrl_uart2: uart2grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX				0x14f
>> +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX				0x14f
>> +		>;
>> +	};
>> +
>> +	pinctrl_uart3: uart3grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_UART3_RXD__UART3_DCE_RX				0x49
>> +			MX8MP_IOMUXC_UART3_TXD__UART3_DCE_TX				0x49
>> +		>;
>> +	};
>> +
>> +	pinctrl_uart4: uart4grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_UART4_RXD__UART4_DCE_RX				0x49
>> +			MX8MP_IOMUXC_UART4_TXD__UART4_DCE_TX				0x49
>> +		>;
>> +	};
>> +
>> +	pinctrl_usb1_vbus: usb1grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SAI2_TXC__GPIO4_IO25				0x19
>> +			MX8MP_IOMUXC_SAI2_TXD0__GPIO4_IO26				0x19
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc2: usdhc2grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x190
>> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d0
>> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d0
>> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d0
>> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d0
>> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d0
>> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x194
>> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d4
>> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d4
>> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d4
>> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d4
>> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d4
>> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x196
>> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d6
>> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d6
>> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d6
>> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d6
>> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d6
>> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc2_gpio: usdhc2gpiogrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12				0x1c4
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc3: usdhc3grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK				0x190
>> +			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD				0x1d0
>> +			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0				0x1d0
>> +			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1				0x1d0
>> +			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2				0x1d0
>> +			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3				0x1d0
>> +			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4				0x1d0
>> +			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5				0x1d0
>> +			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6				0x1d0
>> +			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7				0x1d0
>> +			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE				0x190
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc3_100mhz: usdhc3-100mhzgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK				0x194
>> +			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD				0x1d4
>> +			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0				0x1d4
>> +			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1				0x1d4
>> +			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2				0x1d4
>> +			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3				0x1d4
>> +			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4				0x1d4
>> +			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5				0x1d4
>> +			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6				0x1d4
>> +			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7				0x1d4
>> +			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE				0x194
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc3_200mhz: usdhc3-200mhzgrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK				0x196
>> +			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD				0x1d6
>> +			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0				0x1d6
>> +			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1				0x1d6
>> +			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2				0x1d6
>> +			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3				0x1d6
>> +			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4				0x1d6
>> +			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5				0x1d6
>> +			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6				0x1d6
>> +			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7				0x1d6
>> +			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE				0x196
>> +		>;
>> +	};
>> +
>> +	pinctrl_wdog: wdoggrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_GPIO1_IO02__WDOG1_WDOG_B				0xc6
>> +		>;
>> +	};
>> +};
>> +
>> +&snvs_pwrkey {
>> +	status = "okay";
>> +};
>> +
>> +&uart2 {
>> +	/* console */
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_uart2>;
>> +	status = "okay";
>> +};
>> +
>> +&uart3 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_uart3>;
>> +	status = "okay";
>> +};
>> +
>> +&uart4 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_uart4>;
>> +	status = "okay";
>> +};
>> +
>> +/* SD Card */
>> +&usdhc2 {
>> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC2>;
>> +	assigned-clock-rates = <400000000>;
>> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
>> +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
>> +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
>> +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
>> +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
>> +	vmmc-supply = <&reg_usdhc2_vmmc>;
>> +	bus-width = <4>;
>> +	status = "okay";
>> +};
>> +
>> +/* eMMc */
>> +&usdhc3 {
>> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC3>;
>> +	assigned-clock-rates = <400000000>;
>> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
>> +	pinctrl-0 = <&pinctrl_usdhc3>;
>> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
>> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
>> +	bus-width = <8>;
>> +	non-removable;
>> +	status = "okay";
>> +};
>> +
>> +&wdog1 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_wdog>;
>> +	fsl,ext-reset-output;
>> +	status = "okay";
>> +};
>> -- 
>> 2.34.1
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: Add device tree for the Debix Model A Board
  2022-12-01 17:10   ` Ahmad Fatoum
  2022-12-01 21:22     ` Kieran Bingham
@ 2022-12-06 10:25     ` Dan Scally
  2022-12-06 10:45       ` Laurent Pinchart
  1 sibling, 1 reply; 11+ messages in thread
From: Dan Scally @ 2022-12-06 10:25 UTC (permalink / raw)
  To: Ahmad Fatoum, krzysztof.kozlowski, shawnguo, robh,
	marcel.ziswiler, leoyang.li, devicetree, linux-arm-kernel
  Cc: s.hauer, kieran.bingham, debix-tech, linux-imx, kernel, festevam,
	laurent.pinchart

Hi Ahmad - thanks for the review

On 01/12/2022 17:10, Ahmad Fatoum wrote:
> Hello Daniel,
>
> On 17.10.22 17:10, Daniel Scally wrote:
>> Add a device tree file describing the Debix Model A board from
>> Polyhex Technology Co.
> Thanks for your patch. Some minor comments below.
>
>> Changes in v3 (Laurent):
>>
>>      - Added IOB copyright notice
>>      - Removed the eth node for the connector that's on the separate I/O
>>      board
> I'd have left the FEC node in and described the PHY, but left the FEC disabled.
> Only the magnetics are on the expansion board, while the PHY is on the
> base board.


Fair enough, though there's quite a lot else on the base board which 
we've left off simply because we're not currently using it. I'm inclined 
to treat this the same for now.


>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright 2019 NXP
>> + * Copyright 2022 Ideas on Board Oy
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/leds/common.h>
>> +#include <dt-bindings/usb/pd.h>
>> +
>> +#include "imx8mp.dtsi"
>> +
>> +/ {
>> +	model = "Polyhex Debix Model A i.MX8MPlus board";
>> +	compatible = "polyhex,imx8mp-debix-model-a", "fsl,imx8mp";
> I see that Model A and Model B share the same SoC and PCB. Could you
> add polyhex,imx8mp-debix as a second compatible? That way, bootloader
> may match against that compatible when they support both.
> You'll need to adjust the binding accordingly.


Sure, makes sense to me

>
>> +
>> +	chosen {
>> +		stdout-path = &uart2;
>> +	};
>> +
>> +	gpio-leds {
>> +		compatible = "gpio-leds";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_gpio_led>;
>> +
>> +		status-led {
>> +			function = LED_FUNCTION_POWER;
>> +			color = <LED_COLOR_ID_RED>;
>> +			gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
>> +			default-state = "on";
>> +		};
>> +	};
>> +
>> +	reg_usdhc2_vmmc: regulator-usdhc2 {
>> +		compatible = "regulator-fixed";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
>> +		regulator-name = "VSD_3V3";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
>> +		enable-active-high;
>> +	};
>> +};
>> +
>> +&A53_0 {
>> +	cpu-supply = <&buck2>;
>> +};
>> +
>> +&A53_1 {
>> +	cpu-supply = <&buck2>;
>> +};
>> +
>> +&A53_2 {
>> +	cpu-supply = <&buck2>;
>> +};
>> +
>> +&A53_3 {
>> +	cpu-supply = <&buck2>;
>> +};
>> +
>> +&eqos {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_eqos>;
>> +	phy-connection-type = "rgmii-id";
>> +	phy-handle = <&ethphy0>;
>> +	status = "okay";
>> +
>> +	mdio {
>> +		compatible = "snps,dwmac-mdio";
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		ethphy0: ethernet-phy@0 {
> Could you append a /* RTL8211E */ comment here? This can be very useful for others
> who need to bring up the same chip in the future.


Sure

>
>> +			compatible = "ethernet-phy-ieee802.3-c22";
>> +			reg = <0>;
> Is the PHY really at address 0 or does it just answer at this address
> because it's the broadcast address?


I confess I'm unsure here, the number here comes from the downstream 
.dts, which in this instance I've trusted...is there any way I can check?

>
>
>> +&iomuxc {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_hog>;
>> +
>> +	pinctrl_hog: hoggrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL			0x400001c3
>> +			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA			0x400001c3
>> +			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD				0x40000019
>> +			MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC				0x40000019
> Why do you hog these?
>
>> +	pinctrl_usb1_vbus: usb1grp {
> This is unused.


Ah both for other elements of the board which aren't included in this 
set, as they don't work properly yet. Apologies; I'll remove those.

>
>> +	pinctrl_usdhc2: usdhc2grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x190
>> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d0
>> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d0
>> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d0
>> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d0
>> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d0
>> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
> Just to make sure this doesn't fry SD-Cards by mistake: VSELECT is indeed
> connected to a 1.8V/3.3V switch powering vqmmc?
>
>> +/* SD Card */
>> +&usdhc2 {
>> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC2>;
>> +	assigned-clock-rates = <400000000>;
> I wonder why this is necessary. Do you see a difference
> in /sys/kernel/debug/mmcX/ios between having this and leaving
> it out?


I don't actually...it's present in the imx8mp-evk.dts which this is 
based on, but in addition to not seeing any difference there the SD card 
still seems fine as far as I can tell (same read / write speed in 
practice) - I'll take it out, thanks

>
>> +	status = "okay";
>> +};
>> +
>> +/* eMMc */
> eMMC

Ack
>> +&usdhc3 {
>> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC3>;
>> +	assigned-clock-rates = <400000000>;
>> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
>> +	pinctrl-0 = <&pinctrl_usdhc3>;
>> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
>> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
>> +	bus-width = <8>;
>> +	non-removable;
>> +	status = "okay";
>> +};
>> +
>> +&wdog1 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_wdog>;
>> +	fsl,ext-reset-output;
>> +	status = "okay";
>> +};
>
> Cheers,
> Ahmad
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: Add device tree for the Debix Model A Board
  2022-12-06 10:25     ` Dan Scally
@ 2022-12-06 10:45       ` Laurent Pinchart
  2022-12-06 10:52         ` Ahmad Fatoum
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2022-12-06 10:45 UTC (permalink / raw)
  To: Dan Scally
  Cc: Ahmad Fatoum, krzysztof.kozlowski, shawnguo, robh,
	marcel.ziswiler, leoyang.li, devicetree, linux-arm-kernel,
	s.hauer, kieran.bingham, debix-tech, linux-imx, kernel, festevam

On Tue, Dec 06, 2022 at 10:25:08AM +0000, Dan Scally wrote:
> Hi Ahmad - thanks for the review
> 
> On 01/12/2022 17:10, Ahmad Fatoum wrote:
> > Hello Daniel,
> >
> > On 17.10.22 17:10, Daniel Scally wrote:
> >> Add a device tree file describing the Debix Model A board from
> >> Polyhex Technology Co.
> >
> > Thanks for your patch. Some minor comments below.
> >
> >> Changes in v3 (Laurent):
> >>
> >>      - Added IOB copyright notice
> >>      - Removed the eth node for the connector that's on the separate I/O
> >>      board
> > I'd have left the FEC node in and described the PHY, but left the FEC disabled.
> > Only the magnetics are on the expansion board, while the PHY is on the
> > base board.
> 
> Fair enough, though there's quite a lot else on the base board which 
> we've left off simply because we're not currently using it. I'm inclined 
> to treat this the same for now.

Overall I think it's a good strategy, if we haven't tested a peripheral
we shouldn't include it yet. Missing pieces can be added later. Of
course if there are pieces that are easy to test already, we can include
them.

> >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> +/*
> >> + * Copyright 2019 NXP
> >> + * Copyright 2022 Ideas on Board Oy
> >> + */
> >> +
> >> +/dts-v1/;
> >> +
> >> +#include <dt-bindings/gpio/gpio.h>
> >> +#include <dt-bindings/leds/common.h>
> >> +#include <dt-bindings/usb/pd.h>
> >> +
> >> +#include "imx8mp.dtsi"
> >> +
> >> +/ {
> >> +	model = "Polyhex Debix Model A i.MX8MPlus board";
> >> +	compatible = "polyhex,imx8mp-debix-model-a", "fsl,imx8mp";
> >
> > I see that Model A and Model B share the same SoC and PCB. Could you
> > add polyhex,imx8mp-debix as a second compatible? That way, bootloader
> > may match against that compatible when they support both.
> > You'll need to adjust the binding accordingly.
> 
> Sure, makes sense to me
> 
> >> +
> >> +	chosen {
> >> +		stdout-path = &uart2;
> >> +	};
> >> +
> >> +	gpio-leds {
> >> +		compatible = "gpio-leds";
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&pinctrl_gpio_led>;
> >> +
> >> +		status-led {
> >> +			function = LED_FUNCTION_POWER;
> >> +			color = <LED_COLOR_ID_RED>;
> >> +			gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
> >> +			default-state = "on";
> >> +		};
> >> +	};
> >> +
> >> +	reg_usdhc2_vmmc: regulator-usdhc2 {
> >> +		compatible = "regulator-fixed";
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
> >> +		regulator-name = "VSD_3V3";
> >> +		regulator-min-microvolt = <3300000>;
> >> +		regulator-max-microvolt = <3300000>;
> >> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> >> +		enable-active-high;
> >> +	};
> >> +};
> >> +
> >> +&A53_0 {
> >> +	cpu-supply = <&buck2>;
> >> +};
> >> +
> >> +&A53_1 {
> >> +	cpu-supply = <&buck2>;
> >> +};
> >> +
> >> +&A53_2 {
> >> +	cpu-supply = <&buck2>;
> >> +};
> >> +
> >> +&A53_3 {
> >> +	cpu-supply = <&buck2>;
> >> +};
> >> +
> >> +&eqos {
> >> +	pinctrl-names = "default";
> >> +	pinctrl-0 = <&pinctrl_eqos>;
> >> +	phy-connection-type = "rgmii-id";
> >> +	phy-handle = <&ethphy0>;
> >> +	status = "okay";
> >> +
> >> +	mdio {
> >> +		compatible = "snps,dwmac-mdio";
> >> +		#address-cells = <1>;
> >> +		#size-cells = <0>;
> >> +
> >> +		ethphy0: ethernet-phy@0 {
> >
> > Could you append a /* RTL8211E */ comment here? This can be very useful for others
> > who need to bring up the same chip in the future.
> 
> Sure
> 
> >> +			compatible = "ethernet-phy-ieee802.3-c22";
> >> +			reg = <0>;
> >
> > Is the PHY really at address 0 or does it just answer at this address
> > because it's the broadcast address?
> 
> I confess I'm unsure here, the number here comes from the downstream 
> .dts, which in this instance I've trusted...is there any way I can check?
> 
> >> +&iomuxc {
> >> +	pinctrl-names = "default";
> >> +	pinctrl-0 = <&pinctrl_hog>;
> >> +
> >> +	pinctrl_hog: hoggrp {
> >> +		fsl,pins = <
> >> +			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL			0x400001c3
> >> +			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA			0x400001c3
> >> +			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD				0x40000019
> >> +			MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC				0x40000019
> >
> > Why do you hog these?
> >
> >> +	pinctrl_usb1_vbus: usb1grp {
> >
> > This is unused.
> 
> Ah both for other elements of the board which aren't included in this 
> set, as they don't work properly yet. Apologies; I'll remove those.
> 
> >> +	pinctrl_usdhc2: usdhc2grp {
> >> +		fsl,pins = <
> >> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x190
> >> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d0
> >> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d0
> >> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d0
> >> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d0
> >> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d0
> >> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
> >
> > Just to make sure this doesn't fry SD-Cards by mistake: VSELECT is indeed
> > connected to a 1.8V/3.3V switch powering vqmmc?
> >
> >> +/* SD Card */
> >> +&usdhc2 {
> >> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC2>;
> >> +	assigned-clock-rates = <400000000>;
> >
> > I wonder why this is necessary. Do you see a difference
> > in /sys/kernel/debug/mmcX/ios between having this and leaving
> > it out?
> 
> I don't actually...it's present in the imx8mp-evk.dts which this is 
> based on, but in addition to not seeing any difference there the SD card 
> still seems fine as far as I can tell (same read / write speed in 
> practice) - I'll take it out, thanks
> 
> >> +	status = "okay";
> >> +};
> >> +
> >> +/* eMMc */
> >
> > eMMC
> 
> Ack
>
> >> +&usdhc3 {
> >> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC3>;
> >> +	assigned-clock-rates = <400000000>;
> >> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> >> +	pinctrl-0 = <&pinctrl_usdhc3>;
> >> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> >> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> >> +	bus-width = <8>;
> >> +	non-removable;
> >> +	status = "okay";
> >> +};
> >> +
> >> +&wdog1 {
> >> +	pinctrl-names = "default";
> >> +	pinctrl-0 = <&pinctrl_wdog>;
> >> +	fsl,ext-reset-output;
> >> +	status = "okay";
> >> +};

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: Add device tree for the Debix Model A Board
  2022-12-06 10:45       ` Laurent Pinchart
@ 2022-12-06 10:52         ` Ahmad Fatoum
  0 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2022-12-06 10:52 UTC (permalink / raw)
  To: Laurent Pinchart, Dan Scally
  Cc: krzysztof.kozlowski, shawnguo, robh, marcel.ziswiler, leoyang.li,
	devicetree, linux-arm-kernel, s.hauer, kieran.bingham,
	debix-tech, linux-imx, kernel, festevam

Hello Laurent, Dan,

On 06.12.22 11:45, Laurent Pinchart wrote:
> On Tue, Dec 06, 2022 at 10:25:08AM +0000, Dan Scally wrote:
>> Hi Ahmad - thanks for the review
>>
>> On 01/12/2022 17:10, Ahmad Fatoum wrote:
>>> Hello Daniel,
>>>
>>> On 17.10.22 17:10, Daniel Scally wrote:
>>>> Add a device tree file describing the Debix Model A board from
>>>> Polyhex Technology Co.
>>>
>>> Thanks for your patch. Some minor comments below.
>>>
>>>> Changes in v3 (Laurent):
>>>>
>>>>      - Added IOB copyright notice
>>>>      - Removed the eth node for the connector that's on the separate I/O
>>>>      board
>>> I'd have left the FEC node in and described the PHY, but left the FEC disabled.
>>> Only the magnetics are on the expansion board, while the PHY is on the
>>> base board.
>>
>> Fair enough, though there's quite a lot else on the base board which 
>> we've left off simply because we're not currently using it. I'm inclined 
>> to treat this the same for now.
> 
> Overall I think it's a good strategy, if we haven't tested a peripheral
> we shouldn't include it yet. Missing pieces can be added later. Of
> course if there are pieces that are easy to test already, we can include
> them.

I generally agree, but I tested the FEC with the magnetics on the I/O board
and the DT description from the older patch set worked correctly.

Cheers,
Ahmad

> 
>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>> +/*
>>>> + * Copyright 2019 NXP
>>>> + * Copyright 2022 Ideas on Board Oy
>>>> + */
>>>> +
>>>> +/dts-v1/;
>>>> +
>>>> +#include <dt-bindings/gpio/gpio.h>
>>>> +#include <dt-bindings/leds/common.h>
>>>> +#include <dt-bindings/usb/pd.h>
>>>> +
>>>> +#include "imx8mp.dtsi"
>>>> +
>>>> +/ {
>>>> +	model = "Polyhex Debix Model A i.MX8MPlus board";
>>>> +	compatible = "polyhex,imx8mp-debix-model-a", "fsl,imx8mp";
>>>
>>> I see that Model A and Model B share the same SoC and PCB. Could you
>>> add polyhex,imx8mp-debix as a second compatible? That way, bootloader
>>> may match against that compatible when they support both.
>>> You'll need to adjust the binding accordingly.
>>
>> Sure, makes sense to me
>>
>>>> +
>>>> +	chosen {
>>>> +		stdout-path = &uart2;
>>>> +	};
>>>> +
>>>> +	gpio-leds {
>>>> +		compatible = "gpio-leds";
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&pinctrl_gpio_led>;
>>>> +
>>>> +		status-led {
>>>> +			function = LED_FUNCTION_POWER;
>>>> +			color = <LED_COLOR_ID_RED>;
>>>> +			gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
>>>> +			default-state = "on";
>>>> +		};
>>>> +	};
>>>> +
>>>> +	reg_usdhc2_vmmc: regulator-usdhc2 {
>>>> +		compatible = "regulator-fixed";
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
>>>> +		regulator-name = "VSD_3V3";
>>>> +		regulator-min-microvolt = <3300000>;
>>>> +		regulator-max-microvolt = <3300000>;
>>>> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
>>>> +		enable-active-high;
>>>> +	};
>>>> +};
>>>> +
>>>> +&A53_0 {
>>>> +	cpu-supply = <&buck2>;
>>>> +};
>>>> +
>>>> +&A53_1 {
>>>> +	cpu-supply = <&buck2>;
>>>> +};
>>>> +
>>>> +&A53_2 {
>>>> +	cpu-supply = <&buck2>;
>>>> +};
>>>> +
>>>> +&A53_3 {
>>>> +	cpu-supply = <&buck2>;
>>>> +};
>>>> +
>>>> +&eqos {
>>>> +	pinctrl-names = "default";
>>>> +	pinctrl-0 = <&pinctrl_eqos>;
>>>> +	phy-connection-type = "rgmii-id";
>>>> +	phy-handle = <&ethphy0>;
>>>> +	status = "okay";
>>>> +
>>>> +	mdio {
>>>> +		compatible = "snps,dwmac-mdio";
>>>> +		#address-cells = <1>;
>>>> +		#size-cells = <0>;
>>>> +
>>>> +		ethphy0: ethernet-phy@0 {
>>>
>>> Could you append a /* RTL8211E */ comment here? This can be very useful for others
>>> who need to bring up the same chip in the future.
>>
>> Sure
>>
>>>> +			compatible = "ethernet-phy-ieee802.3-c22";
>>>> +			reg = <0>;
>>>
>>> Is the PHY really at address 0 or does it just answer at this address
>>> because it's the broadcast address?
>>
>> I confess I'm unsure here, the number here comes from the downstream 
>> .dts, which in this instance I've trusted...is there any way I can check?
>>
>>>> +&iomuxc {
>>>> +	pinctrl-names = "default";
>>>> +	pinctrl-0 = <&pinctrl_hog>;
>>>> +
>>>> +	pinctrl_hog: hoggrp {
>>>> +		fsl,pins = <
>>>> +			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL			0x400001c3
>>>> +			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA			0x400001c3
>>>> +			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD				0x40000019
>>>> +			MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC				0x40000019
>>>
>>> Why do you hog these?
>>>
>>>> +	pinctrl_usb1_vbus: usb1grp {
>>>
>>> This is unused.
>>
>> Ah both for other elements of the board which aren't included in this 
>> set, as they don't work properly yet. Apologies; I'll remove those.
>>
>>>> +	pinctrl_usdhc2: usdhc2grp {
>>>> +		fsl,pins = <
>>>> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x190
>>>> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d0
>>>> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d0
>>>> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d0
>>>> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d0
>>>> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d0
>>>> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
>>>
>>> Just to make sure this doesn't fry SD-Cards by mistake: VSELECT is indeed
>>> connected to a 1.8V/3.3V switch powering vqmmc?
>>>
>>>> +/* SD Card */
>>>> +&usdhc2 {
>>>> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC2>;
>>>> +	assigned-clock-rates = <400000000>;
>>>
>>> I wonder why this is necessary. Do you see a difference
>>> in /sys/kernel/debug/mmcX/ios between having this and leaving
>>> it out?
>>
>> I don't actually...it's present in the imx8mp-evk.dts which this is 
>> based on, but in addition to not seeing any difference there the SD card 
>> still seems fine as far as I can tell (same read / write speed in 
>> practice) - I'll take it out, thanks
>>
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +/* eMMc */
>>>
>>> eMMC
>>
>> Ack
>>
>>>> +&usdhc3 {
>>>> +	assigned-clocks = <&clk IMX8MP_CLK_USDHC3>;
>>>> +	assigned-clock-rates = <400000000>;
>>>> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
>>>> +	pinctrl-0 = <&pinctrl_usdhc3>;
>>>> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
>>>> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
>>>> +	bus-width = <8>;
>>>> +	non-removable;
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +&wdog1 {
>>>> +	pinctrl-names = "default";
>>>> +	pinctrl-0 = <&pinctrl_wdog>;
>>>> +	fsl,ext-reset-output;
>>>> +	status = "okay";
>>>> +};
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-12-06 10:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-17 15:10 [PATCH v4 0/3] Debix Model A board devicetree Daniel Scally
2022-10-17 15:10 ` [PATCH v4 1/3] dt-bindings: vendor-prefixes: Add Polyhex Technology Co Daniel Scally
2022-10-17 15:10 ` [PATCH v4 2/3] dt-bindings: arm: fsl: Enumerate Debix Model A Board Daniel Scally
2022-10-17 15:10 ` [PATCH v4 3/3] arm64: dts: Add device tree for the " Daniel Scally
2022-10-29  3:50   ` Shawn Guo
2022-12-06  8:05     ` Dan Scally
2022-12-01 17:10   ` Ahmad Fatoum
2022-12-01 21:22     ` Kieran Bingham
2022-12-06 10:25     ` Dan Scally
2022-12-06 10:45       ` Laurent Pinchart
2022-12-06 10:52         ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).