Re: [PATCH 1/2] dt-bindings: media: renesas,vin: Document renesas-vin-ycbcr-8b-g property

[Niklas <niklas.soderlund@ragnatech.se>]

Hi Lad,

Thanks for your patch.

On 2020-07-24 15:58:51 +0100, Lad Prabhakar wrote:
> Add a DT property "renesas-vin-ycbcr-8b-g" to select YCbCr422 8-bit data
> input pins.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/media/renesas,vin.yaml | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml
> index 53c0a72..7dfb781 100644
> --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml
> @@ -106,6 +106,12 @@ properties:
>  
>            remote-endpoint: true
>  
> +          renesas-vin-ycbcr-8b-g:

I think the preferred format for vendor specific properties are 
"<vendor>,<property>".

This nit apart I'm not sure a property is the right way here. Could it 
not be possible on some designs to have two different sensors one wired 
to DATA[7:0] and the other to DATA[15:8] and by controlling the 
VNDRM2_YDS register at runtime switch between the two? If so adding a DT 
property to hard-code one of the two options would prevent this. I fear 
we need to think of a runtime way to deal with this.

The best way to do that I think is to extend the port@0 node to allow 
for two endpoints, one for each of the two possible parallel sensors.  
This would then have to be expressed in the media graph and selection if 
YDS should be set or not depend on which media links are enabled.

> +            type: boolean
> +            description:
> +              If present this property specifies to selects VIN_G[7:0] as data pins for YCbCr422 8-bit data.
> +            default: false
> +
>          required:
>            - remote-endpoint
>  
> @@ -168,6 +174,13 @@ properties:
>  
>                remote-endpoint: true
>  
> +              renesas-vin-ycbcr-8b-g:
> +                type: boolean
> +                description:
> +                  If present this property specifies to selects VIN_G[7:0] as data pins for
> +                  YCbCr422 8-bit data.
> +                default: false
> +
>              required:
>                - remote-endpoint
>  
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund