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

From: Stefan Wahren
Date: Tue Oct 08 2024 - 11:54:15 EST

Next message: Andy Shevchenko: "Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver"
Previous message: Borislav Petkov: "Re: [PATCH v7 1/3] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU"
In reply to: Lukasz Majewski: "Re: [PATCH v6 2/2] dts: nxp: mxs: Add descriptions for imx287 based btt3-[012] devices"
Next in thread: Lukasz Majewski: "Re: [PATCH v6 2/2] dts: nxp: mxs: Add descriptions for imx287 based btt3-[012] devices"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Lukasz,

Am 08.10.24 um 13:41 schrieb Lukasz Majewski:

Hi Stefan,

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?

Yes, exactly. This is long-standing device.

+};
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.

I do keep the bootargs from first version of the device/DTS to avoid
any "unexpected" regressions.

Not strong opinion about this this, but it sounds like your customer use
a limited bootloader like imx-bootlets and the whole cmdline is
configured via DT.

+
+ memory@40000000 {
+ reg = <0x40000000 0x10000000>;
+ device_type = "memory";
+ };
+
+ poweroff {
+ compatible = "gpio-poweroff";
+ gpios = <&gpio0 24 0>;

Please use the GPIO polarity defines.

Ok.

+ };
+
+ 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.

Yes, this is replaced during production. In fact it could be 00 00 00
00 00 00 as well.

The IP address assigned here allows the device to be recognizable on
the network even when the full "flashing" is not successful.

Could you please add a short comment about this?

+ 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.

Yes - I will remove it.

+ 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.

IIRC - there was no schema warning for it.

Nevertheless please use dashes like on the other nodes

Regards

Next message: Andy Shevchenko: "Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver"
Previous message: Borislav Petkov: "Re: [PATCH v7 1/3] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU"
In reply to: Lukasz Majewski: "Re: [PATCH v6 2/2] dts: nxp: mxs: Add descriptions for imx287 based btt3-[012] devices"
Next in thread: Lukasz Majewski: "Re: [PATCH v6 2/2] dts: nxp: mxs: Add descriptions for imx287 based btt3-[012] devices"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]