Re: [PATCH RESEND v2 5/5] extcon: extcon-max8997: Simplify driver using devm

Chanwoo Choi
2021-06-10T09:38:53Z

On 6/8/21 7:10 PM, Matti Vaittinen wrote:
> Simplify driver by switching to use the resource managed IRQ
> requesting and resource managed work-queue initialization.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changelog:
> v2:
> - IRQ freeing fix splitted in own patch
>
> Please note that the change is compile-tested only. All proper testing is
> highly appreciated.
> ---
> drivers/extcon/extcon-max8997.c | 47 +++++++++++----------------------
> 1 file changed, 16 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
> index c15a612067af..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) {
> @@ -733,7 +736,7 @@ static int max8997_muic_probe(struct platform_device *pdev)
> 2, info->status);
> if (ret) {
> dev_err(info->dev, "failed to read MUIC register\n");
> - goto err_irq;
> + return ret;
> }
> cable_type = max8997_muic_get_cable_type(info,
> MAX8997_CABLE_GROUP_ADC, &attached);
> @@ -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);
> - return ret;
> -}
> -
> -static int max8997_muic_remove(struct platform_device *pdev)
> -{
> - struct max8997_muic_info *info = platform_get_drvdata(pdev);
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(muic_irqs); i++)
> - free_irq(muic_irqs[i].virq, info);
> - cancel_work_sync(&info->irq_work);
> -
> - return 0;
> }
>
> static struct platform_driver max8997_muic_driver = {
> @@ -780,7 +766,6 @@ static struct platform_driver max8997_muic_driver = {
> .name = DEV_NAME,
> },
> .probe = max8997_muic_probe,
> - .remove = max8997_muic_remove,
> };
>
> module_platform_driver(max8997_muic_driver);
>

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

--
Best Regards,
Chanwoo Choi
Samsung Electronics