Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

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

Hi Bastian,

On Mon, Apr 08, 2013 at 02:52:26PM +0200, Bastian Hecht wrote:
> +static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
> +{
> +	if (ts->reset_gpio >= 0)

This should be gpio_is_valid() I think.

> +		gpio_direction_output(ts->reset_gpio, poweron);
> +}
> +
>  static int st1232_ts_probe(struct i2c_client *client,
>  					const struct i2c_device_id *id)
>  {
>  	struct st1232_ts_data *ts;
> +	struct st1232_pdata *pdata = client->dev.platform_data;
>  	struct input_dev *input_dev;
>  	int error;
>  
> @@ -167,6 +178,25 @@ static int st1232_ts_probe(struct i2c_client *client,
>  	ts->client = client;
>  	ts->input_dev = input_dev;
>  
> +	if (client->dev.of_node)
> +		ts->reset_gpio = of_get_gpio(client->dev.of_node, 0);
> +	else if (pdata)
> +		ts->reset_gpio = pdata->reset_gpio;

I believe we should favor platform data, then firmware, so that you can
override if necessary.

> +	else
> +		ts->reset_gpio = -ENODEV;
> +
> +	if (ts->reset_gpio >= 0) {

gpio_is_valid() again?

> +		error = devm_gpio_request(&client->dev, ts->reset_gpio, NULL);

Frankly I do not like mixing managed and noon-managed resources. Maybe
the entire driver needs to be converted to managed resources?

Thanks.

-- 
Dmitry