Received-SPF: pass (google.com: domain of linux-kernel+bounces-35087-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 23 Jan 2024 11:26:41 +0100 (CET)
From: Jiri Kosina <jikos@kernel.org>
To: Doug Anderson <dianders@chromium.org>
cc: Kai-Heng Feng <kai.heng.feng@canonical.com>, benjamin.tissoires@redhat.com, 
    Hans de Goede <hdegoede@redhat.com>, Maxime Ripard <mripard@kernel.org>, 
    =?ISO-8859-15?Q?Thomas_Wei=DFschuh?= <linux@weissschuh.net>, 
    Johan Hovold <johan+linaro@kernel.org>, linux-input@vger.kernel.org, 
    linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] HID: i2c-hid: Skip SET_POWER SLEEP for Cirque touchpad
 on system suspend
In-Reply-To: <CAD=FV=VCoB_uBontRk3WVn-nqzZnutSMCHBpdBqNOFwrYv=8xw@mail.gmail.com>
Message-ID: <nycvar.YFH.7.76.2401231126300.29548@cbobk.fhfr.pm>
References: <20240115045054.1170294-1-kai.heng.feng@canonical.com> <CAD=FV=VCoB_uBontRk3WVn-nqzZnutSMCHBpdBqNOFwrYv=8xw@mail.gmail.com>
User-Agent: Alpine 2.21 (LSU 202 2017-01-01)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

On Wed, 17 Jan 2024, Doug Anderson wrote:

> > There's a Cirque touchpad that wakes system up without anything 
> > touched the touchpad. The input report is empty when this happens. The 
> > reason is stated in HID over I2C spec, 7.2.8.2: "If the DEVICE wishes 
> > to wake the HOST from its low power state, it can issue a wake by 
> > asserting the interrupt."
> >
> > This is fine if OS can put system back to suspend by identifying input 
> > wakeup count stays the same on resume, like Chrome OS Dark Resume [0]. 
> > But for regular distro such policy is lacking.
> >
> > Though the change doesn't bring any impact on power consumption for 
> > touchpad is minimal, other i2c-hid device may depends on SLEEP control 
> > power. So use a quirk to limit the change scope.
> >
> > [0] https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/power_manager/docs/dark_resume.md
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > v2:
> >  - Use quirk instead of applying the change universally.
> >
> >  drivers/hid/hid-ids.h              | 3 +++
> >  drivers/hid/i2c-hid/i2c-hid-core.c | 6 +++++-
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> This seems OK to me. As per my repy to v1, it doesn't feel fully 
> explained what's going on. Why does it only wake the system without 
> touches if you put it to sleep first? Is it truly a hardware quirk with 
> the Cirque touchpad or is it a bug on the board you have it hooked up 
> to? In any case, perhaps it doesn't matter since you said you measured 
> power here and, on this touchpad it doesn't seem to save significant 
> extra power to go into sleep mode. ...so I guess I'd be OK w/
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs