Re: [PATCH v4] usb-core bInterval quirk
I'll fix it and resubmit. There is no reason I can't get a closing brace right.
Sorry.
Jim
On Sat, Jul 26, 2014 at 9:53 PM, Alan Stern wrote:
> On Sat, 26 Jul 2014, James P Michels III wrote:
>
>> This patch adds a usb quirk to support devices with interupt endpoints
>> and bInterval values expressed as microframes. The quirk causes the
>> parse endpoint function to modify the reported bInterval to a standards
>> conforming value.
>>
>> There is currently code in the endpoint parser that checks for
>> bIntervals that are outside of the valid range (1-16 for USB 2+ high
>> speed and super speed interupt endpoints). In this case, the code assumes
>> the bInterval is being reported in 1ms frames. As well, the correction
>> is only applied if the original bInterval value is out of the 1-16 range.
>>
>> With this quirk applied to the device, the bInterval will be
>> accurately adjusted from microframes to an exponent.
>>
>> Signed-off-by: James P Michels III
>
> The closing brace for the "if" statement is still in the wrong column
> (it's supposed to line up with the 'i' in "if"), but aside from that:
>
> Acked-by: Alan Stern
>
>> --- a/drivers/usb/core/config.c
>> +++ b/drivers/usb/core/config.c
>> @@ -199,6 +199,17 @@ static int usb_parse_endpoint(struct device *ddev, int
>> cfgno, int inum,
>> if (n == 0)
>> n = 9; /* 32 ms = 2^(9-1) uframes */
>> j = 16;
>> +
>> + /*
>> + * Adjust bInterval for quirked devices.
>> + * This quirk fixes bIntervals reported in
>> + * linear microframes.
>> + */
>> + if (to_usb_device(ddev)->quirks &
>> + USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL) {
>> + n = clamp(fls(d->bInterval), i, j);
>> + i = j = 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
[PATCH v5] usb-core bInterval quirk
This patch adds a usb quirk to support devices with interupt endpoints
and bInterval values expressed as microframes. The quirk causes the
parse endpoint function to modify the reported bInterval to a standards
conforming value.
There is currently code in the endpoint parser that checks for
bIntervals that are outside of the valid range (1-16 for USB 2+ high
speed and super speed interupt endpoints). In this case, the code assumes
the bInterval is being reported in 1ms frames. As well, the correction
is only applied if the original bInterval value is out of the 1-16 range.
With this quirk applied to the device, the bInterval will be
accurately adjusted from microframes to an exponent.
Signed-off-by: James P Michels III
---
drivers/usb/core/config.c | 11 +++
drivers/usb/core/quirks.c | 4
include/linux/usb/quirks.h | 11 +++
3 files changed, 26 insertions(+)
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 1ab4df1..b2a540b 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -199,6 +199,17 @@ static int usb_parse_endpoint(struct device *ddev, int
cfgno, int inum,
if (n == 0)
n = 9; /* 32 ms = 2^(9-1) uframes */
j = 16;
+
+ /*
+* Adjust bInterval for quirked devices.
+* This quirk fixes bIntervals reported in
+* linear microframes.
+*/
+ if (to_usb_device(ddev)->quirks &
+ USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL) {
+ n = clamp(fls(d->bInterval), i, j);
+ i = j = n;
+ }
break;
default:/* USB_SPEED_FULL or _LOW */
/* For low-speed, 10 ms is the official minimum.
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 739ee8e..33c42de 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -145,6 +145,10 @@ static const struct usb_device_id usb_quirk_list[] = {
/* SKYMEDI USB_DRIVE */
{ USB_DEVICE(0x1516, 0x8628), .driver_info = USB_QUIRK_RESET_RESUME },
+ /* Razer - Razer Blade Keyboard */
+ { USB_DEVICE(0x1532, 0x0116), .driver_info =
+ USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },
+
/* BUILDWIN Photo Frame */
{ USB_DEVICE(0x1908, 0x1315), .driver_info =
USB_QUIRK_HONOR_BNUMINTERFACES },
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index 52f944d..55a17b1 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -30,4 +30,15 @@
descriptor */
#define USB_QUIRK_DELAY_INIT 0x0040
+/*
+ * For high speed and super speed interupt endpoints, the USB 2.0 and
+ * USB 3.0 spec require the interval in microframes
+ * (1 microframe = 125 microseconds) to be calculated as
+ * interval = 2 ^ (bInterval-1).
+ *
+ * Devices with this quirk report their bInterval as the result of this
+ * calculation instead of the exponent variable used in the calculation.
+ */
+#define USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL 0x0080
+
#endif /* __LINUX_USB_QUIRKS_H */
--
1.9.1
--
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
XHCI_TRUST_TX_LENGTH quirk?
Sorry if this is answered somewhere, but I couldn't find anything specific on Google as far as how to determine if this quirk is needed or if its configurable at boot/run-time. Its an HP laptop, and it looks like other HP models have the problem. I most often get this when turning on a USB TV Tuner, which is also HP branded but shows up as a Hauppage device. I get glitches on the TV output, less with tvtime than other viewers, and these glitches go away if I transcode a DVD while watching TV .. yeah, I know that makes no sense. Not sure if the glitches are related to the dmesg warnings. I'll attach the lspci to the message in case anyone needs it. Please advise on the next step to proceed. I'm running a Ubuntu kernel, but I imagine a stock kernel would react the same. uname -a ... Linux Taro 3.13.0-32-lowlatency #57-Ubuntu SMP PREEMPT Tue Jul 15 04:08:59 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux dmesg output (all the repeats deleted) ... [270905.416782] xhci_hcd :00:10.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? [270905.432813] xhci_hcd :00:10.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? [270907.943397] retire_capture_urb: 34 callbacks suppressed [270910.103357] handle_tx_event: 90 callbacks suppressed [270910.103372] xhci_hcd :00:10.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? [270910.153968] xhci_hcd :00:10.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? lspci_-.txt.gz Description: GNU Zip compressed data lspci_-n.txt.gz Description: GNU Zip compressed data
System Malfunction with XHCI in 3.66 rc6
This is likely the same problem as the one I just posted about, but with 3.16 I get call traces and my USB bus came down and I couldn't even shut down the system. I've since gone back to my usual 3.13/Ubuntu kernel. Here's what happened. A snippet from dmesg is below, if anyone wants the whole thing, I've saved it. I just figured I'd report it! -- Evan Jul 22 23:58:26 Taro kernel: [ 9257.987466] CPU: 2 PID: 0 Comm: swapper/2 Not tainted 3.16.0-031600rc6-lowlatency #201407210035 Jul 22 23:58:26 Taro kernel: [ 9257.987471] Hardware name: Hewlett-Packard HP Pavilion TS 15 Notebook PC /216B, BIOS F.13 11/21/2013 Jul 22 23:58:26 Taro kernel: [ 9257.987476] 01f1 88021ed03c98 81789213 0007 Jul 22 23:58:26 Taro kernel: [ 9257.987485] 88021ed03cd8 81071afc 815b6f00 Jul 22 23:58:26 Taro kernel: [ 9257.987493] 8800a9fe8000 88021ed03d70 8801ec91b680 0001c5999000 Jul 22 23:58:26 Taro kernel: [ 9257.987501] Call Trace: Jul 22 23:58:26 Taro kernel: [ 9257.987506] [] dump_stack+0x4e/0x71 Jul 22 23:58:26 Taro kernel: [ 9257.987526] [] warn_slowpath_common+0x8c/0xc0 Jul 22 23:58:26 Taro kernel: [ 9257.987535] [] ? trace_xhci_dbg_ring_expansion+0xa0/0xa0 Jul 22 23:58:26 Taro kernel: [ 9257.987542] [] warn_slowpath_null+0x1a/0x20 Jul 22 23:58:26 Taro kernel: [ 9257.987550] [] xhci_find_new_dequeue_state+0x1b1/0x2e0 Jul 22 23:58:26 Taro kernel: [ 9257.987559] [] xhci_handle_cmd_stop_ep.isra.42+0x122/0x2b0 Jul 22 23:58:26 Taro kernel: [ 9257.987568] [] handle_cmd_completion+0x2d6/0x4a0 Jul 22 23:58:26 Taro kernel: [ 9257.987576] [] xhci_handle_event+0x1ae/0x2a0 Jul 22 23:58:26 Taro kernel: [ 9257.987584] [] xhci_irq+0x120/0x1f0 Jul 22 23:58:26 Taro kernel: [ 9257.987592] [] xhci_msi_irq+0x11/0x20 Jul 22 23:58:26 Taro kernel: [ 9257.987602] [] handle_irq_event_percpu+0x3e/0x210 Jul 22 23:58:26 Taro kernel: [ 9257.987612] [] handle_irq_event+0x48/0x70 Jul 22 23:58:26 Taro kernel: [ 9257.987619] [] handle_edge_irq+0x77/0x110 Jul 22 23:58:26 Taro kernel: [ 9257.987626] [] handle_irq+0x22/0x40 Jul 22 23:58:26 Taro kernel: [ 9257.987634] [] do_IRQ+0x5e/0x110 Jul 22 23:58:26 Taro kernel: [ 9257.987642] [] common_interrupt+0x6d/0x6d Jul 22 23:58:26 Taro kernel: [ 9257.987645] [] ? sched_clock+0x9/0x10 Jul 22 23:58:26 Taro kernel: [ 9257.987661] [] ? cpuidle_enter_state+0x5e/0xe0 Jul 22 23:58:26 Taro kernel: [ 9257.987668] [] ? cpuidle_enter_state+0x57/0xe0 Jul 22 23:58:26 Taro kernel: [ 9257.987676] [] cpuidle_enter+0x17/0x20 Jul 22 23:58:26 Taro kernel: [ 9257.987684] [] cpuidle_idle_call+0xe9/0x2c0 Jul 22 23:58:26 Taro kernel: [ 9257.987692] [] cpu_idle_loop+0x255/0x270 Jul 22 23:58:26 Taro kernel: [ 9257.987701] [] cpu_startup_entry+0x5b/0x60 Jul 22 23:58:26 Taro kernel: [ 9257.987708] [] start_secondary+0xd5/0xe0 Jul 22 23:58:26 Taro kernel: [ 9257.987715] ---[ end trace b40b77c49a400b3d ]--- Jul 22 23:58:26 Taro kernel: [ 9257.987807] recv_control_msg() Failed receiving control message, error -110. Jul 22 23:58:28 Taro kernel: [ 9258.988890] send_control_msg() Failed sending control message, error -110. -- 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 1/3] usb: gadget: f_fs: virtual address mapping
On 07/25/2014 04:18 PM, Michal Nazarewicz wrote: > On Fri, Jul 25 2014, Robert Baldyga wrote: >> This patch adds virtual endpoint address mapping to functionfs. >> >> So far endpoint addresses given by user through endpoint descriptors >> were ignored, and replaced by physical endpoint addresses. Endpoint >> address in wIndex field of setup requesti, addressed to endpoint, was >> the physical endpoint address, and names of files in functionfs >> directory was numered in order, and were the same as indexes of >> ffs_epfile in epfile array. In result user has no way to indicate >> which file in functionfs is associated with which particular >> requested endpoint. He also didn't know which endpoint is recipient >> of setup request. > > Couldn't that be solved by simply providing the mapping to user space? There would be only small differences in code (add mapping instead of changing file names) so why would we not want do it in more intuitive way? > >> There was also one more problem - if endpoint addresses in descriptors >> were non-consecutive, there were created redundant files, which could >> cause problems in kernel, when user tryed to read/write to them. >> It was result of fact that maximum endpoint address was taken as >> total number of endpoints in funciton. > > This is kinda unrelated though. I mean it's a separate bug. Yes, but it can be fixed by the way, as a side effect, so there is no sense (and probably no simple way) to move it into separate patch. > >> This patch solves this problems by introducing virtual endpoint address >> mapping. Now each function has separate endpoint address space. Numbers >> of endpoint files in functionfs and addresses in setup requests are >> mapped to addresses choosen by user in endpoint descriptors. >> >> It also introduces additional testing if desctiptors given by user are >> consistent - if number of endpoints and their addresses in each speed >> are the same. >> >> Signed-off-by: Robert Baldyga >> --- >> drivers/usb/gadget/f_fs.c | 78 >> +++ >> drivers/usb/gadget/u_fs.h | 2 ++ >> 2 files changed, 68 insertions(+), 12 deletions(-) > -- 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/3] usb: gadget: f_fs: add ioctl returning ep descriptor
On 07/25/2014 04:15 PM, Michal Nazarewicz wrote:
> On Fri, Jul 25 2014, Robert Baldyga wrote:
>> This patch introduces ioctl named FUNCTIONFS_ENDPOINT_DESC, which
>> returns endpoint descriptor to userspace. It works only if function
>> is active.
>
> I would argue that user space should never need to know the real
> descriptor. Is this ioctl needed for anything in particular?
It's needed, at least, to inform user space about maximum possible
wMaxPacketSize value for the endpoint, which is returned by autoconfig
through the endpoint descriptor when value set by user is zero.
Descriptor returned form this ioctl is mostly the same as descriptor
created by user. I guess you meant that real descriptor is this one,
which contains real endpoint address, because it's probably only datum
in real descriptor which should be never known by user space.
This ioctl would be also useful in case, when FunctionFS daemon obtains
endpoint descriptors from another daemon, and do not have access to
original descriptor structures given do FunctionFS.
>
>>
>> Signed-off-by: Robert Baldyga
>> ---
>> drivers/usb/gadget/f_fs.c | 17 +
>> include/uapi/linux/usb/functionfs.h | 6 ++
>> 2 files changed, 23 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
>> index 490b30f..a2e18cc 100644
>> --- a/drivers/usb/gadget/f_fs.c
>> +++ b/drivers/usb/gadget/f_fs.c
>> @@ -1031,6 +1031,23 @@ static long ffs_epfile_ioctl(struct file *file,
>> unsigned code,
>> case FUNCTIONFS_ENDPOINT_REVMAP:
>> ret = epfile->ep->num;
>> break;
>> +case FUNCTIONFS_ENDPOINT_DESC:
>> +{
>> +int desc_idx;
>> +struct usb_endpoint_descriptor *desc;
>> +
>> +if (epfile->ffs->gadget->speed == USB_SPEED_SUPER)
>> +desc_idx = 2;
>> +else if (epfile->ffs->gadget->speed == USB_SPEED_HIGH)
>> +desc_idx = 1;
>> +else
>> +desc_idx = 0;
>> +desc = epfile->ep->descs[desc_idx];
>> +ret = copy_to_user((void *)value, desc, sizeof(*desc));
>> +if (ret)
>> +ret = -EFAULT;
>> +break;
>> +}
>> default:
>> ret = -ENOTTY;
>> }
>> diff --git a/include/uapi/linux/usb/functionfs.h
>> b/include/uapi/linux/usb/functionfs.h
>> index 1dc473a..1ab6f06 100644
>> --- a/include/uapi/linux/usb/functionfs.h
>> +++ b/include/uapi/linux/usb/functionfs.h
>> @@ -197,6 +197,12 @@ struct usb_functionfs_event {
>> */
>> #define FUNCTIONFS_ENDPOINT_REVMAP _IO('g', 129)
>>
>> +/*
>> + * Returns endpoint descriptor. In funciton is not active returns -ENODEV.
>> + */
>> +#define FUNCTIONFS_ENDPOINT_DESC_IOR('g', 130, \
>> + struct usb_endpoint_descriptor)
>> +
>>
>>
>> #endif /* _UAPI__LINUX_FUNCTIONFS_H__ */
>> --
>> 1.9.1
>>
>
--
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/3] usb: gadget: f_fs: add ioctl returning ep descriptor
Hello,
On 2014-07-25 16:15, Michal Nazarewicz wrote:
On Fri, Jul 25 2014, Robert Baldyga wrote:
This patch introduces ioctl named FUNCTIONFS_ENDPOINT_DESC, which
returns endpoint descriptor to userspace. It works only if function
is active.
I would argue that user space should never need to know the real
descriptor. Is this ioctl needed for anything in particular?
Some proprietary broken usb protocols might embed real physical endpoint
(or interface) number into the data stream. In such case functionfs driver
is unable to do proper endpoint/interface number fixup.
Signed-off-by: Robert Baldyga
---
drivers/usb/gadget/f_fs.c | 17 +
include/uapi/linux/usb/functionfs.h | 6 ++
2 files changed, 23 insertions(+)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 490b30f..a2e18cc 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -1031,6 +1031,23 @@ static long ffs_epfile_ioctl(struct file *file, unsigned
code,
case FUNCTIONFS_ENDPOINT_REVMAP:
ret = epfile->ep->num;
break;
+ case FUNCTIONFS_ENDPOINT_DESC:
+ {
+ int desc_idx;
+ struct usb_endpoint_descriptor *desc;
+
+ if (epfile->ffs->gadget->speed == USB_SPEED_SUPER)
+ desc_idx = 2;
+ else if (epfile->ffs->gadget->speed == USB_SPEED_HIGH)
+ desc_idx = 1;
+ else
+ desc_idx = 0;
+ desc = epfile->ep->descs[desc_idx];
+ ret = copy_to_user((void *)value, desc, sizeof(*desc));
+ if (ret)
+ ret = -EFAULT;
+ break;
+ }
default:
ret = -ENOTTY;
}
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 1dc473a..1ab6f06 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -197,6 +197,12 @@ struct usb_functionfs_event {
*/
#define FUNCTIONFS_ENDPOINT_REVMAP _IO('g', 129)
+/*
+ * Returns endpoint descriptor. In funciton is not active returns -ENODEV.
+ */
+#defineFUNCTIONFS_ENDPOINT_DESC_IOR('g', 130, \
+struct usb_endpoint_descriptor)
+
#endif /* _UAPI__LINUX_FUNCTIONFS_H__ */
--
1.9.1
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
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/3] usb: gadget: f_fs: add ioctl returning ep descriptor
On Mon, Jul 28, 2014 at 08:42:44AM +0200, Marek Szyprowski wrote: > Hello, > > On 2014-07-25 16:15, Michal Nazarewicz wrote: > >On Fri, Jul 25 2014, Robert Baldyga wrote: > >>This patch introduces ioctl named FUNCTIONFS_ENDPOINT_DESC, which > >>returns endpoint descriptor to userspace. It works only if function > >>is active. > >I would argue that user space should never need to know the real > >descriptor. Is this ioctl needed for anything in particular? > > Some proprietary broken usb protocols might embed real physical endpoint > (or interface) number into the data stream. In such case functionfs driver > is unable to do proper endpoint/interface number fixup. Do you have an example of such a messed up thing? greg k-h -- 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
