lkml.org 
[lkml]   [2020]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 04/10] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini SOM
    Hi Krzysztof,

    On Wed, Dec 2, 2020 at 11:04 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
    >
    > On Wed, Dec 02, 2020 at 05:42:35PM +0530, Jagan Teki wrote:
    > > i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM from Engicam.
    >
    > s/SOM/SoM/
    >
    > >
    > > General features:
    > > - NXP i.MX8MM
    >
    > i.MX 8M Mini
    > as named by NXP:
    > https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/i-mx-applications-processors/i-mx-8-processors/i-mx-8m-mini-arm-cortex-a53-cortex-m4-audio-voice-video:i.MX8MMINI
    >
    > > - Up to 2GB LDDR4
    > > - 8/16GB eMMC
    > > - Gigabit Ethernet
    > > - USB 2.0 Host/OTG
    > > - PCIe Gen2 interface
    > > - I2S
    > > - MIPI DSI to LVDS
    > > - rest of i.MX8MM features
    >
    > Ditto
    >
    > >
    > > i.Core MX8M Mini needs to mount on top of Engicam baseboards for
    > > creating complete platform boards.
    > >
    > > Possible baseboards are,
    > > - EDIMM2.2
    > > - C.TOUCH 2.0
    >
    > Don't describe baseboards. You add here only SoM.

    It's just information on how this SoM is being used. Let me know any
    issues while explaining the combinations being used here.

    >
    > >
    > > Add support for it.
    > >
    > > Signed-off-by: Matteo Lisi <matteo.lisi@engicam.com>
    > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
    > > ---
    > > .../freescale/imx8mm-engicam-icore-mx8mm.dtsi | 209 ++++++++++++++++++
    > > 1 file changed, 209 insertions(+)
    > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm.dtsi
    > >
    > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm.dtsi
    > > new file mode 100644
    > > index 000000000000..b87917c40587
    > > --- /dev/null
    > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm.dtsi
    > > @@ -0,0 +1,209 @@
    > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
    > > +/*
    > > + * Copyright (c) 2018 NXP
    > > + * Copyright (c) 2019 Engicam srl
    > > + * Copyright (c) 2020 Amarula Solutons(India)
    > > + */
    > > +
    > > +/ {
    >
    > Missing "model".

    SoM dtsi won't comprise the model it has its own binding and while
    combination board has a model that will include this binding. Please
    check to exist binding file.
    arch/arm64/boot/dts/rockchip/px30-engicam-px30-core.dtsi

    >
    > > + compatible = "engicam,icore-mx8mm", "fsl,imx8mm";
    > > +};
    > > +
    >
    > No memory node? Isn't the memory a property of SoM?
    >
    > > +&A53_0 {
    > > + cpu-supply = <&reg_buck4>;
    > > +};
    >
    > Supplies for the other cores.
    >
    > > +
    > > +&i2c1 {
    > > + clock-frequency = <400000>;
    > > + pinctrl-names = "default";
    > > + pinctrl-0 = <&pinctrl_i2c1>;
    > > + status = "okay";
    > > +
    > > + pf8100@8 {
    >
    > Node name should describe generic class of a device, so probably you
    > wanted here "pmic".

    True, will update.

    >
    > > + compatible = "nxp,pf8x00";
    > > + reg = <0x08>;
    > > +
    > > + regulators {
    > > + reg_ldo1: ldo1 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    >
    > First min/max constraints. Then always-on and boot-on properties.
    >
    > > + regulator-max-microvolt = <5000000>;
    > > + regulator-min-microvolt = <1500000>;
    > > + };
    > > +
    > > + reg_ldo2: ldo2 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <5000000>;
    > > + regulator-min-microvolt = <1500000>;
    > > + };
    > > +
    > > + reg_ldo3: ldo3 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <5000000>;
    > > + regulator-min-microvolt = <1500000>;
    > > + };
    > > +
    > > + reg_ldo4: ldo4 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <5000000>;
    > > + regulator-min-microvolt = <1500000>;
    > > + };
    > > +
    > > + reg_buck1: buck1 {
    > > + fsl,ilim-ma = <4500>;
    >
    > Where is this property documented?

    Sorry it would be nxp,ilim-ma I will update in next version.

    >
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <1800000>;
    > > + regulator-min-microvolt = <400000>;
    > > + };
    > > +
    > > + reg_buck2: buck2 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <1800000>;
    > > + regulator-min-microvolt = <400000>;
    > > + };
    > > +
    > > + reg_buck3: buck3 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <1800000>;
    > > + regulator-min-microvolt = <400000>;
    > > + };
    > > +
    > > + reg_buck4: buck4 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <1800000>;
    > > + regulator-min-microvolt = <400000>;
    > > + fast-slew = <1>;
    >
    > Where is this property documented?
    >
    > > + };
    > > +
    > > + reg_buck5: buck5 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <1800000>;
    > > + regulator-min-microvolt = <400000>;
    > > + };
    > > +
    > > + reg_buck6: buck6 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <1800000>;
    > > + regulator-min-microvolt = <400000>;
    > > + };
    > > +
    > > + reg_buck7: buck7 {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <3300000>;
    > > + regulator-min-microvolt = <3300000>;
    > > + };
    > > +
    > > + reg_vsnvs: vsnvs {
    > > + regulator-always-on;
    > > + regulator-boot-on;
    > > + regulator-max-microvolt = <3300000>;
    > > + regulator-min-microvolt = <1800000>;
    > > + };
    > > + };
    > > + };
    > > +};
    > > +
    > > +&iomuxc {
    > > + pinctrl_i2c1: i2c1grp {
    > > + fsl,pins = <
    > > + MX8MM_IOMUXC_I2C1_SCL_I2C1_SCL 0x400001c3
    > > + MX8MM_IOMUXC_I2C1_SDA_I2C1_SDA 0x400001c3
    > > + >;
    > > + };
    > > +
    > > + pinctrl_uart2: uart2grp {
    >
    > Not used.
    >
    > > + fsl,pins = <
    > > + MX8MM_IOMUXC_UART2_RXD_UART2_DCE_RX 0x140
    > > + MX8MM_IOMUXC_UART2_TXD_UART2_DCE_TX 0x140
    > > + >;
    > > + };
    > > +
    > > + pinctrl_usdhc1_gpio: usdhc1grpgpio {
    >
    > This should complain on bindings check. Please run dtbs_check. The "grp"
    > should be a suffix in node name, so "usdhc1gpiogrp".

    Yes, will update it on the next version.

    Jagan.

    \
     
     \ /
      Last update: 2020-12-02 20:35    [W:2.953 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site