This is a series of patches that fixes the PCIe endpoint controller driver
for the Rockchip RK3399 SoC. The driver was introduced in commit
cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
The original driver had issues and would not allow for the RK3399 to
operate in PCIe endpoint mode correctly. This patch series fixes that so
that the PCIe core controller of the RK3399 SoC can now act as a PCIe
endpoint. This is v5 of the patch series and addresses the comments received
during the review of the v4 [1]. The changes to the v4 are minor and none of
them change the logic of the driver.
Thank you in advance for reviewing this patch series and hopefully
getting this merged. Having a functional PCIe endpoint controller
driver for the RK3399 would allow to develop further PCIe endpoint
functions through the Linux PCIe endpoint framework using this SoC.
[1] https://lore.kernel.org/linux-pci/[email protected]/
Summary of changes to V4 :
* [minor] Added "reviewed-by" tags from the mailing list to patches 1,7,8,9,10
* [minor] Added missing "fixes" tag in patches 10,11
* [minor] Added missing "signed-off-by" tag to patch 11
* [minor] Moved split of long line from patch 8 to 3 where it was introduced
Summary of changes to V3 :
* Set the fields in the standard order in the dtsi and removed unnecessary
change in associated documentation, thanks to Krzysztof for pointing this out.
* Added comment to explain how MSI-X capabilities advertisement was removed.
* Changed Damien Le Moal <[email protected]> address to his
kernel.org address Damien Le Moal <[email protected]> in tags.
* [minor] Simplified code where a power of 2 was applied followed by a ilog2
operation.
* [minor] Small code changes, replaced constant variable by macro, splitted
remaining long lines.
Summary of changes to V2 :
* Fix issue with memory mapping from PCIe space to physical space
There was a small mistake with the number of bits passed from the AXI
physical address to the PCIe space address.
* Disable the advertisement of MSI-X capabilities by the endpoint
According to the technical reference manual the controller cannot
generate MSI-X, so the controller should not advertise this capability.
* Add the alignment value to the endpoint attributes.
* [minor] Clean code (line length, variable names, small refactorings).
As pointed out by reviews on the V2.
* [minor] Fix error in variable name.
* [minor] Remove a patch that introduced unnecessary late parameter checks.
General problem statement and overview of the patch series :
Problem: The Rockchip RK3399 PCIe endpoint controller driver introduced in
commit cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe
controller") did not work.
Summary of problems with the driver :
* Missing dtsi entry
* Could not update Device ID (DID)
* The endpoint could not be configured by a host computer because the
endpoint kept sending Configuration Request Retry Status (CRS) messages
* The kernel would sometimes hang on probe due to access to registers in
a clock domain of which the PLLs were not locked
* The memory window mapping and address translation mechanism had
conflicting mappings and did not follow the technical reference manual
as to how the address translation should be done
* Legacy IRQs were not generated by the endpoint
* Message Signaled interrupts (MSI) were not generated by the endpoint
* MSI-X capabilities were advertised but the controller cannot generate
them according to the technical reference manual
The problems have been addressed and validated through tests (see below).
Summary of patches :
This patch series is composed of 11 patches that do the following :
1) Remove writes to unused registers in the PCIe core register space.
The registers that were written to is marked "unused" and read
only in the technical reference manual of the RK3399 SoC.
2) Write PCI Device ID (DID) to correct register, the DID was written to
a read only register and therefore would not update the DID.
3) Assert PCI Configuration Enable bit after probe so that it would stop
sending Configuration Request Retry Status (CRS) messages to the
host once configured, without this the host would retry until
timeout and cancel the PCI configuration.
4) Add poll and timeout to wait for PHY PLLs to be locked, this
is the only patch that also applies to the root complex function
of the PCIe core controller, without this the kernel would
sometimes access registers in the PHY PLL clock domain when the PLLs
were not yet locked and the system would hang. This was hackily solved
in other non mainline patches (e.g., in armbian) with a "msleep()"
that was added after PHY PLL configuration but without realizing
why it was needed. A poll with timeout seems like a sane approach.
5) Add dtsi entry for RK3399 PCIe endpoint core. The new entry is
in "disabled" status by default, so unless it is explicitly enabled
it will not conflict with the PCIe root complex controller entry.
Developers that will enable it would know that the root complex function
then must be disabled, this can be done in the board level DTS.
6) Update the RK3399 example in the documentation to a valid one.
7) Fix legacy IRQ generation for RK3399 PCIe endpoint core, the legacy IRQs
were not sent by the device because their generation did not follow the
instructions in the technical reference manual. They now work.
8) Fix window mapping and address translation for endpoint. The window
mapping and address translation did not follow the technical reference
manual and a single memory region was used which resulted in conflicting
address translations for memory allocated in that region. The current
patch allows to allocate up to 32 memory windows with 1MB pages.
9) Use u32 variable to access 32-bit registers, u16 variables were used to
access and manipulate data of 32-bit registers, this would lead to
overflows e.g., when left shifting more than 16 bits.
10) Don't advertise MSI-X in PCIe capabilities because according to the TRM
the controller is not capable of generating them.
11) Set address alignment for the endpoint mode.
Validation on real hardware:
This patch series has been tested by me with kernels 6.0.19, 6.1.21-24,
and 5.19 on real hardware, a FriendlyElec NanoPC-T4 RK3399 based single
computer board connected to a host computer through PCIe x1 and x4. The
driver was also tested by Damien Le Moal <[email protected]>
on a Pine Rockpro64 board [2,3,4].
[2] https://lore.kernel.org/linux-pci/[email protected]/
[3] https://lore.kernel.org/linux-pci/[email protected]/
[4] https://lore.kernel.org/linux-pci/[email protected]/
The PCIe endpoint test function driver was loaded on the SoC and the PCIe
endpoint test driver was loaded on the host computer. The following tests were
executed through this setup :
* enumeration of the PCIe endpoint device (lspci)
lspci -vvv
* validation of PCI header and capabilities
setpci and lspci -xxxx
* device was recognized by host computer dans PCIe endpoint test driver
was loaded
lspci -v states "Kernel modules: pci_endpoint_test"
* tested the BARs 0-5
sudo /usr/bin/pcitest -b 0
...
sudo /usr/bin/pcitest -b 5
* tested legacy interrupt through the test driver
sudo /usr/bin/pcitest -i 0
sudo /usr/bin/pcitest -l
* tested MSI interrupt through the test driver
sudo /usr/bin/pcitest -i 1
sudo /usr/bin/pcitest -m 1
* tested read/write to and from host through the test driver with checksum
sudo /usr/bin/pcitest -r -s 1024
sudo /usr/bin/pcitest -w -s 1024
* tested read/write with DMA enabled (all read/write tests also did IRQ)
sudo /usr/bin/pcitest -r -d -s 8192
sudo /usr/bin/pcitest -w -d -s 8192
* tested larged transfers e.g., 100kB with and without DMA
Commands used on the SoC to launch the endpoint function (configfs) :
modprobe -i pci-epf-test
mkdir -p /sys/kernel/config/pci_ep/functions/pci_epf_test/pci_epf_test.0
echo 0xb500 > /sys/kernel/config/pci_ep/functions/pci_epf_test/pci_epf_test.0/deviceid
echo 0x104c > /sys/kernel/config/pci_ep/functions/pci_epf_test/pci_epf_test.0/vendorid
echo 16 > /sys/kernel/config/pci_ep/functions/pci_epf_test/pci_epf_test.0/msi_interrupts
ln -s /sys/kernel/config/pci_ep/functions/pci_epf_test/pci_epf_test.0 \
/sys/kernel/config/pci_ep/controllers/fd000000.pcie-ep/
echo 1 > /sys/kernel/config/pci_ep/controllers/fd000000.pcie-ep/start
Note: to enable the endpoint controller on the board the file :
arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts
Was edited to set the status of &pcie0 to "disabled" and &pcie0_ep
to "okay". This is not submitted as a patch because most users
will use the PCIe core controller in host (root complex) mode
rather than endpoint mode.
I have tested and confirmed all functionality required for the
endpoint with the test driver and tools. With the initial driver commit
cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
the device would not even be enumerated by the host computer (mainly because
of CRS messages being sent back to the root complex) and tests would not pass
(driver would not even be loaded because DID was not set correctly)
and then only the BAR test would pass. Now all tests pass as stated above.
Best regards
Rick
Damien Le Moal (1):
PCI: rockchip: Set address alignment for endpoint mode
Rick Wertenbroek (10):
PCI: rockchip: Remove writes to unused registers
PCI: rockchip: Write PCI Device ID to correct register
PCI: rockchip: Assert PCI Configuration Enable bit after probe
PCI: rockchip: Add poll and timeout to wait for PHY PLLs to be locked
arm64: dts: rockchip: Add dtsi entry for RK3399 PCIe endpoint core
dt-bindings: PCI: Update the RK3399 example to a valid one
PCI: rockchip: Fix legacy IRQ generation for RK3399 PCIe endpoint core
PCI: rockchip: Fix window mapping and address translation for endpoint
PCI: rockchip: Use u32 variable to access 32-bit registers
PCI: rockchip: Don't advertise MSI-X in PCIe capabilities
.../bindings/pci/rockchip,rk3399-pcie-ep.yaml | 4 +-
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 27 +++
drivers/pci/controller/pcie-rockchip-ep.c | 221 ++++++++----------
drivers/pci/controller/pcie-rockchip.c | 17 ++
drivers/pci/controller/pcie-rockchip.h | 49 ++--
5 files changed, 181 insertions(+), 137 deletions(-)
--
2.25.1
Remove write accesses to registers that are marked "unused" (and
therefore read-only) in the technical reference manual (TRM)
(see RK3399 TRM 17.6.8.1)
Tested-by: Damien Le Moal <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Signed-off-by: Rick Wertenbroek <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index d1a200b93b2b..d5c477020417 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -61,10 +61,6 @@ static void rockchip_pcie_clear_ep_ob_atu(struct rockchip_pcie *rockchip,
ROCKCHIP_PCIE_AT_OB_REGION_DESC0(region));
rockchip_pcie_write(rockchip, 0,
ROCKCHIP_PCIE_AT_OB_REGION_DESC1(region));
- rockchip_pcie_write(rockchip, 0,
- ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR0(region));
- rockchip_pcie_write(rockchip, 0,
- ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR1(region));
}
static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
@@ -114,12 +110,6 @@ static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
PCIE_CORE_OB_REGION_ADDR0_LO_ADDR);
addr1 = upper_32_bits(cpu_addr);
}
-
- /* CPU bus address region */
- rockchip_pcie_write(rockchip, addr0,
- ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR0(r));
- rockchip_pcie_write(rockchip, addr1,
- ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR1(r));
}
static int rockchip_pcie_ep_write_header(struct pci_epc *epc, u8 fn, u8 vfn,
--
2.25.1
Write PCI Device ID (DID) to the correct register. The Device ID was not
updated through the correct register. Device ID was written to a read-only
register and therefore did not work. The Device ID is now set through the
correct register. This is documented in the RK3399 TRM section 17.6.6.1.1
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Cc: [email protected]
Reviewed-by: Damien Le Moal <[email protected]>
Tested-by: Damien Le Moal <[email protected]>
Signed-off-by: Rick Wertenbroek <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 6 ++++--
drivers/pci/controller/pcie-rockchip.h | 2 ++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index d5c477020417..9b835377bd9e 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -115,6 +115,7 @@ static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
static int rockchip_pcie_ep_write_header(struct pci_epc *epc, u8 fn, u8 vfn,
struct pci_epf_header *hdr)
{
+ u32 reg;
struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
struct rockchip_pcie *rockchip = &ep->rockchip;
@@ -127,8 +128,9 @@ static int rockchip_pcie_ep_write_header(struct pci_epc *epc, u8 fn, u8 vfn,
PCIE_CORE_CONFIG_VENDOR);
}
- rockchip_pcie_write(rockchip, hdr->deviceid << 16,
- ROCKCHIP_PCIE_EP_FUNC_BASE(fn) + PCI_VENDOR_ID);
+ reg = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_DID_VID);
+ reg = (reg & 0xFFFF) | (hdr->deviceid << 16);
+ rockchip_pcie_write(rockchip, reg, PCIE_EP_CONFIG_DID_VID);
rockchip_pcie_write(rockchip,
hdr->revid |
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 32c3a859c26b..51a123e5c0cf 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -133,6 +133,8 @@
#define PCIE_RC_RP_ATS_BASE 0x400000
#define PCIE_RC_CONFIG_NORMAL_BASE 0x800000
#define PCIE_RC_CONFIG_BASE 0xa00000
+#define PCIE_EP_CONFIG_BASE 0xa00000
+#define PCIE_EP_CONFIG_DID_VID (PCIE_EP_CONFIG_BASE + 0x00)
#define PCIE_RC_CONFIG_RID_CCR (PCIE_RC_CONFIG_BASE + 0x08)
#define PCIE_RC_CONFIG_DCR (PCIE_RC_CONFIG_BASE + 0xc4)
#define PCIE_RC_CONFIG_DCR_CSPL_SHIFT 18
--
2.25.1
Assert PCI Configuration Enable bit after probe. When this bit is left to
0 in the endpoint mode, the RK3399 PCIe endpoint core will generate
configuration request retry status (CRS) messages back to the root complex.
Assert this bit after probe to allow the RK3399 PCIe endpoint core to reply
to configuration requests from the root complex.
This is documented in section 17.5.8.1.2 of the RK3399 TRM.
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Cc: [email protected]
Reviewed-by: Damien Le Moal <[email protected]>
Tested-by: Damien Le Moal <[email protected]>
Signed-off-by: Rick Wertenbroek <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 9b835377bd9e..d00baed65eba 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -623,6 +623,9 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
ep->irq_pci_addr = ROCKCHIP_PCIE_EP_DUMMY_IRQ_ADDR;
+ rockchip_pcie_write(rockchip, PCIE_CLIENT_CONF_ENABLE,
+ PCIE_CLIENT_CONFIG);
+
return 0;
err_epc_mem_exit:
pci_epc_mem_exit(epc);
--
2.25.1
The RK3399 PCIe controller should wait until the PHY PLLs are locked.
Add poll and timeout to wait for PHY PLLs to be locked. If they cannot
be locked generate error message and jump to error handler. Accessing
registers in the PHY clock domain when PLLs are not locked causes hang
The PHY PLLs status is checked through a side channel register.
This is documented in the TRM section 17.5.8.1 "PCIe Initialization
Sequence".
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Cc: [email protected]
Reviewed-by: Damien Le Moal <[email protected]>
Tested-by: Damien Le Moal <[email protected]>
Signed-off-by: Rick Wertenbroek <[email protected]>
---
drivers/pci/controller/pcie-rockchip.c | 17 +++++++++++++++++
drivers/pci/controller/pcie-rockchip.h | 2 ++
2 files changed, 19 insertions(+)
diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
index 990a00e08bc5..1aa84035a8bc 100644
--- a/drivers/pci/controller/pcie-rockchip.c
+++ b/drivers/pci/controller/pcie-rockchip.c
@@ -14,6 +14,7 @@
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/gpio/consumer.h>
+#include <linux/iopoll.h>
#include <linux/of_pci.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
@@ -153,6 +154,12 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
}
EXPORT_SYMBOL_GPL(rockchip_pcie_parse_dt);
+#define rockchip_pcie_read_addr(addr) rockchip_pcie_read(rockchip, addr)
+/* 100 ms max wait time for PHY PLLs to lock */
+#define RK_PHY_PLL_LOCK_TIMEOUT_US 100000
+/* Sleep should be less than 20ms */
+#define RK_PHY_PLL_LOCK_SLEEP_US 1000
+
int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
{
struct device *dev = rockchip->dev;
@@ -254,6 +261,16 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
}
}
+ err = readx_poll_timeout(rockchip_pcie_read_addr,
+ PCIE_CLIENT_SIDE_BAND_STATUS,
+ regs, !(regs & PCIE_CLIENT_PHY_ST),
+ RK_PHY_PLL_LOCK_SLEEP_US,
+ RK_PHY_PLL_LOCK_TIMEOUT_US);
+ if (err) {
+ dev_err(dev, "PHY PLLs could not lock, %d\n", err);
+ goto err_power_off_phy;
+ }
+
/*
* Please don't reorder the deassert sequence of the following
* four reset pins.
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 51a123e5c0cf..f3a5ff1cf7f4 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -38,6 +38,8 @@
#define PCIE_CLIENT_MODE_EP HIWORD_UPDATE(0x0040, 0)
#define PCIE_CLIENT_GEN_SEL_1 HIWORD_UPDATE(0x0080, 0)
#define PCIE_CLIENT_GEN_SEL_2 HIWORD_UPDATE_BIT(0x0080)
+#define PCIE_CLIENT_SIDE_BAND_STATUS (PCIE_CLIENT_BASE + 0x20)
+#define PCIE_CLIENT_PHY_ST BIT(12)
#define PCIE_CLIENT_DEBUG_OUT_0 (PCIE_CLIENT_BASE + 0x3c)
#define PCIE_CLIENT_DEBUG_LTSSM_MASK GENMASK(5, 0)
#define PCIE_CLIENT_DEBUG_LTSSM_L1 0x18
--
2.25.1
Update the example in the documentation to a valid example.
Address for mem-base was invalid, it pointed to address
0x8000'0000 which is the upper region of the DDR which
is not necessarily populated depending on the board.
This address should point to the base of the memory
window region of the controller which is 0xfa00'0000.
Add missing pinctrl.
Signed-off-by: Rick Wertenbroek <[email protected]>
---
.../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
index 88386a6d7011..6b62f6f58efe 100644
--- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
@@ -47,7 +47,7 @@ examples:
pcie-ep@f8000000 {
compatible = "rockchip,rk3399-pcie-ep";
- reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0x80000000 0x0 0x20000>;
+ reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
reg-names = "apb-base", "mem-base";
clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
<&cru PCLK_PCIE>, <&cru SCLK_PCIE_PM>;
@@ -63,6 +63,8 @@ examples:
phys = <&pcie_phy 0>, <&pcie_phy 1>, <&pcie_phy 2>, <&pcie_phy 3>;
phy-names = "pcie-phy-0", "pcie-phy-1", "pcie-phy-2", "pcie-phy-3";
rockchip,max-outbound-regions = <16>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie_clkreqnb_cpm>;
};
};
...
--
2.25.1
Add dtsi entry for RK3399 PCIe endpoint core in the device tree.
The status is "disabled" by default, so it will not be loaded unless
explicitly chosen to. The RK3399 PCIe endpoit core should be enabled
with the RK3399 PCIe root complex disabled because the RK3399 PCIe
controller can only work one mode at the time, either in "root complex"
mode or in "endpoint" mode.
Tested-by: Damien Le Moal <[email protected]>
Signed-off-by: Rick Wertenbroek <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 27 ++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 928948e7c7bb..9da0b6d77c8d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -265,6 +265,33 @@ pcie0_intc: interrupt-controller {
};
};
+ pcie0_ep: pcie-ep@f8000000 {
+ compatible = "rockchip,rk3399-pcie-ep";
+ reg = <0x0 0xfd000000 0x0 0x1000000>,
+ <0x0 0xfa000000 0x0 0x2000000>;
+ reg-names = "apb-base", "mem-base";
+ clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
+ <&cru PCLK_PCIE>, <&cru SCLK_PCIE_PM>;
+ clock-names = "aclk", "aclk-perf",
+ "hclk", "pm";
+ max-functions = /bits/ 8 <8>;
+ num-lanes = <4>;
+ resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
+ <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
+ <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>,
+ <&cru SRST_A_PCIE>;
+ reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
+ "pm", "pclk", "aclk";
+ phys = <&pcie_phy 0>, <&pcie_phy 1>,
+ <&pcie_phy 2>, <&pcie_phy 3>;
+ phy-names = "pcie-phy-0", "pcie-phy-1",
+ "pcie-phy-2", "pcie-phy-3";
+ rockchip,max-outbound-regions = <32>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie_clkreqnb_cpm>;
+ status = "disabled";
+ };
+
gmac: ethernet@fe300000 {
compatible = "rockchip,rk3399-gmac";
reg = <0x0 0xfe300000 0x0 0x10000>;
--
2.25.1
Fix legacy IRQ generation for RK3399 PCIe endpoint core according to
the technical reference manual (TRM). Assert and deassert legacy
interrupt (INTx) through the legacy interrupt control register
("PCIE_CLIENT_LEGACY_INT_CTRL") instead of manually generating a PCIe
message. The generation of the legacy interrupt was tested and validated
with the PCIe endpoint test driver.
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Cc: [email protected]
Tested-by: Damien Le Moal <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Signed-off-by: Rick Wertenbroek <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 45 ++++++-----------------
drivers/pci/controller/pcie-rockchip.h | 6 ++-
2 files changed, 16 insertions(+), 35 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index d00baed65eba..d6d4b0f2e733 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -337,48 +337,25 @@ static int rockchip_pcie_ep_get_msi(struct pci_epc *epc, u8 fn, u8 vfn)
}
static void rockchip_pcie_ep_assert_intx(struct rockchip_pcie_ep *ep, u8 fn,
- u8 intx, bool is_asserted)
+ u8 intx, bool do_assert)
{
struct rockchip_pcie *rockchip = &ep->rockchip;
- u32 r = ep->max_regions - 1;
- u32 offset;
- u32 status;
- u8 msg_code;
-
- if (unlikely(ep->irq_pci_addr != ROCKCHIP_PCIE_EP_PCI_LEGACY_IRQ_ADDR ||
- ep->irq_pci_fn != fn)) {
- rockchip_pcie_prog_ep_ob_atu(rockchip, fn, r,
- AXI_WRAPPER_NOR_MSG,
- ep->irq_phys_addr, 0, 0);
- ep->irq_pci_addr = ROCKCHIP_PCIE_EP_PCI_LEGACY_IRQ_ADDR;
- ep->irq_pci_fn = fn;
- }
intx &= 3;
- if (is_asserted) {
+
+ if (do_assert) {
ep->irq_pending |= BIT(intx);
- msg_code = ROCKCHIP_PCIE_MSG_CODE_ASSERT_INTA + intx;
+ rockchip_pcie_write(rockchip,
+ PCIE_CLIENT_INT_IN_ASSERT |
+ PCIE_CLIENT_INT_PEND_ST_PEND,
+ PCIE_CLIENT_LEGACY_INT_CTRL);
} else {
ep->irq_pending &= ~BIT(intx);
- msg_code = ROCKCHIP_PCIE_MSG_CODE_DEASSERT_INTA + intx;
+ rockchip_pcie_write(rockchip,
+ PCIE_CLIENT_INT_IN_DEASSERT |
+ PCIE_CLIENT_INT_PEND_ST_NORMAL,
+ PCIE_CLIENT_LEGACY_INT_CTRL);
}
-
- status = rockchip_pcie_read(rockchip,
- ROCKCHIP_PCIE_EP_FUNC_BASE(fn) +
- ROCKCHIP_PCIE_EP_CMD_STATUS);
- status &= ROCKCHIP_PCIE_EP_CMD_STATUS_IS;
-
- if ((status != 0) ^ (ep->irq_pending != 0)) {
- status ^= ROCKCHIP_PCIE_EP_CMD_STATUS_IS;
- rockchip_pcie_write(rockchip, status,
- ROCKCHIP_PCIE_EP_FUNC_BASE(fn) +
- ROCKCHIP_PCIE_EP_CMD_STATUS);
- }
-
- offset =
- ROCKCHIP_PCIE_MSG_ROUTING(ROCKCHIP_PCIE_MSG_ROUTING_LOCAL_INTX) |
- ROCKCHIP_PCIE_MSG_CODE(msg_code) | ROCKCHIP_PCIE_MSG_NO_DATA;
- writel(0, ep->irq_cpu_addr + offset);
}
static int rockchip_pcie_ep_send_legacy_irq(struct rockchip_pcie_ep *ep, u8 fn,
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index f3a5ff1cf7f4..ffc68a3a5fee 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -38,6 +38,11 @@
#define PCIE_CLIENT_MODE_EP HIWORD_UPDATE(0x0040, 0)
#define PCIE_CLIENT_GEN_SEL_1 HIWORD_UPDATE(0x0080, 0)
#define PCIE_CLIENT_GEN_SEL_2 HIWORD_UPDATE_BIT(0x0080)
+#define PCIE_CLIENT_LEGACY_INT_CTRL (PCIE_CLIENT_BASE + 0x0c)
+#define PCIE_CLIENT_INT_IN_ASSERT HIWORD_UPDATE_BIT(0x0002)
+#define PCIE_CLIENT_INT_IN_DEASSERT HIWORD_UPDATE(0x0002, 0)
+#define PCIE_CLIENT_INT_PEND_ST_PEND HIWORD_UPDATE_BIT(0x0001)
+#define PCIE_CLIENT_INT_PEND_ST_NORMAL HIWORD_UPDATE(0x0001, 0)
#define PCIE_CLIENT_SIDE_BAND_STATUS (PCIE_CLIENT_BASE + 0x20)
#define PCIE_CLIENT_PHY_ST BIT(12)
#define PCIE_CLIENT_DEBUG_OUT_0 (PCIE_CLIENT_BASE + 0x3c)
@@ -227,7 +232,6 @@
#define ROCKCHIP_PCIE_EP_MSI_CTRL_ME BIT(16)
#define ROCKCHIP_PCIE_EP_MSI_CTRL_MASK_MSI_CAP BIT(24)
#define ROCKCHIP_PCIE_EP_DUMMY_IRQ_ADDR 0x1
-#define ROCKCHIP_PCIE_EP_PCI_LEGACY_IRQ_ADDR 0x3
#define ROCKCHIP_PCIE_EP_FUNC_BASE(fn) (((fn) << 12) & GENMASK(19, 12))
#define ROCKCHIP_PCIE_AT_IB_EP_FUNC_BAR_ADDR0(fn, bar) \
(PCIE_RC_RP_ATS_BASE + 0x0840 + (fn) * 0x0040 + (bar) * 0x0008)
--
2.25.1
Previously u16 variables were used to access 32-bit registers, this
resulted in not all of the data being read from the registers. Also
the left shift of more than 16-bits would result in moving data out
of the variable. Use u32 variables to access 32-bit registers
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Cc: [email protected]
Tested-by: Damien Le Moal <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Signed-off-by: Rick Wertenbroek <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 10 +++++-----
drivers/pci/controller/pcie-rockchip.h | 1 +
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 771f1bb93251..63fbb379638b 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -271,15 +271,15 @@ static int rockchip_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 vfn,
{
struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
struct rockchip_pcie *rockchip = &ep->rockchip;
- u16 flags;
+ u32 flags;
flags = rockchip_pcie_read(rockchip,
ROCKCHIP_PCIE_EP_FUNC_BASE(fn) +
ROCKCHIP_PCIE_EP_MSI_CTRL_REG);
flags &= ~ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_MASK;
flags |=
- ((multi_msg_cap << 1) << ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_OFFSET) |
- PCI_MSI_FLAGS_64BIT;
+ (multi_msg_cap << ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_OFFSET) |
+ (PCI_MSI_FLAGS_64BIT << ROCKCHIP_PCIE_EP_MSI_FLAGS_OFFSET);
flags &= ~ROCKCHIP_PCIE_EP_MSI_CTRL_MASK_MSI_CAP;
rockchip_pcie_write(rockchip, flags,
ROCKCHIP_PCIE_EP_FUNC_BASE(fn) +
@@ -291,7 +291,7 @@ static int rockchip_pcie_ep_get_msi(struct pci_epc *epc, u8 fn, u8 vfn)
{
struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
struct rockchip_pcie *rockchip = &ep->rockchip;
- u16 flags;
+ u32 flags;
flags = rockchip_pcie_read(rockchip,
ROCKCHIP_PCIE_EP_FUNC_BASE(fn) +
@@ -352,7 +352,7 @@ static int rockchip_pcie_ep_send_msi_irq(struct rockchip_pcie_ep *ep, u8 fn,
u8 interrupt_num)
{
struct rockchip_pcie *rockchip = &ep->rockchip;
- u16 flags, mme, data, data_mask;
+ u32 flags, mme, data, data_mask;
u8 msi_count;
u64 pci_addr;
u32 r;
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index bef6d7098a2f..501d859420b4 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -227,6 +227,7 @@
#define ROCKCHIP_PCIE_EP_CMD_STATUS 0x4
#define ROCKCHIP_PCIE_EP_CMD_STATUS_IS BIT(19)
#define ROCKCHIP_PCIE_EP_MSI_CTRL_REG 0x90
+#define ROCKCHIP_PCIE_EP_MSI_FLAGS_OFFSET 16
#define ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_OFFSET 17
#define ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_MASK GENMASK(19, 17)
#define ROCKCHIP_PCIE_EP_MSI_CTRL_MME_OFFSET 20
--
2.25.1
The RK3399 PCI endpoint core has 33 windows for PCIe space, now in the
driver up to 32 fixed size (1M) windows are used and pages are allocated
and mapped accordingly. The driver first used a single window and allocated
space inside which caused translation issues (between CPU space and PCI
space) because a window can only have a single translation at a given
time, which if multiple pages are allocated inside will cause conflicts.
Now each window is a single region of 1M which will always guarantee that
the translation is not in conflict.
Set the translation register addresses for physical function. As documented
in the technical reference manual (TRM) section 17.5.5 "PCIe Address
Translation" and section 17.6.8 "Address Translation Registers Description"
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Cc: [email protected]
Tested-by: Damien Le Moal <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Signed-off-by: Rick Wertenbroek <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 128 ++++++++++------------
drivers/pci/controller/pcie-rockchip.h | 35 +++---
2 files changed, 75 insertions(+), 88 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index d6d4b0f2e733..771f1bb93251 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -64,52 +64,29 @@ static void rockchip_pcie_clear_ep_ob_atu(struct rockchip_pcie *rockchip,
}
static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
- u32 r, u32 type, u64 cpu_addr,
- u64 pci_addr, size_t size)
+ u32 r, u64 cpu_addr, u64 pci_addr,
+ size_t size)
{
- u64 sz = 1ULL << fls64(size - 1);
- int num_pass_bits = ilog2(sz);
- u32 addr0, addr1, desc0, desc1;
- bool is_nor_msg = (type == AXI_WRAPPER_NOR_MSG);
+ int num_pass_bits = fls64(size - 1);
+ u32 addr0, addr1, desc0;
- /* The minimal region size is 1MB */
if (num_pass_bits < 8)
num_pass_bits = 8;
- cpu_addr -= rockchip->mem_res->start;
- addr0 = ((is_nor_msg ? 0x10 : (num_pass_bits - 1)) &
- PCIE_CORE_OB_REGION_ADDR0_NUM_BITS) |
- (lower_32_bits(cpu_addr) & PCIE_CORE_OB_REGION_ADDR0_LO_ADDR);
- addr1 = upper_32_bits(is_nor_msg ? cpu_addr : pci_addr);
- desc0 = ROCKCHIP_PCIE_AT_OB_REGION_DESC0_DEVFN(fn) | type;
- desc1 = 0;
-
- if (is_nor_msg) {
- rockchip_pcie_write(rockchip, 0,
- ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0(r));
- rockchip_pcie_write(rockchip, 0,
- ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR1(r));
- rockchip_pcie_write(rockchip, desc0,
- ROCKCHIP_PCIE_AT_OB_REGION_DESC0(r));
- rockchip_pcie_write(rockchip, desc1,
- ROCKCHIP_PCIE_AT_OB_REGION_DESC1(r));
- } else {
- /* PCI bus address region */
- rockchip_pcie_write(rockchip, addr0,
- ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0(r));
- rockchip_pcie_write(rockchip, addr1,
- ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR1(r));
- rockchip_pcie_write(rockchip, desc0,
- ROCKCHIP_PCIE_AT_OB_REGION_DESC0(r));
- rockchip_pcie_write(rockchip, desc1,
- ROCKCHIP_PCIE_AT_OB_REGION_DESC1(r));
-
- addr0 =
- ((num_pass_bits - 1) & PCIE_CORE_OB_REGION_ADDR0_NUM_BITS) |
- (lower_32_bits(cpu_addr) &
- PCIE_CORE_OB_REGION_ADDR0_LO_ADDR);
- addr1 = upper_32_bits(cpu_addr);
- }
+ addr0 = ((num_pass_bits - 1) & PCIE_CORE_OB_REGION_ADDR0_NUM_BITS) |
+ (lower_32_bits(pci_addr) & PCIE_CORE_OB_REGION_ADDR0_LO_ADDR);
+ addr1 = upper_32_bits(pci_addr);
+ desc0 = ROCKCHIP_PCIE_AT_OB_REGION_DESC0_DEVFN(fn) | AXI_WRAPPER_MEM_WRITE;
+
+ /* PCI bus address region */
+ rockchip_pcie_write(rockchip, addr0,
+ ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0(r));
+ rockchip_pcie_write(rockchip, addr1,
+ ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR1(r));
+ rockchip_pcie_write(rockchip, desc0,
+ ROCKCHIP_PCIE_AT_OB_REGION_DESC0(r));
+ rockchip_pcie_write(rockchip, 0,
+ ROCKCHIP_PCIE_AT_OB_REGION_DESC1(r));
}
static int rockchip_pcie_ep_write_header(struct pci_epc *epc, u8 fn, u8 vfn,
@@ -248,26 +225,20 @@ static void rockchip_pcie_ep_clear_bar(struct pci_epc *epc, u8 fn, u8 vfn,
ROCKCHIP_PCIE_AT_IB_EP_FUNC_BAR_ADDR1(fn, bar));
}
+static inline u32 rockchip_ob_region(phys_addr_t addr)
+{
+ return (addr >> ilog2(SZ_1M)) & 0x1f;
+}
+
static int rockchip_pcie_ep_map_addr(struct pci_epc *epc, u8 fn, u8 vfn,
phys_addr_t addr, u64 pci_addr,
size_t size)
{
struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
struct rockchip_pcie *pcie = &ep->rockchip;
- u32 r;
+ u32 r = rockchip_ob_region(addr);
- r = find_first_zero_bit(&ep->ob_region_map, BITS_PER_LONG);
- /*
- * Region 0 is reserved for configuration space and shouldn't
- * be used elsewhere per TRM, so leave it out.
- */
- if (r >= ep->max_regions - 1) {
- dev_err(&epc->dev, "no free outbound region\n");
- return -EINVAL;
- }
-
- rockchip_pcie_prog_ep_ob_atu(pcie, fn, r, AXI_WRAPPER_MEM_WRITE, addr,
- pci_addr, size);
+ rockchip_pcie_prog_ep_ob_atu(pcie, fn, r, addr, pci_addr, size);
set_bit(r, &ep->ob_region_map);
ep->ob_addr[r] = addr;
@@ -282,15 +253,11 @@ static void rockchip_pcie_ep_unmap_addr(struct pci_epc *epc, u8 fn, u8 vfn,
struct rockchip_pcie *rockchip = &ep->rockchip;
u32 r;
- for (r = 0; r < ep->max_regions - 1; r++)
+ for (r = 0; r < ep->max_regions; r++)
if (ep->ob_addr[r] == addr)
break;
- /*
- * Region 0 is reserved for configuration space and shouldn't
- * be used elsewhere per TRM, so leave it out.
- */
- if (r == ep->max_regions - 1)
+ if (r == ep->max_regions)
return;
rockchip_pcie_clear_ep_ob_atu(rockchip, r);
@@ -387,7 +354,8 @@ static int rockchip_pcie_ep_send_msi_irq(struct rockchip_pcie_ep *ep, u8 fn,
struct rockchip_pcie *rockchip = &ep->rockchip;
u16 flags, mme, data, data_mask;
u8 msi_count;
- u64 pci_addr, pci_addr_mask = 0xff;
+ u64 pci_addr;
+ u32 r;
/* Check MSI enable bit */
flags = rockchip_pcie_read(&ep->rockchip,
@@ -421,21 +389,20 @@ static int rockchip_pcie_ep_send_msi_irq(struct rockchip_pcie_ep *ep, u8 fn,
ROCKCHIP_PCIE_EP_FUNC_BASE(fn) +
ROCKCHIP_PCIE_EP_MSI_CTRL_REG +
PCI_MSI_ADDRESS_LO);
- pci_addr &= GENMASK_ULL(63, 2);
/* Set the outbound region if needed. */
- if (unlikely(ep->irq_pci_addr != (pci_addr & ~pci_addr_mask) ||
+ if (unlikely(ep->irq_pci_addr != (pci_addr & PCIE_ADDR_MASK) ||
ep->irq_pci_fn != fn)) {
- rockchip_pcie_prog_ep_ob_atu(rockchip, fn, ep->max_regions - 1,
- AXI_WRAPPER_MEM_WRITE,
+ r = rockchip_ob_region(ep->irq_phys_addr);
+ rockchip_pcie_prog_ep_ob_atu(rockchip, fn, r,
ep->irq_phys_addr,
- pci_addr & ~pci_addr_mask,
- pci_addr_mask + 1);
- ep->irq_pci_addr = (pci_addr & ~pci_addr_mask);
+ pci_addr & PCIE_ADDR_MASK,
+ ~PCIE_ADDR_MASK + 1);
+ ep->irq_pci_addr = (pci_addr & PCIE_ADDR_MASK);
ep->irq_pci_fn = fn;
}
- writew(data, ep->irq_cpu_addr + (pci_addr & pci_addr_mask));
+ writew(data, ep->irq_cpu_addr + (pci_addr & ~PCIE_ADDR_MASK));
return 0;
}
@@ -516,6 +483,8 @@ static int rockchip_pcie_parse_ep_dt(struct rockchip_pcie *rockchip,
if (err < 0 || ep->max_regions > MAX_REGION_LIMIT)
ep->max_regions = MAX_REGION_LIMIT;
+ ep->ob_region_map = 0;
+
err = of_property_read_u8(dev->of_node, "max-functions",
&ep->epc->max_functions);
if (err < 0)
@@ -536,7 +505,8 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
struct rockchip_pcie *rockchip;
struct pci_epc *epc;
size_t max_regions;
- int err;
+ struct pci_epc_mem_window *windows = NULL;
+ int err, i;
ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
if (!ep)
@@ -583,15 +553,27 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
/* Only enable function 0 by default */
rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG);
- err = pci_epc_mem_init(epc, rockchip->mem_res->start,
- resource_size(rockchip->mem_res), PAGE_SIZE);
+ windows = devm_kcalloc(dev, ep->max_regions,
+ sizeof(struct pci_epc_mem_window), GFP_KERNEL);
+ if (!windows) {
+ err = -ENOMEM;
+ goto err_uninit_port;
+ }
+ for (i = 0; i < ep->max_regions; i++) {
+ windows[i].phys_base = rockchip->mem_res->start + (SZ_1M * i);
+ windows[i].size = SZ_1M;
+ windows[i].page_size = SZ_1M;
+ }
+ err = pci_epc_multi_mem_init(epc, windows, ep->max_regions);
+ devm_kfree(dev, windows);
+
if (err < 0) {
dev_err(dev, "failed to initialize the memory space\n");
goto err_uninit_port;
}
ep->irq_cpu_addr = pci_epc_mem_alloc_addr(epc, &ep->irq_phys_addr,
- SZ_128K);
+ SZ_1M);
if (!ep->irq_cpu_addr) {
dev_err(dev, "failed to reserve memory space for MSI\n");
err = -ENOMEM;
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index ffc68a3a5fee..bef6d7098a2f 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -139,6 +139,7 @@
#define PCIE_RC_RP_ATS_BASE 0x400000
#define PCIE_RC_CONFIG_NORMAL_BASE 0x800000
+#define PCIE_EP_PF_CONFIG_REGS_BASE 0x800000
#define PCIE_RC_CONFIG_BASE 0xa00000
#define PCIE_EP_CONFIG_BASE 0xa00000
#define PCIE_EP_CONFIG_DID_VID (PCIE_EP_CONFIG_BASE + 0x00)
@@ -157,10 +158,11 @@
#define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)
#define PCIE_RC_CONFIG_THP_CAP_NEXT_MASK GENMASK(31, 20)
+#define PCIE_ADDR_MASK 0xffffff00
#define PCIE_CORE_AXI_CONF_BASE 0xc00000
#define PCIE_CORE_OB_REGION_ADDR0 (PCIE_CORE_AXI_CONF_BASE + 0x0)
#define PCIE_CORE_OB_REGION_ADDR0_NUM_BITS 0x3f
-#define PCIE_CORE_OB_REGION_ADDR0_LO_ADDR 0xffffff00
+#define PCIE_CORE_OB_REGION_ADDR0_LO_ADDR PCIE_ADDR_MASK
#define PCIE_CORE_OB_REGION_ADDR1 (PCIE_CORE_AXI_CONF_BASE + 0x4)
#define PCIE_CORE_OB_REGION_DESC0 (PCIE_CORE_AXI_CONF_BASE + 0x8)
#define PCIE_CORE_OB_REGION_DESC1 (PCIE_CORE_AXI_CONF_BASE + 0xc)
@@ -168,7 +170,7 @@
#define PCIE_CORE_AXI_INBOUND_BASE 0xc00800
#define PCIE_RP_IB_ADDR0 (PCIE_CORE_AXI_INBOUND_BASE + 0x0)
#define PCIE_CORE_IB_REGION_ADDR0_NUM_BITS 0x3f
-#define PCIE_CORE_IB_REGION_ADDR0_LO_ADDR 0xffffff00
+#define PCIE_CORE_IB_REGION_ADDR0_LO_ADDR PCIE_ADDR_MASK
#define PCIE_RP_IB_ADDR1 (PCIE_CORE_AXI_INBOUND_BASE + 0x4)
/* Size of one AXI Region (not Region 0) */
@@ -232,13 +234,15 @@
#define ROCKCHIP_PCIE_EP_MSI_CTRL_ME BIT(16)
#define ROCKCHIP_PCIE_EP_MSI_CTRL_MASK_MSI_CAP BIT(24)
#define ROCKCHIP_PCIE_EP_DUMMY_IRQ_ADDR 0x1
-#define ROCKCHIP_PCIE_EP_FUNC_BASE(fn) (((fn) << 12) & GENMASK(19, 12))
+#define ROCKCHIP_PCIE_EP_PCI_LEGACY_IRQ_ADDR 0x3
+#define ROCKCHIP_PCIE_EP_FUNC_BASE(fn) \
+ (PCIE_EP_PF_CONFIG_REGS_BASE + (((fn) << 12) & GENMASK(19, 12)))
+#define ROCKCHIP_PCIE_EP_VIRT_FUNC_BASE(fn) \
+ (PCIE_EP_PF_CONFIG_REGS_BASE + 0x10000 + (((fn) << 12) & GENMASK(19, 12)))
#define ROCKCHIP_PCIE_AT_IB_EP_FUNC_BAR_ADDR0(fn, bar) \
- (PCIE_RC_RP_ATS_BASE + 0x0840 + (fn) * 0x0040 + (bar) * 0x0008)
+ (PCIE_CORE_AXI_CONF_BASE + 0x0828 + (fn) * 0x0040 + (bar) * 0x0008)
#define ROCKCHIP_PCIE_AT_IB_EP_FUNC_BAR_ADDR1(fn, bar) \
- (PCIE_RC_RP_ATS_BASE + 0x0844 + (fn) * 0x0040 + (bar) * 0x0008)
-#define ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0(r) \
- (PCIE_RC_RP_ATS_BASE + 0x0000 + ((r) & 0x1f) * 0x0020)
+ (PCIE_CORE_AXI_CONF_BASE + 0x082c + (fn) * 0x0040 + (bar) * 0x0008)
#define ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN_MASK GENMASK(19, 12)
#define ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN(devfn) \
(((devfn) << 12) & \
@@ -246,20 +250,21 @@
#define ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK GENMASK(27, 20)
#define ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0_BUS(bus) \
(((bus) << 20) & ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK)
+#define PCIE_RC_EP_ATR_OB_REGIONS_1_32 (PCIE_CORE_AXI_CONF_BASE + 0x0020)
+#define ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR0(r) \
+ (PCIE_RC_EP_ATR_OB_REGIONS_1_32 + 0x0000 + ((r) & 0x1f) * 0x0020)
#define ROCKCHIP_PCIE_AT_OB_REGION_PCI_ADDR1(r) \
- (PCIE_RC_RP_ATS_BASE + 0x0004 + ((r) & 0x1f) * 0x0020)
+ (PCIE_RC_EP_ATR_OB_REGIONS_1_32 + 0x0004 + ((r) & 0x1f) * 0x0020)
#define ROCKCHIP_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID BIT(23)
#define ROCKCHIP_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK GENMASK(31, 24)
#define ROCKCHIP_PCIE_AT_OB_REGION_DESC0_DEVFN(devfn) \
(((devfn) << 24) & ROCKCHIP_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK)
#define ROCKCHIP_PCIE_AT_OB_REGION_DESC0(r) \
- (PCIE_RC_RP_ATS_BASE + 0x0008 + ((r) & 0x1f) * 0x0020)
-#define ROCKCHIP_PCIE_AT_OB_REGION_DESC1(r) \
- (PCIE_RC_RP_ATS_BASE + 0x000c + ((r) & 0x1f) * 0x0020)
-#define ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR0(r) \
- (PCIE_RC_RP_ATS_BASE + 0x0018 + ((r) & 0x1f) * 0x0020)
-#define ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR1(r) \
- (PCIE_RC_RP_ATS_BASE + 0x001c + ((r) & 0x1f) * 0x0020)
+ (PCIE_RC_EP_ATR_OB_REGIONS_1_32 + 0x0008 + ((r) & 0x1f) * 0x0020)
+#define ROCKCHIP_PCIE_AT_OB_REGION_DESC1(r) \
+ (PCIE_RC_EP_ATR_OB_REGIONS_1_32 + 0x000c + ((r) & 0x1f) * 0x0020)
+#define ROCKCHIP_PCIE_AT_OB_REGION_DESC2(r) \
+ (PCIE_RC_EP_ATR_OB_REGIONS_1_32 + 0x0010 + ((r) & 0x1f) * 0x0020)
#define ROCKCHIP_PCIE_CORE_EP_FUNC_BAR_CFG0(fn) \
(PCIE_CORE_CTRL_MGMT_BASE + 0x0240 + (fn) * 0x0008)
--
2.25.1
The RK3399 PCIe endpoint controller cannot generate MSI-X IRQs.
This is documented in the RK3399 technical reference manual (TRM)
section 17.5.9 "Interrupt Support".
MSI-X capability should therefore not be advertised. Remove the
MSI-X capability by editing the capability linked-list. The
previous entry is the MSI capability, therefore get the next
entry from the MSI-X capability entry and set it as next entry
for the MSI capability. This in effect removes MSI-X from the list.
Linked list before : MSI cap -> MSI-X cap -> PCIe Device cap -> ...
Linked list now : MSI cap -> PCIe Device cap -> ...
Tested-by: Damien Le Moal <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Cc: [email protected]
Signed-off-by: Rick Wertenbroek <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 24 +++++++++++++++++++++++
drivers/pci/controller/pcie-rockchip.h | 5 +++++
2 files changed, 29 insertions(+)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 63fbb379638b..edfced311a9f 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -507,6 +507,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
size_t max_regions;
struct pci_epc_mem_window *windows = NULL;
int err, i;
+ u32 cfg_msi, cfg_msix_cp;
ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
if (!ep)
@@ -582,6 +583,29 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
ep->irq_pci_addr = ROCKCHIP_PCIE_EP_DUMMY_IRQ_ADDR;
+ /*
+ * MSI-X is not supported but the controller still advertises the MSI-X
+ * capability by default, which can lead to the Root Complex side
+ * allocating MSI-X vectors which cannot be used. Avoid this by skipping
+ * the MSI-X capability entry in the PCIe capabilities linked-list: get
+ * the next pointer from the MSI-X entry and set that in the MSI
+ * capability entry (which is the previous entry). This way the MSI-X
+ * entry is skipped (left out of the linked-list) and not advertised.
+ */
+ cfg_msi = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_BASE +
+ ROCKCHIP_PCIE_EP_MSI_CTRL_REG);
+
+ cfg_msi &= ~ROCKCHIP_PCIE_EP_MSI_CP1_MASK;
+
+ cfg_msix_cp = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_BASE +
+ ROCKCHIP_PCIE_EP_MSIX_CAP_REG) &
+ ROCKCHIP_PCIE_EP_MSIX_CAP_CP_MASK;
+
+ cfg_msi |= cfg_msix_cp;
+
+ rockchip_pcie_write(rockchip, cfg_msi,
+ PCIE_EP_CONFIG_BASE + ROCKCHIP_PCIE_EP_MSI_CTRL_REG);
+
rockchip_pcie_write(rockchip, PCIE_CLIENT_CONF_ENABLE,
PCIE_CLIENT_CONFIG);
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 501d859420b4..fe0333778fd9 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -227,6 +227,8 @@
#define ROCKCHIP_PCIE_EP_CMD_STATUS 0x4
#define ROCKCHIP_PCIE_EP_CMD_STATUS_IS BIT(19)
#define ROCKCHIP_PCIE_EP_MSI_CTRL_REG 0x90
+#define ROCKCHIP_PCIE_EP_MSI_CP1_OFFSET 8
+#define ROCKCHIP_PCIE_EP_MSI_CP1_MASK GENMASK(15, 8)
#define ROCKCHIP_PCIE_EP_MSI_FLAGS_OFFSET 16
#define ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_OFFSET 17
#define ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_MASK GENMASK(19, 17)
@@ -234,6 +236,9 @@
#define ROCKCHIP_PCIE_EP_MSI_CTRL_MME_MASK GENMASK(22, 20)
#define ROCKCHIP_PCIE_EP_MSI_CTRL_ME BIT(16)
#define ROCKCHIP_PCIE_EP_MSI_CTRL_MASK_MSI_CAP BIT(24)
+#define ROCKCHIP_PCIE_EP_MSIX_CAP_REG 0xb0
+#define ROCKCHIP_PCIE_EP_MSIX_CAP_CP_OFFSET 8
+#define ROCKCHIP_PCIE_EP_MSIX_CAP_CP_MASK GENMASK(15, 8)
#define ROCKCHIP_PCIE_EP_DUMMY_IRQ_ADDR 0x1
#define ROCKCHIP_PCIE_EP_PCI_LEGACY_IRQ_ADDR 0x3
#define ROCKCHIP_PCIE_EP_FUNC_BASE(fn) \
--
2.25.1
From: Damien Le Moal <[email protected]>
The address translation unit of the rockchip EP controller does not use
the lower 8 bits of a PCIe-space address to map local memory. Thus we
must set the align feature field to 256 to let the user know about this
constraint.
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Cc: [email protected]
Signed-off-by: Damien Le Moal <[email protected]>
Signed-off-by: Rick Wertenbroek <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index edfced311a9f..0af0e965fb57 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -442,6 +442,7 @@ static const struct pci_epc_features rockchip_pcie_epc_features = {
.linkup_notifier = false,
.msi_capable = true,
.msix_capable = false,
+ .align = 256,
};
static const struct pci_epc_features*
--
2.25.1
On 18/04/2023 09:46, Rick Wertenbroek wrote:
> Update the example in the documentation to a valid example.
> Address for mem-base was invalid, it pointed to address
> 0x8000'0000 which is the upper region of the DDR which
> is not necessarily populated depending on the board.
> This address should point to the base of the memory
> window region of the controller which is 0xfa00'0000.
> Add missing pinctrl.
>
> Signed-off-by: Rick Wertenbroek <[email protected]>
> ---
> .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> index 88386a6d7011..6b62f6f58efe 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> @@ -47,7 +47,7 @@ examples:
>
> pcie-ep@f8000000 {
> compatible = "rockchip,rk3399-pcie-ep";
> - reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0x80000000 0x0 0x20000>;
> + reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
> reg-names = "apb-base", "mem-base";
> clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
> <&cru PCLK_PCIE>, <&cru SCLK_PCIE_PM>;
> @@ -63,6 +63,8 @@ examples:
> phys = <&pcie_phy 0>, <&pcie_phy 1>, <&pcie_phy 2>, <&pcie_phy 3>;
> phy-names = "pcie-phy-0", "pcie-phy-1", "pcie-phy-2", "pcie-phy-3";
> rockchip,max-outbound-regions = <16>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie_clkreqnb_cpm>;
This is just example of the binding, you do not need to fill all
unrelated (generic) properties like pinctrl.
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Wed, Apr 19, 2023 at 10:01:25PM +0200, Krzysztof Kozlowski wrote:
> On 18/04/2023 09:46, Rick Wertenbroek wrote:
> > Update the example in the documentation to a valid example.
> > Address for mem-base was invalid, it pointed to address
> > 0x8000'0000 which is the upper region of the DDR which
> > is not necessarily populated depending on the board.
> > This address should point to the base of the memory
> > window region of the controller which is 0xfa00'0000.
> > Add missing pinctrl.
> >
> > Signed-off-by: Rick Wertenbroek <[email protected]>
> > ---
> > .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> > index 88386a6d7011..6b62f6f58efe 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> > +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> > @@ -47,7 +47,7 @@ examples:
> >
> > pcie-ep@f8000000 {
> > compatible = "rockchip,rk3399-pcie-ep";
> > - reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0x80000000 0x0 0x20000>;
> > + reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
> > reg-names = "apb-base", "mem-base";
> > clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
> > <&cru PCLK_PCIE>, <&cru SCLK_PCIE_PM>;
> > @@ -63,6 +63,8 @@ examples:
> > phys = <&pcie_phy 0>, <&pcie_phy 1>, <&pcie_phy 2>, <&pcie_phy 3>;
> > phy-names = "pcie-phy-0", "pcie-phy-1", "pcie-phy-2", "pcie-phy-3";
> > rockchip,max-outbound-regions = <16>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pcie_clkreqnb_cpm>;
>
> This is just example of the binding, you do not need to fill all
> unrelated (generic) properties like pinctrl.
Should I merge it as-is ?
Thanks,
Lorenzo
> Acked-by: Krzysztof Kozlowski <[email protected]>
>
> Best regards,
> Krzysztof
>
On 21/04/2023 11:26, Lorenzo Pieralisi wrote:
> On Wed, Apr 19, 2023 at 10:01:25PM +0200, Krzysztof Kozlowski wrote:
>> On 18/04/2023 09:46, Rick Wertenbroek wrote:
>>> Update the example in the documentation to a valid example.
>>> Address for mem-base was invalid, it pointed to address
>>> 0x8000'0000 which is the upper region of the DDR which
>>> is not necessarily populated depending on the board.
>>> This address should point to the base of the memory
>>> window region of the controller which is 0xfa00'0000.
>>> Add missing pinctrl.
>>>
>>> Signed-off-by: Rick Wertenbroek <[email protected]>
>>> ---
>>> .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>> index 88386a6d7011..6b62f6f58efe 100644
>>> --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>> @@ -47,7 +47,7 @@ examples:
>>>
>>> pcie-ep@f8000000 {
>>> compatible = "rockchip,rk3399-pcie-ep";
>>> - reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0x80000000 0x0 0x20000>;
>>> + reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
>>> reg-names = "apb-base", "mem-base";
>>> clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
>>> <&cru PCLK_PCIE>, <&cru SCLK_PCIE_PM>;
>>> @@ -63,6 +63,8 @@ examples:
>>> phys = <&pcie_phy 0>, <&pcie_phy 1>, <&pcie_phy 2>, <&pcie_phy 3>;
>>> phy-names = "pcie-phy-0", "pcie-phy-1", "pcie-phy-2", "pcie-phy-3";
>>> rockchip,max-outbound-regions = <16>;
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&pcie_clkreqnb_cpm>;
>>
>> This is just example of the binding, you do not need to fill all
>> unrelated (generic) properties like pinctrl.
>
> Should I merge it as-is ?
Yeah, go ahead. That was the note for the future that generic properties
are not always needed or even helpful in the example.
Best regards,
Krzysztof
On Tue, 18 Apr 2023 09:46:47 +0200, Rick Wertenbroek wrote:
> This is a series of patches that fixes the PCIe endpoint controller driver
> for the Rockchip RK3399 SoC. The driver was introduced in commit
> cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
> The original driver had issues and would not allow for the RK3399 to
> operate in PCIe endpoint mode correctly. This patch series fixes that so
> that the PCIe core controller of the RK3399 SoC can now act as a PCIe
> endpoint. This is v5 of the patch series and addresses the comments received
> during the review of the v4 [1]. The changes to the v4 are minor and none of
> them change the logic of the driver.
>
> [...]
Applied to controller/rockchip, thanks!
[01/11] PCI: rockchip: Remove writes to unused registers
https://git.kernel.org/pci/pci/c/92a9c57c325d
[02/11] PCI: rockchip: Write PCI Device ID to correct register
https://git.kernel.org/pci/pci/c/1f1c42ece18d
[03/11] PCI: rockchip: Assert PCI Configuration Enable bit after probe
https://git.kernel.org/pci/pci/c/f397fd4ac1fa
[04/11] PCI: rockchip: Add poll and timeout to wait for PHY PLLs to be locked
https://git.kernel.org/pci/pci/c/9dd3c7c4c8c3
[06/11] dt-bindings: PCI: Update the RK3399 example to a valid one
https://git.kernel.org/pci/pci/c/21a2960d5ea2
[07/11] PCI: rockchip: Fix legacy IRQ generation for RK3399 PCIe endpoint core
https://git.kernel.org/pci/pci/c/166e89d99dd8
[08/11] PCI: rockchip: Fix window mapping and address translation for endpoint
https://git.kernel.org/pci/pci/c/dc73ed0f1b8b
[09/11] PCI: rockchip: Use u32 variable to access 32-bit registers
https://git.kernel.org/pci/pci/c/8962b2cb3911
[10/11] PCI: rockchip: Don't advertise MSI-X in PCIe capabilities
https://git.kernel.org/pci/pci/c/a52587e0bee1
[11/11] PCI: rockchip: Set address alignment for endpoint mode
https://git.kernel.org/pci/pci/c/7e6689b34a81
Thanks,
Lorenzo
On Tue, 18 Apr 2023 09:46:47 +0200, Rick Wertenbroek wrote:
> This is a series of patches that fixes the PCIe endpoint controller driver
> for the Rockchip RK3399 SoC. The driver was introduced in commit
> cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
> The original driver had issues and would not allow for the RK3399 to
> operate in PCIe endpoint mode correctly. This patch series fixes that so
> that the PCIe core controller of the RK3399 SoC can now act as a PCIe
> endpoint. This is v5 of the patch series and addresses the comments received
> during the review of the v4 [1]. The changes to the v4 are minor and none of
> them change the logic of the driver.
>
> [...]
Applied, thanks!
[05/11] arm64: dts: rockchip: Add dtsi entry for RK3399 PCIe endpoint core
commit: 9755a52d65350233f74e234b1cf2804bd5a1839e
Best regards,
--
Heiko Stuebner <[email protected]>