Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1238470 > unrolled thread

Re: [PATCH] input: gpio_keys: Make pdev vs dev usage more consistent

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] input: gpio_keys: Make pdev vs dev usage more consistent Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2015-10-02 19:50 +0200
    Re: [PATCH] input: gpio_keys: Make pdev vs dev usage more consistent Bjorn Andersson <bjorn.andersson@sonymobile.com> - 2015-10-02 21:10 +0200

#1238470 — Re: [PATCH] input: gpio_keys: Make pdev vs dev usage more consistent

On Mon, Jul 27, 2015 at 06:50:18PM -0700, Bjorn Andersson wrote:
> As gpio_keys_setup_key() only operates on the device, pass a pointer to
> this from the probe instead of a platform_device and make the usage
> consistent. Also make probe() more consistent by dropping the local copy
> of the device pointer.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>

Don't really see the difference either way... We trade &pdev->dev for
dev in one place and trade dev for &pdev->dev in another. I'd rather
skip it.

Thanks.

> ---
>  drivers/input/keyboard/gpio_keys.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 3ce3298ac09e..2ffad10e2825 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -440,13 +440,12 @@ static void gpio_keys_quiesce_key(void *data)
>  		del_timer_sync(&bdata->release_timer);
>  }
>  
> -static int gpio_keys_setup_key(struct platform_device *pdev,
> +static int gpio_keys_setup_key(struct device *dev,
>  				struct input_dev *input,
>  				struct gpio_button_data *bdata,
>  				const struct gpio_keys_button *button)
>  {
>  	const char *desc = button->desc ? button->desc : "gpio_keys";
> -	struct device *dev = &pdev->dev;
>  	irq_handler_t isr;
>  	unsigned long irqflags;
>  	int irq;
> @@ -458,7 +457,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>  
>  	if (gpio_is_valid(button->gpio)) {
>  
> -		error = devm_gpio_request_one(&pdev->dev, button->gpio,
> +		error = devm_gpio_request_one(dev, button->gpio,
>  					      GPIOF_IN, desc);
>  		if (error < 0) {
>  			dev_err(dev, "Failed to request GPIO %d, error %d\n",
> @@ -520,9 +519,9 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>  	 * Install custom action to cancel release timer and
>  	 * workqueue item.
>  	 */
> -	error = devm_add_action(&pdev->dev, gpio_keys_quiesce_key, bdata);
> +	error = devm_add_action(dev, gpio_keys_quiesce_key, bdata);
>  	if (error) {
> -		dev_err(&pdev->dev,
> +		dev_err(dev,
>  			"failed to register quiesce action, error: %d\n",
>  			error);
>  		return error;
> @@ -535,7 +534,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>  	if (!button->can_disable)
>  		irqflags |= IRQF_SHARED;
>  
> -	error = devm_request_any_context_irq(&pdev->dev, bdata->irq,
> +	error = devm_request_any_context_irq(dev, bdata->irq,
>  					     isr, irqflags, desc, bdata);
>  	if (error < 0) {
>  		dev_err(dev, "Unable to claim irq %d; error %d\n",
> @@ -694,31 +693,31 @@ gpio_keys_get_devtree_pdata(struct device *dev)
>  
>  static int gpio_keys_probe(struct platform_device *pdev)
>  {
> -	struct device *dev = &pdev->dev;
> -	const struct gpio_keys_platform_data *pdata = dev_get_platdata(dev);
> +	const struct gpio_keys_platform_data *pdata;
>  	struct gpio_keys_drvdata *ddata;
>  	struct input_dev *input;
>  	size_t size;
>  	int i, error;
>  	int wakeup = 0;
>  
> +	pdata = dev_get_platdata(&pdev->dev);
>  	if (!pdata) {
> -		pdata = gpio_keys_get_devtree_pdata(dev);
> +		pdata = gpio_keys_get_devtree_pdata(&pdev->dev);
>  		if (IS_ERR(pdata))
>  			return PTR_ERR(pdata);
>  	}
>  
>  	size = sizeof(struct gpio_keys_drvdata) +
>  			pdata->nbuttons * sizeof(struct gpio_button_data);
> -	ddata = devm_kzalloc(dev, size, GFP_KERNEL);
> +	ddata = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
>  	if (!ddata) {
> -		dev_err(dev, "failed to allocate state\n");
> +		dev_err(&pdev->dev, "failed to allocate state\n");
>  		return -ENOMEM;
>  	}
>  
> -	input = devm_input_allocate_device(dev);
> +	input = devm_input_allocate_device(&pdev->dev);
>  	if (!input) {
> -		dev_err(dev, "failed to allocate input device\n");
> +		dev_err(&pdev->dev, "failed to allocate input device\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -748,7 +747,7 @@ static int gpio_keys_probe(struct platform_device *pdev)
>  		const struct gpio_keys_button *button = &pdata->buttons[i];
>  		struct gpio_button_data *bdata = &ddata->data[i];
>  
> -		error = gpio_keys_setup_key(pdev, input, bdata, button);
> +		error = gpio_keys_setup_key(&pdev->dev, input, bdata, button);
>  		if (error)
>  			return error;
>  
> @@ -758,14 +757,16 @@ static int gpio_keys_probe(struct platform_device *pdev)
>  
>  	error = sysfs_create_group(&pdev->dev.kobj, &gpio_keys_attr_group);
>  	if (error) {
> -		dev_err(dev, "Unable to export keys/switches, error: %d\n",
> +		dev_err(&pdev->dev,
> +			"Unable to export keys/switches, error: %d\n",
>  			error);
>  		return error;
>  	}
>  
>  	error = input_register_device(input);
>  	if (error) {
> -		dev_err(dev, "Unable to register input device, error: %d\n",
> +		dev_err(&pdev->dev,
> +			"Unable to register input device, error: %d\n",
>  			error);
>  		goto err_remove_group;
>  	}
> -- 
> 1.8.2.2
> 

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

[toc] | [next] | [standalone]

#1238536

On Fri 02 Oct 10:47 PDT 2015, Dmitry Torokhov wrote:

> On Mon, Jul 27, 2015 at 06:50:18PM -0700, Bjorn Andersson wrote:
> > As gpio_keys_setup_key() only operates on the device, pass a pointer to
> > this from the probe instead of a platform_device and make the usage
> > consistent. Also make probe() more consistent by dropping the local copy
> > of the device pointer.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> 
> Don't really see the difference either way... We trade &pdev->dev for
> dev in one place and trade dev for &pdev->dev in another. I'd rather
> skip it.
> 
> Thanks.
> 

No worries, my dislike of the mixing of the two uses within the first
function was enough to hack up a patch - feel free to drop it as you
don't think it's worth the churn.


Thanks for merging the error handling patch though.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web