linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver
@ 2023-01-26 13:50 Rick Wertenbroek
  2023-01-26 13:50 ` [PATCH 1/8] PCI: rockchip: Removed writes to unused registers Rick Wertenbroek
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 13:50 UTC (permalink / raw)
  To: alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rick Wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Rodrigo Vivi, Mikko Kovanen,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

This is a series of patches that fixes the PCIe endpoint controller driver
for the Rockchip RK3399 SoC. It is based on Linux kernel 6.0.19

The original driver in mainline had issues and would not allow for the
RK3399 to operate in PCIe endpoint mode. This patch series fixes that so
that the PCIe core controller of the RK3399 SoC can now act as a PCIe
endpoint.

This patch series has been tested on kernel 6.0.19 (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 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 /sur/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

Summary of changes :

This patch series is composed of 8 patches that do the following :
* Removed 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.
* Fixed setup to the PCI Device ID (DID), this was written to a read
  only register and therefore would not update the DID.
* Fixed setup of the PCIe endpoint controller 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.
* Added a poll with timeout to check the PHY PLL lock status, 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.
* Added a dtsi entry for the PCIe endpoint controller. 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.
* Fixed the window translation between CPU space and PCI space.
  Allows up to 32 memory windows, with (1MB) page allocation and mapping.
* Fixed the legacy IRQ (INTx) generation of the PCIe core in
  endpoint mode.
* Fixed the generation of message signalled interrupts (MSI) of the
  PCIe core in endpoint mode.

Thank you in advance for reviewing these changes 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.

I have tested and confirmed all basic functionality required for the
endpoint with the test driver and tools. With the previous state of
the driver 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

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.

Rick Wertenbroek (8):
  PCI: rockchip: Removed writes to unused registers
  PCI: rockchip: Fixed setup of Device ID
  PCI: rockchip: Fixed endpoint controller Configuration Request Retry
    Status
  PCI: rockchip: Added poll and timeout to wait for PHY PLLs to be
    locked
  PCI: rockchip: Added dtsi entry for PCIe endpoint controller
  PCI: rockchip: Fixed window mapping and address translation for
    endpoint
  PCI: rockchip: Fixed legacy IRQ generation for endpoint
  PCI: rockchip: Fixed MSI generation from PCIe endpoint core

 arch/arm64/boot/dts/rockchip/rk3399.dtsi  |  25 ++++
 drivers/pci/controller/pcie-rockchip-ep.c | 149 +++++++++++-----------
 drivers/pci/controller/pcie-rockchip.c    |  16 +++
 drivers/pci/controller/pcie-rockchip.h    |  36 ++++--
 4 files changed, 137 insertions(+), 89 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/8] PCI: rockchip: Removed writes to unused registers
  2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
@ 2023-01-26 13:50 ` Rick Wertenbroek
  2023-01-26 13:50 ` [PATCH 2/8] PCI: rockchip: Fixed setup of Device ID Rick Wertenbroek
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 13:50 UTC (permalink / raw)
  To: alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rick Wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Rodrigo Vivi, Greg Kroah-Hartman, Mikko Kovanen,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

Removed write accesses to registers that are marked "unused" in the
technical reference manual (TRM) (see RK3399 TRM 17.6.8.1)

Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
 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 d1a200b93..d5c477020 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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/8] PCI: rockchip: Fixed setup of Device ID
  2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
  2023-01-26 13:50 ` [PATCH 1/8] PCI: rockchip: Removed writes to unused registers Rick Wertenbroek
@ 2023-01-26 13:50 ` Rick Wertenbroek
  2023-01-26 13:50 ` [PATCH 3/8] PCI: rockchip: Fixed endpoint controller Configuration Request Retry Status Rick Wertenbroek
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 13:50 UTC (permalink / raw)
  To: alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rick Wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Rodrigo Vivi, Mikko Kovanen, Greg Kroah-Hartman,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

Device ID was written to a read-only register and therefore did not work.
The Device ID is now updated through the correct register.
This is documented in the RK3399 TRM section 17.6.6.1.1

Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
 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 d5c477020..9b835377b 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 32c3a859c..51a123e5c 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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/8] PCI: rockchip: Fixed endpoint controller Configuration Request Retry Status
  2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
  2023-01-26 13:50 ` [PATCH 1/8] PCI: rockchip: Removed writes to unused registers Rick Wertenbroek
  2023-01-26 13:50 ` [PATCH 2/8] PCI: rockchip: Fixed setup of Device ID Rick Wertenbroek
@ 2023-01-26 13:50 ` Rick Wertenbroek
  2023-01-26 13:50 ` [PATCH 4/8] PCI: rockchip: Added poll and timeout to wait for PHY PLLs to be locked Rick Wertenbroek
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 13:50 UTC (permalink / raw)
  To: alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rick Wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Mikko Kovanen, Rodrigo Vivi,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

The endpoint was left in the config mode after probe, this would mean the
core would send configuration requestion retry status (CRS) messages back
to the root complex. The conf_en bit should be asserted after probe. This
is documented in section 17.5.8.1.2 of the RK3399 TRM.

Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-ep.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 9b835377b..4c84e403e 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -623,6 +623,8 @@ 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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/8] PCI: rockchip: Added poll and timeout to wait for PHY PLLs to be locked
  2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
                   ` (2 preceding siblings ...)
  2023-01-26 13:50 ` [PATCH 3/8] PCI: rockchip: Fixed endpoint controller Configuration Request Retry Status Rick Wertenbroek
@ 2023-01-26 13:50 ` Rick Wertenbroek
  2023-01-26 14:42   ` Bjorn Helgaas
  2023-01-26 13:50 ` [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller Rick Wertenbroek
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 13:50 UTC (permalink / raw)
  To: alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rick Wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Rodrigo Vivi, Mikko Kovanen, Greg Kroah-Hartman,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

The Rockchip PCIe controller did not wait until the PHY PLLs were locked.
This could cause hangs. Now the PHY PLLs status is checked through a side
channel bit with a poll and timeout. If the PHY PLLs cannot lock an error
is generated. This is documented in the TRM section 17.5.8.1 PCIe
Initalization Sequence.

Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
 drivers/pci/controller/pcie-rockchip.c | 16 ++++++++++++++++
 drivers/pci/controller/pcie-rockchip.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
index 990a00e08..5f2e2dd5d 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,15 @@ 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 51a123e5c..f3a5ff1cf 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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller
  2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
                   ` (3 preceding siblings ...)
  2023-01-26 13:50 ` [PATCH 4/8] PCI: rockchip: Added poll and timeout to wait for PHY PLLs to be locked Rick Wertenbroek
@ 2023-01-26 13:50 ` Rick Wertenbroek
  2023-01-26 15:23   ` Krzysztof Kozlowski
                     ` (2 more replies)
  2023-01-26 13:50 ` [PATCH 6/8] PCI: rockchip: Fixed window mapping and address translation for endpoint Rick Wertenbroek
                   ` (3 subsequent siblings)
  8 siblings, 3 replies; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 13:50 UTC (permalink / raw)
  To: alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rick Wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Mikko Kovanen, Rodrigo Vivi,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

Added missing PCIe endpoint controller entry in the device tree. This
entry is documented in :
Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
The status is disabled by default, so it will not be loaded unless
explicitly chosen to.

Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 9d5b0e8c9..5f7251118 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -265,6 +265,31 @@ pcie0_intc: interrupt-controller {
 		};
 	};
 
+	pcie0_ep: pcie-ep@f8000000 {
+		compatible = "rockchip,rk3399-pcie-ep";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		rockchip,max-outbound-regions = <32>;
+		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>;
+		reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
+		reg-names = "apb-base", "mem-base";
+		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";
+		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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 6/8] PCI: rockchip: Fixed window mapping and address translation for endpoint
  2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
                   ` (4 preceding siblings ...)
  2023-01-26 13:50 ` [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller Rick Wertenbroek
@ 2023-01-26 13:50 ` Rick Wertenbroek
  2023-01-26 13:50 ` [PATCH 7/8] PCI: rockchip: Fixed legacy IRQ generation " Rick Wertenbroek
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 13:50 UTC (permalink / raw)
  To: alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rick Wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Rodrigo Vivi, Mikko Kovanen, Greg Kroah-Hartman,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

The RK3399 PCI EP core has 33 windows for PCIe space, here in the driver
up to 32 fixed size (1M) windows are used and pages are allocated /
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 regions 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.

Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-ep.c | 62 ++++++++++++-----------
 drivers/pci/controller/pcie-rockchip.h    | 25 ++++-----
 2 files changed, 46 insertions(+), 41 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 4c84e403e..a682a941d 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -76,11 +76,17 @@ static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
 	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);
+	if (is_nor_msg) {
+		dev_warn(rockchip->dev, "NOR MSG\n");
+		cpu_addr -= rockchip->mem_res->start;
+		addr0 = (0x10 & 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);
+	} else {
+		addr0 = (num_pass_bits & 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) | type;
 	desc1 = 0;
 
@@ -103,12 +109,6 @@ static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
 				    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);
 	}
 }
 
@@ -256,15 +256,7 @@ static int rockchip_pcie_ep_map_addr(struct pci_epc *epc, u8 fn, u8 vfn,
 	struct rockchip_pcie *pcie = &ep->rockchip;
 	u32 r;
 
-	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;
-	}
+	r = (addr >> ilog2(SZ_1M)) & 0x1f;
 
 	rockchip_pcie_prog_ep_ob_atu(pcie, fn, r, AXI_WRAPPER_MEM_WRITE, addr,
 				     pci_addr, size);
@@ -282,15 +274,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);
@@ -539,6 +527,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)
@@ -559,7 +549,10 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
 	struct rockchip_pcie *rockchip;
 	struct pci_epc *epc;
 	size_t max_regions;
+	struct pci_epc_mem_window *windows = NULL;
 	int err;
+	u32 cfg;
+	int i;
 
 	ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
 	if (!ep)
@@ -606,15 +599,26 @@ 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 f3a5ff1cf..72e427a0f 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -134,6 +134,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)
@@ -228,13 +229,14 @@
 #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_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) & \
@@ -242,20 +244,19 @@
 #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)
+		(PCIE_RC_EP_ATR_OB_REGIONS_1_32 + 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 + 0x000c + ((r) & 0x1f) * 0x0020)
 
 #define ROCKCHIP_PCIE_CORE_EP_FUNC_BAR_CFG0(fn) \
 		(PCIE_CORE_CTRL_MGMT_BASE + 0x0240 + (fn) * 0x0008)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 7/8] PCI: rockchip: Fixed legacy IRQ generation for endpoint
  2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
                   ` (5 preceding siblings ...)
  2023-01-26 13:50 ` [PATCH 6/8] PCI: rockchip: Fixed window mapping and address translation for endpoint Rick Wertenbroek
@ 2023-01-26 13:50 ` Rick Wertenbroek
  2023-01-26 15:25   ` Krzysztof Kozlowski
  2023-01-28  9:19   ` kernel test robot
  2023-01-26 13:50 ` [PATCH 8/8] PCI: rockchip: Fixed MSI generation from PCIe endpoint core Rick Wertenbroek
  2023-01-26 14:52 ` [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Bjorn Helgaas
  8 siblings, 2 replies; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 13:50 UTC (permalink / raw)
  To: alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rick Wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Rodrigo Vivi, Greg Kroah-Hartman, Mikko Kovanen,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

Added generation of legacy IRQ (INTx) for the RK3399 SoC PCIe EP core.
The generation of the legacy interrupt was validated with the PCIe EP
test driver. Generation of IRQ through the core is documented in the
TRM and is done through the PCIE_CLIENT_LEGACY_INT_CTRL register of
the core.

Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-ep.c | 32 ++++++-----------------
 drivers/pci/controller/pcie-rockchip.h    |  6 +++++
 2 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index a682a941d..a58c9d56b 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -333,15 +333,6 @@ static void rockchip_pcie_ep_assert_intx(struct rockchip_pcie_ep *ep, u8 fn,
 	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) {
 		ep->irq_pending |= BIT(intx);
@@ -351,22 +342,15 @@ static void rockchip_pcie_ep_assert_intx(struct rockchip_pcie_ep *ep, u8 fn,
 		msg_code = ROCKCHIP_PCIE_MSG_CODE_DEASSERT_INTA + intx;
 	}
 
-	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);
+	if (is_asserted) {
+		rockchip_pcie_write(&ep->rockchip,
+			PCIE_CLIENT_INT_IN_ASSERT | PCIE_CLIENT_INT_PEND_ST_PEND,
+			PCIE_CLIENT_LEGACY_INT_CTRL);
+	} else {
+		rockchip_pcie_write(&ep->rockchip,
+			PCIE_CLIENT_INT_IN_DEASSERT | PCIE_CLIENT_INT_PEND_ST_NORMAL,
+			PCIE_CLIENT_LEGACY_INT_CTRL);
 	}
-
-	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 72e427a0f..e90c2a2b8 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -39,6 +39,12 @@
 #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_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)
 #define   PCIE_CLIENT_DEBUG_LTSSM_MASK		GENMASK(5, 0)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 8/8] PCI: rockchip: Fixed MSI generation from PCIe endpoint core
  2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
                   ` (6 preceding siblings ...)
  2023-01-26 13:50 ` [PATCH 7/8] PCI: rockchip: Fixed legacy IRQ generation " Rick Wertenbroek
@ 2023-01-26 13:50 ` Rick Wertenbroek
  2023-01-26 15:26   ` Krzysztof Kozlowski
  2023-01-26 14:52 ` [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Bjorn Helgaas
  8 siblings, 1 reply; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 13:50 UTC (permalink / raw)
  To: alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rick Wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Rodrigo Vivi, Mikko Kovanen,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

The generation of MSI interrupts from the RK3399 PCIe endpoint core was
broken. The main issue came from u16 variables to be used to access u32
registers, this would lead to shifts of more than 16 bits moving data
out of the variable etc. Address translation for sending the MSI messages
over PCIe has also been fixed.

Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-ep.c | 37 +++++++++++++++--------
 drivers/pci/controller/pcie-rockchip.h    |  1 +
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index a58c9d56b..b26f16bed 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -288,19 +288,31 @@ static void rockchip_pcie_ep_unmap_addr(struct pci_epc *epc, u8 fn, u8 vfn,
 }
 
 static int rockchip_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 vfn,
-				    u8 multi_msg_cap)
+				    u8 mmc)
 {
 	struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
 	struct rockchip_pcie *rockchip = &ep->rockchip;
-	u16 flags;
+	u32 flags;
+	u32 multi_msg_cap;
+
+	if (fn) {
+		dev_err(&epc->dev, "This endpoint controller only supports a single physical function\n");
+		return -EINVAL;
+	}
+
+	if (mmc > 0x5) {
+		dev_err(&epc->dev, "Number of MSI IRQs cannot be more than 32\n");
+		return -EINVAL;
+	}
 
 	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;
+	multi_msg_cap = mmc;
 	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) +
@@ -312,7 +324,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) +
@@ -380,9 +392,10 @@ 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, pci_addr_mask = 0xff;
+	u32 r;
 
 	/* Check MSI enable bit */
 	flags = rockchip_pcie_read(&ep->rockchip,
@@ -416,16 +429,16 @@ 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) ||
 		     ep->irq_pci_fn != fn)) {
-		rockchip_pcie_prog_ep_ob_atu(rockchip, fn, ep->max_regions - 1,
-					     AXI_WRAPPER_MEM_WRITE,
-					     ep->irq_phys_addr,
-					     pci_addr & ~pci_addr_mask,
-					     pci_addr_mask + 1);
+		r = (ep->irq_phys_addr >> ilog2(SZ_1M)) & 0x1f;
+		rockchip_pcie_prog_ep_ob_atu(rockchip, fn, r,
+					AXI_WRAPPER_MEM_WRITE,
+					ep->irq_phys_addr,
+					pci_addr & ~pci_addr_mask,
+					pci_addr_mask + 1);
 		ep->irq_pci_addr = (pci_addr & ~pci_addr_mask);
 		ep->irq_pci_fn = fn;
 	}
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index e90c2a2b8..11dbf53cd 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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/8] PCI: rockchip: Added poll and timeout to wait for PHY PLLs to be locked
  2023-01-26 13:50 ` [PATCH 4/8] PCI: rockchip: Added poll and timeout to wait for PHY PLLs to be locked Rick Wertenbroek
@ 2023-01-26 14:42   ` Bjorn Helgaas
  0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2023-01-26 14:42 UTC (permalink / raw)
  To: Rick Wertenbroek
  Cc: alberto.dassatti, xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Rodrigo Vivi, Mikko Kovanen, Greg Kroah-Hartman,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

On Thu, Jan 26, 2023 at 02:50:44PM +0100, Rick Wertenbroek wrote:
> The Rockchip PCIe controller did not wait until the PHY PLLs were locked.
> This could cause hangs. Now the PHY PLLs status is checked through a side
> channel bit with a poll and timeout. If the PHY PLLs cannot lock an error
> is generated. This is documented in the TRM section 17.5.8.1 PCIe
> Initalization Sequence.

s/Initalization/Initialization/

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver
  2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
                   ` (7 preceding siblings ...)
  2023-01-26 13:50 ` [PATCH 8/8] PCI: rockchip: Fixed MSI generation from PCIe endpoint core Rick Wertenbroek
@ 2023-01-26 14:52 ` Bjorn Helgaas
  2023-01-26 15:23   ` Rick Wertenbroek
  8 siblings, 1 reply; 22+ messages in thread
From: Bjorn Helgaas @ 2023-01-26 14:52 UTC (permalink / raw)
  To: Rick Wertenbroek
  Cc: alberto.dassatti, xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Rodrigo Vivi, Mikko Kovanen,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

Hi Rick,

Thanks very much for your work.

On Thu, Jan 26, 2023 at 02:50:40PM +0100, Rick Wertenbroek wrote:
> This is a series of patches that fixes the PCIe endpoint controller driver
> for the Rockchip RK3399 SoC. It is based on Linux kernel 6.0.19
> 
> The original driver in mainline had issues and would not allow for the
> RK3399 to operate in PCIe endpoint mode. This patch series fixes that so
> that the PCIe core controller of the RK3399 SoC can now act as a PCIe
> endpoint.

So we merged cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip
PCIe controller") when it actually didn't work?  Ouch.  Thanks for
fixing it and testing it.

> Rick Wertenbroek (8):
>   PCI: rockchip: Removed writes to unused registers
>   PCI: rockchip: Fixed setup of Device ID
>   PCI: rockchip: Fixed endpoint controller Configuration Request Retry
>     Status
>   PCI: rockchip: Added poll and timeout to wait for PHY PLLs to be
>     locked
>   PCI: rockchip: Added dtsi entry for PCIe endpoint controller
>   PCI: rockchip: Fixed window mapping and address translation for
>     endpoint
>   PCI: rockchip: Fixed legacy IRQ generation for endpoint
>   PCI: rockchip: Fixed MSI generation from PCIe endpoint core

For the next iteration, can you please update these subject lines and
commit logs to:

  - Use imperative mood, i.e., read like a command, instead of a past
    tense description of what was done.  For example, say "Remove
    writes to unused registers" instead of "Removed writes ..."

  - Be more specific when possible.  "Fix" conveys no information
    about the actual code change.  For example, "Fixed endpoint
    controller Configuration Request Retry Status" gives a general
    idea, but it would be more useful if it said something about
    clearing config mode after probe.

  - Say what the patch does in the commit log.  The current ones often
    describe a *problem*, but do not explicitly say what the patch
    does.  The commit log should be complete in itself even without
    the subject line, so it usually contains a slightly expanded
    version of the subject line.

  - Split patches that do more than one logical thing.  The commit log
    for "Fixed MSI generation ..." talks about a u16/u32 shift issue,
    but the patch also adds an unrelated check for multi-function
    devices.

  - If a patch is a fix for an existing issue and may need to be
    backported, identify the commit that introduced the issue and add
    "Fixes: " lines.  This helps distros figure out whether and how
    far to backport patches.

  - Refer to the device consistently.  I see:
      RK3399 PCI EP core
      RK3399 SoC PCIe EP core
      RK3399 PCIe endpoint core
    I vote for "RK3399 PCIe Endpoint core".

Notes about imperative mood:
  https://chris.beams.io/posts/git-commit/
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.0#n94

>  arch/arm64/boot/dts/rockchip/rk3399.dtsi  |  25 ++++
>  drivers/pci/controller/pcie-rockchip-ep.c | 149 +++++++++++-----------
>  drivers/pci/controller/pcie-rockchip.c    |  16 +++
>  drivers/pci/controller/pcie-rockchip.h    |  36 ++++--
>  4 files changed, 137 insertions(+), 89 deletions(-)
> 
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller
  2023-01-26 13:50 ` [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller Rick Wertenbroek
@ 2023-01-26 15:23   ` Krzysztof Kozlowski
  2023-01-26 15:30     ` Rick Wertenbroek
  2023-01-27  8:42   ` ALOK TIWARI
  2023-01-30 15:04   ` Rob Herring
  2 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-26 15:23 UTC (permalink / raw)
  To: Rick Wertenbroek, alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Mikko Kovanen, Rodrigo Vivi,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

On 26/01/2023 14:50, Rick Wertenbroek wrote:
> Added missing PCIe endpoint controller entry in the device tree. This
> entry is documented in :
> Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt

There is no such file

> The status is disabled by default, so it will not be loaded unless
> explicitly chosen to.

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

> 
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 9d5b0e8c9..5f7251118 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -265,6 +265,31 @@ pcie0_intc: interrupt-controller {
>  		};
>  	};
>  
> +	pcie0_ep: pcie-ep@f8000000 {
> +		compatible = "rockchip,rk3399-pcie-ep";

reg is usually second property...
> +		#address-cells = <3>;
> +		#size-cells = <2>;
Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver
  2023-01-26 14:52 ` [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Bjorn Helgaas
@ 2023-01-26 15:23   ` Rick Wertenbroek
  2023-01-26 15:49     ` Bjorn Helgaas
  0 siblings, 1 reply; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 15:23 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: alberto.dassatti, xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Rodrigo Vivi, Mikko Kovanen,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

Hello, Bjorn
Thank you for your prompt reply and helpful comments.

Le jeu. 26 janv. 2023 à 15:52, Bjorn Helgaas <helgaas@kernel.org> a écrit :
>
> Hi Rick,
>
> Thanks very much for your work.
>
> On Thu, Jan 26, 2023 at 02:50:40PM +0100, Rick Wertenbroek wrote:
> > This is a series of patches that fixes the PCIe endpoint controller driver
> > for the Rockchip RK3399 SoC. It is based on Linux kernel 6.0.19
> >
> > The original driver in mainline had issues and would not allow for the
> > RK3399 to operate in PCIe endpoint mode. This patch series fixes that so
> > that the PCIe core controller of the RK3399 SoC can now act as a PCIe
> > endpoint.
>
> So we merged cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip
> PCIe controller") when it actually didn't work?  Ouch.  Thanks for
> fixing it and testing it.

It seems it wasn't fully tested, the code compiles and kernel module loads,
but further functionality didn't seem to have been tested
(e.g., lspci, and with the pcitest tool and pci_endpoit_test_driver).

>
> For the next iteration, can you please update these subject lines and
> commit logs to:

Thank you, I will prepare the changes and add them to the next iteration
with changes from other comments that may arise.

>
>   - Use imperative mood, i.e., read like a command, instead of a past
>     tense description of what was done.  For example, say "Remove
>     writes to unused registers" instead of "Removed writes ..."
>
>   - Be more specific when possible.  "Fix" conveys no information
>     about the actual code change.  For example, "Fixed endpoint
>     controller Configuration Request Retry Status" gives a general
>     idea, but it would be more useful if it said something about
>     clearing config mode after probe.
>
>   - Say what the patch does in the commit log.  The current ones often
>     describe a *problem*, but do not explicitly say what the patch
>     does.  The commit log should be complete in itself even without
>     the subject line, so it usually contains a slightly expanded
>     version of the subject line.
>
>   - Split patches that do more than one logical thing.  The commit log
>     for "Fixed MSI generation ..." talks about a u16/u32 shift issue,
>     but the patch also adds an unrelated check for multi-function
>     devices.

I will. I tried to split everything into the function it was related to, but I
now understand I should split even more so that the commit message
and changes are more tightly linked.

>
>   - If a patch is a fix for an existing issue and may need to be
>     backported, identify the commit that introduced the issue and add
>     "Fixes: " lines.  This helps distros figure out whether and how
>     far to backport patches.

Does this mean I should refer to the commit cf590b078391
("PCI: rockchip: Add EP driver for Rockchip PCIe controller") ?
Because it wasn't working in the first place ?

>
>   - Refer to the device consistently.  I see:
>       RK3399 PCI EP core
>       RK3399 SoC PCIe EP core
>       RK3399 PCIe endpoint core
>     I vote for "RK3399 PCIe Endpoint core".

I agree.

>
> Notes about imperative mood:
>   https://chris.beams.io/posts/git-commit/
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.0#n94

Thank you for all the pointers, I'll take them into account for the
next iteration. This is the first time I actually submitted a series of
patches to the LKML so it's all relatively new to me.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/8] PCI: rockchip: Fixed legacy IRQ generation for endpoint
  2023-01-26 13:50 ` [PATCH 7/8] PCI: rockchip: Fixed legacy IRQ generation " Rick Wertenbroek
@ 2023-01-26 15:25   ` Krzysztof Kozlowski
  2023-01-28  9:19   ` kernel test robot
  1 sibling, 0 replies; 22+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-26 15:25 UTC (permalink / raw)
  To: Rick Wertenbroek, alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Rodrigo Vivi, Greg Kroah-Hartman, Mikko Kovanen,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

On 26/01/2023 14:50, Rick Wertenbroek wrote:
> Added generation of legacy IRQ (INTx) for the RK3399 SoC PCIe EP core.

Here and in all other patches and subjects: Use imperative, not past tense.
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

"Fix legacy IRQ", not "Fixed legacy IRQ".

> The generation of the legacy interrupt was validated with the PCIe EP
> test driver. Generation of IRQ through the core is documented in the
> TRM and is done through the PCIE_CLIENT_LEGACY_INT_CTRL register of
> the core.
> 

If this is a fix, you need fixes tag and maybe cc-stable. Also the bug
should be described - it's effect, impact. Then the patch should be
first in the series (or even entirely separate).
Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 8/8] PCI: rockchip: Fixed MSI generation from PCIe endpoint core
  2023-01-26 13:50 ` [PATCH 8/8] PCI: rockchip: Fixed MSI generation from PCIe endpoint core Rick Wertenbroek
@ 2023-01-26 15:26   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 22+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-26 15:26 UTC (permalink / raw)
  To: Rick Wertenbroek, alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Rodrigo Vivi, Mikko Kovanen,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

On 26/01/2023 14:50, Rick Wertenbroek wrote:
> The generation of MSI interrupts from the RK3399 PCIe endpoint core was
> broken. The main issue came from u16 variables to be used to access u32
> registers, this would lead to shifts of more than 16 bits moving data
> out of the variable etc. Address translation for sending the MSI messages
> over PCIe has also been fixed.
> 
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>

Same comments as other fix. Missing Fixes tag, cc-stable, move to the
beginning of patchset.


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller
  2023-01-26 15:23   ` Krzysztof Kozlowski
@ 2023-01-26 15:30     ` Rick Wertenbroek
  2023-01-26 15:43       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-26 15:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: alberto.dassatti, xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Mikko Kovanen, Rodrigo Vivi,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

On Thu, Jan 26, 2023 at 4:23 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/01/2023 14:50, Rick Wertenbroek wrote:
> > Added missing PCIe endpoint controller entry in the device tree. This
> > entry is documented in :
> > Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
>
> There is no such file

Sorry but the file exists see :
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt?h=v6.0.19
It also exists in 6.1 :
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt?h=linux-6.1.y

>
> > The status is disabled by default, so it will not be loaded unless
> > explicitly chosen to.
>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
>

Sorry about this, you are right, I'll fix it for the next iteration.
Thank you

> >
> > Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > index 9d5b0e8c9..5f7251118 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > @@ -265,6 +265,31 @@ pcie0_intc: interrupt-controller {
> >               };
> >       };
> >
> > +     pcie0_ep: pcie-ep@f8000000 {
> > +             compatible = "rockchip,rk3399-pcie-ep";
>
> reg is usually second property...
> > +             #address-cells = <3>;
> > +             #size-cells = <2>;
> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller
  2023-01-26 15:30     ` Rick Wertenbroek
@ 2023-01-26 15:43       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 22+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-26 15:43 UTC (permalink / raw)
  To: Rick Wertenbroek
  Cc: alberto.dassatti, xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Mikko Kovanen, Rodrigo Vivi,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

On 26/01/2023 16:30, Rick Wertenbroek wrote:
> On Thu, Jan 26, 2023 at 4:23 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 26/01/2023 14:50, Rick Wertenbroek wrote:
>>> Added missing PCIe endpoint controller entry in the device tree. This
>>> entry is documented in :
>>> Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
>>
>> There is no such file
> 
> Sorry but the file exists see :
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt?h=v6.0.19
> It also exists in 6.1 :
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt?h=linux-6.1.y

It's some old tree. It could have existed in the past. But it does not
exist. Don't refer to non-existing paths.

Your code needs then rebasing if you refer to some old trees. Please
always work on latest maintainer's tree. Optionally on linux-next.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver
  2023-01-26 15:23   ` Rick Wertenbroek
@ 2023-01-26 15:49     ` Bjorn Helgaas
  0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2023-01-26 15:49 UTC (permalink / raw)
  To: Rick Wertenbroek
  Cc: alberto.dassatti, xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Rodrigo Vivi, Mikko Kovanen,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

On Thu, Jan 26, 2023 at 04:23:57PM +0100, Rick Wertenbroek wrote:
> Le jeu. 26 janv. 2023 à 15:52, Bjorn Helgaas <helgaas@kernel.org> a écrit :
> > Thanks very much for your work.
> >
> > On Thu, Jan 26, 2023 at 02:50:40PM +0100, Rick Wertenbroek wrote:
> > > This is a series of patches that fixes the PCIe endpoint controller driver
> > > for the Rockchip RK3399 SoC. It is based on Linux kernel 6.0.19
> > >
> > > The original driver in mainline had issues and would not allow for the
> > > RK3399 to operate in PCIe endpoint mode. This patch series fixes that so
> > > that the PCIe core controller of the RK3399 SoC can now act as a PCIe
> > > endpoint.
> >
> > So we merged cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip
> > PCIe controller") when it actually didn't work?  Ouch.  Thanks for
> > fixing it and testing it.
> 
> It seems it wasn't fully tested, the code compiles and kernel module loads,
> but further functionality didn't seem to have been tested
> (e.g., lspci, and with the pcitest tool and pci_endpoit_test_driver).

OK, I guess that happens sometimes.  Glad you're getting it into
shape!

> Does this mean I should refer to the commit cf590b078391
> ("PCI: rockchip: Add EP driver for Rockchip PCIe controller") ?
> Because it wasn't working in the first place ?

Yes, I think so.

> Thank you for all the pointers, I'll take them into account for the
> next iteration. This is the first time I actually submitted a series of
> patches to the LKML so it's all relatively new to me.

Welcome to Linux, and great start!

Bjorn

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller
  2023-01-26 13:50 ` [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller Rick Wertenbroek
  2023-01-26 15:23   ` Krzysztof Kozlowski
@ 2023-01-27  8:42   ` ALOK TIWARI
  2023-01-30 13:52     ` Rick Wertenbroek
  2023-01-30 15:04   ` Rob Herring
  2 siblings, 1 reply; 22+ messages in thread
From: ALOK TIWARI @ 2023-01-27  8:42 UTC (permalink / raw)
  To: Rick Wertenbroek, alberto.dassatti
  Cc: xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Mikko Kovanen, Rodrigo Vivi,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

   DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dtb
../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning 
(pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not 
configuration space
   DTC arch/arm64/boot/dts/amlogic/meson-gxm-s912-libretech-pc.dtb
../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning 
(pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not 
configuration space
   HDRINST usr/include/linux/aio_abi.h
   HDRINST usr/include/linux/am437x-vpfe.h
../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning 
(pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not 
configuration space
   DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm017-dc3.dtb


Thanks,

Alok

On 1/26/2023 7:20 PM, Rick Wertenbroek wrote:
> Added missing PCIe endpoint controller entry in the device tree. This
> entry is documented in :
> Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
> The status is disabled by default, so it will not be loaded unless
> explicitly chosen to.
>
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> ---
>   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 9d5b0e8c9..5f7251118 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -265,6 +265,31 @@ pcie0_intc: interrupt-controller {
>   		};
>   	};
>   
> +	pcie0_ep: pcie-ep@f8000000 {
> +		compatible = "rockchip,rk3399-pcie-ep";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		rockchip,max-outbound-regions = <32>;
> +		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>;
> +		reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
> +		reg-names = "apb-base", "mem-base";
> +		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";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pcie_clkreqnb_cpm>;
> +		status = "disabled";
> +	};
> +
>   	gmac: ethernet@fe300000 {
>   		compatible = "rockchip,rk3399-gmac";
>   		reg = <0x0 0xfe300000 0x0 0x10000>;

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/8] PCI: rockchip: Fixed legacy IRQ generation for endpoint
  2023-01-26 13:50 ` [PATCH 7/8] PCI: rockchip: Fixed legacy IRQ generation " Rick Wertenbroek
  2023-01-26 15:25   ` Krzysztof Kozlowski
@ 2023-01-28  9:19   ` kernel test robot
  1 sibling, 0 replies; 22+ messages in thread
From: kernel test robot @ 2023-01-28  9:19 UTC (permalink / raw)
  To: Rick Wertenbroek, alberto.dassatti
  Cc: oe-kbuild-all, xxm, wenrui.li, rick.wertenbroek,
	Rick Wertenbroek, Rob Herring, Krzysztof Kozlowski,
	Heiko Stuebner, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Bjorn Helgaas, Jani Nikula,
	Rodrigo Vivi, Greg Kroah-Hartman, Mikko Kovanen, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel, linux-pci

Hi Rick,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rockchip/for-next]
[also build test WARNING on linus/master v6.2-rc5 next-20230127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rick-Wertenbroek/PCI-rockchip-Removed-writes-to-unused-registers/20230128-155300
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
patch link:    https://lore.kernel.org/r/20230126135049.708524-8-rick.wertenbroek%40gmail.com
patch subject: [PATCH 7/8] PCI: rockchip: Fixed legacy IRQ generation for endpoint
config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230128/202301281758.YRZrsGZ9-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/53e861f3393ad4ebae5f2e133f4783b919036e19
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Rick-Wertenbroek/PCI-rockchip-Removed-writes-to-unused-registers/20230128-155300
        git checkout 53e861f3393ad4ebae5f2e133f4783b919036e19
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/pci/controller/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/pci/controller/pcie-rockchip-ep.c: In function 'rockchip_pcie_ep_assert_intx':
>> drivers/pci/controller/pcie-rockchip-ep.c:334:12: warning: variable 'msg_code' set but not used [-Wunused-but-set-variable]
     334 |         u8 msg_code;
         |            ^~~~~~~~
   drivers/pci/controller/pcie-rockchip-ep.c:333:13: warning: unused variable 'status' [-Wunused-variable]
     333 |         u32 status;
         |             ^~~~~~
   drivers/pci/controller/pcie-rockchip-ep.c:332:13: warning: unused variable 'offset' [-Wunused-variable]
     332 |         u32 offset;
         |             ^~~~~~
   drivers/pci/controller/pcie-rockchip-ep.c:331:13: warning: unused variable 'r' [-Wunused-variable]
     331 |         u32 r = ep->max_regions - 1;
         |             ^
   drivers/pci/controller/pcie-rockchip-ep.c:330:31: warning: unused variable 'rockchip' [-Wunused-variable]
     330 |         struct rockchip_pcie *rockchip = &ep->rockchip;
         |                               ^~~~~~~~
   drivers/pci/controller/pcie-rockchip-ep.c: In function 'rockchip_pcie_ep_probe':
   drivers/pci/controller/pcie-rockchip-ep.c:538:13: warning: unused variable 'cfg' [-Wunused-variable]
     538 |         u32 cfg;
         |             ^~~


vim +/msg_code +334 drivers/pci/controller/pcie-rockchip-ep.c

cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  326  
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  327  static void rockchip_pcie_ep_assert_intx(struct rockchip_pcie_ep *ep, u8 fn,
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  328  					 u8 intx, bool is_asserted)
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  329  {
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  330  	struct rockchip_pcie *rockchip = &ep->rockchip;
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  331  	u32 r = ep->max_regions - 1;
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  332  	u32 offset;
c577f4a5a08bb9 drivers/pci/controller/pcie-rockchip-ep.c Colin Ian King   2019-03-30  333  	u32 status;
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09 @334  	u8 msg_code;
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  335  
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  336  	intx &= 3;
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  337  	if (is_asserted) {
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  338  		ep->irq_pending |= BIT(intx);
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  339  		msg_code = ROCKCHIP_PCIE_MSG_CODE_ASSERT_INTA + intx;
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  340  	} else {
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  341  		ep->irq_pending &= ~BIT(intx);
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  342  		msg_code = ROCKCHIP_PCIE_MSG_CODE_DEASSERT_INTA + intx;
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  343  	}
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  344  
53e861f3393ad4 drivers/pci/controller/pcie-rockchip-ep.c Rick Wertenbroek 2023-01-26  345  	if (is_asserted) {
53e861f3393ad4 drivers/pci/controller/pcie-rockchip-ep.c Rick Wertenbroek 2023-01-26  346  		rockchip_pcie_write(&ep->rockchip,
53e861f3393ad4 drivers/pci/controller/pcie-rockchip-ep.c Rick Wertenbroek 2023-01-26  347  			PCIE_CLIENT_INT_IN_ASSERT | PCIE_CLIENT_INT_PEND_ST_PEND,
53e861f3393ad4 drivers/pci/controller/pcie-rockchip-ep.c Rick Wertenbroek 2023-01-26  348  			PCIE_CLIENT_LEGACY_INT_CTRL);
53e861f3393ad4 drivers/pci/controller/pcie-rockchip-ep.c Rick Wertenbroek 2023-01-26  349  	} else {
53e861f3393ad4 drivers/pci/controller/pcie-rockchip-ep.c Rick Wertenbroek 2023-01-26  350  		rockchip_pcie_write(&ep->rockchip,
53e861f3393ad4 drivers/pci/controller/pcie-rockchip-ep.c Rick Wertenbroek 2023-01-26  351  			PCIE_CLIENT_INT_IN_DEASSERT | PCIE_CLIENT_INT_PEND_ST_NORMAL,
53e861f3393ad4 drivers/pci/controller/pcie-rockchip-ep.c Rick Wertenbroek 2023-01-26  352  			PCIE_CLIENT_LEGACY_INT_CTRL);
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  353  	}
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  354  }
cf590b07839133 drivers/pci/host/pcie-rockchip-ep.c       Shawn Lin        2018-05-09  355  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller
  2023-01-27  8:42   ` ALOK TIWARI
@ 2023-01-30 13:52     ` Rick Wertenbroek
  0 siblings, 0 replies; 22+ messages in thread
From: Rick Wertenbroek @ 2023-01-30 13:52 UTC (permalink / raw)
  To: ALOK TIWARI
  Cc: alberto.dassatti, xxm, wenrui.li, rick.wertenbroek, Rob Herring,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Mikko Kovanen, Rodrigo Vivi,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

On Fri, Jan 27, 2023 at 9:43 AM ALOK TIWARI <alok.a.tiwari@oracle.com> wrote:
>
>    DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dtb
> ../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning
> (pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not
> configuration space
>    DTC arch/arm64/boot/dts/amlogic/meson-gxm-s912-libretech-pc.dtb
> ../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning
> (pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not
> configuration space
>    HDRINST usr/include/linux/aio_abi.h
>    HDRINST usr/include/linux/am437x-vpfe.h
> ../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning
> (pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not
> configuration space
>    DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm017-dc3.dtb
>
>
> Thanks,
>
> Alok

These warnings are for the pcie (host controller), I did not touch that entry.
Plus they are for another file (arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi)
They are related to "pci_device_reg" warnings (can be suppressed with
"-Wno-pci_device_reg").

Sorry, but this does not seem to be related to my change and I don't know
how to fix this.

Rick

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller
  2023-01-26 13:50 ` [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller Rick Wertenbroek
  2023-01-26 15:23   ` Krzysztof Kozlowski
  2023-01-27  8:42   ` ALOK TIWARI
@ 2023-01-30 15:04   ` Rob Herring
  2 siblings, 0 replies; 22+ messages in thread
From: Rob Herring @ 2023-01-30 15:04 UTC (permalink / raw)
  To: Rick Wertenbroek
  Cc: alberto.dassatti, xxm, wenrui.li, rick.wertenbroek,
	Krzysztof Kozlowski, Heiko Stuebner, Shawn Lin,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	Jani Nikula, Greg Kroah-Hartman, Mikko Kovanen, Rodrigo Vivi,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-pci

On Thu, Jan 26, 2023 at 7:52 AM Rick Wertenbroek
<rick.wertenbroek@gmail.com> wrote:
>
> Added missing PCIe endpoint controller entry in the device tree. This
> entry is documented in :
> Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
> The status is disabled by default, so it will not be loaded unless
> explicitly chosen to.
>
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 9d5b0e8c9..5f7251118 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -265,6 +265,31 @@ pcie0_intc: interrupt-controller {
>                 };
>         };
>
> +       pcie0_ep: pcie-ep@f8000000 {
> +               compatible = "rockchip,rk3399-pcie-ep";
> +               #address-cells = <3>;
> +               #size-cells = <2>;

These are only needed when you have child nodes. Additionally, it
would not be a PCI bus which is the only case that has 3 address
cells.

There's a schema for this in linux-next now. Please test this change
with that. It should point out the above issue and maybe others.

> +               rockchip,max-outbound-regions = <32>;
> +               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>;
> +               reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
> +               reg-names = "apb-base", "mem-base";
> +               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";
> +               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
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-01-30 15:04 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-26 13:50 [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Rick Wertenbroek
2023-01-26 13:50 ` [PATCH 1/8] PCI: rockchip: Removed writes to unused registers Rick Wertenbroek
2023-01-26 13:50 ` [PATCH 2/8] PCI: rockchip: Fixed setup of Device ID Rick Wertenbroek
2023-01-26 13:50 ` [PATCH 3/8] PCI: rockchip: Fixed endpoint controller Configuration Request Retry Status Rick Wertenbroek
2023-01-26 13:50 ` [PATCH 4/8] PCI: rockchip: Added poll and timeout to wait for PHY PLLs to be locked Rick Wertenbroek
2023-01-26 14:42   ` Bjorn Helgaas
2023-01-26 13:50 ` [PATCH 5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller Rick Wertenbroek
2023-01-26 15:23   ` Krzysztof Kozlowski
2023-01-26 15:30     ` Rick Wertenbroek
2023-01-26 15:43       ` Krzysztof Kozlowski
2023-01-27  8:42   ` ALOK TIWARI
2023-01-30 13:52     ` Rick Wertenbroek
2023-01-30 15:04   ` Rob Herring
2023-01-26 13:50 ` [PATCH 6/8] PCI: rockchip: Fixed window mapping and address translation for endpoint Rick Wertenbroek
2023-01-26 13:50 ` [PATCH 7/8] PCI: rockchip: Fixed legacy IRQ generation " Rick Wertenbroek
2023-01-26 15:25   ` Krzysztof Kozlowski
2023-01-28  9:19   ` kernel test robot
2023-01-26 13:50 ` [PATCH 8/8] PCI: rockchip: Fixed MSI generation from PCIe endpoint core Rick Wertenbroek
2023-01-26 15:26   ` Krzysztof Kozlowski
2023-01-26 14:52 ` [PATCH 0/8] PCI: rockchip: Fix PCIe endpoint controller driver Bjorn Helgaas
2023-01-26 15:23   ` Rick Wertenbroek
2023-01-26 15:49     ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).