Re: [PATCH 2/2] media: dt-bindings: vpif: extend the example with an output port

From: Bartosz Golaszewski
Date: Tue Feb 21 2017 - 10:08:20 EST


2017-02-21 15:57 GMT+01:00 Rob Herring <robh+dt@xxxxxxxxxx>:
> On Thu, Feb 16, 2017 at 12:03 PM, Bartosz Golaszewski
> <bgolaszewski@xxxxxxxxxxxx> wrote:
>> This makes the example more or less correspond with the da850-evm
>> hardware setup.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
>> ---
>> .../devicetree/bindings/media/ti,da850-vpif.txt | 36 +++++++++++++++++-----
>> 1 file changed, 29 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> index 9c7510b..d6f93a1 100644
>> --- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> @@ -16,7 +16,7 @@ Required properties:
>> Video Capture:
>>
>> VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
>> -single 16-bit channel. It should contain at least one port child node
>> +single 16-bit channel. It should contain one or two port child nodes
>
> When do you not have an output? Seems kind of useless. I'm guessing
> this is an oversight in the binding and there should always be port 1
> and a connector node, so document it that way.
>

Sometimes the output pins are not hooked up to anything - for example
on the da850-lcdk board we only use input for camera capture, while on
da850-evm we have both composite and s-video outputs and inputs. I
think it makes sens to leave out the output port if we don't use it.

> This also needs to state what port 0 and port 1 are (i.e. input or
> output and type of data).
>

I'll fix that.

>> with child 'endpoint' node. Please refer to the bindings defined in
>> Documentation/devicetree/bindings/media/video-interfaces.txt.
>>
>> @@ -28,19 +28,26 @@ I2C-connected TVP5147 decoder:
>> reg = <0x217000 0x1000>;
>> interrupts = <92>;
>>
>> - port {
>> - vpif_ch0: endpoint@0 {
>> + port@0 {
>> + vpif_input_ch0: endpoint@0 {
>> reg = <0>;
>> bus-width = <8>;
>> - remote-endpoint = <&composite>;
>> + remote-endpoint = <&composite_in>;
>> };
>>
>> - vpif_ch1: endpoint@1 {
>> + vpif_input_ch1: endpoint@1 {
>> reg = <1>;
>> bus-width = <8>;
>> data-shift = <8>;
>> };
>> };
>> +
>> + port@1 {
>> + vpif_output_ch0: endpoint {
>> + bus-width = <8>;
>
> Isn't this an analog input? bus-width doesn't make sense for that.
>

Not yet - the signals first go to a video DAC (adv7343 in the example).

Thanks,
Bartosz