Messages in this thread | | | Subject | Re: [PATCH 4/4] extcon: extcon-max8997: Fix IRQ freeing at error path | From | Krzysztof Kozlowski <> | Date | Mon, 10 May 2021 10:21:41 -0400 |
| |
On 10/05/2021 04:12, Matti Vaittinen wrote: > If reading MAX8997_MUIC_REG_STATUS1 fails at probe the driver exits > without freeing the requested IRQs.
The driver frees IRQ on probe failure, so maybe you meant missing IRQ mapping dispose?
> > Simplify driver and fix the IRQ problem by switching to use the > resource managed IRQ requesting and resource managed work-queue > initialization. > > Fixes: 3e34c8198960 ("extcon: max8997: Avoid forcing UART path on drive probe") > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > > Please note that the change is compile-tested only. All proper testing is > highly appreciated. > --- > drivers/extcon/extcon-max8997.c | 45 +++++++++++---------------------- > 1 file changed, 15 insertions(+), 30 deletions(-) > > diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c > index e1408075ef7d..bbc592823570 100644 > --- a/drivers/extcon/extcon-max8997.c > +++ b/drivers/extcon/extcon-max8997.c > @@ -5,6 +5,7 @@ > // Copyright (C) 2012 Samsung Electronics > // Donggeun Kim <dg77.kim@samsung.com> > > +#include <linux/devm-helpers.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/i2c.h> > @@ -650,27 +651,30 @@ static int max8997_muic_probe(struct platform_device *pdev) > mutex_init(&info->mutex); > > INIT_WORK(&info->irq_work, max8997_muic_irq_work); > + ret = devm_work_autocancel(&pdev->dev, &info->irq_work, > + max8997_muic_irq_work); > + if (ret) > + return ret; > > for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) { > struct max8997_muic_irq *muic_irq = &muic_irqs[i]; > unsigned int virq = 0; > > virq = irq_create_mapping(max8997->irq_domain, muic_irq->irq); > - if (!virq) { > - ret = -EINVAL; > - goto err_irq; > - } > + if (!virq) > + return -EINVAL; > + > muic_irq->virq = virq; > > - ret = request_threaded_irq(virq, NULL, > - max8997_muic_irq_handler, > - IRQF_NO_SUSPEND, > - muic_irq->name, info); > + ret = devm_request_threaded_irq(&pdev->dev, virq, NULL, > + max8997_muic_irq_handler, > + IRQF_NO_SUSPEND, > + muic_irq->name, info); > if (ret) { > dev_err(&pdev->dev, > "failed: irq request (IRQ: %d, error :%d)\n", > muic_irq->irq, ret); > - goto err_irq; > + return ret; > } > } > > @@ -678,14 +682,13 @@ static int max8997_muic_probe(struct platform_device *pdev) > info->edev = devm_extcon_dev_allocate(&pdev->dev, max8997_extcon_cable); > if (IS_ERR(info->edev)) { > dev_err(&pdev->dev, "failed to allocate memory for extcon\n"); > - ret = PTR_ERR(info->edev); > - goto err_irq; > + return PTR_ERR(info->edev); > } > > ret = devm_extcon_dev_register(&pdev->dev, info->edev); > if (ret) { > dev_err(&pdev->dev, "failed to register extcon device\n"); > - goto err_irq; > + return ret; > } > > if (pdata && pdata->muic_pdata) { > @@ -756,23 +759,6 @@ static int max8997_muic_probe(struct platform_device *pdev) > delay_jiffies); > > return 0; > - > -err_irq: > - while (--i >= 0) > - free_irq(muic_irqs[i].virq, info);
Here it will unwind what was done.
> - return ret; > -} > -
Best regards, Krzysztof
| |