Re: [PATCH 3/3] clk: sifive: prci: Add release_reset hooks for gemgxlpll/cltxpll

[Conor Dooley <conor.dooley@microchip.com>]

[-- Attachment #1: Type: text/plain, Size: 8715 bytes --]

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@gmail.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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]