diff mbox series

[resend,03/10] Input: goodix - Make resetting the controller at probe independent from the GPIO setup

Message ID 20200221164735.508324-3-hdegoede@redhat.com (mailing list archive)
State Superseded
Headers show
Series [resend,01/10] Input: goodix - Refactor IRQ pin GPIO accesses | expand

Commit Message

Hans de Goede Feb. 21, 2020, 4:47 p.m. UTC
Before this commit we would always reset the controller at probe when we
have access to the GPIOs which are necessary to do a reset.

Doing the reset requires access to the GPIOs, but just because we have
access to the GPIOs does not mean that we should always reset the
controller at probe. On X86 ACPI platforms the BIOS / UEFI firmware will
already have reset the controller and it will have loaded the device
specific config into the controller. Doing the reset sometimes causes the
controller to loose its configuration, so on X86 ACPI platforms this is not
a good idea.

This commit adds a new reset_controller_at_probe boolean to control the
reset at probe behavior.

This commits sets the new bool to true when we set irq_pin_access_method
to irq_pin_access_gpio, so there are no functional changes.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1786317
BugLink: https://github.com/nexus511/gpd-ubuntu-packages/issues/10
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199207
Cc: Dmitry Mastykin <mastichi@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/touchscreen/goodix.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Bastien Nocera March 2, 2020, 11:14 a.m. UTC | #1
On Fri, 2020-02-21 at 17:47 +0100, Hans de Goede wrote:
> Before this commit we would always reset the controller at probe when
> we
> have access to the GPIOs which are necessary to do a reset.
> 
> Doing the reset requires access to the GPIOs, but just because we
> have
> access to the GPIOs does not mean that we should always reset the
> controller at probe. On X86 ACPI platforms the BIOS / UEFI firmware
> will
> already have reset the controller and it will have loaded the device
> specific config into the controller. Doing the reset sometimes causes
> the
> controller to loose its configuration, so on X86 ACPI platforms this 

lose.

> is not
> a good idea.
> 
> This commit adds a new reset_controller_at_probe boolean to control
> the
> reset at probe behavior.
> 
> This commits sets the new bool to true when we set
> irq_pin_access_method
> to irq_pin_access_gpio, so there are no functional changes.
> 
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1786317
> BugLink: https://github.com/nexus511/gpd-ubuntu-packages/issues/10
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199207
> Cc: Dmitry Mastykin <mastichi@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Looks good apart from the typo in the commit message

Reviewed-by: Bastien Nocera <hadess@hadess.net>

> ---
>  drivers/input/touchscreen/goodix.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index eccf07adfae1..dd5a8f9e8ada 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -56,6 +56,7 @@ struct goodix_ts_data {
>  	u16 id;
>  	u16 version;
>  	const char *cfg_name;
> +	bool reset_controller_at_probe;
>  	bool load_cfg_from_disk;
>  	struct completion firmware_loading_complete;
>  	unsigned long irq_flags;
> @@ -656,6 +657,7 @@ static int goodix_get_gpio_config(struct
> goodix_ts_data *ts)
>  	ts->gpiod_rst = gpiod;
>  
>  	if (ts->gpiod_int && ts->gpiod_rst) {
> +		ts->reset_controller_at_probe = true;
>  		ts->load_cfg_from_disk = true;
>  		ts->irq_pin_access_method = irq_pin_access_gpio;
>  	}
> @@ -932,7 +934,7 @@ static int goodix_ts_probe(struct i2c_client
> *client,
>  	if (error)
>  		return error;
>  
> -	if (ts->irq_pin_access_method == irq_pin_access_gpio) {
> +	if (ts->reset_controller_at_probe) {
>  		/* reset the controller */
>  		error = goodix_reset(ts);
>  		if (error) {
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index eccf07adfae1..dd5a8f9e8ada 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -56,6 +56,7 @@  struct goodix_ts_data {
 	u16 id;
 	u16 version;
 	const char *cfg_name;
+	bool reset_controller_at_probe;
 	bool load_cfg_from_disk;
 	struct completion firmware_loading_complete;
 	unsigned long irq_flags;
@@ -656,6 +657,7 @@  static int goodix_get_gpio_config(struct goodix_ts_data *ts)
 	ts->gpiod_rst = gpiod;
 
 	if (ts->gpiod_int && ts->gpiod_rst) {
+		ts->reset_controller_at_probe = true;
 		ts->load_cfg_from_disk = true;
 		ts->irq_pin_access_method = irq_pin_access_gpio;
 	}
@@ -932,7 +934,7 @@  static int goodix_ts_probe(struct i2c_client *client,
 	if (error)
 		return error;
 
-	if (ts->irq_pin_access_method == irq_pin_access_gpio) {
+	if (ts->reset_controller_at_probe) {
 		/* reset the controller */
 		error = goodix_reset(ts);
 		if (error) {