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

Chen Wang
2025-01-07T00:44:05Z

On 2025/1/7 8:18, Bjorn Helgaas wrote:
> 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?

Yes, that match my understanding.