Linux clock framework development
 help / color / mirror / Atom feed
* [PATCH v2 0/3] clk: xilinx: vcu: Sequence update and couple of fixes
@ 2025-01-02 17:03 Rohit Visavalia
  2025-01-02 17:03 ` [PATCH v2 1/3] clk: xilinx: vcu: unregister pll_post only if registered correctly Rohit Visavalia
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Rohit Visavalia @ 2025-01-02 17:03 UTC (permalink / raw)
  To: mturquette, sboyd, michal.simek, vishal.sagar
  Cc: javier.carrasco.cruz, geert+renesas, u.kleine-koenig, linux-clk,
	linux-arm-kernel, linux-kernel, Rohit Visavalia

This patch series updates VCU init/reset sequence and does 2 driver fixes.
patch1 fixes call trace related to pll_post clk.
Patch2 fixes VCU parent clock issue by preventing pll_ref to be changed.
Patch3 adds optional reset-gpio support.

---
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 (3):
  clk: xilinx: vcu: unregister pll_post only if registered correctly
  clk: xilinx: vcu: don't set pll_ref as parent of VCU(enc/dec) clocks
  clk: xilinx: vcu: Update vcu init/reset sequence

 drivers/clk/xilinx/xlnx_vcu.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/3] clk: xilinx: vcu: unregister pll_post only if registered correctly
  2025-01-02 17:03 [PATCH v2 0/3] clk: xilinx: vcu: Sequence update and couple of fixes Rohit Visavalia
@ 2025-01-02 17:03 ` Rohit Visavalia
  2025-01-02 17:03 ` [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of VCU(enc/dec) clocks Rohit Visavalia
  2025-01-02 17:03 ` [PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence Rohit Visavalia
  2 siblings, 0 replies; 9+ messages in thread
From: Rohit Visavalia @ 2025-01-02 17:03 UTC (permalink / raw)
  To: mturquette, sboyd, michal.simek, vishal.sagar
  Cc: javier.carrasco.cruz, geert+renesas, u.kleine-koenig, linux-clk,
	linux-arm-kernel, linux-kernel, 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>
---
Changes in v2:
  - Changed patches sequence to have patches with "Fixes" as preceding in order
  - Added Fixes tag
  - Link to v1: https://lore.kernel.org/linux-clk/20241226122023.3439559-4-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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of VCU(enc/dec) clocks
  2025-01-02 17:03 [PATCH v2 0/3] clk: xilinx: vcu: Sequence update and couple of fixes Rohit Visavalia
  2025-01-02 17:03 ` [PATCH v2 1/3] clk: xilinx: vcu: unregister pll_post only if registered correctly Rohit Visavalia
@ 2025-01-02 17:03 ` Rohit Visavalia
  2025-01-02 19:28   ` Geert Uytterhoeven
  2025-01-02 17:03 ` [PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence Rohit Visavalia
  2 siblings, 1 reply; 9+ messages in thread
From: Rohit Visavalia @ 2025-01-02 17:03 UTC (permalink / raw)
  To: mturquette, sboyd, michal.simek, vishal.sagar
  Cc: javier.carrasco.cruz, geert+renesas, u.kleine-koenig, linux-clk,
	linux-arm-kernel, linux-kernel, Rohit Visavalia

CCF will try to adjust parent clock to set desire clock frequency of
child clock. So if pll_ref is not a fixed-clock then while setting rate
of enc/dec clocks pll_ref may get change, which may make VCU malfunction.

Signed-off-by: Rohit Visavalia <rohit.visavalia@amd.com>
---
Changes in v2:
  - none
  - Link to v1: https://lore.kernel.org/linux-clk/20241226122023.3439559-3-rohit.visavalia@amd.com
---
 drivers/clk/xilinx/xlnx_vcu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/xilinx/xlnx_vcu.c b/drivers/clk/xilinx/xlnx_vcu.c
index 88b3fd8250c2..c520ae1ba65e 100644
--- a/drivers/clk/xilinx/xlnx_vcu.c
+++ b/drivers/clk/xilinx/xlnx_vcu.c
@@ -547,7 +547,7 @@ static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
 		return PTR_ERR(hw);
 	xvcu->pll_post = hw;
 
-	parent_data[0].fw_name = "pll_ref";
+	parent_data[0].fw_name = "dummy_name";
 	parent_data[1].hw = xvcu->pll_post;
 
 	hws[CLK_XVCU_ENC_CORE] =
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence
  2025-01-02 17:03 [PATCH v2 0/3] clk: xilinx: vcu: Sequence update and couple of fixes Rohit Visavalia
  2025-01-02 17:03 ` [PATCH v2 1/3] clk: xilinx: vcu: unregister pll_post only if registered correctly Rohit Visavalia
  2025-01-02 17:03 ` [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of VCU(enc/dec) clocks Rohit Visavalia
@ 2025-01-02 17:03 ` Rohit Visavalia
  2025-01-02 19:30   ` Geert Uytterhoeven
  2 siblings, 1 reply; 9+ messages in thread
From: Rohit Visavalia @ 2025-01-02 17:03 UTC (permalink / raw)
  To: mturquette, sboyd, michal.simek, vishal.sagar
  Cc: javier.carrasco.cruz, geert+renesas, u.kleine-koenig, linux-clk,
	linux-arm-kernel, linux-kernel, 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>
---
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-2-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 c520ae1ba65e..50f7c3ecb07c 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of VCU(enc/dec) clocks
  2025-01-02 17:03 ` [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of VCU(enc/dec) clocks Rohit Visavalia
@ 2025-01-02 19:28   ` Geert Uytterhoeven
  2025-01-06 10:04     ` Visavalia, Rohit
  0 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2025-01-02 19:28 UTC (permalink / raw)
  To: Rohit Visavalia
  Cc: mturquette, sboyd, michal.simek, vishal.sagar,
	javier.carrasco.cruz, geert+renesas, u.kleine-koenig, linux-clk,
	linux-arm-kernel, linux-kernel

Hi Rohit,

On Thu, Jan 2, 2025 at 6:04 PM Rohit Visavalia <rohit.visavalia@amd.com> wrote:
> CCF will try to adjust parent clock to set desire clock frequency of
> child clock. So if pll_ref is not a fixed-clock then while setting rate
> of enc/dec clocks pll_ref may get change, which may make VCU malfunction.
>
> Signed-off-by: Rohit Visavalia <rohit.visavalia@amd.com>
> ---
> Changes in v2:
>   - none
>   - Link to v1: https://lore.kernel.org/linux-clk/20241226122023.3439559-3-rohit.visavalia@amd.com
> ---
>  drivers/clk/xilinx/xlnx_vcu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/xilinx/xlnx_vcu.c b/drivers/clk/xilinx/xlnx_vcu.c
> index 88b3fd8250c2..c520ae1ba65e 100644
> --- a/drivers/clk/xilinx/xlnx_vcu.c
> +++ b/drivers/clk/xilinx/xlnx_vcu.c
> @@ -547,7 +547,7 @@ static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
>                 return PTR_ERR(hw);
>         xvcu->pll_post = hw;
>
> -       parent_data[0].fw_name = "pll_ref";
> +       parent_data[0].fw_name = "dummy_name";
>         parent_data[1].hw = xvcu->pll_post;
>
>         hws[CLK_XVCU_ENC_CORE] =

You completely ignored Stephen's comment, which suggests to not pas
CLK_SET_RATE_PARENT instead (see xvcu_clk_hw_register_leaf()).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence
  2025-01-02 17:03 ` [PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence Rohit Visavalia
@ 2025-01-02 19:30   ` Geert Uytterhoeven
  2025-01-06 10:04     ` Visavalia, Rohit
  0 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2025-01-02 19:30 UTC (permalink / raw)
  To: Rohit Visavalia
  Cc: mturquette, sboyd, michal.simek, vishal.sagar,
	javier.carrasco.cruz, geert+renesas, u.kleine-koenig, linux-clk,
	linux-arm-kernel, linux-kernel

Hi Rohit,

On Thu, Jan 2, 2025 at 6:04 PM Rohit Visavalia <rohit.visavalia@amd.com> wrote:
> 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>
> ---
> 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

Thanks for the update!

> --- a/drivers/clk/xilinx/xlnx_vcu.c
> +++ b/drivers/clk/xilinx/xlnx_vcu.c
> @@ -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);

This requires updating the DT bindings _first_.

> +       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);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of VCU(enc/dec) clocks
  2025-01-02 19:28   ` Geert Uytterhoeven
@ 2025-01-06 10:04     ` Visavalia, Rohit
  2025-01-28  9:54       ` Visavalia, Rohit
  0 siblings, 1 reply; 9+ messages in thread
From: Visavalia, Rohit @ 2025-01-06 10:04 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: mturquette@baylibre.com, sboyd@kernel.org, Simek, Michal,
	Sagar, Vishal, 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

Hi Geert,

Thank for the review.

>-----Original Message-----
>From: Geert Uytterhoeven <geert@linux-m68k.org>
>Sent: Friday, January 3, 2025 12:58 AM
>To: Visavalia, Rohit <rohit.visavalia@amd.com>
>Cc: mturquette@baylibre.com; sboyd@kernel.org; Simek, Michal
><michal.simek@amd.com>; Sagar, Vishal <vishal.sagar@amd.com>;
>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
>Subject: Re: [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of
>VCU(enc/dec) clocks
>
>Hi Rohit,
>
>On Thu, Jan 2, 2025 at 6:04 PM Rohit Visavalia <rohit.visavalia@amd.com> wrote:
>> CCF will try to adjust parent clock to set desire clock frequency of
>> child clock. So if pll_ref is not a fixed-clock then while setting
>> rate of enc/dec clocks pll_ref may get change, which may make VCU
>malfunction.
>>
>> Signed-off-by: Rohit Visavalia <rohit.visavalia@amd.com>
>> ---
>> Changes in v2:
>>   - none
>>   - Link to v1:
>> https://lore.kernel.org/linux-clk/20241226122023.3439559-3-rohit.visav
>> alia@amd.com
>> ---
>>  drivers/clk/xilinx/xlnx_vcu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/xilinx/xlnx_vcu.c
>> b/drivers/clk/xilinx/xlnx_vcu.c index 88b3fd8250c2..c520ae1ba65e
>> 100644
>> --- a/drivers/clk/xilinx/xlnx_vcu.c
>> +++ b/drivers/clk/xilinx/xlnx_vcu.c
>> @@ -547,7 +547,7 @@ static int xvcu_register_clock_provider(struct
>xvcu_device *xvcu)
>>                 return PTR_ERR(hw);
>>         xvcu->pll_post = hw;
>>
>> -       parent_data[0].fw_name = "pll_ref";
>> +       parent_data[0].fw_name = "dummy_name";
>>         parent_data[1].hw = xvcu->pll_post;
>>
>>         hws[CLK_XVCU_ENC_CORE] =
>
>You completely ignored Stephen's comment, which suggests to not pas
>CLK_SET_RATE_PARENT instead (see xvcu_clk_hw_register_leaf()).

Thanks for pointing this out. Let me take care this in v3 patch series.

>
>Gr{oetje,eeting}s,
>
>                        Geert
>
>--
>Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
>In personal conversations with technical people, I call myself a hacker. But when
>I'm talking to journalists I just say "programmer" or something like that.
>                                -- Linus Torvalds

Thanks
Rohit

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence
  2025-01-02 19:30   ` Geert Uytterhoeven
@ 2025-01-06 10:04     ` Visavalia, Rohit
  0 siblings, 0 replies; 9+ messages in thread
From: Visavalia, Rohit @ 2025-01-06 10:04 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: mturquette@baylibre.com, sboyd@kernel.org, Simek, Michal,
	Sagar, Vishal, 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

Hi Geert,

Thanks for the review.

>-----Original Message-----
>From: Geert Uytterhoeven <geert@linux-m68k.org>
>Sent: Friday, January 3, 2025 1:01 AM
>To: Visavalia, Rohit <rohit.visavalia@amd.com>
>Cc: mturquette@baylibre.com; sboyd@kernel.org; Simek, Michal
><michal.simek@amd.com>; Sagar, Vishal <vishal.sagar@amd.com>;
>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
>Subject: Re: [PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence
>
>Hi Rohit,
>
>On Thu, Jan 2, 2025 at 6:04 PM Rohit Visavalia <rohit.visavalia@amd.com> wrote:
>> 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>
>> ---
>> 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
>
>Thanks for the update!
>
>> --- a/drivers/clk/xilinx/xlnx_vcu.c
>> +++ b/drivers/clk/xilinx/xlnx_vcu.c
>> @@ -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);
>
>This requires updating the DT bindings _first_.

Yes, shared patch for the same.
Link: https://lore.kernel.org/linux-clk/20250102163700.759712-1-rohit.visavalia@amd.com/

>
>> +       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);
>
>Gr{oetje,eeting}s,
>
>                        Geert
>
>--
>Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
>In personal conversations with technical people, I call myself a hacker. But when
>I'm talking to journalists I just say "programmer" or something like that.
>                                -- Linus Torvalds

Thanks
Rohit

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of VCU(enc/dec) clocks
  2025-01-06 10:04     ` Visavalia, Rohit
@ 2025-01-28  9:54       ` Visavalia, Rohit
  0 siblings, 0 replies; 9+ messages in thread
From: Visavalia, Rohit @ 2025-01-28  9:54 UTC (permalink / raw)
  To: Geert Uytterhoeven, sboyd@kernel.org, Stephen Boyd
  Cc: mturquette@baylibre.com, Simek, Michal, Sagar, Vishal,
	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

Hi @Geert Uytterhoeven, @Stephen Boyd <sboyd@kernel.org> 

>-----Original Message-----
>From: Visavalia, Rohit
>Sent: Monday, January 6, 2025 3:34 PM
>To: Geert Uytterhoeven <geert@linux-m68k.org>
>Cc: mturquette@baylibre.com; sboyd@kernel.org; Simek, Michal
><michal.simek@amd.com>; Sagar, Vishal <vishal.sagar@amd.com>;
>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
>Subject: RE: [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of
>VCU(enc/dec) clocks
>
>Hi Geert,
>
>Thank for the review.
>
>>-----Original Message-----
>>From: Geert Uytterhoeven <geert@linux-m68k.org>
>>Sent: Friday, January 3, 2025 12:58 AM
>>To: Visavalia, Rohit <rohit.visavalia@amd.com>
>>Cc: mturquette@baylibre.com; sboyd@kernel.org; Simek, Michal
>><michal.simek@amd.com>; Sagar, Vishal <vishal.sagar@amd.com>;
>>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
>>Subject: Re: [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as
>>parent of
>>VCU(enc/dec) clocks
>>
>>Hi Rohit,
>>
>>On Thu, Jan 2, 2025 at 6:04 PM Rohit Visavalia <rohit.visavalia@amd.com> wrote:
>>> CCF will try to adjust parent clock to set desire clock frequency of
>>> child clock. So if pll_ref is not a fixed-clock then while setting
>>> rate of enc/dec clocks pll_ref may get change, which may make VCU
>>malfunction.
>>>
>>> Signed-off-by: Rohit Visavalia <rohit.visavalia@amd.com>
>>> ---
>>> Changes in v2:
>>>   - none
>>>   - Link to v1:
>>> https://lore.kernel.org/linux-clk/20241226122023.3439559-3-rohit.visa
>>> v
>>> alia@amd.com
>>> ---
>>>  drivers/clk/xilinx/xlnx_vcu.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/xilinx/xlnx_vcu.c
>>> b/drivers/clk/xilinx/xlnx_vcu.c index 88b3fd8250c2..c520ae1ba65e
>>> 100644
>>> --- a/drivers/clk/xilinx/xlnx_vcu.c
>>> +++ b/drivers/clk/xilinx/xlnx_vcu.c
>>> @@ -547,7 +547,7 @@ static int xvcu_register_clock_provider(struct
>>xvcu_device *xvcu)
>>>                 return PTR_ERR(hw);
>>>         xvcu->pll_post = hw;
>>>
>>> -       parent_data[0].fw_name = "pll_ref";
>>> +       parent_data[0].fw_name = "dummy_name";
>>>         parent_data[1].hw = xvcu->pll_post;
>>>
>>>         hws[CLK_XVCU_ENC_CORE] =
>>
>>You completely ignored Stephen's comment, which suggests to not pas
>>CLK_SET_RATE_PARENT instead (see xvcu_clk_hw_register_leaf()).
>
>Thanks for pointing this out. Let me take care this in v3 patch series.

We can drop this patch[PATCH v2 2/3], as original issue was occurring when "pll_ref" clock is being changed to get the best desire clock rate for encoder and decoder. And now with recent changes in Xilinx clock driver(https://github.com/torvalds/linux/commit/b782921ddd7f84f524723090377903f399fdbbcb & https://github.com/torvalds/linux/commit/1fe15be1fb613534ecbac5f8c3f8744f757d237d) encoder and decoder are getting parent as "pll_post" to get best desire clock rate and not changing rate of "pll_ref".

Thanks
Rohit
>
>>
>>Gr{oetje,eeting}s,
>>
>>                        Geert
>>
>>--
>>Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
>>geert@linux-m68k.org
>>
>>In personal conversations with technical people, I call myself a
>>hacker. But when I'm talking to journalists I just say "programmer" or something
>like that.
>>                                -- Linus Torvalds
>
>Thanks
>Rohit

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-01-28  9:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-02 17:03 [PATCH v2 0/3] clk: xilinx: vcu: Sequence update and couple of fixes Rohit Visavalia
2025-01-02 17:03 ` [PATCH v2 1/3] clk: xilinx: vcu: unregister pll_post only if registered correctly Rohit Visavalia
2025-01-02 17:03 ` [PATCH v2 2/3] clk: xilinx: vcu: don't set pll_ref as parent of VCU(enc/dec) clocks Rohit Visavalia
2025-01-02 19:28   ` Geert Uytterhoeven
2025-01-06 10:04     ` Visavalia, Rohit
2025-01-28  9:54       ` Visavalia, Rohit
2025-01-02 17:03 ` [PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence Rohit Visavalia
2025-01-02 19:30   ` Geert Uytterhoeven
2025-01-06 10:04     ` Visavalia, Rohit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox