On Fri, 5 May 2023, Dmitry Torokhov wrote:
> More and more drivers rely on devres to manage their resources, however
> if bus' probe() and release() methods are not trivial and control some
> of resources as well (for example enable or disable clocks, or attach
> device to a power domain), we need to make sure that driver-allocated
> resources are released immediately after driver's remove() method
> returns, and not postponed until driver core gets around to releasing
> resources.
>
> In case of HID we should not try to close the report and release
> associated memory until after all devres callbacks are executed. To fix
> that we open a new devres group before calling driver's probe() and
> explicitly release it when we return from driver's remove().
>
> This is similar to what we did for I2C bus in commit 5b5475826c52 ("i2c:
> ensure timely release of driver-allocated resources"). It is tempting to
> try and move this into driver core, but actually doing so is challenging,
> we need to split bus' remove() method into pre- and post-remove methods,
> which would make the logic even less clear.
>
> Reported-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> Link: https://lore.kernel.org/r/20230505232417.1377393-1-swboyd@xxxxxxxxxxxx
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
> drivers/hid/hid-core.c | 55 ++++++++++++++++++++++++++++--------------
> include/linux/hid.h | 1 +
> 2 files changed, 38 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index c4ac9081194c..02a43bba9091 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2602,35 +2602,29 @@ static bool hid_device_check_match(struct hid_device *hdev,
> return !hid_ignore_special_drivers;
> }
>
> -static int hid_device_probe(struct device *dev)
> +static int __hid_device_probe(struct hid_device *hdev)
> {
> - struct hid_driver *hdrv = to_hid_driver(dev->driver);
> - struct hid_device *hdev = to_hid_device(dev);
> + struct hid_driver *hdrv = to_hid_driver(hdev->dev.driver);
> const struct hid_device_id *id;
> int ret;
>
> - if (down_interruptible(&hdev->driver_input_lock)) {
> - ret = -EINTR;
> - goto end;
> - }
> hdev->io_started = false;
> -
> clear_bit(ffs(HID_STAT_REPROBED), &hdev->status);
>
> - if (hdev->driver) {
> - ret = 0;
> - goto unlock;
> - }
> + if (hdev->driver)
> + return 0;
>
> - if (!hid_device_check_match(hdev, hdrv, &id)) {
> - ret = -ENODEV;
> - goto unlock;
> - }
Dmitry, which tree is this patch against, please? The code above looks
different in current tree (and hasn't been touched for a while).
Thanks,
--
Jiri Kosina
SUSE Labs
