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

Message ID 20201221113151.94515-7-jagan@amarulasolutions.com
State New
Headers show
Series
  • arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini
Related show

Commit Message

Jagan Teki Dec. 21, 2020, 11:31 a.m. UTC
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

Comments

Krzysztof Kozlowski Dec. 21, 2020, 2:06 p.m. UTC | #1
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.

Best regards,
Krzysztof
Jagan Teki Dec. 21, 2020, 7:33 p.m. UTC | #2
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.
Krzysztof Kozlowski Dec. 21, 2020, 9:06 p.m. UTC | #3
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@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.

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?

Best regards,
Krzysztof
Jagan Teki Dec. 22, 2020, 8:50 a.m. UTC | #4
On Tue, Dec 22, 2020 at 2:36 AM Krzysztof Kozlowski <krzk@kernel.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@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.
>
> 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 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.

Hope this would clear.

Jagan.
Krzysztof Kozlowski Dec. 22, 2020, 8:53 a.m. UTC | #5
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@kernel.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@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.
> >
> > 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.

Best regards,
Krzysztof
Jagan Teki Dec. 22, 2020, 9:05 a.m. UTC | #6
On Tue, Dec 22, 2020 at 2:23 PM Krzysztof Kozlowski <krzk@kernel.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@kernel.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@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.
> > >
> > > 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.

Patch

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"
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.dts b/arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.dts
new file mode 100644
index 000000000000..672aee1800c4
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.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"
+#include "imx8mm-engicam-edimm2.2.dtsi"
+#include "imx8mm-icore-mx8mm.dtsi"
+
+/ {
+	model = "Engicam i.Core MX8M Mini EDIMM2.2 Starter Kit";
+	compatible = "engicam,icore-mx8mm-edimm2.2", "engicam,icore-mx8mm",
+		     "fsl,imx8mm";
+
+	chosen {
+		stdout-path = &uart2;
+	};
+};