Re: [PATCH v2 6/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini EDIMM2.2 Starter Kit

From: Jagan Teki <jagan@amarulasolutions.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Li Yang <leoyang.li@nxp.com>, Fabio Estevam <festevam@gmail.com>,
	devicetree <devicetree@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	NXP Linux Team <linux-imx@nxp.com>,
	linux-amarula <linux-amarula@amarulasolutions.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Matteo Lisi <matteo.lisi@engicam.com>
Subject: Re: [PATCH v2 6/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini EDIMM2.2 Starter Kit
Date: Tue, 22 Dec 2020 01:03:07 +0530	[thread overview]
Message-ID: <CAMty3ZDWoorJ6y2zATAyd10TqbOAcjMdAkdini5tKt1euY0_7Q@mail.gmail.com> (raw)
In-Reply-To: <20201221140642.GF31176@kozik-lap>

On Mon, Dec 21, 2020 at 7:36 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Dec 21, 2020 at 05:01:51PM +0530, Jagan Teki wrote:
> > Engicam EDIMM2.2 Starter Kit is an EDIMM 2.2 Form Factor Capacitive
> > Evaluation Board.
> >
> > Genaral features:
> > - LCD 7" C.Touch
> > - microSD slot
> > - Ethernet 1Gb
> > - Wifi/BT
> > - 2x LVDS Full HD interfaces
> > - 3x USB 2.0
> > - 1x USB 3.0
> > - HDMI Out
> > - Mini PCIe
> > - MIPI CSI
> > - 2x CAN
> > - Audio Out
> >
> > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from Engicam.
> >
> > i.Core MX8M Mini needs to mount on top of this Evaluation board for
> > creating complete i.Core MX8M Mini EDIMM2.2 Starter Kit.
> >
> > PCIe, DSI, CSI nodes will add it into imx8mm-engicam-edimm2.2.dtsi once
> > Mainline Linux supported.
> >
> > Add support for it.
> >
> > Signed-off-by: Matteo Lisi <matteo.lisi@engicam.com>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v2:
> > - updated commit message
> > - dropped engicam from filename since it aligned with imx6 engicam
> >   dts files naming conventions.
> >
> >  arch/arm64/boot/dts/freescale/Makefile        |  1 +
> >  .../freescale/imx8mm-engicam-edimm2.2.dtsi    |  7 +++++++
> >  .../freescale/imx8mm-icore-mx8mm-edimm2.2.dts | 21 +++++++++++++++++++
> >  3 files changed, 29 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.dts
> >
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index 8d49a2c74604..43783076f856 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -33,6 +33,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-ctouch2.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-edimm2.2.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-kontron-n801x-s.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi
> > new file mode 100644
> > index 000000000000..294df07289a2
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi
> > @@ -0,0 +1,7 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2020 Engicam srl
> > + * Copyright (c) 2020 Amarula Solutions(India)
> > + */
> > +
> > +#include "imx8mm-engicam-common.dtsi"
>
> It seems you ignored my comments from previous email. That's not how we
> go with the process.
>
> Don't create confusing or overcomplicated hierarchy of includes. Don't
> create files which do nothing.

Idea is to move common nodes in separate dtsi instead of adding
redundant nodes into respective areas. let me know if it still
confusing.

Jagan.