[PATCH 1/3] PCI: rockchip: split out rockchip_cfg_atu

[PATCH 1/3] PCI: rockchip: split out rockchip_cfg_atu

[Shawn Lin]

We split out a new function, rockchip_cfg_atu, in order to
re-configure the atu when missing these information after
wakeup from S3.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/pci/host/pcie-rockchip.c | 119 ++++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 51 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index b55037a..19399fc 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -169,6 +169,7 @@
 #define IB_ROOT_PORT_REG_SIZE_SHIFT		3
 #define AXI_WRAPPER_IO_WRITE			0x6
 #define AXI_WRAPPER_MEM_WRITE			0x2
+#define AXI_WRAPPER_NOR_MSG			0xc
 
 #define MAX_AXI_IB_ROOTPORT_REGION_NUM		3
 #define MIN_AXI_ADDR_BITS_PASSED		8
@@ -210,6 +211,13 @@ struct rockchip_pcie {
 	int	link_gen;
 	struct	device *dev;
 	struct	irq_domain *irq_domain;
+	u32     io_size;
+	int     offset;
+	phys_addr_t io_bus_addr;
+	int     msg_region_nr;
+	void	__iomem *msg_region;
+	u32     mem_size;
+	phys_addr_t mem_bus_addr;
 };
 
 static u32 rockchip_pcie_read(struct rockchip_pcie *rockchip, u32 reg)
@@ -1149,6 +1157,57 @@ static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie *rockchip,
 	return 0;
 }
 
+static int rockchip_cfg_atu(struct rockchip_pcie *rockchip)
+{
+	int offset;
+	int err;
+	int reg_no;
+
+	for (reg_no = 0; reg_no < (rockchip->mem_size >> 20); reg_no++) {
+		err = rockchip_pcie_prog_ob_atu(rockchip, reg_no + 1,
+						AXI_WRAPPER_MEM_WRITE,
+						20 - 1,
+						rockchip->mem_bus_addr +
+						(reg_no << 20),
+						0);
+		if (err) {
+			dev_err(rockchip->dev,
+					"program RC mem outbound ATU failed\n");
+			return err;
+		}
+	}
+
+	err = rockchip_pcie_prog_ib_atu(rockchip, 2, 32 - 1, 0x0, 0);
+	if (err) {
+		dev_err(rockchip->dev, "program RC mem inbound ATU failed\n");
+		return err;
+	}
+
+	offset = rockchip->mem_size >> 20;
+	for (reg_no = 0; reg_no < (rockchip->io_size >> 20); reg_no++) {
+		err = rockchip_pcie_prog_ob_atu(rockchip,
+						reg_no + 1 + offset,
+						AXI_WRAPPER_IO_WRITE,
+						20 - 1,
+						rockchip->io_bus_addr +
+						(reg_no << 20),
+						0);
+		if (err) {
+			dev_err(rockchip->dev,
+					"program RC io outbound ATU failed\n");
+			return err;
+		}
+	}
+
+	/* assign message regions */
+	rockchip->msg_region_nr = reg_no + 1 + offset;
+	rockchip_pcie_prog_ob_atu(rockchip, reg_no + 1 + offset,
+				  AXI_WRAPPER_NOR_MSG,
+				  20 - 1, 0, 0);
+	rockchip->msg_region = ioremap(rockchip->mem_bus_addr +
+				       ((reg_no - 1) << 20), SZ_1M);
+	return err;
+}
 static int rockchip_pcie_probe(struct platform_device *pdev)
 {
 	struct rockchip_pcie *rockchip;
@@ -1158,13 +1217,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	resource_size_t io_base;
 	struct resource	*mem;
 	struct resource	*io;
-	phys_addr_t io_bus_addr = 0;
-	u32 io_size;
-	phys_addr_t mem_bus_addr = 0;
-	u32 mem_size = 0;
-	int reg_no;
 	int err;
-	int offset;
 
 	LIST_HEAD(res);
 
@@ -1231,14 +1284,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 		goto err_vpcie;
 
 	/* Get the I/O and memory ranges from DT */
-	io_size = 0;
 	resource_list_for_each_entry(win, &res) {
 		switch (resource_type(win->res)) {
 		case IORESOURCE_IO:
 			io = win->res;
 			io->name = "I/O";
-			io_size = resource_size(io);
-			io_bus_addr = io->start - win->offset;
+			rockchip->io_size = resource_size(io);
+			rockchip->io_bus_addr = io->start - win->offset;
 			err = pci_remap_iospace(io, io_base);
 			if (err) {
 				dev_warn(dev, "error %d: failed to map resource %pR\n",
@@ -1249,8 +1301,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 		case IORESOURCE_MEM:
 			mem = win->res;
 			mem->name = "MEM";
-			mem_size = resource_size(mem);
-			mem_bus_addr = mem->start - win->offset;
+			rockchip->mem_size = resource_size(mem);
+			rockchip->mem_bus_addr = mem->start - win->offset;
 			break;
 		case IORESOURCE_BUS:
 			rockchip->root_bus_nr = win->res->start;
@@ -1260,49 +1312,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (mem_size) {
-		for (reg_no = 0; reg_no < (mem_size >> 20); reg_no++) {
-			err = rockchip_pcie_prog_ob_atu(rockchip, reg_no + 1,
-							AXI_WRAPPER_MEM_WRITE,
-							20 - 1,
-							mem_bus_addr +
-							(reg_no << 20),
-							0);
-			if (err) {
-				dev_err(dev, "program RC mem outbound ATU failed\n");
-				goto err_vpcie;
-			}
-		}
-	}
-
-	err = rockchip_pcie_prog_ib_atu(rockchip, 2, 32 - 1, 0x0, 0);
-	if (err) {
-		dev_err(dev, "program RC mem inbound ATU failed\n");
+	err = rockchip_cfg_atu(rockchip);
+	if (err)
 		goto err_vpcie;
-	}
-
-	offset = mem_size >> 20;
-
-	if (io_size) {
-		for (reg_no = 0; reg_no < (io_size >> 20); reg_no++) {
-			err = rockchip_pcie_prog_ob_atu(rockchip,
-							reg_no + 1 + offset,
-							AXI_WRAPPER_IO_WRITE,
-							20 - 1,
-							io_bus_addr +
-							(reg_no << 20),
-							0);
-			if (err) {
-				dev_err(dev, "program RC io outbound ATU failed\n");
-				goto err_vpcie;
-			}
-		}
-	}
-
 	bus = pci_scan_root_bus(&pdev->dev, 0, &rockchip_pcie_ops, rockchip, &res);
 	if (!bus) {
 		err = -ENOMEM;
-		goto err_vpcie;
+		goto err_scan;
 	}
 
 	pci_bus_size_bridges(bus);
@@ -1315,7 +1331,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	dev_warn(dev, "only 32-bit config accesses supported; smaller writes may corrupt adjacent RW1C fields\n");
 
 	return err;
-
+err_scan:
+	iounmap(rockchip->msg_region);
 err_vpcie:
 	if (!IS_ERR(rockchip->vpcie3v3))
 		regulator_disable(rockchip->vpcie3v3);
-- 
1.9.1

[PATCH 2/3] PCI: rockchip: move the deassert of pm/aclk/pclk after phy_init

[Shawn Lin]

Move them after phy_init as we want to optimize the logic
of reset control and reuse rockchip_pcie_init_port later
which should fully follow the cold boot procedure of ROM
code.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/pci/host/pcie-rockchip.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 19399fc..71d056d 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -476,26 +476,6 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 		return err;
 	}
 
-	udelay(10);
-
-	err = reset_control_deassert(rockchip->pm_rst);
-	if (err) {
-		dev_err(dev, "deassert pm_rst err %d\n", err);
-		return err;
-	}
-
-	err = reset_control_deassert(rockchip->aclk_rst);
-	if (err) {
-		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
-		return err;
-	}
-
-	err = reset_control_deassert(rockchip->pclk_rst);
-	if (err) {
-		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
-		return err;
-	}
-
 	err = phy_init(rockchip->phy);
 	if (err < 0) {
 		dev_err(dev, "fail to init phy, err %d\n", err);
@@ -526,6 +506,26 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 		return err;
 	}
 
+	udelay(10);
+
+	err = reset_control_deassert(rockchip->pm_rst);
+	if (err) {
+		dev_err(dev, "deassert pm_rst err %d\n", err);
+		return err;
+	}
+
+	err = reset_control_deassert(rockchip->aclk_rst);
+	if (err) {
+		dev_err(dev, "deassert aclk_rst err %d\n", err);
+		return err;
+	}
+
+	err = reset_control_deassert(rockchip->pclk_rst);
+	if (err) {
+		dev_err(dev, "deassert pclk_rst err %d\n", err);
+		return err;
+	}
+
 	if (rockchip->link_gen == 2)
 		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_2,
 				    PCIE_CLIENT_CONFIG);
-- 
1.9.1

[PATCH 3/3] PCI: rockchip: Add system PM support

[Shawn Lin]

This patch adds system PM support for Rockchip's RC.
For pre S3, the EP is configured into D3 state which guarantees
the link state should be in L1. So we could send PME_Turn_Off message
to the EP and wait for its ACK to make the link state into L2 or L3
without the aux-supply. This could help save more power which I think
should be very important for mobile devices.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/pci/host/pcie-rockchip.c | 90 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 89 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 71d056d..720535b 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -20,6 +20,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/iopoll.h>
 #include <linux/irq.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
@@ -55,6 +56,10 @@
 #define   PCIE_CLIENT_MODE_RC		  HIWORD_UPDATE_BIT(0x0040)
 #define   PCIE_CLIENT_GEN_SEL_1		  HIWORD_UPDATE(0x0080, 0)
 #define   PCIE_CLIENT_GEN_SEL_2		  HIWORD_UPDATE_BIT(0x0080)
+#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
+#define   PCIE_CLIENT_DEBUG_LTSSM_L2		0x19
 #define PCIE_CLIENT_BASIC_STATUS1	(PCIE_CLIENT_BASE + 0x48)
 #define   PCIE_CLIENT_LINK_STATUS_UP		0x00300000
 #define   PCIE_CLIENT_LINK_STATUS_MASK		0x00300000
@@ -173,6 +178,7 @@
 
 #define MAX_AXI_IB_ROOTPORT_REGION_NUM		3
 #define MIN_AXI_ADDR_BITS_PASSED		8
+#define PCIE_RC_SEND_PME_OFF			0x11960
 #define ROCKCHIP_VENDOR_ID			0x1d87
 #define PCIE_ECAM_BUS(x)			(((x) & 0xff) << 20)
 #define PCIE_ECAM_DEV(x)			(((x) & 0x1f) << 15)
@@ -181,6 +187,9 @@
 #define PCIE_ECAM_ADDR(bus, dev, func, reg) \
 	  (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
 	   PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
+#define PCIE_LINK_IS_L2(x) \
+		((x & PCIE_CLIENT_DEBUG_LTSSM_MASK) == \
+		 PCIE_CLIENT_DEBUG_LTSSM_L2)
 
 #define RC_REGION_0_ADDR_TRANS_H		0x00000000
 #define RC_REGION_0_ADDR_TRANS_L		0x00000000
@@ -1205,9 +1214,80 @@ static int rockchip_cfg_atu(struct rockchip_pcie *rockchip)
 				  AXI_WRAPPER_NOR_MSG,
 				  20 - 1, 0, 0);
 	rockchip->msg_region = ioremap(rockchip->mem_bus_addr +
-				       ((reg_no - 1) << 20), SZ_1M);
+				       ((reg_no + offset) << 20), SZ_1M);
 	return err;
 }
+
+static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
+{
+	u32 value;
+	int err;
+
+	/* send PME_TURN_OFF message */
+	writel(0x0, rockchip->msg_region + PCIE_RC_SEND_PME_OFF);
+
+	/* read LTSSM and wait for falling into L2 link state */
+	err = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_DEBUG_OUT_0,
+				 value, PCIE_LINK_IS_L2(value), 20,
+				 jiffies_to_usecs(5 * HZ));
+	if (err) {
+		dev_err(rockchip->dev, "PCIe link enter L2 timeout!\n");
+		return err;
+	}
+
+	return 0;
+}
+
+static int rockchip_pcie_suspend_noirq(struct device *dev)
+{
+	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
+	int ret;
+
+	/* disable core and cli int since we don't need to ack PME_ACK */
+	rockchip_pcie_write(rockchip, (PCIE_CLIENT_INT_CLI << 16) |
+			    PCIE_CLIENT_INT_CLI, PCIE_CLIENT_INT_MASK);
+	rockchip_pcie_write(rockchip, (u32)PCIE_CORE_INT, PCIE_CORE_INT_MASK);
+
+	ret = rockchip_pcie_wait_l2(rockchip);
+	if (ret)
+		return ret;
+
+	phy_power_off(rockchip->phy);
+	phy_exit(rockchip->phy);
+
+	clk_disable_unprepare(rockchip->clk_pcie_pm);
+	clk_disable_unprepare(rockchip->hclk_pcie);
+	clk_disable_unprepare(rockchip->aclk_perf_pcie);
+	clk_disable_unprepare(rockchip->aclk_pcie);
+
+	return ret;
+}
+
+static int rockchip_pcie_resume_noirq(struct device *dev)
+{
+	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
+	int err;
+
+	clk_prepare_enable(rockchip->clk_pcie_pm);
+	clk_prepare_enable(rockchip->hclk_pcie);
+	clk_prepare_enable(rockchip->aclk_perf_pcie);
+	clk_prepare_enable(rockchip->aclk_pcie);
+
+	err = rockchip_pcie_init_port(rockchip);
+	if (err)
+		return err;
+
+	err = rockchip_cfg_atu(rockchip);
+	if (err)
+		return err;
+
+	/* Need this to enter L1 again */
+	rockchip_pcie_update_txcredit_mui(rockchip);
+	rockchip_pcie_enable_interrupts(rockchip);
+
+	return 0;
+}
+
 static int rockchip_pcie_probe(struct platform_device *pdev)
 {
 	struct rockchip_pcie *rockchip;
@@ -1228,6 +1308,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	if (!rockchip)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, rockchip);
+
 	rockchip->dev = dev;
 
 	err = rockchip_pcie_parse_dt(rockchip);
@@ -1352,6 +1434,11 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	return err;
 }
 
+static const struct dev_pm_ops rockchip_pcie_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rockchip_pcie_suspend_noirq,
+				      rockchip_pcie_resume_noirq)
+};
+
 static const struct of_device_id rockchip_pcie_of_match[] = {
 	{ .compatible = "rockchip,rk3399-pcie", },
 	{}
@@ -1361,6 +1448,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	.driver = {
 		.name = "rockchip-pcie",
 		.of_match_table = rockchip_pcie_of_match,
+		.pm = &rockchip_pcie_pm_ops,
 	},
 	.probe = rockchip_pcie_probe,
 
-- 
1.9.1

Re: [PATCH 1/3] PCI: rockchip: split out rockchip_cfg_atu

[Brian Norris]

On Wed, Nov 23, 2016 at 09:19:11AM +0800, Shawn Lin wrote:
> We split out a new function, rockchip_cfg_atu, in order to
> re-configure the atu when missing these information after
> wakeup from S3.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  drivers/pci/host/pcie-rockchip.c | 119 ++++++++++++++++++++++-----------------
>  1 file changed, 68 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index b55037a..19399fc 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -169,6 +169,7 @@
>  #define IB_ROOT_PORT_REG_SIZE_SHIFT		3
>  #define AXI_WRAPPER_IO_WRITE			0x6
>  #define AXI_WRAPPER_MEM_WRITE			0x2
> +#define AXI_WRAPPER_NOR_MSG			0xc

You're also adding this 'normal message' support in a refactoring patch.
This should be in another patch -- preferably patch 3 I think.

>  
>  #define MAX_AXI_IB_ROOTPORT_REGION_NUM		3
>  #define MIN_AXI_ADDR_BITS_PASSED		8
> @@ -210,6 +211,13 @@ struct rockchip_pcie {
>  	int	link_gen;
>  	struct	device *dev;
>  	struct	irq_domain *irq_domain;
> +	u32     io_size;
> +	int     offset;
> +	phys_addr_t io_bus_addr;
> +	int     msg_region_nr;
> +	void	__iomem *msg_region;
> +	u32     mem_size;
> +	phys_addr_t mem_bus_addr;
>  };
>  
>  static u32 rockchip_pcie_read(struct rockchip_pcie *rockchip, u32 reg)
> @@ -1149,6 +1157,57 @@ static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie *rockchip,
>  	return 0;
>  }
>  
> +static int rockchip_cfg_atu(struct rockchip_pcie *rockchip)
> +{
> +	int offset;
> +	int err;
> +	int reg_no;
> +
> +	for (reg_no = 0; reg_no < (rockchip->mem_size >> 20); reg_no++) {
> +		err = rockchip_pcie_prog_ob_atu(rockchip, reg_no + 1,
> +						AXI_WRAPPER_MEM_WRITE,
> +						20 - 1,
> +						rockchip->mem_bus_addr +
> +						(reg_no << 20),
> +						0);
> +		if (err) {
> +			dev_err(rockchip->dev,
> +					"program RC mem outbound ATU failed\n");
> +			return err;
> +		}
> +	}
> +
> +	err = rockchip_pcie_prog_ib_atu(rockchip, 2, 32 - 1, 0x0, 0);
> +	if (err) {
> +		dev_err(rockchip->dev, "program RC mem inbound ATU failed\n");
> +		return err;
> +	}
> +
> +	offset = rockchip->mem_size >> 20;
> +	for (reg_no = 0; reg_no < (rockchip->io_size >> 20); reg_no++) {
> +		err = rockchip_pcie_prog_ob_atu(rockchip,
> +						reg_no + 1 + offset,
> +						AXI_WRAPPER_IO_WRITE,
> +						20 - 1,
> +						rockchip->io_bus_addr +
> +						(reg_no << 20),
> +						0);
> +		if (err) {
> +			dev_err(rockchip->dev,
> +					"program RC io outbound ATU failed\n");
> +			return err;
> +		}
> +	}
> +
> +	/* assign message regions */
> +	rockchip->msg_region_nr = reg_no + 1 + offset;
> +	rockchip_pcie_prog_ob_atu(rockchip, reg_no + 1 + offset,
> +				  AXI_WRAPPER_NOR_MSG,
> +				  20 - 1, 0, 0);

Same here.

> +	rockchip->msg_region = ioremap(rockchip->mem_bus_addr +
> +				       ((reg_no - 1) << 20), SZ_1M);

(And here.)

ioremap() can fail; check for NULL.

Also, when you start reusing rockchip_cfg_atu() in patch 3, you'll be
leaking virtual address space, as you'll keep remapping this every time.
You should straighten that out. Either some kind of check for
'if (!rockchip->msg_region)', or just do the map/unmap where it's
actually used (in patch 3).

Brian

> +	return err;
> +}
>  static int rockchip_pcie_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_pcie *rockchip;
> @@ -1158,13 +1217,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	resource_size_t io_base;
>  	struct resource	*mem;
>  	struct resource	*io;
> -	phys_addr_t io_bus_addr = 0;
> -	u32 io_size;
> -	phys_addr_t mem_bus_addr = 0;
> -	u32 mem_size = 0;
> -	int reg_no;
>  	int err;
> -	int offset;
>  
>  	LIST_HEAD(res);
>  
> @@ -1231,14 +1284,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  		goto err_vpcie;
>  
>  	/* Get the I/O and memory ranges from DT */
> -	io_size = 0;
>  	resource_list_for_each_entry(win, &res) {
>  		switch (resource_type(win->res)) {
>  		case IORESOURCE_IO:
>  			io = win->res;
>  			io->name = "I/O";
> -			io_size = resource_size(io);
> -			io_bus_addr = io->start - win->offset;
> +			rockchip->io_size = resource_size(io);
> +			rockchip->io_bus_addr = io->start - win->offset;
>  			err = pci_remap_iospace(io, io_base);
>  			if (err) {
>  				dev_warn(dev, "error %d: failed to map resource %pR\n",
> @@ -1249,8 +1301,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  		case IORESOURCE_MEM:
>  			mem = win->res;
>  			mem->name = "MEM";
> -			mem_size = resource_size(mem);
> -			mem_bus_addr = mem->start - win->offset;
> +			rockchip->mem_size = resource_size(mem);
> +			rockchip->mem_bus_addr = mem->start - win->offset;
>  			break;
>  		case IORESOURCE_BUS:
>  			rockchip->root_bus_nr = win->res->start;
> @@ -1260,49 +1312,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	if (mem_size) {
> -		for (reg_no = 0; reg_no < (mem_size >> 20); reg_no++) {
> -			err = rockchip_pcie_prog_ob_atu(rockchip, reg_no + 1,
> -							AXI_WRAPPER_MEM_WRITE,
> -							20 - 1,
> -							mem_bus_addr +
> -							(reg_no << 20),
> -							0);
> -			if (err) {
> -				dev_err(dev, "program RC mem outbound ATU failed\n");
> -				goto err_vpcie;
> -			}
> -		}
> -	}
> -
> -	err = rockchip_pcie_prog_ib_atu(rockchip, 2, 32 - 1, 0x0, 0);
> -	if (err) {
> -		dev_err(dev, "program RC mem inbound ATU failed\n");
> +	err = rockchip_cfg_atu(rockchip);
> +	if (err)
>  		goto err_vpcie;
> -	}
> -
> -	offset = mem_size >> 20;
> -
> -	if (io_size) {
> -		for (reg_no = 0; reg_no < (io_size >> 20); reg_no++) {
> -			err = rockchip_pcie_prog_ob_atu(rockchip,
> -							reg_no + 1 + offset,
> -							AXI_WRAPPER_IO_WRITE,
> -							20 - 1,
> -							io_bus_addr +
> -							(reg_no << 20),
> -							0);
> -			if (err) {
> -				dev_err(dev, "program RC io outbound ATU failed\n");
> -				goto err_vpcie;
> -			}
> -		}
> -	}
> -
>  	bus = pci_scan_root_bus(&pdev->dev, 0, &rockchip_pcie_ops, rockchip, &res);
>  	if (!bus) {
>  		err = -ENOMEM;
> -		goto err_vpcie;
> +		goto err_scan;
>  	}
>  
>  	pci_bus_size_bridges(bus);
> @@ -1315,7 +1331,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	dev_warn(dev, "only 32-bit config accesses supported; smaller writes may corrupt adjacent RW1C fields\n");
>  
>  	return err;
> -
> +err_scan:
> +	iounmap(rockchip->msg_region);
>  err_vpcie:
>  	if (!IS_ERR(rockchip->vpcie3v3))
>  		regulator_disable(rockchip->vpcie3v3);
> -- 
> 1.9.1
> 
> 

Re: [PATCH 3/3] PCI: rockchip: Add system PM support

[Brian Norris]

On Wed, Nov 23, 2016 at 09:19:13AM +0800, Shawn Lin wrote:
> This patch adds system PM support for Rockchip's RC.
> For pre S3, the EP is configured into D3 state which guarantees
> the link state should be in L1. So we could send PME_Turn_Off message
> to the EP and wait for its ACK to make the link state into L2 or L3
> without the aux-supply. This could help save more power which I think
> should be very important for mobile devices.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  drivers/pci/host/pcie-rockchip.c | 90 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index 71d056d..720535b 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -20,6 +20,7 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
> +#include <linux/iopoll.h>
>  #include <linux/irq.h>
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
> @@ -55,6 +56,10 @@
>  #define   PCIE_CLIENT_MODE_RC		  HIWORD_UPDATE_BIT(0x0040)
>  #define   PCIE_CLIENT_GEN_SEL_1		  HIWORD_UPDATE(0x0080, 0)
>  #define   PCIE_CLIENT_GEN_SEL_2		  HIWORD_UPDATE_BIT(0x0080)
> +#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
> +#define   PCIE_CLIENT_DEBUG_LTSSM_L2		0x19
>  #define PCIE_CLIENT_BASIC_STATUS1	(PCIE_CLIENT_BASE + 0x48)
>  #define   PCIE_CLIENT_LINK_STATUS_UP		0x00300000
>  #define   PCIE_CLIENT_LINK_STATUS_MASK		0x00300000
> @@ -173,6 +178,7 @@
>  
>  #define MAX_AXI_IB_ROOTPORT_REGION_NUM		3
>  #define MIN_AXI_ADDR_BITS_PASSED		8
> +#define PCIE_RC_SEND_PME_OFF			0x11960
>  #define ROCKCHIP_VENDOR_ID			0x1d87
>  #define PCIE_ECAM_BUS(x)			(((x) & 0xff) << 20)
>  #define PCIE_ECAM_DEV(x)			(((x) & 0x1f) << 15)
> @@ -181,6 +187,9 @@
>  #define PCIE_ECAM_ADDR(bus, dev, func, reg) \
>  	  (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
>  	   PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
> +#define PCIE_LINK_IS_L2(x) \
> +		((x & PCIE_CLIENT_DEBUG_LTSSM_MASK) == \

Wrap the 'x' in parentheses, for safety in case the caller passes
something complicated.

> +		 PCIE_CLIENT_DEBUG_LTSSM_L2)
>  
>  #define RC_REGION_0_ADDR_TRANS_H		0x00000000
>  #define RC_REGION_0_ADDR_TRANS_L		0x00000000
> @@ -1205,9 +1214,80 @@ static int rockchip_cfg_atu(struct rockchip_pcie *rockchip)
>  				  AXI_WRAPPER_NOR_MSG,
>  				  20 - 1, 0, 0);
>  	rockchip->msg_region = ioremap(rockchip->mem_bus_addr +
> -				       ((reg_no - 1) << 20), SZ_1M);
> +				       ((reg_no + offset) << 20), SZ_1M);

^^^ You're leaking this, now that you call rockchip_cfg_atu() every
time you resume.

>  	return err;
>  }
> +
> +static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
> +{
> +	u32 value;
> +	int err;
> +
> +	/* send PME_TURN_OFF message */
> +	writel(0x0, rockchip->msg_region + PCIE_RC_SEND_PME_OFF);
> +
> +	/* read LTSSM and wait for falling into L2 link state */
> +	err = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_DEBUG_OUT_0,
> +				 value, PCIE_LINK_IS_L2(value), 20,
> +				 jiffies_to_usecs(5 * HZ));

You really want to wait a whole 5 seconds for this? Last I saw, you were
doing testing with about 500ms or less. As I read the spec, there's cap
on per-device time to ACK the request, and I recall that was on the
order of 10s of milliseconds. But technically that can add up if you
have a large hierarchy of devices attached...

> +	if (err) {
> +		dev_err(rockchip->dev, "PCIe link enter L2 timeout!\n");
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rockchip_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> +	int ret;
> +
> +	/* disable core and cli int since we don't need to ack PME_ACK */
> +	rockchip_pcie_write(rockchip, (PCIE_CLIENT_INT_CLI << 16) |
> +			    PCIE_CLIENT_INT_CLI, PCIE_CLIENT_INT_MASK);
> +	rockchip_pcie_write(rockchip, (u32)PCIE_CORE_INT, PCIE_CORE_INT_MASK);
> +
> +	ret = rockchip_pcie_wait_l2(rockchip);
> +	if (ret)
> +		return ret;

You leave core and client interrupts masked if you timeout here?

Brian

> +
> +	phy_power_off(rockchip->phy);
> +	phy_exit(rockchip->phy);
> +
> +	clk_disable_unprepare(rockchip->clk_pcie_pm);
> +	clk_disable_unprepare(rockchip->hclk_pcie);
> +	clk_disable_unprepare(rockchip->aclk_perf_pcie);
> +	clk_disable_unprepare(rockchip->aclk_pcie);
> +
> +	return ret;
> +}
> +
> +static int rockchip_pcie_resume_noirq(struct device *dev)
> +{
> +	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> +	int err;
> +
> +	clk_prepare_enable(rockchip->clk_pcie_pm);
> +	clk_prepare_enable(rockchip->hclk_pcie);
> +	clk_prepare_enable(rockchip->aclk_perf_pcie);
> +	clk_prepare_enable(rockchip->aclk_pcie);
> +
> +	err = rockchip_pcie_init_port(rockchip);
> +	if (err)
> +		return err;
> +
> +	err = rockchip_cfg_atu(rockchip);
> +	if (err)
> +		return err;
> +
> +	/* Need this to enter L1 again */
> +	rockchip_pcie_update_txcredit_mui(rockchip);
> +	rockchip_pcie_enable_interrupts(rockchip);
> +
> +	return 0;
> +}
> +
>  static int rockchip_pcie_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_pcie *rockchip;
> @@ -1228,6 +1308,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	if (!rockchip)
>  		return -ENOMEM;
>  
> +	platform_set_drvdata(pdev, rockchip);
> +
>  	rockchip->dev = dev;
>  
>  	err = rockchip_pcie_parse_dt(rockchip);
> @@ -1352,6 +1434,11 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	return err;
>  }
>  
> +static const struct dev_pm_ops rockchip_pcie_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rockchip_pcie_suspend_noirq,
> +				      rockchip_pcie_resume_noirq)
> +};
> +
>  static const struct of_device_id rockchip_pcie_of_match[] = {
>  	{ .compatible = "rockchip,rk3399-pcie", },
>  	{}
> @@ -1361,6 +1448,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	.driver = {
>  		.name = "rockchip-pcie",
>  		.of_match_table = rockchip_pcie_of_match,
> +		.pm = &rockchip_pcie_pm_ops,
>  	},
>  	.probe = rockchip_pcie_probe,
>  
> -- 
> 1.9.1
> 
> 

Re: [PATCH 3/3] PCI: rockchip: Add system PM support

[Brian Norris]

On Wed, Nov 23, 2016 at 09:19:13AM +0800, Shawn Lin wrote:
> This patch adds system PM support for Rockchip's RC.
> For pre S3, the EP is configured into D3 state which guarantees
> the link state should be in L1. So we could send PME_Turn_Off message
> to the EP and wait for its ACK to make the link state into L2 or L3
> without the aux-supply. This could help save more power which I think
> should be very important for mobile devices.
> 
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> 
>  drivers/pci/host/pcie-rockchip.c | 90 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index 71d056d..720535b 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -20,6 +20,7 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
> +#include <linux/iopoll.h>
>  #include <linux/irq.h>
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
> @@ -55,6 +56,10 @@
>  #define   PCIE_CLIENT_MODE_RC		  HIWORD_UPDATE_BIT(0x0040)
>  #define   PCIE_CLIENT_GEN_SEL_1		  HIWORD_UPDATE(0x0080, 0)
>  #define   PCIE_CLIENT_GEN_SEL_2		  HIWORD_UPDATE_BIT(0x0080)
> +#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
> +#define   PCIE_CLIENT_DEBUG_LTSSM_L2		0x19
>  #define PCIE_CLIENT_BASIC_STATUS1	(PCIE_CLIENT_BASE + 0x48)
>  #define   PCIE_CLIENT_LINK_STATUS_UP		0x00300000
>  #define   PCIE_CLIENT_LINK_STATUS_MASK		0x00300000
> @@ -173,6 +178,7 @@
>  
>  #define MAX_AXI_IB_ROOTPORT_REGION_NUM		3
>  #define MIN_AXI_ADDR_BITS_PASSED		8
> +#define PCIE_RC_SEND_PME_OFF			0x11960
>  #define ROCKCHIP_VENDOR_ID			0x1d87
>  #define PCIE_ECAM_BUS(x)			(((x) & 0xff) << 20)
>  #define PCIE_ECAM_DEV(x)			(((x) & 0x1f) << 15)
> @@ -181,6 +187,9 @@
>  #define PCIE_ECAM_ADDR(bus, dev, func, reg) \
>  	  (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
>  	   PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
> +#define PCIE_LINK_IS_L2(x) \
> +		((x & PCIE_CLIENT_DEBUG_LTSSM_MASK) == \

Wrap the 'x' in parentheses, for safety in case the caller passes
something complicated.

> +		 PCIE_CLIENT_DEBUG_LTSSM_L2)
>  
>  #define RC_REGION_0_ADDR_TRANS_H		0x00000000
>  #define RC_REGION_0_ADDR_TRANS_L		0x00000000
> @@ -1205,9 +1214,80 @@ static int rockchip_cfg_atu(struct rockchip_pcie *rockchip)
>  				  AXI_WRAPPER_NOR_MSG,
>  				  20 - 1, 0, 0);
>  	rockchip->msg_region = ioremap(rockchip->mem_bus_addr +
> -				       ((reg_no - 1) << 20), SZ_1M);
> +				       ((reg_no + offset) << 20), SZ_1M);

^^^ You're leaking this, now that you call rockchip_cfg_atu() every
time you resume.

>  	return err;
>  }
> +
> +static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
> +{
> +	u32 value;
> +	int err;
> +
> +	/* send PME_TURN_OFF message */
> +	writel(0x0, rockchip->msg_region + PCIE_RC_SEND_PME_OFF);
> +
> +	/* read LTSSM and wait for falling into L2 link state */
> +	err = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_DEBUG_OUT_0,
> +				 value, PCIE_LINK_IS_L2(value), 20,
> +				 jiffies_to_usecs(5 * HZ));

You really want to wait a whole 5 seconds for this? Last I saw, you were
doing testing with about 500ms or less. As I read the spec, there's cap
on per-device time to ACK the request, and I recall that was on the
order of 10s of milliseconds. But technically that can add up if you
have a large hierarchy of devices attached...

> +	if (err) {
> +		dev_err(rockchip->dev, "PCIe link enter L2 timeout!\n");
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rockchip_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> +	int ret;
> +
> +	/* disable core and cli int since we don't need to ack PME_ACK */
> +	rockchip_pcie_write(rockchip, (PCIE_CLIENT_INT_CLI << 16) |
> +			    PCIE_CLIENT_INT_CLI, PCIE_CLIENT_INT_MASK);
> +	rockchip_pcie_write(rockchip, (u32)PCIE_CORE_INT, PCIE_CORE_INT_MASK);
> +
> +	ret = rockchip_pcie_wait_l2(rockchip);
> +	if (ret)
> +		return ret;

You leave core and client interrupts masked if you timeout here?

Brian

> +
> +	phy_power_off(rockchip->phy);
> +	phy_exit(rockchip->phy);
> +
> +	clk_disable_unprepare(rockchip->clk_pcie_pm);
> +	clk_disable_unprepare(rockchip->hclk_pcie);
> +	clk_disable_unprepare(rockchip->aclk_perf_pcie);
> +	clk_disable_unprepare(rockchip->aclk_pcie);
> +
> +	return ret;
> +}
> +
> +static int rockchip_pcie_resume_noirq(struct device *dev)
> +{
> +	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> +	int err;
> +
> +	clk_prepare_enable(rockchip->clk_pcie_pm);
> +	clk_prepare_enable(rockchip->hclk_pcie);
> +	clk_prepare_enable(rockchip->aclk_perf_pcie);
> +	clk_prepare_enable(rockchip->aclk_pcie);
> +
> +	err = rockchip_pcie_init_port(rockchip);
> +	if (err)
> +		return err;
> +
> +	err = rockchip_cfg_atu(rockchip);
> +	if (err)
> +		return err;
> +
> +	/* Need this to enter L1 again */
> +	rockchip_pcie_update_txcredit_mui(rockchip);
> +	rockchip_pcie_enable_interrupts(rockchip);
> +
> +	return 0;
> +}
> +
>  static int rockchip_pcie_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_pcie *rockchip;
> @@ -1228,6 +1308,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	if (!rockchip)
>  		return -ENOMEM;
>  
> +	platform_set_drvdata(pdev, rockchip);
> +
>  	rockchip->dev = dev;
>  
>  	err = rockchip_pcie_parse_dt(rockchip);
> @@ -1352,6 +1434,11 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	return err;
>  }
>  
> +static const struct dev_pm_ops rockchip_pcie_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rockchip_pcie_suspend_noirq,
> +				      rockchip_pcie_resume_noirq)
> +};
> +
>  static const struct of_device_id rockchip_pcie_of_match[] = {
>  	{ .compatible = "rockchip,rk3399-pcie", },
>  	{}
> @@ -1361,6 +1448,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	.driver = {
>  		.name = "rockchip-pcie",
>  		.of_match_table = rockchip_pcie_of_match,
> +		.pm = &rockchip_pcie_pm_ops,
>  	},
>  	.probe = rockchip_pcie_probe,
>  
> -- 
> 1.9.1
> 
> 

Re: [PATCH 1/3] PCI: rockchip: split out rockchip_cfg_atu

[Shawn Lin]

在 2016/11/23 9:59, Brian Norris 写道:
> On Wed, Nov 23, 2016 at 09:19:11AM +0800, Shawn Lin wrote:
>> We split out a new function, rockchip_cfg_atu, in order to
>> re-configure the atu when missing these information after
>> wakeup from S3.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>>  drivers/pci/host/pcie-rockchip.c | 119 ++++++++++++++++++++++-----------------
>>  1 file changed, 68 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
>> index b55037a..19399fc 100644
>> --- a/drivers/pci/host/pcie-rockchip.c
>> +++ b/drivers/pci/host/pcie-rockchip.c
>> @@ -169,6 +169,7 @@
>>  #define IB_ROOT_PORT_REG_SIZE_SHIFT		3
>>  #define AXI_WRAPPER_IO_WRITE			0x6
>>  #define AXI_WRAPPER_MEM_WRITE			0x2
>> +#define AXI_WRAPPER_NOR_MSG			0xc
>
> You're also adding this 'normal message' support in a refactoring patch.
> This should be in another patch -- preferably patch 3 I think.
>

okay.

>>
>>  #define MAX_AXI_IB_ROOTPORT_REGION_NUM		3
>>  #define MIN_AXI_ADDR_BITS_PASSED		8
>> @@ -210,6 +211,13 @@ struct rockchip_pcie {
>>  	int	link_gen;
>>  	struct	device *dev;
>>  	struct	irq_domain *irq_domain;
>> +	u32     io_size;
>> +	int     offset;
>> +	phys_addr_t io_bus_addr;
>> +	int     msg_region_nr;
>> +	void	__iomem *msg_region;
>> +	u32     mem_size;
>> +	phys_addr_t mem_bus_addr;
>>  };
>>
>>  static u32 rockchip_pcie_read(struct rockchip_pcie *rockchip, u32 reg)
>> @@ -1149,6 +1157,57 @@ static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie *rockchip,
>>  	return 0;
>>  }
>>
>> +static int rockchip_cfg_atu(struct rockchip_pcie *rockchip)
>> +{
>> +	int offset;
>> +	int err;
>> +	int reg_no;
>> +
>> +	for (reg_no = 0; reg_no < (rockchip->mem_size >> 20); reg_no++) {
>> +		err = rockchip_pcie_prog_ob_atu(rockchip, reg_no + 1,
>> +						AXI_WRAPPER_MEM_WRITE,
>> +						20 - 1,
>> +						rockchip->mem_bus_addr +
>> +						(reg_no << 20),
>> +						0);
>> +		if (err) {
>> +			dev_err(rockchip->dev,
>> +					"program RC mem outbound ATU failed\n");
>> +			return err;
>> +		}
>> +	}
>> +
>> +	err = rockchip_pcie_prog_ib_atu(rockchip, 2, 32 - 1, 0x0, 0);
>> +	if (err) {
>> +		dev_err(rockchip->dev, "program RC mem inbound ATU failed\n");
>> +		return err;
>> +	}
>> +
>> +	offset = rockchip->mem_size >> 20;
>> +	for (reg_no = 0; reg_no < (rockchip->io_size >> 20); reg_no++) {
>> +		err = rockchip_pcie_prog_ob_atu(rockchip,
>> +						reg_no + 1 + offset,
>> +						AXI_WRAPPER_IO_WRITE,
>> +						20 - 1,
>> +						rockchip->io_bus_addr +
>> +						(reg_no << 20),
>> +						0);
>> +		if (err) {
>> +			dev_err(rockchip->dev,
>> +					"program RC io outbound ATU failed\n");
>> +			return err;
>> +		}
>> +	}
>> +
>> +	/* assign message regions */
>> +	rockchip->msg_region_nr = reg_no + 1 + offset;
>> +	rockchip_pcie_prog_ob_atu(rockchip, reg_no + 1 + offset,
>> +				  AXI_WRAPPER_NOR_MSG,
>> +				  20 - 1, 0, 0);
>
> Same here.
>
>> +	rockchip->msg_region = ioremap(rockchip->mem_bus_addr +
>> +				       ((reg_no - 1) << 20), SZ_1M);
>
> (And here.)
>
> ioremap() can fail; check for NULL.
>

Sure.

> Also, when you start reusing rockchip_cfg_atu() in patch 3, you'll be
> leaking virtual address space, as you'll keep remapping this every time.

How about just check if rockchip->msg_region was already mapped?
Otherwise we don't remap it again when calling rockchip_cfg_atu.

> You should straighten that out. Either some kind of check for
> 'if (!rockchip->msg_region)', or just do the map/unmap where it's
> actually used (in patch 3).

Should we really need to unmap it? As this driver won't be a module and
I think it's okay to keep the rockchip->msg_region always.

>
> Brian
>
>> +	return err;
>> +}
>>  static int rockchip_pcie_probe(struct platform_device *pdev)
>>  {
>>  	struct rockchip_pcie *rockchip;
>> @@ -1158,13 +1217,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>>  	resource_size_t io_base;
>>  	struct resource	*mem;
>>  	struct resource	*io;
>> -	phys_addr_t io_bus_addr = 0;
>> -	u32 io_size;
>> -	phys_addr_t mem_bus_addr = 0;
>> -	u32 mem_size = 0;
>> -	int reg_no;
>>  	int err;
>> -	int offset;
>>
>>  	LIST_HEAD(res);
>>
>> @@ -1231,14 +1284,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>>  		goto err_vpcie;
>>
>>  	/* Get the I/O and memory ranges from DT */
>> -	io_size = 0;
>>  	resource_list_for_each_entry(win, &res) {
>>  		switch (resource_type(win->res)) {
>>  		case IORESOURCE_IO:
>>  			io = win->res;
>>  			io->name = "I/O";
>> -			io_size = resource_size(io);
>> -			io_bus_addr = io->start - win->offset;
>> +			rockchip->io_size = resource_size(io);
>> +			rockchip->io_bus_addr = io->start - win->offset;
>>  			err = pci_remap_iospace(io, io_base);
>>  			if (err) {
>>  				dev_warn(dev, "error %d: failed to map resource %pR\n",
>> @@ -1249,8 +1301,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>>  		case IORESOURCE_MEM:
>>  			mem = win->res;
>>  			mem->name = "MEM";
>> -			mem_size = resource_size(mem);
>> -			mem_bus_addr = mem->start - win->offset;
>> +			rockchip->mem_size = resource_size(mem);
>> +			rockchip->mem_bus_addr = mem->start - win->offset;
>>  			break;
>>  		case IORESOURCE_BUS:
>>  			rockchip->root_bus_nr = win->res->start;
>> @@ -1260,49 +1312,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>>  		}
>>  	}
>>
>> -	if (mem_size) {
>> -		for (reg_no = 0; reg_no < (mem_size >> 20); reg_no++) {
>> -			err = rockchip_pcie_prog_ob_atu(rockchip, reg_no + 1,
>> -							AXI_WRAPPER_MEM_WRITE,
>> -							20 - 1,
>> -							mem_bus_addr +
>> -							(reg_no << 20),
>> -							0);
>> -			if (err) {
>> -				dev_err(dev, "program RC mem outbound ATU failed\n");
>> -				goto err_vpcie;
>> -			}
>> -		}
>> -	}
>> -
>> -	err = rockchip_pcie_prog_ib_atu(rockchip, 2, 32 - 1, 0x0, 0);
>> -	if (err) {
>> -		dev_err(dev, "program RC mem inbound ATU failed\n");
>> +	err = rockchip_cfg_atu(rockchip);
>> +	if (err)
>>  		goto err_vpcie;
>> -	}
>> -
>> -	offset = mem_size >> 20;
>> -
>> -	if (io_size) {
>> -		for (reg_no = 0; reg_no < (io_size >> 20); reg_no++) {
>> -			err = rockchip_pcie_prog_ob_atu(rockchip,
>> -							reg_no + 1 + offset,
>> -							AXI_WRAPPER_IO_WRITE,
>> -							20 - 1,
>> -							io_bus_addr +
>> -							(reg_no << 20),
>> -							0);
>> -			if (err) {
>> -				dev_err(dev, "program RC io outbound ATU failed\n");
>> -				goto err_vpcie;
>> -			}
>> -		}
>> -	}
>> -
>>  	bus = pci_scan_root_bus(&pdev->dev, 0, &rockchip_pcie_ops, rockchip, &res);
>>  	if (!bus) {
>>  		err = -ENOMEM;
>> -		goto err_vpcie;
>> +		goto err_scan;
>>  	}
>>
>>  	pci_bus_size_bridges(bus);
>> @@ -1315,7 +1331,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>>  	dev_warn(dev, "only 32-bit config accesses supported; smaller writes may corrupt adjacent RW1C fields\n");
>>
>>  	return err;
>> -
>> +err_scan:
>> +	iounmap(rockchip->msg_region);
>>  err_vpcie:
>>  	if (!IS_ERR(rockchip->vpcie3v3))
>>  		regulator_disable(rockchip->vpcie3v3);
>> --
>> 1.9.1
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Best Regards
Shawn Lin

Re: [PATCH 1/3] PCI: rockchip: split out rockchip_cfg_atu

[Brian Norris]

On Wed, Nov 23, 2016 at 10:15:16AM +0800, Shawn Lin wrote:
> 在 2016/11/23 9:59, Brian Norris 写道:
> >On Wed, Nov 23, 2016 at 09:19:11AM +0800, Shawn Lin wrote:
> >>diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> >>index b55037a..19399fc 100644
> >>--- a/drivers/pci/host/pcie-rockchip.c
> >>+++ b/drivers/pci/host/pcie-rockchip.c

...

> >>+	rockchip->msg_region = ioremap(rockchip->mem_bus_addr +
> >>+				       ((reg_no - 1) << 20), SZ_1M);
> >
> >(And here.)
> >
> >ioremap() can fail; check for NULL.
> >
> 
> Sure.
> 
> >Also, when you start reusing rockchip_cfg_atu() in patch 3, you'll be
> >leaking virtual address space, as you'll keep remapping this every time.
> 
> How about just check if rockchip->msg_region was already mapped?
> Otherwise we don't remap it again when calling rockchip_cfg_atu.

That'd work, even if it's a little awkward. That's basically one of my
suggestions below.

> >You should straighten that out. Either some kind of check for
> >'if (!rockchip->msg_region)', or just do the map/unmap where it's
> >actually used (in patch 3).
> 
> Should we really need to unmap it? As this driver won't be a module and
> I think it's okay to keep the rockchip->msg_region always.

No, I guess we don't really need to unmap it. I just meant, you could
map/unmap every time you use it, or make sure you just map it once (and
only once).

Also (if it helps), you could use devm_ioremap(), in case you ever do
make it removable.

Brian

Re: [PATCH 1/3] PCI: rockchip: split out rockchip_cfg_atu

[Shawn Lin]

在 2016/11/23 10:29, Brian Norris 写道:
> On Wed, Nov 23, 2016 at 10:15:16AM +0800, Shawn Lin wrote:
>> 在 2016/11/23 9:59, Brian Norris 写道:
>>> On Wed, Nov 23, 2016 at 09:19:11AM +0800, Shawn Lin wrote:
>>>> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
>>>> index b55037a..19399fc 100644
>>>> --- a/drivers/pci/host/pcie-rockchip.c
>>>> +++ b/drivers/pci/host/pcie-rockchip.c
>
> ...
>
>>>> +	rockchip->msg_region = ioremap(rockchip->mem_bus_addr +
>>>> +				       ((reg_no - 1) << 20), SZ_1M);
>>>
>>> (And here.)
>>>
>>> ioremap() can fail; check for NULL.
>>>
>>
>> Sure.
>>
>>> Also, when you start reusing rockchip_cfg_atu() in patch 3, you'll be
>>> leaking virtual address space, as you'll keep remapping this every time.
>>
>> How about just check if rockchip->msg_region was already mapped?
>> Otherwise we don't remap it again when calling rockchip_cfg_atu.
>
> That'd work, even if it's a little awkward. That's basically one of my
> suggestions below.
>
>>> You should straighten that out. Either some kind of check for
>>> 'if (!rockchip->msg_region)', or just do the map/unmap where it's
>>> actually used (in patch 3).
>>
>> Should we really need to unmap it? As this driver won't be a module and
>> I think it's okay to keep the rockchip->msg_region always.
>
> No, I guess we don't really need to unmap it. I just meant, you could
> map/unmap every time you use it, or make sure you just map it once (and
> only once).
>

Got it.

> Also (if it helps), you could use devm_ioremap(), in case you ever do
> make it removable.

That sounds good. :)

>
> Brian
>
>
>


-- 
Best Regards
Shawn Lin

Re: [PATCH 3/3] PCI: rockchip: Add system PM support

[Shawn Lin]

在 2016/11/23 10:08, Brian Norris 写道:
> On Wed, Nov 23, 2016 at 09:19:13AM +0800, Shawn Lin wrote:
>> This patch adds system PM support for Rockchip's RC.
>> For pre S3, the EP is configured into D3 state which guarantees
>> the link state should be in L1. So we could send PME_Turn_Off message
>> to the EP and wait for its ACK to make the link state into L2 or L3
>> without the aux-supply. This could help save more power which I think
>> should be very important for mobile devices.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>>  drivers/pci/host/pcie-rockchip.c | 90 +++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 89 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
>> index 71d056d..720535b 100644
>> --- a/drivers/pci/host/pcie-rockchip.c
>> +++ b/drivers/pci/host/pcie-rockchip.c
>> @@ -20,6 +20,7 @@
>>  #include <linux/gpio/consumer.h>
>>  #include <linux/init.h>
>>  #include <linux/interrupt.h>
>> +#include <linux/iopoll.h>
>>  #include <linux/irq.h>
>>  #include <linux/irqchip/chained_irq.h>
>>  #include <linux/irqdomain.h>
>> @@ -55,6 +56,10 @@
>>  #define   PCIE_CLIENT_MODE_RC		  HIWORD_UPDATE_BIT(0x0040)
>>  #define   PCIE_CLIENT_GEN_SEL_1		  HIWORD_UPDATE(0x0080, 0)
>>  #define   PCIE_CLIENT_GEN_SEL_2		  HIWORD_UPDATE_BIT(0x0080)
>> +#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
>> +#define   PCIE_CLIENT_DEBUG_LTSSM_L2		0x19
>>  #define PCIE_CLIENT_BASIC_STATUS1	(PCIE_CLIENT_BASE + 0x48)
>>  #define   PCIE_CLIENT_LINK_STATUS_UP		0x00300000
>>  #define   PCIE_CLIENT_LINK_STATUS_MASK		0x00300000
>> @@ -173,6 +178,7 @@
>>
>>  #define MAX_AXI_IB_ROOTPORT_REGION_NUM		3
>>  #define MIN_AXI_ADDR_BITS_PASSED		8
>> +#define PCIE_RC_SEND_PME_OFF			0x11960
>>  #define ROCKCHIP_VENDOR_ID			0x1d87
>>  #define PCIE_ECAM_BUS(x)			(((x) & 0xff) << 20)
>>  #define PCIE_ECAM_DEV(x)			(((x) & 0x1f) << 15)
>> @@ -181,6 +187,9 @@
>>  #define PCIE_ECAM_ADDR(bus, dev, func, reg) \
>>  	  (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
>>  	   PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
>> +#define PCIE_LINK_IS_L2(x) \
>> +		((x & PCIE_CLIENT_DEBUG_LTSSM_MASK) == \
>
> Wrap the 'x' in parentheses, for safety in case the caller passes
> something complicated.
>

Will fix.

>> +		 PCIE_CLIENT_DEBUG_LTSSM_L2)
>>
>>  #define RC_REGION_0_ADDR_TRANS_H		0x00000000
>>  #define RC_REGION_0_ADDR_TRANS_L		0x00000000
>> @@ -1205,9 +1214,80 @@ static int rockchip_cfg_atu(struct rockchip_pcie *rockchip)
>>  				  AXI_WRAPPER_NOR_MSG,
>>  				  20 - 1, 0, 0);
>>  	rockchip->msg_region = ioremap(rockchip->mem_bus_addr +
>> -				       ((reg_no - 1) << 20), SZ_1M);
>> +				       ((reg_no + offset) << 20), SZ_1M);
>
> ^^^ You're leaking this, now that you call rockchip_cfg_atu() every
> time you resume.
>
>>  	return err;
>>  }
>> +
>> +static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
>> +{
>> +	u32 value;
>> +	int err;
>> +
>> +	/* send PME_TURN_OFF message */
>> +	writel(0x0, rockchip->msg_region + PCIE_RC_SEND_PME_OFF);
>> +
>> +	/* read LTSSM and wait for falling into L2 link state */
>> +	err = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_DEBUG_OUT_0,
>> +				 value, PCIE_LINK_IS_L2(value), 20,
>> +				 jiffies_to_usecs(5 * HZ));
>
> You really want to wait a whole 5 seconds for this? Last I saw, you were
> doing testing with about 500ms or less. As I read the spec, there's cap
> on per-device time to ACK the request, and I recall that was on the
> order of 10s of milliseconds. But technically that can add up if you
> have a large hierarchy of devices attached...
>

I have no very clear thought about how long we should set up the
timeout for PME_ACK. As you point out that a large hierarchy of devices
need quite a long time I guess.

A possible work for PCIe core is walk through the hierarchy of devices
and calculate the max ACK timeout(including the latency of HUB).. But
I guess ACPI-base platforms don't need linux-pci to handle L2 stuff at
S3 at all, instead it will be handled by firmware, so still I don't
know how they will calculate it.

>> +	if (err) {
>> +		dev_err(rockchip->dev, "PCIe link enter L2 timeout!\n");
>> +		return err;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_pcie_suspend_noirq(struct device *dev)
>> +{
>> +	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	/* disable core and cli int since we don't need to ack PME_ACK */
>> +	rockchip_pcie_write(rockchip, (PCIE_CLIENT_INT_CLI << 16) |
>> +			    PCIE_CLIENT_INT_CLI, PCIE_CLIENT_INT_MASK);
>> +	rockchip_pcie_write(rockchip, (u32)PCIE_CORE_INT, PCIE_CORE_INT_MASK);
>> +
>> +	ret = rockchip_pcie_wait_l2(rockchip);
>> +	if (ret)
>> +		return ret;
>
> You leave core and client interrupts masked if you timeout here?
>

Woops, will fix it.

> Brian
>
>> +
>> +	phy_power_off(rockchip->phy);
>> +	phy_exit(rockchip->phy);
>> +
>> +	clk_disable_unprepare(rockchip->clk_pcie_pm);
>> +	clk_disable_unprepare(rockchip->hclk_pcie);
>> +	clk_disable_unprepare(rockchip->aclk_perf_pcie);
>> +	clk_disable_unprepare(rockchip->aclk_pcie);
>> +
>> +	return ret;
>> +}
>> +
>> +static int rockchip_pcie_resume_noirq(struct device *dev)
>> +{
>> +	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
>> +	int err;
>> +
>> +	clk_prepare_enable(rockchip->clk_pcie_pm);
>> +	clk_prepare_enable(rockchip->hclk_pcie);
>> +	clk_prepare_enable(rockchip->aclk_perf_pcie);
>> +	clk_prepare_enable(rockchip->aclk_pcie);
>> +
>> +	err = rockchip_pcie_init_port(rockchip);
>> +	if (err)
>> +		return err;
>> +
>> +	err = rockchip_cfg_atu(rockchip);
>> +	if (err)
>> +		return err;
>> +
>> +	/* Need this to enter L1 again */
>> +	rockchip_pcie_update_txcredit_mui(rockchip);
>> +	rockchip_pcie_enable_interrupts(rockchip);
>> +
>> +	return 0;
>> +}
>> +
>>  static int rockchip_pcie_probe(struct platform_device *pdev)
>>  {
>>  	struct rockchip_pcie *rockchip;
>> @@ -1228,6 +1308,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>>  	if (!rockchip)
>>  		return -ENOMEM;
>>
>> +	platform_set_drvdata(pdev, rockchip);
>> +
>>  	rockchip->dev = dev;
>>
>>  	err = rockchip_pcie_parse_dt(rockchip);
>> @@ -1352,6 +1434,11 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>>  	return err;
>>  }
>>
>> +static const struct dev_pm_ops rockchip_pcie_pm_ops = {
>> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rockchip_pcie_suspend_noirq,
>> +				      rockchip_pcie_resume_noirq)
>> +};
>> +
>>  static const struct of_device_id rockchip_pcie_of_match[] = {
>>  	{ .compatible = "rockchip,rk3399-pcie", },
>>  	{}
>> @@ -1361,6 +1448,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>>  	.driver = {
>>  		.name = "rockchip-pcie",
>>  		.of_match_table = rockchip_pcie_of_match,
>> +		.pm = &rockchip_pcie_pm_ops,
>>  	},
>>  	.probe = rockchip_pcie_probe,
>>
>> --
>> 1.9.1
>>
>>
>
>
>


-- 
Best Regards
Shawn Lin

Re: [PATCH 3/3] PCI: rockchip: Add system PM support

[Brian Norris]

On Wed, Nov 23, 2016 at 10:39:30AM +0800, Shawn Lin wrote:
> 在 2016/11/23 10:08, Brian Norris 写道:
> >On Wed, Nov 23, 2016 at 09:19:13AM +0800, Shawn Lin wrote:
> >>diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> >>index 71d056d..720535b 100644
> >>--- a/drivers/pci/host/pcie-rockchip.c
> >>+++ b/drivers/pci/host/pcie-rockchip.c

...

> >>+static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
> >>+{
> >>+	u32 value;
> >>+	int err;
> >>+
> >>+	/* send PME_TURN_OFF message */
> >>+	writel(0x0, rockchip->msg_region + PCIE_RC_SEND_PME_OFF);
> >>+
> >>+	/* read LTSSM and wait for falling into L2 link state */
> >>+	err = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_DEBUG_OUT_0,
> >>+				 value, PCIE_LINK_IS_L2(value), 20,
> >>+				 jiffies_to_usecs(5 * HZ));
> >
> >You really want to wait a whole 5 seconds for this? Last I saw, you were
> >doing testing with about 500ms or less. As I read the spec, there's cap
> >on per-device time to ACK the request, and I recall that was on the
> >order of 10s of milliseconds. But technically that can add up if you
> >have a large hierarchy of devices attached...
> >
> 
> I have no very clear thought about how long we should set up the
> timeout for PME_ACK. As you point out that a large hierarchy of devices
> need quite a long time I guess.
> 
> A possible work for PCIe core is walk through the hierarchy of devices
> and calculate the max ACK timeout(including the latency of HUB).. But
> I guess ACPI-base platforms don't need linux-pci to handle L2 stuff at
> S3 at all, instead it will be handled by firmware, so still I don't
> know how they will calculate it.

I'm not sure if that's necessary. I was just curious on how this got
determined, when it seemed that 500ms was plenty.

We shouldn't be hitting this (and if we do, then that means we'd
probably need to reassess anyway), so maybe "too large" is fine.

Brian

Re: [PATCH 3/3] PCI: rockchip: Add system PM support

[Shawn Lin]

在 2016/11/23 10:45, Brian Norris 写道:
> On Wed, Nov 23, 2016 at 10:39:30AM +0800, Shawn Lin wrote:
>> 在 2016/11/23 10:08, Brian Norris 写道:
>>> On Wed, Nov 23, 2016 at 09:19:13AM +0800, Shawn Lin wrote:
>>>> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
>>>> index 71d056d..720535b 100644
>>>> --- a/drivers/pci/host/pcie-rockchip.c
>>>> +++ b/drivers/pci/host/pcie-rockchip.c
>
> ...
>
>>>> +static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
>>>> +{
>>>> +	u32 value;
>>>> +	int err;
>>>> +
>>>> +	/* send PME_TURN_OFF message */
>>>> +	writel(0x0, rockchip->msg_region + PCIE_RC_SEND_PME_OFF);
>>>> +
>>>> +	/* read LTSSM and wait for falling into L2 link state */
>>>> +	err = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_DEBUG_OUT_0,
>>>> +				 value, PCIE_LINK_IS_L2(value), 20,
>>>> +				 jiffies_to_usecs(5 * HZ));
>>>
>>> You really want to wait a whole 5 seconds for this? Last I saw, you were
>>> doing testing with about 500ms or less. As I read the spec, there's cap
>>> on per-device time to ACK the request, and I recall that was on the
>>> order of 10s of milliseconds. But technically that can add up if you
>>> have a large hierarchy of devices attached...
>>>
>>
>> I have no very clear thought about how long we should set up the
>> timeout for PME_ACK. As you point out that a large hierarchy of devices
>> need quite a long time I guess.
>>
>> A possible work for PCIe core is walk through the hierarchy of devices
>> and calculate the max ACK timeout(including the latency of HUB).. But
>> I guess ACPI-base platforms don't need linux-pci to handle L2 stuff at
>> S3 at all, instead it will be handled by firmware, so still I don't
>> know how they will calculate it.
>
> I'm not sure if that's necessary. I was just curious on how this got
> determined, when it seemed that 500ms was plenty.
>

Refer to https://lists.launchpad.net/kernel-packages/msg123315.html

"When the host (Linux/Driver) sends the PME_TURN_OFF message it should
wait for PME_TO_ACK from the device. Microsoft Windows versions 
typically wait for 5 seconds."

This is the reason for why I added 5s for timeout.

> We shouldn't be hitting this (and if we do, then that means we'd
> probably need to reassess anyway), so maybe "too large" is fine.
>
> Brian
>
>
>


-- 
Best Regards
Shawn Lin

Re: [PATCH 3/3] PCI: rockchip: Add system PM support

[Brian Norris]

On Wed, Nov 23, 2016 at 10:51:50AM +0800, Shawn Lin wrote:
> 在 2016/11/23 10:45, Brian Norris 写道:
> >I'm not sure if that's necessary. I was just curious on how this got
> >determined, when it seemed that 500ms was plenty.
> >
> 
> Refer to https://lists.launchpad.net/kernel-packages/msg123315.html
> 
> "When the host (Linux/Driver) sends the PME_TURN_OFF message it should
> wait for PME_TO_ACK from the device. Microsoft Windows versions
> typically wait for 5 seconds."
> 
> This is the reason for why I added 5s for timeout.

Huh, good find. Seems like something that could go in either the commit
message or the comments.

FWIW, that bug report seems to also be related to the question at hand
-- how to handle L2 link state entry. That bug notes that Linux "doesn't
wait for PME_TO_ACK from the device", but AFAICT, Linux PCI drivers
don't typically send PME_TURN_OFF at all.

So right now, we're just relying on our platform suspend code like this.
It'd be interesting to know if other root complexes had programmable
Message Request support like this available to the host CPU, or if they
all have something hardcoded into their BIOS/ACPI support or something
(or just don't use PME_TURN_OFF at all).

Anyway, I guess that's just a future-work question, in case we want to
consolidate support like this into the PCI core framework.

Brian