[PATCH v2 0/9] rockchip: Fix PCIe and NVMe support on RK3568

[PATCH v2 0/9] rockchip: Fix PCIe and NVMe support on RK3568

[Jonas Karlman]

This series fixes and enables PCIe and NVMe support on RK3568.

Patch 1 adds a dev_read_addr_size_index_ptr function.
Patch 2-3 fixes main issue in the driver to be usable on RK3568.
Patch 4 fixes a long wait time during probe when no device is attached.
Patch 5 hides BARs of the root complex that could claim the entire
memory region during PCI autoconfig.
Patch 6 adds support for the gpios prop to the fixed regulators driver.
Patch 7 adds a missing clock to the clock driver.
Patch 8 enables PCIe and NVMe support on rk3568-rock-3a.
Patch 9 updates the device tree with new reg and ranges values.

For a clean apply of patch 8, the series at [1] may be needed.

I have tested that a Samsung 970 EVO NVMe is detected on a ROCK 3A,

  BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
  00.00.00   0x1d87     0x3566     Bridge device           0x04
  01.00.00   0x144d     0xa808     Mass storage controller 0x08

and I have also verified that the network controller continues to be
detected on a ROCK 5B.

  BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
  00.00.00   0x1d87     0x3588     Bridge device           0x04
  01.00.00   0x10ec     0x8125     Network controller      0x00

Changes in v2:
- Rebased on custodians/u-boot-rockchip master branch and the defconfig
  and spi v2 series at [1].
- Drop dependency on basic reference counting of gpio regulators series,
  pcie2x1 is disabled for rk3568-rock-3a to avoid a system freeze due to
  disable of a shared vpcie3v3-supply regulator while in use by pcie3x2.
- Add and use dev_read_addr_size_index_ptr function.
- Update commit messages.
- Collect r-b tags.

This series can also be found at [2].

[1] https://patchwork.ozlabs.org/project/uboot/cover/20230517182624.1765359-1-jonas@kwiboo.se/
[2] https://github.com/Kwiboo/u-boot-rockchip/commits/rk3568-pcie-v2

Jonas Karlman (9):
  core: read: add dev_read_addr_size_index_ptr function
  pci: pcie_dw_rockchip: Get config region from reg prop
  pci: pcie_dw_rockchip: Use regulator_set_enable_if_allowed
  pci: pcie_dw_rockchip: Speed up link probe
  pci: pcie_dw_rockchip: Hide BARs of the root complex
  regulator: fixed: Add support for gpios prop
  rockchip: clk: clk_rk3568: Add CLK_PCIEPHY2_REF support
  rockchip: rk3568-rock-3a: Enable PCIe and NVMe support
  rockchip: rk356x: Update PCIe config, IO and memory regions

 arch/arm/dts/rk3568-rock-3a-u-boot.dtsi |  16 ++++
 arch/arm/dts/rk3568.dtsi                |  14 +--
 arch/arm/dts/rk356x.dtsi                |   7 +-
 configs/rock-3a-rk3568_defconfig        |   4 +
 drivers/clk/rockchip/clk_rk3568.c       |   1 +
 drivers/core/read.c                     |  11 +++
 drivers/pci/pcie_dw_common.c            |  10 +-
 drivers/pci/pcie_dw_rockchip.c          | 120 +++++++++++++++---------
 drivers/power/regulator/fixed.c         |   5 +-
 include/dm/read.h                       |  21 +++++
 10 files changed, 153 insertions(+), 56 deletions(-)

-- 
2.40.1

[PATCH v2 1/9] core: read: add dev_read_addr_size_index_ptr function

[Jonas Karlman]

Add dev_read_addr_size_index_ptr function with the same functionality as
dev_read_addr_size_index, but instead a return pointer is given.
Use map_sysmem() function as cast for the return.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- New patch

 drivers/core/read.c | 11 +++++++++++
 include/dm/read.h   | 21 +++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/core/read.c b/drivers/core/read.c
index 0289a2edb6a4..d3c939530aa5 100644
--- a/drivers/core/read.c
+++ b/drivers/core/read.c
@@ -150,6 +150,17 @@ fdt_addr_t dev_read_addr_size_index(const struct udevice *dev, int index,
 		return devfdt_get_addr_size_index(dev, index, size);
 }
 
+void *dev_read_addr_size_index_ptr(const struct udevice *dev, int index,
+				   fdt_size_t *size)
+{
+	fdt_addr_t addr = dev_read_addr_size_index(dev, index, size);
+
+	if (addr == FDT_ADDR_T_NONE)
+		return NULL;
+
+	return map_sysmem(addr, 0);
+}
+
 void *dev_remap_addr_index(const struct udevice *dev, int index)
 {
 	fdt_addr_t addr = dev_read_addr_index(dev, index);
diff --git a/include/dm/read.h b/include/dm/read.h
index 56ac076c9f13..7dd43d61a665 100644
--- a/include/dm/read.h
+++ b/include/dm/read.h
@@ -246,6 +246,20 @@ void *dev_read_addr_index_ptr(const struct udevice *dev, int index);
 fdt_addr_t dev_read_addr_size_index(const struct udevice *dev, int index,
 				    fdt_size_t *size);
 
+/**
+ * dev_read_addr_size_index_ptr() - Get the indexed reg property of a device
+ *                                  as a pointer
+ *
+ * @dev: Device to read from
+ * @index: the 'reg' property can hold a list of <addr, size> pairs
+ *	   and @index is used to select which one is required
+ * @size: place to put size value (on success)
+ *
+ * Return: pointer or NULL if not found
+ */
+void *dev_read_addr_size_index_ptr(const struct udevice *dev, int index,
+				   fdt_size_t *size);
+
 /**
  * dev_remap_addr_index() - Get the indexed reg property of a device
  *                               as a memory-mapped I/O pointer
@@ -957,6 +971,13 @@ static inline fdt_addr_t dev_read_addr_size_index(const struct udevice *dev,
 	return devfdt_get_addr_size_index(dev, index, size);
 }
 
+static inline void *dev_read_addr_size_index_ptr(const struct udevice *dev,
+						 int index,
+						 fdt_size_t *size)
+{
+	return devfdt_get_addr_size_index_ptr(dev, index, size);
+}
+
 static inline fdt_addr_t dev_read_addr_name(const struct udevice *dev,
 					    const char *name)
 {
-- 
2.40.1

[PATCH v2 3/9] pci: pcie_dw_rockchip: Use regulator_set_enable_if_allowed

[Jonas Karlman]

The vpcie3v3 regulator is typically a fixed regulator controlled using
gpio. Change to use enable and disable calls on the regulator instead
of trying to set a voltage value.

Also remove the delay to match linux driver, for a fixed regulator the
startup-delay-us prop can be used in case a startup delay is needed.
Limited testing on ROCK 3A, ROCK 5B, Quartz64, Odroid-M1 has shown that
this delay was not needed.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- Update commit message

 drivers/pci/pcie_dw_rockchip.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index 83737e62bc6a..1b8a1409f6df 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -288,21 +288,16 @@ static int rockchip_pcie_init_port(struct udevice *dev)
 	struct rk_pcie *priv = dev_get_priv(dev);
 
 	/* Set power and maybe external ref clk input */
-	if (priv->vpcie3v3) {
-		ret = regulator_set_value(priv->vpcie3v3, 3300000);
-		if (ret) {
-			dev_err(priv->dw.dev, "failed to enable vpcie3v3 (ret=%d)\n",
-				ret);
-			return ret;
-		}
+	ret = regulator_set_enable_if_allowed(priv->vpcie3v3, true);
+	if (ret && ret != -ENOSYS) {
+		dev_err(dev, "failed to enable vpcie3v3 (ret=%d)\n", ret);
+		return ret;
 	}
 
-	udelay(MACRO_US * 1000);
-
 	ret = generic_phy_init(&priv->phy);
 	if (ret) {
 		dev_err(dev, "failed to init phy (ret=%d)\n", ret);
-		return ret;
+		goto err_disable_regulator;
 	}
 
 	ret = generic_phy_power_on(&priv->phy);
@@ -345,6 +340,8 @@ err_power_off_phy:
 	generic_phy_power_off(&priv->phy);
 err_exit_phy:
 	generic_phy_exit(&priv->phy);
+err_disable_regulator:
+	regulator_set_enable_if_allowed(priv->vpcie3v3, false);
 
 	return ret;
 }
-- 
2.40.1

[PATCH v2 2/9] pci: pcie_dw_rockchip: Get config region from reg prop

[Jonas Karlman]

Get the config region to use from the reg prop. Also update the
referenced region index used in comment.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
v2:
- Use dev_read_addr_size_index_ptr
- Collect r-b tag

 drivers/pci/pcie_dw_common.c   | 10 ++++++----
 drivers/pci/pcie_dw_rockchip.c |  7 +++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie_dw_common.c b/drivers/pci/pcie_dw_common.c
index 9f8b016d1149..74fb6df412c7 100644
--- a/drivers/pci/pcie_dw_common.c
+++ b/drivers/pci/pcie_dw_common.c
@@ -141,9 +141,9 @@ static uintptr_t set_cfg_address(struct pcie_dw *pcie,
 
 	/*
 	 * Not accessing root port configuration space?
-	 * Region #0 is used for Outbound CFG space access.
+	 * Region #1 is used for Outbound CFG space access.
 	 * Direction = Outbound
-	 * Region Index = 0
+	 * Region Index = 1
 	 */
 	d = PCI_MASK_BUS(d);
 	d = PCI_ADD_BUS(bus, d);
@@ -328,8 +328,10 @@ void pcie_dw_setup_host(struct pcie_dw *pci)
 			pci->prefetch.bus_start = hose->regions[ret].bus_start;  /* PREFETCH_bus_addr */
 			pci->prefetch.size = hose->regions[ret].size;	    /* PREFETCH size */
 		} else if (hose->regions[ret].flags == PCI_REGION_SYS_MEMORY) {
-			pci->cfg_base = (void *)(pci->io.phys_start - pci->io.size);
-			pci->cfg_size = pci->io.size;
+			if (!pci->cfg_base) {
+				pci->cfg_base = (void *)(pci->io.phys_start - pci->io.size);
+				pci->cfg_size = pci->io.size;
+			}
 		} else {
 			dev_err(pci->dev, "invalid flags type!\n");
 		}
diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index 6da618055cbe..83737e62bc6a 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -366,6 +366,13 @@ static int rockchip_pcie_parse_dt(struct udevice *dev)
 
 	dev_dbg(dev, "APB address is 0x%p\n", priv->apb_base);
 
+	priv->dw.cfg_base = dev_read_addr_size_index_ptr(dev, 2,
+							 &priv->dw.cfg_size);
+	if (!priv->dw.cfg_base)
+		return -EINVAL;
+
+	dev_dbg(dev, "CFG address is 0x%p\n", priv->dw.cfg_base);
+
 	ret = gpio_request_by_name(dev, "reset-gpios", 0,
 				   &priv->rst_gpio, GPIOD_IS_OUT);
 	if (ret) {
-- 
2.40.1

[PATCH v2 4/9] pci: pcie_dw_rockchip: Speed up link probe

[Jonas Karlman]

Use a similar pattern and delay values as the linux mainline driver to
speed up failing when nothing is connected.

Reduce fail speed from around 5+ seconds down to around one second on a
Radxa ROCK 3 Model A, where pcie2x1 is probed before pcie3x2 M2 slot.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- No change

 drivers/pci/pcie_dw_rockchip.c | 68 ++++++++++++++++++----------------
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index 1b8a1409f6df..82a8b9c96e2b 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -61,9 +61,6 @@ struct rk_pcie {
 #define PCIE_CLIENT_DBG_TRANSITION_DATA	0xffff0000
 #define PCIE_CLIENT_DBF_EN		0xffff0003
 
-/* Parameters for the waiting for #perst signal */
-#define MACRO_US			1000
-
 static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
 {
 	if ((uintptr_t)addr & (size - 1)) {
@@ -242,43 +239,46 @@ static int rk_pcie_link_up(struct rk_pcie *priv, u32 cap_speed)
 	/* DW pre link configurations */
 	rk_pcie_configure(priv, cap_speed);
 
-	/* Rest the device */
-	if (dm_gpio_is_valid(&priv->rst_gpio)) {
-		dm_gpio_set_value(&priv->rst_gpio, 0);
-		/*
-		 * Minimal is 100ms from spec but we see
-		 * some wired devices need much more, such as 600ms.
-		 * Add a enough delay to cover all cases.
-		 */
-		udelay(MACRO_US * 1000);
-		dm_gpio_set_value(&priv->rst_gpio, 1);
-	}
-
 	rk_pcie_disable_ltssm(priv);
 	rk_pcie_link_status_clear(priv);
 	rk_pcie_enable_debug(priv);
 
+	/* Reset the device */
+	if (dm_gpio_is_valid(&priv->rst_gpio))
+		dm_gpio_set_value(&priv->rst_gpio, 0);
+
 	/* Enable LTSSM */
 	rk_pcie_enable_ltssm(priv);
 
-	for (retries = 0; retries < 5; retries++) {
-		if (is_link_up(priv)) {
-			dev_info(priv->dw.dev, "PCIe Link up, LTSSM is 0x%x\n",
-				 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
-			rk_pcie_debug_dump(priv);
-			return 0;
-		}
-
-		dev_info(priv->dw.dev, "PCIe Linking... LTSSM is 0x%x\n",
-			 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
-		rk_pcie_debug_dump(priv);
-		udelay(MACRO_US * 1000);
+	/*
+	 * PCIe requires the refclk to be stable for 100ms prior to releasing
+	 * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI
+	 * Express Card Electromechanical Specification, 1.1. However, we don't
+	 * know if the refclk is coming from RC's PHY or external OSC. If it's
+	 * from RC, so enabling LTSSM is the just right place to release #PERST.
+	 */
+	mdelay(100);
+	if (dm_gpio_is_valid(&priv->rst_gpio))
+		dm_gpio_set_value(&priv->rst_gpio, 1);
+
+	/* Check if the link is up or not */
+	for (retries = 0; retries < 10; retries++) {
+		if (is_link_up(priv))
+			break;
+
+		mdelay(100);
+	}
+
+	if (retries >= 10) {
+		dev_err(priv->dw.dev, "PCIe-%d Link Fail\n",
+			dev_seq(priv->dw.dev));
+		return -EIO;
 	}
 
-	dev_err(priv->dw.dev, "PCIe-%d Link Fail\n", dev_seq(priv->dw.dev));
-	/* Link maybe in Gen switch recovery but we need to wait more 1s */
-	udelay(MACRO_US * 1000);
-	return -EIO;
+	dev_info(priv->dw.dev, "PCIe Link up, LTSSM is 0x%x\n",
+		 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
+	rk_pcie_debug_dump(priv);
+	return 0;
 }
 
 static int rockchip_pcie_init_port(struct udevice *dev)
@@ -287,6 +287,12 @@ static int rockchip_pcie_init_port(struct udevice *dev)
 	u32 val;
 	struct rk_pcie *priv = dev_get_priv(dev);
 
+	ret = reset_assert_bulk(&priv->rsts);
+	if (ret) {
+		dev_err(dev, "failed to assert resets (ret=%d)\n", ret);
+		return ret;
+	}
+
 	/* Set power and maybe external ref clk input */
 	ret = regulator_set_enable_if_allowed(priv->vpcie3v3, true);
 	if (ret && ret != -ENOSYS) {
-- 
2.40.1

[PATCH v2 5/9] pci: pcie_dw_rockchip: Hide BARs of the root complex

[Jonas Karlman]

PCI Autoconfig read the Root Complex BARs and try to claim the entire
1 GiB memory region on RK3568, leaving no space for any attached device.

With a memory region less than 1 GiB this was not a real issue:

  PCI Autoconfig: Bus Memory region: [0-3eefffff],
  PCI Autoconfig: Bus I/O region: [3ef00000-3effffff],
  PCI Autoconfig: Found P2P bridge, device 0
  PCI Autoconfig: BAR 0, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000
  PCI: Failed autoconfig bar 10
  PCI Autoconfig: BAR 1, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000
  PCI: Failed autoconfig bar 14
  PCI Autoconfig: ROM, size=0x10000, address=0x10000 bus_lower=0x20000

  PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x100000 bus_lower=0x104000

With a memory region of the entire 1 GiB this leads to:

  PCI Autoconfig: Bus Memory region: [40000000-7fffffff],
  PCI Autoconfig: Bus I/O region: [f0100000-f01fffff],
  PCI Autoconfig: Found P2P bridge, device 0
  PCI Autoconfig: BAR 0, Mem, size=0x40000000, address=0x40000000 bus_lower=0x80000000
  PCI Autoconfig: BAR 1, Mem, size=0x40000000, No room in resource, avail start=80000000 / size=40000000, need=40000000
  PCI: Failed autoconfig bar 14
  PCI Autoconfig: ROM, size=0x10000, No room in resource, avail start=80000000 / size=40000000, need=10000

  PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail start=80000000 / size=40000000, need=4000
  PCI: Failed autoconfig bar 10

After this change with a memory region of the entire 1 GiB:

  PCI Autoconfig: Bus Memory region: [40000000-7fffffff],
  PCI Autoconfig: Bus I/O region: [f0100000-f01fffff],
  PCI Autoconfig: Found P2P bridge, device 0
  PCI Autoconfig: ROM, size=0x10000, address=0x40000000 bus_lower=0x40010000

  PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x40100000 bus_lower=0x40104000

Return an invalid value during config read of Root Complex BARs during
autoconfig to work around such issue.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- Update commit message

 drivers/pci/pcie_dw_rockchip.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index 82a8b9c96e2b..f56773c2e58c 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -146,6 +146,32 @@ static inline void rk_pcie_writel_apb(struct rk_pcie *rk_pcie, u32 reg,
 	__rk_pcie_write_apb(rk_pcie, rk_pcie->apb_base, reg, 0x4, val);
 }
 
+/**
+ * The BARs of bridge should be hidden during enumeration to avoid
+ * allocation of the entire memory region by PCIe core on RK3568.
+ */
+static bool rk_pcie_hide_rc_bar(struct pcie_dw *pcie, pci_dev_t bdf,
+				uint offset)
+{
+	int bus = PCI_BUS(bdf) - pcie->first_busno;
+
+	return bus == 0 && PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+	       offset >= PCI_BASE_ADDRESS_0 && offset <= PCI_BASE_ADDRESS_1;
+}
+
+static int rk_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
+			       uint offset, ulong *valuep,
+			       enum pci_size_t size)
+{
+	struct pcie_dw *pcie = dev_get_priv(bus);
+	int ret = pcie_dw_read_config(bus, bdf, offset, valuep, size);
+
+	if (!ret && rk_pcie_hide_rc_bar(pcie, bdf, offset))
+		*valuep = pci_get_ff(size);
+
+	return ret;
+}
+
 /**
  * rk_pcie_configure() - Configure link capabilities and speed
  *
@@ -476,7 +502,7 @@ rockchip_pcie_probe_err_init_port:
 }
 
 static const struct dm_pci_ops rockchip_pcie_ops = {
-	.read_config	= pcie_dw_read_config,
+	.read_config	= rk_pcie_read_config,
 	.write_config	= pcie_dw_write_config,
 };
 
-- 
2.40.1

[PATCH v2 6/9] regulator: fixed: Add support for gpios prop

[Jonas Karlman]

The commit 12df2c182ccb ("regulator: dt-bindings: fixed-regulator: allow
gpios property") in linux v6.3-rc1 added support for use of either a
gpios or gpio prop with a fixed-regulator.

This adds support for the new gpios prop to the fixed-regulator driver.
gpios prop is used by vcc3v3-pcie-regulator on Radxa ROCK 3 Model A.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
v2:
- Update commit message
- Collect r-b tag

 drivers/power/regulator/fixed.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/power/regulator/fixed.c b/drivers/power/regulator/fixed.c
index 90004d1601a9..e921a5984ef7 100644
--- a/drivers/power/regulator/fixed.c
+++ b/drivers/power/regulator/fixed.c
@@ -25,6 +25,7 @@ static int fixed_regulator_of_to_plat(struct udevice *dev)
 {
 	struct dm_regulator_uclass_plat *uc_pdata;
 	struct regulator_common_plat *dev_pdata;
+	bool gpios;
 
 	dev_pdata = dev_get_plat(dev);
 	uc_pdata = dev_get_uclass_plat(dev);
@@ -33,7 +34,9 @@ static int fixed_regulator_of_to_plat(struct udevice *dev)
 
 	uc_pdata->type = REGULATOR_TYPE_FIXED;
 
-	return regulator_common_of_to_plat(dev, dev_pdata, "gpio");
+	gpios = dev_read_bool(dev, "gpios");
+	return regulator_common_of_to_plat(dev, dev_pdata,
+					   gpios ? "gpios" : "gpio");
 }
 
 static int fixed_regulator_get_value(struct udevice *dev)
-- 
2.40.1

[PATCH v2 7/9] rockchip: clk: clk_rk3568: Add CLK_PCIEPHY2_REF support

[Jonas Karlman]

Add dummy support for the CLK_PCIEPHY2_REF clock.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
v2:
- Collect r-b tag

 drivers/clk/rockchip/clk_rk3568.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/rockchip/clk_rk3568.c b/drivers/clk/rockchip/clk_rk3568.c
index 6bdd96f35b5c..0df82f597152 100644
--- a/drivers/clk/rockchip/clk_rk3568.c
+++ b/drivers/clk/rockchip/clk_rk3568.c
@@ -427,6 +427,7 @@ static ulong rk3568_pmuclk_set_rate(struct clk *clk, ulong rate)
 		break;
 	case CLK_PCIEPHY0_REF:
 	case CLK_PCIEPHY1_REF:
+	case CLK_PCIEPHY2_REF:
 		return 0;
 	default:
 		return -ENOENT;
-- 
2.40.1

[PATCH v2 8/9] rockchip: rk3568-rock-3a: Enable PCIe and NVMe support

[Jonas Karlman]

Add missing pinctrl and defconfig options to enable PCIe and NVMe
support on Radxa ROCK 3 Model A.

Use of pcie20m1_pins and pcie30x2m1_pins ensure IO mux selection M1.
The following pcie_reset_h and pcie3x2_reset_h ensure GPIO func is
restored to the perstn pin, a workaround to avoid having to define
a new rockchip,pins.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- Update commit message
- Disable pcie2x1 to work around a possible sys freeze issue

 arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 16 ++++++++++++++++
 configs/rock-3a-rk3568_defconfig        |  4 ++++
 2 files changed, 20 insertions(+)

diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
index bbf54f888fa0..bbfce7f4c247 100644
--- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
+++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
@@ -36,8 +36,24 @@
 	bootph-all;
 };
 
+&pcie2x1 {
+	pinctrl-0 = <&pcie20m1_pins &pcie_reset_h>;
+	/* Shared vpcie3v3-supply may cause a sys freeze, disable for now */
+	status = "disabled";
+};
+
+&pcie3x2 {
+	pinctrl-0 = <&pcie30x2m1_pins &pcie3x2_reset_h>;
+};
+
 &pinctrl {
 	bootph-all;
+
+	pcie {
+		pcie3x2_reset_h: pcie3x2-reset-h {
+			rockchip,pins = <2 RK_PD6 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
 };
 
 &pcfg_pull_none {
diff --git a/configs/rock-3a-rk3568_defconfig b/configs/rock-3a-rk3568_defconfig
index 64864a300153..16775015afa2 100644
--- a/configs/rock-3a-rk3568_defconfig
+++ b/configs/rock-3a-rk3568_defconfig
@@ -23,6 +23,7 @@ CONFIG_DEBUG_UART_CLOCK=24000000
 CONFIG_SPL_SPI_FLASH_SUPPORT=y
 CONFIG_SPL_SPI=y
 CONFIG_SYS_LOAD_ADDR=0xc00800
+CONFIG_PCI=y
 CONFIG_DEBUG_UART=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
@@ -46,6 +47,7 @@ CONFIG_CMD_GPIO=y
 CONFIG_CMD_GPT=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
 CONFIG_CMD_USB=y
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_CMD_PMIC=y
@@ -70,6 +72,8 @@ CONFIG_SPI_FLASH_MACRONIX=y
 CONFIG_SPI_FLASH_XTX=y
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_GMAC_ROCKCHIP=y
+CONFIG_NVME_PCI=y
+CONFIG_PCIE_DW_ROCKCHIP=y
 CONFIG_PHY_ROCKCHIP_INNO_USB2=y
 CONFIG_PHY_ROCKCHIP_NANENG_COMBOPHY=y
 CONFIG_SPL_PINCTRL=y
-- 
2.40.1

[PATCH v2 9/9] rockchip: rk356x: Update PCIe config, IO and memory regions

[Jonas Karlman]

Update config, IO and memory regions used based on [1] with pcie3x2
config reg address and reg size corrected.

Before this change:

  PCI Autoconfig: Bus Memory region: [0-3eefffff],
  PCI Autoconfig: Bus I/O region: [3ef00000-3effffff],

After this change:

  PCI Autoconfig: Bus Memory region: [40000000-7fffffff],
  PCI Autoconfig: Bus I/O region: [f0100000-f01fffff],

[1] https://lore.kernel.org/lkml/20221112114125.1637543-2-aholmes@omnom.net/

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
v2:
- Update commit message
- Collect r-b tag

 arch/arm/dts/rk3568.dtsi | 14 ++++++++------
 arch/arm/dts/rk356x.dtsi |  7 ++++---
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/arm/dts/rk3568.dtsi b/arch/arm/dts/rk3568.dtsi
index ba67b58f05b7..f1be76a54ceb 100644
--- a/arch/arm/dts/rk3568.dtsi
+++ b/arch/arm/dts/rk3568.dtsi
@@ -94,9 +94,10 @@
 		power-domains = <&power RK3568_PD_PIPE>;
 		reg = <0x3 0xc0400000 0x0 0x00400000>,
 		      <0x0 0xfe270000 0x0 0x00010000>,
-		      <0x3 0x7f000000 0x0 0x01000000>;
-		ranges = <0x01000000 0x0 0x3ef00000 0x3 0x7ef00000 0x0 0x00100000>,
-			 <0x02000000 0x0 0x00000000 0x3 0x40000000 0x0 0x3ef00000>;
+		      <0x0 0xf2000000 0x0 0x00100000>;
+		ranges = <0x01000000 0x0 0xf2100000 0x0 0xf2100000 0x0 0x00100000>,
+			 <0x02000000 0x0 0xf2200000 0x0 0xf2200000 0x0 0x01e00000>,
+			 <0x03000000 0x0 0x40000000 0x3 0x40000000 0x0 0x40000000>;
 		reg-names = "dbi", "apb", "config";
 		resets = <&cru SRST_PCIE30X1_POWERUP>;
 		reset-names = "pipe";
@@ -146,9 +147,10 @@
 		power-domains = <&power RK3568_PD_PIPE>;
 		reg = <0x3 0xc0800000 0x0 0x00400000>,
 		      <0x0 0xfe280000 0x0 0x00010000>,
-		      <0x3 0xbf000000 0x0 0x01000000>;
-		ranges = <0x01000000 0x0 0x3ef00000 0x3 0xbef00000 0x0 0x00100000>,
-			 <0x02000000 0x0 0x00000000 0x3 0x80000000 0x0 0x3ef00000>;
+		      <0x0 0xf0000000 0x0 0x00100000>;
+		ranges = <0x01000000 0x0 0xf0100000 0x0 0xf0100000 0x0 0x00100000>,
+			 <0x02000000 0x0 0xf0200000 0x0 0xf0200000 0x0 0x01e00000>,
+			 <0x03000000 0x0 0x40000000 0x3 0x80000000 0x0 0x40000000>;
 		reg-names = "dbi", "apb", "config";
 		resets = <&cru SRST_PCIE30X2_POWERUP>;
 		reset-names = "pipe";
diff --git a/arch/arm/dts/rk356x.dtsi b/arch/arm/dts/rk356x.dtsi
index 6492ace0de6b..e0591c194bec 100644
--- a/arch/arm/dts/rk356x.dtsi
+++ b/arch/arm/dts/rk356x.dtsi
@@ -951,7 +951,7 @@
 		compatible = "rockchip,rk3568-pcie";
 		reg = <0x3 0xc0000000 0x0 0x00400000>,
 		      <0x0 0xfe260000 0x0 0x00010000>,
-		      <0x3 0x3f000000 0x0 0x01000000>;
+		      <0x0 0xf4000000 0x0 0x00100000>;
 		reg-names = "dbi", "apb", "config";
 		interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>,
@@ -980,8 +980,9 @@
 		phys = <&combphy2 PHY_TYPE_PCIE>;
 		phy-names = "pcie-phy";
 		power-domains = <&power RK3568_PD_PIPE>;
-		ranges = <0x01000000 0x0 0x3ef00000 0x3 0x3ef00000 0x0 0x00100000
-			  0x02000000 0x0 0x00000000 0x3 0x00000000 0x0 0x3ef00000>;
+		ranges = <0x01000000 0x0 0xf4100000 0x0 0xf4100000 0x0 0x00100000>,
+			 <0x02000000 0x0 0xf4200000 0x0 0xf4200000 0x0 0x01e00000>,
+			 <0x03000000 0x0 0x40000000 0x3 0x00000000 0x0 0x40000000>;
 		resets = <&cru SRST_PCIE20_POWERUP>;
 		reset-names = "pipe";
 		#address-cells = <3>;
-- 
2.40.1

Re: [PATCH v2 1/9] core: read: add dev_read_addr_size_index_ptr function

[Kever Yang]

On 2023/5/18 06:53, Jonas Karlman wrote:
> Add dev_read_addr_size_index_ptr function with the same functionality as
> dev_read_addr_size_index, but instead a return pointer is given.
> Use map_sysmem() function as cast for the return.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> v2:
> - New patch
>
>   drivers/core/read.c | 11 +++++++++++
>   include/dm/read.h   | 21 +++++++++++++++++++++
>   2 files changed, 32 insertions(+)
>
> diff --git a/drivers/core/read.c b/drivers/core/read.c
> index 0289a2edb6a4..d3c939530aa5 100644
> --- a/drivers/core/read.c
> +++ b/drivers/core/read.c
> @@ -150,6 +150,17 @@ fdt_addr_t dev_read_addr_size_index(const struct udevice *dev, int index,
>   		return devfdt_get_addr_size_index(dev, index, size);
>   }
>   
> +void *dev_read_addr_size_index_ptr(const struct udevice *dev, int index,
> +				   fdt_size_t *size)
> +{
> +	fdt_addr_t addr = dev_read_addr_size_index(dev, index, size);
> +
> +	if (addr == FDT_ADDR_T_NONE)
> +		return NULL;
> +
> +	return map_sysmem(addr, 0);
> +}
> +
>   void *dev_remap_addr_index(const struct udevice *dev, int index)
>   {
>   	fdt_addr_t addr = dev_read_addr_index(dev, index);
> diff --git a/include/dm/read.h b/include/dm/read.h
> index 56ac076c9f13..7dd43d61a665 100644
> --- a/include/dm/read.h
> +++ b/include/dm/read.h
> @@ -246,6 +246,20 @@ void *dev_read_addr_index_ptr(const struct udevice *dev, int index);
>   fdt_addr_t dev_read_addr_size_index(const struct udevice *dev, int index,
>   				    fdt_size_t *size);
>   
> +/**
> + * dev_read_addr_size_index_ptr() - Get the indexed reg property of a device
> + *                                  as a pointer
> + *
> + * @dev: Device to read from
> + * @index: the 'reg' property can hold a list of <addr, size> pairs
> + *	   and @index is used to select which one is required
> + * @size: place to put size value (on success)
> + *
> + * Return: pointer or NULL if not found
> + */
> +void *dev_read_addr_size_index_ptr(const struct udevice *dev, int index,
> +				   fdt_size_t *size);
> +
>   /**
>    * dev_remap_addr_index() - Get the indexed reg property of a device
>    *                               as a memory-mapped I/O pointer
> @@ -957,6 +971,13 @@ static inline fdt_addr_t dev_read_addr_size_index(const struct udevice *dev,
>   	return devfdt_get_addr_size_index(dev, index, size);
>   }
>   
> +static inline void *dev_read_addr_size_index_ptr(const struct udevice *dev,
> +						 int index,
> +						 fdt_size_t *size)
> +{
> +	return devfdt_get_addr_size_index_ptr(dev, index, size);
> +}
> +
>   static inline fdt_addr_t dev_read_addr_name(const struct udevice *dev,
>   					    const char *name)
>   {

Re: [PATCH v2 3/9] pci: pcie_dw_rockchip: Use regulator_set_enable_if_allowed

[Kever Yang]

Hi Jonas,

On 2023/5/18 06:53, Jonas Karlman wrote:
> The vpcie3v3 regulator is typically a fixed regulator controlled using
> gpio. Change to use enable and disable calls on the regulator instead
> of trying to set a voltage value.
>
> Also remove the delay to match linux driver, for a fixed regulator the
> startup-delay-us prop can be used in case a startup delay is needed.
> Limited testing on ROCK 3A, ROCK 5B, Quartz64, Odroid-M1 has shown that
> this delay was not needed.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> v2:
> - Update commit message
>
>   drivers/pci/pcie_dw_rockchip.c | 17 +++++++----------
>   1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> index 83737e62bc6a..1b8a1409f6df 100644
> --- a/drivers/pci/pcie_dw_rockchip.c
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -288,21 +288,16 @@ static int rockchip_pcie_init_port(struct udevice *dev)
>   	struct rk_pcie *priv = dev_get_priv(dev);
>   
>   	/* Set power and maybe external ref clk input */
> -	if (priv->vpcie3v3) {
> -		ret = regulator_set_value(priv->vpcie3v3, 3300000);
> -		if (ret) {
> -			dev_err(priv->dw.dev, "failed to enable vpcie3v3 (ret=%d)\n",
> -				ret);
> -			return ret;
> -		}
> +	ret = regulator_set_enable_if_allowed(priv->vpcie3v3, true);
> +	if (ret && ret != -ENOSYS) {
> +		dev_err(dev, "failed to enable vpcie3v3 (ret=%d)\n", ret);
> +		return ret;
>   	}
>   
> -	udelay(MACRO_US * 1000);
> -
>   	ret = generic_phy_init(&priv->phy);
>   	if (ret) {
>   		dev_err(dev, "failed to init phy (ret=%d)\n", ret);
> -		return ret;
> +		goto err_disable_regulator;
>   	}
>   
>   	ret = generic_phy_power_on(&priv->phy);
> @@ -345,6 +340,8 @@ err_power_off_phy:
>   	generic_phy_power_off(&priv->phy);
>   err_exit_phy:
>   	generic_phy_exit(&priv->phy);
> +err_disable_regulator:
> +	regulator_set_enable_if_allowed(priv->vpcie3v3, false);
>   
>   	return ret;
>   }

Re: [PATCH v2 4/9] pci: pcie_dw_rockchip: Speed up link probe

[Kever Yang]

On 2023/5/18 06:53, Jonas Karlman wrote:
> Use a similar pattern and delay values as the linux mainline driver to
> speed up failing when nothing is connected.
>
> Reduce fail speed from around 5+ seconds down to around one second on a
> Radxa ROCK 3 Model A, where pcie2x1 is probed before pcie3x2 M2 slot.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> v2:
> - No change
>
>   drivers/pci/pcie_dw_rockchip.c | 68 ++++++++++++++++++----------------
>   1 file changed, 37 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> index 1b8a1409f6df..82a8b9c96e2b 100644
> --- a/drivers/pci/pcie_dw_rockchip.c
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -61,9 +61,6 @@ struct rk_pcie {
>   #define PCIE_CLIENT_DBG_TRANSITION_DATA	0xffff0000
>   #define PCIE_CLIENT_DBF_EN		0xffff0003
>   
> -/* Parameters for the waiting for #perst signal */
> -#define MACRO_US			1000
> -
>   static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
>   {
>   	if ((uintptr_t)addr & (size - 1)) {
> @@ -242,43 +239,46 @@ static int rk_pcie_link_up(struct rk_pcie *priv, u32 cap_speed)
>   	/* DW pre link configurations */
>   	rk_pcie_configure(priv, cap_speed);
>   
> -	/* Rest the device */
> -	if (dm_gpio_is_valid(&priv->rst_gpio)) {
> -		dm_gpio_set_value(&priv->rst_gpio, 0);
> -		/*
> -		 * Minimal is 100ms from spec but we see
> -		 * some wired devices need much more, such as 600ms.
> -		 * Add a enough delay to cover all cases.
> -		 */
> -		udelay(MACRO_US * 1000);
> -		dm_gpio_set_value(&priv->rst_gpio, 1);
> -	}
> -
>   	rk_pcie_disable_ltssm(priv);
>   	rk_pcie_link_status_clear(priv);
>   	rk_pcie_enable_debug(priv);
>   
> +	/* Reset the device */
> +	if (dm_gpio_is_valid(&priv->rst_gpio))
> +		dm_gpio_set_value(&priv->rst_gpio, 0);
> +
>   	/* Enable LTSSM */
>   	rk_pcie_enable_ltssm(priv);
>   
> -	for (retries = 0; retries < 5; retries++) {
> -		if (is_link_up(priv)) {
> -			dev_info(priv->dw.dev, "PCIe Link up, LTSSM is 0x%x\n",
> -				 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
> -			rk_pcie_debug_dump(priv);
> -			return 0;
> -		}
> -
> -		dev_info(priv->dw.dev, "PCIe Linking... LTSSM is 0x%x\n",
> -			 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
> -		rk_pcie_debug_dump(priv);
> -		udelay(MACRO_US * 1000);
> +	/*
> +	 * PCIe requires the refclk to be stable for 100ms prior to releasing
> +	 * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI
> +	 * Express Card Electromechanical Specification, 1.1. However, we don't
> +	 * know if the refclk is coming from RC's PHY or external OSC. If it's
> +	 * from RC, so enabling LTSSM is the just right place to release #PERST.
> +	 */
> +	mdelay(100);
> +	if (dm_gpio_is_valid(&priv->rst_gpio))
> +		dm_gpio_set_value(&priv->rst_gpio, 1);
> +
> +	/* Check if the link is up or not */
> +	for (retries = 0; retries < 10; retries++) {
> +		if (is_link_up(priv))
> +			break;
> +
> +		mdelay(100);
> +	}
> +
> +	if (retries >= 10) {
> +		dev_err(priv->dw.dev, "PCIe-%d Link Fail\n",
> +			dev_seq(priv->dw.dev));
> +		return -EIO;
>   	}
>   
> -	dev_err(priv->dw.dev, "PCIe-%d Link Fail\n", dev_seq(priv->dw.dev));
> -	/* Link maybe in Gen switch recovery but we need to wait more 1s */
> -	udelay(MACRO_US * 1000);
> -	return -EIO;
> +	dev_info(priv->dw.dev, "PCIe Link up, LTSSM is 0x%x\n",
> +		 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
> +	rk_pcie_debug_dump(priv);
> +	return 0;
>   }
>   
>   static int rockchip_pcie_init_port(struct udevice *dev)
> @@ -287,6 +287,12 @@ static int rockchip_pcie_init_port(struct udevice *dev)
>   	u32 val;
>   	struct rk_pcie *priv = dev_get_priv(dev);
>   
> +	ret = reset_assert_bulk(&priv->rsts);
> +	if (ret) {
> +		dev_err(dev, "failed to assert resets (ret=%d)\n", ret);
> +		return ret;
> +	}
> +
>   	/* Set power and maybe external ref clk input */
>   	ret = regulator_set_enable_if_allowed(priv->vpcie3v3, true);
>   	if (ret && ret != -ENOSYS) {

Re: [PATCH v2 5/9] pci: pcie_dw_rockchip: Hide BARs of the root complex

[Kever Yang]

Hi Jonas,

     Sorry for reply late for this patch.

     I have check internally again for this BAR issue, the BAR alloce 
fail for RC would not affect other process, and it always works on 
vendor U-Boot tree.

     And if we have to handle this, we can disable the BAR instead of 
this workaround.


Thanks,

- Kever

On 2023/5/18 06:53, Jonas Karlman wrote:
> PCI Autoconfig read the Root Complex BARs and try to claim the entire
> 1 GiB memory region on RK3568, leaving no space for any attached device.
>
> With a memory region less than 1 GiB this was not a real issue:
>
>    PCI Autoconfig: Bus Memory region: [0-3eefffff],
>    PCI Autoconfig: Bus I/O region: [3ef00000-3effffff],
>    PCI Autoconfig: Found P2P bridge, device 0
>    PCI Autoconfig: BAR 0, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000
>    PCI: Failed autoconfig bar 10
>    PCI Autoconfig: BAR 1, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000
>    PCI: Failed autoconfig bar 14
>    PCI Autoconfig: ROM, size=0x10000, address=0x10000 bus_lower=0x20000
>
>    PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x100000 bus_lower=0x104000
>
> With a memory region of the entire 1 GiB this leads to:
>
>    PCI Autoconfig: Bus Memory region: [40000000-7fffffff],
>    PCI Autoconfig: Bus I/O region: [f0100000-f01fffff],
>    PCI Autoconfig: Found P2P bridge, device 0
>    PCI Autoconfig: BAR 0, Mem, size=0x40000000, address=0x40000000 bus_lower=0x80000000
>    PCI Autoconfig: BAR 1, Mem, size=0x40000000, No room in resource, avail start=80000000 / size=40000000, need=40000000
>    PCI: Failed autoconfig bar 14
>    PCI Autoconfig: ROM, size=0x10000, No room in resource, avail start=80000000 / size=40000000, need=10000
>
>    PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail start=80000000 / size=40000000, need=4000
>    PCI: Failed autoconfig bar 10
>
> After this change with a memory region of the entire 1 GiB:
>
>    PCI Autoconfig: Bus Memory region: [40000000-7fffffff],
>    PCI Autoconfig: Bus I/O region: [f0100000-f01fffff],
>    PCI Autoconfig: Found P2P bridge, device 0
>    PCI Autoconfig: ROM, size=0x10000, address=0x40000000 bus_lower=0x40010000
>
>    PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x40100000 bus_lower=0x40104000
>
> Return an invalid value during config read of Root Complex BARs during
> autoconfig to work around such issue.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - Update commit message
>
>   drivers/pci/pcie_dw_rockchip.c | 28 +++++++++++++++++++++++++++-
>   1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> index 82a8b9c96e2b..f56773c2e58c 100644
> --- a/drivers/pci/pcie_dw_rockchip.c
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -146,6 +146,32 @@ static inline void rk_pcie_writel_apb(struct rk_pcie *rk_pcie, u32 reg,
>   	__rk_pcie_write_apb(rk_pcie, rk_pcie->apb_base, reg, 0x4, val);
>   }
>   
> +/**
> + * The BARs of bridge should be hidden during enumeration to avoid
> + * allocation of the entire memory region by PCIe core on RK3568.
> + */
> +static bool rk_pcie_hide_rc_bar(struct pcie_dw *pcie, pci_dev_t bdf,
> +				uint offset)
> +{
> +	int bus = PCI_BUS(bdf) - pcie->first_busno;
> +
> +	return bus == 0 && PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
> +	       offset >= PCI_BASE_ADDRESS_0 && offset <= PCI_BASE_ADDRESS_1;
> +}
> +
> +static int rk_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
> +			       uint offset, ulong *valuep,
> +			       enum pci_size_t size)
> +{
> +	struct pcie_dw *pcie = dev_get_priv(bus);
> +	int ret = pcie_dw_read_config(bus, bdf, offset, valuep, size);
> +
> +	if (!ret && rk_pcie_hide_rc_bar(pcie, bdf, offset))
> +		*valuep = pci_get_ff(size);
> +
> +	return ret;
> +}
> +
>   /**
>    * rk_pcie_configure() - Configure link capabilities and speed
>    *
> @@ -476,7 +502,7 @@ rockchip_pcie_probe_err_init_port:
>   }
>   
>   static const struct dm_pci_ops rockchip_pcie_ops = {
> -	.read_config	= pcie_dw_read_config,
> +	.read_config	= rk_pcie_read_config,
>   	.write_config	= pcie_dw_write_config,
>   };
>   

Re: [PATCH v2 5/9] pci: pcie_dw_rockchip: Hide BARs of the root complex

[Jonas Karlman]

Hi Kever,
On 2023-06-29 12:25, Kever Yang wrote:
> Hi Jonas,
> 
>      Sorry for reply late for this patch.
> 
>      I have check internally again for this BAR issue, the BAR alloce 
> fail for RC would not affect other process, and it always works on 
> vendor U-Boot tree.

The issue is not that the RC BAR allocation fails and that it affect
other PCI devices BAR allocation after that, the issue is that with
updated ranges the RC BAR allocation is successful and the entire memory
region gets allocated and that in turn affect other PCI devices from a
successful probe.

There are multiple parts in play here:

1. Two BARs of 1 GiB each is reported for RC on RK3568, 0xC0000000 is read
   back instead of 0x0000000.
   As seen by "PCI Autoconfig: BAR X, Mem, size=0x40000000" below.

2. With the ranges from "rockchip: rk356x: Update PCIe config, IO and
   memory regions", now also merged in linux, the first BAR for RC does
   allocate the entire memory region.
   As seen by "With a memory region of the entire 1 GiB this leads to".

   PCI Autoconfig: BAR 0, Mem, size=0x40000000, address=0x40000000 bus_lower=0x80000000

3. Any other BAR processed will fail with "No room in resource" and the
   PCI autoconfig fails and no device is discovered.
   As seen by "With a memory region of the entire 1 GiB this leads to".

   PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail start=80000000 / size=40000000, need=4000
   PCI: Failed autoconfig bar 10

On downstream vendor U-Boot this is not an issue because the start
address is 0x0 instead of 0x40000000. U-Boot skips first 0x1000 of the
range when the start address is 0x0, meaning the available range is less
then 1 GiB, and the BAR allocation for RC fails.

As seen by "With a memory region less than 1 GiB this was not a real
issue" this also happen until memory ranges are adjusted in device tree.

  PCI Autoconfig: BAR 0, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000

The issue with RC allocating the entire 1 GiB region would probably
affect vendor U-Boot if it would use 0x40000000 as start address instead
of 0x0.

There has been reports on linux ML that the use of 0x0 as start address
caused issues for some PCI devices and that the use of 0x40000000 as
start address fixed such issue.

> 
>      And if we have to handle this, we can disable the BAR instead of 
> this workaround.

Please provide details on how we can configure the HW to not report any
BARs when in RC mode.

Using a very similar approach that other pci drivers have used, e.g. the
Intel FPGA PCIe host controller driver, is the only way that I found
that worked consistently.
See https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_intel_fpga.c#L87-103

Regards,
Jonas

> 
> 
> Thanks,
> 
> - Kever
> 
> On 2023/5/18 06:53, Jonas Karlman wrote:
>> PCI Autoconfig read the Root Complex BARs and try to claim the entire
>> 1 GiB memory region on RK3568, leaving no space for any attached device.
>>
>> With a memory region less than 1 GiB this was not a real issue:
>>
>>    PCI Autoconfig: Bus Memory region: [0-3eefffff],
>>    PCI Autoconfig: Bus I/O region: [3ef00000-3effffff],
>>    PCI Autoconfig: Found P2P bridge, device 0
>>    PCI Autoconfig: BAR 0, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000
>>    PCI: Failed autoconfig bar 10
>>    PCI Autoconfig: BAR 1, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000
>>    PCI: Failed autoconfig bar 14
>>    PCI Autoconfig: ROM, size=0x10000, address=0x10000 bus_lower=0x20000
>>
>>    PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x100000 bus_lower=0x104000
>>
>> With a memory region of the entire 1 GiB this leads to:
>>
>>    PCI Autoconfig: Bus Memory region: [40000000-7fffffff],
>>    PCI Autoconfig: Bus I/O region: [f0100000-f01fffff],
>>    PCI Autoconfig: Found P2P bridge, device 0
>>    PCI Autoconfig: BAR 0, Mem, size=0x40000000, address=0x40000000 bus_lower=0x80000000
>>    PCI Autoconfig: BAR 1, Mem, size=0x40000000, No room in resource, avail start=80000000 / size=40000000, need=40000000
>>    PCI: Failed autoconfig bar 14
>>    PCI Autoconfig: ROM, size=0x10000, No room in resource, avail start=80000000 / size=40000000, need=10000
>>
>>    PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail start=80000000 / size=40000000, need=4000
>>    PCI: Failed autoconfig bar 10
>>
>> After this change with a memory region of the entire 1 GiB:
>>
>>    PCI Autoconfig: Bus Memory region: [40000000-7fffffff],
>>    PCI Autoconfig: Bus I/O region: [f0100000-f01fffff],
>>    PCI Autoconfig: Found P2P bridge, device 0
>>    PCI Autoconfig: ROM, size=0x10000, address=0x40000000 bus_lower=0x40010000
>>
>>    PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x40100000 bus_lower=0x40104000
>>
>> Return an invalid value during config read of Root Complex BARs during
>> autoconfig to work around such issue.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v2:
>> - Update commit message
>>
>>   drivers/pci/pcie_dw_rockchip.c | 28 +++++++++++++++++++++++++++-
>>   1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
>> index 82a8b9c96e2b..f56773c2e58c 100644
>> --- a/drivers/pci/pcie_dw_rockchip.c
>> +++ b/drivers/pci/pcie_dw_rockchip.c
>> @@ -146,6 +146,32 @@ static inline void rk_pcie_writel_apb(struct rk_pcie *rk_pcie, u32 reg,
>>   	__rk_pcie_write_apb(rk_pcie, rk_pcie->apb_base, reg, 0x4, val);
>>   }
>>   
>> +/**
>> + * The BARs of bridge should be hidden during enumeration to avoid
>> + * allocation of the entire memory region by PCIe core on RK3568.
>> + */
>> +static bool rk_pcie_hide_rc_bar(struct pcie_dw *pcie, pci_dev_t bdf,
>> +				uint offset)
>> +{
>> +	int bus = PCI_BUS(bdf) - pcie->first_busno;
>> +
>> +	return bus == 0 && PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
>> +	       offset >= PCI_BASE_ADDRESS_0 && offset <= PCI_BASE_ADDRESS_1;
>> +}
>> +
>> +static int rk_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
>> +			       uint offset, ulong *valuep,
>> +			       enum pci_size_t size)
>> +{
>> +	struct pcie_dw *pcie = dev_get_priv(bus);
>> +	int ret = pcie_dw_read_config(bus, bdf, offset, valuep, size);
>> +
>> +	if (!ret && rk_pcie_hide_rc_bar(pcie, bdf, offset))
>> +		*valuep = pci_get_ff(size);
>> +
>> +	return ret;
>> +}
>> +
>>   /**
>>    * rk_pcie_configure() - Configure link capabilities and speed
>>    *
>> @@ -476,7 +502,7 @@ rockchip_pcie_probe_err_init_port:
>>   }
>>   
>>   static const struct dm_pci_ops rockchip_pcie_ops = {
>> -	.read_config	= pcie_dw_read_config,
>> +	.read_config	= rk_pcie_read_config,
>>   	.write_config	= pcie_dw_write_config,
>>   };
>>   

Re: [PATCH v2 8/9] rockchip: rk3568-rock-3a: Enable PCIe and NVMe support

[Jonas Karlman]

On 2023-05-18 00:53, Jonas Karlman wrote:
> Add missing pinctrl and defconfig options to enable PCIe and NVMe
> support on Radxa ROCK 3 Model A.
> 
> Use of pcie20m1_pins and pcie30x2m1_pins ensure IO mux selection M1.
> The following pcie_reset_h and pcie3x2_reset_h ensure GPIO func is
> restored to the perstn pin, a workaround to avoid having to define
> a new rockchip,pins.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - Update commit message
> - Disable pcie2x1 to work around a possible sys freeze issue
> 
>  arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 16 ++++++++++++++++
>  configs/rock-3a-rk3568_defconfig        |  4 ++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
> index bbf54f888fa0..bbfce7f4c247 100644
> --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
> +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
> @@ -36,8 +36,24 @@
>  	bootph-all;
>  };
>  
> +&pcie2x1 {
> +	pinctrl-0 = <&pcie20m1_pins &pcie_reset_h>;
> +	/* Shared vpcie3v3-supply may cause a sys freeze, disable for now */
> +	status = "disabled";

This workaround is no longer needed after the commit 4fcba5d556b4
("regulator: implement basic reference counter").

Will send a v3 with this workaround removed and a few AHCI/SCSI related
options enabled early next week.

Regards,
Jonas

> +};
> +
> +&pcie3x2 {
> +	pinctrl-0 = <&pcie30x2m1_pins &pcie3x2_reset_h>;
> +};
> +
>  &pinctrl {
>  	bootph-all;
> +
> +	pcie {
> +		pcie3x2_reset_h: pcie3x2-reset-h {
> +			rockchip,pins = <2 RK_PD6 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
>  };
>  
>  &pcfg_pull_none {
> diff --git a/configs/rock-3a-rk3568_defconfig b/configs/rock-3a-rk3568_defconfig
> index 64864a300153..16775015afa2 100644
> --- a/configs/rock-3a-rk3568_defconfig
> +++ b/configs/rock-3a-rk3568_defconfig
> @@ -23,6 +23,7 @@ CONFIG_DEBUG_UART_CLOCK=24000000
>  CONFIG_SPL_SPI_FLASH_SUPPORT=y
>  CONFIG_SPL_SPI=y
>  CONFIG_SYS_LOAD_ADDR=0xc00800
> +CONFIG_PCI=y
>  CONFIG_DEBUG_UART=y
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
> @@ -46,6 +47,7 @@ CONFIG_CMD_GPIO=y
>  CONFIG_CMD_GPT=y
>  CONFIG_CMD_I2C=y
>  CONFIG_CMD_MMC=y
> +CONFIG_CMD_PCI=y
>  CONFIG_CMD_USB=y
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_CMD_PMIC=y
> @@ -70,6 +72,8 @@ CONFIG_SPI_FLASH_MACRONIX=y
>  CONFIG_SPI_FLASH_XTX=y
>  CONFIG_ETH_DESIGNWARE=y
>  CONFIG_GMAC_ROCKCHIP=y
> +CONFIG_NVME_PCI=y
> +CONFIG_PCIE_DW_ROCKCHIP=y
>  CONFIG_PHY_ROCKCHIP_INNO_USB2=y
>  CONFIG_PHY_ROCKCHIP_NANENG_COMBOPHY=y
>  CONFIG_SPL_PINCTRL=y