Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 23 Oct 2021 16:42:52 +0200
From:   Pali =?utf-8?B?Um9ow6Fy?= <pali@kernel.org>
To:     Rob Herring <robh@kernel.org>
Cc:     Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
        Marek =?utf-8?B?QmVow7pu?= <kabel@kernel.org>,
        Matthew Wilcox <willy@infradead.org>,
        Alex Williamson <alex.williamson@redhat.com>,
        linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: RFC: Change PCI DTS scheme for port/link specific properties
Message-ID: <20211023144252.z7ou2l2tvm6cvtf7@pali>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: NeoMutt/20180716
Precedence: bulk

Hello Rob!

I think that the current DT scheme for PCI buses and devices defined in
Linux kernel tree has wrong definitions of port/link specific properties:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/pci.txt

Port or link specific properties are at least: max-link-speed,
reset-gpios and supports-clkreq. And are currently defined as properties
of host bridges.

Host Bridge contains one or more PCIe Root Ports (each represented as
PCI Bridge device) and to each PCIe Root Port can be connected exactly
one PCIe Upstream device.

PCIe Upstream device can be either endpoint PCIe card or it can be also
PCIe switch is consists of exactly one Upstream Port (represented as PCI
Bridge device) and then one or more Downstream Port devices (each
represent as PCI Bridge device). To each Downstream Port can be
connected again exactly one PCIe Upstream device.

Port or link specific properties (e.g. max-link-speed, reset-gpios and
supports-clkreq) define "the PCIe link" between the Root/Downstream
device and Endpoint/Upstream device. And it is basically Root/Downstream
device which configures the port or link. So I think that these
properties should not be in Host Bridge DTS node, but rather in DTS node
which represents Root Port (or Downstream Port in case of PCIe switches).

Mauro wrote in another email, that he has PCIe topology with
single-root-port host bridge to which is connected multi-port PCIe
switch and he needs to control reset-gpios of devices connected to
downstream ports of PCIe switch.

Current pci.txt DT scheme is fully unsuitable for this kind of setup as
basically PCIe switch is completely independent device of host bridge
and so host bridge really should not define in its node properties which
do not belong to host bridge itself.

Rob, what do you think, how to solve this issue?

I would suggest to define that max-link-speed, reset-gpios and
supports-clkreq properties should be in node for Root Port and
Downstream Port devices and not in host bridge nodes.

So DTS for PCIe controller which has 2 root ports where to first root
port is connected PCIe switch with 2 cards and to second root port is
connected just endpoint card:

pcie-host-bridge {
	compatible = "vendor-controller-string"; /* e.g. designware, etc... */

	pcie-root-port-1@0,0 {
		reg = <0x00000000 0 0 0 0>; /* root port at device 0 */
		reset-gpios = ...; /* resets card connected to root-port-1 which is pcie-switch-1-upstream-port */
		max-link-speed = <3> /* link between root-port-1 and switch is GEN3 */

		pcie-switch-1-upstream-port@0,0 {
			reg = ...; /* upstream port at device 0 */

			pcie-switch-1-downstream-port-1@X,0 {
				reg = ...; /* downstream port 1 at switch specific address */
				reset-gpios = ...; /* resets card connected to switch's port 1 */
				max-link-speed = <1> /* link between this port and card is GEN1 */

				/* optional node for endpoint card */
				/* pcie-card@0,0 { ... }; */
			};

			pcie-switch-1-downstream-port-2@Y,0 {
				reg = ...; /* downstream port 2 at switch specific address */
				reset-gpios = ...; /* resets card connected to switch's port 2 */
				max-link-speed = <1> /* link between this port and card is GEN1 */

				/* optional node for endpoint card */
				/* pcie-card@0,0 { ... }; */
			};
		};
	};

	pcie-root-port-2@1,0 {
		reg = <0x00000800 0 0 0 0>; /* root port at device 1 */
		reset-gpios = ...; /* resets card connected to root-port-2 */
		max-link-speed = <2> /* link between root-port-2 and card below is just GEN2 */

		/* optional node for endpoint card */
		/* pcie-card@0,0 { ... }; */
	};
};

Any opinion?