Re: usb: usbtest: TEST 13: set/clear 1 halts is getting failed with super speed
Thanks, Alan Stern for a quick reply. Can you please suggest a probable area to look out for the failure of the endpoint halt test case for the superspeed devices. On Thu, Jul 5, 2018 at 8:31 PM Alan Stern wrote: > > On Thu, 5 Jul 2018, Pradeep Das wrote: > > > Hi, > > > > Posting this query again as I received some mail delivery failure > > notification on the previous attempt. > > > > I am running usbtest test cases and facing following issues: > > > > Setup: > > I am using two custom board running on Linux. One is configured as > > host and loaded with usbtest.ko module. Another one is configured as a > > device with gadget zero drivers. Host and device are interfaced in > > super speed mode. testusb application is used on host side test > > features. Both boards uses synopsis USB controller. > > > > Observation: > > With super speed interface, all test cases are passing except test 13 > > ( set/clear 1 halts). The error is shown in this test case > > usb 2-1: verify_still_halted failed, iterations left 0, status -71 > > (not -32) usbtest > > 2-1:3.0: halts failed, iterations left 999 > > > > Analysis: > > Digging deeper into the issue I have below observation: > > --- Same test case with the same setup is passing for the high-speed > > interface > > --- I tried the same test case only on out endpoint. Seeing the same > > issue. The test will try to send 1024 byte of data (bulk out) packet. > > This will lead two trb, one for 1024 byte data followed by zero > > packets. For first data packet, I am getting EPIPE(-32) error which is > > expected. Same also expected for zero data packet. But for this trb, > > we are getting EPROT (-71) which is failing test case. > > > > Can we use usbtest for super speed interface? > > Yes. It should work. If it doesn't, something is wrong. > > > What is expected error code if we will try to send a data packet on a > > halted pipe? I checked spec but didn't get any specific mention. > > The expected error code is -32 (-EPIPE). It is documented in > Documentation/driver-api/usb/error-codes.rst. > > Alan Stern > > > Please help regarding the issue. Any input will be highly appreciated. > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] USB: serial: cp210x: clean up line-speed handling
Hi, > Karoly, how did your line-speed tests with cp2102n go? I indeed tested this. I first built a version of the module where I skip calling cp210x_quantise_baudrate(). Then I attached a scope to the TX line of my UART adapter and looked at various non-standard values in both low (<10k) and high (>2M) ranges. In all cases it looks like you can set any custom value to the cp2102n, assuming you use the right program to set the baudrate (most I've tried either do not allow non-standard rates or give an IOCTL error). So basically I can confirm that the chip does not snap to values in table 1 of AN205. I was able to set very odd rates, and measurements with the scope verified that they were actually applied correctly by the cp2102n (sometimes ofc with a few percent error here and there). > ... I think we should > just handle the higher cp2102n rates as we do with cp2104/8, that is by > mapping the lower rates to the rates supported by legacy devices, while > allowing any higher rates (up to the device maximum) to be requested > without trying to report back the actual rate chosen (for now). Based on the test above, my proposal for the cp2102n only is to not do any kind of software snapping / quantisation in the driver module, except for capping out at the maximum of 3Mbaud. Otherwise we'd be limiting the capabilities of the chip in the software artificially. As for reporting the actual baudrate back to userspace, the formula is documented clearly by SiLabs, so if that's possible somehow, I'm in favor of it. The question is, how? Karoly -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb: usbtest: TEST 13: set/clear 1 halts is getting failed with super speed
On Thu, 5 Jul 2018, Pradeep Das wrote: > Hi, > > Posting this query again as I received some mail delivery failure > notification on the previous attempt. > > I am running usbtest test cases and facing following issues: > > Setup: > I am using two custom board running on Linux. One is configured as > host and loaded with usbtest.ko module. Another one is configured as a > device with gadget zero drivers. Host and device are interfaced in > super speed mode. testusb application is used on host side test > features. Both boards uses synopsis USB controller. > > Observation: > With super speed interface, all test cases are passing except test 13 > ( set/clear 1 halts). The error is shown in this test case > usb 2-1: verify_still_halted failed, iterations left 0, status -71 > (not -32) usbtest > 2-1:3.0: halts failed, iterations left 999 > > Analysis: > Digging deeper into the issue I have below observation: > --- Same test case with the same setup is passing for the high-speed interface > --- I tried the same test case only on out endpoint. Seeing the same > issue. The test will try to send 1024 byte of data (bulk out) packet. > This will lead two trb, one for 1024 byte data followed by zero > packets. For first data packet, I am getting EPIPE(-32) error which is > expected. Same also expected for zero data packet. But for this trb, > we are getting EPROT (-71) which is failing test case. > > Can we use usbtest for super speed interface? Yes. It should work. If it doesn't, something is wrong. > What is expected error code if we will try to send a data packet on a > halted pipe? I checked spec but didn't get any specific mention. The expected error code is -32 (-EPIPE). It is documented in Documentation/driver-api/usb/error-codes.rst. Alan Stern > Please help regarding the issue. Any input will be highly appreciated. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] USB: serial: cp210x: honour device-type maximum line speed
Newer cp210x devices support higher line speeds than the older ones
which supported a discrete set of speeds up to 921.6 kbaud.
To support these higher speeds, we have for some time mapped speeds
lower than 1 Mbaud to the speeds supported by older devices, while
allowing the device to pick the closest possible rate for higher speeds
(without trying to guess and report back what rate was actually chosen).
As this implementation can lead to undefined behaviour for older devices
which do not support the higher rates, let's use the later-added
device-type detection to determine the maximum supported speed.
This will also be useful when adding support for cp2102n which can
handle rates up to 3 Mbaud.
As per the data sheets the following maximum speeds are used
cp2101 921.6 kbaud
cp2102/31 Mbaud
cp2104/82 Mbaud
cp2105
- ECI port 2 Mbaud
- SCI port 921.6 kbaud
while keeping the maximum 2 Mbaud for unknown device types in order to
avoid any regressions.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/cp210x.c | 41 ++---
1 file changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index e1a4d4b3d3aa..7772c4ce45a6 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -244,6 +244,7 @@ struct cp210x_serial_private {
boolgpio_registered;
#endif
u8 partnum;
+ speed_t max_speed;
};
struct cp210x_port_private {
@@ -1067,19 +1068,20 @@ static speed_t cp210x_get_an205_rate(speed_t baud)
static void cp210x_change_speed(struct tty_struct *tty,
struct usb_serial_port *port, struct ktermios *old_termios)
{
+ struct cp210x_serial_private *priv = usb_get_serial_data(port->serial);
u32 baud;
baud = tty->termios.c_ospeed;
/* This maps the requested rate to a rate valid on cp2102 or cp2103,
-* or to an arbitrary rate in [1M,2M].
+* or to an arbitrary rate in [1M, max_speed]
*
* NOTE: B0 is not implemented.
*/
if (baud < 100)
baud = cp210x_get_an205_rate(baud);
- else if (baud > 200)
- baud = 200;
+ else if (baud > priv->max_speed)
+ baud = priv->max_speed;
dev_dbg(&port->dev, "%s - setting baud rate to %u\n", __func__, baud);
if (cp210x_write_u32_reg(port, CP210X_SET_BAUDRATE, baud)) {
@@ -1510,6 +1512,37 @@ static int cp210x_port_remove(struct usb_serial_port
*port)
return 0;
}
+static void cp210x_init_max_speed(struct usb_serial *serial)
+{
+ struct cp210x_serial_private *priv = usb_get_serial_data(serial);
+ speed_t max;
+
+ switch (priv->partnum) {
+ case CP210X_PARTNUM_CP2101:
+ max = 921600;
+ break;
+ case CP210X_PARTNUM_CP2102:
+ case CP210X_PARTNUM_CP2103:
+ max = 100;
+ break;
+ case CP210X_PARTNUM_CP2104:
+ case CP210X_PARTNUM_CP2108:
+ max = 200;
+ break;
+ case CP210X_PARTNUM_CP2105:
+ if (cp210x_interface_num(serial) == 0)
+ max = 200; /* ECI */
+ else
+ max = 921600; /* SCI */
+ break;
+ default:
+ max = 200;
+ break;
+ }
+
+ priv->max_speed = max;
+}
+
static int cp210x_attach(struct usb_serial *serial)
{
int result;
@@ -1530,6 +1563,8 @@ static int cp210x_attach(struct usb_serial *serial)
usb_set_serial_data(serial, priv);
+ cp210x_init_max_speed(serial);
+
if (priv->partnum == CP210X_PARTNUM_CP2105) {
result = cp2105_shared_gpio_init(serial);
if (result < 0) {
--
2.18.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] USB: serial: cp210x: clean up line-speed handling
This cleans up the current line-speed handling somewhat, while avoiding requesting an unsupported line speed (which can lead to undefined behaviour) by determining the maximum speed supported by the device type in question. Karoly, how did your line-speed tests with cp2102n go? I think we should just handle the higher cp2102n rates as we do with cp2104/8, that is by mapping the lower rates to the rates supported by legacy devices, while allowing any higher rates (up to the device maximum) to be requested without trying to report back the actual rate chosen (for now). Johan Johan Hovold (2): USB: serial: cp210x: make line-speed quantisation data driven USB: serial: cp210x: honour device-type maximum line speed drivers/usb/serial/cp210x.c | 136 1 file changed, 92 insertions(+), 44 deletions(-) -- 2.18.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] USB: serial: cp210x: make line-speed quantisation data driven
Older cp210x devices only support a fixed set of line speeds to which a
requested speed is mapped. Reimplement this mapping using a table
instead of a long if-else construct.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/cp210x.c | 99 +
1 file changed, 56 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 626a29d9aa58..e1a4d4b3d3aa 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -767,48 +767,6 @@ static int cp210x_get_line_ctl(struct usb_serial_port
*port, u16 *ctl)
return 0;
}
-/*
- * cp210x_quantise_baudrate
- * Quantises the baud rate as per AN205 Table 1
- */
-static unsigned int cp210x_quantise_baudrate(unsigned int baud)
-{
- if (baud <= 300)
- baud = 300;
- else if (baud <= 600) baud = 600;
- else if (baud <= 1200) baud = 1200;
- else if (baud <= 1800) baud = 1800;
- else if (baud <= 2400) baud = 2400;
- else if (baud <= 4000) baud = 4000;
- else if (baud <= 4803) baud = 4800;
- else if (baud <= 7207) baud = 7200;
- else if (baud <= 9612) baud = 9600;
- else if (baud <= 14428)baud = 14400;
- else if (baud <= 16062)baud = 16000;
- else if (baud <= 19250)baud = 19200;
- else if (baud <= 28912)baud = 28800;
- else if (baud <= 38601)baud = 38400;
- else if (baud <= 51558)baud = 51200;
- else if (baud <= 56280)baud = 56000;
- else if (baud <= 58053)baud = 57600;
- else if (baud <= 64111)baud = 64000;
- else if (baud <= 77608)baud = 76800;
- else if (baud <= 117028) baud = 115200;
- else if (baud <= 129347) baud = 128000;
- else if (baud <= 156868) baud = 153600;
- else if (baud <= 237832) baud = 230400;
- else if (baud <= 254234) baud = 25;
- else if (baud <= 273066) baud = 256000;
- else if (baud <= 491520) baud = 460800;
- else if (baud <= 567138) baud = 50;
- else if (baud <= 670254) baud = 576000;
- else if (baud < 100)
- baud = 921600;
- else if (baud > 200)
- baud = 200;
- return baud;
-}
-
static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *port)
{
int result;
@@ -1028,6 +986,58 @@ static void cp210x_get_termios_port(struct
usb_serial_port *port,
*cflagp = cflag;
}
+struct cp210x_rate {
+ speed_t rate;
+ speed_t high;
+};
+
+static const struct cp210x_rate cp210x_an205_table1[] = {
+ { 300, 300 },
+ { 600, 600 },
+ { 1200, 1200 },
+ { 1800, 1800 },
+ { 2400, 2400 },
+ { 4000, 4000 },
+ { 4800, 4803 },
+ { 7200, 7207 },
+ { 9600, 9612 },
+ { 14400, 14428 },
+ { 16000, 16062 },
+ { 19200, 19250 },
+ { 28800, 28912 },
+ { 38400, 38601 },
+ { 51200, 51558 },
+ { 56000, 56280 },
+ { 57600, 58053 },
+ { 64000, 64111 },
+ { 76800, 77608 },
+ { 115200, 117028 },
+ { 128000, 129347 },
+ { 153600, 156868 },
+ { 230400, 237832 },
+ { 25, 254234 },
+ { 256000, 273066 },
+ { 460800, 491520 },
+ { 50, 567138 },
+ { 576000, 670254 },
+ { 921600, UINT_MAX }
+};
+
+/*
+ * Quantises the baud rate as per AN205 Table 1
+ */
+static speed_t cp210x_get_an205_rate(speed_t baud)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(cp210x_an205_table1); ++i) {
+ if (baud <= cp210x_an205_table1[i].high)
+ break;
+ }
+
+ return cp210x_an205_table1[i].rate;
+}
+
/*
* CP2101 supports the following baud rates:
*
@@ -1066,7 +1076,10 @@ static void cp210x_change_speed(struct tty_struct *tty,
*
* NOTE: B0 is not implemented.
*/
- baud = cp210x_quantise_baudrate(baud);
+ if (baud < 100)
+ baud = cp210x_get_an205_rate(baud);
+ else if (baud > 200)
+ baud = 200;
dev_dbg(&port->dev, "%s - setting baud rate to %u\n", __func__, baud);
if (cp210x_write_u32_reg(port, CP210X_SET_BAUDRATE, baud)) {
--
2.18.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] usb: dwc2: DMA alignment fixes
Here are two patches that improve DMA alignment handling of the dwc2
driver significantly.
The first one ("usb: dwc2: Fix DMA alignment to start at allocated
boundary") fixes an actual crash regression on some platforms introduced
by commit 3bc04e28a030 ("usb: dwc2: host: Get aligned DMA in a more
supported way") and should be considered for inclusion in stable kernel
release.
The second patch ("usb: dwc2: Fix inefficient copy of unaligned
buffers") is a port of an optimization found from similar drivers to
dwc2. In my tests it offered a significant performance improvement in
certain use cases. Since it touches the same area that I was already
modifying I decided to split it into separate patch and include in the
series.
Antti Seppälä (2):
usb: dwc2: Fix DMA alignment to start at allocated boundary
usb: dwc2: Fix inefficient copy of unaligned buffers
drivers/usb/dwc2/hcd.c | 54 +-
1 file changed, 31 insertions(+), 23 deletions(-)
--
2.13.6
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: dwc2: Fix DMA alignment to start at allocated boundary
The commit 3bc04e28a030 ("usb: dwc2: host: Get aligned DMA in a more
supported way") introduced a common way to align DMA allocations.
The code in the commit aligns the struct dma_aligned_buffer but the
actual DMA address pointed by data[0] gets aligned to an offset from
the allocated boundary by the kmalloc_ptr and the old_xfer_buffer
pointers.
This is against the recommendation in Documentation/DMA-API.txt which
states:
Therefore, it is recommended that driver writers who don't take
special care to determine the cache line size at run time only map
virtual regions that begin and end on page boundaries (which are
guaranteed also to be cache line boundaries).
The effect of this is that architectures with non-coherent DMA caches
may run into memory corruption or kernel crashes with Unhandled
kernel unaligned accesses exceptions.
Fix the alignment by positioning the DMA area in front of the allocation
and use memory at the end of the area for storing the orginal
transfer_buffer pointer. This may have the added benefit of increased
performance as the DMA area is now fully aligned on all architectures.
Tested with Lantiq xRX200 (MIPS) and RPi Model B Rev 2 (ARM).
Fixes: 3bc04e28a030 ("usb: dwc2: host: Get aligned DMA in a more
supported way")
Signed-off-by: Antti Seppälä
---
drivers/usb/dwc2/hcd.c | 44 +++-
1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index b1104be3429c..2ed0ac18e053 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2665,34 +2665,29 @@ static int dwc2_alloc_split_dma_aligned_buf(struct
dwc2_hsotg *hsotg,
#define DWC2_USB_DMA_ALIGN 4
-struct dma_aligned_buffer {
- void *kmalloc_ptr;
- void *old_xfer_buffer;
- u8 data[0];
-};
-
static void dwc2_free_dma_aligned_buffer(struct urb *urb)
{
- struct dma_aligned_buffer *temp;
+ void *stored_xfer_buffer;
if (!(urb->transfer_flags & URB_ALIGNED_TEMP_BUFFER))
return;
- temp = container_of(urb->transfer_buffer,
- struct dma_aligned_buffer, data);
+ /* Restore urb->transfer_buffer from the end of the allocated area */
+ memcpy(&stored_xfer_buffer, urb->transfer_buffer +
+ urb->transfer_buffer_length, sizeof(urb->transfer_buffer));
if (usb_urb_dir_in(urb))
- memcpy(temp->old_xfer_buffer, temp->data,
+ memcpy(stored_xfer_buffer, urb->transfer_buffer,
urb->transfer_buffer_length);
- urb->transfer_buffer = temp->old_xfer_buffer;
- kfree(temp->kmalloc_ptr);
+ kfree(urb->transfer_buffer);
+ urb->transfer_buffer = stored_xfer_buffer;
urb->transfer_flags &= ~URB_ALIGNED_TEMP_BUFFER;
}
static int dwc2_alloc_dma_aligned_buffer(struct urb *urb, gfp_t mem_flags)
{
- struct dma_aligned_buffer *temp, *kmalloc_ptr;
+ void *kmalloc_ptr;
size_t kmalloc_size;
if (urb->num_sgs || urb->sg ||
@@ -2700,22 +2695,29 @@ static int dwc2_alloc_dma_aligned_buffer(struct urb
*urb, gfp_t mem_flags)
!((uintptr_t)urb->transfer_buffer & (DWC2_USB_DMA_ALIGN - 1)))
return 0;
- /* Allocate a buffer with enough padding for alignment */
+ /*
+* Allocate a buffer with enough padding for original transfer_buffer
+* pointer. This allocation is guaranteed to be aligned properly for
+* DMA
+*/
kmalloc_size = urb->transfer_buffer_length +
- sizeof(struct dma_aligned_buffer) + DWC2_USB_DMA_ALIGN - 1;
+ sizeof(urb->transfer_buffer);
kmalloc_ptr = kmalloc(kmalloc_size, mem_flags);
if (!kmalloc_ptr)
return -ENOMEM;
- /* Position our struct dma_aligned_buffer such that data is aligned */
- temp = PTR_ALIGN(kmalloc_ptr + 1, DWC2_USB_DMA_ALIGN) - 1;
- temp->kmalloc_ptr = kmalloc_ptr;
- temp->old_xfer_buffer = urb->transfer_buffer;
+ /*
+* Position value of original urb->transfer_buffer pointer to the end
+* of allocation for later referencing
+*/
+ memcpy(kmalloc_ptr + urb->transfer_buffer_length,
+ &urb->transfer_buffer, sizeof(urb->transfer_buffer));
+
if (usb_urb_dir_out(urb))
- memcpy(temp->data, urb->transfer_buffer,
+ memcpy(kmalloc_ptr, urb->transfer_buffer,
urb->transfer_buffer_length);
- urb->transfer_buffer = temp->data;
+ urb->transfer_buffer = kmalloc_ptr;
urb->transfer_flags |= URB_ALIGNED_TEMP_BUFFER;
--
2.13.6
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: dwc2: Fix inefficient copy of unaligned buffers
Make sure only to copy any actual data rather than the whole buffer,
when releasing the temporary buffer used for unaligned non-isochronous
transfers.
Taken directly from commit 0efd937e27d5e ("USB: ehci-tegra: fix inefficient
copy of unaligned buffers")
Tested with Lantiq xRX200 (MIPS) and RPi Model B Rev 2 (ARM)
Signed-off-by: Antti Seppälä
---
drivers/usb/dwc2/hcd.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 2ed0ac18e053..6e2cdd7b93d4 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2668,6 +2668,7 @@ static int dwc2_alloc_split_dma_aligned_buf(struct
dwc2_hsotg *hsotg,
static void dwc2_free_dma_aligned_buffer(struct urb *urb)
{
void *stored_xfer_buffer;
+ size_t length;
if (!(urb->transfer_flags & URB_ALIGNED_TEMP_BUFFER))
return;
@@ -2676,9 +2677,14 @@ static void dwc2_free_dma_aligned_buffer(struct urb *urb)
memcpy(&stored_xfer_buffer, urb->transfer_buffer +
urb->transfer_buffer_length, sizeof(urb->transfer_buffer));
- if (usb_urb_dir_in(urb))
- memcpy(stored_xfer_buffer, urb->transfer_buffer,
- urb->transfer_buffer_length);
+ if (usb_urb_dir_in(urb)) {
+ if (usb_pipeisoc(urb->pipe))
+ length = urb->transfer_buffer_length;
+ else
+ length = urb->actual_length;
+
+ memcpy(stored_xfer_buffer, urb->transfer_buffer, length);
+ }
kfree(urb->transfer_buffer);
urb->transfer_buffer = stored_xfer_buffer;
--
2.13.6
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Zero Packets in Isochronous Transfer Reception
On Wed, 4 Jul 2018, R0b0t1 wrote: > On Sun, Jul 1, 2018 at 9:12 AM, Alan Stern wrote: > > On Sat, 30 Jun 2018, R0b0t1 wrote: > > > >> The problem seems more noticeable when using the Python libusb > >> bindings but it still exists when using libusb directly. Can anyone > >> suggest what to look into? > > > > Have you tried using usbmon to capture the data as it is received by > > the kernel? > > > > Thank you for the suggestion. Having looked at usbmon output it > appears the kernel is receiving all zero packets for some transfers. > It also looks like some packets are dropped, but I am not sure how to > tell. Are blank transfers dropped data also? It would help if you posted a few examples. Isochronous packets are different from other kinds; to interpret them properly you need to look at the frame descriptors. > If I try to reconstitute the audio stream by ignoring all zero packets > the stream still experiences pops and clicks though they are much > quieter. That is the best indication I have that packets are being > dropped. The discontinuities in the waves lead to the pops and clicks. There are a few reasons why isochronous packets may get dropped. Possibly the host driver did not ask for them in time. And possibly the peripheral wasn't able to send them in time. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 2/2] usb: gadget: storage: Remove reference counting
On Wed, 4 Jul 2018, Jaejoong Kim wrote:
> The kref used to be needed because sharing of fsg_common among multiple USB
> function instances was handled by fsg. Now this is managed by configfs, we
> don't need it anymore. So let's eliminate kref from this driver.
>
> Signed-off-by: Jaejoong Kim
Acked-by: Alan Stern
> ---
> Changes in V2:
> - Rewrite commit title & message.
> - Remove kref instead of removing unused kref EXPORT_SYMBOL (Alan and
> Michal)
>
> V1 patches
> https://patchwork.kernel.org/patch/10463709/
> https://patchwork.kernel.org/patch/10463713/
>
> drivers/usb/gadget/function/f_mass_storage.c | 27 +--
> drivers/usb/gadget/function/f_mass_storage.h | 4
> 2 files changed, 5 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c
> b/drivers/usb/gadget/function/f_mass_storage.c
> index 1b580eb..ca8a4b5 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -206,7 +206,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -312,8 +311,6 @@ struct fsg_common {
> void*private_data;
>
> char inquiry_string[INQUIRY_STRING_LEN];
> -
> - struct kref ref;
> };
>
> struct fsg_dev {
> @@ -2551,25 +2548,11 @@ static DEVICE_ATTR(file, 0, file_show, file_store);
>
> /** FSG COMMON **/
>
> -static void fsg_common_release(struct kref *ref);
> -
> static void fsg_lun_release(struct device *dev)
> {
> /* Nothing needs to be done */
> }
>
> -void fsg_common_get(struct fsg_common *common)
> -{
> - kref_get(&common->ref);
> -}
> -EXPORT_SYMBOL_GPL(fsg_common_get);
> -
> -void fsg_common_put(struct fsg_common *common)
> -{
> - kref_put(&common->ref, fsg_common_release);
> -}
> -EXPORT_SYMBOL_GPL(fsg_common_put);
> -
> static struct fsg_common *fsg_common_setup(struct fsg_common *common)
> {
> if (!common) {
> @@ -2582,7 +2565,6 @@ static struct fsg_common *fsg_common_setup(struct
> fsg_common *common)
> }
> init_rwsem(&common->filesem);
> spin_lock_init(&common->lock);
> - kref_init(&common->ref);
> init_completion(&common->thread_notifier);
> init_waitqueue_head(&common->io_wait);
> init_waitqueue_head(&common->fsg_wait);
> @@ -2870,9 +2852,8 @@ void fsg_common_set_inquiry_string(struct fsg_common
> *common, const char *vn,
> }
> EXPORT_SYMBOL_GPL(fsg_common_set_inquiry_string);
>
> -static void fsg_common_release(struct kref *ref)
> +static void fsg_common_release(struct fsg_common *common)
> {
> - struct fsg_common *common = container_of(ref, struct fsg_common, ref);
> int i;
>
> /* If the thread isn't already dead, tell it to exit now */
> @@ -3346,7 +3327,7 @@ static void fsg_free_inst(struct usb_function_instance
> *fi)
> struct fsg_opts *opts;
>
> opts = fsg_opts_from_func_inst(fi);
> - fsg_common_put(opts->common);
> + fsg_common_release(opts->common);
> kfree(opts);
> }
>
> @@ -3370,7 +3351,7 @@ static struct usb_function_instance
> *fsg_alloc_inst(void)
> rc = fsg_common_set_num_buffers(opts->common,
> CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS);
> if (rc)
> - goto release_opts;
> + goto release_common;
>
> pr_info(FSG_DRIVER_DESC ", version: " FSG_DRIVER_VERSION "\n");
>
> @@ -3393,6 +3374,8 @@ static struct usb_function_instance
> *fsg_alloc_inst(void)
>
> release_buffers:
> fsg_common_free_buffers(opts->common);
> +release_common:
> + kfree(opts->common);
> release_opts:
> kfree(opts);
> return ERR_PTR(rc);
> diff --git a/drivers/usb/gadget/function/f_mass_storage.h
> b/drivers/usb/gadget/function/f_mass_storage.h
> index 58857fc..3b8c4ce 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.h
> +++ b/drivers/usb/gadget/function/f_mass_storage.h
> @@ -115,10 +115,6 @@ fsg_opts_from_func_inst(const struct
> usb_function_instance *fi)
> return container_of(fi, struct fsg_opts, func_inst);
> }
>
> -void fsg_common_get(struct fsg_common *common);
> -
> -void fsg_common_put(struct fsg_common *common);
> -
> void fsg_common_set_sysfs(struct fsg_common *common, bool sysfs);
>
> int fsg_common_set_num_buffers(struct fsg_common *common, unsigned int n);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cp20x GPIO from user application
[ Adding linux-usb as other may be interested in this. ] On Thu, Jul 05, 2018 at 01:57:29PM +0100, Antonio Santagiuliana wrote: > Thank you for the information. > What I don't understand is that if I use the CP2105 are its GPIOs usable by > this driver ? Yes, they should be. But if I remember correctly, the pins in question are muxed with different functions, so you need to make sure that the device is configured correctly (e.g. using some silabs tool). > If so, which is the interface to the user space ? I can open the driver > usbserial but from the Raspberry pi user space I don't understand where the > GPIOs from this chip are mapped to or how I should map them by creating a > node with mknod ? You can access them from /sys/class/gpio or using the new gpiolib chardev interface as any other gpios. If the gpios are recognised and registered, they should also show up in sysfs as a child device to the USB interface in question, for example as: /sys/bus/usb/devices/1-1.1\:1.0/gpio/gpiochipN Good luck, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next] usb: typec: altmodes: Fix missing unlock on error in dp_altmode_activate()
On Wed, Jul 04, 2018 at 09:25:43AM +, Wei Yongjun wrote:
> Add the missing unlock before return from function
> dp_altmode_activate() in the error handling case.
>
> Fixes: 0e3bb7d6894d ("usb: typec: Add driver for DisplayPort alternate mode")
> Signed-off-by: Wei Yongjun
> ---
> drivers/usb/typec/altmodes/displayport.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/altmodes/displayport.c
> b/drivers/usb/typec/altmodes/displayport.c
> index ef12b15..83e3a38 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -349,8 +349,10 @@ static int dp_altmode_activate(struct typec_altmode
> *alt, int activate)
> cap = DP_CAP_CAPABILITY(dp->alt->vdo);
>
> if ((con == DP_CONF_DFP_D && !(cap & DP_CAP_DFP_D)) ||
> - (con == DP_CONF_UFP_D && !(cap & DP_CAP_UFP_D)))
> - return -EINVAL;
> + (con == DP_CONF_UFP_D && !(cap & DP_CAP_UFP_D))) {
> + ret = -EINVAL;
> + goto err_unlock;
> + }
>
> conf = dp->data.conf & ~DP_CONF_DUAL_D;
> conf |= con;
Thank you for the patch. Colin King found the same issue, and his
patch is also fixing some other issue with this function, so let's use
that instead.
Br,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB Type-C hub detection is flaky
Hi, On Fri, Jun 29, 2018 at 10:26:46PM +0200, Timur Krist?f wrote: > Hi Heikki, > > On Fri, 2018-06-29 at 14:42 +0300, Heikki Krogerus wrote: > > Hi Tim, > > > > On Tue, Jun 26, 2018 at 02:10:57PM +0200, Timur Krist?f wrote: > > > > Can you send the dmesg output after you have plugged the powered > > > > and > > > > non-powered hubs: > > > > > > Right now I have the Dell Type-C to USB 3.0 Type-A adapter with the > > > USB > > > 2.0 hub here. There is a Logitech received plugged into the hub. > > > (So I > > > can quickly see if it works or not.) > > > > > > After some trial-and-error I figured out when it works and when > > > not. > > > Tests were performed on Fedora 28 with kernel 4.17.2. > > > > > > Basically it always works just on the first try. In other words, > > > when I > > > power up the USB 2.0 hub and plug it in, then it works. If I plug > > > it > > > out, but keep it powered and plug it back in again, it stops > > > working. > > > > I wonder if the PD controller or EC firmware is seeing the > > disconnection. Perhaps we can test it. > > How does the PD controller come into the picture? > (Assuming you mean PD = Power Delivery?) PD controllers handle CC logic as well. It means that they are the components that detect is there something connected to the port or not. With USB Type-C connectors, detecting connection happens using Configuration Channel (CC) which is a line going through the USB Type-C cable. You always have a specific chip on your platform for handling the CC line. It may be a component called TCPC (Type-C Port Controller), or it may be PD controller. The USB Power Delivery protocol is also used for other things besides just for negotiating the power levels. The alternate modes for example use the Vendor Defined Messages (VDM) for communication which are special messages defined in USB Power Deliver Specification. > The device I was talking about does not (and should not) deliver any > power to the laptop. Again this is a powered USB 2.0 hub with the Dell > Type-C to Type-A adapter, so it does not support PD. > > (Next week I will buy a Type-C hub with PD support so will be able to > test with that too.) > > > Can you check if have the USB Type-C ports and partners in sysfs > > folder /sys/class/typec? > > > > % ls /sys/class/typec/ > > port0 port0-partner port1 > > On kernel 4.17.2 (as packaged by Fedora 28), that directory is empty, > even though I have something plugged in to all ports. > > > It may be that you need to cherry-pick a few patches from Greg's > > tree [1][2] that fix an issue we had with the UCSI device on those > > Dell laptops. Can you build your own kernel? > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/co > > mmit/?h=usb-linus&id=d2d2e3c46be5d6dd8001d0eebdf7cafb9bc7006b > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/co > > mmit/?h=usb-linus&id=1f9f9d168ce619608572b01771c47a41b15429e6 > > How I usually build a kernel is I just use Fedora's config and tools, > ie. I patch the latest Fedora kernel and build it using 'fedpkg' > locally. Hope this approach is OK. > > Now I got the latest from the f28 branch (which is 4.17.3 with some > patches). I added the two patches from above. > > With the patched kernel, the directory is non-empty but it does not > correctly react to hotplugging and unplugging. > > > But if you can see the ports and partners under that sysfs folder, > > after you unplug the device, does the port-partner disappears? > > When I have something in every port on bootup I see this: > > $ ls /sys/class/typec/ > port0 port0-partner port1 port1-partner port2 port2-partner > > But when I boot the machine with nothing plugged in, and then plug the > things in later: > > - Type-C adapter with the USB 2.0 hub does not get a "partner" when it > is hotplugged, only when it is plugged in at boot time. The partner > correctly disappears when I plug it out but never appears again. Oh, that most likely means we are not getting any notifications (interrupts) from the EC firmware. Either the EC firmware is not generating them, or something goes wrong. I think there are now two problems we are seeing. The UCSI interface does not seem to be behaving properly on this laptop :-(. If you unload and then reload ucsi_acpi module you probable see the port-partner getting registered. Can you check if there is any trace output coming from ucsi driver when you connect a device to the port: % mount none -t debugfs /sys/kernel/debug % echo 1 > /sys/kernel/debug/tracing/events/ucsi/enable plug something to the port % cat /sys/kernel/debug/tracing/trace > - Type-C charger appears as a "partner" when hot-plugged, but the > partner does not disappear even if I plug it out. Despite this, > charging continues to work when I plug it back in to any port. Though > no other partner gets created when I plug it into a different port. OK, so if the part
dwc3 usb2 as host not working
Hi I have started playing around with a board based on the DragonBoard 820c and I cannot manage to get the USB2 working as host. If I connect a mouse, before I start the board I can see how the system powers up the port (vbus) and enumerates the device, just to kill it a couple of seconds later. If I try to connect anything to the port after the board has been started, I get absolutely no message on dmesg about the event, also no irqs on the hcd, only on the gpios related to the cable detection. Any ideas of what can be wrong/how to proceed? Thanks [6.153511] dwc3 read:0xc144 0x120c93b [6.157267] dwc3 read:0xc148 0x27012412 [6.161088] dwc3 read:0xc14c 0x4108084 [6.164818] dwc3 read:0xc150 0x47822004 [6.168640] dwc3 read:0xc154 0x4202088 [6.172369] dwc3 read:0xc158 0x7870020 [6.176204] dwc3 read:0xc15c 0x0 [6.179923] dwc3 read:0xc600 0x47a [6.183463] dwc3 read:0xc120 0x5533270a [6.186476] dwc3 write:0xc128 0x40e0f [6.190446] phy phy-7412000.phy.4: qusb2_phy_init(): Initializing QUSB2 phy [6.194349] qusb2: setbits: offset 0xb4 val 0x23 [6.200865] qusb2: conf: offset 0x80 val 0xf8 [6.205697] qusb2: conf: offset 0x84 val 0xb3 [6.209948] qusb2: conf: offset 0x88 val 0x83 [6.214287] qusb2: conf: offset 0x8c val 0xc0 [6.218628] qusb2: conf: offset 0x8 val 0x30 [6.222968] qusb2: conf: offset 0xc val 0x79 [6.227308] qusb2: conf: offset 0x10 val 0x21 [6.231562] qusb2: conf: offset 0x9c val 0x14 [6.235815] qusb2: conf: offset 0x1c val 0x9f [6.240155] qusb2: conf: offset 0x18 val 0x0 [6.244504] qusb2: setbits: offset 0x84 val 0xb0 [6.248840] qusb2: clrbits: offset 0xb4 val 0x1 [6.253613] qusb2: pll_test: offset 0x4 val 0x80 [6.257792] dwc3 read:0xc2c0 0x10c0002 [6.262519] dwc3 write:0xc2c0 0x10e0002 [6.266076] dwc3 read:0xc200 0x2500 [6.269811] Disabling power management [6.273280] dwc3 write:0xc200 0x2400 [6.277099] dwc3 read:0xc110 0x102000 [6.280833] dwc3 write:0xc110 0x102000 [6.284396] phy phy-7412000.phy.4: qusb2_phy_poweron(): Powering-on QUSB2 phy [6.288078] regulator: /usr/src/kernel/drivers/regulator/qcom_smd-regulator.c:60 id 24 type 1634690156 [6.295531] dwc3 write:0xc400 0xff05b000 [6.304448] dwc3 write:0xc404 0x0 [6.308526] dwc3 write:0xc408 0x1000 [6.311735] dwc3 write:0xc40c 0x0 [6.315382] dwc3 read:0xc11c 0xc0002 [6.318584] dwc3 write:0xc11c 0xc0002 [6.322241] dwc3 read:0xc110 0x102000 [6.325797] dwc3 write:0xc110 0x101000 [6.329990] xhci-hcd xhci-hcd.0.auto: xHCI Host Controller [6.333134] xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 1 [6.338585] xhci-hcd xhci-hcd.0.auto: xHCI capability registers at 0d408000: [6.346119] xhci-hcd xhci-hcd.0.auto: CAPLENGTH AND HCIVERSION 0x120: [6.350898] extcon gpio: ID 0x0 VBUS 0x1 (0x8000ba02e2a0) [6.350900] extcon: extcon_set_state_sync \xc3\xaed:2 state:1 [6.366517] xhci-hcd xhci-hcd.0.auto: CAPLENGTH: 0x20 [6.371548] xhci-hcd xhci-hcd.0.auto: HCIVERSION: 0x100 [6.376761] xhci-hcd xhci-hcd.0.auto: HCSPARAMS 1: 0x1000140 [6.381793] xhci-hcd xhci-hcd.0.auto: Max device slots: 64 [6.387693] xhci-hcd xhci-hcd.0.auto: Max interrupters: 1 [6.393335] xhci-hcd xhci-hcd.0.auto: Max ports: 1 [6.398631] xhci-hcd xhci-hcd.0.auto: HCSPARAMS 2: 0xcf1 [6.403838] xhci-hcd xhci-hcd.0.auto: Isoc scheduling threshold: 1 [6.409482] xhci-hcd xhci-hcd.0.auto: Maximum allowed segments in event ring: 15 [6.415821] xhci-hcd xhci-hcd.0.auto: HCSPARAMS 3 0x7ff000a: [6.423206] xhci-hcd xhci-hcd.0.auto: Worst case U1 device exit latency: 10 [6.429014] xhci-hcd xhci-hcd.0.auto: Worst case U2 device exit latency: 2047 [6.436046] xhci-hcd xhci-hcd.0.auto: HCC PARAMS 0x220f665: [6.443162] xhci-hcd xhci-hcd.0.auto: HC generates 64 bit addresses [6.448717] xhci-hcd xhci-hcd.0.auto: HC hasn't Contiguous Frame ID Capability [6.455319] xhci-hcd xhci-hcd.0.auto: HC can generate Stopped - Short Package event [6.462781] xhci-hcd xhci-hcd.0.auto: FIXME: more HCCPARAMS debugging [6.470506] xhci-hcd xhci-hcd.0.auto: RTSOFF 0x440: [6.476929] xhci-hcd xhci-hcd.0.auto: xHCI operational registers at 0d408020: [6.481793] xhci-hcd xhci-hcd.0.auto: USBCMD 0x0: [6.489776] xhci-hcd xhci-hcd.0.auto: HC is being stopped [6.494465] xhci-hcd xhci-hcd.0.auto: HC has finished hard reset [6.499846] xhci-hcd xhci-hcd.0.auto: Event Interrupts disabled [6.506096] xhci-hcd xhci-hcd.0.auto: Host System Error Interrupts disabled [6.512261] xhci-hcd xhci-hcd.0.auto: HC has finished light reset [6.519378] xhci-hcd xhci-hcd.0.auto: USBSTS 0x11: [6.525453] xhci-hcd xhci-hcd.0.auto: Event ring is empty [6.530315] xhci-hcd xhci-hcd.0.auto: No Host System Error [6.535783] xhci-hcd xhci-hcd.0.auto: HC is halted [6.541689] xhci-hcd x
Re: Incorrect PID in drivers/usb/serial/kl5kusb105.h
On Wed, Jul 04, 2018 at 05:18:34PM +0100, Chris Jakob wrote: > Thanks. > Apologies I may have been a little premature as the device still does > not seem to be working (although it is now recognised) > > dmesg provides: > [6.864734] kl5kusb105: loading out-of-tree module taints kernel. > [6.864827] kl5kusb105: module verification failed: signature and/or > required key missing - tainting kernel > [6.865276] usbcore: registered new interface driver kl5kusb105 > [6.865311] usbserial: USB Serial support registered for KL5KUSB105D / > PalmConnect > [6.865352] kl5kusb105 2-1.2:1.0: KL5KUSB105D / PalmConnect converter > detected > [6.868735] usb 2-1.2: KL5KUSB105D / PalmConnect converter now attached to > ttyUSB0 > [6.897271] usb 3-2: pl2303 converter now attached to ttyUSB1 > [7.779557] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready > [7.849385] r8169 :02:00.0 eth0: link down > [7.849388] r8169 :02:00.0 eth0: link down > [7.849487] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready > [8.774276] kl5kusb105d ttyUSB0: Change port settings failed (error = -32) > [8.774289] usb 2-1.2: 5 byte block, baudrate 6, databits 8, u1 0, u2 1 > [8.774778] kl5kusb105d ttyUSB0: Enabling read failed (error = -32) > [8.776142] kl5kusb105d ttyUSB0: Change port settings failed (error = -32) > [8.776152] usb 2-1.2: 5 byte block, baudrate 6, databits 8, u1 0, u2 1 > [8.776527] kl5kusb105d ttyUSB0: Enabling read failed (error = -32) > [8.777904] kl5kusb105d ttyUSB0: Change port settings failed (error = -32) > [8.777913] usb 2-1.2: 5 byte block, baudrate 6, databits 8, u1 0, u2 1 > [8.778275] kl5kusb105d ttyUSB0: Enabling read failed (error = -32) > [8.779639] kl5kusb105d ttyUSB0: Change port settings failed (error = -32) > [8.779648] usb 2-1.2: 5 byte block, baudrate 6, databits 8, u1 0, u2 1 > [8.780004] kl5kusb105d ttyUSB0: Enabling read failed (error = -32) > [8.781387] kl5kusb105d ttyUSB0: Change port settings failed (error = -32) > [8.781396] usb 2-1.2: 5 byte block, baudrate 6, databits 8, u1 0, u2 1 > [8.781755] kl5kusb105d ttyUSB0: Enabling read failed (error = -32) > > It may be that my device is not working properly or I have incorrectly > built/installed the driver (not my area of expertise) rebooting has > had no effect I'm afraid the answer is just that the driver does not appear to support kl5kusb105 the device is is named after. Those -EPIPE (-32) errors above indicates that the device does not recognise the protocol currently implemented and the driver consequently aborts any open attempts. If you take look at the file header: * It seems that KLSI bought some silicon-design information from ScanLogic, * whose SL11R processor is at the core of the KL5KUSB chipset from KLSI. * KLSI has firmware available for their devices; it is probable that the * firmware differs from that used by KLSI in their products. If you have an * original KLSI device and can provide some information on it, I would be * most interested in adding support for it here. If you have any information * on the protocol used (or find errors in my reverse-engineered stuff), please * let me know. * * The code was only tested with a PalmConnect USB adapter; if you * are adventurous, try it with any KLSI-based device and let me know how it * breaks so that I can fix it! that seems to imply that the driver has never been tested with a KLSI device and that firmware differences could be expected. The device id typo, and your tests seems to confirm all of this. > Hopefully this is now in plain text…unless mac mail decides otherwise :( Yep, that worked. Unless you have access to a vendor driver and are in the mood of reverse engineering the protocol, I'm afraid there's not much we can do here. I should probably just remove the (incorrect) device-id entry as well. Thanks, Johan > lsusb -v as follows: > Bus 002 Device 003: ID 05e9:000c Kawasaki LSI USB-to-RS-232 > Device Descriptor: > bLength18 > bDescriptorType 1 > bcdUSB 1.00 > bDeviceClass 255 Vendor Specific Class > bDeviceSubClass 0 > bDeviceProtocol 0 > bMaxPacketSize0 8 > idVendor 0x05e9 Kawasaki LSI > idProduct 0x000c USB-to-RS-232 > bcdDevice1.01 > iManufacturer 1 (error) > iProduct2 (error) > iSerial 3 (error) > bNumConfigurations 1 > Configuration Descriptor: > bLength 9 > bDescriptorType 2 > wTotalLength 39 > bNumInterfaces 1 > bConfigurationValue 1 > iConfiguration 0 > bmAttributes 0x80 > (Bus Powered) > MaxPower 94mA > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber0 > bAlternateSett
usb: usbtest: TEST 13: set/clear 1 halts is getting failed with super speed
Hi, Posting this query again as I received some mail delivery failure notification on the previous attempt. I am running usbtest test cases and facing following issues: Setup: I am using two custom board running on Linux. One is configured as host and loaded with usbtest.ko module. Another one is configured as a device with gadget zero drivers. Host and device are interfaced in super speed mode. testusb application is used on host side test features. Both boards uses synopsis USB controller. Observation: With super speed interface, all test cases are passing except test 13 ( set/clear 1 halts). The error is shown in this test case usb 2-1: verify_still_halted failed, iterations left 0, status -71 (not -32) usbtest 2-1:3.0: halts failed, iterations left 999 Analysis: Digging deeper into the issue I have below observation: --- Same test case with the same setup is passing for the high-speed interface --- I tried the same test case only on out endpoint. Seeing the same issue. The test will try to send 1024 byte of data (bulk out) packet. This will lead two trb, one for 1024 byte data followed by zero packets. For first data packet, I am getting EPIPE(-32) error which is expected. Same also expected for zero data packet. But for this trb, we are getting EPROT (-71) which is failing test case. Can we use usbtest for super speed interface? What is expected error code if we will try to send a data packet on a halted pipe? I checked spec but didn't get any specific mention. Please help regarding the issue. Any input will be highly appreciated. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 2/2] usb: gadget: storage: Remove reference counting
Acked-by: Michal Nazarewicz
2018-07-04 5:51 GMT+01:00 Jaejoong Kim :
> The kref used to be needed because sharing of fsg_common among multiple USB
> function instances was handled by fsg. Now this is managed by configfs, we
> don't need it anymore. So let's eliminate kref from this driver.
>
> Signed-off-by: Jaejoong Kim
> ---
> Changes in V2:
> - Rewrite commit title & message.
> - Remove kref instead of removing unused kref EXPORT_SYMBOL (Alan and
> Michal)
>
> V1 patches
> https://patchwork.kernel.org/patch/10463709/
> https://patchwork.kernel.org/patch/10463713/
>
> drivers/usb/gadget/function/f_mass_storage.c | 27 +--
> drivers/usb/gadget/function/f_mass_storage.h | 4
> 2 files changed, 5 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c
> b/drivers/usb/gadget/function/f_mass_storage.c
> index 1b580eb..ca8a4b5 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -206,7 +206,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -312,8 +311,6 @@ struct fsg_common {
> void*private_data;
>
> char inquiry_string[INQUIRY_STRING_LEN];
> -
> - struct kref ref;
> };
>
> struct fsg_dev {
> @@ -2551,25 +2548,11 @@ static DEVICE_ATTR(file, 0, file_show, file_store);
>
> /** FSG COMMON **/
>
> -static void fsg_common_release(struct kref *ref);
> -
> static void fsg_lun_release(struct device *dev)
> {
> /* Nothing needs to be done */
> }
>
> -void fsg_common_get(struct fsg_common *common)
> -{
> - kref_get(&common->ref);
> -}
> -EXPORT_SYMBOL_GPL(fsg_common_get);
> -
> -void fsg_common_put(struct fsg_common *common)
> -{
> - kref_put(&common->ref, fsg_common_release);
> -}
> -EXPORT_SYMBOL_GPL(fsg_common_put);
> -
> static struct fsg_common *fsg_common_setup(struct fsg_common *common)
> {
> if (!common) {
> @@ -2582,7 +2565,6 @@ static struct fsg_common *fsg_common_setup(struct
> fsg_common *common)
> }
> init_rwsem(&common->filesem);
> spin_lock_init(&common->lock);
> - kref_init(&common->ref);
> init_completion(&common->thread_notifier);
> init_waitqueue_head(&common->io_wait);
> init_waitqueue_head(&common->fsg_wait);
> @@ -2870,9 +2852,8 @@ void fsg_common_set_inquiry_string(struct fsg_common
> *common, const char *vn,
> }
> EXPORT_SYMBOL_GPL(fsg_common_set_inquiry_string);
>
> -static void fsg_common_release(struct kref *ref)
> +static void fsg_common_release(struct fsg_common *common)
> {
> - struct fsg_common *common = container_of(ref, struct fsg_common, ref);
> int i;
>
> /* If the thread isn't already dead, tell it to exit now */
> @@ -3346,7 +3327,7 @@ static void fsg_free_inst(struct usb_function_instance
> *fi)
> struct fsg_opts *opts;
>
> opts = fsg_opts_from_func_inst(fi);
> - fsg_common_put(opts->common);
> + fsg_common_release(opts->common);
> kfree(opts);
> }
>
> @@ -3370,7 +3351,7 @@ static struct usb_function_instance
> *fsg_alloc_inst(void)
> rc = fsg_common_set_num_buffers(opts->common,
>
> CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS);
> if (rc)
> - goto release_opts;
> + goto release_common;
>
> pr_info(FSG_DRIVER_DESC ", version: " FSG_DRIVER_VERSION "\n");
>
> @@ -3393,6 +3374,8 @@ static struct usb_function_instance
> *fsg_alloc_inst(void)
>
> release_buffers:
> fsg_common_free_buffers(opts->common);
> +release_common:
> + kfree(opts->common);
> release_opts:
> kfree(opts);
> return ERR_PTR(rc);
> diff --git a/drivers/usb/gadget/function/f_mass_storage.h
> b/drivers/usb/gadget/function/f_mass_storage.h
> index 58857fc..3b8c4ce 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.h
> +++ b/drivers/usb/gadget/function/f_mass_storage.h
> @@ -115,10 +115,6 @@ fsg_opts_from_func_inst(const struct
> usb_function_instance *fi)
> return container_of(fi, struct fsg_opts, func_inst);
> }
>
> -void fsg_common_get(struct fsg_common *common);
> -
> -void fsg_common_put(struct fsg_common *common);
> -
> void fsg_common_set_sysfs(struct fsg_common *common, bool sysfs);
>
> int fsg_common_set_num_buffers(struct fsg_common *common, unsigned int n);
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
