[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250122222147.GA1117670@bhelgaas>
Date: Wed, 22 Jan 2025 16:21:47 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Chen Wang <unicornxw@...il.com>
Cc: kw@...ux.com, u.kleine-koenig@...libre.com, aou@...s.berkeley.edu,
arnd@...db.de, bhelgaas@...gle.com, unicorn_wang@...look.com,
conor+dt@...nel.org, guoren@...nel.org, inochiama@...look.com,
krzk+dt@...nel.org, lee@...nel.org, lpieralisi@...nel.org,
manivannan.sadhasivam@...aro.org, palmer@...belt.com,
paul.walmsley@...ive.com, pbrobinson@...il.com, robh@...nel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, linux-riscv@...ts.infradead.org,
chao.wei@...hgo.com, xiaoguang.xing@...hgo.com,
fengchun.li@...hgo.com
Subject: Re: [PATCH v3 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
On Wed, Jan 15, 2025 at 03:06:37PM +0800, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@...look.com>
>
> Add binding for Sophgo SG2042 PCIe host controller.
> + sophgo,link-id:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + SG2042 uses Cadence IP, every IP is composed of 2 cores (called link0
> + & link1 as Cadence's term). Each core corresponds to a host bridge,
> + and each host bridge has only one root port. Their configuration
> + registers are completely independent. SG2042 integrates two Cadence IPs,
> + so there can actually be up to four host bridges. "sophgo,link-id" is
> + used to identify which core/link the PCIe host bridge node corresponds to.
IIUC, the registers of Cadence IP 1 and IP 2 are completely
independent, and if you describe both of them, you would have separate
"pcie@...00000" stanzas with separate 'reg' and 'ranges' properties.
>From the driver, it does not look like the registers for Link0 and
Link1 are independent, since the driver claims the
"sophgo,sg2042-pcie-host", which includes two Cores, and it tests
pcie->link_id to select the correct register address and bit mask.
"sophgo,link-id" corresponds to Cadence documentation, but I think it
is somewhat misleading in the binding because a PCIe "Link" refers to
the downstream side of a Root Port. If we use "link-id" to identify
either Core0 or Core1 of a Cadence IP, it sort of bakes in the
idea that there can never be more than one Root Port per Core.
Since each Core is the root of a separate PCI hierarchy, it seems like
maybe there should be a stanza for the Core so there's a place where
per-hierarchy things like "linux,pci-domain" properties could go,
e.g.,
pcie@...00000 { // IP 1, single-link mode
compatible = "sophgo,sg2042-pcie-host";
reg = <...>;
ranges = <...>;
core0 {
sophgo,core-id = <0>;
linux,pci-domain = <0>;
port {
num-lanes = <4>; // all lanes
};
};
};
pcie@...00000 { // IP 2, dual-link mode
compatible = "sophgo,sg2042-pcie-host";
reg = <...>;
ranges = <...>;
core0 {
sophgo,core-id = <0>;
linux,pci-domain = <1>;
port {
num-lanes = <2>; // half of lanes
};
};
core1 {
sophgo,core-id = <1>;
linux,pci-domain = <2>;
port {
num-lanes = <2>; // half of lanes
};
};
};
> + 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.
> +
> + 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.
> +
> + Sophgo defines some new register files to add support for their MSI
> + controller inside PCIe. These new register files are defined in DTS as
> + syscon node ("sophgo,sg2042-pcie-ctrl"), i.e. "cdns_pcie0_ctrl" /
> + "cdns_pcie1_ctrl". 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.
> +
> + "sophgo,link-id" is defined to distinguish the two RC's in one Cadence IP,
> + so we can know what registers (bits) we should use.
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + pcie@...00000 {
> + compatible = "sophgo,sg2042-pcie-host";
> + device_type = "pci";
> + reg = <0x62000000 0x00800000>,
> + <0x48000000 0x00001000>;
> + reg-names = "reg", "cfg";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x81000000 0 0x00000000 0xde000000 0 0x00010000>,
> + <0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
> + bus-range = <0x00 0xff>;
> + vendor-id = <0x1f1c>;
> + device-id = <0x2042>;
> + cdns,no-bar-match-nbits = <48>;
> + sophgo,link-id = <0>;
> + sophgo,syscon-pcie-ctrl = <&cdns_pcie1_ctrl>;
> + msi-parent = <&msi_pcie>;
> + msi_pcie: msi {
> + compatible = "sophgo,sg2042-pcie-msi";
> + msi-controller;
> + interrupt-parent = <&intc>;
> + interrupts = <123 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "msi";
> + };
> + };
It would be helpful for me if the example showed how both link-id 0
and link-id 1 would be used (or whatever they end up being named).
I assume both have to be somewhere in the same pcie@...00000 device to
make this work.
Bjorn
Powered by blists - more mailing lists