Skip to content

Commit

Permalink
HID: i2c-hid: Suspend i2c-hid devices in remove
Browse files Browse the repository at this point in the history
In the i2c-hid remove() function we currently try to power off,
depopulate our child device, and free our resources. That's OK, but...

* If the i2c-hid device is on a power rail that can't turn off (either
  an always-on or a shared power rail) we won't try to put the device
  in a low power state during remove(). This probably doesn't matter
  for very many devices but it could be nice in some instances.

* If the i2c-hid device somehow manages to generate an interrupt after
  we tried to power off it is conceivable that the interrupt could
  arrive during or after the call to hid_destroy_device() but before
  the call to free_irq(). That could cause a crash since our IRQ
  handler isn't expecting it. One could imagine this happening in
  the case where we couldn't turn off (see the previous bullet) or,
  possibly, if the interrupt line could glitch shortly after the
  device powered off.

Let's call the suspend code during remove to avoid these issues. That
will put the device into a low power state and also disable
interrupts.

Technically, one could consider this a "fix" of commit 4a200c3
("HID: i2c-hid: introduce HID over i2c specification implementation").
However, since the above bullet points are more theoretical than
problems seen on real systems and since the remove() of an i2c-hid
touchscreen isn't terribly likely to be called in production, it's
probably not worth the bother of trying to backport it.

Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>
Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20230727101636.v4.8.Ic3ecad4a825905f4e4ce2a772b17f3c9cb2d60a2@changeid
  • Loading branch information
Douglas Anderson committed Aug 1, 2023
1 parent d93d284 commit 5f8838e
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions drivers/hid/i2c-hid/i2c-hid-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -941,7 +941,7 @@ static void i2c_hid_core_shutdown_tail(struct i2c_hid *ihid)
ihid->ops->shutdown_tail(ihid->ops);
}

static int i2c_hid_core_suspend(struct i2c_hid *ihid)
static int i2c_hid_core_suspend(struct i2c_hid *ihid, bool force_poweroff)
{
struct i2c_client *client = ihid->client;
struct hid_device *hid = ihid->hid;
Expand All @@ -956,7 +956,7 @@ static int i2c_hid_core_suspend(struct i2c_hid *ihid)

disable_irq(client->irq);

if (!device_may_wakeup(&client->dev))
if (force_poweroff || !device_may_wakeup(&client->dev))
i2c_hid_core_power_down(ihid);

return 0;
Expand Down Expand Up @@ -1143,7 +1143,7 @@ void i2c_hid_core_remove(struct i2c_client *client)
struct i2c_hid *ihid = i2c_get_clientdata(client);
struct hid_device *hid;

i2c_hid_core_power_down(ihid);
i2c_hid_core_suspend(ihid, true);

hid = ihid->hid;
hid_destroy_device(hid);
Expand Down Expand Up @@ -1171,7 +1171,7 @@ static int i2c_hid_core_pm_suspend(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);

return i2c_hid_core_suspend(ihid);
return i2c_hid_core_suspend(ihid, false);
}

static int i2c_hid_core_pm_resume(struct device *dev)
Expand Down

0 comments on commit 5f8838e

Please sign in to comment.