[PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property

[Fei Shao]

Hi,

On Wed, Apr 19, 2023 at 11:41 PM Doug Anderson <dianders at chromium.org> wrote:
>
> Hi,
>
> On Wed, Apr 19, 2023 at 8:18 AM Jeff LaBundy <jeff at labundy.com> wrote:
> >
> > Hi Doug and Fei,
> >
> > Thank you for the informative discussion; I can empathize with the pain
> > these issues bring.
> >
> > On Wed, Apr 19, 2023 at 07:38:13AM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Wed, Apr 19, 2023 at 3:44 AM Fei Shao <fshao at chromium.org> wrote:
> > > >
> > > > Hi Jeff,
> > > >
> > > > On Wed, Apr 19, 2023 at 8:21 AM Jeff LaBundy <jeff at labundy.com> wrote:
> > > > >
> > > > > Hi Fei,
> > > > >
> > > > > On Tue, Apr 18, 2023 at 08:49:51PM +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.
> > > > > >
> > > > > > This property is used to indicate that the touchscreen is powered in
> > > > > > suspend. If it's set, the driver will stop asserting the reset GPIO in
> > > > > > power-down, and it will do it in power-up instead to ensure that the
> > > > > > state is always reset after resuming.
> > > > > >
> > > > > > Signed-off-by: Fei Shao <fshao at chromium.org>
> > > > > > ---
> > > > >
> > > > > This is an interesting problem; were you able to root-cause why the silicon
> > > > > exhibits this behavior? Simply asserting reset should not cause it to draw
> > > > > additional power, let alone 14 mW. This almost sounds like a back-powering
> > > > > problem during suspend.
> > > > >
> > > > There was a fix for this behavior before so I didn't dig into it on
> > > > the silicon side.
> > > > I can ask internally and see if we can have Goodix to confirm this is
> > > > a known HW erratum.
> > >
> > > Certainly it doesn't hurt to check, but it's not really that shocking
> > > to me that asserting reset could cause a power draw on some hardware.
> > > Reset puts hardware into a default state and that's not necessarily
> > > low power. I guess ideally hardware would act like it's "off" when
> > > reset is asserted and then then init to the default state on the edge
> > > as reset was deasserted, but I not all hardware is designed in an
> > > ideal way.
> >
> > While that is true in theory, I have never, ever seen that to be the case
> > when there is not some other underlying problem.
> >
> > What I have seen, however, is that asserting reset actually causes the GPIO
> > to sink current from some other supply and through the IC. I loosely suspect
> > that if you probe the IC's rails and digital I/O during the failure condition,
> > you may find one of them resting at some mid-rail voltage or diode drop. It
> > seems you have a similar suspicion.
> >

I reached out to our EE with your thoughts.
He said that he understands the concern, but this doesn't apply in our
schematics because there's only one supply.
Also he simulated a few scenarios that could explain the
back-powering, but none of them is possible without having the
problematic circuit/rsense layout from within the IC itself.

> > In that case, it may mean that some other supply in the system should actually
> > be kept on, or that supplies are being brought down out of order. In which
> > case, the solution should actually be a patch to the affected platform(s) dts
> > and not the mainline driver.
>
> I agree that it's a bandaid, but I'm not hopeful that a better
> solution will be found.
>
> Specifically, I'd expect a current leak in the system when you turn a
> supply off and then assert a GPIO high. That's when the device can
> start backpowering itself from a GPIO. In this case, it's the
> opposite. We're keeping the supply on and asserting the (active low)
> reset GPIO to cause the higher power draw. In another design it was
> confirmed that the power draw went away when we were able to turn the
> regulator off (but still keep the active low reset GPIO asserted).
> We've also confirmed that power is good if we keep the supply on and
> _don't_ assert the reset GPIO. Both of these two experiments provide
> some evidence that the system is configured properly and we're not
> backpowering something.
>
> I guess I should revise the above, though. I could believe that there
> is a current leak but on the touchscreen controller board itself,
> which is a black box to us. I have certainly been involved in products
> in the past where the default state of the system at reset caused a
> minor current leak (I remember an EE telling me that as soon as
> software started running I should quickly change the direction of a
> GPIO) and it wouldn't shock me if the touchscreen controller board had
> a problem like this. If there is a problem like this on the
> touchscreen controller board there's not much we can do to workaround
> it.
>
> Unfortunately, if the problem ends up needing a hardware change to fix
> more correctly (which I suspect it does), our hands are tied a bit.
> This is not prototype hardware but is final hardware.
>
> I guess one further note is that, at least on the project I was
> involved in that had a similar problem, folks in China did a bunch of
> analysis on this and went as far as adding an extra regulator to the
> main board schematic to "fix" it. Had the issue just been something
> where we were misconfiguing GPIOs or leaving a regulator in the wrong
> state then they (probably) would have identified it rather than
> spinning the board.

Thank you Doug for providing the details and explanation, and sorry
that I also missed your original reply...
I only considered the ideal scenarios for the always_on usage but not
the cases you brought up. The concerns make sense to me.

I'm still awaiting the response from Goodix, but +1 that if it turns
out to be a GT7375P hw issue, we're not able to do much about that
except relying on the driver workaround.
One more note I want to add is that the first attempt of the fix was
added ~2yrs ago, so it's not an one-off on a particular platform, plus
`sc7180-trogdor-homestar` and `mt8186-corsola-steelix` are two
different designs come from two different teams, but they ended up
with the same symptom.
With that said, I think we have more confidence to say it's a
component misbehavior, and we just fell into the edge case that was
not covered or considered by its design.

Regards,
Fei

>
> -Doug