[PATCH 08/10] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini C.TOUCH 2.0

[Jagan Teki]

On Thu, Dec 3, 2020 at 12:55 AM Krzysztof Kozlowski <krzk at kernel.org> wrote:
>
> On Thu, Dec 03, 2020 at 12:50:37AM +0530, Jagan Teki wrote:
> > Hi Krzysztof,
> >
> > On Wed, Dec 2, 2020 at 11:15 PM Krzysztof Kozlowski <krzk at kernel.org> wrote:
> > >
> > > On Wed, Dec 02, 2020 at 05:42:39PM +0530, Jagan Teki wrote:
> > > > i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM from Engicam.
> > > >
> > > > C.TOUCH 2.0 is a general purpose carrier board with capacitive
> > > > touch interface support.
> > > >
> > > > i.Core MX8M Mini needs to mount on top of this Carrier board for
> > > > creating complete i.Core MX8M Mini C.TOUCH 2.0 board.
> > > >
> > > > Add support for it.
> > > >
> > > > Signed-off-by: Matteo Lisi <matteo.lisi at engicam.com>
> > > > Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> > > > ---
> > > >  arch/arm64/boot/dts/freescale/Makefile        |  1 +
> > > >  .../imx8mm-engicam-icore-mx8mm-ctouch2.dts    | 21 +++++++++++++++++++
> > > >  2 files changed, 22 insertions(+)
> > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts
> > > >
> > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > > > index 4369d783dade..8191db4c64fa 100644
> > > > --- a/arch/arm64/boot/dts/freescale/Makefile
> > > > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > > > @@ -30,6 +30,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-rdb.dtb
> > > >  dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2162a-qds.dtb
> > > >
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb
> > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-ctouch2.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-edimm2.2.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts
> > > > new file mode 100644
> > > > index 000000000000..aa3c03ad3109
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts
> > > > @@ -0,0 +1,21 @@
> > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > > +/*
> > > > + * Copyright (c) 2019 NXP
> > > > + * Copyright (c) 2019 Engicam srl
> > > > + * Copyright (c) 2020 Amarula Solutions(India)
> > > > + */
> > > > +
> > > > +/dts-v1/;
> > > > +#include "imx8mm.dtsi"
> > >
> > > You have multiple DTSI files to only include one DTSI. I was trying to
> > > follow the logic here but I failed...
> > >
> > > This is ctouch, so it should include SoM, which you call icore. But it
> > > also includes ctouch2 which *only* includes common DTSI. It's then
> > > exactly the same as starter kit which includes edimm (which includes
> > > common) and icore.
> >
> > I hope you have checked the cover letter where I have mentioned all
> > the combinations.
> >
> > 1. SoM, Starter Kit, Carrier Board, Open Frame are three different hardware.
> >
> > 2. i.Core MX8M Mini is SoM
> >
> > 3. EDIMM 2.2 is Starter Kit
> >
> > 4. C.TOUCH 2.0 is Carrier board
> >
> > 5. 10"1 Open Frame board for LVDS
> >
> > The combination of respective hardware mounting is,
> >
> > 1. SOM+Starter Kitt => i.Core MX8M Mini EDIMM 2.2 Starter Kit
> >
> > 2. SOM+C.TOUCH 2.0 => i.Core MX8M Mini C.TOUCH 2.0 Carrier board
> >
> > 3. SOM+C.TOUCH 2.0+10.1" OF => i.Core MX8M Mini C.TOUCH 2.0 10.1" Open
> > Frame board
>
> It does not explain why you created 3 empty DTSI and 2 empty DTS files.
>
> >
> > About the bindings, (please check the
> > arch/arm64/boot/dts/rockchip/px30-engicam-*), It's been discussed
> > before with Rob for these boards bindings.
>
> Refer to my specific comments about bindings.
>
> >
> > To, compare with what we have described with rockchip
> >
> > SoM binding,
> > - engicam,icore-mx8mm is binding for i.Core MX8M Mini SoM
> > - engicam,px30-core is binding for PX30.Core SoM
> >
> > EDIMM 2.2 is Starter Kit binding,
> > - engicam,icore-mx8mm-edimm2.2 is binding for EDIMM 2.2 is Starter Kit
> > in i.MX8MM
> > - engicam,px30-core-edimm2.2 is binding for EDIMM 2.2 is Starter Kit in PX30
> >
> > C.TOUCH 2.0 is Carrier board binding,
> > - engicam,icore-mx8mm-ctouch2 is binding for C.TOUCH 2.0 is Carrier
> > board in i.MX8MM
> > - engicam,px30-core-ctouch2 is binding for C.TOUCH 2.0 is Carrier board in PX30
> >
> > C.TOUCH 2.0 10"1 OF binding,
> > - engicam,icore-mx8mm-ctouch2-of10 is binding for C.TOUCH 2.0 10"1 in imx8MM
> > - engicam,px30-core-ctouch2-of10 for C.TOUCH 2.0 10"1 in PX30
> >
> > So, there are 3 board combinations of which each board has a binding
> > of SoM and respective carrier binding like i.Core MX8M Mini EDIMM 2.2
> > Starter Kit has
> > "engicam,icore-mx8mm-edimm2.2", "engicam,icore-mx8mm"
> > "engicam,icore-mx8mm-ctouch2", "engicam,icore-mx8mm"
> > "engicam,icore-mx8mm-ctouch2-of10", "engicam,icore-mx8mm"
> >
> > Some of the DTS files are using the engicam-common.dtsi nodes and for
> > ie reason those are empty and some need to have lvds display node
> > which is still underworking.
>
> Therefore add them when you have any contents for these DTS files.
>
> >
> > Hope this information helpful. Let me know for further inputs.
>
> Thanks for the information but it was not much helpful. It does not
> answer at all why you have so many empty files, why you include
> imx8mm.dtsi not in the SoM but somewhere else.

Sorry, I have missed it.

All these three carrier board dtsi files,

imx8mm-engicam-edimm2.2.dtsi
imx8mm-engicam-icore-mx8mm-ctouch2.dts
imx8mm-engicam-icore-mx8mm-ctouch2-of10.dts

are included imx8mm-engicam-common.dtsi. ie is the reason these are empty.

And I agree with your point of adding whenever it ready. I will drop
display related carrier imx8mm-engicam-icore-mx8mm-ctouch2-of10.dts
when DSI, LVDS ready, and update on next versions.

Thanks for the review.

Jagan.