MIME-Version: 1.0
References: <20230215113249.47727-1-william.qiu@starfivetech.com>
 <20230215113249.47727-4-william.qiu@starfivetech.com> <CAJM55Z8gVEZS4Ws2Gi7_JbdkS-4y3_8mQvR4ZxLCWZ4A1y9X1g@mail.gmail.com>
In-Reply-To: <CAJM55Z8gVEZS4Ws2Gi7_JbdkS-4y3_8mQvR4ZxLCWZ4A1y9X1g@mail.gmail.com>
From:   Emil Renner Berthing <emil.renner.berthing@canonical.com>
Date:   Wed, 15 Feb 2023 13:22:00 +0100
Message-ID: <CAJM55Z-h+CUmWtkn31Ek+qvxrOr5_Jz3QRRLqWYLz2A0E+h+rA@mail.gmail.com>
Subject: Re: [PATCH v4 3/4] riscv: dts: starfive: Add mmc node
To:     William Qiu <william.qiu@starfivetech.com>
Cc:     linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
        linux-mmc@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Jaehoon Chung <jh80.chung@samsung.com>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 15 Feb 2023 at 13:12, Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:
>
> On Wed, 15 Feb 2023 at 12:35, William Qiu <william.qiu@starfivetech.com> wrote:
> >
> > Add the mmc node for the StarFive JH7110 SoC.
> > Set mmco node to emmc and set mmc1 node to sd.
> >
> > Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> > ---
> >  .../jh7110-starfive-visionfive-2.dtsi         | 23 +++++++++
> >  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 47 +++++++++++++++++++
> >  2 files changed, 70 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > index c60280b89c73..e1a0248e907f 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > @@ -42,6 +42,29 @@ &rtc_osc {
> >         clock-frequency = <32768>;
> >  };
> >
> > +&mmc0 {
> > +       max-frequency = <100000000>;
> > +       bus-width = <8>;
> > +       cap-mmc-highspeed;
> > +       mmc-ddr-1_8v;
> > +       mmc-hs200-1_8v;
> > +       non-removable;
> > +       cap-mmc-hw-reset;
> > +       post-power-on-delay-ms = <200>;
> > +       status = "okay";
> > +};
> > +
> > +&mmc1 {
> > +       max-frequency = <100000000>;
> > +       bus-width = <4>;
> > +       no-sdio;
> > +       no-mmc;
> > +       broken-cd;
> > +       cap-sd-highspeed;
> > +       post-power-on-delay-ms = <200>;
> > +       status = "okay";
> > +};

These nodes are also still oddly placed in the middle of the external
clocks. Again please keep the external clocks at the top and then
order the nodes alphabetically to have some sort of system.

> >  &gmac0_rmii_refin {
> >         clock-frequency = <50000000>;
> >  };
> > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > index 64d260ea1f29..17f7b3ee6ca3 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > @@ -314,6 +314,11 @@ uart2: serial@10020000 {
> >                         status = "disabled";
> >                 };
> >
> > +               stg_syscon: syscon@10240000 {
> > +                       compatible = "starfive,jh7110-stg-syscon", "syscon";
> > +                       reg = <0x0 0x10240000 0x0 0x1000>;
> > +               };
> > +
> >                 uart3: serial@12000000 {
> >                         compatible = "snps,dw-apb-uart";
> >                         reg = <0x0 0x12000000 0x0 0x10000>;
> > @@ -370,6 +375,11 @@ syscrg: clock-controller@13020000 {
> >                         #reset-cells = <1>;
> >                 };
> >
> > +               sys_syscon: syscon@13030000 {
> > +                       compatible = "starfive,jh7110-sys-syscon", "syscon";
> > +                       reg = <0x0 0x13030000 0x0 0x1000>;
> > +               };
> > +
> >                 gpio: gpio@13040000 {
> >                         compatible = "starfive,jh7110-sys-pinctrl";
> >                         reg = <0x0 0x13040000 0x0 0x10000>;
> > @@ -397,6 +407,11 @@ aoncrg: clock-controller@17000000 {
> >                         #reset-cells = <1>;
> >                 };
> >
> > +               aon_syscon: syscon@17010000 {
> > +                       compatible = "starfive,jh7110-aon-syscon", "syscon";
> > +                       reg = <0x0 0x17010000 0x0 0x1000>;
> > +               };
> > +
> >                 gpioa: gpio@17020000 {
> >                         compatible = "starfive,jh7110-aon-pinctrl";
> >                         reg = <0x0 0x17020000 0x0 0x10000>;
> > @@ -407,5 +422,37 @@ gpioa: gpio@17020000 {
> >                         gpio-controller;
> >                         #gpio-cells = <2>;
> >                 };
> > +
> > +               mmc0: mmc@16010000 {
> > +                       compatible = "starfive,jh7110-mmc";
> > +                       reg = <0x0 0x16010000 0x0 0x10000>;
> > +                       clocks = <&syscrg JH7110_SYSCLK_SDIO0_AHB>,
> > +                                <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>;
> > +                       clock-names = "biu","ciu";
> > +                       resets = <&syscrg JH7110_SYSRST_SDIO0_AHB>;
> > +                       reset-names = "reset";
> > +                       interrupts = <74>;
> > +                       fifo-depth = <32>;
> > +                       fifo-watermark-aligned;
> > +                       data-addr = <0>;
> > +                       starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
> > +                       status = "disabled";
> > +               };
> > +
> > +               mmc1: mmc@16020000 {
> > +                       compatible = "starfive,jh7110-mmc";
> > +                       reg = <0x0 0x16020000 0x0 0x10000>;
> > +                       clocks = <&syscrg JH7110_SYSCLK_SDIO1_AHB>,
> > +                                <&syscrg JH7110_SYSCLK_SDIO1_SDCARD>;
> > +                       clock-names = "biu","ciu";
> > +                       resets = <&syscrg JH7110_SYSRST_SDIO1_AHB>;
> > +                       reset-names = "reset";
> > +                       interrupts = <75>;
> > +                       fifo-depth = <32>;
> > +                       fifo-watermark-aligned;
> > +                       data-addr = <0>;
> > +                       starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>;
> > +                       status = "disabled";
> > +               };
>
> Hi William,
>
> These nodes still don't seem to be sorted by address, eg. by the
> number after the @
> Also please move the dt-binding patch before this one, so dtb_check
> won't fail no matter where git bisect happens to land.
>
> /Emil
>
> >         };
> >  };
> > --
> > 2.34.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv