[bug report] usb: cdns3: Add Cadence USB3 DRD Driver

[bug report] usb: cdns3: Add Cadence USB3 DRD Driver

[Dan Carpenter]

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

RE: [bug report] usb: cdns3: Add Cadence USB3 DRD Driver

[Pawel Laszczak]

Hi Dan,

>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.

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.

>
>   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

Regards,
Pawell