[PATCH v3 0/2] clk: xilinx: vcu: Sequence update and call trace fix

[PATCH v3 0/2] clk: xilinx: vcu: Sequence update and call trace fix

[Rohit Visavalia]

This patch series updates VCU init/reset sequence and does a driver fix.
patch1 fixes call trace related to pll_post clk.
Patch2 adds optional reset-gpio support.

---
Changes in v3:
  - Removed patch https://lore.kernel.org/linux-clk/20250102170359.761670-3-rohit.visavalia@amd.com
    as it is not required with latest xilinx clk driver.
  - Link to v2: https://lore.kernel.org/linux-clk/20250102170359.761670-1-rohit.visavalia@amd.com/

Changes in v2:
  - Changed patches sequence to have patches with "Fixes" as preceding in order
  - Used dev_err_probe()
  - Moved warning to dev_dbg() and updated print with more detail
  - Link to v1: https://lore.kernel.org/linux-clk/20241226122023.3439559-1-rohit.visavalia@amd.com

Rohit Visavalia (2):
  clk: xilinx: vcu: unregister pll_post only if registered correctly
  clk: xilinx: vcu: Update vcu init/reset sequence

 drivers/clk/xilinx/xlnx_vcu.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH v3 1/2] clk: xilinx: vcu: unregister pll_post only if registered correctly

[Rohit Visavalia]

If registration of pll_post is failed, it will be set to NULL or ERR,
unregistering same will fail with following call trace:

Unable to handle kernel NULL pointer dereference at virtual address 008
pc : clk_hw_unregister+0xc/0x20
lr : clk_hw_unregister_fixed_factor+0x18/0x30
sp : ffff800011923850
...
Call trace:
 clk_hw_unregister+0xc/0x20
 clk_hw_unregister_fixed_factor+0x18/0x30
 xvcu_unregister_clock_provider+0xcc/0xf4 [xlnx_vcu]
 xvcu_probe+0x2bc/0x53c [xlnx_vcu]

Fixes: 4472e1849db7 ("soc: xilinx: vcu: make pll post divider explicit")
Signed-off-by: Rohit Visavalia <rohit.visavalia@amd.com>
---
 drivers/clk/xilinx/xlnx_vcu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/xilinx/xlnx_vcu.c b/drivers/clk/xilinx/xlnx_vcu.c
index 81501b48412e..88b3fd8250c2 100644
--- a/drivers/clk/xilinx/xlnx_vcu.c
+++ b/drivers/clk/xilinx/xlnx_vcu.c
@@ -587,8 +587,8 @@ static void xvcu_unregister_clock_provider(struct xvcu_device *xvcu)
 		xvcu_clk_hw_unregister_leaf(hws[CLK_XVCU_ENC_MCU]);
 	if (!IS_ERR_OR_NULL(hws[CLK_XVCU_ENC_CORE]))
 		xvcu_clk_hw_unregister_leaf(hws[CLK_XVCU_ENC_CORE]);
-
-	clk_hw_unregister_fixed_factor(xvcu->pll_post);
+	if (!IS_ERR_OR_NULL(xvcu->pll_post))
+		clk_hw_unregister_fixed_factor(xvcu->pll_post);
 }
 
 /**
-- 
2.25.1

[PATCH v3 2/2] clk: xilinx: vcu: Update vcu init/reset sequence

[Rohit Visavalia]

Updated vcu init/reset sequence as per design changes.
If VCU reset GPIO is available then do assert and de-assert it before
enabling/disabling gasket isolation.
This GPIO is added because gasket isolation will be removed during startup
that requires access to SLCR register space. Post startup, the ownership of
the register interface lies with logiCORE IP.

Signed-off-by: Rohit Visavalia <rohit.visavalia@amd.com>
---
 drivers/clk/xilinx/xlnx_vcu.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/clk/xilinx/xlnx_vcu.c b/drivers/clk/xilinx/xlnx_vcu.c
index 88b3fd8250c2..1ded67bee06c 100644
--- a/drivers/clk/xilinx/xlnx_vcu.c
+++ b/drivers/clk/xilinx/xlnx_vcu.c
@@ -11,6 +11,7 @@
 #include <linux/clk-provider.h>
 #include <linux/device.h>
 #include <linux/errno.h>
+#include <linux/gpio/consumer.h>
 #include <linux/io.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/xlnx-vcu.h>
@@ -51,6 +52,7 @@
  * @dev: Platform device
  * @pll_ref: pll ref clock source
  * @aclk: axi clock source
+ * @reset_gpio: vcu reset gpio
  * @logicore_reg_ba: logicore reg base address
  * @vcu_slcr_ba: vcu_slcr Register base address
  * @pll: handle for the VCU PLL
@@ -61,6 +63,7 @@ struct xvcu_device {
 	struct device *dev;
 	struct clk *pll_ref;
 	struct clk *aclk;
+	struct gpio_desc *reset_gpio;
 	struct regmap *logicore_reg_ba;
 	void __iomem *vcu_slcr_ba;
 	struct clk_hw *pll;
@@ -676,6 +679,24 @@ static int xvcu_probe(struct platform_device *pdev)
 	 * Bit 0 : Gasket isolation
 	 * Bit 1 : put VCU out of reset
 	 */
+	xvcu->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
+						   GPIOD_OUT_LOW);
+	if (IS_ERR(xvcu->reset_gpio)) {
+		ret = PTR_ERR(xvcu->reset_gpio);
+		dev_err_probe(&pdev->dev, ret, "failed to get reset gpio for vcu.\n");
+		goto error_get_gpio;
+	}
+
+	if (xvcu->reset_gpio) {
+		gpiod_set_value(xvcu->reset_gpio, 0);
+		/* min 2 clock cycle of vcu pll_ref, slowest freq is 33.33KHz */
+		usleep_range(60, 120);
+		gpiod_set_value(xvcu->reset_gpio, 1);
+		usleep_range(60, 120);
+	} else {
+		dev_dbg(&pdev->dev, "No reset gpio info found in dts for VCU. This may result in incorrect functionality if VCU isolation is removed after initialization in designs where the VCU reset is driven by gpio.\n");
+	}
+
 	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
 
 	ret = xvcu_register_clock_provider(xvcu);
@@ -690,6 +711,7 @@ static int xvcu_probe(struct platform_device *pdev)
 
 error_clk_provider:
 	xvcu_unregister_clock_provider(xvcu);
+error_get_gpio:
 	clk_disable_unprepare(xvcu->aclk);
 	return ret;
 }
@@ -711,6 +733,13 @@ static void xvcu_remove(struct platform_device *pdev)
 	xvcu_unregister_clock_provider(xvcu);
 
 	/* Add the Gasket isolation and put the VCU in reset. */
+	if (xvcu->reset_gpio) {
+		gpiod_set_value(xvcu->reset_gpio, 0);
+		/* min 2 clock cycle of vcu pll_ref, slowest freq is 33.33KHz */
+		usleep_range(60, 120);
+		gpiod_set_value(xvcu->reset_gpio, 1);
+		usleep_range(60, 120);
+	}
 	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
 
 	clk_disable_unprepare(xvcu->aclk);
-- 
2.25.1

RE: [PATCH v3 0/2] clk: xilinx: vcu: Sequence update and call trace fix

[Visavalia, Rohit]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi All,

Just a gentle reminder regarding this patch series review.

Thanks & Regards
Rohit

>-----Original Message-----
>From: Rohit Visavalia <rohit.visavalia@amd.com>
>Sent: Monday, February 10, 2025 5:06 PM
>To: mturquette@baylibre.com; sboyd@kernel.org; Simek, Michal
><michal.simek@amd.com>; Sagar, Vishal <vishal.sagar@amd.com>
>Cc: javier.carrasco.cruz@gmail.com; geert+renesas@glider.be; u.kleine-
>koenig@baylibre.com; linux-clk@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Visavalia, Rohit
><rohit.visavalia@amd.com>
>Subject: [PATCH v3 0/2] clk: xilinx: vcu: Sequence update and call trace fix
>
>This patch series updates VCU init/reset sequence and does a driver fix.
>patch1 fixes call trace related to pll_post clk.
>Patch2 adds optional reset-gpio support.
>
>---
>Changes in v3:
>  - Removed patch https://lore.kernel.org/linux-clk/20250102170359.761670-3-
>rohit.visavalia@amd.com
>    as it is not required with latest xilinx clk driver.
>  - Link to v2: https://lore.kernel.org/linux-clk/20250102170359.761670-1-
>rohit.visavalia@amd.com/
>
>Changes in v2:
>  - Changed patches sequence to have patches with "Fixes" as preceding in order
>  - Used dev_err_probe()
>  - Moved warning to dev_dbg() and updated print with more detail
>  - Link to v1: https://lore.kernel.org/linux-clk/20241226122023.3439559-1-
>rohit.visavalia@amd.com
>
>Rohit Visavalia (2):
>  clk: xilinx: vcu: unregister pll_post only if registered correctly
>  clk: xilinx: vcu: Update vcu init/reset sequence
>
> drivers/clk/xilinx/xlnx_vcu.c | 33 +++++++++++++++++++++++++++++++--
> 1 file changed, 31 insertions(+), 2 deletions(-)
>
>--
>2.25.1

Re: [PATCH v3 1/2] clk: xilinx: vcu: unregister pll_post only if registered correctly

[Stephen Boyd]

Quoting Rohit Visavalia (2025-02-10 03:36:13)
> If registration of pll_post is failed, it will be set to NULL or ERR,
> unregistering same will fail with following call trace:
> 
> Unable to handle kernel NULL pointer dereference at virtual address 008
> pc : clk_hw_unregister+0xc/0x20
> lr : clk_hw_unregister_fixed_factor+0x18/0x30
> sp : ffff800011923850
> ...
> Call trace:
>  clk_hw_unregister+0xc/0x20
>  clk_hw_unregister_fixed_factor+0x18/0x30
>  xvcu_unregister_clock_provider+0xcc/0xf4 [xlnx_vcu]
>  xvcu_probe+0x2bc/0x53c [xlnx_vcu]
> 
> Fixes: 4472e1849db7 ("soc: xilinx: vcu: make pll post divider explicit")
> Signed-off-by: Rohit Visavalia <rohit.visavalia@amd.com>
> ---

Applied to clk-next

Re: [PATCH v3 2/2] clk: xilinx: vcu: Update vcu init/reset sequence

[Stephen Boyd]

Quoting Rohit Visavalia (2025-02-10 03:36:14)
> Updated vcu init/reset sequence as per design changes.
> If VCU reset GPIO is available then do assert and de-assert it before
> enabling/disabling gasket isolation.
> This GPIO is added because gasket isolation will be removed during startup
> that requires access to SLCR register space. Post startup, the ownership of
> the register interface lies with logiCORE IP.
> 
> Signed-off-by: Rohit Visavalia <rohit.visavalia@amd.com>
> ---

Applied to clk-next