Received-SPF: pass (google.com: domain of linux-kernel+bounces-22604-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240103003355.747335-1-kai.heng.feng@canonical.com>
 <CAD=FV=VmGNB5dP5WO7=txNDScNfhCDEsfFFivXqz+PH6rt=x8g@mail.gmail.com> <CAAd53p4gPkbDyNLiYGtcvqWwEgBqVhri+qgh+=Ha0xsVfYy92g@mail.gmail.com>
In-Reply-To: <CAAd53p4gPkbDyNLiYGtcvqWwEgBqVhri+qgh+=Ha0xsVfYy92g@mail.gmail.com>
From: Doug Anderson <dianders@chromium.org>
Date: Wed, 10 Jan 2024 10:34:23 -0800
Message-ID: <CAD=FV=WwCL0Z4rOMGD+i2zOTZMz6qi_Ctm_wWC2sbv0STGi8AQ@mail.gmail.com>
Subject: Re: [PATCH] HID: i2c-hid: Remove SET_POWER SLEEP on system suspend
To: Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: jikos@kernel.org, benjamin.tissoires@redhat.com, 
	Maxime Ripard <mripard@kernel.org>, =?UTF-8?Q?Thomas_Wei=C3=9Fschuh?= <linux@weissschuh.net>, 
	Johan Hovold <johan+linaro@kernel.org>, Dmitry Torokhov <dmitry.torokhov@gmail.com>, 
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Tue, Jan 9, 2024 at 11:31=E2=80=AFPM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> > I'd also note that I'm really not sure what ChromeOS dark resume has
> > to do with anything here. Dark resume is used for certain types of
> > events that wakeup the system where we can identify that the event
> > shouldn't turn the screen on, then we do some processing, then we go
> > back to sleep. I'm nearly certain that a trackpad / touchscreen wakeup
> > event would never qualify for "dark resume". If we see a
> > trackpad/touchscreen event then we'll wakeup the system. If the system
> > is in a state where trackpad/touchscreen events shouldn't wake us up
> > then we disable those wakeups before going to suspend...
>
> Doesn't Dark Resume use wakeup count to decide whether the system
> should wake up or go back to suspend?
> For this case the input report is empty, hence wakeup count remains
> the same after the wakeup. I assumed Dark Resume will check the wakeup
> count and decide to put the system back to suspend.

Ah, I understand now. So you're saying that the issue wouldn't be so
bad (or maybe we wouldn't notice it) on systems with dark resume.
However, even with dark resume we're not in a super great shape. Doing
a dark resume isn't exactly a lightweight operation, since it can take
a bit of time to resume the system, realize that there were no wakeup
events, and then go back to sleep. I'm not a total expert on dark
resume, but I believe that even with dark resume, there may also be
artifacts that a user might notice (like perhaps USB devices powering
up or perhaps the suspend LED on the system showing that we're not in
suspend anymore).


> > It seems to me like the board you're testing on has some strange bug
> > and that bug should be fixed, or (in the worst case) you should send a
> > patch to detect this broken touchpad and disable wakeup for it.
>
> It's desired to keep the wakeup capability, disabling wakeup isn't ideal =
here.
> I'll write a patch to use touchpad specific quirk instead of applying
> the change universally.

Thanks! I'd also be curious if this is a problem for everyone with the
Cirque touchpad or if it's board-specific. I could imagine the
behavior you describe as coming about due to a missing or
misconfigured pull resistor on the IRQ line. ...or perhaps a pull
resistor pulling up to the wrong voltage rail...

-Doug