devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Fix Goodix touchscreen power leakage for MT8186 boards
@ 2023-04-26 14:44 Fei Shao
  2023-04-26 14:44 ` [PATCH v3 1/2] dt-bindings: input: goodix: Add "goodix,no-reset-during-suspend" property Fei Shao
  0 siblings, 1 reply; 3+ messages in thread
From: Fei Shao @ 2023-04-26 14:44 UTC (permalink / raw)
  To: Jeff LaBundy, Douglas Anderson, Benjamin Tissoires, Rob Herring
  Cc: linux-mediatek, Fei Shao, Dmitry Torokhov, Jiri Kosina,
	Krzysztof Kozlowski, Matthias Kaehlcke, Stephen Kitt, devicetree,
	linux-input, linux-kernel

These changes are based on the series in [1], which modified the
i2c-hid-of-goodix driver and removed the workaround for a power leakage
issue, so the issue revisits on Mediatek MT8186 boards (Steelix).

The root cause is that the touchscreen can be powered in different ways
depending on the hardware designs, and it's not as easy to come up with
a solution that is both simple and elegant for all the known designs.

To address the issue, I ended up adding a new boolean property for the
driver so that we can control the power up/down sequence depending on
that.

Adding a new property might not be the cleanest approach for this, but
at least the intention would be easy enough to understand, and it
introduces relatively small change to the code and fully preserves the
original control flow.
I hope this is something acceptable, and I'm open to any better
approaches.

[1] https://lore.kernel.org/all/20230207024816.525938-1-dianders@chromium.org/

Changes in v3:
- In power-down, only skip the GPIO but not the regulator calls if the
  flag is set

Changes in v2:
- Use a more accurate property name and with "goodix," prefix.
- Do not change the regulator_enable logic during power-up.

Fei Shao (2):
  dt-bindings: input: goodix: Add "goodix,no-reset-during-suspend"
    property
  HID: i2c-hid: goodix: Add support for "goodix,no-reset-during-suspend"
    property

 .../bindings/input/goodix,gt7375p.yaml           |  9 +++++++++
 drivers/hid/i2c-hid/i2c-hid-of-goodix.c          | 16 +++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

-- 
2.40.1.495.gc816e09b53d-goog


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v3 1/2] dt-bindings: input: goodix: Add "goodix,no-reset-during-suspend" property
  2023-04-26 14:44 [PATCH v3 0/2] Fix Goodix touchscreen power leakage for MT8186 boards Fei Shao
@ 2023-04-26 14:44 ` Fei Shao
  2023-04-27  1:01   ` Jeff LaBundy
  0 siblings, 1 reply; 3+ messages in thread
From: Fei Shao @ 2023-04-26 14:44 UTC (permalink / raw)
  To: Jeff LaBundy, Douglas Anderson, Benjamin Tissoires, Rob Herring
  Cc: linux-mediatek, Fei Shao, Matthias Brugger, Dmitry Torokhov,
	Krzysztof Kozlowski, devicetree, linux-input, linux-kernel

We observed that on Chromebook device Steelix, if Goodix GT7375P
touchscreen is powered in suspend (because, for example, it connects to
an always-on regulator) and with the reset GPIO asserted, it will
introduce about 14mW power leakage.

To address that, we add this property to skip reset during suspend.
If it's set, the driver will stop asserting the reset GPIO during
power-down. Refer to the comments in the driver for details.

Signed-off-by: Fei Shao <fshao@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
---

(no changes since v2)

Changes in v2:
- Use a more accurate property name and with "goodix," prefix.

 .../devicetree/bindings/input/goodix,gt7375p.yaml        | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
index ce18d7dadae2..1edad1da1196 100644
--- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
+++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
@@ -43,6 +43,15 @@ properties:
       itself as long as it allows the main board to make signals compatible
       with what the touchscreen is expecting for its IO rails.
 
+  goodix,no-reset-during-suspend:
+    description:
+      Set this to true to enforce the driver to not assert the reset GPIO
+      during suspend.
+      Due to potential touchscreen hardware flaw, back-powering could happen in
+      suspend if the power supply is on and with active-low reset GPIO asserted.
+      This property is used to avoid the back-powering issue.
+    type: boolean
+
 required:
   - compatible
   - reg
-- 
2.40.1.495.gc816e09b53d-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings: input: goodix: Add "goodix,no-reset-during-suspend" property
  2023-04-26 14:44 ` [PATCH v3 1/2] dt-bindings: input: goodix: Add "goodix,no-reset-during-suspend" property Fei Shao
@ 2023-04-27  1:01   ` Jeff LaBundy
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff LaBundy @ 2023-04-27  1:01 UTC (permalink / raw)
  To: Fei Shao
  Cc: Douglas Anderson, Benjamin Tissoires, Rob Herring, linux-mediatek,
	Matthias Brugger, Dmitry Torokhov, Krzysztof Kozlowski,
	devicetree, linux-input, linux-kernel

Hi Fei,

On Wed, Apr 26, 2023 at 10:44:21PM +0800, Fei Shao wrote:
> We observed that on Chromebook device Steelix, if Goodix GT7375P
> touchscreen is powered in suspend (because, for example, it connects to
> an always-on regulator) and with the reset GPIO asserted, it will
> introduce about 14mW power leakage.
> 
> To address that, we add this property to skip reset during suspend.
> If it's set, the driver will stop asserting the reset GPIO during
> power-down. Refer to the comments in the driver for details.
> 
> Signed-off-by: Fei Shao <fshao@chromium.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---

Reviewed-by: Jeff LaBundy <jeff@labundy.com>

> 
> (no changes since v2)
> 
> Changes in v2:
> - Use a more accurate property name and with "goodix," prefix.
> 
>  .../devicetree/bindings/input/goodix,gt7375p.yaml        | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> index ce18d7dadae2..1edad1da1196 100644
> --- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> +++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> @@ -43,6 +43,15 @@ properties:
>        itself as long as it allows the main board to make signals compatible
>        with what the touchscreen is expecting for its IO rails.
>  
> +  goodix,no-reset-during-suspend:
> +    description:
> +      Set this to true to enforce the driver to not assert the reset GPIO
> +      during suspend.
> +      Due to potential touchscreen hardware flaw, back-powering could happen in
> +      suspend if the power supply is on and with active-low reset GPIO asserted.
> +      This property is used to avoid the back-powering issue.
> +    type: boolean
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.40.1.495.gc816e09b53d-goog
> 

Many thanks to you and Doug for the informative discussion.

Kind regards,
Jeff LaBundy

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-04-27  1:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-26 14:44 [PATCH v3 0/2] Fix Goodix touchscreen power leakage for MT8186 boards Fei Shao
2023-04-26 14:44 ` [PATCH v3 1/2] dt-bindings: input: goodix: Add "goodix,no-reset-during-suspend" property Fei Shao
2023-04-27  1:01   ` Jeff LaBundy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).