Re: [PATCH v2] net: ethernet: davicom: fix devicetree irq resource

Francois Romieu [off-list ref] writes:

Robert Jarzmik [off-list ref] :
[...]

quoted

diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c
index cf94b72dbacd..2bae5c8c1f85 100644
--- a/drivers/net/ethernet/davicom/dm9000.c
+++ b/drivers/net/ethernet/davicom/dm9000.c

[...]

quoted

@@ -1500,15 +1496,21 @@ dm9000_probe(struct platform_device *pdev)
 
 	db->addr_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	db->data_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	db->irq_res  = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 
-	if (db->addr_res == NULL || db->data_res == NULL ||
-	    db->irq_res == NULL) {
-		dev_err(db->dev, "insufficient resources\n");
+	if (!db->addr_res || !db->data_res) {
+		dev_err(db->dev, "insufficient resources addr=%p data=%p\n",
+			db->addr_res, db->data_res);
 		ret = -ENOENT;
 		goto out;
 	}
 
+	ndev->irq = platform_get_irq(pdev, 0);
+	if (ndev->irq < 0) {
+		dev_err(db->dev, "interrupt resource unavailable: %d\n",
+			ndev->irq);
+		return ndev->irq;
+	}
+

Leak: please set 'ret = ndev->irq;' and proceed to the 'out' label.

Indeed, the etherdev, good catch.

If you have some spare time, it would be nice to avoid db->irq_wake leak
on probe failure or driver removal.

Sorry but not in this patch.

I suppose the right patch would be to use devm_*() in the probe function for
ioremaps and request_irqs, which would address this point.

Cheers.

-- 
Robert

Robert Jarzmik [off-list ref] :

Francois Romieu [off-list ref] writes:

[...]

quoted

If you have some spare time, it would be nice to avoid db->irq_wake leak
on probe failure or driver removal.

Sorry but not in this patch.

Of course. Different topic => different patch.

I suppose the right patch would be to use devm_*() in the probe function for
ioremaps and request_irqs, which would address this point.

I'd rather avoid devm_* on netdev.

-- 
Ueimor