[PATCH] PCI: tegra: Correctly handle return code of pm_runtime_get_sync()

[PATCH] PCI: tegra: Correctly handle return code of pm_runtime_get_sync()

[David Engraf]

pm_runtime_get_sync() returns the device's usage counter. This might
be >0 if the device is already powered up or CONFIG_PM is disabled.

Abort probe function on real error only.

Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
 drivers/pci/controller/pci-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 673a1725ef38..090b632965e2 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2798,7 +2798,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(pcie->dev);
 	err = pm_runtime_get_sync(pcie->dev);
-	if (err) {
+	if (err < 0) {
 		dev_err(dev, "fail to enable pcie controller: %d\n", err);
 		goto teardown_msi;
 	}
-- 
2.17.1

Re: [PATCH] PCI: tegra: Correctly handle return code of pm_runtime_get_sync()

[Andrew Murray]

On Mon, Dec 16, 2019 at 10:34:15AM +0100, David Engraf wrote:
> pm_runtime_get_sync() returns the device's usage counter. This might
> be >0 if the device is already powered up or CONFIG_PM is disabled.
> 
> Abort probe function on real error only.
> 
> Signed-off-by: David Engraf <david.engraf@sysgo.com>

Thanks for this, looks OK to me.

As this is a fix, can you add the following fixes tag?

Fixes: da76ba50963b ("PCI: tegra: Add power management support")

And whilst doing that can you rename the subject to "PCI: tegra: Fix return
value check of pm_runtime_get_sync" - it's slightly shorter and has the
word fix in the title.

Thanks,

Andrew Murray

> ---
>  drivers/pci/controller/pci-tegra.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 673a1725ef38..090b632965e2 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2798,7 +2798,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
>  
>  	pm_runtime_enable(pcie->dev);
>  	err = pm_runtime_get_sync(pcie->dev);
> -	if (err) {
> +	if (err < 0) {
>  		dev_err(dev, "fail to enable pcie controller: %d\n", err);
>  		goto teardown_msi;
>  	}
> -- 
> 2.17.1
> 

[PATCH v2] PCI: tegra: Fix return value check of pm_runtime_get_sync

[David Engraf]

pm_runtime_get_sync() returns the device's usage counter. This might
be >0 if the device is already powered up or CONFIG_PM is disabled.

Abort probe function on real error only.

Fixes: da76ba50963b ("PCI: tegra: Add power management support")
Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
 drivers/pci/controller/pci-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 673a1725ef38..090b632965e2 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2798,7 +2798,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(pcie->dev);
 	err = pm_runtime_get_sync(pcie->dev);
-	if (err) {
+	if (err < 0) {
 		dev_err(dev, "fail to enable pcie controller: %d\n", err);
 		goto teardown_msi;
 	}
-- 
2.17.1

Re: [PATCH v2] PCI: tegra: Fix return value check of pm_runtime_get_sync

[Andrew Murray]

On Mon, Dec 16, 2019 at 12:18:25PM +0100, David Engraf wrote:
> pm_runtime_get_sync() returns the device's usage counter. This might
> be >0 if the device is already powered up or CONFIG_PM is disabled.
> 
> Abort probe function on real error only.
> 
> Fixes: da76ba50963b ("PCI: tegra: Add power management support")
> Signed-off-by: David Engraf <david.engraf@sysgo.com>

Thanks for this - apologies my feedback wasn't as quick as your respin.

Acked-by: Andrew Murray <andrew.murray@arm.com>

> ---
>  drivers/pci/controller/pci-tegra.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 673a1725ef38..090b632965e2 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2798,7 +2798,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
>  
>  	pm_runtime_enable(pcie->dev);
>  	err = pm_runtime_get_sync(pcie->dev);
> -	if (err) {
> +	if (err < 0) {
>  		dev_err(dev, "fail to enable pcie controller: %d\n", err);
>  		goto teardown_msi;
>  	}
> -- 
> 2.17.1
> 

Re: [PATCH v2] PCI: tegra: Fix return value check of pm_runtime_get_sync

[Bjorn Helgaas]

On Mon, Dec 16, 2019 at 12:18:25PM +0100, David Engraf wrote:
> pm_runtime_get_sync() returns the device's usage counter. This might
> be >0 if the device is already powered up or CONFIG_PM is disabled.
> 
> Abort probe function on real error only.
> 
> Fixes: da76ba50963b ("PCI: tegra: Add power management support")
> Signed-off-by: David Engraf <david.engraf@sysgo.com>

I added Andrew's ack and a stable tag for v4.17+.  Also cc'd
Manikanta, author of da76ba50963b.

I put this on my pci/host-tegra branch for v5.6 for now.  Lorenzo may
move this when he returns.

> ---
>  drivers/pci/controller/pci-tegra.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 673a1725ef38..090b632965e2 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2798,7 +2798,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
>  
>  	pm_runtime_enable(pcie->dev);
>  	err = pm_runtime_get_sync(pcie->dev);
> -	if (err) {
> +	if (err < 0) {
>  		dev_err(dev, "fail to enable pcie controller: %d\n", err);
>  		goto teardown_msi;
>  	}
> -- 
> 2.17.1
> 

Re: [PATCH v2] PCI: tegra: Fix return value check of pm_runtime_get_sync

[Lorenzo Pieralisi]

On Tue, Dec 17, 2019 at 08:36:32AM -0600, Bjorn Helgaas wrote:
> On Mon, Dec 16, 2019 at 12:18:25PM +0100, David Engraf wrote:
> > pm_runtime_get_sync() returns the device's usage counter. This might
> > be >0 if the device is already powered up or CONFIG_PM is disabled.
> > 
> > Abort probe function on real error only.
> > 
> > Fixes: da76ba50963b ("PCI: tegra: Add power management support")
> > Signed-off-by: David Engraf <david.engraf@sysgo.com>
> 
> I added Andrew's ack and a stable tag for v4.17+.  Also cc'd
> Manikanta, author of da76ba50963b.
> 
> I put this on my pci/host-tegra branch for v5.6 for now.  Lorenzo may
> move this when he returns.

Hi Bjorn,

I could not find pci/host-tegra in your public repo, have you deleted it
in the meanwhile ?

I am happy to merge it myself, asking to check if there is anything
I am missing.

Thanks,
Lorenzo

Re: [PATCH v2] PCI: tegra: Fix return value check of pm_runtime_get_sync

[Bjorn Helgaas]

On Thu, Jan 09, 2020 at 12:13:09PM +0000, Lorenzo Pieralisi wrote:
> On Tue, Dec 17, 2019 at 08:36:32AM -0600, Bjorn Helgaas wrote:
> > On Mon, Dec 16, 2019 at 12:18:25PM +0100, David Engraf wrote:
> > > pm_runtime_get_sync() returns the device's usage counter. This might
> > > be >0 if the device is already powered up or CONFIG_PM is disabled.
> > > 
> > > Abort probe function on real error only.
> > > 
> > > Fixes: da76ba50963b ("PCI: tegra: Add power management support")
> > > Signed-off-by: David Engraf <david.engraf@sysgo.com>
> > 
> > I added Andrew's ack and a stable tag for v4.17+.  Also cc'd
> > Manikanta, author of da76ba50963b.
> > 
> > I put this on my pci/host-tegra branch for v5.6 for now.  Lorenzo may
> > move this when he returns.
> 
> Hi Bjorn,
> 
> I could not find pci/host-tegra in your public repo, have you deleted it
> in the meanwhile ?

Welcome back, Lorenzo!  Sorry, I forgot to push the pci/host-tegra
branch.  It has been in -next for a while, though.  I pushed it now.
Happy to drop the branch if you want to add more Tegra stuff, just let
me know.

Bjorn

Re: [PATCH v2] PCI: tegra: Fix return value check of pm_runtime_get_sync

[Lorenzo Pieralisi]

On Thu, Jan 09, 2020 at 12:23:33PM -0600, Bjorn Helgaas wrote:
> On Thu, Jan 09, 2020 at 12:13:09PM +0000, Lorenzo Pieralisi wrote:
> > On Tue, Dec 17, 2019 at 08:36:32AM -0600, Bjorn Helgaas wrote:
> > > On Mon, Dec 16, 2019 at 12:18:25PM +0100, David Engraf wrote:
> > > > pm_runtime_get_sync() returns the device's usage counter. This might
> > > > be >0 if the device is already powered up or CONFIG_PM is disabled.
> > > > 
> > > > Abort probe function on real error only.
> > > > 
> > > > Fixes: da76ba50963b ("PCI: tegra: Add power management support")
> > > > Signed-off-by: David Engraf <david.engraf@sysgo.com>
> > > 
> > > I added Andrew's ack and a stable tag for v4.17+.  Also cc'd
> > > Manikanta, author of da76ba50963b.
> > > 
> > > I put this on my pci/host-tegra branch for v5.6 for now.  Lorenzo may
> > > move this when he returns.
> > 
> > Hi Bjorn,
> > 
> > I could not find pci/host-tegra in your public repo, have you deleted it
> > in the meanwhile ?
> 
> Welcome back, Lorenzo!  Sorry, I forgot to push the pci/host-tegra
> branch.  It has been in -next for a while, though.  I pushed it now.
> Happy to drop the branch if you want to add more Tegra stuff, just let
> me know.

Thank you, I will have to add at least another tegra patch so I think
it is best to drop your branch, I applied this patch to my pci/tegra
branch and pushed it out in the meanwhile.

Thanks,
Lorenzo