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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to