mbox series

[00/20] Add support for SATA/PCIe/USB2[3]/VIN/CSI on R8A774E1

Message ID 1594919915-5225-1-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com
Headers show
Series Add support for SATA/PCIe/USB2[3]/VIN/CSI on R8A774E1 | expand

Message

Prabhakar Mahadev Lad July 16, 2020, 5:18 p.m. UTC
Hi All,

This patch series adds support for the following peripherals on RZ/G2H SoC
 * PCIe
 * SATA
 * USB2
 * USB3
 * Audio
 * VIN
 * CSI

Cheers,
Prabhakar

Lad Prabhakar (20):
  dt-bindings: pci: rcar-pci: Add device tree support for r8a774e1
  arm64: dts: renesas: r8a774e1: Add PCIe device nodes
  dt-bindings: ata: renesas,rcar-sata: Add r8a774e1 support
  arm64: dts: renesas: r8a774e1: Add SATA controller node
  dt-bindings: phy: renesas,usb2-phy: Add r8a774e1 support
  arm64: dts: renesas: r8a774e1: Add USB2.0 phy and host (EHCI/OHCI)
    device nodes
  dt-bindings: usb: renesas,usb3-peri: Document r8a774e1 support
  dt-bindings: usb: usb-xhci: Document r8a774e1 support
  dt-bindings: phy: renesas,usb3-phy: Add r8a774e1 support
  arm64: dts: renesas: r8a774e1: Add USB3.0 device nodes
  dt-bindings: usb: renesas,usbhs: Add r8a774e1 support
  dt-bindings: dma: renesas,usb-dmac: Add binding for r8a774e1
  arm64: dts: renesas: r8a774e1: Add USB-DMAC and HSUSB device nodes
  dt-bindings: sound: renesas,rsnd: Document r8a774e1 bindings
  arm64: dts: renesas: r8a774e1: Add audio support
  dt-bindings: media: renesas,csi2: Add R8A774E1 support
  dt-bindings: media: renesas,vin: Add R8A774E1 support
  media: rcar-csi2: Enable support for R8A774E1
  media: rcar-vin: Enable support for R8A774E1
  arm64: dts: renesas: r8a774e1: Add VIN and CSI-2 nodes

 .../bindings/ata/renesas,rcar-sata.yaml       |   1 +
 .../bindings/dma/renesas,usb-dmac.yaml        |   1 +
 .../bindings/media/renesas,csi2.yaml          |   1 +
 .../bindings/media/renesas,vin.yaml           |   1 +
 .../devicetree/bindings/pci/rcar-pci.txt      |   1 +
 .../bindings/phy/renesas,usb2-phy.yaml        |   1 +
 .../bindings/phy/renesas,usb3-phy.yaml        |   1 +
 .../bindings/sound/renesas,rsnd.txt           |   1 +
 .../bindings/usb/renesas,usb3-peri.yaml       |   1 +
 .../bindings/usb/renesas,usbhs.yaml           |   1 +
 .../devicetree/bindings/usb/usb-xhci.txt      |   1 +
 arch/arm64/boot/dts/renesas/r8a774e1.dtsi     | 989 +++++++++++++++++-
 drivers/media/platform/rcar-vin/rcar-core.c   |  40 +
 drivers/media/platform/rcar-vin/rcar-csi2.c   |   4 +
 14 files changed, 1022 insertions(+), 22 deletions(-)

Comments

Niklas Söderlund July 17, 2020, 1:13 p.m. UTC | #1
Hi Lad,

Thanks for your work.

On 2020-07-16 18:18:33 +0100, Lad Prabhakar wrote:
> Add the MIPI CSI-2 driver support for RZ/G2H (R8A774E1) SoC.
> The CSI-2 module of RZ/G2H is similar to R-Car H3.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

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

> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index c6cc4f473a07..2325e3b103e4 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -1090,6 +1090,10 @@ static const struct of_device_id rcar_csi2_of_table[] = {
>  		.compatible = "renesas,r8a774c0-csi2",
>  		.data = &rcar_csi2_info_r8a77990,
>  	},
> +	{
> +		.compatible = "renesas,r8a774e1-csi2",
> +		.data = &rcar_csi2_info_r8a7795,
> +	},
>  	{
>  		.compatible = "renesas,r8a7795-csi2",
>  		.data = &rcar_csi2_info_r8a7795,
> -- 
> 2.17.1
>
Niklas Söderlund July 17, 2020, 1:19 p.m. UTC | #2
Hi Lad,

Thanks for your work.

On 2020-07-16 18:18:34 +0100, Lad Prabhakar wrote:
> Add the SoC specific information for RZ/G2H (R8A774E1) SoC. Also add
> the routing information between CSI2 and VIN (which is similar to
> R-Car H3 except it lacks CSI41).
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

I do not have access to the datasheet so I can't verify the routing 
table so I trust it is correct.

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

> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 40 +++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 7440c8965d27..4fb76d1df308 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -944,6 +944,42 @@ static const struct rvin_info rcar_info_gen2 = {
>  	.max_height = 2048,
>  };
>  
> +static const struct rvin_group_route rcar_info_r8a774e1_routes[] = {
> +	{ .csi = RVIN_CSI40, .channel = 0, .vin = 0, .mask = BIT(0) | BIT(3) },
> +	{ .csi = RVIN_CSI20, .channel = 0, .vin = 0, .mask = BIT(1) | BIT(4) },
> +	{ .csi = RVIN_CSI40, .channel = 1, .vin = 0, .mask = BIT(2) },
> +	{ .csi = RVIN_CSI20, .channel = 0, .vin = 1, .mask = BIT(0) },
> +	{ .csi = RVIN_CSI40, .channel = 1, .vin = 1, .mask = BIT(1) | BIT(3) },
> +	{ .csi = RVIN_CSI40, .channel = 0, .vin = 1, .mask = BIT(2) },
> +	{ .csi = RVIN_CSI20, .channel = 1, .vin = 1, .mask = BIT(4) },
> +	{ .csi = RVIN_CSI20, .channel = 1, .vin = 2, .mask = BIT(0) },
> +	{ .csi = RVIN_CSI40, .channel = 0, .vin = 2, .mask = BIT(1) },
> +	{ .csi = RVIN_CSI20, .channel = 0, .vin = 2, .mask = BIT(2) },
> +	{ .csi = RVIN_CSI40, .channel = 2, .vin = 2, .mask = BIT(3) },
> +	{ .csi = RVIN_CSI20, .channel = 2, .vin = 2, .mask = BIT(4) },
> +	{ .csi = RVIN_CSI40, .channel = 1, .vin = 3, .mask = BIT(0) },
> +	{ .csi = RVIN_CSI20, .channel = 1, .vin = 3, .mask = BIT(1) | BIT(2) },
> +	{ .csi = RVIN_CSI40, .channel = 3, .vin = 3, .mask = BIT(3) },
> +	{ .csi = RVIN_CSI20, .channel = 3, .vin = 3, .mask = BIT(4) },
> +	{ .csi = RVIN_CSI20, .channel = 0, .vin = 4, .mask = BIT(1) | BIT(4) },
> +	{ .csi = RVIN_CSI20, .channel = 0, .vin = 5, .mask = BIT(0) },
> +	{ .csi = RVIN_CSI20, .channel = 1, .vin = 5, .mask = BIT(4) },
> +	{ .csi = RVIN_CSI20, .channel = 1, .vin = 6, .mask = BIT(0) },
> +	{ .csi = RVIN_CSI20, .channel = 0, .vin = 6, .mask = BIT(2) },
> +	{ .csi = RVIN_CSI20, .channel = 2, .vin = 6, .mask = BIT(4) },
> +	{ .csi = RVIN_CSI20, .channel = 1, .vin = 7, .mask = BIT(1) | BIT(2) },
> +	{ .csi = RVIN_CSI20, .channel = 3, .vin = 7, .mask = BIT(4) },
> +	{ /* Sentinel */ }
> +};
> +
> +static const struct rvin_info rcar_info_r8a774e1 = {
> +	.model = RCAR_GEN3,
> +	.use_mc = true,
> +	.max_width = 4096,
> +	.max_height = 4096,
> +	.routes = rcar_info_r8a774e1_routes,
> +};
> +
>  static const struct rvin_group_route rcar_info_r8a7795_routes[] = {
>  	{ .csi = RVIN_CSI40, .channel = 0, .vin = 0, .mask = BIT(0) | BIT(3) },
>  	{ .csi = RVIN_CSI20, .channel = 0, .vin = 0, .mask = BIT(1) | BIT(4) },
> @@ -1220,6 +1256,10 @@ static const struct of_device_id rvin_of_id_table[] = {
>  		.compatible = "renesas,vin-r8a774c0",
>  		.data = &rcar_info_r8a77990,
>  	},
> +	{
> +		.compatible = "renesas,vin-r8a774e1",
> +		.data = &rcar_info_r8a774e1,
> +	},
>  	{
>  		.compatible = "renesas,vin-r8a7778",
>  		.data = &rcar_info_m1,
> -- 
> 2.17.1
>
Mark Brown July 17, 2020, 3:39 p.m. UTC | #3
On Thu, 16 Jul 2020 18:18:15 +0100, Lad Prabhakar wrote:
> This patch series adds support for the following peripherals on RZ/G2H SoC
>  * PCIe
>  * SATA
>  * USB2
>  * USB3
>  * Audio
>  * VIN
>  * CSI
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] dt-bindings: sound: renesas, rsnd: Document r8a774e1 bindings
      commit: 92e37407811b98a7eb54eb6a6b3d65847a46e0e6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Wolfram Sang July 22, 2020, 8:50 a.m. UTC | #4
> This patch series adds support for the following peripherals on RZ/G2H SoC
>  * PCIe
>  * SATA
>  * USB2
>  * USB3
>  * Audio
>  * VIN
>  * CSI

Nice. But please update your recipients list. No need to have the i2c
mailing list in there.
Wolfram Sang July 22, 2020, 8:58 a.m. UTC | #5
On Thu, Jul 16, 2020 at 06:18:17PM +0100, Lad Prabhakar wrote:
> Add PCIe{0,1} device nodes for R8A774E1 SoC.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

Hmm, doesn't apply on top of 5.8-rc6 for me. Is there a branch to pull
for easier review?
Wolfram Sang July 22, 2020, 9 a.m. UTC | #6
On Wed, Jul 22, 2020 at 10:58:49AM +0200, Wolfram Sang wrote:
> On Thu, Jul 16, 2020 at 06:18:17PM +0100, Lad Prabhakar wrote:
> > Add PCIe{0,1} device nodes for R8A774E1 SoC.
> > 
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> 
> Hmm, doesn't apply on top of 5.8-rc6 for me. Is there a branch to pull
> for easier review?

My fault, I missed the first series. Please note such dependencies in
the cover letter.
Lad, Prabhakar July 22, 2020, 9:03 a.m. UTC | #7
Hi Wolfram,

On Wed, Jul 22, 2020 at 10:00 AM Wolfram Sang <wsa@kernel.org> wrote:
>
> On Wed, Jul 22, 2020 at 10:58:49AM +0200, Wolfram Sang wrote:
> > On Thu, Jul 16, 2020 at 06:18:17PM +0100, Lad Prabhakar wrote:
> > > Add PCIe{0,1} device nodes for R8A774E1 SoC.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> >
> > Hmm, doesn't apply on top of 5.8-rc6 for me. Is there a branch to pull
> > for easier review?
>
> My fault, I missed the first series. Please note such dependencies in
> the cover letter.
>
I didn't mention as they were already applied to Geert's tree [1]

[1] https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git/log/?h=renesas-arm-dt-for-v5.9

Cheers,
--Prabhakar
Geert Uytterhoeven Aug. 5, 2020, 11:08 a.m. UTC | #8
On Thu, Jul 16, 2020 at 7:18 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add PCIe{0,1} device nodes for R8A774E1 SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.10.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Aug. 5, 2020, 11:09 a.m. UTC | #9
On Thu, Jul 16, 2020 at 7:19 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add the SATA controller node to the RZ/G2H SoC specific dtsi.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.10.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Aug. 5, 2020, 11:09 a.m. UTC | #10
On Thu, Jul 16, 2020 at 7:19 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add USB2.0 phy and host (EHCI/OHCI) device nodes on RZ/G2H SoC dtsi.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.10.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Aug. 5, 2020, 11:14 a.m. UTC | #11
On Thu, Jul 16, 2020 at 7:19 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add usb3.0 phy, host and function device nodes on RZ/G2H SoC dtsi.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.10.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Aug. 5, 2020, 11:14 a.m. UTC | #12
On Thu, Jul 16, 2020 at 7:19 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add usb dmac and hsusb device nodes to the RZ/G2H SoC dtsi.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.10.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Aug. 5, 2020, 11:15 a.m. UTC | #13
On Thu, Jul 16, 2020 at 7:20 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add sound support for the RZ/G2H SoC (a.k.a. R8A774E1).
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.10.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Aug. 5, 2020, 11:18 a.m. UTC | #14
Hi Prabhakar,

On Thu, Jul 16, 2020 at 7:20 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add VIN and CSI-2 nodes to RZ/G2H (R8A774E1) SoC dtsi.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

However, before I queue this in renesas-devel for v5.10, I'd like to
have some clarification about the issue below.

> --- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi

> +               vin4: video@e6ef4000 {
> +                       compatible = "renesas,vin-r8a774e1";
> +                       reg = <0 0xe6ef4000 0 0x1000>;
> +                       interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&cpg CPG_MOD 807>;
> +                       power-domains = <&sysc R8A774E1_PD_ALWAYS_ON>;
> +                       resets = <&cpg 807>;
> +                       renesas,id = <4>;
> +                       status = "disabled";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               port@1 {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;

"make dtbs W=1" says:

    arch/arm64/boot/dts/renesas/r8a774e1.dtsi:1562.12-1572.7: Warning
(graph_child_address): /soc/video@e6ef4000/ports/port@1: graph node
has single child node 'endpoint@0', #address-cells/#size-cells are not
necessary

(same for vin5-7 below)

> +
> +                                       reg = <1>;
> +
> +                                       vin4csi20: endpoint@0 {
> +                                               reg = <0>;
> +                                               remote-endpoint = <&csi20vin4>;
> +                                       };

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
Lad, Prabhakar Aug. 6, 2020, 11:16 a.m. UTC | #15
Hi Geert,

Thank you for the review.

On Wed, Aug 5, 2020 at 12:19 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, Jul 16, 2020 at 7:20 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Add VIN and CSI-2 nodes to RZ/G2H (R8A774E1) SoC dtsi.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> However, before I queue this in renesas-devel for v5.10, I'd like to
> have some clarification about the issue below.
>
> > --- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
>
> > +               vin4: video@e6ef4000 {
> > +                       compatible = "renesas,vin-r8a774e1";
> > +                       reg = <0 0xe6ef4000 0 0x1000>;
> > +                       interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&cpg CPG_MOD 807>;
> > +                       power-domains = <&sysc R8A774E1_PD_ALWAYS_ON>;
> > +                       resets = <&cpg 807>;
> > +                       renesas,id = <4>;
> > +                       status = "disabled";
> > +
> > +                       ports {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +
> > +                               port@1 {
> > +                                       #address-cells = <1>;
> > +                                       #size-cells = <0>;
>
> "make dtbs W=1" says:
>
>     arch/arm64/boot/dts/renesas/r8a774e1.dtsi:1562.12-1572.7: Warning
> (graph_child_address): /soc/video@e6ef4000/ports/port@1: graph node
> has single child node 'endpoint@0', #address-cells/#size-cells are not
> necessary
>
> (same for vin5-7 below)
>
Referring to commit 5e53dbf4edb4d ("arm64: dts: renesas: r8a77990: Fix
VIN endpoint numbering") we definitely need endpoint numbering.
Probably the driver needs to be fixed to handle such cases.

Cheers,
Prabhakar

> > +
> > +                                       reg = <1>;
> > +
> > +                                       vin4csi20: endpoint@0 {
> > +                                               reg = <0>;
> > +                                               remote-endpoint = <&csi20vin4>;
> > +                                       };
>
> 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
Geert Uytterhoeven Aug. 6, 2020, 11:47 a.m. UTC | #16
Hi Prabhakar,

On Thu, Aug 6, 2020 at 1:17 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Wed, Aug 5, 2020 at 12:19 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Jul 16, 2020 at 7:20 PM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > Add VIN and CSI-2 nodes to RZ/G2H (R8A774E1) SoC dtsi.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > However, before I queue this in renesas-devel for v5.10, I'd like to
> > have some clarification about the issue below.
> >
> > > --- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> >
> > > +               vin4: video@e6ef4000 {
> > > +                       compatible = "renesas,vin-r8a774e1";
> > > +                       reg = <0 0xe6ef4000 0 0x1000>;
> > > +                       interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > +                       clocks = <&cpg CPG_MOD 807>;
> > > +                       power-domains = <&sysc R8A774E1_PD_ALWAYS_ON>;
> > > +                       resets = <&cpg 807>;
> > > +                       renesas,id = <4>;
> > > +                       status = "disabled";
> > > +
> > > +                       ports {
> > > +                               #address-cells = <1>;
> > > +                               #size-cells = <0>;
> > > +
> > > +                               port@1 {
> > > +                                       #address-cells = <1>;
> > > +                                       #size-cells = <0>;
> >
> > "make dtbs W=1" says:
> >
> >     arch/arm64/boot/dts/renesas/r8a774e1.dtsi:1562.12-1572.7: Warning
> > (graph_child_address): /soc/video@e6ef4000/ports/port@1: graph node
> > has single child node 'endpoint@0', #address-cells/#size-cells are not
> > necessary
> >
> > (same for vin5-7 below)
> >
> Referring to commit 5e53dbf4edb4d ("arm64: dts: renesas: r8a77990: Fix
> VIN endpoint numbering") we definitely need endpoint numbering.
> Probably the driver needs to be fixed to handle such cases.

> > > +
> > > +                                       reg = <1>;
> > > +
> > > +                                       vin4csi20: endpoint@0 {
> > > +                                               reg = <0>;
> > > +                                               remote-endpoint = <&csi20vin4>;

On R-Car E3, the single endpoint is at address 2, so "make dtbs W=1"doesn't
complain. Here it is at address 0.

Niklas?

Gr{oetje,eeting}s,

                        Geert
Niklas Söderlund Aug. 7, 2020, 11:27 a.m. UTC | #17
Hi Geert, Lad,

On 2020-08-06 13:47:58 +0200, Geert Uytterhoeven wrote:
> Hi Prabhakar,
> 
> On Thu, Aug 6, 2020 at 1:17 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Wed, Aug 5, 2020 at 12:19 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Thu, Jul 16, 2020 at 7:20 PM Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > Add VIN and CSI-2 nodes to RZ/G2H (R8A774E1) SoC dtsi.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> > >
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >
> > > However, before I queue this in renesas-devel for v5.10, I'd like to
> > > have some clarification about the issue below.
> > >
> > > > --- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> > >
> > > > +               vin4: video@e6ef4000 {
> > > > +                       compatible = "renesas,vin-r8a774e1";
> > > > +                       reg = <0 0xe6ef4000 0 0x1000>;
> > > > +                       interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                       clocks = <&cpg CPG_MOD 807>;
> > > > +                       power-domains = <&sysc R8A774E1_PD_ALWAYS_ON>;
> > > > +                       resets = <&cpg 807>;
> > > > +                       renesas,id = <4>;
> > > > +                       status = "disabled";
> > > > +
> > > > +                       ports {
> > > > +                               #address-cells = <1>;
> > > > +                               #size-cells = <0>;
> > > > +
> > > > +                               port@1 {
> > > > +                                       #address-cells = <1>;
> > > > +                                       #size-cells = <0>;
> > >
> > > "make dtbs W=1" says:
> > >
> > >     arch/arm64/boot/dts/renesas/r8a774e1.dtsi:1562.12-1572.7: Warning
> > > (graph_child_address): /soc/video@e6ef4000/ports/port@1: graph node
> > > has single child node 'endpoint@0', #address-cells/#size-cells are not
> > > necessary
> > >
> > > (same for vin5-7 below)
> > >
> > Referring to commit 5e53dbf4edb4d ("arm64: dts: renesas: r8a77990: Fix
> > VIN endpoint numbering") we definitely need endpoint numbering.
> > Probably the driver needs to be fixed to handle such cases.
> 
> > > > +
> > > > +                                       reg = <1>;
> > > > +
> > > > +                                       vin4csi20: endpoint@0 {
> > > > +                                               reg = <0>;
> > > > +                                               remote-endpoint = <&csi20vin4>;
> 
> On R-Car E3, the single endpoint is at address 2, so "make dtbs W=1"doesn't
> complain. Here it is at address 0.
> 
> Niklas?

First the R-Car VIN driver makes decisions based on which endpoint is 
described, each endpoint 0-3 represents a different CSI-2 block on the 
other end (0: CSI20, 1: CSI21, 2: CSI40 and 3: CSI41).

Then how to handle the warning I'm not sure. I can only really see 2 
options.

1. Ignore the warning.
2. Remove #address-cells, #size-cells and reg properties from port@ if 
   the only endpoint described is endpoint@0.

I would prefers option 2. that is what we do in other cases (for example 
on Gen2 boards that only have a single parallel sensor in some early DTS 
files we don't have the ports node and just describe a single port with 
the same reasoning.

We are not at risk at someone describing a second CSI-2 bock as an 
overlay so I see no real harm in option 2. What are your thoughts Geert?  
You know more about DT then me.

> 
> 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
Geert Uytterhoeven Aug. 7, 2020, 11:36 a.m. UTC | #18
Hi Niklas,

On Fri, Aug 7, 2020 at 1:27 PM Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
> On 2020-08-06 13:47:58 +0200, Geert Uytterhoeven wrote:
> > On Thu, Aug 6, 2020 at 1:17 PM Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> wrote:
> > > On Wed, Aug 5, 2020 at 12:19 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Thu, Jul 16, 2020 at 7:20 PM Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > > Add VIN and CSI-2 nodes to RZ/G2H (R8A774E1) SoC dtsi.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> > > >
> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > >
> > > > However, before I queue this in renesas-devel for v5.10, I'd like to
> > > > have some clarification about the issue below.
> > > >
> > > > > --- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> > > > > +++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> > > >
> > > > > +               vin4: video@e6ef4000 {
> > > > > +                       compatible = "renesas,vin-r8a774e1";
> > > > > +                       reg = <0 0xe6ef4000 0 0x1000>;
> > > > > +                       interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +                       clocks = <&cpg CPG_MOD 807>;
> > > > > +                       power-domains = <&sysc R8A774E1_PD_ALWAYS_ON>;
> > > > > +                       resets = <&cpg 807>;
> > > > > +                       renesas,id = <4>;
> > > > > +                       status = "disabled";
> > > > > +
> > > > > +                       ports {
> > > > > +                               #address-cells = <1>;
> > > > > +                               #size-cells = <0>;
> > > > > +
> > > > > +                               port@1 {
> > > > > +                                       #address-cells = <1>;
> > > > > +                                       #size-cells = <0>;
> > > >
> > > > "make dtbs W=1" says:
> > > >
> > > >     arch/arm64/boot/dts/renesas/r8a774e1.dtsi:1562.12-1572.7: Warning
> > > > (graph_child_address): /soc/video@e6ef4000/ports/port@1: graph node
> > > > has single child node 'endpoint@0', #address-cells/#size-cells are not
> > > > necessary
> > > >
> > > > (same for vin5-7 below)
> > > >
> > > Referring to commit 5e53dbf4edb4d ("arm64: dts: renesas: r8a77990: Fix
> > > VIN endpoint numbering") we definitely need endpoint numbering.
> > > Probably the driver needs to be fixed to handle such cases.
> >
> > > > > +
> > > > > +                                       reg = <1>;
> > > > > +
> > > > > +                                       vin4csi20: endpoint@0 {
> > > > > +                                               reg = <0>;
> > > > > +                                               remote-endpoint = <&csi20vin4>;
> >
> > On R-Car E3, the single endpoint is at address 2, so "make dtbs W=1"doesn't
> > complain. Here it is at address 0.
> >
> > Niklas?
>
> First the R-Car VIN driver makes decisions based on which endpoint is
> described, each endpoint 0-3 represents a different CSI-2 block on the
> other end (0: CSI20, 1: CSI21, 2: CSI40 and 3: CSI41).

That's my understanding, too.

> Then how to handle the warning I'm not sure. I can only really see 2
> options.
>
> 1. Ignore the warning.
> 2. Remove #address-cells, #size-cells and reg properties from port@ if
>    the only endpoint described is endpoint@0.
>
> I would prefers option 2. that is what we do in other cases (for example
> on Gen2 boards that only have a single parallel sensor in some early DTS
> files we don't have the ports node and just describe a single port with
> the same reasoning.
>
> We are not at risk at someone describing a second CSI-2 bock as an
> overlay so I see no real harm in option 2.

Yeah, no overlay possible for on-SoC wiring ;-)

> What are your thoughts Geert?
> You know more about DT then me.

You have too much faith in me ;-)

AFAIK we don't get this warning for e.g. SPI buses, which can have a
single device at address 0, and #{address,size}-cells is mandatory
there. So endpoints (or SPI?) are treated special?

Gr{oetje,eeting}s,

                        Geert
Niklas Söderlund Aug. 8, 2020, 7:48 a.m. UTC | #19
Hi Geert and Lad,

On 2020-08-07 13:36:46 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Fri, Aug 7, 2020 at 1:27 PM Niklas Söderlund
> <niklas.soderlund@ragnatech.se> wrote:
> > On 2020-08-06 13:47:58 +0200, Geert Uytterhoeven wrote:
> > > On Thu, Aug 6, 2020 at 1:17 PM Lad, Prabhakar
> > > <prabhakar.csengg@gmail.com> wrote:
> > > > On Wed, Aug 5, 2020 at 12:19 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Thu, Jul 16, 2020 at 7:20 PM Lad Prabhakar
> > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > > > Add VIN and CSI-2 nodes to RZ/G2H (R8A774E1) SoC dtsi.
> > > > > >
> > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> > > > >
> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > >
> > > > > However, before I queue this in renesas-devel for v5.10, I'd like to
> > > > > have some clarification about the issue below.
> > > > >
> > > > > > --- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
> > > > >
> > > > > > +               vin4: video@e6ef4000 {
> > > > > > +                       compatible = "renesas,vin-r8a774e1";
> > > > > > +                       reg = <0 0xe6ef4000 0 0x1000>;
> > > > > > +                       interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > +                       clocks = <&cpg CPG_MOD 807>;
> > > > > > +                       power-domains = <&sysc R8A774E1_PD_ALWAYS_ON>;
> > > > > > +                       resets = <&cpg 807>;
> > > > > > +                       renesas,id = <4>;
> > > > > > +                       status = "disabled";
> > > > > > +
> > > > > > +                       ports {
> > > > > > +                               #address-cells = <1>;
> > > > > > +                               #size-cells = <0>;
> > > > > > +
> > > > > > +                               port@1 {
> > > > > > +                                       #address-cells = <1>;
> > > > > > +                                       #size-cells = <0>;
> > > > >
> > > > > "make dtbs W=1" says:
> > > > >
> > > > >     arch/arm64/boot/dts/renesas/r8a774e1.dtsi:1562.12-1572.7: Warning
> > > > > (graph_child_address): /soc/video@e6ef4000/ports/port@1: graph node
> > > > > has single child node 'endpoint@0', #address-cells/#size-cells are not
> > > > > necessary
> > > > >
> > > > > (same for vin5-7 below)
> > > > >
> > > > Referring to commit 5e53dbf4edb4d ("arm64: dts: renesas: r8a77990: Fix
> > > > VIN endpoint numbering") we definitely need endpoint numbering.
> > > > Probably the driver needs to be fixed to handle such cases.
> > >
> > > > > > +
> > > > > > +                                       reg = <1>;
> > > > > > +
> > > > > > +                                       vin4csi20: endpoint@0 {
> > > > > > +                                               reg = <0>;
> > > > > > +                                               remote-endpoint = <&csi20vin4>;
> > >
> > > On R-Car E3, the single endpoint is at address 2, so "make dtbs W=1"doesn't
> > > complain. Here it is at address 0.
> > >
> > > Niklas?
> >
> > First the R-Car VIN driver makes decisions based on which endpoint is
> > described, each endpoint 0-3 represents a different CSI-2 block on the
> > other end (0: CSI20, 1: CSI21, 2: CSI40 and 3: CSI41).
> 
> That's my understanding, too.
> 
> > Then how to handle the warning I'm not sure. I can only really see 2
> > options.
> >
> > 1. Ignore the warning.
> > 2. Remove #address-cells, #size-cells and reg properties from port@ if
> >    the only endpoint described is endpoint@0.
> >
> > I would prefers option 2. that is what we do in other cases (for example
> > on Gen2 boards that only have a single parallel sensor in some early DTS
> > files we don't have the ports node and just describe a single port with
> > the same reasoning.
> >
> > We are not at risk at someone describing a second CSI-2 bock as an
> > overlay so I see no real harm in option 2.
> 
> Yeah, no overlay possible for on-SoC wiring ;-)
> 
> > What are your thoughts Geert?
> > You know more about DT then me.
> 
> You have too much faith in me ;-)
> 
> AFAIK we don't get this warning for e.g. SPI buses, which can have a
> single device at address 0, and #{address,size}-cells is mandatory
> there. So endpoints (or SPI?) are treated special?

That is a good question, I don't know if either of those are treated 
special. Lad could you look into this?

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