[PATCH v9 0/7] clk: clk-wizard: clock-wizard: Driver updates

[PATCH v9 0/7] clk: clk-wizard: clock-wizard: Driver updates

[Shubhrajyoti Datta]

In the thread [1] Greg suggested that we move the driver
to the clk from the staging.
Add patches to address the concerns regarding the fractional and
set rate support in the TODO.

The patch set does the following
- Trivial fixes for kernel doc.
- Move the driver to the clk folder
- Add capability to set rate.
- Add fractional support.
- Add support for configurable outputs.
- Make the output names unique so that multiple instances
do not crib.

Changes in the v3:
Added the cover-letter.
Add patches for rate setting and fractional support
Add patches for warning.
Remove the driver from staging as suggested

v4:
Reorder the patches.
Merge the CLK_IS_BASIC patch.
Add the yaml form of binding document

v5:
Fix a mismerge

v6:
Fix the yaml warning
use poll timedout

v7:
Binding doc updates
Use common divisor function.

v8:
Fix Robs comments

v9:
Fix device tree warnings

[1] https://spinics.net/lists/linux-driver-devel/msg117326.html

Shubhrajyoti Datta (7):
  dt-bindings: add documentation of xilinx clocking wizard
  clk: clock-wizard: Add the clockwizard to clk directory
  clk: clock-wizard: Fix kernel-doc warning
  clk: clock-wizard: Add support for dynamic reconfiguration
  clk: clock-wizard: Add support for fractional support
  clk: clock-wizard: Remove the hardcoding of the clock outputs
  clk: clock-wizard: Update the fixed factor divisors

 .../bindings/clock/xlnx,clocking-wizard.yaml       |  65 ++
 drivers/clk/Kconfig                                |   9 +
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-xlnx-clock-wizard.c                | 689 +++++++++++++++++++++
 drivers/staging/Kconfig                            |   2 -
 drivers/staging/Makefile                           |   1 -
 drivers/staging/clocking-wizard/Kconfig            |  10 -
 drivers/staging/clocking-wizard/Makefile           |   2 -
 drivers/staging/clocking-wizard/TODO               |  12 -
 .../clocking-wizard/clk-xlnx-clock-wizard.c        | 333 ----------
 drivers/staging/clocking-wizard/dt-binding.txt     |  30 -
 11 files changed, 764 insertions(+), 390 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
 create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c
 delete mode 100644 drivers/staging/clocking-wizard/Kconfig
 delete mode 100644 drivers/staging/clocking-wizard/Makefile
 delete mode 100644 drivers/staging/clocking-wizard/TODO
 delete mode 100644 drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
 delete mode 100644 drivers/staging/clocking-wizard/dt-binding.txt

-- 
2.1.1

[PATCH v9 1/7] dt-bindings: add documentation of xilinx clocking wizard

[Shubhrajyoti Datta]

Add the devicetree binding for the xilinx clocking wizard.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v6:
Fix a yaml warning
v7:
Add vendor prefix speed-grade
v8:
Fix the warnings
v9:
Fix the warnings

 .../bindings/clock/xlnx,clocking-wizard.yaml       | 65 ++++++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml

diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
new file mode 100644
index 0000000..d209140
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Xilinx clocking wizard
+
+maintainers:
+  - Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
+
+description:
+  The clocking wizard is a soft ip clocking block of Xilinx versal. It
+  reads required input clock frequencies from the devicetree and acts as clock
+  clock output.
+
+properties:
+  compatible:
+    const: xlnx,clocking-wizard
+
+  reg:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 1
+
+  clocks:
+    items:
+      - description: clock input
+      - description: axi clock
+
+  clock-names:
+    items:
+      - const: clk_in1
+      - const: s_axi_aclk
+
+  clock-output-names: true
+
+  xlnx,speed-grade:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2, 3]
+    description:
+      Speed grade of the device.
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - clocks
+  - clock-names
+  - xlnx,speed-grade
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller  {
+        compatible = "xlnx,clocking-wizard";
+        reg = <0xb0000000 0x10000>;
+        #clock-cells = <1>;
+        xlnx,speed-grade = <1>;
+        clock-names = "clk_in1", "s_axi_aclk";
+        clocks = <&clkc 15>, <&clkc 15>;
+    };
+...
-- 
2.1.1

[PATCH v9 2/7] clk: clock-wizard: Add the clockwizard to clk directory

[Shubhrajyoti Datta]

Add clocking wizard driver to clk.
And delete the driver from the staging as it is in drivers/clk.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v7:
Combined the patch for deletion and add of the driver
dropping the ack from Greg for the staging as it is a combined patch.
Add vendor prefix to speedgrade
v8:
No change
v9:
No change

 drivers/clk/Kconfig                                |  9 +++++++
 drivers/clk/Makefile                               |  1 +
 .../clk-xlnx-clock-wizard.c                        |  6 +++--
 drivers/staging/Kconfig                            |  2 --
 drivers/staging/Makefile                           |  1 -
 drivers/staging/clocking-wizard/Kconfig            | 10 --------
 drivers/staging/clocking-wizard/Makefile           |  2 --
 drivers/staging/clocking-wizard/TODO               | 12 ---------
 drivers/staging/clocking-wizard/dt-binding.txt     | 30 ----------------------
 9 files changed, 14 insertions(+), 59 deletions(-)
 rename drivers/{staging/clocking-wizard => clk}/clk-xlnx-clock-wizard.c (98%)
 delete mode 100644 drivers/staging/clocking-wizard/Kconfig
 delete mode 100644 drivers/staging/clocking-wizard/Makefile
 delete mode 100644 drivers/staging/clocking-wizard/TODO
 delete mode 100644 drivers/staging/clocking-wizard/dt-binding.txt

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index c715d46..d210ed2 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -359,6 +359,15 @@ config COMMON_CLK_FIXED_MMIO
 	help
 	  Support for Memory Mapped IO Fixed clocks
 
+config COMMON_CLK_XLNX_CLKWZRD
+	tristate "Xilinx Clocking Wizard"
+	depends on COMMON_CLK && OF
+	help
+	  Support for the Xilinx Clocking Wizard IP core clock generator.
+	  Adds support for clocking wizard and compatible.
+	  This driver supports the Xilinx clocking wizard programmable clock
+	  synthesizer. The number of output is configurable in the design.
+
 source "drivers/clk/actions/Kconfig"
 source "drivers/clk/analogbits/Kconfig"
 source "drivers/clk/baikal-t1/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index da8fcf1..1ad6414 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_ARCH_VT8500)		+= clk-vt8500.o
 obj-$(CONFIG_COMMON_CLK_VC5)		+= clk-versaclock5.o
 obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
 obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o
+obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD)	+= clk-xlnx-clock-wizard.o
 
 # please keep this section sorted lexicographically by directory path name
 obj-y					+= actions/
diff --git a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
similarity index 98%
rename from drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
rename to drivers/clk/clk-xlnx-clock-wizard.c
index e52a64b..1bab68e 100644
--- a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -2,9 +2,11 @@
 /*
  * Xilinx 'Clocking Wizard' driver
  *
- *  Copyright (C) 2013 - 2014 Xilinx
+ *  Copyright (C) 2013 - 2020 Xilinx
  *
  *  Sören Brinkmann <soren.brinkmann@xilinx.com>
+ *  Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
+ *
  */
 
 #include <linux/platform_device.h>
@@ -146,7 +148,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	if (IS_ERR(clk_wzrd->base))
 		return PTR_ERR(clk_wzrd->base);
 
-	ret = of_property_read_u32(np, "speed-grade", &clk_wzrd->speed_grade);
+	ret = of_property_read_u32(np, "xlnx,speed-grade", &clk_wzrd->speed_grade);
 	if (!ret) {
 		if (clk_wzrd->speed_grade < 1 || clk_wzrd->speed_grade > 3) {
 			dev_warn(&pdev->dev, "invalid speed grade '%d'\n",
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 2d03104..38a9287 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -76,8 +76,6 @@ source "drivers/staging/gs_fpgaboot/Kconfig"
 
 source "drivers/staging/unisys/Kconfig"
 
-source "drivers/staging/clocking-wizard/Kconfig"
-
 source "drivers/staging/fbtft/Kconfig"
 
 source "drivers/staging/fsl-dpaa2/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 757a892..58bceb8 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -28,7 +28,6 @@ obj-$(CONFIG_FIREWIRE_SERIAL)	+= fwserial/
 obj-$(CONFIG_GOLDFISH)		+= goldfish/
 obj-$(CONFIG_GS_FPGABOOT)	+= gs_fpgaboot/
 obj-$(CONFIG_UNISYSSPAR)	+= unisys/
-obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD)	+= clocking-wizard/
 obj-$(CONFIG_FB_TFT)		+= fbtft/
 obj-$(CONFIG_FSL_DPAA2)		+= fsl-dpaa2/
 obj-$(CONFIG_MOST)		+= most/
diff --git a/drivers/staging/clocking-wizard/Kconfig b/drivers/staging/clocking-wizard/Kconfig
deleted file mode 100644
index 69cf514..0000000
--- a/drivers/staging/clocking-wizard/Kconfig
+++ /dev/null
@@ -1,10 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-#
-# Xilinx Clocking Wizard Driver
-#
-
-config COMMON_CLK_XLNX_CLKWZRD
-	tristate "Xilinx Clocking Wizard"
-	depends on COMMON_CLK && OF && IOMEM
-	help
-	  Support for the Xilinx Clocking Wizard IP core clock generator.
diff --git a/drivers/staging/clocking-wizard/Makefile b/drivers/staging/clocking-wizard/Makefile
deleted file mode 100644
index b1f9152..0000000
--- a/drivers/staging/clocking-wizard/Makefile
+++ /dev/null
@@ -1,2 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD)	+= clk-xlnx-clock-wizard.o
diff --git a/drivers/staging/clocking-wizard/TODO b/drivers/staging/clocking-wizard/TODO
deleted file mode 100644
index ebe99db..0000000
--- a/drivers/staging/clocking-wizard/TODO
+++ /dev/null
@@ -1,12 +0,0 @@
-TODO:
-	- support for fractional multiplier
-	- support for fractional divider (output 0 only)
-	- support for set_rate() operations (may benefit from Stephen Boyd's
-	  refactoring of the clk primitives: https://lkml.org/lkml/2014/9/5/766)
-	- review arithmetic
-	  - overflow after multiplication?
-	  - maximize accuracy before divisions
-
-Patches to:
-	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-	Sören Brinkmann <soren.brinkmann@xilinx.com>
diff --git a/drivers/staging/clocking-wizard/dt-binding.txt b/drivers/staging/clocking-wizard/dt-binding.txt
deleted file mode 100644
index efb67ff..0000000
--- a/drivers/staging/clocking-wizard/dt-binding.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-Binding for Xilinx Clocking Wizard IP Core
-
-This binding uses the common clock binding[1]. Details about the devices can be
-found in the product guide[2].
-
-[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
-[2] Clocking Wizard Product Guide
-https://www.xilinx.com/support/documentation/ip_documentation/clk_wiz/v5_1/pg065-clk-wiz.pdf
-
-Required properties:
- - compatible: Must be 'xlnx,clocking-wizard'
- - reg: Base and size of the cores register space
- - clocks: Handle to input clock
- - clock-names: Tuple containing 'clk_in1' and 's_axi_aclk'
- - clock-output-names: Names for the output clocks
-
-Optional properties:
- - speed-grade: Speed grade of the device (valid values are 1..3)
-
-Example:
-	clock-generator@40040000 {
-		reg = <0x40040000 0x1000>;
-		compatible = "xlnx,clocking-wizard";
-		speed-grade = <1>;
-		clock-names = "clk_in1", "s_axi_aclk";
-		clocks = <&clkc 15>, <&clkc 15>;
-		clock-output-names = "clk_out0", "clk_out1", "clk_out2",
-				     "clk_out3", "clk_out4", "clk_out5",
-				     "clk_out6", "clk_out7";
-	};
-- 
2.1.1

[PATCH v9 3/7] clk: clock-wizard: Fix kernel-doc warning

[Shubhrajyoti Datta]

Update description for the clocking wizard structure

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/clk/clk-xlnx-clock-wizard.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index 1bab68e..fb2d555 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -40,7 +40,8 @@ enum clk_wzrd_int_clks {
 };
 
 /**
- * struct clk_wzrd:
+ * struct clk_wzrd - Clock wizard private data structure
+ *
  * @clk_data:		Clock data
  * @nb:			Notifier block
  * @base:		Memory base
-- 
2.1.1

[PATCH v9 4/7] clk: clock-wizard: Add support for dynamic reconfiguration

[Shubhrajyoti Datta]

The patch adds support for dynamic reconfiguration of clock output rate.
Output clocks are registered as dividers and set rate callback function
is used for dynamic reconfiguration.

Based on the initial work from Chirag.

Signed-off-by: Chirag Parekh <chirag.parekh@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v6:
Remove the typecast.
use min for capping frequency.
use polled timeout

v7:
Use devm functions
Add the spinlock

 drivers/clk/clk-xlnx-clock-wizard.c | 181 ++++++++++++++++++++++++++++++++++--
 1 file changed, 175 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index fb2d555..5581b24 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/iopoll.h>
 
 #define WZRD_NUM_OUTPUTS	7
 #define WZRD_ACLK_MAX_FREQ	250000000UL
@@ -31,8 +32,23 @@
 #define WZRD_DIVCLK_DIVIDE_SHIFT	0
 #define WZRD_DIVCLK_DIVIDE_MASK		(0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
 #define WZRD_CLKOUT_DIVIDE_SHIFT	0
+#define WZRD_CLKOUT_DIVIDE_WIDTH	8
 #define WZRD_CLKOUT_DIVIDE_MASK		(0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
 
+#define WZRD_DR_MAX_INT_DIV_VALUE	255
+#define WZRD_DR_NUM_RETRIES		10000
+#define WZRD_DR_STATUS_REG_OFFSET	0x04
+#define WZRD_DR_LOCK_BIT_MASK		0x00000001
+#define WZRD_DR_INIT_REG_OFFSET		0x25C
+#define WZRD_DR_DIV_TO_PHASE_OFFSET	4
+#define WZRD_DR_BEGIN_DYNA_RECONF	0x03
+
+/* Get the mask from width */
+#define div_mask(width)			((1 << (width)) - 1)
+
+/* Extract divider instance from clock hardware instance */
+#define to_clk_wzrd_divider(_hw) container_of(_hw, struct clk_wzrd_divider, hw)
+
 enum clk_wzrd_int_clks {
 	wzrd_clk_mul,
 	wzrd_clk_mul_div,
@@ -64,6 +80,29 @@ struct clk_wzrd {
 	bool suspended;
 };
 
+/**
+ * struct clk_wzrd_divider - clock divider specific to clk_wzrd
+ *
+ * @hw:		handle between common and hardware-specific interfaces
+ * @base:	base address of register containing the divider
+ * @offset:	offset address of register containing the divider
+ * @shift:	shift to the divider bit field
+ * @width:	width of the divider bit field
+ * @flags:	clk_wzrd divider flags
+ * @table:	array of value/divider pairs, last entry should have div = 0
+ * @lock:	register lock
+ */
+struct clk_wzrd_divider {
+	struct clk_hw hw;
+	void __iomem *base;
+	u16 offset;
+	u8 shift;
+	u8 width;
+	u8 flags;
+	const struct clk_div_table *table;
+	spinlock_t *lock;  /* divider lock */
+};
+
 #define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb)
 
 /* maximum frequencies for input/output clocks per speed grade */
@@ -73,6 +112,132 @@ static const unsigned long clk_wzrd_max_freq[] = {
 	1066000000UL
 };
 
+/* spin lock variable for clk_wzrd */
+static DEFINE_SPINLOCK(clkwzrd_lock);
+
+static unsigned long clk_wzrd_recalc_rate(struct clk_hw *hw,
+					  unsigned long parent_rate)
+{
+	struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
+	void __iomem *div_addr = divider->base + divider->offset;
+	unsigned int val;
+
+	val = readl(div_addr) >> divider->shift;
+	val &= div_mask(divider->width);
+
+	return divider_recalc_rate(hw, parent_rate, val, divider->table,
+			divider->flags, divider->width);
+}
+
+static int clk_wzrd_dynamic_reconfig(struct clk_hw *hw, unsigned long rate,
+				     unsigned long parent_rate)
+{
+	int err = 0;
+	u32 value;
+	unsigned long flags = 0;
+	struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
+	void __iomem *div_addr = divider->base + divider->offset;
+
+	if (divider->lock)
+		spin_lock_irqsave(divider->lock, flags);
+	else
+		__acquire(divider->lock);
+
+	value = DIV_ROUND_CLOSEST(parent_rate, rate);
+
+	/* Cap the value to max */
+	min_t(u32, value, WZRD_DR_MAX_INT_DIV_VALUE);
+
+	/* Set divisor and clear phase offset */
+	writel(value, div_addr);
+	writel(0x00, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET);
+
+	/* Check status register */
+	err = readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET,
+				 value, value & WZRD_DR_LOCK_BIT_MASK,
+				 WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
+	if (err)
+		goto err_reconfig;
+
+	/* Initiate reconfiguration */
+	writel(WZRD_DR_BEGIN_DYNA_RECONF,
+	       divider->base + WZRD_DR_INIT_REG_OFFSET);
+
+	/* Check status register */
+	err = readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET,
+				 value, value & WZRD_DR_LOCK_BIT_MASK,
+				 WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
+err_reconfig:
+	if (divider->lock)
+		spin_unlock_irqrestore(divider->lock, flags);
+	else
+		__release(divider->lock);
+	return err;
+}
+
+static long clk_wzrd_round_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long *prate)
+{
+	u8 div;
+
+	/*
+	 * since we don't change parent rate we just round rate to closest
+	 * achievable
+	 */
+	div = DIV_ROUND_CLOSEST(*prate, rate);
+
+	return (*prate / div);
+}
+
+static const struct clk_ops clk_wzrd_clk_divider_ops = {
+	.round_rate = clk_wzrd_round_rate,
+	.set_rate = clk_wzrd_dynamic_reconfig,
+	.recalc_rate = clk_wzrd_recalc_rate,
+};
+
+static struct clk *clk_wzrd_register_divider(struct device *dev,
+					     const char *name,
+					     const char *parent_name,
+					     unsigned long flags,
+					     void __iomem *base, u16 offset,
+					     u8 shift, u8 width,
+					     u8 clk_divider_flags,
+					     const struct clk_div_table *table,
+					     spinlock_t *lock)
+{
+	struct clk_wzrd_divider *div;
+	struct clk_hw *hw;
+	struct clk_init_data init;
+	int ret;
+
+	div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
+	if (!div)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &clk_wzrd_clk_divider_ops;
+	init.flags = flags;
+	init.parent_names =  &parent_name;
+	init.num_parents =  1;
+
+	div->base = base;
+	div->offset = offset;
+	div->shift = shift;
+	div->width = width;
+	div->flags = clk_divider_flags;
+	div->lock = lock;
+	div->hw.init = &init;
+	div->table = table;
+
+	/* register the clock */
+	hw = &div->hw;
+	ret = devm_clk_hw_register(dev, hw);
+	if (ret)
+		hw = ERR_PTR(ret);
+
+	return hw->clk;
+}
+
 static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
 				 void *data)
 {
@@ -223,7 +388,8 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_fixed_factor
 			(&pdev->dev, clk_name,
 			 __clk_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
-			 0, 1, reg);
+			flags, ctrl_reg, 0, 8, CLK_DIVIDER_ONE_BASED |
+			CLK_DIVIDER_ALLOW_ZERO, &clkwzrd_lock);
 	if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div])) {
 		dev_err(&pdev->dev, "unable to register divider clock\n");
 		ret = PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div]);
@@ -241,11 +407,14 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 			ret = -EINVAL;
 			goto err_rm_int_clks;
 		}
-		reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2) + i * 12);
-		reg &= WZRD_CLKOUT_DIVIDE_MASK;
-		reg >>= WZRD_CLKOUT_DIVIDE_SHIFT;
-		clk_wzrd->clkout[i] = clk_register_fixed_factor
-			(&pdev->dev, clkout_name, clk_name, 0, 1, reg);
+		clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev,
+								clkout_name,
+				clk_name, 0,
+				clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
+				WZRD_CLKOUT_DIVIDE_SHIFT,
+				WZRD_CLKOUT_DIVIDE_WIDTH,
+				CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
+				NULL, &clkwzrd_lock);
 		if (IS_ERR(clk_wzrd->clkout[i])) {
 			int j;
 
-- 
2.1.1

[PATCH v9 5/7] clk: clock-wizard: Add support for fractional support

[Shubhrajyoti Datta]

Currently the set rate granularity is to integral divisors.
Add support for the fractional divisors.
Only the first output0 is fractional in the hardware.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v7:
Remove unnecessary comments
use mult_frac
use a common divisor function.

 drivers/clk/clk-xlnx-clock-wizard.c | 219 ++++++++++++++++++++++++++++++++----
 1 file changed, 199 insertions(+), 20 deletions(-)

diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index 5581b24..ed3b0ef 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -29,20 +29,25 @@
 
 #define WZRD_CLKFBOUT_MULT_SHIFT	8
 #define WZRD_CLKFBOUT_MULT_MASK		(0xff << WZRD_CLKFBOUT_MULT_SHIFT)
+#define WZRD_CLKFBOUT_FRAC_SHIFT	16
+#define WZRD_CLKFBOUT_FRAC_MASK		(0x3ff << WZRD_CLKFBOUT_FRAC_SHIFT)
 #define WZRD_DIVCLK_DIVIDE_SHIFT	0
 #define WZRD_DIVCLK_DIVIDE_MASK		(0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
 #define WZRD_CLKOUT_DIVIDE_SHIFT	0
 #define WZRD_CLKOUT_DIVIDE_WIDTH	8
 #define WZRD_CLKOUT_DIVIDE_MASK		(0xff << WZRD_DIVCLK_DIVIDE_SHIFT)
+#define WZRD_CLKOUT_FRAC_SHIFT		8
+#define WZRD_CLKOUT_FRAC_MASK		0x3ff
 
 #define WZRD_DR_MAX_INT_DIV_VALUE	255
-#define WZRD_DR_NUM_RETRIES		10000
 #define WZRD_DR_STATUS_REG_OFFSET	0x04
 #define WZRD_DR_LOCK_BIT_MASK		0x00000001
 #define WZRD_DR_INIT_REG_OFFSET		0x25C
 #define WZRD_DR_DIV_TO_PHASE_OFFSET	4
 #define WZRD_DR_BEGIN_DYNA_RECONF	0x03
 
+#define WZRD_USEC_POLL		10
+#define WZRD_TIMEOUT_POLL		1000
 /* Get the mask from width */
 #define div_mask(width)			((1 << (width)) - 1)
 
@@ -52,6 +57,7 @@
 enum clk_wzrd_int_clks {
 	wzrd_clk_mul,
 	wzrd_clk_mul_div,
+	wzrd_clk_mul_frac,
 	wzrd_clk_int_max
 };
 
@@ -186,7 +192,7 @@ static long clk_wzrd_round_rate(struct clk_hw *hw, unsigned long rate,
 	 */
 	div = DIV_ROUND_CLOSEST(*prate, rate);
 
-	return (*prate / div);
+	return *prate / div;
 }
 
 static const struct clk_ops clk_wzrd_clk_divider_ops = {
@@ -195,6 +201,117 @@ static const struct clk_ops clk_wzrd_clk_divider_ops = {
 	.recalc_rate = clk_wzrd_recalc_rate,
 };
 
+static unsigned long clk_wzrd_recalc_ratef(struct clk_hw *hw,
+					   unsigned long parent_rate)
+{
+	unsigned int val;
+	u32 div, frac;
+	struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
+	void __iomem *div_addr = divider->base + divider->offset;
+
+	val = readl(div_addr);
+	div = val & div_mask(divider->width);
+	frac = (val >> WZRD_CLKOUT_FRAC_SHIFT) & WZRD_CLKOUT_FRAC_MASK;
+
+	return mult_frac(parent_rate, 1000, (div * 1000) + frac);
+}
+
+static int clk_wzrd_dynamic_reconfig_f(struct clk_hw *hw, unsigned long rate,
+				       unsigned long parent_rate)
+{
+	int err;
+	u32 value, pre;
+	unsigned long rate_div, f, clockout0_div;
+	struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw);
+	void __iomem *div_addr = divider->base + divider->offset;
+
+	rate_div = ((parent_rate * 1000) / rate);
+	clockout0_div = rate_div / 1000;
+
+	pre = DIV_ROUND_CLOSEST((parent_rate * 1000), rate);
+	f = (u32)(pre - (clockout0_div * 1000));
+	f = f & WZRD_CLKOUT_FRAC_MASK;
+	f = f << WZRD_CLKOUT_DIVIDE_WIDTH;
+
+	value = (f  | (clockout0_div & WZRD_CLKOUT_DIVIDE_MASK));
+
+	/* Set divisor and clear phase offset */
+	writel(value, div_addr);
+	writel(0x0, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET);
+
+	/* Check status register */
+	err = readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
+				 value & WZRD_DR_LOCK_BIT_MASK,
+				 WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
+	if (err)
+		return err;
+
+	/* Initiate reconfiguration */
+	writel(WZRD_DR_BEGIN_DYNA_RECONF,
+	       divider->base + WZRD_DR_INIT_REG_OFFSET);
+
+	/* Check status register */
+	return readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value,
+				value & WZRD_DR_LOCK_BIT_MASK,
+				WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
+}
+
+static long clk_wzrd_round_rate_f(struct clk_hw *hw, unsigned long rate,
+				  unsigned long *prate)
+{
+	return rate;
+}
+
+static const struct clk_ops clk_wzrd_clk_divider_ops_f = {
+	.round_rate = clk_wzrd_round_rate_f,
+	.set_rate = clk_wzrd_dynamic_reconfig_f,
+	.recalc_rate = clk_wzrd_recalc_ratef,
+};
+
+static struct clk *clk_wzrd_register_divf(struct device *dev,
+					  const char *name,
+					  const char *parent_name,
+					  unsigned long flags,
+					  void __iomem *base, u16 offset,
+					  u8 shift, u8 width,
+					  u8 clk_divider_flags,
+					  const struct clk_div_table *table,
+					  spinlock_t *lock)
+{
+	struct clk_wzrd_divider *div;
+	struct clk_hw *hw;
+	struct clk_init_data init;
+	int ret;
+
+	div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
+	if (!div)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+
+	init.ops = &clk_wzrd_clk_divider_ops_f;
+
+	init.flags = flags;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	div->base = base;
+	div->offset = offset;
+	div->shift = shift;
+	div->width = width;
+	div->flags = clk_divider_flags;
+	div->lock = lock;
+	div->hw.init = &init;
+	div->table = table;
+
+	hw = &div->hw;
+	ret =  devm_clk_hw_register(dev, hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return hw->clk;
+}
+
 static struct clk *clk_wzrd_register_divider(struct device *dev,
 					     const char *name,
 					     const char *parent_name,
@@ -229,7 +346,6 @@ static struct clk *clk_wzrd_register_divider(struct device *dev,
 	div->hw.init = &init;
 	div->table = table;
 
-	/* register the clock */
 	hw = &div->hw;
 	ret = devm_clk_hw_register(dev, hw);
 	if (ret)
@@ -237,7 +353,6 @@ static struct clk *clk_wzrd_register_divider(struct device *dev,
 
 	return hw->clk;
 }
-
 static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
 				 void *data)
 {
@@ -267,6 +382,61 @@ static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
 	}
 }
 
+static int clk_wzrd_register_dividers(struct platform_device *pdev,
+				      const char *clk_name)
+{
+	int i, ret;
+	struct device_node *np = pdev->dev.of_node;
+	int outputs;
+	unsigned long flags = 0;
+	struct clk_wzrd *clk_wzrd = platform_get_drvdata(pdev);
+	const char *clkout_name;
+
+	outputs = of_property_count_strings(np, "clock-output-names");
+	if (outputs == 1)
+		flags = CLK_SET_RATE_PARENT;
+
+	for (i = outputs - 1; i >= 0 ; i--) {
+		if (of_property_read_string_index(np, "clock-output-names", i,
+						  &clkout_name)) {
+			dev_err(&pdev->dev,
+				"clock output name not specified\n");
+			ret = -EINVAL;
+			return ret;
+		}
+
+		if (!i)
+			clk_wzrd->clkout[i] = clk_wzrd_register_divf
+				(&pdev->dev, clkout_name,
+				clk_name, flags,
+				clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
+				WZRD_CLKOUT_DIVIDE_SHIFT,
+				WZRD_CLKOUT_DIVIDE_WIDTH,
+				CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
+				NULL, &clkwzrd_lock);
+		else
+			clk_wzrd->clkout[i] = clk_wzrd_register_divider
+				(&pdev->dev, clkout_name,
+				clk_name, 0,
+				clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
+				WZRD_CLKOUT_DIVIDE_SHIFT,
+				WZRD_CLKOUT_DIVIDE_WIDTH,
+				CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
+				NULL, &clkwzrd_lock);
+
+		if (IS_ERR(clk_wzrd->clkout[i])) {
+			int j;
+
+			for (j = i + 1; j < outputs; j++)
+				clk_unregister(clk_wzrd->clkout[j]);
+			dev_err(&pdev->dev,
+				"unable to register divider clock\n");
+			ret = PTR_ERR(clk_wzrd->clkout[i]);
+			return ret;
+		}
+	}
+	return 0;
+}
 static int __maybe_unused clk_wzrd_suspend(struct device *dev)
 {
 	struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
@@ -298,8 +468,8 @@ static SIMPLE_DEV_PM_OPS(clk_wzrd_dev_pm_ops, clk_wzrd_suspend,
 
 static int clk_wzrd_probe(struct platform_device *pdev)
 {
-	int i, ret;
-	u32 reg;
+	int ret;
+	u32 reg, reg_f, mult;
 	unsigned long rate;
 	const char *clk_name;
 	struct clk_wzrd *clk_wzrd;
@@ -349,17 +519,13 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		goto err_disable_clk;
 	}
 
-	/* we don't support fractional div/mul yet */
-	reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
-		    WZRD_CLKFBOUT_FRAC_EN;
-	reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) &
-		     WZRD_CLKOUT0_FRAC_EN;
-	if (reg)
-		dev_warn(&pdev->dev, "fractional div/mul not supported\n");
+	reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0));
+	reg_f = reg & WZRD_CLKFBOUT_FRAC_MASK;
+	reg_f =  reg_f >> WZRD_CLKFBOUT_FRAC_SHIFT;
 
-	/* register multiplier */
-	reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
-		     WZRD_CLKFBOUT_MULT_MASK) >> WZRD_CLKFBOUT_MULT_SHIFT;
+	reg = reg & WZRD_CLKFBOUT_MULT_MASK;
+	reg =  reg >> WZRD_CLKFBOUT_MULT_SHIFT;
+	mult = (reg * 1000) + reg_f;
 	clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(&pdev->dev));
 	if (!clk_name) {
 		ret = -ENOMEM;
@@ -368,8 +534,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	clk_wzrd->clks_internal[wzrd_clk_mul] = clk_register_fixed_factor
 			(&pdev->dev, clk_name,
 			 __clk_get_name(clk_wzrd->clk_in1),
-			 0, reg, 1);
-	kfree(clk_name);
+			0, mult, 1000);
 	if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul])) {
 		dev_err(&pdev->dev, "unable to register fixed-factor clock\n");
 		ret = PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul]);
@@ -407,8 +572,18 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 			ret = -EINVAL;
 			goto err_rm_int_clks;
 		}
-		clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev,
-								clkout_name,
+		if (!i)
+			clk_wzrd->clkout[i] = clk_wzrd_register_divf
+				(&pdev->dev, clkout_name,
+				clk_name, 0,
+				clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
+				WZRD_CLKOUT_DIVIDE_SHIFT,
+				WZRD_CLKOUT_DIVIDE_WIDTH,
+				CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
+				NULL, &clkwzrd_lock);
+		else
+			clk_wzrd->clkout[i] = clk_wzrd_register_divider
+				(&pdev->dev, clkout_name,
 				clk_name, 0,
 				clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
 				WZRD_CLKOUT_DIVIDE_SHIFT,
@@ -427,6 +602,10 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		}
 	}
 
+	ret = clk_wzrd_register_dividers(pdev, clk_name);
+	if (ret)
+		goto err_rm_int_clks;
+
 	kfree(clk_name);
 
 	clk_wzrd->clk_data.clks = clk_wzrd->clkout;
-- 
2.1.1

[PATCH v9 6/7] clk: clock-wizard: Remove the hardcoding of the clock outputs

[Shubhrajyoti Datta]

The number of output clocks are configurable in the hardware.
Currently the driver registers the maximum number of outputs.
Fix the same by registering only the outputs that are there.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v4:
Assign output in this patch

 drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index ed3b0ef..d403a74 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	unsigned long rate;
 	const char *clk_name;
 	struct clk_wzrd *clk_wzrd;
+	int outputs;
 	struct device_node *np = pdev->dev.of_node;
 
 	clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
@@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		goto err_disable_clk;
 	}
 
+	outputs = of_property_count_strings(np, "clock-output-names");
 	/* register div */
 	reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
 			WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
@@ -562,7 +564,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	}
 
 	/* register div per output */
-	for (i = WZRD_NUM_OUTPUTS - 1; i >= 0 ; i--) {
+	for (i = outputs - 1; i >= 0 ; i--) {
 		const char *clkout_name;
 
 		if (of_property_read_string_index(np, "clock-output-names", i,
@@ -593,7 +595,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		if (IS_ERR(clk_wzrd->clkout[i])) {
 			int j;
 
-			for (j = i + 1; j < WZRD_NUM_OUTPUTS; j++)
+			for (j = i + 1; j < outputs; j++)
 				clk_unregister(clk_wzrd->clkout[j]);
 			dev_err(&pdev->dev,
 				"unable to register divider clock\n");
-- 
2.1.1

[PATCH v9 7/7] clk: clock-wizard: Update the fixed factor divisors

[Shubhrajyoti Datta]

Update the fixed factor clock registration to register the divisors.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/clk/clk-xlnx-clock-wizard.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
index d403a74..7f09522 100644
--- a/drivers/clk/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -472,8 +472,10 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	u32 reg, reg_f, mult;
 	unsigned long rate;
 	const char *clk_name;
+	void __iomem *ctrl_reg;
 	struct clk_wzrd *clk_wzrd;
 	int outputs;
+	unsigned long flags = 0;
 	struct device_node *np = pdev->dev.of_node;
 
 	clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
@@ -543,16 +545,17 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	}
 
 	outputs = of_property_count_strings(np, "clock-output-names");
-	/* register div */
-	reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
-			WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
+	if (outputs == 1)
+		flags = CLK_SET_RATE_PARENT;
 	clk_name = kasprintf(GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
 	if (!clk_name) {
 		ret = -ENOMEM;
 		goto err_rm_int_clk;
 	}
 
-	clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_fixed_factor
+	ctrl_reg = clk_wzrd->base + WZRD_CLK_CFG_REG(0);
+	/* register div */
+	clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_divider
 			(&pdev->dev, clk_name,
 			 __clk_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
 			flags, ctrl_reg, 0, 8, CLK_DIVIDER_ONE_BASED |
@@ -577,7 +580,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		if (!i)
 			clk_wzrd->clkout[i] = clk_wzrd_register_divf
 				(&pdev->dev, clkout_name,
-				clk_name, 0,
+				clk_name, flags,
 				clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
 				WZRD_CLKOUT_DIVIDE_SHIFT,
 				WZRD_CLKOUT_DIVIDE_WIDTH,
-- 
2.1.1

Re: [PATCH v9 0/7] clk: clk-wizard: clock-wizard: Driver updates

[Miquel Raynal]

Hi Shubhrajyoti,

Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
2021 10:19:44 +0530:

> In the thread [1] Greg suggested that we move the driver
> to the clk from the staging.
> Add patches to address the concerns regarding the fractional and
> set rate support in the TODO.
> 
> The patch set does the following
> - Trivial fixes for kernel doc.
> - Move the driver to the clk folder
> - Add capability to set rate.
> - Add fractional support.
> - Add support for configurable outputs.
> - Make the output names unique so that multiple instances
> do not crib.

I think we prefer to move "clean" drivers out of the staging tree
rather than "to be fixed" code. So I would invert the order of the
patches in this series to make more sense:
* 3/7-7/7 (various fixes/improvements)
* 1/7 (bindings)
* 2/7 (move to clk)

> Shubhrajyoti Datta (7):
>   dt-bindings: add documentation of xilinx clocking wizard
>   clk: clock-wizard: Add the clockwizard to clk directory
>   clk: clock-wizard: Fix kernel-doc warning
>   clk: clock-wizard: Add support for dynamic reconfiguration
>   clk: clock-wizard: Add support for fractional support
>   clk: clock-wizard: Remove the hardcoding of the clock outputs
>   clk: clock-wizard: Update the fixed factor divisors
> 
>  .../bindings/clock/xlnx,clocking-wizard.yaml       |  65 ++
>  drivers/clk/Kconfig                                |   9 +
>  drivers/clk/Makefile                               |   1 +
>  drivers/clk/clk-xlnx-clock-wizard.c                | 689 +++++++++++++++++++++
>  drivers/staging/Kconfig                            |   2 -
>  drivers/staging/Makefile                           |   1 -
>  drivers/staging/clocking-wizard/Kconfig            |  10 -
>  drivers/staging/clocking-wizard/Makefile           |   2 -
>  drivers/staging/clocking-wizard/TODO               |  12 -
>  .../clocking-wizard/clk-xlnx-clock-wizard.c        | 333 ----------
>  drivers/staging/clocking-wizard/dt-binding.txt     |  30 -
>  11 files changed, 764 insertions(+), 390 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
>  create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c
>  delete mode 100644 drivers/staging/clocking-wizard/Kconfig
>  delete mode 100644 drivers/staging/clocking-wizard/Makefile
>  delete mode 100644 drivers/staging/clocking-wizard/TODO
>  delete mode 100644 drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
>  delete mode 100644 drivers/staging/clocking-wizard/dt-binding.txt
> 

Thanks,
Miquèl

Re: [PATCH v9 2/7] clk: clock-wizard: Add the clockwizard to clk directory

[Miquel Raynal]

Hi Shubhrajyoti,

Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
2021 10:19:46 +0530:

> Add clocking wizard driver to clk.
> And delete the driver from the staging as it is in drivers/clk.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v7:
> Combined the patch for deletion and add of the driver
> dropping the ack from Greg for the staging as it is a combined patch.
> Add vendor prefix to speedgrade
> v8:
> No change
> v9:
> No change
> 
>  drivers/clk/Kconfig                                |  9 +++++++
>  drivers/clk/Makefile                               |  1 +
>  .../clk-xlnx-clock-wizard.c                        |  6 +++--
>  drivers/staging/Kconfig                            |  2 --
>  drivers/staging/Makefile                           |  1 -
>  drivers/staging/clocking-wizard/Kconfig            | 10 --------
>  drivers/staging/clocking-wizard/Makefile           |  2 --
>  drivers/staging/clocking-wizard/TODO               | 12 ---------
>  drivers/staging/clocking-wizard/dt-binding.txt     | 30 ----------------------
>  9 files changed, 14 insertions(+), 59 deletions(-)
>  rename drivers/{staging/clocking-wizard => clk}/clk-xlnx-clock-wizard.c (98%)
>  delete mode 100644 drivers/staging/clocking-wizard/Kconfig
>  delete mode 100644 drivers/staging/clocking-wizard/Makefile
>  delete mode 100644 drivers/staging/clocking-wizard/TODO
>  delete mode 100644 drivers/staging/clocking-wizard/dt-binding.txt
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index c715d46..d210ed2 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -359,6 +359,15 @@ config COMMON_CLK_FIXED_MMIO
>  	help
>  	  Support for Memory Mapped IO Fixed clocks
>  
> +config COMMON_CLK_XLNX_CLKWZRD
> +	tristate "Xilinx Clocking Wizard"
> +	depends on COMMON_CLK && OF
> +	help
> +	  Support for the Xilinx Clocking Wizard IP core clock generator.
> +	  Adds support for clocking wizard and compatible.
> +	  This driver supports the Xilinx clocking wizard programmable clock
> +	  synthesizer. The number of output is configurable in the design.
> +
>  source "drivers/clk/actions/Kconfig"
>  source "drivers/clk/analogbits/Kconfig"
>  source "drivers/clk/baikal-t1/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index da8fcf1..1ad6414 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_ARCH_VT8500)		+= clk-vt8500.o
>  obj-$(CONFIG_COMMON_CLK_VC5)		+= clk-versaclock5.o
>  obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
>  obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o
> +obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD)	+= clk-xlnx-clock-wizard.o
>  
>  # please keep this section sorted lexicographically by directory path name
>  obj-y					+= actions/
> diff --git a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> similarity index 98%
> rename from drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
> rename to drivers/clk/clk-xlnx-clock-wizard.c
> index e52a64b..1bab68e 100644
> --- a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> @@ -2,9 +2,11 @@
>  /*
>   * Xilinx 'Clocking Wizard' driver
>   *
> - *  Copyright (C) 2013 - 2014 Xilinx
> + *  Copyright (C) 2013 - 2020 Xilinx
>   *
>   *  Sören Brinkmann <soren.brinkmann@xilinx.com>
> + *  Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> + *
>   */
>  
>  #include <linux/platform_device.h>
> @@ -146,7 +148,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  	if (IS_ERR(clk_wzrd->base))
>  		return PTR_ERR(clk_wzrd->base);
>  
> -	ret = of_property_read_u32(np, "speed-grade", &clk_wzrd->speed_grade);
> +	ret = of_property_read_u32(np, "xlnx,speed-grade", &clk_wzrd->speed_grade);

Should not be part of this commit.

You should do a mv src.c dest.c, that's all.

>  	if (!ret) {
>  		if (clk_wzrd->speed_grade < 1 || clk_wzrd->speed_grade > 3) {
>  			dev_warn(&pdev->dev, "invalid speed grade '%d'\n",


Thanks,
Miquèl

Re: [PATCH v9 1/7] dt-bindings: add documentation of xilinx clocking wizard

[Miquel Raynal]

Hi Shubhrajyoti,

Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
2021 10:19:45 +0530:

> Add the devicetree binding for the xilinx clocking wizard.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v6:
> Fix a yaml warning
> v7:
> Add vendor prefix speed-grade
> v8:
> Fix the warnings
> v9:
> Fix the warnings
> 
>  .../bindings/clock/xlnx,clocking-wizard.yaml       | 65 ++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> new file mode 100644
> index 0000000..d209140
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Xilinx clocking wizard
> +
> +maintainers:
> +  - Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> +
> +description:
> +  The clocking wizard is a soft ip clocking block of Xilinx versal. It
> +  reads required input clock frequencies from the devicetree and acts as clock
> +  clock output.
> +
> +properties:
> +  compatible:
> +    const: xlnx,clocking-wizard
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  clocks:
> +    items:
> +      - description: clock input
> +      - description: axi clock
> +
> +  clock-names:
> +    items:
> +      - const: clk_in1
> +      - const: s_axi_aclk
> +
> +  clock-output-names: true
> +
> +  xlnx,speed-grade:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2, 3]
> +    description:
> +      Speed grade of the device.

A bit of explanation of what this describes would be welcome.

Don't forget that binding are not tied to any driver implementation,
these are supposed to be hardware description properties.

> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +  - clocks
> +  - clock-names
> +  - xlnx,speed-grade
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    clock-controller  {
> +        compatible = "xlnx,clocking-wizard";
> +        reg = <0xb0000000 0x10000>;
> +        #clock-cells = <1>;
> +        xlnx,speed-grade = <1>;
> +        clock-names = "clk_in1", "s_axi_aclk";
> +        clocks = <&clkc 15>, <&clkc 15>;
> +    };
> +...

Thanks,
Miquèl

Re: [PATCH v9 3/7] clk: clock-wizard: Fix kernel-doc warning

[Miquel Raynal]

Hi Shubhrajyoti,

Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
2021 10:19:47 +0530:

> Update description for the clocking wizard structure

"Fix the clocking wizard main structure kernel documentation." ?

> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  drivers/clk/clk-xlnx-clock-wizard.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> index 1bab68e..fb2d555 100644
> --- a/drivers/clk/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> @@ -40,7 +40,8 @@ enum clk_wzrd_int_clks {
>  };
>  
>  /**
> - * struct clk_wzrd:
> + * struct clk_wzrd - Clock wizard private data structure
> + *
>   * @clk_data:		Clock data
>   * @nb:			Notifier block
>   * @base:		Memory base


Thanks,
Miquèl

Re: [PATCH v9 4/7] clk: clock-wizard: Add support for dynamic reconfiguration

[Miquel Raynal]

Hi Shubhrajyoti,

Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
2021 10:19:48 +0530:

> The patch adds support for dynamic reconfiguration of clock output rate.
> Output clocks are registered as dividers and set rate callback function
> is used for dynamic reconfiguration.
> 
> Based on the initial work from Chirag.
> 
> Signed-off-by: Chirag Parekh <chirag.parekh@xilinx.com>

The first SoB should match the author.

Either your just did minor changes and keep Chirag's Authorship + SoB
and add your SoB as you already did below, or either use the
Codevelopped tag or if you really change a lot of things you can keep
the authorship and give Chirag credit with a Suggested-by or something
like this.

> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---

Thanks,
Miquèl

Re: [PATCH v9 6/7] clk: clock-wizard: Remove the hardcoding of the clock outputs

[Miquel Raynal]

Hi Shubhrajyoti,

Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
2021 10:19:50 +0530:

> The number of output clocks are configurable in the hardware.
> Currently the driver registers the maximum number of outputs.
> Fix the same by registering only the outputs that are there.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v4:
> Assign output in this patch
> 
>  drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> index ed3b0ef..d403a74 100644
> --- a/drivers/clk/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  	unsigned long rate;
>  	const char *clk_name;
>  	struct clk_wzrd *clk_wzrd;
> +	int outputs;
>  	struct device_node *np = pdev->dev.of_node;
>  
>  	clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
> @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  		goto err_disable_clk;
>  	}
>  
> +	outputs = of_property_count_strings(np, "clock-output-names");

A check on outputs validity is probably welcome.

Also I usually prefer noutputs or nb_outputs for such variable name,
which implies a number rather than an array, but this is personal taste.

>  	/* register div */
>  	reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
>  			WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
> @@ -562,7 +564,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  	}
>  
>  	/* register div per output */
> -	for (i = WZRD_NUM_OUTPUTS - 1; i >= 0 ; i--) {
> +	for (i = outputs - 1; i >= 0 ; i--) {
>  		const char *clkout_name;
>  
>  		if (of_property_read_string_index(np, "clock-output-names", i,
> @@ -593,7 +595,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  		if (IS_ERR(clk_wzrd->clkout[i])) {
>  			int j;
>  
> -			for (j = i + 1; j < WZRD_NUM_OUTPUTS; j++)
> +			for (j = i + 1; j < outputs; j++)
>  				clk_unregister(clk_wzrd->clkout[j]);
>  			dev_err(&pdev->dev,
>  				"unable to register divider clock\n");


Thanks,
Miquèl

Re: [PATCH v9 1/7] dt-bindings: add documentation of xilinx clocking wizard

[Rob Herring]

On Thu, 18 Feb 2021 10:19:45 +0530, Shubhrajyoti Datta wrote:
> Add the devicetree binding for the xilinx clocking wizard.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v6:
> Fix a yaml warning
> v7:
> Add vendor prefix speed-grade
> v8:
> Fix the warnings
> v9:
> Fix the warnings
> 
>  .../bindings/clock/xlnx,clocking-wizard.yaml       | 65 ++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.example.dts:19.26-26.11: Warning (unit_address_vs_reg): /example-0/clock-controller: node has a reg or ranges property, but no unit name

See https://patchwork.ozlabs.org/patch/1441521

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v9 1/7] dt-bindings: add documentation of xilinx clocking wizard

[Stephen Boyd]

Quoting Miquel Raynal (2021-02-18 00:28:04)
> Hi Shubhrajyoti,
> 
> Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
> 2021 10:19:45 +0530:
> 
> > Add the devicetree binding for the xilinx clocking wizard.
> > 
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > ---
> > v6:
> > Fix a yaml warning
> > v7:
> > Add vendor prefix speed-grade
> > v8:
> > Fix the warnings
> > v9:
> > Fix the warnings
> > 
> >  .../bindings/clock/xlnx,clocking-wizard.yaml       | 65 ++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > new file mode 100644
> > index 0000000..d209140
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > @@ -0,0 +1,65 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Xilinx clocking wizard
> > +
> > +maintainers:
> > +  - Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > +
> > +description:
> > +  The clocking wizard is a soft ip clocking block of Xilinx versal. It
> > +  reads required input clock frequencies from the devicetree and acts as clock
> > +  clock output.
> > +
> > +properties:
> > +  compatible:
> > +    const: xlnx,clocking-wizard
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  "#clock-cells":
> > +    const: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: clock input
> > +      - description: axi clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: clk_in1
> > +      - const: s_axi_aclk
> > +
> > +  clock-output-names: true
> > +
> > +  xlnx,speed-grade:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 2, 3]
> > +    description:
> > +      Speed grade of the device.
> 
> A bit of explanation of what this describes would be welcome.
> 
> Don't forget that binding are not tied to any driver implementation,
> these are supposed to be hardware description properties.

Would opp tables work for this?

Re: [PATCH v9 6/7] clk: clock-wizard: Remove the hardcoding of the clock outputs

[Stephen Boyd]

Quoting Miquel Raynal (2021-02-18 00:37:15)
> Hi Shubhrajyoti,
> 
> Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
> 2021 10:19:50 +0530:
> 
> > The number of output clocks are configurable in the hardware.
> > Currently the driver registers the maximum number of outputs.
> > Fix the same by registering only the outputs that are there.
> > 
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > ---
> > v4:
> > Assign output in this patch
> > 
> >  drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> > index ed3b0ef..d403a74 100644
> > --- a/drivers/clk/clk-xlnx-clock-wizard.c
> > +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> > @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> >       unsigned long rate;
> >       const char *clk_name;
> >       struct clk_wzrd *clk_wzrd;
> > +     int outputs;
> >       struct device_node *np = pdev->dev.of_node;
> >  
> >       clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
> > @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> >               goto err_disable_clk;
> >       }
> >  
> > +     outputs = of_property_count_strings(np, "clock-output-names");
> 
> A check on outputs validity is probably welcome.
> 
> Also I usually prefer noutputs or nb_outputs for such variable name,
> which implies a number rather than an array, but this is personal taste.

Ideally we get rid of clock-output-names and generate them at runtime
instead based on some combination of device name and something else.

Re: [PATCH v9 6/7] clk: clock-wizard: Remove the hardcoding of the clock outputs

[Shubhrajyoti Datta]

Hi Stephen,

On Fri, Feb 19, 2021 at 6:55 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Miquel Raynal (2021-02-18 00:37:15)
> > Hi Shubhrajyoti,
> >
> > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
> > 2021 10:19:50 +0530:
> >
> > > The number of output clocks are configurable in the hardware.
> > > Currently the driver registers the maximum number of outputs.
> > > Fix the same by registering only the outputs that are there.
> > >
> > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > > ---
> > > v4:
> > > Assign output in this patch
> > >
> > >  drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> > > index ed3b0ef..d403a74 100644
> > > --- a/drivers/clk/clk-xlnx-clock-wizard.c
> > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> > > @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> > >       unsigned long rate;
> > >       const char *clk_name;
> > >       struct clk_wzrd *clk_wzrd;
> > > +     int outputs;
> > >       struct device_node *np = pdev->dev.of_node;
> > >
> > >       clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
> > > @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> > >               goto err_disable_clk;
> > >       }
> > >
> > > +     outputs = of_property_count_strings(np, "clock-output-names");
> >
> > A check on outputs validity is probably welcome.
> >
> > Also I usually prefer noutputs or nb_outputs for such variable name,
> > which implies a number rather than an array, but this is personal taste.
>
> Ideally we get rid of clock-output-names and generate them at runtime
> instead based on some combination of device name and something else.

Makes sense. However it may break the current binding.
Do you think that shoud be okay?

Re: [PATCH v9 6/7] clk: clock-wizard: Remove the hardcoding of the clock outputs

[Stephen Boyd]

Quoting Shubhrajyoti Datta (2021-02-21 22:47:26)
> Hi Stephen,
> 
> On Fri, Feb 19, 2021 at 6:55 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Miquel Raynal (2021-02-18 00:37:15)
> > > Hi Shubhrajyoti,
> > >
> > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
> > > 2021 10:19:50 +0530:
> > >
> > > > The number of output clocks are configurable in the hardware.
> > > > Currently the driver registers the maximum number of outputs.
> > > > Fix the same by registering only the outputs that are there.
> > > >
> > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > > > ---
> > > > v4:
> > > > Assign output in this patch
> > > >
> > > >  drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++--
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> > > > index ed3b0ef..d403a74 100644
> > > > --- a/drivers/clk/clk-xlnx-clock-wizard.c
> > > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> > > > @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> > > >       unsigned long rate;
> > > >       const char *clk_name;
> > > >       struct clk_wzrd *clk_wzrd;
> > > > +     int outputs;
> > > >       struct device_node *np = pdev->dev.of_node;
> > > >
> > > >       clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
> > > > @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> > > >               goto err_disable_clk;
> > > >       }
> > > >
> > > > +     outputs = of_property_count_strings(np, "clock-output-names");
> > >
> > > A check on outputs validity is probably welcome.
> > >
> > > Also I usually prefer noutputs or nb_outputs for such variable name,
> > > which implies a number rather than an array, but this is personal taste.
> >
> > Ideally we get rid of clock-output-names and generate them at runtime
> > instead based on some combination of device name and something else.
> 
> Makes sense. However it may break the current binding.
> Do you think that shoud be okay?

I think it is OK given that the current binding is for the staging tree.
The assumption is those bindings aren't stable.

Re: [PATCH v9 6/7] clk: clock-wizard: Remove the hardcoding of the clock outputs

[Shubhrajyoti Datta]

On Tue, Feb 23, 2021 at 6:32 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Shubhrajyoti Datta (2021-02-21 22:47:26)
> > Hi Stephen,
> >
> > On Fri, Feb 19, 2021 at 6:55 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Miquel Raynal (2021-02-18 00:37:15)
> > > > Hi Shubhrajyoti,
> > > >
> > > > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
> > > > 2021 10:19:50 +0530:
> > > >
> > > > > The number of output clocks are configurable in the hardware.
> > > > > Currently the driver registers the maximum number of outputs.
> > > > > Fix the same by registering only the outputs that are there.
> > > > >
> > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > > > > ---
> > > > > v4:
> > > > > Assign output in this patch
> > > > >
> > > > >  drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++--
> > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> > > > > index ed3b0ef..d403a74 100644
> > > > > --- a/drivers/clk/clk-xlnx-clock-wizard.c
> > > > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> > > > > @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> > > > >       unsigned long rate;
> > > > >       const char *clk_name;
> > > > >       struct clk_wzrd *clk_wzrd;
> > > > > +     int outputs;
> > > > >       struct device_node *np = pdev->dev.of_node;
> > > > >
> > > > >       clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
> > > > > @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> > > > >               goto err_disable_clk;
> > > > >       }
> > > > >
> > > > > +     outputs = of_property_count_strings(np, "clock-output-names");
> > > >
> > > > A check on outputs validity is probably welcome.
> > > >
> > > > Also I usually prefer noutputs or nb_outputs for such variable name,
> > > > which implies a number rather than an array, but this is personal taste.
> > >
> > > Ideally we get rid of clock-output-names and generate them at runtime
> > > instead based on some combination of device name and something else.
> >
> > Makes sense. However it may break the current binding.
> > Do you think that shoud be okay?
>
> I think it is OK given that the current binding is for the staging tree.
> The assumption is those bindings aren't stable.
Updated it in next version.

Re: [PATCH v9 1/7] dt-bindings: add documentation of xilinx clocking wizard

[Shubhrajyoti Datta]

On Fri, Feb 19, 2021 at 6:54 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Miquel Raynal (2021-02-18 00:28:04)
> > Hi Shubhrajyoti,
> >
> > Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
> > 2021 10:19:45 +0530:
> >
> > > Add the devicetree binding for the xilinx clocking wizard.
> > >
> > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > > ---
> > > v6:
> > > Fix a yaml warning
> > > v7:
> > > Add vendor prefix speed-grade
> > > v8:
> > > Fix the warnings
> > > v9:
> > > Fix the warnings
> > >
> > >  .../bindings/clock/xlnx,clocking-wizard.yaml       | 65 ++++++++++++++++++++++
> > >  1 file changed, 65 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > > new file mode 100644
> > > index 0000000..d209140
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > > @@ -0,0 +1,65 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#"
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > +
> > > +title: Xilinx clocking wizard
> > > +
> > > +maintainers:
> > > +  - Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > > +
> > > +description:
> > > +  The clocking wizard is a soft ip clocking block of Xilinx versal. It
> > > +  reads required input clock frequencies from the devicetree and acts as clock
> > > +  clock output.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: xlnx,clocking-wizard
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  "#clock-cells":
> > > +    const: 1
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: clock input
> > > +      - description: axi clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: clk_in1
> > > +      - const: s_axi_aclk
> > > +
> > > +  clock-output-names: true
> > > +
> > > +  xlnx,speed-grade:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [1, 2, 3]
> > > +    description:
> > > +      Speed grade of the device.
> >
> > A bit of explanation of what this describes would be welcome.
> >
> > Don't forget that binding are not tied to any driver implementation,
> > these are supposed to be hardware description properties.
>
> Would opp tables work for this?
This is the parameter is for speed of the fabric.

Re: [PATCH v9 3/7] clk: clock-wizard: Fix kernel-doc warning

[Shubhrajyoti Datta]

On Thu, Feb 18, 2021 at 1:59 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Shubhrajyoti,
>
> Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
> 2021 10:19:47 +0530:
>
> > Update description for the clocking wizard structure
>
> "Fix the clocking wizard main structure kernel documentation." ?
will update in next version.

>
> >
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > ---
> >  drivers/clk/clk-xlnx-clock-wizard.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> > index 1bab68e..fb2d555 100644
> > --- a/drivers/clk/clk-xlnx-clock-wizard.c
> > +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> > @@ -40,7 +40,8 @@ enum clk_wzrd_int_clks {
> >  };
> >
> >  /**
> > - * struct clk_wzrd:
> > + * struct clk_wzrd - Clock wizard private data structure
> > + *
> >   * @clk_data:                Clock data
> >   * @nb:                      Notifier block
> >   * @base:            Memory base
>
>
> Thanks,
> Miquèl

Re: [PATCH v9 0/7] clk: clk-wizard: clock-wizard: Driver updates

[Shubhrajyoti Datta]

On Thu, Feb 18, 2021 at 1:54 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Shubhrajyoti,
>
> Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
> 2021 10:19:44 +0530:
>
> > In the thread [1] Greg suggested that we move the driver
> > to the clk from the staging.
> > Add patches to address the concerns regarding the fractional and
> > set rate support in the TODO.
> >
> > The patch set does the following
> > - Trivial fixes for kernel doc.
> > - Move the driver to the clk folder
> > - Add capability to set rate.
> > - Add fractional support.
> > - Add support for configurable outputs.
> > - Make the output names unique so that multiple instances
> > do not crib.
>
> I think we prefer to move "clean" drivers out of the staging tree
> rather than "to be fixed" code. So I would invert the order of the
> patches in this series to make more sense:
> * 3/7-7/7 (various fixes/improvements)
> * 1/7 (bindings)
> * 2/7 (move to clk)
>
Will update in next version

> > Shubhrajyoti Datta (7):
> >   dt-bindings: add documentation of xilinx clocking wizard
> >   clk: clock-wizard: Add the clockwizard to clk directory
> >   clk: clock-wizard: Fix kernel-doc warning
> >   clk: clock-wizard: Add support for dynamic reconfiguration
> >   clk: clock-wizard: Add support for fractional support
> >   clk: clock-wizard: Remove the hardcoding of the clock outputs
> >   clk: clock-wizard: Update the fixed factor divisors
> >
> >  .../bindings/clock/xlnx,clocking-wizard.yaml       |  65 ++
> >  drivers/clk/Kconfig                                |   9 +
> >  drivers/clk/Makefile                               |   1 +
> >  drivers/clk/clk-xlnx-clock-wizard.c                | 689 +++++++++++++++++++++
> >  drivers/staging/Kconfig                            |   2 -
> >  drivers/staging/Makefile                           |   1 -
> >  drivers/staging/clocking-wizard/Kconfig            |  10 -
> >  drivers/staging/clocking-wizard/Makefile           |   2 -
> >  drivers/staging/clocking-wizard/TODO               |  12 -
> >  .../clocking-wizard/clk-xlnx-clock-wizard.c        | 333 ----------
> >  drivers/staging/clocking-wizard/dt-binding.txt     |  30 -
> >  11 files changed, 764 insertions(+), 390 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> >  create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c
> >  delete mode 100644 drivers/staging/clocking-wizard/Kconfig
> >  delete mode 100644 drivers/staging/clocking-wizard/Makefile
> >  delete mode 100644 drivers/staging/clocking-wizard/TODO
> >  delete mode 100644 drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
> >  delete mode 100644 drivers/staging/clocking-wizard/dt-binding.txt
> >
>
> Thanks,
> Miquèl

Re: [PATCH v9 1/7] dt-bindings: add documentation of xilinx clocking wizard

[Stephen Boyd]

Quoting Shubhrajyoti Datta (2021-02-24 06:10:08)
> On Fri, Feb 19, 2021 at 6:54 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > > > +
> > > > +  xlnx,speed-grade:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [1, 2, 3]
> > > > +    description:
> > > > +      Speed grade of the device.
> > >
> > > A bit of explanation of what this describes would be welcome.
> > >
> > > Don't forget that binding are not tied to any driver implementation,
> > > these are supposed to be hardware description properties.
> >
> > Would opp tables work for this?
> This is the parameter is for speed of the fabric.

Ok. Yes or no? Is it configuring the speed of the fabric? Sounds like
assigned-clock-rates or assigned-interconnect-bandwidth or something
like that.

Re: [PATCH v9 1/7] dt-bindings: add documentation of xilinx clocking wizard

[Shubhrajyoti Datta]

Hi Stephen,

On Wed, Mar 3, 2021 at 4:37 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Shubhrajyoti Datta (2021-02-24 06:10:08)
> > On Fri, Feb 19, 2021 at 6:54 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > > > +
> > > > > +  xlnx,speed-grade:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +    enum: [1, 2, 3]
> > > > > +    description:
> > > > > +      Speed grade of the device.
> > > >
> > > > A bit of explanation of what this describes would be welcome.
> > > >
> > > > Don't forget that binding are not tied to any driver implementation,
> > > > these are supposed to be hardware description properties.
> > >
> > > Would opp tables work for this?
> > This is the parameter is for speed of the fabric.
>
> Ok. Yes or no? Is it configuring the speed of the fabric? Sounds like
> assigned-clock-rates or assigned-interconnect-bandwidth or something
> like that.

I do not think we could use opp tables.
Xilinx has products where we have partly FPGA and patrly PS(hardware
is not programmable) so the ip could be in PL
and the processor  in PS.

The speed grade influences a variety of timing parameters in the FPGA,
including fabric (slice), multiplier/DSP48x, BlockRAM, I/O, and other
resources parameters.

Basically the timing of the fabric is determined by it. Also we are
notifying the speed grade to the driver no configuration is done.
We are telling which speed-grade fabric we are running on.

 There is no correlation between these numbers. It is really a
relative metric of performance within a specific family.