[PATCH v4 6/9] dt-bindings: clock: nxp,imx95-blk-ctl: Add ldb child node

Laurentiu Palcu posted 9 patches 4 weeks, 1 day ago
There is a newer version of this series
[PATCH v4 6/9] dt-bindings: clock: nxp,imx95-blk-ctl: Add ldb child node
Posted by Laurentiu Palcu 4 weeks, 1 day ago
Since the BLK CTL registers, like the LVDS CSR, can be used to control the
LVDS Display Bridge controllers, add 'ldb' child node to handle
these use cases.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 .../bindings/clock/nxp,imx95-blk-ctl.yaml     | 24 ++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
index 27403b4c52d62..f83d96701bb04 100644
--- a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
+++ b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
@@ -39,6 +39,28 @@ properties:
       ID in its "clocks" phandle cell. See
       include/dt-bindings/clock/nxp,imx95-clock.h
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: nxp,imx94-lvds-csr
+then:
+  properties:
+    "#address-cells":
+      const: 1
+
+    "#size-cells":
+      const: 1
+
+  patternProperties:
+    "^ldb@[0-9a-f]+$":
+      type: object
+      $ref: /schemas/display/bridge/fsl,ldb.yaml#
+
+  required:
+    - '#address-cells'
+    - '#size-cells'
+
 required:
   - compatible
   - reg
@@ -46,7 +68,7 @@ required:
   - power-domains
   - clocks
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
-- 
2.49.0
Re: [PATCH v4 6/9] dt-bindings: clock: nxp,imx95-blk-ctl: Add ldb child node
Posted by Krzysztof Kozlowski 4 weeks, 1 day ago
On Wed, Sep 03, 2025 at 03:33:24PM +0300, Laurentiu Palcu wrote:
> Since the BLK CTL registers, like the LVDS CSR, can be used to control the
> LVDS Display Bridge controllers, add 'ldb' child node to handle
> these use cases.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  .../bindings/clock/nxp,imx95-blk-ctl.yaml     | 24 ++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)

Nothing in the changelog explains this patch.

What happened with entire previous review?

> 
> diff --git a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> index 27403b4c52d62..f83d96701bb04 100644
> --- a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> +++ b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> @@ -39,6 +39,28 @@ properties:
>        ID in its "clocks" phandle cell. See
>        include/dt-bindings/clock/nxp,imx95-clock.h
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: nxp,imx94-lvds-csr
> +then:
> +  properties:
> +    "#address-cells":
> +      const: 1
> +
> +    "#size-cells":
> +      const: 1
> +
> +  patternProperties:
> +    "^ldb@[0-9a-f]+$":

No, don't define nodes in if:then:. Where did you get this syntax from?

> +      type: object
> +      $ref: /schemas/display/bridge/fsl,ldb.yaml#
> +
> +  required:
> +    - '#address-cells'
> +    - '#size-cells'
> +
>  required:
>    - compatible
>    - reg
> @@ -46,7 +68,7 @@ required:
>    - power-domains
>    - clocks
>  
> -additionalProperties: false
> +unevaluatedProperties: false

NAK, so schema warned you above syntax is wrong and you decided to
silence the warning with this hack, right?

Best regards,
Krzysztof
Re: [PATCH v4 6/9] dt-bindings: clock: nxp,imx95-blk-ctl: Add ldb child node
Posted by Laurentiu Palcu 4 weeks ago
On Thu, Sep 04, 2025 at 09:27:31AM +0200, Krzysztof Kozlowski wrote:
> On Wed, Sep 03, 2025 at 03:33:24PM +0300, Laurentiu Palcu wrote:
> > Since the BLK CTL registers, like the LVDS CSR, can be used to control the
> > LVDS Display Bridge controllers, add 'ldb' child node to handle
> > these use cases.
> > 
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > ---
> >  .../bindings/clock/nxp,imx95-blk-ctl.yaml     | 24 ++++++++++++++++++-
> >  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> Nothing in the changelog explains this patch.
> 
> What happened with entire previous review?
I will try adding a changlog to the individual patches in the future... :/

> 
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> > index 27403b4c52d62..f83d96701bb04 100644
> > --- a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> > +++ b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> > @@ -39,6 +39,28 @@ properties:
> >        ID in its "clocks" phandle cell. See
> >        include/dt-bindings/clock/nxp,imx95-clock.h
> >  
> > +if:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        const: nxp,imx94-lvds-csr
> > +then:
> > +  properties:
> > +    "#address-cells":
> > +      const: 1
> > +
> > +    "#size-cells":
> > +      const: 1
> > +
> > +  patternProperties:
> > +    "^ldb@[0-9a-f]+$":
> 
> No, don't define nodes in if:then:. Where did you get this syntax from?
I guess I looked over various bindings using patternProperties in if:then:
blocks but I completely missed the fact that the nodes were actually defined
outside... And you gave me a good hint in a reply for v4 but, somehow, I failed
to completely understand what you suggested... :/

Hopefully, the following *is* what you meant:

...
patternProperties:
  "^ldb@[0-9a-f]+$":
    type: object
    $ref: /schemas/display/bridge/fsl,ldb.yaml#

if:
  not:
    properties:
      compatible:
        contains:
          const: nxp,imx94-lvds-csr
then:
  patternProperties:
    "^ldb@[0-9a-f]+$": false
else:
  required:
    - '#address-cells'
    - '#size-cells'
...

> 
> > +      type: object
> > +      $ref: /schemas/display/bridge/fsl,ldb.yaml#
> > +
> > +  required:
> > +    - '#address-cells'
> > +    - '#size-cells'
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -46,7 +68,7 @@ required:
> >    - power-domains
> >    - clocks
> >  
> > -additionalProperties: false
> > +unevaluatedProperties: false
> 
> NAK, so schema warned you above syntax is wrong and you decided to
> silence the warning with this hack, right?
Indeed, the checks gave me a warning and I thought it was the right thing to
use 'unevaluatedProperties: false' since additionalProperties cannot recognize
properties declared in subschemas...

Anyway, with the change above, 'additionalProperties: false' will do just fine.

Thanks,
Laurentiu

> 
> Best regards,
> Krzysztof
>
Re: [PATCH v4 6/9] dt-bindings: clock: nxp,imx95-blk-ctl: Add ldb child node
Posted by Frank Li 4 weeks ago
On Thu, Sep 04, 2025 at 03:14:53PM +0300, Laurentiu Palcu wrote:
> On Thu, Sep 04, 2025 at 09:27:31AM +0200, Krzysztof Kozlowski wrote:
> > On Wed, Sep 03, 2025 at 03:33:24PM +0300, Laurentiu Palcu wrote:
> > > Since the BLK CTL registers, like the LVDS CSR, can be used to control the
> > > LVDS Display Bridge controllers, add 'ldb' child node to handle
> > > these use cases.
> > >
> > > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > > ---
> > >  .../bindings/clock/nxp,imx95-blk-ctl.yaml     | 24 ++++++++++++++++++-
> > >  1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > Nothing in the changelog explains this patch.
> >
> > What happened with entire previous review?
> I will try adding a changlog to the individual patches in the future... :/
>
> >
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> > > index 27403b4c52d62..f83d96701bb04 100644
> > > --- a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> > > +++ b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> > > @@ -39,6 +39,28 @@ properties:
> > >        ID in its "clocks" phandle cell. See
> > >        include/dt-bindings/clock/nxp,imx95-clock.h
> > >
> > > +if:
> > > +  properties:
> > > +    compatible:
> > > +      contains:
> > > +        const: nxp,imx94-lvds-csr
> > > +then:
> > > +  properties:
> > > +    "#address-cells":
> > > +      const: 1
> > > +
> > > +    "#size-cells":
> > > +      const: 1
> > > +
> > > +  patternProperties:
> > > +    "^ldb@[0-9a-f]+$":
> >
> > No, don't define nodes in if:then:. Where did you get this syntax from?
> I guess I looked over various bindings using patternProperties in if:then:
> blocks but I completely missed the fact that the nodes were actually defined
> outside... And you gave me a good hint in a reply for v4 but, somehow, I failed
> to completely understand what you suggested... :/
>
> Hopefully, the following *is* what you meant:
>
> ...
> patternProperties:
>   "^ldb@[0-9a-f]+$":
>     type: object
>     $ref: /schemas/display/bridge/fsl,ldb.yaml#
>
> if:
>   not:
>     properties:
>       compatible:
>         contains:
>           const: nxp,imx94-lvds-csr
> then:
>   patternProperties:
>     "^ldb@[0-9a-f]+$": false
> else:
>   required:
>     - '#address-cells'
>     - '#size-cells'
> ...
>
> >
> > > +      type: object
> > > +      $ref: /schemas/display/bridge/fsl,ldb.yaml#
> > > +
> > > +  required:
> > > +    - '#address-cells'
> > > +    - '#size-cells'
> > > +
> > >  required:
> > >    - compatible
> > >    - reg
> > > @@ -46,7 +68,7 @@ required:
> > >    - power-domains
> > >    - clocks
> > >
> > > -additionalProperties: false
> > > +unevaluatedProperties: false
> >
> > NAK, so schema warned you above syntax is wrong and you decided to
> > silence the warning with this hack, right?
> Indeed, the checks gave me a warning and I thought it was the right thing to
> use 'unevaluatedProperties: false' since additionalProperties cannot recognize
> properties declared in subschemas...
>
> Anyway, with the change above, 'additionalProperties: false' will do just fine.

Laurentiu:

	You can ping me by nxp team first if you have problem or can't
understand Krzysztof's means.

	Krzysztof is quite busy!

Frank

>
> Thanks,
> Laurentiu
>
> >
> > Best regards,
> > Krzysztof
> >