* [PATCH 1/1] IDE: Deletion of an unnecessary check before the function call "module_put"
[not found] ` <5317A59D.4@users.sourceforge.net>
@ 2014-11-21 19:26 ` SF Markus Elfring
2014-12-12 20:31 ` David Miller
2015-02-02 21:22 ` [PATCH] ata: Delete unnecessary checks before the function call "pci_dev_put" SF Markus Elfring
2015-02-03 8:30 ` [PATCH 0/2] sata_mv: Deletion of a few unnecessary checks SF Markus Elfring
2 siblings, 1 reply; 9+ messages in thread
From: SF Markus Elfring @ 2014-11-21 19:26 UTC (permalink / raw)
To: David S. Miller, linux-ide; +Cc: LKML, kernel-janitors, Julia Lawall
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 21 Nov 2014 20:22:32 +0100
The module_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/ide/ide.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
index 2ce6268..e29b02c 100644
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -101,8 +101,7 @@ void ide_device_put(ide_drive_t *drive)
struct device *host_dev = drive->hwif->host->dev[0];
struct module *module = host_dev ? host_dev->driver->owner : NULL;
- if (module)
- module_put(module);
+ module_put(module);
#endif
put_device(&drive->gendev);
}
--
2.1.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] ata: Delete unnecessary checks before the function call "pci_dev_put"
[not found] ` <5317A59D.4@users.sourceforge.net>
2014-11-21 19:26 ` [PATCH 1/1] IDE: Deletion of an unnecessary check before the function call "module_put" SF Markus Elfring
@ 2015-02-02 21:22 ` SF Markus Elfring
2015-02-03 12:06 ` Tejun Heo
2015-02-03 8:30 ` [PATCH 0/2] sata_mv: Deletion of a few unnecessary checks SF Markus Elfring
2 siblings, 1 reply; 9+ messages in thread
From: SF Markus Elfring @ 2015-02-02 21:22 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Tejun Heo, linux-ide
Cc: LKML, kernel-janitors, Julia Lawall
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 2 Feb 2015 22:08:29 +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/ata/pata_cs5530.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/pata_cs5530.c b/drivers/ata/pata_cs5530.c
index 48ae4b4..f9ca72e 100644
--- a/drivers/ata/pata_cs5530.c
+++ b/drivers/ata/pata_cs5530.c
@@ -276,10 +276,8 @@ static int cs5530_init_chip(void)
pci_dev_put(cs5530_0);
return 0;
fail_put:
- if (master_0)
- pci_dev_put(master_0);
- if (cs5530_0)
- pci_dev_put(cs5530_0);
+ pci_dev_put(master_0);
+ pci_dev_put(cs5530_0);
return -ENODEV;
}
--
2.2.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 0/2] sata_mv: Deletion of a few unnecessary checks
[not found] ` <5317A59D.4@users.sourceforge.net>
2014-11-21 19:26 ` [PATCH 1/1] IDE: Deletion of an unnecessary check before the function call "module_put" SF Markus Elfring
2015-02-02 21:22 ` [PATCH] ata: Delete unnecessary checks before the function call "pci_dev_put" SF Markus Elfring
@ 2015-02-03 8:30 ` SF Markus Elfring
2015-02-03 8:33 ` [PATCH 1/2] sata_mv: Delete unnecessary checks before the function call "phy_power_off" SF Markus Elfring
2015-02-03 8:36 ` [PATCH 2/2] sata_mv: More error handling for phy_power_off() in mv_platform_remove() SF Markus Elfring
2 siblings, 2 replies; 9+ messages in thread
From: SF Markus Elfring @ 2015-02-03 8:30 UTC (permalink / raw)
To: Tejun Heo, linux-ide; +Cc: LKML, kernel-janitors, Julia Lawall
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 3 Feb 2015 09:12:22 +0100
Another update suggestion was taken into account after a patch was applied
from static source code analysis.
Markus Elfring (2):
Delete unnecessary checks before the function call "phy_power_off"
More error handling for phy_power_off() in mv_platform_remove()
drivers/ata/sata_mv.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
--
2.2.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] sata_mv: Delete unnecessary checks before the function call "phy_power_off"
2015-02-03 8:30 ` [PATCH 0/2] sata_mv: Deletion of a few unnecessary checks SF Markus Elfring
@ 2015-02-03 8:33 ` SF Markus Elfring
2015-02-03 12:06 ` Tejun Heo
2015-02-03 8:36 ` [PATCH 2/2] sata_mv: More error handling for phy_power_off() in mv_platform_remove() SF Markus Elfring
1 sibling, 1 reply; 9+ messages in thread
From: SF Markus Elfring @ 2015-02-03 8:33 UTC (permalink / raw)
To: Tejun Heo, linux-ide; +Cc: LKML, kernel-janitors, Julia Lawall
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 2 Feb 2015 22:55:53 +0100
The phy_power_off() 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/ata/sata_mv.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index f9a0e34..f8c33e3 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4185,8 +4185,7 @@ err:
clk_disable_unprepare(hpriv->port_clks[port]);
clk_put(hpriv->port_clks[port]);
}
- if (hpriv->port_phys[port])
- phy_power_off(hpriv->port_phys[port]);
+ phy_power_off(hpriv->port_phys[port]);
}
return rc;
@@ -4216,8 +4215,7 @@ static int mv_platform_remove(struct platform_device *pdev)
clk_disable_unprepare(hpriv->port_clks[port]);
clk_put(hpriv->port_clks[port]);
}
- if (hpriv->port_phys[port])
- phy_power_off(hpriv->port_phys[port]);
+ phy_power_off(hpriv->port_phys[port]);
}
return 0;
}
--
2.2.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] sata_mv: More error handling for phy_power_off() in mv_platform_remove()
2015-02-03 8:30 ` [PATCH 0/2] sata_mv: Deletion of a few unnecessary checks SF Markus Elfring
2015-02-03 8:33 ` [PATCH 1/2] sata_mv: Delete unnecessary checks before the function call "phy_power_off" SF Markus Elfring
@ 2015-02-03 8:36 ` SF Markus Elfring
2015-02-03 12:10 ` Tejun Heo
1 sibling, 1 reply; 9+ messages in thread
From: SF Markus Elfring @ 2015-02-03 8:36 UTC (permalink / raw)
To: Tejun Heo, linux-ide; +Cc: LKML, kernel-janitors, Julia Lawall
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 2 Feb 2015 23:30:34 +0100
The return value from the phy_power_off() function was not used by the
mv_platform_remove() function.
Let us improve error detection and eventually return a corresponding
failure code.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/ata/sata_mv.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index f8c33e3..4f9bc33 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4203,7 +4203,7 @@ static int mv_platform_remove(struct platform_device *pdev)
{
struct ata_host *host = platform_get_drvdata(pdev);
struct mv_host_priv *hpriv = host->private_data;
- int port;
+ int port, rc;
ata_host_detach(host);
if (!IS_ERR(hpriv->clk)) {
@@ -4215,7 +4215,9 @@ static int mv_platform_remove(struct platform_device *pdev)
clk_disable_unprepare(hpriv->port_clks[port]);
clk_put(hpriv->port_clks[port]);
}
- phy_power_off(hpriv->port_phys[port]);
+ rc = phy_power_off(hpriv->port_phys[port]);
+ if (rc)
+ return rc;
}
return 0;
}
--
2.2.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/2] sata_mv: More error handling for phy_power_off() in mv_platform_remove()
2015-02-03 8:36 ` [PATCH 2/2] sata_mv: More error handling for phy_power_off() in mv_platform_remove() SF Markus Elfring
@ 2015-02-03 12:10 ` Tejun Heo
0 siblings, 0 replies; 9+ messages in thread
From: Tejun Heo @ 2015-02-03 12:10 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: linux-ide, LKML, kernel-janitors, Julia Lawall
On Tue, Feb 03, 2015 at 09:36:01AM +0100, SF Markus Elfring wrote:
...
> @@ -4215,7 +4215,9 @@ static int mv_platform_remove(struct platform_device *pdev)
> clk_disable_unprepare(hpriv->port_clks[port]);
> clk_put(hpriv->port_clks[port]);
> }
> - phy_power_off(hpriv->port_phys[port]);
> + rc = phy_power_off(hpriv->port_phys[port]);
> + if (rc)
> + return rc;
So, this is a removal function which is ignoring failure from turning
off phy, which seems like the right thing to do. The same thing with
suspend routines. If something auxliary which isn't strictly
necessary in reaching suspend state fails, the failure should be
ignored.
Running static code analysis to locate trivial irregularities and
performing identity transformations is fine, even great, but you're
changing the behavior here without actually understanding what's going
on. Please don't do these things automatically.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 9+ messages in thread