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

[PATCH v3 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 functionality 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!

v2 -> v3:
- Move kernel doc variable comment to bottom in clk_hw transition patch (2)
- Remove extra line (and add Krzysztof's R-b tag) in binding patch (5)
- Add Krzysztof's A-b tag to dynamic reconfig binding patch (8)
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  |  31 +-
 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, 284 insertions(+), 163 deletions(-)
 create mode 100644 drivers/uio/uio_xlnx_clk_mon.c

-- 
2.46.0

[PATCH v3 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>
---
 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 v3 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>
---
 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..cd795a4952099 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
  * @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
+ * @clk_data:		Output clock data
  */
 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

[PATCH v3 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>
---
 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 cd795a4952099..f332e0eee56c8 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 v3 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>
---
 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 f332e0eee56c8..1a65a7d153c35 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 v3 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>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/clock/xlnx,clocking-wizard.yaml  | 24 ++++++++++++++++++-
 1 file changed, 23 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..2b9903f05ef34 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -39,6 +39,13 @@ 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 +69,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

[PATCH v3 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>
---
 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 1a65a7d153c35..35dad2fda254b 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 {
  * @clks_internal:	Internal clocks
  * @speed_grade:	Speed grade of the device
  * @suspended:		Flag indicating power state of the device
+ * @adev:		User clock monitor auxiliary device
  * @clk_data:		Output clock data
  */
 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

[PATCH v3 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>
---
 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 v3 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>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../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 2b9903f05ef34..72dc6c586f7f5 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -47,6 +47,12 @@ properties:
     items:
       - 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]
@@ -90,6 +96,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

[PATCH v3 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>
---
 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 35dad2fda254b..b30d8106ac279 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 v3 7/9] uio: add Xilinx user clock monitor support

[Greg Kroah-Hartman]

On Mon, Aug 26, 2024 at 12:38:36PM +0000, Harry Austen wrote:
> 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>
> ---
>  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.

Why do you want a UIO api for a clock device?  What userspace code is
going to access the hardware this way?  Why not use the normal
kernel/user apis instead?

thanks,

greg k-h

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

[Harry Austen]

On Mon Aug 26, 2024 at 2:11 PM BST, Greg Kroah-Hartman wrote:
> On Mon, Aug 26, 2024 at 12:38:36PM +0000, Harry Austen wrote:
> > 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>
> > ---
> >  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.
>
> Why do you want a UIO api for a clock device?  What userspace code is
> going to access the hardware this way?  Why not use the normal
> kernel/user apis instead?

I was just trying to provide userspace access to these _unexpected_ clock
status event indications (clock stopped, underrun, overrun or glitched) and UIO
seemed like an easy way to do it and leave interrupt enablement and monitoring
up to userspace. I'm not aware of any existing clock event notification
framework. Are you suggesting that such a generic event notification mechanism
should be added to the clk subsystem? e.g. additional clk_ops callbacks etc.?

>
> thanks,
>
> greg k-h

Thanks for the review,
Harry

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

[Stephen Boyd]

Quoting Harry Austen (2024-08-27 12:08:52)
> On Mon Aug 26, 2024 at 2:11 PM BST, Greg Kroah-Hartman wrote:
> > On Mon, Aug 26, 2024 at 12:38:36PM +0000, Harry Austen wrote:
> > > 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>
> > > ---
> > >  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.
> >
> > Why do you want a UIO api for a clock device?  What userspace code is
> > going to access the hardware this way?  Why not use the normal
> > kernel/user apis instead?
> 
> I was just trying to provide userspace access to these _unexpected_ clock
> status event indications (clock stopped, underrun, overrun or glitched) and UIO

Maybe unexpected events can be indicated through the EDAC subsystem,
except that is usually about memory or cache errors, not device driver
issues.

> seemed like an easy way to do it and leave interrupt enablement and monitoring
> up to userspace. I'm not aware of any existing clock event notification
> framework. Are you suggesting that such a generic event notification mechanism
> should be added to the clk subsystem? e.g. additional clk_ops callbacks etc.?
> 

I've been thinking of adding devcoredump support to clk drivers when
they hit an error condition. The idea is it would be a coredump for the
device register state when the clk driver detects an error. Maybe you
can use devcoredump for this?

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

[Greg Kroah-Hartman]

On Tue, Aug 27, 2024 at 04:40:52PM -0700, Stephen Boyd wrote:
> Quoting Harry Austen (2024-08-27 12:08:52)
> > On Mon Aug 26, 2024 at 2:11 PM BST, Greg Kroah-Hartman wrote:
> > > On Mon, Aug 26, 2024 at 12:38:36PM +0000, Harry Austen wrote:
> > > > 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>
> > > > ---
> > > >  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.
> > >
> > > Why do you want a UIO api for a clock device?  What userspace code is
> > > going to access the hardware this way?  Why not use the normal
> > > kernel/user apis instead?
> > 
> > I was just trying to provide userspace access to these _unexpected_ clock
> > status event indications (clock stopped, underrun, overrun or glitched) and UIO

That is going to be a brand-new user/kernel api that isn't documented
anywhere and will be unique to this one device.  Please don't do that.

> Maybe unexpected events can be indicated through the EDAC subsystem,
> except that is usually about memory or cache errors, not device driver
> issues.

If you all need a new way to report issues like this to userspace, then
let's create the correct api for it, don't require userspace to mmap a
random device and expect to poke around in it safely to get the
information.

Odds are that mmap will change with the next version of this device,
right?

thanks,

greg k-h

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

[Stephen Boyd]

Quoting Greg Kroah-Hartman (2024-08-28 00:10:46)
> On Tue, Aug 27, 2024 at 04:40:52PM -0700, Stephen Boyd wrote:
> > Quoting Harry Austen (2024-08-27 12:08:52)
> > > On Mon Aug 26, 2024 at 2:11 PM BST, Greg Kroah-Hartman wrote:
> > > > Why do you want a UIO api for a clock device?  What userspace code is
> > > > going to access the hardware this way?  Why not use the normal
> > > > kernel/user apis instead?
> > > 
> > > I was just trying to provide userspace access to these _unexpected_ clock
> > > status event indications (clock stopped, underrun, overrun or glitched) and UIO
> 
> That is going to be a brand-new user/kernel api that isn't documented
> anywhere and will be unique to this one device.  Please don't do that.
> 
> > Maybe unexpected events can be indicated through the EDAC subsystem,
> > except that is usually about memory or cache errors, not device driver
> > issues.
> 
> If you all need a new way to report issues like this to userspace, then
> let's create the correct api for it, don't require userspace to mmap a
> random device and expect to poke around in it safely to get the
> information.
> 
> Odds are that mmap will change with the next version of this device,
> right?

Agreed. I'm wondering if we don't even need to invent anything new
though and can simply use devcoredump. Harry?

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

[Harry Austen]

On Thu Aug 29, 2024 at 7:17 PM BST, Stephen Boyd wrote:
> Quoting Greg Kroah-Hartman (2024-08-28 00:10:46)
> > On Tue, Aug 27, 2024 at 04:40:52PM -0700, Stephen Boyd wrote:
> > > Quoting Harry Austen (2024-08-27 12:08:52)
> > > > On Mon Aug 26, 2024 at 2:11 PM BST, Greg Kroah-Hartman wrote:
> > > > > Why do you want a UIO api for a clock device?  What userspace code is
> > > > > going to access the hardware this way?  Why not use the normal
> > > > > kernel/user apis instead?
> > > >
> > > > I was just trying to provide userspace access to these _unexpected_ clock
> > > > status event indications (clock stopped, underrun, overrun or glitched) and UIO
> >
> > That is going to be a brand-new user/kernel api that isn't documented
> > anywhere and will be unique to this one device.  Please don't do that.
> >
> > > Maybe unexpected events can be indicated through the EDAC subsystem,
> > > except that is usually about memory or cache errors, not device driver
> > > issues.
> >
> > If you all need a new way to report issues like this to userspace, then
> > let's create the correct api for it, don't require userspace to mmap a
> > random device and expect to poke around in it safely to get the
> > information.
> >
> > Odds are that mmap will change with the next version of this device,
> > right?
>
> Agreed. I'm wondering if we don't even need to invent anything new
> though and can simply use devcoredump. Harry?

I am not familiar with devcoredump, so will experiment with it for the user
clock monitor support. Thanks for the suggestion! In the mean time, I think it
might make more sense to split out the other _tidyup_ patches into a separate
patchset, so that can be merged separately first. Will do that next.

Thanks for the review!
Harry