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