Thread (3 messages) 3 messages, 3 authors, 2025-10-21

Re: [PATCH] misc: rp1: Fix some error handling paths

From: Dan Carpenter <hidden>
Date: 2025-10-21 05:26:26
Also in: kernel-janitors, lkml

On Mon, Oct 20, 2025 at 09:11:16PM +0200, Christophe JAILLET wrote:
quoted hunk ↗ jump to hunk
Error handling in the probe and the clean-up path in the remove function
should be adjusted depending on if data is taken from DT or from overlay at
runtime.

of_overlay_remove() should not be called when of_overlay_remove() was not
called.

of_node_put() should be called in the remove function to avoid a potential
reference leak.

Fixes: 49d63971f963 ("misc: rp1: RaspberryPi RP1 misc driver")
Signed-off-by: Christophe JAILLET <redacted>
---
This patch is compile tested only.

I think (hope...) that a cleaner solution is possible. So feel free to
improve it or completely change it if needed.
---
 drivers/misc/rp1/rp1_pci.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/rp1/rp1_pci.c b/drivers/misc/rp1/rp1_pci.c
index 803832006ec8..9105269488a9 100644
--- a/drivers/misc/rp1/rp1_pci.c
+++ b/drivers/misc/rp1/rp1_pci.c
@@ -44,6 +44,8 @@ struct rp1_dev {
 	struct irq_data *pcie_irqds[64];
 	void __iomem *bar1;
 	int ovcs_id;	/* overlay changeset id */
+	struct device_node *rp1_node;	/* useful only if skip_ovl == true */
+	bool skip_ovl;
 	bool level_triggered_irq[RP1_INT_END];
 };
 
@@ -289,10 +291,14 @@ static int rp1_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto err_unload_overlay;
 	}
 
+	rp1->skip_ovl = skip_ovl;
+	rp1->rp1_node = rp1_node;
This is a reference to the device tree node.  We normally drop the
refcount when we're done reading what we want from it.  So we can
call of_node_put(rp1->rp1_node) here on the success path.

We could just declare it with __free(device_node) and use cleanup.h.
quoted hunk ↗ jump to hunk
+
 	return 0;
 
 err_unload_overlay:
-	of_overlay_remove(&rp1->ovcs_id);
+	if (!skip_ovl)
+		of_overlay_remove(&rp1->ovcs_id);
 err_unregister_interrupts:
 	rp1_unregister_interrupts(pdev);
 err_put_node:
@@ -308,8 +314,12 @@ static void rp1_remove(struct pci_dev *pdev)
 	struct device *dev = &pdev->dev;
 
 	of_platform_depopulate(dev);
-	of_overlay_remove(&rp1->ovcs_id);
It might be nice if of_overlay_remove() ignored when rp1->ovcs_id is
set to zero.  Then we could call it unconditionally.

regards,
dan carpenter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help