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

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

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