[PATCH 0/8] clk: clocking-wizard: Add static-config clock provider support

[PATCH 0/8] clk: clocking-wizard: Add static-config clock provider support

[Shubhrajyoti Datta]

 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.

 Currently the xlnx,static-config is not functional as it doesnot
 model the output clocks. The series fixes the same.

 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


Shubhrajyoti Datta (8):
  dt-bindings: clock: clocking-wizard: Add xlnx,clk-mul-div property
  dt-bindings: clock: clocking-wizard: Make reg optional for
    static-config
  dt-bindings: clock: clocking-wizard: Make s_axi_aclk optional for
    static-config
  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

 .../bindings/clock/xlnx,clocking-wizard.yaml  |  50 ++++-
 drivers/clk/xilinx/clk-xlnx-clock-wizard.c    | 181 ++++++++++++++++--
 2 files changed, 205 insertions(+), 26 deletions(-)

-- 
2.34.1

[PATCH 1/8] dt-bindings: clock: clocking-wizard: Add xlnx,clk-mul-div property

[Shubhrajyoti Datta]

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.

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

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

diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
index b497c28e8094..8316654b0a91 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -57,6 +57,21 @@ 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
@@ -66,6 +81,14 @@ required:
   - xlnx,speed-grade
   - xlnx,nr-outputs
 
+allOf:
+  - if:
+      required:
+        - xlnx,static-config
+    then:
+      required:
+        - xlnx,clk-mul-div
+
 additionalProperties: false
 
 examples:
@@ -77,6 +100,7 @@ examples:
         xlnx,static-config;
         xlnx,speed-grade = <1>;
         xlnx,nr-outputs = <6>;
+        xlnx,clk-mul-div = <12 1>, <10 2>, <8 1>, <6 1>, <4 2>, <2 1>;
         clock-names = "clk_in1", "s_axi_aclk";
         clocks = <&clkc 15>, <&clkc 15>;
     };
-- 
2.49.1

[PATCH 2/8] dt-bindings: clock: clocking-wizard: Make reg optional for static-config

[Shubhrajyoti Datta]

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

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

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

diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
index 8316654b0a91..aa397550d107 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
@@ -74,7 +74,6 @@ properties:
 
 required:
   - compatible
-  - reg
   - "#clock-cells"
   - clocks
   - clock-names
@@ -88,20 +87,33 @@ allOf:
     then:
       required:
         - xlnx,clk-mul-div
+    else:
+      required:
+        - reg
 
 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>, <&clkc 18>;
+        clock-names = "clk_in1", "s_axi_aclk";
         xlnx,nr-outputs = <6>;
+        xlnx,speed-grade = <1>;
+        xlnx,static-config;
         xlnx,clk-mul-div = <12 1>, <10 2>, <8 1>, <6 1>, <4 2>, <2 1>;
-        clock-names = "clk_in1", "s_axi_aclk";
-        clocks = <&clkc 15>, <&clkc 15>;
     };
 ...
-- 
2.49.1

[PATCH 3/8] dt-bindings: clock: clocking-wizard: Make s_axi_aclk optional for static-config

[Shubhrajyoti Datta]

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.

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

 .../bindings/clock/xlnx,clocking-wizard.yaml     | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
index aa397550d107..0daefe89ea89 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
@@ -87,9 +89,19 @@ allOf:
     then:
       required:
         - xlnx,clk-mul-div
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names:
+          maxItems: 1
     else:
       required:
         - reg
+      properties:
+        clocks:
+          minItems: 2
+        clock-names:
+          minItems: 2
 
 additionalProperties: false
 
@@ -109,8 +121,8 @@ examples:
     clock-controller {
         compatible = "xlnx,clocking-wizard";
         #clock-cells = <1>;
-        clocks = <&clkc 15>, <&clkc 18>;
-        clock-names = "clk_in1", "s_axi_aclk";
+        clocks = <&clkc 15>;
+        clock-names = "clk_in1";
         xlnx,nr-outputs = <6>;
         xlnx,speed-grade = <1>;
         xlnx,static-config;
-- 
2.49.1

Re: [PATCH 3/8] dt-bindings: clock: clocking-wizard: Make s_axi_aclk optional for static-config

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Restricting `clocks` and `clock-names` to `maxItems: 1` in static-config mode breaks backward compatibility with existing device trees.
--

commit e42f118e6d12d9aebf43f9b7f7b451d1e25632ab
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

dt-bindings: clock: clocking-wizard: Make s_axi_aclk optional for static-config

This commit updates the device tree binding for the Xilinx clocking
wizard to allow a single clock input when configured in static-config
mode. It modifies the top-level clock properties to have minItems: 1
and enforces maxItems: 1 in the static-config conditional block.

> diff --git a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> index aa397550d107c..0daefe89ea89f 100644
> --- a/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> +++ b/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml

[ ... ]

> @@ -87,9 +89,19 @@ allOf:
>      then:
>        required:
>          - xlnx,clk-mul-div
> +      properties:
> +        clocks:
> +          maxItems: 1
> +        clock-names:
> +          maxItems: 1

[Severity: High]
The commit message states the intention is to make the s_axi_aclk optional
for static-config. However, by setting maxItems to 1 here, does this strictly
forbid the second clock instead?

Because the previous schema required s_axi_aclk, existing device trees that
specify both xlnx,static-config and s_axi_aclk will now fail dtbs_check
validation.

If the goal is to make the clock optional while maintaining backward
compatibility with older device trees, would it be better to leave maxItems
unrestricted in this block? This would allow older device trees with both
clocks to continue passing validation while new ones can safely omit it.

>      else:
>        required:
>          - reg

[ ... ]

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

[PATCH 4/8] clk: clocking-wizard: Do not map the memory for static-config

[Shubhrajyoti Datta]

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

 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.49.1

Re: [PATCH 4/8] clk: clocking-wizard: Do not map the memory for static-config

[sashiko-bot]

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

Pre-existing issues:
- [High] Registering the same struct notifier_block to two different clock notifiers corrupts the intrusive linked lists.
--

commit 8b915ba31e389cdcf5a743f96eb5c3695bc448c3
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

clk: clocking-wizard: Do not map the memory for static-config

This patch prevents the memory mapping of the AXI register block if the
device is configured with xlnx,static-config. It achieves this by moving
devm_platform_ioremap_resource() under the non-static-config conditional
branch, since only the dynamic path requires the AXI register block.

> 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

[ ... ]

> @@ -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) {

[ ... ]

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

[Severity: High]
This isn't a bug introduced by this patch, but it is a pre-existing issue.

Does registering the same struct notifier_block (&clk_wzrd->nb) to two
different clocks in clk_wzrd_probe() corrupt the SRCU notifier chains?

The clock notifier framework uses srcu_notifier_chain_register(), which
links the provided struct notifier_block into an intrusive linked list by
modifying its next pointer. Because clk_in1 and axi_clk are distinct clocks
with separate notifier chains, adding the exact same notifier_block instance
to the second chain overwrites its next pointer.

Could this merge the two chains, causing events from the first clock to
traverse into the second clock's chain? Furthermore, when the notifiers are
unregistered, would the unregistration logic leave the first clock's chain
pointing to elements from the second clock's chain, leading to a potential
use-after-free?

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

[PATCH 5/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check

[Shubhrajyoti Datta]

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

 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.49.1

[PATCH 6/8] clk: clocking-wizard: Add static-config clock provider support

[Shubhrajyoti Datta]

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

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

diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index ffc78c90bee6..5470a717fccc 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)
@@ -1224,6 +1314,51 @@ 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;
+		}
+
+		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)
+			return dev_err_probe(&pdev->dev, ret,
+					     "unable to register clock provider\n");
 	}
 
 	return 0;
-- 
2.49.1

Re: [PATCH 6/8] clk: clocking-wizard: Add static-config clock provider support

[sashiko-bot]

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

Pre-existing issues:
- [High] Division by zero in `clk_wzrd_determine_rate` when the requested rate is significantly larger than the parent rate.
- [High] Logic bug with ignored `min_t` return value leads to unbounded register writes.
- [High] Division by zero in `clk_wzrd_get_divisors` and `clk_wzrd_get_divisors_ver` when the target rate is 0.
--

commit b31cd102641055f0a34c6e79498f21e726bb2a91
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

clk: clocking-wizard: Add static-config clock provider support

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.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index ffc78c90bee6f..5470a717fccc8 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c

[ ... ]

This isn't a bug introduced by this patch, but I noticed a potential division
by zero in the unpatched code:

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_determine_rate() {
    ...
    div = DIV_ROUND_CLOSEST(req->best_parent_rate, req->rate);

    req->rate = req->best_parent_rate / div;
    ...
}

[Severity: High]
If the requested rate is significantly larger than the best parent rate, 
could div evaluate to 0 and trigger a division by zero exception here?


This is a pre-existing issue, but the capping logic appears to be unused
in the following code:

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_dynamic_reconfig() {
    ...
    /* 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]
Does this code leave value unbounded before the register write since the
return value of min_t() is not assigned to anything?


This isn't a bug introduced by this patch, but there might be another division
by zero risk in the divisor calculations:

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_get_divisors_ver() {
    ...
    o = DIV_ROUND_CLOSEST_ULL(vco_freq, rate);
    ...
}

And similarly in clk_wzrd_get_divisors():

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_get_divisors() {
    ...
    o = DIV_ROUND_CLOSEST_ULL(vco_freq, rate);
    ...
}

[Severity: High]
If rate is exactly 0, can this cause a kernel panic due to a division
by zero?

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

[PATCH 7/8] clk: clocking-wizard: Skip s_axi_aclk for static-config

[Shubhrajyoti Datta]

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

 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 5470a717fccc..fe73ee02b54e 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;
@@ -1258,22 +1257,24 @@ static int clk_wzrd_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	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.49.1

Re: [PATCH 7/8] clk: clocking-wizard: Skip s_axi_aclk for static-config

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver registers the same `struct notifier_block` instance (`clk_wzrd->nb`) to two different clock notification chains (`clk_in1` and `axi_clk`), causing notifier list corruption.
--

commit 2cde73f9cf8529ad3e29f28db978947c4f80fd0e
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 5470a717fccc8..fe73ee02b54ee 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
[ ... ]
> @@ -1258,22 +1257,24 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	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;
> +		}
> +

[Severity: High]
This is a pre-existing issue, but does using the same struct notifier_block
instance for multiple clock notifiers corrupt the notification chain?

Looking further down in clk_wzrd_probe(), the same &clk_wzrd->nb instance
is registered to two different clocks:

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

Since a struct notifier_block has a single next pointer, registering the
same instance to a second chain overwrites its next pointer. Doesn't this
merge the two clock notification chains and corrupt the first notifier list?

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

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

[PATCH 8/8] clk: clocking-wizard: Use dev_err_probe() when mapping registers

[Shubhrajyoti Datta]

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

 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 fe73ee02b54e..381e396aef0e 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -1277,7 +1277,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.49.1

Re: [PATCH 8/8] clk: clocking-wizard: Use dev_err_probe() when mapping registers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Redundant error logging and incorrect commit justification for devm_platform_ioremap_resource()
--

commit fc6668cc718e0e8c82476eab0f48f985072a67de
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

clk: clocking-wizard: Use dev_err_probe() when mapping registers

This commit wraps the error return of devm_platform_ioremap_resource() in
dev_err_probe() to align with other error paths. The commit message states
this is for consistent logging and deferred-probe behavior.

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

[Severity: Medium]
Is the deferred-probe justification accurate for this specific function?
Since devm_platform_ioremap_resource() handles memory resources, it does
not return -EPROBE_DEFER.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index fe73ee02b54ee..381e396aef0e7 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> @@ -1277,7 +1277,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");

[Severity: Medium]
Does this result in duplicate error messages in the kernel log?

The devm_platform_ioremap_resource() helper internally uses dev_err_probe()
to log its own detailed error messages for missing resources, request
failures, and ioremap failures. Wrapping the return value with an additional
dev_err_probe() in the driver might emit a redundant error string.

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