Message-ID: <CAMty3ZA58bHw3cL3de1RV2J+D_0ZemuMJYFg1RYmT_nPh0gspQ@mail.gmail.com>
Date:   Tue, 22 Dec 2020 14:35:55 +0530
From:   Jagan Teki <jagan@...rulasolutions.com>
To:     Krzysztof Kozlowski <krzk@...nel.org>
Cc:     Rob Herring <robh+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
        Li Yang <leoyang.li@....com>,
        Fabio Estevam <festevam@...il.com>,
        devicetree <devicetree@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        NXP Linux Team <linux-imx@....com>,
        linux-amarula <linux-amarula@...rulasolutions.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Matteo Lisi <matteo.lisi@...icam.com>
Subject: Re: [PATCH v2 6/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini
 EDIMM2.2 Starter Kit

On Tue, Dec 22, 2020 at 2:23 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On Tue, Dec 22, 2020 at 02:20:55PM +0530, Jagan Teki wrote:
> > On Tue, Dec 22, 2020 at 2:36 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
> > >
> > > On Tue, Dec 22, 2020 at 01:03:07AM +0530, Jagan Teki wrote:
> > > > On Mon, Dec 21, 2020 at 7:36 PM Krzysztof Kozlowski <krzk@...nel.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@...icam.com>
> > > > > > Signed-off-by: Jagan Teki <jagan@...rulasolutions.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.
> > >
> > > A file which *only* includes another file does not fulfill this idea of
> > > moving common nodes to a separate DTSI file. Or if I still miss
> > > something, please point me, what common nodes are stored in
> > > imx8mm-engicam-edimm2.2.dtsi?
> >
> > imx8mm-engicam-edimm2.2.dtsi for EDIMM2.2 Carrier
> > imx8mm-engicam-ctouch2.dtsi for C.TOUCH2 Carrier
> > imx8mm-engicam-common.dtsi for common nodes for above 2 carrier boards.
> >
> > Yes, imx8mm-engicam-edimm2.2.dtsi is empty now
>
> Then that's the answer. We do not create empty files.
>
> > but nodes like PCIe,
> > CSI, DSI will support once the respective drivers are part of Mainline
> > but those are not supported in C.TOUCH2 carrier board dtsi. There are
> > some GPIO pins differences between EDIMM2.2 and C.TOUCH2 carriers on
> > WiFi/BT so those will be part of the respective carrier dtsi.
>
> It's the same clear as before. Don't create empty files. Once you decide
> to bring new features, you create a new file.

Okay, then will add w/o common file in next version.

Jagan.

Powered by blists - more mailing lists