On Sep 04 2024, Li Zetao wrote: > Currently, the shield module needs to maintain hid resources > by itself. Consider using devm_hid_hw_start_and_open helper to ensure > that hid resources are consistent with the device life cycle, and release > hid resources before device is released. At the same time, it can avoid > the goto-release encoding, drop the err_stop and err_ts_create lables, and > directly return the error code when an error occurs. > > Signed-off-by: Li Zetao <lizetao1@xxxxxxxxxx> > --- > drivers/hid/hid-nvidia-shield.c | 20 +++----------------- > 1 file changed, 3 insertions(+), 17 deletions(-) > > diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c > index ff9078ad1961..747996a21dd9 100644 > --- a/drivers/hid/hid-nvidia-shield.c > +++ b/drivers/hid/hid-nvidia-shield.c > @@ -1070,27 +1070,15 @@ static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id) > > ts = container_of(shield_dev, struct thunderstrike, base); > > - ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT); > + ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDINPUT); > if (ret) { > - hid_err(hdev, "Failed to start HID device\n"); > - goto err_ts_create; > - } > - > - ret = hid_hw_open(hdev); > - if (ret) { > - hid_err(hdev, "Failed to open HID device\n"); > - goto err_stop; > + thunderstrike_destroy(ts); > + return ret; > } > > thunderstrike_device_init_info(shield_dev); > > return ret; > - > -err_stop: > - hid_hw_stop(hdev); > -err_ts_create: > - thunderstrike_destroy(ts); > - return ret; > } > > static void shield_remove(struct hid_device *hdev) > @@ -1100,11 +1088,9 @@ static void shield_remove(struct hid_device *hdev) > > ts = container_of(dev, struct thunderstrike, base); > > - hid_hw_close(hdev); > thunderstrike_destroy(ts); > del_timer_sync(&ts->psy_stats_timer); > cancel_work_sync(&ts->hostcmd_req_work); > - hid_hw_stop(hdev); There is definitely something fishy here. The 2 calls to hid_hw_close and hid_hw_stop are not one after the other and at the very end of the .remove(). The patch is changing the behavior, and this might be an issue. Cheers, Benjamin > } > > static const struct hid_device_id shield_devices[] = { > -- > 2.34.1 >