Re: [RESEND v4 08/10] arm64: dts: imx8dxl: Add i.MX8DXL evk board support

From: Abel Vesa <abel.vesa@nxp.com>
To: Shawn Guo <shawnguo@kernel.org>
Cc: Rob Herring <robh@kernel.org>,
	Dong Aisheng <aisheng.dong@nxp.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Fabio Estevam <festevam@gmail.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	linux-i2c@vger.kernel.org, linux-serial@vger.kernel.org,
	NXP Linux Team <linux-imx@nxp.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Jacky Bai <ping.bai@nxp.com>
Subject: Re: [RESEND v4 08/10] arm64: dts: imx8dxl: Add i.MX8DXL evk board support
Date: Thu, 10 Feb 2022 23:27:10 +0200	[thread overview]
Message-ID: <YgWDLo/+bdoyqOAY@abelvesa> (raw)
In-Reply-To: <20220126125355.GS4686@dragon>

On 22-01-26 20:53:55, Shawn Guo wrote:
> On Thu, Dec 16, 2021 at 08:48:12PM +0200, Abel Vesa wrote:
> > From: Jacky Bai <ping.bai@nxp.com>
> > 
> > Add i.MX8DXL EVK board support.
> > 
> > Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/Makefile        |   1 +
> >  arch/arm64/boot/dts/freescale/imx8dxl-evk.dts | 266 ++++++++++++++++++
> >  2 files changed, 267 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index 5018b8b1e5f2..f117d3e811ba 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mq-pico-pi.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-thor96.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-rmb3.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-zest.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8dxl-evk.dtb
> 
> Keep the list sorted.
> 
> >  dtb-$(CONFIG_ARCH_MXC) += imx8qm-mek.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-ai_ml.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-colibri-eval-v3.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> > new file mode 100644
> > index 000000000000..68dfe722af6d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> > @@ -0,0 +1,266 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019-2021 NXP
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "imx8dxl.dtsi"
> > +
> > +/ {
> > +	model = "Freescale i.MX8DXL EVK";
> > +	compatible = "fsl,imx8dxl-evk", "fsl,imx8dxl";
> > +
> > +	chosen {
> > +		stdout-path = &lpuart0;
> > +	};
> > +
> > +	memory@80000000 {
> > +		device_type = "memory";
> > +		reg = <0x00000000 0x80000000 0 0x40000000>;
> > +	};

 ...

> > +	pinctrl_usdhc1: usdhc1grp {
> > +		fsl,pins = <
> > +			IMX8DXL_EMMC0_CLK_CONN_EMMC0_CLK	0x06000041
> > +			IMX8DXL_EMMC0_CMD_CONN_EMMC0_CMD	0x00000021
> > +			IMX8DXL_EMMC0_DATA0_CONN_EMMC0_DATA0	0x00000021
> > +			IMX8DXL_EMMC0_DATA1_CONN_EMMC0_DATA1	0x00000021
> > +			IMX8DXL_EMMC0_DATA2_CONN_EMMC0_DATA2	0x00000021
> > +			IMX8DXL_EMMC0_DATA3_CONN_EMMC0_DATA3	0x00000021
> > +			IMX8DXL_EMMC0_DATA4_CONN_EMMC0_DATA4	0x00000021
> > +			IMX8DXL_EMMC0_DATA5_CONN_EMMC0_DATA5	0x00000021
> > +			IMX8DXL_EMMC0_DATA6_CONN_EMMC0_DATA6	0x00000021
> > +			IMX8DXL_EMMC0_DATA7_CONN_EMMC0_DATA7	0x00000021
> > +			IMX8DXL_EMMC0_STROBE_CONN_EMMC0_STROBE	0x00000041
> > +		>;
> > +	};
> > +
> > +	pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
> 
> For sake of consistency, we probably should still end the node name with 'grp'.
> 

I think we should either leave as is or use usdhc1-100mhz-grp.

I, for one, would leave as is and then maybe we can do a replace for all imx
platforms as a separate patch at some point.

Let me know what you would prefer.

All other comments will be addressed in the next version of this
patchset.

> Shawn
> 
> > +		fsl,pins = <
> > +			IMX8DXL_EMMC0_CLK_CONN_EMMC0_CLK	0x06000041
> > +			IMX8DXL_EMMC0_CMD_CONN_EMMC0_CMD	0x00000021
> > +			IMX8DXL_EMMC0_DATA0_CONN_EMMC0_DATA0	0x00000021
> > +			IMX8DXL_EMMC0_DATA1_CONN_EMMC0_DATA1	0x00000021
> > +			IMX8DXL_EMMC0_DATA2_CONN_EMMC0_DATA2	0x00000021
> > +			IMX8DXL_EMMC0_DATA3_CONN_EMMC0_DATA3	0x00000021
> > +			IMX8DXL_EMMC0_DATA4_CONN_EMMC0_DATA4	0x00000021
> > +			IMX8DXL_EMMC0_DATA5_CONN_EMMC0_DATA5	0x00000021
> > +			IMX8DXL_EMMC0_DATA6_CONN_EMMC0_DATA6	0x00000021
> > +			IMX8DXL_EMMC0_DATA7_CONN_EMMC0_DATA7	0x00000021
> > +			IMX8DXL_EMMC0_STROBE_CONN_EMMC0_STROBE	0x00000041
> > +		>;
> > +	};
> > +

 ...

> > +	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
> > +		fsl,pins = <
> > +			IMX8DXL_ENET0_RGMII_RXC_CONN_USDHC1_CLK		0x06000041
> > +			IMX8DXL_ENET0_RGMII_RX_CTL_CONN_USDHC1_CMD	0x00000021
> > +			IMX8DXL_ENET0_RGMII_RXD0_CONN_USDHC1_DATA0	0x00000021
> > +			IMX8DXL_ENET0_RGMII_RXD1_CONN_USDHC1_DATA1	0x00000021
> > +			IMX8DXL_ENET0_RGMII_RXD2_CONN_USDHC1_DATA2	0x00000021
> > +			IMX8DXL_ENET0_RGMII_RXD3_CONN_USDHC1_DATA3	0x00000021
> > +			IMX8DXL_ENET0_RGMII_TXD0_CONN_USDHC1_VSELECT	0x00000021
> > +		>;
> > +	};
> > +};
> > -- 
> > 2.31.1
> >