[ This code is really old, but I was just looking at it as an example of some Smatch warnings around hid_device_io_start() and I don't really understand how that function works. ] Hello Benjamin Tissoires, The patch 2f31c5252910: "HID: Introduce hidpp, a module to handle Logitech hid++ devices" from Sep 30, 2014, leads to the following Smatch static checker warning: drivers/hid/hid-logitech-hidpp.c:4205 hidpp_probe() warn: '&hdev->driver_input_lock' both locked and unlocked. drivers/hid/hid-logitech-hidpp.c 4117 /* 4118 * Plain USB connections need to actually call start and open 4119 * on the transport driver to allow incoming data. 4120 */ 4121 ret = hid_hw_start(hdev, 0); 4122 if (ret) { 4123 hid_err(hdev, "hw start failed\n"); 4124 goto hid_hw_start_fail; 4125 } 4126 4127 ret = hid_hw_open(hdev); 4128 if (ret < 0) { 4129 dev_err(&hdev->dev, "%s:hid_hw_open returned error:%d\n", 4130 __func__, ret); 4131 goto hid_hw_open_fail; 4132 } 4133 4134 /* Allow incoming packets */ 4135 hid_device_io_start(hdev); IO starts here. 4136 4137 if (hidpp->quirks & HIDPP_QUIRK_UNIFYING) 4138 hidpp_unifying_init(hidpp); 4139 4140 connected = hidpp_root_get_protocol_version(hidpp) == 0; 4141 atomic_set(&hidpp->connected, connected); 4142 if (!(hidpp->quirks & HIDPP_QUIRK_UNIFYING)) { 4143 if (!connected) { 4144 ret = -ENODEV; 4145 hid_err(hdev, "Device not connected"); 4146 goto hid_hw_init_fail; 4147 } 4148 4149 hidpp_overwrite_name(hdev); 4150 } 4151 4152 if (connected && hidpp->protocol_major >= 2) { 4153 ret = hidpp_set_wireless_feature_index(hidpp); 4154 if (ret == -ENOENT) 4155 hidpp->wireless_feature_index = 0; 4156 else if (ret) 4157 goto hid_hw_init_fail; 4158 } 4159 4160 if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) { 4161 ret = wtp_get_config(hidpp); 4162 if (ret) 4163 goto hid_hw_init_fail; 4164 } else if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)) { 4165 ret = g920_get_config(hidpp, &data); 4166 if (ret) 4167 goto hid_hw_init_fail; 4168 } 4169 4170 hidpp_connect_event(hidpp); 4171 4172 /* Reset the HID node state */ 4173 hid_device_io_stop(hdev); We stop the IO here, but not if g920_get_config() fails for example. I would normally put a hid_device_io_stop() before the goto, I guess? Unrelated, to your driver but in ccp_probe() it calls hid_device_io_start(). Should there be a matching hid_device_io_stop() in the ccp_remove() function? 4174 hid_hw_close(hdev); 4175 hid_hw_stop(hdev); 4176 4177 if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) 4178 connect_mask &= ~HID_CONNECT_HIDINPUT; 4179 4180 /* Now export the actual inputs and hidraw nodes to the world */ 4181 ret = hid_hw_start(hdev, connect_mask); 4182 if (ret) { 4183 hid_err(hdev, "%s:hid_hw_start returned error\n", __func__); 4184 goto hid_hw_start_fail; 4185 } 4186 4187 if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) { 4188 ret = hidpp_ff_init(hidpp, &data); 4189 if (ret) 4190 hid_warn(hidpp->hid_dev, 4191 "Unable to initialize force feedback support, errno %d\n", 4192 ret); Should this do some cleanup if hidpp_ff_init() fails? 4193 } 4194 4195 return ret; 4196 4197 hid_hw_init_fail: 4198 hid_hw_close(hdev); 4199 hid_hw_open_fail: 4200 hid_hw_stop(hdev); 4201 hid_hw_start_fail: 4202 sysfs_remove_group(&hdev->dev.kobj, &ps_attribute_group); 4203 cancel_work_sync(&hidpp->work); 4204 mutex_destroy(&hidpp->send_mutex); --> 4205 return ret; 4206 } regards, dan carpenter