Hi Krzysztof,
Thank you for the review.
Le jeu. 28 mars 2024 à 18:04, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> a écrit :
>
> On 28/03/2024 21:23, Gilles Talis wrote:
> > The Emcraft Systems NavQ+ kit is a mobile robotics platform
> > based on NXP i.MX8 MPlus SoC.
> >
> > The following interfaces and devices are enabled:
> > - eMMC
> > - Gigabit Ethernet
> > - RTC
> > - SD-Card
> > - UART console
> >
> > Signed-off-by: Gilles Talis <gilles.talis@xxxxxxxxx>
> > ---
> > arch/arm64/boot/dts/freescale/Makefile | 1 +
> > .../arm64/boot/dts/freescale/imx8mp-navqp.dts | 435 ++++++++++++++++++
> > 2 files changed, 436 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
> >
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index 045250d0a040..bf99864c0bc4 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -166,6 +166,7 @@ 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-msc-sm2s-ep1.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-navqp.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mp-skov-revb-hdmi.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mp-skov-revb-lt6.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-navqp.dts b/arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
> > new file mode 100644
> > index 000000000000..8182e71008f8
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
> > @@ -0,0 +1,435 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright 2021 Emcraft Systems
> > + * Copyright 2024 Gilles Talis <gilles.talis@xxxxxxxxx>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include "imx8mp.dtsi"
> > +
> > +/ {
> > + model = "Emcraft Systems i.MX8MPlus NavQ+ Kit";
> > + compatible = "emcraft,imx8mp-navqp", "fsl,imx8mp";
> > +
> > + chosen {
> > + stdout-path = &uart2;
> > + };
> > +
> > + leds {
> > + compatible = "gpio-leds";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_gpio_led>;
> > +
> > + status {
>
> 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 = "status";
>
> Please provide color and function properties, if reasonable, and then
> remove label property.
>
> > + gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
> > + default-state = "on";
> > + };
> > + };
> > +
>
> ...
>
> > + pinctrl_i2c6: i2c6grp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_UART4_RXD__I2C6_SCL 0x400001c3
> > + MX8MP_IOMUXC_UART4_TXD__I2C6_SDA 0x400001c3
> > + >;
> > + };
> > +
> > + pinctrl_pmic: pmicirq {
> > + 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_uart2: uart2grp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX 0x49
> > + MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX 0x49
> > + >;
> > + };
> > +
> > + 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: usdhc2grp-100mhz {
>
> Hm, you upstreamed something based on downstream source. Please avoid
> that. Instead take upstream, clean DTS and customize it to your needs.
> Otherwise you send patch with the same issues we fixed.
OK. Got it. Will avoid that in the future.
>
> Standard form letter:
> 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).
>
> Nodes end on 'grp' I believe.
I did run it, but I might have messed up something, as I did not get
the results your automatic check reported. Let me run it again and fix
the issues.
>
>
> Best regards,
> Krzysztof
>
regards
Gilles.
