[PATCH v4 0/6] media: rcar-vin: Brush endpoint properties

[PATCH v4 0/6] media: rcar-vin: Brush endpoint properties

[Jacopo Mondi]

Hello,
  4th round for the VIN endpoint brushing series.

Slightly enlarged the linux-media receiver list, as this new version
introduces a common property in 'video-interfaces.txt'.

Compared to v3 I have dropped the last controversial parts:

- The custom 'renesas,hsync-as-de' property has been dropped: do not handle
  CHS bit for the moment.
- Do not remove properties not parsed by the driver and not listed in the
  interface bindings from Gen2 boards. As this lead to a long discussion, I
  have now proposed a patch to clearly state that properties not listed in the
  interface bindings can be optionally specified, but they don't affect the
  interface behaviour. To avoid more discussions this patch may be dropped
  and things will stay the way they are now.

For the common 'data-enable-active' property, I guess with Rob's ack we should
be fine there and I hope the rest of the series won't slow down its acceptance.

Thanks
   j

Jacopo Mondi (6):
  dt-bindings: media: rcar-vin: Describe optional ep properties
  dt-bindings: media: Document data-enable-active property
  media: v4l2-fwnode: parse 'data-enable-active' prop
  dt-bindings: media: rcar-vin: Add 'data-enable-active'
  media: rcar-vin: Handle data-enable polarity
  dt-bindings: media: rcar-vin: Clarify optional props

 Documentation/devicetree/bindings/media/rcar_vin.txt   | 18 ++++++++++++++++++
 .../devicetree/bindings/media/video-interfaces.txt     |  2 ++
 drivers/media/platform/rcar-vin/rcar-dma.c             |  5 +++++
 drivers/media/v4l2-core/v4l2-fwnode.c                  |  4 ++++
 include/media/v4l2-mediabus.h                          |  2 ++
 5 files changed, 31 insertions(+)

--
2.7.4

[PATCH v4 0/6] media: rcar-vin: Brush endpoint properties

[Jacopo Mondi]

Hello,
  4th round for the VIN endpoint brushing series.

Slightly enlarged the linux-media receiver list, as this new version
introduces a common property in 'video-interfaces.txt'.

Compared to v3 I have dropped the last controversial parts:

- The custom 'renesas,hsync-as-de' property has been dropped: do not handle
  CHS bit for the moment.
- Do not remove properties not parsed by the driver and not listed in the
  interface bindings from Gen2 boards. As this lead to a long discussion, I
  have now proposed a patch to clearly state that properties not listed in the
  interface bindings can be optionally specified, but they don't affect the
  interface behaviour. To avoid more discussions this patch may be dropped
  and things will stay the way they are now.

For the common 'data-enable-active' property, I guess with Rob's ack we should
be fine there and I hope the rest of the series won't slow down its acceptance.

Thanks
   j

Jacopo Mondi (6):
  dt-bindings: media: rcar-vin: Describe optional ep properties
  dt-bindings: media: Document data-enable-active property
  media: v4l2-fwnode: parse 'data-enable-active' prop
  dt-bindings: media: rcar-vin: Add 'data-enable-active'
  media: rcar-vin: Handle data-enable polarity
  dt-bindings: media: rcar-vin: Clarify optional props

 Documentation/devicetree/bindings/media/rcar_vin.txt   | 18 ++++++++++++++++++
 .../devicetree/bindings/media/video-interfaces.txt     |  2 ++
 drivers/media/platform/rcar-vin/rcar-dma.c             |  5 +++++
 drivers/media/v4l2-core/v4l2-fwnode.c                  |  4 ++++
 include/media/v4l2-mediabus.h                          |  2 ++
 5 files changed, 31 insertions(+)

--
2.7.4

[PATCH v4 0/6] media: rcar-vin: Brush endpoint properties

[Jacopo Mondi]

Hello,
  4th round for the VIN endpoint brushing series.

Slightly enlarged the linux-media receiver list, as this new version
introduces a common property in 'video-interfaces.txt'.

Compared to v3 I have dropped the last controversial parts:

- The custom 'renesas,hsync-as-de' property has been dropped: do not handle
  CHS bit for the moment.
- Do not remove properties not parsed by the driver and not listed in the
  interface bindings from Gen2 boards. As this lead to a long discussion, I
  have now proposed a patch to clearly state that properties not listed in the
  interface bindings can be optionally specified, but they don't affect the
  interface behaviour. To avoid more discussions this patch may be dropped
  and things will stay the way they are now.

For the common 'data-enable-active' property, I guess with Rob's ack we should
be fine there and I hope the rest of the series won't slow down its acceptance.

Thanks
   j

Jacopo Mondi (6):
  dt-bindings: media: rcar-vin: Describe optional ep properties
  dt-bindings: media: Document data-enable-active property
  media: v4l2-fwnode: parse 'data-enable-active' prop
  dt-bindings: media: rcar-vin: Add 'data-enable-active'
  media: rcar-vin: Handle data-enable polarity
  dt-bindings: media: rcar-vin: Clarify optional props

 Documentation/devicetree/bindings/media/rcar_vin.txt   | 18 ++++++++++++++++++
 .../devicetree/bindings/media/video-interfaces.txt     |  2 ++
 drivers/media/platform/rcar-vin/rcar-dma.c             |  5 +++++
 drivers/media/v4l2-core/v4l2-fwnode.c                  |  4 ++++
 include/media/v4l2-mediabus.h                          |  2 ++
 5 files changed, 31 insertions(+)

--
2.7.4

[PATCH v4 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Jacopo Mondi]

Describe the optional endpoint properties for endpoint nodes of port@0
and port@1 of the R-Car VIN driver device tree bindings documentation.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..87f93ec 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -53,6 +53,14 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       from external SoC pins described in video-interfaces.txt[1].
       Describing more then one endpoint in port 0 is invalid. Only VIN
       instances that are connected to external pins should have port 0.
+
+      - Optional properties for endpoint nodes of port@0:
+        - hsync-active: see [1] for description. Default is active high.
+        - vsync-active: see [1] for description. Default is active high.
+
+        If both HSYNC and VSYNC polarities are not specified, embedded
+        synchronization is selected.
+
     - port 1 - sub-nodes describing one or more endpoints connected to
       the VIN from local SoC CSI-2 receivers. The endpoint numbers must
       use the following schema.
@@ -62,6 +70,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
         - Endpoint 2 - sub-node describing the endpoint connected to CSI40
         - Endpoint 3 - sub-node describing the endpoint connected to CSI41
 
+      Endpoint nodes of port@1 do not support any optional endpoint property.
+
 Device node example for Gen2 platforms
 --------------------------------------
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Jacopo Mondi]

Describe the optional endpoint properties for endpoint nodes of port@0
and port@1 of the R-Car VIN driver device tree bindings documentation.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..87f93ec 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -53,6 +53,14 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       from external SoC pins described in video-interfaces.txt[1].
       Describing more then one endpoint in port 0 is invalid. Only VIN
       instances that are connected to external pins should have port 0.
+
+      - Optional properties for endpoint nodes of port@0:
+        - hsync-active: see [1] for description. Default is active high.
+        - vsync-active: see [1] for description. Default is active high.
+
+        If both HSYNC and VSYNC polarities are not specified, embedded
+        synchronization is selected.
+
     - port 1 - sub-nodes describing one or more endpoints connected to
       the VIN from local SoC CSI-2 receivers. The endpoint numbers must
       use the following schema.
@@ -62,6 +70,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
         - Endpoint 2 - sub-node describing the endpoint connected to CSI40
         - Endpoint 3 - sub-node describing the endpoint connected to CSI41
 
+      Endpoint nodes of port@1 do not support any optional endpoint property.
+
 Device node example for Gen2 platforms
 --------------------------------------
 
-- 
2.7.4

[PATCH v4 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Jacopo Mondi]

Describe the optional endpoint properties for endpoint nodes of port at 0
and port at 1 of the R-Car VIN driver device tree bindings documentation.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..87f93ec 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -53,6 +53,14 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       from external SoC pins described in video-interfaces.txt[1].
       Describing more then one endpoint in port 0 is invalid. Only VIN
       instances that are connected to external pins should have port 0.
+
+      - Optional properties for endpoint nodes of port at 0:
+        - hsync-active: see [1] for description. Default is active high.
+        - vsync-active: see [1] for description. Default is active high.
+
+        If both HSYNC and VSYNC polarities are not specified, embedded
+        synchronization is selected.
+
     - port 1 - sub-nodes describing one or more endpoints connected to
       the VIN from local SoC CSI-2 receivers. The endpoint numbers must
       use the following schema.
@@ -62,6 +70,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
         - Endpoint 2 - sub-node describing the endpoint connected to CSI40
         - Endpoint 3 - sub-node describing the endpoint connected to CSI41
 
+      Endpoint nodes of port at 1 do not support any optional endpoint property.
+
 Device node example for Gen2 platforms
 --------------------------------------
 
-- 
2.7.4

[PATCH v4 2/6] dt-bindings: media: Document data-enable-active property

[Jacopo Mondi]

Add 'data-enable-active' property to endpoint node properties list.

The property allows to specify the polarity of the data-enable signal, which
when in active state determinates when data lines have to sampled for valid
pixel data.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 258b8df..9839d26 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -109,6 +109,8 @@ Optional endpoint properties
   Note, that if HSYNC and VSYNC polarities are not specified, embedded
   synchronization may be required, where supported.
 - data-active: similar to HSYNC and VSYNC, specifies data line polarity.
+- data-enable-active: similar to HSYNC and VSYNC, specifies the data enable
+  signal polarity.
 - field-even-active: field signal level during the even field data transmission.
 - pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
   signal.
-- 
2.7.4

[PATCH v4 2/6] dt-bindings: media: Document data-enable-active property

[Jacopo Mondi]

Add 'data-enable-active' property to endpoint node properties list.

The property allows to specify the polarity of the data-enable signal, which
when in active state determinates when data lines have to sampled for valid
pixel data.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 258b8df..9839d26 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -109,6 +109,8 @@ Optional endpoint properties
   Note, that if HSYNC and VSYNC polarities are not specified, embedded
   synchronization may be required, where supported.
 - data-active: similar to HSYNC and VSYNC, specifies data line polarity.
+- data-enable-active: similar to HSYNC and VSYNC, specifies the data enable
+  signal polarity.
 - field-even-active: field signal level during the even field data transmission.
 - pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
   signal.
-- 
2.7.4

[PATCH v4 2/6] dt-bindings: media: Document data-enable-active property

[Jacopo Mondi]

Add 'data-enable-active' property to endpoint node properties list.

The property allows to specify the polarity of the data-enable signal, which
when in active state determinates when data lines have to sampled for valid
pixel data.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 258b8df..9839d26 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -109,6 +109,8 @@ Optional endpoint properties
   Note, that if HSYNC and VSYNC polarities are not specified, embedded
   synchronization may be required, where supported.
 - data-active: similar to HSYNC and VSYNC, specifies data line polarity.
+- data-enable-active: similar to HSYNC and VSYNC, specifies the data enable
+  signal polarity.
 - field-even-active: field signal level during the even field data transmission.
 - pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
   signal.
-- 
2.7.4

[PATCH v4 3/6] media: v4l2-fwnode: parse 'data-enable-active' prop

[Jacopo Mondi]

Parse the newly defined 'data-enable-active' property in parallel endpoint
parsing function.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++
 include/media/v4l2-mediabus.h         | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 3f77aa3..6105191 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -154,6 +154,10 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus(
 		flags |= v ? V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH :
 			V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW;
 
+	if (!fwnode_property_read_u32(fwnode, "data-enable-active", &v))
+		flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH :
+			V4L2_MBUS_DATA_ENABLE_LOW;
+
 	bus->flags = flags;
 
 }
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 4d8626c..4bbb5f3 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -45,6 +45,8 @@
 /* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */
 #define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH		BIT(12)
 #define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW		BIT(13)
+#define V4L2_MBUS_DATA_ENABLE_HIGH		BIT(14)
+#define V4L2_MBUS_DATA_ENABLE_LOW		BIT(15)
 
 /* Serial flags */
 /* How many lanes the client can use */
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/6] media: v4l2-fwnode: parse 'data-enable-active' prop

[Jacopo Mondi]

Parse the newly defined 'data-enable-active' property in parallel endpoint
parsing function.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++
 include/media/v4l2-mediabus.h         | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 3f77aa3..6105191 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -154,6 +154,10 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus(
 		flags |= v ? V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH :
 			V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW;
 
+	if (!fwnode_property_read_u32(fwnode, "data-enable-active", &v))
+		flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH :
+			V4L2_MBUS_DATA_ENABLE_LOW;
+
 	bus->flags = flags;
 
 }
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 4d8626c..4bbb5f3 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -45,6 +45,8 @@
 /* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */
 #define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH		BIT(12)
 #define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW		BIT(13)
+#define V4L2_MBUS_DATA_ENABLE_HIGH		BIT(14)
+#define V4L2_MBUS_DATA_ENABLE_LOW		BIT(15)
 
 /* Serial flags */
 /* How many lanes the client can use */
-- 
2.7.4

[PATCH v4 3/6] media: v4l2-fwnode: parse 'data-enable-active' prop

[Jacopo Mondi]

Parse the newly defined 'data-enable-active' property in parallel endpoint
parsing function.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++
 include/media/v4l2-mediabus.h         | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 3f77aa3..6105191 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -154,6 +154,10 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus(
 		flags |= v ? V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH :
 			V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW;
 
+	if (!fwnode_property_read_u32(fwnode, "data-enable-active", &v))
+		flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH :
+			V4L2_MBUS_DATA_ENABLE_LOW;
+
 	bus->flags = flags;
 
 }
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 4d8626c..4bbb5f3 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -45,6 +45,8 @@
 /* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */
 #define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH		BIT(12)
 #define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW		BIT(13)
+#define V4L2_MBUS_DATA_ENABLE_HIGH		BIT(14)
+#define V4L2_MBUS_DATA_ENABLE_LOW		BIT(15)
 
 /* Serial flags */
 /* How many lanes the client can use */
-- 
2.7.4

[PATCH v4 4/6] dt-bindings: media: rcar-vin: Add 'data-enable-active'

[Jacopo Mondi]

Describe optional endpoint property 'data-enable-active' for R-Car VIN.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 87f93ec..8130849 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -57,6 +57,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       - Optional properties for endpoint nodes of port@0:
         - 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
+          description. Default is active high.
 
         If both HSYNC and VSYNC polarities are not specified, embedded
         synchronization is selected.
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 4/6] dt-bindings: media: rcar-vin: Add 'data-enable-active'

[Jacopo Mondi]

Describe optional endpoint property 'data-enable-active' for R-Car VIN.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 87f93ec..8130849 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -57,6 +57,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       - Optional properties for endpoint nodes of port@0:
         - 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
+          description. Default is active high.
 
         If both HSYNC and VSYNC polarities are not specified, embedded
         synchronization is selected.
-- 
2.7.4

[PATCH v4 4/6] dt-bindings: media: rcar-vin: Add 'data-enable-active'

[Jacopo Mondi]

Describe optional endpoint property 'data-enable-active' for R-Car VIN.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas at ragnatech.se
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 87f93ec..8130849 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -57,6 +57,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       - Optional properties for endpoint nodes of port@0:
         - 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
+          description. Default is active high.
 
         If both HSYNC and VSYNC polarities are not specified, embedded
         synchronization is selected.
-- 
2.7.4

[PATCH v4 5/6] media: rcar-vin: Handle data-enable polarity

[Jacopo Mondi]

Handle data-enable signal polarity. If the polarity is not specifically
requested to be active low, use the active high default.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index d2b7002..9145b56 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -123,6 +123,7 @@
 /* Video n Data Mode Register 2 bits */
 #define VNDMR2_VPS		(1 << 30)
 #define VNDMR2_HPS		(1 << 29)
+#define VNDMR2_CES		(1 << 28)
 #define VNDMR2_FTEV		(1 << 17)
 #define VNDMR2_VLV(n)		((n & 0xf) << 12)
 
@@ -698,6 +699,10 @@ static int rvin_setup(struct rvin_dev *vin)
 		/* Vsync Signal Polarity Select */
 		if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
 			dmr2 |= VNDMR2_VPS;
+
+		/* Data Enable Polarity Select */
+		if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
+			dmr2 |= VNDMR2_CES;
 	}
 
 	/*
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 5/6] media: rcar-vin: Handle data-enable polarity

[Jacopo Mondi]

Handle data-enable signal polarity. If the polarity is not specifically
requested to be active low, use the active high default.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index d2b7002..9145b56 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -123,6 +123,7 @@
 /* Video n Data Mode Register 2 bits */
 #define VNDMR2_VPS		(1 << 30)
 #define VNDMR2_HPS		(1 << 29)
+#define VNDMR2_CES		(1 << 28)
 #define VNDMR2_FTEV		(1 << 17)
 #define VNDMR2_VLV(n)		((n & 0xf) << 12)
 
@@ -698,6 +699,10 @@ static int rvin_setup(struct rvin_dev *vin)
 		/* Vsync Signal Polarity Select */
 		if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
 			dmr2 |= VNDMR2_VPS;
+
+		/* Data Enable Polarity Select */
+		if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
+			dmr2 |= VNDMR2_CES;
 	}
 
 	/*
-- 
2.7.4

[PATCH v4 5/6] media: rcar-vin: Handle data-enable polarity

[Jacopo Mondi]

Handle data-enable signal polarity. If the polarity is not specifically
requested to be active low, use the active high default.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index d2b7002..9145b56 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -123,6 +123,7 @@
 /* Video n Data Mode Register 2 bits */
 #define VNDMR2_VPS		(1 << 30)
 #define VNDMR2_HPS		(1 << 29)
+#define VNDMR2_CES		(1 << 28)
 #define VNDMR2_FTEV		(1 << 17)
 #define VNDMR2_VLV(n)		((n & 0xf) << 12)
 
@@ -698,6 +699,10 @@ static int rvin_setup(struct rvin_dev *vin)
 		/* Vsync Signal Polarity Select */
 		if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
 			dmr2 |= VNDMR2_VPS;
+
+		/* Data Enable Polarity Select */
+		if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
+			dmr2 |= VNDMR2_CES;
 	}
 
 	/*
-- 
2.7.4

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Jacopo Mondi]

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@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@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:
+
         - 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
-- 
2.7.4

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Jacopo Mondi]

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@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@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:
+
         - 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
-- 
2.7.4

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Jacopo Mondi]

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:
+
         - 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
-- 
2.7.4

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Sakari Ailus]

Hi Jacopo,

On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> 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@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@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 don't think this should be needed. You should only have properties that
describe the hardware configuration in a given system.

>          - 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
> -- 
> 2.7.4
> 

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Sakari Ailus]

Hi Jacopo,

On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> 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@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@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 don't think this should be needed. You should only have properties that
describe the hardware configuration in a given system.

>          - 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
> -- 
> 2.7.4
> 

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Sakari Ailus]

Hi Jacopo,

On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> 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 don't think this should be needed. You should only have properties that
describe the hardware configuration in a given system.

>          - 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
> -- 
> 2.7.4
> 

-- 
Sakari Ailus
sakari.ailus at linux.intel.com

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Laurent Pinchart]

Hello,

On Tuesday, 12 June 2018 18:45:53 EEST Sakari Ailus wrote:
> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > 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@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@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 don't think this should be needed. You should only have properties that
> describe the hardware configuration in a given system.

I agree. Please list explicitly all properties that are applicable for this 
device, and don't tell which one(s) are used by the driver.

> >          - 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

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Laurent Pinchart]

Hello,

On Tuesday, 12 June 2018 18:45:53 EEST Sakari Ailus wrote:
> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > 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@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@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 don't think this should be needed. You should only have properties that
> describe the hardware configuration in a given system.

I agree. Please list explicitly all properties that are applicable for this 
device, and don't tell which one(s) are used by the driver.

> >          - 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

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Laurent Pinchart]

Hello,

On Tuesday, 12 June 2018 18:45:53 EEST Sakari Ailus wrote:
> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > 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 don't think this should be needed. You should only have properties that
> describe the hardware configuration in a given system.

I agree. Please list explicitly all properties that are applicable for this 
device, and don't tell which one(s) are used by the driver.

> >          - 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

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[jacopo mondi]

[-- Attachment #1.1: Type: text/plain, Size: 2961 bytes --]

Hi Sakari,

On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > 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@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@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 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.

Thanks
   j

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

> >          - 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
> > --
> > 2.7.4
> >
>
> --
> Sakari Ailus
> sakari.ailus@linux.intel.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 2961 bytes --]

Hi Sakari,

On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > 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@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@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 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.

Thanks
   j

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

> >          - 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
> > --
> > 2.7.4
> >
>
> --
> Sakari Ailus
> sakari.ailus@linux.intel.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[jacopo mondi]

Hi Sakari,

On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > 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 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.

Thanks
   j

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

> >          - 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
> > --
> > 2.7.4
> >
>
> --
> Sakari Ailus
> sakari.ailus at linux.intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180613/e8557938/attachment.sig>

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Niklas Söderlund]

Hi Jacopo, Sakari and Laurent,

Jacopo, thanks for your patch and continued work on resolving this.

On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote:
> Hi Sakari,
> 
> On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > > 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@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@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.

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@0 optional properties for both 
generations of hardware?

> >
> > 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?

> 
> Thanks
>    j
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> 
> > >          - 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
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Sakari Ailus
> > sakari.ailus@linux.intel.com



-- 
Regards,
Niklas Söderlund

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Niklas Söderlund]

Hi Jacopo, Sakari and Laurent,

Jacopo, thanks for your patch and continued work on resolving this.

On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote:
> Hi Sakari,
> 
> On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > > 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@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@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.

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@0 optional properties for both 
generations of hardware?

> >
> > 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?

> 
> Thanks
>    j
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> 
> > >          - 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
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Sakari Ailus
> > sakari.ailus@linux.intel.com



-- 
Regards,
Niklas Söderlund

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Niklas Söderlund]

Hi Jacopo, Sakari and Laurent,

Jacopo, thanks for your patch and continued work on resolving this.

On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote:
> Hi Sakari,
> 
> On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > > 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@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@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.

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@0 optional properties for both 
generations of hardware?

> >
> > 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?

> 
> Thanks
>    j
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> 
> > >          - 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
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Sakari Ailus
> > sakari.ailus@linux.intel.com



-- 
Regards,
Niklas S�derlund

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Niklas Söderlund]

Hi Jacopo, Sakari and Laurent,

Jacopo, thanks for your patch and continued work on resolving this.

On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote:
> Hi Sakari,
> 
> On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > > 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.

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?

> >
> > 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?

> 
> Thanks
>    j
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> 
> > >          - 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
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Sakari Ailus
> > sakari.ailus at linux.intel.com



-- 
Regards,
Niklas S?derlund

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Laurent Pinchart]

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:
> > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> >>> 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@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@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@0 optional properties for both
> generations of hardware?

That would be nice too :-)

> >> 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.

> > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > 
> >>> - 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

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Laurent Pinchart]

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:
> > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> >>> 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@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@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@0 optional properties for both
> generations of hardware?

That would be nice too :-)

> >> 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.

> > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > 
> >>> - 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

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Laurent Pinchart]

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:
> > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> >>> 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 :-)

> >> 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.

> > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > 
> >>> - 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

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[jacopo mondi]

[-- Attachment #1.1: Type: text/plain, Size: 7006 bytes --]

Hi Laurent, Niklas,

On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote:
> 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:
> > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > >>> 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@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@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.
>

Fine, I'll surrender then, you're too many to fight against :p

Joking aside, I see your points, and I'll resend dropping this last
bit and documenting which properties applies to the interface.

I can list:
- vsync-active (added by this series)
- hsync-active (added by this series)
- data-enable-active (added by this series)
- field-active-even (to be added)

I haven't find a way to configure the plck-sample value in the VINs
registers. Should we skip it?

I also think 'bus-width' and 'data-shift' are not directly configurable in
registers but depends on SoC, the selected input interface, and the
input image format (see tables at 26.3.1 datasheet revision 1.00), so
it makes sense list them as acceptable properties.

Do I have your ack on this so I can re-spin?

> > 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@0 optional properties for both
> > generations of hardware?
>
> That would be nice too :-)

I see..

Well, I would keep the Gen2/Gen3 sections separate, as they have a
different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could
have several 'port' nodes enclosed in a 'ports' one).

But I could indeed reference the optional endpoint properties of
'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do
you have any preference?

Thanks
   j


>
> > >> 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.
>
> > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > >
> > >>> - 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
>
>
>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 7006 bytes --]

Hi Laurent, Niklas,

On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote:
> 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:
> > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > >>> 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@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@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.
>

Fine, I'll surrender then, you're too many to fight against :p

Joking aside, I see your points, and I'll resend dropping this last
bit and documenting which properties applies to the interface.

I can list:
- vsync-active (added by this series)
- hsync-active (added by this series)
- data-enable-active (added by this series)
- field-active-even (to be added)

I haven't find a way to configure the plck-sample value in the VINs
registers. Should we skip it?

I also think 'bus-width' and 'data-shift' are not directly configurable in
registers but depends on SoC, the selected input interface, and the
input image format (see tables at 26.3.1 datasheet revision 1.00), so
it makes sense list them as acceptable properties.

Do I have your ack on this so I can re-spin?

> > 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@0 optional properties for both
> > generations of hardware?
>
> That would be nice too :-)

I see..

Well, I would keep the Gen2/Gen3 sections separate, as they have a
different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could
have several 'port' nodes enclosed in a 'ports' one).

But I could indeed reference the optional endpoint properties of
'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do
you have any preference?

Thanks
   j


>
> > >> 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.
>
> > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > >
> > >>> - 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
>
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[jacopo mondi]

Hi Laurent, Niklas,

On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote:
> 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:
> > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > >>> 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.
>

Fine, I'll surrender then, you're too many to fight against :p

Joking aside, I see your points, and I'll resend dropping this last
bit and documenting which properties applies to the interface.

I can list:
- vsync-active (added by this series)
- hsync-active (added by this series)
- data-enable-active (added by this series)
- field-active-even (to be added)

I haven't find a way to configure the plck-sample value in the VINs
registers. Should we skip it?

I also think 'bus-width' and 'data-shift' are not directly configurable in
registers but depends on SoC, the selected input interface, and the
input image format (see tables at 26.3.1 datasheet revision 1.00), so
it makes sense list them as acceptable properties.

Do I have your ack on this so I can re-spin?

> > 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 :-)

I see..

Well, I would keep the Gen2/Gen3 sections separate, as they have a
different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could
have several 'port' nodes enclosed in a 'ports' one).

But I could indeed reference the optional endpoint properties of
'port at 0' of Gen3 bindings in Gen2 ones. Or the other way around. Do
you have any preference?

Thanks
   j


>
> > >> 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.
>
> > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > >
> > >>> - 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
>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180703/77435bfe/attachment.sig>

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Niklas Söderlund]

Hi Jacopo,

Thanks for not giving up on this work :-)

On 2018-07-03 21:52:38 +0200, Jacopo Mondi wrote:
> Hi Laurent, Niklas,
> 
> On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote:
> > 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:
> > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > > >>> 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@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@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.
> >
> 
> Fine, I'll surrender then, you're too many to fight against :p
> 
> Joking aside, I see your points, and I'll resend dropping this last
> bit and documenting which properties applies to the interface.
> 
> I can list:
> - vsync-active (added by this series)
> - hsync-active (added by this series)
> - data-enable-active (added by this series)
> - field-active-even (to be added)
> 
> I haven't find a way to configure the plck-sample value in the VINs
> registers. Should we skip it?

IIRC my findings agree with yours, no register to control plck-sample in 
the VIN register set.

> 
> I also think 'bus-width' and 'data-shift' are not directly configurable in
> registers but depends on SoC, the selected input interface, and the
> input image format (see tables at 26.3.1 datasheet revision 1.00), so
> it makes sense list them as acceptable properties.
> 
> Do I have your ack on this so I can re-spin?

I think I understand your plan and I think it sounds ok, please try it 
:-)

> 
> > > 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@0 optional properties for both
> > > generations of hardware?
> >
> > That would be nice too :-)
> 
> I see..
> 
> Well, I would keep the Gen2/Gen3 sections separate, as they have a
> different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could
> have several 'port' nodes enclosed in a 'ports' one).
> 
> But I could indeed reference the optional endpoint properties of
> 'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do
> you have any preference?

My preference would be to list the properties in the Gen2 section and in 
the Gen3 port@0 section refer to the Gen2 optional properties.

> 
> Thanks
>    j
> 
> 
> >
> > > >> 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.
> >
> > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > > >
> > > >>> - 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
> >
> >
> >



-- 
Regards,
Niklas Söderlund

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Niklas Söderlund]

Hi Jacopo,

Thanks for not giving up on this work :-)

On 2018-07-03 21:52:38 +0200, Jacopo Mondi wrote:
> Hi Laurent, Niklas,
> 
> On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote:
> > 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:
> > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > > >>> 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@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@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.
> >
> 
> Fine, I'll surrender then, you're too many to fight against :p
> 
> Joking aside, I see your points, and I'll resend dropping this last
> bit and documenting which properties applies to the interface.
> 
> I can list:
> - vsync-active (added by this series)
> - hsync-active (added by this series)
> - data-enable-active (added by this series)
> - field-active-even (to be added)
> 
> I haven't find a way to configure the plck-sample value in the VINs
> registers. Should we skip it?

IIRC my findings agree with yours, no register to control plck-sample in 
the VIN register set.

> 
> I also think 'bus-width' and 'data-shift' are not directly configurable in
> registers but depends on SoC, the selected input interface, and the
> input image format (see tables at 26.3.1 datasheet revision 1.00), so
> it makes sense list them as acceptable properties.
> 
> Do I have your ack on this so I can re-spin?

I think I understand your plan and I think it sounds ok, please try it 
:-)

> 
> > > 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@0 optional properties for both
> > > generations of hardware?
> >
> > That would be nice too :-)
> 
> I see..
> 
> Well, I would keep the Gen2/Gen3 sections separate, as they have a
> different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could
> have several 'port' nodes enclosed in a 'ports' one).
> 
> But I could indeed reference the optional endpoint properties of
> 'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do
> you have any preference?

My preference would be to list the properties in the Gen2 section and in 
the Gen3 port@0 section refer to the Gen2 optional properties.

> 
> Thanks
>    j
> 
> 
> >
> > > >> 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.
> >
> > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > > >
> > > >>> - 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
> >
> >
> >



-- 
Regards,
Niklas Söderlund

Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Niklas Söderlund]

Hi Jacopo,

Thanks for not giving up on this work :-)

On 2018-07-03 21:52:38 +0200, Jacopo Mondi wrote:
> Hi Laurent, Niklas,
> 
> On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote:
> > 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:
> > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > > >>> 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@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@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.
> >
> 
> Fine, I'll surrender then, you're too many to fight against :p
> 
> Joking aside, I see your points, and I'll resend dropping this last
> bit and documenting which properties applies to the interface.
> 
> I can list:
> - vsync-active (added by this series)
> - hsync-active (added by this series)
> - data-enable-active (added by this series)
> - field-active-even (to be added)
> 
> I haven't find a way to configure the plck-sample value in the VINs
> registers. Should we skip it?

IIRC my findings agree with yours, no register to control plck-sample in 
the VIN register set.

> 
> I also think 'bus-width' and 'data-shift' are not directly configurable in
> registers but depends on SoC, the selected input interface, and the
> input image format (see tables at 26.3.1 datasheet revision 1.00), so
> it makes sense list them as acceptable properties.
> 
> Do I have your ack on this so I can re-spin?

I think I understand your plan and I think it sounds ok, please try it 
:-)

> 
> > > 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@0 optional properties for both
> > > generations of hardware?
> >
> > That would be nice too :-)
> 
> I see..
> 
> Well, I would keep the Gen2/Gen3 sections separate, as they have a
> different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could
> have several 'port' nodes enclosed in a 'ports' one).
> 
> But I could indeed reference the optional endpoint properties of
> 'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do
> you have any preference?

My preference would be to list the properties in the Gen2 section and in 
the Gen3 port@0 section refer to the Gen2 optional properties.

> 
> Thanks
>    j
> 
> 
> >
> > > >> 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.
> >
> > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > > >
> > > >>> - 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
> >
> >
> >



-- 
Regards,
Niklas S�derlund

[PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props

[Niklas Söderlund]

Hi Jacopo,

Thanks for not giving up on this work :-)

On 2018-07-03 21:52:38 +0200, Jacopo Mondi wrote:
> Hi Laurent, Niklas,
> 
> On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote:
> > 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:
> > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote:
> > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote:
> > > >>> 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.
> >
> 
> Fine, I'll surrender then, you're too many to fight against :p
> 
> Joking aside, I see your points, and I'll resend dropping this last
> bit and documenting which properties applies to the interface.
> 
> I can list:
> - vsync-active (added by this series)
> - hsync-active (added by this series)
> - data-enable-active (added by this series)
> - field-active-even (to be added)
> 
> I haven't find a way to configure the plck-sample value in the VINs
> registers. Should we skip it?

IIRC my findings agree with yours, no register to control plck-sample in 
the VIN register set.

> 
> I also think 'bus-width' and 'data-shift' are not directly configurable in
> registers but depends on SoC, the selected input interface, and the
> input image format (see tables at 26.3.1 datasheet revision 1.00), so
> it makes sense list them as acceptable properties.
> 
> Do I have your ack on this so I can re-spin?

I think I understand your plan and I think it sounds ok, please try it 
:-)

> 
> > > 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 :-)
> 
> I see..
> 
> Well, I would keep the Gen2/Gen3 sections separate, as they have a
> different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could
> have several 'port' nodes enclosed in a 'ports' one).
> 
> But I could indeed reference the optional endpoint properties of
> 'port at 0' of Gen3 bindings in Gen2 ones. Or the other way around. Do
> you have any preference?

My preference would be to list the properties in the Gen2 section and in 
the Gen3 port at 0 section refer to the Gen2 optional properties.

> 
> Thanks
>    j
> 
> 
> >
> > > >> 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.
> >
> > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html
> > > >
> > > >>> - 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
> >
> >
> >



-- 
Regards,
Niklas S?derlund