[PATCH 1/1] PCI: Deletion of unnecessary checks before three function calls

[PATCH 1/1] PCI: Deletion of unnecessary checks before three function calls

[SF Markus Elfring]

The functions pci_dev_put(), pci_pme_wakeup_bus() and put_device() test
whether their argument is NULL and then return immediately. Thus the test
around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/pci/pci-acpi.c     | 3 +--
 drivers/pci/probe.c        | 3 +--
 drivers/pci/search.c       | 3 +--
 drivers/pci/xen-pcifront.c | 3 +--
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 37263b0..a8fe5de 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -60,8 +60,7 @@ static void pci_acpi_wake_dev(struct work_struct *work)
 	pci_wakeup_event(pci_dev);
 	pm_runtime_resume(&pci_dev->dev);

-	if (pci_dev->subordinate)
-		pci_pme_wakeup_bus(pci_dev->subordinate);
+	pci_pme_wakeup_bus(pci_dev->subordinate);
 }

 /**
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4170113..e93f16e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -86,8 +86,7 @@ static void release_pcibus_dev(struct device *dev)
 {
 	struct pci_bus *pci_bus = to_pci_bus(dev);

-	if (pci_bus->bridge)
-		put_device(pci_bus->bridge);
+	put_device(pci_bus->bridge);
 	pci_bus_remove_resources(pci_bus);
 	pci_release_bus_of_node(pci_bus);
 	kfree(pci_bus);
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 827ad83..2d806bd 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -305,8 +305,7 @@ static struct pci_dev *pci_get_dev_by_id(const struct
pci_device_id *id,
 			      match_pci_dev_by_id);
 	if (dev)
 		pdev = to_pci_dev(dev);
-	if (from)
-		pci_dev_put(from);
+	pci_dev_put(from);
 	return pdev;
 }

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 53df39a..46664cc 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -596,8 +596,7 @@ static pci_ers_result_t pcifront_common_process(int cmd,
 	pcidev = pci_get_bus_and_slot(bus, devfn);
 	if (!pcidev || !pcidev->driver) {
 		dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
-		if (pcidev)
-			pci_dev_put(pcidev);
+		pci_dev_put(pcidev);
 		return result;
 	}
 	pdrv = pcidev->driver;
-- 
2.1.3

Re: [PATCH 1/1] PCI: Deletion of unnecessary checks before three function calls

[Bjorn Helgaas]

On Sun, Nov 02, 2014 at 04:12:30PM +0100, SF Markus Elfring wrote:
> The functions pci_dev_put(), pci_pme_wakeup_bus() and put_device() test
> whether their argument is NULL and then return immediately. Thus the test
> around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Applied to pci/misc for v3.19, thanks!

> ---
>  drivers/pci/pci-acpi.c     | 3 +--
>  drivers/pci/probe.c        | 3 +--
>  drivers/pci/search.c       | 3 +--
>  drivers/pci/xen-pcifront.c | 3 +--
>  4 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 37263b0..a8fe5de 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -60,8 +60,7 @@ static void pci_acpi_wake_dev(struct work_struct *work)
>  	pci_wakeup_event(pci_dev);
>  	pm_runtime_resume(&pci_dev->dev);
> 
> -	if (pci_dev->subordinate)
> -		pci_pme_wakeup_bus(pci_dev->subordinate);
> +	pci_pme_wakeup_bus(pci_dev->subordinate);
>  }
> 
>  /**
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4170113..e93f16e 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -86,8 +86,7 @@ static void release_pcibus_dev(struct device *dev)
>  {
>  	struct pci_bus *pci_bus = to_pci_bus(dev);
> 
> -	if (pci_bus->bridge)
> -		put_device(pci_bus->bridge);
> +	put_device(pci_bus->bridge);
>  	pci_bus_remove_resources(pci_bus);
>  	pci_release_bus_of_node(pci_bus);
>  	kfree(pci_bus);
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index 827ad83..2d806bd 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -305,8 +305,7 @@ static struct pci_dev *pci_get_dev_by_id(const struct
> pci_device_id *id,
>  			      match_pci_dev_by_id);
>  	if (dev)
>  		pdev = to_pci_dev(dev);
> -	if (from)
> -		pci_dev_put(from);
> +	pci_dev_put(from);
>  	return pdev;
>  }
> 
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 53df39a..46664cc 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -596,8 +596,7 @@ static pci_ers_result_t pcifront_common_process(int cmd,
>  	pcidev = pci_get_bus_and_slot(bus, devfn);
>  	if (!pcidev || !pcidev->driver) {
>  		dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
> -		if (pcidev)
> -			pci_dev_put(pcidev);
> +		pci_dev_put(pcidev);
>  		return result;
>  	}
>  	pdrv = pcidev->driver;
> -- 
> 2.1.3
> 
> 

[PATCH 1/1] PCI: hotplug: Deletion of an unnecessary check before the function call "pci_dev_put"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Nov 2014 17:42:23 +0100

The pci_dev_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/pci/hotplug/cpci_hotplug_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/cpci_hotplug_core.c b/drivers/pci/hotplug/cpci_hotplug_core.c
index e09cf78..82c969b 100644
--- a/drivers/pci/hotplug/cpci_hotplug_core.c
+++ b/drivers/pci/hotplug/cpci_hotplug_core.c
@@ -211,8 +211,7 @@ static void release_slot(struct hotplug_slot *hotplug_slot)
 
 	kfree(slot->hotplug_slot->info);
 	kfree(slot->hotplug_slot);
-	if (slot->dev)
-		pci_dev_put(slot->dev);
+	pci_dev_put(slot->dev);
 	kfree(slot);
 }
 
-- 
2.1.3

Re: [PATCH 1/1] PCI: hotplug: Deletion of an unnecessary check before the function call "pci_dev_put"

[Bjorn Helgaas]

On Thu, Nov 20, 2014 at 05:47:29PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 20 Nov 2014 17:42:23 +0100
> 
> The pci_dev_put() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Applied to next-pci/misc for v3.19, thanks.  This branch will be rebased
when v3.19-rc1 is released.

Bjorn

> ---
>  drivers/pci/hotplug/cpci_hotplug_core.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/cpci_hotplug_core.c b/drivers/pci/hotplug/cpci_hotplug_core.c
> index e09cf78..82c969b 100644
> --- a/drivers/pci/hotplug/cpci_hotplug_core.c
> +++ b/drivers/pci/hotplug/cpci_hotplug_core.c
> @@ -211,8 +211,7 @@ static void release_slot(struct hotplug_slot *hotplug_slot)
>  
>  	kfree(slot->hotplug_slot->info);
>  	kfree(slot->hotplug_slot);
> -	if (slot->dev)
> -		pci_dev_put(slot->dev);
> +	pci_dev_put(slot->dev);
>  	kfree(slot);
>  }
>  
> -- 
> 2.1.3
> 

[PATCH] PCI-iproc: Delete unnecessary checks before two function calls

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Jun 2015 16:42:04 +0200

The functions phy_exit() and phy_power_off() test whether their argument
is NULL and then return immediately.
Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/pci/host/pcie-iproc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index d77481e..f875821 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -239,12 +239,9 @@ err_rm_root_bus:
 	pci_remove_root_bus(bus);
 
 err_power_off_phy:
-	if (pcie->phy)
-		phy_power_off(pcie->phy);
+	phy_power_off(pcie->phy);
 err_exit_phy:
-	if (pcie->phy)
-		phy_exit(pcie->phy);
-
+	phy_exit(pcie->phy);
 	return ret;
 }
 EXPORT_SYMBOL(iproc_pcie_setup);
-- 
2.4.4

Re: [PATCH] PCI-iproc: Delete unnecessary checks before two function calls

[Ray Jui]

Hi Markus,

On 6/28/2015 7:52 AM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 28 Jun 2015 16:42:04 +0200
> 
> The functions phy_exit() and phy_power_off() test whether their argument
> is NULL and then return immediately.
> Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/pci/host/pcie-iproc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
> index d77481e..f875821 100644
> --- a/drivers/pci/host/pcie-iproc.c
> +++ b/drivers/pci/host/pcie-iproc.c
> @@ -239,12 +239,9 @@ err_rm_root_bus:
>  	pci_remove_root_bus(bus);
>  
>  err_power_off_phy:
> -	if (pcie->phy)
> -		phy_power_off(pcie->phy);
> +	phy_power_off(pcie->phy);
>  err_exit_phy:
> -	if (pcie->phy)
> -		phy_exit(pcie->phy);
> -
> +	phy_exit(pcie->phy);
>  	return ret;
>  }
>  EXPORT_SYMBOL(iproc_pcie_setup);
> 

Thanks for catching this. Could you please help to make similar changes
to both phy_init and phy_power_on calls in the driver, to make it
consistent?

Thanks,

Ray

Re: [PATCH] PCI-iproc: Delete unnecessary checks before two function calls

[Bjorn Helgaas]

On Sun, Jun 28, 2015 at 04:52:16PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 28 Jun 2015 16:42:04 +0200
> 
> The functions phy_exit() and phy_power_off() test whether their argument
> is NULL and then return immediately.
> Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

I haven't seen a followup to Ray's review, but in the interest of making
progress, I updated and applied the patch as appended.  I also reviewed
other phy_*() calls under drivers/pci, and they all look OK (with no
unnecessary tests for NULL).

This is on the pci/host-iproc branch for v4.3.

> ---
>  drivers/pci/host/pcie-iproc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
> index d77481e..f875821 100644
> --- a/drivers/pci/host/pcie-iproc.c
> +++ b/drivers/pci/host/pcie-iproc.c
> @@ -239,12 +239,9 @@ err_rm_root_bus:
>  	pci_remove_root_bus(bus);
>  
>  err_power_off_phy:
> -	if (pcie->phy)
> -		phy_power_off(pcie->phy);
> +	phy_power_off(pcie->phy);
>  err_exit_phy:
> -	if (pcie->phy)
> -		phy_exit(pcie->phy);
> -
> +	phy_exit(pcie->phy);
>  	return ret;
>  }
>  EXPORT_SYMBOL(iproc_pcie_setup);
> -- 
> 2.4.4

commit 55b5e16332eb9ffc1cbaf975585f4521417ab427
Author: Markus Elfring <elfring@users.sourceforge.net>
Date:   Sun Jun 28 16:42:04 2015 +0200

    PCI: iproc: Delete unnecessary checks before phy calls
    
    The functions phy_exit() and phy_power_off() test whether their argument is
    NULL and then return immediately.  Thus the test around the calls is not
    needed.
    
    This issue was detected by using the Coccinelle software.
    
    [bhelgaas: also phy_init() and phy_power_on(), as Ray Jui suggested]
    [bhelgaas: also remove tests in iproc_pcie_remove()]
    Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index d77481e..9a00dca 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -191,19 +191,16 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
 	if (!pcie || !pcie->dev || !pcie->base)
 		return -EINVAL;
 
-	if (pcie->phy) {
-		ret = phy_init(pcie->phy);
-		if (ret) {
-			dev_err(pcie->dev, "unable to initialize PCIe PHY\n");
-			return ret;
-		}
-
-		ret = phy_power_on(pcie->phy);
-		if (ret) {
-			dev_err(pcie->dev, "unable to power on PCIe PHY\n");
-			goto err_exit_phy;
-		}
+	ret = phy_init(pcie->phy);
+	if (ret) {
+		dev_err(pcie->dev, "unable to initialize PCIe PHY\n");
+		return ret;
+	}
 
+	ret = phy_power_on(pcie->phy);
+	if (ret) {
+		dev_err(pcie->dev, "unable to power on PCIe PHY\n");
+		goto err_exit_phy;
 	}
 
 	iproc_pcie_reset(pcie);
@@ -239,12 +236,9 @@ err_rm_root_bus:
 	pci_remove_root_bus(bus);
 
 err_power_off_phy:
-	if (pcie->phy)
-		phy_power_off(pcie->phy);
+	phy_power_off(pcie->phy);
 err_exit_phy:
-	if (pcie->phy)
-		phy_exit(pcie->phy);
-
+	phy_exit(pcie->phy);
 	return ret;
 }
 EXPORT_SYMBOL(iproc_pcie_setup);
@@ -254,10 +248,8 @@ int iproc_pcie_remove(struct iproc_pcie *pcie)
 	pci_stop_root_bus(pcie->root_bus);
 	pci_remove_root_bus(pcie->root_bus);
 
-	if (pcie->phy) {
-		phy_power_off(pcie->phy);
-		phy_exit(pcie->phy);
-	}
+	phy_power_off(pcie->phy);
+	phy_exit(pcie->phy);
 
 	return 0;
 }

Re: [PATCH] PCI-iproc: Delete unnecessary checks before two function calls

[Ray Jui]

Hi Bjorn,

On 7/14/2015 1:10 PM, Bjorn Helgaas wrote:
> On Sun, Jun 28, 2015 at 04:52:16PM +0200, SF Markus Elfring wrote:
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Sun, 28 Jun 2015 16:42:04 +0200
>>
>> The functions phy_exit() and phy_power_off() test whether their argument
>> is NULL and then return immediately.
>> Thus the test around the calls is not needed.
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> 
> I haven't seen a followup to Ray's review, but in the interest of making
> progress, I updated and applied the patch as appended.  I also reviewed
> other phy_*() calls under drivers/pci, and they all look OK (with no
> unnecessary tests for NULL).
> 
> This is on the pci/host-iproc branch for v4.3.
> 

Hmmm....I searched my mailbox but cannot find an email with this patch
(while I remember I reviewed and commented on the initial version of
this patch). It must have gone into some sub-folder or deleted by me by
accident. My bad.

Nevertheless,the current patch looks good to me!

Thanks,

Ray

>> ---
>>  drivers/pci/host/pcie-iproc.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
>> index d77481e..f875821 100644
>> --- a/drivers/pci/host/pcie-iproc.c
>> +++ b/drivers/pci/host/pcie-iproc.c
>> @@ -239,12 +239,9 @@ err_rm_root_bus:
>>  	pci_remove_root_bus(bus);
>>  
>>  err_power_off_phy:
>> -	if (pcie->phy)
>> -		phy_power_off(pcie->phy);
>> +	phy_power_off(pcie->phy);
>>  err_exit_phy:
>> -	if (pcie->phy)
>> -		phy_exit(pcie->phy);
>> -
>> +	phy_exit(pcie->phy);
>>  	return ret;
>>  }
>>  EXPORT_SYMBOL(iproc_pcie_setup);
>> -- 
>> 2.4.4
> 
> commit 55b5e16332eb9ffc1cbaf975585f4521417ab427
> Author: Markus Elfring <elfring@users.sourceforge.net>
> Date:   Sun Jun 28 16:42:04 2015 +0200
> 
>     PCI: iproc: Delete unnecessary checks before phy calls
>     
>     The functions phy_exit() and phy_power_off() test whether their argument is
>     NULL and then return immediately.  Thus the test around the calls is not
>     needed.
>     
>     This issue was detected by using the Coccinelle software.
>     
>     [bhelgaas: also phy_init() and phy_power_on(), as Ray Jui suggested]
>     [bhelgaas: also remove tests in iproc_pcie_remove()]
>     Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
> index d77481e..9a00dca 100644
> --- a/drivers/pci/host/pcie-iproc.c
> +++ b/drivers/pci/host/pcie-iproc.c
> @@ -191,19 +191,16 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
>  	if (!pcie || !pcie->dev || !pcie->base)
>  		return -EINVAL;
>  
> -	if (pcie->phy) {
> -		ret = phy_init(pcie->phy);
> -		if (ret) {
> -			dev_err(pcie->dev, "unable to initialize PCIe PHY\n");
> -			return ret;
> -		}
> -
> -		ret = phy_power_on(pcie->phy);
> -		if (ret) {
> -			dev_err(pcie->dev, "unable to power on PCIe PHY\n");
> -			goto err_exit_phy;
> -		}
> +	ret = phy_init(pcie->phy);
> +	if (ret) {
> +		dev_err(pcie->dev, "unable to initialize PCIe PHY\n");
> +		return ret;
> +	}
>  
> +	ret = phy_power_on(pcie->phy);
> +	if (ret) {
> +		dev_err(pcie->dev, "unable to power on PCIe PHY\n");
> +		goto err_exit_phy;
>  	}
>  
>  	iproc_pcie_reset(pcie);
> @@ -239,12 +236,9 @@ err_rm_root_bus:
>  	pci_remove_root_bus(bus);
>  
>  err_power_off_phy:
> -	if (pcie->phy)
> -		phy_power_off(pcie->phy);
> +	phy_power_off(pcie->phy);
>  err_exit_phy:
> -	if (pcie->phy)
> -		phy_exit(pcie->phy);
> -
> +	phy_exit(pcie->phy);
>  	return ret;
>  }
>  EXPORT_SYMBOL(iproc_pcie_setup);
> @@ -254,10 +248,8 @@ int iproc_pcie_remove(struct iproc_pcie *pcie)
>  	pci_stop_root_bus(pcie->root_bus);
>  	pci_remove_root_bus(pcie->root_bus);
>  
> -	if (pcie->phy) {
> -		phy_power_off(pcie->phy);
> -		phy_exit(pcie->phy);
> -	}
> +	phy_power_off(pcie->phy);
> +	phy_exit(pcie->phy);
>  
>  	return 0;
>  }
> 

Re: [PATCH] PCI-iproc: Delete unnecessary checks before two function calls

[Bjorn Helgaas]

On Tue, Jul 14, 2015 at 01:23:23PM -0700, Ray Jui wrote:
> Hi Bjorn,
> 
> On 7/14/2015 1:10 PM, Bjorn Helgaas wrote:
> > On Sun, Jun 28, 2015 at 04:52:16PM +0200, SF Markus Elfring wrote:
> >> From: Markus Elfring <elfring@users.sourceforge.net>
> >> Date: Sun, 28 Jun 2015 16:42:04 +0200
> >>
> >> The functions phy_exit() and phy_power_off() test whether their argument
> >> is NULL and then return immediately.
> >> Thus the test around the calls is not needed.
> >>
> >> This issue was detected by using the Coccinelle software.
> >>
> >> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > 
> > I haven't seen a followup to Ray's review, but in the interest of making
> > progress, I updated and applied the patch as appended.  I also reviewed
> > other phy_*() calls under drivers/pci, and they all look OK (with no
> > unnecessary tests for NULL).
> > 
> > This is on the pci/host-iproc branch for v4.3.
> > 
> 
> Hmmm....I searched my mailbox but cannot find an email with this patch
> (while I remember I reviewed and commented on the initial version of
> this patch). It must have gone into some sub-folder or deleted by me by
> accident. My bad.
> 
> Nevertheless,the current patch looks good to me!

Thanks for checking it out!  Can I add your Reviewed-by to the patch below?

> > commit 55b5e16332eb9ffc1cbaf975585f4521417ab427
> > Author: Markus Elfring <elfring@users.sourceforge.net>
> > Date:   Sun Jun 28 16:42:04 2015 +0200
> > 
> >     PCI: iproc: Delete unnecessary checks before phy calls
> >     
> >     The functions phy_exit() and phy_power_off() test whether their argument is
> >     NULL and then return immediately.  Thus the test around the calls is not
> >     needed.
> >     
> >     This issue was detected by using the Coccinelle software.
> >     
> >     [bhelgaas: also phy_init() and phy_power_on(), as Ray Jui suggested]
> >     [bhelgaas: also remove tests in iproc_pcie_remove()]
> >     Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> >     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
> > index d77481e..9a00dca 100644
> > --- a/drivers/pci/host/pcie-iproc.c
> > +++ b/drivers/pci/host/pcie-iproc.c
> > @@ -191,19 +191,16 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
> >  	if (!pcie || !pcie->dev || !pcie->base)
> >  		return -EINVAL;
> >  
> > -	if (pcie->phy) {
> > -		ret = phy_init(pcie->phy);
> > -		if (ret) {
> > -			dev_err(pcie->dev, "unable to initialize PCIe PHY\n");
> > -			return ret;
> > -		}
> > -
> > -		ret = phy_power_on(pcie->phy);
> > -		if (ret) {
> > -			dev_err(pcie->dev, "unable to power on PCIe PHY\n");
> > -			goto err_exit_phy;
> > -		}
> > +	ret = phy_init(pcie->phy);
> > +	if (ret) {
> > +		dev_err(pcie->dev, "unable to initialize PCIe PHY\n");
> > +		return ret;
> > +	}
> >  
> > +	ret = phy_power_on(pcie->phy);
> > +	if (ret) {
> > +		dev_err(pcie->dev, "unable to power on PCIe PHY\n");
> > +		goto err_exit_phy;
> >  	}
> >  
> >  	iproc_pcie_reset(pcie);
> > @@ -239,12 +236,9 @@ err_rm_root_bus:
> >  	pci_remove_root_bus(bus);
> >  
> >  err_power_off_phy:
> > -	if (pcie->phy)
> > -		phy_power_off(pcie->phy);
> > +	phy_power_off(pcie->phy);
> >  err_exit_phy:
> > -	if (pcie->phy)
> > -		phy_exit(pcie->phy);
> > -
> > +	phy_exit(pcie->phy);
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL(iproc_pcie_setup);
> > @@ -254,10 +248,8 @@ int iproc_pcie_remove(struct iproc_pcie *pcie)
> >  	pci_stop_root_bus(pcie->root_bus);
> >  	pci_remove_root_bus(pcie->root_bus);
> >  
> > -	if (pcie->phy) {
> > -		phy_power_off(pcie->phy);
> > -		phy_exit(pcie->phy);
> > -	}
> > +	phy_power_off(pcie->phy);
> > +	phy_exit(pcie->phy);
> >  
> >  	return 0;
> >  }
> > 

Re: [PATCH] PCI-iproc: Delete unnecessary checks before two function calls

[Ray Jui]

On 7/14/2015 1:51 PM, Bjorn Helgaas wrote:
> On Tue, Jul 14, 2015 at 01:23:23PM -0700, Ray Jui wrote:
>> Hi Bjorn,
>>
>> On 7/14/2015 1:10 PM, Bjorn Helgaas wrote:
>>> On Sun, Jun 28, 2015 at 04:52:16PM +0200, SF Markus Elfring wrote:
>>>> From: Markus Elfring <elfring@users.sourceforge.net>
>>>> Date: Sun, 28 Jun 2015 16:42:04 +0200
>>>>
>>>> The functions phy_exit() and phy_power_off() test whether their argument
>>>> is NULL and then return immediately.
>>>> Thus the test around the calls is not needed.
>>>>
>>>> This issue was detected by using the Coccinelle software.
>>>>
>>>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>>>
>>> I haven't seen a followup to Ray's review, but in the interest of making
>>> progress, I updated and applied the patch as appended.  I also reviewed
>>> other phy_*() calls under drivers/pci, and they all look OK (with no
>>> unnecessary tests for NULL).
>>>
>>> This is on the pci/host-iproc branch for v4.3.
>>>
>>
>> Hmmm....I searched my mailbox but cannot find an email with this patch
>> (while I remember I reviewed and commented on the initial version of
>> this patch). It must have gone into some sub-folder or deleted by me by
>> accident. My bad.
>>
>> Nevertheless,the current patch looks good to me!
> 
> Thanks for checking it out!  Can I add your Reviewed-by to the patch below?
> 

Sure thanks!

Reviewed-by: Ray Jui <rjui@broadcom.com>

Ray