Skip to contentSkip to conversation

usb: cdns3: Add Cadence USB3 DRD Driver

bug report Dan Carpenter 2 participants 2 messages Sep 24, 2019 – Sep 25, 2019 USB

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw

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

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

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