[PATCH v6 0/3] PCI: rockchip: Move PCIE_WAKE handling into rockchip pcie driver

[PATCH v6 0/3] 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 v6:
Fix device_init_wake error handling, and add some comments.

Changes in v5:
Rebase
Move to pci.txt
Use "wakeup" instead of "wake"

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 (3):
  PCI: rockchip: Add support for pcie wake irq
  dt-bindings: PCI: Add definition of pcie wake irq
  arm64: dts: rockchip: Handle pcie wake in pcie driver for Gru

 Documentation/devicetree/bindings/pci/pci.txt |  2 ++
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 15 +++++++++------
 drivers/pci/host/pcie-rockchip.c              | 27 +++++++++++++++++++++------
 3 files changed, 32 insertions(+), 12 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 v6 1/3] 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 v6:
Fix device_init_wake error handling, and add some comments.

Changes in v5:
Rebase

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 | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 9051c6c8fea4..268513b6c9c4 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>
 
@@ -995,6 +996,17 @@ static int rockchip_pcie_setup_irq(struct rockchip_pcie *rockchip)
 		return err;
 	}
 
+	irq = platform_get_irq_byname(pdev, "wakeup");
+	if (irq >= 0) {
+		/* Must init wakeup before setting dedicated wakeup irq. */
+		device_init_wakeup(dev, true);
+		err = dev_pm_set_dedicated_wake_irq(dev, irq);
+		if (err) {
+			dev_err(dev, "failed to setup PCIe wakeup IRQ\n");
+			device_init_wakeup(dev, false);
+		}
+	}
+
 	return 0;
 }
 
@@ -1123,10 +1135,6 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 		return PTR_ERR(rockchip->clk_pcie_pm);
 	}
 
-	err = rockchip_pcie_setup_irq(rockchip);
-	if (err)
-		return err;
-
 	rockchip->vpcie12v = devm_regulator_get_optional(dev, "vpcie12v");
 	if (IS_ERR(rockchip->vpcie12v)) {
 		if (PTR_ERR(rockchip->vpcie12v) == -EPROBE_DEFER)
@@ -1155,7 +1163,7 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 		dev_info(dev, "no vpcie0v9 regulator found\n");
 	}
 
-	return 0;
+	return rockchip_pcie_setup_irq(rockchip);
 }
 
 static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
@@ -1546,7 +1554,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 
 	err = rockchip_pcie_enable_clocks(rockchip);
 	if (err)
-		return err;
+		/* Cleanup rockchip_pcie_parse_dt() */
+		goto err_disable_wake;
 
 	err = rockchip_pcie_set_vpcie(rockchip);
 	if (err) {
@@ -1656,6 +1665,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 		regulator_disable(rockchip->vpcie0v9);
 err_set_vpcie:
 	rockchip_pcie_disable_clocks(rockchip);
+err_disable_wake:
+	/* It's harmless to disable wake even not enabled */
+	dev_pm_clear_wake_irq(dev);
+	device_init_wakeup(dev, false);
 	return err;
 }
 
@@ -1682,6 +1695,8 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
 	if (!IS_ERR(rockchip->vpcie0v9))
 		regulator_disable(rockchip->vpcie0v9);
 
+	dev_pm_clear_wake_irq(dev);
+	device_init_wakeup(dev, false);
 	return 0;
 }
 
-- 
2.11.0

Re: [PATCH v6 1/3] PCI: rockchip: Add support for pcie wake irq

[Bjorn Helgaas]

On Sat, Oct 14, 2017 at 03:50:56AM +0800, Jeffy Chen wrote:
> Add support for PCIE_WAKE pin in rockchip pcie driver.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v6:
> Fix device_init_wake error handling, and add some comments.
> 
> Changes in v5:
> Rebase
> 
> 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 | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index 9051c6c8fea4..268513b6c9c4 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>
>  
> @@ -995,6 +996,17 @@ static int rockchip_pcie_setup_irq(struct rockchip_pcie *rockchip)
>  		return err;
>  	}
>  
> +	irq = platform_get_irq_byname(pdev, "wakeup");
> +	if (irq >= 0) {
> +		/* Must init wakeup before setting dedicated wakeup irq. */
> +		device_init_wakeup(dev, true);
> +		err = dev_pm_set_dedicated_wake_irq(dev, irq);
> +		if (err) {
> +			dev_err(dev, "failed to setup PCIe wakeup IRQ\n");
> +			device_init_wakeup(dev, false);
> +		}
> +	}

There's nothing Rockchip-specific here, so I'm hoping you can explore
putting this support in the PCI core, so any system that describes the
WAKE# connection in the DT can benefit.

>  	return 0;
>  }
>  
> @@ -1123,10 +1135,6 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
>  		return PTR_ERR(rockchip->clk_pcie_pm);
>  	}
>  
> -	err = rockchip_pcie_setup_irq(rockchip);
> -	if (err)
> -		return err;
> -
>  	rockchip->vpcie12v = devm_regulator_get_optional(dev, "vpcie12v");
>  	if (IS_ERR(rockchip->vpcie12v)) {
>  		if (PTR_ERR(rockchip->vpcie12v) == -EPROBE_DEFER)
> @@ -1155,7 +1163,7 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
>  		dev_info(dev, "no vpcie0v9 regulator found\n");
>  	}
>  
> -	return 0;
> +	return rockchip_pcie_setup_irq(rockchip);
>  }
>  
>  static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
> @@ -1546,7 +1554,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  
>  	err = rockchip_pcie_enable_clocks(rockchip);
>  	if (err)
> -		return err;
> +		/* Cleanup rockchip_pcie_parse_dt() */
> +		goto err_disable_wake;
>  
>  	err = rockchip_pcie_set_vpcie(rockchip);
>  	if (err) {
> @@ -1656,6 +1665,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  		regulator_disable(rockchip->vpcie0v9);
>  err_set_vpcie:
>  	rockchip_pcie_disable_clocks(rockchip);
> +err_disable_wake:
> +	/* It's harmless to disable wake even not enabled */
> +	dev_pm_clear_wake_irq(dev);
> +	device_init_wakeup(dev, false);
>  	return err;
>  }
>  
> @@ -1682,6 +1695,8 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
>  	if (!IS_ERR(rockchip->vpcie0v9))
>  		regulator_disable(rockchip->vpcie0v9);
>  
> +	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 v6 1/3] PCI: rockchip: Add support for pcie wake irq

[Brian Norris]

On Mon, Oct 16, 2017 at 03:03:50PM -0500, Bjorn Helgaas wrote:
> On Sat, Oct 14, 2017 at 03:50:56AM +0800, Jeffy Chen wrote:
> > Add support for PCIE_WAKE pin in rockchip pcie driver.
> > 
> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> > ---
> > 
> > Changes in v6:
> > Fix device_init_wake error handling, and add some comments.
> > 
> > Changes in v5:
> > Rebase
> > 
> > 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 | 27 +++++++++++++++++++++------
> >  1 file changed, 21 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> > index 9051c6c8fea4..268513b6c9c4 100644
> > --- a/drivers/pci/host/pcie-rockchip.c
> > +++ b/drivers/pci/host/pcie-rockchip.c
...
> > @@ -995,6 +996,17 @@ static int rockchip_pcie_setup_irq(struct rockchip_pcie *rockchip)
> >  		return err;
> >  	}
> >  
> > +	irq = platform_get_irq_byname(pdev, "wakeup");
> > +	if (irq >= 0) {
> > +		/* Must init wakeup before setting dedicated wakeup irq. */
> > +		device_init_wakeup(dev, true);
> > +		err = dev_pm_set_dedicated_wake_irq(dev, irq);
> > +		if (err) {
> > +			dev_err(dev, "failed to setup PCIe wakeup IRQ\n");
> > +			device_init_wakeup(dev, false);
> > +		}
> > +	}
> 
> There's nothing Rockchip-specific here, so I'm hoping you can explore
> putting this support in the PCI core, so any system that describes the
> WAKE# connection in the DT can benefit.

I guess it could work to look into pci_create_root_bus(), and
do something like the following?

	if (IS_ENABLED(CONFIG_OF) && parent && parent->of_node)
		... do OF parsing for generic features like WAKE# ...

Brian

Re: [PATCH v6 1/3] PCI: rockchip: Add support for pcie wake irq

[Bjorn Helgaas]

On Tue, Oct 17, 2017 at 06:03:14PM -0700, Brian Norris wrote:
> On Mon, Oct 16, 2017 at 03:03:50PM -0500, Bjorn Helgaas wrote:
> > On Sat, Oct 14, 2017 at 03:50:56AM +0800, Jeffy Chen wrote:
> > > Add support for PCIE_WAKE pin in rockchip pcie driver.
> > > 
> > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> > > ---
> > > 
> > > Changes in v6:
> > > Fix device_init_wake error handling, and add some comments.
> > > 
> > > Changes in v5:
> > > Rebase
> > > 
> > > 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 | 27 +++++++++++++++++++++------
> > >  1 file changed, 21 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> > > index 9051c6c8fea4..268513b6c9c4 100644
> > > --- a/drivers/pci/host/pcie-rockchip.c
> > > +++ b/drivers/pci/host/pcie-rockchip.c
> ...
> > > @@ -995,6 +996,17 @@ static int rockchip_pcie_setup_irq(struct rockchip_pcie *rockchip)
> > >  		return err;
> > >  	}
> > >  
> > > +	irq = platform_get_irq_byname(pdev, "wakeup");
> > > +	if (irq >= 0) {
> > > +		/* Must init wakeup before setting dedicated wakeup irq. */
> > > +		device_init_wakeup(dev, true);
> > > +		err = dev_pm_set_dedicated_wake_irq(dev, irq);
> > > +		if (err) {
> > > +			dev_err(dev, "failed to setup PCIe wakeup IRQ\n");
> > > +			device_init_wakeup(dev, false);
> > > +		}
> > > +	}
> > 
> > There's nothing Rockchip-specific here, so I'm hoping you can explore
> > putting this support in the PCI core, so any system that describes the
> > WAKE# connection in the DT can benefit.
> 
> I guess it could work to look into pci_create_root_bus(), and
> do something like the following?
> 
> 	if (IS_ENABLED(CONFIG_OF) && parent && parent->of_node)
> 		... do OF parsing for generic features like WAKE# ...

That's exactly the sort of thing I was thinking.

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

Re: [PATCH v6 1/3] PCI: rockchip: Add support for pcie wake irq

[jeffy]

Hi guys,

On 10/18/2017 09:29 PM, Bjorn Helgaas wrote:
>> >
>> >I guess it could work to look into pci_create_root_bus(), and
>> >do something like the following?
>> >
>> >	if (IS_ENABLED(CONFIG_OF) && parent && parent->of_node)
>> >		... do OF parsing for generic features like WAKE# ...
> That's exactly the sort of thing I was thinking.
>
i was looking at that too, but i'm not very clear about how to handle 
the platform_ops's set_wakeup...




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

[PATCH v6 2/3] dt-bindings: PCI: Add definition of 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 v6: None
Changes in v5:
Move to pci.txt

Changes in v3: None
Changes in v2: None

 Documentation/devicetree/bindings/pci/pci.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt
index c77981c5dd18..5ae3a9e0134d 100644
--- a/Documentation/devicetree/bindings/pci/pci.txt
+++ b/Documentation/devicetree/bindings/pci/pci.txt
@@ -24,3 +24,5 @@ driver implementation may support the following properties:
    unsupported link speed, for instance, trying to do training for
    unsupported link speed, etc.  Must be '4' for gen4, '3' for gen3, '2'
    for gen2, and '1' for gen1. Any other values are invalid.
+- interrupts: Interrupt specifier for each name in interrupt-names.
+- interrupt-names: May contains "wakeup" for PCI WAKE# interrupt.
-- 
2.11.0

Re: [PATCH v6 2/3] dt-bindings: PCI: Add definition of pcie wake irq

[Rob Herring]

On Sat, Oct 14, 2017 at 03:50:57AM +0800, Jeffy Chen wrote:
> Add an optional interrupt for PCIE_WAKE pin.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v6: None
> Changes in v5:
> Move to pci.txt
> 
> Changes in v3: None
> Changes in v2: None
> 
>  Documentation/devicetree/bindings/pci/pci.txt | 2 ++
>  1 file changed, 2 insertions(+)

Please add acks when posting new versions.