[V3,06/10] input: touchscreen: ili210x: Add reset GPIO support
diff mbox series

Message ID 20190103012938.7442-7-marex@denx.de
State Superseded
Headers show
Series
  • input: touchscreen: ili210x: Add ILI2511 support
Related show

Commit Message

Marek Vasut Jan. 3, 2019, 1:29 a.m. UTC
The touchscreen can have a reset GPIO connected to it, add support
for such an arrangement.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Henrik Rydberg <rydberg@bitmath.org>
Cc: Olivier Sobrie <olivier@sobrie.be>
Cc: Philipp Puschmann <pp@emlix.com>
To: linux-input@vger.kernel.org
---
V2: No change
V3: Use devm_add_action_or_reset() for reset GPIO teardown
---
 drivers/input/touchscreen/ili210x.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Dmitry Torokhov Jan. 15, 2019, 12:34 a.m. UTC | #1
On Thu, Jan 03, 2019 at 02:29:34AM +0100, Marek Vasut wrote:
> The touchscreen can have a reset GPIO connected to it, add support
> for such an arrangement.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Henrik Rydberg <rydberg@bitmath.org>
> Cc: Olivier Sobrie <olivier@sobrie.be>
> Cc: Philipp Puschmann <pp@emlix.com>
> To: linux-input@vger.kernel.org
> ---
> V2: No change
> V3: Use devm_add_action_or_reset() for reset GPIO teardown
> ---
>  drivers/input/touchscreen/ili210x.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index 436bfedb6769..a193ddb06e10 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -5,6 +5,7 @@
>  #include <linux/input.h>
>  #include <linux/input/mt.h>
>  #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>  
>  #define MAX_TOUCHES		2
>  #define DEFAULT_POLL_PERIOD	20
> @@ -44,6 +45,7 @@ struct ili210x {
>  	struct input_dev *input;
>  	unsigned int poll_period;
>  	struct delayed_work dwork;
> +	struct gpio_desc *reset_gpio;
>  };
>  
>  static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
> @@ -167,11 +169,19 @@ static const struct attribute_group ili210x_attr_group = {
>  	.attrs = ili210x_attributes,
>  };
>  
> +static void ili210x_power_down(void *data)
> +{
> +	struct gpio_desc *reset_gpio = data;
> +
> +	gpiod_set_value_cansleep(reset_gpio, 1);
> +}
> +
>  static int ili210x_i2c_probe(struct i2c_client *client,
>  				       const struct i2c_device_id *id)
>  {
>  	struct device *dev = &client->dev;
>  	struct ili210x *priv;
> +	struct gpio_desc *reset_gpio;
>  	struct input_dev *input;
>  	struct panel_info panel;
>  	struct firmware_version firmware;
> @@ -185,6 +195,22 @@ static int ili210x_i2c_probe(struct i2c_client *client,
>  		return -EINVAL;
>  	}
>  
> +	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(reset_gpio))
> +		return PTR_ERR(reset_gpio);
> +
> +	if (reset_gpio) {
> +		error = devm_add_action_or_reset(dev, ili210x_power_down,
> +						 reset_gpio);
> +		if (error)
> +			return error;
> +
> +		gpiod_set_value_cansleep(reset_gpio, 1);

You already requested it as "high", there is no need to try to driver it
high again here.

> +		usleep_range(50, 100);
> +		gpiod_set_value_cansleep(reset_gpio, 0);
> +		mdelay(100);

usleep above but mdelay here? I do not think we have to spin here.

> +	}
> +
>  	/* Get firmware version */
>  	error = ili210x_read_reg(client, REG_FIRMWARE_VERSION,
>  				 &firmware, sizeof(firmware));
> @@ -217,6 +243,7 @@ static int ili210x_i2c_probe(struct i2c_client *client,
>  	priv->input = input;
>  	priv->poll_period = DEFAULT_POLL_PERIOD;
>  	INIT_DELAYED_WORK(&priv->dwork, ili210x_work);
> +	priv->reset_gpio = reset_gpio;
>  
>  	/* Setup input device */
>  	input->name = "ILI210x Touchscreen";
> -- 
> 2.19.2
> 

Thanks.
Marek Vasut Jan. 15, 2019, 2:07 a.m. UTC | #2
On 1/15/19 1:34 AM, Dmitry Torokhov wrote:
> On Thu, Jan 03, 2019 at 02:29:34AM +0100, Marek Vasut wrote:
>> The touchscreen can have a reset GPIO connected to it, add support
>> for such an arrangement.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: Henrik Rydberg <rydberg@bitmath.org>
>> Cc: Olivier Sobrie <olivier@sobrie.be>
>> Cc: Philipp Puschmann <pp@emlix.com>
>> To: linux-input@vger.kernel.org
>> ---
>> V2: No change
>> V3: Use devm_add_action_or_reset() for reset GPIO teardown
>> ---
>>  drivers/input/touchscreen/ili210x.c | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
>> index 436bfedb6769..a193ddb06e10 100644
>> --- a/drivers/input/touchscreen/ili210x.c
>> +++ b/drivers/input/touchscreen/ili210x.c
>> @@ -5,6 +5,7 @@
>>  #include <linux/input.h>
>>  #include <linux/input/mt.h>
>>  #include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>>  
>>  #define MAX_TOUCHES		2
>>  #define DEFAULT_POLL_PERIOD	20
>> @@ -44,6 +45,7 @@ struct ili210x {
>>  	struct input_dev *input;
>>  	unsigned int poll_period;
>>  	struct delayed_work dwork;
>> +	struct gpio_desc *reset_gpio;
>>  };
>>  
>>  static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
>> @@ -167,11 +169,19 @@ static const struct attribute_group ili210x_attr_group = {
>>  	.attrs = ili210x_attributes,
>>  };
>>  
>> +static void ili210x_power_down(void *data)
>> +{
>> +	struct gpio_desc *reset_gpio = data;
>> +
>> +	gpiod_set_value_cansleep(reset_gpio, 1);
>> +}
>> +
>>  static int ili210x_i2c_probe(struct i2c_client *client,
>>  				       const struct i2c_device_id *id)
>>  {
>>  	struct device *dev = &client->dev;
>>  	struct ili210x *priv;
>> +	struct gpio_desc *reset_gpio;
>>  	struct input_dev *input;
>>  	struct panel_info panel;
>>  	struct firmware_version firmware;
>> @@ -185,6 +195,22 @@ static int ili210x_i2c_probe(struct i2c_client *client,
>>  		return -EINVAL;
>>  	}
>>  
>> +	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>> +	if (IS_ERR(reset_gpio))
>> +		return PTR_ERR(reset_gpio);
>> +
>> +	if (reset_gpio) {
>> +		error = devm_add_action_or_reset(dev, ili210x_power_down,
>> +						 reset_gpio);
>> +		if (error)
>> +			return error;
>> +
>> +		gpiod_set_value_cansleep(reset_gpio, 1);
> 
> You already requested it as "high", there is no need to try to driver it
> high again here.
> 
>> +		usleep_range(50, 100);
>> +		gpiod_set_value_cansleep(reset_gpio, 0);
>> +		mdelay(100);
> 
> usleep above but mdelay here? I do not think we have to spin here.

Both fixed

Patch
diff mbox series

diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 436bfedb6769..a193ddb06e10 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -5,6 +5,7 @@ 
 #include <linux/input.h>
 #include <linux/input/mt.h>
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 
 #define MAX_TOUCHES		2
 #define DEFAULT_POLL_PERIOD	20
@@ -44,6 +45,7 @@  struct ili210x {
 	struct input_dev *input;
 	unsigned int poll_period;
 	struct delayed_work dwork;
+	struct gpio_desc *reset_gpio;
 };
 
 static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
@@ -167,11 +169,19 @@  static const struct attribute_group ili210x_attr_group = {
 	.attrs = ili210x_attributes,
 };
 
+static void ili210x_power_down(void *data)
+{
+	struct gpio_desc *reset_gpio = data;
+
+	gpiod_set_value_cansleep(reset_gpio, 1);
+}
+
 static int ili210x_i2c_probe(struct i2c_client *client,
 				       const struct i2c_device_id *id)
 {
 	struct device *dev = &client->dev;
 	struct ili210x *priv;
+	struct gpio_desc *reset_gpio;
 	struct input_dev *input;
 	struct panel_info panel;
 	struct firmware_version firmware;
@@ -185,6 +195,22 @@  static int ili210x_i2c_probe(struct i2c_client *client,
 		return -EINVAL;
 	}
 
+	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset_gpio))
+		return PTR_ERR(reset_gpio);
+
+	if (reset_gpio) {
+		error = devm_add_action_or_reset(dev, ili210x_power_down,
+						 reset_gpio);
+		if (error)
+			return error;
+
+		gpiod_set_value_cansleep(reset_gpio, 1);
+		usleep_range(50, 100);
+		gpiod_set_value_cansleep(reset_gpio, 0);
+		mdelay(100);
+	}
+
 	/* Get firmware version */
 	error = ili210x_read_reg(client, REG_FIRMWARE_VERSION,
 				 &firmware, sizeof(firmware));
@@ -217,6 +243,7 @@  static int ili210x_i2c_probe(struct i2c_client *client,
 	priv->input = input;
 	priv->poll_period = DEFAULT_POLL_PERIOD;
 	INIT_DELAYED_WORK(&priv->dwork, ili210x_work);
+	priv->reset_gpio = reset_gpio;
 
 	/* Setup input device */
 	input->name = "ILI210x Touchscreen";