[PATCH v1 0/2] clk: fixed-mmio: Add optional ready registers

[PATCH v1 0/2] clk: fixed-mmio: Add optional ready registers

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

I'm not sure if this is a good idea but while doing some stuff in emulation
I had a need to wait for a fixed-mmio-clock to go ready before using
devices it drives. I figured it may be useful to have a generic way to
describe a simple polling for readiness.

Cheers,
Edgar 

Edgar E. Iglesias (2):
  dt-bindings: clk: fixed-mmio-clock: Add optional ready reg
  clk: fixed-mmio: Add optional poll for clk readiness

 .../bindings/clock/fixed-mmio-clock.yaml      | 38 ++++++++++++++++++-
 drivers/clk/clk-fixed-mmio.c                  | 35 +++++++++++++++++
 2 files changed, 72 insertions(+), 1 deletion(-)


base-commit: 4856ebd997159f198e3177e515bda01143727463
-- 
2.43.0

[PATCH v1 1/2] dt-bindings: clk: fixed-mmio-clock: Add optional ready reg

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Add an optional ready register and properties describing bitfields
that signal when the clock is ready. This can for example be useful
to describe PLL lock bits.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 .../bindings/clock/fixed-mmio-clock.yaml      | 38 ++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml
index e22fc272d023..90033ba389e8 100644
--- a/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml
@@ -10,6 +10,11 @@ description:
   This binding describes a fixed-rate clock for which the frequency can
   be read from a single 32-bit memory mapped I/O register.
 
+  An optional ready register can be specified in a second reg entry.
+  The ready register will be polled until it signals ready prior to reading
+  the fixed rate. This is useful for example to optionally wait for a PLL
+  to lock.
+
   It was designed for test systems, like FPGA, not for complete,
   finished SoCs.
 
@@ -21,7 +26,10 @@ properties:
     const: fixed-mmio-clock
 
   reg:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: Fixed rate register
+      - description: Optional clock ready register
 
   "#clock-cells":
     const: 0
@@ -29,6 +37,25 @@ properties:
   clock-output-names:
     maxItems: 1
 
+  ready-timeout:
+    description:
+      Optional timeout in micro-seconds when polling for clock readiness.
+      0 means no timeout.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0
+
+  ready-mask:
+    description:
+      Optional mask to apply when reading the ready register.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0xffffffff
+
+  ready-value:
+    description:
+      When a ready register is specified in reg, poll the ready reg until
+      ready-reg & ready-mask == ready-value.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
 required:
   - compatible
   - reg
@@ -44,4 +71,13 @@ examples:
       reg = <0xfd020004 0x4>;
       clock-output-names = "sysclk";
     };
+  - |
+    pclk: pclk@fd040000 {
+      compatible = "fixed-mmio-clock";
+      #clock-cells = <0>;
+      reg = <0xfd040000 0x4 0xfd040004 0x4>;
+      ready-mask = <1>;
+      ready-value = <1>;
+      clock-output-names = "pclk";
+    };
 ...
-- 
2.43.0

[PATCH v1 2/2] clk: fixed-mmio: Add optional poll for clk readiness

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Add optional poll for clk readiness prior to reading the fixed rate.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 drivers/clk/clk-fixed-mmio.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/clk/clk-fixed-mmio.c b/drivers/clk/clk-fixed-mmio.c
index 3bfcf4cd98a2..4b5ba1ad06ac 100644
--- a/drivers/clk/clk-fixed-mmio.c
+++ b/drivers/clk/clk-fixed-mmio.c
@@ -11,10 +11,36 @@
 
 #include <linux/clk-provider.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
 
+static int fixed_mmio_clk_wait_ready(struct device_node *node,
+				     void __iomem *base)
+{
+	u32 ready_mask;
+	u32 ready_val;
+	u32 timeout;
+	u32 v;
+
+	if (of_property_read_u32(node, "ready-timeout", &timeout))
+		timeout = 0;
+
+	if (of_property_read_u32(node, "ready-mask", &ready_mask))
+		ready_mask = ~0;
+
+	if (of_property_read_u32(node, "ready-val", &ready_val)) {
+		pr_err("%pOFn: missing ready-val property\n", node);
+		return -EINVAL;
+	}
+
+	pr_info("%pOFn: wait for clock\n", node);
+	return readl_relaxed_poll_timeout_atomic(base, v,
+						 (v & ready_mask) == ready_val,
+						 1, timeout);
+}
+
 static struct clk_hw *fixed_mmio_clk_setup(struct device_node *node)
 {
 	struct clk_hw *clk;
@@ -23,6 +49,15 @@ static struct clk_hw *fixed_mmio_clk_setup(struct device_node *node)
 	u32 freq;
 	int ret;
 
+	base = of_iomap(node, 1);
+	if (base) {
+		/* Wait for clk to get ready. */
+		ret = fixed_mmio_clk_wait_ready(node, base);
+		iounmap(base);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
 	base = of_iomap(node, 0);
 	if (!base) {
 		pr_err("%pOFn: failed to map address\n", node);
-- 
2.43.0

Re: [PATCH v1 1/2] dt-bindings: clk: fixed-mmio-clock: Add optional ready reg

[Krzysztof Kozlowski]

On 25/05/2025 21:08, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> 
> Add an optional ready register and properties describing bitfields
> that signal when the clock is ready. This can for example be useful
> to describe PLL lock bits.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  .../bindings/clock/fixed-mmio-clock.yaml      | 38 ++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml
> index e22fc272d023..90033ba389e8 100644
> --- a/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml
> @@ -10,6 +10,11 @@ description:
>    This binding describes a fixed-rate clock for which the frequency can
>    be read from a single 32-bit memory mapped I/O register.
>  
> +  An optional ready register can be specified in a second reg entry.
> +  The ready register will be polled until it signals ready prior to reading
> +  the fixed rate. This is useful for example to optionally wait for a PLL
> +  to lock.
> +
>    It was designed for test systems, like FPGA, not for complete,
>    finished SoCs.
>  
> @@ -21,7 +26,10 @@ properties:
>      const: fixed-mmio-clock
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: Fixed rate register
> +      - description: Optional clock ready register
>  

I am not convinced we actually want this. If you have more complicated
clocks which need more than one register, then maybe this is too complex
for generic device and you should just make this part of clock controller.

Also I wonder how a clock, which is not controllable, cannot be gated,
can be ready or not. Issue is easily visible in your driver:
1. Probe the driver
2. Clock is not ready - you wait...
3. and wait and entire probe is waiting and busy-looping
4. Probed.
5. Unbind device
6. Rebind and again we check if clock is ready? Why? Nothing changed in
the hardware, clock was not disabled.

Although above is maybe better question for driver design, but it still
makes me wonder whether you are just putting driver complexity into DT.

>    "#clock-cells":
>      const: 0
> @@ -29,6 +37,25 @@ properties:
>    clock-output-names:
>      maxItems: 1
>  
> +  ready-timeout:
> +    description:
> +      Optional timeout in micro-seconds when polling for clock readiness.
> +      0 means no timeout.

Use a proper unit suffix.
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

> +    $ref: /schemas/types.yaml#/definitions/uint32

Drop

> +    default: 0
> +
> +  ready-mask:
> +    description:
> +      Optional mask to apply when reading the ready register.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 0xffffffff
> +
> +  ready-value:
> +    description:
> +      When a ready register is specified in reg, poll the ready reg until
> +      ready-reg & ready-mask == ready-value.
> +    $ref: /schemas/types.yaml#/definitions/uint32


> +
>  required:
>    - compatible
>    - reg
> @@ -44,4 +71,13 @@ examples:
>        reg = <0xfd020004 0x4>;
>        clock-output-names = "sysclk";
>      };
> +  - |
> +    pclk: pclk@fd040000 {

clock@

And drop unused label

> +      compatible = "fixed-mmio-clock";
> +      #clock-cells = <0>;
> +      reg = <0xfd040000 0x4 0xfd040004 0x4>;
> +      ready-mask = <1>;
> +      ready-value = <1>;
> +      clock-output-names = "pclk";
> +    };
>  ...


Best regards,
Krzysztof

Re: [PATCH v1 1/2] dt-bindings: clk: fixed-mmio-clock: Add optional ready reg

[Edgar E. Iglesias]

On Mon, May 26, 2025 at 06:53:14AM +0200, Krzysztof Kozlowski wrote:
> On 25/05/2025 21:08, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> > 
> > Add an optional ready register and properties describing bitfields
> > that signal when the clock is ready. This can for example be useful
> > to describe PLL lock bits.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > ---
> >  .../bindings/clock/fixed-mmio-clock.yaml      | 38 ++++++++++++++++++-
> >  1 file changed, 37 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml
> > index e22fc272d023..90033ba389e8 100644
> > --- a/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml
> > +++ b/Documentation/devicetree/bindings/clock/fixed-mmio-clock.yaml
> > @@ -10,6 +10,11 @@ description:
> >    This binding describes a fixed-rate clock for which the frequency can
> >    be read from a single 32-bit memory mapped I/O register.
> >  
> > +  An optional ready register can be specified in a second reg entry.
> > +  The ready register will be polled until it signals ready prior to reading
> > +  the fixed rate. This is useful for example to optionally wait for a PLL
> > +  to lock.
> > +
> >    It was designed for test systems, like FPGA, not for complete,
> >    finished SoCs.
> >  
> > @@ -21,7 +26,10 @@ properties:
> >      const: fixed-mmio-clock
> >  
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    items:
> > +      - description: Fixed rate register
> > +      - description: Optional clock ready register
> >  
> 
> I am not convinced we actually want this. If you have more complicated
> clocks which need more than one register, then maybe this is too complex
> for generic device and you should just make this part of clock controller.

Right.

> 
> Also I wonder how a clock, which is not controllable, cannot be gated,
> can be ready or not. Issue is easily visible in your driver:
> 1. Probe the driver
> 2. Clock is not ready - you wait...
> 3. and wait and entire probe is waiting and busy-looping
> 4. Probed.
> 5. Unbind device
> 6. Rebind and again we check if clock is ready? Why? Nothing changed in
> the hardware, clock was not disabled.

Yeah, in my particular case, once the clock is ready it will never go
back to "unready" until cold restart.

> 
> Although above is maybe better question for driver design, but it still
> makes me wonder whether you are just putting driver complexity into DT.

Yes, I felt that this was simple enough to have a generic mechanism but perhaps not.


> 
> >    "#clock-cells":
> >      const: 0
> > @@ -29,6 +37,25 @@ properties:
> >    clock-output-names:
> >      maxItems: 1
> >  
> > +  ready-timeout:
> > +    description:
> > +      Optional timeout in micro-seconds when polling for clock readiness.
> > +      0 means no timeout.
> 
> Use a proper unit suffix.
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> Drop

Fixed for v2.


> 
> > +    default: 0
> > +
> > +  ready-mask:
> > +    description:
> > +      Optional mask to apply when reading the ready register.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    default: 0xffffffff
> > +
> > +  ready-value:
> > +    description:
> > +      When a ready register is specified in reg, poll the ready reg until
> > +      ready-reg & ready-mask == ready-value.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> 
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -44,4 +71,13 @@ examples:
> >        reg = <0xfd020004 0x4>;
> >        clock-output-names = "sysclk";
> >      };
> > +  - |
> > +    pclk: pclk@fd040000 {
> 
> clock@
> 
> And drop unused label

Fixed.


> 
> > +      compatible = "fixed-mmio-clock";
> > +      #clock-cells = <0>;
> > +      reg = <0xfd040000 0x4 0xfd040004 0x4>;
> > +      ready-mask = <1>;
> > +      ready-value = <1>;
> > +      clock-output-names = "pclk";
> > +    };
> >  ...
> 
> 
> Best regards,
> Krzysztof