Re: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host

Bjorn Helgaas
2025-01-07T00:18:41Z

On Thu, Dec 19, 2024 at 10:34:50AM +0800, Chen Wang wrote:
> hello ~
>
> On 2024/12/11 1:33, Bjorn Helgaas wrote:
> > On Mon, Dec 09, 2024 at 03:19:38PM +0800, Chen Wang wrote:
> > > Add binding for Sophgo SG2042 PCIe host controller.
> > > + sophgo,pcie-port:
> [......]
> > > + The Cadence IP has two modes of operation, selected by a strap pin.
> > > +
> > > + In the single-link mode, the Cadence PCIe core instance associated
> > > + with Link0 is connected to all the lanes and the Cadence PCIe core
> > > + instance associated with Link1 is inactive.
> > > +
> > > + In the dual-link mode, the Cadence PCIe core instance associated
> > > + with Link0 is connected to the lower half of the lanes and the
> > > + Cadence PCIe core instance associated with Link1 is connected to
> > > + the upper half of the lanes.
> > I assume this means there are two separate Root Ports, one for Link0
> > and a second for Link1?
> >
> > > + SG2042 contains 2 Cadence IPs and configures the Cores as below:
> > > +
> > > + +-- Core(Link0) <---> pcie_rc0 +-----------------+
> > > + | | |
> > > + Cadence IP 1 --+ | cdns_pcie0_ctrl |
> > > + | | |
> > > + +-- Core(Link1) <---> disabled +-----------------+
> > > +
> > > + +-- Core(Link0) <---> pcie_rc1 +-----------------+
> > > + | | |
> > > + Cadence IP 2 --+ | cdns_pcie1_ctrl |
> > > + | | |
> > > + +-- Core(Link1) <---> pcie_rc2 +-----------------+
> > > +
> > > + pcie_rcX is pcie node ("sophgo,sg2042-pcie-host") defined in DTS.
> > > + cdns_pcie0_ctrl is syscon node ("sophgo,sg2042-pcie-ctrl") defined in DTS
> > > +
> > > + cdns_pcieX_ctrl contains some registers shared by pcie_rcX, even two
> > > + RC(Link)s may share different bits of the same register. For example,
> > > + cdns_pcie1_ctrl contains registers shared by link0 & link1 for Cadence IP 2.
> > An RC doesn't have a Link. A Root Port does.
> >
> > > + "sophgo,pcie-port" is defined to flag which core(link) the rc maps to, with
> > > + this we can know what registers(bits) we should use.
> > > +
> > > + sophgo,syscon-pcie-ctrl:
> > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > + description:
> > > + Phandle to the PCIe System Controller DT node. It's required to
> > > + access some MSI operation registers shared by PCIe RCs.
> > I think this probably means "shared by PCIe Root Ports", not RCs.
> > It's unlikely that this hardware has multiple Root Complexes.
>
> I just double confirmed with sophgo engineers, they told me that the actual
> PCIe design is that there is only one root port under a host bridge. I am
> sorry that my original description and diagram may not make this clear, so
> please allow me to introduce this historical background in detail again.
> Please read it patiently :):
>
> The IP provided by Cadence contains two independent cores (called "links"
> according to the terminology of their manual, the first one is called link0
> and the second one is called link1). Each core corresponds to a host bridge,
> and each host bridge has only one root port, and their configuration
> registers are completely independent. That is to say，one cadence IP
> encapsulates two independent host bridges. SG2042 integrates two Cadence
> IPs, so there can actually be up to four host bridges.
>
>
> Taking a Cadence IP as an example, the two host bridges can be connected to
> different lanes through configuration, which has been described in the
> original message. At present, the configuration of SG2042 is to let core0
> (link0) in the first ip occupy all lanes in the ip, and let core0 (link0)
> and core1 (link1) in the second ip each use half of the lanes in the ip. So
> in the end we only use 3 cores, that's why 3 host bridge nodes are
> configured in dts.

Host bridges are logically separate PCI hierarchies, so these three
host bridges could be in three separate PCI domains, and each one
could use buses 00-ff. Each one contains a single Root Port, so
enumerating could look like this:

0000:00:00.0 Root Port to [bus 01-ff]
0001:00:00.0 Root Port to [bus 01-ff]
0002:00:00.0 Root Port to [bus 01-ff]

Does that match with your understanding?