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: <20220608095623.22327-1-tmaimon77@gmail.com> <20220608095623.22327-19-tmaimon77@gmail.com>
 <24ad8ba0-4244-1159-328d-12d0e67951e1@linaro.org> <CAP6Zq1iXaN8D-g2O=cD-XERGj3BROQO=NJ66mquVsOw8nSM=0A@mail.gmail.com>
In-Reply-To: <CAP6Zq1iXaN8D-g2O=cD-XERGj3BROQO=NJ66mquVsOw8nSM=0A@mail.gmail.com>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Fri, 10 Jun 2022 09:57:18 +0200
Message-ID: <CAMuHMdU53RWvKXd0cPcPA8SiMA820stcpK4_UsTDGDAfByhcYg@mail.gmail.com>
Subject: Re: [PATCH v2 18/20] arm64: dts: nuvoton: Add initial NPCM8XX device tree
To:     Tomer Maimon <tmaimon77@gmail.com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Avi Fishman <avifishman70@gmail.com>,
        Tali Perry <tali.perry1@gmail.com>,
        Joel Stanley <joel@jms.id.au>,
        Patrick Venture <venture@google.com>,
        Nancy Yuen <yuenn@google.com>,
        Benjamin Fair <benjaminfair@google.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Greg KH <gregkh@linuxfoundation.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Wim Van Sebroeck <wim@linux-watchdog.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
        Olof Johansson <olof@lixom.net>,
        Jiri Slaby <jirislaby@kernel.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Marcel Ziswiler <marcel.ziswiler@toradex.com>,
        Vinod Koul <vkoul@kernel.org>,
        Biju Das <biju.das.jz@bp.renesas.com>,
        Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>,
        Robert Hancock <robert.hancock@calian.com>,
        =?UTF-8?Q?Jonathan_Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>,
        Lubomir Rintel <lkundrak@v3.sk>,
        devicetree <devicetree@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-clk <linux-clk@vger.kernel.org>,
        "open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
        LINUXWATCHDOG <linux-watchdog@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Tomer,

On Fri, Jun 10, 2022 at 12:30 AM Tomer Maimon <tmaimon77@gmail.com> wrote:
> On Wed, 8 Jun 2022 at 13:21, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> > On 08/06/2022 11:56, Tomer Maimon wrote:
> > > This adds initial device tree support for the
> > > Nuvoton NPCM845 Board Management controller (BMC) SoC family.
> > >
> > > The NPCM845 based quad-core Cortex-A35 ARMv8 architecture and
> > > have various peripheral IPs.
> > >
> > > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>

> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > > @@ -0,0 +1,197 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +// Copyright (c) 2021 Nuvoton Technology tomer.maimon@nuvoton.com
> > > +
> > > +#include <dt-bindings/clock/nuvoton,npcm8xx-clock.h>
> > > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +#include <dt-bindings/interrupt-controller/irq.h>
> > > +
> > > +/ {
> > > +     #address-cells = <2>;
> > > +     #size-cells = <2>;
> > > +     interrupt-parent = <&gic>;
> > > +
> > > +     /* external reference clock */
> > > +     clk_refclk: clk-refclk {
> > > +             compatible = "fixed-clock";
> > > +             #clock-cells = <0>;
> > > +             clock-frequency = <25000000>;
> >
> > Ignored comment.
> Could we use it as a default clock-frequency?

If the oscillator is present on the board, and not an SoC builtin, its
clock frequency should be described in the board DTS.
Some clocks may be optional, and left unpopulated.
Others clocks may be fed with different frequencies than the default.

> >
> > > +             clock-output-names = "refclk";
> > > +     };
> > > +
> > > +     /* external reference clock for cpu. float in normal operation */
> > > +     clk_sysbypck: clk-sysbypck {
> > > +             compatible = "fixed-clock";
> > > +             #clock-cells = <0>;
> > > +             clock-frequency = <1000000000>;
> >
> > Ignored comment.
> same as above
> >
> > > +             clock-output-names = "sysbypck";
> > > +     };
> > > +
> > > +     /* external reference clock for MC. float in normal operation */
> > > +     clk_mcbypck: clk-mcbypck {
> > > +             compatible = "fixed-clock";
> > > +             #clock-cells = <0>;
> > > +             clock-frequency = <1050000000>;
> same as above
> > > +             clock-output-names = "mcbypck";
> > > +     };

>  "+             cpu0: cpu@0 {
>  +                     device_type = "cpu";
>  +                     compatible = "arm,cortex-a35";
>  +                     clocks = <&clk NPCM8XX_CLK_CPU>;
>  +                     reg = <0x0 0x0>;
> Why do you have two address cells? A bit more complicated and not
> necessary, I think."
> the arm,cortex-a35 is 64 Bit this is why we use  #address-cells = <2>;
> and therefore reg = <0x0 0x0>;

These addresses are not addresses on the main memory bus (which
is indeed 64-bit), but on the logical CPU bus.
Now, Documentation/devicetree/bindings/arm/cpus.yaml says you can
have #address-cells = <2> if you have non-zero MPIDR_EL1 high bits.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds