Hello Krzysztof,
On 28.10.24 11:24, Krzysztof Kozlowski wrote:
On 28/10/2024 09:23, Heiko Schocher wrote:
Add support for the i.MX8MP based aristainetos3 boards from ABB.
The board uses a ABB specific SoM from ADLink, based on NXP
i.MX8MP SoC. The SoM is used on 3 different carrier boards,
with small differences, which are all catched up in
devicetree overlays. The kernel image, the basic dtb
and all dtbos are collected in a fitimage. As bootloader
is used U-Boot which detects in his SPL stage the carrier
board by probing some i2c devices. When the correct
carrier is probed, the SPL applies all needed dtbos to
the dtb with which U-Boot gets loaded. Same principle
later before linux image boot, U-Boot applies the dtbos
needed for the carrier board before booting Linux.
Signed-off-by: Heiko Schocher <hs@xxxxxxx>
---
checkpatch dropped the following warnings:
arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi:248: warning: DT compatible string "ethernet-phy-id2000.a231" appears un-documented -- check ./Documentation/devicetree/bindings/
ignored, as this compatible string is usedin other dts too, for example in
arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi
arch/arm64/boot/dts/freescale/Makefile | 5 +
.../imx8mp-aristainetos3-adpismarc.dtsi | 64 +
.../imx8mp-aristainetos3-adpismarc.dtso | 14 +
.../imx8mp-aristainetos3-helios-lvds.dtsi | 89 ++
.../imx8mp-aristainetos3-helios-lvds.dtso | 13 +
.../imx8mp-aristainetos3-helios.dtsi | 103 ++
.../imx8mp-aristainetos3-helios.dtso | 13 +
.../imx8mp-aristainetos3-proton2s.dtsi | 176 +++
.../imx8mp-aristainetos3-proton2s.dtso | 13 +
.../imx8mp-aristainetos3a-som-v1.dts | 18 +
.../imx8mp-aristainetos3a-som-v1.dtsi | 1210 +++++++++++++++++
11 files changed, 1718 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtsi
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtso
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtsi
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtso
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios.dtsi
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios.dtso
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-proton2s.dtsi
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-proton2s.dtso
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dts
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 9d3df8b218a2..7c3586509b8b 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -163,6 +163,11 @@ imx8mn-tqma8mqnl-mba8mx-usbotg-dtbs += imx8mn-tqma8mqnl-mba8mx.dtb imx8mn-tqma8m
dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx-lvds-tm070jvhg33.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx-usbotg.dtb
+dtb-$(CONFIG_ARCH_MXC) += imx8mp-aristainetos3a-som-v1.dtb \
+ imx8mp-aristainetos3-adpismarc.dtbo \
+ imx8mp-aristainetos3-proton2s.dtbo \
+ imx8mp-aristainetos3-helios.dtbo \
+ imx8mp-aristainetos3-helios-lvds.dtbo
dtb-$(CONFIG_ARCH_MXC) += imx8mp-beacon-kit.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mp-data-modul-edm-sbc.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mp-debix-model-a.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtsi
new file mode 100644
index 000000000000..cc0cddaa33ea
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtsi
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2024 Heiko Schocher <hs@xxxxxxx>
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+&ecspi1 {
+ spidev0: spi@0 {
+ reg = <0>;
+ compatible = "rohm,dh2228fv";
Hm? I have some doubts, what device is here?
$ grep -lr dh2228fv drivers/
drivers/spi/spidev.c
Customer uses an userspace implementation...
+ spi-max-frequency = <500000>;
+ };
+};
+
+&ecspi2 {
+ spidev1: spi@0 {
+ reg = <0>;
+ compatible = "rohm,dh2228fv";
+ spi-max-frequency = <500000>;
+ };
+};
+
+&i2c2 {
+ /* SX1509(2) U1001@IPi SMARC Plus */
+ gpio8: i2c2_gpioext0@3e {
Uh, no, please never send us downstream code.
Please follow DTS coding style in all upstream submissions.
driver is in here:
$ grep -lr probe-reset drivers/pinctrl/
drivers/pinctrl/pinctrl-sx150x.c
Also:
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Sorry for the nodenames...
And yes, I remove the comments.
+ /* GPIO Expander 2 Mapping :
+ * - 0: E_GPIO1_0 <=> IPi SMARC Plus CN101_PIN29: E_GPIO1_0
+ * - 1: E_GPIO1_1 <=> IPi SMARC Plus CN101_PIN31: E_GPIO1_1
+ * - 2: E_GPIO1_2 <=> IPi SMARC Plus CN101_PIN32: E_GPIO1_2
+ * - 3: E_GPIO1_3 <=> IPi SMARC Plus CN101_PIN33: E_GPIO1_3
+ * - 4: E_GPIO1_4 <=> IPi SMARC Plus CN101_PIN35: E_GPIO1_4
+ * - 5: E_GPIO1_5 <=> IPi SMARC Plus CN101_PIN36: E_GPIO1_5
+ * - 6: E_GPIO1_6 <=> IPi SMARC Plus CN101_PIN37: E_GPIO1_6
+ * - 7: E_GPIO1_7 <=> IPi SMARC Plus CN101_PIN38: E_GPIO1_7
+ * - 8: E_GPIO2_8 <=> IPi SMARC Plus CN101_PIN40: E_GPIO2_8
+ * - 9: TP1002 <=> IPi SMARC Plus TP1002 (won't use)
+ * - 10: TP1003 <=> IPi SMARC Plus TP1003 (won't use)
+ * - 11: TP1004 <=> IPi SMARC Plus TP1004 (won't use)
+ * - 12: TP1005 <=> IPi SMARC Plus TP1005 (won't use)
+ * - 13: TP1006 <=> IPi SMARC Plus TP1006 (won't use)
+ * - 14: TP1007 <=> IPi SMARC Plus TP1007 (won't use)
+ * - 15: TP1008 <=> IPi SMARC Plus TP1008 (won't use)
+ * - 16: OSCIO <=> IPi SMARC Plus TP1001 (won't use)
+ */
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ compatible = "semtech,sx1509q";
+ reg = <0x3e>;
+
+ semtech,probe-reset;
+ gpio-controller;
+ interrupt-controller;
+
+ interrupt-parent = <&gpio6>;
+ interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
+ };
+
Drop
+};
+
+&flexcan1 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtso b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtso
new file mode 100644
index 000000000000..5a9adccbf7cf
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtso
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2024 Heiko Schocher <hs@xxxxxxx>
+ */
+/dts-v1/;
+/plugin/;
+
+#include "imx8mp-aristainetos3-adpismarc.dtsi"
+
+&{/} {
+ model = "Aristainetos3 ADLink PI SMARC carrier";
+ compatible = "abb,aristainetos3-adpismarc", "imx8mp-aristianetos3",
+ "abb,aristianetos3-som", "fsl,imx8mp";
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Thanks for the hint! I have to fix my scripts...
And why this is DTSO, I have no clue...
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtsi
new file mode 100644
index 000000000000..55aabd6fc1f7
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtsi
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2024 Heiko Schocher <hs@xxxxxxx>
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/pwm/pwm.h>
+
+&{/} {
+ panel: panel {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_lcd0_vdd_en>;
+ compatible = "lg,lb070wv8";
+ backlight = <&lvds_backlight>;
+ enable-gpios = <&gpio1 13 GPIO_ACTIVE_HIGH>;
+
+ port {
+ panel_in: endpoint {
+ remote-endpoint = <&ldb_lvds_ch0>;
+ };
+ };
+ };
+};
+
+&gpio3 {
+ mipi_lvds_select {
No, read coding style.
+ gpio-hog;
+ gpios = <23 GPIO_ACTIVE_HIGH>;
+ output-low;
+ line-name = "mipi_lvds_select";
+ };
+};
+
+&hdmi_blk_ctrl {
+ status = "disabled";
+};
+
+&hdmi_pvi {
+ status = "disabled";
+};
+
+&hdmi_tx {
+ status = "disabled";
+};
+
+&hdmi_tx_phy {
+ status = "disabled";
+};
+
+&irqsteer_hdmi {
+ status = "disabled";
+};
+
+&ldb_lvds_ch0 {
+ fsl,data-mapping = "jeida";
+ fsl,data-width = <24>;
+ remote-endpoint = <&panel_in>;
+};
+
+&lcdif1 {
+ status = "disabled";
+};
+
+&lcdif2 {
+ status = "okay";
+};
+
+&lcdif3 {
+ status = "disabled";
+};
+
+&lvds_backlight {
+ status = "okay";
+};
+
+&lvds_bridge {
+ status = "okay";
+};
+
+&media_blk_ctrl {
+ /*
+ * The internal divider will always divide the output LVDS clock by 7
+ * so our display needs 33246000 Hz, so set VIDEO_PLL1 to
+ * 33246000 * 7 = 232722000 Hz
+ */
+ assigned-clock-rates = <500000000>, <200000000>, <0>, <0>, <232722000>;
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtso b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtso
new file mode 100644
index 000000000000..06d1883b962a
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtso
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2024 Heiko Schocher <hs@xxxxxxx>
+ */
+/dts-v1/;
+/plugin/;
+
+#include "imx8mp-aristainetos3-helios-lvds.dtsi"
+
+&{/} {
+ model = "Aristainetos3 helios LVDS carrier";
+ compatible = "abb,aristainetos3-helios-lvds", "abb,aristainetos3-helios", "abb,aristianetos3-som", "fsl,imx8mp";
Read not only DTS coding style, but also kernel coding style. Lines are
supposed to be wrapped according to kernel coding style.
Yes, indeed, I have to fix my scripts...
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios.dtsi
new file mode 100644
index 000000000000..b4b1cb3b0cb3
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios.dtsi
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2024 Heiko Schocher <hs@xxxxxxx>
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+
+&{/} {
+ helios_gpio_leds {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
+ compatible = "gpio-leds";
+
+ helios_blue {
So this was absolutely never tested.
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
+ label = "helios:blue";
Use function and color instead.
I finished review here. Rest of the code does not look good, really. You
have so many, really so many, trivial issues which tools point out, that
using humans for such review is just waste of our time.
Sorry for that, will call the tools...
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs@xxxxxxx
