Re: [PATCH v4 2/2] arm64: dts: imx: Add imx8mp-iota2-lumpy board

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24. 09. 24 19:37, Marco Felsch wrote:
Hi Frank,

On 24-09-24, Frank Li wrote:
On Tue, Sep 24, 2024 at 12:39:41PM +0200, Michal Vokáč wrote:
The IOTA2 Lumpy board is based on the i.MX8MPlus EVK.

Basic features are:
- 4GB LPDDR4
- 64GB eMMC
- 2x 1GB Ethernet
- USB 3.0 Type-C dual role port, without power delivery
- USB 3.0 Type-A host port
- RGB LED - PWM driven
- speaker - PWM driven
- RTC with super capacitor backup

Signed-off-by: Michal Vokáč <michal.vokac@xxxxxxxxx>
---
v4:
- Moved the iomuxc node to the end of the file.
- Moved the bus-width and non-removeable properties below
   the pinctrl-* properties in &usdhc3 node.
- Moved the fsl,ext-reset-output below the pinctrl-* properties
   in &wdog1 node.
v3:
- Dropped pinctrl-names property from &usb_dwc3_1 node.
v2:
- Dropped unused property from pwm4 node.
- Sorted all nodes and properties using dt-format tool from Frank.

  arch/arm64/boot/dts/freescale/Makefile        |   1 +
  .../boot/dts/freescale/imx8mp-iota2-lumpy.dts | 423 ++++++++++++++++++

Suggest use https://github.com/lznuaa/dt-format
sort node. any issue, let me know.

Thanks for the link :) would be nice to have this script to be part of
the kernel. The script follows the rules in [1] I'm just used to have
common properties like pinctrl-* in front of the device specific
properties e.g. "enable-active-high". But this rule is not part of [1]
so I can't blame the script.

Regards,
   Marco

[1] https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node

Thank you for the review Frank & Marco.
I quickly went through the file again and found another few properties
that could be better ordered according to the kernel documentation [1].

  2 files changed, 424 insertions(+)
  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-iota2-lumpy.dts

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 9d3df8b218a2..aa26a50b7bb4 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -171,6 +171,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk2.dtb
  dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk3.dtb
  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
  dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
+dtb-$(CONFIG_ARCH_MXC) += imx8mp-iota2-lumpy.dtb
  dtb-$(CONFIG_ARCH_MXC) += imx8mp-msc-sm2s-ep1.dtb
  dtb-$(CONFIG_ARCH_MXC) += imx8mp-navqp.dtb
  dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-iota2-lumpy.dts b/arch/arm64/boot/dts/freescale/imx8mp-iota2-lumpy.dts
new file mode 100644
index 000000000000..9eb58e818dc7
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-iota2-lumpy.dts
@@ -0,0 +1,423 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2023 Y Soft
+ */
+
+/dts-v1/;
+
+#include "imx8mp.dtsi"
+
+/ {
+	compatible = "ysoft,imx8mp-iota2-lumpy", "fsl,imx8mp";
+	model = "Y Soft i.MX8MPlus IOTA2 Lumpy board";
+
+	beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 500000 0>;
+	};
+
+	chosen {
+		stdout-path = &uart2;
+	};
+
+	gpio_keys: gpio-keys {
+		compatible = "gpio-keys";
+		pinctrl-0 = <&pinctrl_gpio_keys>;
+		pinctrl-names = "default";
+
+		button-reset {
+			gpios = <&gpio1 7 GPIO_ACTIVE_LOW>;
+			label = "Factory RESET";
+			linux,code = <BTN_0>;
+		};
+	};
+
+	reg_usb_host: regulator-usb-host {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;

The enable-active-high and gpio should go bellow regulator-*.

+		pinctrl-0 = <&pinctrl_usb_host_vbus>;
+		pinctrl-names = "default";
+		regulator-max-microvolt = <5000000>;
+		regulator-min-microvolt = <5000000>;
+		regulator-name = "usb-host";
+	};
+
+	memory@40000000 {
+		reg = <0x0 0x40000000 0 0x80000000>,
+		      <0x1 0x00000000 0 0x80000000>;
+		device_type = "memory";
+	};
+};
+
+&A53_0 {
+	cpu-supply = <&reg_arm>;
+};
+
+&A53_1 {
+	cpu-supply = <&reg_arm>;
+};
+
+&A53_2 {
+	cpu-supply = <&reg_arm>;
+};
+
+&A53_3 {
+	cpu-supply = <&reg_arm>;
+};
+
+&eqos {
+	phy-handle = <&ethphy0>;
+	phy-mode = "rgmii-id";
+	pinctrl-0 = <&pinctrl_eqos>;
+	pinctrl-names = "default";
+	status = "okay";
+
+	mdio {
+		compatible = "snps,dwmac-mdio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ethphy0: ethernet-phy@0 {
+			reg = <0>;
+			interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
+			interrupt-parent = <&gpio3>;
+			micrel,led-mode = <0>;

The micrel,* is a vendor specific property. It should go bellow the reset-*.

+			pinctrl-0 = <&pinctrl_ethphy0>;
+			pinctrl-names = "default";
+			reset-assert-us = <1000>;
+			reset-deassert-us = <1000>;
+			reset-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;
+		};
+	};
+};
+
+&fec {
+	fsl,magic-packet;
+	phy-handle = <&ethphy1>;
+	phy-mode = "rgmii-id";
+	pinctrl-0 = <&pinctrl_fec>;
+	pinctrl-names = "default";
+	status = "okay";
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ethphy1: ethernet-phy@0 {
+			reg = <0>;
+			interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
+			interrupt-parent = <&gpio3>;
+			micrel,led-mode = <0>;

Same as above, micrel,* should go bellow common properties.
I will send a v5 with these fixed.

Michal

+			pinctrl-0 = <&pinctrl_ethphy1>;
+			pinctrl-names = "default";
+			reset-assert-us = <1000>;
+			reset-deassert-us = <1000>;
+			reset-gpios = <&gpio3 20 GPIO_ACTIVE_LOW>;
+		};
+	};
+};
+




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux