Re: [PATCH v3] Input: ad7879: support auxiliary GPIOs via gpiolib

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

Hi Mike,

On Sun, Jan 17, 2010 at 10:36:42AM -0500, Mike Frysinger wrote:
> +
> +#ifdef CONFIG_GPIOLIB
> +int ad7879_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
>  {

Sparse recommends making this static and it seems like a good idea. The
same goes for the 3 new fucntions below.

> +
> +static inline int ad7879_gpio_add(struct device *dev)

Why inline? Let's compiler decide, it's certainly not a hot path.

> +{
> +	struct ad7879 *ts = dev_get_drvdata(dev);
> +	struct ad7879_platform_data *pdata = dev->platform_data;
> +	int ret = 0;
> +
> +	if (pdata->gpio_export) {
> +		ts->gc.direction_input = ad7879_gpio_direction_input;
> +		ts->gc.direction_output = ad7879_gpio_direction_output;
> +		ts->gc.get = ad7879_gpio_get_value;
> +		ts->gc.set = ad7879_gpio_set_value;
> +		ts->gc.can_sleep = 1;
> +		ts->gc.base = pdata->gpio_base;
> +		ts->gc.ngpio = 1;
> +		ts->gc.label = "AD7879-GPIO";
> +		ts->gc.owner = THIS_MODULE;
> +		ts->gc.dev = dev;
> +
> +		ret = gpiochip_add(&ts->gc);
> +		if (ret)
> +			dev_err(dev, "failed to register gpio %d\n",
> +				ts->gc.base);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ad7879_gpio_remove(struct device *dev)

There is no chance we can handle errors returned by this function so I'd
make it 'void'.

Let me know if you agree with the above, there is no need to resubmit.

Thanks.

-- 
Dmitry