Le 17/05/2021 à 10:01, Andy Shevchenko a écrit :
On Sun, May 16, 2021 at 04:36:55PM +0200, Christophe JAILLET wrote:
There is no error handling path in the probe function.
Switch to managed resource so that errors in the probe are handled easily
and simplify the remove function accordingly.
Yes, that's what I suggested earlier to another contributor.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Thanks!
P.S. You may consider the following things as well:
1) converting to use pci_set_drvdata() / pci_get_drvdata()
I can send a patch for that if you want.
But it looks really low value for a driver that is already very short
and clean.
2) providing devm_pvpanic_probe() [via devm_add_action() /
devm_add_action_or_reset()]
I don't follow you here.
The goal would be to avoid the remove function and "record" the needed
action directly in the probe?
If this is it, I would only see an unusual pattern and a harder to
follow logic.
Did I miss something?
What would be the benefit?
CJ
Fixes: db3a4f0abefd ("misc/pvpanic: add PCI driver")
Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
---
drivers/misc/pvpanic/pvpanic-pci.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/pvpanic/pvpanic-pci.c b/drivers/misc/pvpanic/pvpanic-pci.c
index 9ecc4e8559d5..046ce4ecc195 100644
--- a/drivers/misc/pvpanic/pvpanic-pci.c
+++ b/drivers/misc/pvpanic/pvpanic-pci.c
@@ -78,15 +78,15 @@ static int pvpanic_pci_probe(struct pci_dev *pdev,
void __iomem *base;
int ret;
- ret = pci_enable_device(pdev);
+ ret = pcim_enable_device(pdev);
if (ret < 0)
return ret;
- base = pci_iomap(pdev, 0, 0);
+ base = pcim_iomap(pdev, 0, 0);
if (!base)
return -ENOMEM;
- pi = kmalloc(sizeof(*pi), GFP_ATOMIC);
+ pi = devm_kmalloc(&pdev->dev, sizeof(*pi), GFP_ATOMIC);
if (!pi)
return -ENOMEM;
@@ -107,9 +107,6 @@ static void pvpanic_pci_remove(struct pci_dev *pdev)
struct pvpanic_instance *pi = dev_get_drvdata(&pdev->dev);
pvpanic_remove(pi);
- iounmap(pi->base);
- kfree(pi);
- pci_disable_device(pdev);
}
static struct pci_driver pvpanic_pci_driver = {
--
2.30.2
