[PATCH 0/3] clk: sifive: Fix chip hang when booting linux directly

[PATCH 0/3] clk: sifive: Fix chip hang when booting linux directly

[Bo Gan]

This patch adds the release_reset hook interface to __prci_wrpll_data.
For gemgxlpll/cltxpll clocks in fu540/fu740, the reset pins also have to
be relased for the device to function properly. This was missing in Linux.
The board (Sifive Unmatched/Unleashed) happened to work because previous
boot stage (u-boot) usually already enables the clocks when trying to boot
from PXE/network, and the release_reset logic is present in u-boot. When
booting directly from firmware (OpenSBI) or when u-boot isn't configured
with networking enabled, the board will hang when cadence/macb driver
starts initializing the device.

Fix that by taking the same logic in u-boot and apply to linux.

Bo Gan (3):
  dt-bindings: reset: sifive: add fu540/fu740 reset indexes
  riscv: dts: sifive: fu740: Use reset index from header
  clk: sifive: prci: Add release_reset hooks for gemgxlpll/cltxpll

 arch/riscv/boot/dts/sifive/fu740-c000.dtsi    |  3 +-
 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 +++++
 include/dt-bindings/reset/sifive-fu540-prci.h | 19 ++++++++++++
 include/dt-bindings/reset/sifive-fu740-prci.h | 19 ++++++++++++
 7 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/reset/sifive-fu540-prci.h
 create mode 100644 include/dt-bindings/reset/sifive-fu740-prci.h

-- 
2.34.1

[PATCH 1/3] dt-bindings: reset: sifive: add fu540/fu740 reset indexes

[Bo Gan]

Add bindings for FU540/FU740 clock/reset controller. The header is taken
from the same path in U-Boot with macros renamed to have FU540/740 prefix.

Signed-off-by: Bo Gan <ganboing@gmail.com>
---
 include/dt-bindings/reset/sifive-fu540-prci.h | 19 +++++++++++++++++++
 include/dt-bindings/reset/sifive-fu740-prci.h | 19 +++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 include/dt-bindings/reset/sifive-fu540-prci.h
 create mode 100644 include/dt-bindings/reset/sifive-fu740-prci.h

diff --git a/include/dt-bindings/reset/sifive-fu540-prci.h b/include/dt-bindings/reset/sifive-fu540-prci.h
new file mode 100644
index 000000000000..dbaf602262d2
--- /dev/null
+++ b/include/dt-bindings/reset/sifive-fu540-prci.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 Sifive, Inc.
+ * Author: Sagar Kadam <sagar.kadam@sifive.com>
+ */
+
+#ifndef __DT_BINDINGS_RESET_SIFIVE_FU540_PRCI_H
+#define __DT_BINDINGS_RESET_SIFIVE_FU540_PRCI_H
+
+/* Reset indexes for use by device tree data and the PRCI driver */
+#define FU540_PRCI_RST_DDR_CTRL_N	0
+#define FU540_PRCI_RST_DDR_AXI_N	1
+#define FU540_PRCI_RST_DDR_AHB_N	2
+#define FU540_PRCI_RST_DDR_PHY_N	3
+/* bit 4 is reserved bit */
+#define FU540_PRCI_RST_RSVD_N		4
+#define FU540_PRCI_RST_GEMGXL_N		5
+
+#endif
diff --git a/include/dt-bindings/reset/sifive-fu740-prci.h b/include/dt-bindings/reset/sifive-fu740-prci.h
new file mode 100644
index 000000000000..74d60ca9f1df
--- /dev/null
+++ b/include/dt-bindings/reset/sifive-fu740-prci.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+/*
+ * Copyright (C) 2020-2021 Sifive, Inc.
+ * Author: Pragnesh Patel <pragnesh.patel@sifive.com>
+ */
+
+#ifndef __DT_BINDINGS_RESET_SIFIVE_FU740_PRCI_H
+#define __DT_BINDINGS_RESET_SIFIVE_FU740_PRCI_H
+
+/* Reset indexes for use by device tree data and the PRCI driver */
+#define FU740_PRCI_RST_DDR_CTRL_N	0
+#define FU740_PRCI_RST_DDR_AXI_N	1
+#define FU740_PRCI_RST_DDR_AHB_N	2
+#define FU740_PRCI_RST_DDR_PHY_N	3
+#define FU740_PRCI_RST_PCIE_POWER_UP_N	4
+#define FU740_PRCI_RST_GEMGXL_N		5
+#define FU740_PRCI_RST_CLTX_N		6
+
+#endif
-- 
2.34.1

[PATCH 2/3] riscv: dts: sifive: fu740: Use reset index from header

[Bo Gan]

The hard-coded index is removed.

Signed-off-by: Bo Gan <ganboing@gmail.com>
---
 arch/riscv/boot/dts/sifive/fu740-c000.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
index 6150f3397bff..a2c09033a9ed 100644
--- a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
+++ b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
@@ -4,6 +4,7 @@
 /dts-v1/;
 
 #include <dt-bindings/clock/sifive-fu740-prci.h>
+#include <dt-bindings/reset/sifive-fu740-prci.h>
 
 / {
 	#address-cells = <2>;
@@ -358,7 +359,7 @@ pcie@e00000000 {
 			clocks = <&prci FU740_PRCI_CLK_PCIE_AUX>;
 			pwren-gpios = <&gpio 5 0>;
 			reset-gpios = <&gpio 8 0>;
-			resets = <&prci 4>;
+			resets = <&prci FU740_PRCI_RST_PCIE_POWER_UP_N>;
 			status = "okay";
 		};
 	};
-- 
2.34.1

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

[Bo Gan]

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.

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

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

[Conor Dooley]

[-- 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 --]

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

[Bo Gan]

Hi Conor,

Thanks for replying so quickly. See inline.

On 8/28/24 00:58, Conor Dooley wrote:
> 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.
> 

That a good idea. I never tried it. It's probably a cleaner solution, and I'll
try it some later time. I used the same logic in u-boot partially because the
way how sifive people coded this up. Specifically, PROCMONCFG is set between
the releases of 2 resets. I assume there's some dependency between those regs.
If reset is triggered from the macb driver side, there's really no proper way
to set PROCMONCFG. Also from the FU740 manual, I can't find any mentioning
about PROCMONCFG. If it's not needed, surely it's better to just let macb do
the resetting. Perhaps I should wait for Sifive folks to fill in the gap.

Thanks!
Bo

<Removed Yash Shah and Pragnesh Patel. Looks like they left Sifive>

>>
>> 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
>>

Re: [PATCH 1/3] dt-bindings: reset: sifive: add fu540/fu740 reset indexes

[Krzysztof Kozlowski]

On 28/08/2024 08:55, Bo Gan wrote:
> Add bindings for FU540/FU740 clock/reset controller. The header is taken
> from the same path in U-Boot with macros renamed to have FU540/740 prefix.
> 
> Signed-off-by: Bo Gan <ganboing@gmail.com>
> ---
>  include/dt-bindings/reset/sifive-fu540-prci.h | 19 +++++++++++++++++++
>  include/dt-bindings/reset/sifive-fu740-prci.h | 19 +++++++++++++++++++
>  2 files changed, 38 insertions(+)
>  create mode 100644 include/dt-bindings/reset/sifive-fu540-prci.h
>  create mode 100644 include/dt-bindings/reset/sifive-fu740-prci.h
> 
> diff --git a/include/dt-bindings/reset/sifive-fu540-prci.h b/include/dt-bindings/reset/sifive-fu540-prci.h
> new file mode 100644
> index 000000000000..dbaf602262d2
> --- /dev/null
> +++ b/include/dt-bindings/reset/sifive-fu540-prci.h

Filename matching compatible.

> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Dual license.


Best regards,
Krzysztof