[PATCH v3 0/8] media: rcar-vin: Brush endpoint properties

[PATCH v3 0/8] media: rcar-vin: Brush endpoint properties

[Jacopo Mondi]

Hello,
   3rd version of VIN endpoint brushing series.

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

Quite some changes compared to v1/v2:
- First patch in the series changed to include Niklas' comments on using
  spaces for indent over tabs in documentation and as suggested by Rob and
  Laurent to refer to 'video-interfaces.txt' in property description.
- As suggested by Rob this series introduces a new property 'data-enable-active'
  to describe data enable signal polarity instead of using 'data-active' which
  refers to the data lanes polarity.
- Use this new property to control CLKENB pin polarity in VIN driver
- Introduce a new custom property to describe the 'use HSYNC as data-enable'
  function provided by VIN. In previous iterations I used the presence of
  'data-active' to enable/disable this functionality. That was confusing and
  not correct as it used the wrong property. As this is a VIN specificity, I
  thought a custom property is more suited.
- Last patch is still there and I know it is debated. My opinion is that it is
  still needed, as the presence of those un-documented and un-parsed properties
  confuses users which may expect changing those properties value to reflect on
  the video interface configuration, which does not happens instead.

Individual changelog per patch when relevant.

Thanks
    j

Jacopo Mondi (8):
  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: rcar-vin: Add 'hsync-as-de' custom prop
  media: rcar-vin: Handle 'hsync-as-de' property
  ARM: dts: rcar-gen2: Remove unused VIN properties

 Documentation/devicetree/bindings/media/rcar_vin.txt     | 16 +++++++++++++++-
 .../devicetree/bindings/media/video-interfaces.txt       |  2 ++
 arch/arm/boot/dts/r8a7790-lager.dts                      |  3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts                    |  3 ---
 arch/arm/boot/dts/r8a7791-porter.dts                     |  1 -
 arch/arm/boot/dts/r8a7793-gose.dts                       |  3 ---
 arch/arm/boot/dts/r8a7794-alt.dts                        |  1 -
 arch/arm/boot/dts/r8a7794-silk.dts                       |  1 -
 drivers/media/platform/rcar-vin/rcar-core.c              |  6 ++++++
 drivers/media/platform/rcar-vin/rcar-dma.c               | 11 +++++++++++
 drivers/media/platform/rcar-vin/rcar-vin.h               |  2 ++
 drivers/media/v4l2-core/v4l2-fwnode.c                    |  5 +++++
 include/media/v4l2-mediabus.h                            |  2 ++
 13 files changed, 43 insertions(+), 13 deletions(-)

--
2.7.4

[PATCH v3 0/8] media: rcar-vin: Brush endpoint properties

[Jacopo Mondi]

Hello,
   3rd version of VIN endpoint brushing series.

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

Quite some changes compared to v1/v2:
- First patch in the series changed to include Niklas' comments on using
  spaces for indent over tabs in documentation and as suggested by Rob and
  Laurent to refer to 'video-interfaces.txt' in property description.
- As suggested by Rob this series introduces a new property 'data-enable-active'
  to describe data enable signal polarity instead of using 'data-active' which
  refers to the data lanes polarity.
- Use this new property to control CLKENB pin polarity in VIN driver
- Introduce a new custom property to describe the 'use HSYNC as data-enable'
  function provided by VIN. In previous iterations I used the presence of
  'data-active' to enable/disable this functionality. That was confusing and
  not correct as it used the wrong property. As this is a VIN specificity, I
  thought a custom property is more suited.
- Last patch is still there and I know it is debated. My opinion is that it is
  still needed, as the presence of those un-documented and un-parsed properties
  confuses users which may expect changing those properties value to reflect on
  the video interface configuration, which does not happens instead.

Individual changelog per patch when relevant.

Thanks
    j

Jacopo Mondi (8):
  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: rcar-vin: Add 'hsync-as-de' custom prop
  media: rcar-vin: Handle 'hsync-as-de' property
  ARM: dts: rcar-gen2: Remove unused VIN properties

 Documentation/devicetree/bindings/media/rcar_vin.txt     | 16 +++++++++++++++-
 .../devicetree/bindings/media/video-interfaces.txt       |  2 ++
 arch/arm/boot/dts/r8a7790-lager.dts                      |  3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts                    |  3 ---
 arch/arm/boot/dts/r8a7791-porter.dts                     |  1 -
 arch/arm/boot/dts/r8a7793-gose.dts                       |  3 ---
 arch/arm/boot/dts/r8a7794-alt.dts                        |  1 -
 arch/arm/boot/dts/r8a7794-silk.dts                       |  1 -
 drivers/media/platform/rcar-vin/rcar-core.c              |  6 ++++++
 drivers/media/platform/rcar-vin/rcar-dma.c               | 11 +++++++++++
 drivers/media/platform/rcar-vin/rcar-vin.h               |  2 ++
 drivers/media/v4l2-core/v4l2-fwnode.c                    |  5 +++++
 include/media/v4l2-mediabus.h                            |  2 ++
 13 files changed, 43 insertions(+), 13 deletions(-)

--
2.7.4

[PATCH v3 0/8] media: rcar-vin: Brush endpoint properties

[Jacopo Mondi]

Hello,
   3rd version of VIN endpoint brushing series.

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

Quite some changes compared to v1/v2:
- First patch in the series changed to include Niklas' comments on using
  spaces for indent over tabs in documentation and as suggested by Rob and
  Laurent to refer to 'video-interfaces.txt' in property description.
- As suggested by Rob this series introduces a new property 'data-enable-active'
  to describe data enable signal polarity instead of using 'data-active' which
  refers to the data lanes polarity.
- Use this new property to control CLKENB pin polarity in VIN driver
- Introduce a new custom property to describe the 'use HSYNC as data-enable'
  function provided by VIN. In previous iterations I used the presence of
  'data-active' to enable/disable this functionality. That was confusing and
  not correct as it used the wrong property. As this is a VIN specificity, I
  thought a custom property is more suited.
- Last patch is still there and I know it is debated. My opinion is that it is
  still needed, as the presence of those un-documented and un-parsed properties
  confuses users which may expect changing those properties value to reflect on
  the video interface configuration, which does not happens instead.

Individual changelog per patch when relevant.

Thanks
    j

Jacopo Mondi (8):
  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: rcar-vin: Add 'hsync-as-de' custom prop
  media: rcar-vin: Handle 'hsync-as-de' property
  ARM: dts: rcar-gen2: Remove unused VIN properties

 Documentation/devicetree/bindings/media/rcar_vin.txt     | 16 +++++++++++++++-
 .../devicetree/bindings/media/video-interfaces.txt       |  2 ++
 arch/arm/boot/dts/r8a7790-lager.dts                      |  3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts                    |  3 ---
 arch/arm/boot/dts/r8a7791-porter.dts                     |  1 -
 arch/arm/boot/dts/r8a7793-gose.dts                       |  3 ---
 arch/arm/boot/dts/r8a7794-alt.dts                        |  1 -
 arch/arm/boot/dts/r8a7794-silk.dts                       |  1 -
 drivers/media/platform/rcar-vin/rcar-core.c              |  6 ++++++
 drivers/media/platform/rcar-vin/rcar-dma.c               | 11 +++++++++++
 drivers/media/platform/rcar-vin/rcar-vin.h               |  2 ++
 drivers/media/v4l2-core/v4l2-fwnode.c                    |  5 +++++
 include/media/v4l2-mediabus.h                            |  2 ++
 13 files changed, 43 insertions(+), 13 deletions(-)

--
2.7.4

[PATCH v3 1/8] 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>

---
v2 -> v3:
- Do not repeat property description, just reference video-interfaces.txt
- Indent with spaces, not tabs as the rest of the document
- Do not remove (yet) the 'bus-width' property from example
---
 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 5c6f2a7..4d91a36 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -54,6 +54,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.
@@ -63,6 +71,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 v3 1/8] 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>

---
v2 -> v3:
- Do not repeat property description, just reference video-interfaces.txt
- Indent with spaces, not tabs as the rest of the document
- Do not remove (yet) the 'bus-width' property from example
---
 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 5c6f2a7..4d91a36 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -54,6 +54,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.
@@ -63,6 +71,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 v3 1/8] 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>

---
v2 -> v3:
- Do not repeat property description, just reference video-interfaces.txt
- Indent with spaces, not tabs as the rest of the document
- Do not remove (yet) the 'bus-width' property from example
---
 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 5c6f2a7..4d91a36 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -54,6 +54,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.
@@ -63,6 +71,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

Re: [PATCH v3 1/8] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Tue, May 29, 2018 at 05:05:52PM +0200, Jacopo Mondi wrote:
> 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>
> 
> ---
> v2 -> v3:
> - Do not repeat property description, just reference video-interfaces.txt
> - Indent with spaces, not tabs as the rest of the document
> - Do not remove (yet) the 'bus-width' property from example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Rob Herring]

On Tue, May 29, 2018 at 05:05:52PM +0200, Jacopo Mondi wrote:
> 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>
> 
> ---
> v2 -> v3:
> - Do not repeat property description, just reference video-interfaces.txt
> - Indent with spaces, not tabs as the rest of the document
> - Do not remove (yet) the 'bus-width' property from example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 1/8] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Tue, May 29, 2018 at 05:05:52PM +0200, Jacopo Mondi wrote:
> 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>
> 
> ---
> v2 -> v3:
> - Do not repeat property description, just reference video-interfaces.txt
> - Indent with spaces, not tabs as the rest of the document
> - Do not remove (yet) the 'bus-width' property from example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 1/8] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Tue, May 29, 2018 at 05:05:52PM +0200, Jacopo Mondi wrote:
> 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>
> 
> ---
> v2 -> v3:
> - Do not repeat property description, just reference video-interfaces.txt
> - Indent with spaces, not tabs as the rest of the document
> - Do not remove (yet) the 'bus-width' property from example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v3 2/8] 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 lanes have to sampled for valid
pixel data.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- new patch
---
 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 v3 2/8] 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 lanes have to sampled for valid
pixel data.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- new patch
---
 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 v3 2/8] 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 lanes have to sampled for valid
pixel data.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- new patch
---
 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

Re: [PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[Rob Herring]

On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> 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 lanes have to sampled for valid
> pixel data.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[Rob Herring]

On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> 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 lanes have to sampled for valid
> pixel data.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[Rob Herring]

On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> 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 lanes have to sampled for valid
> pixel data.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[Sakari Ailus]

Hi Jacopo,

Thanks for the patch.

On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> 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 lanes have to sampled for valid
> pixel data.

Lanes or lines? I understand this is forthe parallel interface.

> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  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.

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

[PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[Sakari Ailus]

Hi Jacopo,

Thanks for the patch.

On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> 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 lanes have to sampled for valid
> pixel data.

Lanes or lines? I understand this is forthe parallel interface.

> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  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.

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

Re: [PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[Sakari Ailus]

Hi Jacopo,

Thanks for the patch.

On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> 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 lanes have to sampled for valid
> pixel data.

Lanes or lines? I understand this is forthe parallel interface.

> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  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.

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

Re: [PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[jacopo mondi]

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

Hi Sakari,

On Fri, Jun 01, 2018 at 01:29:10PM +0300, Sakari Ailus wrote:
> Hi Jacopo,
>
> Thanks for the patch.
>
> On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> > 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 lanes have to sampled for valid
> > pixel data.
>
> Lanes or lines? I understand this is forthe parallel interface.
>

Now I'm confused. Are the parallel data 'lines' and the CSI-2 one
'lanes'? I thought 'lanes' applies to both.. am I wrong?

Thanks
   j

> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > v3:
> > - new patch
> > ---
> >  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.
>
> --
> 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

[PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[jacopo mondi]

Hi Sakari,

On Fri, Jun 01, 2018 at 01:29:10PM +0300, Sakari Ailus wrote:
> Hi Jacopo,
>
> Thanks for the patch.
>
> On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> > 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 lanes have to sampled for valid
> > pixel data.
>
> Lanes or lines? I understand this is forthe parallel interface.
>

Now I'm confused. Are the parallel data 'lines' and the CSI-2 one
'lanes'? I thought 'lanes' applies to both.. am I wrong?

Thanks
   j

> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > v3:
> > - new patch
> > ---
> >  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.
>
> --
> 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/20180601/da16dd9b/attachment.sig>

Re: [PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[jacopo mondi]

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

Hi Sakari,

On Fri, Jun 01, 2018 at 01:29:10PM +0300, Sakari Ailus wrote:
> Hi Jacopo,
>
> Thanks for the patch.
>
> On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> > 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 lanes have to sampled for valid
> > pixel data.
>
> Lanes or lines? I understand this is forthe parallel interface.
>

Now I'm confused. Are the parallel data 'lines' and the CSI-2 one
'lanes'? I thought 'lanes' applies to both.. am I wrong?

Thanks
   j

> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > v3:
> > - new patch
> > ---
> >  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.
>
> --
> Sakari Ailus
> sakari.ailus@linux.intel.com

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

Re: [PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[Sakari Ailus]

On Fri, Jun 01, 2018 at 04:58:27PM +0200, jacopo mondi wrote:
> Hi Sakari,
> 
> On Fri, Jun 01, 2018 at 01:29:10PM +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > Thanks for the patch.
> >
> > On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> > > 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 lanes have to sampled for valid
> > > pixel data.
> >
> > Lanes or lines? I understand this is forthe parallel interface.
> >
> 
> Now I'm confused. Are the parallel data 'lines' and the CSI-2 one
> 'lanes'? I thought 'lanes' applies to both.. am I wrong?

"Lane" is conventionally refer to a differential pair of wires on a serial
bus such as CSI-2. "Line" is used of a wire on a parallel bus.

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

[PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[Sakari Ailus]

On Fri, Jun 01, 2018 at 04:58:27PM +0200, jacopo mondi wrote:
> Hi Sakari,
> 
> On Fri, Jun 01, 2018 at 01:29:10PM +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > Thanks for the patch.
> >
> > On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> > > 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 lanes have to sampled for valid
> > > pixel data.
> >
> > Lanes or lines? I understand this is forthe parallel interface.
> >
> 
> Now I'm confused. Are the parallel data 'lines' and the CSI-2 one
> 'lanes'? I thought 'lanes' applies to both.. am I wrong?

"Lane" is conventionally refer to a differential pair of wires on a serial
bus such as CSI-2. "Line" is used of a wire on a parallel bus.

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

Re: [PATCH v3 2/8] dt-bindings: media: Document data-enable-active property

[Sakari Ailus]

On Fri, Jun 01, 2018 at 04:58:27PM +0200, jacopo mondi wrote:
> Hi Sakari,
> 
> On Fri, Jun 01, 2018 at 01:29:10PM +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > Thanks for the patch.
> >
> > On Tue, May 29, 2018 at 05:05:53PM +0200, Jacopo Mondi wrote:
> > > 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 lanes have to sampled for valid
> > > pixel data.
> >
> > Lanes or lines? I understand this is forthe parallel interface.
> >
> 
> Now I'm confused. Are the parallel data 'lines' and the CSI-2 one
> 'lanes'? I thought 'lanes' applies to both.. am I wrong?

"Lane" is conventionally refer to a differential pair of wires on a serial
bus such as CSI-2. "Line" is used of a wire on a parallel bus.

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

[PATCH v3 3/8] 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>

---
v3:
- new patch
---
 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 v3 3/8] 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>

---
v3:
- new patch
---
 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 v3 3/8] 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>

---
v3:
- new patch
---
 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

Re: [PATCH v3 3/8] media: v4l2-fwnode: parse 'data-enable-active' prop

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 17:05:54 +0200, Jacopo Mondi wrote:
> 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>

> 
> ---
> v3:
> - new patch
> ---
>  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
> 

-- 
Regards,
Niklas Söderlund

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

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 17:05:54 +0200, Jacopo Mondi wrote:
> 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>

> 
> ---
> v3:
> - new patch
> ---
>  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
> 

-- 
Regards,
Niklas S?derlund

Re: [PATCH v3 3/8] media: v4l2-fwnode: parse 'data-enable-active' prop

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 17:05:54 +0200, Jacopo Mondi wrote:
> 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>

> 
> ---
> v3:
> - new patch
> ---
>  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
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH v3 3/8] media: v4l2-fwnode: parse 'data-enable-active' prop

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 17:05:54 +0200, Jacopo Mondi wrote:
> 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>

> 
> ---
> v3:
> - new patch
> ---
>  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
> 

-- 
Regards,
Niklas Söderlund

[PATCH v3 4/8] 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>
---
v3:
- new patch
---

 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 4d91a36..ff53226 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -58,6 +58,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 v3 4/8] 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>
---
v3:
- new patch
---

 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 4d91a36..ff53226 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -58,6 +58,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       - 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.
+        - 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 v3 4/8] 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>
---
v3:
- new patch
---

 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 4d91a36..ff53226 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -58,6 +58,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

Re: [PATCH v3 4/8] dt-bindings: media: rcar-vin: Add 'data-enable-active'

[Rob Herring]

On Tue, May 29, 2018 at 05:05:55PM +0200, Jacopo Mondi wrote:
> Describe optional endpoint property 'data-enable-active' for R-Car VIN.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
> 
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Rob Herring]

On Tue, May 29, 2018 at 05:05:55PM +0200, Jacopo Mondi wrote:
> Describe optional endpoint property 'data-enable-active' for R-Car VIN.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
> 
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 4/8] dt-bindings: media: rcar-vin: Add 'data-enable-active'

[Rob Herring]

On Tue, May 29, 2018 at 05:05:55PM +0200, Jacopo Mondi wrote:
> Describe optional endpoint property 'data-enable-active' for R-Car VIN.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
> 
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 4/8] dt-bindings: media: rcar-vin: Add 'data-enable-active'

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 17:05:55 +0200, Jacopo Mondi wrote:
> Describe optional endpoint property 'data-enable-active' for R-Car VIN.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v3:
> - new patch
> ---
> 
>  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 4d91a36..ff53226 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -58,6 +58,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
> 

-- 
Regards,
Niklas Söderlund

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

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 17:05:55 +0200, Jacopo Mondi wrote:
> Describe optional endpoint property 'data-enable-active' for R-Car VIN.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v3:
> - new patch
> ---
> 
>  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 4d91a36..ff53226 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -58,6 +58,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>        - 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.
> +        - 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
> 

-- 
Regards,
Niklas S?derlund

Re: [PATCH v3 4/8] dt-bindings: media: rcar-vin: Add 'data-enable-active'

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 17:05:55 +0200, Jacopo Mondi wrote:
> Describe optional endpoint property 'data-enable-active' for R-Car VIN.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v3:
> - new patch
> ---
> 
>  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 4d91a36..ff53226 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -58,6 +58,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
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH v3 4/8] dt-bindings: media: rcar-vin: Add 'data-enable-active'

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 17:05:55 +0200, Jacopo Mondi wrote:
> Describe optional endpoint property 'data-enable-active' for R-Car VIN.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v3:
> - new patch
> ---
> 
>  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 4d91a36..ff53226 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -58,6 +58,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
> 

-- 
Regards,
Niklas Söderlund

[PATCH v3 5/8] 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>
---
v3:
- use new property to set the CES bit
---
 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 v3 5/8] 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>
---
v3:
- use new property to set the CES bit
---
 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 v3 5/8] 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>
---
v3:
- use new property to set the CES bit
---
 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

Re: [PATCH v3 5/8] media: rcar-vin: Handle data-enable polarity

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:56 +0200, Jacopo Mondi wrote:
> 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>

> ---
> v3:
> - use new property to set the CES bit
> ---
>  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
> 

-- 
Regards,
Niklas Söderlund

[PATCH v3 5/8] media: rcar-vin: Handle data-enable polarity

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:56 +0200, Jacopo Mondi wrote:
> 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>

> ---
> v3:
> - use new property to set the CES bit
> ---
>  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
> 

-- 
Regards,
Niklas S?derlund

Re: [PATCH v3 5/8] media: rcar-vin: Handle data-enable polarity

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:56 +0200, Jacopo Mondi wrote:
> 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>

> ---
> v3:
> - use new property to set the CES bit
> ---
>  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
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH v3 5/8] media: rcar-vin: Handle data-enable polarity

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:56 +0200, Jacopo Mondi wrote:
> 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>

> ---
> v3:
> - use new property to set the CES bit
> ---
>  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
> 

-- 
Regards,
Niklas Söderlund

[PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Jacopo Mondi]

Document the boolean custom property 'renesas,hsync-as-de' that indicates
that the HSYNC signal is internally used as data-enable, when the
CLKENB signal is not connected.

As this is a VIN specificity create a custom property specific to the R-Car
VIN driver.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- new patch
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index ff53226..024c109 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
         - vsync-active: see [1] for description. Default is active high.
         - data-enable-active: polarity of CLKENB signal, see [1] for
           description. Default is active high.
+        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
+          is internally used as data-enable when the CLKENB signal is
+          not available.

         If both HSYNC and VSYNC polarities are not specified, embedded
         synchronization is selected.
--
2.7.4

[PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Jacopo Mondi]

Document the boolean custom property 'renesas,hsync-as-de' that indicates
that the HSYNC signal is internally used as data-enable, when the
CLKENB signal is not connected.

As this is a VIN specificity create a custom property specific to the R-Car
VIN driver.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- new patch
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index ff53226..024c109 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
         - vsync-active: see [1] for description. Default is active high.
         - data-enable-active: polarity of CLKENB signal, see [1] for
           description. Default is active high.
+        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
+          is internally used as data-enable when the CLKENB signal is
+          not available.

         If both HSYNC and VSYNC polarities are not specified, embedded
         synchronization is selected.
--
2.7.4

[PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Jacopo Mondi]

Document the boolean custom property 'renesas,hsync-as-de' that indicates
that the HSYNC signal is internally used as data-enable, when the
CLKENB signal is not connected.

As this is a VIN specificity create a custom property specific to the R-Car
VIN driver.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- new patch
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index ff53226..024c109 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
         - vsync-active: see [1] for description. Default is active high.
         - data-enable-active: polarity of CLKENB signal, see [1] for
           description. Default is active high.
+        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
+          is internally used as data-enable when the CLKENB signal is
+          not available.

         If both HSYNC and VSYNC polarities are not specified, embedded
         synchronization is selected.
--
2.7.4

Re: [PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Rob Herring]

On Tue, May 29, 2018 at 05:05:57PM +0200, Jacopo Mondi wrote:
> Document the boolean custom property 'renesas,hsync-as-de' that indicates
> that the HSYNC signal is internally used as data-enable, when the
> CLKENB signal is not connected.
> 
> As this is a VIN specificity create a custom property specific to the R-Car
> VIN driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Rob Herring]

On Tue, May 29, 2018 at 05:05:57PM +0200, Jacopo Mondi wrote:
> Document the boolean custom property 'renesas,hsync-as-de' that indicates
> that the HSYNC signal is internally used as data-enable, when the
> CLKENB signal is not connected.
> 
> As this is a VIN specificity create a custom property specific to the R-Car
> VIN driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Rob Herring]

On Tue, May 29, 2018 at 05:05:57PM +0200, Jacopo Mondi wrote:
> Document the boolean custom property 'renesas,hsync-as-de' that indicates
> that the HSYNC signal is internally used as data-enable, when the
> CLKENB signal is not connected.
> 
> As this is a VIN specificity create a custom property specific to the R-Car
> VIN driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:57 +0200, Jacopo Mondi wrote:
> Document the boolean custom property 'renesas,hsync-as-de' that indicates
> that the HSYNC signal is internally used as data-enable, when the
> CLKENB signal is not connected.
> 
> As this is a VIN specificity create a custom property specific to the R-Car
> VIN driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index ff53226..024c109 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - vsync-active: see [1] for description. Default is active high.
>          - data-enable-active: polarity of CLKENB signal, see [1] for
>            description. Default is active high.
> +        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
> +          is internally used as data-enable when the CLKENB signal is
> +          not available.

I'm not sure I like this, is there really a need to add a custom 
property for this? The datasheet states that when the CLKENB pin is not 
connected the driver should enable 'Clock Enable Hsync Select (CHS)'.  
With the new generic property 'data-enable-active' which describes the 
polarity of the CLKENB pin we also gain the knowledge if the CLKENB pin 
is connected or not.

I propose we drop this custom property and instead let the driver check 
if the CLKENB polarity is described or not and use that to determine if 
CHS bit should be set or not. IMHO that is much simpler then having two 
properties describing the same pin.

> 
>          If both HSYNC and VSYNC polarities are not specified, embedded
>          synchronization is selected.
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

[PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:57 +0200, Jacopo Mondi wrote:
> Document the boolean custom property 'renesas,hsync-as-de' that indicates
> that the HSYNC signal is internally used as data-enable, when the
> CLKENB signal is not connected.
> 
> As this is a VIN specificity create a custom property specific to the R-Car
> VIN driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index ff53226..024c109 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - vsync-active: see [1] for description. Default is active high.
>          - data-enable-active: polarity of CLKENB signal, see [1] for
>            description. Default is active high.
> +        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
> +          is internally used as data-enable when the CLKENB signal is
> +          not available.

I'm not sure I like this, is there really a need to add a custom 
property for this? The datasheet states that when the CLKENB pin is not 
connected the driver should enable 'Clock Enable Hsync Select (CHS)'.  
With the new generic property 'data-enable-active' which describes the 
polarity of the CLKENB pin we also gain the knowledge if the CLKENB pin 
is connected or not.

I propose we drop this custom property and instead let the driver check 
if the CLKENB polarity is described or not and use that to determine if 
CHS bit should be set or not. IMHO that is much simpler then having two 
properties describing the same pin.

> 
>          If both HSYNC and VSYNC polarities are not specified, embedded
>          synchronization is selected.
> --
> 2.7.4
> 

-- 
Regards,
Niklas S?derlund

Re: [PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:57 +0200, Jacopo Mondi wrote:
> Document the boolean custom property 'renesas,hsync-as-de' that indicates
> that the HSYNC signal is internally used as data-enable, when the
> CLKENB signal is not connected.
> 
> As this is a VIN specificity create a custom property specific to the R-Car
> VIN driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index ff53226..024c109 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - vsync-active: see [1] for description. Default is active high.
>          - data-enable-active: polarity of CLKENB signal, see [1] for
>            description. Default is active high.
> +        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
> +          is internally used as data-enable when the CLKENB signal is
> +          not available.

I'm not sure I like this, is there really a need to add a custom 
property for this? The datasheet states that when the CLKENB pin is not 
connected the driver should enable 'Clock Enable Hsync Select (CHS)'.  
With the new generic property 'data-enable-active' which describes the 
polarity of the CLKENB pin we also gain the knowledge if the CLKENB pin 
is connected or not.

I propose we drop this custom property and instead let the driver check 
if the CLKENB polarity is described or not and use that to determine if 
CHS bit should be set or not. IMHO that is much simpler then having two 
properties describing the same pin.

> 
>          If both HSYNC and VSYNC polarities are not specified, embedded
>          synchronization is selected.
> --
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:57 +0200, Jacopo Mondi wrote:
> Document the boolean custom property 'renesas,hsync-as-de' that indicates
> that the HSYNC signal is internally used as data-enable, when the
> CLKENB signal is not connected.
> 
> As this is a VIN specificity create a custom property specific to the R-Car
> VIN driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index ff53226..024c109 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - vsync-active: see [1] for description. Default is active high.
>          - data-enable-active: polarity of CLKENB signal, see [1] for
>            description. Default is active high.
> +        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
> +          is internally used as data-enable when the CLKENB signal is
> +          not available.

I'm not sure I like this, is there really a need to add a custom 
property for this? The datasheet states that when the CLKENB pin is not 
connected the driver should enable 'Clock Enable Hsync Select (CHS)'.  
With the new generic property 'data-enable-active' which describes the 
polarity of the CLKENB pin we also gain the knowledge if the CLKENB pin 
is connected or not.

I propose we drop this custom property and instead let the driver check 
if the CLKENB polarity is described or not and use that to determine if 
CHS bit should be set or not. IMHO that is much simpler then having two 
properties describing the same pin.

> 
>          If both HSYNC and VSYNC polarities are not specified, embedded
>          synchronization is selected.
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[jacopo mondi]

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

Hi Niklas,

On Mon, Jun 04, 2018 at 02:19:33PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-29 17:05:57 +0200, Jacopo Mondi wrote:
> > Document the boolean custom property 'renesas,hsync-as-de' that indicates
> > that the HSYNC signal is internally used as data-enable, when the
> > CLKENB signal is not connected.
> >
> > As this is a VIN specificity create a custom property specific to the R-Car
> > VIN driver.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > v3:
> > - new patch
> > ---
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > index ff53226..024c109 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> >          - vsync-active: see [1] for description. Default is active high.
> >          - data-enable-active: polarity of CLKENB signal, see [1] for
> >            description. Default is active high.
> > +        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
> > +          is internally used as data-enable when the CLKENB signal is
> > +          not available.
>
> I'm not sure I like this, is there really a need to add a custom
> property for this? The datasheet states that when the CLKENB pin is not
> connected the driver should enable 'Clock Enable Hsync Select (CHS)'.
> With the new generic property 'data-enable-active' which describes the
> polarity of the CLKENB pin we also gain the knowledge if the CLKENB pin
> is connected or not.

That was my first proposal, we discussed that in
Re: [PATCH 3/6] media: rcar-vin: Handle data-active property
Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

Let's sum it up in this way:
Instead of having to deal again with the "what happens if there is no
data-enable-active and we're running on BT.656 where there is no HSYNC signal"[1]
I decided to go with a custom property.

>
> I propose we drop this custom property and instead let the driver check
> if the CLKENB polarity is described or not and use that to determine if
> CHS bit should be set or not. IMHO that is much simpler then having two
> properties describing the same pin.
>

It is my understanding that both Gen2 and Gen3 boards CVBS input are
BT.656 and none of them has CLKENB input. So 'data-enable-active' is
never there but in this case we should not set CHS. So the absence of
'data-enable-active' has different consequences if we're running on
parallel or BT.656 bus, and that feels confusing to me, so I decided
to make it an explicit property.

Also, as the interface manual describes the "use HSYNC in place of CLKENB"
when on parallel bus as a design choice, that should imo be documented.

Again, this is very minor stuff. I'll leave this out from next
version, maybe we can talk about this f2f next week.

Thanks
  j

> >
> >          If both HSYNC and VSYNC polarities are not specified, embedded
> >          synchronization is selected.
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

[-- Attachment #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

[PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[jacopo mondi]

Hi Niklas,

On Mon, Jun 04, 2018 at 02:19:33PM +0200, Niklas S?derlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-29 17:05:57 +0200, Jacopo Mondi wrote:
> > Document the boolean custom property 'renesas,hsync-as-de' that indicates
> > that the HSYNC signal is internally used as data-enable, when the
> > CLKENB signal is not connected.
> >
> > As this is a VIN specificity create a custom property specific to the R-Car
> > VIN driver.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > v3:
> > - new patch
> > ---
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > index ff53226..024c109 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> >          - vsync-active: see [1] for description. Default is active high.
> >          - data-enable-active: polarity of CLKENB signal, see [1] for
> >            description. Default is active high.
> > +        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
> > +          is internally used as data-enable when the CLKENB signal is
> > +          not available.
>
> I'm not sure I like this, is there really a need to add a custom
> property for this? The datasheet states that when the CLKENB pin is not
> connected the driver should enable 'Clock Enable Hsync Select (CHS)'.
> With the new generic property 'data-enable-active' which describes the
> polarity of the CLKENB pin we also gain the knowledge if the CLKENB pin
> is connected or not.

That was my first proposal, we discussed that in
Re: [PATCH 3/6] media: rcar-vin: Handle data-active property
Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

Let's sum it up in this way:
Instead of having to deal again with the "what happens if there is no
data-enable-active and we're running on BT.656 where there is no HSYNC signal"[1]
I decided to go with a custom property.

>
> I propose we drop this custom property and instead let the driver check
> if the CLKENB polarity is described or not and use that to determine if
> CHS bit should be set or not. IMHO that is much simpler then having two
> properties describing the same pin.
>

It is my understanding that both Gen2 and Gen3 boards CVBS input are
BT.656 and none of them has CLKENB input. So 'data-enable-active' is
never there but in this case we should not set CHS. So the absence of
'data-enable-active' has different consequences if we're running on
parallel or BT.656 bus, and that feels confusing to me, so I decided
to make it an explicit property.

Also, as the interface manual describes the "use HSYNC in place of CLKENB"
when on parallel bus as a design choice, that should imo be documented.

Again, this is very minor stuff. I'll leave this out from next
version, maybe we can talk about this f2f next week.

Thanks
  j

> >
> >          If both HSYNC and VSYNC polarities are not specified, embedded
> >          synchronization is selected.
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas S?derlund
-------------- 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/20180612/d1d7273b/attachment.sig>

Re: [PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop

[jacopo mondi]

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

Hi Niklas,

On Mon, Jun 04, 2018 at 02:19:33PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-29 17:05:57 +0200, Jacopo Mondi wrote:
> > Document the boolean custom property 'renesas,hsync-as-de' that indicates
> > that the HSYNC signal is internally used as data-enable, when the
> > CLKENB signal is not connected.
> >
> > As this is a VIN specificity create a custom property specific to the R-Car
> > VIN driver.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > v3:
> > - new patch
> > ---
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > index ff53226..024c109 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> >          - vsync-active: see [1] for description. Default is active high.
> >          - data-enable-active: polarity of CLKENB signal, see [1] for
> >            description. Default is active high.
> > +        - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
> > +          is internally used as data-enable when the CLKENB signal is
> > +          not available.
>
> I'm not sure I like this, is there really a need to add a custom
> property for this? The datasheet states that when the CLKENB pin is not
> connected the driver should enable 'Clock Enable Hsync Select (CHS)'.
> With the new generic property 'data-enable-active' which describes the
> polarity of the CLKENB pin we also gain the knowledge if the CLKENB pin
> is connected or not.

That was my first proposal, we discussed that in
Re: [PATCH 3/6] media: rcar-vin: Handle data-active property
Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

Let's sum it up in this way:
Instead of having to deal again with the "what happens if there is no
data-enable-active and we're running on BT.656 where there is no HSYNC signal"[1]
I decided to go with a custom property.

>
> I propose we drop this custom property and instead let the driver check
> if the CLKENB polarity is described or not and use that to determine if
> CHS bit should be set or not. IMHO that is much simpler then having two
> properties describing the same pin.
>

It is my understanding that both Gen2 and Gen3 boards CVBS input are
BT.656 and none of them has CLKENB input. So 'data-enable-active' is
never there but in this case we should not set CHS. So the absence of
'data-enable-active' has different consequences if we're running on
parallel or BT.656 bus, and that feels confusing to me, so I decided
to make it an explicit property.

Also, as the interface manual describes the "use HSYNC in place of CLKENB"
when on parallel bus as a design choice, that should imo be documented.

Again, this is very minor stuff. I'll leave this out from next
version, maybe we can talk about this f2f next week.

Thanks
  j

> >
> >          If both HSYNC and VSYNC polarities are not specified, embedded
> >          synchronization is selected.
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

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

[PATCH v3 7/8] media: rcar-vin: Handle 'hsync-as-de' property

[Jacopo Mondi]

Parse and handle 'hsync-as-de' custom property and set the CHS flag during
the VIN interface setup.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- new patch
- use the new custom property to set the CHS bit
---
 drivers/media/platform/rcar-vin/rcar-core.c | 6 ++++++
 drivers/media/platform/rcar-vin/rcar-dma.c  | 6 ++++++
 drivers/media/platform/rcar-vin/rcar-vin.h  | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 3062171..71710b8 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -589,6 +589,7 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
 	struct rvin_dev *vin = dev_get_drvdata(dev);
 	struct rvin_parallel_entity *rvpe =
 		container_of(asd, struct rvin_parallel_entity, asd);
+	const struct fwnode_handle *fwnode = vep->base.local_fwnode;

 	if (vep->base.port || vep->base.id)
 		return -ENOTCONN;
@@ -610,6 +611,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
 		return -EINVAL;
 	}

+	if (fwnode_property_read_bool(fwnode, "renesas,hsync-as-de"))
+		vin->parallel->chs = true;
+	else
+		vin->parallel->chs = false;
+
 	return 0;
 }

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 9145b56..01d0737 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -124,6 +124,7 @@
 #define VNDMR2_VPS		(1 << 30)
 #define VNDMR2_HPS		(1 << 29)
 #define VNDMR2_CES		(1 << 28)
+#define VNDMR2_CHS		(1 << 23)
 #define VNDMR2_FTEV		(1 << 17)
 #define VNDMR2_VLV(n)		((n & 0xf) << 12)

@@ -703,6 +704,11 @@ static int rvin_setup(struct rvin_dev *vin)
 		/* Data Enable Polarity Select */
 		if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
 			dmr2 |= VNDMR2_CES;
+
+		/* Use HSYNC as data-enable signal */
+		if (vin->parallel->mbus_type == V4L2_MBUS_PARALLEL &&
+		    vin->parallel->chs)
+			dmr2 |= VNDMR2_CHS;
 	}

 	/*
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 8bc3704..846f978 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -78,6 +78,7 @@ struct rvin_video_format {
  * @subdev:	subdevice matched using async framework
  * @mbus_type:	media bus type
  * @mbus_flags:	media bus configuration flags
+ * @chs:	use HSYNC as data-enable flag
  * @source_pad:	source pad of remote subdevice
  * @sink_pad:	sink pad of remote subdevice
  *
@@ -88,6 +89,7 @@ struct rvin_parallel_entity {

 	enum v4l2_mbus_type mbus_type;
 	unsigned int mbus_flags;
+	bool chs;

 	unsigned int source_pad;
 	unsigned int sink_pad;
--
2.7.4

[PATCH v3 7/8] media: rcar-vin: Handle 'hsync-as-de' property

[Jacopo Mondi]

Parse and handle 'hsync-as-de' custom property and set the CHS flag during
the VIN interface setup.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- new patch
- use the new custom property to set the CHS bit
---
 drivers/media/platform/rcar-vin/rcar-core.c | 6 ++++++
 drivers/media/platform/rcar-vin/rcar-dma.c  | 6 ++++++
 drivers/media/platform/rcar-vin/rcar-vin.h  | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 3062171..71710b8 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -589,6 +589,7 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
 	struct rvin_dev *vin = dev_get_drvdata(dev);
 	struct rvin_parallel_entity *rvpe =
 		container_of(asd, struct rvin_parallel_entity, asd);
+	const struct fwnode_handle *fwnode = vep->base.local_fwnode;

 	if (vep->base.port || vep->base.id)
 		return -ENOTCONN;
@@ -610,6 +611,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
 		return -EINVAL;
 	}

+	if (fwnode_property_read_bool(fwnode, "renesas,hsync-as-de"))
+		vin->parallel->chs = true;
+	else
+		vin->parallel->chs = false;
+
 	return 0;
 }

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 9145b56..01d0737 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -124,6 +124,7 @@
 #define VNDMR2_VPS		(1 << 30)
 #define VNDMR2_HPS		(1 << 29)
 #define VNDMR2_CES		(1 << 28)
+#define VNDMR2_CHS		(1 << 23)
 #define VNDMR2_FTEV		(1 << 17)
 #define VNDMR2_VLV(n)		((n & 0xf) << 12)

@@ -703,6 +704,11 @@ static int rvin_setup(struct rvin_dev *vin)
 		/* Data Enable Polarity Select */
 		if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
 			dmr2 |= VNDMR2_CES;
+
+		/* Use HSYNC as data-enable signal */
+		if (vin->parallel->mbus_type == V4L2_MBUS_PARALLEL &&
+		    vin->parallel->chs)
+			dmr2 |= VNDMR2_CHS;
 	}

 	/*
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 8bc3704..846f978 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -78,6 +78,7 @@ struct rvin_video_format {
  * @subdev:	subdevice matched using async framework
  * @mbus_type:	media bus type
  * @mbus_flags:	media bus configuration flags
+ * @chs:	use HSYNC as data-enable flag
  * @source_pad:	source pad of remote subdevice
  * @sink_pad:	sink pad of remote subdevice
  *
@@ -88,6 +89,7 @@ struct rvin_parallel_entity {

 	enum v4l2_mbus_type mbus_type;
 	unsigned int mbus_flags;
+	bool chs;

 	unsigned int source_pad;
 	unsigned int sink_pad;
--
2.7.4

[PATCH v3 7/8] media: rcar-vin: Handle 'hsync-as-de' property

[Jacopo Mondi]

Parse and handle 'hsync-as-de' custom property and set the CHS flag during
the VIN interface setup.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- new patch
- use the new custom property to set the CHS bit
---
 drivers/media/platform/rcar-vin/rcar-core.c | 6 ++++++
 drivers/media/platform/rcar-vin/rcar-dma.c  | 6 ++++++
 drivers/media/platform/rcar-vin/rcar-vin.h  | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 3062171..71710b8 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -589,6 +589,7 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
 	struct rvin_dev *vin = dev_get_drvdata(dev);
 	struct rvin_parallel_entity *rvpe =
 		container_of(asd, struct rvin_parallel_entity, asd);
+	const struct fwnode_handle *fwnode = vep->base.local_fwnode;

 	if (vep->base.port || vep->base.id)
 		return -ENOTCONN;
@@ -610,6 +611,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
 		return -EINVAL;
 	}

+	if (fwnode_property_read_bool(fwnode, "renesas,hsync-as-de"))
+		vin->parallel->chs = true;
+	else
+		vin->parallel->chs = false;
+
 	return 0;
 }

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 9145b56..01d0737 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -124,6 +124,7 @@
 #define VNDMR2_VPS		(1 << 30)
 #define VNDMR2_HPS		(1 << 29)
 #define VNDMR2_CES		(1 << 28)
+#define VNDMR2_CHS		(1 << 23)
 #define VNDMR2_FTEV		(1 << 17)
 #define VNDMR2_VLV(n)		((n & 0xf) << 12)

@@ -703,6 +704,11 @@ static int rvin_setup(struct rvin_dev *vin)
 		/* Data Enable Polarity Select */
 		if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
 			dmr2 |= VNDMR2_CES;
+
+		/* Use HSYNC as data-enable signal */
+		if (vin->parallel->mbus_type == V4L2_MBUS_PARALLEL &&
+		    vin->parallel->chs)
+			dmr2 |= VNDMR2_CHS;
 	}

 	/*
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 8bc3704..846f978 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -78,6 +78,7 @@ struct rvin_video_format {
  * @subdev:	subdevice matched using async framework
  * @mbus_type:	media bus type
  * @mbus_flags:	media bus configuration flags
+ * @chs:	use HSYNC as data-enable flag
  * @source_pad:	source pad of remote subdevice
  * @sink_pad:	sink pad of remote subdevice
  *
@@ -88,6 +89,7 @@ struct rvin_parallel_entity {

 	enum v4l2_mbus_type mbus_type;
 	unsigned int mbus_flags;
+	bool chs;

 	unsigned int source_pad;
 	unsigned int sink_pad;
--
2.7.4

[PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Jacopo Mondi]

The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
driver and only confuse users. Remove them in all Gen2 SoC that use
them.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- remove bus-width from dt-bindings example
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
 arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
 arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
 arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
 arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
 arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
 7 files changed, 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 024c109..c6d7f60 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -128,7 +128,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)

                 vin1ep0: endpoint {
                         remote-endpoint = <&adv7180>;
-                        bus-width = <8>;
                 };
         };
 };
diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 092610e..9cdabfcf 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -885,10 +885,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -904,7 +902,6 @@
 	port {
 		vin1ep0: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index 8ab793d..033c9e3 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -857,10 +857,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -875,7 +873,6 @@
 	port {
 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index a01101b..c16e870 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -388,7 +388,6 @@
 	port {
 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
index aa209f6..60aaddb 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -765,10 +765,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -784,7 +782,6 @@
 	port {
 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180_out>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
index e170275..8ed7a71 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -388,7 +388,6 @@
 	port {
 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index 7808aae..6adfcd6 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -477,7 +477,6 @@
 	port {
 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
--
2.7.4

[PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Jacopo Mondi]

The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
driver and only confuse users. Remove them in all Gen2 SoC that use
them.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- remove bus-width from dt-bindings example
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
 arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
 arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
 arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
 arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
 arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
 7 files changed, 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 024c109..c6d7f60 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -128,7 +128,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)

                 vin1ep0: endpoint {
                         remote-endpoint = <&adv7180>;
-                        bus-width = <8>;
                 };
         };
 };
diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 092610e..9cdabfcf 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -885,10 +885,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -904,7 +902,6 @@
 	port {
 		vin1ep0: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index 8ab793d..033c9e3 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -857,10 +857,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -875,7 +873,6 @@
 	port {
 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index a01101b..c16e870 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -388,7 +388,6 @@
 	port {
 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
index aa209f6..60aaddb 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -765,10 +765,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -784,7 +782,6 @@
 	port {
 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180_out>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
index e170275..8ed7a71 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -388,7 +388,6 @@
 	port {
 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index 7808aae..6adfcd6 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -477,7 +477,6 @@
 	port {
 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
--
2.7.4

[PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Jacopo Mondi]

The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
driver and only confuse users. Remove them in all Gen2 SoC that use
them.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
v3:
- remove bus-width from dt-bindings example
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
 arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
 arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
 arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
 arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
 arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
 7 files changed, 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 024c109..c6d7f60 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -128,7 +128,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)

                 vin1ep0: endpoint {
                         remote-endpoint = <&adv7180>;
-                        bus-width = <8>;
                 };
         };
 };
diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 092610e..9cdabfcf 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -885,10 +885,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -904,7 +902,6 @@
 	port {
 		vin1ep0: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index 8ab793d..033c9e3 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -857,10 +857,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -875,7 +873,6 @@
 	port {
 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index a01101b..c16e870 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -388,7 +388,6 @@
 	port {
 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
index aa209f6..60aaddb 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -765,10 +765,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -784,7 +782,6 @@
 	port {
 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180_out>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
index e170275..8ed7a71 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -388,7 +388,6 @@
 	port {
 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index 7808aae..6adfcd6 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -477,7 +477,6 @@
 	port {
 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
--
2.7.4

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Rob Herring]

On Tue, May 29, 2018 at 05:05:59PM +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that use
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - remove bus-width from dt-bindings example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
>  arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
>  7 files changed, 13 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Rob Herring]

On Tue, May 29, 2018 at 05:05:59PM +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that use
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - remove bus-width from dt-bindings example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
>  arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
>  7 files changed, 13 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Rob Herring]

On Tue, May 29, 2018 at 05:05:59PM +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that use
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - remove bus-width from dt-bindings example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
>  arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
>  7 files changed, 13 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that use
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

The more I think about this the more I lean towards that this patch 
should be dropped. The properties accurately describes the hardware and 
I think there is value in that. That the driver currently don't parse or 
make use of them don't in my view reduce there value. Maybe you should 
break out this patch to a separate series?

> ---
> v3:
> - remove bus-width from dt-bindings example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
>  arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
>  7 files changed, 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 024c109..c6d7f60 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -128,7 +128,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 092610e..9cdabfcf 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -885,10 +885,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -904,7 +902,6 @@
>  	port {
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 8ab793d..033c9e3 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -857,10 +857,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -875,7 +873,6 @@
>  	port {
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index a01101b..c16e870 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -388,7 +388,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index aa209f6..60aaddb 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -765,10 +765,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -784,7 +782,6 @@
>  	port {
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index e170275..8ed7a71 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -388,7 +388,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index 7808aae..6adfcd6 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -477,7 +477,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

[PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that use
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

The more I think about this the more I lean towards that this patch 
should be dropped. The properties accurately describes the hardware and 
I think there is value in that. That the driver currently don't parse or 
make use of them don't in my view reduce there value. Maybe you should 
break out this patch to a separate series?

> ---
> v3:
> - remove bus-width from dt-bindings example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
>  arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
>  7 files changed, 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 024c109..c6d7f60 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -128,7 +128,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 092610e..9cdabfcf 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -885,10 +885,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -904,7 +902,6 @@
>  	port {
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 8ab793d..033c9e3 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -857,10 +857,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -875,7 +873,6 @@
>  	port {
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index a01101b..c16e870 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -388,7 +388,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index aa209f6..60aaddb 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -765,10 +765,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -784,7 +782,6 @@
>  	port {
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index e170275..8ed7a71 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -388,7 +388,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index 7808aae..6adfcd6 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -477,7 +477,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas S?derlund

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that use
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

The more I think about this the more I lean towards that this patch 
should be dropped. The properties accurately describes the hardware and 
I think there is value in that. That the driver currently don't parse or 
make use of them don't in my view reduce there value. Maybe you should 
break out this patch to a separate series?

> ---
> v3:
> - remove bus-width from dt-bindings example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
>  arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
>  7 files changed, 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 024c109..c6d7f60 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -128,7 +128,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 092610e..9cdabfcf 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -885,10 +885,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -904,7 +902,6 @@
>  	port {
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 8ab793d..033c9e3 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -857,10 +857,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -875,7 +873,6 @@
>  	port {
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index a01101b..c16e870 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -388,7 +388,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index aa209f6..60aaddb 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -765,10 +765,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -784,7 +782,6 @@
>  	port {
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index e170275..8ed7a71 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -388,7 +388,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index 7808aae..6adfcd6 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -477,7 +477,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that use
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

The more I think about this the more I lean towards that this patch 
should be dropped. The properties accurately describes the hardware and 
I think there is value in that. That the driver currently don't parse or 
make use of them don't in my view reduce there value. Maybe you should 
break out this patch to a separate series?

> ---
> v3:
> - remove bus-width from dt-bindings example
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
>  arch/arm/boot/dts/r8a7790-lager.dts                  | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts                | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts                 | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts                   | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts                    | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts                   | 1 -
>  7 files changed, 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 024c109..c6d7f60 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -128,7 +128,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 092610e..9cdabfcf 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -885,10 +885,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -904,7 +902,6 @@
>  	port {
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 8ab793d..033c9e3 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -857,10 +857,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -875,7 +873,6 @@
>  	port {
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index a01101b..c16e870 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -388,7 +388,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index aa209f6..60aaddb 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -765,10 +765,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -784,7 +782,6 @@
>  	port {
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index e170275..8ed7a71 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -388,7 +388,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index 7808aae..6adfcd6 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -477,7 +477,6 @@
>  	port {
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Simon Horman]

On Mon, Jun 04, 2018 at 02:23:25PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that use
> > them.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> The more I think about this the more I lean towards that this patch 
> should be dropped. The properties accurately describes the hardware and 
> I think there is value in that. That the driver currently don't parse or 
> make use of them don't in my view reduce there value. Maybe you should 
> break out this patch to a separate series?

I also think there is value in describing the hardware not the state of the
driver at this time.  Is there any missmatch between these properties and
the bindings?

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

[PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Simon Horman]

On Mon, Jun 04, 2018 at 02:23:25PM +0200, Niklas S?derlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that use
> > them.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> The more I think about this the more I lean towards that this patch 
> should be dropped. The properties accurately describes the hardware and 
> I think there is value in that. That the driver currently don't parse or 
> make use of them don't in my view reduce there value. Maybe you should 
> break out this patch to a separate series?

I also think there is value in describing the hardware not the state of the
driver at this time.  Is there any missmatch between these properties and
the bindings?

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Simon Horman]

On Mon, Jun 04, 2018 at 02:23:25PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that use
> > them.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> The more I think about this the more I lean towards that this patch 
> should be dropped. The properties accurately describes the hardware and 
> I think there is value in that. That the driver currently don't parse or 
> make use of them don't in my view reduce there value. Maybe you should 
> break out this patch to a separate series?

I also think there is value in describing the hardware not the state of the
driver at this time.  Is there any missmatch between these properties and
the bindings?

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[jacopo mondi]

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

Hi Simon,

On Tue, Jun 05, 2018 at 09:49:38AM +0200, Simon Horman wrote:
> On Mon, Jun 04, 2018 at 02:23:25PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> > > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > > driver and only confuse users. Remove them in all Gen2 SoC that use
> > > them.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > The more I think about this the more I lean towards that this patch
> > should be dropped. The properties accurately describes the hardware and
> > I think there is value in that. That the driver currently don't parse or
> > make use of them don't in my view reduce there value. Maybe you should
> > break out this patch to a separate series?
>
> I also think there is value in describing the hardware not the state of the
> driver at this time.  Is there any missmatch between these properties and
> the bindings?

Niklas and I discussed a bit offline on this yesterday. My main
concern, and sorry for being pedant on this, is that changing those
properties value does not change the interface behaviour, and this
could cause troubles when integrating image sensor not known to be
working on the VIN interface.

This said, the documentation of those (and all other) properties is in the
generic "video-interfaces.txt" file and it is my understanding, but I think
Laurent and Rob agree on this as well from their replies to my previous series,
that each driver should list which properties it actually supports, as
some aspects are very implementation specific, like default values and
what happens if the property is not specified [1]. Nonetheless, all
properties describing hardware features and documented in the generic
file should be accepted in DTS, as those aims to be OS-independent and
even independent from the single driver implementation.

A possible middle-ground would be documenting in the VIN device tree
bindings description properties whose values actually affect the VIN
interface configuration and state in bindings that all generic properties
described in 'video-interfaces.txt' are valid ones but if not
explicitly listed there their value won't affect the interface
configuration. Niklas suggested this, and I quite like the fact it
makes clear that, say, changing the 'pclk-sample' property won't
change how the VIN interface would sample data but at the same time
allows for extensive description of the hardware.

Would something like this work in your opinion?

As a consequence, this patch should be dropped as Niklas and you
suggested.

Thanks
   j

[1] Ie. An interface that supports BT.656 encoding will use it if
'hsync-active' and 'vsync-active' are not specified. One that does not
support embedded synchronization would instead use defaults for those
two properties. This is specific to the single interface (or even the
single driver implementation actually) and should be listed in the single
driver's bindings description imo.

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

[PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[jacopo mondi]

Hi Simon,

On Tue, Jun 05, 2018 at 09:49:38AM +0200, Simon Horman wrote:
> On Mon, Jun 04, 2018 at 02:23:25PM +0200, Niklas S?derlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> > > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > > driver and only confuse users. Remove them in all Gen2 SoC that use
> > > them.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > The more I think about this the more I lean towards that this patch
> > should be dropped. The properties accurately describes the hardware and
> > I think there is value in that. That the driver currently don't parse or
> > make use of them don't in my view reduce there value. Maybe you should
> > break out this patch to a separate series?
>
> I also think there is value in describing the hardware not the state of the
> driver at this time.  Is there any missmatch between these properties and
> the bindings?

Niklas and I discussed a bit offline on this yesterday. My main
concern, and sorry for being pedant on this, is that changing those
properties value does not change the interface behaviour, and this
could cause troubles when integrating image sensor not known to be
working on the VIN interface.

This said, the documentation of those (and all other) properties is in the
generic "video-interfaces.txt" file and it is my understanding, but I think
Laurent and Rob agree on this as well from their replies to my previous series,
that each driver should list which properties it actually supports, as
some aspects are very implementation specific, like default values and
what happens if the property is not specified [1]. Nonetheless, all
properties describing hardware features and documented in the generic
file should be accepted in DTS, as those aims to be OS-independent and
even independent from the single driver implementation.

A possible middle-ground would be documenting in the VIN device tree
bindings description properties whose values actually affect the VIN
interface configuration and state in bindings that all generic properties
described in 'video-interfaces.txt' are valid ones but if not
explicitly listed there their value won't affect the interface
configuration. Niklas suggested this, and I quite like the fact it
makes clear that, say, changing the 'pclk-sample' property won't
change how the VIN interface would sample data but at the same time
allows for extensive description of the hardware.

Would something like this work in your opinion?

As a consequence, this patch should be dropped as Niklas and you
suggested.

Thanks
   j

[1] Ie. An interface that supports BT.656 encoding will use it if
'hsync-active' and 'vsync-active' are not specified. One that does not
support embedded synchronization would instead use defaults for those
two properties. This is specific to the single interface (or even the
single driver implementation actually) and should be listed in the single
driver's bindings description imo.
-------------- 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/20180605/5a7706b3/attachment.sig>

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[jacopo mondi]

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

Hi Simon,

On Tue, Jun 05, 2018 at 09:49:38AM +0200, Simon Horman wrote:
> On Mon, Jun 04, 2018 at 02:23:25PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> > > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > > driver and only confuse users. Remove them in all Gen2 SoC that use
> > > them.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > The more I think about this the more I lean towards that this patch
> > should be dropped. The properties accurately describes the hardware and
> > I think there is value in that. That the driver currently don't parse or
> > make use of them don't in my view reduce there value. Maybe you should
> > break out this patch to a separate series?
>
> I also think there is value in describing the hardware not the state of the
> driver at this time.  Is there any missmatch between these properties and
> the bindings?

Niklas and I discussed a bit offline on this yesterday. My main
concern, and sorry for being pedant on this, is that changing those
properties value does not change the interface behaviour, and this
could cause troubles when integrating image sensor not known to be
working on the VIN interface.

This said, the documentation of those (and all other) properties is in the
generic "video-interfaces.txt" file and it is my understanding, but I think
Laurent and Rob agree on this as well from their replies to my previous series,
that each driver should list which properties it actually supports, as
some aspects are very implementation specific, like default values and
what happens if the property is not specified [1]. Nonetheless, all
properties describing hardware features and documented in the generic
file should be accepted in DTS, as those aims to be OS-independent and
even independent from the single driver implementation.

A possible middle-ground would be documenting in the VIN device tree
bindings description properties whose values actually affect the VIN
interface configuration and state in bindings that all generic properties
described in 'video-interfaces.txt' are valid ones but if not
explicitly listed there their value won't affect the interface
configuration. Niklas suggested this, and I quite like the fact it
makes clear that, say, changing the 'pclk-sample' property won't
change how the VIN interface would sample data but at the same time
allows for extensive description of the hardware.

Would something like this work in your opinion?

As a consequence, this patch should be dropped as Niklas and you
suggested.

Thanks
   j

[1] Ie. An interface that supports BT.656 encoding will use it if
'hsync-active' and 'vsync-active' are not specified. One that does not
support embedded synchronization would instead use defaults for those
two properties. This is specific to the single interface (or even the
single driver implementation actually) and should be listed in the single
driver's bindings description imo.

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

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Geert Uytterhoeven]

Hi Jacopo,

On Tue, Jun 5, 2018 at 10:12 AM, jacopo mondi <jacopo@jmondi.org> wrote:
> On Tue, Jun 05, 2018 at 09:49:38AM +0200, Simon Horman wrote:
>> On Mon, Jun 04, 2018 at 02:23:25PM +0200, Niklas Söderlund wrote:
>> > On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
>> > > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
>> > > driver and only confuse users. Remove them in all Gen2 SoC that use
>> > > them.
>> > >
>> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> >
>> > The more I think about this the more I lean towards that this patch
>> > should be dropped. The properties accurately describes the hardware and
>> > I think there is value in that. That the driver currently don't parse or
>> > make use of them don't in my view reduce there value. Maybe you should
>> > break out this patch to a separate series?
>>
>> I also think there is value in describing the hardware not the state of the
>> driver at this time.  Is there any missmatch between these properties and
>> the bindings?
>
> Niklas and I discussed a bit offline on this yesterday. My main
> concern, and sorry for being pedant on this, is that changing those
> properties value does not change the interface behaviour, and this
> could cause troubles when integrating image sensor not known to be
> working on the VIN interface.
>
> This said, the documentation of those (and all other) properties is in the
> generic "video-interfaces.txt" file and it is my understanding, but I think
> Laurent and Rob agree on this as well from their replies to my previous series,
> that each driver should list which properties it actually supports, as

s/driver/device-specific binding/

> some aspects are very implementation specific, like default values and
> what happens if the property is not specified [1]. Nonetheless, all

In se defaults are not (Linux) implementation-specific, but fixed in the
DT bindings.

> properties describing hardware features and documented in the generic
> file should be accepted in DTS, as those aims to be OS-independent and
> even independent from the single driver implementation.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Geert Uytterhoeven]

Hi Jacopo,

On Tue, Jun 5, 2018 at 10:12 AM, jacopo mondi <jacopo@jmondi.org> wrote:
> On Tue, Jun 05, 2018 at 09:49:38AM +0200, Simon Horman wrote:
>> On Mon, Jun 04, 2018 at 02:23:25PM +0200, Niklas S?derlund wrote:
>> > On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
>> > > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
>> > > driver and only confuse users. Remove them in all Gen2 SoC that use
>> > > them.
>> > >
>> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> >
>> > The more I think about this the more I lean towards that this patch
>> > should be dropped. The properties accurately describes the hardware and
>> > I think there is value in that. That the driver currently don't parse or
>> > make use of them don't in my view reduce there value. Maybe you should
>> > break out this patch to a separate series?
>>
>> I also think there is value in describing the hardware not the state of the
>> driver at this time.  Is there any missmatch between these properties and
>> the bindings?
>
> Niklas and I discussed a bit offline on this yesterday. My main
> concern, and sorry for being pedant on this, is that changing those
> properties value does not change the interface behaviour, and this
> could cause troubles when integrating image sensor not known to be
> working on the VIN interface.
>
> This said, the documentation of those (and all other) properties is in the
> generic "video-interfaces.txt" file and it is my understanding, but I think
> Laurent and Rob agree on this as well from their replies to my previous series,
> that each driver should list which properties it actually supports, as

s/driver/device-specific binding/

> some aspects are very implementation specific, like default values and
> what happens if the property is not specified [1]. Nonetheless, all

In se defaults are not (Linux) implementation-specific, but fixed in the
DT bindings.

> properties describing hardware features and documented in the generic
> file should be accepted in DTS, as those aims to be OS-independent and
> even independent from the single driver implementation.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties

[Geert Uytterhoeven]

Hi Jacopo,

On Tue, Jun 5, 2018 at 10:12 AM, jacopo mondi <jacopo@jmondi.org> wrote:
> On Tue, Jun 05, 2018 at 09:49:38AM +0200, Simon Horman wrote:
>> On Mon, Jun 04, 2018 at 02:23:25PM +0200, Niklas Söderlund wrote:
>> > On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
>> > > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
>> > > driver and only confuse users. Remove them in all Gen2 SoC that use
>> > > them.
>> > >
>> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> >
>> > The more I think about this the more I lean towards that this patch
>> > should be dropped. The properties accurately describes the hardware and
>> > I think there is value in that. That the driver currently don't parse or
>> > make use of them don't in my view reduce there value. Maybe you should
>> > break out this patch to a separate series?
>>
>> I also think there is value in describing the hardware not the state of the
>> driver at this time.  Is there any missmatch between these properties and
>> the bindings?
>
> Niklas and I discussed a bit offline on this yesterday. My main
> concern, and sorry for being pedant on this, is that changing those
> properties value does not change the interface behaviour, and this
> could cause troubles when integrating image sensor not known to be
> working on the VIN interface.
>
> This said, the documentation of those (and all other) properties is in the
> generic "video-interfaces.txt" file and it is my understanding, but I think
> Laurent and Rob agree on this as well from their replies to my previous series,
> that each driver should list which properties it actually supports, as

s/driver/device-specific binding/

> some aspects are very implementation specific, like default values and
> what happens if the property is not specified [1]. Nonetheless, all

In se defaults are not (Linux) implementation-specific, but fixed in the
DT bindings.

> properties describing hardware features and documented in the generic
> file should be accepted in DTS, as those aims to be OS-independent and
> even independent from the single driver implementation.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds