Hi Niklas,

On Wednesday, 27 June 2018 08:24:31 EEST Niklas S?derlund wrote:

On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote:

quoted

On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:

quoted

On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:

quoted

Add a note to the R-Car VIN interface bindings to clarify that all
properties listed as generic properties in video-interfaces.txt can
be included in port at 0 endpoint, but if not explicitly listed in the
interface bindings documentation, they do not modify it behaviour.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---

 Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt

b/Documentation/devicetree/bindings/media/rcar_vin.txt index
8130849..03544c7 100644

--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt

@@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on

SoC.

       instances that are connected to external pins should have port
       0.

       - Optional properties for endpoint nodes of port at 0:
+
+        All properties described in [1] and which apply to the
selected
+        media bus type could be optionally listed here to better
describe
+        the current hardware configuration, but only the following
ones do
+        actually modify the VIN interface behaviour:
+

I'm not sure the description have to be as explicit to that the
properties in 'video-interfaces.txt' are not currently used by the
driver. I find it not relevant which ones are used or not, what is
important for me is that all properties in 'video-interfaces.txt' which
can be used to describe the specific bus are valid for the DT
description.

I agree with you. The driver is irrelevant in this context. What matters is 
which properties are applicable to the bus. For instance, if the VIN parallel 
input supports configurable polarities for the h/v sync signals, hsync-active 
and vsync-active should be listed in the bindings. On the other hand, if the 
polarities are fixed, then the properties are not needed.

On a side note, in rcar_vin.txt we have this section describing the Gen2
bindings:

  The per-board settings Gen2 platforms:
   - port sub-node describing a single endpoint connected to the vin
     as described in video-interfaces.txt[1]. Only the first one will
     be considered as each vin interface has one input port.

This whole series only deals with documenting the Gen3 optional
properties and not the Gen2. Maybe with parallel input support for Gen3
patches on there way to making it upstream this series should be
extended to in a good way merge the port at 0 optional properties for both
generations of hardware?

That would be nice too :-)

quoted

quoted

I don't think this should be needed. You should only have properties
that describe the hardware configuration in a given system.

There has been quite some debate on this, and please bear with me
here for re-proposing it: I started by removing properties in some DT
files for older Renesas board which listed endpoint properties not
documented in the VIN's bindings and not parsed by the VIN driver [1]
Niklas (but Simon and Geert seems to agree here) opposed to that
patch, as those properties where described in 'video-interfaces.txt' and
even if not parsed by the current driver implementation, they actually
describe hardware. I rebated that only properties listed in the device
bindings documentation should actually be used, and having properties
not parsed by the driver confuses users, which may expect changing
them modifies the interface configuration, which does not happens at
the moment.

This came out as a middle ground from a discussion with Niklas. As
stated in the cover letter if this patch makes someone uncomfortable, feel
free to drop it not to hold back the rest of the series which has been
well received instead.

What I don't agree with and sparked this debate from my side was the
deletion of properties in dts files which correctly does describe the
bus but which are not currently parsed by the driver. To me that is
decreasing the value of the dts. If on the other hand the goal is to
deprecate a property from the video-interfaces.txt by slowly removing
them from dts where the driver don't use them I'm all for it. But I
don't think this is the case here right?

I think you're right, I don't think that's the case.

We should not remove properties from the dts files when they correctly 
describe the hardware and have an added-value. On the other hand, if a 
property correctly describes the hardware, but is constrained to a single 
value due to hardware limitations, then we can omit it.

quoted

[1] https://www.spinics.net/lists/arm-kernel/msg656302.html

quoted

quoted

- hsync-active: see [1] for description. Default is active high.
- vsync-active: see [1] for description. Default is active high.
- data-enable-active: polarity of CLKENB signal, see [1] for

-- 
Regards,

Laurent Pinchart