Re: [PATCH v4] Touchscreen driver for FT5x06 based EDT displays

[Anatolij Gustschin <agust@denx.de>]

Hi Simon,

thanks for reworking. I've tested the driver on our board, now it
also works with our pin configuration. I have some minor comments,
please see below.

On Tue,  6 Mar 2012 17:15:27 +0100
simon.budig@kernelconcepts.de wrote:

> From: Simon Budig <simon.budig@kernelconcepts.de>
> 
> This is a driver for the EDT "Polytouch" family of touch controllers
> based on the FocalTech FT5x06 line of chips.
> 
> Signed-off-by: Simon Budig <simon.budig@kernelconcepts.de>
> ---
...
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> new file mode 100644
> index 0000000..eb31025
> --- /dev/null
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
...
> +	tsdata->reset_pin = pdata->reset_pin;
> +	mutex_init(&tsdata->mutex);
> +
> +	if (tsdata->reset_pin >= 0) {
> +		error = gpio_request(tsdata->reset_pin, NULL);

It would be helpful to have labels for requested gpio pins. Can you pass
"ft5x06 reset" instead of NULL here?

> +		if (error < 0) {
> +			dev_err(&client->dev,
> +			        "Failed to request GPIO %d as reset pin, error %d\n",
> +			         tsdata->reset_pin, error);
> +			error = -ENOMEM;

Please drop this 'error = -ENOMEM;'. We should return
error code returned by gpio_request() here.

> +			goto err_free_tsdata;
> +		}
> +
> +		/* this pulls reset down, enabling the low active reset */
> +		if (gpio_direction_output(tsdata->reset_pin, 0) < 0) {
> +			dev_info(&client->dev, "switching to output failed\n");
> +			goto err_free_reset_pin;
> +		}
> +	}
> +
> +	/* request IRQ pin */
> +	tsdata->irq_pin = pdata->irq_pin;
> +	tsdata->irq = gpio_to_irq(tsdata->irq_pin);
> +
> +	error = gpio_request(tsdata->irq_pin, NULL);

gpio pin label, too?

Thanks,
Anatolij