[PATCH/RFT v2 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[PATCH/RFT v2 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Jacopo Mondi]

Hello renesas list,
   this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
Ebisu board.

It's an RFT, as I don't have an Ebisu to test with :(

The series adds supports for the following items:

- PFC: add VIN groups and functions
- R-Car VIN and R-Car CSI-2: add support for R8A77990
- R8A77990: Add I2C, VIN and CSI-2 nodes (v1 re-sent by Laurent in D3 DU series)
- Ebisu: describe HDMI and CVBS inputs

Laurent has tested v1 and confirmed that the current mainline implementation
of the adv748x driver does not work in the configuration used by the Ebisu
board.

This series should then be applied on top of the just sent:
[PATCH v2 0/5] media: adv748x: Allow probe with a single output endpoint
To allow the adv7482 to probe with a single output port enabled.

This series, based on v4.19-rc2 with adv748x series on top, is available at:
git://jmondi.org/linux ebisu/linus-master/hdmi_cvbs_v2

Thanks
   j

Jacopo Mondi (6):
  media: dt-bindings: rcar-vin: Add R8A77990 support
  media: rcar-vin: Add support for R-Car R8A77990
  media: dt-bindings: rcar-csi2: Add R8A77990
  media: rcar-csi2: Add R8A77990 support
  pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
  arm64: dts: renesas: ebisu: Add HDMI and CVBS input

Koji Matsuoka (1):
  arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

Takeshi Kihara (1):
  arm64: dts: r8a77990: Add I2C device nodes

 .../devicetree/bindings/media/rcar_vin.txt         |   1 +
 .../bindings/media/renesas,rcar-csi2.txt           |   1 +
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  86 +++++++
 arch/arm64/boot/dts/renesas/r8a77990.dtsi          | 202 +++++++++++++++++
 drivers/media/platform/rcar-vin/rcar-core.c        |  20 ++
 drivers/media/platform/rcar-vin/rcar-csi2.c        |   9 +
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c              | 250 +++++++++++++++++++++
 7 files changed, 569 insertions(+)

--
2.7.4

[PATCH v2 1/8] media: dt-bindings: rcar-vin: Add R8A77990 support

[Jacopo Mondi]

Add compatible string for R-Car E3 R8A77990 to the list of SoCs supported by
rcar-vin driver.

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

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 2f42005..dfd6058 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -23,6 +23,7 @@ on Gen3 platforms to a CSI-2 receiver.
    - "renesas,vin-r8a7796" for the R8A7796 device
    - "renesas,vin-r8a77965" for the R8A77965 device
    - "renesas,vin-r8a77970" for the R8A77970 device
+   - "renesas,vin-r8a77990" for the R8A77990 device
    - "renesas,vin-r8a77995" for the R8A77995 device
    - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
      device.
-- 
2.7.4

Re: [PATCH v2 1/8] media: dt-bindings: rcar-vin: Add R8A77990 support

[Simon Horman]

On Wed, Sep 05, 2018 at 05:29:38PM +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of SoCs supported by
> rcar-vin driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH v2 1/8] media: dt-bindings: rcar-vin: Add R8A77990 support

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wednesday, 5 September 2018 18:29:38 EEST Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of SoCs supported by
> rcar-vin driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
> b/Documentation/devicetree/bindings/media/rcar_vin.txt index
> 2f42005..dfd6058 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -23,6 +23,7 @@ on Gen3 platforms to a CSI-2 receiver.
>     - "renesas,vin-r8a7796" for the R8A7796 device
>     - "renesas,vin-r8a77965" for the R8A77965 device
>     - "renesas,vin-r8a77970" for the R8A77970 device
> +   - "renesas,vin-r8a77990" for the R8A77990 device
>     - "renesas,vin-r8a77995" for the R8A77995 device
>     - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
>       device.


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/8] media: dt-bindings: rcar-vin: Add R8A77990 support

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-09-05 17:29:38 +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of SoCs supported by
> rcar-vin driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

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

> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 2f42005..dfd6058 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -23,6 +23,7 @@ on Gen3 platforms to a CSI-2 receiver.
>     - "renesas,vin-r8a7796" for the R8A7796 device
>     - "renesas,vin-r8a77965" for the R8A77965 device
>     - "renesas,vin-r8a77970" for the R8A77970 device
> +   - "renesas,vin-r8a77990" for the R8A77990 device
>     - "renesas,vin-r8a77995" for the R8A77995 device
>     - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
>       device.
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH v2 2/8] media: rcar-vin: Add support for R-Car R8A77990

[Jacopo Mondi]

Add R-Car E3 R8A77990 SoC to the rcar-vin supported ones.
Based on the experimental patch from Magnus Damm.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index ce09799..d443c09 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -1085,6 +1085,22 @@ static const struct rvin_info rcar_info_r8a77970 = {
 	.routes = rcar_info_r8a77970_routes,
 };
 
+static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
+	{ .csi = RVIN_CSI40, .channel = 0, .vin = 4, .mask = BIT(0) | BIT(3) },
+	{ .csi = RVIN_CSI40, .channel = 0, .vin = 5, .mask = BIT(2) },
+	{ .csi = RVIN_CSI40, .channel = 1, .vin = 4, .mask = BIT(2) },
+	{ .csi = RVIN_CSI40, .channel = 1, .vin = 5, .mask = BIT(1) | BIT(3) },
+	{ /* Sentinel */ }
+};
+
+static const struct rvin_info rcar_info_r8a77990 = {
+	.model = RCAR_GEN3,
+	.use_mc = true,
+	.max_width = 4096,
+	.max_height = 4096,
+	.routes = rcar_info_r8a77990_routes,
+};
+
 static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
 	{ /* Sentinel */ }
 };
@@ -1143,6 +1159,10 @@ static const struct of_device_id rvin_of_id_table[] = {
 		.data = &rcar_info_r8a77970,
 	},
 	{
+		.compatible = "renesas,vin-r8a77990",
+		.data = &rcar_info_r8a77990,
+	},
+	{
 		.compatible = "renesas,vin-r8a77995",
 		.data = &rcar_info_r8a77995,
 	},
-- 
2.7.4

Re: [PATCH v2 2/8] media: rcar-vin: Add support for R-Car R8A77990

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wednesday, 5 September 2018 18:29:39 EEST Jacopo Mondi wrote:
> Add R-Car E3 R8A77990 SoC to the rcar-vin supported ones.
> Based on the experimental patch from Magnus Damm.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c index ce09799..d443c09 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -1085,6 +1085,22 @@ static const struct rvin_info rcar_info_r8a77970 = {
>  	.routes = rcar_info_r8a77970_routes,
>  };
> 
> +static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
> +	{ .csi = RVIN_CSI40, .channel = 0, .vin = 4, .mask = BIT(0) | BIT(3) },
> +	{ .csi = RVIN_CSI40, .channel = 0, .vin = 5, .mask = BIT(2) },
> +	{ .csi = RVIN_CSI40, .channel = 1, .vin = 4, .mask = BIT(2) },
> +	{ .csi = RVIN_CSI40, .channel = 1, .vin = 5, .mask = BIT(1) | BIT(3) },
> +	{ /* Sentinel */ }
> +};
> +
> +static const struct rvin_info rcar_info_r8a77990 = {
> +	.model = RCAR_GEN3,
> +	.use_mc = true,
> +	.max_width = 4096,
> +	.max_height = 4096,
> +	.routes = rcar_info_r8a77990_routes,
> +};
> +
>  static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
>  	{ /* Sentinel */ }
>  };
> @@ -1143,6 +1159,10 @@ static const struct of_device_id rvin_of_id_table[] =
> { .data = &rcar_info_r8a77970,
>  	},
>  	{
> +		.compatible = "renesas,vin-r8a77990",
> +		.data = &rcar_info_r8a77990,
> +	},
> +	{
>  		.compatible = "renesas,vin-r8a77995",
>  		.data = &rcar_info_r8a77995,
>  	},

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/8] media: rcar-vin: Add support for R-Car R8A77990

[Niklas Söderlund]

Hi Jacopo,

Thanks for for work.

On 2018-09-05 17:29:39 +0200, Jacopo Mondi wrote:
> Add R-Car E3 R8A77990 SoC to the rcar-vin supported ones.
> Based on the experimental patch from Magnus Damm.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

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

> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index ce09799..d443c09 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -1085,6 +1085,22 @@ static const struct rvin_info rcar_info_r8a77970 = {
>  	.routes = rcar_info_r8a77970_routes,
>  };
>  
> +static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
> +	{ .csi = RVIN_CSI40, .channel = 0, .vin = 4, .mask = BIT(0) | BIT(3) },
> +	{ .csi = RVIN_CSI40, .channel = 0, .vin = 5, .mask = BIT(2) },
> +	{ .csi = RVIN_CSI40, .channel = 1, .vin = 4, .mask = BIT(2) },
> +	{ .csi = RVIN_CSI40, .channel = 1, .vin = 5, .mask = BIT(1) | BIT(3) },
> +	{ /* Sentinel */ }
> +};
> +
> +static const struct rvin_info rcar_info_r8a77990 = {
> +	.model = RCAR_GEN3,
> +	.use_mc = true,
> +	.max_width = 4096,
> +	.max_height = 4096,
> +	.routes = rcar_info_r8a77990_routes,
> +};
> +
>  static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
>  	{ /* Sentinel */ }
>  };
> @@ -1143,6 +1159,10 @@ static const struct of_device_id rvin_of_id_table[] = {
>  		.data = &rcar_info_r8a77970,
>  	},
>  	{
> +		.compatible = "renesas,vin-r8a77990",
> +		.data = &rcar_info_r8a77990,
> +	},
> +	{
>  		.compatible = "renesas,vin-r8a77995",
>  		.data = &rcar_info_r8a77995,
>  	},
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH v2 3/8] media: dt-bindings: rcar-csi2: Add R8A77990

[Jacopo Mondi]

Add compatible string for R-Car E3 R8A77990 to the list of supported SoCs.

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

diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
index 2d385b6..2824489 100644
--- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
+++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
@@ -12,6 +12,7 @@ Mandatory properties
    - "renesas,r8a7796-csi2" for the R8A7796 device.
    - "renesas,r8a77965-csi2" for the R8A77965 device.
    - "renesas,r8a77970-csi2" for the R8A77970 device.
+   - "renesas,r8a77990-csi2" for the R8A77990 device.
 
  - reg: the register base and size for the device registers
  - interrupts: the interrupt for the device
-- 
2.7.4

Re: [PATCH v2 3/8] media: dt-bindings: rcar-csi2: Add R8A77990

[Simon Horman]

On Wed, Sep 05, 2018 at 05:29:40PM +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of supported SoCs.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> index 2d385b6..2824489 100644
> --- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> @@ -12,6 +12,7 @@ Mandatory properties
>     - "renesas,r8a7796-csi2" for the R8A7796 device.
>     - "renesas,r8a77965-csi2" for the R8A77965 device.
>     - "renesas,r8a77970-csi2" for the R8A77970 device.
> +   - "renesas,r8a77990-csi2" for the R8A77990 device.
>  
>   - reg: the register base and size for the device registers
>   - interrupts: the interrupt for the device
> -- 
> 2.7.4
> 

Re: [PATCH v2 3/8] media: dt-bindings: rcar-csi2: Add R8A77990

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wednesday, 5 September 2018 18:29:40 EEST Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of supported SoCs.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt index
> 2d385b6..2824489 100644
> --- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> @@ -12,6 +12,7 @@ Mandatory properties
>     - "renesas,r8a7796-csi2" for the R8A7796 device.
>     - "renesas,r8a77965-csi2" for the R8A77965 device.
>     - "renesas,r8a77970-csi2" for the R8A77970 device.
> +   - "renesas,r8a77990-csi2" for the R8A77990 device.
> 
>   - reg: the register base and size for the device registers
>   - interrupts: the interrupt for the device


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/8] media: dt-bindings: rcar-csi2: Add R8A77990

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-09-05 17:29:40 +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of supported SoCs.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

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

> ---
>  Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> index 2d385b6..2824489 100644
> --- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> @@ -12,6 +12,7 @@ Mandatory properties
>     - "renesas,r8a7796-csi2" for the R8A7796 device.
>     - "renesas,r8a77965-csi2" for the R8A77965 device.
>     - "renesas,r8a77970-csi2" for the R8A77970 device.
> +   - "renesas,r8a77990-csi2" for the R8A77990 device.
>  
>   - reg: the register base and size for the device registers
>   - interrupts: the interrupt for the device
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH v2 4/8] media: rcar-csi2: Add R8A77990 support

[Jacopo Mondi]

Add support for R-Car E3 R8A77965 to R-Car CSI-2 driver.
Based on the experimental patch from Magnus Damm.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index dc5ae80..f82b668 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -959,6 +959,11 @@ static const struct rcar_csi2_info rcar_csi2_info_r8a77970 = {
 	.confirm_start = rcsi2_confirm_start_v3m_e3,
 };
 
+static const struct rcar_csi2_info rcar_csi2_info_r8a77990 = {
+	.init_phtw = rcsi2_init_phtw_v3m_e3,
+	.confirm_start = rcsi2_confirm_start_v3m_e3,
+};
+
 static const struct of_device_id rcar_csi2_of_table[] = {
 	{
 		.compatible = "renesas,r8a7795-csi2",
@@ -976,6 +981,10 @@ static const struct of_device_id rcar_csi2_of_table[] = {
 		.compatible = "renesas,r8a77970-csi2",
 		.data = &rcar_csi2_info_r8a77970,
 	},
+	{
+		.compatible = "renesas,r8a77990-csi2",
+		.data = &rcar_csi2_info_r8a77990,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
-- 
2.7.4

Re: [PATCH v2 4/8] media: rcar-csi2: Add R8A77990 support

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wednesday, 5 September 2018 18:29:41 EEST Jacopo Mondi wrote:
> Add support for R-Car E3 R8A77965 to R-Car CSI-2 driver.
> Based on the experimental patch from Magnus Damm.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> b/drivers/media/platform/rcar-vin/rcar-csi2.c index dc5ae80..f82b668 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -959,6 +959,11 @@ static const struct rcar_csi2_info
> rcar_csi2_info_r8a77970 = { .confirm_start = rcsi2_confirm_start_v3m_e3,
>  };
> 
> +static const struct rcar_csi2_info rcar_csi2_info_r8a77990 = {
> +	.init_phtw = rcsi2_init_phtw_v3m_e3,
> +	.confirm_start = rcsi2_confirm_start_v3m_e3,
> +};
> +
>  static const struct of_device_id rcar_csi2_of_table[] = {
>  	{
>  		.compatible = "renesas,r8a7795-csi2",
> @@ -976,6 +981,10 @@ static const struct of_device_id rcar_csi2_of_table[] =
> { .compatible = "renesas,r8a77970-csi2",
>  		.data = &rcar_csi2_info_r8a77970,
>  	},
> +	{
> +		.compatible = "renesas,r8a77990-csi2",
> +		.data = &rcar_csi2_info_r8a77990,
> +	},
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 4/8] media: rcar-csi2: Add R8A77990 support

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-09-05 17:29:41 +0200, Jacopo Mondi wrote:
> Add support for R-Car E3 R8A77965 to R-Car CSI-2 driver.
> Based on the experimental patch from Magnus Damm.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

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

> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index dc5ae80..f82b668 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -959,6 +959,11 @@ static const struct rcar_csi2_info rcar_csi2_info_r8a77970 = {
>  	.confirm_start = rcsi2_confirm_start_v3m_e3,
>  };
>  
> +static const struct rcar_csi2_info rcar_csi2_info_r8a77990 = {
> +	.init_phtw = rcsi2_init_phtw_v3m_e3,
> +	.confirm_start = rcsi2_confirm_start_v3m_e3,
> +};
> +
>  static const struct of_device_id rcar_csi2_of_table[] = {
>  	{
>  		.compatible = "renesas,r8a7795-csi2",
> @@ -976,6 +981,10 @@ static const struct of_device_id rcar_csi2_of_table[] = {
>  		.compatible = "renesas,r8a77970-csi2",
>  		.data = &rcar_csi2_info_r8a77970,
>  	},
> +	{
> +		.compatible = "renesas,r8a77990-csi2",
> +		.data = &rcar_csi2_info_r8a77990,
> +	},
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[Jacopo Mondi]

This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c | 250 ++++++++++++++++++++++++++++++++++
 1 file changed, 250 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
index b81c807..0797940 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
@@ -1831,6 +1831,194 @@ static const unsigned int usb30_id_mux[] = {
 	USB3HS0_ID_MARK,
 };
 
+/* - VIN4 ------------------------------------------------------------------- */
+static const union vin_data vin4_data_a_pins = {
+	.data24 = {
+		RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
+		RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
+		RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
+		RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
+		RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+		RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+		RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
+		RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+		RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
+		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
+		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
+		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
+	},
+};
+
+static const union vin_data vin4_data_a_mux = {
+	.data24 = {
+		VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
+		VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
+		VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
+		VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
+		VI4_DATA8_MARK,   VI4_DATA9_MARK,
+		VI4_DATA10_MARK,  VI4_DATA11_MARK,
+		VI4_DATA12_MARK,  VI4_DATA13_MARK,
+		VI4_DATA14_MARK,  VI4_DATA15_MARK,
+		VI4_DATA16_MARK,  VI4_DATA17_MARK,
+		VI4_DATA18_MARK,  VI4_DATA19_MARK,
+		VI4_DATA20_MARK,  VI4_DATA21_MARK,
+		VI4_DATA22_MARK,  VI4_DATA23_MARK,
+	},
+};
+
+static const union vin_data vin4_data_b_pins = {
+	.data24 = {
+		RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
+		RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
+		RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
+		RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
+		RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+		RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+		RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
+		RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+		RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
+		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 15),
+		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
+		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
+	},
+};
+
+static const union vin_data vin4_data_b_mux = {
+	.data24 = {
+		VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
+		VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
+		VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
+		VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
+		VI4_DATA8_MARK,   VI4_DATA9_MARK,
+		VI4_DATA10_MARK,  VI4_DATA11_MARK,
+		VI4_DATA12_MARK,  VI4_DATA13_MARK,
+		VI4_DATA14_MARK,  VI4_DATA15_MARK,
+		VI4_DATA16_MARK,  VI4_DATA17_MARK,
+		VI4_DATA18_MARK,  VI4_DATA19_MARK,
+		VI4_DATA20_MARK,  VI4_DATA21_MARK,
+		VI4_DATA22_MARK,  VI4_DATA23_MARK,
+	},
+};
+
+static const unsigned int vin4_sync_pins[] = {
+	/* HSYNC, VSYNC */
+	RCAR_GP_PIN(2, 25), RCAR_GP_PIN(2, 24),
+};
+
+static const unsigned int vin4_sync_mux[] = {
+	VI4_HSYNC_N_MARK, VI4_VSYNC_N_MARK,
+};
+
+static const unsigned int vin4_field_pins[] = {
+	RCAR_GP_PIN(2, 23),
+};
+
+static const unsigned int vin4_field_mux[] = {
+	VI4_FIELD_MARK,
+};
+
+static const unsigned int vin4_clkenb_pins[] = {
+	RCAR_GP_PIN(1, 2),
+};
+
+static const unsigned int vin4_clkenb_mux[] = {
+	VI4_CLKENB_MARK,
+};
+
+static const unsigned int vin4_clk_pins[] = {
+	RCAR_GP_PIN(2, 22),
+};
+
+static const unsigned int vin4_clk_mux[] = {
+	VI4_CLK_MARK,
+};
+
+/* - VIN5 ------------------------------------------------------------------- */
+static const union vin_data vin5_data_a_pins = {
+	.data16 = {
+		RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 2),
+		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 12),
+		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
+		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
+		RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
+		RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 11),
+		RCAR_GP_PIN(0, 8),  RCAR_GP_PIN(0, 10),
+		RCAR_GP_PIN(0, 2),  RCAR_GP_PIN(0, 3),
+	},
+};
+
+static const union vin_data vin5_data_a_mux = {
+	.data16 = {
+		VI5_DATA0_A_MARK,  VI5_DATA1_A_MARK,
+		VI5_DATA2_A_MARK,  VI5_DATA3_A_MARK,
+		VI5_DATA4_A_MARK,  VI5_DATA5_A_MARK,
+		VI5_DATA6_A_MARK,  VI5_DATA7_A_MARK,
+		VI5_DATA8_A_MARK,  VI5_DATA9_A_MARK,
+		VI5_DATA10_A_MARK, VI5_DATA11_A_MARK,
+		VI5_DATA12_A_MARK, VI5_DATA13_A_MARK,
+		VI5_DATA14_A_MARK, VI5_DATA15_A_MARK,
+	},
+};
+
+static const union vin_data vin5_data_b_pins = {
+	.data8 = {
+		RCAR_GP_PIN(2, 23), RCAR_GP_PIN(0, 4),
+		RCAR_GP_PIN(0, 7),  RCAR_GP_PIN(0, 12),
+		RCAR_GP_PIN(0, 13), RCAR_GP_PIN(0, 14),
+		RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
+	},
+};
+
+static const union vin_data vin5_data_b_mux = {
+	.data8 = {
+		VI5_DATA0_B_MARK,  VI5_DATA1_B_MARK,
+		VI5_DATA2_B_MARK,  VI5_DATA3_B_MARK,
+		VI5_DATA4_B_MARK,  VI5_DATA5_B_MARK,
+		VI5_DATA6_B_MARK,  VI5_DATA7_B_MARK,
+	},
+};
+
+static const unsigned int vin5_sync_a_pins[] = {
+	/* HSYNC_N, VSYNC_N */
+	RCAR_GP_PIN(1, 8), RCAR_GP_PIN(1, 9),
+};
+
+static const unsigned int vin5_sync_a_mux[] = {
+	VI5_HSYNC_N_A_MARK, VI5_VSYNC_N_A_MARK,
+};
+
+static const unsigned int vin5_field_a_pins[] = {
+	RCAR_GP_PIN(1, 10),
+};
+
+static const unsigned int vin5_field_a_mux[] = {
+	VI5_FIELD_A_MARK,
+};
+
+static const unsigned int vin5_clkenb_a_pins[] = {
+	RCAR_GP_PIN(0, 1),
+};
+
+static const unsigned int vin5_clkenb_a_mux[] = {
+	VI5_CLKENB_A_MARK,
+};
+
+static const unsigned int vin5_clk_a_pins[] = {
+	RCAR_GP_PIN(1, 0),
+};
+
+static const unsigned int vin5_clk_a_mux[] = {
+	VI5_CLK_A_MARK,
+};
+
+static const unsigned int vin5_clk_b_pins[] = {
+	RCAR_GP_PIN(2, 22),
+};
+
+static const unsigned int vin5_clk_b_mux[] = {
+	VI5_CLK_B_MARK,
+};
+
 static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(avb_link),
 	SH_PFC_PIN_GROUP(avb_magic),
@@ -1889,6 +2077,32 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(usb0_id),
 	SH_PFC_PIN_GROUP(usb30),
 	SH_PFC_PIN_GROUP(usb30_id),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 8),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 10),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 12),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 16),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 20),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 24),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 8),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 10),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 12),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 16),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 20),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 24),
+	SH_PFC_PIN_GROUP(vin4_sync),
+	SH_PFC_PIN_GROUP(vin4_field),
+	SH_PFC_PIN_GROUP(vin4_clkenb),
+	SH_PFC_PIN_GROUP(vin4_clk),
+	VIN_DATA_PIN_GROUP(vin5_data_a, 8),
+	VIN_DATA_PIN_GROUP(vin5_data_a, 10),
+	VIN_DATA_PIN_GROUP(vin5_data_a, 12),
+	VIN_DATA_PIN_GROUP(vin5_data_a, 16),
+	VIN_DATA_PIN_GROUP(vin5_data_b, 8),
+	SH_PFC_PIN_GROUP(vin5_sync_a),
+	SH_PFC_PIN_GROUP(vin5_field_a),
+	SH_PFC_PIN_GROUP(vin5_clkenb_a),
+	SH_PFC_PIN_GROUP(vin5_clk_a),
+	SH_PFC_PIN_GROUP(vin5_clk_b),
 };
 
 static const char * const avb_groups[] = {
@@ -1996,6 +2210,40 @@ static const char * const usb30_groups[] = {
 	"usb30_id",
 };
 
+static const char * const vin4_groups[] = {
+	"vin4_data8_a",
+	"vin4_data10_a",
+	"vin4_data12_a",
+	"vin4_data16_a",
+	"vin4_data20_a",
+	"vin4_data24_a",
+	"vin4_data8_b",
+	"vin4_data10_b",
+	"vin4_data12_b",
+	"vin4_data16_b",
+	"vin4_data20_b",
+	"vin4_data24_b",
+	"vin4_data8_sft8",
+	"vin4_sync",
+	"vin4_field",
+	"vin4_clkenb",
+	"vin4_clk",
+};
+
+static const char * const vin5_groups[] = {
+	"vin5_data8_a",
+	"vin5_data8_sft8_a",
+	"vin5_data10_a",
+	"vin5_data12_a",
+	"vin5_data16_a",
+	"vin5_data8_b",
+	"vin5_sync_a",
+	"vin5_field_a",
+	"vin5_clkenb_a",
+	"vin5_clk_a",
+	"vin5_clk_b",
+};
+
 static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(avb),
 	SH_PFC_FUNCTION(i2c1),
@@ -2013,6 +2261,8 @@ static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(scif_clk),
 	SH_PFC_FUNCTION(usb0),
 	SH_PFC_FUNCTION(usb30),
+	SH_PFC_FUNCTION(vin4),
+	SH_PFC_FUNCTION(vin5),
 };
 
 static const struct pinmux_cfg_reg pinmux_config_regs[] = {
-- 
2.7.4

Re: [PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[Simon Horman]

On Wed, Sep 05, 2018 at 05:29:42PM +0200, Jacopo Mondi wrote:
> This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a77990.c | 250 ++++++++++++++++++++++++++++++++++
>  1 file changed, 250 insertions(+)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> index b81c807..0797940 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> @@ -1831,6 +1831,194 @@ static const unsigned int usb30_id_mux[] = {
>  	USB3HS0_ID_MARK,
>  };
>  
> +/* - VIN4 ------------------------------------------------------------------- */
> +static const union vin_data vin4_data_a_pins = {
> +	.data24 = {
> +		RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
> +		RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
> +		RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
> +		RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
> +		RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> +		RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> +		RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
> +		RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> +		RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
> +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
> +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
> +	},
> +};
> +
> +static const union vin_data vin4_data_a_mux = {
> +	.data24 = {
> +		VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
> +		VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
> +		VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
> +		VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
> +		VI4_DATA8_MARK,   VI4_DATA9_MARK,
> +		VI4_DATA10_MARK,  VI4_DATA11_MARK,
> +		VI4_DATA12_MARK,  VI4_DATA13_MARK,
> +		VI4_DATA14_MARK,  VI4_DATA15_MARK,
> +		VI4_DATA16_MARK,  VI4_DATA17_MARK,
> +		VI4_DATA18_MARK,  VI4_DATA19_MARK,
> +		VI4_DATA20_MARK,  VI4_DATA21_MARK,
> +		VI4_DATA22_MARK,  VI4_DATA23_MARK,
> +	},
> +};
> +
> +static const union vin_data vin4_data_b_pins = {
> +	.data24 = {
> +		RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
> +		RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
> +		RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
> +		RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),

I am curious to know why the data B pins below (8 - 23)
are duplicates of the corresponding data A pins in vin4_data_a_pins.

> +		RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> +		RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> +		RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
> +		RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> +		RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
> +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 15),
> +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
> +	},
> +};
> +
> +static const union vin_data vin4_data_b_mux = {
> +	.data24 = {
> +		VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
> +		VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
> +		VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
> +		VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
> +		VI4_DATA8_MARK,   VI4_DATA9_MARK,
> +		VI4_DATA10_MARK,  VI4_DATA11_MARK,
> +		VI4_DATA12_MARK,  VI4_DATA13_MARK,
> +		VI4_DATA14_MARK,  VI4_DATA15_MARK,
> +		VI4_DATA16_MARK,  VI4_DATA17_MARK,
> +		VI4_DATA18_MARK,  VI4_DATA19_MARK,
> +		VI4_DATA20_MARK,  VI4_DATA21_MARK,
> +		VI4_DATA22_MARK,  VI4_DATA23_MARK,
> +	},
> +};
> +
> +static const unsigned int vin4_sync_pins[] = {
> +	/* HSYNC, VSYNC */
> +	RCAR_GP_PIN(2, 25), RCAR_GP_PIN(2, 24),
> +};
> +
> +static const unsigned int vin4_sync_mux[] = {
> +	VI4_HSYNC_N_MARK, VI4_VSYNC_N_MARK,
> +};
> +
> +static const unsigned int vin4_field_pins[] = {
> +	RCAR_GP_PIN(2, 23),
> +};
> +
> +static const unsigned int vin4_field_mux[] = {
> +	VI4_FIELD_MARK,
> +};
> +
> +static const unsigned int vin4_clkenb_pins[] = {
> +	RCAR_GP_PIN(1, 2),
> +};
> +
> +static const unsigned int vin4_clkenb_mux[] = {
> +	VI4_CLKENB_MARK,
> +};
> +
> +static const unsigned int vin4_clk_pins[] = {
> +	RCAR_GP_PIN(2, 22),
> +};
> +
> +static const unsigned int vin4_clk_mux[] = {
> +	VI4_CLK_MARK,
> +};
> +
> +/* - VIN5 ------------------------------------------------------------------- */
> +static const union vin_data vin5_data_a_pins = {
> +	.data16 = {
> +		RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 2),
> +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 12),
> +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
> +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> +		RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
> +		RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 11),
> +		RCAR_GP_PIN(0, 8),  RCAR_GP_PIN(0, 10),
> +		RCAR_GP_PIN(0, 2),  RCAR_GP_PIN(0, 3),
> +	},
> +};
> +
> +static const union vin_data vin5_data_a_mux = {
> +	.data16 = {
> +		VI5_DATA0_A_MARK,  VI5_DATA1_A_MARK,
> +		VI5_DATA2_A_MARK,  VI5_DATA3_A_MARK,
> +		VI5_DATA4_A_MARK,  VI5_DATA5_A_MARK,
> +		VI5_DATA6_A_MARK,  VI5_DATA7_A_MARK,
> +		VI5_DATA8_A_MARK,  VI5_DATA9_A_MARK,
> +		VI5_DATA10_A_MARK, VI5_DATA11_A_MARK,
> +		VI5_DATA12_A_MARK, VI5_DATA13_A_MARK,
> +		VI5_DATA14_A_MARK, VI5_DATA15_A_MARK,
> +	},
> +};
> +
> +static const union vin_data vin5_data_b_pins = {
> +	.data8 = {
> +		RCAR_GP_PIN(2, 23), RCAR_GP_PIN(0, 4),
> +		RCAR_GP_PIN(0, 7),  RCAR_GP_PIN(0, 12),
> +		RCAR_GP_PIN(0, 13), RCAR_GP_PIN(0, 14),
> +		RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
> +	},
> +};
> +
> +static const union vin_data vin5_data_b_mux = {
> +	.data8 = {
> +		VI5_DATA0_B_MARK,  VI5_DATA1_B_MARK,
> +		VI5_DATA2_B_MARK,  VI5_DATA3_B_MARK,
> +		VI5_DATA4_B_MARK,  VI5_DATA5_B_MARK,
> +		VI5_DATA6_B_MARK,  VI5_DATA7_B_MARK,
> +	},
> +};
> +
> +static const unsigned int vin5_sync_a_pins[] = {
> +	/* HSYNC_N, VSYNC_N */
> +	RCAR_GP_PIN(1, 8), RCAR_GP_PIN(1, 9),
> +};
> +
> +static const unsigned int vin5_sync_a_mux[] = {
> +	VI5_HSYNC_N_A_MARK, VI5_VSYNC_N_A_MARK,
> +};
> +
> +static const unsigned int vin5_field_a_pins[] = {
> +	RCAR_GP_PIN(1, 10),
> +};
> +
> +static const unsigned int vin5_field_a_mux[] = {
> +	VI5_FIELD_A_MARK,
> +};
> +
> +static const unsigned int vin5_clkenb_a_pins[] = {
> +	RCAR_GP_PIN(0, 1),
> +};
> +
> +static const unsigned int vin5_clkenb_a_mux[] = {
> +	VI5_CLKENB_A_MARK,
> +};
> +
> +static const unsigned int vin5_clk_a_pins[] = {
> +	RCAR_GP_PIN(1, 0),
> +};
> +
> +static const unsigned int vin5_clk_a_mux[] = {
> +	VI5_CLK_A_MARK,
> +};
> +
> +static const unsigned int vin5_clk_b_pins[] = {
> +	RCAR_GP_PIN(2, 22),
> +};
> +
> +static const unsigned int vin5_clk_b_mux[] = {
> +	VI5_CLK_B_MARK,
> +};
> +
>  static const struct sh_pfc_pin_group pinmux_groups[] = {
>  	SH_PFC_PIN_GROUP(avb_link),
>  	SH_PFC_PIN_GROUP(avb_magic),
> @@ -1889,6 +2077,32 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
>  	SH_PFC_PIN_GROUP(usb0_id),
>  	SH_PFC_PIN_GROUP(usb30),
>  	SH_PFC_PIN_GROUP(usb30_id),
> +	VIN_DATA_PIN_GROUP(vin4_data_a, 8),
> +	VIN_DATA_PIN_GROUP(vin4_data_a, 10),
> +	VIN_DATA_PIN_GROUP(vin4_data_a, 12),
> +	VIN_DATA_PIN_GROUP(vin4_data_a, 16),
> +	VIN_DATA_PIN_GROUP(vin4_data_a, 20),
> +	VIN_DATA_PIN_GROUP(vin4_data_a, 24),
> +	VIN_DATA_PIN_GROUP(vin4_data_b, 8),
> +	VIN_DATA_PIN_GROUP(vin4_data_b, 10),
> +	VIN_DATA_PIN_GROUP(vin4_data_b, 12),
> +	VIN_DATA_PIN_GROUP(vin4_data_b, 16),
> +	VIN_DATA_PIN_GROUP(vin4_data_b, 20),
> +	VIN_DATA_PIN_GROUP(vin4_data_b, 24),
> +	SH_PFC_PIN_GROUP(vin4_sync),
> +	SH_PFC_PIN_GROUP(vin4_field),
> +	SH_PFC_PIN_GROUP(vin4_clkenb),
> +	SH_PFC_PIN_GROUP(vin4_clk),
> +	VIN_DATA_PIN_GROUP(vin5_data_a, 8),
> +	VIN_DATA_PIN_GROUP(vin5_data_a, 10),
> +	VIN_DATA_PIN_GROUP(vin5_data_a, 12),
> +	VIN_DATA_PIN_GROUP(vin5_data_a, 16),
> +	VIN_DATA_PIN_GROUP(vin5_data_b, 8),
> +	SH_PFC_PIN_GROUP(vin5_sync_a),
> +	SH_PFC_PIN_GROUP(vin5_field_a),
> +	SH_PFC_PIN_GROUP(vin5_clkenb_a),
> +	SH_PFC_PIN_GROUP(vin5_clk_a),
> +	SH_PFC_PIN_GROUP(vin5_clk_b),
>  };
>  
>  static const char * const avb_groups[] = {
> @@ -1996,6 +2210,40 @@ static const char * const usb30_groups[] = {
>  	"usb30_id",
>  };
>  
> +static const char * const vin4_groups[] = {
> +	"vin4_data8_a",
> +	"vin4_data10_a",
> +	"vin4_data12_a",
> +	"vin4_data16_a",
> +	"vin4_data20_a",
> +	"vin4_data24_a",
> +	"vin4_data8_b",
> +	"vin4_data10_b",
> +	"vin4_data12_b",
> +	"vin4_data16_b",
> +	"vin4_data20_b",
> +	"vin4_data24_b",
> +	"vin4_data8_sft8",
> +	"vin4_sync",
> +	"vin4_field",
> +	"vin4_clkenb",
> +	"vin4_clk",
> +};
> +
> +static const char * const vin5_groups[] = {
> +	"vin5_data8_a",
> +	"vin5_data8_sft8_a",
> +	"vin5_data10_a",
> +	"vin5_data12_a",
> +	"vin5_data16_a",
> +	"vin5_data8_b",
> +	"vin5_sync_a",
> +	"vin5_field_a",
> +	"vin5_clkenb_a",
> +	"vin5_clk_a",
> +	"vin5_clk_b",
> +};
> +
>  static const struct sh_pfc_function pinmux_functions[] = {
>  	SH_PFC_FUNCTION(avb),
>  	SH_PFC_FUNCTION(i2c1),
> @@ -2013,6 +2261,8 @@ static const struct sh_pfc_function pinmux_functions[] = {
>  	SH_PFC_FUNCTION(scif_clk),
>  	SH_PFC_FUNCTION(usb0),
>  	SH_PFC_FUNCTION(usb30),
> +	SH_PFC_FUNCTION(vin4),
> +	SH_PFC_FUNCTION(vin5),
>  };
>  
>  static const struct pinmux_cfg_reg pinmux_config_regs[] = {
> -- 
> 2.7.4
> 

Re: [PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[jacopo mondi]

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

Hi Simon,
   thanks for looking into this patch

On Mon, Sep 10, 2018 at 03:01:15PM +0200, Simon Horman wrote:
> On Wed, Sep 05, 2018 at 05:29:42PM +0200, Jacopo Mondi wrote:
> > This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/pinctrl/sh-pfc/pfc-r8a77990.c | 250 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 250 insertions(+)
> >
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > index b81c807..0797940 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > @@ -1831,6 +1831,194 @@ static const unsigned int usb30_id_mux[] = {
> >  	USB3HS0_ID_MARK,
> >  };
> >
> > +/* - VIN4 ------------------------------------------------------------------- */
> > +static const union vin_data vin4_data_a_pins = {
> > +	.data24 = {
> > +		RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
> > +		RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
> > +		RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
> > +		RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
> > +		RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> > +		RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> > +		RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
> > +		RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> > +		RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
> > +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
> > +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> > +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
> > +	},
> > +};
> > +
> > +static const union vin_data vin4_data_a_mux = {
> > +	.data24 = {
> > +		VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
> > +		VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
> > +		VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
> > +		VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
> > +		VI4_DATA8_MARK,   VI4_DATA9_MARK,
> > +		VI4_DATA10_MARK,  VI4_DATA11_MARK,
> > +		VI4_DATA12_MARK,  VI4_DATA13_MARK,
> > +		VI4_DATA14_MARK,  VI4_DATA15_MARK,
> > +		VI4_DATA16_MARK,  VI4_DATA17_MARK,
> > +		VI4_DATA18_MARK,  VI4_DATA19_MARK,
> > +		VI4_DATA20_MARK,  VI4_DATA21_MARK,
> > +		VI4_DATA22_MARK,  VI4_DATA23_MARK,
> > +	},
> > +};
> > +
> > +static const union vin_data vin4_data_b_pins = {
> > +	.data24 = {
> > +		RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
> > +		RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
> > +		RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
> > +		RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
>
> I am curious to know why the data B pins below (8 - 23)
> are duplicates of the corresponding data A pins in vin4_data_a_pins.
>

On R-Car E3 only pins [0-7] of VIN4 interface have an '_a' and '_b'
options. Pins from [8-23] are "shared".

We can discuss how we want this to be handled, but according to Table
6D.5 (pag. 383 of R-Car chip manual revision 1.00) this table is
correct.

Currently there are two open questions on this PFC patch:
1) This one here you reported
... (see below)

> > +		RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> > +		RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> > +		RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
> > +		RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> > +		RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
> > +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 15),
> > +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> > +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
> > +	},
> > +};
> > +
> > +static const union vin_data vin4_data_b_mux = {
> > +	.data24 = {
> > +		VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
> > +		VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
> > +		VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
> > +		VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
> > +		VI4_DATA8_MARK,   VI4_DATA9_MARK,
> > +		VI4_DATA10_MARK,  VI4_DATA11_MARK,
> > +		VI4_DATA12_MARK,  VI4_DATA13_MARK,
> > +		VI4_DATA14_MARK,  VI4_DATA15_MARK,
> > +		VI4_DATA16_MARK,  VI4_DATA17_MARK,
> > +		VI4_DATA18_MARK,  VI4_DATA19_MARK,
> > +		VI4_DATA20_MARK,  VI4_DATA21_MARK,
> > +		VI4_DATA22_MARK,  VI4_DATA23_MARK,
> > +	},
> > +};
> > +
> > +static const unsigned int vin4_sync_pins[] = {
> > +	/* HSYNC, VSYNC */
> > +	RCAR_GP_PIN(2, 25), RCAR_GP_PIN(2, 24),
> > +};
> > +
> > +static const unsigned int vin4_sync_mux[] = {
> > +	VI4_HSYNC_N_MARK, VI4_VSYNC_N_MARK,
> > +};
> > +
> > +static const unsigned int vin4_field_pins[] = {
> > +	RCAR_GP_PIN(2, 23),
> > +};
> > +
> > +static const unsigned int vin4_field_mux[] = {
> > +	VI4_FIELD_MARK,
> > +};
> > +
> > +static const unsigned int vin4_clkenb_pins[] = {
> > +	RCAR_GP_PIN(1, 2),
> > +};
> > +
> > +static const unsigned int vin4_clkenb_mux[] = {
> > +	VI4_CLKENB_MARK,
> > +};
> > +
> > +static const unsigned int vin4_clk_pins[] = {
> > +	RCAR_GP_PIN(2, 22),
> > +};
> > +
> > +static const unsigned int vin4_clk_mux[] = {
> > +	VI4_CLK_MARK,
> > +};
> > +
> > +/* - VIN5 ------------------------------------------------------------------- */
> > +static const union vin_data vin5_data_a_pins = {
> > +	.data16 = {
> > +		RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 2),
> > +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 12),
> > +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
> > +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> > +		RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
> > +		RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 11),
> > +		RCAR_GP_PIN(0, 8),  RCAR_GP_PIN(0, 10),
> > +		RCAR_GP_PIN(0, 2),  RCAR_GP_PIN(0, 3),
> > +	},
> > +};
> > +
> > +static const union vin_data vin5_data_a_mux = {
> > +	.data16 = {
> > +		VI5_DATA0_A_MARK,  VI5_DATA1_A_MARK,
> > +		VI5_DATA2_A_MARK,  VI5_DATA3_A_MARK,
> > +		VI5_DATA4_A_MARK,  VI5_DATA5_A_MARK,
> > +		VI5_DATA6_A_MARK,  VI5_DATA7_A_MARK,
> > +		VI5_DATA8_A_MARK,  VI5_DATA9_A_MARK,
> > +		VI5_DATA10_A_MARK, VI5_DATA11_A_MARK,
> > +		VI5_DATA12_A_MARK, VI5_DATA13_A_MARK,
> > +		VI5_DATA14_A_MARK, VI5_DATA15_A_MARK,
> > +	},
> > +};
> > +
> > +static const union vin_data vin5_data_b_pins = {
> > +	.data8 = {
> > +		RCAR_GP_PIN(2, 23), RCAR_GP_PIN(0, 4),
> > +		RCAR_GP_PIN(0, 7),  RCAR_GP_PIN(0, 12),
> > +		RCAR_GP_PIN(0, 13), RCAR_GP_PIN(0, 14),
> > +		RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
> > +	},
> > +};
> > +
> > +static const union vin_data vin5_data_b_mux = {
> > +	.data8 = {
> > +		VI5_DATA0_B_MARK,  VI5_DATA1_B_MARK,
> > +		VI5_DATA2_B_MARK,  VI5_DATA3_B_MARK,
> > +		VI5_DATA4_B_MARK,  VI5_DATA5_B_MARK,
> > +		VI5_DATA6_B_MARK,  VI5_DATA7_B_MARK,
> > +	},
> > +};
> > +
> > +static const unsigned int vin5_sync_a_pins[] = {
> > +	/* HSYNC_N, VSYNC_N */
> > +	RCAR_GP_PIN(1, 8), RCAR_GP_PIN(1, 9),
> > +};
> > +
> > +static const unsigned int vin5_sync_a_mux[] = {
> > +	VI5_HSYNC_N_A_MARK, VI5_VSYNC_N_A_MARK,
> > +};
> > +
> > +static const unsigned int vin5_field_a_pins[] = {
> > +	RCAR_GP_PIN(1, 10),
> > +};
> > +
> > +static const unsigned int vin5_field_a_mux[] = {
> > +	VI5_FIELD_A_MARK,
> > +};
> > +
> > +static const unsigned int vin5_clkenb_a_pins[] = {
> > +	RCAR_GP_PIN(0, 1),
> > +};
> > +
> > +static const unsigned int vin5_clkenb_a_mux[] = {
> > +	VI5_CLKENB_A_MARK,
> > +};

2) VIN5 synchronism signals (V/HSYNC, CLKENB, FIELD) are marked as
   "_A" only, while VIN4 ones have not _A or _B extensions and are
   shared between _A and _B group. The VIN5_#_A extension is an
   indication that synchronism signals for group _B are not
   multiplexed but active be default according to Morimoto-san, that
   is about to confirm this with HW team. In that case, we need to
   decide if to provide an 'vin5_sync_b' group anyway to let user
   select it from DTS. Otherwise it won't be possible to select
   synchronism pins for VIN5_B group (which is maybe fine if they're
   not multiplexed at all).

Thanks
   j

> > +
> > +static const unsigned int vin5_clk_a_pins[] = {
> > +	RCAR_GP_PIN(1, 0),
> > +};
> > +
> > +static const unsigned int vin5_clk_a_mux[] = {
> > +	VI5_CLK_A_MARK,
> > +};
> > +
> > +static const unsigned int vin5_clk_b_pins[] = {
> > +	RCAR_GP_PIN(2, 22),
> > +};
> > +
> > +static const unsigned int vin5_clk_b_mux[] = {
> > +	VI5_CLK_B_MARK,
> > +};
> > +
> >  static const struct sh_pfc_pin_group pinmux_groups[] = {
> >  	SH_PFC_PIN_GROUP(avb_link),
> >  	SH_PFC_PIN_GROUP(avb_magic),
> > @@ -1889,6 +2077,32 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
> >  	SH_PFC_PIN_GROUP(usb0_id),
> >  	SH_PFC_PIN_GROUP(usb30),
> >  	SH_PFC_PIN_GROUP(usb30_id),
> > +	VIN_DATA_PIN_GROUP(vin4_data_a, 8),
> > +	VIN_DATA_PIN_GROUP(vin4_data_a, 10),
> > +	VIN_DATA_PIN_GROUP(vin4_data_a, 12),
> > +	VIN_DATA_PIN_GROUP(vin4_data_a, 16),
> > +	VIN_DATA_PIN_GROUP(vin4_data_a, 20),
> > +	VIN_DATA_PIN_GROUP(vin4_data_a, 24),
> > +	VIN_DATA_PIN_GROUP(vin4_data_b, 8),
> > +	VIN_DATA_PIN_GROUP(vin4_data_b, 10),
> > +	VIN_DATA_PIN_GROUP(vin4_data_b, 12),
> > +	VIN_DATA_PIN_GROUP(vin4_data_b, 16),
> > +	VIN_DATA_PIN_GROUP(vin4_data_b, 20),
> > +	VIN_DATA_PIN_GROUP(vin4_data_b, 24),
> > +	SH_PFC_PIN_GROUP(vin4_sync),
> > +	SH_PFC_PIN_GROUP(vin4_field),
> > +	SH_PFC_PIN_GROUP(vin4_clkenb),
> > +	SH_PFC_PIN_GROUP(vin4_clk),
> > +	VIN_DATA_PIN_GROUP(vin5_data_a, 8),
> > +	VIN_DATA_PIN_GROUP(vin5_data_a, 10),
> > +	VIN_DATA_PIN_GROUP(vin5_data_a, 12),
> > +	VIN_DATA_PIN_GROUP(vin5_data_a, 16),
> > +	VIN_DATA_PIN_GROUP(vin5_data_b, 8),
> > +	SH_PFC_PIN_GROUP(vin5_sync_a),
> > +	SH_PFC_PIN_GROUP(vin5_field_a),
> > +	SH_PFC_PIN_GROUP(vin5_clkenb_a),
> > +	SH_PFC_PIN_GROUP(vin5_clk_a),
> > +	SH_PFC_PIN_GROUP(vin5_clk_b),
> >  };
> >
> >  static const char * const avb_groups[] = {
> > @@ -1996,6 +2210,40 @@ static const char * const usb30_groups[] = {
> >  	"usb30_id",
> >  };
> >
> > +static const char * const vin4_groups[] = {
> > +	"vin4_data8_a",
> > +	"vin4_data10_a",
> > +	"vin4_data12_a",
> > +	"vin4_data16_a",
> > +	"vin4_data20_a",
> > +	"vin4_data24_a",
> > +	"vin4_data8_b",
> > +	"vin4_data10_b",
> > +	"vin4_data12_b",
> > +	"vin4_data16_b",
> > +	"vin4_data20_b",
> > +	"vin4_data24_b",
> > +	"vin4_data8_sft8",
> > +	"vin4_sync",
> > +	"vin4_field",
> > +	"vin4_clkenb",
> > +	"vin4_clk",
> > +};
> > +
> > +static const char * const vin5_groups[] = {
> > +	"vin5_data8_a",
> > +	"vin5_data8_sft8_a",
> > +	"vin5_data10_a",
> > +	"vin5_data12_a",
> > +	"vin5_data16_a",
> > +	"vin5_data8_b",
> > +	"vin5_sync_a",
> > +	"vin5_field_a",
> > +	"vin5_clkenb_a",
> > +	"vin5_clk_a",
> > +	"vin5_clk_b",
> > +};
> > +
> >  static const struct sh_pfc_function pinmux_functions[] = {
> >  	SH_PFC_FUNCTION(avb),
> >  	SH_PFC_FUNCTION(i2c1),
> > @@ -2013,6 +2261,8 @@ static const struct sh_pfc_function pinmux_functions[] = {
> >  	SH_PFC_FUNCTION(scif_clk),
> >  	SH_PFC_FUNCTION(usb0),
> >  	SH_PFC_FUNCTION(usb30),
> > +	SH_PFC_FUNCTION(vin4),
> > +	SH_PFC_FUNCTION(vin5),
> >  };
> >
> >  static const struct pinmux_cfg_reg pinmux_config_regs[] = {
> > --
> > 2.7.4
> >

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

Re: [PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[Geert Uytterhoeven]

Hi Jacopo,

On Tue, Sep 11, 2018 at 9:44 AM jacopo mondi <jacopo@jmondi.org> wrote:
> On Mon, Sep 10, 2018 at 03:01:15PM +0200, Simon Horman wrote:
> > On Wed, Sep 05, 2018 at 05:29:42PM +0200, Jacopo Mondi wrote:
> > > This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.

> Currently there are two open questions on this PFC patch:

> 2) VIN5 synchronism signals (V/HSYNC, CLKENB, FIELD) are marked as
>    "_A" only, while VIN4 ones have not _A or _B extensions and are
>    shared between _A and _B group. The VIN5_#_A extension is an
>    indication that synchronism signals for group _B are not
>    multiplexed but active be default according to Morimoto-san, that
>    is about to confirm this with HW team. In that case, we need to
>    decide if to provide an 'vin5_sync_b' group anyway to let user
>    select it from DTS. Otherwise it won't be possible to select
>    synchronism pins for VIN5_B group (which is maybe fine if they're
>    not multiplexed at all).

If the a "B" sync group exists, the pins are probably configurable as GPIOs,
too, so we probably do need a group for them in the driver.

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

Re: [PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[jacopo mondi]

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

Hi Geert,

On Tue, Sep 11, 2018 at 10:15:23AM +0200, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Tue, Sep 11, 2018 at 9:44 AM jacopo mondi <jacopo@jmondi.org> wrote:
> > On Mon, Sep 10, 2018 at 03:01:15PM +0200, Simon Horman wrote:
> > > On Wed, Sep 05, 2018 at 05:29:42PM +0200, Jacopo Mondi wrote:
> > > > This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.
>
> > Currently there are two open questions on this PFC patch:
>
> > 2) VIN5 synchronism signals (V/HSYNC, CLKENB, FIELD) are marked as
> >    "_A" only, while VIN4 ones have not _A or _B extensions and are
> >    shared between _A and _B group. The VIN5_#_A extension is an
> >    indication that synchronism signals for group _B are not
> >    multiplexed but active be default according to Morimoto-san, that
> >    is about to confirm this with HW team. In that case, we need to
> >    decide if to provide an 'vin5_sync_b' group anyway to let user
> >    select it from DTS. Otherwise it won't be possible to select
> >    synchronism pins for VIN5_B group (which is maybe fine if they're
> >    not multiplexed at all).
>
> If the a "B" sync group exists, the pins are probably configurable as GPIOs,
> too, so we probably do need a group for them in the driver.
>

The chip manual does not report any _b group, and I don't have any E3
pin-related documentation like I have for M3-W/N
(r01uh0802ej0100-r-car-3rd-pin.pdf,
ASOM-C18-201_R-CarM3_pinfunction.xls etc etc)

How to find it out? Morimoto-san have you heard any news from HW team?

Thanks
   j

> 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

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

Re: [PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[jacopo mondi]

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

Hi again,
   I actually noticed I'm handling VIN4 and VIN5 un-consistently
here...

On Tue, Sep 11, 2018 at 09:44:48AM +0200, jacopo mondi wrote:
> Hi Simon,
>    thanks for looking into this patch
>
> On Mon, Sep 10, 2018 at 03:01:15PM +0200, Simon Horman wrote:
> > On Wed, Sep 05, 2018 at 05:29:42PM +0200, Jacopo Mondi wrote:
> > > This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  drivers/pinctrl/sh-pfc/pfc-r8a77990.c | 250 ++++++++++++++++++++++++++++++++++
> > >  1 file changed, 250 insertions(+)
> > >
> > > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > > index b81c807..0797940 100644
> > > --- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > > @@ -1831,6 +1831,194 @@ static const unsigned int usb30_id_mux[] = {
> > >  	USB3HS0_ID_MARK,
> > >  };
> > >
> > > +/* - VIN4 ------------------------------------------------------------------- */
> > > +static const union vin_data vin4_data_a_pins = {
> > > +	.data24 = {
> > > +		RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
> > > +		RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
> > > +		RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
> > > +		RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
> > > +		RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> > > +		RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> > > +		RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
> > > +		RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> > > +		RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
> > > +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
> > > +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> > > +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
> > > +	},
> > > +};
> > > +
> > > +static const union vin_data vin4_data_a_mux = {
> > > +	.data24 = {
> > > +		VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
> > > +		VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
> > > +		VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
> > > +		VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
> > > +		VI4_DATA8_MARK,   VI4_DATA9_MARK,
> > > +		VI4_DATA10_MARK,  VI4_DATA11_MARK,
> > > +		VI4_DATA12_MARK,  VI4_DATA13_MARK,
> > > +		VI4_DATA14_MARK,  VI4_DATA15_MARK,
> > > +		VI4_DATA16_MARK,  VI4_DATA17_MARK,
> > > +		VI4_DATA18_MARK,  VI4_DATA19_MARK,
> > > +		VI4_DATA20_MARK,  VI4_DATA21_MARK,
> > > +		VI4_DATA22_MARK,  VI4_DATA23_MARK,
> > > +	},
> > > +};
> > > +
> > > +static const union vin_data vin4_data_b_pins = {
> > > +	.data24 = {
> > > +		RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
> > > +		RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
> > > +		RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
> > > +		RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
> >
> > I am curious to know why the data B pins below (8 - 23)
> > are duplicates of the corresponding data A pins in vin4_data_a_pins.
> >
>
> On R-Car E3 only pins [0-7] of VIN4 interface have an '_a' and '_b'
> options. Pins from [8-23] are "shared".
>
> We can discuss how we want this to be handled, but according to Table
> 6D.5 (pag. 383 of R-Car chip manual revision 1.00) this table is
> correct.
>
> Currently there are two open questions on this PFC patch:
> 1) This one here you reported

It does not end here, I'm sorry.

VIN4 and VIN5 are described differently, it seems to me that we have

vin4_data[0-7]_[a|b]
vin4_data[8-23]
vin4_sync

vin5_data[0-7]_[a|b]
vin5_data[8-15]_a
vin5_sync_a

So I handled it differently, as I've registered the following data groups
for VIN4

> > > +	"vin4_data8_a",
> > > +	"vin4_data10_a",
> > > +	"vin4_data12_a",
> > > +	"vin4_data16_a",
> > > +	"vin4_data20_a",
> > > +	"vin4_data24_a",
> > > +	"vin4_data8_b",
> > > +	"vin4_data10_b",
> > > +	"vin4_data12_b",
> > > +	"vin4_data16_b",
> > > +	"vin4_data20_b",
> > > +	"vin4_data24_b",

And the following ones for VIN5


> > > +	"vin5_data8_a",
> > > +	"vin5_data10_a",
> > > +	"vin5_data12_a",
> > > +	"vin5_data16_a",
> > > +	"vin5_data8_b",

If I would have been doing the same as I did for VIN4, I should have
had "vin5_data10_b", "vin5_data12_b" and so on, with only the first 8
pin being different between all _a and _b groups.

I didn't do that because the VIN5 pins in the [8-15] range have a clear _a
indications, but the more I think about this, the more I think that's
a typographical mistake in the chip manual, and the VIN5 groups should
not have any _a suffix, except for the first 8 pins, where a
corresponding _b group actually exists. Or there is maybe an
explanation why VIN4 and VIN5 are different, but I don't see it right
now...

Thanks
   j


> ... (see below)
>
> > > +		RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> > > +		RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> > > +		RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
> > > +		RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> > > +		RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
> > > +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 15),
> > > +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> > > +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
> > > +	},
> > > +};
> > > +
> > > +static const union vin_data vin4_data_b_mux = {
> > > +	.data24 = {
> > > +		VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
> > > +		VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
> > > +		VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
> > > +		VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
> > > +		VI4_DATA8_MARK,   VI4_DATA9_MARK,
> > > +		VI4_DATA10_MARK,  VI4_DATA11_MARK,
> > > +		VI4_DATA12_MARK,  VI4_DATA13_MARK,
> > > +		VI4_DATA14_MARK,  VI4_DATA15_MARK,
> > > +		VI4_DATA16_MARK,  VI4_DATA17_MARK,
> > > +		VI4_DATA18_MARK,  VI4_DATA19_MARK,
> > > +		VI4_DATA20_MARK,  VI4_DATA21_MARK,
> > > +		VI4_DATA22_MARK,  VI4_DATA23_MARK,
> > > +	},
> > > +};
> > > +
> > > +static const unsigned int vin4_sync_pins[] = {
> > > +	/* HSYNC, VSYNC */
> > > +	RCAR_GP_PIN(2, 25), RCAR_GP_PIN(2, 24),
> > > +};
> > > +
> > > +static const unsigned int vin4_sync_mux[] = {
> > > +	VI4_HSYNC_N_MARK, VI4_VSYNC_N_MARK,
> > > +};
> > > +
> > > +static const unsigned int vin4_field_pins[] = {
> > > +	RCAR_GP_PIN(2, 23),
> > > +};
> > > +
> > > +static const unsigned int vin4_field_mux[] = {
> > > +	VI4_FIELD_MARK,
> > > +};
> > > +
> > > +static const unsigned int vin4_clkenb_pins[] = {
> > > +	RCAR_GP_PIN(1, 2),
> > > +};
> > > +
> > > +static const unsigned int vin4_clkenb_mux[] = {
> > > +	VI4_CLKENB_MARK,
> > > +};
> > > +
> > > +static const unsigned int vin4_clk_pins[] = {
> > > +	RCAR_GP_PIN(2, 22),
> > > +};
> > > +
> > > +static const unsigned int vin4_clk_mux[] = {
> > > +	VI4_CLK_MARK,
> > > +};
> > > +
> > > +/* - VIN5 ------------------------------------------------------------------- */
> > > +static const union vin_data vin5_data_a_pins = {
> > > +	.data16 = {
> > > +		RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 2),
> > > +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 12),
> > > +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
> > > +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> > > +		RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
> > > +		RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 11),
> > > +		RCAR_GP_PIN(0, 8),  RCAR_GP_PIN(0, 10),
> > > +		RCAR_GP_PIN(0, 2),  RCAR_GP_PIN(0, 3),
> > > +	},
> > > +};
> > > +
> > > +static const union vin_data vin5_data_a_mux = {
> > > +	.data16 = {
> > > +		VI5_DATA0_A_MARK,  VI5_DATA1_A_MARK,
> > > +		VI5_DATA2_A_MARK,  VI5_DATA3_A_MARK,
> > > +		VI5_DATA4_A_MARK,  VI5_DATA5_A_MARK,
> > > +		VI5_DATA6_A_MARK,  VI5_DATA7_A_MARK,
> > > +		VI5_DATA8_A_MARK,  VI5_DATA9_A_MARK,
> > > +		VI5_DATA10_A_MARK, VI5_DATA11_A_MARK,
> > > +		VI5_DATA12_A_MARK, VI5_DATA13_A_MARK,
> > > +		VI5_DATA14_A_MARK, VI5_DATA15_A_MARK,
> > > +	},
> > > +};
> > > +
> > > +static const union vin_data vin5_data_b_pins = {
> > > +	.data8 = {
> > > +		RCAR_GP_PIN(2, 23), RCAR_GP_PIN(0, 4),
> > > +		RCAR_GP_PIN(0, 7),  RCAR_GP_PIN(0, 12),
> > > +		RCAR_GP_PIN(0, 13), RCAR_GP_PIN(0, 14),
> > > +		RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
> > > +	},
> > > +};
> > > +
> > > +static const union vin_data vin5_data_b_mux = {
> > > +	.data8 = {
> > > +		VI5_DATA0_B_MARK,  VI5_DATA1_B_MARK,
> > > +		VI5_DATA2_B_MARK,  VI5_DATA3_B_MARK,
> > > +		VI5_DATA4_B_MARK,  VI5_DATA5_B_MARK,
> > > +		VI5_DATA6_B_MARK,  VI5_DATA7_B_MARK,
> > > +	},
> > > +};
> > > +
> > > +static const unsigned int vin5_sync_a_pins[] = {
> > > +	/* HSYNC_N, VSYNC_N */
> > > +	RCAR_GP_PIN(1, 8), RCAR_GP_PIN(1, 9),
> > > +};
> > > +
> > > +static const unsigned int vin5_sync_a_mux[] = {
> > > +	VI5_HSYNC_N_A_MARK, VI5_VSYNC_N_A_MARK,
> > > +};
> > > +
> > > +static const unsigned int vin5_field_a_pins[] = {
> > > +	RCAR_GP_PIN(1, 10),
> > > +};
> > > +
> > > +static const unsigned int vin5_field_a_mux[] = {
> > > +	VI5_FIELD_A_MARK,
> > > +};
> > > +
> > > +static const unsigned int vin5_clkenb_a_pins[] = {
> > > +	RCAR_GP_PIN(0, 1),
> > > +};
> > > +
> > > +static const unsigned int vin5_clkenb_a_mux[] = {
> > > +	VI5_CLKENB_A_MARK,
> > > +};
>
> 2) VIN5 synchronism signals (V/HSYNC, CLKENB, FIELD) are marked as
>    "_A" only, while VIN4 ones have not _A or _B extensions and are
>    shared between _A and _B group. The VIN5_#_A extension is an
>    indication that synchronism signals for group _B are not
>    multiplexed but active be default according to Morimoto-san, that
>    is about to confirm this with HW team. In that case, we need to
>    decide if to provide an 'vin5_sync_b' group anyway to let user
>    select it from DTS. Otherwise it won't be possible to select
>    synchronism pins for VIN5_B group (which is maybe fine if they're
>    not multiplexed at all).
>
> Thanks
>    j
>
> > > +
> > > +static const unsigned int vin5_clk_a_pins[] = {
> > > +	RCAR_GP_PIN(1, 0),
> > > +};
> > > +
> > > +static const unsigned int vin5_clk_a_mux[] = {
> > > +	VI5_CLK_A_MARK,
> > > +};
> > > +
> > > +static const unsigned int vin5_clk_b_pins[] = {
> > > +	RCAR_GP_PIN(2, 22),
> > > +};
> > > +
> > > +static const unsigned int vin5_clk_b_mux[] = {
> > > +	VI5_CLK_B_MARK,
> > > +};
> > > +
> > >  static const struct sh_pfc_pin_group pinmux_groups[] = {
> > >  	SH_PFC_PIN_GROUP(avb_link),
> > >  	SH_PFC_PIN_GROUP(avb_magic),
> > > @@ -1889,6 +2077,32 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
> > >  	SH_PFC_PIN_GROUP(usb0_id),
> > >  	SH_PFC_PIN_GROUP(usb30),
> > >  	SH_PFC_PIN_GROUP(usb30_id),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_a, 8),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_a, 10),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_a, 12),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_a, 16),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_a, 20),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_a, 24),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_b, 8),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_b, 10),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_b, 12),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_b, 16),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_b, 20),
> > > +	VIN_DATA_PIN_GROUP(vin4_data_b, 24),
> > > +	SH_PFC_PIN_GROUP(vin4_sync),
> > > +	SH_PFC_PIN_GROUP(vin4_field),
> > > +	SH_PFC_PIN_GROUP(vin4_clkenb),
> > > +	SH_PFC_PIN_GROUP(vin4_clk),
> > > +	VIN_DATA_PIN_GROUP(vin5_data_a, 8),
> > > +	VIN_DATA_PIN_GROUP(vin5_data_a, 10),
> > > +	VIN_DATA_PIN_GROUP(vin5_data_a, 12),
> > > +	VIN_DATA_PIN_GROUP(vin5_data_a, 16),
> > > +	VIN_DATA_PIN_GROUP(vin5_data_b, 8),
> > > +	SH_PFC_PIN_GROUP(vin5_sync_a),
> > > +	SH_PFC_PIN_GROUP(vin5_field_a),
> > > +	SH_PFC_PIN_GROUP(vin5_clkenb_a),
> > > +	SH_PFC_PIN_GROUP(vin5_clk_a),
> > > +	SH_PFC_PIN_GROUP(vin5_clk_b),
> > >  };
> > >
> > >  static const char * const avb_groups[] = {
> > > @@ -1996,6 +2210,40 @@ static const char * const usb30_groups[] = {
> > >  	"usb30_id",
> > >  };
> > >
> > > +static const char * const vin4_groups[] = {
> > > +	"vin4_data8_a",
> > > +	"vin4_data10_a",
> > > +	"vin4_data12_a",
> > > +	"vin4_data16_a",
> > > +	"vin4_data20_a",
> > > +	"vin4_data24_a",
> > > +	"vin4_data8_b",
> > > +	"vin4_data10_b",
> > > +	"vin4_data12_b",
> > > +	"vin4_data16_b",
> > > +	"vin4_data20_b",
> > > +	"vin4_data24_b",
> > > +	"vin4_data8_sft8",
> > > +	"vin4_sync",
> > > +	"vin4_field",
> > > +	"vin4_clkenb",
> > > +	"vin4_clk",
> > > +};
> > > +
> > > +static const char * const vin5_groups[] = {
> > > +	"vin5_data8_a",
> > > +	"vin5_data8_sft8_a",
> > > +	"vin5_data10_a",
> > > +	"vin5_data12_a",
> > > +	"vin5_data16_a",
> > > +	"vin5_data8_b",
> > > +	"vin5_sync_a",
> > > +	"vin5_field_a",
> > > +	"vin5_clkenb_a",
> > > +	"vin5_clk_a",
> > > +	"vin5_clk_b",
> > > +};
> > > +
> > >  static const struct sh_pfc_function pinmux_functions[] = {
> > >  	SH_PFC_FUNCTION(avb),
> > >  	SH_PFC_FUNCTION(i2c1),
> > > @@ -2013,6 +2261,8 @@ static const struct sh_pfc_function pinmux_functions[] = {
> > >  	SH_PFC_FUNCTION(scif_clk),
> > >  	SH_PFC_FUNCTION(usb0),
> > >  	SH_PFC_FUNCTION(usb30),
> > > +	SH_PFC_FUNCTION(vin4),
> > > +	SH_PFC_FUNCTION(vin5),
> > >  };
> > >
> > >  static const struct pinmux_cfg_reg pinmux_config_regs[] = {
> > > --
> > > 2.7.4
> > >



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

Re: [PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[jacopo mondi]

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

Hi again,
   thanks to Morimoto-san, we got answers from the HW team.
I'm pasting them here below.

On Tue, Sep 11, 2018 at 11:44:30AM +0200, jacopo mondi wrote:
> Hi again,
>    I actually noticed I'm handling VIN4 and VIN5 un-consistently
> here...
>
> On Tue, Sep 11, 2018 at 09:44:48AM +0200, jacopo mondi wrote:
> > Hi Simon,
> >    thanks for looking into this patch
> >
> > On Mon, Sep 10, 2018 at 03:01:15PM +0200, Simon Horman wrote:
> > > On Wed, Sep 05, 2018 at 05:29:42PM +0200, Jacopo Mondi wrote:
> > > > This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > ---
> > > >  drivers/pinctrl/sh-pfc/pfc-r8a77990.c | 250 ++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 250 insertions(+)
> > > >
> > > > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > > > index b81c807..0797940 100644
> > > > --- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > > > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > > > @@ -1831,6 +1831,194 @@ static const unsigned int usb30_id_mux[] = {
> > > >  	USB3HS0_ID_MARK,
> > > >  };
> > > >
> > > > +/* - VIN4 ------------------------------------------------------------------- */
> > > > +static const union vin_data vin4_data_a_pins = {
> > > > +	.data24 = {
> > > > +		RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
> > > > +		RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
> > > > +		RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
> > > > +		RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
> > > > +		RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> > > > +		RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> > > > +		RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
> > > > +		RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> > > > +		RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
> > > > +		RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
> > > > +		RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> > > > +		RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
> > > > +	},
> > > > +};
> > > > +
> > > > +static const union vin_data vin4_data_a_mux = {
> > > > +	.data24 = {
> > > > +		VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
> > > > +		VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
> > > > +		VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
> > > > +		VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
> > > > +		VI4_DATA8_MARK,   VI4_DATA9_MARK,
> > > > +		VI4_DATA10_MARK,  VI4_DATA11_MARK,
> > > > +		VI4_DATA12_MARK,  VI4_DATA13_MARK,
> > > > +		VI4_DATA14_MARK,  VI4_DATA15_MARK,
> > > > +		VI4_DATA16_MARK,  VI4_DATA17_MARK,
> > > > +		VI4_DATA18_MARK,  VI4_DATA19_MARK,
> > > > +		VI4_DATA20_MARK,  VI4_DATA21_MARK,
> > > > +		VI4_DATA22_MARK,  VI4_DATA23_MARK,
> > > > +	},
> > > > +};
> > > > +
> > > > +static const union vin_data vin4_data_b_pins = {
> > > > +	.data24 = {
> > > > +		RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
> > > > +		RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
> > > > +		RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
> > > > +		RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
> > >
> > > I am curious to know why the data B pins below (8 - 23)
> > > are duplicates of the corresponding data A pins in vin4_data_a_pins.
> > >
> >
> > On R-Car E3 only pins [0-7] of VIN4 interface have an '_a' and '_b'
> > options. Pins from [8-23] are "shared".
> >
> > We can discuss how we want this to be handled, but according to Table
> > 6D.5 (pag. 383 of R-Car chip manual revision 1.00) this table is
> > correct.
> >
> > Currently there are two open questions on this PFC patch:
> > 1) This one here you reported
>
> It does not end here, I'm sorry.
>
> VIN4 and VIN5 are described differently, it seems to me that we have
>
> vin4_data[0-7]_[a|b]
> vin4_data[8-23]
> vin4_sync
>
> vin5_data[0-7]_[a|b]
> vin5_data[8-15]_a
> vin5_sync_a
>
> So I handled it differently, as I've registered the following data groups
> for VIN4
>
> > > > +	"vin4_data8_a",
> > > > +	"vin4_data10_a",
> > > > +	"vin4_data12_a",
> > > > +	"vin4_data16_a",
> > > > +	"vin4_data20_a",
> > > > +	"vin4_data24_a",
> > > > +	"vin4_data8_b",
> > > > +	"vin4_data10_b",
> > > > +	"vin4_data12_b",
> > > > +	"vin4_data16_b",
> > > > +	"vin4_data20_b",
> > > > +	"vin4_data24_b",
>
> And the following ones for VIN5
>
>
> > > > +	"vin5_data8_a",
> > > > +	"vin5_data10_a",
> > > > +	"vin5_data12_a",
> > > > +	"vin5_data16_a",
> > > > +	"vin5_data8_b",
>
> If I would have been doing the same as I did for VIN4, I should have
> had "vin5_data10_b", "vin5_data12_b" and so on, with only the first 8
> pin being different between all _a and _b groups.
>
> I didn't do that because the VIN5 pins in the [8-15] range have a clear _a
> indications, but the more I think about this, the more I think that's
> a typographical mistake in the chip manual, and the VIN5 groups should
> not have any _a suffix, except for the first 8 pins, where a
> corresponding _b group actually exists. Or there is maybe an
> explanation why VIN4 and VIN5 are different, but I don't see it right
> now...
>

[VI4]
- Data[15:8] are     shared on A/B
- Data[7:0]  are not shared on A/B
- clock/sync are     shared on A/B

[VI5]
- A can use data[15:0]
- B can use data[7:0] only. BT.656 YUV422-8bit support only
- A/B uses each clock (not shared)
- A only has sync

So I think this patch is correct, and the following registered groups
matches the hardware capabilities

+	VIN_DATA_PIN_GROUP(vin4_data_a, 8),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 10),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 12),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 16),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 20),
+	VIN_DATA_PIN_GROUP(vin4_data_a, 24),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 8),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 10),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 12),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 16),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 20),
+	VIN_DATA_PIN_GROUP(vin4_data_b, 24),
+	SH_PFC_PIN_GROUP(vin4_sync),
+	SH_PFC_PIN_GROUP(vin4_field),
+	SH_PFC_PIN_GROUP(vin4_clkenb),
+	SH_PFC_PIN_GROUP(vin4_clk),
+	VIN_DATA_PIN_GROUP(vin5_data_a, 8),
+	VIN_DATA_PIN_GROUP(vin5_data_a, 10),
+	VIN_DATA_PIN_GROUP(vin5_data_a, 12),
+	VIN_DATA_PIN_GROUP(vin5_data_a, 16),
+	VIN_DATA_PIN_GROUP(vin5_data_b, 8),
+	SH_PFC_PIN_GROUP(vin5_sync_a),
+	SH_PFC_PIN_GROUP(vin5_field_a),
+	SH_PFC_PIN_GROUP(vin5_clkenb_a),
+	SH_PFC_PIN_GROUP(vin5_clk_a),
+	SH_PFC_PIN_GROUP(vin5_clk_b),


Thanks
   j

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

Re: [PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[Geert Uytterhoeven]

Hi Jacopo,

On Wed, Sep 5, 2018 at 5:30 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks for your patch!

> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c

> +/* - VIN5 ------------------------------------------------------------------- */
> +static const union vin_data vin5_data_a_pins = {
> +       .data16 = {

Please note that union vin_data has space for 24 entries, so the last 8
are unused.  We have several other drivers doing this, though, also
for .data12, and it is still a win, compared to duplicating the arrays.
We might want to introduce vin_data16 and vin_data12 unions in the
future, to avoid the unused entries. But for now, this is fine.

> +               RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 2),
> +               RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 12),
> +               RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
> +               RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
> +               RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
> +               RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 11),
> +               RCAR_GP_PIN(0, 8),  RCAR_GP_PIN(0, 10),
> +               RCAR_GP_PIN(0, 2),  RCAR_GP_PIN(0, 3),
> +       },
> +};

> +static const union vin_data vin5_data_b_pins = {
> +       .data8 = {

This one...

> +               RCAR_GP_PIN(2, 23), RCAR_GP_PIN(0, 4),
> +               RCAR_GP_PIN(0, 7),  RCAR_GP_PIN(0, 12),
> +               RCAR_GP_PIN(0, 13), RCAR_GP_PIN(0, 14),
> +               RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
> +       },
> +};
> +
> +static const union vin_data vin5_data_b_mux = {
> +       .data8 = {

... and this one are definitely a loss, as there are 16 unused entries, and
you don't use or need the union feature...

> +               VI5_DATA0_B_MARK,  VI5_DATA1_B_MARK,
> +               VI5_DATA2_B_MARK,  VI5_DATA3_B_MARK,
> +               VI5_DATA4_B_MARK,  VI5_DATA5_B_MARK,
> +               VI5_DATA6_B_MARK,  VI5_DATA7_B_MARK,
> +       },
> +};

> @@ -1889,6 +2077,32 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
>         SH_PFC_PIN_GROUP(usb0_id),
>         SH_PFC_PIN_GROUP(usb30),
>         SH_PFC_PIN_GROUP(usb30_id),
> +       VIN_DATA_PIN_GROUP(vin4_data_a, 8),
> +       VIN_DATA_PIN_GROUP(vin4_data_a, 10),
> +       VIN_DATA_PIN_GROUP(vin4_data_a, 12),
> +       VIN_DATA_PIN_GROUP(vin4_data_a, 16),
> +       VIN_DATA_PIN_GROUP(vin4_data_a, 20),
> +       VIN_DATA_PIN_GROUP(vin4_data_a, 24),
> +       VIN_DATA_PIN_GROUP(vin4_data_b, 8),
> +       VIN_DATA_PIN_GROUP(vin4_data_b, 10),
> +       VIN_DATA_PIN_GROUP(vin4_data_b, 12),
> +       VIN_DATA_PIN_GROUP(vin4_data_b, 16),
> +       VIN_DATA_PIN_GROUP(vin4_data_b, 20),
> +       VIN_DATA_PIN_GROUP(vin4_data_b, 24),
> +       SH_PFC_PIN_GROUP(vin4_sync),
> +       SH_PFC_PIN_GROUP(vin4_field),
> +       SH_PFC_PIN_GROUP(vin4_clkenb),
> +       SH_PFC_PIN_GROUP(vin4_clk),
> +       VIN_DATA_PIN_GROUP(vin5_data_a, 8),
> +       VIN_DATA_PIN_GROUP(vin5_data_a, 10),
> +       VIN_DATA_PIN_GROUP(vin5_data_a, 12),
> +       VIN_DATA_PIN_GROUP(vin5_data_a, 16),
> +       VIN_DATA_PIN_GROUP(vin5_data_b, 8),

... here. So please revert the union change for this group.

> +       SH_PFC_PIN_GROUP(vin5_sync_a),
> +       SH_PFC_PIN_GROUP(vin5_field_a),
> +       SH_PFC_PIN_GROUP(vin5_clkenb_a),
> +       SH_PFC_PIN_GROUP(vin5_clk_a),
> +       SH_PFC_PIN_GROUP(vin5_clk_b),
>  };
>
>  static const char * const avb_groups[] = {
> @@ -1996,6 +2210,40 @@ static const char * const usb30_groups[] = {
>         "usb30_id",
>  };
>
> +static const char * const vin4_groups[] = {
> +       "vin4_data8_a",
> +       "vin4_data10_a",
> +       "vin4_data12_a",
> +       "vin4_data16_a",
> +       "vin4_data20_a",
> +       "vin4_data24_a",
> +       "vin4_data8_b",
> +       "vin4_data10_b",
> +       "vin4_data12_b",
> +       "vin4_data16_b",
> +       "vin4_data20_b",
> +       "vin4_data24_b",
> +       "vin4_data8_sft8",

You dropped the sft8 pins, but forgot to remove the sft8 group name.

> +       "vin4_sync",
> +       "vin4_field",
> +       "vin4_clkenb",
> +       "vin4_clk",
> +};
> +
> +static const char * const vin5_groups[] = {
> +       "vin5_data8_a",
> +       "vin5_data8_sft8_a",

Likewise.

> +       "vin5_data10_a",
> +       "vin5_data12_a",
> +       "vin5_data16_a",
> +       "vin5_data8_b",
> +       "vin5_sync_a",
> +       "vin5_field_a",
> +       "vin5_clkenb_a",
> +       "vin5_clk_a",
> +       "vin5_clk_b",
> +};

The rest looks OK to me, and matches the datasheet clarification.

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

Re: [PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[jacopo mondi]

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

Hi Geert, Simon,
   sorry to resurect this one, but while upporting VIN pin definition
for R8A77965 I have noticed something in this patch.

Please see below.

On Tue, Oct 02, 2018 at 11:25:31AM +0200, Geert Uytterhoeven wrote:
> Hi Jacopo,

[snip]

> > @@ -1889,6 +2077,32 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
> >         SH_PFC_PIN_GROUP(usb0_id),
> >         SH_PFC_PIN_GROUP(usb30),
> >         SH_PFC_PIN_GROUP(usb30_id),
> > +       VIN_DATA_PIN_GROUP(vin4_data_a, 8),
> > +       VIN_DATA_PIN_GROUP(vin4_data_a, 10),
> > +       VIN_DATA_PIN_GROUP(vin4_data_a, 12),
> > +       VIN_DATA_PIN_GROUP(vin4_data_a, 16),
> > +       VIN_DATA_PIN_GROUP(vin4_data_a, 20),
> > +       VIN_DATA_PIN_GROUP(vin4_data_a, 24),
> > +       VIN_DATA_PIN_GROUP(vin4_data_b, 8),
> > +       VIN_DATA_PIN_GROUP(vin4_data_b, 10),
> > +       VIN_DATA_PIN_GROUP(vin4_data_b, 12),
> > +       VIN_DATA_PIN_GROUP(vin4_data_b, 16),
> > +       VIN_DATA_PIN_GROUP(vin4_data_b, 20),
> > +       VIN_DATA_PIN_GROUP(vin4_data_b, 24),

look here...

[snip]

> >
> > +static const char * const vin4_groups[] = {
> > +       "vin4_data8_a",
> > +       "vin4_data10_a",
> > +       "vin4_data12_a",
> > +       "vin4_data16_a",
> > +       "vin4_data20_a",
> > +       "vin4_data24_a",
> > +       "vin4_data8_b",
> > +       "vin4_data10_b",
> > +       "vin4_data12_b",
> > +       "vin4_data16_b",
> > +       "vin4_data20_b",
> > +       "vin4_data24_b",

Then here.

VIN_DATA_PIN_GROUP() expands as:

#define VIN_DATA_PIN_GROUP(n, s)				\
	{							\
		.name = #n#s,					\
		.pins = n##_pins.data##s,			\
		.mux = n##_mux.data##s,				\
		.nr_pins = ARRAY_SIZE(n##_pins.data##s),	\
	}

So these groups should not be named
        "vin4_dataX_a" and
        "vin4_dataX_b"

But instead
        "vin4_data_aX" and
        "vin4_data_bX"

Am I wrong?

The only Gen3 SoC in mainline which uses the VIN data pins defined
through this macro is D3, which fortunately does not have any 'a' or
'b' group.

$ git grep vin\.*_data* arch/arm64/boot/dts/
arch/arm64/boot/dts/renesas/r8a77995-draak.dts:        groups =  "vin4_data8", "vin4_sync", "vin4_clk";

$ cat drivers/pinctrl/sh-pfc/pfc-r8a77995.c | grep "vin4_data, 8"
        VIN_DATA_PIN_GROUP(vin4_data, 8),

Going forward we might see some user of vin data groups having to
refer to "vin4_data_a8" and so on, which is not nice compared to
"vin4_data8_a".

What do you think?
In any case, this patch is indeed wrong. Or we align the group names
to what the macro produces, or change the macro, but I cannot tell how
to do that in a sane way? (introduce a new one that wants a 'group'
argument too?)

Thanks
  j

> > +       "vin4_data8_sft8",
>
> You dropped the sft8 pins, but forgot to remove the sft8 group name.
>
> > +       "vin4_sync",
> > +       "vin4_field",
> > +       "vin4_clkenb",
> > +       "vin4_clk",
> > +};
> > +
> > +static const char * const vin5_groups[] = {
> > +       "vin5_data8_a",
> > +       "vin5_data8_sft8_a",
>
> Likewise.
>
> > +       "vin5_data10_a",
> > +       "vin5_data12_a",
> > +       "vin5_data16_a",
> > +       "vin5_data8_b",
> > +       "vin5_sync_a",
> > +       "vin5_field_a",
> > +       "vin5_clkenb_a",
> > +       "vin5_clk_a",
> > +       "vin5_clk_b",
> > +};
>
> The rest looks OK to me, and matches the datasheet clarification.
>
> 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

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

Re: [PATCH v2 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[Geert Uytterhoeven]

Hi Jacopo,

On Fri, Oct 19, 2018 at 6:55 PM jacopo mondi <jacopo@jmondi.org> wrote:
>    sorry to resurect this one, but while upporting VIN pin definition
> for R8A77965 I have noticed something in this patch.
>
> Please see below.
>
> On Tue, Oct 02, 2018 at 11:25:31AM +0200, Geert Uytterhoeven wrote:
> > Hi Jacopo,
>
> [snip]
>
> > > @@ -1889,6 +2077,32 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
> > >         SH_PFC_PIN_GROUP(usb0_id),
> > >         SH_PFC_PIN_GROUP(usb30),
> > >         SH_PFC_PIN_GROUP(usb30_id),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_a, 8),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_a, 10),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_a, 12),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_a, 16),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_a, 20),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_a, 24),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_b, 8),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_b, 10),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_b, 12),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_b, 16),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_b, 20),
> > > +       VIN_DATA_PIN_GROUP(vin4_data_b, 24),
>
> look here...
>
> [snip]
>
> > >
> > > +static const char * const vin4_groups[] = {
> > > +       "vin4_data8_a",
> > > +       "vin4_data10_a",
> > > +       "vin4_data12_a",
> > > +       "vin4_data16_a",
> > > +       "vin4_data20_a",
> > > +       "vin4_data24_a",
> > > +       "vin4_data8_b",
> > > +       "vin4_data10_b",
> > > +       "vin4_data12_b",
> > > +       "vin4_data16_b",
> > > +       "vin4_data20_b",
> > > +       "vin4_data24_b",
>
> Then here.
>
> VIN_DATA_PIN_GROUP() expands as:
>
> #define VIN_DATA_PIN_GROUP(n, s)                                \
>         {                                                       \
>                 .name = #n#s,                                   \
>                 .pins = n##_pins.data##s,                       \
>                 .mux = n##_mux.data##s,                         \
>                 .nr_pins = ARRAY_SIZE(n##_pins.data##s),        \
>         }
>
> So these groups should not be named
>         "vin4_dataX_a" and
>         "vin4_dataX_b"
>
> But instead
>         "vin4_data_aX" and
>         "vin4_data_bX"
>
> Am I wrong?

Nice catch!

For consistency with other groups, they should be named "vin4_dataX_a"
and "vin4_dataX_b".

> The only Gen3 SoC in mainline which uses the VIN data pins defined
> through this macro is D3, which fortunately does not have any 'a' or
> 'b' group.
>
> $ git grep vin\.*_data* arch/arm64/boot/dts/
> arch/arm64/boot/dts/renesas/r8a77995-draak.dts:        groups =  "vin4_data8", "vin4_sync", "vin4_clk";
>
> $ cat drivers/pinctrl/sh-pfc/pfc-r8a77995.c | grep "vin4_data, 8"
>         VIN_DATA_PIN_GROUP(vin4_data, 8),
>
> Going forward we might see some user of vin data groups having to
> refer to "vin4_data_a8" and so on, which is not nice compared to
> "vin4_data8_a".
>
> What do you think?
> In any case, this patch is indeed wrong. Or we align the group names
> to what the macro produces, or change the macro, but I cannot tell how
> to do that in a sane way? (introduce a new one that wants a 'group'
> argument too?)

I only gave this a brief look, but it seems like we need more/better macros.

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

[PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Jacopo Mondi]

From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device tree.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 +++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index ae89260..0ae7bbe 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -337,6 +337,85 @@
 			status = "disabled";
 		};
 
+		csi40: csi2@feaa0000 {
+			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
+			reg = <0 0xfeaa0000 0 0x10000>;
+			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 716>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 716>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					reg = <1>;
+
+					csi40vin4: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&vin4csi40>;
+					};
+					csi40vin5: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&vin5csi40>;
+					};
+				};
+			};
+		};
+
+		vin4: video@e6ef4000 {
+			compatible = "renesas,vin-r8a77990";
+			reg = <0 0xe6ef4000 0 0x1000>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 807>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 807>;
+			renesas,id = <4>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					reg = <1>;
+
+					vin4csi40: endpoint {
+						remote-endpoint= <&csi40vin4>;
+					};
+				};
+			};
+		};
+
+		vin5: video@e6ef5000 {
+			compatible = "renesas,vin-r8a77990";
+			reg = <0 0xe6ef5000 0 0x1000>;
+			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 806>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 806>;
+			renesas,id = <5>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					reg = <1>;
+
+					vin5csi40: endpoint {
+						remote-endpoint= <&csi40vin5>;
+					};
+				};
+			};
+		};
+
 		scif2: serial@e6e88000 {
 			compatible = "renesas,scif-r8a77990",
 				     "renesas,rcar-gen3-scif", "renesas,scif";
-- 
2.7.4

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Simon Horman]

On Wed, Sep 05, 2018 at 05:29:43PM +0200, Jacopo Mondi wrote:
> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device tree.
> 
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks.

I took the liberty of re-arranging the node order to preserve the existing
sort order in the .dtsi file.

The result is as follows:


>From 053f0c7e8547095a959cbd3bbaf602388c8bbca7 Mon Sep 17 00:00:00 2001
From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Date: Wed, 5 Sep 2018 17:29:43 +0200
Subject: [PATCH] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device tree.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
[simon: sorted nodes by bus address, then IP block]
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 +++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index e3009c5f5210..c010358ba076 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -481,6 +481,54 @@
 			status = "disabled";
 		};
 
+		vin4: video@e6ef4000 {
+			compatible = "renesas,vin-r8a77990";
+			reg = <0 0xe6ef4000 0 0x1000>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 807>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 807>;
+			renesas,id = <4>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					reg = <1>;
+
+					vin4csi40: endpoint {
+						remote-endpoint= <&csi40vin4>;
+					};
+				};
+			};
+		};
+
+		vin5: video@e6ef5000 {
+			compatible = "renesas,vin-r8a77990";
+			reg = <0 0xe6ef5000 0 0x1000>;
+			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 806>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 806>;
+			renesas,id = <5>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					reg = <1>;
+
+					vin5csi40: endpoint {
+						remote-endpoint= <&csi40vin5>;
+					};
+				};
+			};
+		};
+
 		xhci0: usb@ee000000 {
 			compatible = "renesas,xhci-r8a77990",
 				     "renesas,rcar-gen3-xhci";
@@ -546,6 +594,37 @@
 			resets = <&cpg 408>;
 		};
 
+		csi40: csi2@feaa0000 {
+			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
+			reg = <0 0xfeaa0000 0 0x10000>;
+			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 716>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 716>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					reg = <1>;
+
+					csi40vin4: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&vin4csi40>;
+					};
+					csi40vin5: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&vin5csi40>;
+					};
+				};
+			};
+		};
+
 		prr: chipid@fff00044 {
 			compatible = "renesas,prr";
 			reg = <0 0xfff00044 0 4>;
-- 
2.11.0

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Simon Horman]

On Thu, Sep 06, 2018 at 11:10:33AM +0200, Simon Horman wrote:
> On Wed, Sep 05, 2018 at 05:29:43PM +0200, Jacopo Mondi wrote:
> > From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > 
> > Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device tree.
> > 
> > Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> Thanks.
> 
> I took the liberty of re-arranging the node order to preserve the existing
> sort order in the .dtsi file.
> 
> The result is as follows:
> 
> 
> >From 053f0c7e8547095a959cbd3bbaf602388c8bbca7 Mon Sep 17 00:00:00 2001
> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Date: Wed, 5 Sep 2018 17:29:43 +0200
> Subject: [PATCH] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes
> 
> Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device tree.
> 
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> [simon: sorted nodes by bus address, then IP block]
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

Sorry, I missread Laurent's reply to 7/7 and now see that he has not
supplied tags for this patch. I have not queued up this patch after all and
am waiting for review.

Sorry for my confusion.

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wednesday, 5 September 2018 18:29:43 EEST Jacopo Mondi wrote:
> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device tree.
> 
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 ++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index ae89260..0ae7bbe 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> @@ -337,6 +337,85 @@
>  			status = "disabled";
>  		};
> 
> +		csi40: csi2@feaa0000 {

I believe Simon would like to keep the nodes sorted by address

> +			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
> +			reg = <0 0xfeaa0000 0 0x10000>;

0x10000 seems pretty large to me.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 716>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 716>;
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					reg = <1>;
> +
> +					csi40vin4: endpoint@0 {
> +						reg = <0>;
> +						remote-endpoint = <&vin4csi40>;
> +					};
> +					csi40vin5: endpoint@1 {
> +						reg = <1>;
> +						remote-endpoint = <&vin5csi40>;
> +					};
> +				};
> +			};
> +		};
> +
> +		vin4: video@e6ef4000 {
> +			compatible = "renesas,vin-r8a77990";
> +			reg = <0 0xe6ef4000 0 0x1000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 807>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 807>;
> +			renesas,id = <4>;
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@1 {
> +					reg = <1>;
> +
> +					vin4csi40: endpoint {
> +						remote-endpoint= <&csi40vin4>;
> +					};
> +				};
> +			};
> +		};
> +
> +		vin5: video@e6ef5000 {
> +			compatible = "renesas,vin-r8a77990";
> +			reg = <0 0xe6ef5000 0 0x1000>;
> +			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 806>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 806>;
> +			renesas,id = <5>;
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@1 {
> +					reg = <1>;
> +
> +					vin5csi40: endpoint {
> +						remote-endpoint= <&csi40vin5>;
> +					};
> +				};
> +			};
> +		};
> +
>  		scif2: serial@e6e88000 {
>  			compatible = "renesas,scif-r8a77990",
>  				     "renesas,rcar-gen3-scif", "renesas,scif";


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Laurent Pinchart]

Hi Jacopo,

On Monday, 10 September 2018 17:12:30 EEST Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Wednesday, 5 September 2018 18:29:43 EEST Jacopo Mondi wrote:
> > From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > 
> > Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device
> > tree.
> > 
> > Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > 
> >  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 ++++++++++++++++++++++++++
> >  1 file changed, 79 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index ae89260..0ae7bbe 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > @@ -337,6 +337,85 @@
> > 
> >  			status = "disabled";
> >  		
> >  		};
> > 
> > +		csi40: csi2@feaa0000 {
> 
> I believe Simon would like to keep the nodes sorted by address
> 
> > +			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
> > +			reg = <0 0xfeaa0000 0 0x10000>;
> 
> 0x10000 seems pretty large to me.
> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > +			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&cpg CPG_MOD 716>;
> > +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > +			resets = <&cpg 716>;
> > +			status = "disabled";
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				port@1 {
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > +
> > +					reg = <1>;
> > +
> > +					csi40vin4: endpoint@0 {
> > +						reg = <0>;
> > +						remote-endpoint = <&vin4csi40>;
> > +					};
> > +					csi40vin5: endpoint@1 {
> > +						reg = <1>;
> > +						remote-endpoint = <&vin5csi40>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +
> > +		vin4: video@e6ef4000 {
> > +			compatible = "renesas,vin-r8a77990";
> > +			reg = <0 0xe6ef4000 0 0x1000>;
> > +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&cpg CPG_MOD 807>;
> > +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > +			resets = <&cpg 807>;
> > +			renesas,id = <4>;
> > +			status = "disabled";
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				port@1 {
> > +					reg = <1>;
> > +
> > +					vin4csi40: endpoint {
> > +						remote-endpoint= <&csi40vin4>;

I've just realized that this endpoint has to be numbered, otherwise MC links 
won't be created.

> > +					};
> > +				};
> > +			};
> > +		};
> > +
> > +		vin5: video@e6ef5000 {
> > +			compatible = "renesas,vin-r8a77990";
> > +			reg = <0 0xe6ef5000 0 0x1000>;
> > +			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&cpg CPG_MOD 806>;
> > +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > +			resets = <&cpg 806>;
> > +			renesas,id = <5>;
> > +			status = "disabled";
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				port@1 {
> > +					reg = <1>;
> > +
> > +					vin5csi40: endpoint {
> > +						remote-endpoint= <&csi40vin5>;

Same here.

> > +					};
> > +				};
> > +			};
> > +		};
> > +
> > 
> >  		scif2: serial@e6e88000 {
> >  		
> >  			compatible = "renesas,scif-r8a77990",
> >  			
> >  				     "renesas,rcar-gen3-scif", "renesas,scif";


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[jacopo mondi]

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

On Mon, Sep 10, 2018 at 05:12:30PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wednesday, 5 September 2018 18:29:43 EEST Jacopo Mondi wrote:
> > From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> >
> > Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device tree.
> >
> > Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 ++++++++++++++++++++++++++++
> >  1 file changed, 79 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index ae89260..0ae7bbe 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > @@ -337,6 +337,85 @@
> >  			status = "disabled";
> >  		};
> >
> > +		csi40: csi2@feaa0000 {
>
> I believe Simon would like to keep the nodes sorted by address
>
> > +			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
> > +			reg = <0 0xfeaa0000 0 0x10000>;
>
> 0x10000 seems pretty large to me.

It seems to me that all Gen3 SoC have this lenght specified

$git grep -A 10 'csi[2|4][0|1]: csi' arch/arm64/boot/dts/renesas/ | grep reg
arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfea80000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeab0000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfea80000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfea80000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a77970.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeab0000 0 0x10000>;
arch/arm64/boot/dts/renesas/r8a77990.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;

Am I missing something?

>
> Apart from that,

I will include the upporting of the following patch to fix the VIN
endpoint numbering in forthcoming v3:
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=1b1b73f7558d867d72e198901b84bec1e6ef1405

As this has already been applied to simon's tree.

Thanks
  j

>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > +			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&cpg CPG_MOD 716>;
> > +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > +			resets = <&cpg 716>;
> > +			status = "disabled";
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				port@1 {
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > +
> > +					reg = <1>;
> > +
> > +					csi40vin4: endpoint@0 {
> > +						reg = <0>;
> > +						remote-endpoint = <&vin4csi40>;
> > +					};
> > +					csi40vin5: endpoint@1 {
> > +						reg = <1>;
> > +						remote-endpoint = <&vin5csi40>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +
> > +		vin4: video@e6ef4000 {
> > +			compatible = "renesas,vin-r8a77990";
> > +			reg = <0 0xe6ef4000 0 0x1000>;
> > +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&cpg CPG_MOD 807>;
> > +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > +			resets = <&cpg 807>;
> > +			renesas,id = <4>;
> > +			status = "disabled";
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				port@1 {
> > +					reg = <1>;
> > +
> > +					vin4csi40: endpoint {
> > +						remote-endpoint= <&csi40vin4>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +
> > +		vin5: video@e6ef5000 {
> > +			compatible = "renesas,vin-r8a77990";
> > +			reg = <0 0xe6ef5000 0 0x1000>;
> > +			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&cpg CPG_MOD 806>;
> > +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > +			resets = <&cpg 806>;
> > +			renesas,id = <5>;
> > +			status = "disabled";
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				port@1 {
> > +					reg = <1>;
> > +
> > +					vin5csi40: endpoint {
> > +						remote-endpoint= <&csi40vin5>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +
> >  		scif2: serial@e6e88000 {
> >  			compatible = "renesas,scif-r8a77990",
> >  				     "renesas,rcar-gen3-scif", "renesas,scif";
>
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

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

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Laurent Pinchart]

Hi Jacopo,

On Tuesday, 30 October 2018 12:14:31 EET jacopo mondi wrote:
> On Mon, Sep 10, 2018 at 05:12:30PM +0300, Laurent Pinchart wrote:
> > On Wednesday, 5 September 2018 18:29:43 EEST Jacopo Mondi wrote:
> >> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> >> 
> >> Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device
> >> tree.
> >> 
> >> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> >> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >> ---
> >> 
> >>  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 ++++++++++++++++++++++++
> >>  1 file changed, 79 insertions(+)
> >> 
> >> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> >> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index ae89260..0ae7bbe
> >> 100644
> >> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> >> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> >> @@ -337,6 +337,85 @@
> >> 
> >>  			status = "disabled";
> >>  		
> >>  		};
> >> 
> >> +		csi40: csi2@feaa0000 {
> > 
> > I believe Simon would like to keep the nodes sorted by address
> > 
> >> +			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
> >> +			reg = <0 0xfeaa0000 0 0x10000>;
> > 
> > 0x10000 seems pretty large to me.
> 
> It seems to me that all Gen3 SoC have this lenght specified
> 
> $git grep -A 10 'csi[2|4][0|1]: csi' arch/arm64/boot/dts/renesas/ | grep reg
> arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeab0000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfea80000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a77970.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeab0000 0 0x10000>;
> arch/arm64/boot/dts/renesas/r8a77990.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> 
> Am I missing something?

Cargo-cult programming ? :-) This will likely not hurt, but such a large 
memory area is not required, and we'll save a bit of memory if we reduce the 
mapping from 64kB to 4kB (or less)

> > Apart from that,
> 
> I will include the upporting of the following patch to fix the VIN
> endpoint numbering in forthcoming v3:
> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit
> /?id=1b1b73f7558d867d72e198901b84bec1e6ef1405
> 
> As this has already been applied to simon's tree.
> 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> >> +			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
> >> +			clocks = <&cpg CPG_MOD 716>;
> >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> >> +			resets = <&cpg 716>;
> >> +			status = "disabled";
> >> +
> >> +			ports {
> >> +				#address-cells = <1>;
> >> +				#size-cells = <0>;
> >> +
> >> +				port@1 {
> >> +					#address-cells = <1>;
> >> +					#size-cells = <0>;
> >> +
> >> +					reg = <1>;
> >> +
> >> +					csi40vin4: endpoint@0 {
> >> +						reg = <0>;
> >> +						remote-endpoint = <&vin4csi40>;
> >> +					};
> >> +					csi40vin5: endpoint@1 {
> >> +						reg = <1>;
> >> +						remote-endpoint = <&vin5csi40>;
> >> +					};
> >> +				};
> >> +			};
> >> +		};
> >> +
> >> +		vin4: video@e6ef4000 {
> >> +			compatible = "renesas,vin-r8a77990";
> >> +			reg = <0 0xe6ef4000 0 0x1000>;
> >> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> >> +			clocks = <&cpg CPG_MOD 807>;
> >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> >> +			resets = <&cpg 807>;
> >> +			renesas,id = <4>;
> >> +			status = "disabled";
> >> +
> >> +			ports {
> >> +				#address-cells = <1>;
> >> +				#size-cells = <0>;
> >> +
> >> +				port@1 {
> >> +					reg = <1>;
> >> +
> >> +					vin4csi40: endpoint {
> >> +						remote-endpoint= <&csi40vin4>;
> >> +					};
> >> +				};
> >> +			};
> >> +		};
> >> +
> >> +		vin5: video@e6ef5000 {
> >> +			compatible = "renesas,vin-r8a77990";
> >> +			reg = <0 0xe6ef5000 0 0x1000>;
> >> +			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
> >> +			clocks = <&cpg CPG_MOD 806>;
> >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> >> +			resets = <&cpg 806>;
> >> +			renesas,id = <5>;
> >> +			status = "disabled";
> >> +
> >> +			ports {
> >> +				#address-cells = <1>;
> >> +				#size-cells = <0>;
> >> +
> >> +				port@1 {
> >> +					reg = <1>;
> >> +
> >> +					vin5csi40: endpoint {
> >> +						remote-endpoint= <&csi40vin5>;
> >> +					};
> >> +				};
> >> +			};
> >> +		};
> >> +
> >>  		scif2: serial@e6e88000 {
> >>  			compatible = "renesas,scif-r8a77990",
> >>  				     "renesas,rcar-gen3-scif", "renesas,scif";

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Simon Horman]

On Tue, Oct 30, 2018 at 02:57:59PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> On Tuesday, 30 October 2018 12:14:31 EET jacopo mondi wrote:
> > On Mon, Sep 10, 2018 at 05:12:30PM +0300, Laurent Pinchart wrote:
> > > On Wednesday, 5 September 2018 18:29:43 EEST Jacopo Mondi wrote:
> > >> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > >> 
> > >> Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device
> > >> tree.
> > >> 
> > >> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > >> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > >> ---
> > >> 
> > >>  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 ++++++++++++++++++++++++
> > >>  1 file changed, 79 insertions(+)
> > >> 
> > >> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > >> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index ae89260..0ae7bbe
> > >> 100644
> > >> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > >> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > >> @@ -337,6 +337,85 @@
> > >> 
> > >>  			status = "disabled";
> > >>  		
> > >>  		};
> > >> 
> > >> +		csi40: csi2@feaa0000 {
> > > 
> > > I believe Simon would like to keep the nodes sorted by address
> > > 
> > >> +			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
> > >> +			reg = <0 0xfeaa0000 0 0x10000>;
> > > 
> > > 0x10000 seems pretty large to me.
> > 
> > It seems to me that all Gen3 SoC have this lenght specified
> > 
> > $git grep -A 10 'csi[2|4][0|1]: csi' arch/arm64/boot/dts/renesas/ | grep reg
> > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeab0000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfea80000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a77970.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeab0000 0 0x10000>;
> > arch/arm64/boot/dts/renesas/r8a77990.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > 
> > Am I missing something?
> 
> Cargo-cult programming ? :-) This will likely not hurt, but such a large 
> memory area is not required, and we'll save a bit of memory if we reduce the 
> mapping from 64kB to 4kB (or less)

Can we please update this patch, and existing dtsi files,
to use an appropriately small register window?

> 
> > > Apart from that,
> > 
> > I will include the upporting of the following patch to fix the VIN
> > endpoint numbering in forthcoming v3:
> > https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit
> > /?id=1b1b73f7558d867d72e198901b84bec1e6ef1405
> > 
> > As this has already been applied to simon's tree.
> > 
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 
> > >> +			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
> > >> +			clocks = <&cpg CPG_MOD 716>;
> > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > >> +			resets = <&cpg 716>;
> > >> +			status = "disabled";
> > >> +
> > >> +			ports {
> > >> +				#address-cells = <1>;
> > >> +				#size-cells = <0>;
> > >> +
> > >> +				port@1 {
> > >> +					#address-cells = <1>;
> > >> +					#size-cells = <0>;
> > >> +
> > >> +					reg = <1>;
> > >> +
> > >> +					csi40vin4: endpoint@0 {
> > >> +						reg = <0>;
> > >> +						remote-endpoint = <&vin4csi40>;
> > >> +					};
> > >> +					csi40vin5: endpoint@1 {
> > >> +						reg = <1>;
> > >> +						remote-endpoint = <&vin5csi40>;
> > >> +					};
> > >> +				};
> > >> +			};
> > >> +		};
> > >> +
> > >> +		vin4: video@e6ef4000 {
> > >> +			compatible = "renesas,vin-r8a77990";
> > >> +			reg = <0 0xe6ef4000 0 0x1000>;
> > >> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > >> +			clocks = <&cpg CPG_MOD 807>;
> > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > >> +			resets = <&cpg 807>;
> > >> +			renesas,id = <4>;
> > >> +			status = "disabled";
> > >> +
> > >> +			ports {
> > >> +				#address-cells = <1>;
> > >> +				#size-cells = <0>;
> > >> +
> > >> +				port@1 {
> > >> +					reg = <1>;
> > >> +
> > >> +					vin4csi40: endpoint {
> > >> +						remote-endpoint= <&csi40vin4>;
> > >> +					};
> > >> +				};
> > >> +			};
> > >> +		};
> > >> +
> > >> +		vin5: video@e6ef5000 {
> > >> +			compatible = "renesas,vin-r8a77990";
> > >> +			reg = <0 0xe6ef5000 0 0x1000>;
> > >> +			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
> > >> +			clocks = <&cpg CPG_MOD 806>;
> > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > >> +			resets = <&cpg 806>;
> > >> +			renesas,id = <5>;
> > >> +			status = "disabled";
> > >> +
> > >> +			ports {
> > >> +				#address-cells = <1>;
> > >> +				#size-cells = <0>;
> > >> +
> > >> +				port@1 {
> > >> +					reg = <1>;
> > >> +
> > >> +					vin5csi40: endpoint {
> > >> +						remote-endpoint= <&csi40vin5>;
> > >> +					};
> > >> +				};
> > >> +			};
> > >> +		};
> > >> +
> > >>  		scif2: serial@e6e88000 {
> > >>  			compatible = "renesas,scif-r8a77990",
> > >>  				     "renesas,rcar-gen3-scif", "renesas,scif";
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[jacopo mondi]

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

Hi Simon,

On Wed, Oct 31, 2018 at 01:48:13PM +0100, Simon Horman wrote:
> On Tue, Oct 30, 2018 at 02:57:59PM +0200, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > On Tuesday, 30 October 2018 12:14:31 EET jacopo mondi wrote:
> > > On Mon, Sep 10, 2018 at 05:12:30PM +0300, Laurent Pinchart wrote:
> > > > On Wednesday, 5 September 2018 18:29:43 EEST Jacopo Mondi wrote:
> > > >> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > > >>
> > > >> Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device
> > > >> tree.
> > > >>
> > > >> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > > >> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > > >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > >> ---
> > > >>
> > > >>  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 ++++++++++++++++++++++++
> > > >>  1 file changed, 79 insertions(+)
> > > >>
> > > >> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > >> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index ae89260..0ae7bbe
> > > >> 100644
> > > >> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > >> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > >> @@ -337,6 +337,85 @@
> > > >>
> > > >>  			status = "disabled";
> > > >>
> > > >>  		};
> > > >>
> > > >> +		csi40: csi2@feaa0000 {
> > > >
> > > > I believe Simon would like to keep the nodes sorted by address
> > > >
> > > >> +			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
> > > >> +			reg = <0 0xfeaa0000 0 0x10000>;
> > > >
> > > > 0x10000 seems pretty large to me.
> > >
> > > It seems to me that all Gen3 SoC have this lenght specified
> > >
> > > $git grep -A 10 'csi[2|4][0|1]: csi' arch/arm64/boot/dts/renesas/ | grep reg
> > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeab0000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfea80000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a77970.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeab0000 0 0x10000>;
> > > arch/arm64/boot/dts/renesas/r8a77990.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > >
> > > Am I missing something?
> >
> > Cargo-cult programming ? :-) This will likely not hurt, but such a large
> > memory area is not required, and we'll save a bit of memory if we reduce the
> > mapping from 64kB to 4kB (or less)
>
> Can we please update this patch, and existing dtsi files,
> to use an appropriately small register window?
>

What if we keep this one as it is and we change all the DTSIs in one
go?

> >
> > > > Apart from that,
> > >
> > > I will include the upporting of the following patch to fix the VIN
> > > endpoint numbering in forthcoming v3:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit
> > > /?id=1b1b73f7558d867d72e198901b84bec1e6ef1405
> > >
> > > As this has already been applied to simon's tree.
> > >
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > >> +			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
> > > >> +			clocks = <&cpg CPG_MOD 716>;
> > > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > > >> +			resets = <&cpg 716>;
> > > >> +			status = "disabled";
> > > >> +
> > > >> +			ports {
> > > >> +				#address-cells = <1>;
> > > >> +				#size-cells = <0>;
> > > >> +
> > > >> +				port@1 {
> > > >> +					#address-cells = <1>;
> > > >> +					#size-cells = <0>;
> > > >> +
> > > >> +					reg = <1>;
> > > >> +
> > > >> +					csi40vin4: endpoint@0 {
> > > >> +						reg = <0>;
> > > >> +						remote-endpoint = <&vin4csi40>;
> > > >> +					};
> > > >> +					csi40vin5: endpoint@1 {
> > > >> +						reg = <1>;
> > > >> +						remote-endpoint = <&vin5csi40>;
> > > >> +					};
> > > >> +				};
> > > >> +			};
> > > >> +		};
> > > >> +
> > > >> +		vin4: video@e6ef4000 {
> > > >> +			compatible = "renesas,vin-r8a77990";
> > > >> +			reg = <0 0xe6ef4000 0 0x1000>;
> > > >> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > >> +			clocks = <&cpg CPG_MOD 807>;
> > > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > > >> +			resets = <&cpg 807>;
> > > >> +			renesas,id = <4>;
> > > >> +			status = "disabled";
> > > >> +
> > > >> +			ports {
> > > >> +				#address-cells = <1>;
> > > >> +				#size-cells = <0>;
> > > >> +
> > > >> +				port@1 {
> > > >> +					reg = <1>;
> > > >> +
> > > >> +					vin4csi40: endpoint {
> > > >> +						remote-endpoint= <&csi40vin4>;
> > > >> +					};
> > > >> +				};
> > > >> +			};
> > > >> +		};
> > > >> +
> > > >> +		vin5: video@e6ef5000 {
> > > >> +			compatible = "renesas,vin-r8a77990";
> > > >> +			reg = <0 0xe6ef5000 0 0x1000>;
> > > >> +			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
> > > >> +			clocks = <&cpg CPG_MOD 806>;
> > > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > > >> +			resets = <&cpg 806>;
> > > >> +			renesas,id = <5>;
> > > >> +			status = "disabled";
> > > >> +
> > > >> +			ports {
> > > >> +				#address-cells = <1>;
> > > >> +				#size-cells = <0>;
> > > >> +
> > > >> +				port@1 {
> > > >> +					reg = <1>;
> > > >> +
> > > >> +					vin5csi40: endpoint {
> > > >> +						remote-endpoint= <&csi40vin5>;
> > > >> +					};
> > > >> +				};
> > > >> +			};
> > > >> +		};
> > > >> +
> > > >>  		scif2: serial@e6e88000 {
> > > >>  			compatible = "renesas,scif-r8a77990",
> > > >>  				     "renesas,rcar-gen3-scif", "renesas,scif";
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> >
> >

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

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Simon Horman]

On Wed, Oct 31, 2018 at 02:18:40PM +0100, jacopo mondi wrote:
> Hi Simon,
> 
> On Wed, Oct 31, 2018 at 01:48:13PM +0100, Simon Horman wrote:
> > On Tue, Oct 30, 2018 at 02:57:59PM +0200, Laurent Pinchart wrote:
> > > Hi Jacopo,
> > >
> > > On Tuesday, 30 October 2018 12:14:31 EET jacopo mondi wrote:
> > > > On Mon, Sep 10, 2018 at 05:12:30PM +0300, Laurent Pinchart wrote:
> > > > > On Wednesday, 5 September 2018 18:29:43 EEST Jacopo Mondi wrote:
> > > > >> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > > > >>
> > > > >> Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device
> > > > >> tree.
> > > > >>
> > > > >> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > > > >> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > > > >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > >> ---
> > > > >>
> > > > >>  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 ++++++++++++++++++++++++
> > > > >>  1 file changed, 79 insertions(+)
> > > > >>
> > > > >> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > > >> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index ae89260..0ae7bbe
> > > > >> 100644
> > > > >> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > > >> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > > >> @@ -337,6 +337,85 @@
> > > > >>
> > > > >>  			status = "disabled";
> > > > >>
> > > > >>  		};
> > > > >>
> > > > >> +		csi40: csi2@feaa0000 {
> > > > >
> > > > > I believe Simon would like to keep the nodes sorted by address
> > > > >
> > > > >> +			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
> > > > >> +			reg = <0 0xfeaa0000 0 0x10000>;
> > > > >
> > > > > 0x10000 seems pretty large to me.
> > > >
> > > > It seems to me that all Gen3 SoC have this lenght specified
> > > >
> > > > $git grep -A 10 'csi[2|4][0|1]: csi' arch/arm64/boot/dts/renesas/ | grep reg
> > > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeab0000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfea80000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a77970.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeab0000 0 0x10000>;
> > > > arch/arm64/boot/dts/renesas/r8a77990.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > >
> > > > Am I missing something?
> > >
> > > Cargo-cult programming ? :-) This will likely not hurt, but such a large
> > > memory area is not required, and we'll save a bit of memory if we reduce the
> > > mapping from 64kB to 4kB (or less)
> >
> > Can we please update this patch, and existing dtsi files,
> > to use an appropriately small register window?
> >
> 
> What if we keep this one as it is and we change all the DTSIs in one
> go?

I would rather we correct this patch than add it with a known problem.

> 
> > >
> > > > > Apart from that,
> > > >
> > > > I will include the upporting of the following patch to fix the VIN
> > > > endpoint numbering in forthcoming v3:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit
> > > > /?id=1b1b73f7558d867d72e198901b84bec1e6ef1405
> > > >
> > > > As this has already been applied to simon's tree.
> > > >
> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > >
> > > > >> +			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
> > > > >> +			clocks = <&cpg CPG_MOD 716>;
> > > > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > > > >> +			resets = <&cpg 716>;
> > > > >> +			status = "disabled";
> > > > >> +
> > > > >> +			ports {
> > > > >> +				#address-cells = <1>;
> > > > >> +				#size-cells = <0>;
> > > > >> +
> > > > >> +				port@1 {
> > > > >> +					#address-cells = <1>;
> > > > >> +					#size-cells = <0>;
> > > > >> +
> > > > >> +					reg = <1>;
> > > > >> +
> > > > >> +					csi40vin4: endpoint@0 {
> > > > >> +						reg = <0>;
> > > > >> +						remote-endpoint = <&vin4csi40>;
> > > > >> +					};
> > > > >> +					csi40vin5: endpoint@1 {
> > > > >> +						reg = <1>;
> > > > >> +						remote-endpoint = <&vin5csi40>;
> > > > >> +					};
> > > > >> +				};
> > > > >> +			};
> > > > >> +		};
> > > > >> +
> > > > >> +		vin4: video@e6ef4000 {
> > > > >> +			compatible = "renesas,vin-r8a77990";
> > > > >> +			reg = <0 0xe6ef4000 0 0x1000>;
> > > > >> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > > >> +			clocks = <&cpg CPG_MOD 807>;
> > > > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > > > >> +			resets = <&cpg 807>;
> > > > >> +			renesas,id = <4>;
> > > > >> +			status = "disabled";
> > > > >> +
> > > > >> +			ports {
> > > > >> +				#address-cells = <1>;
> > > > >> +				#size-cells = <0>;
> > > > >> +
> > > > >> +				port@1 {
> > > > >> +					reg = <1>;
> > > > >> +
> > > > >> +					vin4csi40: endpoint {
> > > > >> +						remote-endpoint= <&csi40vin4>;
> > > > >> +					};
> > > > >> +				};
> > > > >> +			};
> > > > >> +		};
> > > > >> +
> > > > >> +		vin5: video@e6ef5000 {
> > > > >> +			compatible = "renesas,vin-r8a77990";
> > > > >> +			reg = <0 0xe6ef5000 0 0x1000>;
> > > > >> +			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
> > > > >> +			clocks = <&cpg CPG_MOD 806>;
> > > > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > > > >> +			resets = <&cpg 806>;
> > > > >> +			renesas,id = <5>;
> > > > >> +			status = "disabled";
> > > > >> +
> > > > >> +			ports {
> > > > >> +				#address-cells = <1>;
> > > > >> +				#size-cells = <0>;
> > > > >> +
> > > > >> +				port@1 {
> > > > >> +					reg = <1>;
> > > > >> +
> > > > >> +					vin5csi40: endpoint {
> > > > >> +						remote-endpoint= <&csi40vin5>;
> > > > >> +					};
> > > > >> +				};
> > > > >> +			};
> > > > >> +		};
> > > > >> +
> > > > >>  		scif2: serial@e6e88000 {
> > > > >>  			compatible = "renesas,scif-r8a77990",
> > > > >>  				     "renesas,rcar-gen3-scif", "renesas,scif";
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> > >
> > >
> > >

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[jacopo mondi]

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

Hi Simon,

On Wed, Oct 31, 2018 at 03:37:39PM +0100, Simon Horman wrote:
> On Wed, Oct 31, 2018 at 02:18:40PM +0100, jacopo mondi wrote:
> > Hi Simon,
> >
> > On Wed, Oct 31, 2018 at 01:48:13PM +0100, Simon Horman wrote:
> > > On Tue, Oct 30, 2018 at 02:57:59PM +0200, Laurent Pinchart wrote:
> > > > Hi Jacopo,
> > > >
> > > > On Tuesday, 30 October 2018 12:14:31 EET jacopo mondi wrote:
> > > > > On Mon, Sep 10, 2018 at 05:12:30PM +0300, Laurent Pinchart wrote:
> > > > > > On Wednesday, 5 September 2018 18:29:43 EEST Jacopo Mondi wrote:
> > > > > >> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > > > > >>
> > > > > >> Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device
> > > > > >> tree.
> > > > > >>
> > > > > >> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > > > > >> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > > > > >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > > >> ---
> > > > > >>
> > > > > >>  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 ++++++++++++++++++++++++
> > > > > >>  1 file changed, 79 insertions(+)
> > > > > >>
> > > > > >> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > > > >> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index ae89260..0ae7bbe
> > > > > >> 100644
> > > > > >> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > > > >> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > > > >> @@ -337,6 +337,85 @@
> > > > > >>
> > > > > >>  			status = "disabled";
> > > > > >>
> > > > > >>  		};
> > > > > >>
> > > > > >> +		csi40: csi2@feaa0000 {
> > > > > >
> > > > > > I believe Simon would like to keep the nodes sorted by address
> > > > > >
> > > > > >> +			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
> > > > > >> +			reg = <0 0xfeaa0000 0 0x10000>;
> > > > > >
> > > > > > 0x10000 seems pretty large to me.
> > > > >
> > > > > It seems to me that all Gen3 SoC have this lenght specified
> > > > >
> > > > > $git grep -A 10 'csi[2|4][0|1]: csi' arch/arm64/boot/dts/renesas/ | grep reg
> > > > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeab0000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfea80000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a77970.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeab0000 0 0x10000>;
> > > > > arch/arm64/boot/dts/renesas/r8a77990.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > >
> > > > > Am I missing something?
> > > >
> > > > Cargo-cult programming ? :-) This will likely not hurt, but such a large
> > > > memory area is not required, and we'll save a bit of memory if we reduce the
> > > > mapping from 64kB to 4kB (or less)
> > >
> > > Can we please update this patch, and existing dtsi files,
> > > to use an appropriately small register window?
> > >
> >
> > What if we keep this one as it is and we change all the DTSIs in one
> > go?
>
> I would rather we correct this patch than add it with a known problem.

Sorry, I was confused. This patch is already in v4.20.

If we want to fix this, a single follow-up patch that changes the
memory area size for all SoCs is required.

Thanks
   j

>
> >
> > > >
> > > > > > Apart from that,
> > > > >
> > > > > I will include the upporting of the following patch to fix the VIN
> > > > > endpoint numbering in forthcoming v3:
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit
> > > > > /?id=1b1b73f7558d867d72e198901b84bec1e6ef1405
> > > > >
> > > > > As this has already been applied to simon's tree.
> > > > >
> > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > >
> > > > > >> +			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
> > > > > >> +			clocks = <&cpg CPG_MOD 716>;
> > > > > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > > > > >> +			resets = <&cpg 716>;
> > > > > >> +			status = "disabled";
> > > > > >> +
> > > > > >> +			ports {
> > > > > >> +				#address-cells = <1>;
> > > > > >> +				#size-cells = <0>;
> > > > > >> +
> > > > > >> +				port@1 {
> > > > > >> +					#address-cells = <1>;
> > > > > >> +					#size-cells = <0>;
> > > > > >> +
> > > > > >> +					reg = <1>;
> > > > > >> +
> > > > > >> +					csi40vin4: endpoint@0 {
> > > > > >> +						reg = <0>;
> > > > > >> +						remote-endpoint = <&vin4csi40>;
> > > > > >> +					};
> > > > > >> +					csi40vin5: endpoint@1 {
> > > > > >> +						reg = <1>;
> > > > > >> +						remote-endpoint = <&vin5csi40>;
> > > > > >> +					};
> > > > > >> +				};
> > > > > >> +			};
> > > > > >> +		};
> > > > > >> +
> > > > > >> +		vin4: video@e6ef4000 {
> > > > > >> +			compatible = "renesas,vin-r8a77990";
> > > > > >> +			reg = <0 0xe6ef4000 0 0x1000>;
> > > > > >> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > > > >> +			clocks = <&cpg CPG_MOD 807>;
> > > > > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > > > > >> +			resets = <&cpg 807>;
> > > > > >> +			renesas,id = <4>;
> > > > > >> +			status = "disabled";
> > > > > >> +
> > > > > >> +			ports {
> > > > > >> +				#address-cells = <1>;
> > > > > >> +				#size-cells = <0>;
> > > > > >> +
> > > > > >> +				port@1 {
> > > > > >> +					reg = <1>;
> > > > > >> +
> > > > > >> +					vin4csi40: endpoint {
> > > > > >> +						remote-endpoint= <&csi40vin4>;
> > > > > >> +					};
> > > > > >> +				};
> > > > > >> +			};
> > > > > >> +		};
> > > > > >> +
> > > > > >> +		vin5: video@e6ef5000 {
> > > > > >> +			compatible = "renesas,vin-r8a77990";
> > > > > >> +			reg = <0 0xe6ef5000 0 0x1000>;
> > > > > >> +			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
> > > > > >> +			clocks = <&cpg CPG_MOD 806>;
> > > > > >> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> > > > > >> +			resets = <&cpg 806>;
> > > > > >> +			renesas,id = <5>;
> > > > > >> +			status = "disabled";
> > > > > >> +
> > > > > >> +			ports {
> > > > > >> +				#address-cells = <1>;
> > > > > >> +				#size-cells = <0>;
> > > > > >> +
> > > > > >> +				port@1 {
> > > > > >> +					reg = <1>;
> > > > > >> +
> > > > > >> +					vin5csi40: endpoint {
> > > > > >> +						remote-endpoint= <&csi40vin5>;
> > > > > >> +					};
> > > > > >> +				};
> > > > > >> +			};
> > > > > >> +		};
> > > > > >> +
> > > > > >>  		scif2: serial@e6e88000 {
> > > > > >>  			compatible = "renesas,scif-r8a77990",
> > > > > >>  				     "renesas,rcar-gen3-scif", "renesas,scif";
> > > >
> > > > --
> > > > Regards,
> > > >
> > > > Laurent Pinchart
> > > >
> > > >
> > > >
>
>

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

Re: [PATCH v2 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Simon Horman]

On Mon, Nov 05, 2018 at 11:32:37AM +0100, jacopo mondi wrote:
> Hi Simon,
> 
> On Wed, Oct 31, 2018 at 03:37:39PM +0100, Simon Horman wrote:
> > On Wed, Oct 31, 2018 at 02:18:40PM +0100, jacopo mondi wrote:
> > > Hi Simon,
> > >
> > > On Wed, Oct 31, 2018 at 01:48:13PM +0100, Simon Horman wrote:
> > > > On Tue, Oct 30, 2018 at 02:57:59PM +0200, Laurent Pinchart wrote:
> > > > > Hi Jacopo,
> > > > >
> > > > > On Tuesday, 30 October 2018 12:14:31 EET jacopo mondi wrote:
> > > > > > On Mon, Sep 10, 2018 at 05:12:30PM +0300, Laurent Pinchart wrote:
> > > > > > > On Wednesday, 5 September 2018 18:29:43 EEST Jacopo Mondi wrote:
> > > > > > >> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > > > > > >>
> > > > > > >> Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device
> > > > > > >> tree.
> > > > > > >>
> > > > > > >> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > > > > > >> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > > > > > >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > > > >> ---
> > > > > > >>
> > > > > > >>  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 ++++++++++++++++++++++++
> > > > > > >>  1 file changed, 79 insertions(+)
> > > > > > >>
> > > > > > >> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > > > > >> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index ae89260..0ae7bbe
> > > > > > >> 100644
> > > > > > >> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > > > > >> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> > > > > > >> @@ -337,6 +337,85 @@
> > > > > > >>
> > > > > > >>  			status = "disabled";
> > > > > > >>
> > > > > > >>  		};
> > > > > > >>
> > > > > > >> +		csi40: csi2@feaa0000 {
> > > > > > >
> > > > > > > I believe Simon would like to keep the nodes sorted by address
> > > > > > >
> > > > > > >> +			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
> > > > > > >> +			reg = <0 0xfeaa0000 0 0x10000>;
> > > > > > >
> > > > > > > 0x10000 seems pretty large to me.
> > > > > >
> > > > > > It seems to me that all Gen3 SoC have this lenght specified
> > > > > >
> > > > > > $git grep -A 10 'csi[2|4][0|1]: csi' arch/arm64/boot/dts/renesas/ | grep reg
> > > > > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a7795.dtsi-    reg = <0 0xfeab0000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfea80000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a7796.dtsi-    reg = <0 0xfeaa0000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfea80000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a77965.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a77970.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a77980.dtsi-   reg = <0 0xfeab0000 0 0x10000>;
> > > > > > arch/arm64/boot/dts/renesas/r8a77990.dtsi-   reg = <0 0xfeaa0000 0 0x10000>;
> > > > > >
> > > > > > Am I missing something?
> > > > >
> > > > > Cargo-cult programming ? :-) This will likely not hurt, but such a large
> > > > > memory area is not required, and we'll save a bit of memory if we reduce the
> > > > > mapping from 64kB to 4kB (or less)
> > > >
> > > > Can we please update this patch, and existing dtsi files,
> > > > to use an appropriately small register window?
> > > >
> > >
> > > What if we keep this one as it is and we change all the DTSIs in one
> > > go?
> >
> > I would rather we correct this patch than add it with a known problem.
> 
> Sorry, I was confused. This patch is already in v4.20.
> 
> If we want to fix this, a single follow-up patch that changes the
> memory area size for all SoCs is required.

Thanks, I was also confused.

Your proposal sounds good to me.

[PATCH v2 7/8] arm64: dts: r8a77990: Add I2C device nodes

[Jacopo Mondi]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

Add device nodes for I2C ch[0-7] to R-Car E3 R8A77990 device tree.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 123 ++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index 0ae7bbe..a1badfe 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -14,6 +14,17 @@
 	#address-cells = <2>;
 	#size-cells = <2>;
 
+	aliases {
+		i2c0 = &i2c0;
+		i2c1 = &i2c1;
+		i2c2 = &i2c2;
+		i2c3 = &i2c3;
+		i2c4 = &i2c4;
+		i2c5 = &i2c5;
+		i2c6 = &i2c6;
+		i2c7 = &i2c7;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -185,6 +196,118 @@
 			resets = <&cpg 906>;
 		};
 
+		i2c0: i2c@e6500000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe6500000 0 0x40>;
+			interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 931>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 931>;
+			i2c-scl-internal-delay-ns = <110>;
+			status = "disabled";
+		};
+
+		i2c1: i2c@e6508000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe6508000 0 0x40>;
+			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 930>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 930>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c2: i2c@e6510000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe6510000 0 0x40>;
+			interrupts = <GIC_SPI 286 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 929>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 929>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c3: i2c@e66d0000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe66d0000 0 0x40>;
+			interrupts = <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 928>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 928>;
+			i2c-scl-internal-delay-ns = <110>;
+			status = "disabled";
+		};
+
+		i2c4: i2c@e66d8000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe66d8000 0 0x40>;
+			interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 927>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 927>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c5: i2c@e66e0000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe66e0000 0 0x40>;
+			interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 919>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 919>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c6: i2c@e66e8000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe66e8000 0 0x40>;
+			interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 918>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 918>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c7: i2c@e6690000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe6690000 0 0x40>;
+			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 1003>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 1003>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
 		pfc: pin-controller@e6060000 {
 			compatible = "renesas,pfc-r8a77990";
 			reg = <0 0xe6060000 0 0x508>;
-- 
2.7.4

Re: [PATCH v2 7/8] arm64: dts: r8a77990: Add I2C device nodes

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wednesday, 5 September 2018 18:29:44 EEST Jacopo Mondi wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> 
> Add device nodes for I2C ch[0-7] to R-Car E3 R8A77990 device tree.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Simon, could you take this in your tree for v4.20 without waiting for the 
whole series to be ready ?

> ---
>  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 123 +++++++++++++++++++++++++++
>  1 file changed, 123 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index 0ae7bbe..a1badfe 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> @@ -14,6 +14,17 @@
>  	#address-cells = <2>;
>  	#size-cells = <2>;
> 
> +	aliases {
> +		i2c0 = &i2c0;
> +		i2c1 = &i2c1;
> +		i2c2 = &i2c2;
> +		i2c3 = &i2c3;
> +		i2c4 = &i2c4;
> +		i2c5 = &i2c5;
> +		i2c6 = &i2c6;
> +		i2c7 = &i2c7;
> +	};
> +
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> @@ -185,6 +196,118 @@
>  			resets = <&cpg 906>;
>  		};
> 
> +		i2c0: i2c@e6500000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "renesas,i2c-r8a77990",
> +				     "renesas,rcar-gen3-i2c";
> +			reg = <0 0xe6500000 0 0x40>;
> +			interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 931>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 931>;
> +			i2c-scl-internal-delay-ns = <110>;
> +			status = "disabled";
> +		};
> +
> +		i2c1: i2c@e6508000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "renesas,i2c-r8a77990",
> +				     "renesas,rcar-gen3-i2c";
> +			reg = <0 0xe6508000 0 0x40>;
> +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 930>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 930>;
> +			i2c-scl-internal-delay-ns = <6>;
> +			status = "disabled";
> +		};
> +
> +		i2c2: i2c@e6510000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "renesas,i2c-r8a77990",
> +				     "renesas,rcar-gen3-i2c";
> +			reg = <0 0xe6510000 0 0x40>;
> +			interrupts = <GIC_SPI 286 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 929>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 929>;
> +			i2c-scl-internal-delay-ns = <6>;
> +			status = "disabled";
> +		};
> +
> +		i2c3: i2c@e66d0000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "renesas,i2c-r8a77990",
> +				     "renesas,rcar-gen3-i2c";
> +			reg = <0 0xe66d0000 0 0x40>;
> +			interrupts = <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 928>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 928>;
> +			i2c-scl-internal-delay-ns = <110>;
> +			status = "disabled";
> +		};
> +
> +		i2c4: i2c@e66d8000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "renesas,i2c-r8a77990",
> +				     "renesas,rcar-gen3-i2c";
> +			reg = <0 0xe66d8000 0 0x40>;
> +			interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 927>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 927>;
> +			i2c-scl-internal-delay-ns = <6>;
> +			status = "disabled";
> +		};
> +
> +		i2c5: i2c@e66e0000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "renesas,i2c-r8a77990",
> +				     "renesas,rcar-gen3-i2c";
> +			reg = <0 0xe66e0000 0 0x40>;
> +			interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 919>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 919>;
> +			i2c-scl-internal-delay-ns = <6>;
> +			status = "disabled";
> +		};
> +
> +		i2c6: i2c@e66e8000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "renesas,i2c-r8a77990",
> +				     "renesas,rcar-gen3-i2c";
> +			reg = <0 0xe66e8000 0 0x40>;
> +			interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 918>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 918>;
> +			i2c-scl-internal-delay-ns = <6>;
> +			status = "disabled";
> +		};
> +
> +		i2c7: i2c@e6690000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "renesas,i2c-r8a77990",
> +				     "renesas,rcar-gen3-i2c";
> +			reg = <0 0xe6690000 0 0x40>;
> +			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 1003>;
> +			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
> +			resets = <&cpg 1003>;
> +			i2c-scl-internal-delay-ns = <6>;
> +			status = "disabled";
> +		};
> +
>  		pfc: pin-controller@e6060000 {
>  			compatible = "renesas,pfc-r8a77990";
>  			reg = <0 0xe6060000 0 0x508>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 7/8] arm64: dts: r8a77990: Add I2C device nodes

[Simon Horman]

On Wed, Sep 05, 2018 at 05:29:44PM +0200, Jacopo Mondi wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> 
> Add device nodes for I2C ch[0-7] to R-Car E3 R8A77990 device tree.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, applied for v4.20.

[PATCH v2 8/8] arm64: dts: renesas: ebisu: Add HDMI and CVBS input

[Jacopo Mondi]

Add HDMI and CVBS inputs device nodes to R-Car E3 Ebisu board.

Both HDMI and CVBS inputs are connected to an ADV7482 video decoder hooked to
the SoC CSI-2 receiver port.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 86 ++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index 2bc3a48..d2faf3e 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -28,6 +28,29 @@
 		/* first 128MB is reserved for secure area. */
 		reg = <0x0 0x48000000 0x0 0x38000000>;
 	};
+
+	cvbs-in {
+		compatible = "composite-video-connector";
+		label = "CVBS IN";
+
+		port {
+			cvbs_con: endpoint {
+				remote-endpoint = <&adv7482_ain7>;
+			};
+		};
+	};
+
+	hdmi-in {
+		compatible = "hdmi-connector";
+		label = "HDMI IN";
+		type = "a";
+
+		port {
+			hdmi_in_con: endpoint {
+				remote-endpoint = <&adv7482_hdmi>;
+			};
+		};
+	};
 };

 &avb {
@@ -47,6 +70,22 @@
 	};
 };

+&csi40 {
+	status = "okay";
+
+	ports {
+		port@0 {
+			reg = <0>;
+
+			csi40_in: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+				remote-endpoint = <&adv7482_txa>;
+			};
+		};
+	};
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -55,6 +94,49 @@
 	clock-frequency = <48000000>;
 };

+&i2c0 {
+	status = "okay";
+
+	video-receiver@70 {
+		compatible = "adi,adv7482";
+		reg = <0x70>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		interrupt-parent = <&gpio0>;
+		interrupt-names = "intrq1", "intrq2";
+		interrupts = <7 IRQ_TYPE_LEVEL_LOW>,
+			     <17 IRQ_TYPE_LEVEL_LOW>;
+
+		port@7 {
+			reg = <7>;
+
+			adv7482_ain7: endpoint {
+				remote-endpoint = <&cvbs_con>;
+			};
+		};
+
+		port@8 {
+			reg = <8>;
+
+			adv7482_hdmi: endpoint {
+				remote-endpoint = <&hdmi_in_con>;
+			};
+		};
+
+		port@a {
+			reg = <0xa>;
+
+			adv7482_txa: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+				remote-endpoint = <&csi40_in>;
+			};
+		};
+	};
+};
+
 &ohci0 {
 	status = "okay";
 };
@@ -94,6 +176,10 @@
 	status = "okay";
 };

+&vin4 {
+	status = "okay";
+};
+
 &xhci0 {
 	pinctrl-0 = <&usb30_pins>;
 	pinctrl-names = "default";
--
2.7.4

Re: [PATCH v2 8/8] arm64: dts: renesas: ebisu: Add HDMI and CVBS input

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wednesday, 5 September 2018 18:29:45 EEST Jacopo Mondi wrote:
> Add HDMI and CVBS inputs device nodes to R-Car E3 Ebisu board.
> 
> Both HDMI and CVBS inputs are connected to an ADV7482 video decoder hooked
> to the SoC CSI-2 receiver port.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 86 +++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts index 2bc3a48..d2faf3e
> 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> @@ -28,6 +28,29 @@
>  		/* first 128MB is reserved for secure area. */
>  		reg = <0x0 0x48000000 0x0 0x38000000>;
>  	};
> +
> +	cvbs-in {
> +		compatible = "composite-video-connector";
> +		label = "CVBS IN";
> +
> +		port {
> +			cvbs_con: endpoint {
> +				remote-endpoint = <&adv7482_ain7>;
> +			};
> +		};
> +	};
> +
> +	hdmi-in {
> +		compatible = "hdmi-connector";
> +		label = "HDMI IN";
> +		type = "a";
> +
> +		port {
> +			hdmi_in_con: endpoint {
> +				remote-endpoint = <&adv7482_hdmi>;
> +			};
> +		};
> +	};
>  };
> 
>  &avb {
> @@ -47,6 +70,22 @@
>  	};
>  };
> 
> +&csi40 {
> +	status = "okay";
> +
> +	ports {
> +		port@0 {
> +			reg = <0>;
> +
> +			csi40_in: endpoint {
> +				clock-lanes = <0>;
> +				data-lanes = <1 2>;
> +				remote-endpoint = <&adv7482_txa>;
> +			};
> +		};
> +	};
> +};
> +
>  &ehci0 {
>  	status = "okay";
>  };
> @@ -55,6 +94,49 @@
>  	clock-frequency = <48000000>;
>  };
> 
> +&i2c0 {
> +	status = "okay";
> +
> +	video-receiver@70 {
> +		compatible = "adi,adv7482";
> +		reg = <0x70>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		interrupt-parent = <&gpio0>;
> +		interrupt-names = "intrq1", "intrq2";
> +		interrupts = <7 IRQ_TYPE_LEVEL_LOW>,
> +			     <17 IRQ_TYPE_LEVEL_LOW>;
> +
> +		port@7 {
> +			reg = <7>;
> +
> +			adv7482_ain7: endpoint {
> +				remote-endpoint = <&cvbs_con>;
> +			};
> +		};
> +
> +		port@8 {
> +			reg = <8>;
> +
> +			adv7482_hdmi: endpoint {
> +				remote-endpoint = <&hdmi_in_con>;
> +			};
> +		};
> +
> +		port@a {
> +			reg = <0xa>;
> +
> +			adv7482_txa: endpoint {
> +				clock-lanes = <0>;
> +				data-lanes = <1 2>;
> +				remote-endpoint = <&csi40_in>;
> +			};
> +		};
> +	};
> +};
> +
>  &ohci0 {
>  	status = "okay";
>  };
> @@ -94,6 +176,10 @@
>  	status = "okay";
>  };
> 
> +&vin4 {
> +	status = "okay";
> +};
> +
>  &xhci0 {
>  	pinctrl-0 = <&usb30_pins>;
>  	pinctrl-names = "default";

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 8/8] arm64: dts: renesas: ebisu: Add HDMI and CVBS input

[Simon Horman]

On Mon, Sep 10, 2018 at 05:21:08PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Wednesday, 5 September 2018 18:29:45 EEST Jacopo Mondi wrote:
> > Add HDMI and CVBS inputs device nodes to R-Car E3 Ebisu board.
> > 
> > Both HDMI and CVBS inputs are connected to an ADV7482 video decoder hooked
> > to the SoC CSI-2 receiver port.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Jacopo,

I assume that this patch depends on 7/8.
Please repost this patch when you repost that one,
addressing Laurent's feedback.

Re: [PATCH v2 8/8] arm64: dts: renesas: ebisu: Add HDMI and CVBS input

[jacopo mondi]

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

Hi Simon,

On Wed, Sep 12, 2018 at 11:29:51AM +0200, Simon Horman wrote:
> On Mon, Sep 10, 2018 at 05:21:08PM +0300, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > On Wednesday, 5 September 2018 18:29:45 EEST Jacopo Mondi wrote:
> > > Add HDMI and CVBS inputs device nodes to R-Car E3 Ebisu board.
> > >
> > > Both HDMI and CVBS inputs are connected to an ADV7482 video decoder hooked
> > > to the SoC CSI-2 receiver port.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Jacopo,
>
> I assume that this patch depends on 7/8.
> Please repost this patch when you repost that one,
> addressing Laurent's feedback.

Sorry, I'm now confused :)
I don't see any pending comment on [7/8] nor on this one ([8/8]).
[7/8] has been applied to your "arm64-dt-for-v4.20" branch already
(just checked).

I have a small comment on [6/8] and one open question on [5/8]. All
the other patches have been reviewed/acked already.

To sum up: do you want me to repost [7/8] when re-sending this series?

Thanks
   j

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

Re: [PATCH v2 8/8] arm64: dts: renesas: ebisu: Add HDMI and CVBS input

[Simon Horman]

On Wed, Sep 12, 2018 at 01:08:21PM +0200, jacopo mondi wrote:
> Hi Simon,
> 
> On Wed, Sep 12, 2018 at 11:29:51AM +0200, Simon Horman wrote:
> > On Mon, Sep 10, 2018 at 05:21:08PM +0300, Laurent Pinchart wrote:
> > > Hi Jacopo,
> > >
> > > Thank you for the patch.
> > >
> > > On Wednesday, 5 September 2018 18:29:45 EEST Jacopo Mondi wrote:
> > > > Add HDMI and CVBS inputs device nodes to R-Car E3 Ebisu board.
> > > >
> > > > Both HDMI and CVBS inputs are connected to an ADV7482 video decoder hooked
> > > > to the SoC CSI-2 receiver port.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Jacopo,
> >
> > I assume that this patch depends on 7/8.
> > Please repost this patch when you repost that one,
> > addressing Laurent's feedback.
> 
> Sorry, I'm now confused :)
> I don't see any pending comment on [7/8] nor on this one ([8/8]).
> [7/8] has been applied to your "arm64-dt-for-v4.20" branch already
> (just checked).
> 
> I have a small comment on [6/8] and one open question on [5/8]. All
> the other patches have been reviewed/acked already.
> 
> To sum up: do you want me to repost [7/8] when re-sending this series?

No need.

I now see that 6/8 and 7/8 are in my tree.  And that this patch, 8/8, has
now been reviewed so I have applied it too.  So I think all the patches in
this series targeted at my tree have now been applied.

Please consider a follow-up patch to address the review of 6/8.

Re: [PATCH/RFT v2 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patches.

On Wednesday, 5 September 2018 18:29:37 EEST Jacopo Mondi wrote:
> Hello renesas list,
>    this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
> Ebisu board.
> 
> It's an RFT, as I don't have an Ebisu to test with :(
> 
> The series adds supports for the following items:
> 
> - PFC: add VIN groups and functions
> - R-Car VIN and R-Car CSI-2: add support for R8A77990
> - R8A77990: Add I2C, VIN and CSI-2 nodes (v1 re-sent by Laurent in D3 DU
> series) - Ebisu: describe HDMI and CVBS inputs
> 
> Laurent has tested v1 and confirmed that the current mainline implementation
> of the adv748x driver does not work in the configuration used by the Ebisu
> board.

I've now tested v2 on Ebisu, and there's definitely progress after numbering 
the VIN endpoints correctly as mentioned in my review of patch 6/8.

I've added (or rather hacked) Ebisu support to the VIN tests, and I can 
capture frames:

# ./set-edid 
EDID set successfully.
# ./yavta-hdmi 4
format: RGB888_1X24 size: 800x600/800x600 field: none/none vdev: /dev/video2
Device /dev/video2 opened.
Device `R_Car_VIN' on `platform:e6ef4000.video' (driver 'rcar_vin') supports 
video, capture, without mplanes.
Video format set: RGB565 (50424752) 800x600 (stride 1600) field none buffer 
size 960000
Video format: RGB565 (50424752) 800x600 (stride 1600) field none buffer size 
960000
Device /dev/video2 opened.
Device `R_Car_VIN' on `platform:e6ef4000.video' (driver 'rcar_vin') supports 
video, capture, without mplanes.
Video format: RGB565 (50424752) 800x600 (stride 1600) field none buffer size 
960000
4 buffers requested.
length: 960000 offset: 0 timestamp type/source: mono/EoF
Buffer 0/0 mapped at address 0x7f81323000.
length: 960000 offset: 962560 timestamp type/source: mono/EoF
Buffer 1/0 mapped at address 0x7f81238000.
length: 960000 offset: 1925120 timestamp type/source: mono/EoF
Buffer 2/0 mapped at address 0x7f8114d000.
length: 960000 offset: 2887680 timestamp type/source: mono/EoF
Buffer 3/0 mapped at address 0x7f81062000.
0 (0) [-] none 0 960000 B 123.674468 123.676485 75.740 fps ts mono/EoF
1 (2) [-] none 1 960000 B 123.675610 123.700566 875.657 fps ts mono/EoF
2 (3) [-] none 2 960000 B 123.675637 123.718865 37037.037 fps ts mono/EoF
3 (1) [-] none 3 960000 B 123.675693 123.737100 17857.143 fps ts mono/EoF
4 (0) [-] none 252 960000 B 123.700914 123.755378 39.649 fps ts mono/EoF
5 (2) [-] none 417 960000 B 123.719047 123.773609 55.148 fps ts mono/EoF
6 (3) [-] none 600 960000 B 123.737330 123.791760 54.696 fps ts mono/EoF
7 (1) [-] none 789 960000 B 123.757476 123.810198 49.638 fps ts mono/EoF
8 (0) [-] none 951 960000 B 123.774199 123.828604 59.798 fps ts mono/EoF
9 (2) [-] none 1128 960000 B 123.792089 123.846743 55.897 fps ts mono/EoF
Captured 10 frames in 0.185478 seconds (53.914726 fps, 51758136.836038 B/s).
4 buffers released.

The frames are however all black.

I've applied the following modifications to the VIN test scripts (on top of 
your D3 patches-. Please feel free to include them in your next version.

diff --git a/scripts/boards.sh b/scripts/boards.sh
index 82806c6e56eb..a29d87918cf2 100644
--- a/scripts/boards.sh
+++ b/scripts/boards.sh
@@ -36,6 +36,11 @@ case $info in
         # FIXME: This is a hackfor D3, but results in an image.
         parallelformat="YUYV8_1X16"
         ;;
+    "Renesas Ebisu board based on r8a77990")
+        gen="gen3"
+        vins="4 5"
+        csis="40"
+        ;;
     "Renesas H3ULCB Kingfisher board based on r8a7795 ES2.0+")
         gen="gen3"
         vins="0 1 2 3 4 5 6 7"
@@ -76,19 +81,22 @@ if [[ "$gen" == "gen3" ]]; then
         esac
     done
 
-    if [[ "$info" == "Renesas Eagle board based on r8a77970" ]]; then
-        cvbsname="adv748x 0-0070 afe"
-        hdminame="adv748x 0-0070 hdmi"
-
-        txaname="adv748x 0-0070 txa"
-        txbname="adv748x 0-0070 txb"
-    elif [[ "$info" == "Renesas Draak board based on r8a77995" ]]; then
-        hdminame="adv7612 0-004c"
-    else
-        cvbsname="adv748x 4-0070 afe"
-        hdminame="adv748x 4-0070 hdmi"
-
-        txaname="adv748x 4-0070 txa"
-        txbname="adv748x 4-0070 txb"
-    fi
+    case $info in
+        "Renesas Eagle board based on r8a77970" | \
+	"Renesas Ebisu board based on r8a77990")
+            cvbsname="adv748x 0-0070 afe"
+            hdminame="adv748x 0-0070 hdmi"
+            txaname="adv748x 0-0070 txa"
+            txbname="adv748x 0-0070 txb"
+	    ;;
+	"Renesas Draak board based on r8a77995")
+            hdminame="adv7612 0-004c"
+            ;;
+        *)
+            cvbsname="adv748x 4-0070 afe"
+            hdminame="adv748x 4-0070 hdmi"
+            txaname="adv748x 4-0070 txa"
+            txbname="adv748x 4-0070 txb"
+	    ;;
+    esac
 fi
diff --git a/yavta-hdmi b/yavta-hdmi
index 2e3b625978fe..7f9e10b42d94 100755
--- a/yavta-hdmi
+++ b/yavta-hdmi
@@ -36,6 +36,7 @@ case $vc in
     4)
         vinname=$vinname4
         dev=/dev/$vin4
+        csipad=1
         ;;
     *)
         echo "Unkown VC '$vc'"

> This series should then be applied on top of the just sent:
> [PATCH v2 0/5] media: adv748x: Allow probe with a single output endpoint
> To allow the adv7482 to probe with a single output port enabled.
> 
> This series, based on v4.19-rc2 with adv748x series on top, is available at:
> git://jmondi.org/linux ebisu/linus-master/hdmi_cvbs_v2
> 
> Thanks
>    j
> 
> Jacopo Mondi (6):
>   media: dt-bindings: rcar-vin: Add R8A77990 support
>   media: rcar-vin: Add support for R-Car R8A77990
>   media: dt-bindings: rcar-csi2: Add R8A77990
>   media: rcar-csi2: Add R8A77990 support
>   pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
>   arm64: dts: renesas: ebisu: Add HDMI and CVBS input
> 
> Koji Matsuoka (1):
>   arm64: dts: r8a77990: Add VIN and CSI-2 device nodes
> 
> Takeshi Kihara (1):
>   arm64: dts: r8a77990: Add I2C device nodes
> 
>  .../devicetree/bindings/media/rcar_vin.txt         |   1 +
>  .../bindings/media/renesas,rcar-csi2.txt           |   1 +
>  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  86 +++++++
>  arch/arm64/boot/dts/renesas/r8a77990.dtsi          | 202 +++++++++++++++++
>  drivers/media/platform/rcar-vin/rcar-core.c        |  20 ++
>  drivers/media/platform/rcar-vin/rcar-csi2.c        |   9 +
>  drivers/pinctrl/sh-pfc/pfc-r8a77990.c              | 250 ++++++++++++++++++
>  7 files changed, 569 insertions(+)

-- 
Regards,

Laurent Pinchart