Re: [PATCH 3/3] arm64: dts: freescale: Add device tree for Emcraft Systems NavQ+ Kit

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

 



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.





[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