[PATCH v2 0/9] clk: clocking-wizard: add user clock monitor support

[PATCH v2 0/9] clk: clocking-wizard: add user clock monitor support

[Harry Austen]

Improve utilised clk/notifier APIs, making use of device managed versions
of functions, make dynamic reconfiguration support optional (because it is
in hardware) and add support for the clock monitor functionailty added in
version 6.0 of the Xilinx clocking wizard IP core, through use of the
auxiliary bus and UIO frameworks.

The combined addition of all of these patches allows, for example, to use
the clocking wizard solely for its user clock monitoring logic, keeping
dynamic reconfiguration support disabled.

This is currently untested on hardware, so any help testing this would be
much appreciated!

v1 -> v2:
- Split and improve clk_hw+devres transition patch (2+3)
- Fix/improve DT binding patches (5+8)
- Utilise auxiliary bus in monitor support patch (6)
- Add dedicated UIO driver for monitor support (7)

Harry Austen (9):
  clk: clocking-wizard: simplify probe/remove with devres helpers
  clk: clocking-wizard: use newer clk_hw API
  clk: clocking-wizard: use devres versions of clk_hw API
  clk: clocking-wizard: move clock registration to separate function
  dt-bindings: clock: xilinx: add description of user monitor interrupt
  clk: clocking-wizard: add user clock monitor support
  uio: add Xilinx user clock monitor support
  dt-bindings: clock: xilinx: describe whether dynamic reconfig is
    enabled
  clk: clocking-wizard: move dynamic reconfig setup behind flag

 .../bindings/clock/xlnx,clocking-wizard.yaml  |  32 +-
 drivers/clk/xilinx/Kconfig                    |   1 +
 drivers/clk/xilinx/clk-xlnx-clock-wizard.c    | 335 +++++++++---------
 drivers/uio/Kconfig                           |   8 +
 drivers/uio/Makefile                          |   1 +
 drivers/uio/uio_xlnx_clk_mon.c                |  71 ++++
 6 files changed, 285 insertions(+), 163 deletions(-)
 create mode 100644 drivers/uio/uio_xlnx_clk_mon.c

-- 
2.46.0

[PATCH v2 1/9] clk: clocking-wizard: simplify probe/remove with devres helpers

[Harry Austen]

Remove need to do various operations in remove callback and error paths
by utilising device managed versions of clock and notifier APIs.

Signed-off-by: Harry Austen <hpausten@protonmail.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
v1 -> v2: Add Stephen's R-b tag

 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 48 ++++++----------------
 1 file changed, 13 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 19eb3fb7ae319..0ca045849ea3e 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -1001,21 +1001,15 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->clk_in1),
 				     "clk_in1 not found\n");
 
-	clk_wzrd->axi_clk = devm_clk_get(&pdev->dev, "s_axi_aclk");
+	clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
 	if (IS_ERR(clk_wzrd->axi_clk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->axi_clk),
 				     "s_axi_aclk not found\n");
-	ret = clk_prepare_enable(clk_wzrd->axi_clk);
-	if (ret) {
-		dev_err(&pdev->dev, "enabling s_axi_aclk failed\n");
-		return ret;
-	}
 	rate = clk_get_rate(clk_wzrd->axi_clk);
 	if (rate > WZRD_ACLK_MAX_FREQ) {
 		dev_err(&pdev->dev, "s_axi_aclk frequency (%lu) too high\n",
 			rate);
-		ret = -EINVAL;
-		goto err_disable_clk;
+		return -EINVAL;
 	}
 
 	data = device_get_match_data(&pdev->dev);
@@ -1023,16 +1017,12 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		is_versal = data->is_versal;
 
 	ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
-	if (ret || nr_outputs > WZRD_NUM_OUTPUTS) {
-		ret = -EINVAL;
-		goto err_disable_clk;
-	}
+	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
+		return -EINVAL;
 
 	clkout_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_out0", dev_name(&pdev->dev));
-	if (!clkout_name) {
-		ret = -ENOMEM;
-		goto err_disable_clk;
-	}
+	if (!clkout_name)
+		return -ENOMEM;
 
 	if (is_versal) {
 		if (nr_outputs == 1) {
@@ -1090,18 +1080,15 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		div = 1000;
 	}
 	clk_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_mul", dev_name(&pdev->dev));
-	if (!clk_name) {
-		ret = -ENOMEM;
-		goto err_disable_clk;
-	}
+	if (!clk_name)
+		return -ENOMEM;
 	clk_wzrd->clks_internal[wzrd_clk_mul] = clk_register_fixed_factor
 			(&pdev->dev, clk_name,
 			 __clk_get_name(clk_wzrd->clk_in1),
 			0, mult, div);
 	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]);
-		goto err_disable_clk;
+		return PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul]);
 	}
 
 	clk_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
@@ -1197,13 +1184,14 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	if (clk_wzrd->speed_grade) {
 		clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
 
-		ret = clk_notifier_register(clk_wzrd->clk_in1,
-					    &clk_wzrd->nb);
+		ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->clk_in1,
+						 &clk_wzrd->nb);
 		if (ret)
 			dev_warn(&pdev->dev,
 				 "unable to register clock notifier\n");
 
-		ret = clk_notifier_register(clk_wzrd->axi_clk, &clk_wzrd->nb);
+		ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->axi_clk,
+						 &clk_wzrd->nb);
 		if (ret)
 			dev_warn(&pdev->dev,
 				 "unable to register clock notifier\n");
@@ -1215,9 +1203,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	clk_unregister(clk_wzrd->clks_internal[1]);
 err_rm_int_clk:
 	clk_unregister(clk_wzrd->clks_internal[0]);
-err_disable_clk:
-	clk_disable_unprepare(clk_wzrd->axi_clk);
-
 	return ret;
 }
 
@@ -1232,13 +1217,6 @@ static void clk_wzrd_remove(struct platform_device *pdev)
 		clk_unregister(clk_wzrd->clkout[i]);
 	for (i = 0; i < wzrd_clk_int_max; i++)
 		clk_unregister(clk_wzrd->clks_internal[i]);
-
-	if (clk_wzrd->speed_grade) {
-		clk_notifier_unregister(clk_wzrd->axi_clk, &clk_wzrd->nb);
-		clk_notifier_unregister(clk_wzrd->clk_in1, &clk_wzrd->nb);
-	}
-
-	clk_disable_unprepare(clk_wzrd->axi_clk);
 }
 
 static const struct of_device_id clk_wzrd_ids[] = {
-- 
2.46.0

[PATCH v2 2/9] clk: clocking-wizard: use newer clk_hw API

[Harry Austen]

Utilise clock provider API with struct clk_hw instances instead of the
consumer-side struct clk.

Signed-off-by: Harry Austen <hpausten@protonmail.com>
---
v1 -> v2:
- Move onecell data to end of struct for single allocation
- Just move to clk_hw API. Move devres transition to subsequent patch

 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 77 +++++++++++-----------
 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 0ca045849ea3e..ccaf30c2d9481 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/math64.h>
 #include <linux/module.h>
+#include <linux/overflow.h>
 #include <linux/err.h>
 #include <linux/iopoll.h>
 
@@ -121,26 +122,24 @@ enum clk_wzrd_int_clks {
 /**
  * struct clk_wzrd - Clock wizard private data structure
  *
- * @clk_data:		Clock data
+ * @clk_data:		Output clock data
  * @nb:			Notifier block
  * @base:		Memory base
  * @clk_in1:		Handle to input clock 'clk_in1'
  * @axi_clk:		Handle to input clock 's_axi_aclk'
  * @clks_internal:	Internal clocks
- * @clkout:		Output clocks
  * @speed_grade:	Speed grade of the device
  * @suspended:		Flag indicating power state of the device
  */
 struct clk_wzrd {
-	struct clk_onecell_data clk_data;
 	struct notifier_block nb;
 	void __iomem *base;
 	struct clk *clk_in1;
 	struct clk *axi_clk;
-	struct clk *clks_internal[wzrd_clk_int_max];
-	struct clk *clkout[WZRD_NUM_OUTPUTS];
+	struct clk_hw *clks_internal[wzrd_clk_int_max];
 	unsigned int speed_grade;
 	bool suspended;
+	struct clk_hw_onecell_data clk_data;
 };
 
 /**
@@ -765,7 +764,7 @@ static const struct clk_ops clk_wzrd_clk_divider_ops_f = {
 	.recalc_rate = clk_wzrd_recalc_ratef,
 };
 
-static struct clk *clk_wzrd_register_divf(struct device *dev,
+static struct clk_hw *clk_wzrd_register_divf(struct device *dev,
 					  const char *name,
 					  const char *parent_name,
 					  unsigned long flags,
@@ -805,10 +804,10 @@ static struct clk *clk_wzrd_register_divf(struct device *dev,
 	if (ret)
 		return ERR_PTR(ret);
 
-	return hw->clk;
+	return hw;
 }
 
-static struct clk *clk_wzrd_ver_register_divider(struct device *dev,
+static struct clk_hw *clk_wzrd_ver_register_divider(struct device *dev,
 						 const char *name,
 						 const char *parent_name,
 						 unsigned long flags,
@@ -852,10 +851,10 @@ static struct clk *clk_wzrd_ver_register_divider(struct device *dev,
 	if (ret)
 		return ERR_PTR(ret);
 
-	return hw->clk;
+	return hw;
 }
 
-static struct clk *clk_wzrd_register_divider(struct device *dev,
+static struct clk_hw *clk_wzrd_register_divider(struct device *dev,
 					     const char *name,
 					     const char *parent_name,
 					     unsigned long flags,
@@ -898,7 +897,7 @@ static struct clk *clk_wzrd_register_divider(struct device *dev,
 	if (ret)
 		return ERR_PTR(ret);
 
-	return hw->clk;
+	return hw;
 }
 
 static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
@@ -978,7 +977,12 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	int nr_outputs;
 	int i, ret;
 
-	clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
+	ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
+	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
+		return -EINVAL;
+
+	clk_wzrd = devm_kzalloc(&pdev->dev, struct_size(clk_wzrd, clk_data.hws, nr_outputs),
+				GFP_KERNEL);
 	if (!clk_wzrd)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, clk_wzrd);
@@ -1016,17 +1020,13 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	if (data)
 		is_versal = data->is_versal;
 
-	ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
-	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
-		return -EINVAL;
-
 	clkout_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_out0", dev_name(&pdev->dev));
 	if (!clkout_name)
 		return -ENOMEM;
 
 	if (is_versal) {
 		if (nr_outputs == 1) {
-			clk_wzrd->clkout[0] = clk_wzrd_ver_register_divider
+			clk_wzrd->clk_data.hws[0] = clk_wzrd_ver_register_divider
 				(&pdev->dev, clkout_name,
 				__clk_get_name(clk_wzrd->clk_in1), 0,
 				clk_wzrd->base, WZRD_CLK_CFG_REG(is_versal, 3),
@@ -1059,7 +1059,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		div = 64;
 	} else {
 		if (nr_outputs == 1) {
-			clk_wzrd->clkout[0] = clk_wzrd_register_divider
+			clk_wzrd->clk_data.hws[0] = clk_wzrd_register_divider
 				(&pdev->dev, clkout_name,
 				__clk_get_name(clk_wzrd->clk_in1), 0,
 				clk_wzrd->base, WZRD_CLK_CFG_REG(is_versal, 3),
@@ -1082,7 +1082,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	clk_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_mul", dev_name(&pdev->dev));
 	if (!clk_name)
 		return -ENOMEM;
-	clk_wzrd->clks_internal[wzrd_clk_mul] = clk_register_fixed_factor
+	clk_wzrd->clks_internal[wzrd_clk_mul] = clk_hw_register_fixed_factor
 			(&pdev->dev, clk_name,
 			 __clk_get_name(clk_wzrd->clk_in1),
 			0, mult, div);
@@ -1108,15 +1108,15 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		if (!div)
 			div = 1;
 
-		clk_mul_name = __clk_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]);
+		clk_mul_name = clk_hw_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]);
 		clk_wzrd->clks_internal[wzrd_clk_mul_div] =
-			clk_register_fixed_factor(&pdev->dev, clk_name,
-						  clk_mul_name, 0, 1, div);
+			clk_hw_register_fixed_factor(&pdev->dev, clk_name,
+						     clk_mul_name, 0, 1, div);
 	} else {
 		ctrl_reg = clk_wzrd->base + WZRD_CLK_CFG_REG(is_versal, 0);
-		clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_divider
+		clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_hw_register_divider
 			(&pdev->dev, clk_name,
-			 __clk_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
+			 clk_hw_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
 			flags, ctrl_reg, 0, 8, CLK_DIVIDER_ONE_BASED |
 			CLK_DIVIDER_ALLOW_ZERO, &clkwzrd_lock);
 	}
@@ -1136,7 +1136,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		}
 
 		if (is_versal) {
-			clk_wzrd->clkout[i] = clk_wzrd_ver_register_divider
+			clk_wzrd->clk_data.hws[i] = clk_wzrd_ver_register_divider
 						(&pdev->dev,
 						 clkout_name, clk_name, 0,
 						 clk_wzrd->base,
@@ -1148,7 +1148,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 						 DIV_O, &clkwzrd_lock);
 		} else {
 			if (!i)
-				clk_wzrd->clkout[i] = clk_wzrd_register_divf
+				clk_wzrd->clk_data.hws[i] = clk_wzrd_register_divf
 					(&pdev->dev, clkout_name, clk_name, flags, clk_wzrd->base,
 					(WZRD_CLK_CFG_REG(is_versal, 2) + i * 12),
 					WZRD_CLKOUT_DIVIDE_SHIFT,
@@ -1156,7 +1156,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 					CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
 					DIV_O, &clkwzrd_lock);
 			else
-				clk_wzrd->clkout[i] = clk_wzrd_register_divider
+				clk_wzrd->clk_data.hws[i] = clk_wzrd_register_divider
 					(&pdev->dev, clkout_name, clk_name, 0, clk_wzrd->base,
 					(WZRD_CLK_CFG_REG(is_versal, 2) + i * 12),
 					WZRD_CLKOUT_DIVIDE_SHIFT,
@@ -1164,22 +1164,25 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 					CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
 					DIV_O, &clkwzrd_lock);
 		}
-		if (IS_ERR(clk_wzrd->clkout[i])) {
+		if (IS_ERR(clk_wzrd->clk_data.hws[i])) {
 			int j;
 
 			for (j = i + 1; j < nr_outputs; j++)
-				clk_unregister(clk_wzrd->clkout[j]);
+				clk_hw_unregister(clk_wzrd->clk_data.hws[j]);
 			dev_err(&pdev->dev,
 				"unable to register divider clock\n");
-			ret = PTR_ERR(clk_wzrd->clkout[i]);
+			ret = PTR_ERR(clk_wzrd->clk_data.hws[i]);
 			goto err_rm_int_clks;
 		}
 	}
 
 out:
-	clk_wzrd->clk_data.clks = clk_wzrd->clkout;
-	clk_wzrd->clk_data.clk_num = ARRAY_SIZE(clk_wzrd->clkout);
-	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_wzrd->clk_data);
+	clk_wzrd->clk_data.num = nr_outputs;
+	ret = of_clk_add_hw_provider(pdev->dev.of_node, of_clk_hw_onecell_get, &clk_wzrd->clk_data);
+	if (ret) {
+		dev_err(&pdev->dev, "unable to register clock provider\n");
+		return ret;
+	}
 
 	if (clk_wzrd->speed_grade) {
 		clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
@@ -1200,9 +1203,9 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	return 0;
 
 err_rm_int_clks:
-	clk_unregister(clk_wzrd->clks_internal[1]);
+	clk_hw_unregister(clk_wzrd->clks_internal[1]);
 err_rm_int_clk:
-	clk_unregister(clk_wzrd->clks_internal[0]);
+	clk_hw_unregister(clk_wzrd->clks_internal[0]);
 	return ret;
 }
 
@@ -1214,9 +1217,9 @@ static void clk_wzrd_remove(struct platform_device *pdev)
 	of_clk_del_provider(pdev->dev.of_node);
 
 	for (i = 0; i < WZRD_NUM_OUTPUTS; i++)
-		clk_unregister(clk_wzrd->clkout[i]);
+		clk_hw_unregister(clk_wzrd->clk_data.hws[i]);
 	for (i = 0; i < wzrd_clk_int_max; i++)
-		clk_unregister(clk_wzrd->clks_internal[i]);
+		clk_hw_unregister(clk_wzrd->clks_internal[i]);
 }
 
 static const struct of_device_id clk_wzrd_ids[] = {
-- 
2.46.0

Re: [PATCH v2 2/9] clk: clocking-wizard: use newer clk_hw API

[Harry Austen]

On Sat Aug 3, 2024 at 11:57 AM BST, Harry Austen wrote:
> Utilise clock provider API with struct clk_hw instances instead of the
> consumer-side struct clk.
>
> Signed-off-by: Harry Austen <hpausten@protonmail.com>
> ---
> v1 -> v2:
> - Move onecell data to end of struct for single allocation
> - Just move to clk_hw API. Move devres transition to subsequent patch
>
>  drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 77 +++++++++++-----------
>  1 file changed, 40 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index 0ca045849ea3e..ccaf30c2d9481 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> @@ -17,6 +17,7 @@
>  #include <linux/of.h>
>  #include <linux/math64.h>
>  #include <linux/module.h>
> +#include <linux/overflow.h>
>  #include <linux/err.h>
>  #include <linux/iopoll.h>
>  
> @@ -121,26 +122,24 @@ enum clk_wzrd_int_clks {
>  /**
>   * struct clk_wzrd - Clock wizard private data structure
>   *
> - * @clk_data:		Clock data
> + * @clk_data:		Output clock data

Realised I probably should have moved this doc comment to the bottom too,
which also resulted in me putting the new `adev` parameter documentation in
a weird location in patch 6. Will fix in v3.

>   * @nb:			Notifier block
>   * @base:		Memory base
>   * @clk_in1:		Handle to input clock 'clk_in1'
>   * @axi_clk:		Handle to input clock 's_axi_aclk'
>   * @clks_internal:	Internal clocks
> - * @clkout:		Output clocks
>   * @speed_grade:	Speed grade of the device
>   * @suspended:		Flag indicating power state of the device
>   */
>  struct clk_wzrd {

[PATCH v2 3/9] clk: clocking-wizard: use devres versions of clk_hw API

[Harry Austen]

Use device managed versions of the clk_hw API, entirely removing the
need for the driver's remove() callback and greatly simplifying the
probe() function's error paths.

Signed-off-by: Harry Austen <hpausten@protonmail.com>
---
v1 -> v2: New

 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 52 +++++-----------------
 1 file changed, 11 insertions(+), 41 deletions(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index ccaf30c2d9481..810d57c46a63c 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -1082,7 +1082,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	clk_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_mul", dev_name(&pdev->dev));
 	if (!clk_name)
 		return -ENOMEM;
-	clk_wzrd->clks_internal[wzrd_clk_mul] = clk_hw_register_fixed_factor
+	clk_wzrd->clks_internal[wzrd_clk_mul] = devm_clk_hw_register_fixed_factor
 			(&pdev->dev, clk_name,
 			 __clk_get_name(clk_wzrd->clk_in1),
 			0, mult, div);
@@ -1092,10 +1092,8 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	}
 
 	clk_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
-	if (!clk_name) {
-		ret = -ENOMEM;
-		goto err_rm_int_clk;
-	}
+	if (!clk_name)
+		return -ENOMEM;
 
 	if (is_versal) {
 		edged = !!(readl(clk_wzrd->base + WZRD_CLK_CFG_REG(is_versal, 20)) &
@@ -1110,11 +1108,11 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 
 		clk_mul_name = clk_hw_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]);
 		clk_wzrd->clks_internal[wzrd_clk_mul_div] =
-			clk_hw_register_fixed_factor(&pdev->dev, clk_name,
-						     clk_mul_name, 0, 1, div);
+			devm_clk_hw_register_fixed_factor(&pdev->dev, clk_name,
+							  clk_mul_name, 0, 1, div);
 	} else {
 		ctrl_reg = clk_wzrd->base + WZRD_CLK_CFG_REG(is_versal, 0);
-		clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_hw_register_divider
+		clk_wzrd->clks_internal[wzrd_clk_mul_div] = devm_clk_hw_register_divider
 			(&pdev->dev, clk_name,
 			 clk_hw_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
 			flags, ctrl_reg, 0, 8, CLK_DIVIDER_ONE_BASED |
@@ -1122,18 +1120,15 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	}
 	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]);
-		goto err_rm_int_clk;
+		return PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div]);
 	}
 
 	/* register div per output */
 	for (i = nr_outputs - 1; i >= 0 ; i--) {
 		clkout_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
 					     "%s_out%d", dev_name(&pdev->dev), i);
-		if (!clkout_name) {
-			ret = -ENOMEM;
-			goto err_rm_int_clk;
-		}
+		if (!clkout_name)
+			return -ENOMEM;
 
 		if (is_versal) {
 			clk_wzrd->clk_data.hws[i] = clk_wzrd_ver_register_divider
@@ -1165,20 +1160,15 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 					DIV_O, &clkwzrd_lock);
 		}
 		if (IS_ERR(clk_wzrd->clk_data.hws[i])) {
-			int j;
-
-			for (j = i + 1; j < nr_outputs; j++)
-				clk_hw_unregister(clk_wzrd->clk_data.hws[j]);
 			dev_err(&pdev->dev,
 				"unable to register divider clock\n");
-			ret = PTR_ERR(clk_wzrd->clk_data.hws[i]);
-			goto err_rm_int_clks;
+			return PTR_ERR(clk_wzrd->clk_data.hws[i]);
 		}
 	}
 
 out:
 	clk_wzrd->clk_data.num = nr_outputs;
-	ret = of_clk_add_hw_provider(pdev->dev.of_node, of_clk_hw_onecell_get, &clk_wzrd->clk_data);
+	ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get, &clk_wzrd->clk_data);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to register clock provider\n");
 		return ret;
@@ -1201,25 +1191,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	}
 
 	return 0;
-
-err_rm_int_clks:
-	clk_hw_unregister(clk_wzrd->clks_internal[1]);
-err_rm_int_clk:
-	clk_hw_unregister(clk_wzrd->clks_internal[0]);
-	return ret;
-}
-
-static void clk_wzrd_remove(struct platform_device *pdev)
-{
-	int i;
-	struct clk_wzrd *clk_wzrd = platform_get_drvdata(pdev);
-
-	of_clk_del_provider(pdev->dev.of_node);
-
-	for (i = 0; i < WZRD_NUM_OUTPUTS; i++)
-		clk_hw_unregister(clk_wzrd->clk_data.hws[i]);
-	for (i = 0; i < wzrd_clk_int_max; i++)
-		clk_hw_unregister(clk_wzrd->clks_internal[i]);
 }
 
 static const struct of_device_id clk_wzrd_ids[] = {
@@ -1238,7 +1209,6 @@ static struct platform_driver clk_wzrd_driver = {
 		.pm = &clk_wzrd_dev_pm_ops,
 	},
 	.probe = clk_wzrd_probe,
-	.remove_new = clk_wzrd_remove,
 };
 module_platform_driver(clk_wzrd_driver);
 
-- 
2.46.0

[PATCH v2 4/9] clk: clocking-wizard: move clock registration to separate function

[Harry Austen]

Provide clear separation of dynamic reconfiguration logic, by moving its
setup procedure to its own dedicated function.

Signed-off-by: Harry Austen <hpausten@protonmail.com>
---
v1 -> v2: No functional change

 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 143 +++++++++++----------
 1 file changed, 75 insertions(+), 68 deletions(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 810d57c46a63c..1f8023d24029f 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -962,72 +962,30 @@ static const struct versal_clk_data versal_data = {
 	.is_versal	= true,
 };
 
-static int clk_wzrd_probe(struct platform_device *pdev)
+static int clk_wzrd_register_output_clocks(struct device *dev, int nr_outputs)
 {
 	const char *clkout_name, *clk_name, *clk_mul_name;
+	struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
 	u32 regl, regh, edge, regld, reghd, edged, div;
-	struct device_node *np = pdev->dev.of_node;
 	const struct versal_clk_data *data;
-	struct clk_wzrd *clk_wzrd;
 	unsigned long flags = 0;
+	bool is_versal = false;
 	void __iomem *ctrl_reg;
 	u32 reg, reg_f, mult;
-	bool is_versal = false;
-	unsigned long rate;
-	int nr_outputs;
-	int i, ret;
-
-	ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
-	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
-		return -EINVAL;
-
-	clk_wzrd = devm_kzalloc(&pdev->dev, struct_size(clk_wzrd, clk_data.hws, nr_outputs),
-				GFP_KERNEL);
-	if (!clk_wzrd)
-		return -ENOMEM;
-	platform_set_drvdata(pdev, clk_wzrd);
-
-	clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(clk_wzrd->base))
-		return PTR_ERR(clk_wzrd->base);
-
-	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",
-				 clk_wzrd->speed_grade);
-			clk_wzrd->speed_grade = 0;
-		}
-	}
-
-	clk_wzrd->clk_in1 = devm_clk_get(&pdev->dev, "clk_in1");
-	if (IS_ERR(clk_wzrd->clk_in1))
-		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->clk_in1),
-				     "clk_in1 not found\n");
+	int i;
 
-	clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
-	if (IS_ERR(clk_wzrd->axi_clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->axi_clk),
-				     "s_axi_aclk not found\n");
-	rate = clk_get_rate(clk_wzrd->axi_clk);
-	if (rate > WZRD_ACLK_MAX_FREQ) {
-		dev_err(&pdev->dev, "s_axi_aclk frequency (%lu) too high\n",
-			rate);
-		return -EINVAL;
-	}
-
-	data = device_get_match_data(&pdev->dev);
+	data = device_get_match_data(dev);
 	if (data)
 		is_versal = data->is_versal;
 
-	clkout_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_out0", dev_name(&pdev->dev));
+	clkout_name = devm_kasprintf(dev, GFP_KERNEL, "%s_out0", dev_name(dev));
 	if (!clkout_name)
 		return -ENOMEM;
 
 	if (is_versal) {
 		if (nr_outputs == 1) {
 			clk_wzrd->clk_data.hws[0] = clk_wzrd_ver_register_divider
-				(&pdev->dev, clkout_name,
+				(dev, clkout_name,
 				__clk_get_name(clk_wzrd->clk_in1), 0,
 				clk_wzrd->base, WZRD_CLK_CFG_REG(is_versal, 3),
 				WZRD_CLKOUT_DIVIDE_SHIFT,
@@ -1035,7 +993,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 				CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
 				DIV_ALL, &clkwzrd_lock);
 
-			goto out;
+			return 0;
 		}
 		/* register multiplier */
 		edge = !!(readl(clk_wzrd->base + WZRD_CLK_CFG_REG(is_versal, 0)) &
@@ -1060,7 +1018,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	} else {
 		if (nr_outputs == 1) {
 			clk_wzrd->clk_data.hws[0] = clk_wzrd_register_divider
-				(&pdev->dev, clkout_name,
+				(dev, clkout_name,
 				__clk_get_name(clk_wzrd->clk_in1), 0,
 				clk_wzrd->base, WZRD_CLK_CFG_REG(is_versal, 3),
 				WZRD_CLKOUT_DIVIDE_SHIFT,
@@ -1068,7 +1026,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 				CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
 				DIV_ALL, &clkwzrd_lock);
 
-			goto out;
+			return 0;
 		}
 		reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(is_versal, 0));
 		reg_f = reg & WZRD_CLKFBOUT_FRAC_MASK;
@@ -1079,19 +1037,19 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		mult = (reg * 1000) + reg_f;
 		div = 1000;
 	}
-	clk_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_mul", dev_name(&pdev->dev));
+	clk_name = devm_kasprintf(dev, GFP_KERNEL, "%s_mul", dev_name(dev));
 	if (!clk_name)
 		return -ENOMEM;
 	clk_wzrd->clks_internal[wzrd_clk_mul] = devm_clk_hw_register_fixed_factor
-			(&pdev->dev, clk_name,
+			(dev, clk_name,
 			 __clk_get_name(clk_wzrd->clk_in1),
 			0, mult, div);
 	if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul])) {
-		dev_err(&pdev->dev, "unable to register fixed-factor clock\n");
+		dev_err(dev, "unable to register fixed-factor clock\n");
 		return PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul]);
 	}
 
-	clk_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
+	clk_name = devm_kasprintf(dev, GFP_KERNEL, "%s_mul_div", dev_name(dev));
 	if (!clk_name)
 		return -ENOMEM;
 
@@ -1108,31 +1066,29 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 
 		clk_mul_name = clk_hw_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]);
 		clk_wzrd->clks_internal[wzrd_clk_mul_div] =
-			devm_clk_hw_register_fixed_factor(&pdev->dev, clk_name,
-							  clk_mul_name, 0, 1, div);
+			devm_clk_hw_register_fixed_factor(dev, clk_name, clk_mul_name, 0, 1, div);
 	} else {
 		ctrl_reg = clk_wzrd->base + WZRD_CLK_CFG_REG(is_versal, 0);
 		clk_wzrd->clks_internal[wzrd_clk_mul_div] = devm_clk_hw_register_divider
-			(&pdev->dev, clk_name,
+			(dev, clk_name,
 			 clk_hw_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
 			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");
+		dev_err(dev, "unable to register divider clock\n");
 		return PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div]);
 	}
 
 	/* register div per output */
 	for (i = nr_outputs - 1; i >= 0 ; i--) {
-		clkout_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
-					     "%s_out%d", dev_name(&pdev->dev), i);
+		clkout_name = devm_kasprintf(dev, GFP_KERNEL, "%s_out%d", dev_name(dev), i);
 		if (!clkout_name)
 			return -ENOMEM;
 
 		if (is_versal) {
 			clk_wzrd->clk_data.hws[i] = clk_wzrd_ver_register_divider
-						(&pdev->dev,
+						(dev,
 						 clkout_name, clk_name, 0,
 						 clk_wzrd->base,
 						 (WZRD_CLK_CFG_REG(is_versal, 3) + i * 8),
@@ -1144,7 +1100,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		} else {
 			if (!i)
 				clk_wzrd->clk_data.hws[i] = clk_wzrd_register_divf
-					(&pdev->dev, clkout_name, clk_name, flags, clk_wzrd->base,
+					(dev, clkout_name, clk_name, flags, clk_wzrd->base,
 					(WZRD_CLK_CFG_REG(is_versal, 2) + i * 12),
 					WZRD_CLKOUT_DIVIDE_SHIFT,
 					WZRD_CLKOUT_DIVIDE_WIDTH,
@@ -1152,7 +1108,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 					DIV_O, &clkwzrd_lock);
 			else
 				clk_wzrd->clk_data.hws[i] = clk_wzrd_register_divider
-					(&pdev->dev, clkout_name, clk_name, 0, clk_wzrd->base,
+					(dev, clkout_name, clk_name, 0, clk_wzrd->base,
 					(WZRD_CLK_CFG_REG(is_versal, 2) + i * 12),
 					WZRD_CLKOUT_DIVIDE_SHIFT,
 					WZRD_CLKOUT_DIVIDE_WIDTH,
@@ -1160,13 +1116,64 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 					DIV_O, &clkwzrd_lock);
 		}
 		if (IS_ERR(clk_wzrd->clk_data.hws[i])) {
-			dev_err(&pdev->dev,
-				"unable to register divider clock\n");
+			dev_err(dev, "unable to register divider clock\n");
 			return PTR_ERR(clk_wzrd->clk_data.hws[i]);
 		}
 	}
 
-out:
+	return 0;
+}
+
+static int clk_wzrd_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct clk_wzrd *clk_wzrd;
+	unsigned long rate;
+	int nr_outputs;
+	int ret;
+
+	ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
+	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
+		return -EINVAL;
+
+	clk_wzrd = devm_kzalloc(&pdev->dev, struct_size(clk_wzrd, clk_data.hws, nr_outputs),
+				GFP_KERNEL);
+	if (!clk_wzrd)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, clk_wzrd);
+
+	clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(clk_wzrd->base))
+		return PTR_ERR(clk_wzrd->base);
+
+	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",
+				 clk_wzrd->speed_grade);
+			clk_wzrd->speed_grade = 0;
+		}
+	}
+
+	clk_wzrd->clk_in1 = devm_clk_get(&pdev->dev, "clk_in1");
+	if (IS_ERR(clk_wzrd->clk_in1))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->clk_in1),
+				     "clk_in1 not found\n");
+
+	clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
+	if (IS_ERR(clk_wzrd->axi_clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->axi_clk),
+				     "s_axi_aclk not found\n");
+	rate = clk_get_rate(clk_wzrd->axi_clk);
+	if (rate > WZRD_ACLK_MAX_FREQ) {
+		dev_err(&pdev->dev, "s_axi_aclk frequency (%lu) too high\n", rate);
+		return -EINVAL;
+	}
+
+	ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
+	if (ret)
+		return ret;
+
 	clk_wzrd->clk_data.num = nr_outputs;
 	ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get, &clk_wzrd->clk_data);
 	if (ret) {
-- 
2.46.0

[PATCH v2 5/9] dt-bindings: clock: xilinx: add description of user monitor interrupt

[Harry Austen]

This Xilinx clocking wizard IP core outputs this interrupt signal to
indicate when one of the four optional user clock inputs is either
stopped, overruns, underruns or glitches.

This functionality was only added from version 6.0 onwards, so restrict
it to particular compatible strings.

Signed-off-by: Harry Austen <hpausten@protonmail.com>
---
v1 -> v2: Fix binding errors by moving interrupts up front, restrict later

 .../bindings/clock/xlnx,clocking-wizard.yaml  | 25 ++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
index 9d5324dc1027a..9e5078cef2962 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -39,6 +39,14 @@ properties:
       - const: clk_in1
       - const: s_axi_aclk
 
+  interrupts:
+    items:
+      - description: user clock monitor interrupt
+
+  interrupt-names:
+    items:
+      - const: monitor
+
 
   xlnx,speed-grade:
     $ref: /schemas/types.yaml#/definitions/uint32
@@ -62,17 +70,32 @@ required:
   - xlnx,speed-grade
   - xlnx,nr-outputs
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          enum:
+            - xlnx,clocking-wizard
+            - xlnx,clocking-wizard-v5.2
+    then:
+      properties:
+        interrupts: false
+        interrupt-names: false
+
 additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/interrupt-controller/irq.h>
     clock-controller@b0000000  {
-        compatible = "xlnx,clocking-wizard";
+        compatible = "xlnx,clocking-wizard-v6.0";
         reg = <0xb0000000 0x10000>;
         #clock-cells = <1>;
         xlnx,speed-grade = <1>;
         xlnx,nr-outputs = <6>;
         clock-names = "clk_in1", "s_axi_aclk";
         clocks = <&clkc 15>, <&clkc 15>;
+        interrupts-extended = <&intc 52 IRQ_TYPE_EDGE_RISING>;
+        interrupt-names = "monitor";
     };
 ...
-- 
2.46.0

Re: [PATCH v2 5/9] dt-bindings: clock: xilinx: add description of user monitor interrupt

[Krzysztof Kozlowski]

On 03/08/2024 12:58, Harry Austen wrote:
> This Xilinx clocking wizard IP core outputs this interrupt signal to
> indicate when one of the four optional user clock inputs is either
> stopped, overruns, underruns or glitches.
> 
> This functionality was only added from version 6.0 onwards, so restrict
> it to particular compatible strings.
> 
> Signed-off-by: Harry Austen <hpausten@protonmail.com>
> ---
> v1 -> v2: Fix binding errors by moving interrupts up front, restrict later
> 
>  .../bindings/clock/xlnx,clocking-wizard.yaml  | 25 ++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> index 9d5324dc1027a..9e5078cef2962 100644
> --- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> @@ -39,6 +39,14 @@ properties:
>        - const: clk_in1
>        - const: s_axi_aclk
>  
> +  interrupts:
> +    items:
> +      - description: user clock monitor interrupt
> +
> +  interrupt-names:
> +    items:
> +      - const: monitor
> +
>  

Why multiple blank lines? Only one.

>    xlnx,speed-grade:
>      $ref: /schemas/types.yaml#/definitions/uint32
> @@ -62,17 +70,32 @@ required:
>    - xlnx,speed-grade
>    - xlnx,nr-outputs

With above fixed:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 5/9] dt-bindings: clock: xilinx: add description of user monitor interrupt

[Harry Austen]

On Sun Aug 4, 2024 at 10:04 AM BST, Krzysztof Kozlowski wrote:
> On 03/08/2024 12:58, Harry Austen wrote:
> > This Xilinx clocking wizard IP core outputs this interrupt signal to
> > indicate when one of the four optional user clock inputs is either
> > stopped, overruns, underruns or glitches.
> >
> > This functionality was only added from version 6.0 onwards, so restrict
> > it to particular compatible strings.
> >
> > Signed-off-by: Harry Austen <hpausten@protonmail.com>
> > ---
> > v1 -> v2: Fix binding errors by moving interrupts up front, restrict later
> >
> >  .../bindings/clock/xlnx,clocking-wizard.yaml  | 25 ++++++++++++++++++-
> >  1 file changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > index 9d5324dc1027a..9e5078cef2962 100644
> > --- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > @@ -39,6 +39,14 @@ properties:
> >        - const: clk_in1
> >        - const: s_axi_aclk
> >
> > +  interrupts:
> > +    items:
> > +      - description: user clock monitor interrupt
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: monitor
> > +
> >
>
> Why multiple blank lines? Only one.

There were two blank lines here previously. I assumed it may have been to
separate out the vendor specific properties. Will remove in v3.

>
> >    xlnx,speed-grade:
> >      $ref: /schemas/types.yaml#/definitions/uint32
> > @@ -62,17 +70,32 @@ required:
> >    - xlnx,speed-grade
> >    - xlnx,nr-outputs
>
> With above fixed:
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks!

>
> Best regards,
> Krzysztof

[PATCH v2 6/9] clk: clocking-wizard: add user clock monitor support

[Harry Austen]

Xilinx clocking wizard IP core supports monitoring of up to four
optional user clock inputs, with a corresponding interrupt for
notification in change of clock state (stop, underrun, overrun or
glitch). Give access to this monitor logic through use of an auxiliary
device.

Use presence of the user monitor interrupt description in devicetree to
indicate whether or not the auxiliary device should be registered. Also,
this functionality is only supported from v6.0 onwards, so add
indication of support to the device match data, in order to be tied to
the utilised compatible string.

Signed-off-by: Harry Austen <hpausten@protonmail.com>
---
v1 -> v2:
- Remove direct UIO dependency by utilising auxiliary device
- Move some logic from probe into clk_wzrd_setup_monitor for tidiness

 drivers/clk/xilinx/Kconfig                 |  1 +
 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 60 ++++++++++++++++++++--
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig
index 051756953558b..87f507bd9b6f3 100644
--- a/drivers/clk/xilinx/Kconfig
+++ b/drivers/clk/xilinx/Kconfig
@@ -21,6 +21,7 @@ config COMMON_CLK_XLNX_CLKWZRD
 	tristate "Xilinx Clocking Wizard"
 	depends on OF
 	depends on HAS_IOMEM
+	select AUXILIARY_BUS
 	help
 	  Support for the Xilinx Clocking Wizard IP core clock generator.
 	  Adds support for clocking wizard and compatible.
diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 1f8023d24029f..557e11017faf9 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -8,6 +8,7 @@
  *
  */
 
+#include <linux/auxiliary_bus.h>
 #include <linux/bitfield.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
@@ -129,6 +130,7 @@ enum clk_wzrd_int_clks {
  * @axi_clk:		Handle to input clock 's_axi_aclk'
  * @clks_internal:	Internal clocks
  * @speed_grade:	Speed grade of the device
+ * @adev:		User clock monitor auxiliary device
  * @suspended:		Flag indicating power state of the device
  */
 struct clk_wzrd {
@@ -139,6 +141,7 @@ struct clk_wzrd {
 	struct clk_hw *clks_internal[wzrd_clk_int_max];
 	unsigned int speed_grade;
 	bool suspended;
+	struct auxiliary_device adev;
 	struct clk_hw_onecell_data clk_data;
 };
 
@@ -171,8 +174,9 @@ struct clk_wzrd_divider {
 	spinlock_t *lock;  /* divider lock */
 };
 
-struct versal_clk_data {
+struct clk_wzrd_data {
 	bool is_versal;
+	bool supports_monitor;
 };
 
 #define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb)
@@ -958,16 +962,58 @@ static int __maybe_unused clk_wzrd_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(clk_wzrd_dev_pm_ops, clk_wzrd_suspend,
 			 clk_wzrd_resume);
 
-static const struct versal_clk_data versal_data = {
-	.is_versal	= true,
+static const struct clk_wzrd_data version_6_0_data = {
+	.is_versal		= false,
+	.supports_monitor	= true,
 };
 
+static const struct clk_wzrd_data versal_data = {
+	.is_versal		= true,
+	.supports_monitor	= true,
+};
+
+static void clk_wzrd_unregister_adev(void *_adev)
+{
+	struct auxiliary_device *adev = _adev;
+
+	auxiliary_device_delete(adev);
+	auxiliary_device_uninit(adev);
+}
+
+static int clk_wzrd_setup_monitor(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct clk_wzrd_data *data = device_get_match_data(dev);
+	struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
+	struct auxiliary_device *adev = &clk_wzrd->adev;
+	int ret;
+
+	if (!data || !data->supports_monitor)
+		return 0;
+
+	adev->name = "clk-mon";
+	adev->dev.parent = dev;
+	adev->dev.platform_data = (__force void *)clk_wzrd->base;
+
+	ret = auxiliary_device_init(adev);
+	if (ret)
+		return ret;
+
+	ret = auxiliary_device_add(adev);
+	if (ret) {
+		auxiliary_device_uninit(adev);
+		return ret;
+	}
+
+	return devm_add_action_or_reset(dev, clk_wzrd_unregister_adev, adev);
+}
+
 static int clk_wzrd_register_output_clocks(struct device *dev, int nr_outputs)
 {
 	const char *clkout_name, *clk_name, *clk_mul_name;
 	struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
 	u32 regl, regh, edge, regld, reghd, edged, div;
-	const struct versal_clk_data *data;
+	const struct clk_wzrd_data *data;
 	unsigned long flags = 0;
 	bool is_versal = false;
 	void __iomem *ctrl_reg;
@@ -1170,6 +1216,10 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	ret = clk_wzrd_setup_monitor(pdev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n");
+
 	ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
 	if (ret)
 		return ret;
@@ -1204,7 +1254,7 @@ static const struct of_device_id clk_wzrd_ids[] = {
 	{ .compatible = "xlnx,versal-clk-wizard", .data = &versal_data },
 	{ .compatible = "xlnx,clocking-wizard"   },
 	{ .compatible = "xlnx,clocking-wizard-v5.2"   },
-	{ .compatible = "xlnx,clocking-wizard-v6.0"  },
+	{ .compatible = "xlnx,clocking-wizard-v6.0", .data = &version_6_0_data },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, clk_wzrd_ids);
-- 
2.46.0

RE: [PATCH v2 6/9] clk: clocking-wizard: add user clock monitor support

[Datta, Shubhrajyoti]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Harry Austen <hpausten@protonmail.com>
> Sent: Saturday, August 3, 2024 4:28 PM
> To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Simek, Michal
> <michal.simek@amd.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Cc: Datta, Shubhrajyoti <shubhrajyoti.datta@amd.com>; Dave Ertman
> <david.m.ertman@intel.com>; Ira Weiny <ira.weiny@intel.com>; linux-
> clk@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Harry Austen
> <hpausten@protonmail.com>
> Subject: [PATCH v2 6/9] clk: clocking-wizard: add user clock monitor support
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Xilinx clocking wizard IP core supports monitoring of up to four optional user
> clock inputs, with a corresponding interrupt for notification in change of
> clock state (stop, underrun, overrun or glitch). Give access to this monitor
> logic through use of an auxiliary device.
>
> Use presence of the user monitor interrupt description in devicetree to
> indicate whether or not the auxiliary device should be registered. Also, this
> functionality is only supported from v6.0 onwards, so add indication of
> support to the device match data, in order to be tied to the utilised
> compatible string.
>
> Signed-off-by: Harry Austen <hpausten@protonmail.com>
> ---
> v1 -> v2:
> - Remove direct UIO dependency by utilising auxiliary device
> - Move some logic from probe into clk_wzrd_setup_monitor for tidiness
>
>  drivers/clk/xilinx/Kconfig                 |  1 +
>  drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 60 ++++++++++++++++++++--
>  2 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig index
> 051756953558b..87f507bd9b6f3 100644
> --- a/drivers/clk/xilinx/Kconfig
> +++ b/drivers/clk/xilinx/Kconfig
> @@ -21,6 +21,7 @@ config COMMON_CLK_XLNX_CLKWZRD
>         tristate "Xilinx Clocking Wizard"
>         depends on OF
>         depends on HAS_IOMEM
> +       select AUXILIARY_BUS
>         help
>           Support for the Xilinx Clocking Wizard IP core clock generator.
>           Adds support for clocking wizard and compatible.
> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-
> xlnx-clock-wizard.c
> index 1f8023d24029f..557e11017faf9 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> @@ -8,6 +8,7 @@
>   *
>   */
>
> +#include <linux/auxiliary_bus.h>
>  #include <linux/bitfield.h>
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
> @@ -129,6 +130,7 @@ enum clk_wzrd_int_clks {
>   * @axi_clk:           Handle to input clock 's_axi_aclk'
>   * @clks_internal:     Internal clocks
>   * @speed_grade:       Speed grade of the device
> + * @adev:              User clock monitor auxiliary device
>   * @suspended:         Flag indicating power state of the device
>   */
>  struct clk_wzrd {
> @@ -139,6 +141,7 @@ struct clk_wzrd {
>         struct clk_hw *clks_internal[wzrd_clk_int_max];
>         unsigned int speed_grade;
>         bool suspended;
> +       struct auxiliary_device adev;
>         struct clk_hw_onecell_data clk_data;  };
>
> @@ -171,8 +174,9 @@ struct clk_wzrd_divider {
>         spinlock_t *lock;  /* divider lock */  };
>
> -struct versal_clk_data {
> +struct clk_wzrd_data {
>         bool is_versal;
> +       bool supports_monitor;
>  };
>
>  #define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb) @@ -
> 958,16 +962,58 @@ static int __maybe_unused clk_wzrd_resume(struct
> device *dev)  static SIMPLE_DEV_PM_OPS(clk_wzrd_dev_pm_ops,
> clk_wzrd_suspend,
>                          clk_wzrd_resume);
>
> -static const struct versal_clk_data versal_data = {
> -       .is_versal      = true,
> +static const struct clk_wzrd_data version_6_0_data = {
> +       .is_versal              = false,
> +       .supports_monitor       = true,
>  };

The clocking wizard monitor support is a design choice.
This will enable it for all the designs.

>
> +static const struct clk_wzrd_data versal_data = {
> +       .is_versal              = true,
> +       .supports_monitor       = true,
> +};


Same here.

> +
> +static void clk_wzrd_unregister_adev(void *_adev) {
> +       struct auxiliary_device *adev = _adev;
> +
> +       auxiliary_device_delete(adev);
> +       auxiliary_device_uninit(adev);
> +}
> +
> +static int clk_wzrd_setup_monitor(struct platform_device *pdev) {
> +       struct device *dev = &pdev->dev;
> +       const struct clk_wzrd_data *data = device_get_match_data(dev);
> +       struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
> +       struct auxiliary_device *adev = &clk_wzrd->adev;
> +       int ret;
> +
> +       if (!data || !data->supports_monitor)
> +               return 0;
> +
> +       adev->name = "clk-mon";
> +       adev->dev.parent = dev;
> +       adev->dev.platform_data = (__force void *)clk_wzrd->base;
> +
> +       ret = auxiliary_device_init(adev);
> +       if (ret)
> +               return ret;
> +
> +       ret = auxiliary_device_add(adev);
> +       if (ret) {
> +               auxiliary_device_uninit(adev);
> +               return ret;
> +       }
> +
> +       return devm_add_action_or_reset(dev, clk_wzrd_unregister_adev,
> +adev); }
> +
>  static int clk_wzrd_register_output_clocks(struct device *dev, int
> nr_outputs)  {
>         const char *clkout_name, *clk_name, *clk_mul_name;
>         struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
>         u32 regl, regh, edge, regld, reghd, edged, div;
> -       const struct versal_clk_data *data;
> +       const struct clk_wzrd_data *data;
>         unsigned long flags = 0;
>         bool is_versal = false;
>         void __iomem *ctrl_reg;
> @@ -1170,6 +1216,10 @@ static int clk_wzrd_probe(struct platform_device
> *pdev)
>                 return -EINVAL;
>         }
>
> +       ret = clk_wzrd_setup_monitor(pdev);
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "failed to setup
> + monitor\n");
> +
>         ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
>         if (ret)
>                 return ret;
> @@ -1204,7 +1254,7 @@ static const struct of_device_id clk_wzrd_ids[] = {
>         { .compatible = "xlnx,versal-clk-wizard", .data = &versal_data },
>         { .compatible = "xlnx,clocking-wizard"   },
>         { .compatible = "xlnx,clocking-wizard-v5.2"   },
> -       { .compatible = "xlnx,clocking-wizard-v6.0"  },
> +       { .compatible = "xlnx,clocking-wizard-v6.0", .data =
> + &version_6_0_data },
>         { },
>  };
>  MODULE_DEVICE_TABLE(of, clk_wzrd_ids);
> --
> 2.46.0
>

Re: [PATCH v2 6/9] clk: clocking-wizard: add user clock monitor support

[Harry Austen]

On Mon Aug 19, 2024 at 1:39 PM BST, Datta, Shubhrajyoti wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> > -----Original Message-----
...
> >
> > -static const struct versal_clk_data versal_data = {
> > -       .is_versal      = true,
> > +static const struct clk_wzrd_data version_6_0_data = {
> > +       .is_versal              = false,
> > +       .supports_monitor       = true,
> >  };
>
> The clocking wizard monitor support is a design choice.
> This will enable it for all the designs.

But the interrupt still has to be described in devicetree for the auxiliary/UIO
device to be registered. The interrupt is only used by the core for the clock
monitor functionality, so if that functionality is not built into the hardware,
then the interrupt description should be omitted. Does that not sound sensible
to you?

>
> >
> > +static const struct clk_wzrd_data versal_data = {
> > +       .is_versal              = true,
> > +       .supports_monitor       = true,
> > +};
>
>
> Same here.

Same reasoning as above.

Thanks for the review!
Harry

>
> > +
> > +static void clk_wzrd_unregister_adev(void *_adev) {
> > +       struct auxiliary_device *adev = _adev;
> > +
> > +       auxiliary_device_delete(adev);
> > +       auxiliary_device_uninit(adev);
> > +}
> > +
> > +static int clk_wzrd_setup_monitor(struct platform_device *pdev) {
> > +       struct device *dev = &pdev->dev;
> > +       const struct clk_wzrd_data *data = device_get_match_data(dev);
> > +       struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
> > +       struct auxiliary_device *adev = &clk_wzrd->adev;
> > +       int ret;
> > +
> > +       if (!data || !data->supports_monitor)
> > +               return 0;
> > +
> > +       adev->name = "clk-mon";
> > +       adev->dev.parent = dev;
> > +       adev->dev.platform_data = (__force void *)clk_wzrd->base;
> > +
> > +       ret = auxiliary_device_init(adev);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = auxiliary_device_add(adev);
> > +       if (ret) {
> > +               auxiliary_device_uninit(adev);
> > +               return ret;
> > +       }
> > +
> > +       return devm_add_action_or_reset(dev, clk_wzrd_unregister_adev,
> > +adev); }
> > +
> >  static int clk_wzrd_register_output_clocks(struct device *dev, int
> > nr_outputs)  {
> >         const char *clkout_name, *clk_name, *clk_mul_name;
> >         struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
> >         u32 regl, regh, edge, regld, reghd, edged, div;
> > -       const struct versal_clk_data *data;
> > +       const struct clk_wzrd_data *data;
> >         unsigned long flags = 0;
> >         bool is_versal = false;
> >         void __iomem *ctrl_reg;
> > @@ -1170,6 +1216,10 @@ static int clk_wzrd_probe(struct platform_device
> > *pdev)
> >                 return -EINVAL;
> >         }
> >
> > +       ret = clk_wzrd_setup_monitor(pdev);
> > +       if (ret)
> > +               return dev_err_probe(&pdev->dev, ret, "failed to setup
> > + monitor\n");
> > +
> >         ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
> >         if (ret)
> >                 return ret;
> > @@ -1204,7 +1254,7 @@ static const struct of_device_id clk_wzrd_ids[] = {
> >         { .compatible = "xlnx,versal-clk-wizard", .data = &versal_data },
> >         { .compatible = "xlnx,clocking-wizard"   },
> >         { .compatible = "xlnx,clocking-wizard-v5.2"   },
> > -       { .compatible = "xlnx,clocking-wizard-v6.0"  },
> > +       { .compatible = "xlnx,clocking-wizard-v6.0", .data =
> > + &version_6_0_data },
> >         { },
> >  };
> >  MODULE_DEVICE_TABLE(of, clk_wzrd_ids);
> > --
> > 2.46.0
> >

[PATCH v2 7/9] uio: add Xilinx user clock monitor support

[Harry Austen]

Xilinx clocking wizard IP core supports monitoring of up to four
optional user clock inputs, with a corresponding interrupt for
notification in change of clock state (stop, underrun, overrun or
glitch). Give userspace access to this monitor logic through use of the
UIO framework.

Implemented as an auxiliary_driver to avoid introducing UIO dependency
to the main clock driver.

Signed-off-by: Harry Austen <hpausten@protonmail.com>
---
v1 -> v2: New

 drivers/uio/Kconfig            |  8 ++++
 drivers/uio/Makefile           |  1 +
 drivers/uio/uio_xlnx_clk_mon.c | 71 ++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 drivers/uio/uio_xlnx_clk_mon.c

diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index b060dcd7c6350..ca8a53de26a67 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -164,4 +164,12 @@ config UIO_DFL
 	    opae-sdk/tools/libopaeuio/
 
 	  If you compile this as a module, it will be called uio_dfl.
+
+config UIO_XLNX_CLK_MON
+	tristate "Xilinx user clock monitor support"
+	depends on COMMON_CLK_XLNX_CLKWZRD
+	help
+	  Userspace I/O interface to the user clock monitor logic within the
+	  Xilinx Clocking Wizard IP core.
+
 endif
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index 1c5f3b5a95cf5..1e8c242265431 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
 obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
 obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
 obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
+obj-$(CONFIG_UIO_XLNX_CLK_MON)	+= uio_xlnx_clk_mon.o
diff --git a/drivers/uio/uio_xlnx_clk_mon.c b/drivers/uio/uio_xlnx_clk_mon.c
new file mode 100644
index 0000000000000..afcbeae98eaaf
--- /dev/null
+++ b/drivers/uio/uio_xlnx_clk_mon.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for user clock monitor logic within Xilinx 'Clocking Wizard' IP core
+ *
+ * Copyright (C) 2024 Harry Austen <hpausten@protonmail.com>
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/bits.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/uio_driver.h>
+
+#define WZRD_INTR_ENABLE	0x10
+
+static int clk_mon_irqcontrol(struct uio_info *info, s32 irq_on)
+{
+	if (irq_on)
+		iowrite32(GENMASK(15, 0), info->mem[0].internal_addr + WZRD_INTR_ENABLE);
+	else
+		iowrite32(0, info->mem[0].internal_addr + WZRD_INTR_ENABLE);
+
+	return 0;
+}
+
+static int probe(struct auxiliary_device *adev, const struct auxiliary_device_id *id)
+{
+	struct platform_device *pdev = to_platform_device(adev->dev.parent);
+	struct device *dev = &adev->dev;
+	struct uio_info *info;
+	int irq;
+
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return 0;
+
+	info->name = KBUILD_MODNAME;
+	info->version = "0.0.1";
+
+	info->mem[0].name = "clock monitor";
+	info->mem[0].memtype = UIO_MEM_PHYS;
+	info->mem[0].addr = platform_get_resource(pdev, IORESOURCE_IO, 0)->start;
+	info->mem[0].size = (WZRD_INTR_ENABLE + 4 + PAGE_SIZE - 1) & PAGE_MASK;
+	info->mem[0].internal_addr = (__force void __iomem *)dev->platform_data;
+
+	info->irq = irq;
+	info->irqcontrol = clk_mon_irqcontrol;
+	return devm_uio_register_device(dev, info);
+}
+
+static struct auxiliary_device_id ids[] = {
+	{ .name = "clk_xlnx_clock_wizard.clk-mon" },
+	{}
+};
+MODULE_DEVICE_TABLE(auxiliary, ids);
+
+static struct auxiliary_driver xlnx_clk_mon_driver = {
+	.id_table = ids,
+	.probe = probe,
+};
+
+module_auxiliary_driver(xlnx_clk_mon_driver);
+
+MODULE_AUTHOR("Harry Austen <hpausten@protonmail.com>");
+MODULE_DESCRIPTION("Driver for Xilinx user clock monitor logic");
+MODULE_LICENSE("GPL");
-- 
2.46.0

[PATCH v2 8/9] dt-bindings: clock: xilinx: describe whether dynamic reconfig is enabled

[Harry Austen]

Xilinx clocking wizard IP core's dynamic reconfiguration support is
optionally enabled at build time. Add a devicetree boolean property to
describe whether the hardware supports this feature or not.

Signed-off-by: Harry Austen <hpausten@protonmail.com>
---
v1 -> v2: Use "flag" instead of boolean type

 .../devicetree/bindings/clock/xlnx,clocking-wizard.yaml    | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
index 9e5078cef2962..8b28a01dbb993 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -48,6 +48,12 @@ properties:
       - const: monitor
 
 
+  xlnx,dynamic-reconfig:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Indicate whether the core has been configured with support for dynamic
+      runtime reconfguration of the clocking primitive MMCM/PLL.
+
   xlnx,speed-grade:
     $ref: /schemas/types.yaml#/definitions/uint32
     enum: [1, 2, 3]
@@ -91,6 +97,7 @@ examples:
         compatible = "xlnx,clocking-wizard-v6.0";
         reg = <0xb0000000 0x10000>;
         #clock-cells = <1>;
+        xlnx,dynamic-reconfig;
         xlnx,speed-grade = <1>;
         xlnx,nr-outputs = <6>;
         clock-names = "clk_in1", "s_axi_aclk";
-- 
2.46.0

Re: [PATCH v2 8/9] dt-bindings: clock: xilinx: describe whether dynamic reconfig is enabled

[Krzysztof Kozlowski]

On 03/08/2024 12:58, Harry Austen wrote:
> Xilinx clocking wizard IP core's dynamic reconfiguration support is
> optionally enabled at build time. Add a devicetree boolean property to
> describe whether the hardware supports this feature or not.
> 
> Signed-off-by: Harry Austen <hpausten@protonmail.com>
> ---
> v1 -> v2: Use "flag" instead of boolean type

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v2 9/9] clk: clocking-wizard: move dynamic reconfig setup behind flag

[Harry Austen]

Xilinx clocking wizard IP core's dynamic reconfiguration support is
optionally enabled at build time. Use the new boolean devicetree
property to indicate whether the hardware supports this feature or not.

Signed-off-by: Harry Austen <hpausten@protonmail.com>
---
v1 -> v2: No functional change

 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 73 +++++++++++-----------
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 557e11017faf9..fb655d53b2029 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -1192,20 +1192,6 @@ 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, "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",
-				 clk_wzrd->speed_grade);
-			clk_wzrd->speed_grade = 0;
-		}
-	}
-
-	clk_wzrd->clk_in1 = devm_clk_get(&pdev->dev, "clk_in1");
-	if (IS_ERR(clk_wzrd->clk_in1))
-		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->clk_in1),
-				     "clk_in1 not found\n");
-
 	clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
 	if (IS_ERR(clk_wzrd->axi_clk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->axi_clk),
@@ -1220,31 +1206,48 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n");
 
-	ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
-	if (ret)
-		return ret;
-
-	clk_wzrd->clk_data.num = nr_outputs;
-	ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get, &clk_wzrd->clk_data);
-	if (ret) {
-		dev_err(&pdev->dev, "unable to register clock provider\n");
-		return ret;
-	}
+	if (of_property_read_bool(np, "xlnx,dynamic-reconfig")) {
+		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",
+					 clk_wzrd->speed_grade);
+				clk_wzrd->speed_grade = 0;
+			}
+		}
 
-	if (clk_wzrd->speed_grade) {
-		clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
+		clk_wzrd->clk_in1 = devm_clk_get(&pdev->dev, "clk_in1");
+		if (IS_ERR(clk_wzrd->clk_in1))
+			return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->clk_in1),
+					     "clk_in1 not found\n");
 
-		ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->clk_in1,
-						 &clk_wzrd->nb);
+		ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
 		if (ret)
-			dev_warn(&pdev->dev,
-				 "unable to register clock notifier\n");
+			return ret;
+
+		clk_wzrd->clk_data.num = nr_outputs;
+		ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
+						  &clk_wzrd->clk_data);
+		if (ret) {
+			dev_err(&pdev->dev, "unable to register clock provider\n");
+			return ret;
+		}
 
-		ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->axi_clk,
-						 &clk_wzrd->nb);
-		if (ret)
-			dev_warn(&pdev->dev,
-				 "unable to register clock notifier\n");
+		if (clk_wzrd->speed_grade) {
+			clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
+
+			ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->clk_in1,
+							 &clk_wzrd->nb);
+			if (ret)
+				dev_warn(&pdev->dev,
+					 "unable to register clock notifier\n");
+
+			ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->axi_clk,
+							 &clk_wzrd->nb);
+			if (ret)
+				dev_warn(&pdev->dev,
+					 "unable to register clock notifier\n");
+		}
 	}
 
 	return 0;
-- 
2.46.0

Re: [PATCH v2 0/9] clk: clocking-wizard: add user clock monitor support

[Krzysztof Kozlowski]

On 03/08/2024 12:57, Harry Austen wrote:
> Improve utilised clk/notifier APIs, making use of device managed versions
> of functions, make dynamic reconfiguration support optional (because it is
> in hardware) and add support for the clock monitor functionailty added in
> version 6.0 of the Xilinx clocking wizard IP core, through use of the
> auxiliary bus and UIO frameworks.
> 
> The combined addition of all of these patches allows, for example, to use
> the clocking wizard solely for its user clock monitoring logic, keeping
> dynamic reconfiguration support disabled.
> 
> This is currently untested on hardware, so any help testing this would be
> much appreciated!
> 
> v1 -> v2:
> - Split and improve clk_hw+devres transition patch (2+3)
> - Fix/improve DT binding patches (5+8)

Be specific, what did you change? Anything can be a fix or improvement.

Best regards,
Krzysztof

Re: [PATCH v2 0/9] clk: clocking-wizard: add user clock monitor support

[Harry Austen]

On Sun Aug 4, 2024 at 10:01 AM BST, Krzysztof Kozlowski wrote:
> On 03/08/2024 12:57, Harry Austen wrote:
> > Improve utilised clk/notifier APIs, making use of device managed versions
> > of functions, make dynamic reconfiguration support optional (because it is
> > in hardware) and add support for the clock monitor functionailty added in
> > version 6.0 of the Xilinx clocking wizard IP core, through use of the
> > auxiliary bus and UIO frameworks.
> >
> > The combined addition of all of these patches allows, for example, to use
> > the clocking wizard solely for its user clock monitoring logic, keeping
> > dynamic reconfiguration support disabled.
> >
> > This is currently untested on hardware, so any help testing this would be
> > much appreciated!
> >
> > v1 -> v2:
> > - Split and improve clk_hw+devres transition patch (2+3)
> > - Fix/improve DT binding patches (5+8)
>
> Be specific, what did you change? Anything can be a fix or improvement.

This was intended as more of a summary, referencing the patches which have
their own more detailed changelogs. But I will be more descriptive in the
cover letter too in future.

>
> Best regards,
> Krzysztof

Re: [PATCH v2 0/9] clk: clocking-wizard: add user clock monitor support

[Krzysztof Kozlowski]

On 04/08/2024 14:28, Harry Austen wrote:
> On Sun Aug 4, 2024 at 10:01 AM BST, Krzysztof Kozlowski wrote:
>> On 03/08/2024 12:57, Harry Austen wrote:
>>> Improve utilised clk/notifier APIs, making use of device managed versions
>>> of functions, make dynamic reconfiguration support optional (because it is
>>> in hardware) and add support for the clock monitor functionailty added in
>>> version 6.0 of the Xilinx clocking wizard IP core, through use of the
>>> auxiliary bus and UIO frameworks.
>>>
>>> The combined addition of all of these patches allows, for example, to use
>>> the clocking wizard solely for its user clock monitoring logic, keeping
>>> dynamic reconfiguration support disabled.
>>>
>>> This is currently untested on hardware, so any help testing this would be
>>> much appreciated!
>>>
>>> v1 -> v2:
>>> - Split and improve clk_hw+devres transition patch (2+3)
>>> - Fix/improve DT binding patches (5+8)
>>
>> Be specific, what did you change? Anything can be a fix or improvement.
> 
> This was intended as more of a summary, referencing the patches which have
> their own more detailed changelogs. But I will be more descriptive in the
> cover letter too in future.

No, if more descriptive changelog is in each patch, then it is fine.

Best regards,
Krzysztof