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

[Bjorn Helgaas <bhelgaas@google.com>]

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;
 }