[PATCH v1] PCI: starfive: Propagate dev_err_probe return value

Anand Moon posted 1 patch 3 months, 3 weeks ago
drivers/pci/controller/plda/pcie-starfive.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH v1] PCI: starfive: Propagate dev_err_probe return value
Posted by Anand Moon 3 months, 3 weeks ago
Ensure that the return value from dev_err_probe() is consistently assigned
back to return in all error paths within starfive_pcie_clk_rst_init() and
starfive_pcie_enable_phy() function. This ensures the original error code
are propagation for debugging.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/pci/controller/plda/pcie-starfive.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
index 3caf53c6c082..192d7a6a7c6c 100644
--- a/drivers/pci/controller/plda/pcie-starfive.c
+++ b/drivers/pci/controller/plda/pcie-starfive.c
@@ -180,7 +180,7 @@ static int starfive_pcie_clk_rst_init(struct starfive_jh7110_pcie *pcie)
 	ret = reset_control_deassert(pcie->resets);
 	if (ret) {
 		clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
-		dev_err_probe(dev, ret, "failed to deassert resets\n");
+		ret = dev_err_probe(dev, ret, "failed to deassert resets\n");
 	}
 
 	return ret;
@@ -241,13 +241,13 @@ static int starfive_pcie_enable_phy(struct device *dev,
 
 	ret = phy_set_mode(pcie->phy, PHY_MODE_PCIE);
 	if (ret) {
-		dev_err_probe(dev, ret, "failed to set pcie mode\n");
+		ret = dev_err_probe(dev, ret, "failed to set pcie mode\n");
 		goto err_phy_on;
 	}
 
 	ret = phy_power_on(pcie->phy);
 	if (ret) {
-		dev_err_probe(dev, ret, "failed to power on pcie phy\n");
+		ret = dev_err_probe(dev, ret, "failed to power on pcie phy\n");
 		goto err_phy_on;
 	}
 

base-commit: 98ac9cc4b4452ed7e714eddc8c90ac4ae5da1a09
-- 
2.50.1
Re: [PATCH] PCI: starfive: Propagate dev_err_probe return value
Posted by Markus Elfring 3 months, 3 weeks ago
> Ensure that the return value from dev_err_probe() is consistently assigned
> back to return in all error paths within starfive_pcie_clk_rst_init() and
> starfive_pcie_enable_phy() function. This ensures the original error code
> are propagation for debugging.

I find the change description improvable.


I propose to take another source code transformation approach better into account.
https://elixir.bootlin.com/linux/v6.17.1/source/drivers/base/core.c#L5031-L5075

Example:
https://elixir.bootlin.com/linux/v6.17.1/source/drivers/pci/controller/plda/pcie-starfive.c#L171-L187

	ret = dev_err_probe(dev, reset_control_deassert(pcie->resets),
			    "failed to deassert resets\n");
	if (ret)
		clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);


How do you think about to achieve such a source code variant also with the help of
the semantic patch language (Coccinelle software)?

Regards,
Markus