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