From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7C7E5C433FE for ; Wed, 5 Oct 2022 14:54:19 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 87ABF84D87; Wed, 5 Oct 2022 16:54:16 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="iBg/5uTY"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id AC64284D8E; Wed, 5 Oct 2022 16:54:14 +0200 (CEST) Received: from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com [IPv6:2a00:1450:4864:20::62e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 7AB6184D78 for ; Wed, 5 Oct 2022 16:54:09 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=oliver.graute@gmail.com Received: by mail-ej1-x62e.google.com with SMTP id au23so15262860ejc.1 for ; Wed, 05 Oct 2022 07:54:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=user-agent:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:mail-followup-to :message-id:subject:cc:to:date:from:from:to:cc:subject:date; bh=qGJFZNNmNEmnPD3Jzh91phU5ndUfF8HG/RjPmEzn+nk=; b=iBg/5uTYxSwwGCdXAOMVDBITpNYj+sxGRSN+BnTRlXp5b6TzGA/fH09uJQTZUgHXRY MO5zW7xrccL19UEuLfLn/foOUYR6XIE+9xrnT2eq+He9xj1TFCoa+PM0WZ8hENyx2JXh I46AJWGzY5lVfzWpic/FLSz06kZdLJBDzIUJWhyckXL42FyRTQxw/hVV6JfiJtDL083u nXjqT4Jq7gVFBqFMlphVdWx/J7S1s/LCLR6nQu+cPXW+ZNpIHKrTwzcObk9ZmOSF/dI1 DZU9kQUlDSyesVCLzjYR+ir9J9FkWyYY7x2Y0B50BEF7yjaOIxH9bDkw0LydV1psPODN A1Qw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=user-agent:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:mail-followup-to :message-id:subject:cc:to:date:from:x-gm-message-state:from:to:cc :subject:date; bh=qGJFZNNmNEmnPD3Jzh91phU5ndUfF8HG/RjPmEzn+nk=; b=leC/KjSbTQ1+pWGctRUvCtWiLaWkMRxX/zTGDjHAAwXG75JZg/LJw9chjx3f4Xko4a nGWHgqZUvV/WkksaDbKRjzKFtbjzZV11zxvmI6ikOFvy1l5cPLJz0Bmit+VaThdrtQTy /4XXMRWa4vofKFyQpf4L58DDUE1TQhkzRWqImHD/I8CVs43e6on2+Q3xGmAP+fX6KDlk +Dbytaemf6nxi6LASC5Fro4c7kCg+BjvbCTDD8QIB1mqlxI940c+NRMqIIv+Hdzk03Fm azMoF1eif9I/nvc8M2n3ObHYPCotpUpmoCCvF6nUQfgUXI7Z7399/KOMpWee87zaRo0Z inkw== X-Gm-Message-State: ACrzQf338MWV6xkf8xGM4SSw732x3i3tlm/oJnpMlwXGfTOm5MIl/G/L PVM4yg0pqCqOw/dlVsgCWhA= X-Google-Smtp-Source: AMsMyM436w8HtlDDFjJ/v+MOQrzHqv/D44O5YjzgnlzqUxOsyS1YJA5ywBOdpGWlU2Ygn90n+TYGRg== X-Received: by 2002:a17:907:a076:b0:78d:23f4:421c with SMTP id ia22-20020a170907a07600b0078d23f4421cmr4124582ejc.697.1664981648870; Wed, 05 Oct 2022 07:54:08 -0700 (PDT) Received: from localhost (business-90-187-74-145.pool2.vodafone-ip.de. [90.187.74.145]) by smtp.gmail.com with ESMTPSA id 17-20020a170906211100b0077d37a5d401sm8816090ejt.33.2022.10.05.07.54.07 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 05 Oct 2022 07:54:07 -0700 (PDT) From: "oliver.graute@kococonnector.com" X-Google-Original-From: "oliver.graute@kococonnector.com" Date: Wed, 5 Oct 2022 16:22:05 +0200 To: Marcel Ziswiler Cc: "sbabic@denx.de" , "andre.przywara@arm.com" , "ye.li@nxp.com" , "peng.fan@nxp.com" , "jagan@amarulasolutions.com" , "agust@denx.de" , "u-boot@lists.denx.de" , "paul.liu@linaro.org" , Denys Drozdov , "marex@denx.de" , "mbrugger@suse.com" , "gaurav.jain@nxp.com" , "michal.simek@amd.com" , "patrick.delaunay@foss.st.com" , "sjg@chromium.org" , "samuel@sholland.org" , "uboot-imx@nxp.com" , "christianshewitt@gmail.com" , "festevam@gmail.com" Subject: Re: [PATCH v4] imx: support i.MX8QM DMSSE20 a1 board Message-ID: <20221005142205.GA25912@optiplex> Mail-Followup-To: Marcel Ziswiler , "sbabic@denx.de" , "andre.przywara@arm.com" , "ye.li@nxp.com" , "peng.fan@nxp.com" , "jagan@amarulasolutions.com" , "agust@denx.de" , "u-boot@lists.denx.de" , "paul.liu@linaro.org" , Denys Drozdov , "marex@denx.de" , "mbrugger@suse.com" , "gaurav.jain@nxp.com" , "michal.simek@amd.com" , "patrick.delaunay@foss.st.com" , "sjg@chromium.org" , "samuel@sholland.org" , "uboot-imx@nxp.com" , "christianshewitt@gmail.com" , "festevam@gmail.com" References: <20220712101446.16751-1-oliver.graute@kococonnector.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On 13/07/22, Marcel Ziswiler wrote: > Hi Oliver > > On Tue, 2022-07-12 at 12:14 +0200, Oliver Graute wrote: > > Add i.MX8QM DMSSE20 a1 board support > > > > U-Boot 2022.07-00033-g2a5cf8e9e7 (Jul 12 2022 - 11:29:05 +0200) > > > > Model: Advantech iMX8QM DMSSE20 > > Board: DMS-SE20A1 8GB > > Build: SCFW 549b1e18, SECO-FW c9de51c0, ATF 5782363 > > Boot:  USB > > DRAM:  8 GiB > > Core:  100 devices, 19 uclasses, devicetree: separate > > MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2 > > Loading Environment from MMC... OK > > In:    serial@5a060000 > > Out:   serial@5a060000 > > Err:   serial@5a060000 > > Net:   eth0: ethernet@5b040000 > > Warning: ethernet@5b050000 (eth1) using random MAC address - aa:a2:be:5b:81:4a > > , eth1: ethernet@5b050000 > > Hit any key to stop autoboot:  0 > > > > Signed-off-by: Oliver Graute > > --- > >  Changes for v4 > >  -update atf fw version > >  -update seco fw version > >  -update scfw version > >  -move CONFIG_IMX_SMMU to imx8qm_dmsse20a1_defconfig > >  -move CONFIG_LOADADDR to imx8qm_dmsse20a1_defconfig > >  -move CONFIG_SYS_LOAD_ADDR to imx8qm_dmsse20a1_defconfig > >  -move CONFIG_SYS_MALLOC_LEN to imx8qm_dmsse20a1_defconfig > >  -move CONFIG_SYS_MMC_IMG_LOAD_PART to imx8qm_dmsse20a1_defconfig > >  -move CONFIG_FSL_USDHC to imx8qm_dmsse20a1_defconfig > >  -replaced CONFIG_SPL_MMC_SUPPORT with CONFIG_SPL_MMC > >  -replaced CONFIG_SPL_SERIAL_SUPPORT with CONFIG_SPL_SERIAL > >  -drop CONFIG_FEC_XCV_TYPE > > > > Changes for v3 > >  -Remove addr parameter from reset_cpu() > >  -moved some configs into defconfig > > > > Changes for v2 > >  -replaced bd_t with struct bd_info > >  -added missing DTS in MAINTAINERS > >  -replaced README with imx8qm-dmsse20-a1.rst > >  -move CMD_FUSE to Kconfig > >  -removed fdt_high > >  -added i2c support > >  -added rtc support > > > >  arch/arm/dts/Makefile                         |   1 + > >  arch/arm/dts/imx8qm-dmsse20-a1.dts            | 407 ++++++++++++++++++ > >  arch/arm/mach-imx/imx8/Kconfig                |   7 + > >  board/advantech/imx8qm_dmsse20_a1/Kconfig     |  17 + > >  board/advantech/imx8qm_dmsse20_a1/MAINTAINERS |   7 + > >  board/advantech/imx8qm_dmsse20_a1/Makefile    |   8 + > >  .../imx8qm_dmsse20_a1/imx8qm_dmsse20_a1.c     | 188 ++++++++ > >  .../advantech/imx8qm_dmsse20_a1/imximage.cfg  |  21 + > >  board/advantech/imx8qm_dmsse20_a1/spl.c       | 224 ++++++++++ > >  common/Kconfig                                |   2 +- > >  configs/imx8qm_dmsse20a1_defconfig            | 105 +++++ > >  doc/board/advantech/imx8qm-dmsse20-a1.rst     |  59 +++ > >  doc/board/advantech/index.rst                 |   1 + > >  include/configs/imx8qm_dmsse20.h              | 164 +++++++ > >  14 files changed, 1210 insertions(+), 1 deletion(-) > >  create mode 100644 arch/arm/dts/imx8qm-dmsse20-a1.dts > >  create mode 100644 board/advantech/imx8qm_dmsse20_a1/Kconfig > >  create mode 100644 board/advantech/imx8qm_dmsse20_a1/MAINTAINERS > >  create mode 100644 board/advantech/imx8qm_dmsse20_a1/Makefile > >  create mode 100644 board/advantech/imx8qm_dmsse20_a1/imx8qm_dmsse20_a1.c > >  create mode 100644 board/advantech/imx8qm_dmsse20_a1/imximage.cfg > >  create mode 100644 board/advantech/imx8qm_dmsse20_a1/spl.c > >  create mode 100644 configs/imx8qm_dmsse20a1_defconfig > >  create mode 100644 doc/board/advantech/imx8qm-dmsse20-a1.rst > >  create mode 100644 include/configs/imx8qm_dmsse20.h > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > > index a7e0d9f6c0..50bc76289f 100644 > > --- a/arch/arm/dts/Makefile > > +++ b/arch/arm/dts/Makefile > > @@ -920,6 +920,7 @@ dtb-$(CONFIG_ARCH_IMX8) += \ > >         fsl-imx8qm-apalis.dtb \ > >         fsl-imx8qm-mek.dtb \ > >         imx8qm-cgtqmx8.dtb \ > > +       imx8qm-dmsse20-a1.dtb \ > >         imx8qm-rom7720-a1.dtb \ > >         fsl-imx8qxp-ai_ml.dtb \ > >         fsl-imx8qxp-colibri.dtb \ > > diff --git a/arch/arm/dts/imx8qm-dmsse20-a1.dts b/arch/arm/dts/imx8qm-dmsse20-a1.dts > > new file mode 100644 > > index 0000000000..81ef7fb2ce > > --- /dev/null > > +++ b/arch/arm/dts/imx8qm-dmsse20-a1.dts > > @@ -0,0 +1,407 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > For device trees it is usually advisable to use the following license: > > GPL-2.0-or-later OR MIT > > Anyway, please use at least latest SPDX notation being GPL-2.0-or-later rather than GPL-2.0+. ok > > > +/* > > + * Copyright 2017 NXP > > That also seems bogus to me. ok > > > + */ > > + > > +/dts-v1/; > > + > > +/* First 128KB is for PSCI ATF. */ > > +/memreserve/ 0x80000000 0x00020000; > > + > > +#include "fsl-imx8qm.dtsi" > > + > > +/ { > > +       model = "Advantech iMX8QM DMSSE20"; > > +       compatible = "fsl,imx8qm-mek", "fsl,imx8qm"; > > + > > +       aliases { > > +          mmc0 = &usdhc1; > > +          mmc2 = &usdhc3; > > +       }; > > + > > +       chosen { > > +               bootargs = "console=ttyLP0,115200 earlycon=lpuart32,0x5a060000,115200"; > > This stuff is completely downstream bogus. I'am confused here because I see such statements in a lot of device trees. > > > +               stdout-path = &lpuart0; > > +       }; > > + > > +       regulators { > > That grouping is also bogus. What do you mean exactly? > > > +               compatible = "simple-bus"; > > +               #address-cells = <1>; > > +               #size-cells = <0>; > > + > > +               reg_usb_otg1_vbus: regulator@0 { > > +                       compatible = "regulator-fixed"; > > +                       reg = <0>; > > Using any such is also bogus (same with above @0 notation, of course). I'am confused here too. What is the right notation now? > > > +                       regulator-name = "usb_otg1_vbus"; > > +                       regulator-min-microvolt = <5000000>; > > +                       regulator-max-microvolt = <5000000>; > > +                       gpio = <&gpio4 3 GPIO_ACTIVE_HIGH>; > > +                       enable-active-high; > > +               }; > > + > > +               reg_usdhc2_vmmc: usdhc2_vmmc { > > +                       compatible = "regulator-fixed"; > > +                       regulator-name = "sw-3p3-sd1"; > > +                       regulator-min-microvolt = <3300000>; > > +                       regulator-max-microvolt = <3300000>; > > +                       gpio = <&gpio4 7 GPIO_ACTIVE_HIGH>; > > +                       enable-active-high; > > +               }; > > + > > +               busfreq { > > +                       status = "disabled"; > > +               }; > > +       }; > > +}; > > + > > +&iomuxc { > > +       pinctrl-names = "default"; > > +       pinctrl-0 = <&pinctrl_hog_1>; > > + > > +       imx8qm-mek { > > +               pinctrl_hog_1: hoggrp-1 { > > That notation vs. below without dash could also be cleaned up. ok will fix snip > > +                       >; > > +               }; > > + > > +               pinctrl_rtc_mc_8803: rtc_mc_8803_grp{ > > Underscores in node names. Ugh! ok will fix > > > +                       fsl,pins = < > > +                               SC_P_SIM0_POWER_EN_DMA_I2C3_SDA         0xc600004c > > +                               SC_P_SIM0_PD_DMA_I2C3_SCL               0xc600004c > > +                       >; > > +               }; > > + > > +               pinctrl_usdhc1: usdhc1grp { > > +                       fsl,pins = < > > +                               SC_P_EMMC0_CLK_CONN_EMMC0_CLK           0x06000041 > > +                               SC_P_EMMC0_CMD_CONN_EMMC0_CMD           0x00000021 > > +                               SC_P_EMMC0_DATA0_CONN_EMMC0_DATA0       0x00000021 > > +                               SC_P_EMMC0_DATA1_CONN_EMMC0_DATA1       0x00000021 > > +                               SC_P_EMMC0_DATA2_CONN_EMMC0_DATA2       0x00000021 > > +                               SC_P_EMMC0_DATA3_CONN_EMMC0_DATA3       0x00000021 > > +                               SC_P_EMMC0_DATA4_CONN_EMMC0_DATA4       0x00000021 > > +                               SC_P_EMMC0_DATA5_CONN_EMMC0_DATA5       0x00000021 > > +                               SC_P_EMMC0_DATA6_CONN_EMMC0_DATA6       0x00000021 > > +                               SC_P_EMMC0_DATA7_CONN_EMMC0_DATA7       0x00000021 > > +                               SC_P_EMMC0_STROBE_CONN_EMMC0_STROBE     0x06000041 > > +                               SC_P_EMMC0_RESET_B_CONN_EMMC0_RESET_B   0x00000021 > > +                       >; > > +               }; > > + > > +               pinctrl_usdhc1_100mhz: usdhc1grp100mhz { > > I believe this speed denomination is the very only place dashes would be used. See e.g. here > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mp-evk.dts#n578 thx for this hint will fix it > > > +       pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>; > > +       pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>; > > +       pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>; > > +       bus-width = <4>; > > +       cd-gpios = <&gpio5 22 GPIO_ACTIVE_LOW>; > > +       wp-gpios = <&gpio5 21 GPIO_ACTIVE_HIGH>; > > +       vmmc-supply = <®_usdhc2_vmmc>; > > I would sort those alphabetically. will fix it > > > +       status = "okay"; > > +}; > > + > > +&usdhc3 { > > +       pinctrl-names = "default","state_100mhz", "state_200mhz"; > > +       pinctrl-0 = <&pinctrl_usdhc3>, <&pinctrl_usdhc3_gpio>; > > +       pinctrl-1 = <&pinctrl_usdhc3_100mhz>, <&pinctrl_usdhc3_gpio>; > > +       pinctrl-2 = <&pinctrl_usdhc3_200mhz>, <&pinctrl_usdhc3_gpio>; > > +       bus-width = <4>; > > +       cd-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>; > > +       wp-gpios = <&gpio4 11 GPIO_ACTIVE_HIGH>; > > +       no-1-8-v; > > Ditto. ok > > > +       status = "okay"; > > +}; > > + > > +&fec1 { > > +       pinctrl-names = "default"; > > +       pinctrl-0 = <&pinctrl_fec1>; > > +       phy-mode = "rgmii-id"; > > +       phy-handle = <ðphy0>; > > +       fsl,ar8031-phy-fixup; > > +       fsl,magic-packet; > > D. ok > > > +       status = "okay"; > > + > > +       mdio { > > +               #address-cells = <1>; > > +               #size-cells = <0>; > > + > > +               ethphy0: ethernet-phy@4 { > > +                       compatible = "ethernet-phy-ieee802.3-c22"; > > +                       reg = <4>; > > +               }; > > +       }; > > +}; > > + > > +&fec2 { > > +       pinctrl-names = "default"; > > +       pinctrl-0 = <&pinctrl_fec2>; > > +       phy-mode = "rgmii-id"; > > +       phy-handle = <ðphy1>; > > +       fsl,ar8031-phy-fixup; > > +       fsl,magic-packet; > > +       status = "okay"; > > +       fsl,mii-exclusive; > > That actually is just downstream only NXP bogus. you mean the `fsl,mii-exclusive`? so I should just drop it? > > > new file mode 100644 > > index 0000000000..262ffcd683 > > --- /dev/null > > +++ b/board/advantech/imx8qm_dmsse20_a1/Makefile > > @@ -0,0 +1,8 @@ > > +# > > +# Copyright 2017 NXP > > D. ok > > > +# > > +# SPDX-License-Identifier:     GPL-2.0+ > > D. ok > > > +# > > + > > +obj-y += imx8qm_dmsse20_a1.o > > +obj-$(CONFIG_SPL_BUILD) += spl.o > > diff --git a/board/advantech/imx8qm_dmsse20_a1/imx8qm_dmsse20_a1.c > > b/board/advantech/imx8qm_dmsse20_a1/imx8qm_dmsse20_a1.c > > new file mode 100644 > > index 0000000000..c3bc26f80d > > --- /dev/null > > +++ b/board/advantech/imx8qm_dmsse20_a1/imx8qm_dmsse20_a1.c > > @@ -0,0 +1,188 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > D. ok > > > +/* > > + * Copyright 2017-2018 NXP > > + * Copyright (C) 2020 Oliver Graute > > How the time flies. ok > > > index 0000000000..e324c7ca37 > > --- /dev/null > > +++ b/board/advantech/imx8qm_dmsse20_a1/imximage.cfg > > @@ -0,0 +1,21 @@ > > +/* SPDX-License-Identifier:    GPL-2.0+ */ > > D. ok > > > +/* > > + * Copyright 2018 NXP > > D. ok > > > index 0000000000..06bb049c3a > > --- /dev/null > > +++ b/board/advantech/imx8qm_dmsse20_a1/spl.c > > @@ -0,0 +1,224 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > D. ok > > > +/* > > + * Copyright 2017-2018 NXP > > D. ok > > > + > > +#endif /* __IMX8QM_DMSSE20_H */ > > Otherwise looks great. Keep up the good work! > thx for the review > Cheers > > Marcel Best Regards, Oliver