Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2] HID: i2c-hid: Unfollow the panel on shutdown
@ 2026-09-08  1:38 Justin Yeh
  2026-09-09 21:34 ` Doug Anderson
  2026-09-11 15:38 ` Jiri Kosina
  0 siblings, 2 replies; 3+ messages in thread
From: Justin Yeh @ 2026-09-08  1:38 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Matthias Brugger,
	AngeloGioacchino Del Regno, Douglas Anderson, Maxime Ripard
  Cc: Project_Global_Chrome_Upstream_Group, linux-arm-kernel,
	linux-input, linux-kernel, linux-mediatek, Justin Yeh

i2c_hid_core_shutdown() sends SET_POWER(SLEEP) even for panel followers,
but nothing in device_shutdown() guarantees whether the panel, and hence
our own power, is still on by the time we get there.

Do what i2c_hid_core_remove() does and just unfollow the panel.
drm_panel_remove_follower() calls panel_unpreparing() only while the
panel is still prepared, which powers us down properly; if it is already
unprepared no transfer is attempted at all. On MT8196 the panel is
unprepared long before us, so this also drops the ~2s i2c bus timeout
from every reboot.

Fixes: 96a37bfd232a ("HID: i2c-hid: Support being a panel follower")
Suggested-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Justin Yeh <justin.yeh@mediatek.com>
---
Changes in v2:
- Unfollow the panel instead of just skipping SET_POWER, so we no longer
  depend on the panel having been unprepared before us (Doug).
- Link to v1: https://lore.kernel.org/all/20260904035009.4114363-1-justin.yeh@mediatek.com/
---
 drivers/hid/i2c-hid/i2c-hid-core.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 0ff07fdab442..c7e46004f513 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1355,7 +1355,16 @@ void i2c_hid_core_shutdown(struct i2c_client *client)
 {
 	struct i2c_hid *ihid = i2c_get_clientdata(client);
 
-	i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
+	/*
+	 * Like in i2c_hid_core_remove(): if we're a follower, the act of
+	 * unfollowing will cause us to be powered down. Otherwise we need to
+	 * manually do it.
+	 */
+	if (ihid->is_panel_follower)
+		drm_panel_remove_follower(&ihid->panel_follower);
+	else
+		i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
+
 	free_irq(client->irq, ihid);
 
 	i2c_hid_core_shutdown_tail(ihid);
-- 
2.45.2


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

* Re: [PATCH v2] HID: i2c-hid: Unfollow the panel on shutdown
  2026-09-08  1:38 [PATCH v2] HID: i2c-hid: Unfollow the panel on shutdown Justin Yeh
@ 2026-09-09 21:34 ` Doug Anderson
  2026-09-11 15:38 ` Jiri Kosina
  1 sibling, 0 replies; 3+ messages in thread
From: Doug Anderson @ 2026-09-09 21:34 UTC (permalink / raw)
  To: Justin Yeh
  Cc: Jiri Kosina, Benjamin Tissoires, Matthias Brugger,
	AngeloGioacchino Del Regno, Maxime Ripard,
	Project_Global_Chrome_Upstream_Group, linux-arm-kernel,
	linux-input, linux-kernel, linux-mediatek

Hi,

On Mon, Sep 7, 2026 at 6:38 PM Justin Yeh <justin.yeh@mediatek.com> wrote:
>
> i2c_hid_core_shutdown() sends SET_POWER(SLEEP) even for panel followers,
> but nothing in device_shutdown() guarantees whether the panel, and hence
> our own power, is still on by the time we get there.
>
> Do what i2c_hid_core_remove() does and just unfollow the panel.
> drm_panel_remove_follower() calls panel_unpreparing() only while the
> panel is still prepared, which powers us down properly; if it is already
> unprepared no transfer is attempted at all. On MT8196 the panel is
> unprepared long before us, so this also drops the ~2s i2c bus timeout
> from every reboot.
>
> Fixes: 96a37bfd232a ("HID: i2c-hid: Support being a panel follower")
> Suggested-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Justin Yeh <justin.yeh@mediatek.com>
> ---
> Changes in v2:
> - Unfollow the panel instead of just skipping SET_POWER, so we no longer
>   depend on the panel having been unprepared before us (Doug).
> - Link to v1: https://lore.kernel.org/all/20260904035009.4114363-1-justin.yeh@mediatek.com/
> ---
>  drivers/hid/i2c-hid/i2c-hid-core.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 0ff07fdab442..c7e46004f513 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1355,7 +1355,16 @@ void i2c_hid_core_shutdown(struct i2c_client *client)
>  {
>         struct i2c_hid *ihid = i2c_get_clientdata(client);
>
> -       i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
> +       /*
> +        * Like in i2c_hid_core_remove(): if we're a follower, the act of
> +        * unfollowing will cause us to be powered down. Otherwise we need to
> +        * manually do it.
> +        */
> +       if (ihid->is_panel_follower)
> +               drm_panel_remove_follower(&ihid->panel_follower);
> +       else
> +               i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);

This seems right to me. One note is for panel followers if the panel
_hasn't_ been powered off by now, the drm_panel_remove_follower() call
will cause "i2c_hid_core_suspend(ihid, true)" to be called. That does
quite a bit more than just "i2c_hid_set_power(ihid,
I2C_HID_PWR_SLEEP)". ...but I think that's fine. I would expect most
systems would be powering off the panel at shutdown time anyway.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

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

* Re: [PATCH v2] HID: i2c-hid: Unfollow the panel on shutdown
  2026-09-08  1:38 [PATCH v2] HID: i2c-hid: Unfollow the panel on shutdown Justin Yeh
  2026-09-09 21:34 ` Doug Anderson
@ 2026-09-11 15:38 ` Jiri Kosina
  1 sibling, 0 replies; 3+ messages in thread
From: Jiri Kosina @ 2026-09-11 15:38 UTC (permalink / raw)
  To: Justin Yeh
  Cc: Benjamin Tissoires, Matthias Brugger, AngeloGioacchino Del Regno,
	Douglas Anderson, Maxime Ripard,
	Project_Global_Chrome_Upstream_Group, linux-arm-kernel,
	linux-input, linux-kernel, linux-mediatek

On Tue, 8 Sep 2026, Justin Yeh wrote:

> i2c_hid_core_shutdown() sends SET_POWER(SLEEP) even for panel followers,
> but nothing in device_shutdown() guarantees whether the panel, and hence
> our own power, is still on by the time we get there.
> 
> Do what i2c_hid_core_remove() does and just unfollow the panel.
> drm_panel_remove_follower() calls panel_unpreparing() only while the
> panel is still prepared, which powers us down properly; if it is already
> unprepared no transfer is attempted at all. On MT8196 the panel is
> unprepared long before us, so this also drops the ~2s i2c bus timeout
> from every reboot.
> 
> Fixes: 96a37bfd232a ("HID: i2c-hid: Support being a panel follower")
> Suggested-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Justin Yeh <justin.yeh@mediatek.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs


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

end of thread, other threads:[~2026-09-11 15:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08  1:38 [PATCH v2] HID: i2c-hid: Unfollow the panel on shutdown Justin Yeh
2026-09-09 21:34 ` Doug Anderson
2026-09-11 15:38 ` Jiri Kosina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox