Re: [PATCH 06/13] dt-bindings: media: ov5640: Require ep properties

[Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>]

Hi Jacopo,

Thank you for the patch.

On Fri, Jul 17, 2020 at 03:28:52PM +0200, Jacopo Mondi wrote:
> Now that the bus-type property has been made mandatory for
> the ov5640 bindings, use the property value to mark endpoint
> properties as required depending on the data interface in use.

Ah, please disregard my comment on 05/13 that asked for this :-)

> Mark as 'required' all the properties that were defined as required
> in the original bindings in textual form. Adjust the example to
> comply with validation requirement but do not update the existing
> users as most of the required properties will be demoted in the
> next patches.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> ---
>  .../devicetree/bindings/media/i2c/ov5640.yaml | 41 +++++++++++++++++--
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.yaml b/Documentation/devicetree/bindings/media/i2c/ov5640.yaml
> index 16e6c2dc629a..8fd54575a1db 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov5640.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.yaml
> @@ -110,9 +110,40 @@ properties:
>            pclk-sample:
>              enum: [0, 1]
>  
> -        required:
> -          - remote-endpoint
> -          - bus-type
> +        allOf:
> +          - if:
> +              properties:
> +                bus-type:
> +                  const: 4
> +            then:
> +                properties:

I think there's one indentation level too much here.

This should really be moved to video-interfaces.yaml, I'll let you
decide if you want to bite the bullet now or if it should be done on
top.

Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

> +                  bus-width: false
> +                  data-shift: false
> +                  hsync-active: false
> +                  vsync-active: false
> +                  pclk-sample: false
> +                required:
> +                  - remote-endpoint
> +                  - bus-type
> +                  - clock-lanes
> +                  - data-lanes
> +
> +          - if:
> +              properties:
> +                bus-type:
> +                  const: 5
> +            then:
> +              properties:
> +                clock-lanes: false
> +                data-lanes: false
> +              required:
> +                - remote-endpoint
> +                - bus-type
> +                - bus-width
> +                - data-shift
> +                - hsync-active
> +                - vsync-active
> +                - pclk-sample
>  
>      additionalProperties: false
>  
> @@ -179,6 +210,10 @@ examples:
>                      remote-endpoint = <&parallel_from_ov5640>;
>                      bus-type = <5>;
>                      bus-width = <10>;
> +                    data-shift = <0>;
> +                    hsync-active = <1>;
> +                    vsync-active = <1>;
> +                    pclk-sample = <1>;
>                  };
>              };
>          };

-- 
Regards,

Laurent Pinchart