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

[PATCH 0/7] 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.

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!

Harry Austen (7):
  clk: clocking-wizard: simplify probe/remove with devres helpers
  clk: clocking-wizard: use newer 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
  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  |  29 +-
 drivers/clk/xilinx/Kconfig                    |   1 +
 drivers/clk/xilinx/clk-xlnx-clock-wizard.c    | 346 +++++++++---------
 3 files changed, 212 insertions(+), 164 deletions(-)

-- 
2.45.2

[PATCH 1/7] 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>
---
 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.45.2

[PATCH 2/7] 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>
---
 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 101 ++++++++-------------
 1 file changed, 39 insertions(+), 62 deletions(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 0ca045849ea3e..30c5cc9dcd7e9 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,24 +122,22 @@ 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 clk_hw_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;
 };
@@ -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,
@@ -1020,13 +1019,18 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
 		return -EINVAL;
 
+	clk_wzrd->clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_wzrd->clk_data, hws,
+					  nr_outputs), GFP_KERNEL);
+	if (!clk_wzrd->clk_data)
+		return -ENOMEM;
+
 	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 +1063,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 +1086,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);
@@ -1092,10 +1096,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)) &
@@ -1108,35 +1110,32 @@ 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);
 	}
 	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->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 +1147,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 +1155,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 +1163,20 @@ 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])) {
-			int j;
-
-			for (j = i + 1; j < nr_outputs; j++)
-				clk_unregister(clk_wzrd->clkout[j]);
+		if (IS_ERR(clk_wzrd->clk_data->hws[i])) {
 			dev_err(&pdev->dev,
 				"unable to register divider clock\n");
-			ret = PTR_ERR(clk_wzrd->clkout[i]);
-			goto err_rm_int_clks;
+			return PTR_ERR(clk_wzrd->clk_data->hws[i]);
 		}
 	}
 
 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 = 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 (clk_wzrd->speed_grade) {
 		clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
@@ -1198,25 +1195,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	}
 
 	return 0;
-
-err_rm_int_clks:
-	clk_unregister(clk_wzrd->clks_internal[1]);
-err_rm_int_clk:
-	clk_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_unregister(clk_wzrd->clkout[i]);
-	for (i = 0; i < wzrd_clk_int_max; i++)
-		clk_unregister(clk_wzrd->clks_internal[i]);
 }
 
 static const struct of_device_id clk_wzrd_ids[] = {
@@ -1235,7 +1213,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.45.2

[PATCH 3/7] 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>
---
 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 151 +++++++++++----------
 1 file changed, 79 insertions(+), 72 deletions(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 30c5cc9dcd7e9..7b262d73310fe 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -962,76 +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;
-
-	clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), 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;
-		}
-	}
+	int i;
 
-	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;
-	}
-
-	data = device_get_match_data(&pdev->dev);
+	data = device_get_match_data(dev);
 	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;
-
-	clk_wzrd->clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_wzrd->clk_data, hws,
-					  nr_outputs), GFP_KERNEL);
-	if (!clk_wzrd->clk_data)
-		return -ENOMEM;
-
-	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,
@@ -1039,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)) &
@@ -1064,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,
@@ -1072,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;
@@ -1083,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] = 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;
 
@@ -1112,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] =
-			clk_hw_register_fixed_factor(&pdev->dev, clk_name,
-						     clk_mul_name, 0, 1, div);
+			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] = 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),
@@ -1148,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,
@@ -1156,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,
@@ -1164,13 +1116,68 @@ 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;
+
+	clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), 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 = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
+	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
+		return -EINVAL;
+
+	clk_wzrd->clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_wzrd->clk_data, hws,
+					  nr_outputs), GFP_KERNEL);
+	if (!clk_wzrd->clk_data)
+		return -ENOMEM;
+
+	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.45.2

[PATCH 4/7] 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>
---
 .../bindings/clock/xlnx,clocking-wizard.yaml  | 22 ++++++++++++++++++-
 1 file changed, 21 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..4609bb56b06b5 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -62,17 +62,37 @@ required:
   - xlnx,speed-grade
   - xlnx,nr-outputs
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          enum:
+            - xlnx,clocking-wizard-v6.0
+            - xlnx,versal-clk-wizard
+    then:
+      properties:
+        interrupts:
+          items:
+            - description: user clock monitor interrupt
+
+        interrupt-names:
+          items:
+            - const: monitor
+
 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.45.2

[PATCH 5/7] 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 userspace access to this monitor logic through use of the
UIO framework.

Use presence of the user monitor interrupt description in devicetree to
indicate whether or not the UIO 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>
---
 drivers/clk/xilinx/Kconfig                 |  1 +
 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 67 ++++++++++++++++++++--
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig
index 051756953558b..907a435694687 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
+	depends on UIO
 	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 7b262d73310fe..2d419e8ad4419 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -20,10 +20,13 @@
 #include <linux/overflow.h>
 #include <linux/err.h>
 #include <linux/iopoll.h>
+#include <linux/uio_driver.h>
 
 #define WZRD_NUM_OUTPUTS	7
 #define WZRD_ACLK_MAX_FREQ	250000000UL
 
+#define WZRD_INTR_ENABLE	0x10
+
 #define WZRD_CLK_CFG_REG(v, n)	(0x200 + 0x130 * (v) + 4 * (n))
 
 #define WZRD_CLKOUT0_FRAC_EN	BIT(18)
@@ -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,55 @@ 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 int clk_wzrd_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 clk_wzrd_setup_monitor(struct device *dev, int irq, const struct resource *res)
+{
+	struct clk_wzrd *clk_wzrd = dev_get_drvdata(dev);
+	struct uio_info *info;
+
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	info->name = KBUILD_MODNAME;
+	info->version = "0.0.1";
+
+	info->mem[0].name = "user monitor";
+	info->mem[0].memtype = UIO_MEM_PHYS;
+	info->mem[0].addr = res->start;
+	info->mem[0].size = WZRD_INTR_ENABLE;
+	info->mem[0].internal_addr = clk_wzrd->base;
+
+	info->irq = irq;
+	info->irqcontrol = clk_wzrd_irqcontrol;
+	return devm_uio_register_device(dev, info);
+}
+
 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;
@@ -1127,10 +1170,11 @@ static int clk_wzrd_register_output_clocks(struct device *dev, int nr_outputs)
 static int clk_wzrd_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	const struct clk_wzrd_data *data;
 	struct clk_wzrd *clk_wzrd;
 	unsigned long rate;
 	int nr_outputs;
-	int ret;
+	int irq, ret;
 
 	clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
 	if (!clk_wzrd)
@@ -1165,6 +1209,17 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	data = device_get_match_data(&pdev->dev);
+	if (data && data->supports_monitor) {
+		irq = platform_get_irq(pdev, 0);
+		if (irq > 0) {
+			ret = clk_wzrd_setup_monitor(&pdev->dev, irq,
+						     platform_get_resource(pdev, IORESOURCE_IO, 0));
+			if (ret)
+				return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n");
+		}
+	}
+
 	ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
 	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
 		return -EINVAL;
@@ -1208,7 +1263,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.45.2

[PATCH 6/7] 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>
---
 .../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 4609bb56b06b5..890aeebf6f375 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -40,6 +40,12 @@ properties:
       - const: s_axi_aclk
 
 
+  xlnx,dynamic-reconfig:
+    type: boolean
+    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]
@@ -88,6 +94,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.45.2

[PATCH 7/7] 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>
---
 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 87 +++++++++++-----------
 1 file changed, 45 insertions(+), 42 deletions(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 2d419e8ad4419..8efe5246c8c0d 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -1185,20 +1185,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,40 +1206,57 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		}
 	}
 
-	ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
-	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
-		return -EINVAL;
-
-	clk_wzrd->clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_wzrd->clk_data, hws,
-					  nr_outputs), GFP_KERNEL);
-	if (!clk_wzrd->clk_data)
-		return -ENOMEM;
+	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;
+			}
+		}
 
-	ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
-	if (ret)
-		return ret;
+		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->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 = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
+		if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
+			return -EINVAL;
 
-	if (clk_wzrd->speed_grade) {
-		clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
+		clk_wzrd->clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_wzrd->clk_data, hws,
+						  nr_outputs), GFP_KERNEL);
+		if (!clk_wzrd->clk_data)
+			return -ENOMEM;
 
-		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.45.2

Re: [PATCH 4/7] dt-bindings: clock: xilinx: add description of user monitor interrupt

[Rob Herring (Arm)]

On Sat, 20 Jul 2024 12:01:48 +0000, 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>
> ---
>  .../bindings/clock/xlnx,clocking-wizard.yaml  | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.example.dtb: clock-controller@b0000000: 'interrupt-names', 'interrupts-extended' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/clock/xlnx,clocking-wizard.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240720120048.36758-5-hpausten@protonmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 4/7] dt-bindings: clock: xilinx: add description of user monitor interrupt

[Rob Herring]

On Sat, Jul 20, 2024 at 12:01:48PM +0000, 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>
> ---
>  .../bindings/clock/xlnx,clocking-wizard.yaml  | 22 ++++++++++++++++++-
>  1 file changed, 21 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..4609bb56b06b5 100644
> --- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> @@ -62,17 +62,37 @@ required:
>    - xlnx,speed-grade
>    - xlnx,nr-outputs
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - xlnx,clocking-wizard-v6.0
> +            - xlnx,versal-clk-wizard
> +    then:
> +      properties:
> +        interrupts:
> +          items:
> +            - description: user clock monitor interrupt
> +
> +        interrupt-names:
> +          items:
> +            - const: monitor

Properties need to be defined at the top-level (outside the if/then 
schema), then restricted here.

> +
>  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.45.2
> 
> 

Re: [PATCH 4/7] dt-bindings: clock: xilinx: add description of user monitor interrupt

[Harry Austen]

On Sat Jul 20, 2024 at 3:37 PM BST, Rob Herring wrote:
> On Sat, Jul 20, 2024 at 12:01:48PM +0000, 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>
> > ---
> >  .../bindings/clock/xlnx,clocking-wizard.yaml  | 22 ++++++++++++++++++-
> >  1 file changed, 21 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..4609bb56b06b5 100644
> > --- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > @@ -62,17 +62,37 @@ required:
> >    - xlnx,speed-grade
> >    - xlnx,nr-outputs
> >
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - xlnx,clocking-wizard-v6.0
> > +            - xlnx,versal-clk-wizard
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          items:
> > +            - description: user clock monitor interrupt
> > +
> > +        interrupt-names:
> > +          items:
> > +            - const: monitor
>
> Properties need to be defined at the top-level (outside the if/then
> schema), then restricted here.

Makes sense. Will do in v2.

Thanks for the review!

>
> > +
> >  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.45.2
> >
> >

Re: [PATCH 6/7] dt-bindings: clock: xilinx: describe whether dynamic reconfig is enabled

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]

On Sat, Jul 20, 2024 at 12:01:58PM +0000, 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>
> ---
>  .../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 4609bb56b06b5..890aeebf6f375 100644
> --- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> @@ -40,6 +40,12 @@ properties:
>        - const: s_axi_aclk
>  
>  
> +  xlnx,dynamic-reconfig:
> +    type: boolean

The type here should be "flag" not boolean, boolean can be set to
"false" and what you're likely doing is just checking for the property
being present. "flag" doesn't allow false.

> +    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]
> @@ -88,6 +94,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.45.2
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Stephen Boyd]

Quoting Harry Austen (2024-07-20 05:01:29)
> 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>

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

[Stephen Boyd]

General comment: do one thing in one patch, i.e. use clk_hw API and
don't also migrate to devm in one patch.

Quoting Harry Austen (2024-07-20 05:01:36)
> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index 0ca045849ea3e..30c5cc9dcd7e9 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>

What is this include for? __counted_by()?

>  #include <linux/err.h>
>  #include <linux/iopoll.h>
>  
> @@ -121,24 +122,22 @@ 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 clk_hw_onecell_data *clk_data;

It could be placed at the end and then one allocation could be used
instead of two.

>         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;
>  };
> @@ -1108,35 +1110,32 @@ 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

Are these going to be using devm so that on failure they get
unregistered?

>                         (&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);
>         }
>         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]);
>         }

Re: [PATCH 5/7] clk: clocking-wizard: add user clock monitor support

[Stephen Boyd]

Quoting Harry Austen (2024-07-20 05:01:53)
> 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.
> 
> Use presence of the user monitor interrupt description in devicetree to
> indicate whether or not the UIO 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>
> ---
> diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig
> index 051756953558b..907a435694687 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
> +       depends on UIO

If I have a pre-v6.0 device I probably don't want UIO though. Perhaps
you should use the auxiliary bus framework to register a device that is
otherwise unused and then have the uio driver live in drivers/uio and
match that device made here. I think you can have 'imply UIO' if you
like to put a weak Kconfig dependency.

>         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 7b262d73310fe..2d419e8ad4419 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> @@ -1165,6 +1209,17 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>  
> +       data = device_get_match_data(&pdev->dev);
> +       if (data && data->supports_monitor) {
> +               irq = platform_get_irq(pdev, 0);
> +               if (irq > 0) {
> +                       ret = clk_wzrd_setup_monitor(&pdev->dev, irq,
> +                                                    platform_get_resource(pdev, IORESOURCE_IO, 0));

Any reason this can't be

		ret = clk_wzrd_setup_monitor(pdev);
		if (ret)
			return ret;

and then all the surrounding code be moved into the function, including
the dev_err_probe()?

> +                       if (ret)
> +                               return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n");
> +               }
> +       }
> +
>         ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);

Re: [PATCH 6/7] dt-bindings: clock: xilinx: describe whether dynamic reconfig is enabled

[Harry Austen]

On Mon Jul 22, 2024 at 6:13 PM BST, Conor Dooley wrote:
> On Sat, Jul 20, 2024 at 12:01:58PM +0000, 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>
> > ---
> >  .../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 4609bb56b06b5..890aeebf6f375 100644
> > --- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> > @@ -40,6 +40,12 @@ properties:
> >        - const: s_axi_aclk
> >  
> >  
> > +  xlnx,dynamic-reconfig:
> > +    type: boolean
>
> The type here should be "flag" not boolean, boolean can be set to
> "false" and what you're likely doing is just checking for the property
> being present. "flag" doesn't allow false.

That sounds like exactly what I want. Will update to "flag" in v2.

Thanks!

>
> > +    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]
> > @@ -88,6 +94,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.45.2
> > 
> > 

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

[Harry Austen]

On Wed Jul 24, 2024 at 12:14 AM BST, Stephen Boyd wrote:
> General comment: do one thing in one patch, i.e. use clk_hw API and
> don't also migrate to devm in one patch.

Fair point. Will split into two patches in v2.

>
> Quoting Harry Austen (2024-07-20 05:01:36)
> > diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> > index 0ca045849ea3e..30c5cc9dcd7e9 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>
>
> What is this include for? __counted_by()?

For struct_size()

>
> >  #include <linux/err.h>
> >  #include <linux/iopoll.h>
> >
> > @@ -121,24 +122,22 @@ 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 clk_hw_onecell_data *clk_data;
>
> It could be placed at the end and then one allocation could be used
> instead of two.

Ah good point. Will move in v2.

>
> >         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;
> >  };
> > @@ -1108,35 +1110,32 @@ 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
>
> Are these going to be using devm so that on failure they get
> unregistered?

I appear to have missed that entirely. Yes, that was the intention. Will fix in v2.

>
> >                         (&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);
> >         }
> >         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]);
> >         }

Re: [PATCH 5/7] clk: clocking-wizard: add user clock monitor support

[Harry Austen]

On Wed Jul 24, 2024 at 12:29 AM BST, Stephen Boyd wrote:
> Quoting Harry Austen (2024-07-20 05:01:53)
> > 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.
> >
> > Use presence of the user monitor interrupt description in devicetree to
> > indicate whether or not the UIO 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>
> > ---
> > diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig
> > index 051756953558b..907a435694687 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
> > +       depends on UIO
>
> If I have a pre-v6.0 device I probably don't want UIO though. Perhaps
> you should use the auxiliary bus framework to register a device that is
> otherwise unused and then have the uio driver live in drivers/uio and
> match that device made here. I think you can have 'imply UIO' if you
> like to put a weak Kconfig dependency.

Yeah I wasn't particularly happy about the UIO usage here. It seems like a
nice way to leave control up to the user in userspace though, rather than
adding a bunch of device attributes to read status registers, configure
interrupts etc. but I agree the dependency isn't good here. Will look into
your suggestion for v2.

>
> >         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 7b262d73310fe..2d419e8ad4419 100644
> > --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> > +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> > @@ -1165,6 +1209,17 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> >                 return -EINVAL;
> >         }
> >
> > +       data = device_get_match_data(&pdev->dev);
> > +       if (data && data->supports_monitor) {
> > +               irq = platform_get_irq(pdev, 0);
> > +               if (irq > 0) {
> > +                       ret = clk_wzrd_setup_monitor(&pdev->dev, irq,
> > +                                                    platform_get_resource(pdev, IORESOURCE_IO, 0));
>
> Any reason this can't be
>
> 		ret = clk_wzrd_setup_monitor(pdev);
> 		if (ret)
> 			return ret;
>
> and then all the surrounding code be moved into the function, including
> the dev_err_probe()?

That makes a lot more sense. Don't know what I was doing. Thanks!

>
> > +                       if (ret)
> > +                               return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n");
> > +               }
> > +       }
> > +
> >         ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);