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;
Message-ID: <c37e1f3a40c404acd81c2c9d5b28b340.sboyd@kernel.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <b7977069-4f82-76a1-10c1-b6400862c2c4@gmail.com>
References: <20230328021912.177301-1-ychuang570808@gmail.com> <20230328021912.177301-9-ychuang570808@gmail.com> <ab4e0bc8834b7e618e9a88ea6a1c30cc.sboyd@kernel.org> <b7977069-4f82-76a1-10c1-b6400862c2c4@gmail.com>
Subject: Re: [PATCH v6 08/12] arm64: dts: nuvoton: Add initial ma35d1 device tree
From:   Stephen Boyd <sboyd@kernel.org>
Cc:     devicetree@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
        arnd@arndb.de, schung@nuvoton.com, mjchen@nuvoton.com,
        Jacky Huang <ychuang3@nuvoton.com>
To:     Jacky Huang <ychuang570808@gmail.com>, gregkh@linuxfoundation.org,
        jirislaby@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        lee@kernel.org, mturquette@baylibre.com, p.zabel@pengutronix.de,
        robh+dt@kernel.org
Date:   Tue, 28 Mar 2023 19:19:10 -0700
User-Agent: alot/0.10
Precedence: bulk

Quoting Jacky Huang (2023-03-28 19:03:24)
> On 2023/3/29 =E4=B8=8A=E5=8D=88 01:57, Stephen Boyd wrote:
> > Quoting Jacky Huang (2023-03-27 19:19:08)
> >> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot=
/dts/nuvoton/ma35d1.dtsi
> >> new file mode 100644
> >> index 000000000000..0740b0b218a7
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> >> @@ -0,0 +1,231 @@
[...]
> >> +
> >> +               L2_0: l2-cache0 {
> > Just l2-cache for the node name. Doesn't it go under the cpu0 node as
> > well?
>=20
> This describes the level-2 cache which is external to and shared by cpu0 =

> & cpu1.
> And only level-1 cache is inside of CPU core.
> L2_0 is must, because both cpu0 and cpu1 has a next-level-cache =3D=20
> <&L2_0> property.

Ok. The name should just be l2-cache then, not l2-cache0.

>=20
> Many identical example of l2-cache node can be found in arm64 dts, such=20
> as k3-arm642.dtsi,
> rk3328.dtsi, mt8195.dtsi, etc. Here is just a copy of similar arm64=20
> multi-core SoCs.
>=20
> So we would like to keep this unchanged. Is it OK for you? Thanks.
>=20

Mostly ok, yes.

>=20
> >> +
> >> +       sys: system-management@40460000 {
> >> +               compatible =3D "nuvoton,ma35d1-sys", "syscon", "simple=
-mfd";
> >> +               reg =3D <0x0 0x40460000 0x0 0x200>;
> >> +
> >> +               reset: reset-controller {
> >> +                       compatible =3D "nuvoton,ma35d1-reset";
> >> +                       #reset-cells =3D <1>;
> >> +               };
> >> +       };
> >> +
> >> +       clk: clock-controller@40460200 {
> >> +               compatible =3D "nuvoton,ma35d1-clk", "syscon";
> >> +               reg =3D <0x00000000 0x40460200 0x0 0x100>;
> >> +               #clock-cells =3D <1>;
> >> +               clocks =3D <&clk_hxt>;
> >> +               nuvoton,sys =3D <&sys>;
> >> +       };
> > It looks like the device at 40460000 is a reset and clock controller.
> > Just make it one node and register the clk or reset device as an
> > auxiliary device.
>=20
> 40460000 is for system control registers, including power contrl,=20
> multifunction pin control,
> usb phy control, IP reset control, power-on setting information, and=20
> many other miscellaneous controls.
> The registers of reset controller is only a subset of system control=20
> registers.
>=20
> 40460200 is for clock controller which is independent of the system=20
> control integration
> The register base of clock controller is very close to system=20
> controller, but in fact the two are independent.

What do you use the syscon for then? The clock driver must want to use
the syscon for something, implying that they are the same device.