Add PCIe support to Sophgo SG2042 SoC
Subsystems (7)
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
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 | 88 +++ .../boot/dts/sophgo/sg2042-milkv-pioneer.dts | 12 + arch/riscv/boot/dts/sophgo/sg2042.dtsi | 82 +++ drivers/pci/controller/cadence/Kconfig | 11 + drivers/pci/controller/cadence/Makefile | 1 + drivers/pci/controller/cadence/pcie-sg2042.c | 611 ++++++++++++++++++ 7 files changed, 807 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: 2d5404caa8c7bb5c4e0435f94b28834ae5456623
Signature · 1 line
2.34.1
PATCH 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 | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +88 -0
@@ -0,0 +1,88 @@ +# 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. + It shares common features with the PCIe core and inherits common properties + defined in Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml. + +maintainers: + - Chen Wang <[email protected]> + +properties: + compatible: + const: sophgo,sg2042-pcie-host + + reg: + maxItems: 2 + + reg-names: + items: + - const: reg + - const: cfg + + sophgo,syscon-pcie-ctrl: + $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to the SYSCON entry + + sophgo,link-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Cadence IP link ID. + + sophgo,internal-msi: + $ref: /schemas/types.yaml#/definitions/flag + description: Identifies whether the PCIE node uses internal MSI controller. + + vendor-id: + const: 0x1f1c + + device-id: + const: 0x2042 + + interrupts: + maxItems: 1 + + interrupt-names: + const: msi + +allOf: + - $ref: cdns-pcie-host.yaml# + +required: + - compatible + - reg + - reg-names + - sophgo,syscon-pcie-ctrl + - sophgo,link-id + - vendor-id + - device-id + - ranges + +additionalProperties: true + +examples: + - | + 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,link-id = <0>; + sophgo,syscon-pcie-ctrl = <&cdns_pcie1_ctrl>; + sophgo,internal-msi; + interrupt-parent = <&intc>; + };
Signature · 1 line
2.34.1
Rob Herring
maintainerRe: [PATCH 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
Quoted 16 lines · Chen Wang
From: Chen Wang <[email protected]>
Add binding for Sophgo SG2042 PCIe host controller.
Signed-off-by Chen Wang <[email protected]>--- .../bindings/pci/sophgo,sg2042-pcie-host.yaml | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yamlDocumentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +0
-0
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 9
Quoted 9 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +9
-0@@ -0,0 +1,88 @@ +# 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: |+
Don't need '|+'
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 12
Quoted 4 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +3
-0+ Sophgo SG2042 PCIe host controller is based on the Cadence PCIe core. + It shares common features with the PCIe core and inherits common properties + defined in Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml.
That's clear from the $ref. No need to say that in prose.
Quoted 9 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +9
-0+ +maintainers: + - Chen Wang <[email protected]> + +properties: + compatible: + const: sophgo,sg2042-pcie-host + + reg:
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 31
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ maxItems: 2 + + reg-names: + items: + - const: reg + - const: cfg + + sophgo,syscon-pcie-ctrl: + $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to the SYSCON entry
Please describe what you need to access.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 35
Quoted 4 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +4
-0+ + sophgo,link-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Cadence IP link ID.
Is this an index or related to the syscon? Nak for the former, use linux,pci-domain. For the latter, add an arg to sophgo,syscon-pcie-ctrl.
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 39
Quoted 4 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +4
-0+ + sophgo,internal-msi: + $ref: /schemas/types.yaml#/definitions/flag + description: Identifies whether the PCIE node uses internal MSI controller.
Wouldn't 'msi-parent' work for this purpose?
Quoted 15 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +15
-0+ + vendor-id: + const: 0x1f1c + + device-id: + const: 0x2042 + + interrupts: + maxItems: 1 + + interrupt-names: + const: msi + +allOf: + - $ref: cdns-pcie-host.yaml#
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 64
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ +required: + - compatible + - reg + - reg-names + - sophgo,syscon-pcie-ctrl + - sophgo,link-id + - vendor-id + - device-id + - ranges
ranges is already required in the common schemas.
Quoted 27 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +24
-0+ +additionalProperties: true + +examples: + - | + 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,link-id = <0>; + sophgo,syscon-pcie-ctrl = <&cdns_pcie1_ctrl>; + sophgo,internal-msi; + interrupt-parent = <&intc>; + };Signature · 1 line
2.34.1
Re: [PATCH 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
Quoted 11 lines · Rob Herring
Quoted 10 lines · Chen Wang
From: Chen Wang <[email protected]>
Add binding for Sophgo SG2042 PCIe host controller.
Signed-off-by Chen Wang <[email protected]>--- .../bindings/pci/sophgo,sg2042-pcie-host.yaml | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+)create mode 100644
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml
Quoted 16 lines
Quoted 6 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +0
-0Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 9
Quoted 9 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +9
-0@@ -0,0 +1,88 @@ +# 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: |+Don't need '|+'
Got, thanks.
Quoted 5 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 12
Quoted 4 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +3
-0+ Sophgo SG2042 PCIe host controller is based on the Cadence PCIe core. + It shares common features with the PCIe core and inherits common properties + defined in Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml.That's clear from the $ref. No need to say that in prose.
Got, thanks.
Quoted 22 lines
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +9
-0+ +maintainers: + - Chen Wang <[email protected]> + +properties: + compatible: + const: sophgo,sg2042-pcie-host + + reg:Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 31
Quoted 10 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +10
-0+ maxItems: 2 + + reg-names: + items: + - const: reg + - const: cfg + + sophgo,syscon-pcie-ctrl: + $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to the SYSCON entryPlease describe what you need to access.
Quoted 6 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 35
Quoted 4 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +4
-0+ + sophgo,link-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Cadence IP link ID.Is this an index or related to the syscon? Nak for the former, use linux,pci-domain. For the latter, add an arg to sophgo,syscon-pcie-ctrl.
Let me give you some background info.
SG2042 uses Cadence IP, every IP is composed of 2 cores(called link0 & link1 as Cadence's term). 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.
So we defined "sophgo,link-id" to flag which core(link) the rc maps to, with this we can know what registers(bits) we should use.
That's why I don't use "linux,pci-domain" and also it's not proper to define it as arg to "sophgo,syscon-pcie-ctrl".
Quoted 5 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 39
Quoted 4 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +4
-0+ + sophgo,internal-msi: + $ref: /schemas/types.yaml#/definitions/flag + description: Identifies whether the PCIE node uses internal MSI controller.Wouldn't 'msi-parent' work for this purpose?
I will check it out, thanks.
Quoted 19 lines
Quoted 19 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +18
-0+ + vendor-id: + const: 0x1f1c + + device-id: + const: 0x2042 + + interrupts: + maxItems: 1 + + interrupt-names: + const: msi + +allOf: + - $ref: cdns-pcie-host.yaml# + +required: + - compatible
Quoted 8 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml line 64
Quoted 7 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +7
-0+ - reg + - reg-names + - sophgo,syscon-pcie-ctrl + - sophgo,link-id + - vendor-id + - device-id + - rangesranges is already required in the common schemas.
Got.
Quoted 27 lines
Quoted 27 lines
Documentation/devicetree/bindings/pci/sophgo,sg2042-pcie-host.yaml new file +24
-0+ +additionalProperties: true + +examples: + - | + 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,link-id = <0>; + sophgo,syscon-pcie-ctrl = <&cdns_pcie1_ctrl>; + sophgo,internal-msi; + interrupt-parent = <&intc>; + };Signature · 1 line
2.34.1
PATCH 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 | 611 +++++++++++++++++++ 3 files changed, 623 insertions(+) create mode 100644 drivers/pci/controller/cadence/pcie-sg2042.c
drivers/pci/controller/cadence/Kconfig +11 -0
@@ -67,4 +67,15 @@ 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. + +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. + 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 +611 -0
611 lines not on this page · raw message
Signature · 1 line
2.34.1
kernel test robot
build botRe: [PATCH 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver
kernel test robot noticed the following build warnings:
Hi Chen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 2d5404caa8c7bb5c4e0435f94b28834ae5456623]
url: https://github.com/intel-lab-lkp/linux/commits/Chen-Wang/dt-bindings-pci-Add-Sophgo-SG2042-PCIe-host/20241111-140237 base: 2d5404caa8c7bb5c4e0435f94b28834ae5456623 patch link: https://lore.kernel.org/r/5051f2375ff6218e7d44ce0c298efd5f9ee56964.1731303328.git.unicorn_wang%40outlook.com patch subject: [PATCH 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver config: um-allyesconfig (https://download.01.org/0day-ci/archive/20241111/[email protected]/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241111/[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 >>):
>> drivers/pci/controller/cadence/pcie-sg2042.c:141:12: warning: 'sg2042_pcie_msi_irq_set_affinity' defined but not used [-Wunused-function]
141 | static int sg2042_pcie_msi_irq_set_affinity(struct irq_data *d,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/sg2042_pcie_msi_irq_set_affinity +141 drivers/pci/controller/cadence/pcie-sg2042.c
140
> 141 static int sg2042_pcie_msi_irq_set_affinity(struct irq_data *d,
142 const struct cpumask *mask,
143 bool force)
144 {
145 if (d->parent_data)
146 return irq_chip_set_affinity_parent(d, mask, force);
147
148 return -EINVAL;
149 }
150
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver
In reply to Chen WangQuoted 11 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/Kconfig
drivers/pci/controller/cadence/Kconfig +0
-0@@ -67,4 +67,15 @@ 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.
drivers/pci/controller/cadence/Kconfig config PCI_J721E_EP · line 79
Quoted 10 lines
drivers/pci/controller/cadence/Kconfig +10
-0+ +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.
Reorder to keep these menu items in alphabetical order by vendor.
Quoted 7 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +6
-0+++ b/drivers/pci/controller/cadence/pcie-sg2042.c + * 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
drivers/pci/controller/cadence/pcie-sg2042.c line 40
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+* 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. +* Method A & B can be configured in DTS with property "sophgo,internal-msi", +* default is Method B.
s/PICe/PCIe/ (multiple)
s/msi/MSI/ (multiple)
s/pcie/PCIe/ (multiple)
Wrap comment (and code below) to fit in 80 columns. Add blank lines between paragraphs.
Quoted 1 line
+#define
SG2042_CDNS_PLAT_CPU_TO_BUS_ADDR0xCFFFFFFFFF
Remove (see below).
drivers/pci/controller/cadence/pcie-sg2042.c line 161
Quoted 11 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +11
-0+static void sg2042_pcie_msi_irq_compose_msi_msg (struct irq_data *d, +struct msi_msg *msg) +{ +struct sg2042_pcie *pcie =irq_data_get_irq_chip_data (d); +struct device *dev = pcie->cdns_pcie->dev; + + msg->address_lo =lower_32_bits (pcie->msi_phys) +BYTE_NUM_PER_MSI_VEC * d->hwirq; + msg->address_hi =upper_32_bits (pcie->msi_phys); + msg->data =1 ; + + pcie->num_applied_vecs = d->hwirq;
This looks questionable. How do you know d->hwirq increases every time this is called?
drivers/pci/controller/cadence/pcie-sg2042.c line 164
Quoted 2 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +2
-0+dev_info (dev,"compose msi msg hwirq[%d] address_hi[%#x] address_lo[%#x]\n" , + (int )d->hwirq, msg->address_hi, msg->address_lo);
This seems too verbose to be a dev_info(). Maybe a dev_dbg() or remove it altogether.
Quoted 5 lines
+ * We use the usual two domain structure, the top one being a generic PCI/MSI
+ * domain, the bottom one being SG2042-specific and handling the actual HW
+ * interrupt allocation.
+ * At the same time, for internal MSI controller(Method A), bottom chip uses a
+ * chained handler to handle the controller's MSI IRQ edge triggered.
Add blank line between paragraphs.
Quoted 5 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +5
-0+static int sg2042_pcie_setup_msi_external (struct sg2042_pcie *pcie) +{ +struct device *dev = pcie->cdns_pcie->dev; +struct device_node *np = dev->of_node; +struct irq_domain *parent_domain;
drivers/pci/controller/cadence/pcie-sg2042.c line 276
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+struct device_node *parent_np; + +if (!of_find_property (np,"interrupt-parent" ,NULL )) { +dev_err (dev,"Can't find interrupt-parent!\n" ); +return -EINVAL ; + } + + parent_np =of_irq_find_parent (np); +if (!parent_np) { +dev_err (dev,"Can't find node of interrupt-parent!\n" );
Can you use some kind of %pOF format to include more information here?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/core-api/printk-formats.rst?id=v6.11#n463
drivers/pci/controller/cadence/pcie-sg2042.c line 283
Quoted 7 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +7
-0+return -ENXIO ; + } + + parent_domain =irq_find_host (parent_np); +of_node_put (parent_np); +if (!parent_domain) { +dev_err (dev,"Can't find domain of interrupt-parent!\n" );
And here?
Quoted 20 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +19
-0+return -ENXIO ; + } + +return sg2042_pcie_create_msi_domain (pcie, parent_domain); +} +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->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 +*/
drivers/pci/controller/cadence/pcie-sg2042.c line 313
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+ ret =dma_set_coherent_mask (dev,DMA_BIT_MASK (32 )); +if (ret <0 ) +return ret; + + pcie->msi_virt =dma_alloc_coherent (dev,BYTE_NUM_PER_MSI_VEC *MAX_MSI_IRQS , + &pcie->msi_phys,GFP_KERNEL ); +if (!pcie->msi_virt) +return -ENOMEM ; + +/* Program the msi address and size */
s/msi/MSI/
Quoted 9 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +9
-0+if (pcie->link_id ==1 ) { +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_LOW , +lower_32_bits (pcie->msi_phys)); +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_HIGH , +upper_32_bits (pcie->msi_phys)); + +regmap_read (pcie->syscon,REG_LINK1_MSI_ADDR_SIZE , &value); + value = (value ®_LINK1_MSI_ADDR_SIZE_MASK ) |MAX_MSI_IRQS ; +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_SIZE , value);
drivers/pci/controller/cadence/pcie-sg2042.c line 332
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+ }else { +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_LOW , +lower_32_bits (pcie->msi_phys)); +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_HIGH , +upper_32_bits (pcie->msi_phys)); + +regmap_read (pcie->syscon,REG_LINK0_MSI_ADDR_SIZE , &value); + value = (value ®_LINK0_MSI_ADDR_SIZE_MASK ) | (MAX_MSI_IRQS <<16 ); +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_SIZE , value); + }
Lot of pcie->link_id checking going on here. Consider saving these offsets in the struct sg2042_pcie so you don't need to test everywhere.
Quoted 17 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +16
-0+return 0 ; +} +static int sg2042_pcie_setup_msi (struct sg2042_pcie *pcie,struct platform_device *pdev) +{ +struct device *dev = pcie->cdns_pcie->dev; +struct fwnode_handle *fwnode =of_node_to_fwnode (dev->of_node); +struct irq_domain *parent_domain; +int ret =0 ; + + parent_domain =irq_domain_create_linear (fwnode,MSI_DEF_NUM_VECTORS , + &sg2042_pcie_msi_domain_ops, pcie); +if (!parent_domain) { +dev_err (dev,"Failed to create IRQ domain\n" ); +return -ENOMEM ; + } +irq_domain_update_bus_token (parent_domain,DOMAIN_BUS_NEXUS );
drivers/pci/controller/cadence/pcie-sg2042.c line 412
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+ + ret =sg2042_pcie_create_msi_domain (pcie, parent_domain); +if (ret) { +irq_domain_remove (parent_domain); +return ret; + } + + ret =sg2042_pcie_init_msi_data (pcie); +if (ret) { +dev_err (dev,"Failed to initialize msi data!\n" );
s/msi/MSI/
Quoted 22 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +22
-0+return ret; + } + + ret =platform_get_irq_byname (pdev,"msi" ); +if (ret <=0 ) { +dev_err (dev,"failed to get MSI irq\n" ); +return ret; + } + pcie->msi_irq = ret; + +irq_set_chained_handler_and_data (pcie->msi_irq, + sg2042_pcie_msi_chained_isr, pcie); + +return 0 ; +} + +static void sg2042_pcie_free_msi (struct sg2042_pcie *pcie) +{ +struct device *dev = pcie->cdns_pcie->dev; + +if (pcie->msi_irq) +irq_set_chained_handler_and_data (pcie->msi_irq,NULL ,NULL );
drivers/pci/controller/cadence/pcie-sg2042.c line 444
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+ +if (pcie->msi_virt) +dma_free_coherent (dev,BYTE_NUM_PER_MSI_VEC *MAX_MSI_IRQS , + pcie->msi_virt, pcie->msi_phys); +} + +static u64 sg2042_cdns_pcie_cpu_addr_fixup (struct cdns_pcie *pcie,u64 cpu_addr) +{ +return cpu_addr &SG2042_CDNS_PLAT_CPU_TO_BUS_ADDR ; +}
Remove. This translation between CPU and PCI bus addresses should be described in the DT "ranges" property of the PCI host bridge.
See
https://lore.kernel.org/linux-pci/[email protected]/
for a similar fix.
drivers/pci/controller/cadence/pcie-sg2042.c line 455
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+static const struct cdns_pcie_ops sg2042_cdns_pcie_ops = { + .cpu_addr_fixup = sg2042_cdns_pcie_cpu_addr_fixup, +}; + +/* +* 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 +* the PCIe peripheral registers, supports 1/2/4 byte aligned access, so +* directly use read should be fine. +* The same is true for write.
Add blank line between paragraphs or rewrap into a single paragraph.
Quoted 14 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +14
-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 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 ;
drivers/pci/controller/cadence/pcie-sg2042.c line 506
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+ + pcie =devm_kzalloc (dev,sizeof (*pcie),GFP_KERNEL ); +if (!pcie) +return -ENOMEM ; + + bridge =devm_pci_alloc_host_bridge (dev,sizeof (*rc)); +if (!bridge) { +dev_err (dev,"Failed to alloc host bridge!\n" ); +return -ENOMEM ; + }
Add blank line.
Quoted 108 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +105
-0+ bridge->ops = &sg2042_pcie_host_ops; + + rc =pci_host_bridge_priv (bridge); + cdns_pcie = &rc->pcie; + cdns_pcie->dev = dev; + cdns_pcie->ops = &sg2042_cdns_pcie_ops; + pcie->cdns_pcie = cdns_pcie; + + np_syscon =of_parse_phandle (np,"sophgo,syscon-pcie-ctrl" ,0 ); +if (!np_syscon) { +dev_err (dev,"Failed to get syscon node\n" ); +return -ENOMEM ; + } + syscon =syscon_node_to_regmap (np_syscon); +if (IS_ERR (syscon)) { +dev_err (dev,"Failed to get regmap for syscon\n" ); +return -ENOMEM ; + } + pcie->syscon = syscon; + +if (of_property_read_u32 (np,"sophgo,link-id" , &pcie->link_id)) { +dev_err (dev,"Unable to parse link ID\n" ); +return -EINVAL ; + } + + pcie->internal_msi =0 ; +if (of_property_read_bool (np,"sophgo,internal-msi" )) + pcie->internal_msi =1 ; + +platform_set_drvdata (pdev, pcie); + +pm_runtime_enable (dev); + + ret =pm_runtime_get_sync (dev); +if (ret <0 ) { +dev_err (dev,"pm_runtime_get_sync failed\n" ); + goto err_get_sync; + } + +if (pcie->internal_msi) { + ret =sg2042_pcie_setup_msi (pcie, pdev); +if (ret <0 ) + goto err_setup_msi; + }else { + ret =sg2042_pcie_setup_msi_external (pcie); +if (ret <0 ) + goto err_setup_msi; + } + + ret =cdns_pcie_init_phy (dev, cdns_pcie); +if (ret) { +dev_err (dev,"Failed to init phy!\n" ); + goto err_setup_msi; + } + + ret =cdns_pcie_host_setup (rc); +if (ret <0 ) { +dev_err (dev,"Failed to setup host!\n" ); + goto err_host_setup; + } + +return 0 ; + +err_host_setup: +cdns_pcie_disable_phy (cdns_pcie); + +err_setup_msi: +sg2042_pcie_free_msi (pcie); + +err_get_sync: +pm_runtime_put (dev); +pm_runtime_disable (dev); + +return ret; +} + +static void sg2042_pcie_shutdown (struct platform_device *pdev) +{ +struct sg2042_pcie *pcie =platform_get_drvdata (pdev); +struct cdns_pcie *cdns_pcie = pcie->cdns_pcie; +struct device *dev = &pdev->dev; + +sg2042_pcie_free_msi (pcie); + +cdns_pcie_disable_phy (cdns_pcie); + +pm_runtime_put (dev); +pm_runtime_disable (dev); +} + +static const struct of_device_id sg2042_pcie_of_match[] = { + { .compatible ="sophgo,sg2042-pcie-host" }, + {}, +}; + +static struct platform_driver sg2042_pcie_driver = { + .driver = { + .name ="sg2042-pcie" , + .of_match_table = sg2042_pcie_of_match, + .pm = &cdns_pcie_pm_ops, + }, + .probe = sg2042_pcie_probe, + .shutdown = sg2042_pcie_shutdown, +}; +builtin_platform_driver (sg2042_pcie_driver);Signature · 1 line
2.34.1
Re: [PATCH 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver
Hello~
Quoted 14 lines · Bjorn Helgaas
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/Kconfigdrivers/pci/controller/cadence/Kconfig +3
-0@@ -67,4 +67,15 @@ 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. + +config PCIE_SG2042 + bool "Sophgo SG2042 PCIe controller (host mode)"
Quoted 8 lines
drivers/pci/controller/cadence/Kconfig config PCI_J721E_EP · line 79
Quoted 7 lines
drivers/pci/controller/cadence/Kconfig +7
-0+ 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.Reorder to keep these menu items in alphabetical order by vendor.
Sorry, I don't understand your question. I think the menu items in this Kconfig file are already sorted alphabetically.
Quoted 20 lines
Quoted 6 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +6
-0+++ b/drivers/pci/controller/cadence/pcie-sg2042.c + * 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 onlydrivers/pci/controller/cadence/pcie-sg2042.c line 40
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +10
-0+* 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. +* Method A & B can be configured in DTS with property "sophgo,internal-msi", +* default is Method B. s/PICe/PCIe/ (multiple)
s/msi/MSI/ (multiple)
s/pcie/PCIe/ (multiple)
Quoted 2 lines
Wrap comment (and code below) to fit in 80 columns. Add blank lines between paragraphs.
Got, will change.
Quoted 3 lines
Quoted 2 lines
+#define
SG2042_CDNS_PLAT_CPU_TO_BUS_ADDR0xCFFFFFFFFFRemove (see below).
Yes, after some testing, seems this address fixup is not need, will remove it.
Quoted 5 lines
Quoted 5 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +5
-0+static void sg2042_pcie_msi_irq_compose_msi_msg (struct irq_data *d, +struct msi_msg *msg) +{ +struct sg2042_pcie *pcie =irq_data_get_irq_chip_data (d); +struct device *dev = pcie->cdns_pcie->dev;
Quoted 8 lines
drivers/pci/controller/cadence/pcie-sg2042.c line 161
Quoted 6 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +6
-0+ + msg->address_lo =lower_32_bits (pcie->msi_phys) +BYTE_NUM_PER_MSI_VEC * d->hwirq; + msg->address_hi =upper_32_bits (pcie->msi_phys); + msg->data =1 ; + + pcie->num_applied_vecs = d->hwirq;This looks questionable. How do you know
d->hwirqincreases every time this is called?
Thanks, it's a bug, I will fix it.
Quoted 4 lines
drivers/pci/controller/cadence/pcie-sg2042.c line 164
Quoted 2 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +2
-0+dev_info (dev,"compose msi msg hwirq[%d] address_hi[%#x] address_lo[%#x]\n" , + (int )d->hwirq, msg->address_hi, msg->address_lo);This seems too verbose to be a
dev_info(). Maybe adev_dbg()or remove it altogether.
Will change it to dev_dbg.
Quoted 6 lines
Quoted 5 lines
+ * We use the usual two domain structure, the top one being a generic PCI/MSI
+ * domain, the bottom one being SG2042-specific and handling the actual HW
+ * interrupt allocation.
+ * At the same time, for internal MSI controller(Method A), bottom chip uses a
+ * chained handler to handle the controller's MSI IRQ edge triggered.Add blank line between paragraphs.
OK.
Quoted 10 lines
Quoted 10 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +9
-0+static int sg2042_pcie_setup_msi_external (struct sg2042_pcie *pcie) +{ +struct device *dev = pcie->cdns_pcie->dev; +struct device_node *np = dev->of_node; +struct irq_domain *parent_domain; +struct device_node *parent_np; + +if (!of_find_property (np,"interrupt-parent" ,NULL )) { +dev_err (dev,"Can't find interrupt-parent!\n" );
Quoted 8 lines
drivers/pci/controller/cadence/pcie-sg2042.c line 276
Quoted 6 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +6
-0+return -EINVAL ; + } + + parent_np =of_irq_find_parent (np); +if (!parent_np) { +dev_err (dev,"Can't find node of interrupt-parent!\n" );Can you use some kind of %pOF format to include more information here?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/core-api/printk-formats.rst?id=v6.11#n463
Thanks, will double check this.
[......]
Quoted 14 lines
Quoted 14 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +14
-0+if (pcie->link_id ==1 ) { +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_LOW , +lower_32_bits (pcie->msi_phys)); +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_HIGH , +upper_32_bits (pcie->msi_phys)); + +regmap_read (pcie->syscon,REG_LINK1_MSI_ADDR_SIZE , &value); + value = (value ®_LINK1_MSI_ADDR_SIZE_MASK ) |MAX_MSI_IRQS ; +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_SIZE , value); + }else { +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_LOW , +lower_32_bits (pcie->msi_phys)); +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_HIGH , +upper_32_bits (pcie->msi_phys));
Quoted 8 lines
drivers/pci/controller/cadence/pcie-sg2042.c line 332
Quoted 5 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +5
-0+ +regmap_read (pcie->syscon,REG_LINK0_MSI_ADDR_SIZE , &value); + value = (value ®_LINK0_MSI_ADDR_SIZE_MASK ) | (MAX_MSI_IRQS <<16 ); +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_SIZE , value); + }Lot of
pcie->link_idchecking going on here. Consider saving these offsets in the structsg2042_pcieso you don't need to test everywhere.
Actually, there are not many places in the code to check link_id. If to add storage information in struct sg2042_pcie, at least four u32 are needed. And this logic will only be called one time in the probe. So I think it is better to keep the current method. What do you think?
[......]
Re: [PATCH 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver
Quoted 25 lines · Chen Wang
Quoted 15 lines · Bjorn Helgaas
Quoted 14 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/Kconfigdrivers/pci/controller/cadence/Kconfig +4
-0@@ -67,4 +67,15 @@ 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. + +config PCIE_SG2042 + bool "Sophgo SG2042 PCIe controller (host mode)" + depends on ARCH_SOPHGO || COMPILE_TESTQuoted 8 lines
drivers/pci/controller/cadence/Kconfig config PCI_J721E_EP · line 79
Quoted 6 lines
drivers/pci/controller/cadence/Kconfig +6
-0+ 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.Reorder to keep these menu items in alphabetical order by vendor.
Quoted 2 lines
Sorry, I don't understand your question. I think the menu items in this
Kconfigfile are already sorted alphabetically.
Here's what menuconfig looks like after this patch:
[*] Cadence platform PCIe controller (host mode) [*] Cadence platform PCIe controller (endpoint mode) [*] TI J721E PCIe controller (host mode) [*] TI J721E PCIe controller (endpoint mode) [ ] Sophgo SG2042 PCIe controller (host mode) (NEW)
"Sophgo" sorts *before* "TI".
Quoted 24 lines
Quoted 15 lines
Quoted 15 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +15
-0+if (pcie->link_id ==1 ) { +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_LOW , +lower_32_bits (pcie->msi_phys)); +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_HIGH , +upper_32_bits (pcie->msi_phys)); + +regmap_read (pcie->syscon,REG_LINK1_MSI_ADDR_SIZE , &value); + value = (value ®_LINK1_MSI_ADDR_SIZE_MASK ) |MAX_MSI_IRQS ; +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_SIZE , value); + }else { +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_LOW , +lower_32_bits (pcie->msi_phys)); +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_HIGH , +upper_32_bits (pcie->msi_phys)); +Quoted 8 lines
drivers/pci/controller/cadence/pcie-sg2042.c line 332
Quoted 4 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +4
-0+regmap_read (pcie->syscon,REG_LINK0_MSI_ADDR_SIZE , &value); + value = (value ®_LINK0_MSI_ADDR_SIZE_MASK ) | (MAX_MSI_IRQS <<16 ); +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_SIZE , value); + }Lot of
pcie->link_idchecking going on here. Consider saving these offsets in the structsg2042_pcieso you don't need to test everywhere.
Quoted 4 lines
Actually, there are not many places in the code to check
link_id. If to add storage information in structsg2042_pcie, at least four u32 are needed. And this logic will only be called one time in the probe. So I think it is better to keep the current method. What do you think?
1) I don't think "link_id" is a very good name since it appears to refer to one of two PCIe Root Ports. mvebu uses "marvell,pcie-port" which looks like a similar usage, although unnecessarily Marvell-specific. And arch/arm/boot/dts/marvell/armada-370-db.dts has separate stanzas for two Root Ports, without needing a property to distinguish them, which would be even better. I'm not sure why arch/arm/boot/dts/marvell/armada-370.dtsi needs "marvell,pcie-port" even though it also has separate stanzas.
2) I think checking pcie->link_id is likely to be harder to maintain in the future, e.g., if a new chip adds more Root Ports, you'll have to touch each use.
Bjorn
Re: [PATCH 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver
Quoted 28 lines · Bjorn Helgaas
Quoted 23 lines · Chen Wang
Quoted 14 lines · Bjorn Helgaas
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/Kconfigdrivers/pci/controller/cadence/Kconfig +3
-0@@ -67,4 +67,15 @@ 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. + +config PCIE_SG2042 + bool "Sophgo SG2042 PCIe controller (host mode)"Quoted 8 lines
drivers/pci/controller/cadence/Kconfig config PCI_J721E_EP · line 79
Quoted 7 lines
drivers/pci/controller/cadence/Kconfig +7
-0+ 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.Reorder to keep these menu items in alphabetical order by vendor.
Quoted 2 lines
Sorry, I don't understand your question. I think the menu items in this
Kconfigfile are already sorted alphabetically.Here's what menuconfig looks like after this patch:
Quoted 7 lines
[*] Cadence platform PCIe controller (host mode) [*] Cadence platform PCIe controller (endpoint mode) [*] TI J721E PCIe controller (host mode) [*] TI J721E PCIe controller (endpoint mode) [ ] Sophgo SG2042 PCIe controller (host mode) (NEW)
"Sophgo" sorts *before* "TI".
I see what you mean. I thought we just had to make sure the item IDs in the Kconfig file were in alphabetical order.
I will make changes in the next version based on your suggestions.
Quoted 35 lines
Quoted 22 lines
Quoted 14 lines
Quoted 14 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +14
-0+if (pcie->link_id ==1 ) { +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_LOW , +lower_32_bits (pcie->msi_phys)); +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_HIGH , +upper_32_bits (pcie->msi_phys)); + +regmap_read (pcie->syscon,REG_LINK1_MSI_ADDR_SIZE , &value); + value = (value ®_LINK1_MSI_ADDR_SIZE_MASK ) |MAX_MSI_IRQS ; +regmap_write (pcie->syscon,REG_LINK1_MSI_ADDR_SIZE , value); + }else { +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_LOW , +lower_32_bits (pcie->msi_phys)); +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_HIGH , +upper_32_bits (pcie->msi_phys));Quoted 8 lines
drivers/pci/controller/cadence/pcie-sg2042.c line 332
Quoted 5 lines
drivers/pci/controller/cadence/pcie-sg2042.c new file +5
-0+ +regmap_read (pcie->syscon,REG_LINK0_MSI_ADDR_SIZE , &value); + value = (value ®_LINK0_MSI_ADDR_SIZE_MASK ) | (MAX_MSI_IRQS <<16 ); +regmap_write (pcie->syscon,REG_LINK0_MSI_ADDR_SIZE , value); + }Lot of
pcie->link_idchecking going on here. Consider saving these offsets in the structsg2042_pcieso you don't need to test everywhere.Quoted 4 lines
Actually, there are not many places in the code to check
link_id. If to add storage information in structsg2042_pcie, at least four u32 are needed. And this logic will only be called one time in the probe. So I think it is better to keep the current method. What do you think?1) I don't think "
link_id" is a very good name since it appears to refer to one of two PCIe Root Ports. mvebu uses "marvell,pcie-port" which looks like a similar usage, although unnecessarily Marvell-specific. Andarch/arm/boot/dts/marvell/armada-370-db.dtshas separate stanzas for two Root Ports, without needing a property to distinguish them, which would be even better. I'm not sure whyarch/arm/boot/dts/marvell/armada-370.dtsineeds "marvell,pcie-port" even though it also has separate stanzas.
Quoted 5 lines
2) I think checking
pcie->link_idis likely to be harder to maintain in the future, e.g., if a new chip adds more Root Ports, you'll have to touch each use.Bjorn
Thanks for your input. I looked at the Marvell solution you recommended. It seems that it is a relatively complex IP design to support mvebu, which can flexibly support the export of 1 to 4 variable root ports.
The Cadence IP used by SG2042 is much simpler, and it is fixed to export up to two root ports. So I plan to implement it in a simple way. Changing "link_id" to "pcie-port" will look better. The writing of link actually refers to the terminology in the Cadence manual, which is actually the concept of port.
By the way, there is no demand for scalability at present, because it is said that Sophgo currently only uses Cadence IP for the SG2042 SoC, and subsequent products should switch to solutions from other suppliers.
Any question please let me know.
Regards,
Chen
PATCH 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
@@ -106,6 +106,7 @@ select: - rockchip,rk3576-qos - rockchip,rk3588-qos - rockchip,rv1126-qos + - sophgo,sg2042-pcie-ctrl - st,spear1340-misc - stericsson,nomadik-pmu - starfive,jh7100-sysmain
@@ -203,6 +204,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 3/5] dt-bindings: mfd: syscon: Add sg2042 pcie ctrl compatible
Quoted 10 lines · Chen Wang
From: Chen Wang <[email protected]>
Document SOPHGO SG2042 compatible for PCIe control registers. These registers are shared by pcie controller nodes.
Signed-off-by Chen Wang <[email protected]>--- Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++ 1 file changed, 2 insertions(+)
PATCH 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 | 82 ++++++++++++++++++++++++++ 1 file changed, 82 insertions(+)
arch/riscv/boot/dts/sophgo/sg2042.dtsi +82 -0
@@ -195,6 +195,88 @@ 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"; + #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,link-id = <0>; + sophgo,syscon-pcie-ctrl = <&cdns_pcie0_ctrl>; + interrupt-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"; + #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,link-id = <0>; + sophgo,syscon-pcie-ctrl = <&cdns_pcie1_ctrl>; + sophgo,internal-msi; + interrupt-parent = <&intc>; + interrupts = <123 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi"; + status = "disabled"; + }; + + 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"; + #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,link-id = <1>; + sophgo,syscon-pcie-ctrl = <&cdns_pcie1_ctrl>; + interrupt-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 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
@@ -45,6 +45,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