On Tue, May 23, 2023 at 03:57:03PM +0200, Jiri Kosina wrote:
> 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).
My bad, I had some patches in my tree that I forgot about. I sent out
a v2.
Thanks.
--
Dmitry
