All of lore.kernel.org
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Lad Prabhakar <prabhakar.csengg@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Marek Vasut <marek.vasut+renesas@gmail.com>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Murray <andrew.murray@arm.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Chris Paterson <Chris.Paterson2@renesas.com>,
	"Lad, Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: Re: [PATCH 3/5] PCI: rcar: Add R-Car PCIe endpoint device tree bindings
Date: Thu, 7 Nov 2019 09:44:31 +0100	[thread overview]
Message-ID: <CAMuHMdVZwgVnq2kwjNJQHfvUH0sk6M7Hz-AJR82jMOsCNfW9wQ@mail.gmail.com> (raw)
In-Reply-To: <20191106193609.19645-4-prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi Prabhakar,

On Wed, Nov 6, 2019 at 8:36 PM Lad Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: "Lad, Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> This patch adds the bindings for the R-Car PCIe endpoint driver.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rcar-pci-ep.txt
> @@ -0,0 +1,43 @@
> +* Renesas R-Car PCIe Endpoint Controller DT description
> +
> +Required properties:
> +           "renesas,pcie-ep-r8a774c0" for the R8A774C0 SoC;
> +           "renesas,pcie-ep-rcar-gen3" for a generic R-Car Gen3 or
> +                                    RZ/G2 compatible device.

Unless I'm missing something, this is for the exact same hardware block as
Documentation/devicetree/bindings/pci/rcar-pci.txt?
So shouldn't you amend those bindings, instead of adding new compatible
values?
Please remember that DT describes hardware, not software policy.
So IMHO choosing between host and endpoint is purely a configuration
issue, and could be indicated by the presence or lack of some DT properties.
E.g. host mode requires both "bus-range" and "device_type" properties,
so their absence could indicate endpoint mode.

> +- reg: Five register ranges as listed in the reg-names property
> +- reg-names: Must include the following names
> +       - "apb-base"
> +       - "memory0"
> +       - "memory1"
> +       - "memory2"
> +       - "memory3"

What is the purpose of the last 4 regions?
Can they be chosen by the driver, at runtime?

> +- resets: Must contain phandles to PCIe-related reset lines exposed by IP block
> +- clocks: from common clock binding: clock specifiers for the PCIe controller
> +        clock.
> +- clock-names: from common clock binding: should be "pcie".
> +
> +Optional Property:
> +- max-functions: Maximum number of functions that can be configured (default 1).
> +
> +Example:
> +
> +SoC-specific DT Entry:
> +
> +       pcie_ep: pcie_ep@fe000000 {
> +               compatible = "renesas,pcie-r8a7791", "renesas,pcie-rcar-gen2";

These compatible values do not match with the ones above
(but they match with what I'd like to see ;-)

> +               reg = <0 0xfe000000 0 0x80000>,
> +                       <0x0 0xfe100000 0 0x100000>,
> +                       <0x0 0xfe200000 0 0x200000>,
> +                       <0x0 0x30000000 0 0x8000000>,
> +                       <0x0 0x38000000 0 0x8000000>;
> +               reg-names = "apb-base", "memory0", "memory1", "memory2", "memory3";
> +               clocks = <&cpg CPG_MOD 319>;
> +               clock-names = "pcie";
> +               power-domains = <&sysc R8A774C0_PD_ALWAYS_ON>;
> +               resets = <&cpg 319>;
> +       };

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

WARNING: multiple messages have this Message-ID
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Lad Prabhakar <prabhakar.csengg@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>,
	Chris Paterson <Chris.Paterson2@renesas.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	"Lad, Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-pci <linux-pci@vger.kernel.org>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
	Magnus Damm <magnus.damm@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Andrew Murray <andrew.murray@arm.com>,
	Will Deacon <will@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Marek Vasut <marek.vasut+renesas@gmail.com>
Subject: Re: [PATCH 3/5] PCI: rcar: Add R-Car PCIe endpoint device tree bindings
Date: Thu, 7 Nov 2019 09:44:31 +0100	[thread overview]
Message-ID: <CAMuHMdVZwgVnq2kwjNJQHfvUH0sk6M7Hz-AJR82jMOsCNfW9wQ@mail.gmail.com> (raw)
In-Reply-To: <20191106193609.19645-4-prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi Prabhakar,

On Wed, Nov 6, 2019 at 8:36 PM Lad Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: "Lad, Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> This patch adds the bindings for the R-Car PCIe endpoint driver.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rcar-pci-ep.txt
> @@ -0,0 +1,43 @@
> +* Renesas R-Car PCIe Endpoint Controller DT description
> +
> +Required properties:
> +           "renesas,pcie-ep-r8a774c0" for the R8A774C0 SoC;
> +           "renesas,pcie-ep-rcar-gen3" for a generic R-Car Gen3 or
> +                                    RZ/G2 compatible device.

Unless I'm missing something, this is for the exact same hardware block as
Documentation/devicetree/bindings/pci/rcar-pci.txt?
So shouldn't you amend those bindings, instead of adding new compatible
values?
Please remember that DT describes hardware, not software policy.
So IMHO choosing between host and endpoint is purely a configuration
issue, and could be indicated by the presence or lack of some DT properties.
E.g. host mode requires both "bus-range" and "device_type" properties,
so their absence could indicate endpoint mode.

> +- reg: Five register ranges as listed in the reg-names property
> +- reg-names: Must include the following names
> +       - "apb-base"
> +       - "memory0"
> +       - "memory1"
> +       - "memory2"
> +       - "memory3"

What is the purpose of the last 4 regions?
Can they be chosen by the driver, at runtime?

> +- resets: Must contain phandles to PCIe-related reset lines exposed by IP block
> +- clocks: from common clock binding: clock specifiers for the PCIe controller
> +        clock.
> +- clock-names: from common clock binding: should be "pcie".
> +
> +Optional Property:
> +- max-functions: Maximum number of functions that can be configured (default 1).
> +
> +Example:
> +
> +SoC-specific DT Entry:
> +
> +       pcie_ep: pcie_ep@fe000000 {
> +               compatible = "renesas,pcie-r8a7791", "renesas,pcie-rcar-gen2";

These compatible values do not match with the ones above
(but they match with what I'd like to see ;-)

> +               reg = <0 0xfe000000 0 0x80000>,
> +                       <0x0 0xfe100000 0 0x100000>,
> +                       <0x0 0xfe200000 0 0x200000>,
> +                       <0x0 0x30000000 0 0x8000000>,
> +                       <0x0 0x38000000 0 0x8000000>;
> +               reg-names = "apb-base", "memory0", "memory1", "memory2", "memory3";
> +               clocks = <&cpg CPG_MOD 319>;
> +               clock-names = "pcie";
> +               power-domains = <&sysc R8A774C0_PD_ALWAYS_ON>;
> +               resets = <&cpg 319>;
> +       };

Gr{oetje,eeting}s,

                        Geert

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

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

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

  parent reply	other threads:[~2019-11-07  8:44 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-06 19:36 [PATCH 0/5] Add support for PCIe controller to work in endpoint mode on R-Car SoCs Lad Prabhakar
2019-11-06 19:36 ` Lad Prabhakar
2019-11-06 19:36 ` [PATCH 1/5] pci: pcie-rcar: preparation for adding endpoint support Lad Prabhakar
2019-11-06 19:36   ` Lad Prabhakar
2019-11-27  4:55   ` Kishon Vijay Abraham I
2019-11-27  4:55     ` Kishon Vijay Abraham I
2019-11-27 20:51     ` Lad, Prabhakar
2019-11-27 20:51       ` Lad, Prabhakar
2019-11-06 19:36 ` [PATCH 2/5] pci: endpoint: add support to handle multiple base for mapping outbound memory Lad Prabhakar
2019-11-06 19:36   ` Lad Prabhakar
2019-11-27  5:14   ` Kishon Vijay Abraham I
2019-11-27  5:14     ` Kishon Vijay Abraham I
2019-11-27  5:14     ` Kishon Vijay Abraham I
2019-11-27 21:21     ` Lad, Prabhakar
2019-11-27 21:21       ` Lad, Prabhakar
2019-11-27 21:21       ` Lad, Prabhakar
2019-12-05 10:22       ` Kishon Vijay Abraham I
2019-12-05 10:22         ` Kishon Vijay Abraham I
2019-12-05 10:22         ` Kishon Vijay Abraham I
2019-12-06 13:08         ` Lad, Prabhakar
2019-12-06 13:08           ` Lad, Prabhakar
2019-12-06 13:08           ` Lad, Prabhakar
2019-11-27  6:05   ` Kishon Vijay Abraham I
2019-11-27  6:05     ` Kishon Vijay Abraham I
2019-11-27  6:05     ` Kishon Vijay Abraham I
2019-11-27 21:28     ` Lad, Prabhakar
2019-11-27 21:28       ` Lad, Prabhakar
2019-11-27 21:28       ` Lad, Prabhakar
2019-11-06 19:36 ` [PATCH 3/5] PCI: rcar: Add R-Car PCIe endpoint device tree bindings Lad Prabhakar
2019-11-06 19:36   ` Lad Prabhakar
2019-11-07  7:39   ` Biju Das
2019-11-07  7:39     ` Biju Das
2019-11-07  8:10     ` Lad, Prabhakar
2019-11-07  8:10       ` Lad, Prabhakar
2019-11-07  8:44   ` Geert Uytterhoeven [this message]
2019-11-07  8:44     ` Geert Uytterhoeven
2019-11-07  9:25     ` Lad, Prabhakar
2019-11-07  9:25       ` Lad, Prabhakar
2019-11-07 20:08       ` Geert Uytterhoeven
2019-11-07 20:08         ` Geert Uytterhoeven
2019-11-07 22:46         ` Lad, Prabhakar
2019-11-07 22:46           ` Lad, Prabhakar
2019-11-08  8:36           ` Geert Uytterhoeven
2019-11-08  8:36             ` Geert Uytterhoeven
2019-11-13  4:08         ` Rob Herring
2019-11-13  4:08           ` Rob Herring
2019-11-27  5:44           ` Kishon Vijay Abraham I
2019-11-27  5:44             ` Kishon Vijay Abraham I
2019-11-27 21:00             ` Lad, Prabhakar
2019-11-27 21:00               ` Lad, Prabhakar
2019-11-06 19:36 ` [PATCH 4/5] pci: rcar: add support for rcar pcie controller in endpoint mode Lad Prabhakar
2019-11-06 19:36   ` Lad Prabhakar
2019-11-06 19:36 ` [PATCH 5/5] misc: pci_endpoint_test: add device-id for RZ/G2 pcie controller Lad Prabhakar
2019-11-06 19:36   ` Lad Prabhakar
2019-11-27  5:45   ` Kishon Vijay Abraham I
2019-11-27  5:45     ` Kishon Vijay Abraham I
2019-11-26 14:33 ` [PATCH 0/5] Add support for PCIe controller to work in endpoint mode on R-Car SoCs Lad, Prabhakar
2019-11-26 14:33   ` Lad, Prabhakar
2019-11-27  6:07   ` Kishon Vijay Abraham I
2019-11-27  6:07     ` Kishon Vijay Abraham I

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMuHMdVZwgVnq2kwjNJQHfvUH0sk6M7Hz-AJR82jMOsCNfW9wQ@mail.gmail.com \
    --to=geert@linux-m68k.org \
    --cc=Chris.Paterson2@renesas.com \
    --cc=andrew.murray@arm.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=kishon@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=magnus.damm@gmail.com \
    --cc=marek.vasut+renesas@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=prabhakar.csengg@gmail.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=robh+dt@kernel.org \
    --cc=will@kernel.org \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.