| Message ID | 20230312184058.552305-1-jajadekroon@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] input: touchscreen: goodix: Hold controller in reset during suspend | expand |
Hi, On 3/12/23 19:40, Jan Jasper de Kroon wrote: > From: Ondřej Jirman <megi@xff.cz> > > The Goodix touchscreen controller used in PinePhone is not properly > suspended during system sleep, leading to high power consumption. This > commit modifies the driver to hold the controller in reset during > system sleep, reducing power consumption by around 40mW. > > The original patch also disabled the regulators during system sleep, but > this could cause issues with other peripherals attached to the same power > supplies. Hence, this commit only holds the controller in reset during > system sleep and does not attempt to power it off. > > Separate commit adds a new device tree property "hold-in-reset-in-suspend" > to control the hold-in-reset behavior of the controller during system > sleep. > > Signed-off-by: Ondřej Jirman <megi@xff.cz> > Signed-off-by: Jan Jasper de Kroon <jajadekroon@gmail.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > V1 -> V2: > - Rewrote the commit message and function name to better reflect it's > behavior. > v2 -> V3: Some patch style cleaning up, to confirm to submission standard. > > drivers/input/touchscreen/goodix.c | 23 +++++++++++++++++++++++ > drivers/input/touchscreen/goodix.h | 1 + > 2 files changed, 24 insertions(+) > > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c > index b348172f19c3..a35a203e04bf 100644 > --- a/drivers/input/touchscreen/goodix.c > +++ b/drivers/input/touchscreen/goodix.c > @@ -1284,6 +1284,7 @@ static void goodix_disable_regulators(void *arg) > > static int goodix_ts_probe(struct i2c_client *client) > { > + struct device_node *np = client->dev.of_node; > struct goodix_ts_data *ts; > const char *cfg_name; > int error; > @@ -1303,6 +1304,7 @@ static int goodix_ts_probe(struct i2c_client *client) > i2c_set_clientdata(client, ts); > init_completion(&ts->firmware_loading_complete); > ts->contact_size = GOODIX_CONTACT_SIZE; > + ts->hold_in_reset_in_suspend = of_property_read_bool(np, "hold-in-reset-in-suspend"); > > error = goodix_get_gpio_config(ts); > if (error) > @@ -1410,6 +1412,13 @@ static int goodix_suspend(struct device *dev) > if (ts->load_cfg_from_disk) > wait_for_completion(&ts->firmware_loading_complete); > > + if (ts->hold_in_reset_in_suspend) { > + goodix_free_irq(ts); > + goodix_irq_direction_output(ts, 0); > + gpiod_direction_output(ts->gpiod_rst, 0); > + return 0; > + } > + > /* We need gpio pins to suspend/resume */ > if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) { > disable_irq(client->irq); > @@ -1455,6 +1464,20 @@ static int goodix_resume(struct device *dev) > u8 config_ver; > int error; > > + if (ts->hold_in_reset_in_suspend) { > + error = goodix_reset(ts); > + if (error) { > + dev_err(dev, "Controller reset failed.\n"); > + return error; > + } > + > + error = goodix_request_irq(ts); > + if (error) > + return error; > + > + return 0; > + } > + > if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) { > enable_irq(client->irq); > return 0; > diff --git a/drivers/input/touchscreen/goodix.h b/drivers/input/touchscreen/goodix.h > index 87797cc88b32..c515d03b742c 100644 > --- a/drivers/input/touchscreen/goodix.h > +++ b/drivers/input/touchscreen/goodix.h > @@ -104,6 +104,7 @@ struct goodix_ts_data { > u8 main_clk[GOODIX_MAIN_CLK_LEN]; > int bak_ref_len; > u8 *bak_ref; > + bool hold_in_reset_in_suspend; > }; > > int goodix_i2c_read(struct i2c_client *client, u16 reg, u8 *buf, int len);
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c index b348172f19c3..a35a203e04bf 100644 --- a/drivers/input/touchscreen/goodix.c +++ b/drivers/input/touchscreen/goodix.c @@ -1284,6 +1284,7 @@ static void goodix_disable_regulators(void *arg) static int goodix_ts_probe(struct i2c_client *client) { + struct device_node *np = client->dev.of_node; struct goodix_ts_data *ts; const char *cfg_name; int error; @@ -1303,6 +1304,7 @@ static int goodix_ts_probe(struct i2c_client *client) i2c_set_clientdata(client, ts); init_completion(&ts->firmware_loading_complete); ts->contact_size = GOODIX_CONTACT_SIZE; + ts->hold_in_reset_in_suspend = of_property_read_bool(np, "hold-in-reset-in-suspend"); error = goodix_get_gpio_config(ts); if (error) @@ -1410,6 +1412,13 @@ static int goodix_suspend(struct device *dev) if (ts->load_cfg_from_disk) wait_for_completion(&ts->firmware_loading_complete); + if (ts->hold_in_reset_in_suspend) { + goodix_free_irq(ts); + goodix_irq_direction_output(ts, 0); + gpiod_direction_output(ts->gpiod_rst, 0); + return 0; + } + /* We need gpio pins to suspend/resume */ if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) { disable_irq(client->irq); @@ -1455,6 +1464,20 @@ static int goodix_resume(struct device *dev) u8 config_ver; int error; + if (ts->hold_in_reset_in_suspend) { + error = goodix_reset(ts); + if (error) { + dev_err(dev, "Controller reset failed.\n"); + return error; + } + + error = goodix_request_irq(ts); + if (error) + return error; + + return 0; + } + if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) { enable_irq(client->irq); return 0; diff --git a/drivers/input/touchscreen/goodix.h b/drivers/input/touchscreen/goodix.h index 87797cc88b32..c515d03b742c 100644 --- a/drivers/input/touchscreen/goodix.h +++ b/drivers/input/touchscreen/goodix.h @@ -104,6 +104,7 @@ struct goodix_ts_data { u8 main_clk[GOODIX_MAIN_CLK_LEN]; int bak_ref_len; u8 *bak_ref; + bool hold_in_reset_in_suspend; }; int goodix_i2c_read(struct i2c_client *client, u16 reg, u8 *buf, int len);