Add PCIe support to Sophgo SG2042 SoC
Subsystems (7)
v2 cover Add PCIe support to Sophgo SG2042 SoC
From: Chen Wang <[email protected]>
Sophgo's SG2042 SoC uses Cadence PCIe core to implement RC mode.
SG2042 PCIe controller supports two ways to report MSI:
Method A, the PICe controller implements an MSI interrupt controller inside, and connect to PLIC upward through one interrupt line. Provides memory-mapped msi address, and by programming the upper 32 bits of the address to zero, it can be compatible with old pcie devices that only support 32-bit msi address.
Method B, the PICe controller connects to PLIC upward through an independent MSI controller "sophgo,sg2042-msi" on the SOC. The MSI controller provides multiple(up to 32) interrupt sources to PLIC. Compared with the first method, the advantage is that the interrupt source is expanded, but because for SG2042, the msi address provided by the MSI controller is fixed and only supports 64-bit address(> 2^32), it is not compatible with old pcie devices that only support 32-bit msi address.
This patchset depends on another patchset for the SG2042 MSI controller[msi]. If you need to test the DTS part, you need to apply the corresponding patchset.
Thanks,
Chen
---
Changes in v2:
The patch series is based on v6.13-rc2.
Fixed following issues as per comments from Rob Herring, Bjorn Helgaas, thanks.
- Improve driver binding description
- Define a new embeded object property msi to replace the "sophgo,internal-msi".
- Rename "sophgo,link-id" to "sophgo,pcie-port" as per suggestion from Bjorn, and add more explanaion for this property.
- Use msi-parent.
- Improve driver code:
- Improve coding style.
- Fix a bug and make sure num_applied_vecs updated with the max value.
- Use the MSI parent model.
- Remove .cpu_addr_fixup.
- Reorder Kconfig menu item to keep them in alphabetical order by vendor.
Changes in v1:
The patch series is based on v6.12-rc7. You can simply review or test the patches at the link [1].
---
Chen Wang (5):
dt-bindings: pci: Add Sophgo SG2042 PCIe host
PCI: sg2042: Add Sophgo SG2042 PCIe driver
dt-bindings: mfd: syscon: Add sg2042 pcie ctrl compatible
riscv: sophgo: dts: add pcie controllers for SG2042
riscv: sophgo: dts: enable pcie for PioneerBox
.../devicetree/bindings/mfd/syscon.yaml | 2 + .../bindings/pci/sophgo,sg2042-pcie-host.yaml | 141 +++++ .../boot/dts/sophgo/sg2042-milkv-pioneer.dts | 12 + arch/riscv/boot/dts/sophgo/sg2042.dtsi | 89 +++ drivers/pci/controller/cadence/Kconfig | 11 + drivers/pci/controller/cadence/Makefile | 1 + drivers/pci/controller/cadence/pcie-sg2042.c | 534 ++++++++++++++++++ 7 files changed, 790 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml create mode 100644 drivers/pci/controller/cadence/pcie-sg2042.c
base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
Signature · 1 line
2.34.1
PATCH v2 1/5 dt-bindings: pci: Add Sophgo SG2042 PCIe host
From: Chen Wang <[email protected]>
Add binding for Sophgo SG2042 PCIe host controller.
--- .../bindings/pci/sophgo,sg2042-pcie-host.yaml | 141 ++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +141 -0
@@ -0,0 +1,141 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/sophgo,sg2042-pcie-host.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sophgo SG2042 PCIe Host (Cadence PCIe Wrapper) + +description: + Sophgo SG2042 PCIe host controller is based on the Cadence PCIe core. + +maintainers: + - Chen Wang <[email protected]> + +properties: + compatible: + const: sophgo,sg2042-pcie-host + + reg: + maxItems: 2 + + reg-names: + items: + - const: reg + - const: cfg + + vendor-id: + const: 0x1f1c + + device-id: + const: 0x2042 + + msi: + type: object + $ref: /schemas/interrupt-controller/msi-controller.yaml# + unevaluatedProperties: false + + properties: + compatible: + items: + - const: sophgo,sg2042-pcie-msi + + interrupts: + maxItems: 1 + + interrupt-names: + const: msi + + msi-parent: true + + sophgo,pcie-port: + $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). "sophgo,pcie-port" is used to identify which + core/link the pcie host controller node corresponds to. + + 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. + 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. + + "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. + +allOf: + - $ref: cdns-pcie-host.yaml# + +required: + - compatible + - reg + - reg-names + - vendor-id + - device-id + - sophgo,syscon-pcie-ctrl + - sophgo,pcie-port + +additionalProperties: true + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + pcie@62000000 { + 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 = <0x80 0xbf>; + vendor-id = <0x1f1c>; + device-id = <0x2042>; + cdns,no-bar-match-nbits = <48>; + sophgo,pcie-port = <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"; + }; + };
Signature · 1 line
2.34.1
Re: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
Quoted 3 lines · Chen Wang
Add binding for Sophgo SG2042 PCIe host controller.
+ sophgo,pcie-port:
This is just an index, isn't it? I don't see why it should include "sophgo" unless it encodes something sophgo-specific.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 54
Quoted 3 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +3
-0+ $ref: /schemas/types.yaml#/definitions/uint32 + description: | + SG2042 uses Cadence IP, every IP is composed of 2 cores(called link0
Add space before "(". More instances below.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 56
Quoted 2 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +2
-0+ & link1 as Cadence's term). "sophgo,pcie-port" is used to identify which + core/link the pcie host controller node corresponds to.
s/pcie/PCIe/ for consistency in the text. More instances below.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 67
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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?
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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 +-----------------+ + | | |
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 88
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 97
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+ "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.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 109
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+required: + - compatible + - reg + - reg-names + - vendor-id + - device-id + - sophgo,syscon-pcie-ctrl + - sophgo,pcie-port
It looks like vendor-id and device-id apply to PCI devices, i.e., things that will show up in lspci, I assume Root Ports in this case. Can we make this explicit in the DT, e.g., something like this?
pcie@62000000 {
compatible = "sophgo,sg2042-pcie-host";
port0: pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};Quoted 34 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +31
-0+additionalProperties: true + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + pcie@62000000 { + 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 = <0x80 0xbf>; + vendor-id = <0x1f1c>; + device-id = <0x2042>; + cdns,no-bar-match-nbits = <48>; + sophgo,pcie-port = <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"; + }; + };Signature · 1 line
2.34.1
Re: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
Quoted 5 lines · Bjorn Helgaas
Quoted 2 lines · Chen Wang
Add binding for Sophgo SG2042 PCIe host controller.
+ sophgo,pcie-port:This is just an index, isn't it? I don't see why it should include "sophgo" unless it encodes something sophgo-specific.
I previously understood that if it is not a standard attribute defined by the dts schema, such as pcie-port, which is defined by me, it must be prefixed with vendor. Is that right?
Quoted 5 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 54
Quoted 4 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +3
-0+ $ref: /schemas/types.yaml#/definitions/uint32 + description: | + SG2042 uses Cadence IP, every IP is composed of 2 cores(called link0Add space before "(". More instances below.
ok
Quoted 4 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 56
Quoted 3 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +2
-0+ & link1 as Cadence's term). "sophgo,pcie-port" is used to identify which + core/link the pcie host controller node corresponds to.s/pcie/PCIe/ for consistency in the text. More instances below.
ok
Quoted 13 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 67
Quoted 11 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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?
Yes. So the naming of pcie_rcX is wrong, I will correct it, thanks.
Quoted 31 lines
Quoted 11 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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 +-----------------+ + | | |Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 88
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 97
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+ "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.
Quoted 2 lines
I think this probably means "shared by PCIe Root Ports", not RCs. It's unlikely that this hardware has multiple Root Complexes.
Ok, I will fix this.
Quoted 13 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 109
Quoted 9 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+required: + - compatible + - reg + - reg-names + - vendor-id + - device-id + - sophgo,syscon-pcie-ctrl + - sophgo,pcie-portIt looks like vendor-id and device-id apply to PCI devices, i.e., things that will show up in lspci, I assume Root Ports in this case. Can we make this explicit in the DT, e.g., something like this?
Quoted 6 lines
pcie@62000000 {
compatible = "sophgo,sg2042-pcie-host";
port0: pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};Sorry, I don't understand your meaning very well. Referring to the topology diagram I drew above, is it okay to write DTS as follows?
pcie@7060000000 {
compatible = "sophgo,sg2042-pcie-host";
...... // other properties
pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};
}
pcie@7062000000 {
compatible = "sophgo,sg2042-pcie-host";
...... // other properties
pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};
}
pcie@7062800000 {
compatible = "sophgo,sg2042-pcie-host";
...... // other properties
pci@1,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};
}
And with this change, I can drop the “pcie-port”property and use the port name to figure out the port number, right?
Quoted 34 lines
Quoted 34 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +31
-0+additionalProperties: true + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + pcie@62000000 { + 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 = <0x80 0xbf>; + vendor-id = <0x1f1c>; + device-id = <0x2042>; + cdns,no-bar-match-nbits = <48>; + sophgo,pcie-port = <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"; + }; + };Signature · 1 line
2.34.1
Re: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
[cc->to: Rob, Krzysztof, Conor because I'm not a DT expert and I'd like their thoughts on this idea of describing Root Ports as separate children]
Quoted 87 lines · Chen Wang
Quoted 50 lines · Bjorn Helgaas
Quoted 33 lines · Chen Wang
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +31
-0+ 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. + 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. + "sophgo,pcie-port" is defined to flag which core(link) the rc maps to, withDocumentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 109
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +9
-0+ this we can know what registers(bits) we should use. +required: + - compatible + - reg + - reg-names + - vendor-id + - device-id + - sophgo,syscon-pcie-ctrl + - sophgo,pcie-portIt looks like vendor-id and device-id apply to PCI devices, i.e., things that will show up in lspci, I assume Root Ports in this case. Can we make this explicit in the DT, e.g., something like this?
Quoted 6 lines
pcie@62000000 { compatible = "sophgo,sg2042-pcie-host"; port0: pci@0,0 { vendor-id = <0x1f1c>; device-id = <0x2042>; };Sorry, I don't understand your meaning very well. Referring to the topology diagram I drew above, is it okay to write DTS as follows?
pcie@7060000000 {
compatible = "sophgo,sg2042-pcie-host";
...... // other properties
pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};
}pcie@7062000000 {
compatible = "sophgo,sg2042-pcie-host";
...... // other properties
pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};
}pcie@7062800000 {
compatible = "sophgo,sg2042-pcie-host";
...... // other properties
pci@1,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};
Quoted 1 line
}
Generally makes sense to me. I'm suggesting that we should start describing Root Ports as children of the host bridge node instead of mixing their properties into the host bridge itself.
Some properties apply to the host bridge, e.g., "bus-range" describes the bus number aperture, and "ranges" describes the address translation between the upstream CPU address space and the PCI address space.
Others apply specifically to a Root Port, e.g., "num-lanes", "max-link-speed", "phys", "vendor-id", "device-id". I think it will help if we can describe these in separate children, especially when there are multiple Root Ports.
Documentation/devicetree/bindings/pci/pci.txt says a Root Port should include a reg property that contains the bus/device/function number of the RP, e.g., Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt has this:
pcie-controller@3000 {
compatible = "nvidia,tegra30-pcie";
pci@1,0 {
reg = <0x000800 0 0 0 0>;
};where the "0x000800 0 0 0 0" means the "pci@1,0" Root Port is at 00:01.0 (bus 00, device 01, function 0). I don't know what the "@1,0" part means.
Quoted 2 lines
And with this change, I can drop the “pcie-port”property and use the port name to figure out the port number, right?
Seems likely to me.
Quoted 31 lines
Quoted 31 lines
Quoted 31 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +31
-0+additionalProperties: true + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + pcie@62000000 { + 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 = <0x80 0xbf>; + vendor-id = <0x1f1c>; + device-id = <0x2042>; + cdns,no-bar-match-nbits = <48>; + sophgo,pcie-port = <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"; + }; + };
Rob Herring
maintainerRe: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
Quoted 103 lines · Bjorn Helgaas
[
cc->to: Rob, Krzysztof, Conor because I'm not a DT expert and I'd like their thoughts on this idea of describing Root Ports as separate children]Quoted 87 lines · Chen Wang
Quoted 50 lines · Bjorn Helgaas
Quoted 33 lines · Chen Wang
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +31
-0+ 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. + 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. + "sophgo,pcie-port" is defined to flag which core(link) the rc maps to, withDocumentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 109
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +9
-0+ this we can know what registers(bits) we should use. +required: + - compatible + - reg + - reg-names + - vendor-id + - device-id + - sophgo,syscon-pcie-ctrl + - sophgo,pcie-portIt looks like vendor-id and device-id apply to PCI devices, i.e., things that will show up in lspci, I assume Root Ports in this case. Can we make this explicit in the DT, e.g., something like this?
Quoted 6 lines
pcie@62000000 { compatible = "sophgo,sg2042-pcie-host"; port0: pci@0,0 { vendor-id = <0x1f1c>; device-id = <0x2042>; };Sorry, I don't understand your meaning very well. Referring to the topology diagram I drew above, is it okay to write DTS as follows?
pcie@7060000000 {
compatible = "sophgo,sg2042-pcie-host";
...... // other properties
pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};
}pcie@7062000000 {
compatible = "sophgo,sg2042-pcie-host";
...... // other properties
pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};
}pcie@7062800000 {
compatible = "sophgo,sg2042-pcie-host";
...... // other properties
pci@1,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};Quoted 1 line
}
Generally makes sense to me. I'm suggesting that we should start describing Root Ports as children of the host bridge node instead of mixing their properties into the host bridge itself.
Some properties apply to the host bridge, e.g., "bus-range" describes the bus number aperture, and "ranges" describes the address translation between the upstream CPU address space and the PCI address space.
Quoted 4 lines
Others apply specifically to a Root Port, e.g., "num-lanes", "max-link-speed", "phys", "vendor-id", "device-id". I think it will help if we can describe these in separate children, especially when there are multiple Root Ports.
Agreed.
Quoted 12 lines
Documentation/devicetree/bindings/pci/pci.txtsays a Root Port should include a reg property that contains the bus/device/function number of the RP, e.g.,Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txthas this:pcie-controller@3000 { compatible = "nvidia,tegra30-pcie"; pci@1,0 { reg = <0x000800 0 0 0 0>; };
Quoted 8 lines
where the "
0x0008000 0 0 0" means the "pci@1,0" Root Port is at 00:01.0 (bus 00, device 01, function 0). I don't know what the "@1,0" part means.Quoted 2 lines
And with this change, I can drop the “pcie-port”property and use the port name to figure out the port number, right?
Seems likely to me.
I don't think device 1 would be the correct address. The RP is almost always device 0.
I think instead the 'syscon-pcie-ctrl' should perhaps be modeled as a phy with the phy binding. Then the host bridge node can have 1 or 2 phy entries depending on if the host uses 1 or 2 links. And the 2nd host should have 'status = "disabled";' when it is not used.
Or perhaps just 'num-lanes' would be enough.
Rob
Re: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
In reply to Bjorn Helgaashello ~
Quoted 3 lines · Bjorn Helgaas
Quoted 2 lines · Chen Wang
Add binding for Sophgo SG2042 PCIe host controller.
+ sophgo,pcie-port:
[......]
Quoted 43 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 67
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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?
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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 +-----------------+ + | | |Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 88
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 97
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+ "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.
Quoted 2 lines
I think this probably means "shared by PCIe Root Ports", not RCs. It's unlikely that this hardware has multiple Root Complexes.
hi, Bjorn,
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.
Because the configurations of these links are independent, the story ends here, but unfortunately, sophgo engineers defined some new register files to add support for their msi controller inside pcie. The problem is they did not separate these register files according to link0 and link1. These new register files are "cdns_pcie0_ctrl" / "cdns_pcie1_ctrl" in the original picture and dts, where the register of "cdns_pcie0_ctrl" is shared by link0 and link1 of the first ip, and "cdns_pcie1_ctrl" is shared by link0 and link1 of the second ip. According to my new description, "cdns_pcieX_ctrl" is not shared by root ports, they are shared by host bridge/rc.
Because the register design of "cdns_pcieX_ctrl" is not strictly segmented according to link0 and link1, in pcie host bridge driver coding we must know whether the host bridge corresponds to link0 or link1 in the ip, so the "sophgo,link-id" attribute is introduced.
Now I think it is not appropriate to change it to "sophgo,pcie-port". The reason is that as mentioned above, there is only one root port under each host bridge in the cadence ip. Link0 and link1 are actually used to distinguish the two host bridges in one ip.
So I suggest to keep the original "sophgo,link-id" and with the prefix because the introduction of this attribute is indeed caused by the private design of sophgo.
Any other good idea please feel free let me know.
Thansk,
Chen
Quoted 12 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 109
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+required: + - compatible + - reg + - reg-names + - vendor-id + - device-id + - sophgo,syscon-pcie-ctrl + - sophgo,pcie-portIt looks like vendor-id and device-id apply to PCI devices, i.e., things that will show up in lspci, I assume Root Ports in this case. Can we make this explicit in the DT, e.g., something like this?
Quoted 6 lines
pcie@62000000 {
compatible = "sophgo,sg2042-pcie-host";
port0: pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};As I mentioned above, there is actually only one root port under a host bridge, so I think it is unnecessary to introduce the port subnode.
In addition, I found that it is also allowed to directly add the vendor-id and device-id properties directly under the host bridge, see https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/pci/pci-host-bridge.yaml
And refer to the dts for those products using cadence ip: arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
In this way, when executing lspci, the vendor id and device id will appear in the line corresponding to the pci brdge device.
[......]
Rob Herring
maintainerRe: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
Quoted 123 lines · Chen Wang
hello ~
Quoted 3 lines · Bjorn Helgaas
Quoted 2 lines · Chen Wang
Add binding for Sophgo SG2042 PCIe host controller.
+ sophgo,pcie-port:[......]
Quoted 43 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 67
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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?
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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 +-----------------+ + | | |Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 88
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 97
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+ "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.Quoted 2 lines
I think this probably means "shared by PCIe Root Ports", not RCs. It's unlikely that this hardware has multiple Root Complexes.
hi, Bjorn,
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.
Because the configurations of these links are independent, the story ends here, but unfortunately, sophgo engineers defined some new register files to add support for their msi controller inside pcie. The problem is they did not separate these register files according to link0 and link1. These new register files are "
cdns_pcie0_ctrl" / "cdns_pcie1_ctrl" in the original picture and dts, where the register of "cdns_pcie0_ctrl" is shared by link0 and link1 of the first ip, and "cdns_pcie1_ctrl" is shared by link0 and link1 of the second ip. According to my new description, "cdns_pcieX_ctrl" is not shared by root ports, they are shared by host bridge/rc.Because the register design of "
cdns_pcieX_ctrl" is not strictly segmented according to link0 and link1, in pcie host bridge driver coding we must know whether the host bridge corresponds to link0 or link1 in the ip, so the "sophgo,link-id" attribute is introduced.Now I think it is not appropriate to change it to "sophgo,pcie-port". The reason is that as mentioned above, there is only one root port under each host bridge in the cadence ip. Link0 and link1 are actually used to distinguish the two host bridges in one ip.
So I suggest to keep the original "sophgo,link-id" and with the prefix because the introduction of this attribute is indeed caused by the private design of sophgo.
Any other good idea please feel free let me know.
Thansk,
Chen
Quoted 12 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 109
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+required: + - compatible + - reg + - reg-names + - vendor-id + - device-id + - sophgo,syscon-pcie-ctrl + - sophgo,pcie-portIt looks like vendor-id and device-id apply to PCI devices, i.e., things that will show up in lspci, I assume Root Ports in this case. Can we make this explicit in the DT, e.g., something like this?
Quoted 8 lines
Quoted 6 lines
pcie@62000000 { compatible = "sophgo,sg2042-pcie-host"; port0: pci@0,0 { vendor-id = <0x1f1c>; device-id = <0x2042>; };As I mentioned above, there is actually only one root port under a host bridge, so I think it is unnecessary to introduce the port subnode.
It doesn't matter how many RPs there are. What matters is what are the properties associated with.
Quoted 4 lines
In addition, I found that it is also allowed to directly add the vendor-id
and device-id properties directly under the host bridge, see https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/pci/pci-host-bridge.yaml
And refer to the dts for those products using cadence ip:arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
It's allowed because we are stuck things with the wrong way. That doesn't mean we should continue to do so.
Quoted 2 lines
In this way, when executing lspci, the vendor id and device id will appear in the line corresponding to the pci brdge device.
That's the RP though, isn't it?
There is one difference in location though. If the properties are in the RP, then they should be handled by the PCI core and override what's read from the RP registers. If the properties are in the host bridge node, then the host bridge driver sets the values in some host bridge specific registers (or has a way to make read-only regs writeable) which get reflected in the RP registers. So perhaps in the host bridge is the correct place.
Rob
Re: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
Quoted 145 lines · Rob Herring
Quoted 122 lines · Chen Wang
hello ~
Quoted 3 lines · Bjorn Helgaas
Quoted 2 lines · Chen Wang
Add binding for Sophgo SG2042 PCIe host controller.
+ sophgo,pcie-port:[......]
Quoted 43 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 67
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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?
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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 +-----------------+ + | | |Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 88
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 97
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+ "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.Quoted 2 lines
I think this probably means "shared by PCIe Root Ports", not RCs. It's unlikely that this hardware has multiple Root Complexes.
hi, Bjorn,
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.
Because the configurations of these links are independent, the story ends here, but unfortunately, sophgo engineers defined some new register files to add support for their msi controller inside pcie. The problem is they did not separate these register files according to link0 and link1. These new register files are "
cdns_pcie0_ctrl" / "cdns_pcie1_ctrl" in the original picture and dts, where the register of "cdns_pcie0_ctrl" is shared by link0 and link1 of the first ip, and "cdns_pcie1_ctrl" is shared by link0 and link1 of the second ip. According to my new description, "cdns_pcieX_ctrl" is not shared by root ports, they are shared by host bridge/rc.Because the register design of "
cdns_pcieX_ctrl" is not strictly segmented according to link0 and link1, in pcie host bridge driver coding we must know whether the host bridge corresponds to link0 or link1 in the ip, so the "sophgo,link-id" attribute is introduced.Now I think it is not appropriate to change it to "sophgo,pcie-port". The reason is that as mentioned above, there is only one root port under each host bridge in the cadence ip. Link0 and link1 are actually used to distinguish the two host bridges in one ip.
So I suggest to keep the original "sophgo,link-id" and with the prefix because the introduction of this attribute is indeed caused by the private design of sophgo.
Any other good idea please feel free let me know.
Thansk,
Chen
Quoted 12 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 109
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+required: + - compatible + - reg + - reg-names + - vendor-id + - device-id + - sophgo,syscon-pcie-ctrl + - sophgo,pcie-portIt looks like vendor-id and device-id apply to PCI devices, i.e., things that will show up in lspci, I assume Root Ports in this case. Can we make this explicit in the DT, e.g., something like this?
Quoted 8 lines
Quoted 6 lines
pcie@62000000 { compatible = "sophgo,sg2042-pcie-host"; port0: pci@0,0 { vendor-id = <0x1f1c>; device-id = <0x2042>; };As I mentioned above, there is actually only one root port under a host bridge, so I think it is unnecessary to introduce the port subnode.
It doesn't matter how many RPs there are. What matters is what are the properties associated with.
Quoted 4 lines
In addition, I found that it is also allowed to directly add the vendor-id
and device-id properties directly under the host bridge, see https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/pci/pci-host-bridge.yaml
And refer to the dts for those products using cadence ip:arch/arm64/boot/dts/ti/k3-j721e-main.dtsiIt's allowed because we are stuck things with the wrong way. That doesn't mean we should continue to do so.
Quoted 2 lines
In this way, when executing lspci, the vendor id and device id will appear in the line corresponding to the pci brdge device.
That's the RP though, isn't it?
Quoted 7 lines
There is one difference in location though. If the properties are in the RP, then they should be handled by the PCI core and override what's read from the RP registers. If the properties are in the host bridge node, then the host bridge driver sets the values in some host bridge specific registers (or has a way to make read-only regs writeable) which get reflected in the RP registers. So perhaps in the host bridge is the correct place.
Yes, cadence host brdige will handle the regiser setting for these ids, see function cdns_pcie_host_setup() in drivers/pci/controller/cadence/pcie-cadence-host.c.
So for this case, in the host bridge is the correct place.
Thanks,
Chen
Quoted 2 lines
Rob
Re: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
In reply to Chen WangHello & Happy New Year, Bjorn
Quoted 112 lines · Chen Wang
hello ~
Quoted 3 lines · Bjorn Helgaas
Quoted 2 lines · Chen Wang
Add binding for Sophgo SG2042 PCIe host controller. + sophgo,pcie-port:
[......]
Quoted 58 lines
Quoted 5 lines
+ The Cadence IP has two modes of operation, selected by a strap pin.
+
+ In the single-link mode, the Cadence PCIe core instance associatedQuoted 8 lines
+ 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?
Quoted 17 lines
+ 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 | + | | |Quoted 14 lines
+ +-- Core(Link1) <--->
pcie_rc2
+-----------------+
+
+pcie_rcXis pcie node ("sophgo,sg2042-pcie-host") defined in DTS.
+cdns_pcie0_ctrlis syscon node ("sophgo,sg2042-pcie-ctrl") defined in DTS
+
+cdns_pcieX_ctrlcontains some registers shared bypcie_rcX, even two
+ RC(Link)s may share different bits of the same register. For example,
+cdns_pcie1_ctrlcontains registers shared by link0 & link1 for Cadence IP 2.An RC doesn't have a Link. A Root Port does.
Quoted 9 lines
+ "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.Quoted 2 lines
I think this probably means "shared by PCIe Root Ports", not RCs. It's unlikely that this hardware has multiple Root Complexes.
hi, Bjorn,
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.
Because the configurations of these links are independent, the story ends here, but unfortunately, sophgo engineers defined some new register files to add support for their msi controller inside pcie. The problem is they did not separate these register files according to link0 and link1. These new register files are "
cdns_pcie0_ctrl" / "cdns_pcie1_ctrl" in the original picture and dts, where the register of "cdns_pcie0_ctrl" is shared by link0 and link1 of the first ip, and "cdns_pcie1_ctrl" is shared by link0 and link1 of the second ip. According to my new description, "cdns_pcieX_ctrl" is not shared by root ports, they are shared by host bridge/rc.Because the register design of "
cdns_pcieX_ctrl" is not strictly segmented according to link0 and link1, in pcie host bridge driver coding we must know whether the host bridge corresponds to link0 or link1 in the ip, so the "sophgo,link-id" attribute is introduced.Now I think it is not appropriate to change it to "sophgo,pcie-port". The reason is that as mentioned above, there is only one root port under each host bridge in the cadence ip. Link0 and link1 are actually used to distinguish the two host bridges in one ip.
So I suggest to keep the original "sophgo,link-id" and with the prefix because the introduction of this attribute is indeed caused by the private design of sophgo.
Any other good idea please feel free let me know.
Thansk,
Chen
Quoted 12 lines
Quoted 8 lines
+required:
+ - compatible
+ - reg
+ - reg-names
+ - vendor-id
+ - device-id
+ - sophgo,syscon-pcie-ctrl
+ - sophgo,pcie-portIt looks like vendor-id and device-id apply to PCI devices, i.e., things that will show up in lspci, I assume Root Ports in this case. Can we make this explicit in the DT, e.g., something like this?
Quoted 6 lines
pcie@62000000 {
compatible = "sophgo,sg2042-pcie-host";
port0: pci@0,0 {
vendor-id = <0x1f1c>;
device-id = <0x2042>;
};
Quoted 8 lines
As I mentioned above, there is actually only one root port under a host bridge, so I think it is unnecessary to introduce the port subnode.
In addition, I found that it is also allowed to directly add the vendor-id and device-id properties directly under the host bridge, see https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/pci/pci-host-bridge.yaml
And refer to the dts for those products using cadence ip:arch/arm64/boot/dts/ti/k3-j721e-main.dtsiIn this way, when executing lspci, the vendor id and device id will appear in the line corresponding to the pci brdge device.
[......]
I see that you haven't replied since my last reply, so I will continue to modify the code as I described. If you have any comments, please let me know, thank you.
Regards,
Chen
Quoted 6 lines
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
In reply to Chen WangQuoted 68 lines · Chen Wang
hello ~
Quoted 3 lines · Bjorn Helgaas
Quoted 2 lines · Chen Wang
Add binding for Sophgo SG2042 PCIe host controller.
+ sophgo,pcie-port:[......]
Quoted 43 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 67
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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?
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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 +-----------------+ + | | |Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 88
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 97
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+ "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.Quoted 2 lines
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.
Quoted 7 lines
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?
Re: [PATCH v2 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
Quoted 80 lines · Bjorn Helgaas
Quoted 67 lines · Chen Wang
hello ~
Quoted 3 lines · Bjorn Helgaas
Quoted 2 lines · Chen Wang
Add binding for Sophgo SG2042 PCIe host controller.
+ sophgo,pcie-port:[......]
Quoted 43 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 67
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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?
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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 +-----------------+ + | | |Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 88
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ 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.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 97
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +8
-0+ "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.Quoted 2 lines
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.
Quoted 7 lines
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:
Quoted 5 lines
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.
PATCH v2 2/5 PCI: sg2042: Add Sophgo SG2042 PCIe driver
In reply to Chen WangFrom: Chen Wang <[email protected]>
Add support for PCIe controller in SG2042 SoC. The controller uses the Cadence PCIe core programmed by pcie-cadence*.c. The PCIe controller will work in host mode only.
--- drivers/pci/controller/cadence/Kconfig | 11 + drivers/pci/controller/cadence/Makefile | 1 + drivers/pci/controller/cadence/pcie-sg2042.c | 534 +++++++++++++++++++ 3 files changed, 546 insertions(+) create mode 100644 drivers/pci/controller/cadence/pcie-sg2042.c
drivers/pci/controller/cadence/Kconfig +11 -0
@@ -42,6 +42,16 @@ config PCIE_CADENCE_PLAT_EP endpoint mode. This PCIe controller may be embedded into many different vendors SoCs. +config PCIE_SG2042 + bool "Sophgo SG2042 PCIe controller (host mode)" + depends on ARCH_SOPHGO || COMPILE_TEST + depends on OF + select PCIE_CADENCE_HOST + help + Say Y here if you want to support the Sophgo SG2042 PCIe platform + controller in host mode. Sophgo SG2042 PCIe controller uses Cadence + PCIe core. + config PCI_J721E bool
@@ -67,4 +77,5 @@ config PCI_J721E_EP Say Y here if you want to support the TI J721E PCIe platform controller in endpoint mode. TI J721E PCIe controller uses Cadence PCIe core. + endmenu
drivers/pci/controller/cadence/Makefile +1 -0
@@ -4,3 +4,4 @@ obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o obj-$(CONFIG_PCI_J721E) += pci-j721e.o +obj-$(CONFIG_PCIE_SG2042) += pcie-sg2042.o
drivers/pci/controller/cadence/pcie-sg2042.c new file +534 -0
534 lines not on this page · raw message
Signature · 1 line
2.34.1
Re: [PATCH v2 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver
drivers/pci/controller/cadence/Makefile obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o · line 7
Quoted 13 lines · Chen Wang
From: Chen Wang <[email protected]>
Add support for PCIe controller in SG2042 SoC. The controller uses the Cadence PCIe core programmed by pcie-cadence*.c. The PCIe controller will work in host mode only.
+++ b/drivers/pci/controller/cadence/Makefile
drivers/pci/controller/cadence/Makefile +1
-0@@ -4,3 +4,4 @@ obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o obj-$(CONFIG_PCI_J721E) += pci-j721e.o +obj-$(CONFIG_PCIE_SG2042) += pcie-sg2042.o
Add the newline.
Quoted 3 lines
+++
b/drivers/pci/controller/cadence/pcie-sg2042.c+#include "
../../../irqchip/irq-msi-lib.h"
This is the only file outside drivers/irqchip/ that includes this. What's special about this driver? Maybe this is a hint that something here belongs in drivers/irqchip/?
Quoted 1 line
+#ifdef
CONFIG_SMP
No other drivers test CONFIG_SMP, why should this be different?
Quoted 16 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +15
-0+static int sg2042_pcie_msi_irq_set_affinity (struct irq_data *d, +const struct cpumask *mask, +bool force) +{ +if (d->parent_data) +return irq_chip_set_affinity_parent (d, mask, force); + +return -EINVAL ; +} +#endif /* CONFIG_SMP */ +static int sg2042_pcie_init_msi_data (struct sg2042_pcie *pcie) +{ +struct device *dev = pcie->cdns_pcie->dev; +u32 value; +int ret;
drivers/pci/controller/cadence/pcie-sg2042.c
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+ +raw_spin_lock_init (&pcie->msi_lock); + +/* +* Though the PCIe controller can address >32-bit address space, to +* facilitate endpoints that support only 32-bit MSI target address, +* the mask is set to 32-bit to make sure that MSI target address is +* always a 32-bit address +*/ + ret =dma_set_coherent_mask (dev,DMA_BIT_MASK (32 ));
Not sure this is needed. Does DT dma-ranges not cover this?
Quoted 1 line
+static int
sg2042_pcie_setup_msi(struct sg2042_pcie *pcie, struct device_node *msi_node)
Wrap to fit in 80 columns like the rest.
Quoted 3 lines
+/*
+ * SG2042 only support 4-byte aligned access, so for the rootbus (i.e. to read
+ * the PCIe controller itself, read32 is required. For non-rootbus (i.e. to read
s/PCIe controller/Root Port/
Quoted 2 lines
+ * the PCIe peripheral registers, supports 1/2/4 byte aligned access, so
+ * directly use read should be fine.
s/use read/using read/
Quoted 5 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +5
-0+static int sg2042_pcie_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct device_node *np = dev->of_node; +struct pci_host_bridge *bridge;
drivers/pci/controller/cadence/pcie-sg2042.c line 420
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+struct device_node *np_syscon; +struct device_node *msi_node; +struct cdns_pcie *cdns_pcie; +struct sg2042_pcie *pcie; +struct cdns_pcie_rc *rc; +struct regmap *syscon; +int ret; + +if (!IS_ENABLED (CONFIG_PCIE_CADENCE_HOST )) +return -ENODEV ;
I don't think this is needed since CONFIG_PCIE_SG2042 selects PCIE_CADENCE_HOST.
Bjorn
Re: [PATCH v2 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver
Quoted 14 lines · Bjorn Helgaas
drivers/pci/controller/cadence/Makefile obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o · line 7
Quoted 12 lines · Chen Wang
From: Chen Wang <[email protected]>
Add support for PCIe controller in SG2042 SoC. The controller uses the Cadence PCIe core programmed by pcie-cadence*.c. The PCIe controller will work in host mode only.
+++ b/drivers/pci/controller/cadence/Makefiledrivers/pci/controller/cadence/Makefile +1
-0@@ -4,3 +4,4 @@ obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o obj-$(CONFIG_PCI_J721E) += pci-j721e.o +obj-$(CONFIG_PCIE_SG2042) += pcie-sg2042.oAdd the newline.
ok
Quoted 6 lines
Quoted 3 lines
+++
b/drivers/pci/controller/cadence/pcie-sg2042.c+#include "../../../irqchip/irq-msi-lib.h"This is the only file outside
drivers/irqchip/that includes this. What's special about this driver? Maybe this is a hint that something here belongs indrivers/irqchip/?
This file is included due to MSI parent model is used in this driver and I used helper functions such as msi_lib_init_dev_msi_info.
Actually I see Marc is working on MSI cleanup and will make irq-msi-lib.h globally available, see https://lore.kernel.org/linux-riscv/[email protected]/
But before his PR is merged, we may have to include the file like this now.
Quoted 3 lines
Quoted 2 lines
+#ifdef
CONFIG_SMPNo other drivers test
CONFIG_SMP, why should this be different?
ok, seems it'ok to remove this test.
Quoted 19 lines
Quoted 19 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +18
-0+static int sg2042_pcie_msi_irq_set_affinity (struct irq_data *d, +const struct cpumask *mask, +bool force) +{ +if (d->parent_data) +return irq_chip_set_affinity_parent (d, mask, force); + +return -EINVAL ; +} +#endif /* CONFIG_SMP */ +static int sg2042_pcie_init_msi_data (struct sg2042_pcie *pcie) +{ +struct device *dev = pcie->cdns_pcie->dev; +u32 value; +int ret; + +raw_spin_lock_init (&pcie->msi_lock); +
Quoted 8 lines
drivers/pci/controller/cadence/pcie-sg2042.c line 216
Quoted 7 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +7
-0+/* +* Though the PCIe controller can address >32-bit address space, to +* facilitate endpoints that support only 32-bit MSI target address, +* the mask is set to 32-bit to make sure that MSI target address is +* always a 32-bit address +*/ + ret =dma_set_coherent_mask (dev,DMA_BIT_MASK (32 ));Not sure this is needed. Does DT dma-ranges not cover this?
I prefer not introduce dma-ranges, we just need to allocate a small continuous space here, and using dma_set_coherent_mask is also one of the commonly used methods.
Quoted 3 lines
Quoted 2 lines
+static int
sg2042_pcie_setup_msi(struct sg2042_pcie *pcie, struct device_node *msi_node)Wrap to fit in 80 columns like the rest.
ok
Quoted 5 lines
Quoted 4 lines
+/*
+ * SG2042 only support 4-byte aligned access, so for the rootbus (i.e. to read
+ * the PCIe controller itself, read32 is required. For non-rootbus (i.e. to reads/PCIe controller/Root Port/
ok
Quoted 4 lines
Quoted 3 lines
+ * the PCIe peripheral registers, supports 1/2/4 byte aligned access, so
+ * directly use read should be fine.s/use read/using read/
ok, thanks
Quoted 10 lines
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +9
-0+static int sg2042_pcie_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct device_node *np = dev->of_node; +struct pci_host_bridge *bridge; +struct device_node *np_syscon; +struct device_node *msi_node; +struct cdns_pcie *cdns_pcie; +struct sg2042_pcie *pcie;
Quoted 8 lines
drivers/pci/controller/cadence/pcie-sg2042.c line 420
Quoted 6 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +6
-0+struct cdns_pcie_rc *rc; +struct regmap *syscon; +int ret; + +if (!IS_ENABLED (CONFIG_PCIE_CADENCE_HOST )) +return -ENODEV ;I don't think this is needed since
CONFIG_PCIE_SG2042selectsPCIE_CADENCE_HOST.
Yes, you are right.
Thanks,
Chen
Quoted 2 lines
Bjorn
kernel test robot
build botRe: [PATCH v2 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver
In reply to Chen Wangkernel test robot noticed the following build warnings:
Hi Chen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on fac04efc5c793dccbd07e2d59af9f90b7fc0dca4]
url: https://github.com/intel-lab-lkp/linux/commits/Chen-Wang/dt-bindings-pci-Add-Sophgo-SG2042-PCIe-host/20241209-152613 base: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4 patch link: https://lore.kernel.org/r/1d82eff3670f60df24228e5c83cf663c6dd61eaf.1733726572.git.unicorn_wang%40outlook.com patch subject: [PATCH v2 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver config: sh-randconfig-r071-20241215 (https://download.01.org/0day-ci/archive/20241215/[email protected]/config) compiler: sh4-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241215/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/pci/controller/cadence/pcie-sg2042.c:23:
>> drivers/pci/controller/cadence/../../../irqchip/irq-msi-lib.h:25:39: warning: 'struct msi_domain_info' declared inside parameter list will not be visible outside of this definition or declaration
25 | struct msi_domain_info *info);
| ^~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:306:15: error: variable 'sg2042_pcie_msi_parent_ops' has initializer but incomplete type
306 | static struct msi_parent_ops sg2042_pcie_msi_parent_ops = {
| ^~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:307:10: error: 'struct msi_parent_ops' has no member named 'required_flags'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:301:41: error: 'MSI_FLAG_USE_DEF_DOM_OPS' undeclared here (not in a function)
301 | #define SG2042_PCIE_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:307:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_REQUIRED'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:302:41: error: 'MSI_FLAG_USE_DEF_CHIP_OPS' undeclared here (not in a function)
302 | MSI_FLAG_USE_DEF_CHIP_OPS)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:307:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_REQUIRED'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/cadence/pcie-sg2042.c:301:40: warning: excess elements in struct initializer
301 | #define SG2042_PCIE_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
| ^
drivers/pci/controller/cadence/pcie-sg2042.c:307:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_REQUIRED'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:301:40: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
301 | #define SG2042_PCIE_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
| ^
drivers/pci/controller/cadence/pcie-sg2042.c:307:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_REQUIRED'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:308:10: error: 'struct msi_parent_ops' has no member named 'supported_flags'
308 | .supported_flags = SG2042_PCIE_MSI_FLAGS_SUPPORTED,
| ^~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:304:41: error: 'MSI_GENERIC_FLAGS_MASK' undeclared here (not in a function)
304 | #define SG2042_PCIE_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:308:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_SUPPORTED'
308 | .supported_flags = SG2042_PCIE_MSI_FLAGS_SUPPORTED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:304:41: warning: excess elements in struct initializer
304 | #define SG2042_PCIE_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:308:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_SUPPORTED'
308 | .supported_flags = SG2042_PCIE_MSI_FLAGS_SUPPORTED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:304:41: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
304 | #define SG2042_PCIE_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:308:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_SUPPORTED'
308 | .supported_flags = SG2042_PCIE_MSI_FLAGS_SUPPORTED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:309:10: error: 'struct msi_parent_ops' has no member named 'bus_select_mask'
309 | .bus_select_mask = MATCH_PCI_MSI,
| ^~~~~~~~~~~~~~~
>> drivers/pci/controller/cadence/../../../irqchip/irq-msi-lib.h:15:33: warning: excess elements in struct initializer
15 | #define MATCH_PCI_MSI (0)
| ^
drivers/pci/controller/cadence/pcie-sg2042.c:309:35: note: in expansion of macro 'MATCH_PCI_MSI'
309 | .bus_select_mask = MATCH_PCI_MSI,
| ^~~~~~~~~~~~~
drivers/pci/controller/cadence/../../../irqchip/irq-msi-lib.h:15:33: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
15 | #define MATCH_PCI_MSI (0)
| ^
drivers/pci/controller/cadence/pcie-sg2042.c:309:35: note: in expansion of macro 'MATCH_PCI_MSI'
309 | .bus_select_mask = MATCH_PCI_MSI,
| ^~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:310:10: error: 'struct msi_parent_ops' has no member named 'bus_select_token'
310 | .bus_select_token = DOMAIN_BUS_NEXUS,
| ^~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:310:35: warning: excess elements in struct initializer
310 | .bus_select_token = DOMAIN_BUS_NEXUS,
| ^~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:310:35: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
drivers/pci/controller/cadence/pcie-sg2042.c:311:10: error: 'struct msi_parent_ops' has no member named 'prefix'
311 | .prefix = "SG2042-",
| ^~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:311:35: warning: excess elements in struct initializer
311 | .prefix = "SG2042-",
| ^~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:311:35: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
drivers/pci/controller/cadence/pcie-sg2042.c:312:10: error: 'struct msi_parent_ops' has no member named 'init_dev_msi_info'
312 | .init_dev_msi_info = msi_lib_init_dev_msi_info,
| ^~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:312:35: warning: excess elements in struct initializer
312 | .init_dev_msi_info = msi_lib_init_dev_msi_info,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:312:35: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
drivers/pci/controller/cadence/pcie-sg2042.c: In function 'sg2042_pcie_setup_msi':
drivers/pci/controller/cadence/pcie-sg2042.c:344:22: error: 'struct irq_domain' has no member named 'msi_parent_ops'
344 | parent_domain->msi_parent_ops = &sg2042_pcie_msi_parent_ops;
| ^~
drivers/pci/controller/cadence/pcie-sg2042.c: At top level:
drivers/pci/controller/cadence/pcie-sg2042.c:306:30: error: storage size of 'sg2042_pcie_msi_parent_ops' isn't known
306 | static struct msi_parent_ops sg2042_pcie_msi_parent_ops = {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~vim +25 drivers/pci/controller/cadence/../../../irqchip/irq-msi-lib.h
72e257c6f058032 Thomas Gleixner 2024-06-23 11 8c41ccec839c622 Thomas Gleixner 2024-06-23 12 #ifdef CONFIG_PCI_MSI 8c41ccec839c622 Thomas Gleixner 2024-06-23 13 #define MATCH_PCI_MSI BIT(DOMAIN_BUS_PCI_MSI) 8c41ccec839c622 Thomas Gleixner 2024-06-23 14 #else 8c41ccec839c622 Thomas Gleixner 2024-06-23 @15 #define MATCH_PCI_MSI (0) 8c41ccec839c622 Thomas Gleixner 2024-06-23 16 #endif 8c41ccec839c622 Thomas Gleixner 2024-06-23 17 496436f4a514a3f Thomas Gleixner 2024-06-23 18 #define MATCH_PLATFORM_MSI BIT(DOMAIN_BUS_PLATFORM_MSI) 496436f4a514a3f Thomas Gleixner 2024-06-23 19 72e257c6f058032 Thomas Gleixner 2024-06-23 20 int msi_lib_irq_domain_select(struct irq_domain *d, struct irq_fwspec *fwspec, 72e257c6f058032 Thomas Gleixner 2024-06-23 21 enum irq_domain_bus_token bus_token); 72e257c6f058032 Thomas Gleixner 2024-06-23 22 72e257c6f058032 Thomas Gleixner 2024-06-23 23 bool msi_lib_init_dev_msi_info(struct device *dev, struct irq_domain *domain, 72e257c6f058032 Thomas Gleixner 2024-06-23 24 struct irq_domain *real_parent,
72e257c6f058032 Thomas Gleixner 2024-06-23 @25 struct msi_domain_info *info);72e257c6f058032 Thomas Gleixner 2024-06-23 26
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [PATCH v2 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver
In reply to Chen Wangkernel test robot noticed the following build errors:
Hi Chen,
kernel test robot noticed the following build errors:
[auto build test ERROR on fac04efc5c793dccbd07e2d59af9f90b7fc0dca4]
url: https://github.com/intel-lab-lkp/linux/commits/Chen-Wang/dt-bindings-pci-Add-Sophgo-SG2042-PCIe-host/20241209-152613 base: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4 patch link: https://lore.kernel.org/r/1d82eff3670f60df24228e5c83cf663c6dd61eaf.1733726572.git.unicorn_wang%40outlook.com patch subject: [PATCH v2 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver config: sh-randconfig-r071-20241215 (https://download.01.org/0day-ci/archive/20241215/[email protected]/config) compiler: sh4-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241215/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
In file included from drivers/pci/controller/cadence/pcie-sg2042.c:23:
drivers/pci/controller/cadence/../../../irqchip/irq-msi-lib.h:25:39: warning: 'struct msi_domain_info' declared inside parameter list will not be visible outside of this definition or declaration
25 | struct msi_domain_info *info);
| ^~~~~~~~~~~~~~~
>> drivers/pci/controller/cadence/pcie-sg2042.c:306:15: error: variable 'sg2042_pcie_msi_parent_ops' has initializer but incomplete type
306 | static struct msi_parent_ops sg2042_pcie_msi_parent_ops = {
| ^~~~~~~~~~~~~~
>> drivers/pci/controller/cadence/pcie-sg2042.c:307:10: error: 'struct msi_parent_ops' has no member named 'required_flags'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~
>> drivers/pci/controller/cadence/pcie-sg2042.c:301:41: error: 'MSI_FLAG_USE_DEF_DOM_OPS' undeclared here (not in a function)
301 | #define SG2042_PCIE_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:307:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_REQUIRED'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/cadence/pcie-sg2042.c:302:41: error: 'MSI_FLAG_USE_DEF_CHIP_OPS' undeclared here (not in a function)
302 | MSI_FLAG_USE_DEF_CHIP_OPS)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:307:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_REQUIRED'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:301:40: warning: excess elements in struct initializer
301 | #define SG2042_PCIE_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
| ^
drivers/pci/controller/cadence/pcie-sg2042.c:307:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_REQUIRED'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:301:40: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
301 | #define SG2042_PCIE_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
| ^
drivers/pci/controller/cadence/pcie-sg2042.c:307:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_REQUIRED'
307 | .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/cadence/pcie-sg2042.c:308:10: error: 'struct msi_parent_ops' has no member named 'supported_flags'
308 | .supported_flags = SG2042_PCIE_MSI_FLAGS_SUPPORTED,
| ^~~~~~~~~~~~~~~
>> drivers/pci/controller/cadence/pcie-sg2042.c:304:41: error: 'MSI_GENERIC_FLAGS_MASK' undeclared here (not in a function)
304 | #define SG2042_PCIE_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:308:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_SUPPORTED'
308 | .supported_flags = SG2042_PCIE_MSI_FLAGS_SUPPORTED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:304:41: warning: excess elements in struct initializer
304 | #define SG2042_PCIE_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:308:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_SUPPORTED'
308 | .supported_flags = SG2042_PCIE_MSI_FLAGS_SUPPORTED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:304:41: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
304 | #define SG2042_PCIE_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:308:35: note: in expansion of macro 'SG2042_PCIE_MSI_FLAGS_SUPPORTED'
308 | .supported_flags = SG2042_PCIE_MSI_FLAGS_SUPPORTED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/cadence/pcie-sg2042.c:309:10: error: 'struct msi_parent_ops' has no member named 'bus_select_mask'
309 | .bus_select_mask = MATCH_PCI_MSI,
| ^~~~~~~~~~~~~~~
drivers/pci/controller/cadence/../../../irqchip/irq-msi-lib.h:15:33: warning: excess elements in struct initializer
15 | #define MATCH_PCI_MSI (0)
| ^
drivers/pci/controller/cadence/pcie-sg2042.c:309:35: note: in expansion of macro 'MATCH_PCI_MSI'
309 | .bus_select_mask = MATCH_PCI_MSI,
| ^~~~~~~~~~~~~
drivers/pci/controller/cadence/../../../irqchip/irq-msi-lib.h:15:33: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
15 | #define MATCH_PCI_MSI (0)
| ^
drivers/pci/controller/cadence/pcie-sg2042.c:309:35: note: in expansion of macro 'MATCH_PCI_MSI'
309 | .bus_select_mask = MATCH_PCI_MSI,
| ^~~~~~~~~~~~~
>> drivers/pci/controller/cadence/pcie-sg2042.c:310:10: error: 'struct msi_parent_ops' has no member named 'bus_select_token'
310 | .bus_select_token = DOMAIN_BUS_NEXUS,
| ^~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:310:35: warning: excess elements in struct initializer
310 | .bus_select_token = DOMAIN_BUS_NEXUS,
| ^~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:310:35: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
>> drivers/pci/controller/cadence/pcie-sg2042.c:311:10: error: 'struct msi_parent_ops' has no member named 'prefix'
311 | .prefix = "SG2042-",
| ^~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:311:35: warning: excess elements in struct initializer
311 | .prefix = "SG2042-",
| ^~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:311:35: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
>> drivers/pci/controller/cadence/pcie-sg2042.c:312:10: error: 'struct msi_parent_ops' has no member named 'init_dev_msi_info'
312 | .init_dev_msi_info = msi_lib_init_dev_msi_info,
| ^~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:312:35: warning: excess elements in struct initializer
312 | .init_dev_msi_info = msi_lib_init_dev_msi_info,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/cadence/pcie-sg2042.c:312:35: note: (near initialization for 'sg2042_pcie_msi_parent_ops')
drivers/pci/controller/cadence/pcie-sg2042.c: In function 'sg2042_pcie_setup_msi':
>> drivers/pci/controller/cadence/pcie-sg2042.c:344:22: error: 'struct irq_domain' has no member named 'msi_parent_ops'
344 | parent_domain->msi_parent_ops = &sg2042_pcie_msi_parent_ops;
| ^~
drivers/pci/controller/cadence/pcie-sg2042.c: At top level:
>> drivers/pci/controller/cadence/pcie-sg2042.c:306:30: error: storage size of 'sg2042_pcie_msi_parent_ops' isn't known
306 | static struct msi_parent_ops sg2042_pcie_msi_parent_ops = {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~vim +/sg2042_pcie_msi_parent_ops +306 drivers/pci/controller/cadence/pcie-sg2042.c
300
> 301 #define SG2042_PCIE_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
> 302 MSI_FLAG_USE_DEF_CHIP_OPS)
303
> 304 #define SG2042_PCIE_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK
305
> 306 static struct msi_parent_ops sg2042_pcie_msi_parent_ops = {
> 307 .required_flags = SG2042_PCIE_MSI_FLAGS_REQUIRED,
> 308 .supported_flags = SG2042_PCIE_MSI_FLAGS_SUPPORTED,
> 309 .bus_select_mask = MATCH_PCI_MSI,
> 310 .bus_select_token = DOMAIN_BUS_NEXUS,
> 311 .prefix = "SG2042-",
> 312 .init_dev_msi_info = msi_lib_init_dev_msi_info,
313 };
314
315 static int sg2042_pcie_setup_msi(struct sg2042_pcie *pcie, struct device_node *msi_node)
316 {
317 struct device *dev = pcie->cdns_pcie->dev;
318 struct fwnode_handle *fwnode = of_node_to_fwnode(dev->of_node);
319 struct irq_domain *parent_domain;
320 int ret = 0;
321
322 if (!of_property_read_bool(msi_node, "msi-controller"))
323 return -ENODEV;
324
325 ret = of_irq_get_byname(msi_node, "msi");
326 if (ret <= 0) {
327 dev_err(dev, "%pOF: failed to get MSI irq\n", msi_node);
328 return ret;
329 }
330 pcie->msi_irq = ret;
331
332 irq_set_chained_handler_and_data(pcie->msi_irq,
333 sg2042_pcie_msi_chained_isr, pcie);
334
335 parent_domain = irq_domain_create_linear(fwnode, MSI_DEF_NUM_VECTORS,
336 &sg2042_pcie_msi_domain_ops, pcie);
337 if (!parent_domain) {
338 dev_err(dev, "%pfw: Failed to create IRQ domain\n", fwnode);
339 return -ENOMEM;
340 }
341 irq_domain_update_bus_token(parent_domain, DOMAIN_BUS_NEXUS);
342
343 parent_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
> 344 parent_domain->msi_parent_ops = &sg2042_pcie_msi_parent_ops; 345
346 pcie->msi_domain = parent_domain;
347
348 ret = sg2042_pcie_init_msi_data(pcie);
349 if (ret) {
350 dev_err(dev, "Failed to initialize MSI data!\n");
351 return ret;
352 }
353
354 return 0;
355 }
356
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
PATCH v2 3/5 dt-bindings: mfd: syscon: Add sg2042 pcie ctrl compatible
In reply to Chen WangFrom: Chen Wang <[email protected]>
Document SOPHGO SG2042 compatible for PCIe control registers. These registers are shared by pcie controller nodes.
--- Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++ 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/mfd/syscon.yaml +2 -0
@@ -107,6 +107,7 @@ select: - rockchip,rk3576-qos - rockchip,rk3588-qos - rockchip,rv1126-qos + - sophgo,sg2042-pcie-ctrl - st,spear1340-misc - stericsson,nomadik-pmu - starfive,jh7100-sysmain
@@ -205,6 +206,7 @@ properties: - rockchip,rk3576-qos - rockchip,rk3588-qos - rockchip,rv1126-qos + - sophgo,sg2042-pcie-ctrl - st,spear1340-misc - stericsson,nomadik-pmu - starfive,jh7100-sysmain
Signature · 1 line
2.34.1
Re: [PATCH v2 3/5] dt-bindings: mfd: syscon: Add sg2042 pcie ctrl compatible
Quoted 4 lines · Chen Wang
From: Chen Wang <[email protected]>
Document SOPHGO SG2042 compatible for PCIe control registers. These registers are shared by pcie controller nodes.
s/pcie/PCIe/ to be consistent (also in subject and other patches).
PATCH v2 4/5 riscv: sophgo: dts: add pcie controllers for SG2042
In reply to Chen WangFrom: Chen Wang <[email protected]>
Add PCIe controller nodes in DTS for Sophgo SG2042. Default they are disabled.
--- arch/riscv/boot/dts/sophgo/sg2042.dtsi | 89 ++++++++++++++++++++++++++ 1 file changed, 89 insertions(+)
arch/riscv/boot/dts/sophgo/sg2042.dtsi +89 -0
@@ -195,6 +195,95 @@ clkgen: clock-controller@7030012000 { #clock-cells = <1>; }; + pcie_rc0: pcie@7060000000 { + compatible = "sophgo,sg2042-pcie-host"; + device_type = "pci"; + reg = <0x70 0x60000000 0x0 0x02000000>, + <0x40 0x00000000 0x0 0x00001000>; + reg-names = "reg", "cfg"; + linux,pci-domain = <0>; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x01000000 0x0 0xc0000000 0x40 0xc0000000 0x0 0x00400000>, + <0x42000000 0x0 0xd0000000 0x40 0xd0000000 0x0 0x10000000>, + <0x02000000 0x0 0xe0000000 0x40 0xe0000000 0x0 0x20000000>, + <0x43000000 0x42 0x00000000 0x42 0x00000000 0x2 0x00000000>, + <0x03000000 0x41 0x00000000 0x41 0x00000000 0x1 0x00000000>; + bus-range = <0x0 0x3f>; + vendor-id = <0x1f1c>; + device-id = <0x2042>; + cdns,no-bar-match-nbits = <48>; + sophgo,pcie-port = <0>; + sophgo,syscon-pcie-ctrl = <&cdns_pcie0_ctrl>; + msi-parent = <&msi>; + status = "disabled"; + }; + + cdns_pcie0_ctrl: syscon@7061800000 { + compatible = "sophgo,sg2042-pcie-ctrl", "syscon"; + reg = <0x70 0x61800000 0x0 0x800000>; + }; + + pcie_rc1: pcie@7062000000 { + compatible = "sophgo,sg2042-pcie-host"; + device_type = "pci"; + reg = <0x70 0x62000000 0x0 0x00800000>, + <0x48 0x00000000 0x0 0x00001000>; + reg-names = "reg", "cfg"; + linux,pci-domain = <1>; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x01000000 0x0 0xc0800000 0x48 0xc0800000 0x0 0x00400000>, + <0x42000000 0x0 0xd0000000 0x48 0xd0000000 0x0 0x10000000>, + <0x02000000 0x0 0xe0000000 0x48 0xe0000000 0x0 0x20000000>, + <0x03000000 0x49 0x00000000 0x49 0x00000000 0x1 0x00000000>, + <0x43000000 0x4a 0x00000000 0x4a 0x00000000 0x2 0x00000000>; + bus-range = <0x80 0xbf>; + vendor-id = <0x1f1c>; + device-id = <0x2042>; + cdns,no-bar-match-nbits = <48>; + sophgo,pcie-port = <0>; + sophgo,syscon-pcie-ctrl = <&cdns_pcie1_ctrl>; + msi-parent = <&msi_pcie>; + status = "disabled"; + msi_pcie: msi { + compatible = "sophgo,sg2042-pcie-msi"; + msi-controller; + interrupt-parent = <&intc>; + interrupts = <123 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi"; + }; + }; + + pcie_rc2: pcie@7062800000 { + compatible = "sophgo,sg2042-pcie-host"; + device_type = "pci"; + reg = <0x70 0x62800000 0x0 0x00800000>, + <0x4c 0x00000000 0x0 0x00001000>; + reg-names = "reg", "cfg"; + linux,pci-domain = <2>; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x01000000 0x0 0xc0c00000 0x4c 0xc0c00000 0x0 0x00400000>, + <0x42000000 0x0 0xf8000000 0x4c 0xf8000000 0x0 0x04000000>, + <0x02000000 0x0 0xfc000000 0x4c 0xfc000000 0x0 0x04000000>, + <0x43000000 0x4e 0x00000000 0x4e 0x00000000 0x2 0x00000000>, + <0x03000000 0x4d 0x00000000 0x4d 0x00000000 0x1 0x00000000>; + bus-range = <0xc0 0xff>; + vendor-id = <0x1f1c>; + device-id = <0x2042>; + cdns,no-bar-match-nbits = <48>; + sophgo,pcie-port = <1>; + sophgo,syscon-pcie-ctrl = <&cdns_pcie1_ctrl>; + msi-parent = <&msi>; + status = "disabled"; + }; + + cdns_pcie1_ctrl: syscon@7063800000 { + compatible = "sophgo,sg2042-pcie-ctrl", "syscon"; + reg = <0x70 0x63800000 0x0 0x800000>; + }; + clint_mswi: interrupt-controller@7094000000 { compatible = "sophgo,sg2042-aclint-mswi", "thead,c900-aclint-mswi"; reg = <0x00000070 0x94000000 0x00000000 0x00004000>;
Signature · 1 line
2.34.1
PATCH v2 5/5 riscv: sophgo: dts: enable pcie for PioneerBox
In reply to Chen WangFrom: Chen Wang <[email protected]>
Enable pcie controllers for PioneerBox, which uses SG2042 SoC.
--- arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts | 12 ++++++++++++ 1 file changed, 12 insertions(+)
arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts +12 -0
@@ -60,6 +60,18 @@ mcu: syscon@17 { }; }; +&pcie_rc0 { + status = "okay"; +}; + +&pcie_rc1 { + status = "okay"; +}; + +&pcie_rc2 { + status = "okay"; +}; + &sd { bus-width = <4>; no-sdio;
Signature · 1 line
2.34.1