Re: [PATCH v3 4/9] dt-bindings: PCI: renesas,r9a08g045s33-pcie: Add documentation for the PCIe IP on Renesas RZ/G3S

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Aug 08, 2025 at 02:25:42PM +0300, Claudiu Beznea wrote:
> On 08.07.2025 19:34, Bjorn Helgaas wrote:
> > On Fri, Jul 04, 2025 at 07:14:04PM +0300, Claudiu wrote:
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> The PCIe IP available on the Renesas RZ/G3S complies with the PCI Express
> >> Base Specification 4.0. It is designed for root complex applications and
> >> features a single-lane (x1) implementation. Add documentation for it.
> > 
> >> +++ b/Documentation/devicetree/bindings/pci/renesas,r9a08g045s33-pcie.yaml

> >> +        pcie@11e40000 {
> >> +            compatible = "renesas,r9a08g045s33-pcie";
> >> +            reg = <0 0x11e40000 0 0x10000>;
> >> +            ranges = <0x02000000 0 0x30000000 0 0x30000000 0 0x8000000>;
> >> +            dma-ranges = <0x42000000 0 0x48000000 0 0x48000000 0 0x38000000>;
> >> +            bus-range = <0x0 0xff>;
> ...

> >> +            device_type = "pci";
> >> +            num-lanes = <1>;
> >> +            #address-cells = <3>;
> >> +            #size-cells = <2>;
> >> +            power-domains = <&cpg>;
> >> +            vendor-id = <0x1912>;
> >> +            device-id = <0x0033>;
> > 
> > Some of this is specific to a Root Port, not to the Root Complex
> > as a whole.  E.g., device-type = "pci", num-lanes, vendor-id,
> > device-id, are Root Port properties.  Some of the resets, clocks,
> > and interrupts might be as well.
> > 
> > I really want to separate those out because even though this
> > particular version of this PCIe controller only supports a single
> > Root Port, there are other controllers (and possibly future
> > iterations of this controller) that support multiple Root Ports,
> > and it makes maintenance easier if the DT bindings and the driver
> > structures are similar.
> 
> I'll ask the Renesas HW team about the resets and clocks as the HW
> manual don't offer any information about this.
> 
> If they will confirm some of the clocks and/or resets could be
> controlled as part of a port then patch 3/9 "PCI: of_property:
> Restore the arguments of the next level parent" in this series will
> not be needed anymore. Would you prefer me to abandon it or post it
> as individual patch, if any?

[PATCH v3 3/9] ("PCI: of_property: Restore the arguments of the next
level parent") isn't specific to Renesas RZ/G3S and it doesn't look
like it has anything to do with clocks or resets.  I don't understand
the patch well enough to know whether you should keep it, but it does
look like you should post it separate from the RZ/G3S driver.

When the devicetree contains required information specific to Root
Ports, I would prefer that to be in a separate "pcie@x,y" stanza, even
if there are clocks or resets that apply to all Root Ports.

"num-lanes" is obviously specific to an individual Root Port because
a Root Complex doesn't have lanes at all.  But in the case of RZ/G3S,
I'm not sure "num-lanes" is required in the devicetree; I don't see it
being used in the driver.  If it's not needed, I would just omit it.

It looks like the driver *does* need "vendor-id" and "device-id"
though, and those also are specific to a Root Port because a Root
Complex is not a PCI device and doesn't have its own Vendor or Device
ID.  So I would like them to be in a per-Root Port stanza.  If there
are resets or clocks that affect a Root Port but not the Root Complex
as a whole, they should also be in the Root Port stanza.

Bjorn