Thread (52 messages) 52 messages, 5 authors, 2024-08-14

Re: [PATCH v5 10/12] PCI: brcmstb: Check return value of all reset_control_xxx calls

From: Manivannan Sadhasivam <hidden>
Date: 2024-08-07 14:11:35
Also in: linux-pci, lkml

On Wed, Jul 31, 2024 at 06:28:24PM -0400, Jim Quinlan wrote:
Always check the return value for invocations of reset_control_xxx() and
propagate the error to the next level.  Although the current functions
in reset-brcmstb.c cannot fail, this may someday change.

Signed-off-by: Jim Quinlan <redacted>
One comment below. With that addressed,

Reviewed-by: Manivannan Sadhasivam <redacted>
quoted hunk ↗ jump to hunk
Reviewed-by: Stanimir Varbanov <redacted>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 102 ++++++++++++++++++--------
 1 file changed, 73 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 0ecca3d9576f..c4ceb1823a79 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
[...]
quoted hunk ↗ jump to hunk
 static int pci_dev_may_wakeup(struct pci_dev *dev, void *data)
@@ -1478,9 +1514,12 @@ static int brcm_pcie_suspend_noirq(struct device *dev)
 {
 	struct brcm_pcie *pcie = dev_get_drvdata(dev);
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
-	int ret;
+	int ret, rret;
+
+	ret = brcm_pcie_turn_off(pcie);
+	if (ret)
+		return ret;
 
-	brcm_pcie_turn_off(pcie);
 	/*
 	 * If brcm_phy_stop() returns an error, just dev_err(). If we
 	 * return the error it will cause the suspend to fail and this is a
@@ -1509,7 +1548,10 @@ static int brcm_pcie_suspend_noirq(struct device *dev)
 						     pcie->sr->supplies);
 			if (ret) {
 				dev_err(dev, "Could not turn off regulators\n");
-				reset_control_reset(pcie->rescal);
+				rret = reset_control_reset(pcie->rescal);
+				if (rret)
+					dev_err(dev, "failed to reset 'rascal' controller ret=%d\n",
+						rret);
I don't think it is really necessary to capture the return value in err path.
Unable to turn off the regulator itself is fatal, so we could just assert reset
and hope for the best.

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help