usb: cdns3: Add Cadence USB3 DRD Driver
Hello Pawel Laszczak,
The patch 7733f6c32e36: "usb: cdns3: Add Cadence USB3 DRD Driver" from Aug 26, 2019, leads to the following static checker warning:
drivers/usb/cdns3/ep0.c:335 cdns3_ep0_feature_handle_device() error: scheduling with locks held: 'spin_lock:lock'
drivers/usb/cdns3/ep0.c
309
310 priv_dev->u2_allowed = !!set;
311 break;
312 case USB_DEVICE_LTM_ENABLE:
313 ret = -EINVAL;
314 break;
315 case USB_DEVICE_TEST_MODE:
316 if (state != USB_STATE_CONFIGURED || speed > USB_SPEED_HIGH)
317 return -EINVAL;
318
319 tmode = le16_to_cpu(ctrl->wIndex);
320
321 if (!set || (tmode & 0xff) != 0)
322 return -EINVAL;
323
324 switch (tmode >> 8) {
325 case TEST_J:
326 case TEST_K:
327 case TEST_SE0_NAK:
328 case TEST_PACKET:
329 cdns3_ep0_complete_setup(priv_dev, 0, 1);
330 /**
331 * Little delay to give the controller some time
332 * for sending status stage.
333 * This time should be less then 3ms.
334 */
335 usleep_range(1000, 2000);
^^^^^^^^^^^^^^^^^^^^^^^^This is called with locks held and IRQs disabled in cdns3_device_thread_irq_handler(). The call tree is:
cdns3_device_thread_irq_handler()
cdns3_check_ep0_interrupt_proceed()
cdns3_ep0_setup_phase()
cdns3_ep0_standard_request()
cdns3_req_ep0_handle_feature()
cdns3_ep0_feature_handle_device()You would need to have certain debug config options enabled to see the might_sleep() warnings and probably USB_DEVICE_TEST_MODE isn't used very often.
336 cdns3_set_register_bit(&priv_dev->regs->usb_cmd, 337 USB_CMD_STMODE | 338 USB_STS_TMODE_SEL(tmode - 1)); 339 break; 340 default: 341 ret = -EINVAL; 342 } 343 break; 344 default: 345 ret = -EINVAL; 346 }
regards,
dan carpenter
Hi Dan,
Quoted 43 lines
Hello Pawel Laszczak,
The patch
7733f6c32e36: "usb: cdns3: Add Cadence USB3 DRD Driver" from Aug 26, 2019, leads to the following static checker warning:drivers/usb/cdns3/ep0.c:335 cdns3_ep0_feature_handle_device() error: scheduling with locks held: 'spin_lock:lock'
drivers/usb/cdns3/ep0.c
309
310priv_dev->u2_allowed= !!set;
311 break;
312 caseUSB_DEVICE_LTM_ENABLE:
313 ret =-EINVAL;
314 break;
315 caseUSB_DEVICE_TEST_MODE:
316 if (state !=USB_STATE_CONFIGURED|| speed >USB_SPEED_HIGH)
317 return-EINVAL;
318
319 tmode =le16_to_cpu(ctrl->wIndex);
320
321 if (!set || (tmode &0xff) != 0)
322 return-EINVAL;
323324 switch (tmode >> 8) { 325 case TEST_J: 326 case TEST_K: 327 case TEST_SE0_NAK: 328 case TEST_PACKET: 329 cdns3_ep0_complete_setup(priv_dev, 0, 1); 330 /** 331 * Little delay to give the controller some time 332 * for sending status stage. 333 * This time should be less then 3ms. 334 */ 335 usleep_range(1000, 2000); ^^^^^^^^^^^^^^^^^^^^^^^^This is called with locks held and IRQs disabled in
cdns3_device_thread_irq_handler(). The call tree is:
cdns3_device_thread_irq_handler()
cdns3_check_ep0_interrupt_proceed()
cdns3_ep0_setup_phase()
Quoted 7 lines
cdns3_ep0_standard_request() cdns3_req_ep0_handle_feature() cdns3_ep0_feature_handle_device()You would need to have certain debug config options enabled to see the
might_sleep()warnings and probablyUSB_DEVICE_TEST_MODEisn't used very often.
It's very rarely used options. This part is required only during USB Certification process to pass some tests. After completion the device under test can be even rebooted.
I think that It should be replaced by udelay/mdelay function. I understand that the problem occurred because we can't use sleep functions when lock is held, or interrupt are disabled.
I will post the fix patch ASAP.
Thank you for letting me know.
Quoted 13 lines
336 cdns3_set_register_bit(&priv_dev->regs->usb_cmd, 337 USB_CMD_STMODE | 338 USB_STS_TMODE_SEL(tmode - 1)); 339 break; 340 default: 341 ret = -EINVAL; 342 } 343 break; 344 default: 345 ret = -EINVAL; 346 }
Quoted 2 lines
regards,
dan carpenter
Regards,
Pawell