Re: [PATCH 1/3] Input: rotary_encoder - convert to devm-* api

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Fri, Jan 15, 2016 at 10:33:48PM +0200, Timo Teräs wrote:
> Use managed resource API for simplifying error paths.
> 
> Signed-off-by: Timo Teräs <timo.teras@iki.fi>
> ---
>  drivers/input/misc/rotary_encoder.c | 73 ++++++++++---------------------------
>  1 file changed, 20 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index 8aee719..0f23d32 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -211,8 +211,8 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
>  	if (!of_id || !np)
>  		return NULL;
>  
> -	pdata = kzalloc(sizeof(struct rotary_encoder_platform_data),
> -			GFP_KERNEL);
> +	pdata = devm_kzalloc(dev, sizeof(struct rotary_encoder_platform_data),
> +			     GFP_KERNEL);
>  	if (!pdata)
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -277,12 +277,10 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
> -	input = input_allocate_device();
> -	if (!encoder || !input) {
> -		err = -ENOMEM;
> -		goto exit_free_mem;
> -	}
> +	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
> +	input = devm_input_allocate_device(dev);
> +	if (!encoder || !input)
> +		return -ENOMEM;
>  
>  	encoder->input = input;
>  	encoder->pdata = pdata;
> @@ -301,16 +299,16 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  	}
>  
>  	/* request the GPIOs */
> -	err = gpio_request_one(pdata->gpio_a, GPIOF_IN, dev_name(dev));
> +	err = devm_gpio_request_one(dev, pdata->gpio_a, GPIOF_IN, dev_name(dev));
>  	if (err) {
>  		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_a);
> -		goto exit_free_mem;
> +		return err;
>  	}
>  
> -	err = gpio_request_one(pdata->gpio_b, GPIOF_IN, dev_name(dev));
> +	err = devm_gpio_request_one(dev, pdata->gpio_b, GPIOF_IN, dev_name(dev));
>  	if (err) {
>  		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_b);
> -		goto exit_free_gpio_a;
> +		return err;
>  	}
>  
>  	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> @@ -331,30 +329,29 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  	default:
>  		dev_err(dev, "'%d' is not a valid steps-per-period value\n",
>  			pdata->steps_per_period);
> -		err = -EINVAL;
> -		goto exit_free_gpio_b;
> +		return-EINVAL;

It is customary to put a space between return and return value.

>  	}
>  
> -	err = request_irq(encoder->irq_a, handler,
> -			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> -			  DRV_NAME, encoder);
> +	err = devm_request_irq(dev, encoder->irq_a, handler,
> +			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			       DRV_NAME, encoder);
>  	if (err) {
>  		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_a);
> -		goto exit_free_gpio_b;
> +		return err;
>  	}
>  
> -	err = request_irq(encoder->irq_b, handler,
> -			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> -			  DRV_NAME, encoder);
> +	err = devm_request_irq(dev, encoder->irq_b, handler,
> +			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			       DRV_NAME, encoder);
>  	if (err) {
>  		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_b);
> -		goto exit_free_irq_a;
> +		return err;
>  	}
>  
>  	err = input_register_device(input);
>  	if (err) {
>  		dev_err(dev, "failed to register input device\n");
> -		goto exit_free_irq_b;
> +		return err;
>  	}
>  
>  	device_init_wakeup(&pdev->dev, pdata->wakeup_source);
> @@ -362,42 +359,12 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, encoder);
>  
>  	return 0;
> -
> -exit_free_irq_b:
> -	free_irq(encoder->irq_b, encoder);
> -exit_free_irq_a:
> -	free_irq(encoder->irq_a, encoder);
> -exit_free_gpio_b:
> -	gpio_free(pdata->gpio_b);
> -exit_free_gpio_a:
> -	gpio_free(pdata->gpio_a);
> -exit_free_mem:
> -	input_free_device(input);
> -	kfree(encoder);
> -	if (!dev_get_platdata(&pdev->dev))
> -		kfree(pdata);
> -
> -	return err;
>  }
>  
>  static int rotary_encoder_remove(struct platform_device *pdev)
>  {
> -	struct rotary_encoder *encoder = platform_get_drvdata(pdev);
> -	const struct rotary_encoder_platform_data *pdata = encoder->pdata;
> -
>  	device_init_wakeup(&pdev->dev, false);

I think we can get rid of this as well (most of the drivers do not reset
wakeup capability on unbind, and if this is needed it is better be done
at bus level), so rotary_encoder_remove() can be removed altogether.

>  
> -	free_irq(encoder->irq_a, encoder);
> -	free_irq(encoder->irq_b, encoder);
> -	gpio_free(pdata->gpio_a);
> -	gpio_free(pdata->gpio_b);
> -
> -	input_unregister_device(encoder->input);
> -	kfree(encoder);
> -
> -	if (!dev_get_platdata(&pdev->dev))
> -		kfree(pdata);
> -
>  	return 0;
>  }

I fixed up the above points and applied.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html