[PATCH v2 0/2] clk: clk-axi-clkgen: make sure to enable the AXI bus clock

[PATCH v2 0/2] clk: clk-axi-clkgen: make sure to enable the AXI bus clock

[Nuno Sa]

Hi,

Here it goes v2. Main difference comes from my discussion with Conor. We
now keep the same old behavior if clock-names is not given and only
enabled the bus clock if provided via clock-names. The goal is to make
sure that old DTs still work.

---
Changes in v2:
- Link to v1: https://lore.kernel.org/r/20241023-axi-clkgen-fix-axiclk-v1-0-980a42ba51c3@analog.com
- Patch 1
  * Make clock-names required.
- Patch 2
  * Only enable the axi clock if clock-names is provided.

---
Nuno Sa (2):
      dt-bindings: clock: axi-clkgen: include AXI clk
      clk: clk-axi-clkgen: make sure to enable the AXI bus clock

 .../devicetree/bindings/clock/adi,axi-clkgen.yaml  | 22 ++++++++++++++++++----
 drivers/clk/clk-axi-clkgen.c                       | 22 ++++++++++++++++++++--
 2 files changed, 38 insertions(+), 6 deletions(-)
---
base-commit: 94be1620fb60ea542170779915917443cda9bba7
change-id: 20241021-axi-clkgen-fix-axiclk-d1d80bcb9ee1
--

Thanks!
- Nuno Sá

[PATCH v2 1/2] dt-bindings: clock: axi-clkgen: include AXI clk

[Nuno Sa]

In order to access the registers of the HW, we need to make sure that
the AXI bus clock is enabled. Hence let's increase the number of clocks
by one and add clock-names to differentiate between parent clocks and
the bus clock.

Fixes: 0e646c52cf0e ("clk: Add axi-clkgen driver")
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 .../devicetree/bindings/clock/adi,axi-clkgen.yaml  | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml b/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml
index 5e942bccf27787d7029f76fc1a284232fb7f279d..2b2041818a0a44456ee986fe29d32346f68835f3 100644
--- a/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml
+++ b/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml
@@ -26,9 +26,21 @@ properties:
     description:
       Specifies the reference clock(s) from which the output frequency is
       derived. This must either reference one clock if only the first clock
-      input is connected or two if both clock inputs are connected.
-    minItems: 1
-    maxItems: 2
+      input is connected or two if both clock inputs are connected. The last
+      clock is the AXI bus clock that needs to be enabled so we can access the
+      core registers.
+    minItems: 2
+    maxItems: 3
+
+  clock-names:
+    oneOf:
+      - items:
+          - const: clkin1
+          - const: s_axi_aclk
+      - items:
+          - const: clkin1
+          - const: clkin2
+          - const: s_axi_aclk
 
   '#clock-cells':
     const: 0
@@ -40,6 +52,7 @@ required:
   - compatible
   - reg
   - clocks
+  - clock-names
   - '#clock-cells'
 
 additionalProperties: false
@@ -50,5 +63,6 @@ examples:
       compatible = "adi,axi-clkgen-2.00.a";
       #clock-cells = <0>;
       reg = <0xff000000 0x1000>;
-      clocks = <&osc 1>;
+      clocks = <&osc 1>, <&clkc 15>;
+      clock-names = "clkin1", "s_axi_aclk";
     };

-- 
2.47.0

Re: [PATCH v2 1/2] dt-bindings: clock: axi-clkgen: include AXI clk

[Conor Dooley]

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

On Tue, Oct 29, 2024 at 02:59:41PM +0100, Nuno Sa wrote:
> In order to access the registers of the HW, we need to make sure that
> the AXI bus clock is enabled. Hence let's increase the number of clocks
> by one and add clock-names to differentiate between parent clocks and
> the bus clock.
> 
> Fixes: 0e646c52cf0e ("clk: Add axi-clkgen driver")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

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

Re: [PATCH v2 1/2] dt-bindings: clock: axi-clkgen: include AXI clk

[Stephen Boyd]

Quoting Nuno Sa (2024-10-29 06:59:41)
> In order to access the registers of the HW, we need to make sure that
> the AXI bus clock is enabled. Hence let's increase the number of clocks
> by one and add clock-names to differentiate between parent clocks and
> the bus clock.
> 
> Fixes: 0e646c52cf0e ("clk: Add axi-clkgen driver")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---

Applied to clk-next

[PATCH v2 2/2] clk: clk-axi-clkgen: make sure to enable the AXI bus clock

[Nuno Sa]

In order to access the registers of the HW, we need to make sure that
the AXI bus clock is enabled. Hence let's increase the number of clocks
by one.

In order to keep backward compatibility and make sure old DTs still work
we check if clock-names is available or not. If it is, then we can
disambiguate between really having the AXI clock or a parent clock and
so we can enable the bus clock. If not, we fallback to what was done
before and don't explicitly enable the AXI bus clock.

Note that if clock-names is given, the axi clock must be the last one in
the phandle array (also enforced in the DT bindings) so that we can reuse
as much code as possible.

Fixes: 0e646c52cf0e ("clk: Add axi-clkgen driver")
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/clk/clk-axi-clkgen.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-axi-clkgen.c b/drivers/clk/clk-axi-clkgen.c
index bf4d8ddc93aea1d4509a9ae50fa69fdb3f707a29..934e53a96dddac8ed61dd109cfc188f3a2a0539a 100644
--- a/drivers/clk/clk-axi-clkgen.c
+++ b/drivers/clk/clk-axi-clkgen.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/platform_device.h>
+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/slab.h>
 #include <linux/io.h>
@@ -512,6 +513,7 @@ static int axi_clkgen_probe(struct platform_device *pdev)
 	struct clk_init_data init;
 	const char *parent_names[2];
 	const char *clk_name;
+	struct clk *axi_clk;
 	unsigned int i;
 	int ret;
 
@@ -528,8 +530,24 @@ static int axi_clkgen_probe(struct platform_device *pdev)
 		return PTR_ERR(axi_clkgen->base);
 
 	init.num_parents = of_clk_get_parent_count(pdev->dev.of_node);
-	if (init.num_parents < 1 || init.num_parents > 2)
-		return -EINVAL;
+
+	axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
+	if (!IS_ERR(axi_clk)) {
+		if (init.num_parents < 2 || init.num_parents > 3)
+			return -EINVAL;
+
+		init.num_parents -= 1;
+	} else {
+		/*
+		 * Legacy... So that old DTs which do not have clock-names still
+		 * work. In this case we don't explicitly enable the AXI bus
+		 * clock.
+		 */
+		if (PTR_ERR(axi_clk) != -ENOENT)
+			return PTR_ERR(axi_clk);
+		if (init.num_parents < 1 || init.num_parents > 2)
+			return -EINVAL;
+	}
 
 	for (i = 0; i < init.num_parents; i++) {
 		parent_names[i] = of_clk_get_parent_name(pdev->dev.of_node, i);

-- 
2.47.0

Re: [PATCH v2 2/2] clk: clk-axi-clkgen: make sure to enable the AXI bus clock

[Stephen Boyd]

Quoting Nuno Sa (2024-10-29 06:59:42)
> In order to access the registers of the HW, we need to make sure that
> the AXI bus clock is enabled. Hence let's increase the number of clocks
> by one.
> 
> In order to keep backward compatibility and make sure old DTs still work
> we check if clock-names is available or not. If it is, then we can
> disambiguate between really having the AXI clock or a parent clock and
> so we can enable the bus clock. If not, we fallback to what was done
> before and don't explicitly enable the AXI bus clock.
> 
> Note that if clock-names is given, the axi clock must be the last one in
> the phandle array (also enforced in the DT bindings) so that we can reuse
> as much code as possible.
> 
> Fixes: 0e646c52cf0e ("clk: Add axi-clkgen driver")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---

Applied to clk-next