Hi Dmitry, On Wed, Apr 20, 2022 at 10:51 PM Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > > Hi Angela, > > On Tue, Apr 19, 2022 at 12:26:32PM +0000, Angela Czubak wrote: > > @@ -529,6 +529,8 @@ static void i2c_hid_get_input(struct i2c_hid *ihid) > > /* host or device initiated RESET completed */ > > if (test_and_clear_bit(I2C_HID_RESET_PENDING, &ihid->flags)) > > wake_up(&ihid->wait); > > + if (ihid->hid && ihid->hid->driver && ihid->hid->driver->reset) > > + ihid->hid->driver->reset(ihid->hid); > > I wonder if this would not be better to execute the reset callback > first, before signalling that the reset has completed instead of racing > with i2c_hid_hw_reset()? > I think it could result in a deadlock. If we don't clear I2C_HID_RESET_PENDING, and if it has been set, then reset_lock is still taken. This way, if the reset callback wants to send a report to the device, it will keep spinning on reset_lock in i2c_hid_output_raw_report(). Since the reset callback will be most likely used to re-configure the device, we need to be able to send any report and not hang on reset_lock. Let me know if you think this not an issue or there is an additional comment needed in the patch so that the reasoning standing by the order of issuing the callback and clearing the bit is clear. > Thanks. > > -- > Dmitry