Hi Niklas, On Mon, Jun 04, 2018 at 02:19:33PM +0200, Niklas Söderlund wrote: > Hi Jacopo, > > Thanks for your work. > > On 2018-05-29 17:05:57 +0200, Jacopo Mondi wrote: > > Document the boolean custom property 'renesas,hsync-as-de' that indicates > > that the HSYNC signal is internally used as data-enable, when the > > CLKENB signal is not connected. > > > > As this is a VIN specificity create a custom property specific to the R-Car > > VIN driver. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> > > --- > > v3: > > - new patch > > --- > > Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > > index ff53226..024c109 100644 > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > @@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > > - vsync-active: see [1] for description. Default is active high. > > - data-enable-active: polarity of CLKENB signal, see [1] for > > description. Default is active high. > > + - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal > > + is internally used as data-enable when the CLKENB signal is > > + not available. > > I'm not sure I like this, is there really a need to add a custom > property for this? The datasheet states that when the CLKENB pin is not > connected the driver should enable 'Clock Enable Hsync Select (CHS)'. > With the new generic property 'data-enable-active' which describes the > polarity of the CLKENB pin we also gain the knowledge if the CLKENB pin > is connected or not. That was my first proposal, we discussed that in Re: [PATCH 3/6] media: rcar-vin: Handle data-active property Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active Let's sum it up in this way: Instead of having to deal again with the "what happens if there is no data-enable-active and we're running on BT.656 where there is no HSYNC signal"[1] I decided to go with a custom property. > > I propose we drop this custom property and instead let the driver check > if the CLKENB polarity is described or not and use that to determine if > CHS bit should be set or not. IMHO that is much simpler then having two > properties describing the same pin. > It is my understanding that both Gen2 and Gen3 boards CVBS input are BT.656 and none of them has CLKENB input. So 'data-enable-active' is never there but in this case we should not set CHS. So the absence of 'data-enable-active' has different consequences if we're running on parallel or BT.656 bus, and that feels confusing to me, so I decided to make it an explicit property. Also, as the interface manual describes the "use HSYNC in place of CLKENB" when on parallel bus as a design choice, that should imo be documented. Again, this is very minor stuff. I'll leave this out from next version, maybe we can talk about this f2f next week. Thanks j > > > > If both HSYNC and VSYNC polarities are not specified, embedded > > synchronization is selected. > > -- > > 2.7.4 > > > > -- > Regards, > Niklas Söderlund
Attachment:
signature.asc
Description: PGP signature
