All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Sverdlin <alexander.sverdlin@gmail.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>, linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4] Input: ep93xx_keypad - switch to using managed resources
Date: Tue, 12 Oct 2021 09:54:32 +0200	[thread overview]
Message-ID: <d1d1a1028c5db26d0b0eed2a5a9626124451fcf9.camel@gmail.com> (raw)
In-Reply-To: <20211012013735.3523140-4-dmitry.torokhov@gmail.com>

Hello Dmitry,

just one question below:

On Mon, 2021-10-11 at 18:37 -0700, Dmitry Torokhov wrote:
> By using managed resources (devm) we are able to streamline error handling
> in probe and remove most of the custom remove method.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/ep93xx_keypad.c | 116 ++++++++-----------------
>  1 file changed, 36 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c
> index a66cfeaf5b21..90157707dc05 100644
> --- a/drivers/input/keyboard/ep93xx_keypad.c
> +++ b/drivers/input/keyboard/ep93xx_keypad.c

...

> @@ -227,61 +234,46 @@ static int ep93xx_keypad_probe(struct platform_device *pdev)
>         struct resource *res;
>         int err;
>  
> -       keypad = kzalloc(sizeof(struct ep93xx_keypad), GFP_KERNEL);
> +       keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
>         if (!keypad)
>                 return -ENOMEM;
>  
>         keypad->pdata = dev_get_platdata(&pdev->dev);
> -       if (!keypad->pdata) {
> -               err = -EINVAL;
> -               goto failed_free;
> -       }
> +       if (!keypad->pdata)
> +               return -EINVAL;
>  
>         keymap_data = keypad->pdata->keymap_data;
> -       if (!keymap_data) {
> -               err = -EINVAL;
> -               goto failed_free;
> -       }
> +       if (!keymap_data)
> +               return -EINVAL;
>  
>         keypad->irq = platform_get_irq(pdev, 0);
> -       if (keypad->irq < 0) {
> -               err = keypad->irq;
> -               goto failed_free;
> -       }
> +       if (keypad->irq < 0)
> +               return keypad->irq;
>  
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       if (!res) {
> -               err = -ENXIO;
> -               goto failed_free;
> -       }
> -
> -       res = request_mem_region(res->start, resource_size(res), pdev->name);
> -       if (!res) {
> -               err = -EBUSY;
> -               goto failed_free;
> -       }
> +       if (!res)
> +               return -ENXIO;
>  
> -       keypad->mmio_base = ioremap(res->start, resource_size(res));
> -       if (keypad->mmio_base == NULL) {
> -               err = -ENXIO;
> -               goto failed_free_mem;
> -       }
> +       keypad->mmio_base = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(keypad->mmio_base))
> +               return PTR_ERR(keypad->mmio_base);
>  
>         err = ep93xx_keypad_acquire_gpio(pdev);
>         if (err)
> -               goto failed_free_io;
> +               return err;
> +
> +       err = devm_add_action_or_reset(&pdev->dev,
> +                                      ep93xx_keypad_release_gpio_action, pdev);
> +       if (err)
> +               return err;
>  
>         keypad->clk = clk_get(&pdev->dev, NULL);

Isn't the conversion to devm_clk_get() missing here?

> -       if (IS_ERR(keypad->clk)) {
> -               err = PTR_ERR(keypad->clk);
> -               goto failed_free_gpio;
> -       }
> +       if (IS_ERR(keypad->clk))
> +               return PTR_ERR(keypad->clk);
>  
> -       input_dev = input_allocate_device();
> -       if (!input_dev) {
> -               err = -ENOMEM;
> -               goto failed_put_clk;
> -       }
> +       input_dev = devm_input_allocate_device(&pdev->dev);
> +       if (!input_dev)
> +               return -ENOMEM;
>  
>         keypad->input_dev = input_dev;
>  
> @@ -289,26 +281,26 @@ static int ep93xx_keypad_probe(struct platform_device *pdev)
>         input_dev->id.bustype = BUS_HOST;
>         input_dev->open = ep93xx_keypad_open;
>         input_dev->close = ep93xx_keypad_close;
> -       input_dev->dev.parent = &pdev->dev;
>  
>         err = matrix_keypad_build_keymap(keymap_data, NULL,
>                                          EP93XX_MATRIX_ROWS, EP93XX_MATRIX_COLS,
>                                          keypad->keycodes, input_dev);
>         if (err)
> -               goto failed_free_dev;
> +               return err;
>  
>         if (keypad->pdata->flags & EP93XX_KEYPAD_AUTOREPEAT)
>                 __set_bit(EV_REP, input_dev->evbit);
>         input_set_drvdata(input_dev, keypad);
>  
> -       err = request_irq(keypad->irq, ep93xx_keypad_irq_handler,
> -                         0, pdev->name, keypad);
> +       err = devm_request_irq(&pdev->dev, keypad->irq,
> +                              ep93xx_keypad_irq_handler,
> +                              0, pdev->name, keypad);
>         if (err)
> -               goto failed_free_dev;
> +               return err;
>  
>         err = input_register_device(input_dev);
>         if (err)
> -               goto failed_free_irq;
> +               return err;
>  
>         platform_set_drvdata(pdev, keypad);
>  

-- 
Alexander Sverdlin.



  reply	other threads:[~2021-10-12  7:54 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-12  1:37 [PATCH 1/4] Input: ep93xx_keypad - annotate suspend/resume as __maybe_unused Dmitry Torokhov
2021-10-12  1:37 ` [PATCH 2/4] Input: ep93xx_keypad - use BIT() and GENMASK() macros Dmitry Torokhov
2021-10-12  6:58   ` Alexander Sverdlin
2021-10-12  1:37 ` [PATCH 3/4] Input: ep93xx_keypad - use dev_pm_set_wake_irq() Dmitry Torokhov
2021-10-12  7:30   ` Alexander Sverdlin
2021-10-12  1:37 ` [PATCH 4/4] Input: ep93xx_keypad - switch to using managed resources Dmitry Torokhov
2021-10-12  7:54   ` Alexander Sverdlin [this message]
2021-10-13  2:34     ` Dmitry Torokhov
2021-10-13  2:36   ` [PATCH v2 " Dmitry Torokhov
2021-10-13  5:42     ` Alexander Sverdlin
2021-10-12  6:31 ` [PATCH 1/4] Input: ep93xx_keypad - annotate suspend/resume as __maybe_unused Alexander Sverdlin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d1d1a1028c5db26d0b0eed2a5a9626124451fcf9.camel@gmail.com \
    --to=alexander.sverdlin@gmail.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.