Re: [PATCH v6 2/2] dts: nxp: mxs: Add descriptions for imx287 based btt3-[012] devices

From: Stefan Wahren
Date: Fri Sep 13 2024 - 08:37:27 EST

Hi Lukasz,

please adjust the subject of your patch accordingly to the subsystem.

Suggestion

ARM: dts: mxs: Add descriptions for imx287 based btt3-[012] devices

Am 12.09.24 um 14:48 schrieb Lukasz Majewski:
The btt3 device' HW revisions from 0 to 2 use imx287 SoC and are to
some extend similar to already upstreamed XEA devices, hence are
using common imx28-lwe.dtsi file.

New, imx28-btt3.dtsi has been added to embrace common DTS
properties for different HW revisions for this device.

As a result - changes introduced in imx28-btt3-[012].dts are
minimal.

Signed-off-by: Lukasz Majewski <lukma@xxxxxxx>

---
Changes for v2:
- Rename dts file from btt3-[012] to imx28-btt3-[012] to match current
linux kernel naming convention
- Remove 'wlf,wm8974' from compatible for codec@1a

Changes for v3:
- Keep alphabethical order for Makefile entries

Changes for v4:
- Change compatible for btt3 board (to 'lwn,imx28-btt3')

Changes for v5:
- Combine patch, which adds btt3-[012] with one adding board entry to
fsl.yaml

Changes for v6:
- Make the patch series for adding entry in fsl.yaml and btt3
---
arch/arm/boot/dts/nxp/mxs/Makefile | 3 +
arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts | 12 +
arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts | 8 +
arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts | 12 +
arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi | 320 +++++++++++++++++++++
5 files changed, 355 insertions(+)
create mode 100644 arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts
create mode 100644 arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts
create mode 100644 arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts
create mode 100644 arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi

diff --git a/arch/arm/boot/dts/nxp/mxs/Makefile b/arch/arm/boot/dts/nxp/mxs/Makefile
index a430d04f9c69..96dd31ea19ba 100644
--- a/arch/arm/boot/dts/nxp/mxs/Makefile
+++ b/arch/arm/boot/dts/nxp/mxs/Makefile
@@ -8,6 +8,9 @@ dtb-$(CONFIG_ARCH_MXS) += \
imx28-apf28.dtb \
imx28-apf28dev.dtb \
imx28-apx4devkit.dtb \
+ imx28-btt3-0.dtb \
+ imx28-btt3-1.dtb \
+ imx28-btt3-2.dtb \
imx28-cfa10036.dtb \
imx28-cfa10037.dtb \
imx28-cfa10049.dtb \
diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts
new file mode 100644
index 000000000000..6ac46e4b21bb
--- /dev/null
+++ b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright 2024
+ * Lukasz Majewski, DENX Software Engineering, lukma@xxxxxxx
+ */
+
+/dts-v1/;
+#include "imx28-btt3.dtsi"
+
+&hog_pins_rev {
+ fsl,pull-up = <MXS_PULL_ENABLE>;
+};
diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts
new file mode 100644
index 000000000000..213fe931c58b
--- /dev/null
+++ b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright 2024
+ * Lukasz Majewski, DENX Software Engineering, lukma@xxxxxxx
+ */
+
+/dts-v1/;
+#include "imx28-btt3.dtsi"
diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts
new file mode 100644
index 000000000000..c787c2d03463
--- /dev/null
+++ b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright 2024
+ * Lukasz Majewski, DENX Software Engineering, lukma@xxxxxxx
+ */
+
+/dts-v1/;
+#include "imx28-btt3.dtsi"
+
+&lcdif {
+ display = <&display_te_b>;
The reason why you don't move the second display into this file is
because you expect a new hardware revision in the future?
+};
diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi b/arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi
new file mode 100644
index 000000000000..94a21ea8d5d2
--- /dev/null
+++ b/arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi
@@ -0,0 +1,320 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright 2024
+ * Lukasz Majewski, DENX Software Engineering, lukma@xxxxxxx
+ */
+/dts-v1/;
+#include "imx28-lwe.dtsi"
+
+/ {
+ model = "BTT3";
+
+ compatible = "lwn,imx28-btt3", "fsl,imx28";
+
+ chosen {
+ bootargs = "root=/dev/mmcblk0p2 rootfstype=ext4 ro rootwait console=ttyAMA0,115200 panic=1 quiet";
+ };
It's a little bit unusual to place so many Linux specific stuff into the
device tree bootargs.
+
+ memory@40000000 {
+ reg = <0x40000000 0x10000000>;
+ device_type = "memory";
+ };
+
+ poweroff {
+ compatible = "gpio-poweroff";
+ gpios = <&gpio0 24 0>;
Please use the GPIO polarity defines.
+ };
+
+ sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,name = "BTTC Audio";
+ simple-audio-card,widgets = "Speaker", "BTTC Speaker";
+ simple-audio-card,routing = "BTTC Speaker", "SPKOUTN", "BTTC Speaker", "SPKOUTP";
+ simple-audio-card,dai-link@0 {
+ format = "left_j";
+ bitclock-master = <&dai0_master>;
+ frame-master = <&dai0_master>;
+ mclk-fs = <256>;
+ dai0_master: cpu {
+ sound-dai = <&saif0>;
+ };
+ codec {
+ sound-dai = <&wm89xx>;
+ clocks = <&saif0>;
+ };
+ };
+ };
+
+ wifi_pwrseq: sdio-pwrseq {
+ compatible = "mmc-pwrseq-simple";
+ pinctrl-names = "default";
+ pinctrl-0 = <&wifi_en_pin_bttc>;
+ reset-gpios = <&gpio0 27 GPIO_ACTIVE_LOW>;
+ /* W1-163 needs 60us for WL_EN to be low and */
+ /* 150ms after high before downloading FW is possible */
+ post-power-on-delay-ms = <200>;
+ power-off-delay-us = <100>;
+ };
+};
+
+&auart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&auart0_2pins_a>;
+ status = "okay";
+};
+
+&auart3 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&auart3_pins_a>;
+ uart-has-rtscts;
+ status = "okay";
+};
+
+&i2c0 {
+ wm89xx: codec@1a {
+ compatible = "wlf,wm8940";
+ reg = <0x1a>;
+ #sound-dai-cells = <0>;
+ };
+};
+
+&lcdif {
+ pinctrl-names = "default";
+ pinctrl-0 = <&lcdif_24bit_pins_a>, <&lcdif_sync_pins_bttc>,
+ <&lcdif_reset_pins_bttc>;
+ lcd-supply = <&reg_3v3>;
+ display = <&display0>;
+ status = "okay";
+ display0: display0 {
+ bits-per-pixel = <32>;
+ bus-width = <24>;
+ display-timings {
+ native-mode = <&timing0>;
+ timing0: timing0 {
+ clock-frequency = <6500000>;
+ hactive = <320>;
+ vactive = <240>;
+ hfront-porch = <20>;
+ hback-porch = <38>;
+ hsync-len = <30>;
+ vfront-porch = <4>;
+ vback-porch = <14>;
+ vsync-len = <4>;
+ hsync-active = <0>;
+ vsync-active = <0>;
+ de-active = <0>;
+ pixelclk-active = <1>;
+ };
+ };
+ };
+ display_te_b: display1 {
+ bits-per-pixel = <32>;
+ bus-width = <24>;
+ display-timings {
+ native-mode = <&timing0>;
+ timing_te_b: timing0 {
+ clock-frequency = <6500000>;
+ hactive = <320>;
+ vactive = <240>;
+ hfront-porch = <20>;
+ hback-porch = <68>;
+ hsync-len = <30>;
+ vfront-porch = <4>;
+ vback-porch = <14>;
+ vsync-len = <4>;
+ hsync-active = <0>;
+ vsync-active = <0>;
+ de-active = <1>;
+ pixelclk-active = <1>;
+ };
+ };
+ };
+
+};
+
+&mac0 {
+ clocks = <&clks 57>, <&clks 57>, <&clks 64>;
+ clock-names = "ipg", "ahb", "enet_out";
+ phy-handle = <&mac0_phy>;
+ phy-mode = "rmii";
+ phy-supply = <&reg_3v3>;
+ local-mac-address = [ 00 11 B8 00 BF 8A ];
Is this replaced dynamically by the bootloader? Otherwise this suggests
all boards use the same MAC address.
+ status = "okay";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mac0_phy: ethernet-phy@0 {
+ /* LAN8720Ai - PHY ID */
+ compatible = "ethernet-phy-id0007.c0f0","ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ smsc,disable-energy-detect;
+ max-speed = <100>;
+
+ reset-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>; /* GPIO4_12 */
I think the comment only repeat what is already defined here.
+ reset-assert-us = <1000>;
+ reset-deassert-us = <1000>;
+ };
+ };
+};
+
+&pinctrl {
+ pinctrl-names = "default";
+ pinctrl-0 = <&hog_pins_a>, <&hog_pins_rev>;
+
+ hog_pins_a: hog@0 {
+ reg = <0>;
+ fsl,pinmux-ids = <
+ MX28_PAD_GPMI_RDY2__GPIO_0_22
+ MX28_PAD_GPMI_RDY3__GPIO_0_23
+ MX28_PAD_GPMI_RDN__GPIO_0_24
+ MX28_PAD_LCD_VSYNC__GPIO_1_28
+ MX28_PAD_SSP2_SS1__GPIO_2_20
+ MX28_PAD_SSP2_SS2__GPIO_2_21
+ MX28_PAD_AUART2_CTS__GPIO_3_10
+ MX28_PAD_AUART2_RTS__GPIO_3_11
+ MX28_PAD_GPMI_WRN__GPIO_0_25
+ MX28_PAD_ENET0_RXD2__GPIO_4_9
+ MX28_PAD_ENET0_TXD2__GPIO_4_11
+ >;
+ fsl,drive-strength = <MXS_DRIVE_4mA>;
+ fsl,voltage = <MXS_VOLTAGE_HIGH>;
+ fsl,pull-up = <MXS_PULL_DISABLE>;
+ };
+
+ hog_pins_rev: hog@1 {
+ reg = <1>;
+ fsl,pinmux-ids = <
+ MX28_PAD_ENET0_RXD3__GPIO_4_10
+ MX28_PAD_ENET0_TX_CLK__GPIO_4_5
+ MX28_PAD_ENET0_COL__GPIO_4_14
+ MX28_PAD_ENET0_CRS__GPIO_4_15
+ >;
+ fsl,drive-strength = <MXS_DRIVE_4mA>;
+ fsl,voltage = <MXS_VOLTAGE_HIGH>;
+ fsl,pull-up = <MXS_PULL_DISABLE>;
+ };
+
+ keypad_pins_bttc: keypad-bttc@0 {
+ reg = <0>;
+ fsl,pinmux-ids = <
+ MX28_PAD_GPMI_D00__GPIO_0_0
+ MX28_PAD_AUART0_CTS__GPIO_3_2
+ MX28_PAD_AUART0_RTS__GPIO_3_3
+ MX28_PAD_GPMI_D03__GPIO_0_3
+ MX28_PAD_GPMI_D04__GPIO_0_4
+ MX28_PAD_GPMI_D05__GPIO_0_5
+ MX28_PAD_GPMI_D06__GPIO_0_6
+ MX28_PAD_GPMI_D07__GPIO_0_7
+ MX28_PAD_GPMI_CE1N__GPIO_0_17
+ MX28_PAD_GPMI_CE2N__GPIO_0_18
+ MX28_PAD_GPMI_CE3N__GPIO_0_19
+ MX28_PAD_GPMI_RDY0__GPIO_0_20
+ >;
+ fsl,drive-strength = <MXS_DRIVE_4mA>;
+ fsl,voltage = <MXS_VOLTAGE_HIGH>;
+ fsl,pull-up = <MXS_PULL_DISABLE>;
+ };
+
+ lcdif_sync_pins_bttc: lcdif-bttc@0 {
+ reg = <0>;
+ fsl,pinmux-ids = <
+ MX28_PAD_LCD_DOTCLK__LCD_DOTCLK
+ MX28_PAD_LCD_ENABLE__LCD_ENABLE
+ MX28_PAD_LCD_HSYNC__LCD_HSYNC
+ MX28_PAD_LCD_RD_E__LCD_VSYNC
+ >;
+ fsl,drive-strength = <MXS_DRIVE_4mA>;
+ fsl,voltage = <MXS_VOLTAGE_HIGH>;
+ fsl,pull-up = <MXS_PULL_DISABLE>;
+ };
+
+ lcdif_reset_pins_bttc: lcdif-bttc@1 {
+ reg = <1>;
+ fsl,pinmux-ids = <
+ MX28_PAD_LCD_RESET__GPIO_3_30
+ >;
+ fsl,drive-strength = <MXS_DRIVE_4mA>;
+ fsl,voltage = <MXS_VOLTAGE_HIGH>;
+ fsl,pull-up = <MXS_PULL_ENABLE>;
+ };
+
+ ssp1_sdio_pins_a: ssp1-sdio@0 {
+ reg = <0>;
+ fsl,pinmux-ids = <
+ MX28_PAD_SSP1_DATA0__SSP1_D0
+ MX28_PAD_GPMI_D01__SSP1_D1
+ MX28_PAD_GPMI_D02__SSP1_D2
+ MX28_PAD_SSP1_DATA3__SSP1_D3
+ MX28_PAD_SSP1_CMD__SSP1_CMD
+ MX28_PAD_SSP1_SCK__SSP1_SCK
+ >;
+ fsl,drive-strength = <MXS_DRIVE_8mA>;
+ fsl,voltage = <MXS_VOLTAGE_HIGH>;
+ fsl,pull-up = <MXS_PULL_ENABLE>;
+ };
+
+ wifi_en_pin_bttc: wifi_en_pin@0 {
This should trigger a schema warning. The node name should use dashes
instead of underscore.
+ reg = <0>;
+ fsl,pinmux-ids = <
+ MX28_PAD_GPMI_CLE__GPIO_0_27
+ >;
+ fsl,drive-strength = <MXS_DRIVE_8mA>;
+ fsl,voltage = <MXS_VOLTAGE_HIGH>;
+ fsl,pull-up = <MXS_PULL_ENABLE>;
+ };
+};
+
+&pwm {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm3_pins_a>;
+ status = "okay";
+};
+
+&reg_usb_5v {
+ gpio = <&gpio1 28 0>;
Please use polarity define.
+};
+
+&saif0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&saif0_pins_a>;
+ #sound-dai-cells = <0>;
+ assigned-clocks = <&clks 53>;
+ assigned-clock-rates = <12000000>;
+ status = "okay";
+};
+
+&saif1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&saif1_pins_a>;
+ fsl,saif-master = <&saif0>;
+ #sound-dai-cells = <0>;
+ status = "okay";
+};
+
+&ssp1 {
+ compatible = "fsl,imx28-mmc";
+ pinctrl-names = "default";
+ pinctrl-0 = <&ssp1_sdio_pins_a>;
+ bus-width = <4>;
+ no-1-8-v; /* force 3.3V VIO */
+ pm-ignore-notify;
This seems to be undocumented.
+ non-removable;
+ vmmc-supply = <&reg_3v3>;
+ mmc-pwrseq = <&wifi_pwrseq>;
+ keep-power-in-suspend;
+ status = "okay";
+
+ wlan@1 {
+ reg = <1>;
+ compatible = "brcm,bcm4329-fmac";
+ };
+};
+
+&ssp2 {
+ compatible = "fsl,imx28-spi";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi2_pins_a>;
+ status = "okay";
+};