Devicetree
 help / color / mirror / Atom feed
* [PATCH v3 0/8]  clk: clocking-wizard: Add static-config clock provider support
@ 2026-07-31  7:20 Shubhrajyoti Datta
  2026-07-31  7:20 ` [PATCH v3 1/8] dt-bindings: clock: clocking-wizard: Add static-config mode support Shubhrajyoti Datta
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Shubhrajyoti Datta @ 2026-07-31  7:20 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: git, shubhrajyoti.datta, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	Shubhrajyoti Datta, devicetree, linux-arm-kernel


 The Xilinx clocking-wizard IP can be used in a static-config mode
 where the hardware is pre-programmed at boot and no dynamic register
 access is required. In that case the driver should skip ioremap,
 read the fixed multiplier/divisor pairs from the device tree, and
 register fixed-factor clocks derived from clk_in1.

 This series:
   - adds the xlnx,clk-mul-div binding to carry mul/div pairs
   - makes the reg property optional for static-config nodes
   - skips ioremap when xlnx,static-config is present
   - moves clk_in1 acquisition before the static-config check so it
     is available in both code paths
   - registers fixed-factor output clocks in static-config mode


Changes in v3:
Added reviewed tag
the number of outputs duplication removed
Split notifier into per-clock callbacks

Changes in v2:
Merged the similar patches
Allow check for current dt even if it is unused.
Patch added

Shubhrajyoti Datta (8):
  dt-bindings: clock: clocking-wizard: Add static-config mode support
  clk: clocking-wizard: Do not map the memory for static-config
  clk: clocking-wizard: Move clk_in1 acquisition before static-config
    check
  clk: clocking-wizard: Add static-config clock provider support
  clk: clocking-wizard: Skip s_axi_aclk for static-config
  clk: clocking-wizard: Use dev_err_probe() when mapping registers
  clk: clocking-wizard: Fix division by zero and unbounded register
    write
  clk: clocking-wizard: Use separate notifier_block for each clock

 .../bindings/clock/xlnx,clocking-wizard.yaml  |  53 +++-
 drivers/clk/xilinx/clk-xlnx-clock-wizard.c    | 229 +++++++++++++++---
 2 files changed, 246 insertions(+), 36 deletions(-)

-- 
2.34.1


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

* [PATCH v3 1/8] dt-bindings: clock: clocking-wizard: Add static-config mode support
  2026-07-31  7:20 [PATCH v3 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
@ 2026-07-31  7:20 ` Shubhrajyoti Datta
  2026-07-31  7:20 ` [PATCH v3 2/8] clk: clocking-wizard: Do not map the memory for static-config Shubhrajyoti Datta
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Shubhrajyoti Datta @ 2026-07-31  7:20 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: git, shubhrajyoti.datta, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	Shubhrajyoti Datta, devicetree, linux-arm-kernel

Static-config MMCM/PLL ratios are fixed at IP build time, so the kernel
cannot read per-output multiply/divide from registers and needs DT pairs
to register fixed-factor clocks. Also add the tuples by pair in example.

In static-config mode the IP exposes only fixed-factor clock outputs and
has no runtime-programmable registers, so reg is not required.
Make reg optional for static-config

In static-config mode the AXI bus interface is unused, so s_axi_aclk
is not required. Allow clocks/clock-names to have only one entry
(clk_in1) when xlnx,static-config is present and enforce two entries
otherwise. Update the static-config example accordingly.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
---

Changes in v3:
Added reviewed tag

Changes in v2:
Merged the similar patches
Allow check for current dt even if it is unused.

 .../bindings/clock/xlnx,clocking-wizard.yaml  | 53 +++++++++++++++++--
 1 file changed, 48 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
index b497c28e8094..865927b9f1be 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -29,11 +29,13 @@ properties:
     const: 1
 
   clocks:
+    minItems: 1
     items:
       - description: clock input
       - description: axi clock
 
   clock-names:
+    minItems: 1
     items:
       - const: clk_in1
       - const: s_axi_aclk
@@ -57,27 +59,68 @@ properties:
     description:
       Number of outputs.
 
+  xlnx,clk-mul-div:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description:
+      Fixed MMCM/PLL multiply/divide ratios, one (multiplier, divisor)
+      pair per clock output relative to clk_in1. The number of entries
+      must equal xlnx,nr-outputs.
+    minItems: 1
+    maxItems: 8
+    items:
+      items:
+        - description: multiplier
+          minimum: 1
+        - description: divisor
+          minimum: 1
+
 required:
   - compatible
-  - reg
   - "#clock-cells"
   - clocks
   - clock-names
   - xlnx,speed-grade
   - xlnx,nr-outputs
 
+allOf:
+  - if:
+      required:
+        - xlnx,static-config
+    then:
+      required:
+        - xlnx,clk-mul-div
+    else:
+      required:
+        - reg
+      properties:
+        clocks:
+          minItems: 2
+        clock-names:
+          minItems: 2
+
 additionalProperties: false
 
 examples:
   - |
-    clock-controller@b0000000  {
+    clock-controller@b0000000 {
         compatible = "xlnx,clocking-wizard";
         reg = <0xb0000000 0x10000>;
         #clock-cells = <1>;
-        xlnx,static-config;
+        clocks = <&clkc 15>, <&clkc 18>;
+        clock-names = "clk_in1", "s_axi_aclk";
+        xlnx,nr-outputs = <6>;
         xlnx,speed-grade = <1>;
+    };
+
+  - |
+    clock-controller {
+        compatible = "xlnx,clocking-wizard";
+        #clock-cells = <1>;
+        clocks = <&clkc 15>;
+        clock-names = "clk_in1";
         xlnx,nr-outputs = <6>;
-        clock-names = "clk_in1", "s_axi_aclk";
-        clocks = <&clkc 15>, <&clkc 15>;
+        xlnx,speed-grade = <1>;
+        xlnx,static-config;
+        xlnx,clk-mul-div = <12 1>, <10 2>, <8 1>, <6 1>, <4 2>, <2 1>;
     };
 ...
-- 
2.34.1


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

* [PATCH v3 2/8] clk: clocking-wizard: Do not map the memory for static-config
  2026-07-31  7:20 [PATCH v3 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
  2026-07-31  7:20 ` [PATCH v3 1/8] dt-bindings: clock: clocking-wizard: Add static-config mode support Shubhrajyoti Datta
@ 2026-07-31  7:20 ` Shubhrajyoti Datta
  2026-07-31  7:33   ` sashiko-bot
  2026-07-31  7:20 ` [PATCH v3 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check Shubhrajyoti Datta
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Shubhrajyoti Datta @ 2026-07-31  7:20 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: git, shubhrajyoti.datta, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	Shubhrajyoti Datta, devicetree, linux-arm-kernel

With xlnx,static-config the MMCM/PLL topology is fixed at synthesis time
and no register programming is performed; only the dynamic path needs
the AXI register block. Move devm_platform_ioremap_resource() under the
non-static-config branch.

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

(no changes since v1)

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

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 4a0136349f71..e082051221be 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -1168,10 +1168,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		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);
-
 	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),
@@ -1183,6 +1179,10 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	}
 
 	if (!of_property_present(np, "xlnx,static-config")) {
+		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) {
-- 
2.34.1


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

* [PATCH v3 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check
  2026-07-31  7:20 [PATCH v3 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
  2026-07-31  7:20 ` [PATCH v3 1/8] dt-bindings: clock: clocking-wizard: Add static-config mode support Shubhrajyoti Datta
  2026-07-31  7:20 ` [PATCH v3 2/8] clk: clocking-wizard: Do not map the memory for static-config Shubhrajyoti Datta
@ 2026-07-31  7:20 ` Shubhrajyoti Datta
  2026-07-31  7:33   ` sashiko-bot
  2026-07-31  7:20 ` [PATCH v3 4/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Shubhrajyoti Datta @ 2026-07-31  7:20 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: git, shubhrajyoti.datta, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	Shubhrajyoti Datta, devicetree, linux-arm-kernel

The clk_in1 is the input clock for both the dynamic reconfig and the
static-config paths. Acquire clk_in1 for static-config as well. Output
clocks are registered as fixed-factor children of clk_in1.

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

(no changes since v1)

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

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index e082051221be..ffc78c90bee6 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -1178,6 +1178,11 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	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),
+				     "failed to get clk_in1\n");
+
 	if (!of_property_present(np, "xlnx,static-config")) {
 		clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
 		if (IS_ERR(clk_wzrd->base))
@@ -1192,11 +1197,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 			}
 		}
 
-		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 = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
 		if (ret)
 			return ret;
-- 
2.34.1


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

* [PATCH v3 4/8] clk: clocking-wizard: Add static-config clock provider support
  2026-07-31  7:20 [PATCH v3 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
                   ` (2 preceding siblings ...)
  2026-07-31  7:20 ` [PATCH v3 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check Shubhrajyoti Datta
@ 2026-07-31  7:20 ` Shubhrajyoti Datta
  2026-07-31  7:20 ` [PATCH v3 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config Shubhrajyoti Datta
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Shubhrajyoti Datta @ 2026-07-31  7:20 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: git, shubhrajyoti.datta, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	Shubhrajyoti Datta, devicetree, linux-arm-kernel

When xlnx,static-config is present the divider/multiplier path is
synthesized inside the Wizard without exposing runtime MMIO
reconfiguration, so omit the AXI register mapping and advertise each
routed output clock as a clk_fixed_factor child of clk_in1 using the
synthesized ratios exported through xlnx,clk-mul-div.

However the parent clock of clk_in1 can still be gated, disabled, or
(re-)enabled after FPGA programming (typical FPGA Manager flows,
CCF-managed parents, PLL fabric power sequencing).

Add a 10 us delay in each output clock's .enable() hook so consumers
wait for reference settling before first access.

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

---

Changes in v3:
the number of outputs duplication removed

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

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index ffc78c90bee6..1332abc8a950 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -12,6 +12,7 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/of.h>
@@ -107,6 +108,7 @@
 #define VER_WZRD_O_MAX			511
 #define WZRD_MIN_ERR			20000
 #define WZRD_FRAC_POINTS		1000
+#define WZRD_ENABLE_DELAY_US		10
 
 /* Get the mask from width */
 #define div_mask(width)			((1 << (width)) - 1)
@@ -697,6 +699,13 @@ static int clk_wzrd_ver_determine_rate_all(struct clk_hw *hw,
 	return 0;
 }
 
+static int clk_wzrd_enable(struct clk_hw *hw)
+{
+	/* Allow the output clock to settle after enable */
+	udelay(WZRD_ENABLE_DELAY_US);
+	return 0;
+}
+
 static const struct clk_ops clk_wzrd_ver_divider_ops = {
 	.determine_rate = clk_wzrd_determine_rate,
 	.set_rate = clk_wzrd_ver_dynamic_reconfig,
@@ -790,6 +799,85 @@ static const struct clk_ops clk_wzrd_clk_divider_ops_f = {
 	.recalc_rate = clk_wzrd_recalc_ratef,
 };
 
+static unsigned long
+clk_wzrd_static_factor_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+	unsigned long long rate;
+
+	rate = (unsigned long long)parent_rate * fix->mult;
+	do_div(rate, fix->div);
+	return (unsigned long)rate;
+}
+
+static int clk_wzrd_static_factor_determine_rate(struct clk_hw *hw,
+						 struct clk_rate_request *req)
+{
+	struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+
+	if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) {
+		unsigned long best_parent;
+
+		best_parent = (req->rate / fix->mult) * fix->div;
+		req->best_parent_rate =
+			clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
+	}
+
+	req->rate = (req->best_parent_rate / fix->div) * fix->mult;
+
+	return 0;
+}
+
+static int clk_wzrd_static_factor_set_rate(struct clk_hw *hw,
+					   unsigned long rate,
+					   unsigned long parent_rate)
+{
+	return 0;
+}
+
+static const struct clk_ops clk_wzrd_static_fixed_factor_ops = {
+	.enable = clk_wzrd_enable,
+	.determine_rate = clk_wzrd_static_factor_determine_rate,
+	.set_rate = clk_wzrd_static_factor_set_rate,
+	.recalc_rate = clk_wzrd_static_factor_recalc_rate,
+};
+
+static struct clk_hw *
+clk_wzrd_devm_register_static_fixed_factor(struct device *dev,
+					   const char *name,
+					   const struct clk_parent_data *parent_data,
+					   unsigned long flags,
+					   unsigned int mult,
+					   unsigned int div)
+{
+	struct clk_init_data init = {};
+	struct clk_fixed_factor *fix;
+	struct clk_hw *hw;
+	int ret;
+
+	fix = devm_kzalloc(dev, sizeof(*fix), GFP_KERNEL);
+	if (!fix)
+		return ERR_PTR(-ENOMEM);
+
+	fix->mult = mult;
+	fix->div = div;
+
+	init.name = name;
+	init.ops = &clk_wzrd_static_fixed_factor_ops;
+	init.flags = flags;
+	init.parent_data = parent_data;
+	init.num_parents = 1;
+
+	fix->hw.init = &init;
+
+	hw = &fix->hw;
+	ret = devm_clk_hw_register(dev, hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return hw;
+}
+
 static struct clk_hw *clk_wzrd_register_divf(struct device *dev,
 					  const char *name,
 					  const char *parent_name,
@@ -1154,9 +1242,11 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct clk_wzrd *clk_wzrd;
+	const char *clk_name;
 	unsigned long rate;
+	struct clk_hw *hw;
 	int nr_outputs;
-	int ret;
+	int ret, i;
 
 	ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);
 	if (ret || nr_outputs > WZRD_NUM_OUTPUTS)
@@ -1166,6 +1256,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 				GFP_KERNEL);
 	if (!clk_wzrd)
 		return -ENOMEM;
+	clk_wzrd->clk_data.num = nr_outputs;
 	platform_set_drvdata(pdev, clk_wzrd);
 
 	clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
@@ -1201,7 +1292,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		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) {
@@ -1224,6 +1314,49 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 				dev_warn(&pdev->dev,
 					 "unable to register clock notifier\n");
 		}
+	} else {
+		u32 mul_div[WZRD_NUM_OUTPUTS * 2];
+		const struct clk_parent_data parent_data = { .fw_name = "clk_in1" };
+		int num_elems = nr_outputs * 2;
+
+		/*
+		 * xlnx,clk-mul-div is a uint32-matrix of <mul div> pairs;
+		 * FDT encodes it as a flat u32 array so we can read it directly.
+		 */
+		ret = of_property_read_u32_array(np, "xlnx,clk-mul-div",
+						 mul_div, num_elems);
+		if (ret) {
+			dev_err(&pdev->dev, "xlnx,clk-mul-div missing or invalid\n");
+			return ret;
+		}
+
+		for (i = 0; i < nr_outputs; i++) {
+			u32 mul = mul_div[2 * i];
+			u32 div = mul_div[2 * i + 1];
+
+			if (!mul || !div)
+				return dev_err_probe(&pdev->dev, -EINVAL,
+						     "invalid mul/div for clkout%d\n", i);
+
+			clk_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+						  "%s_out%d", dev_name(&pdev->dev), i);
+			if (!clk_name)
+				return -ENOMEM;
+
+			hw = clk_wzrd_devm_register_static_fixed_factor(&pdev->dev, clk_name,
+									&parent_data,
+									CLK_SET_RATE_PARENT,
+									mul, div);
+			if (IS_ERR(hw))
+				return PTR_ERR(hw);
+			clk_wzrd->clk_data.hws[i] = hw;
+		}
+
+		ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
+						  &clk_wzrd->clk_data);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "unable to register clock provider\n");
 	}
 
 	return 0;
-- 
2.34.1


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

* [PATCH v3 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config
  2026-07-31  7:20 [PATCH v3 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
                   ` (3 preceding siblings ...)
  2026-07-31  7:20 ` [PATCH v3 4/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
@ 2026-07-31  7:20 ` Shubhrajyoti Datta
  2026-07-31  7:37   ` sashiko-bot
  2026-07-31  7:20 ` [PATCH v3 6/8] clk: clocking-wizard: Use dev_err_probe() when mapping registers Shubhrajyoti Datta
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Shubhrajyoti Datta @ 2026-07-31  7:20 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: git, shubhrajyoti.datta, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	Shubhrajyoti Datta, devicetree, linux-arm-kernel

For static-config mode the AXI bus interface is not used, so there is
no need to get and enable s_axi_aclk. Move the axi_clk setup inside
the non-static-config branch.

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

(no changes since v1)

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

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 1332abc8a950..c79733f58082 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -1243,7 +1243,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct clk_wzrd *clk_wzrd;
 	const char *clk_name;
-	unsigned long rate;
 	struct clk_hw *hw;
 	int nr_outputs;
 	int ret, i;
@@ -1259,22 +1258,24 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 	clk_wzrd->clk_data.num = nr_outputs;
 	platform_set_drvdata(pdev, clk_wzrd);
 
-	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;
-	}
-
 	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),
 				     "failed to get clk_in1\n");
 
 	if (!of_property_present(np, "xlnx,static-config")) {
+		unsigned long rate;
+
+		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;
+		}
+
 		clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
 		if (IS_ERR(clk_wzrd->base))
 			return PTR_ERR(clk_wzrd->base);
-- 
2.34.1


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

* [PATCH v3 6/8] clk: clocking-wizard: Use dev_err_probe() when mapping registers
  2026-07-31  7:20 [PATCH v3 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
                   ` (4 preceding siblings ...)
  2026-07-31  7:20 ` [PATCH v3 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config Shubhrajyoti Datta
@ 2026-07-31  7:20 ` Shubhrajyoti Datta
  2026-07-31  7:20 ` [PATCH v3 7/8] clk: clocking-wizard: Fix division by zero and unbounded register write Shubhrajyoti Datta
  2026-07-31  7:20 ` [PATCH v3 8/8] clk: clocking-wizard: Use separate notifier_block for each clock Shubhrajyoti Datta
  7 siblings, 0 replies; 13+ messages in thread
From: Shubhrajyoti Datta @ 2026-07-31  7:20 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: git, shubhrajyoti.datta, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	Shubhrajyoti Datta, devicetree, linux-arm-kernel

Align the devm_platform_ioremap_resource() error path with clk_in1 and
s_axi_aclk handling for consistent logging and deferred-probe behavior.

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

(no changes since v1)

 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index c79733f58082..4ddcb4135306 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -1278,7 +1278,8 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 
 		clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
 		if (IS_ERR(clk_wzrd->base))
-			return PTR_ERR(clk_wzrd->base);
+			return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->base),
+					     "failed to map registers\n");
 
 		ret = of_property_read_u32(np, "xlnx,speed-grade", &clk_wzrd->speed_grade);
 		if (!ret) {
-- 
2.34.1


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

* [PATCH v3 7/8] clk: clocking-wizard: Fix division by zero and unbounded register write
  2026-07-31  7:20 [PATCH v3 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
                   ` (5 preceding siblings ...)
  2026-07-31  7:20 ` [PATCH v3 6/8] clk: clocking-wizard: Use dev_err_probe() when mapping registers Shubhrajyoti Datta
@ 2026-07-31  7:20 ` Shubhrajyoti Datta
  2026-07-31  7:33   ` sashiko-bot
  2026-07-31  7:20 ` [PATCH v3 8/8] clk: clocking-wizard: Use separate notifier_block for each clock Shubhrajyoti Datta
  7 siblings, 1 reply; 13+ messages in thread
From: Shubhrajyoti Datta @ 2026-07-31  7:20 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: git, shubhrajyoti.datta, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	Shubhrajyoti Datta, devicetree, linux-arm-kernel

1. clk_wzrd_determine_rate(): DIV_ROUND_CLOSEST() can return 0 when
   the requested rate greatly exceeds the parent rate, causing a
   division by zero on the subsequent parent_rate / div. Clamp div
   to a minimum of 1.

2. clk_wzrd_dynamic_reconfig(): min_t() result was not assigned back
   to value, so the cap to WZRD_DR_MAX_INT_DIV_VALUE was never applied
   before the register write.

3. clk_wzrd_get_divisors() and clk_wzrd_get_divisors_ver(): If rate is
   0, DIV_ROUND_CLOSEST_ULL divides by zero. Return -EINVAL early.

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

(no changes since v2)

Changes in v2:
Patch added

 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index 4ddcb4135306..b16656a11f4b 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -296,7 +296,7 @@ static int clk_wzrd_dynamic_reconfig(struct clk_hw *hw, unsigned long rate,
 	value = DIV_ROUND_CLOSEST(parent_rate, rate);
 
 	/* Cap the value to max */
-	min_t(u32, value, WZRD_DR_MAX_INT_DIV_VALUE);
+	value = min_t(u32, value, WZRD_DR_MAX_INT_DIV_VALUE);
 
 	/* Set divisor and clear phase offset */
 	writel(value, div_addr);
@@ -334,6 +334,8 @@ static int clk_wzrd_determine_rate(struct clk_hw *hw,
 	 * achievable
 	 */
 	div = DIV_ROUND_CLOSEST(req->best_parent_rate, req->rate);
+	if (!div)
+		div = 1;
 
 	req->rate = req->best_parent_rate / div;
 
@@ -348,6 +350,9 @@ static int clk_wzrd_get_divisors_ver(struct clk_hw *hw, unsigned long rate,
 	u32 m, d, o;
 	u32 mmin, mmax, dmin, dmax, omin, omax;
 
+	if (!rate)
+		return -EINVAL;
+
 	mmin = VER_WZRD_M_MIN;
 	mmax = VER_WZRD_M_MAX;
 	dmin = VER_WZRD_D_MIN;
@@ -390,6 +395,9 @@ static int clk_wzrd_get_divisors(struct clk_hw *hw, unsigned long rate,
 	u64 m, d, o;
 	u64 mmin, mmax, dmin, dmax, omin, omax, mdmin, mdmax;
 
+	if (!rate)
+		return -EINVAL;
+
 	mmin = WZRD_M_MIN << 3;
 	mmax = WZRD_M_MAX << 3;
 	dmin = WZRD_D_MIN;
-- 
2.34.1


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

* [PATCH v3 8/8] clk: clocking-wizard: Use separate notifier_block for each clock
  2026-07-31  7:20 [PATCH v3 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
                   ` (6 preceding siblings ...)
  2026-07-31  7:20 ` [PATCH v3 7/8] clk: clocking-wizard: Fix division by zero and unbounded register write Shubhrajyoti Datta
@ 2026-07-31  7:20 ` Shubhrajyoti Datta
  7 siblings, 0 replies; 13+ messages in thread
From: Shubhrajyoti Datta @ 2026-07-31  7:20 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: git, shubhrajyoti.datta, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	Shubhrajyoti Datta, devicetree, linux-arm-kernel

A struct notifier_block has a single 'next' pointer, so registering the
same instance to two different notification chains corrupts the linked
lists. Use a dedicated nb_axi for the s_axi_aclk notifier chain.

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

Changes in v3:
Split notifier into per-clock callbacks

 drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 42 +++++++++++++++++-----
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index b16656a11f4b..cfad11593b2b 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -126,7 +126,8 @@ enum clk_wzrd_int_clks {
 /**
  * struct clk_wzrd - Clock wizard private data structure
  *
- * @nb:			Notifier block
+ * @nb:			Notifier block for clk_in1
+ * @nb_axi:		Notifier block for s_axi_aclk
  * @base:		Memory base
  * @clk_in1:		Handle to input clock 'clk_in1'
  * @axi_clk:		Handle to input clock 's_axi_aclk'
@@ -137,6 +138,7 @@ enum clk_wzrd_int_clks {
  */
 struct clk_wzrd {
 	struct notifier_block nb;
+	struct notifier_block nb_axi;
 	void __iomem *base;
 	struct clk *clk_in1;
 	struct clk *axi_clk;
@@ -184,6 +186,7 @@ struct versal_clk_data {
 };
 
 #define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb)
+#define to_clk_wzrd_axi(_nb) container_of(_nb, struct clk_wzrd, nb_axi)
 
 /* maximum frequencies for input/output clocks per speed grade */
 static const unsigned long clk_wzrd_max_freq[] = {
@@ -1024,6 +1027,30 @@ static struct clk_hw *clk_wzrd_register_divider(struct device *dev,
 
 static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
 				 void *data)
+{
+	unsigned long max;
+	struct clk_notifier_data *ndata = data;
+	struct clk_wzrd *clk_wzrd = to_clk_wzrd_axi(nb);
+
+	if (clk_wzrd->suspended)
+		return NOTIFY_OK;
+
+	max = WZRD_ACLK_MAX_FREQ;
+
+	switch (event) {
+	case PRE_RATE_CHANGE:
+		if (ndata->new_rate > max)
+			return NOTIFY_BAD;
+		return NOTIFY_OK;
+	case POST_RATE_CHANGE:
+	case ABORT_RATE_CHANGE:
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
+static int clk_wzrd_clk_notifier_in1(struct notifier_block *nb, unsigned long event,
+				     void *data)
 {
 	unsigned long max;
 	struct clk_notifier_data *ndata = data;
@@ -1032,12 +1059,7 @@ static int clk_wzrd_clk_notifier(struct notifier_block *nb, unsigned long event,
 	if (clk_wzrd->suspended)
 		return NOTIFY_OK;
 
-	if (ndata->clk == clk_wzrd->clk_in1)
-		max = clk_wzrd_max_freq[clk_wzrd->speed_grade - 1];
-	else if (ndata->clk == clk_wzrd->axi_clk)
-		max = WZRD_ACLK_MAX_FREQ;
-	else
-		return NOTIFY_DONE;	/* should never happen */
+	max = clk_wzrd_max_freq[clk_wzrd->speed_grade - 1];
 
 	switch (event) {
 	case PRE_RATE_CHANGE:
@@ -1310,7 +1332,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		}
 
 		if (clk_wzrd->speed_grade) {
-			clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
+			clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier_in1;
 
 			ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->clk_in1,
 							 &clk_wzrd->nb);
@@ -1318,8 +1340,10 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 				dev_warn(&pdev->dev,
 					 "unable to register clock notifier\n");
 
+			clk_wzrd->nb_axi.notifier_call = clk_wzrd_clk_notifier;
+
 			ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->axi_clk,
-							 &clk_wzrd->nb);
+							 &clk_wzrd->nb_axi);
 			if (ret)
 				dev_warn(&pdev->dev,
 					 "unable to register clock notifier\n");
-- 
2.34.1


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

* Re: [PATCH v3 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check
  2026-07-31  7:20 ` [PATCH v3 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check Shubhrajyoti Datta
@ 2026-07-31  7:33   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-31  7:33 UTC (permalink / raw)
  To: Shubhrajyoti Datta; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Missing implementation for static-config mode clock registration
- [High] Unconditional s_axi_aclk acquisition breaks static-config mode probe

Pre-existing issues:
- [High] Double registration of the same notifier_block corrupts SRCU notifier chains
--

--- Patch [3]: [PATCH v3 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check ---
commit 7ab44b637bfcd95c4cb1c02dbfa9fac870a83ce3
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

clk: clocking-wizard: Move clk_in1 acquisition before static-config check
    
The clk_in1 is the input clock for both the dynamic reconfig and the
static-config paths. Acquire clk_in1 for static-config as well. Output
clocks are registered as fixed-factor children of clk_in1.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index e082051221bef..ffc78c90bee6f 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> @@ -1178,6 +1178,11 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  

[Severity: High]
Does unconditionally fetching s_axi_aclk break the probe for static-config
mode?

Earlier in clk_wzrd_probe():

	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");

If the device tree bindings explicitly omit s_axi_aclk when using
xlnx,static-config, wouldn't this unconditional fetch return -ENOENT and
abort the probe?

> +	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),
> +				     "failed to get clk_in1\n");
> +
>  	if (!of_property_present(np, "xlnx,static-config")) {

[Severity: High]
Does this code skip registering output clocks for the static-config mode?

The commit message states that "Output clocks are registered as fixed-factor
children of clk_in1".

However, since all the clock registration logic is inside the
!of_property_present(np, "xlnx,static-config") block, it seems this mode
just returns 0 without parsing xlnx,clk-mul-div or calling any clock
registration functions.

>  		clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
>  		if (IS_ERR(clk_wzrd->base))

[ ... ]

> @@ -1192,11 +1197,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  			}
>  		}
>  
> -		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 = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
>  		if (ret)
>  			return ret;

[Severity: High]
This isn't a bug introduced by this patch, but does registering the same
notifier block twice corrupt the SRCU notifier chains?

Later in clk_wzrd_probe():

	ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->clk_in1,
					 &clk_wzrd->nb);
	...
	ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->axi_clk,
					 &clk_wzrd->nb);

The struct notifier_block has a single next pointer used by the kernel's
list implementation. Registering the single clk_wzrd->nb instance to two
distinct lists overwrites its next pointer, which can physically merge the
two independent clock notifier lists and drop any notifiers that followed in
the first list.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731072049.957624-1-shubhrajyoti.datta@amd.com?part=3

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

* Re: [PATCH v3 2/8] clk: clocking-wizard: Do not map the memory for static-config
  2026-07-31  7:20 ` [PATCH v3 2/8] clk: clocking-wizard: Do not map the memory for static-config Shubhrajyoti Datta
@ 2026-07-31  7:33   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-31  7:33 UTC (permalink / raw)
  To: Shubhrajyoti Datta; +Cc: devicetree, conor+dt, robh

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Unconditional AXI clock fetch breaks probe in static-config mode.

Pre-existing issues:
- [High] Driver completely skips parsing and registering output clocks when static-config is enabled, acting as a silent stub.
- [High] The driver shares a single `struct notifier_block` instance to register rate-change notifiers for two different clocks, corrupting the kernel's SRCU notifier chains.
- [High] Ineffective value cap allows writing out-of-range values to hardware registers.
--

--- Patch [2]: [PATCH v3 2/8] clk: clocking-wizard: Do not map the memory for static-config ---
commit 5be3eccdee47c9c51ffe9fd16deab92141ef8346
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
Subject: clk: clocking-wizard: Do not map the memory for static-config

With xlnx,static-config the MMCM/PLL topology is fixed at synthesis time
and no register programming is performed; only the dynamic path needs
the AXI register block. Move devm_platform_ioremap_resource() under the
non-static-config branch.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index 4a0136349f71a..e082051221bef 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c

[ ... ]

> static int clk_wzrd_dynamic_reconfig(struct clk_hw *hw, unsigned long rate,
> 				     unsigned long parent_rate)
> {
[ ... ]
> 	value = DIV_ROUND_CLOSEST(parent_rate, rate);
> 
> 	/* Cap the value to max */
> 	min_t(u32, value, WZRD_DR_MAX_INT_DIV_VALUE);
> 
> 	/* Set divisor and clear phase offset */
> 	writel(value, div_addr);

[Severity: High]
This is a pre-existing issue, but is the value cap effective here?

The min_t() macro evaluates as a standalone statement and doesn't modify its
arguments in place. Because the returned minimum value is discarded, the
'value' variable remains entirely uncapped when passed to writel(). If the
calculated divisor exceeds the expected width, it could corrupt adjacent bit
fields within the hardware register.

[ ... ]

> @@ -1168,10 +1168,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  		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);
> -
>  	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),

[Severity: High]
Does this unconditionally fail the probe when static-config is enabled?

The commit message notes that only the dynamic path needs the AXI block, which
implies "s_axi_aclk" might be omitted from the device tree for static
configurations. If the clock is missing, devm_clk_get_enabled() will return
-ENOENT and immediately abort the probe before reaching the static-config
check.

> @@ -1183,6 +1179,10 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  	}
>  
>  	if (!of_property_present(np, "xlnx,static-config")) {
> +		clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
> +		if (IS_ERR(clk_wzrd->base))
> +			return PTR_ERR(clk_wzrd->base);
> +

[Severity: High]
This is a pre-existing issue, but does this branch entirely skip clock
registration for static configurations?

When "xlnx,static-config" is present, the driver skips this block and simply
returns 0 at the end of the probe function. No fixed-factor clocks are
registered, meaning any downstream consumers requesting a clock from this
provider will indefinitely defer their probe (-EPROBE_DEFER).

[ ... ]

> 			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);

[Severity: High]
This is a pre-existing issue, but does this corrupt the SRCU notifier chain?

The exact same notifier_block instance (&clk_wzrd->nb) is registered to two
independent clock notifier chains (clk_in1 and axi_clk). The notifier API links
the structure into a singly-linked list via the 'next' pointer embedded inside
the notifier_block.

Registering the same block twice overwrites the 'next' pointer for the first
chain, which can permanently break the lists and lead to kernel oopses or
infinite loops during traversal.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731072049.957624-1-shubhrajyoti.datta@amd.com?part=2

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

* Re: [PATCH v3 7/8] clk: clocking-wizard: Fix division by zero and unbounded register write
  2026-07-31  7:20 ` [PATCH v3 7/8] clk: clocking-wizard: Fix division by zero and unbounded register write Shubhrajyoti Datta
@ 2026-07-31  7:33   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-31  7:33 UTC (permalink / raw)
  To: Shubhrajyoti Datta; +Cc: robh, devicetree, conor+dt

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [High] Divide-by-zero vulnerabilities remain in multiple callback functions for rate == 0.
- [High] Computed divisor in clk_wzrd_determine_rate() is silently truncated, leading to incorrect clock rates.
- [High] Missing upper bounds check in clk_wzrd_ver_dynamic_reconfig() causes register corruption.
- [High] Missing lower bounds check allows 0 to be written to hardware divisor registers.
- [High] Unchecked large rates cause hardware to be programmed with 0, leading to divide-by-zero during recalc.
- [High] High atomic latency and preemption stall during dynamic reconfiguration.
--

--- Patch [7]: [PATCH v3 7/8] clk: clocking-wizard: Fix division by zero and unbounded register write ---
commit 5e3507219b53560de47af4a3c5f59ba7a3f31a01
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
Subject: clk: clocking-wizard: Fix division by zero and unbounded register write

This commit addresses a division by zero in clk_wzrd_determine_rate and
adds checks for zero rates in divisor calculations. It also fixes an
unbounded register write in clk_wzrd_dynamic_reconfig by capturing the
result of the min_t calculation.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index 4ddcb41353061..b16656a11f4b5 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> @@ -296,7 +296,7 @@ static int clk_wzrd_dynamic_reconfig(struct clk_hw *hw, unsigned long rate,
>  	value = DIV_ROUND_CLOSEST(parent_rate, rate);

[Severity: High]
This is a pre-existing issue, but if a consumer requests a clock rate of
0, won't DIV_ROUND_CLOSEST() evaluate a division by zero here?

The patch successfully identifies this issue in clk_wzrd_get_divisors(),
but it appears other callbacks like clk_wzrd_dynamic_reconfig() and
clk_wzrd_ver_dynamic_reconfig() still pass rate directly as a divisor.

[Severity: High]
This is also a pre-existing issue, but does this need a lower bounds check?

If a consumer requests a rate significantly larger than the parent rate,
value evaluates to 0. It bypasses the upper bound and writes 0 directly to
the clock wizard hardware divisor registers.

[Severity: High]
This isn't a bug introduced by this patch, but while you added a cap for
value here, clk_wzrd_ver_dynamic_reconfig() seems to still lack an upper
bounds check.

A small rate there yields a large value, causing the regh variable to
exceed its 8-bit mask and overflow when shifted by WZRD_CLKFBOUT_H_SHIFT,
corrupting adjacent hardware register values.

[Severity: High]
This is a pre-existing issue, but could a similar problem cause a panic
in the fractional clock callbacks?

clk_wzrd_determine_rate_f() is an empty stub that fails to bound req->rate.
As a result, clk_wzrd_dynamic_reconfig_f() can divide by rate, evaluate
to 0, and program a 0 divisor. Later, clk_wzrd_recalc_ratef() reads this 0
from the register and calls mult_frac() with 0 as the divisor, which would
crash the system.

>  
>  	/* Cap the value to max */
> -	min_t(u32, value, WZRD_DR_MAX_INT_DIV_VALUE);
> +	value = min_t(u32, value, WZRD_DR_MAX_INT_DIV_VALUE);

[ ... ]

> @@ -334,6 +334,8 @@ static int clk_wzrd_determine_rate(struct clk_hw *hw,
>  	 * achievable
>  	 */
>  	div = DIV_ROUND_CLOSEST(req->best_parent_rate, req->rate);

[Severity: High]
This is a pre-existing issue, but does this still evaluate a division by
zero if req->rate is 0?

[Severity: High]
This is a pre-existing issue, but won't div be silently truncated here?

Since div is declared as a u8 at the top of clk_wzrd_determine_rate(), if
the requested rate yields a divisor of 256 or larger, it truncates. For
example, 256 truncates to 0, and the new if (!div) logic below will
incorrectly force it to 1, returning a massive clock rate.

Should div be declared as a u32 and correctly capped at the hardware limit
instead?

> +	if (!div)
> +		div = 1;
>  
>  	req->rate = req->best_parent_rate / div;

[ ... ]

> @@ -348,6 +350,9 @@ static int clk_wzrd_get_divisors_ver(struct clk_hw *hw, unsigned long rate,
>  	u32 m, d, o;
>  	u32 mmin, mmax, dmin, dmax, omin, omax;
>  
> +	if (!rate)
> +		return -EINVAL;
> +

[Severity: High]
This is a pre-existing issue, but does this function cause severe
scheduling latency?

When called from clk_wzrd_dynamic_all_ver() or clk_wzrd_dynamic_all(), an
IRQ-disabled spinlock is held across the nested loops in this function
which run for over 50,000 to 100,000 iterations. Performing this massive
search space with 64-bit integer divisions inside a spin_lock_irqsave()
critical section could stall the CPU for milliseconds.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731072049.957624-1-shubhrajyoti.datta@amd.com?part=7

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

* Re: [PATCH v3 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config
  2026-07-31  7:20 ` [PATCH v3 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config Shubhrajyoti Datta
@ 2026-07-31  7:37   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-31  7:37 UTC (permalink / raw)
  To: Shubhrajyoti Datta; +Cc: robh, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The provided concern incorrectly states that the callback resolves its context using an invalid offset due to `nb_axi`. In the actual code, `nb_axi` does not exist, and `clk_wzrd->nb` is used for both registrations, so the offset calculation is correct. However, a critical bug exists at the same location: the exact same `notifier_block` instance (`clk_wzrd->nb`) is registered to two different clock notifier chains (`clk_in1` and `axi_clk`). A `notifier_block` has only one `next` pointer and cannot be added to multiple SRCU notifier chains simultaneously (or twice to the same chain). Doing so corrupts the notifier linked list.
--

--- Patch [5]: [PATCH v3 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config ---
commit 8f005ad6b615119302b96a70915b5c138c7ff09f
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

clk: clocking-wizard: Skip s_axi_aclk for static-config
    
For static-config mode the AXI bus interface is not used, so there is
no need to get and enable s_axi_aclk. Move the axi_clk setup inside
the non-static-config branch.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index 1332abc8a9501..c79733f58082d 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
[ ... ]
> @@ -1259,22 +1258,24 @@ static int clk_wzrd_probe(struct platform_device *pdev)
[ ... ]
>  	if (!of_property_present(np, "xlnx,static-config")) {
> +		unsigned long rate;
> +
> +		clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the axi_clk
setup here, I noticed a pre-existing issue further down in clk_wzrd_probe()
where the exact same notifier_block is registered for both clk_in1 and
axi_clk:

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_probe() {
	...
	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);
		...
		ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->axi_clk,
						 &clk_wzrd->nb);
	...
}

Does this corrupt the SRCU notifier linked list? A notifier_block has only
one next pointer and cannot be added to multiple SRCU notifier chains
simultaneously (or twice to the same chain).

If the clocks are different, nb->next would be overwritten, merging the
chains and corrupting traversal. If the clocks are the same, registering
the same block twice creates a circular linked list (n->next = n), which
can cause an infinite loop and hard lockup during clock rate changes.

Could we add a second notifier_block to struct clk_wzrd to handle the
axi_clk registration separately?

> +		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");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731072049.957624-1-shubhrajyoti.datta@amd.com?part=5

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

end of thread, other threads:[~2026-07-31  7:37 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-31  7:20 [PATCH v3 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
2026-07-31  7:20 ` [PATCH v3 1/8] dt-bindings: clock: clocking-wizard: Add static-config mode support Shubhrajyoti Datta
2026-07-31  7:20 ` [PATCH v3 2/8] clk: clocking-wizard: Do not map the memory for static-config Shubhrajyoti Datta
2026-07-31  7:33   ` sashiko-bot
2026-07-31  7:20 ` [PATCH v3 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check Shubhrajyoti Datta
2026-07-31  7:33   ` sashiko-bot
2026-07-31  7:20 ` [PATCH v3 4/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
2026-07-31  7:20 ` [PATCH v3 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config Shubhrajyoti Datta
2026-07-31  7:37   ` sashiko-bot
2026-07-31  7:20 ` [PATCH v3 6/8] clk: clocking-wizard: Use dev_err_probe() when mapping registers Shubhrajyoti Datta
2026-07-31  7:20 ` [PATCH v3 7/8] clk: clocking-wizard: Fix division by zero and unbounded register write Shubhrajyoti Datta
2026-07-31  7:33   ` sashiko-bot
2026-07-31  7:20 ` [PATCH v3 8/8] clk: clocking-wizard: Use separate notifier_block for each clock Shubhrajyoti Datta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox