diff mbox series

[1/3] dt-bindings: media: i2c: max9286: Add support for per-port supplies

Message ID 20211216220946.20771-2-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State Superseded
Delegated to: Kieran Bingham
Headers show
Series media: i2c: max9286: Small new features | expand

Commit Message

Laurent Pinchart Dec. 16, 2021, 10:09 p.m. UTC 
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Power supplies for the ports can be controlled per port depending on the
hardware design. Support per-port supplies in the DT bindings, mutually
exclusive with the global supply.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../bindings/media/i2c/maxim,max9286.yaml     | 25 ++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Comments

Jacopo Mondi Dec. 17, 2021, 10:47 a.m. UTC | #1 
Hi LAurent

On Fri, Dec 17, 2021 at 12:09:44AM +0200, Laurent Pinchart wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Power supplies for the ports can be controlled per port depending on the
> hardware design. Support per-port supplies in the DT bindings, mutually
> exclusive with the global supply.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../bindings/media/i2c/maxim,max9286.yaml     | 25 ++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> index 02f656e78700..33aa307e8ee5 100644
> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> @@ -39,7 +39,7 @@ properties:
>      maxItems: 1
>
>    poc-supply:
> -    description: Regulator providing Power over Coax to the cameras
> +    description: Regulator providing Power over Coax to all the ports
>

Can anything but a camera be connected to a port ?

>    enable-gpios:
>      description: GPIO connected to the \#PWDN pin with inverted polarity
> @@ -160,6 +160,10 @@ properties:
>
>              additionalProperties: false
>
> +patternProperties:
> +  "^port[0-3]-poc-supply$":
> +    description: Regulator providing Power over Coax for a particular port
> +
>  required:
>    - compatible
>    - reg
> @@ -167,6 +171,25 @@ required:
>    - i2c-mux
>    - gpio-controller
>
> +allOf:
> +  - if:
> +      required:
> +        - poc-supply
> +    then:
> +      allOf:
> +        - not:
> +            required:
> +              - port0-poc-supply
> +        - not:
> +            required:
> +              - port1-poc-supply
> +        - not:
> +            required:
> +              - port2-poc-supply
> +        - not:
> +            required:
> +              - port3-poc-supply

Isn't this simply expressed as

if:
  required:
    - poc-supply
then:
  properties:
    port0-poc-supply: false
    port1-poc-supply: false
    port2-poc-supply: false
    port3-poc-supply: false

I tried tweaking the DTS file example with the above applied as

        poc-supply = <&camera_poc_12v>;
        port0-poc-supply = <&camera0_poc>;

And validation fails as expected
.../maxim,max9286.example.dt.yaml: gmsl-deserializer@2c: port0-poc-supply: False schema does not allow [[4294967295]]

Also, could you make sure this does not conflict with the introduction
of gpio-poc in "dt-bindings: media: max9286: Define 'maxim,gpio-poc'".

Thanks
   j


> +
>  additionalProperties: false
>
>  examples:
> --
> Regards,
>
> Laurent Pinchart
>
Rob Herring Dec. 17, 2021, 2:18 p.m. UTC | #2 
On Fri, Dec 17, 2021 at 12:09:44AM +0200, Laurent Pinchart wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Power supplies for the ports can be controlled per port depending on the
> hardware design. Support per-port supplies in the DT bindings, mutually
> exclusive with the global supply.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../bindings/media/i2c/maxim,max9286.yaml     | 25 ++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> index 02f656e78700..33aa307e8ee5 100644
> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> @@ -39,7 +39,7 @@ properties:
>      maxItems: 1
>  
>    poc-supply:
> -    description: Regulator providing Power over Coax to the cameras
> +    description: Regulator providing Power over Coax to all the ports
>  
>    enable-gpios:
>      description: GPIO connected to the \#PWDN pin with inverted polarity
> @@ -160,6 +160,10 @@ properties:
>  
>              additionalProperties: false
>  
> +patternProperties:
> +  "^port[0-3]-poc-supply$":
> +    description: Regulator providing Power over Coax for a particular port
> +
>  required:
>    - compatible
>    - reg
> @@ -167,6 +171,25 @@ required:
>    - i2c-mux
>    - gpio-controller
>  
> +allOf:
> +  - if:
> +      required:
> +        - poc-supply
> +    then:
> +      allOf:
> +        - not:
> +            required:
> +              - port0-poc-supply
> +        - not:
> +            required:
> +              - port1-poc-supply
> +        - not:
> +            required:
> +              - port2-poc-supply
> +        - not:
> +            required:
> +              - port3-poc-supply

I think you can invert the if and move patternProperties to the 'then' 
and...

> +
>  additionalProperties: false

then use unevaluatedProperties here.

Rob
Laurent Pinchart Dec. 17, 2021, 3:58 p.m. UTC | #3 
Hi Jacopo,

On Fri, Dec 17, 2021 at 11:47:10AM +0100, Jacopo Mondi wrote:
> On Fri, Dec 17, 2021 at 12:09:44AM +0200, Laurent Pinchart wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Power supplies for the ports can be controlled per port depending on the
> > hardware design. Support per-port supplies in the DT bindings, mutually
> > exclusive with the global supply.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../bindings/media/i2c/maxim,max9286.yaml     | 25 ++++++++++++++++++-
> >  1 file changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > index 02f656e78700..33aa307e8ee5 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > @@ -39,7 +39,7 @@ properties:
> >      maxItems: 1
> >
> >    poc-supply:
> > -    description: Regulator providing Power over Coax to the cameras
> > +    description: Regulator providing Power over Coax to all the ports
> >
> 
> Can anything but a camera be connected to a port ?
> 
> >    enable-gpios:
> >      description: GPIO connected to the \#PWDN pin with inverted polarity
> > @@ -160,6 +160,10 @@ properties:
> >
> >              additionalProperties: false
> >
> > +patternProperties:
> > +  "^port[0-3]-poc-supply$":
> > +    description: Regulator providing Power over Coax for a particular port
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -167,6 +171,25 @@ required:
> >    - i2c-mux
> >    - gpio-controller
> >
> > +allOf:
> > +  - if:
> > +      required:
> > +        - poc-supply
> > +    then:
> > +      allOf:
> > +        - not:
> > +            required:
> > +              - port0-poc-supply
> > +        - not:
> > +            required:
> > +              - port1-poc-supply
> > +        - not:
> > +            required:
> > +              - port2-poc-supply
> > +        - not:
> > +            required:
> > +              - port3-poc-supply
> 
> Isn't this simply expressed as
> 
> if:
>   required:
>     - poc-supply
> then:
>   properties:
>     port0-poc-supply: false
>     port1-poc-supply: false
>     port2-poc-supply: false
>     port3-poc-supply: false

I would have sworn I had tried that and that it didn't work... I now
have

allOf:
  - if:
      required:
        - poc-supply
    then:
      patternProperties:
        '^port[0-3]-poc-supply$': false

additionalProperties: false

and it seems to do the job. I'll use that in a v2.

> I tried tweaking the DTS file example with the above applied as
> 
>         poc-supply = <&camera_poc_12v>;
>         port0-poc-supply = <&camera0_poc>;
> 
> And validation fails as expected
> .../maxim,max9286.example.dt.yaml: gmsl-deserializer@2c: port0-poc-supply: False schema does not allow [[4294967295]]
> 
> Also, could you make sure this does not conflict with the introduction
> of gpio-poc in "dt-bindings: media: max9286: Define 'maxim,gpio-poc'".
> 
> > +
> >  additionalProperties: false
> >
> >  examples:
Laurent Pinchart Dec. 17, 2021, 3:59 p.m. UTC | #4 
Hi Rob,

On Fri, Dec 17, 2021 at 08:18:06AM -0600, Rob Herring wrote:
> On Fri, Dec 17, 2021 at 12:09:44AM +0200, Laurent Pinchart wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Power supplies for the ports can be controlled per port depending on the
> > hardware design. Support per-port supplies in the DT bindings, mutually
> > exclusive with the global supply.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../bindings/media/i2c/maxim,max9286.yaml     | 25 ++++++++++++++++++-
> >  1 file changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > index 02f656e78700..33aa307e8ee5 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > @@ -39,7 +39,7 @@ properties:
> >      maxItems: 1
> >  
> >    poc-supply:
> > -    description: Regulator providing Power over Coax to the cameras
> > +    description: Regulator providing Power over Coax to all the ports
> >  
> >    enable-gpios:
> >      description: GPIO connected to the \#PWDN pin with inverted polarity
> > @@ -160,6 +160,10 @@ properties:
> >  
> >              additionalProperties: false
> >  
> > +patternProperties:
> > +  "^port[0-3]-poc-supply$":
> > +    description: Regulator providing Power over Coax for a particular port
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -167,6 +171,25 @@ required:
> >    - i2c-mux
> >    - gpio-controller
> >  
> > +allOf:
> > +  - if:
> > +      required:
> > +        - poc-supply
> > +    then:
> > +      allOf:
> > +        - not:
> > +            required:
> > +              - port0-poc-supply
> > +        - not:
> > +            required:
> > +              - port1-poc-supply
> > +        - not:
> > +            required:
> > +              - port2-poc-supply
> > +        - not:
> > +            required:
> > +              - port3-poc-supply
> 
> I think you can invert the if and move patternProperties to the 'then' 
> and...
> 
> > +
> >  additionalProperties: false
> 
> then use unevaluatedProperties here.

I ended up doing the following, which I would have sworn I had tried
before:

allOf:
  - if:
      required:
        - poc-supply
    then:
      patternProperties:
        '^port[0-3]-poc-supply$': false

additionalProperties: false

It seems to work, so I'll use it in v2.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
index 02f656e78700..33aa307e8ee5 100644
--- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
@@ -39,7 +39,7 @@  properties:
     maxItems: 1
 
   poc-supply:
-    description: Regulator providing Power over Coax to the cameras
+    description: Regulator providing Power over Coax to all the ports
 
   enable-gpios:
     description: GPIO connected to the \#PWDN pin with inverted polarity
@@ -160,6 +160,10 @@  properties:
 
             additionalProperties: false
 
+patternProperties:
+  "^port[0-3]-poc-supply$":
+    description: Regulator providing Power over Coax for a particular port
+
 required:
   - compatible
   - reg
@@ -167,6 +171,25 @@  required:
   - i2c-mux
   - gpio-controller
 
+allOf:
+  - if:
+      required:
+        - poc-supply
+    then:
+      allOf:
+        - not:
+            required:
+              - port0-poc-supply
+        - not:
+            required:
+              - port1-poc-supply
+        - not:
+            required:
+              - port2-poc-supply
+        - not:
+            required:
+              - port3-poc-supply
+
 additionalProperties: false
 
 examples: