[PATCH 0/3] J6/J6Eco: Add PM support to PCIe

[PATCH 0/3] J6/J6Eco: Add PM support to PCIe

[Kishon Vijay Abraham I]

This series adds PM support to pci-dra7xx so that PCI clocks can be disabled
during suspend and enabled back during resume without affecting
PCI functionality.

This series is dependent on [1] for proper PM functionality.

[1] -> http://newscentral.exsees.com/item/595269de5c35c59c386b91ce8efd9872-eedf6742bb6159c3b1a90625c4d43407

Kishon Vijay Abraham I (3):
  PCI: host: pci-dra7xx: Disable pm_runtime if get_sync failure
  PCI: host: pcie-designware: add support for suspend and resume
  PCI: host: pci-dra7xx: add pm support to pci dra7xx

 drivers/pci/host/pci-dra7xx.c      |   78 +++++++++++++++++++++++++++++++++++-
 drivers/pci/host/pcie-designware.c |   20 +++++++++
 drivers/pci/host/pcie-designware.h |    2 +
 3 files changed, 99 insertions(+), 1 deletion(-)

-- 
1.7.9.5

[PATCH 1/3] PCI: host: pci-dra7xx: Disable pm_runtime on get_sync failure

[Kishon Vijay Abraham I]

Fix the error handling code in case pm_runtime_get_sync fails. Now
when pm_runtime_get_sync fails pm_runtime_disable is invoked so that
there is no unbalanced pm_runtime_enable calls.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/pci/host/pci-dra7xx.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 80db09e..d8b6d66 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -384,7 +384,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 	ret = pm_runtime_get_sync(dev);
 	if (IS_ERR_VALUE(ret)) {
 		dev_err(dev, "pm_runtime_get_sync failed\n");
-		goto err_phy;
+		goto err_get_sync;
 	}
 
 	reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
@@ -401,6 +401,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 
 err_add_port:
 	pm_runtime_put(dev);
+
+err_get_sync:
 	pm_runtime_disable(dev);
 
 err_phy:
-- 
1.7.9.5

[PATCH 2/3] PCI: host: pcie-designware: add support for suspend and resume

[Kishon Vijay Abraham I]

Certain platforms require MSE bit to be cleared to set the master
in standby mode. (In DRA7xx TRM_vE, section 24.9.4.5.2.2.1 PCIe
Controller Master Standby Behavior advises to use the clearing
of the local MSE bit to set the master in standby. Without this
some of the clocks do not idle).

Cleared the MSE bit on suspend and enabled it back on resume.
This is required to get suspend/resume working.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/pci/host/pcie-designware.c |   20 ++++++++++++++++++++
 drivers/pci/host/pcie-designware.h |    2 ++
 2 files changed, 22 insertions(+)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 69486be..cfb2bd6 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -811,6 +811,26 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
 }
 
+void dw_pcie_suspend_rc(struct pcie_port *pp)
+{
+	u32 val;
+
+	/* clear MSE */
+	dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
+	val &= ~PCI_COMMAND_MEMORY;
+	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
+}
+
+void dw_pcie_resume_rc(struct pcie_port *pp)
+{
+	u32 val;
+
+	/* set MSE */
+	dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
+	val |= PCI_COMMAND_MEMORY;
+	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
+}
+
 MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
 MODULE_DESCRIPTION("Designware PCIe host controller driver");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index d0bbd27..0df2dfa 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -83,5 +83,7 @@ void dw_pcie_msi_init(struct pcie_port *pp);
 int dw_pcie_link_up(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
+void dw_pcie_suspend_rc(struct pcie_port *pp);
+void dw_pcie_resume_rc(struct pcie_port *pp);
 
 #endif /* _PCIE_DESIGNWARE_H */
-- 
1.7.9.5

Re: [PATCH 2/3] PCI: host: pcie-designware: add support for suspend and resume

[Pratyush Anand]

Hi Kishon,

On Fri, Jul 3, 2015 at 4:33 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Certain platforms require MSE bit to be cleared to set the master
> in standby mode. (In DRA7xx TRM_vE, section 24.9.4.5.2.2.1 PCIe
> Controller Master Standby Behavior advises to use the clearing
> of the local MSE bit to set the master in standby. Without this
> some of the clocks do not idle).
>
> Cleared the MSE bit on suspend and enabled it back on resume.
> This is required to get suspend/resume working.

Have you tested with a card having IO space memory as well.
I think you might need to require clear and enable PCI_COMMAND_IO
in suspend and resume respectively.

~Pratyush

Re: [PATCH 2/3] PCI: host: pcie-designware: add support for suspend and resume

[Kishon Vijay Abraham I]

Hi Pratyush,

On Friday 10 July 2015 06:52 PM, Pratyush Anand wrote:
> Hi Kishon,
> 
> On Fri, Jul 3, 2015 at 4:33 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Certain platforms require MSE bit to be cleared to set the master
>> in standby mode. (In DRA7xx TRM_vE, section 24.9.4.5.2.2.1 PCIe
>> Controller Master Standby Behavior advises to use the clearing
>> of the local MSE bit to set the master in standby. Without this
>> some of the clocks do not idle).
>>
>> Cleared the MSE bit on suspend and enabled it back on resume.
>> This is required to get suspend/resume working.
> 
> Have you tested with a card having IO space memory as well.

I don't have any cards with IO space memory. Do you have any suggestion for
cards with IO space memory?
> I think you might need to require clear and enable PCI_COMMAND_IO
> in suspend and resume respectively.

okay.

Thanks
Kishon

Re: [PATCH 2/3] PCI: host: pcie-designware: add support for suspend and resume

[Pratyush Anand]

Hi Kishon,

On Fri, Jul 10, 2015 at 8:19 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:

[...]

>> Have you tested with a card having IO space memory as well.
>
> I don't have any cards with IO space memory. Do you have any suggestion for
> cards with IO space memory?

Sorry, I never used it either :(
IIRC, then Tim and Marek (in cc) had reported long back with some
issues of IO transaction
with SKY2 based card. They may let you know about exact card ID.

~Pratyush

Re: [PATCH 2/3] PCI: host: pcie-designware: add support for suspend and resume

[Marek Vasut]

On Friday, July 10, 2015 at 05:30:46 PM, Pratyush Anand wrote:
> Hi Kishon,

Hi,

> On Fri, Jul 10, 2015 at 8:19 PM, Kishon Vijay Abraham I <kishon@ti.com>
> wrote:
> 
> [...]
> 
> >> Have you tested with a card having IO space memory as well.
> > 
> > I don't have any cards with IO space memory. Do you have any suggestion
> > for cards with IO space memory?
> 
> Sorry, I never used it either :(
> IIRC, then Tim and Marek (in cc) had reported long back with some
> issues of IO transaction
> with SKY2 based card. They may let you know about exact card ID.

That was Tim, I didn't ever try with SKY2 , sorry.

Best regards,
Marek Vasut

Re: [PATCH 2/3] PCI: host: pcie-designware: add support for suspend and resume

[Jingoo Han]

On Friday, July 03, 2015 8:04 PM, Kishon Vijay Abraham I wrote:
> 
> Certain platforms require MSE bit to be cleared to set the master
> in standby mode. (In DRA7xx TRM_vE, section 24.9.4.5.2.2.1 PCIe

This patch is a work-around specific for DRA7xx chips?
If so, please move this patch to 'pci-dra7xx.c'.
I don't want to include chip-specific codes, because
'pcie-designware.c' is designed for including common codes.

Best regards,
Jingoo Han

> Controller Master Standby Behavior advises to use the clearing
> of the local MSE bit to set the master in standby. Without this
> some of the clocks do not idle).
> 
> Cleared the MSE bit on suspend and enabled it back on resume.
> This is required to get suspend/resume working.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  drivers/pci/host/pcie-designware.c |   20 ++++++++++++++++++++
>  drivers/pci/host/pcie-designware.h |    2 ++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 69486be..cfb2bd6 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -811,6 +811,26 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>  	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
>  }
> 
> +void dw_pcie_suspend_rc(struct pcie_port *pp)
> +{
> +	u32 val;
> +
> +	/* clear MSE */
> +	dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
> +	val &= ~PCI_COMMAND_MEMORY;
> +	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
> +}
> +
> +void dw_pcie_resume_rc(struct pcie_port *pp)
> +{
> +	u32 val;
> +
> +	/* set MSE */
> +	dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
> +	val |= PCI_COMMAND_MEMORY;
> +	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
> +}
> +
>  MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
>  MODULE_DESCRIPTION("Designware PCIe host controller driver");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index d0bbd27..0df2dfa 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -83,5 +83,7 @@ void dw_pcie_msi_init(struct pcie_port *pp);
>  int dw_pcie_link_up(struct pcie_port *pp);
>  void dw_pcie_setup_rc(struct pcie_port *pp);
>  int dw_pcie_host_init(struct pcie_port *pp);
> +void dw_pcie_suspend_rc(struct pcie_port *pp);
> +void dw_pcie_resume_rc(struct pcie_port *pp);
> 
>  #endif /* _PCIE_DESIGNWARE_H */
> --
> 1.7.9.5

Re: [PATCH 2/3] PCI: host: pcie-designware: add support for suspend and resume

[Kishon Vijay Abraham I]

Hi,

On Sunday 12 July 2015 04:01 PM, Jingoo Han wrote:
> On Friday, July 03, 2015 8:04 PM, Kishon Vijay Abraham I wrote:
>>
>> Certain platforms require MSE bit to be cleared to set the master
>> in standby mode. (In DRA7xx TRM_vE, section 24.9.4.5.2.2.1 PCIe
> 
> This patch is a work-around specific for DRA7xx chips?
> If so, please move this patch to 'pci-dra7xx.c'.
> I don't want to include chip-specific codes, because
> 'pcie-designware.c' is designed for including common codes.

Alright. Will change it that way in the next version.

Thanks
Kishon
> 
> Best regards,
> Jingoo Han
> 
>> Controller Master Standby Behavior advises to use the clearing
>> of the local MSE bit to set the master in standby. Without this
>> some of the clocks do not idle).
>>
>> Cleared the MSE bit on suspend and enabled it back on resume.
>> This is required to get suspend/resume working.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> ---
>>  drivers/pci/host/pcie-designware.c |   20 ++++++++++++++++++++
>>  drivers/pci/host/pcie-designware.h |    2 ++
>>  2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>> index 69486be..cfb2bd6 100644
>> --- a/drivers/pci/host/pcie-designware.c
>> +++ b/drivers/pci/host/pcie-designware.c
>> @@ -811,6 +811,26 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>>  	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
>>  }
>>
>> +void dw_pcie_suspend_rc(struct pcie_port *pp)
>> +{
>> +	u32 val;
>> +
>> +	/* clear MSE */
>> +	dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
>> +	val &= ~PCI_COMMAND_MEMORY;
>> +	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
>> +}
>> +
>> +void dw_pcie_resume_rc(struct pcie_port *pp)
>> +{
>> +	u32 val;
>> +
>> +	/* set MSE */
>> +	dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
>> +	val |= PCI_COMMAND_MEMORY;
>> +	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
>> +}
>> +
>>  MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
>>  MODULE_DESCRIPTION("Designware PCIe host controller driver");
>>  MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
>> index d0bbd27..0df2dfa 100644
>> --- a/drivers/pci/host/pcie-designware.h
>> +++ b/drivers/pci/host/pcie-designware.h
>> @@ -83,5 +83,7 @@ void dw_pcie_msi_init(struct pcie_port *pp);
>>  int dw_pcie_link_up(struct pcie_port *pp);
>>  void dw_pcie_setup_rc(struct pcie_port *pp);
>>  int dw_pcie_host_init(struct pcie_port *pp);
>> +void dw_pcie_suspend_rc(struct pcie_port *pp);
>> +void dw_pcie_resume_rc(struct pcie_port *pp);
>>
>>  #endif /* _PCIE_DESIGNWARE_H */
>> --
>> 1.7.9.5
> 

[PATCH 3/3] PCI: host: pci-dra7xx: add pm support to pci dra7xx

[Kishon Vijay Abraham I]

Add PM support to pci-dra7xx so that PCI clocks can be disabled
during suspend and enabled back during resume without affecting
PCI functionality.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/pci/host/pci-dra7xx.c |   74 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index d8b6d66..1f5c039 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -433,6 +433,79 @@ static int __exit dra7xx_pcie_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int dra7xx_pcie_suspend(struct device *dev)
+{
+	struct dra7xx_pcie *dra7xx = dev_get_drvdata(dev);
+	struct pcie_port *pp = &dra7xx->pp;
+
+	dw_pcie_suspend_rc(pp);
+
+	return 0;
+}
+
+static int dra7xx_pcie_resume(struct device *dev)
+{
+	struct dra7xx_pcie *dra7xx = dev_get_drvdata(dev);
+	struct pcie_port *pp = &dra7xx->pp;
+
+	dw_pcie_resume_rc(pp);
+
+	return 0;
+}
+static int dra7xx_pcie_suspend_noirq(struct device *dev)
+{
+	struct dra7xx_pcie *dra7xx = dev_get_drvdata(dev);
+	int count = dra7xx->phy_count;
+
+	while (count--) {
+		phy_power_off(dra7xx->phy[count]);
+		phy_exit(dra7xx->phy[count]);
+	}
+
+	return 0;
+}
+static int dra7xx_pcie_resume_noirq(struct device *dev)
+{
+	struct dra7xx_pcie *dra7xx = dev_get_drvdata(dev);
+	int phy_count = dra7xx->phy_count;
+	int ret;
+	int i;
+
+	for (i = 0; i < phy_count; i++) {
+		ret = phy_init(dra7xx->phy[i]);
+		if (ret < 0)
+			goto err_phy;
+
+		ret = phy_power_on(dra7xx->phy[i]);
+		if (ret < 0) {
+			phy_exit(dra7xx->phy[i]);
+			goto err_phy;
+		}
+	}
+
+	return 0;
+
+err_phy:
+	while (--i >= 0) {
+		phy_power_off(dra7xx->phy[i]);
+		phy_exit(dra7xx->phy[i]);
+	}
+
+	return ret;
+}
+
+static const struct dev_pm_ops dra7xx_pcie_pm_ops = {
+	.suspend_noirq = dra7xx_pcie_suspend_noirq,
+	.suspend = dra7xx_pcie_suspend,
+	.resume_noirq = dra7xx_pcie_resume_noirq,
+	.resume = dra7xx_pcie_resume,
+};
+#define DEV_PM_OPS	(&dra7xx_pcie_pm_ops)
+#else
+#define DEV_PM_OPS	NULL
+#endif
+
 static const struct of_device_id of_dra7xx_pcie_match[] = {
 	{ .compatible = "ti,dra7-pcie", },
 	{},
@@ -444,6 +517,7 @@ static struct platform_driver dra7xx_pcie_driver = {
 	.driver = {
 		.name	= "dra7-pcie",
 		.of_match_table = of_dra7xx_pcie_match,
+		.pm	= DEV_PM_OPS,
 	},
 };
 
-- 
1.7.9.5

Re: [PATCH 3/3] PCI: host: pci-dra7xx: add pm support to pci dra7xx

[Grygorii Strashko]

Hi Kishon,

On 07/03/2015 02:03 PM, Kishon Vijay Abraham I wrote:
> Add PM support to pci-dra7xx so that PCI clocks can be disabled
> during suspend and enabled back during resume without affecting
> PCI functionality.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>   drivers/pci/host/pci-dra7xx.c |   74 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 74 insertions(+)
>
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index d8b6d66..1f5c039 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -433,6 +433,79 @@ static int __exit dra7xx_pcie_remove(struct platform_device *pdev)
>   	return 0;
>   }
>
> +#ifdef CONFIG_PM_SLEEP

[...]

> +
> +static const struct dev_pm_ops dra7xx_pcie_pm_ops = {
> +	.suspend_noirq = dra7xx_pcie_suspend_noirq,
> +	.suspend = dra7xx_pcie_suspend,
> +	.resume_noirq = dra7xx_pcie_resume_noirq,
> +	.resume = dra7xx_pcie_resume,

Could you use here SET_SYSTEM_SLEEP_PM_OPS()
and SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() macro, pls?

> +};
> +#define DEV_PM_OPS	(&dra7xx_pcie_pm_ops)
> +#else
> +#define DEV_PM_OPS	NULL
> +#endif
> +
>   static const struct of_device_id of_dra7xx_pcie_match[] = {
>   	{ .compatible = "ti,dra7-pcie", },
>   	{},
> @@ -444,6 +517,7 @@ static struct platform_driver dra7xx_pcie_driver = {
>   	.driver = {
>   		.name	= "dra7-pcie",
>   		.of_match_table = of_dra7xx_pcie_match,
> +		.pm	= DEV_PM_OPS,
>   	},
>   };
>
>


-- 
regards,
-grygorii

Re: [PATCH 3/3] PCI: host: pci-dra7xx: add pm support to pci dra7xx

[Kishon Vijay Abraham I]

Hi,

On Friday 03 July 2015 05:34 PM, Grygorii Strashko wrote:
> Hi Kishon,
> 
> On 07/03/2015 02:03 PM, Kishon Vijay Abraham I wrote:
>> Add PM support to pci-dra7xx so that PCI clocks can be disabled
>> during suspend and enabled back during resume without affecting
>> PCI functionality.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>   drivers/pci/host/pci-dra7xx.c |   74 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 74 insertions(+)
>>
>> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
>> index d8b6d66..1f5c039 100644
>> --- a/drivers/pci/host/pci-dra7xx.c
>> +++ b/drivers/pci/host/pci-dra7xx.c
>> @@ -433,6 +433,79 @@ static int __exit dra7xx_pcie_remove(struct platform_device *pdev)
>>   	return 0;
>>   }
>>
>> +#ifdef CONFIG_PM_SLEEP
> 
> [...]
> 
>> +
>> +static const struct dev_pm_ops dra7xx_pcie_pm_ops = {
>> +	.suspend_noirq = dra7xx_pcie_suspend_noirq,
>> +	.suspend = dra7xx_pcie_suspend,
>> +	.resume_noirq = dra7xx_pcie_resume_noirq,
>> +	.resume = dra7xx_pcie_resume,
> 
> Could you use here SET_SYSTEM_SLEEP_PM_OPS()
> and SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() macro, pls?

sure!

Thanks
Kishon