[PATCH v6 09/17] clk: sifive: fu540-prci: Add clock initialization for SPL

Bin Meng bmeng.cn at gmail.com
Sat Apr 25 03:32:57 CEST 2020


Hi Pragnesh,

On Sat, Apr 25, 2020 at 12:27 AM Pragnesh Patel
<pragnesh.patel at sifive.com> wrote:
>
> Hi Bin,
>
> >-----Original Message-----
> >From: Bin Meng <bmeng.cn at gmail.com>
> >Sent: 24 April 2020 19:25
> >To: Pragnesh Patel <pragnesh.patel at sifive.com>
> >Cc: Jagan Teki <jagan at amarulasolutions.com>; U-Boot-Denx <u-
> >boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; Palmer Dabbelt
> ><palmerdabbelt at google.com>; Paul Walmsley <paul.walmsley at sifive.com>;
> >Troy Benjegerdes <troy.benjegerdes at sifive.com>; Anup Patel
> ><anup.patel at wdc.com>; Sagar Kadam <sagar.kadam at sifive.com>; Rick Chen
> ><rick at andestech.com>; Lukasz Majewski <lukma at denx.de>; Simon Glass
> ><sjg at chromium.org>
> >Subject: Re: [PATCH v6 09/17] clk: sifive: fu540-prci: Add clock initialization for
> >SPL
> >
> >[External Email] Do not click links or attachments unless you recognize the
> >sender and know the content is safe
> >
> >Hi Pragnesh,
> >
> >On Fri, Apr 24, 2020 at 9:34 PM Pragnesh Patel <pragnesh.patel at sifive.com>
> >wrote:
> >>
> >>
> >>
> >> Hi Bin,
> >>
> >> >-----Original Message-----
> >> >From: Bin Meng <bmeng.cn at gmail.com>
> >> >Sent: 24 April 2020 18:31
> >> >To: Pragnesh Patel <pragnesh.patel at sifive.com>
> >> >Cc: Jagan Teki <jagan at amarulasolutions.com>; U-Boot-Denx <u-
> >> >boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; Palmer
> >> >Dabbelt <palmerdabbelt at google.com>; Paul Walmsley
> >> ><paul.walmsley at sifive.com>; Troy Benjegerdes
> >> ><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>; Sagar
> >> >Kadam <sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com>;
> >> >Lukasz Majewski <lukma at denx.de>; Simon Glass <sjg at chromium.org>
> >> >Subject: Re: [PATCH v6 09/17] clk: sifive: fu540-prci: Add clock
> >> >initialization for SPL
> >> >
> >> >[External Email] Do not click links or attachments unless you
> >> >recognize the sender and know the content is safe
> >> >
> >> >Hi Pragnesh,
> >> >
> >> >On Fri, Apr 24, 2020 at 6:08 PM Pragnesh Patel
> >> ><pragnesh.patel at sifive.com>
> >> >wrote:
> >> >>
> >> >> Hi Bin, Jagan
> >> >>
> >> >> >-----Original Message-----
> >> >> >From: Bin Meng <bmeng.cn at gmail.com>
> >> >> >Sent: 20 April 2020 14:30
> >> >> >To: Jagan Teki <jagan at amarulasolutions.com>
> >> >> >Cc: Pragnesh Patel <pragnesh.patel at sifive.com>; U-Boot-Denx <u-
> >> >> >boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; Palmer
> >> >> >Dabbelt <palmerdabbelt at google.com>; Paul Walmsley
> >> >> ><paul.walmsley at sifive.com>; Troy Benjegerdes
> >> >> ><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>;
> >> >> >Sagar Kadam <sagar.kadam at sifive.com>; Rick Chen
> >> >> ><rick at andestech.com>; Lukasz Majewski <lukma at denx.de>; Simon
> >Glass
> >> >> ><sjg at chromium.org>
> >> >> >Subject: Re: [PATCH v6 09/17] clk: sifive: fu540-prci: Add clock
> >> >> >initialization for SPL
> >> >> >
> >> >> >[External Email] Do not click links or attachments unless you
> >> >> >recognize the sender and know the content is safe
> >> >> >
> >> >> >Hi Jagan, Pragnesh,
> >> >> >
> >> >> >On Tue, Apr 7, 2020 at 3:35 AM Jagan Teki
> >> >> ><jagan at amarulasolutions.com>
> >> >> >wrote:
> >> >> >>
> >> >> >> On Sun, Mar 29, 2020 at 10:37 PM Pragnesh Patel
> >> >> >> <pragnesh.patel at sifive.com> wrote:
> >> >> >> >
> >> >> >> > Set corepll, ddrpll and ethernet PLL for u-boot-spl
> >> >> >> >
> >> >> >> > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
> >> >> >> > ---
> >> >> >> >  drivers/clk/sifive/fu540-prci.c | 118
> >> >> >> > ++++++++++++++++++++++++++++++++
> >> >> >> >  1 file changed, 118 insertions(+)
> >> >> >> >
> >> >> >> > diff --git a/drivers/clk/sifive/fu540-prci.c
> >> >> >> > b/drivers/clk/sifive/fu540-prci.c index e6214cd821..3a73c2c8d1
> >> >> >> > 100644
> >> >> >> > --- a/drivers/clk/sifive/fu540-prci.c
> >> >> >> > +++ b/drivers/clk/sifive/fu540-prci.c
> >> >> >> > @@ -41,6 +41,10 @@
> >> >> >> >  #include <linux/clk/analogbits-wrpll-cln28hpc.h>
> >> >> >> >  #include <dt-bindings/clock/sifive-fu540-prci.h>
> >> >> >> >
> >> >> >> > +#define DDRCTLPLL_F    55
> >> >> >> > +#define DDRCTLPLL_Q    2
> >> >> >> > +#define MHz            1000000
> >> >> >> > +
> >> >> >> >  /*
> >> >> >> >   * EXPECTED_CLK_PARENT_COUNT: how many parent clocks this
> >> >> >> > driver
> >> >> >expects:
> >> >> >> >   *     hfclk and rtcclk
> >> >> >> > @@ -152,6 +156,27 @@
> >> >> >> >  #define PRCI_CLKMUXSTATUSREG_TLCLKSEL_STATUS_MASK \
> >> >> >> >                         (0x1 <<
> >> >> >> > PRCI_CLKMUXSTATUSREG_TLCLKSEL_STATUS_SHIFT)
> >> >> >> >
> >> >> >> > +/* PROCMONCFG */
> >> >> >> > +#define PRCI_PROCMONCFG_OFFSET                 0xF0
> >> >> >> > +#define PRCI_PROCMONCFG_CORE_CLOCK_SHIFT       24
> >> >> >> > +#define PRCI_PROCMONCFG_CORE_CLOCK_MASK \
> >> >> >> > +                       (0x1 <<
> >> >> >> > +PRCI_PROCMONCFG_CORE_CLOCK_SHIFT)
> >> >> >> > +
> >> >> >> > +#define PLL_R(x) \
> >> >> >> > +       ((x) << PRCI_DDRPLLCFG0_DIVR_SHIFT) &
> >> >> >> > +PRCI_DDRPLLCFG0_DIVR_MASK #define PLL_F(x) \
> >> >> >> > +       ((x) << PRCI_DDRPLLCFG0_DIVF_SHIFT) &
> >> >> >> > +PRCI_DDRPLLCFG0_DIVF_MASK #define PLL_Q(x) \
> >> >> >> > +       ((x) << PRCI_DDRPLLCFG0_DIVQ_SHIFT) &
> >> >> >> > +PRCI_DDRPLLCFG0_DIVQ_MASK #define PLL_RANGE(x) \
> >> >> >> > +       ((x) << PRCI_DDRPLLCFG0_RANGE_SHIFT) &
> >> >> >> > +PRCI_DDRPLLCFG0_RANGE_MASK #define PLL_BYPASS(x) \
> >> >> >> > +       ((x) << PRCI_DDRPLLCFG0_BYPASS_SHIFT) &
> >> >> >> > +PRCI_DDRPLLCFG0_BYPASS_MASK #define PLL_FSE(x) \
> >> >> >> > +       ((x) << PRCI_DDRPLLCFG0_FSE_SHIFT) &
> >> >> >> > +PRCI_DDRPLLCFG0_FSE_MASK #define PLL_LOCK(x) \
> >> >> >> > +       ((x) << PRCI_DDRPLLCFG0_LOCK_SHIFT) &
> >> >> >> > +PRCI_DDRPLLCFG0_LOCK_MASK
> >> >> >> > +
> >> >> >> >  /*
> >> >> >> >   * Private structures
> >> >> >> >   */
> >> >> >> > @@ -654,6 +679,93 @@ static int
> >> >> >> > sifive_fu540_prci_disable(struct clk
> >> >*clk)
> >> >> >> >         return pc->ops->enable_clk(pc, 0);  }
> >> >> >> >
> >> >> >> > +#ifdef CONFIG_SPL_BUILD
> >> >> >> > +static void corepll_init(struct udevice *dev) {
> >> >> >> > +       u32 v;
> >> >> >> > +       struct clk clock;
> >> >> >> > +       struct __prci_data *pd = dev_get_priv(dev);
> >> >> >> > +
> >> >> >> > +       v = __prci_readl(pd, PRCI_CLKMUXSTATUSREG_OFFSET);
> >> >> >> > +       v &= PRCI_CLKMUXSTATUSREG_TLCLKSEL_STATUS_MASK;
> >> >> >> > +
> >> >> >> > +       clock.id = PRCI_CLK_COREPLL;
> >> >> >> > +
> >> >> >> > +       if (v) {
> >> >> >> > +               /* corepll 500 Mhz */
> >> >> >> > +               sifive_fu540_prci_set_rate(&clock, 500UL * MHz);
> >> >> >> > +       } else {
> >> >> >> > +               /* corepll 1 Ghz */
> >> >> >> > +               sifive_fu540_prci_set_rate(&clock, 1000UL * MHz);
> >> >> >> > +       }
> >> >> >> > +
> >> >> >> > +
> >> >> >> > +sifive_fu540_prci_clock_enable(&__prci_init_clocks[clock.id],
> >> >> >> > +1); }
> >> >> >> > +
> >> >> >> > +static void ddr_init(struct udevice *dev) {
> >> >> >> > +       u32 v;
> >> >> >> > +       struct clk clock;
> >> >> >> > +       struct __prci_data *pd = dev_get_priv(dev);
> >> >> >> > +
> >> >> >> > +       //DDR init
> >> >> >> > +       u32 ddrctlmhz =
> >> >> >> > +               (PLL_R(0)) |
> >> >> >> > +               (PLL_F(DDRCTLPLL_F)) |
> >> >> >> > +               (PLL_Q(DDRCTLPLL_Q)) |
> >> >> >> > +               (PLL_RANGE(0x4)) |
> >> >> >> > +               (PLL_BYPASS(0)) |
> >> >> >> > +               (PLL_FSE(1));
> >> >> >> > +       __prci_writel(ddrctlmhz, PRCI_DDRPLLCFG0_OFFSET, pd);
> >> >> >> > +
> >> >> >> > +       clock.id = PRCI_CLK_DDRPLL;
> >> >> >> > +
> >> >> >> > + sifive_fu540_prci_clock_enable(&__prci_init_clocks[clock.id]
> >> >> >> > + ,
> >> >> >> > + 1);
> >> >> >> > +
> >> >> >> > +       /* Release DDR reset */
> >> >> >> > +       v = __prci_readl(pd, PRCI_DEVICESRESETREG_OFFSET);
> >> >> >> > +       v |= PRCI_DEVICESRESETREG_DDR_CTRL_RST_N_MASK;
> >> >> >> > +       __prci_writel(v, PRCI_DEVICESRESETREG_OFFSET, pd);
> >> >> >> > +
> >> >> >> > +       // HACK to get the '1 full controller clock cycle'.
> >> >> >> > +       asm volatile ("fence");
> >> >> >> > +       v = __prci_readl(pd, PRCI_DEVICESRESETREG_OFFSET);
> >> >> >> > +       v |= (PRCI_DEVICESRESETREG_DDR_AXI_RST_N_MASK |
> >> >> >> > +             PRCI_DEVICESRESETREG_DDR_AHB_RST_N_MASK |
> >> >> >> > +             PRCI_DEVICESRESETREG_DDR_PHY_RST_N_MASK);
> >> >> >> > +       __prci_writel(v, PRCI_DEVICESRESETREG_OFFSET, pd);
> >> >> >> > +       // HACK to get the '1 full controller clock cycle'.
> >> >> >> > +       asm volatile ("fence");
> >> >> >> > +
> >> >> >> > +       /* These take like 16 cycles to actually propagate. We
> >> >> >> > + can't go
> >> >sending
> >> >> >> > +        * stuff before they come out of reset. So wait.
> >> >> >> > + (TODO: Add a
> >> >register
> >> >> >> > +        * to read the current reset states, or DDR Control device?)
> >> >> >> > +        */
> >> >> >> > +       for (int i = 0; i < 256; i++)
> >> >> >> > +               asm volatile ("nop"); }
> >> >> >> > +
> >> >> >> > +static void ethernet_init(struct udevice *dev) {
> >> >> >> > +       u32 v;
> >> >> >> > +       struct clk clock;
> >> >> >> > +       struct __prci_data *pd = dev_get_priv(dev);
> >> >> >> > +
> >> >> >> > +       /* GEMGXL init */
> >> >> >> > +       clock.id = PRCI_CLK_GEMGXLPLL;
> >> >> >> > +       sifive_fu540_prci_set_rate(&clock, 125UL * MHz);
> >> >> >> > +
> >> >> >> > + sifive_fu540_prci_clock_enable(&__prci_init_clocks[clock.id]
> >> >> >> > + ,
> >> >> >> > + 1);
> >> >> >> > +
> >> >> >> > +       /* Release GEMGXL reset */
> >> >> >> > +       v = __prci_readl(pd, PRCI_DEVICESRESETREG_OFFSET);
> >> >> >> > +       v |= PRCI_DEVICESRESETREG_GEMGXL_RST_N_MASK;
> >> >> >> > +       __prci_writel(v, PRCI_DEVICESRESETREG_OFFSET, pd);
> >> >> >> > +
> >> >> >> > +       /* Procmon => core clock */
> >> >> >> > +       __prci_writel(PRCI_PROCMONCFG_CORE_CLOCK_MASK,
> >> >> >PRCI_PROCMONCFG_OFFSET,
> >> >> >> > +                     pd);
> >> >> >> > +}
> >> >> >> > +#endif
> >> >> >> > +
> >> >> >> >  static int sifive_fu540_prci_probe(struct udevice *dev)  {
> >> >> >> >         int i, err;
> >> >> >> > @@ -679,6 +791,12 @@ static int sifive_fu540_prci_probe(struct
> >> >> >> > udevice
> >> >> >*dev)
> >> >> >> >                         __prci_wrpll_read_cfg0(pd, pc->pwd);
> >> >> >> >         }
> >> >> >> >
> >> >> >> > +#ifdef CONFIG_SPL_BUILD
> >> >> >> > +       corepll_init(dev);
> >> >> >> > +       ddr_init(dev);
> >> >> >> > +       ethernet_init(dev);
> >> >> >> > +#endif
> >> >> >>
> >> >> >> 1. Why would ethernet clocks require for SPL 2. Why these clocks
> >> >> >> are special for SPL, can't we use it for U-Boot proper 3. This
> >> >> >> look like raw clock initialization instead of having in conventional dm
> >way.
> >> >> >> since these are here just to use pd.  May be reuse the required
> >> >> >> clock of what u-boot proper using or move them into spl code.
> >> >>
> >> >> 1. ethernet clock is not necessary for SPL but SPL performs
> >> >> ethernet PHY reset sequence (board/sifive/fu540/spl.c -
> >> >> "gem_phy_reset") and for
> >> >that ethernet clock needs to be enabled.
> >> >
> >> >Can we delay that for U-Boot proper to enable PRCI ethernet clock and
> >> >reset the PHY ?
> >>
> >> We can but what happens if someone wants to skip U-Boot (SPL -> OpenSBI
> >+ Linux) ?
> >>
> >
> >I am not sure if that's a desired boot flow. If that's something we want to
> >support, yes we will have to do it in SPL.
> >
> >> >
> >> >>
> >> >> 2. There is nothing special of this clocks for SPL, we can use this for U-
> >Boot.
> >> >> I am planning to add this clocks in device tree but I am not sure
> >> >> how to add coreclk (corepll) in device tree Do you guys have any
> >> >> suggestion for
> >> >this ?
> >> >>
> >> >
> >> >Does the example in
> >>
> >>http://patchwork.ozlabs.org/project/uboot/patch/20200423023320.138009
> >> >0-
> >> >20-seanga2 at gmail.com/
> >> >help?
> >> >
> >> >+ cpu0: cpu at 0 {
> >> >+ device_type = "cpu";
> >> >+ compatible = "kendryte,k210", "sifive,rocket0", "riscv"; reg = <0>;
> >> >+ riscv,isa = "rv64imafdgc"; mmu-type = "sv39"; i-cache-block-size =
> >> >+ <64>; i-cache-size = <0x8000>; d-cache-block-size = <64>;
> >> >+ d-cache-size = <0x8000>; clocks = <&sysclk K210_CLK_CPU>;
> >> >
> >> >You need this patch to get clock in the CPU node.
> >> >
> >>
> >>http://patchwork.ozlabs.org/project/uboot/patch/20200423023320.138009
> >> >0-
> >> >19-seanga2 at gmail.com/
>
> This patch helps but I also need to do clk_set_rate() for coreclk.
>
> static void corepll_init(struct udevice *dev)
> {
>         .....
>         if (v) {
>                 /* corepll 500 Mhz */
>                 sifive_fu540_prci_set_rate(&clock, 500UL * MHz);
>         } else {
>                 /* corepll 1 Ghz */
>                 sifive_fu540_prci_set_rate(&clock, 1000UL * MHz);
>         }
> }
>
> I think, if "clock-frequency" is provided in dts then it's better to call clk_set_rate()
>

I think so.

> Any suggestions are welcome.

Regards,
Bin


More information about the U-Boot mailing list