Re: [PATCH 2/4] usb: dwc3: gadget: check if dep->frame_number is still valid
Hi Felipe,
On 8/20/2018 3:34 AM, Felipe Balbi wrote:
> Gadget driver may take an unbounded amount of time to queue requests
> after XferNotReady. This is important for isochronous endpoints which
> need to be started for a specific (micro-)frame.
>
> Before kicking the transfer, let's check if current frame number is
> still less than our aligned frame number that we got from the
> previous XferNotReady. If it isn't, then we'll increment
> dep->frame_number to make sure it's ahead of current frame number.
>
> Signed-off-by: Felipe Balbi
> ---
> drivers/usb/dwc3/core.h | 2 ++
> drivers/usb/dwc3/gadget.c | 11 +++
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 285ce0ef3b91..3acf8788a680 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -685,6 +685,8 @@ struct dwc3_ep {
> u8 type;
> u8 resource_index;
> u32 frame_number;
> +#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff
> +
> u32 interval;
>
> charname[20];
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index f61a4250c883..0bac9b02f28b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1257,6 +1257,9 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
>
> static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
> {
> + u16 current_frame_number;
> + u16 frame_number;
> +
> if (list_empty(>pending_list)) {
> dev_info(dep->dwc->dev, "%s: ran out of requests\n",
> dep->name);
> @@ -1265,6 +1268,14 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep
> *dep)
> }
>
> dep->frame_number = DWC3_ALIGN_FRAME(dep);
> + current_frame_number = __dwc3_gadget_get_frame(dep->dwc);
> + frame_number = dep->frame_number & DWC3_EP_FRAME_NUMBER_MASK;
> +
> + if (frame_number <= current_frame_number) {
This is not right. You're comparing (what should be a 16-bit but masked)
frame_number to a 14-bit current_frame_number.
If the current frame number is 2+ seconds ahead of dep->frame_number,
then this calculation is wrong.
> + dep->frame_number += current_frame_number - frame_number;
> + dep->frame_number = DWC3_ALIGN_FRAME(dep);
> + }
> +
> return __dwc3_gadget_kick_transfer(dep);
> }
>
Thinh
Re: Question about a patch for Worlde controller
On Mon, Aug 20, 2018 at 12:03:47PM +, Maxence Duprès wrote: > Hello, > > I found nothing about this patch on git: > > Something gone wrong with it ? No, it got caught in my system and didn't get pushed out fully, my fault. I have it in my queue again to apply once 4.19-rc1 is out and get it into 4.19-final, so all is good, no need to worry. thanks, greg k-h
Re: [PATCH v3] USB: serial: ftdi_sio: Add support for CBUS GPIO
On Mon, Aug 20, 2018 at 05:19:40PM +0200, Loic Poulain wrote: > Hi Johan, > > On 4 August 2018 at 12:17, Loic Poulain wrote: > > Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS > > pins, allowing host to control them via simple USB control transfers. > > To make use of a CBUS pin in Bit Bang mode, the pin must be configured > > to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular > > USB to Serial function. > > > > In this implementation, a GPIO controller is registered on FTDI probe > > if at least one CBUS pin is configured for I/O mode. For now, only > > FTX devices are supported. > > > > This patch is based on previous Stefan Agner implementation tentative > > on LKML [1]. > > > > [1] Message-Id: 1434838377-8042-1-git-send-email-ste...@agner.ch > > > > Signed-off-by: Loic Poulain > > Any comments on this v3? I'm just back from vacation and haven't had time to look at this one yet at all. Sorry. Johan
Re: [PATCH v3] USB: serial: ftdi_sio: Add support for CBUS GPIO
Hi Johan, On 4 August 2018 at 12:17, Loic Poulain wrote: > Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS > pins, allowing host to control them via simple USB control transfers. > To make use of a CBUS pin in Bit Bang mode, the pin must be configured > to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular > USB to Serial function. > > In this implementation, a GPIO controller is registered on FTDI probe > if at least one CBUS pin is configured for I/O mode. For now, only > FTX devices are supported. > > This patch is based on previous Stefan Agner implementation tentative > on LKML [1]. > > [1] Message-Id: 1434838377-8042-1-git-send-email-ste...@agner.ch > > Signed-off-by: Loic Poulain Any comments on this v3? Regards, Loic
ASM3142 controller ERROR Transfer event TRB DMA ptr not part of current TD
Hi! I'm experiencing an issue with combination of ASMedia ASM3142 controller (http://www.asmedia.com.tw/eng/e_show_products.php?cate_index=175=179) and specific USB3 device type - industrial camera. After few seconds from the start of image acquisition USB transfer hangs (device reports that streaming endpoint has stalled) with the following messages repeated several times in kernel log: [ 137.337248] xhci_hcd :02:00.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 2 comp_code 1 [ 137.337259] xhci_hcd :02:00.0: Looking for event-dma 0004841d9000 trb-start 0004841dabf0 trb-end 0004841dafe0 seg-start 0004841da000 seg-end 0004841daff0 . [ 137.346576] xhci_hcd :02:00.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 2 comp_code 13 [ 137.346585] xhci_hcd :02:00.0: Looking for event-dma 0004841d9b60 trb-start 0004841dabf0 trb-end 0004841dafe0 seg-start 0004841da000 seg-end 0004841daff0 . Endpoint 2 (OUT) is used for control commands (with 0x82 IN endpoint for ACKs) and image data is streamed through IN endpoint 0x81. The fact that 2 bulk endpoints are used in parallel is most likely what triggers this bug. Other devices work fine with this controller, e.g. USB3 storage, but they don't have such USB endpoints configuration. Also the camera works fine with this controller on Windows, but only with drivers from ASMedia, not with Microsoft's. I've tried several Linux kernel versions including latest kernel.org release 4.18.3. Here is lspci -vvv -s :02:00.0 output: 02:00.0 USB controller: ASMedia Technology Inc. Device 2142 (prog-if 30 [XHCI]) Subsystem: ASMedia Technology Inc. Device 2142 Physical Slot: 6 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR-
Re: cdc-acm linux kernel
A: No. Q: Should I include quotations after my reply? On Mon, Aug 20, 2018 at 05:57:57PM +0300, Алексей Болдырев wrote: > > Ok, and what makes you believe this chip exposes serial ports? Why > > did you think CDC-ACM would work in the first place? > > This chipset avaible from mobile phome Mikromax x806. The phone is not > android. > https://market.yandex.ru/product--telefon-micromax-x806/13874918 Ok, but I have no idea what those USB interfaces are used for. They are clearly not CDC (e.g. exposing a modem). If you have some documentation suggesting they are used for a serial-type interface, you'd need to determine what protocol is used so we could possibly add an entry to an existing driver. Johan
Re: cdc-acm linux kernel
>Ok, and what makes you believe this chip exposes serial ports? Why did you think CDC-ACM would work in the first place? This chipset avaible from mobile phome Mikromax x806. The phone is not android. https://market.yandex.ru/product--telefon-micromax-x806/13874918 20.08.2018, 17:27, "Johan Hovold" : > A: Because it messes up the order in which people normally read text. > Q: Why is top-posting such a bad thing? > > http://daringfireball.net/2007/07/on_top > > On Mon, Aug 20, 2018 at 04:57:23PM +0300, Алексей Болдырев wrote: > >> 20.08.2018, 16:54, "Johan Hovold" : >> > [ Please respond inline instead of top-posting. ] >> > >> > On Mon, Aug 20, 2018 at 04:50:25PM +0300, Алексей Болдырев wrote: >> >> 20.08.2018, 16:46, "Johan Hovold" : >> >> > [ Reshuffling your reply, and responding inline below ] >> >> > >> >> > On Mon, Aug 20, 2018 at 04:27:37PM +0300, Алексей Болдырев wrote: >> >> >> > 20.08.2018, 11:42, "Johan Hovold" : >> >> >> >> [ Adding linux-usb on CC. ] >> >> >> >> >> >> >> >> On Fri, Aug 17, 2018 at 10:41:20PM +0300, Алексей Болдырев wrote: >> >> >> >>> please add support to device from cdc-acm: >> >> >> >>> >> >> >> >>> Bus 004 Device 003: ID 1782:3d00 Spreadtrum Communications Inc. >> >> >> >> >> >> >> >> Can you please post the output of "lsusb -v" for this device? >> >> > >> >> >> Bus 004 Device 004: ID 1782:3d00 Spreadtrum Communications Inc. >> > >> > [...] >> > >> >> > Thanks for the details. This isn't a CDC device, so this probably >> needs >> >> > to be handled by a USB serial driver. What kind of device is it? Does >> it >> >> > have more than one port? >> >> > >> >> > Judging from the above you should get two ttyUSBx devices if you do: >> >> > >> >> > # modprobe usbserial >> >> > # echo 1782 3d00 > /sys/bus/usb-serial/drivers/generic/new_id >> >> > >> >> > as root. >> >> > >> >> > Are those two ports usable? >> > >> >> I tried to do so, but for some reason I did not answer the serial >> >> port? >> > >> > Ok, so the generic driver does not work. What kind of device is this? A >> > USB-serial adapter? >> > >> > Do you have any way of figuring out what chip is used (e.g. an FTDI or >> > pl2303 chip?), for example, by opening the device? > >> The device is mobile phone used chipset qualcomm snapdragon. > > Ok, and what makes you believe this chip exposes serial ports? Why did > you think CDC-ACM would work in the first place? > > Perhaps what you're really after is something like adb? > > Johan
Re: cdc-acm linux kernel
A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? http://daringfireball.net/2007/07/on_top On Mon, Aug 20, 2018 at 04:57:23PM +0300, Алексей Болдырев wrote: > 20.08.2018, 16:54, "Johan Hovold" : > > [ Please respond inline instead of top-posting. ] > > > > On Mon, Aug 20, 2018 at 04:50:25PM +0300, Алексей Болдырев wrote: > >> 20.08.2018, 16:46, "Johan Hovold" : > >> > [ Reshuffling your reply, and responding inline below ] > >> > > >> > On Mon, Aug 20, 2018 at 04:27:37PM +0300, Алексей Болдырев wrote: > >> >> > 20.08.2018, 11:42, "Johan Hovold" : > >> >> >> [ Adding linux-usb on CC. ] > >> >> >> > >> >> >> On Fri, Aug 17, 2018 at 10:41:20PM +0300, Алексей Болдырев wrote: > >> >> >>> please add support to device from cdc-acm: > >> >> >>> > >> >> >>> Bus 004 Device 003: ID 1782:3d00 Spreadtrum Communications Inc. > >> >> >> > >> >> >> Can you please post the output of "lsusb -v" for this device? > >> > > >> >> Bus 004 Device 004: ID 1782:3d00 Spreadtrum Communications Inc. > > > > [...] > > > >> > Thanks for the details. This isn't a CDC device, so this probably needs > >> > to be handled by a USB serial driver. What kind of device is it? Does it > >> > have more than one port? > >> > > >> > Judging from the above you should get two ttyUSBx devices if you do: > >> > > >> > # modprobe usbserial > >> > # echo 1782 3d00 > /sys/bus/usb-serial/drivers/generic/new_id > >> > > >> > as root. > >> > > >> > Are those two ports usable? > > > >> I tried to do so, but for some reason I did not answer the serial > >> port? > > > > Ok, so the generic driver does not work. What kind of device is this? A > > USB-serial adapter? > > > > Do you have any way of figuring out what chip is used (e.g. an FTDI or > > pl2303 chip?), for example, by opening the device? > The device is mobile phone used chipset qualcomm snapdragon. Ok, and what makes you believe this chip exposes serial ports? Why did you think CDC-ACM would work in the first place? Perhaps what you're really after is something like adb? Johan
Re: usb: gadget: f_mass_storage: detect eject
On Sat, 18 Aug 2018, Nuno Gonçalves wrote: > Hi, > > I am trying to get some kind of notification when the host ejects a > f_mass_storage disk. > > When it happens, and the lun is removable, then the config > /sys/kernel/config/usb_gadget/*/functions/*/lun.*/file is cleared. > > I understand that sysfs files can not be monitored with inotify, but > some might be with select/poll/epoll. But this is a configfs. > > I know nothing works for me. I don't know if this is a bug or expected. It is expected. No notifications for host-initiated ejects were ever put into the f_mass_storage driver. I have never tried to use f_mass_storage under configfs. When you do, does the driver create its normal sysfs files? Alan Stern > Any insight? > > Thanks! > Nuno > >
Re: cdc-acm linux kernel
The device is mobile phone used chipset qualcomm snapdragon. 20.08.2018, 16:54, "Johan Hovold" : > [ Please respond inline instead of top-posting. ] > > On Mon, Aug 20, 2018 at 04:50:25PM +0300, Алексей Болдырев wrote: >> 20.08.2018, 16:46, "Johan Hovold" : >> > [ Reshuffling your reply, and responding inline below ] >> > >> > On Mon, Aug 20, 2018 at 04:27:37PM +0300, Алексей Болдырев wrote: >> >> > 20.08.2018, 11:42, "Johan Hovold" : >> >> >> [ Adding linux-usb on CC. ] >> >> >> >> >> >> On Fri, Aug 17, 2018 at 10:41:20PM +0300, Алексей Болдырев wrote: >> >> >>> please add support to device from cdc-acm: >> >> >>> >> >> >>> Bus 004 Device 003: ID 1782:3d00 Spreadtrum Communications Inc. >> >> >> >> >> >> Can you please post the output of "lsusb -v" for this device? >> > >> >> Bus 004 Device 004: ID 1782:3d00 Spreadtrum Communications Inc. > > [...] > >> > Thanks for the details. This isn't a CDC device, so this probably needs >> > to be handled by a USB serial driver. What kind of device is it? Does it >> > have more than one port? >> > >> > Judging from the above you should get two ttyUSBx devices if you do: >> > >> > # modprobe usbserial >> > # echo 1782 3d00 > /sys/bus/usb-serial/drivers/generic/new_id >> > >> > as root. >> > >> > Are those two ports usable? > >> I tried to do so, but for some reason I did not answer the serial >> port? > > Ok, so the generic driver does not work. What kind of device is this? A > USB-serial adapter? > > Do you have any way of figuring out what chip is used (e.g. an FTDI or > pl2303 chip?), for example, by opening the device? > > Thanks, > Johan
Re: cdc-acm linux kernel
[ Please respond inline instead of top-posting. ] On Mon, Aug 20, 2018 at 04:50:25PM +0300, Алексей Болдырев wrote: > 20.08.2018, 16:46, "Johan Hovold" : > > [ Reshuffling your reply, and responding inline below ] > > > > On Mon, Aug 20, 2018 at 04:27:37PM +0300, Алексей Болдырев wrote: > >> > 20.08.2018, 11:42, "Johan Hovold" : > >> >> [ Adding linux-usb on CC. ] > >> >> > >> >> On Fri, Aug 17, 2018 at 10:41:20PM +0300, Алексей Болдырев wrote: > >> >>> please add support to device from cdc-acm: > >> >>> > >> >>> Bus 004 Device 003: ID 1782:3d00 Spreadtrum Communications Inc. > >> >> > >> >> Can you please post the output of "lsusb -v" for this device? > > > >> Bus 004 Device 004: ID 1782:3d00 Spreadtrum Communications Inc. [...] > > Thanks for the details. This isn't a CDC device, so this probably needs > > to be handled by a USB serial driver. What kind of device is it? Does it > > have more than one port? > > > > Judging from the above you should get two ttyUSBx devices if you do: > > > > # modprobe usbserial > > # echo 1782 3d00 > /sys/bus/usb-serial/drivers/generic/new_id > > > > as root. > > > > Are those two ports usable? > I tried to do so, but for some reason I did not answer the serial > port? Ok, so the generic driver does not work. What kind of device is this? A USB-serial adapter? Do you have any way of figuring out what chip is used (e.g. an FTDI or pl2303 chip?), for example, by opening the device? Thanks, Johan
Re: cdc-acm linux kernel
I tried to do so, but for some reason I did not answer the serial port? 20.08.2018, 16:46, "Johan Hovold" : > [ Reshuffling your reply, and responding inline below ] > > On Mon, Aug 20, 2018 at 04:27:37PM +0300, Алексей Болдырев wrote: >> > 20.08.2018, 11:42, "Johan Hovold" : >> >> [ Adding linux-usb on CC. ] >> >> >> >> On Fri, Aug 17, 2018 at 10:41:20PM +0300, Алексей Болдырев wrote: >> >>> please add support to device from cdc-acm: >> >>> >> >>> Bus 004 Device 003: ID 1782:3d00 Spreadtrum Communications Inc. >> >> >> >> Can you please post the output of "lsusb -v" for this device? > >> Bus 004 Device 004: ID 1782:3d00 Spreadtrum Communications Inc. >> Couldn't open device, some information will be missing >> Device Descriptor: >> bLength 18 >> bDescriptorType 1 >> bcdUSB 1.10 >> bDeviceClass 0 (Defined at Interface level) >> bDeviceSubClass 0 >> bDeviceProtocol 0 >> bMaxPacketSize0 64 >> idVendor 0x1782 Spreadtrum Communications Inc. >> idProduct 0x3d00 >> bcdDevice 0.01 >> iManufacturer 0 >> iProduct 0 >> iSerial 0 >> bNumConfigurations 1 >> Configuration Descriptor: >> bLength 9 >> bDescriptorType 2 >> wTotalLength 55 >> bNumInterfaces 2 >> bConfigurationValue 1 >> iConfiguration 0 >> bmAttributes 0xc0 >> Self Powered >> MaxPower 100mA >> Interface Descriptor: >> bLength 9 >> bDescriptorType 4 >> bInterfaceNumber 0 >> bAlternateSetting 0 >> bNumEndpoints 2 >> bInterfaceClass 255 Vendor Specific Class >> bInterfaceSubClass 0 >> bInterfaceProtocol 0 >> iInterface 0 >> Endpoint Descriptor: >> bLength 7 >> bDescriptorType 5 >> bEndpointAddress 0x81 EP 1 IN >> bmAttributes 2 >> Transfer Type Bulk >> Synch Type None >> Usage Type Data >> wMaxPacketSize 0x0040 1x 64 bytes >> bInterval 0 >> Endpoint Descriptor: >> bLength 7 >> bDescriptorType 5 >> bEndpointAddress 0x02 EP 2 OUT >> bmAttributes 2 >> Transfer Type Bulk >> Synch Type None >> Usage Type Data >> wMaxPacketSize 0x0040 1x 64 bytes >> bInterval 0 >> Interface Descriptor: >> bLength 9 >> bDescriptorType 4 >> bInterfaceNumber 1 >> bAlternateSetting 0 >> bNumEndpoints 2 >> bInterfaceClass 255 Vendor Specific Class >> bInterfaceSubClass 0 >> bInterfaceProtocol 0 >> iInterface 0 >> Endpoint Descriptor: >> bLength 7 >> bDescriptorType 5 >> bEndpointAddress 0x83 EP 3 IN >> bmAttributes 2 >> Transfer Type Bulk >> Synch Type None >> Usage Type Data >> wMaxPacketSize 0x0040 1x 64 bytes >> bInterval 0 >> Endpoint Descriptor: >> bLength 7 >> bDescriptorType 5 >> bEndpointAddress 0x04 EP 4 OUT >> bmAttributes 2 >> Transfer Type Bulk >> Synch Type None >> Usage Type Data >> wMaxPacketSize 0x0040 1x 64 bytes >> bInterval 0 > > Thanks for the details. This isn't a CDC device, so this probably needs > to be handled by a USB serial driver. What kind of device is it? Does it > have more than one port? > > Judging from the above you should get two ttyUSBx devices if you do: > > # modprobe usbserial > # echo 1782 3d00 > /sys/bus/usb-serial/drivers/generic/new_id > > as root. > > Are those two ports usable? > > Thanks, > Johan
Re: cdc-acm linux kernel
[ Reshuffling your reply, and responding inline below ] On Mon, Aug 20, 2018 at 04:27:37PM +0300, Алексей Болдырев wrote: > > 20.08.2018, 11:42, "Johan Hovold" : > >> [ Adding linux-usb on CC. ] > >> > >> On Fri, Aug 17, 2018 at 10:41:20PM +0300, Алексей Болдырев wrote: > >>> please add support to device from cdc-acm: > >>> > >>> Bus 004 Device 003: ID 1782:3d00 Spreadtrum Communications Inc. > >> > >> Can you please post the output of "lsusb -v" for this device? > Bus 004 Device 004: ID 1782:3d00 Spreadtrum Communications Inc. > Couldn't open device, some information will be missing > Device Descriptor: > bLength18 > bDescriptorType 1 > bcdUSB 1.10 > bDeviceClass0 (Defined at Interface level) > bDeviceSubClass 0 > bDeviceProtocol 0 > bMaxPacketSize064 > idVendor 0x1782 Spreadtrum Communications Inc. > idProduct 0x3d00 > bcdDevice0.01 > iManufacturer 0 > iProduct0 > iSerial 0 > bNumConfigurations 1 > Configuration Descriptor: > bLength 9 > bDescriptorType 2 > wTotalLength 55 > bNumInterfaces 2 > bConfigurationValue 1 > iConfiguration 0 > bmAttributes 0xc0 > Self Powered > MaxPower 100mA > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber0 > bAlternateSetting 0 > bNumEndpoints 2 > bInterfaceClass 255 Vendor Specific Class > bInterfaceSubClass 0 > bInterfaceProtocol 0 > iInterface 0 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x81 EP 1 IN > bmAttributes2 > Transfer TypeBulk > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 0 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x02 EP 2 OUT > bmAttributes2 > Transfer TypeBulk > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 0 > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber1 > bAlternateSetting 0 > bNumEndpoints 2 > bInterfaceClass 255 Vendor Specific Class > bInterfaceSubClass 0 > bInterfaceProtocol 0 > iInterface 0 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x83 EP 3 IN > bmAttributes2 > Transfer TypeBulk > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 0 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x04 EP 4 OUT > bmAttributes2 > Transfer TypeBulk > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 0 Thanks for the details. This isn't a CDC device, so this probably needs to be handled by a USB serial driver. What kind of device is it? Does it have more than one port? Judging from the above you should get two ttyUSBx devices if you do: # modprobe usbserial # echo 1782 3d00 > /sys/bus/usb-serial/drivers/generic/new_id as root. Are those two ports usable? Thanks, Johan
Re: cdc-acm linux kernel
Bus 004 Device 004: ID 1782:3d00 Spreadtrum Communications Inc. Couldn't open device, some information will be missing Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 1.10 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x1782 Spreadtrum Communications Inc. idProduct 0x3d00 bcdDevice0.01 iManufacturer 0 iProduct0 iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 55 bNumInterfaces 2 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower 100mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 0 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x02 EP 2 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber1 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 0 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x04 EP 4 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 0 > > 20.08.2018, 11:42, "Johan Hovold" : >> [ Adding linux-usb on CC. ] >> >> On Fri, Aug 17, 2018 at 10:41:20PM +0300, Алексей Болдырев wrote: >>> please add support to device from cdc-acm: >>> >>> Bus 004 Device 003: ID 1782:3d00 Spreadtrum Communications Inc. >> >> Can you please post the output of "lsusb -v" for this device? >> >> Thanks, >> Johan
Re: Possible race condition in f_mass_storage gadget during deinitialization. Kernel warning issued
In my current workspace the kernel used is 4.17.7 . Unfortunately I don't have the resources now to verify with newer version but I might look into that later if it's necessary. I've only compared source code between my worktree and mainline sources and verified that code around this area still looks almost exactly the same (with minor changes like renames of API calls here and there). Looking forward for Minas opinion. pon., 20 sie 2018 o 13:20 Felipe Balbi napisał(a): > > > Hi, > > Adrian Ambrożewicz writes: > > Hello, > > > > I'm consistently observing kernel warnings related to Mass Storage USB > > Gadget de-initialization flow. After investigation I believe that I've > > found root cause of these warnings, however I'm unable to estimate the > > impact. I would like to put the issue into discussion about possible > > side-effects. > > > > My usage model is to emulate file system using f_mass_storage gadget. > > Whenever I pull the USB cable out I see warning related to > > "dwc2_hsotg_init_fifo" which is commented in the following way: "/* > > Reset fifo map if not correctly cleared during previous session */". > > which kernel are you using? Have you tried latest mainline? (currently > at v4.18). > > > Assumption I've made were the following: > > 1) disconnect handler notifies all gadgets with call_gadget(.., disconnect) > > 2) gadgets are then responsible to clear the ep fifos with usb_ep_disable() > > 3) disconnect handler initializes the fifo maps to clear state > > > > Unfortunately in my case the warning occurs every time when using > > f_mass_storage gadget. By comparison with f_hid gadget I've come up > > with conclusion that it's caused by race condition in the way the > > "disable" flow is implemented in mass storage. > > > > "Correct" flow implemented by HID gadget is the following: > > * dwc2_hsotg_irq (USBRst) > > ** dwc2_hsotg_disconnect > > *** call_gadget(disconnect) > > hidg_disable > > * usb_ep_disable // fifos utilized by device are cleared in fifo_map > > ** dwc_hsotg_core_init_disconnected > > *** dwc2_hsotg_init_fifo // fifo_map is empty - no warning here > > > > By comparison here is flow observed in Mass Storage gadget. Race > > condition is introduced by utilizing separate worker thread to handle > > events: > > THREAD #1 > > * dwc2_hsotg_irq (USBRst) > > ** dwc2_hsotg_disconnect > > *** call_gadget(disconnect) > > fsg_disable() > > * raise_exception(FSG_STATE_CONFIG_CHANGE) // Race between Thread > > #1 and Thread #2 starts here. > > ** dwc_hsotg_core_init_disconnected > > *** dwc2_hsotg_init_fifo // fifo_map is object of the race between > > this function, and call to usb_ep_disable() > > > > THREAD #2 > > * handle_exception(FSG_STATE_CONFIG_CHANGE) > > ** do_set_interface(NULL) > > *** usb_ep_disable() > > > > My questions are the following: > > - is this known issue? > > - is it risky? What are possible outcomes? > > - If this race condition is dangerous what would be proper fix ? > > Should fsg_disable() call be blocked until handle_exception() finishes > > the cleanup? I know that it's just a WA for "misaligned" Mass Storage > > gadget architecture, but are there alternatives? > > > > I would really appreciate professional insight on that matter, > > Let's see if Minas has anything to say about this. > > -- > balbi
Re: [PATCH 3/4] usb: dwc3: trace: log ep commands in hex
Hi Andy,
On Monday, 20 August 2018 15:06:31 EEST Andy Shevchenko wrote:
> On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart wrote:
> >> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s",
> >> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s",
> >
> > How about 0x%x ?
>
> Side note: # is one character less for the same.
Doesn't that print 0 instead of 0x0 ? There's no ambiguity with 0, but I find
that always printing the 0x is more consistent. I'll leave that up to Felipe,
I'm OK with both options.
--
Regards,
Laurent Pinchart
Re: [PATCH 3/4] usb: dwc3: trace: log ep commands in hex
On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart
wrote:
>> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s",
>> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s",
>
> How about 0x%x ?
Side note: # is one character less for the same.
--
With Best Regards,
Andy Shevchenko
Question about a patch for Worlde controller
Hello, I found nothing about this patch on git: Something gone wrong with it ? This is a note to let you know that I've just added the patch titled USB: add quirk for WORLDE Controller KS49 or Prodipe MIDI 49C USB to my usb git tree which can be found at git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git in the usb-testing branch. The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.) The patch will be merged to the usb-next branch sometime soon, after it passes testing, and the merge window is open. If you have any questions about this process, please let me know.
Re: [PATCH 2/4] usb: dwc3: gadget: check if dep->frame_number is still valid
Hi Felipe,
(CC'ing Kieran)
Thank you for the patch.
On Monday, 20 August 2018 13:29:59 EEST Felipe Balbi wrote:
> Gadget driver may take an unbounded amount of time to queue requests
> after XferNotReady. This is important for isochronous endpoints which
> need to be started for a specific (micro-)frame.
>
> Before kicking the transfer, let's check if current frame number is
> still less than our aligned frame number that we got from the
> previous XferNotReady. If it isn't, then we'll increment
> dep->frame_number to make sure it's ahead of current frame number.
>
> Signed-off-by: Felipe Balbi
> ---
> drivers/usb/dwc3/core.h | 2 ++
> drivers/usb/dwc3/gadget.c | 11 +++
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 285ce0ef3b91..3acf8788a680 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -685,6 +685,8 @@ struct dwc3_ep {
> u8 type;
> u8 resource_index;
> u32 frame_number;
> +#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff
> +
> u32 interval;
>
> charname[20];
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index f61a4250c883..0bac9b02f28b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1257,6 +1257,9 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
>
> static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
> {
> + u16 current_frame_number;
> + u16 frame_number;
> +
> if (list_empty(>pending_list)) {
> dev_info(dep->dwc->dev, "%s: ran out of requests\n",
> dep->name);
> @@ -1265,6 +1268,14 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep
> *dep) }
>
> dep->frame_number = DWC3_ALIGN_FRAME(dep);
> + current_frame_number = __dwc3_gadget_get_frame(dep->dwc);
> + frame_number = dep->frame_number & DWC3_EP_FRAME_NUMBER_MASK;
> +
> + if (frame_number <= current_frame_number) {
> + dep->frame_number += current_frame_number - frame_number;
> + dep->frame_number = DWC3_ALIGN_FRAME(dep);
> + }
> +
This greatly improves the situation with the UVC function driver. However,
we're still receiving errors from time to time related to missed transfer
intervals, and those errors tend to occur in bursts. Kieran should be able to
provide you with more information. Before acking this patch I would thus like
to make sure that we're not missing part of the problem.
> return __dwc3_gadget_kick_transfer(dep);
> }
--
Regards,
Laurent Pinchart
Re: [PATCH 4/4] usb: dwc3: gadget: remove unnecessary dev_info()
Hi Felipe,
Thank you for the patch.
On Monday, 20 August 2018 13:30:01 EEST Felipe Balbi wrote:
> Running out of requests on isochronous endpoints is part of normal
> operation. We don't really need to know about it every time it
> happens.
>
> Signed-off-by: Felipe Balbi
Reviewed-by: Laurent Pinchart
> ---
> drivers/usb/dwc3/gadget.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 0bac9b02f28b..8d3e0f2cde8b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1261,8 +1261,6 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep
> *dep) u16 frame_number;
>
> if (list_empty(>pending_list)) {
> - dev_info(dep->dwc->dev, "%s: ran out of requests\n",
> - dep->name);
> dep->flags |= DWC3_EP_PENDING_REQUEST;
> return -EAGAIN;
> }
--
Regards,
Laurent Pinchart
Re: [PATCH 3/4] usb: dwc3: trace: log ep commands in hex
Hi Felipe,
Thank you for the patch.
On Monday, 20 August 2018 13:30:00 EEST Felipe Balbi wrote:
> They are much more useful in hexadecimal than in decimal. Moreover,
> generic commands are already logged in hex.
>
> Signed-off-by: Felipe Balbi
> ---
> drivers/usb/dwc3/trace.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
> index f22714cce070..50fb6f2d92dd 100644
> --- a/drivers/usb/dwc3/trace.h
> +++ b/drivers/usb/dwc3/trace.h
> @@ -199,7 +199,7 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd,
> __entry->param2 = params->param2;
> __entry->cmd_status = cmd_status;
> ),
> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s",
> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s",
How about 0x%x ? Otherwise one could get confused when the command
representation in hex doesn't contain a letter (especially given that this
patch transitions from decimal to hexadecimal).
> __get_str(name), dwc3_gadget_ep_cmd_string(__entry->cmd),
> __entry->cmd, __entry->param0,
> __entry->param1, __entry->param2,
--
Regards,
Laurent Pinchart
Re: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n
On Mon, Aug 20, 2018 at 5:22 AM Yoshihiro Shimoda wrote: > > From: Arnd Bergmann, Sent: Tuesday, August 14, 2018 11:27 PM > > > > On Tue, Aug 14, 2018 at 3:58 PM Alan Stern > > wrote: > > > On Mon, 13 Aug 2018, Arnd Bergmann wrote: > > > > diff --git a/drivers/usb/gadget/udc/Kconfig > > > > b/drivers/usb/gadget/udc/Kconfig > > > > index 0a16cbd4e528..663a8bd67a7b 100644 > > > > --- a/drivers/usb/gadget/udc/Kconfig > > > > +++ b/drivers/usb/gadget/udc/Kconfig > > > > @@ -193,6 +193,7 @@ config USB_RENESAS_USB3 > > > > tristate 'Renesas USB3.0 Peripheral controller' > > > > depends on ARCH_RENESAS || COMPILE_TEST > > > > depends on EXTCON > > > > + depends on USB || !USB > > > > > > Is this some weird standard idiom? It looks really strange. > > > > Yes, and yes. > > > > A less common way to write it is > > > > depends on (USB != m) || m > > > > which some people prefer, but I find even weirder. > > Thank you for the patch! > > On other thread, John mentions gadget-side drivers should not depend on > host-side [1]. > So, I submitted a patch today [2]. What do you think about my submitted patch? > > [1] > https://marc.info/?l=linux-usb=153433776202861=2 I don't really think this is a big issue, as you can still build the driver with CONFIG_USB=n, so it's not a strict dependency. > [2] > https://patchwork.kernel.org/patch/10569847/ On the other hand, your patch looks nice and is somewhat less confusing, so you have my Ack on that. Arnd
Re: [PATCH 1/4] usb: dwc3: gadget: return errors from __dwc3_gadget_start_isoc()
Hi Felipe,
Thank you for the patch.
On Monday, 20 August 2018 13:29:58 EEST Felipe Balbi wrote:
> Sometimes, errors happen when kicking transfers from
> __dwc3_gadget_start_isoc(). In those cases, we need to pass along the
> error so gadget driver can make informed decisions.
>
> Signed-off-by: Felipe Balbi
> ---
> drivers/usb/dwc3/gadget.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 69bf137aab37..f61a4250c883 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1255,17 +1255,17 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
> return DWC3_DSTS_SOFFN(reg);
> }
>
> -static void __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
> +static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
> {
> if (list_empty(>pending_list)) {
> dev_info(dep->dwc->dev, "%s: ran out of requests\n",
> dep->name);
> dep->flags |= DWC3_EP_PENDING_REQUEST;
> - return;
> + return -EAGAIN;
> }
>
> dep->frame_number = DWC3_ALIGN_FRAME(dep);
> - __dwc3_gadget_kick_transfer(dep);
> + return __dwc3_gadget_kick_transfer(dep);
> }
>
> static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request
> *req) @@ -1306,8 +1306,7 @@ static int __dwc3_gadget_ep_queue(struct
> dwc3_ep *dep, struct dwc3_request *req)
>
> if ((dep->flags & DWC3_EP_PENDING_REQUEST)) {
> if (!(dep->flags & DWC3_EP_TRANSFER_STARTED)) {
> - __dwc3_gadget_start_isoc(dep);
> - return 0;
> + return __dwc3_gadget_start_isoc(dep);
Do I understandand correctly that the new -EAGAIN error returned above when an
underrun has occurred will never be returned here, as we call
__dwc3_gadget_start_isoc() after adding a request to the pending_list, covered
by the same spinlock that is taken by the IRQ handler to remove requests from
the list ? I don't think that's an issue, but I want to make sure nothing is
overlooked.
> }
> }
> }
> @@ -2427,7 +2426,7 @@ static void
> dwc3_gadget_endpoint_transfer_not_ready(struct dwc3_ep *dep, const struct
> dwc3_event_depevt *event)
> {
> dwc3_gadget_endpoint_frame_from_event(dep, event);
> - __dwc3_gadget_start_isoc(dep);
> + (void) __dwc3_gadget_start_isoc(dep);
No need for a cast here.
> }
>
> static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
--
Regards,
Laurent Pinchart
Re: Possible race condition in f_mass_storage gadget during deinitialization. Kernel warning issued
Hi, Adrian Ambrożewicz writes: > Hello, > > I'm consistently observing kernel warnings related to Mass Storage USB > Gadget de-initialization flow. After investigation I believe that I've > found root cause of these warnings, however I'm unable to estimate the > impact. I would like to put the issue into discussion about possible > side-effects. > > My usage model is to emulate file system using f_mass_storage gadget. > Whenever I pull the USB cable out I see warning related to > "dwc2_hsotg_init_fifo" which is commented in the following way: "/* > Reset fifo map if not correctly cleared during previous session */". which kernel are you using? Have you tried latest mainline? (currently at v4.18). > Assumption I've made were the following: > 1) disconnect handler notifies all gadgets with call_gadget(.., disconnect) > 2) gadgets are then responsible to clear the ep fifos with usb_ep_disable() > 3) disconnect handler initializes the fifo maps to clear state > > Unfortunately in my case the warning occurs every time when using > f_mass_storage gadget. By comparison with f_hid gadget I've come up > with conclusion that it's caused by race condition in the way the > "disable" flow is implemented in mass storage. > > "Correct" flow implemented by HID gadget is the following: > * dwc2_hsotg_irq (USBRst) > ** dwc2_hsotg_disconnect > *** call_gadget(disconnect) > hidg_disable > * usb_ep_disable // fifos utilized by device are cleared in fifo_map > ** dwc_hsotg_core_init_disconnected > *** dwc2_hsotg_init_fifo // fifo_map is empty - no warning here > > By comparison here is flow observed in Mass Storage gadget. Race > condition is introduced by utilizing separate worker thread to handle > events: > THREAD #1 > * dwc2_hsotg_irq (USBRst) > ** dwc2_hsotg_disconnect > *** call_gadget(disconnect) > fsg_disable() > * raise_exception(FSG_STATE_CONFIG_CHANGE) // Race between Thread > #1 and Thread #2 starts here. > ** dwc_hsotg_core_init_disconnected > *** dwc2_hsotg_init_fifo // fifo_map is object of the race between > this function, and call to usb_ep_disable() > > THREAD #2 > * handle_exception(FSG_STATE_CONFIG_CHANGE) > ** do_set_interface(NULL) > *** usb_ep_disable() > > My questions are the following: > - is this known issue? > - is it risky? What are possible outcomes? > - If this race condition is dangerous what would be proper fix ? > Should fsg_disable() call be blocked until handle_exception() finishes > the cleanup? I know that it's just a WA for "misaligned" Mass Storage > gadget architecture, but are there alternatives? > > I would really appreciate professional insight on that matter, Let's see if Minas has anything to say about this. -- balbi
Possible race condition in f_mass_storage gadget during deinitialization. Kernel warning issued
Hello, I'm consistently observing kernel warnings related to Mass Storage USB Gadget de-initialization flow. After investigation I believe that I've found root cause of these warnings, however I'm unable to estimate the impact. I would like to put the issue into discussion about possible side-effects. My usage model is to emulate file system using f_mass_storage gadget. Whenever I pull the USB cable out I see warning related to "dwc2_hsotg_init_fifo" which is commented in the following way: "/* Reset fifo map if not correctly cleared during previous session */". Assumption I've made were the following: 1) disconnect handler notifies all gadgets with call_gadget(.., disconnect) 2) gadgets are then responsible to clear the ep fifos with usb_ep_disable() 3) disconnect handler initializes the fifo maps to clear state Unfortunately in my case the warning occurs every time when using f_mass_storage gadget. By comparison with f_hid gadget I've come up with conclusion that it's caused by race condition in the way the "disable" flow is implemented in mass storage. "Correct" flow implemented by HID gadget is the following: * dwc2_hsotg_irq (USBRst) ** dwc2_hsotg_disconnect *** call_gadget(disconnect) hidg_disable * usb_ep_disable // fifos utilized by device are cleared in fifo_map ** dwc_hsotg_core_init_disconnected *** dwc2_hsotg_init_fifo // fifo_map is empty - no warning here By comparison here is flow observed in Mass Storage gadget. Race condition is introduced by utilizing separate worker thread to handle events: THREAD #1 * dwc2_hsotg_irq (USBRst) ** dwc2_hsotg_disconnect *** call_gadget(disconnect) fsg_disable() * raise_exception(FSG_STATE_CONFIG_CHANGE) // Race between Thread #1 and Thread #2 starts here. ** dwc_hsotg_core_init_disconnected *** dwc2_hsotg_init_fifo // fifo_map is object of the race between this function, and call to usb_ep_disable() THREAD #2 * handle_exception(FSG_STATE_CONFIG_CHANGE) ** do_set_interface(NULL) *** usb_ep_disable() My questions are the following: - is this known issue? - is it risky? What are possible outcomes? - If this race condition is dangerous what would be proper fix ? Should fsg_disable() call be blocked until handle_exception() finishes the cleanup? I know that it's just a WA for "misaligned" Mass Storage gadget architecture, but are there alternatives? I would really appreciate professional insight on that matter, Regards, Adrian
[PATCH 1/4] usb: dwc3: gadget: return errors from __dwc3_gadget_start_isoc()
Sometimes, errors happen when kicking transfers from
__dwc3_gadget_start_isoc(). In those cases, we need to pass along the
error so gadget driver can make informed decisions.
Signed-off-by: Felipe Balbi
---
drivers/usb/dwc3/gadget.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 69bf137aab37..f61a4250c883 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1255,17 +1255,17 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
return DWC3_DSTS_SOFFN(reg);
}
-static void __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
+static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
{
if (list_empty(>pending_list)) {
dev_info(dep->dwc->dev, "%s: ran out of requests\n",
dep->name);
dep->flags |= DWC3_EP_PENDING_REQUEST;
- return;
+ return -EAGAIN;
}
dep->frame_number = DWC3_ALIGN_FRAME(dep);
- __dwc3_gadget_kick_transfer(dep);
+ return __dwc3_gadget_kick_transfer(dep);
}
static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request
*req)
@@ -1306,8 +1306,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep,
struct dwc3_request *req)
if ((dep->flags & DWC3_EP_PENDING_REQUEST)) {
if (!(dep->flags & DWC3_EP_TRANSFER_STARTED)) {
- __dwc3_gadget_start_isoc(dep);
- return 0;
+ return __dwc3_gadget_start_isoc(dep);
}
}
}
@@ -2427,7 +2426,7 @@ static void
dwc3_gadget_endpoint_transfer_not_ready(struct dwc3_ep *dep,
const struct dwc3_event_depevt *event)
{
dwc3_gadget_endpoint_frame_from_event(dep, event);
- __dwc3_gadget_start_isoc(dep);
+ (void) __dwc3_gadget_start_isoc(dep);
}
static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
--
2.18.0
[PATCH 4/4] usb: dwc3: gadget: remove unnecessary dev_info()
Running out of requests on isochronous endpoints is part of normal
operation. We don't really need to know about it every time it
happens.
Signed-off-by: Felipe Balbi
---
drivers/usb/dwc3/gadget.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0bac9b02f28b..8d3e0f2cde8b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1261,8 +1261,6 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
u16 frame_number;
if (list_empty(>pending_list)) {
- dev_info(dep->dwc->dev, "%s: ran out of requests\n",
- dep->name);
dep->flags |= DWC3_EP_PENDING_REQUEST;
return -EAGAIN;
}
--
2.18.0
[PATCH 3/4] usb: dwc3: trace: log ep commands in hex
They are much more useful in hexadecimal than in decimal. Moreover,
generic commands are already logged in hex.
Signed-off-by: Felipe Balbi
---
drivers/usb/dwc3/trace.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
index f22714cce070..50fb6f2d92dd 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -199,7 +199,7 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd,
__entry->param2 = params->param2;
__entry->cmd_status = cmd_status;
),
- TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s",
+ TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s",
__get_str(name), dwc3_gadget_ep_cmd_string(__entry->cmd),
__entry->cmd, __entry->param0,
__entry->param1, __entry->param2,
--
2.18.0
[PATCH 2/4] usb: dwc3: gadget: check if dep->frame_number is still valid
Gadget driver may take an unbounded amount of time to queue requests
after XferNotReady. This is important for isochronous endpoints which
need to be started for a specific (micro-)frame.
Before kicking the transfer, let's check if current frame number is
still less than our aligned frame number that we got from the
previous XferNotReady. If it isn't, then we'll increment
dep->frame_number to make sure it's ahead of current frame number.
Signed-off-by: Felipe Balbi
---
drivers/usb/dwc3/core.h | 2 ++
drivers/usb/dwc3/gadget.c | 11 +++
2 files changed, 13 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 285ce0ef3b91..3acf8788a680 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -685,6 +685,8 @@ struct dwc3_ep {
u8 type;
u8 resource_index;
u32 frame_number;
+#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff
+
u32 interval;
charname[20];
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index f61a4250c883..0bac9b02f28b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1257,6 +1257,9 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
{
+ u16 current_frame_number;
+ u16 frame_number;
+
if (list_empty(>pending_list)) {
dev_info(dep->dwc->dev, "%s: ran out of requests\n",
dep->name);
@@ -1265,6 +1268,14 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
}
dep->frame_number = DWC3_ALIGN_FRAME(dep);
+ current_frame_number = __dwc3_gadget_get_frame(dep->dwc);
+ frame_number = dep->frame_number & DWC3_EP_FRAME_NUMBER_MASK;
+
+ if (frame_number <= current_frame_number) {
+ dep->frame_number += current_frame_number - frame_number;
+ dep->frame_number = DWC3_ALIGN_FRAME(dep);
+ }
+
return __dwc3_gadget_kick_transfer(dep);
}
--
2.18.0
Re: cdc-acm linux kernel
[ Adding linux-usb on CC. ] On Fri, Aug 17, 2018 at 10:41:20PM +0300, Алексей Болдырев wrote: > please add support to device from cdc-acm: > > Bus 004 Device 003: ID 1782:3d00 Spreadtrum Communications Inc. Can you please post the output of "lsusb -v" for this device? Thanks, Johan
