[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240828-duplex-skillful-752582090412@wendy>
Date: Wed, 28 Aug 2024 08:58:43 +0100
From: Conor Dooley <conor.dooley@...rochip.com>
To: Bo Gan <ganboing@...il.com>
CC: <zong.li@...ive.com>, <linux-clk@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-riscv@...ts.infradead.org>,
<Pragnesh.patel@...ive.com>, <aou@...s.berkeley.edu>,
<erik.danie@...ive.com>, <hes@...ive.com>, <mturquette@...libre.com>,
<palmer@...belt.com>, <palmerdabbelt@...gle.com>, <paul.walmsley@...ive.com>,
<pragnesh.patel@...nfive.com>, <sboyd@...nel.org>, <schwab@...ux-m68k.org>,
<yash.shah@...ive.com>
Subject: Re: [PATCH 3/3] clk: sifive: prci: Add release_reset hooks for
gemgxlpll/cltxpll
On Tue, Aug 27, 2024 at 11:55:20PM -0700, Bo Gan wrote:
> This patch adds the release_reset hook interface to __prci_wrpll_data.
> During clock enablement, the function (if present) will be called after PLL
> registers are configured. It aligns the logic to the driver in u-boot. When
> there's a previous bootloader stage, such as u-boot, it usually enables the
> gemgxlpll clock when trying to PXE/network boot. The kernel boots fine, but
> we should not depend on it being our previous stage, and the logic within:
>
> a. We (linux) can get directly invoked by firmware (OpenSBI).
> b. U-boot doesn't necessarily have to initialize ethernet and enable the
> clock (when not enabled in CONFIG).
>
> When the kernel is the first to initialize gemgxlpll, it must also release
> the corresponding reset. Otherwise the chip will just hang during macb
> initialization, and even external JTAG debugger will lose control over the
> risc-v debug module. (Observed with my Sifive Unmatched Rev.B board)
>
> The patch took the dt-bindings and logics directly from u-boot with some
> additional modifications:
> - Use __prci_writel after __prci_readl to have barrier semantic. U-boot
> has the strong version of readl/writel, but linux has the relaxed ones.
> - Use pd->reset.rcdev.ops to access the reset regs.
> - Split reset bindings for FU540/FU740 and use them directly, instead of
> looking it up through reset-names.
The macb driver already supports using a reset at boot time (see zynq and
mpfs) if hooked up in the devicetree, why doesn't that work for you in
this situation?
Thanks,
Conor.
>
> Signed-off-by: Bo Gan <ganboing@...il.com>
> ---
> drivers/clk/sifive/fu540-prci.h | 16 ++++++++++++++++
> drivers/clk/sifive/fu740-prci.h | 31 +++++++++++++++++++++++++++++++
> drivers/clk/sifive/sifive-prci.c | 23 +++++++++++++++++++++++
> drivers/clk/sifive/sifive-prci.h | 8 ++++++++
> 4 files changed, 78 insertions(+)
>
> diff --git a/drivers/clk/sifive/fu540-prci.h b/drivers/clk/sifive/fu540-prci.h
> index e0173324f3c5..9d2ca18f47a4 100644
> --- a/drivers/clk/sifive/fu540-prci.h
> +++ b/drivers/clk/sifive/fu540-prci.h
> @@ -23,9 +23,24 @@
> #include <linux/module.h>
>
> #include <dt-bindings/clock/sifive-fu540-prci.h>
> +#include <dt-bindings/reset/sifive-fu540-prci.h>
>
> #include "sifive-prci.h"
>
> +/**
> + * sifive_fu540_prci_ethernet_release_reset() - Release ethernet reset
> + * @pd: struct __prci_data * for the PRCI containing the Ethernet CLK mux reg
> + *
> + */
> +static void sifive_fu540_prci_ethernet_release_reset(struct __prci_data *pd)
> +{
> + /* Release GEMGXL reset */
> + pd->reset.rcdev.ops->deassert(&pd->reset.rcdev, FU540_PRCI_RST_GEMGXL_N);
> +
> + /* Procmon => core clock */
> + sifive_prci_set_procmoncfg(pd, PRCI_PROCMONCFG_CORE_CLOCK_MASK);
> +}
> +
> /* PRCI integration data for each WRPLL instance */
>
> static struct __prci_wrpll_data sifive_fu540_prci_corepll_data = {
> @@ -43,6 +58,7 @@ static struct __prci_wrpll_data sifive_fu540_prci_ddrpll_data = {
> static struct __prci_wrpll_data sifive_fu540_prci_gemgxlpll_data = {
> .cfg0_offs = PRCI_GEMGXLPLLCFG0_OFFSET,
> .cfg1_offs = PRCI_GEMGXLPLLCFG1_OFFSET,
> + .release_reset = sifive_fu540_prci_ethernet_release_reset,
> };
>
> /* Linux clock framework integration */
> diff --git a/drivers/clk/sifive/fu740-prci.h b/drivers/clk/sifive/fu740-prci.h
> index f31cd30fc395..dd0f54277a99 100644
> --- a/drivers/clk/sifive/fu740-prci.h
> +++ b/drivers/clk/sifive/fu740-prci.h
> @@ -10,9 +10,38 @@
> #include <linux/module.h>
>
> #include <dt-bindings/clock/sifive-fu740-prci.h>
> +#include <dt-bindings/reset/sifive-fu740-prci.h>
>
> #include "sifive-prci.h"
>
> +/**
> + * sifive_fu740_prci_ethernet_release_reset() - Release ethernet reset
> + * @pd: struct __prci_data * for the PRCI containing the Ethernet CLK mux reg
> + *
> + */
> +static void sifive_fu740_prci_ethernet_release_reset(struct __prci_data *pd)
> +{
> + /* Release GEMGXL reset */
> + pd->reset.rcdev.ops->deassert(&pd->reset.rcdev, FU740_PRCI_RST_GEMGXL_N);
> +
> + /* Procmon => core clock */
> + sifive_prci_set_procmoncfg(pd, PRCI_PROCMONCFG_CORE_CLOCK_MASK);
> +
> + /* Release Chiplink reset */
> + pd->reset.rcdev.ops->deassert(&pd->reset.rcdev, FU740_PRCI_RST_CLTX_N);
> +}
> +
> +/**
> + * sifive_fu740_prci_cltx_release_reset() - Release cltx reset
> + * @pd: struct __prci_data * for the PRCI containing the Ethernet CLK mux reg
> + *
> + */
> +static void sifive_fu740_prci_cltx_release_reset(struct __prci_data *pd)
> +{
> + /* Release CLTX reset */
> + pd->reset.rcdev.ops->deassert(&pd->reset.rcdev, FU740_PRCI_RST_CLTX_N);
> +}
> +
> /* PRCI integration data for each WRPLL instance */
>
> static struct __prci_wrpll_data sifive_fu740_prci_corepll_data = {
> @@ -30,6 +59,7 @@ static struct __prci_wrpll_data sifive_fu740_prci_ddrpll_data = {
> static struct __prci_wrpll_data sifive_fu740_prci_gemgxlpll_data = {
> .cfg0_offs = PRCI_GEMGXLPLLCFG0_OFFSET,
> .cfg1_offs = PRCI_GEMGXLPLLCFG1_OFFSET,
> + .release_reset = sifive_fu740_prci_ethernet_release_reset,
> };
>
> static struct __prci_wrpll_data sifive_fu740_prci_dvfscorepll_data = {
> @@ -49,6 +79,7 @@ static struct __prci_wrpll_data sifive_fu740_prci_hfpclkpll_data = {
> static struct __prci_wrpll_data sifive_fu740_prci_cltxpll_data = {
> .cfg0_offs = PRCI_CLTXPLLCFG0_OFFSET,
> .cfg1_offs = PRCI_CLTXPLLCFG1_OFFSET,
> + .release_reset = sifive_fu740_prci_cltx_release_reset,
> };
>
> /* Linux clock framework integration */
> diff --git a/drivers/clk/sifive/sifive-prci.c b/drivers/clk/sifive/sifive-prci.c
> index caba0400f8a2..ae8055a84466 100644
> --- a/drivers/clk/sifive/sifive-prci.c
> +++ b/drivers/clk/sifive/sifive-prci.c
> @@ -249,6 +249,9 @@ int sifive_prci_clock_enable(struct clk_hw *hw)
> if (pwd->disable_bypass)
> pwd->disable_bypass(pd);
>
> + if (pwd->release_reset)
> + pwd->release_reset(pd);
> +
> return 0;
> }
>
> @@ -448,6 +451,26 @@ void sifive_prci_hfpclkpllsel_use_hfpclkpll(struct __prci_data *pd)
> r = __prci_readl(pd, PRCI_HFPCLKPLLSEL_OFFSET); /* barrier */
> }
>
> +/*
> + * PROCMONCFG
> + */
> +
> +/**
> + * sifive_prci_set_procmoncfg() - set PROCMONCFG
> + * @pd: struct __prci_data * PRCI context
> + * @val: u32 value to write to PROCMONCFG register
> + *
> + * Set the PROCMONCFG register to @val
> + *
> + * Context: Any context. Caller must prevent concurrent changes to the
> + * PROCMONCFG_OFFSET register.
> + */
> +void sifive_prci_set_procmoncfg(struct __prci_data *pd, u32 val)
> +{
> + __prci_writel(val, PRCI_PROCMONCFG_OFFSET, pd);
> + __prci_readl(pd, PRCI_PROCMONCFG_OFFSET); /* barrier */
> +}
> +
> /* PCIE AUX clock APIs for enable, disable. */
> int sifive_prci_pcie_aux_clock_is_enabled(struct clk_hw *hw)
> {
> diff --git a/drivers/clk/sifive/sifive-prci.h b/drivers/clk/sifive/sifive-prci.h
> index 91658a88af4e..825a0aef9fd5 100644
> --- a/drivers/clk/sifive/sifive-prci.h
> +++ b/drivers/clk/sifive/sifive-prci.h
> @@ -210,6 +210,9 @@
>
> /* 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)
>
> /*
> * Private structures
> @@ -235,6 +238,7 @@ struct __prci_data {
> * @disable_bypass: fn ptr to code to not bypass the WRPLL (or NULL)
> * @cfg0_offs: WRPLL CFG0 register offset (in bytes) from the PRCI base address
> * @cfg1_offs: WRPLL CFG1 register offset (in bytes) from the PRCI base address
> + * @release_reset: fn ptr to code to release clock reset
> *
> * @enable_bypass and @disable_bypass are used for WRPLL instances
> * that contain a separate external glitchless clock mux downstream
> @@ -246,6 +250,7 @@ struct __prci_wrpll_data {
> void (*disable_bypass)(struct __prci_data *pd);
> u8 cfg0_offs;
> u8 cfg1_offs;
> + void (*release_reset)(struct __prci_data *pd);
> };
>
> /**
> @@ -290,6 +295,9 @@ void sifive_prci_corepllsel_use_corepll(struct __prci_data *pd);
> void sifive_prci_hfpclkpllsel_use_hfclk(struct __prci_data *pd);
> void sifive_prci_hfpclkpllsel_use_hfpclkpll(struct __prci_data *pd);
>
> +/* PROCMONCFG */
> +void sifive_prci_set_procmoncfg(struct __prci_data *pd, u32 val);
> +
> /* Linux clock framework integration */
> long sifive_prci_wrpll_round_rate(struct clk_hw *hw, unsigned long rate,
> unsigned long *parent_rate);
> --
> 2.34.1
>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists