linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider
@ 2020-12-21 15:06 Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 01/15] ARM: dts: vcu: define indexes for output clocks Michael Tretter
                   ` (15 more replies)
  0 siblings, 16 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

Hello,

the xlnx_vcu soc driver is actually a clock provider of a PLL and four output
clocks created from the PLL via dividers.

This is v2 of the series to transform the driver into a proper clock provider
driver.

The main changes compared to v1 are:

Get rid of the "dummy" clock and remove the manual switching of the mux in the
output clocks. The driver now uses the pll_ref as a bypass clock. As this is
not documented, I am not sure if this is actually the case, but without
another signal for an external clock to the ip core, this seems plausible and
avoids changes to the device tree binding. The reparenting happens
automatically when setting a rate on the output clocks.

Add a few patches to cleanup checkpatch warnings on the driver itself.

Move the entire driver from drivers/soc to drivers/clk, because the driver is
now actually only clock provider driver.

A more detailed changelog is attached to the respective patches.

The series is based on the zynqmp/soc-next branch in the Xilinx downstream
repository [0] which should be merged to mainline soon.

Michael

[0] https://github.com/Xilinx/linux-xlnx/tree/zynqmp/soc-next

Changelog:

v2:
- Remove dummy clock and explicit re-parenting of mux clocks
- Add patches to fix checkpatch warnings
- Move driver from drivers/soc to drivers/clk
- Use clk_parent_data instead of parent_names
- Add missing decoder clocks
- Fix smatch warnings
- Fix kernel-doc

Michael Tretter (15):
  ARM: dts: vcu: define indexes for output clocks
  clk: divider: fix initialization with parent_hw
  soc: xilinx: vcu: drop coreclk from struct xlnx_vcu
  soc: xilinx: vcu: add helper to wait for PLL locked
  soc: xilinx: vcu: add helpers for configuring PLL
  soc: xilinx: vcu: implement PLL disable
  soc: xilinx: vcu: register PLL as fixed rate clock
  soc: xilinx: vcu: implement clock provider for output clocks
  soc: xilinx: vcu: make pll post divider explicit
  soc: xilinx: vcu: make the PLL configurable
  soc: xilinx: vcu: remove calculation of PLL configuration
  soc: xilinx: vcu: use bitfields for register definition
  soc: xilinx: vcu: fix repeated word the in comment
  soc: xilinx: vcu: fix alignment to open parenthesis
  clk: xilinx: move xlnx_vcu clock driver from soc

 drivers/clk/Kconfig                  |   1 +
 drivers/clk/Makefile                 |   1 +
 drivers/clk/clk-divider.c            |   9 +-
 drivers/clk/xilinx/Kconfig           |  19 +
 drivers/clk/xilinx/Makefile          |   2 +
 drivers/clk/xilinx/xlnx_vcu.c        | 742 +++++++++++++++++++++++++++
 drivers/soc/xilinx/Kconfig           |  17 -
 drivers/soc/xilinx/Makefile          |   1 -
 drivers/soc/xilinx/xlnx_vcu.c        | 628 -----------------------
 include/dt-bindings/clock/xlnx-vcu.h |  15 +
 10 files changed, 787 insertions(+), 648 deletions(-)
 create mode 100644 drivers/clk/xilinx/Kconfig
 create mode 100644 drivers/clk/xilinx/Makefile
 create mode 100644 drivers/clk/xilinx/xlnx_vcu.c
 delete mode 100644 drivers/soc/xilinx/xlnx_vcu.c
 create mode 100644 include/dt-bindings/clock/xlnx-vcu.h

-- 
2.20.1


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

* [PATCH v2 01/15] ARM: dts: vcu: define indexes for output clocks
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 02/15] clk: divider: fix initialization with parent_hw Michael Tretter
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

The VCU System-Level Control has 4 output clocks. Define indexes for
these clocks to allow to reference them in the device tree.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Stephen Boyd <sboyd@kernel.org>
---
Changelog:

v2:
- Add "vcu" to commit subject
---
 include/dt-bindings/clock/xlnx-vcu.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 include/dt-bindings/clock/xlnx-vcu.h

diff --git a/include/dt-bindings/clock/xlnx-vcu.h b/include/dt-bindings/clock/xlnx-vcu.h
new file mode 100644
index 000000000000..1ed76b9563b6
--- /dev/null
+++ b/include/dt-bindings/clock/xlnx-vcu.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 Pengutronix, Michael Tretter <kernel@pengutronix.de>
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_XLNX_VCU_H
+#define _DT_BINDINGS_CLOCK_XLNX_VCU_H
+
+#define CLK_XVCU_ENC_CORE		0
+#define CLK_XVCU_ENC_MCU		1
+#define CLK_XVCU_DEC_CORE		2
+#define CLK_XVCU_DEC_MCU		3
+#define CLK_XVCU_NUM_CLOCKS		4
+
+#endif /* _DT_BINDINGS_CLOCK_XLNX_VCU_H */
-- 
2.20.1


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

* [PATCH v2 02/15] clk: divider: fix initialization with parent_hw
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 01/15] ARM: dts: vcu: define indexes for output clocks Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 03/15] soc: xilinx: vcu: drop coreclk from struct xlnx_vcu Michael Tretter
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

If a driver registers a divider clock with a parent_hw instead of the
parent_name, the parent_hw is ignored and the clock does not have a
parent.

Fix this by initializing the parents the same way they are initialized
for clock gates.

Fixes: ff258817137a ("clk: divider: Add support for specifying parents via DT/pointers")
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
Changelog:

v2:
- Add fixes tag
---
 drivers/clk/clk-divider.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 8de12cb0c43d..f32157cb4013 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -493,8 +493,13 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
 	else
 		init.ops = &clk_divider_ops;
 	init.flags = flags;
-	init.parent_names = (parent_name ? &parent_name: NULL);
-	init.num_parents = (parent_name ? 1 : 0);
+	init.parent_names = parent_name ? &parent_name : NULL;
+	init.parent_hws = parent_hw ? &parent_hw : NULL;
+	init.parent_data = parent_data;
+	if (parent_name || parent_hw || parent_data)
+		init.num_parents = 1;
+	else
+		init.num_parents = 0;
 
 	/* struct clk_divider assignments */
 	div->reg = reg;
-- 
2.20.1


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

* [PATCH v2 03/15] soc: xilinx: vcu: drop coreclk from struct xlnx_vcu
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 01/15] ARM: dts: vcu: define indexes for output clocks Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 02/15] clk: divider: fix initialization with parent_hw Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 04/15] soc: xilinx: vcu: add helper to wait for PLL locked Michael Tretter
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

The coreclk field is newer read after being written to xlnx_vcu. Remove
the coreclk field from the xlnx_vcu and use a function local variable
instead.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2: none
---
 drivers/soc/xilinx/xlnx_vcu.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index 14daad4efc58..7da9643820a8 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -73,7 +73,6 @@
  * @aclk: axi clock source
  * @logicore_reg_ba: logicore reg base address
  * @vcu_slcr_ba: vcu_slcr Register base address
- * @coreclk: core clock frequency
  */
 struct xvcu_device {
 	struct device *dev;
@@ -81,7 +80,6 @@ struct xvcu_device {
 	struct clk *aclk;
 	struct regmap *logicore_reg_ba;
 	void __iomem *vcu_slcr_ba;
-	u32 coreclk;
 };
 
 static struct regmap_config vcu_settings_regmap_config = {
@@ -358,10 +356,10 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 		return -EINVAL;
 	}
 
-	xvcu->coreclk = pll_clk / divisor_core;
+	coreclk = pll_clk / divisor_core;
 	mcuclk = pll_clk / divisor_mcu;
 	dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
-	dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
+	dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", coreclk);
 	dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
 
 	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
-- 
2.20.1


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

* [PATCH v2 04/15] soc: xilinx: vcu: add helper to wait for PLL locked
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (2 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 03/15] soc: xilinx: vcu: drop coreclk from struct xlnx_vcu Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 05/15] soc: xilinx: vcu: add helpers for configuring PLL Michael Tretter
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

Extract a helper function to wait until the PLL is locked. Also,
disabling the bypass was buried in the exit path on the wait loop.
Separate the different steps and add a helper function to make the code
more readable.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2: none
---
 drivers/soc/xilinx/xlnx_vcu.c | 46 ++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index 7da9643820a8..0fd8356a3776 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -256,6 +256,22 @@ static void xvcu_write_field_reg(void __iomem *iomem, int offset,
 	xvcu_write(iomem, offset, val);
 }
 
+static int xvcu_pll_wait_for_lock(struct xvcu_device *xvcu)
+{
+	void __iomem *base = xvcu->vcu_slcr_ba;
+	unsigned long timeout;
+	u32 lock_status;
+
+	timeout = jiffies + msecs_to_jiffies(2000);
+	do {
+		lock_status = xvcu_read(base, VCU_PLL_STATUS);
+		if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK)
+			return 0;
+	} while (!time_after(jiffies, timeout));
+
+	return -ETIMEDOUT;
+}
+
 /**
  * xvcu_set_vcu_pll_info - Set the VCU PLL info
  * @xvcu:	Pointer to the xvcu_device structure
@@ -428,8 +444,6 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
  */
 static int xvcu_set_pll(struct xvcu_device *xvcu)
 {
-	u32 lock_status;
-	unsigned long timeout;
 	int ret;
 
 	ret = xvcu_set_vcu_pll_info(xvcu);
@@ -447,24 +461,18 @@ static int xvcu_set_pll(struct xvcu_device *xvcu)
 	xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
 			     0, VCU_PLL_CTRL_RESET_MASK,
 			     VCU_PLL_CTRL_RESET_SHIFT);
-	/*
-	 * Defined the timeout for the max time to wait the
-	 * PLL_STATUS to be locked.
-	 */
-	timeout = jiffies + msecs_to_jiffies(2000);
-	do {
-		lock_status = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_STATUS);
-		if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK) {
-			xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
-					     0, VCU_PLL_CTRL_BYPASS_MASK,
-					     VCU_PLL_CTRL_BYPASS_SHIFT);
-			return 0;
-		}
-	} while (!time_after(jiffies, timeout));
 
-	/* PLL is not locked even after the timeout of the 2sec */
-	dev_err(xvcu->dev, "PLL is not locked\n");
-	return -ETIMEDOUT;
+	ret = xvcu_pll_wait_for_lock(xvcu);
+	if (ret) {
+		dev_err(xvcu->dev, "PLL is not locked\n");
+		return ret;
+	}
+
+	xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+			     0, VCU_PLL_CTRL_BYPASS_MASK,
+			     VCU_PLL_CTRL_BYPASS_SHIFT);
+
+	return ret;
 }
 
 /**
-- 
2.20.1


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

* [PATCH v2 05/15] soc: xilinx: vcu: add helpers for configuring PLL
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (3 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 04/15] soc: xilinx: vcu: add helper to wait for PLL locked Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 06/15] soc: xilinx: vcu: implement PLL disable Michael Tretter
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

The xvcu_set_vcu_pll_info function sets the rate of the PLL and enables
it, which makes it difficult to cleanly convert the driver to the common
clock framework.

Split the function and add separate functions for setting the rate,
enabling the clock and disabling the clock.

Also move the enable of the reference clock from probe to the helper
that enables the PLL.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2: none
---
 drivers/soc/xilinx/xlnx_vcu.c | 171 +++++++++++++++++++++-------------
 1 file changed, 104 insertions(+), 67 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index 0fd8356a3776..ff66551a5966 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -272,6 +272,105 @@ static int xvcu_pll_wait_for_lock(struct xvcu_device *xvcu)
 	return -ETIMEDOUT;
 }
 
+static const struct xvcu_pll_cfg *xvcu_find_cfg(int div)
+{
+	const struct xvcu_pll_cfg *cfg = NULL;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(xvcu_pll_cfg) - 1; i++)
+		if (xvcu_pll_cfg[i].fbdiv == div)
+			cfg = &xvcu_pll_cfg[i];
+
+	return cfg;
+}
+
+static int xvcu_pll_set_div(struct xvcu_device *xvcu, int div)
+{
+	void __iomem *base = xvcu->vcu_slcr_ba;
+	const struct xvcu_pll_cfg *cfg = NULL;
+	u32 vcu_pll_ctrl;
+	u32 cfg_val;
+
+	cfg = xvcu_find_cfg(div);
+	if (!cfg)
+		return -EINVAL;
+
+	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
+	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
+	vcu_pll_ctrl |= (cfg->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
+			 VCU_PLL_CTRL_FBDIV_SHIFT;
+	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
+
+	cfg_val = (cfg->res << VCU_PLL_CFG_RES_SHIFT) |
+		   (cfg->cp << VCU_PLL_CFG_CP_SHIFT) |
+		   (cfg->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
+		   (cfg->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
+		   (cfg->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
+	xvcu_write(base, VCU_PLL_CFG, cfg_val);
+
+	return 0;
+}
+
+static int xvcu_pll_set_rate(struct xvcu_device *xvcu,
+			     unsigned long rate, unsigned long parent_rate)
+{
+	return xvcu_pll_set_div(xvcu, rate / parent_rate);
+}
+
+static int xvcu_pll_enable(struct xvcu_device *xvcu)
+{
+	void __iomem *base = xvcu->vcu_slcr_ba;
+	u32 vcu_pll_ctrl;
+	int ret;
+
+	ret = clk_prepare_enable(xvcu->pll_ref);
+	if (ret) {
+		dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
+		return ret;
+	}
+
+	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
+	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
+			  VCU_PLL_CTRL_POR_IN_SHIFT);
+	vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_POR_IN_MASK) <<
+			 VCU_PLL_CTRL_POR_IN_SHIFT;
+	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
+			  VCU_PLL_CTRL_PWR_POR_SHIFT);
+	vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_PWR_POR_MASK) <<
+			 VCU_PLL_CTRL_PWR_POR_SHIFT;
+	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
+
+	xvcu_write_field_reg(base, VCU_PLL_CTRL,
+			     1, VCU_PLL_CTRL_BYPASS_MASK,
+			     VCU_PLL_CTRL_BYPASS_SHIFT);
+	xvcu_write_field_reg(base, VCU_PLL_CTRL,
+			     1, VCU_PLL_CTRL_RESET_MASK,
+			     VCU_PLL_CTRL_RESET_SHIFT);
+	xvcu_write_field_reg(base, VCU_PLL_CTRL,
+			     0, VCU_PLL_CTRL_RESET_MASK,
+			     VCU_PLL_CTRL_RESET_SHIFT);
+
+	ret = xvcu_pll_wait_for_lock(xvcu);
+	if (ret) {
+		dev_err(xvcu->dev, "PLL is not locked\n");
+		goto err;
+	}
+
+	xvcu_write_field_reg(base, VCU_PLL_CTRL,
+			     0, VCU_PLL_CTRL_BYPASS_MASK,
+			     VCU_PLL_CTRL_BYPASS_SHIFT);
+
+	return ret;
+err:
+	clk_disable_unprepare(xvcu->pll_ref);
+	return ret;
+}
+
+static void xvcu_pll_disable(struct xvcu_device *xvcu)
+{
+	clk_disable_unprepare(xvcu->pll_ref);
+}
+
 /**
  * xvcu_set_vcu_pll_info - Set the VCU PLL info
  * @xvcu:	Pointer to the xvcu_device structure
@@ -292,8 +391,8 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	u32 refclk, coreclk, mcuclk, inte, deci;
 	u32 divisor_mcu, divisor_core, fvco;
 	u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
-	u32 cfg_val, mod, ctrl;
-	int ret, i;
+	u32 mod, ctrl;
+	int i;
 	const struct xvcu_pll_cfg *found = NULL;
 
 	regmap_read(xvcu->logicore_reg_ba, VCU_PLL_CLK, &inte);
@@ -312,19 +411,6 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
 	dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
 
-	clk_disable_unprepare(xvcu->pll_ref);
-	ret = clk_set_rate(xvcu->pll_ref, refclk);
-	if (ret)
-		dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
-
-	ret = clk_prepare_enable(xvcu->pll_ref);
-	if (ret) {
-		dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
-		return ret;
-	}
-
-	refclk = clk_get_rate(xvcu->pll_ref);
-
 	/*
 	 * The divide-by-2 should be always enabled (==1)
 	 * to meet the timing in the design.
@@ -378,19 +464,6 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", coreclk);
 	dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
 
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
-	vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
-			 VCU_PLL_CTRL_FBDIV_SHIFT;
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
-			  VCU_PLL_CTRL_POR_IN_SHIFT);
-	vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_POR_IN_MASK) <<
-			 VCU_PLL_CTRL_POR_IN_SHIFT;
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
-			  VCU_PLL_CTRL_PWR_POR_SHIFT);
-	vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_PWR_POR_MASK) <<
-			 VCU_PLL_CTRL_PWR_POR_SHIFT;
-	xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CTRL, vcu_pll_ctrl);
-
 	/* Set divisor for the core and mcu clock */
 	ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL);
 	ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
@@ -422,15 +495,7 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
 	xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
 
-	/* Set RES, CP, LFHF, LOCK_CNT and LOCK_DLY cfg values */
-	cfg_val = (found->res << VCU_PLL_CFG_RES_SHIFT) |
-		   (found->cp << VCU_PLL_CFG_CP_SHIFT) |
-		   (found->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
-		   (found->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
-		   (found->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
-	xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CFG, cfg_val);
-
-	return 0;
+	return xvcu_pll_set_rate(xvcu, fvco, refclk);
 }
 
 /**
@@ -452,27 +517,7 @@ static int xvcu_set_pll(struct xvcu_device *xvcu)
 		return ret;
 	}
 
-	xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
-			     1, VCU_PLL_CTRL_BYPASS_MASK,
-			     VCU_PLL_CTRL_BYPASS_SHIFT);
-	xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
-			     1, VCU_PLL_CTRL_RESET_MASK,
-			     VCU_PLL_CTRL_RESET_SHIFT);
-	xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
-			     0, VCU_PLL_CTRL_RESET_MASK,
-			     VCU_PLL_CTRL_RESET_SHIFT);
-
-	ret = xvcu_pll_wait_for_lock(xvcu);
-	if (ret) {
-		dev_err(xvcu->dev, "PLL is not locked\n");
-		return ret;
-	}
-
-	xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
-			     0, VCU_PLL_CTRL_BYPASS_MASK,
-			     VCU_PLL_CTRL_BYPASS_SHIFT);
-
-	return ret;
+	return xvcu_pll_enable(xvcu);
 }
 
 /**
@@ -555,12 +600,6 @@ static int xvcu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = clk_prepare_enable(xvcu->pll_ref);
-	if (ret) {
-		dev_err(&pdev->dev, "pll_ref clock enable failed\n");
-		goto error_aclk;
-	}
-
 	/*
 	 * Do the Gasket isolation and put the VCU out of reset
 	 * Bit 0 : Gasket isolation
@@ -580,8 +619,6 @@ static int xvcu_probe(struct platform_device *pdev)
 	return 0;
 
 error_pll_ref:
-	clk_disable_unprepare(xvcu->pll_ref);
-error_aclk:
 	clk_disable_unprepare(xvcu->aclk);
 	return ret;
 }
@@ -605,7 +642,7 @@ static int xvcu_remove(struct platform_device *pdev)
 	/* Add the the Gasket isolation and put the VCU in reset. */
 	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
 
-	clk_disable_unprepare(xvcu->pll_ref);
+	xvcu_pll_disable(xvcu);
 	clk_disable_unprepare(xvcu->aclk);
 
 	return 0;
-- 
2.20.1


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

* [PATCH v2 06/15] soc: xilinx: vcu: implement PLL disable
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (4 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 05/15] soc: xilinx: vcu: add helpers for configuring PLL Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 07/15] soc: xilinx: vcu: register PLL as fixed rate clock Michael Tretter
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

The disabling of the PLL is not fully implemented, because according to
the ZynqMP register reference the RESET, POR_IN and PWR_POR bits have to
be set to bring the PLL into reset.

Set the bits to disable the PLL.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2: none
---
 drivers/soc/xilinx/xlnx_vcu.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index ff66551a5966..34f3299afc0d 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -329,6 +329,10 @@ static int xvcu_pll_enable(struct xvcu_device *xvcu)
 		return ret;
 	}
 
+	xvcu_write_field_reg(base, VCU_PLL_CTRL,
+			     1, VCU_PLL_CTRL_BYPASS_MASK,
+			     VCU_PLL_CTRL_BYPASS_SHIFT);
+
 	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
 	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
 			  VCU_PLL_CTRL_POR_IN_SHIFT);
@@ -340,15 +344,9 @@ static int xvcu_pll_enable(struct xvcu_device *xvcu)
 			 VCU_PLL_CTRL_PWR_POR_SHIFT;
 	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
 
-	xvcu_write_field_reg(base, VCU_PLL_CTRL,
-			     1, VCU_PLL_CTRL_BYPASS_MASK,
-			     VCU_PLL_CTRL_BYPASS_SHIFT);
-	xvcu_write_field_reg(base, VCU_PLL_CTRL,
-			     1, VCU_PLL_CTRL_RESET_MASK,
-			     VCU_PLL_CTRL_RESET_SHIFT);
-	xvcu_write_field_reg(base, VCU_PLL_CTRL,
-			     0, VCU_PLL_CTRL_RESET_MASK,
-			     VCU_PLL_CTRL_RESET_SHIFT);
+	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_RESET_MASK << VCU_PLL_CTRL_RESET_SHIFT);
+	vcu_pll_ctrl |= (0 & VCU_PLL_CTRL_RESET_MASK) << VCU_PLL_CTRL_RESET_SHIFT;
+	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
 
 	ret = xvcu_pll_wait_for_lock(xvcu);
 	if (ret) {
@@ -368,6 +366,18 @@ static int xvcu_pll_enable(struct xvcu_device *xvcu)
 
 static void xvcu_pll_disable(struct xvcu_device *xvcu)
 {
+	void __iomem *base = xvcu->vcu_slcr_ba;
+	u32 vcu_pll_ctrl;
+
+	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
+	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK << VCU_PLL_CTRL_POR_IN_SHIFT);
+	vcu_pll_ctrl |= (1 & VCU_PLL_CTRL_POR_IN_MASK) << VCU_PLL_CTRL_POR_IN_SHIFT;
+	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK << VCU_PLL_CTRL_PWR_POR_SHIFT);
+	vcu_pll_ctrl |= (1 & VCU_PLL_CTRL_PWR_POR_MASK) << VCU_PLL_CTRL_PWR_POR_SHIFT;
+	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_RESET_MASK << VCU_PLL_CTRL_RESET_SHIFT);
+	vcu_pll_ctrl |= (1 & VCU_PLL_CTRL_RESET_MASK) << VCU_PLL_CTRL_RESET_SHIFT;
+	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
+
 	clk_disable_unprepare(xvcu->pll_ref);
 }
 
-- 
2.20.1


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

* [PATCH v2 07/15] soc: xilinx: vcu: register PLL as fixed rate clock
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (5 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 06/15] soc: xilinx: vcu: implement PLL disable Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 08/15] soc: xilinx: vcu: implement clock provider for output clocks Michael Tretter
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

Currently, xvcu_pll_set_rate configures the PLL to a clock rate that is
pre-calculated when probing the driver. To still make the clock
framework aware of the PLL and to allow to configure other clocks based
on the PLL rate, register the PLL as a fixed rate clock.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2:
- Fix kernel-doc of struct xvcu_device
- Add missing clk_hw_unregister_fixed_rate
---
 drivers/soc/xilinx/Kconfig    |  2 +-
 drivers/soc/xilinx/xlnx_vcu.c | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
index 0b1708dae361..9fe703772e5a 100644
--- a/drivers/soc/xilinx/Kconfig
+++ b/drivers/soc/xilinx/Kconfig
@@ -3,7 +3,7 @@ menu "Xilinx SoC drivers"
 
 config XILINX_VCU
 	tristate "Xilinx VCU logicoreIP Init"
-	depends on HAS_IOMEM
+	depends on HAS_IOMEM && COMMON_CLK
 	select REGMAP_MMIO
 	help
 	  Provides the driver to enable and disable the isolation between the
diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index 34f3299afc0d..6a733a181982 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -7,6 +7,7 @@
  * Contacts   Dhaval Shah <dshah@xilinx.com>
  */
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/io.h>
@@ -73,6 +74,7 @@
  * @aclk: axi clock source
  * @logicore_reg_ba: logicore reg base address
  * @vcu_slcr_ba: vcu_slcr Register base address
+ * @pll: handle for the VCU PLL
  */
 struct xvcu_device {
 	struct device *dev;
@@ -80,6 +82,7 @@ struct xvcu_device {
 	struct clk *aclk;
 	struct regmap *logicore_reg_ba;
 	void __iomem *vcu_slcr_ba;
+	struct clk_hw *pll;
 };
 
 static struct regmap_config vcu_settings_regmap_config = {
@@ -403,7 +406,9 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
 	u32 mod, ctrl;
 	int i;
+	int ret;
 	const struct xvcu_pll_cfg *found = NULL;
+	struct clk_hw *hw;
 
 	regmap_read(xvcu->logicore_reg_ba, VCU_PLL_CLK, &inte);
 	regmap_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC, &deci);
@@ -505,7 +510,18 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
 	xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
 
-	return xvcu_pll_set_rate(xvcu, fvco, refclk);
+	ret = xvcu_pll_set_rate(xvcu, fvco, refclk);
+	if (ret)
+		return ret;
+
+	hw = clk_hw_register_fixed_rate(xvcu->dev, "vcu_pll",
+					__clk_get_name(xvcu->pll_ref),
+					0, pll_clk);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	xvcu->pll = hw;
+
+	return 0;
 }
 
 /**
@@ -652,6 +668,7 @@ static int xvcu_remove(struct platform_device *pdev)
 	/* Add the the Gasket isolation and put the VCU in reset. */
 	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
 
+	clk_hw_unregister_fixed_rate(xvcu->pll);
 	xvcu_pll_disable(xvcu);
 	clk_disable_unprepare(xvcu->aclk);
 
-- 
2.20.1


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

* [PATCH v2 08/15] soc: xilinx: vcu: implement clock provider for output clocks
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (6 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 07/15] soc: xilinx: vcu: register PLL as fixed rate clock Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 09/15] soc: xilinx: vcu: make pll post divider explicit Michael Tretter
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

The VCU System-Level Control uses an internal PLL to drive the core and
MCU clock for the allegro encoder and decoder based on an external PL
clock.

In order be able to ensure that the clocks are enabled and to get their
rate from other drivers, the module must implement a clock provider and
register the clocks at the common clock framework. Other drivers are
then able to access the clock via devicetree bindings.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2:
- Fix kernel-doc for struct xvcu_device
- Fix smatch warning regarding wrong returned error pointer
- Use goto for error handling in xvcu_clk_hw_register_leaf
- Use allocated spinlocks
- Use clk_parent_data instead of parent_names
- Add decoder clocks
- Use pll_ref clock as parent in PLL bypass
---
 drivers/soc/xilinx/xlnx_vcu.c | 197 +++++++++++++++++++++++++++-------
 1 file changed, 160 insertions(+), 37 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index 6a733a181982..e38e9c8325a7 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -18,6 +18,8 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 
+#include <dt-bindings/clock/xlnx-vcu.h>
+
 /* vcu slcr registers, bitmask and shift */
 #define VCU_PLL_CTRL			0x24
 #define VCU_PLL_CTRL_RESET_MASK		0x01
@@ -50,11 +52,6 @@
 #define VCU_ENC_MCU_CTRL		0x34
 #define VCU_DEC_CORE_CTRL		0x38
 #define VCU_DEC_MCU_CTRL		0x3c
-#define VCU_PLL_DIVISOR_MASK		0x3f
-#define VCU_PLL_DIVISOR_SHIFT		4
-#define VCU_SRCSEL_MASK			0x01
-#define VCU_SRCSEL_SHIFT		0
-#define VCU_SRCSEL_PLL			1
 
 #define VCU_PLL_STATUS			0x60
 #define VCU_PLL_STATUS_LOCK_STATUS_MASK	0x01
@@ -75,6 +72,7 @@
  * @logicore_reg_ba: logicore reg base address
  * @vcu_slcr_ba: vcu_slcr Register base address
  * @pll: handle for the VCU PLL
+ * @clk_data: clocks provided by the vcu clock provider
  */
 struct xvcu_device {
 	struct device *dev;
@@ -83,6 +81,7 @@ struct xvcu_device {
 	struct regmap *logicore_reg_ba;
 	void __iomem *vcu_slcr_ba;
 	struct clk_hw *pll;
+	struct clk_hw_onecell_data *clk_data;
 };
 
 static struct regmap_config vcu_settings_regmap_config = {
@@ -404,7 +403,7 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	u32 refclk, coreclk, mcuclk, inte, deci;
 	u32 divisor_mcu, divisor_core, fvco;
 	u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
-	u32 mod, ctrl;
+	u32 mod;
 	int i;
 	int ret;
 	const struct xvcu_pll_cfg *found = NULL;
@@ -479,37 +478,6 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", coreclk);
 	dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
 
-	/* Set divisor for the core and mcu clock */
-	ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL);
-	ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
-	ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
-		 VCU_PLL_DIVISOR_SHIFT;
-	ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
-	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
-	xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL, ctrl);
-
-	ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL);
-	ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
-	ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
-		 VCU_PLL_DIVISOR_SHIFT;
-	ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
-	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
-	xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL, ctrl);
-
-	ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL);
-	ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
-	ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
-	ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
-	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
-	xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL, ctrl);
-
-	ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL);
-	ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
-	ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
-	ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
-	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
-	xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
-
 	ret = xvcu_pll_set_rate(xvcu, fvco, refclk);
 	if (ret)
 		return ret;
@@ -546,6 +514,151 @@ static int xvcu_set_pll(struct xvcu_device *xvcu)
 	return xvcu_pll_enable(xvcu);
 }
 
+static struct clk_hw *xvcu_clk_hw_register_leaf(struct device *dev,
+						const char *name,
+						const struct clk_parent_data *parent_data,
+						u8 num_parents,
+						void __iomem *reg)
+{
+	u8 mux_flags = CLK_MUX_ROUND_CLOSEST;
+	u8 divider_flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO |
+			   CLK_DIVIDER_ROUND_CLOSEST;
+	struct clk_hw *mux = NULL;
+	struct clk_hw *divider = NULL;
+	struct clk_hw *gate = NULL;
+	char *name_mux;
+	char *name_div;
+	int err;
+	/* Protect register shared by clocks */
+	spinlock_t *lock;
+
+	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
+	if (!lock)
+		return ERR_PTR(-ENOMEM);
+	spin_lock_init(lock);
+
+	name_mux = devm_kasprintf(dev, GFP_KERNEL, "%s%s", name, "_mux");
+	if (!name_mux)
+		return ERR_PTR(-ENOMEM);
+	mux = clk_hw_register_mux_parent_data(dev, name_mux,
+					      parent_data, num_parents,
+					      CLK_SET_RATE_PARENT,
+					      reg, 0, 1, mux_flags, lock);
+	if (IS_ERR(mux))
+		return mux;
+
+	name_div = devm_kasprintf(dev, GFP_KERNEL, "%s%s", name, "_div");
+	if (!name_div) {
+		err = -ENOMEM;
+		goto unregister_mux;
+	}
+	divider = clk_hw_register_divider_parent_hw(dev, name_div, mux,
+						    CLK_SET_RATE_PARENT,
+						    reg, 4, 6, divider_flags,
+						    lock);
+	if (IS_ERR(divider)) {
+		err = PTR_ERR(divider);
+		goto unregister_mux;
+	}
+
+	gate = clk_hw_register_gate_parent_hw(dev, name, divider,
+					      CLK_SET_RATE_PARENT, reg, 12, 0,
+					      lock);
+	if (IS_ERR(gate)) {
+		err = PTR_ERR(gate);
+		goto unregister_divider;
+	}
+
+	return gate;
+
+unregister_divider:
+	clk_hw_unregister_divider(divider);
+unregister_mux:
+	clk_hw_unregister_mux(mux);
+
+	return ERR_PTR(err);
+}
+
+static void xvcu_clk_hw_unregister_leaf(struct clk_hw *hw)
+{
+	struct clk_hw *gate = hw;
+	struct clk_hw *divider;
+	struct clk_hw *mux;
+
+	if (!gate)
+		return;
+
+	divider = clk_hw_get_parent(gate);
+	clk_hw_unregister_gate(gate);
+	if (!divider)
+		return;
+
+	mux = clk_hw_get_parent(divider);
+	clk_hw_unregister_mux(mux);
+	if (!divider)
+		return;
+
+	clk_hw_unregister_divider(divider);
+}
+
+static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
+{
+	struct device *dev = xvcu->dev;
+	struct clk_parent_data parent_data[2] = { 0 };
+	struct clk_hw_onecell_data *data;
+	struct clk_hw **hws;
+	void __iomem *reg_base = xvcu->vcu_slcr_ba;
+
+	data = devm_kzalloc(dev, struct_size(data, hws, CLK_XVCU_NUM_CLOCKS), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+	data->num = CLK_XVCU_NUM_CLOCKS;
+	hws = data->hws;
+
+	xvcu->clk_data = data;
+
+	parent_data[0].fw_name = "pll_ref";
+	parent_data[1].hw = xvcu->pll;
+
+	hws[CLK_XVCU_ENC_CORE] =
+		xvcu_clk_hw_register_leaf(dev, "venc_core_clk",
+					  parent_data,
+					  ARRAY_SIZE(parent_data),
+					  reg_base + VCU_ENC_CORE_CTRL);
+	hws[CLK_XVCU_ENC_MCU] =
+		xvcu_clk_hw_register_leaf(dev, "venc_mcu_clk",
+					  parent_data,
+					  ARRAY_SIZE(parent_data),
+					  reg_base + VCU_ENC_MCU_CTRL);
+	hws[CLK_XVCU_DEC_CORE] =
+		xvcu_clk_hw_register_leaf(dev, "vdec_core_clk",
+					  parent_data,
+					  ARRAY_SIZE(parent_data),
+					  reg_base + VCU_DEC_CORE_CTRL);
+	hws[CLK_XVCU_DEC_MCU] =
+		xvcu_clk_hw_register_leaf(dev, "vdec_mcu_clk",
+					  parent_data,
+					  ARRAY_SIZE(parent_data),
+					  reg_base + VCU_DEC_MCU_CTRL);
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, data);
+}
+
+static void xvcu_unregister_clock_provider(struct xvcu_device *xvcu)
+{
+	struct clk_hw_onecell_data *data = xvcu->clk_data;
+	struct clk_hw **hws = data->hws;
+
+	if (!IS_ERR_OR_NULL(hws[CLK_XVCU_DEC_MCU]))
+		xvcu_clk_hw_unregister_leaf(hws[CLK_XVCU_DEC_MCU]);
+	if (!IS_ERR_OR_NULL(hws[CLK_XVCU_DEC_CORE]))
+		xvcu_clk_hw_unregister_leaf(hws[CLK_XVCU_DEC_CORE]);
+	if (!IS_ERR_OR_NULL(hws[CLK_XVCU_ENC_MCU]))
+		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]);
+}
+
 /**
  * xvcu_probe - Probe existence of the logicoreIP
  *			and initialize PLL
@@ -640,10 +753,18 @@ static int xvcu_probe(struct platform_device *pdev)
 		goto error_pll_ref;
 	}
 
+	ret = xvcu_register_clock_provider(xvcu);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register clock provider\n");
+		goto error_clk_provider;
+	}
+
 	dev_set_drvdata(&pdev->dev, xvcu);
 
 	return 0;
 
+error_clk_provider:
+	xvcu_unregister_clock_provider(xvcu);
 error_pll_ref:
 	clk_disable_unprepare(xvcu->aclk);
 	return ret;
@@ -665,6 +786,8 @@ static int xvcu_remove(struct platform_device *pdev)
 	if (!xvcu)
 		return -ENODEV;
 
+	xvcu_unregister_clock_provider(xvcu);
+
 	/* Add the the Gasket isolation and put the VCU in reset. */
 	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
 
-- 
2.20.1


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

* [PATCH v2 09/15] soc: xilinx: vcu: make pll post divider explicit
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (7 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 08/15] soc: xilinx: vcu: implement clock provider for output clocks Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2021-01-07 10:01   ` Michal Simek
  2020-12-21 15:06 ` [PATCH v2 10/15] soc: xilinx: vcu: make the PLL configurable Michael Tretter
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

According to the downstream driver documentation due to timing
constraints the output divider of the PLL has to be set to 1/2. Add a
helper function for that check instead of burying the code in one large
setup function.

The bit is undocumented and marked as reserved in the register
reference.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2:
- Use clk_hw instead of name in xvcu_register_pll_post
---
 drivers/soc/xilinx/xlnx_vcu.c | 51 ++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index e38e9c8325a7..6dc58cf58d77 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -81,6 +81,7 @@ struct xvcu_device {
 	struct regmap *logicore_reg_ba;
 	void __iomem *vcu_slcr_ba;
 	struct clk_hw *pll;
+	struct clk_hw *pll_post;
 	struct clk_hw_onecell_data *clk_data;
 };
 
@@ -274,6 +275,29 @@ static int xvcu_pll_wait_for_lock(struct xvcu_device *xvcu)
 	return -ETIMEDOUT;
 }
 
+static struct clk_hw *xvcu_register_pll_post(struct device *dev,
+					     const char *name,
+					     const struct clk_hw *parent_hw,
+					     void __iomem *reg_base)
+{
+	u32 div;
+	u32 vcu_pll_ctrl;
+
+	/*
+	 * The output divider of the PLL must be set to 1/2 to meet the
+	 * timing in the design.
+	 */
+	vcu_pll_ctrl = xvcu_read(reg_base, VCU_PLL_CTRL);
+	div = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
+	div = div & VCU_PLL_CTRL_CLKOUTDIV_MASK;
+	if (div != 1)
+		return ERR_PTR(-EINVAL);
+
+	return clk_hw_register_fixed_factor(dev, "vcu_pll_post",
+					    clk_hw_get_name(parent_hw),
+					    CLK_SET_RATE_PARENT, 1, 2);
+}
+
 static const struct xvcu_pll_cfg *xvcu_find_cfg(int div)
 {
 	const struct xvcu_pll_cfg *cfg = NULL;
@@ -402,7 +426,7 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 {
 	u32 refclk, coreclk, mcuclk, inte, deci;
 	u32 divisor_mcu, divisor_core, fvco;
-	u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
+	u32 pll_clk;
 	u32 mod;
 	int i;
 	int ret;
@@ -425,19 +449,6 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
 	dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
 
-	/*
-	 * The divide-by-2 should be always enabled (==1)
-	 * to meet the timing in the design.
-	 * Otherwise, it's an error
-	 */
-	vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
-	clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
-	clkoutdiv = clkoutdiv & VCU_PLL_CTRL_CLKOUTDIV_MASK;
-	if (clkoutdiv != 1) {
-		dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
-		return -EINVAL;
-	}
-
 	for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i >= 0; i--) {
 		const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
 
@@ -484,7 +495,7 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 
 	hw = clk_hw_register_fixed_rate(xvcu->dev, "vcu_pll",
 					__clk_get_name(xvcu->pll_ref),
-					0, pll_clk);
+					0, fvco);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 	xvcu->pll = hw;
@@ -607,6 +618,7 @@ static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
 	struct clk_parent_data parent_data[2] = { 0 };
 	struct clk_hw_onecell_data *data;
 	struct clk_hw **hws;
+	struct clk_hw *hw;
 	void __iomem *reg_base = xvcu->vcu_slcr_ba;
 
 	data = devm_kzalloc(dev, struct_size(data, hws, CLK_XVCU_NUM_CLOCKS), GFP_KERNEL);
@@ -617,8 +629,13 @@ static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
 
 	xvcu->clk_data = data;
 
+	hw = xvcu_register_pll_post(dev, "vcu_pll_post", xvcu->pll, reg_base);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	xvcu->pll_post = hw;
+
 	parent_data[0].fw_name = "pll_ref";
-	parent_data[1].hw = xvcu->pll;
+	parent_data[1].hw = xvcu->pll_post;
 
 	hws[CLK_XVCU_ENC_CORE] =
 		xvcu_clk_hw_register_leaf(dev, "venc_core_clk",
@@ -657,6 +674,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);
 }
 
 /**
-- 
2.20.1


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

* [PATCH v2 10/15] soc: xilinx: vcu: make the PLL configurable
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (8 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 09/15] soc: xilinx: vcu: make pll post divider explicit Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 11/15] soc: xilinx: vcu: remove calculation of PLL configuration Michael Tretter
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

Do not configure the PLL when probing the driver, but register the clock
in the clock framework and do the configuration based on the respective
callbacks.

This is necessary to allow the consumers, i.e., encoder and decoder
drivers, of the xlnx_vcu clock provider to set the clock rate and
actually enable the clocks without relying on some pre-configuration.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2:
- Remove duplicate xvcu_register_pll call
---
 drivers/soc/xilinx/xlnx_vcu.c | 140 +++++++++++++++++++++++++---------
 1 file changed, 103 insertions(+), 37 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index 6dc58cf58d77..c03075fc8779 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -259,9 +259,18 @@ static void xvcu_write_field_reg(void __iomem *iomem, int offset,
 	xvcu_write(iomem, offset, val);
 }
 
-static int xvcu_pll_wait_for_lock(struct xvcu_device *xvcu)
+#define to_vcu_pll(_hw) container_of(_hw, struct vcu_pll, hw)
+
+struct vcu_pll {
+	struct clk_hw hw;
+	void __iomem *reg_base;
+	unsigned long fvco_min;
+	unsigned long fvco_max;
+};
+
+static int xvcu_pll_wait_for_lock(struct vcu_pll *pll)
 {
-	void __iomem *base = xvcu->vcu_slcr_ba;
+	void __iomem *base = pll->reg_base;
 	unsigned long timeout;
 	u32 lock_status;
 
@@ -310,9 +319,9 @@ static const struct xvcu_pll_cfg *xvcu_find_cfg(int div)
 	return cfg;
 }
 
-static int xvcu_pll_set_div(struct xvcu_device *xvcu, int div)
+static int xvcu_pll_set_div(struct vcu_pll *pll, int div)
 {
-	void __iomem *base = xvcu->vcu_slcr_ba;
+	void __iomem *base = pll->reg_base;
 	const struct xvcu_pll_cfg *cfg = NULL;
 	u32 vcu_pll_ctrl;
 	u32 cfg_val;
@@ -337,24 +346,49 @@ static int xvcu_pll_set_div(struct xvcu_device *xvcu, int div)
 	return 0;
 }
 
-static int xvcu_pll_set_rate(struct xvcu_device *xvcu,
+static long xvcu_pll_round_rate(struct clk_hw *hw,
+				unsigned long rate, unsigned long *parent_rate)
+{
+	struct vcu_pll *pll = to_vcu_pll(hw);
+	unsigned int feedback_div;
+
+	rate = clamp_t(unsigned long, rate, pll->fvco_min, pll->fvco_max);
+
+	feedback_div = DIV_ROUND_CLOSEST_ULL(rate, *parent_rate);
+	feedback_div = clamp_t(unsigned int, feedback_div, 25, 125);
+
+	return *parent_rate * feedback_div;
+}
+
+static unsigned long xvcu_pll_recalc_rate(struct clk_hw *hw,
+					  unsigned long parent_rate)
+{
+	struct vcu_pll *pll = to_vcu_pll(hw);
+	void __iomem *base = pll->reg_base;
+	unsigned int div;
+	u32 vcu_pll_ctrl;
+
+	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
+	div = (vcu_pll_ctrl >> VCU_PLL_CTRL_FBDIV_SHIFT) & VCU_PLL_CTRL_FBDIV_MASK;
+
+	return div * parent_rate;
+}
+
+static int xvcu_pll_set_rate(struct clk_hw *hw,
 			     unsigned long rate, unsigned long parent_rate)
 {
-	return xvcu_pll_set_div(xvcu, rate / parent_rate);
+	struct vcu_pll *pll = to_vcu_pll(hw);
+
+	return xvcu_pll_set_div(pll, rate / parent_rate);
 }
 
-static int xvcu_pll_enable(struct xvcu_device *xvcu)
+static int xvcu_pll_enable(struct clk_hw *hw)
 {
-	void __iomem *base = xvcu->vcu_slcr_ba;
+	struct vcu_pll *pll = to_vcu_pll(hw);
+	void __iomem *base = pll->reg_base;
 	u32 vcu_pll_ctrl;
 	int ret;
 
-	ret = clk_prepare_enable(xvcu->pll_ref);
-	if (ret) {
-		dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
-		return ret;
-	}
-
 	xvcu_write_field_reg(base, VCU_PLL_CTRL,
 			     1, VCU_PLL_CTRL_BYPASS_MASK,
 			     VCU_PLL_CTRL_BYPASS_SHIFT);
@@ -374,9 +408,9 @@ static int xvcu_pll_enable(struct xvcu_device *xvcu)
 	vcu_pll_ctrl |= (0 & VCU_PLL_CTRL_RESET_MASK) << VCU_PLL_CTRL_RESET_SHIFT;
 	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
 
-	ret = xvcu_pll_wait_for_lock(xvcu);
+	ret = xvcu_pll_wait_for_lock(pll);
 	if (ret) {
-		dev_err(xvcu->dev, "PLL is not locked\n");
+		pr_err("VCU PLL is not locked\n");
 		goto err;
 	}
 
@@ -384,15 +418,14 @@ static int xvcu_pll_enable(struct xvcu_device *xvcu)
 			     0, VCU_PLL_CTRL_BYPASS_MASK,
 			     VCU_PLL_CTRL_BYPASS_SHIFT);
 
-	return ret;
 err:
-	clk_disable_unprepare(xvcu->pll_ref);
 	return ret;
 }
 
-static void xvcu_pll_disable(struct xvcu_device *xvcu)
+static void xvcu_pll_disable(struct clk_hw *hw)
 {
-	void __iomem *base = xvcu->vcu_slcr_ba;
+	struct vcu_pll *pll = to_vcu_pll(hw);
+	void __iomem *base = pll->reg_base;
 	u32 vcu_pll_ctrl;
 
 	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
@@ -403,8 +436,49 @@ static void xvcu_pll_disable(struct xvcu_device *xvcu)
 	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_RESET_MASK << VCU_PLL_CTRL_RESET_SHIFT);
 	vcu_pll_ctrl |= (1 & VCU_PLL_CTRL_RESET_MASK) << VCU_PLL_CTRL_RESET_SHIFT;
 	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
+}
+
+static const struct clk_ops vcu_pll_ops = {
+	.enable = xvcu_pll_enable,
+	.disable = xvcu_pll_disable,
+	.round_rate = xvcu_pll_round_rate,
+	.recalc_rate = xvcu_pll_recalc_rate,
+	.set_rate = xvcu_pll_set_rate,
+};
 
-	clk_disable_unprepare(xvcu->pll_ref);
+static struct clk_hw *xvcu_register_pll(struct device *dev,
+					void __iomem *reg_base,
+					const char *name, const char *parent,
+					unsigned long flags)
+{
+	struct vcu_pll *pll;
+	struct clk_hw *hw;
+	struct clk_init_data init;
+	int ret;
+
+	init.name = name;
+	init.parent_names = &parent;
+	init.ops = &vcu_pll_ops;
+	init.num_parents = 1;
+	init.flags = flags;
+
+	pll = devm_kmalloc(dev, sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	pll->hw.init = &init;
+	pll->reg_base = reg_base;
+	pll->fvco_min = FVCO_MIN;
+	pll->fvco_max = FVCO_MAX;
+
+	hw = &pll->hw;
+	ret = devm_clk_hw_register(dev, hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	clk_hw_set_rate_range(hw, pll->fvco_min, pll->fvco_max);
+
+	return hw;
 }
 
 /**
@@ -429,9 +503,7 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	u32 pll_clk;
 	u32 mod;
 	int i;
-	int ret;
 	const struct xvcu_pll_cfg *found = NULL;
-	struct clk_hw *hw;
 
 	regmap_read(xvcu->logicore_reg_ba, VCU_PLL_CLK, &inte);
 	regmap_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC, &deci);
@@ -489,17 +561,6 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
 	dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", coreclk);
 	dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
 
-	ret = xvcu_pll_set_rate(xvcu, fvco, refclk);
-	if (ret)
-		return ret;
-
-	hw = clk_hw_register_fixed_rate(xvcu->dev, "vcu_pll",
-					__clk_get_name(xvcu->pll_ref),
-					0, fvco);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
-	xvcu->pll = hw;
-
 	return 0;
 }
 
@@ -522,7 +583,7 @@ static int xvcu_set_pll(struct xvcu_device *xvcu)
 		return ret;
 	}
 
-	return xvcu_pll_enable(xvcu);
+	return 0;
 }
 
 static struct clk_hw *xvcu_clk_hw_register_leaf(struct device *dev,
@@ -629,6 +690,13 @@ static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
 
 	xvcu->clk_data = data;
 
+	hw = xvcu_register_pll(dev, reg_base,
+			       "vcu_pll", __clk_get_name(xvcu->pll_ref),
+			       CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	xvcu->pll = hw;
+
 	hw = xvcu_register_pll_post(dev, "vcu_pll_post", xvcu->pll, reg_base);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
@@ -810,8 +878,6 @@ static int xvcu_remove(struct platform_device *pdev)
 	/* Add the the Gasket isolation and put the VCU in reset. */
 	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
 
-	clk_hw_unregister_fixed_rate(xvcu->pll);
-	xvcu_pll_disable(xvcu);
 	clk_disable_unprepare(xvcu->aclk);
 
 	return 0;
-- 
2.20.1


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

* [PATCH v2 11/15] soc: xilinx: vcu: remove calculation of PLL configuration
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (9 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 10/15] soc: xilinx: vcu: make the PLL configurable Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 12/15] soc: xilinx: vcu: use bitfields for register definition Michael Tretter
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

As the consumers are now responsible for setting the clock rate via
clock framework, the clock rate is now calculated using round_rate and
the driver does not need to calculate the clock rate beforehand.

Remove the code that calculates the PLL configuration.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2: none
---
 drivers/soc/xilinx/xlnx_vcu.c | 117 ----------------------------------
 1 file changed, 117 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index c03075fc8779..a2de1f8ca7dd 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -59,10 +59,6 @@
 #define MHZ				1000000
 #define FVCO_MIN			(1500U * MHZ)
 #define FVCO_MAX			(3000U * MHZ)
-#define DIVISOR_MIN			0
-#define DIVISOR_MAX			63
-#define FRAC				100
-#define LIMIT				(10 * MHZ)
 
 /**
  * struct xvcu_device - Xilinx VCU init device structure
@@ -481,111 +477,6 @@ static struct clk_hw *xvcu_register_pll(struct device *dev,
 	return hw;
 }
 
-/**
- * xvcu_set_vcu_pll_info - Set the VCU PLL info
- * @xvcu:	Pointer to the xvcu_device structure
- *
- * Programming the VCU PLL based on the user configuration
- * (ref clock freq, core clock freq, mcu clock freq).
- * Core clock frequency has higher priority than mcu clock frequency
- * Errors in following cases
- *    - When mcu or clock clock get from logicoreIP is 0
- *    - When VCU PLL DIV related bits value other than 1
- *    - When proper data not found for given data
- *    - When sis570_1 clocksource related operation failed
- *
- * Return:	Returns status, either success or error+reason
- */
-static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
-{
-	u32 refclk, coreclk, mcuclk, inte, deci;
-	u32 divisor_mcu, divisor_core, fvco;
-	u32 pll_clk;
-	u32 mod;
-	int i;
-	const struct xvcu_pll_cfg *found = NULL;
-
-	regmap_read(xvcu->logicore_reg_ba, VCU_PLL_CLK, &inte);
-	regmap_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC, &deci);
-	regmap_read(xvcu->logicore_reg_ba, VCU_CORE_CLK, &coreclk);
-	coreclk *= MHZ;
-	regmap_read(xvcu->logicore_reg_ba, VCU_MCU_CLK, &mcuclk);
-	mcuclk *= MHZ;
-	if (!mcuclk || !coreclk) {
-		dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
-		return -EINVAL;
-	}
-
-	refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
-	dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
-	dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
-	dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
-
-	for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i >= 0; i--) {
-		const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
-
-		fvco = cfg->fbdiv * refclk;
-		if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
-			pll_clk = fvco / VCU_PLL_DIV2;
-			if (fvco % VCU_PLL_DIV2 != 0)
-				pll_clk++;
-			mod = pll_clk % coreclk;
-			if (mod < LIMIT) {
-				divisor_core = pll_clk / coreclk;
-			} else if (coreclk - mod < LIMIT) {
-				divisor_core = pll_clk / coreclk;
-				divisor_core++;
-			} else {
-				continue;
-			}
-			if (divisor_core >= DIVISOR_MIN &&
-			    divisor_core <= DIVISOR_MAX) {
-				found = cfg;
-				divisor_mcu = pll_clk / mcuclk;
-				mod = pll_clk % mcuclk;
-				if (mcuclk - mod < LIMIT)
-					divisor_mcu++;
-				break;
-			}
-		}
-	}
-
-	if (!found) {
-		dev_err(xvcu->dev, "Invalid clock combination.\n");
-		return -EINVAL;
-	}
-
-	coreclk = pll_clk / divisor_core;
-	mcuclk = pll_clk / divisor_mcu;
-	dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
-	dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", coreclk);
-	dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
-
-	return 0;
-}
-
-/**
- * xvcu_set_pll - PLL init sequence
- * @xvcu:	Pointer to the xvcu_device structure
- *
- * Call the api to set the PLL info and once that is done then
- * init the PLL sequence to make the PLL stable.
- *
- * Return:	Returns status, either success or error+reason
- */
-static int xvcu_set_pll(struct xvcu_device *xvcu)
-{
-	int ret;
-
-	ret = xvcu_set_vcu_pll_info(xvcu);
-	if (ret) {
-		dev_err(xvcu->dev, "failed to set pll info\n");
-		return ret;
-	}
-
-	return 0;
-}
-
 static struct clk_hw *xvcu_clk_hw_register_leaf(struct device *dev,
 						const char *name,
 						const struct clk_parent_data *parent_data,
@@ -833,13 +724,6 @@ static int xvcu_probe(struct platform_device *pdev)
 	 */
 	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
 
-	/* Do the PLL Settings based on the ref clk,core and mcu clk freq */
-	ret = xvcu_set_pll(xvcu);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to set the pll\n");
-		goto error_pll_ref;
-	}
-
 	ret = xvcu_register_clock_provider(xvcu);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register clock provider\n");
@@ -852,7 +736,6 @@ static int xvcu_probe(struct platform_device *pdev)
 
 error_clk_provider:
 	xvcu_unregister_clock_provider(xvcu);
-error_pll_ref:
 	clk_disable_unprepare(xvcu->aclk);
 	return ret;
 }
-- 
2.20.1


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

* [PATCH v2 12/15] soc: xilinx: vcu: use bitfields for register definition
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (10 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 11/15] soc: xilinx: vcu: remove calculation of PLL configuration Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 13/15] soc: xilinx: vcu: fix repeated word the in comment Michael Tretter
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

This makes the register accesses more readable and is closer to what is
usually used in the kernel.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
Changelog:

v2: none
---
 drivers/soc/xilinx/xlnx_vcu.c | 115 ++++++++++------------------------
 1 file changed, 34 insertions(+), 81 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index a2de1f8ca7dd..4ba9b40df983 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -6,6 +6,7 @@
  *
  * Contacts   Dhaval Shah <dshah@xilinx.com>
  */
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/device.h>
@@ -20,41 +21,26 @@
 
 #include <dt-bindings/clock/xlnx-vcu.h>
 
-/* vcu slcr registers, bitmask and shift */
 #define VCU_PLL_CTRL			0x24
-#define VCU_PLL_CTRL_RESET_MASK		0x01
-#define VCU_PLL_CTRL_RESET_SHIFT	0
-#define VCU_PLL_CTRL_BYPASS_MASK	0x01
-#define VCU_PLL_CTRL_BYPASS_SHIFT	3
-#define VCU_PLL_CTRL_FBDIV_MASK		0x7f
-#define VCU_PLL_CTRL_FBDIV_SHIFT	8
-#define VCU_PLL_CTRL_POR_IN_MASK	0x01
-#define VCU_PLL_CTRL_POR_IN_SHIFT	1
-#define VCU_PLL_CTRL_PWR_POR_MASK	0x01
-#define VCU_PLL_CTRL_PWR_POR_SHIFT	2
-#define VCU_PLL_CTRL_CLKOUTDIV_MASK	0x03
-#define VCU_PLL_CTRL_CLKOUTDIV_SHIFT	16
-#define VCU_PLL_CTRL_DEFAULT		0
-#define VCU_PLL_DIV2			2
+#define VCU_PLL_CTRL_RESET		BIT(0)
+#define VCU_PLL_CTRL_POR_IN		BIT(1)
+#define VCU_PLL_CTRL_PWR_POR		BIT(2)
+#define VCU_PLL_CTRL_BYPASS		BIT(3)
+#define VCU_PLL_CTRL_FBDIV		GENMASK(14, 8)
+#define VCU_PLL_CTRL_CLKOUTDIV		GENMASK(18, 16)
 
 #define VCU_PLL_CFG			0x28
-#define VCU_PLL_CFG_RES_MASK		0x0f
-#define VCU_PLL_CFG_RES_SHIFT		0
-#define VCU_PLL_CFG_CP_MASK		0x0f
-#define VCU_PLL_CFG_CP_SHIFT		5
-#define VCU_PLL_CFG_LFHF_MASK		0x03
-#define VCU_PLL_CFG_LFHF_SHIFT		10
-#define VCU_PLL_CFG_LOCK_CNT_MASK	0x03ff
-#define VCU_PLL_CFG_LOCK_CNT_SHIFT	13
-#define VCU_PLL_CFG_LOCK_DLY_MASK	0x7f
-#define VCU_PLL_CFG_LOCK_DLY_SHIFT	25
+#define VCU_PLL_CFG_RES			GENMASK(3, 0)
+#define VCU_PLL_CFG_CP			GENMASK(8, 5)
+#define VCU_PLL_CFG_LFHF		GENMASK(12, 10)
+#define VCU_PLL_CFG_LOCK_CNT		GENMASK(22, 13)
+#define VCU_PLL_CFG_LOCK_DLY		GENMASK(31, 25)
 #define VCU_ENC_CORE_CTRL		0x30
 #define VCU_ENC_MCU_CTRL		0x34
 #define VCU_DEC_CORE_CTRL		0x38
 #define VCU_DEC_MCU_CTRL		0x3c
-
 #define VCU_PLL_STATUS			0x60
-#define VCU_PLL_STATUS_LOCK_STATUS_MASK	0x01
+#define VCU_PLL_STATUS_LOCK_STATUS	BIT(0)
 
 #define MHZ				1000000
 #define FVCO_MIN			(1500U * MHZ)
@@ -236,25 +222,6 @@ static inline void xvcu_write(void __iomem *iomem, u32 offset, u32 value)
 	iowrite32(value, iomem + offset);
 }
 
-/**
- * xvcu_write_field_reg - Write to the vcu reg field
- * @iomem:	vcu reg space base address
- * @offset:	vcu reg offset from base
- * @field:	vcu reg field to write to
- * @mask:	vcu reg mask
- * @shift:	vcu reg number of bits to shift the bitfield
- */
-static void xvcu_write_field_reg(void __iomem *iomem, int offset,
-				 u32 field, u32 mask, int shift)
-{
-	u32 val = xvcu_read(iomem, offset);
-
-	val &= ~(mask << shift);
-	val |= (field & mask) << shift;
-
-	xvcu_write(iomem, offset, val);
-}
-
 #define to_vcu_pll(_hw) container_of(_hw, struct vcu_pll, hw)
 
 struct vcu_pll {
@@ -273,7 +240,7 @@ static int xvcu_pll_wait_for_lock(struct vcu_pll *pll)
 	timeout = jiffies + msecs_to_jiffies(2000);
 	do {
 		lock_status = xvcu_read(base, VCU_PLL_STATUS);
-		if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK)
+		if (lock_status & VCU_PLL_STATUS_LOCK_STATUS)
 			return 0;
 	} while (!time_after(jiffies, timeout));
 
@@ -293,8 +260,7 @@ static struct clk_hw *xvcu_register_pll_post(struct device *dev,
 	 * timing in the design.
 	 */
 	vcu_pll_ctrl = xvcu_read(reg_base, VCU_PLL_CTRL);
-	div = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
-	div = div & VCU_PLL_CTRL_CLKOUTDIV_MASK;
+	div = FIELD_GET(VCU_PLL_CTRL_CLKOUTDIV, vcu_pll_ctrl);
 	if (div != 1)
 		return ERR_PTR(-EINVAL);
 
@@ -327,16 +293,15 @@ static int xvcu_pll_set_div(struct vcu_pll *pll, int div)
 		return -EINVAL;
 
 	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
-	vcu_pll_ctrl |= (cfg->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
-			 VCU_PLL_CTRL_FBDIV_SHIFT;
+	vcu_pll_ctrl &= ~VCU_PLL_CTRL_FBDIV;
+	vcu_pll_ctrl |= FIELD_PREP(VCU_PLL_CTRL_FBDIV, cfg->fbdiv);
 	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
 
-	cfg_val = (cfg->res << VCU_PLL_CFG_RES_SHIFT) |
-		   (cfg->cp << VCU_PLL_CFG_CP_SHIFT) |
-		   (cfg->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
-		   (cfg->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
-		   (cfg->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
+	cfg_val = FIELD_PREP(VCU_PLL_CFG_RES, cfg->res) |
+		  FIELD_PREP(VCU_PLL_CFG_CP, cfg->cp) |
+		  FIELD_PREP(VCU_PLL_CFG_LFHF, cfg->lfhf) |
+		  FIELD_PREP(VCU_PLL_CFG_LOCK_CNT, cfg->lock_cnt) |
+		  FIELD_PREP(VCU_PLL_CFG_LOCK_DLY, cfg->lock_dly);
 	xvcu_write(base, VCU_PLL_CFG, cfg_val);
 
 	return 0;
@@ -365,7 +330,7 @@ static unsigned long xvcu_pll_recalc_rate(struct clk_hw *hw,
 	u32 vcu_pll_ctrl;
 
 	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
-	div = (vcu_pll_ctrl >> VCU_PLL_CTRL_FBDIV_SHIFT) & VCU_PLL_CTRL_FBDIV_MASK;
+	div = FIELD_GET(VCU_PLL_CTRL_FBDIV, vcu_pll_ctrl);
 
 	return div * parent_rate;
 }
@@ -385,23 +350,14 @@ static int xvcu_pll_enable(struct clk_hw *hw)
 	u32 vcu_pll_ctrl;
 	int ret;
 
-	xvcu_write_field_reg(base, VCU_PLL_CTRL,
-			     1, VCU_PLL_CTRL_BYPASS_MASK,
-			     VCU_PLL_CTRL_BYPASS_SHIFT);
-
 	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
-			  VCU_PLL_CTRL_POR_IN_SHIFT);
-	vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_POR_IN_MASK) <<
-			 VCU_PLL_CTRL_POR_IN_SHIFT;
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
-			  VCU_PLL_CTRL_PWR_POR_SHIFT);
-	vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_PWR_POR_MASK) <<
-			 VCU_PLL_CTRL_PWR_POR_SHIFT;
+	vcu_pll_ctrl |= VCU_PLL_CTRL_BYPASS;
 	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
 
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_RESET_MASK << VCU_PLL_CTRL_RESET_SHIFT);
-	vcu_pll_ctrl |= (0 & VCU_PLL_CTRL_RESET_MASK) << VCU_PLL_CTRL_RESET_SHIFT;
+	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
+	vcu_pll_ctrl &= ~VCU_PLL_CTRL_POR_IN;
+	vcu_pll_ctrl &= ~VCU_PLL_CTRL_PWR_POR;
+	vcu_pll_ctrl &= ~VCU_PLL_CTRL_RESET;
 	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
 
 	ret = xvcu_pll_wait_for_lock(pll);
@@ -410,9 +366,9 @@ static int xvcu_pll_enable(struct clk_hw *hw)
 		goto err;
 	}
 
-	xvcu_write_field_reg(base, VCU_PLL_CTRL,
-			     0, VCU_PLL_CTRL_BYPASS_MASK,
-			     VCU_PLL_CTRL_BYPASS_SHIFT);
+	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
+	vcu_pll_ctrl &= ~VCU_PLL_CTRL_BYPASS;
+	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
 
 err:
 	return ret;
@@ -425,12 +381,9 @@ static void xvcu_pll_disable(struct clk_hw *hw)
 	u32 vcu_pll_ctrl;
 
 	vcu_pll_ctrl = xvcu_read(base, VCU_PLL_CTRL);
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK << VCU_PLL_CTRL_POR_IN_SHIFT);
-	vcu_pll_ctrl |= (1 & VCU_PLL_CTRL_POR_IN_MASK) << VCU_PLL_CTRL_POR_IN_SHIFT;
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK << VCU_PLL_CTRL_PWR_POR_SHIFT);
-	vcu_pll_ctrl |= (1 & VCU_PLL_CTRL_PWR_POR_MASK) << VCU_PLL_CTRL_PWR_POR_SHIFT;
-	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_RESET_MASK << VCU_PLL_CTRL_RESET_SHIFT);
-	vcu_pll_ctrl |= (1 & VCU_PLL_CTRL_RESET_MASK) << VCU_PLL_CTRL_RESET_SHIFT;
+	vcu_pll_ctrl |= VCU_PLL_CTRL_POR_IN;
+	vcu_pll_ctrl |= VCU_PLL_CTRL_PWR_POR;
+	vcu_pll_ctrl |= VCU_PLL_CTRL_RESET;
 	xvcu_write(base, VCU_PLL_CTRL, vcu_pll_ctrl);
 }
 
-- 
2.20.1


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

* [PATCH v2 13/15] soc: xilinx: vcu: fix repeated word the in comment
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (11 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 12/15] soc: xilinx: vcu: use bitfields for register definition Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 14/15] soc: xilinx: vcu: fix alignment to open parenthesis Michael Tretter
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

Fixes the following checkpatch warning:

	WARNING: Possible repeated word: 'the'
	#703: FILE: drivers/soc/xilinx/xlnx_vcu.c:703:
	+       /* Add the the Gasket isolation and put the VCU in reset. */

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2:
- New patch
---
 drivers/soc/xilinx/xlnx_vcu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index 4ba9b40df983..74e667dcf82e 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -711,7 +711,7 @@ static int xvcu_remove(struct platform_device *pdev)
 
 	xvcu_unregister_clock_provider(xvcu);
 
-	/* Add the the Gasket isolation and put the VCU in reset. */
+	/* Add the Gasket isolation and put the VCU in reset. */
 	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
 
 	clk_disable_unprepare(xvcu->aclk);
-- 
2.20.1


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

* [PATCH v2 14/15] soc: xilinx: vcu: fix alignment to open parenthesis
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (12 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 13/15] soc: xilinx: vcu: fix repeated word the in comment Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2020-12-21 15:06 ` [PATCH v2 15/15] clk: xilinx: move xlnx_vcu clock driver from soc Michael Tretter
  2021-01-07 10:10 ` [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michal Simek
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

Fixes the following checkpatch check:

	CHECK: Alignment should match open parenthesis
	#610: FILE: drivers/soc/xilinx/xlnx_vcu.c:610:
	+       xvcu->vcu_slcr_ba = devm_ioremap(&pdev->dev, res->start,
	+                                                resource_size(res));

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2:
- New patch
---
 drivers/soc/xilinx/xlnx_vcu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index 74e667dcf82e..bc1efd22970e 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -618,7 +618,7 @@ static int xvcu_probe(struct platform_device *pdev)
 	}
 
 	xvcu->vcu_slcr_ba = devm_ioremap(&pdev->dev, res->start,
-						 resource_size(res));
+					 resource_size(res));
 	if (!xvcu->vcu_slcr_ba) {
 		dev_err(&pdev->dev, "vcu_slcr register mapping failed.\n");
 		return -ENOMEM;
-- 
2.20.1


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

* [PATCH v2 15/15] clk: xilinx: move xlnx_vcu clock driver from soc
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (13 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 14/15] soc: xilinx: vcu: fix alignment to open parenthesis Michael Tretter
@ 2020-12-21 15:06 ` Michael Tretter
  2021-01-07 10:10 ` [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michal Simek
  15 siblings, 0 replies; 18+ messages in thread
From: Michael Tretter @ 2020-12-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, michals, kernel, mturquette, sboyd

The xlnx_vcu driver is actually a clock controller driver which provides
clocks that can be used by a driver for the encoder/decoder units. There
is no reason to keep this driver in soc. Move the driver to clk.

NOTE: The register mapping actually contains registers for AXI
performance monitoring, but these are not used by the driver.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v2:
- New patch
---
 drivers/clk/Kconfig                    |  1 +
 drivers/clk/Makefile                   |  1 +
 drivers/clk/xilinx/Kconfig             | 19 +++++++++++++++++++
 drivers/clk/xilinx/Makefile            |  2 ++
 drivers/{soc => clk}/xilinx/xlnx_vcu.c |  0
 drivers/soc/xilinx/Kconfig             | 17 -----------------
 drivers/soc/xilinx/Makefile            |  1 -
 7 files changed, 23 insertions(+), 18 deletions(-)
 create mode 100644 drivers/clk/xilinx/Kconfig
 create mode 100644 drivers/clk/xilinx/Makefile
 rename drivers/{soc => clk}/xilinx/xlnx_vcu.c (100%)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index c715d4681a0b..f4b13e7c84de 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -383,6 +383,7 @@ source "drivers/clk/tegra/Kconfig"
 source "drivers/clk/ti/Kconfig"
 source "drivers/clk/uniphier/Kconfig"
 source "drivers/clk/x86/Kconfig"
+source "drivers/clk/xilinx/Kconfig"
 source "drivers/clk/zynqmp/Kconfig"
 
 endif
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index da8fcf147eb1..44477b84acef 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -121,6 +121,7 @@ obj-y					+= versatile/
 ifeq ($(CONFIG_COMMON_CLK), y)
 obj-$(CONFIG_X86)			+= x86/
 endif
+obj-y					+= xilinx/
 obj-$(CONFIG_ARCH_ZX)			+= zte/
 obj-$(CONFIG_ARCH_ZYNQ)			+= zynq/
 obj-$(CONFIG_COMMON_CLK_ZYNQMP)         += zynqmp/
diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig
new file mode 100644
index 000000000000..5224114176ed
--- /dev/null
+++ b/drivers/clk/xilinx/Kconfig
@@ -0,0 +1,19 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config XILINX_VCU
+	tristate "Xilinx VCU logicoreIP Init"
+	depends on HAS_IOMEM && COMMON_CLK
+	select REGMAP_MMIO
+	help
+	  Provides the driver to enable and disable the isolation between the
+	  processing system and programmable logic part by using the logicoreIP
+	  register set. This driver also configures the frequency based on the
+	  clock information from the logicoreIP register set.
+
+	  If you say yes here you get support for the logicoreIP.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called xlnx_vcu.
+
diff --git a/drivers/clk/xilinx/Makefile b/drivers/clk/xilinx/Makefile
new file mode 100644
index 000000000000..dee8fd51e303
--- /dev/null
+++ b/drivers/clk/xilinx/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_XILINX_VCU)	+= xlnx_vcu.o
diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/clk/xilinx/xlnx_vcu.c
similarity index 100%
rename from drivers/soc/xilinx/xlnx_vcu.c
rename to drivers/clk/xilinx/xlnx_vcu.c
diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
index 9fe703772e5a..53af9115dc31 100644
--- a/drivers/soc/xilinx/Kconfig
+++ b/drivers/soc/xilinx/Kconfig
@@ -1,23 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 menu "Xilinx SoC drivers"
 
-config XILINX_VCU
-	tristate "Xilinx VCU logicoreIP Init"
-	depends on HAS_IOMEM && COMMON_CLK
-	select REGMAP_MMIO
-	help
-	  Provides the driver to enable and disable the isolation between the
-	  processing system and programmable logic part by using the logicoreIP
-	  register set. This driver also configures the frequency based on the
-	  clock information from the logicoreIP register set.
-
-	  If you say yes here you get support for the logicoreIP.
-
-	  If unsure, say N.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called xlnx_vcu.
-
 config ZYNQMP_POWER
 	bool "Enable Xilinx Zynq MPSoC Power Management driver"
 	depends on PM && ZYNQMP_FIRMWARE
diff --git a/drivers/soc/xilinx/Makefile b/drivers/soc/xilinx/Makefile
index f66bfea5de17..9854e6f6086b 100644
--- a/drivers/soc/xilinx/Makefile
+++ b/drivers/soc/xilinx/Makefile
@@ -1,4 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_XILINX_VCU)	+= xlnx_vcu.o
 obj-$(CONFIG_ZYNQMP_POWER)	+= zynqmp_power.o
 obj-$(CONFIG_ZYNQMP_PM_DOMAINS) += zynqmp_pm_domains.o
-- 
2.20.1


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

* Re: [PATCH v2 09/15] soc: xilinx: vcu: make pll post divider explicit
  2020-12-21 15:06 ` [PATCH v2 09/15] soc: xilinx: vcu: make pll post divider explicit Michael Tretter
@ 2021-01-07 10:01   ` Michal Simek
  0 siblings, 0 replies; 18+ messages in thread
From: Michal Simek @ 2021-01-07 10:01 UTC (permalink / raw)
  To: Michael Tretter, linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, kernel, mturquette, sboyd



On 21. 12. 20 16:06, Michael Tretter wrote:
> According to the downstream driver documentation due to timing
> constraints the output divider of the PLL has to be set to 1/2. Add a
> helper function for that check instead of burying the code in one large
> setup function.
> 
> The bit is undocumented and marked as reserved in the register
> reference.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> Changelog:
> 
> v2:
> - Use clk_hw instead of name in xvcu_register_pll_post
> ---
>  drivers/soc/xilinx/xlnx_vcu.c | 51 ++++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
> index e38e9c8325a7..6dc58cf58d77 100644
> --- a/drivers/soc/xilinx/xlnx_vcu.c
> +++ b/drivers/soc/xilinx/xlnx_vcu.c
> @@ -81,6 +81,7 @@ struct xvcu_device {
>  	struct regmap *logicore_reg_ba;
>  	void __iomem *vcu_slcr_ba;
>  	struct clk_hw *pll;
> +	struct clk_hw *pll_post;

drivers/soc/xilinx/xlnx_vcu.c:86: warning: Function parameter or member
'pll_post' not described in 'xvcu_device'

Thanks,
Michal

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

* Re: [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider
  2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
                   ` (14 preceding siblings ...)
  2020-12-21 15:06 ` [PATCH v2 15/15] clk: xilinx: move xlnx_vcu clock driver from soc Michael Tretter
@ 2021-01-07 10:10 ` Michal Simek
  15 siblings, 0 replies; 18+ messages in thread
From: Michal Simek @ 2021-01-07 10:10 UTC (permalink / raw)
  To: Michael Tretter, linux-arm-kernel, linux-clk
  Cc: rajanv, tejasp, dshah, rvisaval, kernel, mturquette, sboyd

Hi,

On 21. 12. 20 16:06, Michael Tretter wrote:
> Hello,
> 
> the xlnx_vcu soc driver is actually a clock provider of a PLL and four output
> clocks created from the PLL via dividers.
> 
> This is v2 of the series to transform the driver into a proper clock provider
> driver.
> 
> The main changes compared to v1 are:
> 
> Get rid of the "dummy" clock and remove the manual switching of the mux in the
> output clocks. The driver now uses the pll_ref as a bypass clock. As this is
> not documented, I am not sure if this is actually the case, but without
> another signal for an external clock to the ip core, this seems plausible and
> avoids changes to the device tree binding. The reparenting happens
> automatically when setting a rate on the output clocks.
> 
> Add a few patches to cleanup checkpatch warnings on the driver itself.
> 
> Move the entire driver from drivers/soc to drivers/clk, because the driver is
> now actually only clock provider driver.
> 
> A more detailed changelog is attached to the respective patches.
> 
> The series is based on the zynqmp/soc-next branch in the Xilinx downstream
> repository [0] which should be merged to mainline soon.
> 
> Michael
> 
> [0] https://github.com/Xilinx/linux-xlnx/tree/zynqmp/soc-next

Series looks good to me but not clock expert.

Stephen: Please let me know if you want me to take it via my tree.
If yes please ack that 15/15.

For the whole series please add
Acked-by: Michal Simek <michal.simek@xilinx.com>
when that minor kerneldoc issue in 9/15 is fixed.

Thanks,
Michal



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

end of thread, other threads:[~2021-01-07 10:11 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-21 15:06 [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michael Tretter
2020-12-21 15:06 ` [PATCH v2 01/15] ARM: dts: vcu: define indexes for output clocks Michael Tretter
2020-12-21 15:06 ` [PATCH v2 02/15] clk: divider: fix initialization with parent_hw Michael Tretter
2020-12-21 15:06 ` [PATCH v2 03/15] soc: xilinx: vcu: drop coreclk from struct xlnx_vcu Michael Tretter
2020-12-21 15:06 ` [PATCH v2 04/15] soc: xilinx: vcu: add helper to wait for PLL locked Michael Tretter
2020-12-21 15:06 ` [PATCH v2 05/15] soc: xilinx: vcu: add helpers for configuring PLL Michael Tretter
2020-12-21 15:06 ` [PATCH v2 06/15] soc: xilinx: vcu: implement PLL disable Michael Tretter
2020-12-21 15:06 ` [PATCH v2 07/15] soc: xilinx: vcu: register PLL as fixed rate clock Michael Tretter
2020-12-21 15:06 ` [PATCH v2 08/15] soc: xilinx: vcu: implement clock provider for output clocks Michael Tretter
2020-12-21 15:06 ` [PATCH v2 09/15] soc: xilinx: vcu: make pll post divider explicit Michael Tretter
2021-01-07 10:01   ` Michal Simek
2020-12-21 15:06 ` [PATCH v2 10/15] soc: xilinx: vcu: make the PLL configurable Michael Tretter
2020-12-21 15:06 ` [PATCH v2 11/15] soc: xilinx: vcu: remove calculation of PLL configuration Michael Tretter
2020-12-21 15:06 ` [PATCH v2 12/15] soc: xilinx: vcu: use bitfields for register definition Michael Tretter
2020-12-21 15:06 ` [PATCH v2 13/15] soc: xilinx: vcu: fix repeated word the in comment Michael Tretter
2020-12-21 15:06 ` [PATCH v2 14/15] soc: xilinx: vcu: fix alignment to open parenthesis Michael Tretter
2020-12-21 15:06 ` [PATCH v2 15/15] clk: xilinx: move xlnx_vcu clock driver from soc Michael Tretter
2021-01-07 10:10 ` [PATCH v2 00/15] soc: xilinx: vcu: Convert driver to clock provider Michal Simek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).