Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230418090312.2818879-3-sergio.paracuellos@gmail.com> <TYAP286MB0315C35BA080B686EB5EA980BC59A@TYAP286MB0315.JPNP286.PROD.OUTLOOK.COM>
In-Reply-To: <TYAP286MB0315C35BA080B686EB5EA980BC59A@TYAP286MB0315.JPNP286.PROD.OUTLOOK.COM>
From:   Sergio Paracuellos <sergio.paracuellos@gmail.com>
Date:   Sat, 17 Jun 2023 07:06:15 +0200
Message-ID: <CAMhs-H_GjHU+r33oeytXiyu+bgGTrGL8Ck=DbsMYnqC-XBNYYw@mail.gmail.com>
Subject: Re: [PATCH v3 2/9] clk: ralink: add clock and reset driver for MTMIPS SoCs
To:     yangshiji66@outlook.com
Cc:     arinc.unal@arinc9.com, devicetree@vger.kernel.org,
        john@phrozen.org, krzysztof.kozlowski+dt@linaro.org,
        linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-mips@vger.kernel.org, matthias.bgg@gmail.com,
        mturquette@baylibre.com, p.zabel@pengutronix.de,
        robh+dt@kernel.org, sboyd@kernel.org, tsbogend@alpha.franken.de
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Shiji Yang!

On Sat, Jun 17, 2023 at 6:51=E2=80=AFAM <yangshiji66@outlook.com> wrote:
>
> Thank you for taking care of these old amazing SoCs. When I ported these
> patches to OpenWrt, I found some issues.

Thanks for testing this!

>
> >+
> >+static const struct of_device_id mtmips_of_match[] =3D {
> >+      {
> >+              .compatible =3D "ralink,rt2880-sysc",
> >+              .data =3D &rt2880_clk_data,
> >+      },
> >+      {
> >+              .compatible =3D "ralink,rt3050-sysc",
> >+              .data =3D &rt3050_clk_data,
> >+      },
> >+      {
> >+              .compatible =3D "ralink,rt3052-sysc",
> >+              .data =3D &rt3052_clk_data,
> >+      },
> >+      {
> >+              .compatible =3D "ralink,rt3352-sysc",
> >+              .data =3D &rt3052_clk_data,
>
> It seems that there should be `.data =3D &rt3352_clk_data,`?

Good catch! Thanks!
>
> >+      },
> >+      {
> >+              .compatible =3D "ralink,rt3883-sysc",
> >+              .data =3D &rt3352_clk_data,
> >+      },
>
> And there should be `.data =3D &rt3883_clk_data,`?

True.

>
> >+      {
> >+              .compatible =3D "ralink,rt5350-sysc",
> >+              .data =3D &rt5350_clk_data,
> >+      },
> >+      {
> >+              .compatible =3D "ralink,mt7620a-sysc",
> >+              .data =3D &mt7620_clk_data,
> >+      },
> >+      {
> >+              .compatible =3D "ralink,mt7620-sysc",
> >+              .data =3D &mt7620_clk_data,
> >+      },
> >+      {
>
> I am confused about the difference between `ralink,mt7620-sysc` and
> `ralink,mt7620a-sysc`. Do you mean mt7620n?
> https://www.mediatek.com/products/home-networking/mt7620n-a

There is no real difference. As you can see both of them use the same
'mt7620_clk_data' but since both compatible strings already exist
somewhere I introduced both of them.

>
> >+              .compatible =3D "ralink,mt7628-sysc",
> >+              .data =3D &mt76x8_clk_data,
> >+      },
> >+      {
> >+              .compatible =3D "ralink,mt7688-sysc",
> >+              .data =3D &mt76x8_clk_data,
> >+      },
> >+      {}
> >+};
> >+
>
> Thanks,
>     Shiji Yang

Thomas, I am going to send v4 with all Reviewed-by and Acked-by tags
added as well as this two minor stuff fixed.

Thanks,
    Sergio Paracuellos