From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0B604ECAAA1 for ; Wed, 31 Aug 2022 03:12:24 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 14D47849EF; Wed, 31 Aug 2022 05:11:33 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=andestech.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 810C484A28; Wed, 31 Aug 2022 05:11:30 +0200 (CEST) Received: from Atcsqr.andestech.com (60-248-80-70.hinet-ip.hinet.net [60.248.80.70]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 7E8C5849C1 for ; Wed, 31 Aug 2022 05:11:25 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=andestech.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ycliang@andestech.com Received: from mail.andestech.com (ATCPCS16.andestech.com [10.0.1.222]) by Atcsqr.andestech.com with ESMTP id 27V3B7YR055609; Wed, 31 Aug 2022 11:11:07 +0800 (+08) (envelope-from ycliang@andestech.com) Received: from ubuntu01 (10.0.12.75) by ATCPCS16.andestech.com (10.0.1.222) with Microsoft SMTP Server id 14.3.498.0; Wed, 31 Aug 2022 11:11:07 +0800 Date: Wed, 31 Aug 2022 03:10:53 +0000 From: Leo Liang To: Icenowy Zheng CC: Rick Chen , Lukasz Majewski , Green Wan , Subject: Re: [PATCH 1/2] dt-bindings: clock: sifive: sync FU740 PRCI clock binding header Message-ID: References: <20220825081119.1694007-1-uwu@icenowy.me> <20220825081119.1694007-2-uwu@icenowy.me> <0eb0be161b3f8dc9cb93fd254181d83039cf1948.camel@icenowy.me> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <0eb0be161b3f8dc9cb93fd254181d83039cf1948.camel@icenowy.me> User-Agent: Mutt/2.0.5 (2021-01-21) X-Originating-IP: [10.0.12.75] X-DNSRBL: X-MAIL: Atcsqr.andestech.com 27V3B7YR055609 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Wed, Aug 31, 2022 at 12:38:01AM +0800, Icenowy Zheng wrote: > 在 2022-08-30星期二的 03:26 +0000，Leo Liang写道： > > Hi Icenowy, > > On Thu, Aug 25, 2022 at 04:11:18PM +0800, Icenowy Zheng wrote: > > > This commit sychornizes the header file for FU740 PRCI clocks with > > > the > > > one from Linux 5.19. > > > > > > The constant values are the same, but all constant names are > > > changed > > > (most are just prefixed with FU740_). > > > > > > Signed-off-by: Icenowy Zheng > > > --- > > >  arch/riscv/dts/fu740-c000-u-boot.dtsi         | 16 +++++------ > > >  arch/riscv/dts/fu740-c000.dtsi                | 28 +++++++++------ > > > ---- > > >  drivers/clk/sifive/fu740-prci.c               | 18 ++++++------ > > >  drivers/clk/sifive/sifive-prci.c              |  4 +-- > > >  include/dt-bindings/clock/sifive-fu740-prci.h | 25 ++++++++------- > > > -- > > >  5 files changed, 45 insertions(+), 46 deletions(-) > > > > > > diff --git a/arch/riscv/dts/fu740-c000-u-boot.dtsi > > > b/arch/riscv/dts/fu740-c000-u-boot.dtsi > > > index a6f7a0873e..917e9bf163 100644 > > > --- a/arch/riscv/dts/fu740-c000-u-boot.dtsi > > > +++ b/arch/riscv/dts/fu740-c000-u-boot.dtsi > > > @@ -7,11 +7,11 @@ > > >   > > >  / { > > >         cpus { > > > -               assigned-clocks = <&prci PRCI_CLK_COREPLL>; > > > +               assigned-clocks = <&prci FU740_PRCI_CLK_COREPLL>; > > >                 assigned-clock-rates = <1200000000>; > > >                 u-boot,dm-spl; > > >                 cpu0: cpu@0 { > > > -                       clocks = <&prci PRCI_CLK_COREPLL>; > > > +                       clocks = <&prci FU740_PRCI_CLK_COREPLL>; > > >                         u-boot,dm-spl; > > >                         status = "okay"; > > >                         cpu0_intc: interrupt-controller { > > > @@ -19,28 +19,28 @@ > > >                         }; > > >                 }; > > >                 cpu1: cpu@1 { > > > -                       clocks = <&prci PRCI_CLK_COREPLL>; > > > +                       clocks = <&prci FU740_PRCI_CLK_COREPLL>; > > >                         u-boot,dm-spl; > > >                         cpu1_intc: interrupt-controller { > > >                                 u-boot,dm-spl; > > >                         }; > > >                 }; > > >                 cpu2: cpu@2 { > > > -                       clocks = <&prci PRCI_CLK_COREPLL>; > > > +                       clocks = <&prci FU740_PRCI_CLK_COREPLL>; > > >                         u-boot,dm-spl; > > >                         cpu2_intc: interrupt-controller { > > >                                  u-boot,dm-spl; > > >                         }; > > >                 }; > > >                 cpu3: cpu@3 { > > > -                       clocks = <&prci PRCI_CLK_COREPLL>; > > > +                       clocks = <&prci FU740_PRCI_CLK_COREPLL>; > > >                         u-boot,dm-spl; > > >                         cpu3_intc: interrupt-controller { > > >                                 u-boot,dm-spl; > > >                         }; > > >                 }; > > >                 cpu4: cpu@4 { > > > -                       clocks = <&prci PRCI_CLK_COREPLL>; > > > +                       clocks = <&prci FU740_PRCI_CLK_COREPLL>; > > >                         u-boot,dm-spl; > > >                         cpu4_intc: interrupt-controller { > > >                                 u-boot,dm-spl; > > > @@ -76,7 +76,7 @@ > > >                         reg = <0x0 0x100b0000 0x0 0x0800 > > >                                0x0 0x100b2000 0x0 0x2000 > > >                                0x0 0x100b8000 0x0 0x1000>; > > > -                       clocks = <&prci PRCI_CLK_DDRPLL>; > > > +                       clocks = <&prci FU740_PRCI_CLK_DDRPLL>; > > >                         clock-frequency = <933333324>; > > >                         u-boot,dm-spl; > > >                 }; > > > @@ -100,7 +100,7 @@ > > >  }; > > >   > > >  ð0 { > > > -       assigned-clocks = <&prci PRCI_CLK_GEMGXLPLL>; > > > +       assigned-clocks = <&prci FU740_PRCI_CLK_GEMGXLPLL>; > > >         assigned-clock-rates = <125125000>; > > >  }; > > >   > > > diff --git a/arch/riscv/dts/fu740-c000.dtsi b/arch/riscv/dts/fu740- > > > c000.dtsi > > > index 649efe400a..0e14aa71e7 100644 > > > --- a/arch/riscv/dts/fu740-c000.dtsi > > > +++ b/arch/riscv/dts/fu740-c000.dtsi > > > @@ -166,7 +166,7 @@ > > >                         reg = <0x0 0x10010000 0x0 0x1000>; > > >                         interrupt-parent = <&plic0>; > > >                         interrupts = <39>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         status = "disabled"; > > >                 }; > > >                 uart1: serial@10011000 { > > > @@ -174,7 +174,7 @@ > > >                         reg = <0x0 0x10011000 0x0 0x1000>; > > >                         interrupt-parent = <&plic0>; > > >                         interrupts = <40>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         status = "disabled"; > > >                 }; > > >                 i2c0: i2c@10030000 { > > > @@ -182,7 +182,7 @@ > > >                         reg = <0x0 0x10030000 0x0 0x1000>; > > >                         interrupt-parent = <&plic0>; > > >                         interrupts = <52>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         reg-shift = <2>; > > >                         reg-io-width = <1>; > > >                         #address-cells = <1>; > > > @@ -194,7 +194,7 @@ > > >                         reg = <0x0 0x10031000 0x0 0x1000>; > > >                         interrupt-parent = <&plic0>; > > >                         interrupts = <53>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         reg-shift = <2>; > > >                         reg-io-width = <1>; > > >                         #address-cells = <1>; > > > @@ -207,7 +207,7 @@ > > >                                0x0 0x20000000 0x0 0x10000000>; > > >                         interrupt-parent = <&plic0>; > > >                         interrupts = <41>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         #address-cells = <1>; > > >                         #size-cells = <0>; > > >                         status = "disabled"; > > > @@ -218,7 +218,7 @@ > > >                                0x0 0x30000000 0x0 0x10000000>; > > >                         interrupt-parent = <&plic0>; > > >                         interrupts = <42>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         #address-cells = <1>; > > >                         #size-cells = <0>; > > >                         status = "disabled"; > > > @@ -228,7 +228,7 @@ > > >                         reg = <0x0 0x10050000 0x0 0x1000>; > > >                         interrupt-parent = <&plic0>; > > >                         interrupts = <43>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         #address-cells = <1>; > > >                         #size-cells = <0>; > > >                         status = "disabled"; > > > @@ -241,8 +241,8 @@ > > >                                0x0 0x100a0000 0x0 0x1000>; > > >                         local-mac-address = [00 00 00 00 00 00]; > > >                         clock-names = "pclk", "hclk"; > > > -                       clocks = <&prci PRCI_CLK_GEMGXLPLL>, > > > -                                <&prci PRCI_CLK_GEMGXLPLL>; > > > +                       clocks = <&prci FU740_PRCI_CLK_GEMGXLPLL>, > > > +                                <&prci FU740_PRCI_CLK_GEMGXLPLL>; > > >                         #address-cells = <1>; > > >                         #size-cells = <0>; > > >                         status = "disabled"; > > > @@ -252,7 +252,7 @@ > > >                         reg = <0x0 0x10020000 0x0 0x1000>; > > >                         interrupt-parent = <&plic0>; > > >                         interrupts = <44 45 46 47>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         #pwm-cells = <3>; > > >                         status = "disabled"; > > >                 }; > > > @@ -261,7 +261,7 @@ > > >                         reg = <0x0 0x10021000 0x0 0x1000>; > > >                         interrupt-parent = <&plic0>; > > >                         interrupts = <48 49 50 51>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         #pwm-cells = <3>; > > >                         status = "disabled"; > > >                 }; > > > @@ -287,7 +287,7 @@ > > >                         #gpio-cells = <2>; > > >                         interrupt-controller; > > >                         #interrupt-cells = <2>; > > > -                       clocks = <&prci PRCI_CLK_PCLK>; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCLK>; > > >                         status = "disabled"; > > >                 }; > > >                 pcie@e00000000 { > > > @@ -318,8 +318,8 @@ > > >                                         <0x0 0x0 0x0 0x4 &plic0 > > > 60>; > > >                         pwren-gpios = <&gpio 5 0>; > > >                         reset-gpios = <&gpio 8 0>; > > > -                       clocks = <&prci PRCI_CLK_PCIEAUX>; > > > -                       clock-names = "pcieaux"; > > > +                       clocks = <&prci FU740_PRCI_CLK_PCIE_AUX>; > > > +                       clock-names = "pcie_aux"; > > > > Will this clock-names modification affects the functionality of > > sifive's clk or pcie driver in u-boot? > > For clock driver, it's just macro name change. > > For PCIe driver, both the U-Boot code and the DT binding does not use > the clock-names property at all. (The U-Boot code tries to get the > clock with index 0 instead of with some specified name) Thanks for your explanation! Reviewed-by: Leo Yu-Chi Liang Best regards, Leo > > > > > Otherwise, LGTM. > > > > Best regards, > > Leo > > > > >                         resets = <&prci PRCI_RST_PCIE_POWER_UP_N>; > > >                         reset-names = "rst_n"; > > >   > > > diff --git a/drivers/clk/sifive/fu740-prci.c > > > b/drivers/clk/sifive/fu740-prci.c > > > index b025050e22..5edc864e4b 100644 > > > --- a/drivers/clk/sifive/fu740-prci.c > > > +++ b/drivers/clk/sifive/fu740-prci.c > > > @@ -103,53 +103,53 @@ static const struct __prci_clock_ops > > > sifive_fu740_prci_pcieaux_clk_ops = { > > >   > > >  /* List of clock controls provided by the PRCI */ > > >  struct __prci_clock __prci_init_clocks_fu740[] = { > > > -       [PRCI_CLK_COREPLL] = { > > > +       [FU740_PRCI_CLK_COREPLL] = { > > >                 .name = "corepll", > > >                 .parent_name = "hfclk", > > >                 .ops = &sifive_fu740_prci_wrpll_clk_ops, > > >                 .pwd = &__prci_corepll_data, > > >         }, > > > -       [PRCI_CLK_DDRPLL] = { > > > +       [FU740_PRCI_CLK_DDRPLL] = { > > >                 .name = "ddrpll", > > >                 .parent_name = "hfclk", > > >                 .ops = &sifive_fu740_prci_wrpll_clk_ops, > > >                 .pwd = &__prci_ddrpll_data, > > >         }, > > > -       [PRCI_CLK_GEMGXLPLL] = { > > > +       [FU740_PRCI_CLK_GEMGXLPLL] = { > > >                 .name = "gemgxlpll", > > >                 .parent_name = "hfclk", > > >                 .ops = &sifive_fu740_prci_wrpll_clk_ops, > > >                 .pwd = &__prci_gemgxlpll_data, > > >         }, > > > -       [PRCI_CLK_DVFSCOREPLL] = { > > > +       [FU740_PRCI_CLK_DVFSCOREPLL] = { > > >                 .name = "dvfscorepll", > > >                 .parent_name = "hfclk", > > >                 .ops = &sifive_fu740_prci_wrpll_clk_ops, > > >                 .pwd = &__prci_dvfscorepll_data, > > >         }, > > > -       [PRCI_CLK_HFPCLKPLL] = { > > > +       [FU740_PRCI_CLK_HFPCLKPLL] = { > > >                 .name = "hfpclkpll", > > >                 .parent_name = "hfclk", > > >                 .ops = &sifive_fu740_prci_wrpll_clk_ops, > > >                 .pwd = &__prci_hfpclkpll_data, > > >         }, > > > -       [PRCI_CLK_CLTXPLL] = { > > > +       [FU740_PRCI_CLK_CLTXPLL] = { > > >                 .name = "cltxpll", > > >                 .parent_name = "hfclk", > > >                 .ops = &sifive_fu740_prci_wrpll_clk_ops, > > >                 .pwd = &__prci_cltxpll_data, > > >         }, > > > -       [PRCI_CLK_TLCLK] = { > > > +       [FU740_PRCI_CLK_TLCLK] = { > > >                 .name = "tlclk", > > >                 .parent_name = "corepll", > > >                 .ops = &sifive_fu740_prci_tlclksel_clk_ops, > > >         }, > > > -       [PRCI_CLK_PCLK] = { > > > +       [FU740_PRCI_CLK_PCLK] = { > > >                 .name = "pclk", > > >                 .parent_name = "hfpclkpll", > > >                 .ops = &sifive_fu740_prci_hfpclkplldiv_clk_ops, > > >         }, > > > -       [PRCI_CLK_PCIEAUX] { > > > +       [FU740_PRCI_CLK_PCIE_AUX] { > > >                 .name = "pcieaux", > > >                 .parent_name = "", > > >                 .ops = &sifive_fu740_prci_pcieaux_clk_ops, > > > diff --git a/drivers/clk/sifive/sifive-prci.c > > > b/drivers/clk/sifive/sifive-prci.c > > > index 52ae268e0c..c8fb600290 100644 > > > --- a/drivers/clk/sifive/sifive-prci.c > > > +++ b/drivers/clk/sifive/sifive-prci.c > > > @@ -685,14 +685,14 @@ static int sifive_prci_probe(struct udevice > > > *dev) > > >                                  * case the design uses hfpclk to > > > drive > > >                                  * Chiplink > > >                                  */ > > > -                               pc = &data- > > > >clks[PRCI_CLK_HFPCLKPLL]; > > > +                               pc = &data- > > > >clks[FU740_PRCI_CLK_HFPCLKPLL]; > > >                                 parent_rate = > > > sifive_prci_parent_rate(pc, data); > > >                                 sifive_prci_wrpll_set_rate(pc, > > > 260000000, > > >                                                            > > > parent_rate); > > >                                 pc->ops->enable_clk(pc, 1); > > >                         } else if (prci_pll_reg & > > > PRCI_PRCIPLL_CLTXPLL) { > > >                                 /* CLTX pll init */ > > > -                               pc = &data->clks[PRCI_CLK_CLTXPLL]; > > > +                               pc = &data- > > > >clks[FU740_PRCI_CLK_CLTXPLL]; > > >                                 parent_rate = > > > sifive_prci_parent_rate(pc, data); > > >                                 sifive_prci_wrpll_set_rate(pc, > > > 260000000, > > >                                                            > > > parent_rate); > > > diff --git a/include/dt-bindings/clock/sifive-fu740-prci.h > > > b/include/dt-bindings/clock/sifive-fu740-prci.h > > > index c1224783c0..672bdadbf6 100644 > > > --- a/include/dt-bindings/clock/sifive-fu740-prci.h > > > +++ b/include/dt-bindings/clock/sifive-fu740-prci.h > > > @@ -1,10 +1,9 @@ > > > -/* SPDX-License-Identifier: GPL-2.0 OR MIT */ > > > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ > > >  /* > > > - * Copyright (C) 2020-2021 SiFive, Inc. > > > + * Copyright (C) 2019 SiFive, Inc. > > >   * Wesley Terpstra > > >   * Paul Walmsley > > >   * Zong Li > > > - * Pragnesh Patel > > >   */ > > >   > > >  #ifndef __DT_BINDINGS_CLOCK_SIFIVE_FU740_PRCI_H > > > @@ -12,14 +11,14 @@ > > >   > > >  /* Clock indexes for use by Device Tree data and the PRCI driver > > > */ > > >   > > > -#define PRCI_CLK_COREPLL       0 > > > -#define PRCI_CLK_DDRPLL                1 > > > -#define PRCI_CLK_GEMGXLPLL     2 > > > -#define PRCI_CLK_DVFSCOREPLL   3 > > > -#define PRCI_CLK_HFPCLKPLL     4 > > > -#define PRCI_CLK_CLTXPLL       5 > > > -#define PRCI_CLK_TLCLK         6 > > > -#define PRCI_CLK_PCLK          7 > > > -#define PRCI_CLK_PCIEAUX       8 > > > +#define FU740_PRCI_CLK_COREPLL         0 > > > +#define FU740_PRCI_CLK_DDRPLL          1 > > > +#define FU740_PRCI_CLK_GEMGXLPLL       2 > > > +#define FU740_PRCI_CLK_DVFSCOREPLL     3 > > > +#define FU740_PRCI_CLK_HFPCLKPLL       4 > > > +#define FU740_PRCI_CLK_CLTXPLL         5 > > > +#define FU740_PRCI_CLK_TLCLK           6 > > > +#define FU740_PRCI_CLK_PCLK            7 > > > +#define FU740_PRCI_CLK_PCIE_AUX                8 > > >   > > > -#endif > > > +#endif /* __DT_BINDINGS_CLOCK_SIFIVE_FU740_PRCI_H */ > > > -- > > > 2.37.1 > > > >