Re: g_dbgp + u_serial: WARNING when unloading g_dbgp module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes:
>> Randy Dunlap <rdunlap@xxxxxxxxxxxxx> writes:
>> > Linux v5.0-11053-gebc551f2b8f9 of March 12, on x86_64:
>> >
>> > with
>> > CONFIG_USB_G_DBGP=m
>> > # CONFIG_USB_G_DBGP_PRINTK is not set
>> > CONFIG_USB_G_DBGP_SERIAL=y
>> > CONFIG_USB_U_SERIAL=m
>> >
>> > Loading g_dbgp module and then unloading it causes:
>> >
>> > WARNING: CPU: 0 PID: 24523 at ../drivers/usb/gadget/function/u_serial.c:1195 gserial_free_line+0x161/0x2b0 [u_serial]
>> >
>> >
>> > Is that expected?
>> 
>> Not really, but I tried reproducing this here and couldn't get the same
>> behavior. In fact, I can't see how that driver can work without a
>> configuration descriptor. Here are all descriptor defined by the driver:
>> 
>>      36:static struct usb_device_descriptor device_desc = {
>>      46:static struct usb_debug_descriptor dbg_desc = {
>>      51:static struct usb_endpoint_descriptor i_desc = {
>>      58:static struct usb_endpoint_descriptor o_desc = {
>> 
>> And here's an excerpt of the setup function:
>> 
>> static int dbgp_setup(struct usb_gadget *gadget,
>> 		      const struct usb_ctrlrequest *ctrl)
>> {
>> 	struct usb_request *req = dbgp.req;
>> 	u8 request = ctrl->bRequest;
>> 	u16 value = le16_to_cpu(ctrl->wValue);
>> 	u16 length = le16_to_cpu(ctrl->wLength);
>> 	int err = -EOPNOTSUPP;
>> 	void *data = NULL;
>> 	u16 len = 0;
>> 
>> 	if (request == USB_REQ_GET_DESCRIPTOR) {
>> 		switch (value>>8) {
>> 		case USB_DT_DEVICE:
>> 			dev_dbg(&dbgp.gadget->dev, "setup: desc device\n");
>> 			len = sizeof device_desc;
>> 			data = &device_desc;
>> 			device_desc.bMaxPacketSize0 = gadget->ep0->maxpacket;
>> 			break;
>> 		case USB_DT_DEBUG:
>> 			dev_dbg(&dbgp.gadget->dev, "setup: desc debug\n");
>> 			len = sizeof dbg_desc;
>> 			data = &dbg_desc;
>> 			break;
>> 		default:
>> 			goto fail;
>> 		}
>> 		err = 0;
>> 	} else if (request == USB_REQ_SET_FEATURE &&
>> 		   value == USB_DEVICE_DEBUG_MODE) {
>> 		dev_dbg(&dbgp.gadget->dev, "setup: feat debug\n");
>> #ifdef CONFIG_USB_G_DBGP_PRINTK
>> 		err = dbgp_enable_ep();
>> #else
>> 		err = dbgp_configure_endpoints(gadget);
>> 		if (err < 0) {
>> 			goto fail;
>> 		}
>> 		err = gserial_connect(dbgp.serial, tty_line);
>> #endif
>> 		if (err < 0)
>> 			goto fail;
>> 	} else
>> 		goto fail;
>> 
>> It always stalls GetDescriptor(Configuration) requests. It's also
>> missing interface descriptors. What gives? How can that work?
>
> Good questions.  Has this driver ever worked?
>
>> Anybody has a specification for how this class is supposed to work? I
>> suppose this is EHCI-specific?
>
> The file I've got is named debugdev-r090.pdf, dated 2003, and I think
> it came from the usb.org web site although I can't find it there now.  
> Perhaps it has been updated to USB-3.1 or removed entirely.  If you 
> would like, I can send you my copy.

Please do, if anything at least for future reference.

> The spec says:
>
> 	All Debug Devices, with the exception of fixed address devices, 
> 	must implement all required standard commands in the core 
> 	device framework.
>
> But that refers to standard _commands_, not standard _descriptors_.  

GetDescriptor() is a standard command, though. I would say that we're
either missing proper Interface and Configuration descriptors, or
usbcore used to have some special cases for ehci debug port which got
"fixed" out of existence over time.

> The spec also says that a debug device cannot be enumerated by standard
> USB-2 means, because it supports an ep0 maxpacket size of only 8 rather
> than 64.
>
> And yes, the driver is EHCI-specific -- it says so right at the start
> of the file.  On the other hand, I don't see any reason in theory why
> it couldn't work with any other host controller that has USB high-speed
> debug support.

Thanks for confirming.

-- 
balbi

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux