linux-kernel - Re: [PATCH v2 2/3] arm64: dts: imx: Add i.mx8mm nitrogen8mm basic dts support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CABkfQAFN5u8wexHHCs-umZj3o2dMBQB4_+8OVAeGe0OvV12g5Q@mail.gmail.com>
Date:   Wed, 13 Jan 2021 18:02:01 +0100
From:   Adrien Grassein <adrien.grassein@...il.com>
To:     Krzysztof Kozlowski <krzk@...nel.org>
Cc:     Rob Herring <robh+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Sascha Hauer <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        DTML <devicetree@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 2/3] arm64: dts: imx: Add i.mx8mm nitrogen8mm basic dts support

Le mer. 13 janv. 2021 à 17:46, Krzysztof Kozlowski <krzk@...nel.org> a écrit :
>
> On Wed, Jan 13, 2021 at 03:34:42PM +0100, Adrien Grassein wrote:
> > Tested with a basic Build Root configuration booting from sdcard.
> >
> > Signed-off-by: Adrien Grassein <adrien.grassein@...il.com>
> > ---
> >  arch/arm64/boot/dts/freescale/Makefile        |   1 +
> >  .../dts/freescale/imx8mm-nitrogen8mm_rev2.dts | 364 ++++++++++++++++++
> >  2 files changed, 365 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-nitrogen8mm_rev2.dts
> >
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index 901d80086b47..b2eb7a5e4db3 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -45,6 +45,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-r2.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-r3.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-nitrogen8mm_rev2.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-phanbell.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-pico-pi.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-thor96.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-nitrogen8mm_rev2.dts b/arch/arm64/boot/dts/freescale/imx8mm-nitrogen8mm_rev2.dts
> > new file mode 100644
> > index 000000000000..506e467ebf16
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-nitrogen8mm_rev2.dts
> > @@ -0,0 +1,364 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Device Tree file for Boundary Devices i.MX8MMini Nitrogen8MM Rev2 board.
> > + * Adrien Grassein <adrien.grassein@...il.com.com>
> > + */
> > +/dts-v1/;
> > +#include "imx8mm.dtsi"
> > +
> > +/ {
> > +     model = "Boundary Devices i.MX8MMini Nitrogen8MM Rev2";
> > +     compatible = "boundary,imx8mm-nitrogen8mm", "fsl,imx8mm";
> > +
> > +     reg_vref_1v8: regulator-vref-1v8 {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "vref-1v8";
> > +             regulator-min-microvolt = <1800000>;
> > +             regulator-max-microvolt = <1800000>;
> > +     };
> > +
> > +     reg_vref_3v3: regulator-vref-3v3 {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "vref-3v3";
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +     };
>
> Why do you need these two regulators? They don't do anything, there is
> no control over them.
>

Sorry, I guess it was usefull for the eMMc, but after a test, it xas not.
It will be removed in v3.

> > +};
> > +
> > +&A53_0 {
> > +     cpu-supply = <&reg_sw3>;
> > +};
> > +
> > +&A53_1 {
> > +     cpu-supply = <&reg_sw3>;
> > +};
> > +
> > +&A53_2 {
> > +     cpu-supply = <&reg_sw3>;
> > +};
> > +
> > +&A53_3 {
> > +     cpu-supply = <&reg_sw3>;
> > +};
> > +
> > +&i2c1 {
> > +     clock-frequency = <400000>;
> > +     pinctrl-names = "default", "gpio";
> > +     pinctrl-0 = <&pinctrl_i2c1>;
> > +     pinctrl-1 = <&pinctrl_i2c1_1>;
> > +     scl-gpios = <&gpio5 14 GPIO_OPEN_DRAIN>;
> > +     sda-gpios = <&gpio5 15 GPIO_OPEN_DRAIN>;
> > +     status = "okay";
> > +
> > +     pmic@8 {
> > +             compatible = "nxp,pf8121a";
> > +             reg = <0x8>;
> > +             status = "okay";
> > +
> > +             regulators {
> > +                     reg_ldo2: ldo2 {
>
> The PMIC regulators should be described fully, so bring them back from
> the v1.
>
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-max-microvolt = <5000000>;
> > +                             regulator-min-microvolt = <1500000>;
> > +                     };
> > +
> > +                     reg_sw3: buck3 {
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-max-microvolt = <1800000>;
> > +                             regulator-min-microvolt =  <400000>;
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> > +&fec1 {
>
> Please order the overridden nodes alphabetically, with iomux exception
> which goes to the end (by convention in NXP). You define i2c1, then fec1
> and then i2c3. Should be fec1, i2c1 and then i2c3.
>
> Best regards,
> Krzysztof


Thanks a lot for you reviews,
Best regards,
Adrien