Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201223110343.126638-1-jagan@amarulasolutions.com>
 <20201223110343.126638-5-jagan@amarulasolutions.com> <20201223115933.GC21091@kozik-lap>
 <CAMty3ZA0zrHS_wVKm8gdqU1q5ONokoT7aG_MBcWtXTJK59Bozg@mail.gmail.com>
 <CAJKOXPfNP+6uv6PXmyTCimRd1hrQUbosnY--55yUQh4G4jfZWw@mail.gmail.com>
 <CAMty3ZBWteW2OZMNwFT5hSRegy-AARqqkR7SAz53fDoM3j5Ong@mail.gmail.com>
 <CAJKOXPey8+k7_uT4LLTwo8-UY51+TwUgWhtkPDRW3AD-BCi1Bw@mail.gmail.com> <CAMty3ZAo+9L4rtkJgBkimJu4SPv=gycVUFaUYtL_RSAhs7yGWw@mail.gmail.com>
In-Reply-To: <CAMty3ZAo+9L4rtkJgBkimJu4SPv=gycVUFaUYtL_RSAhs7yGWw@mail.gmail.com>
From:   Krzysztof Kozlowski <krzk@kernel.org>
Date:   Mon, 28 Dec 2020 09:34:16 +0100
Message-ID: <CAJKOXPdwHGmi+2-w4ymCN6D6RbBrfwk0imiHwXHTQ6nXR=x5yA@mail.gmail.com>
Subject: Re: [PATCH v3 4/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini
 C.TOUCH 2.0
To:     Jagan Teki <jagan@amarulasolutions.com>
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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, 28 Dec 2020 at 09:21, Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > #include "imx8mm.dtsi"
> > > #include "imx8mm-beacon-som.dtsi"
> > > #include "imx8mm-beacon-baseboard.dtsi"
> > >
> > > (SoC dtsi, SoM dtsi, Carrier board dtsi)
> > >
> > > > design which makes any sense. We do not create empty DTS files which
> > > > only include one more DTSI. The contents of
> > > > imx8mm-engicam-ctouch2.dtsi should be directly in
> > > > imx8mm-icore-mx8mm-ctouch2.dts. That's the same problem as with v1 -
> > > > you overcomplicate simple stuff. It really looks like you ignored the
> > > > comments from v1 in multiple places.
> > >
> > > As explained above, the design is pretty much the same as the existing SoM's.
> > >
> > > imx8mm-engicam-ctouch2.dtsi is not just a dtsi file where nodes are
> > > enabled. It has nodes enabled for Carrier board, so keeping nodes
> > > separately will
> >
> > The files represent real devices or their components. So you have a
> > SOM - a DTSI file. You have a carrier board - a DTS file. That's
> > simple design which is mostly followed, unless something over
> > complicated passes the review.
> >
> > > 1. More verbose for which IP's are available in the carrier board
> >
> > No difference when carrier DTSI is the DTS. Exactly the same.
> >
> > > 2. Easy to extend if someone can create another SoM with a similar Carrier.
> >
> > Not really, if they include carrier DTSI they need to override a lot.
> > So usually (including practice - I did it) they *copy* the carrier to
> > create their own design.
>
> But what if the new board has slite change to use exiting carrier like
> what ctouch2 10" OF. Can we add ctouch2 dtsi as a separate file for
> this case?

If you submit another DTS using the imx8mm-engicam-ctouch2.dtsi - with
its own differences of course (not copying other DTS...) - then having
a DTSI makes sense. In current form, still NAK for all the reasons I
explained more than once.

Best regards,
Krzysztof