[PATCH v4 0/4] PCI: rockchip: Move PCIE_WAKE handling into rockchip pcie driver

[PATCH v4 0/4] PCI: rockchip: Move PCIE_WAKE handling into rockchip pcie driver

[Jeffy Chen]

Currently we are handling pcie wake in mrvl wifi driver. But Brian
suggests to move it into rockchip pcie driver.

Tested on my chromebook bob(with cros 4.4 kernel and mrvl wifi).


Changes in v4:
Rebase on newest for-next branch, also fix error handling by:
1e7f570a1b86 PCI: rockchip: Idle inactive PHY(s)

Changes in v3:
Fix error handling

Changes in v2:
Use dev_pm_set_dedicated_wake_irq
        -- Suggested by Brian Norris <briannorris@chromium.com>

Jeffy Chen (4):
  PCI: rockchip: Fix error handlings
  PCI: rockchip: Add support for pcie wake irq
  dt-bindings: PCI: rockchip: Add support for pcie wake irq
  arm64: dts: rockchip: Handle pcie wake in pcie driver for Gru

 .../devicetree/bindings/pci/rockchip-pcie.txt      |  20 ++-
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi       |  15 +-
 drivers/pci/host/pcie-rockchip.c                   | 179 ++++++++++++---------
 3 files changed, 126 insertions(+), 88 deletions(-)

-- 
2.11.0



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 1/4] PCI: rockchip: Fix error handlings

[Jeffy Chen]

Fix error handlings in probe & resume.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v4:
Rebase on newest for-next branch, also fix error handling by:
1e7f570a1b86 PCI: rockchip: Idle inactive PHY(s)

Changes in v3: None
Changes in v2: None

 drivers/pci/host/pcie-rockchip.c | 160 +++++++++++++++++++++------------------
 1 file changed, 87 insertions(+), 73 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 2eccd532c256..5d85ec2e2fb0 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -561,32 +561,32 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 		err = phy_init(rockchip->phys[i]);
 		if (err) {
 			dev_err(dev, "init phy%d err %d\n", i, err);
-			return err;
+			goto err_exit_phy;
 		}
 	}
 
 	err = reset_control_assert(rockchip->core_rst);
 	if (err) {
 		dev_err(dev, "assert core_rst err %d\n", err);
-		return err;
+		goto err_exit_phy;
 	}
 
 	err = reset_control_assert(rockchip->mgmt_rst);
 	if (err) {
 		dev_err(dev, "assert mgmt_rst err %d\n", err);
-		return err;
+		goto err_exit_phy;
 	}
 
 	err = reset_control_assert(rockchip->mgmt_sticky_rst);
 	if (err) {
 		dev_err(dev, "assert mgmt_sticky_rst err %d\n", err);
-		return err;
+		goto err_exit_phy;
 	}
 
 	err = reset_control_assert(rockchip->pipe_rst);
 	if (err) {
 		dev_err(dev, "assert pipe_rst err %d\n", err);
-		return err;
+		goto err_exit_phy;
 	}
 
 	udelay(10);
@@ -594,19 +594,19 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 	err = reset_control_deassert(rockchip->pm_rst);
 	if (err) {
 		dev_err(dev, "deassert pm_rst err %d\n", err);
-		return err;
+		goto err_exit_phy;
 	}
 
 	err = reset_control_deassert(rockchip->aclk_rst);
 	if (err) {
 		dev_err(dev, "deassert aclk_rst err %d\n", err);
-		return err;
+		goto err_exit_phy;
 	}
 
 	err = reset_control_deassert(rockchip->pclk_rst);
 	if (err) {
 		dev_err(dev, "deassert pclk_rst err %d\n", err);
-		return err;
+		goto err_exit_phy;
 	}
 
 	if (rockchip->link_gen == 2)
@@ -628,7 +628,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 		err = phy_power_on(rockchip->phys[i]);
 		if (err) {
 			dev_err(dev, "power on phy%d err %d\n", i, err);
-			return err;
+			goto err_power_off_phy;
 		}
 	}
 
@@ -639,25 +639,25 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 	err = reset_control_deassert(rockchip->mgmt_sticky_rst);
 	if (err) {
 		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
-		return err;
+		goto err_power_off_phy;
 	}
 
 	err = reset_control_deassert(rockchip->core_rst);
 	if (err) {
 		dev_err(dev, "deassert core_rst err %d\n", err);
-		return err;
+		goto err_power_off_phy;
 	}
 
 	err = reset_control_deassert(rockchip->mgmt_rst);
 	if (err) {
 		dev_err(dev, "deassert mgmt_rst err %d\n", err);
-		return err;
+		goto err_power_off_phy;
 	}
 
 	err = reset_control_deassert(rockchip->pipe_rst);
 	if (err) {
 		dev_err(dev, "deassert pipe_rst err %d\n", err);
-		return err;
+		goto err_power_off_phy;
 	}
 
 	/* Fix the transmitted FTS count desired to exit from L0s. */
@@ -690,7 +690,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 				 500 * USEC_PER_MSEC);
 	if (err) {
 		dev_err(dev, "PCIe link training gen1 timeout!\n");
-		return -ETIMEDOUT;
+		goto err_power_off_phy;
 	}
 
 	if (rockchip->link_gen == 2) {
@@ -751,6 +751,26 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_DCSR);
 
 	return 0;
+err_power_off_phy:
+	while (i--)
+		phy_power_off(rockchip->phys[i]);
+	i = MAX_LANE_NUM;
+err_exit_phy:
+	while (i--)
+		phy_exit(rockchip->phys[i]);
+	return err;
+}
+
+static void rockchip_pcie_deinit_phys(struct rockchip_pcie *rockchip)
+{
+	int i;
+
+	for (i = 0; i < MAX_LANE_NUM; i++) {
+		/* inactive lane is already powered off */
+		if (rockchip->legacy_phy || rockchip->lanes_map & BIT(i))
+			phy_power_off(rockchip->phys[i]);
+		phy_exit(rockchip->phys[i]);
+	}
 }
 
 static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
@@ -1131,7 +1151,7 @@ static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
 		err = regulator_enable(rockchip->vpcie12v);
 		if (err) {
 			dev_err(dev, "fail to enable vpcie12v regulator\n");
-			goto err_out;
+			return err;
 		}
 	}
 
@@ -1160,7 +1180,6 @@ static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
 	}
 
 	return 0;
-
 err_disable_1v8:
 	if (!IS_ERR(rockchip->vpcie1v8))
 		regulator_disable(rockchip->vpcie1v8);
@@ -1170,7 +1189,6 @@ static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
 err_disable_12v:
 	if (!IS_ERR(rockchip->vpcie12v))
 		regulator_disable(rockchip->vpcie12v);
-err_out:
 	return err;
 }
 
@@ -1364,7 +1382,7 @@ static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
 static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
 {
 	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
-	int ret, i;
+	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) |
@@ -1377,12 +1395,7 @@ static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
 		return ret;
 	}
 
-	for (i = 0; i < MAX_LANE_NUM; i++) {
-		/* inactive lane is already powered off */
-		if (rockchip->lanes_map & BIT(i))
-			phy_power_off(rockchip->phys[i]);
-		phy_exit(rockchip->phys[i]);
-	}
+	rockchip_pcie_deinit_phys(rockchip);
 
 	clk_disable_unprepare(rockchip->clk_pcie_pm);
 	clk_disable_unprepare(rockchip->hclk_pcie);
@@ -1391,7 +1404,6 @@ static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
 
 	if (!IS_ERR(rockchip->vpcie0v9))
 		regulator_disable(rockchip->vpcie0v9);
-
 	return ret;
 }
 
@@ -1410,43 +1422,46 @@ static int __maybe_unused rockchip_pcie_resume_noirq(struct device *dev)
 
 	err = clk_prepare_enable(rockchip->clk_pcie_pm);
 	if (err)
-		goto err_pcie_pm;
+		goto err_disable_0v9;
 
 	err = clk_prepare_enable(rockchip->hclk_pcie);
 	if (err)
-		goto err_hclk_pcie;
+		goto err_disable_clk_pcie_pm;
 
 	err = clk_prepare_enable(rockchip->aclk_perf_pcie);
 	if (err)
-		goto err_aclk_perf_pcie;
+		goto err_disable_hclk_pcie;
 
 	err = clk_prepare_enable(rockchip->aclk_pcie);
 	if (err)
-		goto err_aclk_pcie;
+		goto err_disable_aclk_perf_pcie;
 
 	err = rockchip_pcie_init_port(rockchip);
 	if (err)
-		goto err_pcie_resume;
+		goto err_disable_aclk_pcie;
 
 	err = rockchip_pcie_cfg_atu(rockchip);
 	if (err)
-		goto err_pcie_resume;
+		goto err_deinit_port;
 
 	/* Need this to enter L1 again */
 	rockchip_pcie_update_txcredit_mui(rockchip);
 	rockchip_pcie_enable_interrupts(rockchip);
 
 	return 0;
-
-err_pcie_resume:
+err_deinit_port:
+	rockchip_pcie_deinit_phys(rockchip);
+err_disable_aclk_pcie:
 	clk_disable_unprepare(rockchip->aclk_pcie);
-err_aclk_pcie:
+err_disable_aclk_perf_pcie:
 	clk_disable_unprepare(rockchip->aclk_perf_pcie);
-err_aclk_perf_pcie:
+err_disable_hclk_pcie:
 	clk_disable_unprepare(rockchip->hclk_pcie);
-err_hclk_pcie:
+err_disable_clk_pcie_pm:
 	clk_disable_unprepare(rockchip->clk_pcie_pm);
-err_pcie_pm:
+err_disable_0v9:
+	if (!IS_ERR(rockchip->vpcie0v9))
+		regulator_disable(rockchip->vpcie0v9);
 	return err;
 }
 
@@ -1483,47 +1498,47 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	err = clk_prepare_enable(rockchip->aclk_pcie);
 	if (err) {
 		dev_err(dev, "unable to enable aclk_pcie clock\n");
-		goto err_aclk_pcie;
+		return err;
 	}
 
 	err = clk_prepare_enable(rockchip->aclk_perf_pcie);
 	if (err) {
 		dev_err(dev, "unable to enable aclk_perf_pcie clock\n");
-		goto err_aclk_perf_pcie;
+		goto err_disable_aclk_pcie;
 	}
 
 	err = clk_prepare_enable(rockchip->hclk_pcie);
 	if (err) {
 		dev_err(dev, "unable to enable hclk_pcie clock\n");
-		goto err_hclk_pcie;
+		goto err_disable_aclk_perf_pcie;
 	}
 
 	err = clk_prepare_enable(rockchip->clk_pcie_pm);
 	if (err) {
 		dev_err(dev, "unable to enable hclk_pcie clock\n");
-		goto err_pcie_pm;
+		goto err_disable_hclk_pcie;
 	}
 
 	err = rockchip_pcie_set_vpcie(rockchip);
 	if (err) {
 		dev_err(dev, "failed to set vpcie regulator\n");
-		goto err_set_vpcie;
+		goto err_disable_clk_pcie_pm;
 	}
 
 	err = rockchip_pcie_init_port(rockchip);
 	if (err)
-		goto err_vpcie;
+		goto err_disable_vpcie;
 
 	rockchip_pcie_enable_interrupts(rockchip);
 
 	err = rockchip_pcie_init_irq_domain(rockchip);
 	if (err < 0)
-		goto err_vpcie;
+		goto err_deinit_port;
 
 	err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
 					       &res, &io_base);
 	if (err)
-		goto err_vpcie;
+		goto err_remove_irq_domain;
 
 	err = devm_request_pci_bus_resources(dev, &res);
 	if (err)
@@ -1561,12 +1576,12 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 
 	err = rockchip_pcie_cfg_atu(rockchip);
 	if (err)
-		goto err_free_res;
+		goto err_unmap_iospace;
 
 	rockchip->msg_region = devm_ioremap(dev, rockchip->msg_bus_addr, SZ_1M);
 	if (!rockchip->msg_region) {
 		err = -ENOMEM;
-		goto err_free_res;
+		goto err_unmap_iospace;
 	}
 
 	list_splice_init(&res, &bridge->windows);
@@ -1591,28 +1606,33 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 		pcie_bus_configure_settings(child);
 
 	pci_bus_add_devices(bus);
-	return 0;
 
+	return 0;
+err_unmap_iospace:
+	pci_unmap_iospace(rockchip->io);
 err_free_res:
 	pci_free_resource_list(&res);
-err_vpcie:
-	if (!IS_ERR(rockchip->vpcie12v))
-		regulator_disable(rockchip->vpcie12v);
-	if (!IS_ERR(rockchip->vpcie3v3))
-		regulator_disable(rockchip->vpcie3v3);
-	if (!IS_ERR(rockchip->vpcie1v8))
-		regulator_disable(rockchip->vpcie1v8);
+err_remove_irq_domain:
+	irq_domain_remove(rockchip->irq_domain);
+err_deinit_port:
+	rockchip_pcie_deinit_phys(rockchip);
+err_disable_vpcie:
 	if (!IS_ERR(rockchip->vpcie0v9))
 		regulator_disable(rockchip->vpcie0v9);
-err_set_vpcie:
+	if (!IS_ERR(rockchip->vpcie1v8))
+		regulator_disable(rockchip->vpcie1v8);
+	if (!IS_ERR(rockchip->vpcie3v3))
+		regulator_disable(rockchip->vpcie3v3);
+	if (!IS_ERR(rockchip->vpcie12v))
+		regulator_disable(rockchip->vpcie12v);
+err_disable_clk_pcie_pm:
 	clk_disable_unprepare(rockchip->clk_pcie_pm);
-err_pcie_pm:
+err_disable_hclk_pcie:
 	clk_disable_unprepare(rockchip->hclk_pcie);
-err_hclk_pcie:
+err_disable_aclk_perf_pcie:
 	clk_disable_unprepare(rockchip->aclk_perf_pcie);
-err_aclk_perf_pcie:
+err_disable_aclk_pcie:
 	clk_disable_unprepare(rockchip->aclk_pcie);
-err_aclk_pcie:
 	return err;
 }
 
@@ -1620,33 +1640,27 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
-	int i;
 
 	pci_stop_root_bus(rockchip->root_bus);
 	pci_remove_root_bus(rockchip->root_bus);
 	pci_unmap_iospace(rockchip->io);
 	irq_domain_remove(rockchip->irq_domain);
 
-	for (i = 0; i < MAX_LANE_NUM; i++) {
-		/* inactive lane is already powered off */
-		if (rockchip->lanes_map & BIT(i))
-			phy_power_off(rockchip->phys[i]);
-		phy_exit(rockchip->phys[i]);
-	}
+	rockchip_pcie_deinit_phys(rockchip);
 
 	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);
 
-	if (!IS_ERR(rockchip->vpcie12v))
-		regulator_disable(rockchip->vpcie12v);
-	if (!IS_ERR(rockchip->vpcie3v3))
-		regulator_disable(rockchip->vpcie3v3);
-	if (!IS_ERR(rockchip->vpcie1v8))
-		regulator_disable(rockchip->vpcie1v8);
 	if (!IS_ERR(rockchip->vpcie0v9))
 		regulator_disable(rockchip->vpcie0v9);
+	if (!IS_ERR(rockchip->vpcie1v8))
+		regulator_disable(rockchip->vpcie1v8);
+	if (!IS_ERR(rockchip->vpcie3v3))
+		regulator_disable(rockchip->vpcie3v3);
+	if (!IS_ERR(rockchip->vpcie12v))
+		regulator_disable(rockchip->vpcie12v);
 
 	return 0;
 }
-- 
2.11.0



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 1/4] PCI: rockchip: Fix error handlings

[Bjorn Helgaas]

On Tue, Aug 22, 2017 at 11:19:31AM +0800, Jeffy Chen wrote:
> Fix error handlings in probe & resume.

Changelog should have a little detail about *what* you're fixing.  For
example, it looks like you are powering off PHYs if the init fails.

> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v4:
> Rebase on newest for-next branch, also fix error handling by:
> 1e7f570a1b86 PCI: rockchip: Idle inactive PHY(s)
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/pci/host/pcie-rockchip.c | 160 +++++++++++++++++++++------------------
>  1 file changed, 87 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index 2eccd532c256..5d85ec2e2fb0 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -561,32 +561,32 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  		err = phy_init(rockchip->phys[i]);
>  		if (err) {
>  			dev_err(dev, "init phy%d err %d\n", i, err);
> -			return err;
> +			goto err_exit_phy;
>  		}
>  	}
>  
>  	err = reset_control_assert(rockchip->core_rst);
>  	if (err) {
>  		dev_err(dev, "assert core_rst err %d\n", err);
> -		return err;
> +		goto err_exit_phy;
>  	}
>  
>  	err = reset_control_assert(rockchip->mgmt_rst);
>  	if (err) {
>  		dev_err(dev, "assert mgmt_rst err %d\n", err);
> -		return err;
> +		goto err_exit_phy;
>  	}
>  
>  	err = reset_control_assert(rockchip->mgmt_sticky_rst);
>  	if (err) {
>  		dev_err(dev, "assert mgmt_sticky_rst err %d\n", err);
> -		return err;
> +		goto err_exit_phy;
>  	}
>  
>  	err = reset_control_assert(rockchip->pipe_rst);
>  	if (err) {
>  		dev_err(dev, "assert pipe_rst err %d\n", err);
> -		return err;
> +		goto err_exit_phy;
>  	}
>  
>  	udelay(10);
> @@ -594,19 +594,19 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  	err = reset_control_deassert(rockchip->pm_rst);
>  	if (err) {
>  		dev_err(dev, "deassert pm_rst err %d\n", err);
> -		return err;
> +		goto err_exit_phy;
>  	}
>  
>  	err = reset_control_deassert(rockchip->aclk_rst);
>  	if (err) {
>  		dev_err(dev, "deassert aclk_rst err %d\n", err);
> -		return err;
> +		goto err_exit_phy;
>  	}
>  
>  	err = reset_control_deassert(rockchip->pclk_rst);
>  	if (err) {
>  		dev_err(dev, "deassert pclk_rst err %d\n", err);
> -		return err;
> +		goto err_exit_phy;
>  	}
>  
>  	if (rockchip->link_gen == 2)
> @@ -628,7 +628,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  		err = phy_power_on(rockchip->phys[i]);
>  		if (err) {
>  			dev_err(dev, "power on phy%d err %d\n", i, err);
> -			return err;
> +			goto err_power_off_phy;
>  		}
>  	}
>  
> @@ -639,25 +639,25 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  	err = reset_control_deassert(rockchip->mgmt_sticky_rst);
>  	if (err) {
>  		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> -		return err;
> +		goto err_power_off_phy;
>  	}
>  
>  	err = reset_control_deassert(rockchip->core_rst);
>  	if (err) {
>  		dev_err(dev, "deassert core_rst err %d\n", err);
> -		return err;
> +		goto err_power_off_phy;
>  	}
>  
>  	err = reset_control_deassert(rockchip->mgmt_rst);
>  	if (err) {
>  		dev_err(dev, "deassert mgmt_rst err %d\n", err);
> -		return err;
> +		goto err_power_off_phy;
>  	}
>  
>  	err = reset_control_deassert(rockchip->pipe_rst);
>  	if (err) {
>  		dev_err(dev, "deassert pipe_rst err %d\n", err);
> -		return err;
> +		goto err_power_off_phy;
>  	}
>  
>  	/* Fix the transmitted FTS count desired to exit from L0s. */
> @@ -690,7 +690,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  				 500 * USEC_PER_MSEC);
>  	if (err) {
>  		dev_err(dev, "PCIe link training gen1 timeout!\n");
> -		return -ETIMEDOUT;
> +		goto err_power_off_phy;
>  	}
>  
>  	if (rockchip->link_gen == 2) {
> @@ -751,6 +751,26 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_DCSR);
>  
>  	return 0;
> +err_power_off_phy:
> +	while (i--)
> +		phy_power_off(rockchip->phys[i]);
> +	i = MAX_LANE_NUM;
> +err_exit_phy:
> +	while (i--)
> +		phy_exit(rockchip->phys[i]);
> +	return err;
> +}
> +
> +static void rockchip_pcie_deinit_phys(struct rockchip_pcie *rockchip)
> +{
> +	int i;
> +
> +	for (i = 0; i < MAX_LANE_NUM; i++) {
> +		/* inactive lane is already powered off */
> +		if (rockchip->legacy_phy || rockchip->lanes_map & BIT(i))
> +			phy_power_off(rockchip->phys[i]);
> +		phy_exit(rockchip->phys[i]);
> +	}
>  }
>  
>  static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
> @@ -1131,7 +1151,7 @@ static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
>  		err = regulator_enable(rockchip->vpcie12v);
>  		if (err) {
>  			dev_err(dev, "fail to enable vpcie12v regulator\n");
> -			goto err_out;
> +			return err;
>  		}
>  	}
>  
> @@ -1160,7 +1180,6 @@ static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
>  	}
>  
>  	return 0;
> -
>  err_disable_1v8:
>  	if (!IS_ERR(rockchip->vpcie1v8))
>  		regulator_disable(rockchip->vpcie1v8);
> @@ -1170,7 +1189,6 @@ static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
>  err_disable_12v:
>  	if (!IS_ERR(rockchip->vpcie12v))
>  		regulator_disable(rockchip->vpcie12v);
> -err_out:
>  	return err;
>  }
>  
> @@ -1364,7 +1382,7 @@ static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
>  static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
>  {
>  	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> -	int ret, i;
> +	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) |
> @@ -1377,12 +1395,7 @@ static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
>  		return ret;
>  	}
>  
> -	for (i = 0; i < MAX_LANE_NUM; i++) {
> -		/* inactive lane is already powered off */
> -		if (rockchip->lanes_map & BIT(i))
> -			phy_power_off(rockchip->phys[i]);
> -		phy_exit(rockchip->phys[i]);
> -	}
> +	rockchip_pcie_deinit_phys(rockchip);
>  
>  	clk_disable_unprepare(rockchip->clk_pcie_pm);
>  	clk_disable_unprepare(rockchip->hclk_pcie);
> @@ -1391,7 +1404,6 @@ static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
>  
>  	if (!IS_ERR(rockchip->vpcie0v9))
>  		regulator_disable(rockchip->vpcie0v9);
> -
>  	return ret;
>  }
>  
> @@ -1410,43 +1422,46 @@ static int __maybe_unused rockchip_pcie_resume_noirq(struct device *dev)
>  
>  	err = clk_prepare_enable(rockchip->clk_pcie_pm);
>  	if (err)
> -		goto err_pcie_pm;
> +		goto err_disable_0v9;
>  
>  	err = clk_prepare_enable(rockchip->hclk_pcie);
>  	if (err)
> -		goto err_hclk_pcie;
> +		goto err_disable_clk_pcie_pm;
>  
>  	err = clk_prepare_enable(rockchip->aclk_perf_pcie);
>  	if (err)
> -		goto err_aclk_perf_pcie;
> +		goto err_disable_hclk_pcie;
>  
>  	err = clk_prepare_enable(rockchip->aclk_pcie);
>  	if (err)
> -		goto err_aclk_pcie;
> +		goto err_disable_aclk_perf_pcie;
>  
>  	err = rockchip_pcie_init_port(rockchip);
>  	if (err)
> -		goto err_pcie_resume;
> +		goto err_disable_aclk_pcie;
>  
>  	err = rockchip_pcie_cfg_atu(rockchip);
>  	if (err)
> -		goto err_pcie_resume;
> +		goto err_deinit_port;
>  
>  	/* Need this to enter L1 again */
>  	rockchip_pcie_update_txcredit_mui(rockchip);
>  	rockchip_pcie_enable_interrupts(rockchip);
>  
>  	return 0;
> -
> -err_pcie_resume:
> +err_deinit_port:
> +	rockchip_pcie_deinit_phys(rockchip);
> +err_disable_aclk_pcie:
>  	clk_disable_unprepare(rockchip->aclk_pcie);
> -err_aclk_pcie:
> +err_disable_aclk_perf_pcie:
>  	clk_disable_unprepare(rockchip->aclk_perf_pcie);
> -err_aclk_perf_pcie:
> +err_disable_hclk_pcie:
>  	clk_disable_unprepare(rockchip->hclk_pcie);
> -err_hclk_pcie:
> +err_disable_clk_pcie_pm:
>  	clk_disable_unprepare(rockchip->clk_pcie_pm);
> -err_pcie_pm:
> +err_disable_0v9:
> +	if (!IS_ERR(rockchip->vpcie0v9))
> +		regulator_disable(rockchip->vpcie0v9);
>  	return err;
>  }
>  
> @@ -1483,47 +1498,47 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	err = clk_prepare_enable(rockchip->aclk_pcie);
>  	if (err) {
>  		dev_err(dev, "unable to enable aclk_pcie clock\n");
> -		goto err_aclk_pcie;
> +		return err;
>  	}
>  
>  	err = clk_prepare_enable(rockchip->aclk_perf_pcie);
>  	if (err) {
>  		dev_err(dev, "unable to enable aclk_perf_pcie clock\n");
> -		goto err_aclk_perf_pcie;
> +		goto err_disable_aclk_pcie;
>  	}
>  
>  	err = clk_prepare_enable(rockchip->hclk_pcie);
>  	if (err) {
>  		dev_err(dev, "unable to enable hclk_pcie clock\n");
> -		goto err_hclk_pcie;
> +		goto err_disable_aclk_perf_pcie;
>  	}
>  
>  	err = clk_prepare_enable(rockchip->clk_pcie_pm);
>  	if (err) {
>  		dev_err(dev, "unable to enable hclk_pcie clock\n");
> -		goto err_pcie_pm;
> +		goto err_disable_hclk_pcie;
>  	}
>  
>  	err = rockchip_pcie_set_vpcie(rockchip);
>  	if (err) {
>  		dev_err(dev, "failed to set vpcie regulator\n");
> -		goto err_set_vpcie;
> +		goto err_disable_clk_pcie_pm;
>  	}
>  
>  	err = rockchip_pcie_init_port(rockchip);
>  	if (err)
> -		goto err_vpcie;
> +		goto err_disable_vpcie;
>  
>  	rockchip_pcie_enable_interrupts(rockchip);
>  
>  	err = rockchip_pcie_init_irq_domain(rockchip);
>  	if (err < 0)
> -		goto err_vpcie;
> +		goto err_deinit_port;
>  
>  	err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
>  					       &res, &io_base);
>  	if (err)
> -		goto err_vpcie;
> +		goto err_remove_irq_domain;
>  
>  	err = devm_request_pci_bus_resources(dev, &res);
>  	if (err)
> @@ -1561,12 +1576,12 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  
>  	err = rockchip_pcie_cfg_atu(rockchip);
>  	if (err)
> -		goto err_free_res;
> +		goto err_unmap_iospace;
>  
>  	rockchip->msg_region = devm_ioremap(dev, rockchip->msg_bus_addr, SZ_1M);
>  	if (!rockchip->msg_region) {
>  		err = -ENOMEM;
> -		goto err_free_res;
> +		goto err_unmap_iospace;
>  	}
>  
>  	list_splice_init(&res, &bridge->windows);
> @@ -1591,28 +1606,33 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  		pcie_bus_configure_settings(child);
>  
>  	pci_bus_add_devices(bus);
> -	return 0;
>  
> +	return 0;
> +err_unmap_iospace:
> +	pci_unmap_iospace(rockchip->io);
>  err_free_res:
>  	pci_free_resource_list(&res);
> -err_vpcie:
> -	if (!IS_ERR(rockchip->vpcie12v))
> -		regulator_disable(rockchip->vpcie12v);
> -	if (!IS_ERR(rockchip->vpcie3v3))
> -		regulator_disable(rockchip->vpcie3v3);
> -	if (!IS_ERR(rockchip->vpcie1v8))
> -		regulator_disable(rockchip->vpcie1v8);
> +err_remove_irq_domain:
> +	irq_domain_remove(rockchip->irq_domain);
> +err_deinit_port:
> +	rockchip_pcie_deinit_phys(rockchip);
> +err_disable_vpcie:
>  	if (!IS_ERR(rockchip->vpcie0v9))
>  		regulator_disable(rockchip->vpcie0v9);
> -err_set_vpcie:
> +	if (!IS_ERR(rockchip->vpcie1v8))
> +		regulator_disable(rockchip->vpcie1v8);
> +	if (!IS_ERR(rockchip->vpcie3v3))
> +		regulator_disable(rockchip->vpcie3v3);
> +	if (!IS_ERR(rockchip->vpcie12v))
> +		regulator_disable(rockchip->vpcie12v);
> +err_disable_clk_pcie_pm:
>  	clk_disable_unprepare(rockchip->clk_pcie_pm);
> -err_pcie_pm:
> +err_disable_hclk_pcie:
>  	clk_disable_unprepare(rockchip->hclk_pcie);
> -err_hclk_pcie:
> +err_disable_aclk_perf_pcie:
>  	clk_disable_unprepare(rockchip->aclk_perf_pcie);
> -err_aclk_perf_pcie:
> +err_disable_aclk_pcie:
>  	clk_disable_unprepare(rockchip->aclk_pcie);
> -err_aclk_pcie:
>  	return err;
>  }
>  
> @@ -1620,33 +1640,27 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> -	int i;
>  
>  	pci_stop_root_bus(rockchip->root_bus);
>  	pci_remove_root_bus(rockchip->root_bus);
>  	pci_unmap_iospace(rockchip->io);
>  	irq_domain_remove(rockchip->irq_domain);
>  
> -	for (i = 0; i < MAX_LANE_NUM; i++) {
> -		/* inactive lane is already powered off */
> -		if (rockchip->lanes_map & BIT(i))
> -			phy_power_off(rockchip->phys[i]);
> -		phy_exit(rockchip->phys[i]);
> -	}
> +	rockchip_pcie_deinit_phys(rockchip);
>  
>  	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);
>  
> -	if (!IS_ERR(rockchip->vpcie12v))
> -		regulator_disable(rockchip->vpcie12v);
> -	if (!IS_ERR(rockchip->vpcie3v3))
> -		regulator_disable(rockchip->vpcie3v3);
> -	if (!IS_ERR(rockchip->vpcie1v8))
> -		regulator_disable(rockchip->vpcie1v8);
>  	if (!IS_ERR(rockchip->vpcie0v9))
>  		regulator_disable(rockchip->vpcie0v9);
> +	if (!IS_ERR(rockchip->vpcie1v8))
> +		regulator_disable(rockchip->vpcie1v8);
> +	if (!IS_ERR(rockchip->vpcie3v3))
> +		regulator_disable(rockchip->vpcie3v3);
> +	if (!IS_ERR(rockchip->vpcie12v))
> +		regulator_disable(rockchip->vpcie12v);
>  
>  	return 0;
>  }
> -- 
> 2.11.0
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 2/4] PCI: rockchip: Add support for pcie wake irq

[Jeffy Chen]

Add support for PCIE_WAKE pin in rockchip pcie driver.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v4: None
Changes in v3:
Fix error handling

Changes in v2:
Use dev_pm_set_dedicated_wake_irq
        -- Suggested by Brian Norris <briannorris@chromium.com>

 drivers/pci/host/pcie-rockchip.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 5d85ec2e2fb0..a0f7267984da 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -37,6 +37,7 @@
 #include <linux/pci_ids.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/reset.h>
 #include <linux/regmap.h>
 
@@ -1111,6 +1112,15 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 		return err;
 	}
 
+	/* Must init wakeup before setting dedicated wake irq. */
+	device_init_wakeup(dev, true);
+	irq = platform_get_irq_byname(pdev, "wake");
+	if (irq >= 0) {
+		err = dev_pm_set_dedicated_wake_irq(dev, irq);
+		if (err)
+			dev_err(dev, "failed to setup PCIe wake IRQ\n");
+	}
+
 	rockchip->vpcie12v = devm_regulator_get_optional(dev, "vpcie12v");
 	if (IS_ERR(rockchip->vpcie12v)) {
 		if (PTR_ERR(rockchip->vpcie12v) == -EPROBE_DEFER)
@@ -1493,12 +1503,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 
 	err = rockchip_pcie_parse_dt(rockchip);
 	if (err)
-		return err;
+		/* It's safe to disable wake even not enabled */
+		goto err_disable_wake;
 
 	err = clk_prepare_enable(rockchip->aclk_pcie);
 	if (err) {
 		dev_err(dev, "unable to enable aclk_pcie clock\n");
-		return err;
+		goto err_disable_wake;
 	}
 
 	err = clk_prepare_enable(rockchip->aclk_perf_pcie);
@@ -1633,6 +1644,9 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	clk_disable_unprepare(rockchip->aclk_perf_pcie);
 err_disable_aclk_pcie:
 	clk_disable_unprepare(rockchip->aclk_pcie);
+err_disable_wake:
+	dev_pm_clear_wake_irq(dev);
+	device_init_wakeup(dev, false);
 	return err;
 }
 
@@ -1662,6 +1676,9 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
 	if (!IS_ERR(rockchip->vpcie12v))
 		regulator_disable(rockchip->vpcie12v);
 
+	dev_pm_clear_wake_irq(dev);
+	device_init_wakeup(dev, false);
+
 	return 0;
 }
 
-- 
2.11.0



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 2/4] PCI: rockchip: Add support for pcie wake irq

[Bjorn Helgaas]

On Tue, Aug 22, 2017 at 11:19:32AM +0800, Jeffy Chen wrote:
> Add support for PCIE_WAKE pin in rockchip pcie driver.

Changelog should include a hint about what DT property this PCIE_WAKE
support depends on.

> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v4: None
> Changes in v3:
> Fix error handling
> 
> Changes in v2:
> Use dev_pm_set_dedicated_wake_irq
>         -- Suggested by Brian Norris <briannorris@chromium.com>
> 
>  drivers/pci/host/pcie-rockchip.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index 5d85ec2e2fb0..a0f7267984da 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -37,6 +37,7 @@
>  #include <linux/pci_ids.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_wakeirq.h>
>  #include <linux/reset.h>
>  #include <linux/regmap.h>
>  
> @@ -1111,6 +1112,15 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
>  		return err;
>  	}
>  
> +	/* Must init wakeup before setting dedicated wake irq. */
> +	device_init_wakeup(dev, true);
> +	irq = platform_get_irq_byname(pdev, "wake");
> +	if (irq >= 0) {
> +		err = dev_pm_set_dedicated_wake_irq(dev, irq);
> +		if (err)
> +			dev_err(dev, "failed to setup PCIe wake IRQ\n");

Error message could include the IRQ #.

> +	}
> +
>  	rockchip->vpcie12v = devm_regulator_get_optional(dev, "vpcie12v");
>  	if (IS_ERR(rockchip->vpcie12v)) {
>  		if (PTR_ERR(rockchip->vpcie12v) == -EPROBE_DEFER)
> @@ -1493,12 +1503,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  
>  	err = rockchip_pcie_parse_dt(rockchip);
>  	if (err)
> -		return err;
> +		/* It's safe to disable wake even not enabled */
> +		goto err_disable_wake;
>  
>  	err = clk_prepare_enable(rockchip->aclk_pcie);
>  	if (err) {
>  		dev_err(dev, "unable to enable aclk_pcie clock\n");
> -		return err;
> +		goto err_disable_wake;
>  	}
>  
>  	err = clk_prepare_enable(rockchip->aclk_perf_pcie);
> @@ -1633,6 +1644,9 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	clk_disable_unprepare(rockchip->aclk_perf_pcie);
>  err_disable_aclk_pcie:
>  	clk_disable_unprepare(rockchip->aclk_pcie);
> +err_disable_wake:
> +	dev_pm_clear_wake_irq(dev);
> +	device_init_wakeup(dev, false);
>  	return err;
>  }
>  
> @@ -1662,6 +1676,9 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
>  	if (!IS_ERR(rockchip->vpcie12v))
>  		regulator_disable(rockchip->vpcie12v);
>  
> +	dev_pm_clear_wake_irq(dev);
> +	device_init_wakeup(dev, false);
> +
>  	return 0;
>  }
>  
> -- 
> 2.11.0
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/4] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Jeffy Chen]

Add an optional interrupt for PCIE_WAKE pin.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 .../devicetree/bindings/pci/rockchip-pcie.txt        | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
index 5678be82530d..9f6504129e80 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
@@ -20,10 +20,13 @@ Required properties:
 - msi-map: Maps a Requester ID to an MSI controller and associated
 	msi-specifier data. See ./pci-msi.txt
 - interrupts: Three interrupt entries must be specified.
-- interrupt-names: Must include the following names
-	- "sys"
-	- "legacy"
-	- "client"
+- interrupt-names: Include the following names
+	Required:
+		- "sys"
+		- "legacy"
+		- "client"
+	Optional:
+		- "wake"
 - resets: Must contain seven entries for each entry in reset-names.
 	   See ../reset/reset.txt for details.
 - reset-names: Must include the following names
@@ -87,10 +90,11 @@ pcie0: pcie@f8000000 {
 	clock-names = "aclk", "aclk-perf",
 		      "hclk", "pm";
 	bus-range = <0x0 0x1>;
-	interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
-		     <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
-		     <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>;
-	interrupt-names = "sys", "legacy", "client";
+	interrupts-extended = <&gic GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
+			      <&gic GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
+			      <&gic GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>,
+			      <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
+	interrupt-names = "sys", "legacy", "client", "wake";
 	assigned-clocks = <&cru SCLK_PCIEPHY_REF>;
 	assigned-clock-parents = <&cru SCLK_PCIEPHY_REF100M>;
 	assigned-clock-rates = <100000000>;
-- 
2.11.0

Re: [PATCH v4 3/4] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Bjorn Helgaas]

On Tue, Aug 22, 2017 at 11:19:33AM +0800, Jeffy Chen wrote:
> Add an optional interrupt for PCIE_WAKE pin.

Rob?

> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  .../devicetree/bindings/pci/rockchip-pcie.txt        | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> index 5678be82530d..9f6504129e80 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> @@ -20,10 +20,13 @@ Required properties:
>  - msi-map: Maps a Requester ID to an MSI controller and associated
>  	msi-specifier data. See ./pci-msi.txt
>  - interrupts: Three interrupt entries must be specified.
> -- interrupt-names: Must include the following names
> -	- "sys"
> -	- "legacy"
> -	- "client"
> +- interrupt-names: Include the following names
> +	Required:
> +		- "sys"
> +		- "legacy"
> +		- "client"
> +	Optional:
> +		- "wake"

Why is there no other PCI binding that includes "wake" as an
interrupt-name?  This feels like something that should be fairly
common across host controllers.  I don't want a Rockport-specific
DT description if it could be made more general.

>  - resets: Must contain seven entries for each entry in reset-names.
>  	   See ../reset/reset.txt for details.
>  - reset-names: Must include the following names
> @@ -87,10 +90,11 @@ pcie0: pcie@f8000000 {
>  	clock-names = "aclk", "aclk-perf",
>  		      "hclk", "pm";
>  	bus-range = <0x0 0x1>;
> -	interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
> -		     <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
> -		     <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>;
> -	interrupt-names = "sys", "legacy", "client";
> +	interrupts-extended = <&gic GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
> +			      <&gic GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
> +			      <&gic GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>,
> +			      <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
> +	interrupt-names = "sys", "legacy", "client", "wake";
>  	assigned-clocks = <&cru SCLK_PCIEPHY_REF>;
>  	assigned-clock-parents = <&cru SCLK_PCIEPHY_REF100M>;
>  	assigned-clock-rates = <100000000>;
> -- 
> 2.11.0
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 3/4] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Brian Norris]

On Thu, Aug 24, 2017 at 11:53:54AM -0500, Bjorn Helgaas wrote:
> On Tue, Aug 22, 2017 at 11:19:33AM +0800, Jeffy Chen wrote:
> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>

> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > index 5678be82530d..9f6504129e80 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > @@ -20,10 +20,13 @@ Required properties:
> >  - msi-map: Maps a Requester ID to an MSI controller and associated
> >  	msi-specifier data. See ./pci-msi.txt
> >  - interrupts: Three interrupt entries must be specified.
> > -- interrupt-names: Must include the following names
> > -	- "sys"
> > -	- "legacy"
> > -	- "client"
> > +- interrupt-names: Include the following names
> > +	Required:
> > +		- "sys"
> > +		- "legacy"
> > +		- "client"
> > +	Optional:
> > +		- "wake"
> 
> Why is there no other PCI binding that includes "wake" as an
> interrupt-name?  This feels like something that should be fairly
> common across host controllers.  I don't want a Rockport-specific

s/port/chip/ :)

> DT description if it could be made more general.

I'm not sure we can really answer that question ("why do no other PCI
bindings have this?"). But one guess would be that every other
controller uses only beacon wake.

It would be OK with me if we made a blanket statement that a controller
with a "wake" interrupt means PCI WAKE# (per the specification). It's
possible this could even be stuck into some generic PCI/DT code
eventually. (I don't think we have a really good place for this today.)

Brian

> >  - resets: Must contain seven entries for each entry in reset-names.
> >  	   See ../reset/reset.txt for details.
> >  - reset-names: Must include the following names
> > @@ -87,10 +90,11 @@ pcie0: pcie@f8000000 {
> >  	clock-names = "aclk", "aclk-perf",
> >  		      "hclk", "pm";
> >  	bus-range = <0x0 0x1>;
> > -	interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
> > -		     <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
> > -		     <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>;
> > -	interrupt-names = "sys", "legacy", "client";
> > +	interrupts-extended = <&gic GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			      <&gic GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			      <&gic GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			      <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
> > +	interrupt-names = "sys", "legacy", "client", "wake";
> >  	assigned-clocks = <&cru SCLK_PCIEPHY_REF>;
> >  	assigned-clock-parents = <&cru SCLK_PCIEPHY_REF100M>;
> >  	assigned-clock-rates = <100000000>;
> > -- 
> > 2.11.0
> > 
> > 

Re: [PATCH v4 3/4] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Shawn Lin]

On 2017/8/25 10:11, Brian Norris wrote:
> On Thu, Aug 24, 2017 at 11:53:54AM -0500, Bjorn Helgaas wrote:
>> On Tue, Aug 22, 2017 at 11:19:33AM +0800, Jeffy Chen wrote:
>>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> 
>>> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>>> index 5678be82530d..9f6504129e80 100644
>>> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>>> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>>> @@ -20,10 +20,13 @@ Required properties:
>>>   - msi-map: Maps a Requester ID to an MSI controller and associated
>>>   	msi-specifier data. See ./pci-msi.txt
>>>   - interrupts: Three interrupt entries must be specified.
>>> -- interrupt-names: Must include the following names
>>> -	- "sys"
>>> -	- "legacy"
>>> -	- "client"
>>> +- interrupt-names: Include the following names
>>> +	Required:
>>> +		- "sys"
>>> +		- "legacy"
>>> +		- "client"
>>> +	Optional:
>>> +		- "wake"
>>
>> Why is there no other PCI binding that includes "wake" as an
>> interrupt-name?  This feels like something that should be fairly
>> common across host controllers.  I don't want a Rockport-specific
> 
> s/port/chip/ :)
> 
>> DT description if it could be made more general.
> 
> I'm not sure we can really answer that question ("why do no other PCI
> bindings have this?"). But one guess would be that every other
> controller uses only beacon wake.
> 
> It would be OK with me if we made a blanket statement that a controller
> with a "wake" interrupt means PCI WAKE# (per the specification). It's
> possible this could even be stuck into some generic PCI/DT code
> eventually. (I don't think we have a really good place for this today.)

I guess we could register a pcie port service for dedicated WAKE# as it 
seems fairly parallel to pme code there, if we need a common place for
that?


> 
> Brian
> 
>>>   - resets: Must contain seven entries for each entry in reset-names.
>>>   	   See ../reset/reset.txt for details.
>>>   - reset-names: Must include the following names
>>> @@ -87,10 +90,11 @@ pcie0: pcie@f8000000 {
>>>   	clock-names = "aclk", "aclk-perf",
>>>   		      "hclk", "pm";
>>>   	bus-range = <0x0 0x1>;
>>> -	interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
>>> -		     <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
>>> -		     <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>;
>>> -	interrupt-names = "sys", "legacy", "client";
>>> +	interrupts-extended = <&gic GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
>>> +			      <&gic GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
>>> +			      <&gic GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>,
>>> +			      <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
>>> +	interrupt-names = "sys", "legacy", "client", "wake";
>>>   	assigned-clocks = <&cru SCLK_PCIEPHY_REF>;
>>>   	assigned-clock-parents = <&cru SCLK_PCIEPHY_REF100M>;
>>>   	assigned-clock-rates = <100000000>;
>>> -- 
>>> 2.11.0
>>>
>>>
> 
> 
> 

Re: [PATCH v4 3/4] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Bjorn Helgaas]

On Thu, Aug 24, 2017 at 07:11:32PM -0700, Brian Norris wrote:
> On Thu, Aug 24, 2017 at 11:53:54AM -0500, Bjorn Helgaas wrote:
> > On Tue, Aug 22, 2017 at 11:19:33AM +0800, Jeffy Chen wrote:
> > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> 
> > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > > index 5678be82530d..9f6504129e80 100644
> > > --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > > +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > > @@ -20,10 +20,13 @@ Required properties:
> > >  - msi-map: Maps a Requester ID to an MSI controller and associated
> > >  	msi-specifier data. See ./pci-msi.txt
> > >  - interrupts: Three interrupt entries must be specified.
> > > -- interrupt-names: Must include the following names
> > > -	- "sys"
> > > -	- "legacy"
> > > -	- "client"
> > > +- interrupt-names: Include the following names
> > > +	Required:
> > > +		- "sys"
> > > +		- "legacy"
> > > +		- "client"
> > > +	Optional:
> > > +		- "wake"
> > 
> > Why is there no other PCI binding that includes "wake" as an
> > interrupt-name?  This feels like something that should be fairly
> > common across host controllers.  I don't want a Rockport-specific
> 
> s/port/chip/ :)

I visited Rockport this summer, guess I had it on the brain :)

> > DT description if it could be made more general.
> 
> I'm not sure we can really answer that question ("why do no other PCI
> bindings have this?"). But one guess would be that every other
> controller uses only beacon wake.
> 
> It would be OK with me if we made a blanket statement that a controller
> with a "wake" interrupt means PCI WAKE# (per the specification). It's
> possible this could even be stuck into some generic PCI/DT code
> eventually. (I don't think we have a really good place for this today.)

I'd just like every controller that uses WAKE# to use the same name in
DT.  Maybe all that means for now is mentioning it in
Documentation/devicetree/bindings/pci/pci.txt instead of (or in
addition to) Documentation/devicetree/bindings/pci/rockchip-pcie.txt

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 3/4] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Rob Herring]

On Tue, Aug 22, 2017 at 11:19:33AM +0800, Jeffy Chen wrote:
> Add an optional interrupt for PCIE_WAKE pin.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  .../devicetree/bindings/pci/rockchip-pcie.txt        | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> index 5678be82530d..9f6504129e80 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> @@ -20,10 +20,13 @@ Required properties:
>  - msi-map: Maps a Requester ID to an MSI controller and associated
>  	msi-specifier data. See ./pci-msi.txt
>  - interrupts: Three interrupt entries must be specified.
> -- interrupt-names: Must include the following names
> -	- "sys"
> -	- "legacy"
> -	- "client"
> +- interrupt-names: Include the following names
> +	Required:
> +		- "sys"
> +		- "legacy"
> +		- "client"
> +	Optional:
> +		- "wake"

Use the wakeup source binding:
Documentation/devicetree/bindings/power/wakeup-source.txt

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 3/4] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Brian Norris]

On Fri, Aug 25, 2017 at 01:14:39PM -0500, Rob Herring wrote:
> On Tue, Aug 22, 2017 at 11:19:33AM +0800, Jeffy Chen wrote:
> > Add an optional interrupt for PCIE_WAKE pin.
> > 
> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> > ---
> > 
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2: None
> > 
> >  .../devicetree/bindings/pci/rockchip-pcie.txt        | 20 ++++++++++++--------
> >  1 file changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > index 5678be82530d..9f6504129e80 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > @@ -20,10 +20,13 @@ Required properties:
> >  - msi-map: Maps a Requester ID to an MSI controller and associated
> >  	msi-specifier data. See ./pci-msi.txt
> >  - interrupts: Three interrupt entries must be specified.
> > -- interrupt-names: Must include the following names
> > -	- "sys"
> > -	- "legacy"
> > -	- "client"
> > +- interrupt-names: Include the following names
> > +	Required:
> > +		- "sys"
> > +		- "legacy"
> > +		- "client"
> > +	Optional:
> > +		- "wake"
> 
> Use the wakeup source binding:
> Documentation/devicetree/bindings/power/wakeup-source.txt

And I suppose this means we'd fall under this paragraph?

    "However if the devices have dedicated interrupt as the wakeup source
    then they need to specify/identify the same using device specific
    interrupt name. In such cases only that interrupt can be used as wakeup
    interrupt."

We don't expect *any* interrupt to qualify as PCI WAKE#; so we should
still also document the interrupt name ("wake"?) in
Documentation/devicetree/bindings/pci/pci.txt as Bjorn suggested, in
addition to using the 'wakeup-source' property documented there.

Brian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 3/4] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Rob Herring]

On Fri, Aug 25, 2017 at 1:20 PM, Brian Norris <briannorris@chromium.org> wrote:
> On Fri, Aug 25, 2017 at 01:14:39PM -0500, Rob Herring wrote:
>> On Tue, Aug 22, 2017 at 11:19:33AM +0800, Jeffy Chen wrote:
>> > Add an optional interrupt for PCIE_WAKE pin.
>> >
>> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> > ---
>> >
>> > Changes in v4: None
>> > Changes in v3: None
>> > Changes in v2: None
>> >
>> >  .../devicetree/bindings/pci/rockchip-pcie.txt        | 20 ++++++++++++--------
>> >  1 file changed, 12 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> > index 5678be82530d..9f6504129e80 100644
>> > --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> > +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> > @@ -20,10 +20,13 @@ Required properties:
>> >  - msi-map: Maps a Requester ID to an MSI controller and associated
>> >     msi-specifier data. See ./pci-msi.txt
>> >  - interrupts: Three interrupt entries must be specified.
>> > -- interrupt-names: Must include the following names
>> > -   - "sys"
>> > -   - "legacy"
>> > -   - "client"
>> > +- interrupt-names: Include the following names
>> > +   Required:
>> > +           - "sys"
>> > +           - "legacy"
>> > +           - "client"
>> > +   Optional:
>> > +           - "wake"
>>
>> Use the wakeup source binding:
>> Documentation/devicetree/bindings/power/wakeup-source.txt
>
> And I suppose this means we'd fall under this paragraph?
>
>     "However if the devices have dedicated interrupt as the wakeup source
>     then they need to specify/identify the same using device specific
>     interrupt name. In such cases only that interrupt can be used as wakeup
>     interrupt."
>
> We don't expect *any* interrupt to qualify as PCI WAKE#; so we should
> still also document the interrupt name ("wake"?) in
> Documentation/devicetree/bindings/pci/pci.txt as Bjorn suggested, in
> addition to using the 'wakeup-source' property documented there.

I believe the defined interrupt name is "wakeup" as example 1 shows.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 3/4] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Brian Norris]

On Mon, Aug 28, 2017 at 04:32:55PM -0500, Rob Herring wrote:
> On Fri, Aug 25, 2017 at 1:20 PM, Brian Norris <briannorris@chromium.org> wrote:
> > On Fri, Aug 25, 2017 at 01:14:39PM -0500, Rob Herring wrote:
> >> Use the wakeup source binding:
> >> Documentation/devicetree/bindings/power/wakeup-source.txt
> >
> > And I suppose this means we'd fall under this paragraph?
> >
> >     "However if the devices have dedicated interrupt as the wakeup source
> >     then they need to specify/identify the same using device specific
> >     interrupt name. In such cases only that interrupt can be used as wakeup
> >     interrupt."
> >
> > We don't expect *any* interrupt to qualify as PCI WAKE#; so we should
> > still also document the interrupt name ("wake"?) in
> > Documentation/devicetree/bindings/pci/pci.txt as Bjorn suggested, in
> > addition to using the 'wakeup-source' property documented there.
> 
> I believe the defined interrupt name is "wakeup" as example 1 shows.

That's an example, not a definition. And the definition I quoted
literally says "device specific interrupt name". The PCIe specification
calls it "WAKE#" all over the place, so I figured that's a good name to
use.

"wakeup" is also fine I suppose, as long as we document that it must be
PCIe WAKE# signal, as per the PCIe specfication.

Brian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 0/4] PCI: rockchip: Move PCIE_WAKE handling into rockchip pcie driver

[Bjorn Helgaas]

On Tue, Aug 22, 2017 at 11:19:30AM +0800, Jeffy Chen wrote:
> 
> Currently we are handling pcie wake in mrvl wifi driver. But Brian
> suggests to move it into rockchip pcie driver.
> 
> Tested on my chromebook bob(with cros 4.4 kernel and mrvl wifi).
> 
> 
> Changes in v4:
> Rebase on newest for-next branch, also fix error handling by:
> 1e7f570a1b86 PCI: rockchip: Idle inactive PHY(s)
> 
> Changes in v3:
> Fix error handling
> 
> Changes in v2:
> Use dev_pm_set_dedicated_wake_irq
>         -- Suggested by Brian Norris <briannorris@chromium.com>
> 
> Jeffy Chen (4):
>   PCI: rockchip: Fix error handlings
>   PCI: rockchip: Add support for pcie wake irq
>   dt-bindings: PCI: rockchip: Add support for pcie wake irq
>   arm64: dts: rockchip: Handle pcie wake in pcie driver for Gru
> 
>  .../devicetree/bindings/pci/rockchip-pcie.txt      |  20 ++-
>  arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi       |  15 +-
>  drivers/pci/host/pcie-rockchip.c                   | 179 ++++++++++++---------
>  3 files changed, 126 insertions(+), 88 deletions(-)

Looking for acks from Shawn and Rob...

And I'm not sure about the DT wake IRQ description.  That seems like it
could potentially be generic than this Rockchip-specific proposal.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 0/4] PCI: rockchip: Move PCIE_WAKE handling into rockchip pcie driver

[jeffy]

Hi Bjorn,

On 08/25/2017 12:55 AM, Bjorn Helgaas wrote:
> Looking for acks from Shawn and Rob...
>
> And I'm not sure about the DT wake IRQ description.  That seems like it
> could potentially be generic than this Rockchip-specific proposal.
>
>
it looks like shawn already take the error handling patch into his 
series, so i'll remove that one in my next version.

i'll wait for those error handling patches to land, then rebase and 
resend this wake irq patches, thanks:)

>