Re: [PATCH v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
Hi George, On 08/29/2013 11:21 AM, George Cherian wrote: Hi Chanwoo, Thanks for the review and sorry for all the trivial mistakes. On 8/29/2013 7:05 AM, Chanwoo Choi wrote: Hi George, You didn't modify this patchset about my comment on v1 patchset. Please pay attention to comment. On 08/29/2013 02:33 AM, George Cherian wrote: Add a generic USB VBUS/ID detection EXTCON driver. This driver expects the ID/VBUS pin are connected via GPIOs. This driver is tested on DRA7x board were the ID pin is routed via GPIOs. The driver supports both VBUS and ID pin configuration and ID pin only configuration. Signed-off-by: George Cherian george.cher...@ti.com --- .../bindings/extcon/extcon-gpio-usbvid.txt | 20 ++ drivers/extcon/Kconfig | 6 + drivers/extcon/Makefile| 1 + drivers/extcon/extcon-gpio-usbvid.c| 286 + You should keep following naming stlye. extcon-gpio-usbvid.c is wrong naming style. - extcon-[device name].c - extcon-gpio-usbvid.c - extcon-dra7xx.c or etc. Actually dra7xx is the SoC name and the USB VBUS/ID detection is not specific to SoC. It uses gpios to detect the VBUS/ID change. So i thought it would be better to have generic gpio based VBUS/ID detection rather than making dra7xx specific. Stephen Warren had this opinion with patch v1. Would you guarantee that this driver support all of extcon devices using gpio pin? I can't agree. This driver has specific dependency on dra7x SoC. First, vbus_irq_handler() determine USB cable state according to gpio_usbvid-vbus_gpio state. If gpio_usbvid-vbus_gpio value is VBUS_OFF(0), this patch set USB cable state as 'false' But, it include potential problems. Other extcon device using gpio would set USB cable state as 'true' when gpio state is 1. This way has dependency on specific SoC. Second, gpio_usbvid_set_initial_state() dertermine both USB-HOST and USB cable state at same time in 'case ID_DETCT' according to 'gpio_usbvid-id_gpio'. But, I think that other extcon devices would not control both USB-HOST and USB cable state at same time. Other extcon devices would support either USB-HOST or USB cable. Third, gpio_usbvid_probe() get both gpio_usbvid-id_irq and gpio_usbvid-vbus_irq by using DT helper function. and gpio_usbvid_request_irq() register each interrupt handler(id_irq and vbus_irq) according to DT data. In result, id_irq_handler() would control both USB-HOST and USB cable state. vbus_irq_handler() would control only USB cable state. Two interrupt handler(id_irq_handler()/vbus_irq_handler()) would control USB cable state according to each gpio state at same time. Also, It include critical problem. Also, you should change the file name of extcon-gpio-usbvid.txt. Finally, You used 'gpio_usbvid' prefix in extcon-gpio-usbvid.c. It has caused the confusion that user would think extcon-gpio-usbvid.c driver support all of extcon driver using gpio irq pin. So I'd like you to use proper prefix including device name. I meant to support all of extcon driver using gpio for USB VBUS/ID detection. 4 files changed, 313 insertions(+) create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio-usbvid.txt create mode 100644 drivers/extcon/extcon-gpio-usbvid.c [snip] index f1d54a3..8097398 100644 --- a/drivers/extcon/Kconfig +++ b/drivers/extcon/Kconfig @@ -64,4 +64,10 @@ config EXTCON_PALMAS Say Y here to enable support for USB peripheral and USB host detection by palmas usb. +config EXTCON_GPIO_USBVID +tristate Generic USB VBUS/ID detection using GPIO EXTCON support +help + Say Y here to enable support for USB VBUS/ID deetction by GPIO. + + Remove blank line. okay endif # MULTISTATE_SWITCH [snip] diff --git a/drivers/extcon/extcon-gpio-usbvid.c b/drivers/extcon/extcon-gpio-usbvid.c new file mode 100644 index 000..e9bc2a97 --- /dev/null +++ b/drivers/extcon/extcon-gpio-usbvid.c @@ -0,0 +1,286 @@ +/* + * Generic USB VBUS-ID pin detection driver + * + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * Author: George Cherian george.cher...@ti.com + * + * Based on extcon-palmas.c + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include linux/module.h +#include linux/interrupt.h +#include linux/kthread.h +#include linux/freezer.h
Re: [PATCH 2/4] usb: r8a66597-hcd: use platform_{get,set}_drvdata()
On Thu, Aug 29, 2013 at 10:54:51AM +0800, Libo Chen wrote:
On 2013/8/28 12:36, Greg KH wrote:
On Tue, Aug 27, 2013 at 04:10:22PM +0800, Libo Chen wrote:
Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with pdev-dev,
so we can directly pass a struct platform_device.
Signed-off-by: Libo Chen libo.c...@huawei.com
---
drivers/usb/host/r8a66597-hcd.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
This patch doesn't apply to my tree :(
Hi Greg,
This patch based on mainline.
Do you mean this patch got a conflicted in
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
I need to rebase on usb.git?
Yes, please use the usb-next branch.
thanks,
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
Re: [PATCH v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
Hi Chanwoo,
On 8/29/2013 11:53 AM, Chanwoo Choi wrote:
[snip]
You should keep following naming stlye. extcon-gpio-usbvid.c is wrong naming
style.
- extcon-[device name].c
- extcon-gpio-usbvid.c - extcon-dra7xx.c or etc.
Actually dra7xx is the SoC name and the USB VBUS/ID detection is not specific
to SoC.
It uses gpios to detect the VBUS/ID change. So i thought it would be better to
have generic
gpio based VBUS/ID detection rather than making dra7xx specific. Stephen Warren
had this opinion
with patch v1.
Would you guarantee that this driver support all of extcon devices using gpio
pin?
This driver would guarantee extcon devices using gpio pins for USB VBUS
and ID detection.
Following is the basic assumption made, correct me if I am wrong.
ID pin ground means - USB-HOST is true (this happens when a device is
connected to USB port and we act as HOST )
ID pin Float means - USB-HOST is false (this happens when nothing is
connected or when we act as a peripheral under a HOST)
VBUS ON means - USB is true (this happens when we are connected under a
HOST as a peripheral)
VBUS OFF means - USB is false ( this happens when we are either
disconnected from a HOST or when we are in HOST mode).
So normally USB is in peripheral mode is enabled when ID pin is float
and VBUS is ON.
and USB is in HOST mode when ID pin is ground and VBUS is OFF.
In all above cases VBUS is referred to the external VBUS supply from an
external HOST.
I can't agree. This driver has specific dependency on dra7x SoC.
First,
vbus_irq_handler() determine USB cable state according to
gpio_usbvid-vbus_gpio state.
If gpio_usbvid-vbus_gpio value is VBUS_OFF(0), this patch set USB cable
state as 'false'
But, it include potential problems. Other extcon device using gpio would set
USB cable state
as 'true' when gpio state is 1. This way has dependency on specific SoC.
no this is not SoC specific. VBUS is referred to the external VBUS
supply from an external HOST.
so if VBUS is zero means its definitely not in connected state.
Second,
gpio_usbvid_set_initial_state() dertermine both USB-HOST and USB cable
state at same time
in 'case ID_DETCT' according to 'gpio_usbvid-id_gpio'. But, I think that other
extcon devices
would not control both USB-HOST and USB cable state at same time.
Other extcon devices would support either USB-HOST or USB cable.
The driver has 2 configurations.
1) supports implementations with both VBUS and ID pin are routed via
gpio's for swicthing roles(compatible gpio-usb-vid).
2) supports implementations with only ID pin routed via gpio for
swicthing roles(compatible gpio-usb-id).
So if you take type as VBUS_ID_DETECT then it is as what you meant.
Third,
gpio_usbvid_probe() get both gpio_usbvid-id_irq and gpio_usbvid-vbus_irq by
using DT helper function.
and gpio_usbvid_request_irq() register each interrupt handler(id_irq and
vbus_irq) according to DT data.
In result,
id_irq_handler() would control both USB-HOST and USB cable state.
only if type is ID_DETECT id_irq_handler control both USB-HOST and USB
cable states
if type is VBUS_ID_DETECT then id_irq_handler only controls USB-HOST.
vbus_irq_handler() would control only USB cable state.
Two interrupt handler(id_irq_handler()/vbus_irq_handler()) would control USB
cable state
according to each gpio state at same time. Also, It include critical problem.
No it depends on the configuration as explained above.
[snip]
+
+#define ID_GND0
+#define ID_FLOAT1
+#define VBUS_OFF0
+#define VBUS_ON1
I think you could only use two constant instead of four constant definition.
you mean only ID_GND and VBUS_OFF?
you could only define two contant (0 and 1) to detect gpio state.
okay
+
+
This blank line isn't necessary.
+static irqreturn_t id_irq_handler(int irq, void *data)
+{
+struct gpio_usbvid *gpio_usbvid = (struct gpio_usbvid *) data;
You should delete blank between ')' and 'data' as follwong:
- (struct gpio_usbvid *)data;
okay
+int id_current;
+
+id_current = gpio_get_value_cansleep(gpio_usbvid-id_gpio);
+if (id_current == ID_GND) {
+if (gpio_usbvid-type == ID_DETECT)
+extcon_set_cable_state(gpio_usbvid-edev,
+USB, false);
+extcon_set_cable_state(gpio_usbvid-edev, USB-HOST, true);
As else statement, you should set USB-HOST cable state to improve readability.
extcon_set_cable_state(gpio_usbvid-edev, USB-HOST, true);
if (gpio_usbvid-type == ID_DETECT)
extcon_set_cable_state(gpio_usbvid-edev,
USB, false);
Actually, USB-HOST state should be set in the id_irq handler. But in cases were
only ID pin is routed to gpio
and VBUS is not used we set the USB state too. The gpio_usbvid-type
differentiates whether its an ID only or
VBUS and ID.
I don't understand. Wht does not you change the order of function call as
following?
Before:
if
Re: Bug 60810 - Kernel oops with controller XHCI while wait usb packet
Hi,
On Wed, Aug 28, 2013 at 09:46:42PM -0700, Giovanni wrote:
https://bugzilla.kernel.org/show_bug.cgi?id=60810
Bug ID: 60810
Summary: Kernel oops with controller XHCI while wait usb packet
Can you give us a detail kernel top commit and config which you built,
in order to locate the break line (address offset) accurately like below:
(gdb) l*handle_cmd_completion+0x502
0xc4b2 is in handle_cmd_completion (drivers/usb/host/xhci-ring.c:1399).
1394
1395switch (le32_to_cpu(xhci-cmd_ring-dequeue-generic.field[3])
1396 TRB_TYPE_BITMASK) {
1397case TRB_TYPE(TRB_ENABLE_SLOT):
1398if (GET_COMP_CODE(le32_to_cpu(event-status)) ==
COMP_SUCCESS)
1399xhci-slot_id = slot_id;
1400else
1401xhci-slot_id = 0;
1402complete(xhci-addr_dev);
1403break;
(gdb)
I built on top v3.10, but it looks like line 1399 isn't the breakpoit.
Thanks,
Rui
--
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: linux-next: Tree for Aug 28 [ xhci build breakage ]
On Wed, Aug 28, 2013 at 11:15 PM, Sarah Sharp sarah.a.sh...@linux.intel.com wrote: On Wed, Aug 28, 2013 at 07:39:14PM +0200, Sedat Dilek wrote: On Wed, Aug 28, 2013 at 7:24 PM, Dmitry Kasatkin d.kasat...@samsung.com wrote: Still noone answered me why drivers/usb/host/xhci-ring.c does NOT include linux/device.h (dev_info_ratelimited() and other defines). I am expecting that... even I see... drivers/usb/host/.xhci-ring.o.cmd:715: include/linux/device.h \ ...where I don't know why this happens. ( For me this is a bit more important than trimming my responses, I keep the history... ) - Sedat - P.S.: List of includes in xhci-ring.c $ grep ^'#include' -nr drivers/usb/host/xhci-ring.c 67:#include linux/scatterlist.h 68:#include linux/slab.h 69:#include xhci.h 70:#include xhci-trace.h Because a header that xhci-ring.c uses includes device.h instead. drivers/usb/host/xhci/xhci-ring.c includes drivers/usb/host/xhci.h which includes include/linux/usb.h which includes include/linux/device.h All USB host controllers depend on including usb.h, so I don't think there's a need for the driver to explicitly include device.h. Thanks for the explanations. On the one hand it is a fine thingie to place include-files at one single place - think of renamed or moved (uapi) include-files. Looking at xhci-ring.c means for me to dig through 3 or 4 files as someone not dealing everyday with USB stuff. What is the effect of CONFIG_DYNAMIC_DEBUG=[y|n] in the affected code? - Sedat - P.S.: The forgotten patch is now in usb-next, but I don't see any credits, coins, gold, platin... -- 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: [alsa-devel] Buffer size for ALSA USB PCM audio
On Wed, Aug 28, 2013 at 7:46 PM, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 28 Aug 2013, Clemens Ladisch wrote: Sorry, what I said applies more to explicit sync endpoints. When using implicit sync, a playback URB is submitted for each completed capture URB, with the number of samples per packet identical to the corresponding capture packet, so the parameters must be identical. Got it. Below is an updated patch. James, the problem you encountered was most likely a result of the faulty treatment of capture endpoints that Clemens pointed out. It was quite obvious in the usbmon traces that the unpatched driver used 8 packets per URB whereas the patched driver used 22. This updated patch should fix that problem. Works fine. With this, it is possible to get clear playback at 64 frames/period too. With vanilla 3.10.9, there was some glitchy distortion to the sound at that latency, so this seems to be an improvement. James -- 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 net,stable] net: usb: Add HP hs2434 device to ZLP exception table
David Miller da...@davemloft.net writes: From: Rob Gardner robma...@gmail.com Date: Wed, 28 Aug 2013 18:40:22 -0600 The exception list means usb_device_id entries for specific devices that are known to need the workaround. There are just two such entries. There isn't even a separate list. So maybe we just have a nomenclature problem, and we could simply be calling this a white list instead of exception list. The other possible meaning of whitelist (all devices that *don't* need the workaround) is unthinkable, as that would be a huge list and much more prone to be unmanageable than what we've got now. Ok I misunderstood. I thought we were discoving that all chips which have been thoroughly investigated end up needing the workaround. I guess it's only a specific family of these chips which seem to all have the problem? This driver is a class driver, which means that it deals with a number of completely independent chip designs from different vendors. We have so far seen and tested chips from ST-Ericsson, Qualcomm and Mediatek with this driver. Given that Microsoft requires MBIM, we are likely to see implementations on all available 3G/LTE chips (if there are any others?) Only devices with Qualcomm chips have the bug in question, and so far we have only observed it in modules made by Sierra Wireless. The firmware in these modules has a base part made by Qualcomm and an application part made by Sierra Wireless. We do not know which part of the firmware is responsible for the bug, but we do know that the Qualcomm base firmware used by these modules support MBIM framing. Based on this, I am guessing that the chip vendor Qualcomm is responsible for the bug. If correct, then the bug is likely to appear in products from many different module vendors. It would not be the first time we saw that... I do have a Huawei MBIM device with a Qualcomm chip, but this module does not have the Qualcomm firmware with MBIM support. The MBIM implementation is therefore assumed to be made by Huawei and running as a firmware application on the device. This device does not have the bug. Now, if we could just identify devices running a Qualcomm base firmware with MBIM support then we could enable the workaround for all those. But the driver does not have any way to identify them. It must base its decision on the USB descriptors, in practice only device ID and product ID. A module vendor can use chips from different sources (Huawei does this), and as Rob says: Laptop vendors will often have modules made with their own vendor ID. Most laptop vendors will also use modules from different vendors. HP, as a current example, is known to use modules both from Sierra Wireless (need the workaround) and Ericsson (does not want the workaround). This makes any sort of vendor matching difficult, and we end up with device specific blacklisting or whitelisting as the only option. If it had been only the two devices we have now, or even only a few tens of devices, this would not have been a big deal. But I do expect that the length of this exception list will be comparable to the list of devices supported by the qmi_wwan driver, i.e. hundreds. And, like that driver, the list will probably never be complete. Given the difficulties detecting the need for the workaround, the list is probably going to be extremely incomplete. Only a small percentage of end users with affected devices will be able to provide the necessary information. For the question of the workaround impact on devices without bug: They will work fine, and the effect will not be user noticable AFAICS. But it will have a negative impact on the device, most likely causing higher power consumption. Alexey has explained this much better than I can, as I do not fully understand the device firmware design and requirements: http://www.spinics.net/lists/linux-usb/msg78078.html Quoting part of his explanation (talking about the *device* CPU and INT - we do not care about the host): If host decides to send ZLP after full NTB, CPU must handle additional INT per every full NTB instead of doing useful work. For FTP transfer with constantly full NTBs you get twice amount of Interrupts. I do agree that this is unfortunate. And hurting standard compliant devices to work around a standard violating bug in other devices does not feel right. But I do not see any other option. Bjørn -- 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/4] ehci: remove ehci_vdbg() verbose debugging statements
This patch removes ehci_vdbg debugging statements from EHCI host controller
driver because they produce too much information, lowering the signal to noise
ratio when debugging, and because they are not used anymore.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/ehci-hub.c | 6 -
drivers/usb/host/ehci-q.c | 7 --
drivers/usb/host/ehci-sched.c | 56 ++-
drivers/usb/host/ehci.h | 5
4 files changed, 7 insertions(+), 67 deletions(-)
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 3bf9f48..835fc08 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -211,8 +211,6 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd
*ehci,
else
t2 |= PORT_WKOC_E | PORT_WKCONN_E;
}
- ehci_vdbg(ehci, port %d, %08x - %08x\n,
- port + 1, t1, t2);
ehci_writel(ehci, t2, reg);
}
@@ -302,8 +300,6 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
}
if (t1 != t2) {
- ehci_vdbg (ehci, port %d, %08x - %08x\n,
- port + 1, t1, t2);
ehci_writel(ehci, t2, reg);
changed = 1;
}
@@ -483,7 +479,6 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
if (test_bit(i, resume_needed)) {
temp = ~(PORT_RWC_BITS | PORT_SUSPEND | PORT_RESUME);
ehci_writel(ehci, temp, ehci-regs-port_status [i]);
- ehci_vdbg (ehci, resumed port %d\n, i + 1);
}
}
@@ -1204,7 +1199,6 @@ static int ehci_hub_control (
wIndex + 1);
temp |= PORT_OWNER;
} else {
- ehci_vdbg (ehci, port %d reset\n, wIndex + 1);
temp |= PORT_RESET;
temp = ~PORT_PE;
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 687..cf9f2fb 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -240,13 +240,6 @@ static int qtd_copy_status (
} else {/* unknown */
status = -EPROTO;
}
-
- ehci_vdbg (ehci,
- dev%d ep%d%s qtd token %08x -- status %d\n,
- usb_pipedevice (urb-pipe),
- usb_pipeendpoint (urb-pipe),
- usb_pipein (urb-pipe) ? in : out,
- token, status);
}
return status;
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 6631089..833c35c 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -327,17 +327,8 @@ static int tt_available (
periodic_tt_usecs (ehci, dev, frame, tt_usecs);
- ehci_vdbg(ehci, tt frame %d check %d usecs start uframe %d in
-schedule %d/%d/%d/%d/%d/%d/%d/%d\n,
- frame, usecs, uframe,
- tt_usecs[0], tt_usecs[1], tt_usecs[2], tt_usecs[3],
- tt_usecs[4], tt_usecs[5], tt_usecs[6], tt_usecs[7]);
-
- if (max_tt_usecs[uframe] = tt_usecs[uframe]) {
- ehci_vdbg(ehci, frame %d uframe %d fully scheduled\n,
- frame, uframe);
+ if (max_tt_usecs[uframe] = tt_usecs[uframe])
return 0;
- }
/* special case for isoc transfers larger than 125us:
* the first and each subsequent fully used uframe
@@ -348,13 +339,8 @@ static int tt_available (
int ufs = (usecs / 125);
int i;
for (i = uframe; i (uframe + ufs) i 8; i++)
- if (0 tt_usecs[i]) {
- ehci_vdbg(ehci,
- multi-uframe xfer can't fit
- in frame %d uframe %d\n,
- frame, i);
+ if (0 tt_usecs[i])
return 0;
- }
}
tt_usecs[uframe] += usecs;
@@ -362,12 +348,8 @@ static int tt_available (
carryover_tt_bandwidth(tt_usecs);
/* fail if the carryover pushed bw past the last uframe's limit
*/
- if (max_tt_usecs[7] tt_usecs[7]) {
- ehci_vdbg(ehci,
- tt unavailable usecs %d frame %d uframe %d\n,
- usecs, frame,
[PATCH 4/4] ehci: enable debugging code when CONFIG_DYNAMIC_DEBUG is set
The debugging code for ehci is enabled to run if the DEBUG flag is defined.
This patch enables the debugging code also when the kernel is configured
with dynamic debugging on.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/ehci-dbg.c | 8
drivers/usb/host/ehci-fsl.c | 2 +-
drivers/usb/host/ehci-hcd.c | 6 +++---
drivers/usb/host/ehci-q.c | 4 ++--
drivers/usb/host/ehci-sched.c | 2 +-
drivers/usb/host/ehci.h | 8
6 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index c2f4489..aa5b603 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -18,7 +18,7 @@
/* this file is part of ehci-hcd.c */
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
/* check the values in the HCSPARAMS register
* (host controller _Structural_ parameters)
@@ -62,7 +62,7 @@ static inline void dbg_hcs_params (struct ehci_hcd *ehci,
char *label) {}
#endif
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
/* check the values in the HCCPARAMS register
* (host controller _Capability_ parameters)
@@ -101,7 +101,7 @@ static inline void dbg_hcc_params (struct ehci_hcd *ehci,
char *label) {}
#endif
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
static void __maybe_unused
dbg_qtd (const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
@@ -301,7 +301,7 @@ static inline int __maybe_unused
dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status)
{ return 0; }
-#endif /* DEBUG */
+#endif /* DEBUG || CONFIG_DYNAMIC_DEBUG */
/* functions have the wrong filename when they're output... */
#define dbg_status(ehci, label, status) { \
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index e44f442..947b009 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -418,7 +418,7 @@ static int ehci_fsl_mpc512x_drv_suspend(struct device *dev)
struct fsl_usb2_platform_data *pdata = dev_get_platdata(dev);
u32 tmp;
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
u32 mode = ehci_readl(ehci, hcd-regs + FSL_SOC_USB_USBMODE);
mode = USBMODE_CM_MASK;
tmp = ehci_readl(ehci, hcd-regs + 0x140); /* usbcmd */
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 09a01fb..5d6022f 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1292,7 +1292,7 @@ static int __init ehci_hcd_init(void)
sizeof(struct ehci_qh), sizeof(struct ehci_qtd),
sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
ehci_debug_root = debugfs_create_dir(ehci, usb_debug_root);
if (!ehci_debug_root) {
retval = -ENOENT;
@@ -1341,7 +1341,7 @@ clean2:
platform_driver_unregister(PLATFORM_DRIVER);
clean0:
#endif
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
debugfs_remove(ehci_debug_root);
ehci_debug_root = NULL;
err_debug:
@@ -1365,7 +1365,7 @@ static void __exit ehci_hcd_cleanup(void)
#ifdef PS3_SYSTEM_BUS_DRIVER
ps3_ehci_driver_unregister(PS3_SYSTEM_BUS_DRIVER);
#endif
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
debugfs_remove(ehci_debug_root);
#endif
clear_bit(USB_EHCI_LOADED, usb_hcds_loaded);
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index cf9f2fb..e321804 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -168,13 +168,13 @@ static void ehci_clear_tt_buffer(struct ehci_hcd *ehci,
struct ehci_qh *qh,
* Note: this routine is never called for Isochronous transfers.
*/
if (urb-dev-tt !usb_pipeint(urb-pipe) !qh-clearing_tt) {
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
struct usb_device *tt = urb-dev-tt-hub;
dev_dbg(tt-dev,
clear tt buffer port %d, a%d ep%d t%08x\n,
urb-dev-ttport, urb-dev-devnum,
usb_pipeendpoint(urb-pipe), token);
-#endif /* DEBUG */
+#endif /* DEBUG || CONFIG_DYNAMIC_DEBUG */
if (!ehci_is_TDI(ehci)
|| urb-dev-tt-hub !=
ehci_to_hcd(ehci)-self.root_hub) {
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 833c35c..85dd24e 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -169,7 +169,7 @@ periodic_usecs (struct ehci_hcd *ehci, unsigned frame,
unsigned uframe)
break;
}
}
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
if (usecs ehci-uframe_periodic_max)
ehci_err (ehci, uframe %d sched overrun: %d
[PATCH 3/4] ehci: remove duplicate debug_async_open() prototype in ehci-dbg.c
This patch removes the duplicate of debug_async_open() prototype following
three lines below the debug_async_open() declaration.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/ehci-dbg.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 5429d26..c2f4489 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -336,7 +336,6 @@ static inline void remove_debug_files (struct ehci_hcd
*bus) { }
static int debug_async_open(struct inode *, struct file *);
static int debug_periodic_open(struct inode *, struct file *);
static int debug_registers_open(struct inode *, struct file *);
-static int debug_async_open(struct inode *, struct file *);
static ssize_t debug_output(struct file*, char __user*, size_t, loff_t*);
static int debug_close(struct inode *, struct file *);
--
1.8.3.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
[PATCH 2/4] ehci: remove debugging statement with ehci statistics in ehci_stop()
This patch removes the ehci statictics information output in ehci_stop() because they do not provide interesting info. At any case, the current statistics can be viewed by reading the 'registers' file in debugfs. Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com --- drivers/usb/host/ehci-hcd.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 73c7299..09a01fb 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -440,14 +440,6 @@ static void ehci_stop (struct usb_hcd *hcd) if (ehci-amd_pll_fix == 1) usb_amd_dev_put(); -#ifdef EHCI_STATS - ehci_dbg(ehci, irq normal %ld err %ld iaa %ld (lost %ld)\n, - ehci-stats.normal, ehci-stats.error, ehci-stats.iaa, - ehci-stats.lost_iaa); - ehci_dbg (ehci, complete %ld unlink %ld\n, - ehci-stats.complete, ehci-stats.unlink); -#endif - dbg_status (ehci, ehci_stop completed, ehci_readl(ehci, ehci-regs-status)); } -- 1.8.3.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
Re: [PATCH v2 1/3] usb: add is_usb_mouse routine and remote wakeup quirk
On Thu, Aug 29, 2013 at 01:07:36AM +0800, Greg Kroah-Hartman wrote: On Wed, Aug 28, 2013 at 04:09:32PM +0800, Huang Rui wrote: On Wed, Aug 28, 2013 at 11:38:28AM +0800, Greg Kroah-Hartman wrote: On Wed, Aug 28, 2013 at 11:13:12AM +0800, Huang Rui wrote: Yes, you're right. This issue is specific to the devices that use Pixart controller, and the Pixart controller is only used by mouse. That's why I filtered for usb mouse. So, to fix one specific controller, you drag in hundreds of thousands of other devices as well? That's not ok. Below answer is our HW guys' feedback. In cover letter, you would see more details. cover letters don't get applied to the kernel changelog, so they don't count for much :) [Q] Why the special devices are only mice? Would high speed devices such as 3G modem or USB Bluetooth adapter trigger this issue? - Current this sensitivity is only confined to devices that use Pixart controllers. This controller is designed for use with LS mouse devices only. We have not observed any other devices failing. There may be a small risk for other devices also but this patch (reset device in resume phase) will cover the cases if required. Then just do this quirk for Pixart controller devices. Don't penalize everyone else. And have you worked with the Pixart company to fix this issue so that future devices they create don't have this issue in it? That's why the USB core doesn't care about the device type, they are all just pipes. So please deal with it on that level, never do something only depending on the type of device plugged into the system. I got it. Do you mean if I want do filter usb devices by usb mouse, I should do it at hid or usbhid level? Neither, you could miss a mouse at those levels as well. Think about userspace control of USB devices, as well as mice that don't use the HID layer (rare, but I've seen it happen.) Again, don't filter for a mouse, filter for the controller that you need to fix. Hint, I bet you didn't fix this bug in Windows this way, right? :) Actually, this issue is found in Windows firstly, and we set a registry to force the driver to reset the device on S3 resume to work around it. Then I reproduce it in Linux OS, and make a same solution. :) So on Windows you do this for all mice devices? Or just Pixart devices? If all mice devices, the Windows developers accepted such a change? Windows has a backdoor of force to reset on resume in regedit like our kernel config. So it doesn't need change codes, just enable the backdoor. I don't know the details, I focus on linux usb driver. Thanks, Rui -- 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: add is_usb_mouse routine and remote wakeup quirk
On Thu, Aug 29, 2013 at 01:07:36AM +0800, Greg Kroah-Hartman wrote: On Wed, Aug 28, 2013 at 04:09:32PM +0800, Huang Rui wrote: On Wed, Aug 28, 2013 at 11:38:28AM +0800, Greg Kroah-Hartman wrote: On Wed, Aug 28, 2013 at 11:13:12AM +0800, Huang Rui wrote: Then just do this quirk for Pixart controller devices. Don't penalize everyone else. And have you worked with the Pixart company to fix this issue so that future devices they create don't have this issue in it? No, it's our host controller issue not Pixart, and we would fix it in future platforms. So I filtered by our southbrige revision. Thanks, Rui -- 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: add is_usb_mouse routine and remote wakeup quirk
On Thu, Aug 29, 2013 at 02:59:50AM +0800, Alan Stern wrote: On Wed, 28 Aug 2013, Greg Kroah-Hartman wrote: On Wed, Aug 28, 2013 at 01:13:46PM -0400, Alan Stern wrote: On Wed, 28 Aug 2013, Greg Kroah-Hartman wrote: [Q] Why the special devices are only mice? Would high speed devices such as 3G modem or USB Bluetooth adapter trigger this issue? - Current this sensitivity is only confined to devices that use Pixart controllers. This controller is designed for use with LS mouse devices only. We have not observed any other devices failing. There may be a small risk for other devices also but this patch (reset device in resume phase) will cover the cases if required. Then just do this quirk for Pixart controller devices. Don't penalize everyone else. Is there any way to detect when a device uses a Pixart controller? I don't see how you could tell. Vendor device id should tell you what you need to know, right? Not necessarily. Manufacturers often get their USB hardware from other suppliers and then overwrite the vendor and product IDs in the firmware with their own. Of course, I don't know if that's true in this case. The guys at AMD should be able to tell easily enough; they have a bunch of these mice to test with. Yes, I tested many mice (lenovo, HP, Logitech and etc.) which triggered this issue, some of them's iManufacture field mark Pixart and some of them's overwrite by Manufacturers. And idVendor and idProduct are different. I don't find a perfect flag to distinguish the devices with Pixart controller. Is there any idea? Thanks, Rui -- 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 v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
On 08/29/2013 04:30 PM, George Cherian wrote: Hi Chanwoo, On 8/29/2013 11:53 AM, Chanwoo Choi wrote: [snip] You should keep following naming stlye. extcon-gpio-usbvid.c is wrong naming style. - extcon-[device name].c - extcon-gpio-usbvid.c - extcon-dra7xx.c or etc. Actually dra7xx is the SoC name and the USB VBUS/ID detection is not specific to SoC. It uses gpios to detect the VBUS/ID change. So i thought it would be better to have generic gpio based VBUS/ID detection rather than making dra7xx specific. Stephen Warren had this opinion with patch v1. Would you guarantee that this driver support all of extcon devices using gpio pin? This driver would guarantee extcon devices using gpio pins for USB VBUS and ID detection. Following is the basic assumption made, correct me if I am wrong. ID pin ground means - USB-HOST is true (this happens when a device is connected to USB port and we act as HOST ) ID pin Float means - USB-HOST is false (this happens when nothing is connected or when we act as a peripheral under a HOST) VBUS ON means - USB is true (this happens when we are connected under a HOST as a peripheral) VBUS OFF means - USB is false ( this happens when we are either disconnected from a HOST or when we are in HOST mode). So normally USB is in peripheral mode is enabled when ID pin is float and VBUS is ON. and USB is in HOST mode when ID pin is ground and VBUS is OFF. In all above cases VBUS is referred to the external VBUS supply from an external HOST. I can't agree. This driver has specific dependency on dra7x SoC. First, vbus_irq_handler() determine USB cable state according to gpio_usbvid-vbus_gpio state. If gpio_usbvid-vbus_gpio value is VBUS_OFF(0), this patch set USB cable state as 'false' But, it include potential problems. Other extcon device using gpio would set USB cable state as 'true' when gpio state is 1. This way has dependency on specific SoC. no this is not SoC specific. VBUS is referred to the external VBUS supply from an external HOST. so if VBUS is zero means its definitely not in connected state. I tested various development board based on Samsung Exynos series SoC. Although some gpio of Exynos series SoC set high state(non zero, 1) as default value, this gpio state could mean off state, disconnected or un-powered state according to gpio. Of course, above explanation about specific gpio don't include all gpios. This is meaning that there is possibility. Second, gpio_usbvid_set_initial_state() dertermine both USB-HOST and USB cable state at same time in 'case ID_DETCT' according to 'gpio_usbvid-id_gpio'. But, I think that other extcon devices would not control both USB-HOST and USB cable state at same time. Other extcon devices would support either USB-HOST or USB cable. The driver has 2 configurations. 1) supports implementations with both VBUS and ID pin are routed via gpio's for swicthing roles(compatible gpio-usb-vid). As you explained about case 1, it is only used on dra7x SoC. Other SoC could not wish to control both USB-HOST and USB cable at same time. 2) supports implementations with only ID pin routed via gpio for swicthing roles(compatible gpio-usb-id). So if you take type as VBUS_ID_DETECT then it is as what you meant. 2) case don't support the detection of USB-HOST cable. Only detect USB cable according to vbus_gpio value. If user wish to detect only USB-HOST cable, should user enter ID_DETECT as ti,gpio-usb-id on DT file? But, id_irq_handler() control both USB-HOST and USB cable at same time. It may not prefer this method. Third, gpio_usbvid_probe() get both gpio_usbvid-id_irq and gpio_usbvid-vbus_irq by using DT helper function. and gpio_usbvid_request_irq() register each interrupt handler(id_irq and vbus_irq) according to DT data. In result, id_irq_handler() would control both USB-HOST and USB cable state. only if type is ID_DETECT id_irq_handler control both USB-HOST and USB cable states if type is VBUS_ID_DETECT then id_irq_handler only controls USB-HOST. I have some confusion. I need additional your explanation. Could this driver register only one interrupt handler either id_irq_handler() or vbus_irq_handler()? or Could this driver register two interrupt handler both id_irq_handler() or vbus_irq_handler()? vbus_irq_handler() would control only USB cable state. Two interrupt handler(id_irq_handler()/vbus_irq_handler()) would control USB cable state according to each gpio state at same time. Also, It include critical problem. No it depends on the configuration as explained above. [snip] + +#define ID_GND0 +#define ID_FLOAT1 +#define VBUS_OFF0 +#define VBUS_ON1 I think you could only use two constant instead of four constant definition. you mean only ID_GND and VBUS_OFF? you could only define two contant (0 and 1) to detect gpio state. okay + + This blank line isn't necessary. +static
Re: [PATCH v1] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Thu, Aug 29, 2013 at 12:05 AM, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 28 Aug 2013, Ming Lei wrote: Think about it this way: Why did you write the USB: EHCI: improve interrupt qh unlink patch in the first place? Essentially the same reason applies to uhci-hcd and ohci-hcd, and they will need similar patches. Right, but drivers which submit URBs from tasklet/workque/... scheduled from complete() may need these patches too, even without giveback URB in complete() change. That's a totally separate issue. This email thread is about changes to ehci-hcd, not bugs in other drivers. Drivers that submit isochronous URBs from tasklets/workqueues/whatever without URB_ISO_ASAP are broken and need to be fixed. You have said the same problem exists on these drivers already without giveback in tasklet patch, so for the problem and solution, there is no difference. What do you mean? We can't fix those other drivers by changing ehci-hcd. It's their fault if they misuse the isochronous API. OK, so submitting isoc URB has to be done from completion handler directly except for the start URBs, otherwise the usage violates isochronous API. Thanks, -- Ming Lei -- 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: [RFC v2] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
Hi Xenia,
thank you for the patch. I tried to reproduce the error with patched 3.10.9
kernel but it seems the kmemleak is indeed gone. Provided I get only these lines
logged which used to be followed by kmemleak findings I believe the original
fixed:
[15885.206032] usb 4-2.1: reset SuperSpeed USB device number 3 using xhci_hcd
[15885.225856] usb 4-2.1: Parent hub missing LPM exit latency info. Power
management will be impacted.
[15885.228445] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with
disabled ep 8803ff81b1c8
[15885.228453] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with
disabled ep 8803ff81b208
[41990.166310] usb 4-2.1: reset SuperSpeed USB device number 9 using xhci_hcd
[41990.187276] usb 4-2.1: Parent hub missing LPM exit latency info. Power
management will be impacted.
[41990.189285] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with
disabled ep 8803ca0381c8
[41990.189287] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with
disabled ep 8803ca038208
[65622.903882] usb 4-2.2: reset SuperSpeed USB device number 10 using xhci_hcd
[65622.927980] usb 4-2.2: Parent hub missing LPM exit latency info. Power
management will be impacted.
[65622.929986] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with
disabled ep 8803ff81b130
[65622.929989] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with
disabled ep 8803ff81b170
Thank you,
Martin
Xenia Ragiadakou wrote:
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.
This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
Differences from version 2:
- fix identation
- initialize udev-bos to null so that check fails if bos is uninitialized
- move bos deallocation inside 'done' and 're_enumerate' paths to ensure
that the deallocation will be performed even if hub_port_init() fails
- remove check (!udev-wusb le16_to_cpu(udev-descriptor.bcdUSB) = 0x0201)
since the checks that follow suffice
drivers/usb/core/hub.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..2455001 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
} /* usb_hub_cleanup() */
static int descriptors_changed(struct usb_device *udev,
- struct usb_device_descriptor *old_device_descriptor)
+ struct usb_device_descriptor *old_device_descriptor,
+ struct usb_host_bos *old_bos)
{
int changed = 0;
unsignedindex;
@@ -4953,6 +4954,16 @@ static int descriptors_changed(struct usb_device *udev,
sizeof(*old_device_descriptor)) != 0)
return 1;
+ if ((old_bos !udev-bos) || (!old_bos udev-bos))
+ return 1;
+ if (udev-bos) {
+ len = udev-bos-desc-wTotalLength;
+ if (len != old_bos-desc-wTotalLength)
+ return 1;
+ if (memcmp(udev-bos-desc, old_bos-desc, le16_to_cpu(len)))
+ return 1;
+ }
+
/* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
@@ -5049,6 +5060,7 @@ static int usb_reset_and_verify_device(struct
usb_device *udev)
struct usb_hub *parent_hub;
struct usb_hcd *hcd = bus_to_hcd(udev-bus);
struct usb_device_descriptordescriptor = udev-descriptor;
+ struct usb_host_bos *bos;
int i, ret = 0;
int port1 = udev-portnum;
@@ -5066,6 +5078,9 @@ static int usb_reset_and_verify_device(struct
usb_device *udev)
}
parent_hub = usb_hub_to_struct_hub(parent_hdev);
+ bos = udev-bos;
+ udev-bos = NULL;
+
/* Disable LPM and LTM while we reset the device and reinstall the alt
* settings. Device-initiated LPM settings, and system exit latency
* settings are cleared when the device is reset, so we have to set
@@ -5099,7 +5114,7 @@ static int usb_reset_and_verify_device(struct
usb_device *udev)
goto re_enumerate;
/* Device might have changed firmware (DFU or similar) */
- if (descriptors_changed(udev, descriptor)) {
+ if
Re: [PATCH v2 0/5] Chipidea Misc patchset
On Thu, Aug 29, 2013 at 10:53:50AM +0300, Alexander Shishkin wrote: Peter Chen peter.c...@freescale.com writes: Changed for v2: - Fixed the build error for patch 5/5 Below are un-queued chipidea patches, some of them were reviewed. I'd like to send 1/5, 3/5 and 4/5 now, have a closer look at 2/5 and queue 5/5 for 3.13 unless you have objections. Thanks, but for me, it is a little long that you have only queued patch one time for each merge window. Now, I have worked on mainline chipidea code, there will be patches often. I'd like you can queue patches more often, no matter your tree or send to Greg. -- Best Regards, Peter Chen -- 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: [RFC 2/3] EHCI: convert the IRQ handler to a tasklet
On Tue, Aug 27, 2013 at 10:39 PM, Alan Stern st...@rowland.harvard.edu wrote: On Tue, 27 Aug 2013, Ming Lei wrote: Yes. A new spinlock would be needed to synchronize the top half and the bottom half. The same spinlock would also be used to avoid scheduling the tasklet when it is already running, like in your implementation. Then every HCD need to copy these kind of implementation... Yes. The pattern would be pretty much the same in each case. So when the above implementation is required in each HCDs change, I am wondering if it is 'pretty easy'. I think it is pretty easy for each HCD. Changing all of them would be a large job. Still not sure it is pretty easy since extra lock things have to be considered: (such as, order between two locks, disabling irq and acquiring lock) Those things would definitely need to be considered. But doing them properly wouldn't require much code. Even though interrupts are masked, the tasklet can still check the EHCI status register to see if new events have occurred while the tasklet was running, as I described above. The tasklet could do this check before returning. Yes, the tasklet can do it but some events(IAA, connection, fata error, ...) are handled with delay. That's true. It's also true that the existing driver handles them with delay. If an IAA, connection, or fatal error event occurs while ehci_work() is running, the event won't be handled until after ehci_work() finishes. If fatal error can be reported early, ehci_work() can scan and handle qh/iTD/siTD easily(quickly) since ehci state is checked at many places. I agree that masking interrupts while the bottom half runs would increase this delay, but I don't think it matters much. For example, consider disconnect events. Leaving interrupts masked might delay the event report for 10 ms (probably a lot less). But compare that to what happens when a device disconnects from an external hub -- hubs are polled for port status changes with an interval of 128 ms! By comparison, a 10-ms delay for the root hub is unimportant. More things done in top half, the change will become more complicated since more synchronizations need to consider. Not at all. ehci-lock will synchronize things perfectly well. It isn't good to hold ehci-lock in ehci_irq(), otherwise, ehci_irq has to spin on this lock when is held in tasklet. That was my point. We are in agreement that it is bad for the top half and the bottom half both to acquire ehci-lock. Your solution is to put everything but the givebacks (which don't need the lock) in the top half, whereas my solution is to put everything in the bottom half. But now you're complaining because that means some of the work won't get done in the top half! You can't have it both ways. If the lock isn't used in both places then the top half has to do either all or none of the work. It can be done with extra complication introduced, but that is we don't want to see. I prefer to enabling EHCI interrupt during tasklet handling. What for? It seems likely that the top half would have to acquire the So we can respond some events(IAA, fatal error, connection change) quickly. For example, when fatal error happened, ehci transfer descriptors might be written incorrectly by host, so it is better to let tasklet see it asap and handle transfer effectively(maybe correctly). You haven't thought this through. _Every_ QH and TD written by the host controller eventually gets scanned by ehci-hcd. This is true whether or not a fatal error occurs. The existing driver doesn't do anything special about incorrect TDs, and it never has. So why should we have to start doing it now? Similar considerations apply to connect and disconnect handling. Suppose a connect event occurs while ehci_work() is running. Suppose that the event can be handled without acquiring ehci-lock, so that the event is reported to usbcore immediately. What would happen next? Answer: Before doing anything else, khubd would issue a Get-Port-Status request, which acquires ehci-lock! Thus the response to the connect event would end up being delayed anyway. The one disadvantage to leaving interrupts masked while the tasklet runs is that new events won't be detected until all the existing givebacks are finished. In the old driver, new events could be detected as soon as the next giveback occurred, because the lock would be released then. In the end, this comes down to a decision about priorities. Do you want to give higher priority to new events or to givebacks? Overall, I don't think it matters. If possible, the events should be put higher priority, like events handling in ehci_irq() now, but as you said, it may not matter. So from the discussion, both approaches are pretty much same I have no objection if anyone plan to implement this approach. Thanks, -- Ming Lei -- To unsubscribe from this list: send the
Re: [PATCH v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
On 8/29/2013 4:07 PM, Chanwoo Choi wrote: On 08/29/2013 04:30 PM, George Cherian wrote: Hi Chanwoo, On 8/29/2013 11:53 AM, Chanwoo Choi wrote: [snip] You should keep following naming stlye. extcon-gpio-usbvid.c is wrong naming style. - extcon-[device name].c - extcon-gpio-usbvid.c - extcon-dra7xx.c or etc. Actually dra7xx is the SoC name and the USB VBUS/ID detection is not specific to SoC. It uses gpios to detect the VBUS/ID change. So i thought it would be better to have generic gpio based VBUS/ID detection rather than making dra7xx specific. Stephen Warren had this opinion with patch v1. Would you guarantee that this driver support all of extcon devices using gpio pin? This driver would guarantee extcon devices using gpio pins for USB VBUS and ID detection. Following is the basic assumption made, correct me if I am wrong. ID pin ground means - USB-HOST is true (this happens when a device is connected to USB port and we act as HOST ) ID pin Float means - USB-HOST is false (this happens when nothing is connected or when we act as a peripheral under a HOST) VBUS ON means - USB is true (this happens when we are connected under a HOST as a peripheral) VBUS OFF means - USB is false ( this happens when we are either disconnected from a HOST or when we are in HOST mode). So normally USB is in peripheral mode is enabled when ID pin is float and VBUS is ON. and USB is in HOST mode when ID pin is ground and VBUS is OFF. In all above cases VBUS is referred to the external VBUS supply from an external HOST. I can't agree. This driver has specific dependency on dra7x SoC. First, vbus_irq_handler() determine USB cable state according to gpio_usbvid-vbus_gpio state. If gpio_usbvid-vbus_gpio value is VBUS_OFF(0), this patch set USB cable state as 'false' But, it include potential problems. Other extcon device using gpio would set USB cable state as 'true' when gpio state is 1. This way has dependency on specific SoC. no this is not SoC specific. VBUS is referred to the external VBUS supply from an external HOST. so if VBUS is zero means its definitely not in connected state. I tested various development board based on Samsung Exynos series SoC. Although some gpio of Exynos series SoC set high state(non zero, 1) as default value, this gpio state could mean off state, disconnected or un-powered state according to gpio. Of course, above explanation about specific gpio don't include all gpios. This is meaning that there is possibility. okay then how about adding a flag for inverted logic too. something like this if(of_property_read_bool(np,inverted_gpio)) gpio_usbvid-gpio_inv = 1; And always check on this before deciding? Second, gpio_usbvid_set_initial_state() dertermine both USB-HOST and USB cable state at same time in 'case ID_DETCT' according to 'gpio_usbvid-id_gpio'. But, I think that other extcon devices would not control both USB-HOST and USB cable state at same time. Other extcon devices would support either USB-HOST or USB cable. The driver has 2 configurations. 1) supports implementations with both VBUS and ID pin are routed via gpio's for swicthing roles(compatible gpio-usb-vid). As you explained about case 1, it is only used on dra7x SoC. No gpio-usb-id is used in dra7xx. dra7xx has got only ID pin routed via gpio. Other SoC could not wish to control both USB-HOST and USB cable at same time. 2) supports implementations with only ID pin routed via gpio for swicthing roles(compatible gpio-usb-id). So if you take type as VBUS_ID_DETECT then it is as what you meant. I think i didnt explain it properly last time. In perfect world you will have both VBUS and ID pin routed via gpios for eg: VBUS via gpio2 and ID via gpio3. if this is the case then we have to use compatible gpio-usb-vid where in 2 gpios will be used 2 irq handlers will be installed and it will control both USB-HOST and USB cables. In this case its possible to have 3 states USB-HOST (true), USB(true) and both false. Now in case of dra7xx the board designers chose not to route the VBUS to gpio and only ID pin is routed. But still we need to differentiate USB-HOST and USB cables In such cases we use compatible gpio-usb-id where in 1 gpio will be used 1 irq handler is installed and it will control both USB-HOST and USB cables. In this case its possible to have only 2 states USB-HOST (true) or USB(true). Now there could be a 3rd scenario were in only VBUS is routed via gpio, that we would not support since we cant assume the value of ID pin and configure the UTMI is not proper. So I have mentioned even in documentation that ID pin is mandatory. We can always assume role depending on ID pin. 2) case don't support the detection of USB-HOST cable. Only detect USB cable according to vbus_gpio value. If user wish to detect only USB-HOST cable, should user enter ID_DETECT as ti,gpio-usb-id on DT file? But, id_irq_handler() control both USB-HOST and USB cable at same time. It may not
Re: [RFC v2] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On 08/29/2013 02:21 PM, Martin MOKREJŠ wrote: Hi Xenia, thank you for the patch. I tried to reproduce the error with patched 3.10.9 kernel but it seems the kmemleak is indeed gone. Provided I get only these lines logged which used to be followed by kmemleak findings I believe the original fixed: [15885.206032] usb 4-2.1: reset SuperSpeed USB device number 3 using xhci_hcd [15885.225856] usb 4-2.1: Parent hub missing LPM exit latency info. Power management will be impacted. [15885.228445] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ff81b1c8 [15885.228453] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ff81b208 [41990.166310] usb 4-2.1: reset SuperSpeed USB device number 9 using xhci_hcd [41990.187276] usb 4-2.1: Parent hub missing LPM exit latency info. Power management will be impacted. [41990.189285] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ca0381c8 [41990.189287] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ca038208 [65622.903882] usb 4-2.2: reset SuperSpeed USB device number 10 using xhci_hcd [65622.927980] usb 4-2.2: Parent hub missing LPM exit latency info. Power management will be impacted. [65622.929986] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ff81b130 [65622.929989] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ff81b170 Thank you, Martin Hi Martin, Thank you for testing it. By the way, if you find something else that needs to be fixed and you don't have time, let me know. I learn a lot when i try to fix something and by my mistakes. regards, ksenia -- 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 v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
On 08/29/2013 08:48 PM, George Cherian wrote: On 8/29/2013 4:07 PM, Chanwoo Choi wrote: On 08/29/2013 04:30 PM, George Cherian wrote: Hi Chanwoo, On 8/29/2013 11:53 AM, Chanwoo Choi wrote: [snip] You should keep following naming stlye. extcon-gpio-usbvid.c is wrong naming style. - extcon-[device name].c - extcon-gpio-usbvid.c - extcon-dra7xx.c or etc. Actually dra7xx is the SoC name and the USB VBUS/ID detection is not specific to SoC. It uses gpios to detect the VBUS/ID change. So i thought it would be better to have generic gpio based VBUS/ID detection rather than making dra7xx specific. Stephen Warren had this opinion with patch v1. Would you guarantee that this driver support all of extcon devices using gpio pin? This driver would guarantee extcon devices using gpio pins for USB VBUS and ID detection. Following is the basic assumption made, correct me if I am wrong. ID pin ground means - USB-HOST is true (this happens when a device is connected to USB port and we act as HOST ) ID pin Float means - USB-HOST is false (this happens when nothing is connected or when we act as a peripheral under a HOST) VBUS ON means - USB is true (this happens when we are connected under a HOST as a peripheral) VBUS OFF means - USB is false ( this happens when we are either disconnected from a HOST or when we are in HOST mode). So normally USB is in peripheral mode is enabled when ID pin is float and VBUS is ON. and USB is in HOST mode when ID pin is ground and VBUS is OFF. In all above cases VBUS is referred to the external VBUS supply from an external HOST. I can't agree. This driver has specific dependency on dra7x SoC. First, vbus_irq_handler() determine USB cable state according to gpio_usbvid-vbus_gpio state. If gpio_usbvid-vbus_gpio value is VBUS_OFF(0), this patch set USB cable state as 'false' But, it include potential problems. Other extcon device using gpio would set USB cable state as 'true' when gpio state is 1. This way has dependency on specific SoC. no this is not SoC specific. VBUS is referred to the external VBUS supply from an external HOST. so if VBUS is zero means its definitely not in connected state. I tested various development board based on Samsung Exynos series SoC. Although some gpio of Exynos series SoC set high state(non zero, 1) as default value, this gpio state could mean off state, disconnected or un-powered state according to gpio. Of course, above explanation about specific gpio don't include all gpios. This is meaning that there is possibility. okay then how about adding a flag for inverted logic too. something like this if(of_property_read_bool(np,inverted_gpio)) gpio_usbvid-gpio_inv = 1; And always check on this before deciding? Second, gpio_usbvid_set_initial_state() dertermine both USB-HOST and USB cable state at same time in 'case ID_DETCT' according to 'gpio_usbvid-id_gpio'. But, I think that other extcon devices would not control both USB-HOST and USB cable state at same time. Other extcon devices would support either USB-HOST or USB cable. The driver has 2 configurations. 1) supports implementations with both VBUS and ID pin are routed via gpio's for swicthing roles(compatible gpio-usb-vid). As you explained about case 1, it is only used on dra7x SoC. No gpio-usb-id is used in dra7xx. dra7xx has got only ID pin routed via gpio. Other SoC could not wish to control both USB-HOST and USB cable at same time. I need your answer about above my opinion for other SoC. 2) supports implementations with only ID pin routed via gpio for swicthing roles(compatible gpio-usb-id). So if you take type as VBUS_ID_DETECT then it is as what you meant. I think i didnt explain it properly last time. In perfect world you will have both VBUS and ID pin routed via gpios for eg: VBUS via gpio2 and ID via gpio3. if this is the case then we have to use compatible gpio-usb-vid where in 2 gpios will be used 2 irq handlers will be installed and it will control both USB-HOST and USB cables. In this case its possible to have 3 states USB-HOST (true), USB(true) and both false. Now in case of dra7xx the board designers chose not to route the VBUS to gpio and only ID pin is routed. But still we need to differentiate USB-HOST and USB cables In such cases we use compatible gpio-usb-id where in 1 gpio will be used 1 irq handler is installed and it will control both USB-HOST and USB cables. In this case its possible to have only 2 states USB-HOST (true) or USB(true). Now there could be a 3rd scenario were in only VBUS is routed via gpio, that we would not support since we cant assume the value of ID pin and configure the UTMI is not proper. So I have mentioned even in documentation that ID pin is mandatory. We can always assume role depending on ID pin. 2) case don't support the detection of USB-HOST cable. Only detect USB cable according to vbus_gpio value. If user
Include parent hub number in current warning message Parent hub missing LPM exit latency info
Hi Xenia, thank you, how about inclusion of the parent hub number in the following message (as of now): Parent hub missing LPM exit latency info. Power management will be impacted. I find it awkward to later on run manually lspci/lsusb to find what is the parent. I think I do NOT get these messages when I have pcie_aspm=off whereas when it is on I get the warning. Why PCIe powersaving affects how USB end devices will be put to sleep I don't know. But that will be the next step to look into. First the warning message. And maybe it could be improve even further to include other relevant capabilities of the parent present/missing. ;-) Here are bits from my dmesg: [1.996290] pci :00:1a.0: calling quirk_usb_early_handoff+0x0/0x760 [2.118464] pci :00:1d.0: calling quirk_usb_early_handoff+0x0/0x760 [2.238447] pci :0b:00.0: calling quirk_usb_early_handoff+0x0/0x760 [4.150572] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver [4.151271] ehci-pci: EHCI PCI platform driver [4.153595] ehci-pci :00:1a.0: enabling bus mastering [4.153602] ehci-pci :00:1a.0: setting latency timer to 64 [4.153634] ehci-pci :00:1a.0: EHCI Host Controller [4.154427] ehci-pci :00:1a.0: new USB bus registered, assigned bus number 1 [4.155159] ehci-pci :00:1a.0: debug port 2 [4.159824] ehci-pci :00:1a.0: cache line size of 64 is not supported [4.159963] ehci-pci :00:1a.0: irq 16, io mem 0xf7f08000 [4.179142] ehci-pci :00:1a.0: USB 2.0 started, EHCI 1.00 [4.180144] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002 [4.180817] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [4.181482] usb usb1: Product: EHCI Host Controller [4.182147] usb usb1: Manufacturer: Linux 3.10.9-default-pciehp ehci_hcd [4.182819] usb usb1: SerialNumber: :00:1a.0 [4.184904] hub 1-0:1.0: USB hub found [4.185628] hub 1-0:1.0: 2 ports detected [4.189263] ehci-pci :00:1d.0: enabling bus mastering [4.189269] ehci-pci :00:1d.0: setting latency timer to 64 [4.189280] ehci-pci :00:1d.0: EHCI Host Controller [4.189951] ehci-pci :00:1d.0: new USB bus registered, assigned bus number 2 [4.190604] ehci-pci :00:1d.0: debug port 2 [4.195178] ehci-pci :00:1d.0: cache line size of 64 is not supported [4.195283] ehci-pci :00:1d.0: irq 23, io mem 0xf7f07000 [4.209148] ehci-pci :00:1d.0: USB 2.0 started, EHCI 1.00 [4.210029] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002 [4.210668] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [4.211301] usb usb2: Product: EHCI Host Controller [4.211921] usb usb2: Manufacturer: Linux 3.10.9-default-pciehp ehci_hcd [4.212543] usb usb2: SerialNumber: :00:1d.0 [4.214248] hub 2-0:1.0: USB hub found [4.214897] hub 2-0:1.0: 2 ports detected [4.217507] xhci_hcd :0b:00.0: enabling bus mastering [4.217518] xhci_hcd :0b:00.0: xHCI Host Controller [4.218187] xhci_hcd :0b:00.0: new USB bus registered, assigned bus number 3 [4.219103] xhci_hcd :0b:00.0: enabling Mem-Wr-Inval [4.219252] xhci_hcd :0b:00.0: irq 45 for MSI/MSI-X [4.219298] xhci_hcd :0b:00.0: irq 46 for MSI/MSI-X [4.219928] usb usb3: New USB device found, idVendor=1d6b, idProduct=0002 [4.220573] usb usb3: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [4.221227] usb usb3: Product: xHCI Host Controller [4.221863] usb usb3: Manufacturer: Linux 3.10.9-default-pciehp xhci_hcd [4.222497] usb usb3: SerialNumber: :0b:00.0 [4.223881] xHCI xhci_add_endpoint called for root hub [4.223883] xHCI xhci_check_bandwidth called for root hub [4.224280] hub 3-0:1.0: USB hub found [4.225002] hub 3-0:1.0: 2 ports detected [4.226582] xhci_hcd :0b:00.0: xHCI Host Controller [4.227264] xhci_hcd :0b:00.0: new USB bus registered, assigned bus number 4 [4.228257] usb usb4: New USB device found, idVendor=1d6b, idProduct=0003 [4.228906] usb usb4: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [4.229571] usb usb4: Product: xHCI Host Controller [4.230197] usb usb4: Manufacturer: Linux 3.10.9-default-pciehp xhci_hcd [4.230823] usb usb4: SerialNumber: :0b:00.0 [4.232302] xHCI xhci_add_endpoint called for root hub [4.232304] xHCI xhci_check_bandwidth called for root hub [4.232725] hub 4-0:1.0: USB hub found [4.233395] hub 4-0:1.0: 2 ports detected [4.502407] usb 1-1: new high-speed USB device number 2 using ehci-pci [4.651502] usb 1-1: New USB device found, idVendor=8087, idProduct=0024 [4.653427] usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [4.660393] hub 1-1:1.0: USB hub found [4.662666] hub 1-1:1.0: 6 ports detected [4.680892] sd 0:0:0:0: [sda] Attached SCSI disk [4.790142] usb 2-1: new high-speed USB device
Re: Include parent hub number in current warning message Parent hub missing LPM exit latency info
Actually, there is some new bug I haven't seen before (this is 3.10.9 kernel). First of all, I see my TI XHCI controller does not use MSI-X anymore, will have to check my .config why is it so. Second, it should have IRQ 45 and 46 according to dmesg. But lspci reports IRQ 16 is used by TI XHCI controller. Funny! I would say this is linux-pci issue but provided XHCI_HCD is special and manages interrupts somewhat on its own you may look into that first before we ask linux-pci developers. Martin MOKREJŠ wrote: Hi Xenia, thank you, how about inclusion of the parent hub number in the following message (as of now): Parent hub missing LPM exit latency info. Power management will be impacted. I find it awkward to later on run manually lspci/lsusb to find what is the parent. I think I do NOT get these messages when I have pcie_aspm=off whereas when it is on I get the warning. Why PCIe powersaving affects how USB end devices will be put to sleep I don't know. But that will be the next step to look into. First the warning message. And maybe it could be improve even further to include other relevant capabilities of the parent present/missing. ;-) Here are bits from my dmesg: [4.159824] ehci-pci :00:1a.0: cache line size of 64 is not supported [4.159963] ehci-pci :00:1a.0: irq 16, io mem 0xf7f08000 [4.179142] ehci-pci :00:1a.0: USB 2.0 started, EHCI 1.00 [4.195178] ehci-pci :00:1d.0: cache line size of 64 is not supported [4.195283] ehci-pci :00:1d.0: irq 23, io mem 0xf7f07000 [4.209148] ehci-pci :00:1d.0: USB 2.0 started, EHCI 1.00 [4.219252] xhci_hcd :0b:00.0: irq 45 for MSI/MSI-X [4.219298] xhci_hcd :0b:00.0: irq 46 for MSI/MSI-X [5.100550] usb 3-2: new high-speed USB device number 2 using xhci_hcd [5.123995] usb 3-2: New USB device found, idVendor=2109, idProduct=0811 [5.125864] usb 3-2: New USB device strings: Mfr=0, Product=1, SerialNumber=0 [5.127705] usb 3-2: Product: USB2.0 Hub [5.159101] hub 3-2:1.0: USB hub found [5.161643] hub 3-2:1.0: 4 ports detected [5.291321] usb 4-2: new SuperSpeed USB device number 2 using xhci_hcd [5.313219] usb 4-2: Parent hub missing LPM exit latency info. Power management will be impacted. [5.319428] usb 4-2: New USB device found, idVendor=2109, idProduct=0811 [5.321417] usb 4-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [5.323353] usb 4-2: Product: 4-Port USB 3.0 Hub [5.325272] usb 4-2: Manufacturer: VIA Labs, Inc. 00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #2 (rev 05) (prog-if 20 [EHCI]) Subsystem: Dell Device 04b3 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0 Interrupt: pin A routed to IRQ 16 00:1d.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #1 (rev 05) (prog-if 20 [EHCI]) Subsystem: Dell Device 04b3 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0 Interrupt: pin A routed to IRQ 23 0b:00.0 USB controller: Texas Instruments TUSB73x0 SuperSpeed USB 3.0 xHCI Host Controller (rev 02) (prog-if 30 [XHCI]) Subsystem: Dell Device 04b3 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0, Cache Line Size: 64 bytes Interrupt: pin A routed to IRQ 16 # cat /proc/interrupts CPU0 0: 22 IO-APIC-edge timer 1: 16 IO-APIC-edge i8042 8: 55 IO-APIC-edge rtc0 9: 0 IO-APIC-fasteoi acpi 12:5090314 IO-APIC-edge i8042 16: 66 IO-APIC-fasteoi ehci_hcd:usb1 23: 28835 IO-APIC-fasteoi ehci_hcd:usb2 40: 0 PCI-MSI-edge pciehp 41: 15985 PCI-MSI-edge i915 42: 14 PCI-MSI-edge mei_me 43: 227029 PCI-MSI-edge ahci 44: 208160 PCI-MSI-edge enp5s0 45: 729889 PCI-MSI-edge xhci_hcd 46: 0 PCI-MSI-edge xhci_hcd 47:940 PCI-MSI-edge snd_hda_intel 48: 1 PCI-MSI-edge iwlwifi NMI: 21635 Non-maskable interrupts LOC:7545378 Local timer interrupts SPU: 0 Spurious interrupts PMI: 21635 Performance monitoring interrupts IWI:1583914 IRQ work interrupts RTR: 0 APIC ICR read retries RES:
Re: [PATCH v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
Hi Chanwoo,
On 8/29/2013 5:42 PM, Chanwoo Choi wrote:
[big snip ]
I tested various development board based on Samsung Exynos series SoC.
Although some gpio of Exynos series SoC set high state(non zero, 1) as default
value,
this gpio state could mean off state, disconnected or un-powered state
according to gpio.
Of course, above explanation about specific gpio don't include all gpios.
This is meaning that there is possibility.
okay then how about adding a flag for inverted logic too. something like this
if(of_property_read_bool(np,inverted_gpio))
gpio_usbvid-gpio_inv = 1;
And always check on this before deciding?
Is this fine ?
Second,
gpio_usbvid_set_initial_state() dertermine both USB-HOST and USB cable
state at same time
in 'case ID_DETCT' according to 'gpio_usbvid-id_gpio'. But, I think that other
extcon devices
would not control both USB-HOST and USB cable state at same time.
Other extcon devices would support either USB-HOST or USB cable.
The driver has 2 configurations.
1) supports implementations with both VBUS and ID pin are routed via gpio's for
swicthing roles(compatible gpio-usb-vid).
As you explained about case 1, it is only used on dra7x SoC.
No gpio-usb-id is used in dra7xx. dra7xx has got only ID pin routed via gpio.
Other SoC could not wish to control both USB-HOST and USB cable at same
time.
I could'nt actually parse this. You meant since the id_irq_handler
handles both USB and USB-HOST cable
its not proper?
I need your answer about above my opinion for other SoC.
So how about this, I have removed the dra7x specific stuffs (gpio-usb-id)
static void gpio_usbvid_set_initial_state(struct gpio_usbvid *gpio_usbvid)
{
int id_current, vbus_current;
id_current = gpio_get_value_cansleep(gpio_usbvid-id_gpio);
if (!!id_current == ID_FLOAT)
extcon_set_cable_state(gpio_usbvid-edev, USB-HOST, false);
else
extcon_set_cable_state(gpio_usbvid-edev, USB-HOST, true);
vbus_current = gpio_get_value_cansleep(gpio_usbvid-vbus_gpio);
if (!!vbus_current == VBUS_ON)
extcon_set_cable_state(gpio_usbvid-edev, USB, true);
else
extcon_set_cable_state(gpio_usbvid-edev, USB, false);
}
and the irq handlers like this
static irqreturn_t id_irq_handler(int irq, void *data)
{
struct gpio_usbvid *gpio_usbvid = (struct gpio_usbvid *)data;
int id_current;
id_current = gpio_get_value_cansleep(gpio_usbvid-id_gpio);
if (id_current == ID_GND)
extcon_set_cable_state(gpio_usbvid-edev, USB-HOST,
true);
else
extcon_set_cable_state(gpio_usbvid-edev, USB-HOST,
false);
return IRQ_HANDLED;
}
static irqreturn_t vbus_irq_handler(int irq, void *data)
{
struct gpio_usbvid *gpio_usbvid = (struct gpio_usbvid *)data;
int vbus_current;
vbus_current = gpio_get_value_cansleep(gpio_usbvid-vbus_gpio);
if (vbus_current == VBUS_OFF)
extcon_set_cable_state(gpio_usbvid-edev, USB, false);
else
extcon_set_cable_state(gpio_usbvid-edev, USB, true);
return IRQ_HANDLED;
}
[snip]
I have some confusion. I need additional your explanation.
Could this driver register only one interrupt handler either id_irq_handler()
or vbus_irq_handler()?
If compatible == ID_DETECT, only one handler -- id_irq_handler() and it will
handle both USB and USB-HOST
or
Could this driver register two interrupt handler both id_irq_handler() or
vbus_irq_handler()?
If compatible == VBUS_ID_DETECT, 2 handlers -- id_irq_handler() will handle
USB-HOST and vbus_irq_handler will handle USB.
As you mentioned, we cannot only control either USB or USB-HOST cable on this
extcon driver.
This extcon driver is only suitable dra7x SoC.
Do you still feel its dra7x specific if i modify it as above?
Because id_irq_handler() control both USB-HOST and USB cable at same time,
you need this setting order between USB-HOST and USB cable.
yes
I think that the setting order between cables isn't general. It is specific
method for dra7x SoC.
So if i remove that part then?
Did you think that SoC should always connect either USB-HOST cable or USB
cable?
No, even if a physical cable is not connected it should default to either
USB-HOST or USB
It isn't general.
If physical cable isn't connected to extcon device, the state both USB-HOST and
USB cable
should certainly be zero. Because The extcon consumer driver could set proper
operation
according to cable state.
okay
I don't know this case except for dra7x SoC. So, I think it has dependency on
specific SoC.
I need your answer about above my opinion.
Hope i could answer you :-)
and can't agree as generic extcon gpio driver.
--
-George
--
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: Memory synchronization vs. interrupt handlers
On Wed, 28 Aug 2013, H. Peter Anvin wrote: On 08/28/2013 12:16 PM, Alan Stern wrote: Russell, Peter, and Ingo: Can you folks enlighten us regarding this issue for some common architectures? On x86, IRET is a serializing instruction; it guarantees hard serialization of absolutely everything. That answers half of the question. What about the other half? Does the CPU automatically serialize everything when it takes an interrupt? I would expect architectures that have weak memory ordering to put appropriate barriers in the IRQ entry/exit code. Then would it be acceptable to mention this in the memory-barriers.txt file? 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
USB-HID
Hi, I have a custom device, which is usb-hid. But, when i plug it in the system, it is not recognized as a usb-hid device. I have put prints on the usbhid_probe() function, to confirm that, but there is no prints in the dmesg, which confirms that this is not recognized as a usb-hid device. NOTE: The same device gets detected as a usb-hid device in windows. Is there any simple code, which can list all the usb-hid devices that are connected to the pc? regards, Kasi. -- 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: [RFC 3/3] EHCI: handle late isochronous submissions
On Thu, 29 Aug 2013, Ming Lei wrote:
On Wed, 28 Aug 2013 12:23:31 -0400 (EDT)
Alan Stern st...@rowland.harvard.edu wrote:
You must never alter ehci-last_iso_frame like this. It violates the
driver's invariants for time to run backward. After all, there may
already be other TDs scheduled for the frames you are about to scan
again; they mustn't get scanned until the frame pointer has wrapped
around.
This last problem in particular means your proposal can't be accepted.
On Thu, Aug 29, 2013 at 8:43 AM, Ming Lei tom.leim...@gmail.com wrote:
No, no other TDs scheduled for the frames since the queue is empty
when iso_stream_rebase is running.
The above isn't true, but no rescan on other TDs introduced:
- suppose URB0 is scheduled via stream0 when underrun without ISO_ASAP
- iso_stream_rebase() find that stream0-next_uframe is behind
ehci-last_iso_frame but URB0 isn't completed before
ehci-last_iso_frame, then adjust it as stream0-next_uframe 3,
and record its old value as ehci-last_iso_frame'.
- when next ehci irq comes, scan_isoc() scans from stream0-next_uframe
to 'now', and the extra scan introduced by iso_stream_rebase() is from
stream0-next_uframe to ehci-last_iso_frame' - 1, during this period,
only TDs belonged to undrun URBs without ISO_ASAP will scanned, and no
other TDs scheduled in these frames at all.(ehci-last_iso_frame doesn't
affect schedule, only decides start point of the next scan)
ehci-last_iso_frame does indeed affect the schedule:
base = ehci-last_iso_frame 3;
next = (next - base) (mod - 1);
start = (stream-next_uframe - base) (mod - 1);
/* Is the schedule already full? */
if (unlikely(start period)) {
ehci_dbg(ehci, iso sched full %p (%u-%u %u mod
%u)\n,
urb, stream-next_uframe, base,
period, mod);
status = -ENOSPC;
goto fail;
}
Note that start depends on base, which is derived from
ehci-last_iso_frame. If you decrease ehci-last_iso_frame, earlier
calculations using the larger value will now be incorrect.
So no sort of run 'backward' things, and URBs are always completed in
time order, aren't they?
No. Here's an example. I will exaggerate some of the values to help
make the point clear.
An SMI blocks interrupts for 100 ms. When the SMI ends, we
give back all the isochronous URBs on endpoints A and B. The
frame counter is now equal to 600, but the URBs we gave back
were scheduled to end in frame 520. The endpoint queues are
empty.
The completion handler for ep A submits an URB lasting for 1000
frames (11 packets with interval = 100); it is scheduled to
start in frame 620 and its last packet is scheduled for frame
596 after wrapping around.
The completion hanlder for ep B then runs, and it submits an
URB lasting 95 frames; it is scheduled to start in frame 521
and its last packet is scheduled for frame 615. Since this is
larger than the current frame counter, you rebase
ehci-last_iso_frame back to 520 or earlier.
Now the next time an interrupt occurs, the driver will scan
the last TD for ep A in frame 596. But that TD shouldn't be
scanned until 1020 ms have elapsed.
Like I said, this is exaggerated and probably will never happen. But
it is legal according to the API and therefore we have to allow it.
Besides, at this point it's not that much extra work for you to add the
sched-first_packet field. With that in place, you don't need to
rebase last_iso_frame, which removes the problem. It also allows the
driver to be improved by not scheduling any TDs that lie between
last_iso_frame and the current frame counter.
Of course, once you do this, your patch will start to look an awful lot
like mine.
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: [alsa-devel] Buffer size for ALSA USB PCM audio
On Thu, 29 Aug 2013, James Stone wrote: On Wed, Aug 28, 2013 at 7:46 PM, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 28 Aug 2013, Clemens Ladisch wrote: Sorry, what I said applies more to explicit sync endpoints. When using implicit sync, a playback URB is submitted for each completed capture URB, with the number of samples per packet identical to the corresponding capture packet, so the parameters must be identical. Got it. Below is an updated patch. James, the problem you encountered was most likely a result of the faulty treatment of capture endpoints that Clemens pointed out. It was quite obvious in the usbmon traces that the unpatched driver used 8 packets per URB whereas the patched driver used 22. This updated patch should fix that problem. Works fine. With this, it is possible to get clear playback at 64 frames/period too. With vanilla 3.10.9, there was some glitchy distortion to the sound at that latency, so this seems to be an improvement. That's good. What happens if you push frames/period even lower? Daniel and Eldad, more testing of the most recent proposed patch would be welcome. 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 1/4] ehci: remove ehci_vdbg() verbose debugging statements
On Thu, 29 Aug 2013, Xenia Ragiadakou wrote: This patch removes ehci_vdbg debugging statements from EHCI host controller driver because they produce too much information, lowering the signal to noise ratio when debugging, and because they are not used anymore. Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com Signed-off-by: Alan Stern st...@rowland.harvard.edu The same goes for patches 2, 3, and 4. 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 v1] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Thu, 29 Aug 2013, Ming Lei wrote: You have said the same problem exists on these drivers already without giveback in tasklet patch, so for the problem and solution, there is no difference. What do you mean? We can't fix those other drivers by changing ehci-hcd. It's their fault if they misuse the isochronous API. OK, so submitting isoc URB has to be done from completion handler directly except for the start URBs, otherwise the usage violates isochronous API. It doesn't _violate_ the API. But it also doesn't enjoy the benefit that the API promises if URBs are submitted by the completion handler. 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: Include parent hub number in current warning message Parent hub missing LPM exit latency info
On Thu, 29 Aug 2013, Martin MOKREJŠ wrote: Hi Xenia, thank you, how about inclusion of the parent hub number in the following message (as of now): Parent hub missing LPM exit latency info. Power management will be impacted. I find it awkward to later on run manually lspci/lsusb to find what is the parent. You don't need to run those programs. I think I do NOT get these messages when I have pcie_aspm=off whereas when it is on I get the warning. Why PCIe powersaving affects how USB end devices will be put to sleep I don't know. But that will be the next step to look into. First the warning message. And maybe it could be improve even further to include other relevant capabilities of the parent present/missing. ;-) Here are bits from my dmesg: [5.291321] usb 4-2: new SuperSpeed USB device number 2 using xhci_hcd [5.313219] usb 4-2: Parent hub missing LPM exit latency info. Power management will be impacted. Since this device is 4-2, the parent hub is usb4. 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: USB-HID
On Thu, 29 Aug 2013, kasi viswanathan wrote: Hi, I have a custom device, which is usb-hid. But, when i plug it in the system, it is not recognized as a usb-hid device. I have put prints on the usbhid_probe() function, to confirm that, but there is no prints in the dmesg, which confirms that this is not recognized as a usb-hid device. This means that the device's descriptors are bad. What does lsusb -v show? NOTE: The same device gets detected as a usb-hid device in windows. Is there any simple code, which can list all the usb-hid devices that are connected to the pc? How will that help? 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: stv090x vs stv0900 support
Hi, Can someone help me, please? Regards, Kishore. -Original Message- From: Krishna Kishore Sent: Wednesday, August 28, 2013 6:05 PM To: Chris Lee; Mariusz Bialonczyk Cc: Linux Media Mailing List; linux-usb@vger.kernel.org Subject: RE: stv090x vs stv0900 support Hi, When read_mac_address is called, khubd timed out error is seen. It looks like USB control msgs are not successfully being sent. Can someone please check attached logs and help me? Regards, Kishore. From: Krishna Kishore Sent: Tuesday, August 27, 2013 12:49 PM To: Chris Lee; Mariusz Bialonczyk Cc: Linux Media Mailing List Subject: RE: stv090x vs stv0900 support Hi Chris, Does it help me in getting through the problem I am facing? On Desktop (Ubuntu 12.04), 7500 does not work. But, on desktop Ubuntu 13.04 it works fine. Since it is working well with 13.04, I tried with 3.8.x kernel on my board. It does not work. So, I can see that kernel version does not matter. I assume you have 7500 DVB receiver. Can you please try this device with Ubuntu 12.04 and 13.04 and see the difference in the behavior? If it helps you, I can send you logs of 12.04 and 13.04. Thanks in advance. Regards, Kishore. -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media-ow...@vger.kernel.org] On Behalf Of Chris Lee Sent: Friday, August 16, 2013 6:21 PM To: Mariusz Bialonczyk Cc: Linux Media Mailing List Subject: Re: stv090x vs stv0900 support Ive found a few bugs in stv090x that I want to get ironed out 100% before I submit the patch, dvb-s2 8psk fec2/3 for example has a slightly higher ber then stv0900, Got that fixed but Im still not happy with the patch, has a few other minor issues with low sr dvb-s qpsk sometimes not locking on the first attempt to tune. The Prof 7500 also seems to have an issue with stb6100 where get_frequency() wont return the correct frequency when other stb6100 devices I have do. Once I get those figured out to the point Im happy I'll submit it for everyones comments. Thanks for the link Mariusz, I'll check it out, maybe youve overcome some of the shortfalls Ive found Chris Lee On Fri, Aug 16, 2013 at 1:19 AM, Mariusz Bialonczyk ma...@skyboo.net wrote: On 07/24/2013 06:39 PM, Chris Lee wrote: Im looking for comments on these two modules, they overlap support for the same demods. stv0900 supporting stv0900 and stv090x supporting stv0900 and stv0903. Ive flipped a few cards from one to the other and they function fine. In some ways stv090x is better suited. Its a pain supporting two modules that are written differently but do the same thing, a fix in one almost always means it has to be implemented in the other as well. I totally agree with you. Im not necessarily suggesting dumping stv0900, but Id like to flip a few cards that I own over to stv090x just to standardize it. The Prof 7301 and Prof 7500. I did it already for 7301, see here: http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure /28082 but due to 'political' reasons it doesn't went upstream. For private use i am still using this patch on recent kernels, because it is working much more stable for my card comparing to stv0900. I think that moving prof 7500 should be relative easy, i even prepared a patch for this but I was not able to test it due to lack of hardware. Whats everyones thoughts on this? It will cut the number of patch''s in half when it comes to these demods. Ive got alot more coming lol :) Oh yes, you could also take into account another duplicate code: stb6100_cfg.h used for stv090x stb6100_proc.h used for stv0900 In my patch I've successfully switched to stb6100_cfg.h. Chris -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html regards, -- Mariusz Białończyk | xmpp/e-mail: ma...@skyboo.net http://manio.skyboo.net | https://github.com/manio -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html SASKEN BUSINESS DISCLAIMER: This message may contain confidential, proprietary or legally privileged information. In case you are not the original intended Recipient of the message, you must not, directly or indirectly, use, disclose, distribute, print, or copy any part of this message and you are requested to delete it and inform the sender. Any views expressed in this message are those of the individual sender unless otherwise stated. Nothing contained in this message shall be construed as an offer or acceptance of any offer by Sasken Communication Technologies Limited (Sasken) unless sent with that express intent and with due authority of Sasken. Sasken has
Re: Include parent hub number in current warning message Parent hub missing LPM exit latency info
On Thu, 29 Aug 2013, Martin MOKREJŠ wrote: Alan Stern wrote: On Thu, 29 Aug 2013, Martin MOKREJŠ wrote: Hi Xenia, thank you, how about inclusion of the parent hub number in the following message (as of now): Parent hub missing LPM exit latency info. Power management will be impacted. I find it awkward to later on run manually lspci/lsusb to find what is the parent. You don't need to run those programs. I think I do NOT get these messages when I have pcie_aspm=off whereas when it is on I get the warning. Why PCIe powersaving affects how USB end devices will be put to sleep I don't know. But that will be the next step to look into. First the warning message. And maybe it could be improve even further to include other relevant capabilities of the parent present/missing. ;-) Here are bits from my dmesg: [5.291321] usb 4-2: new SuperSpeed USB device number 2 using xhci_hcd [5.313219] usb 4-2: Parent hub missing LPM exit latency info. Power management will be impacted. Since this device is 4-2, the parent hub is usb4. Hmm, and it's corresponding PCI device? I know, not always the case but ... The correspondence between a USB bus and the controller's PCI device path is harder to figure out. You can't get it from lspci or lsusb -- although you can get it (with a little difficulty) from lsusb -v. Perhaps the easiest way to do it is simply with ls: $ ls -l /sys/bus/usb/devices/usb1 lrwxrwxrwx 1 root root 0 Aug 29 09:48 /sys/bus/usb/devices/usb1 - ../../../devices/pci:00/:00:1d.7/usb1/ The :00:1d.7 embedded near the end of the output is the controller's PCI address. Note, however, that the change you asked Ksenia to perform (printing the parent hub's name) won't give you the PCI device. In your case, for instance, it would simply print usb4. 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: Include parent hub number in current warning message Parent hub missing LPM exit latency info
Alan Stern wrote: On Thu, 29 Aug 2013, Martin MOKREJŠ wrote: Alan Stern wrote: On Thu, 29 Aug 2013, Martin MOKREJŠ wrote: Hi Xenia, thank you, how about inclusion of the parent hub number in the following message (as of now): Parent hub missing LPM exit latency info. Power management will be impacted. I find it awkward to later on run manually lspci/lsusb to find what is the parent. You don't need to run those programs. I think I do NOT get these messages when I have pcie_aspm=off whereas when it is on I get the warning. Why PCIe powersaving affects how USB end devices will be put to sleep I don't know. But that will be the next step to look into. First the warning message. And maybe it could be improve even further to include other relevant capabilities of the parent present/missing. ;-) Here are bits from my dmesg: [5.291321] usb 4-2: new SuperSpeed USB device number 2 using xhci_hcd [5.313219] usb 4-2: Parent hub missing LPM exit latency info. Power management will be impacted. Since this device is 4-2, the parent hub is usb4. Hmm, and it's corresponding PCI device? I know, not always the case but ... The correspondence between a USB bus and the controller's PCI device path is harder to figure out. You can't get it from lspci or lsusb -- although you can get it (with a little difficulty) from lsusb -v. Perhaps the easiest way to do it is simply with ls: $ ls -l /sys/bus/usb/devices/usb1 lrwxrwxrwx 1 root root 0 Aug 29 09:48 /sys/bus/usb/devices/usb1 - ../../../devices/pci:00/:00:1d.7/usb1/ The :00:1d.7 embedded near the end of the output is the controller's PCI address. Note, however, that the change you asked Ksenia to perform (printing the parent hub's name) won't give you the PCI device. In your case, for instance, it would simply print usb4. You are right but of course I wanted this kind of answer ;), ideally along with the PCI/ASPM/whatever relevant features value why the LPM is not available. Running all lspci/lsusb hours later does not ensure I get the the information how it was configured at that time in the past. -- 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: [Bug] [Sony VAIO SVT15117CXS] USB 2.0 ports not working with any USB device
On Wed, 28 Aug 2013, Kevin Archer wrote: unbind yielded nothing using 3.11.0-031100rc5-generic lsusb -v -s 1:2 Bus 001 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub ... Hub Port Status: Port 1: .0103 power enable connect Port 2: .0100 power Port 3: .0507 highspeed power suspend enable connect Port 4: .0100 power Port 5: .0100 power Port 6: .0100 power This shows the touchscreen on port 1 and the camera on port 3, but not the Bluetooth controller on port 2. Did you have it unplugged at the time? lsusb -v -s 2:2 Bus 002 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub ... Hub Port Status: Port 1: .0100 power Port 2: .0100 power Port 3: .0100 power Port 4: .0100 power Port 5: .0100 power Port 6: .0100 power And this shows nothing plugged in at all. Did you run the same sort of test as before, plugging the flash drive and the Bluetooth controller into this hub? And did you run lsusb -v while the devices were plugged in? I don't know what sort of change could have caused this to happen. Even if the system doesn't realize your devices are plugged in, the hub should show the appropriate port status. It may turn out that bisection is the only way to find the cause of the problem. 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 0/5] Chipidea Misc patchset
On Thu, Aug 29, 2013 at 05:25:55PM +0800, Peter Chen wrote: On Thu, Aug 29, 2013 at 10:53:50AM +0300, Alexander Shishkin wrote: Peter Chen peter.c...@freescale.com writes: Changed for v2: - Fixed the build error for patch 5/5 Below are un-queued chipidea patches, some of them were reviewed. I'd like to send 1/5, 3/5 and 4/5 now, have a closer look at 2/5 and queue 5/5 for 3.13 unless you have objections. Thanks, but for me, it is a little long that you have only queued patch one time for each merge window. Now, I have worked on mainline chipidea code, there will be patches often. I'd like you can queue patches more often, no matter your tree or send to Greg. Yes, you should always accept patches, and then send them on up the maintainer tree when the various windows open up. thanks, 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
Re: stv090x vs stv0900 support
On Thu, Aug 29, 2013 at 03:20:32PM +, Krishna Kishore wrote: Hi, Can someone help me, please? Personally, I'm not allowed to do so because of: SASKEN BUSINESS DISCLAIMER: This message may contain confidential, proprietary or legally privileged information. In case you are not the original intended Recipient of the message, you must not, directly or indirectly, use, disclose, distribute, print, or copy any part of this message and you are requested to delete it and inform the sender. Any views expressed in this message are those of the individual sender unless otherwise stated. Nothing contained in this message shall be construed as an offer or acceptance of any offer by Sasken Communication Technologies Limited (Sasken) unless sent with that express intent and with due authority of Sasken. Sasken has taken enough precautions to prevent the spread of viruses. However the company accepts no liability for any damage caused by any virus transmitted by this email. Read Disclaimer at http://www.sasken.com/extras/mail_disclaimer.html Which is really incompatible with open mailing list discussions... sorry, 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
Re: Include parent hub number in current warning message Parent hub missing LPM exit latency info
On Thu, 29 Aug 2013, Martin MOKREJŠ wrote: Since this device is 4-2, the parent hub is usb4. Actually, even if that would be a another USB HUB and not a PCI device (root hub), I would be happy if it extracted something like: Bus 004 Device 006: ID 2109:0810 $iManufacturer and $iProduct for me. Bus 004 Device 006: ID 2109:0810 Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 3 bMaxPacketSize0 9 idVendor 0x2109 idProduct 0x0810 bcdDevice3.74 iManufacturer 1 VIA Labs, Inc. iProduct2 4-Port USB 3.0 Hub iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 31 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower2mA This really is asking too much of the kernel. That's why we have userspace utilities like lsusb. For example, it wouldn't be hard to write a shell script that would take a device name like 4-2 and print out the information you want. 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: Include parent hub number in current warning message Parent hub missing LPM exit latency info
On 08/29/2013 04:31 PM, Martin MOKREJŠ wrote: Actually, there is some new bug I haven't seen before (this is 3.10.9 kernel). First of all, I see my TI XHCI controller does not use MSI-X anymore, will have to check my .config why is it so. Second, it should have IRQ 45 and 46 according to dmesg. But lspci reports IRQ 16 is used by TI XHCI controller. Funny! I would say this is linux-pci issue but provided XHCI_HCD is special and manages interrupts somewhat on its own you may look into that first before we ask linux-pci developers. Martin MOKREJŠ wrote: Hi Martin, regarding the different IRQ lines reported by lspci and dmesg maybe it is due to the fact that lspci reports the irq field of the pci_dev (when the option -b is not set, because if it is set it will report the physical IRQ line stored the xhc pci configuration register) while dmesg and /proc/interruputs output the msi/msix vector entries. So i suspect that if the controller had not msi/msix capabilities the pci_dev-irq would be also reported in /proc/interrupts. However, i am so new to the field that the most probable is that i say nonsense :) That is what i came to by having a quick look in xhci and pciutils source code. Somebody else would be more suitable to clear out this discrepancy between lspci and /proc/interrupts IRQ line values. best regards, ksenia -- 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: linux-next: Tree for Aug 28 [ xhci build breakage ]
On Thu, Aug 29, 2013 at 10:02:13AM +0200, Sedat Dilek wrote: P.S.: The forgotten patch is now in usb-next, but I don't see any credits, coins, gold, platin... Thank you for reporting this. Your name was mentioned in the tag Greg pulled: https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-nextid=ff49896aa45de286f3cbfda800fc92665374546a As for the non-credit tokens of appreciation, I don't have any gold bars handy, and I don't do bitcoins, sorry. :) Sarah Sharp -- 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
Set SEL for device-initiated U1 failed. errors
Hi Sarah, I'm getting the following warnings from the 3.10.9 kernel all the time when I unplug a USB 3 storage device from my laptop: [203282.987687] usb 4-1: USB disconnect, device number 21 [203282.992904] usb 4-1: Set SEL for device-initiated U1 failed. [203282.992909] usb 4-1: Set SEL for device-initiated U2 failed. What can a normal user do with these failed messages? If nothing, shouldn't we just turn them into debug messages instead? Or is this an indication of another problem somewhere else? This doesn't happen when unplugging a USB 2 device from the same port. thanks, 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
Re: [PATCH] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs
On Wed, Aug 28, 2013 at 04:08:12PM -0700, Julius Werner wrote: So the 2.41a has BESL support, but may not set the BLC flag. What happens if we use the HIRD encoding instead? Will things break? It seems like we would need to disable USB 2.0 LPM on that host all together, if it expects BESL encoding, but advertises HIRD encoding. Wait a second, just for clarity: are you saying that BESL-capable controllers do not support the old HIRD mechanism and thus just break on non-BESL aware OSes? Yes. I would've assumed that they somehow notice if software doesn't write to the new register and automatically fall back to HIRD... it seems like a weird decision to break hardware backwards compatibility like that (after all, it would mean that Linux 3.10 and older would also break on LynxPoint systems right now). Let me dig this older state out of my brain. ISTR yelling at the xHCI spec architect for breaking hosts for this very reason (originally the BLC flag was not in the spec at all)... If a host supports the HIRD encoding, it sets Hardware LMP Capability (HLC), bit 19, in the USB 2.0 port protocol register. That bit is set whether the host supports HIRD or BESL encoding. Bit 20 (BLC) is set if the host supports BESL. When the driver goes to write a value in the USB2 Port Hardware LPM Control Register, if the driver is only aware of the HIRD specifications, it will use the HIRD encodings in bits 13:10, regardless of whether the host has BESL support instead of HIRD support. If the driver has support for BESL and the host has BESL support, it will use the BESL encodings in those bits instead. If you take a look at Table 13: BESL/HIRD Encoding from the xHCI spec version including errata to 08/14/2012, you'll see the numbers written into bits 13:10 mean different timeouts, based on whether the host supports HIRD or BESL. So, basically, if the xHCI driver only supports HIRD and is loaded on a host that supports BESL, then the driver will write a timeout value in bits 13:10 that means something different than what the driver thinks it means. This could lead to issues with USB 2.0 devices that support Link PM. Yes, this is broken. Distros that want to run on hosts that have BESL support need to have the BESL patches from Mathias. Sarah Sharp -- 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 v4 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DWC3 core
On Thu, 2013-08-22 at 15:41 -0500, Felipe Balbi wrote: On Wed, Aug 21, 2013 at 10:13:28AM -0500, Kumar Gala wrote: On Aug 21, 2013, at 8:06 AM, Ivan T. Ivanov wrote: On Tue, 2013-08-20 at 18:01 +0100, Pawel Moll wrote: On Tue, 2013-08-20 at 16:06 +0100, Pawel Moll wrote: On Tue, 2013-08-20 at 16:01 +0100, Kumar Gala wrote: On Aug 20, 2013, at 9:54 AM, Ivan T. Ivanov wrote: Hi, On Tue, 2013-08-20 at 09:33 -0500, Felipe Balbi wrote: On Tue, Aug 20, 2013 at 05:09:11PM +0300, Ivan T. Ivanov wrote: Hi, On Tue, 2013-08-20 at 08:37 -0500, Felipe Balbi wrote: Hi, On Tue, Aug 20, 2013 at 04:32:23PM +0300, Ivan T. Ivanov wrote: On Tue, Aug 20, 2013 at 12:56:04PM +0300, Ivan T. Ivanov wrote: From: Ivan T. Ivanov iiva...@mm-sol.com These drivers handles control and configuration of the HS and SS USB PHY transceivers. They are part of the driver which manage Synopsys DesignWare USB3 controller stack inside Qualcomm SoC's. Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com --- drivers/usb/phy/Kconfig | 11 ++ drivers/usb/phy/Makefile |2 + drivers/usb/phy/phy-msm-dwc3-hs.c | 327 drivers/usb/phy/phy-msm-dwc3-ss.c | 374 + please rename these PHY drivers, they have nothing to do with DWC3. PHYs don't care about the USB controller. I think they are SNPS DesignWare PHY's, additionally wrapped with Qualcomm logic. I could substitute dwc3 with just dw, which will be more correct. alright, thank you. Let's add Paul to the loop since he might have very good insight in the synopsys PHYs. mental note: if any other platform shows up with Synopsys PHY, ask them to use this driver instead :-) I really doubt that this will bi possible. Control of the PHY's is not directly trough ULPI, UTMI or PIPE3 interfaces, but trough QSCRATCH registers, which of course is highly Qualcomm specific. isn't it a memory mapped IP ? doesn't synopsys provide their own set of registers ? From what I see it is not directly mapped. How QSCRATCH write and reads transactions are translated to DW IP is unclear to me. I think the question is how does SW access them? I afraid the answer may be: it depends on the SOC. In my past I had to initialize a (SATA) PHY by implementing a software JTAG state machine, as the PHY's registers were not memory mapped *at all*. And the IP itself came from Synopsys, Cadence or yet another EDA company... Having said all that... If the PHY's spec at least defined layout of the registers in question and driver was using regmap API to talk to the device (initially regmap-mmio), it has some chances to become universal. The SOCs designed like my one would have to provide a custom regmap implementation. Sound reasonable. Unfortunately I don't have PHY's IP spec. Looking into this it appears that the register wrapper around the IP is most likely highly specific to qualcomm as I'm not seeing a register interface around the DWC HS PHY. Then it's set, we'll go with this driver :-) Does this mean that driver could be merged? Regards, Ivan -- 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] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs
On Wed, Aug 28, 2013 at 10:15:34PM +, Paul Zimmerman wrote: From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] Sent: Wednesday, August 28, 2013 2:52 PM On Mon, Aug 26, 2013 at 07:37:56PM +, Paul Zimmerman wrote: From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] Sent: Monday, August 26, 2013 12:14 PM On Thu, Aug 22, 2013 at 05:11:49AM +, Paul Zimmerman wrote: Just to set the record straight :) The PORTHLPMC registers do exist on DWC3, starting with the 1.90a version of the core or thereabouts. They only supported the HIRD flavor of LPM, though. Only fairly recently has support for BESL been added, around version 2.41a or so. And the 2.41a version that supports BESL properly sets the BLC flag in the USB 2.0 Protocol extended capabilities for all the USB 2.0 ports? Has this functionality been well-tested? In 2.41a it is described as an early adopter feature in the databook, and no mention is made of the BLC flag. So the answer there is maybe. Starting with 2.50a it is a full-fledged feature and the databook does describe the BLC flag. So the 2.41a has BESL support, but may not set the BLC flag. What happens if we use the HIRD encoding instead? Will things break? It seems like we would need to disable USB 2.0 LPM on that host all together, if it expects BESL encoding, but advertises HIRD encoding. I imagine things would break, but I don't know for sure. I don't have a 2.41a version of the core to test this with. Maybe the LPM support should be disabled by default, and enabled with a quirk? That seems safer to me. I don't think that's a sustainable option. I expect that the majority of hosts that support USB 2.0 Link PM in the future will have BESL support. It makes no sense to maintain an ever-growing list of hosts that support BESL. We did something similar for the Intel EHCI to xHCI port switchover. Every time someone added a new skew with a different PCI device ID, we had to add that to the list of hosts that had the port switchover. The list grew and grew, and backporting and notifying distros of new list entries was a pain. It just wasn't sustainable, and we ended up ripping out the list and dynamically looking for the Intel EHCI companion host instead. So, no, I don't want to go there. I would rather have an xHCI quirk that disables USB 2.0 LPM instead. So it may be safer to say that the feature is present starting with 2.50a. Is there a way we can tell the difference between a 2.41a xHCI host and a 2.50a host? If so, we can add a quirk to disable LPM on the 2.41a host. Once you know it is a Synopsys core, there is a vendor-specific register that shows the version. But that register is at offset 0xC120, which is above the normal xHCI register space I believe, so we may not be able to depend on it being accessible. And you have the problem of how to determine if it is a Synopsys core to begin with. So IMHO, having LPM disabled by default, and enabled with a quirk based on the PCI Vendor/Product ID, or a DT attribute for platform devices, would be the way to go. I think DT attributes would be the best way to go. I think the patch should be changed to set those USB 2.0 LPM function pointers for the platform devices, but add a new xHCI host quirk, XHCI_DISABLE_USB2_LPM. Make the LPM functions return immediately if that quirk is set. Then set the quirk based on DT attributes for the Synopsis 2.41a host. In 2.51a it has been well-tested in simulation. In actual hardware, it has only been briefly tested in an ad-hoc sort of way, since none of the standard drivers in the market support the feature yet, as far as we know. Once the support is fully working in the Linux driver we can try testing it there. Can you please test Julius' patch on the 2.41a, 2.50a, and 2.51a hosts? Please test against usb-next, since that includes a fix for the BESL patches. As I said, I don't have the 2.41a version available to test. I do have 2.50a and 2.60a available, so I can try those. Ok, let me know if those work. In the meantime, can you help Julius create a patch to add DT attributes to distinguish between the different versions? Sarah Sharp -- 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: [alsa-devel] Buffer size for ALSA USB PCM audio
At 32 frames/period (reported round-trip latency 1.33ms), it starts up but there are too many xruns for it to be usable. James On Thu, Aug 29, 2013 at 4:00 PM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 29 Aug 2013, James Stone wrote: On Wed, Aug 28, 2013 at 7:46 PM, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 28 Aug 2013, Clemens Ladisch wrote: Sorry, what I said applies more to explicit sync endpoints. When using implicit sync, a playback URB is submitted for each completed capture URB, with the number of samples per packet identical to the corresponding capture packet, so the parameters must be identical. Got it. Below is an updated patch. James, the problem you encountered was most likely a result of the faulty treatment of capture endpoints that Clemens pointed out. It was quite obvious in the usbmon traces that the unpatched driver used 8 packets per URB whereas the patched driver used 22. This updated patch should fix that problem. Works fine. With this, it is possible to get clear playback at 64 frames/period too. With vanilla 3.10.9, there was some glitchy distortion to the sound at that latency, so this seems to be an improvement. That's good. What happens if you push frames/period even lower? Daniel and Eldad, more testing of the most recent proposed patch would be welcome. 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] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On Thu, Aug 29, 2013 at 08:37:57PM +0300, Xenia Ragiadakou wrote: In usb_reset_and_verify_device(), hub_port_init() allocates a new bos descriptor to hold the value read by the device. The new bos descriptor has to be compared with the old one in order to figure out if device 's firmware has changed in which case the device has to be reenumerated. In the original code, none of the two descriptors was deallocated leading to memory leaks. This patch compares the old bos descriptor with the new one to detect change in firmware and releases the newly allocated bos descriptor to prevent memory leak. Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com Shouldn't there be a Reported-by: and Tested-by: field here as well? thanks, 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
Re: [PATCH] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs
If you take a look at Table 13: BESL/HIRD Encoding from the xHCI spec version including errata to 08/14/2012 Could you please provide a link to that errata? I still cannot find it... but from your explanation, that design decision sounds pretty horrible. Why didn't they just choose not to set old HLC flag in BESL controllers? Seems like the only purpose it provides there is to make old drivers break. Anyway, looks like we are stuck with it now, and need to deal with those mislabeled DWC3 versions. I agree with you that we should blacklist instead of whitelist, but I don't think the device tree is the best place to put that... we would have to figure out the exact DWC3 version for every processor/SoC dtsi file to determine if they are affected, and remember to keep that up to date as we added more. I would instead propose to check for the revision register directly in the DWC3 stack. I think I could add a little check to dwc3_host_init() and hack the quirk bit into the newly created XHCI controller instance if required. However, I only have an old (unaffected) 1.85 controller for testing, so I would need Synopsys to provide me with the exact revision numbers affected (as read from the register) and to test the change for us. Also, do we want to make it an XHCI_DISABLE_USB2_LPM or a XHCI_FORCE_USB2_BESL quirk? Assuming their BESL implementation is otherwise correct except for omitting that bit, the latter one should make those controllers work better. -- 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] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs
From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] Sent: Thursday, August 29, 2013 10:42 AM On Wed, Aug 28, 2013 at 10:15:34PM +, Paul Zimmerman wrote: From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] Sent: Wednesday, August 28, 2013 2:52 PM On Mon, Aug 26, 2013 at 07:37:56PM +, Paul Zimmerman wrote: From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] Sent: Monday, August 26, 2013 12:14 PM On Thu, Aug 22, 2013 at 05:11:49AM +, Paul Zimmerman wrote: Just to set the record straight :) The PORTHLPMC registers do exist on DWC3, starting with the 1.90a version of the core or thereabouts. They only supported the HIRD flavor of LPM, though. Only fairly recently has support for BESL been added, around version 2.41a or so. And the 2.41a version that supports BESL properly sets the BLC flag in the USB 2.0 Protocol extended capabilities for all the USB 2.0 ports? Has this functionality been well-tested? In 2.41a it is described as an early adopter feature in the databook, and no mention is made of the BLC flag. So the answer there is maybe. Starting with 2.50a it is a full-fledged feature and the databook does describe the BLC flag. So the 2.41a has BESL support, but may not set the BLC flag. What happens if we use the HIRD encoding instead? Will things break? It seems like we would need to disable USB 2.0 LPM on that host all together, if it expects BESL encoding, but advertises HIRD encoding. I imagine things would break, but I don't know for sure. I don't have a 2.41a version of the core to test this with. Maybe the LPM support should be disabled by default, and enabled with a quirk? That seems safer to me. I don't think that's a sustainable option. I expect that the majority of hosts that support USB 2.0 Link PM in the future will have BESL support. It makes no sense to maintain an ever-growing list of hosts that support BESL. We did something similar for the Intel EHCI to xHCI port switchover. Every time someone added a new skew with a different PCI device ID, we had to add that to the list of hosts that had the port switchover. The list grew and grew, and backporting and notifying distros of new list entries was a pain. It just wasn't sustainable, and we ended up ripping out the list and dynamically looking for the Intel EHCI companion host instead. Yes, but that was a case of things not working at all, correct? The worst that can happen with LPM disabled is that a new host will consume a little more power than necessary, until someone gets around to adding a quirk for it. Whereas if you enable it by default, things could be broken until the quirk is added. So, no, I don't want to go there. I would rather have an xHCI quirk that disables USB 2.0 LPM instead. ... I think DT attributes would be the best way to go. I think the patch should be changed to set those USB 2.0 LPM function pointers for the platform devices, but add a new xHCI host quirk, XHCI_DISABLE_USB2_LPM. Make the LPM functions return immediately if that quirk is set. Then set the quirk based on DT attributes for the Synopsis 2.41a host. In 2.51a it has been well-tested in simulation. In actual hardware, it has only been briefly tested in an ad-hoc sort of way, since none of the standard drivers in the market support the feature yet, as far as we know. Once the support is fully working in the Linux driver we can try testing it there. Can you please test Julius' patch on the 2.41a, 2.50a, and 2.51a hosts? Please test against usb-next, since that includes a fix for the BESL patches. As I said, I don't have the 2.41a version available to test. I do have 2.50a and 2.60a available, so I can try those. Ok, let me know if those work. In the meantime, can you help Julius create a patch to add DT attributes to distinguish between the different versions? I don't think there's a need to distinguish between different versions, is there? Don't we need just a single DT attribute that means does not support BESL? -- Paul -- 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] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On 08/29/2013 08:53 PM, Greg KH wrote: On Thu, Aug 29, 2013 at 08:37:57PM +0300, Xenia Ragiadakou wrote: In usb_reset_and_verify_device(), hub_port_init() allocates a new bos descriptor to hold the value read by the device. The new bos descriptor has to be compared with the old one in order to figure out if device 's firmware has changed in which case the device has to be reenumerated. In the original code, none of the two descriptors was deallocated leading to memory leaks. This patch compares the old bos descriptor with the new one to detect change in firmware and releases the newly allocated bos descriptor to prevent memory leak. Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com Shouldn't there be a Reported-by: and Tested-by: field here as well? thanks, greg k-h Do you mean i need to add, under the signed-off, Reported-by: Martin MOKREJŠ mmokr...@gmail.com Tested-by: Martin MOKREJŠ mmokr...@gmail.com and resend as PATCH v2 or just PATCH? Also does it need a Reviewed-by: Alan Stern st...@rowland.harvard.edu ? ksenia -- 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: Set SEL for device-initiated U1 failed. errors
On Thu, Aug 29, 2013 at 10:06:16AM -0700, Greg Kroah-Hartman wrote:
Hi Sarah,
I'm getting the following warnings from the 3.10.9 kernel all the time
when I unplug a USB 3 storage device from my laptop:
[203282.987687] usb 4-1: USB disconnect, device number 21
[203282.992904] usb 4-1: Set SEL for device-initiated U1 failed.
[203282.992909] usb 4-1: Set SEL for device-initiated U2 failed.
What can a normal user do with these failed messages? If nothing,
shouldn't we just turn them into debug messages instead?
Yes, those messages should probably be toned down to debug level instead
of warning level. If a device doesn't respond to the Set SEL request
when USB 3.0 LPM is enabled, the user has a buggy device. Of course, I
doubt anyone is going to return a drive based on those messages.
That error message happens because the USB core is attempting to disable
LPM for a disconnected device. The control transfer to set SEL fails,
resulting in those messages. The xHCI driver still needs to disable the
U1 and U2 timeouts for the port, so the core still needs to call into
usb_set_lpm_timeout. However, we could skip the control transfer to the
device.
The problem is that the USB core doesn't mark the device as DISCONNECTED
until after it attempts to disable LPM. The device is still marked as
being in the configured state, because we don't return early in this
function:
static int usb_set_device_initiated_lpm(struct usb_device *udev,
enum usb3_link_state state, bool enable)
{
int ret;
int feature;
switch (state) {
case USB3_LPM_U1:
feature = USB_DEVICE_U1_ENABLE;
break;
case USB3_LPM_U2:
feature = USB_DEVICE_U2_ENABLE;
break;
default:
dev_warn(udev-dev, %s: Can't %s non-U1 or U2 state.\n,
__func__, enable ? enable : disable);
return -EINVAL;
}
if (udev-state != USB_STATE_CONFIGURED) {
dev_dbg(udev-dev, %s: Can't %s %s state
for unconfigured device.\n,
__func__, enable ? enable : disable,
usb3_lpm_names[state]);
return 0;
}
if (enable) {
/*
* Now send the control transfer to enable device-initiated LPM
* for either U1 or U2.
*/
ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
USB_REQ_SET_FEATURE,
USB_RECIP_DEVICE,
feature,
0, NULL, 0,
USB_CTRL_SET_TIMEOUT);
} else {
ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
USB_REQ_CLEAR_FEATURE,
USB_RECIP_DEVICE,
feature,
0, NULL, 0,
USB_CTRL_SET_TIMEOUT);
}
if (ret 0) {
dev_warn(udev-dev, %s of device-initiated %s failed.\n,
enable ? Enable : Disable,
usb3_lpm_names[state]);
return -EBUSY;
}
return 0;
}
So I don't know how the LPM code can know the device is disconnected, and thus
it should skip the control transfer. Do we get an -ENODEV in that case?
The least we could do is change that dev_warn to dev_dbg instead.
Or is this an indication of another problem somewhere else?
This doesn't happen when unplugging a USB 2 device from the same port.
Set SEL is a USB 3.0 specific transfer, and is only used for devices
that support USB 3.0 Link PM (and don't have the U1/U2 device exit
latencies set to all zeros).
Sarah Sharp
--
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] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On Thu, Aug 29, 2013 at 10:53:13AM -0700, Greg KH wrote: On Thu, Aug 29, 2013 at 08:37:57PM +0300, Xenia Ragiadakou wrote: In usb_reset_and_verify_device(), hub_port_init() allocates a new bos descriptor to hold the value read by the device. The new bos descriptor has to be compared with the old one in order to figure out if device 's firmware has changed in which case the device has to be reenumerated. In the original code, none of the two descriptors was deallocated leading to memory leaks. This patch compares the old bos descriptor with the new one to detect change in firmware and releases the newly allocated bos descriptor to prevent memory leak. Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com Shouldn't there be a Reported-by: and Tested-by: field here as well? I don't think Xenia and I have discussed tags outside of Signed-off-bys. It's probably something we should add to the patch tutorial (and SubmittingPatches as well). Xenia, when someone reports a problem, you should use the Reported-by: tag to credit them. If they test your patch, and find it fixes their problem, you should add a Tested-by: tag. If you know you need specific people to review your patches, you can add a Cc: tag. There's also a Suggested-by: tag that might be useful here. In this case, Alan Stern suggested that the fix for Martin's problem would be to fix the BOS descriptor leak. You probably want to credit Alan in this patch. Sarah Sharp -- 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: Bug 60810 - Kernel oops with controller XHCI while wait usb packet
Hi Giovanni, Mathias mentioned that this patch might fix your issue: http://marc.info/?l=linux-usbm=136269803207465w=2 Can you please compile a custom 3.10 stable kernel with that patch applied and see if it fixes your issue? Sarah Sharp On Wed, Aug 28, 2013 at 09:46:42PM -0700, Giovanni wrote: https://bugzilla.kernel.org/show_bug.cgi?id=60810 Bug ID: 60810 Summary: Kernel oops with controller XHCI while wait usb packet Regards Giovanni -- 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 -- 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: Include parent hub number in current warning message Parent hub missing LPM exit latency info
Alan Stern wrote: On Thu, 29 Aug 2013, Martin MOKREJŠ wrote: Since this device is 4-2, the parent hub is usb4. Actually, even if that would be a another USB HUB and not a PCI device (root hub), I would be happy if it extracted something like: Bus 004 Device 006: ID 2109:0810 $iManufacturer and $iProduct for me. Bus 004 Device 006: ID 2109:0810 Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 3 bMaxPacketSize0 9 idVendor 0x2109 idProduct 0x0810 bcdDevice3.74 iManufacturer 1 VIA Labs, Inc. iProduct2 4-Port USB 3.0 Hub iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 31 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower2mA This really is asking too much of the kernel. That's why we have userspace utilities like lsusb. For example, it wouldn't be hard to write a shell script that would take a device name like 4-2 and print out the information you want. The problem is that I want the information to be logged automatically in syslog. Think of laptop-mode-tools or acpid or ACPI events from BIOS fiddling with my devices and causing those resets. Sometimes PCI restores their config space and it is way too late to run manually some utility hours later. Please log automatically whatever is doable. I just wanted to raise this up. I don't think usb core driver will call a shell script, so ... Just try to do something in this direction. I understand, the parent could be a PCI device or another USB device so it gets more complicated quickly but the relevant information must be gathered immediately. Martin -- 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: 3.4.4: disabling irq
On Thu, 29 Aug 2013, Udo van den Heuvel wrote:
On 2013-08-28 21:37, Alan Stern wrote:
No, if you unload the ohci-hcd driver then the webcam won't be used.
Are you certain that merely stopping the daemon program will prevent
the problem?
Quite certain but retesting can confirm that.
Maybe without activity, the OHCI controller goes into suspend. You can
prevent it by doing:
echo on /sys/bus/pci/devices/:00:13.1/usb?/power/control
/sys/bus/pci/devices/:00:13.0 has a file irq containing 18.
subdirectory usb8 under there has no power/control subdir/file.
Is this a configuration issue or kernel version difference?
It could be a config issue. The file will be present only if
CONFIG_PM_RUNTIME is enabled.
By the way, since you switched to the new computer, are there any
devices attached to this USB bus other than the webcam?
The wired mouse (logitech M-BD58) is connected via usb.
Is it connected to the :00:13.1 controller or to the :00:13.0
controller?
Also, at one point you had a second webcam attached to a different OHCI
controller, as well as a Bluetooth device and a USB-serial device.
Are they still present?
Nope. Presently not connected.
I'll have some hubs this weekend to play with.
In the meantime, here is a diagnostic patch you can try out.
Alan Stern
Index: usb-3.11/drivers/usb/host/ohci-hcd.c
===
--- usb-3.11.orig/drivers/usb/host/ohci-hcd.c
+++ usb-3.11/drivers/usb/host/ohci-hcd.c
@@ -592,7 +592,10 @@ static int ohci_run (struct ohci_hcd *oh
u32 mask, val;
int first = ohci-fminterval == 0;
struct usb_hcd *hcd = ohci_to_hcd(ohci);
+ static int alan_id;
+ ohci-alan_time = jiffies;
+ ohci-alan_limit = (++alan_id) * 1000;
ohci-rh_state = OHCI_RH_HALTED;
/* boot firmware should have set this up (5.1.1.3.1) */
@@ -810,6 +813,20 @@ static irqreturn_t ohci_irq (struct usb_
return IRQ_HANDLED;
}
+ if (ohci-alan_count 0)
+ return IRQ_NOTMINE;
+ else if (time_after(jiffies, ohci-alan_time)) {
+ ohci-alan_time = jiffies + HZ/10;
+ ohci-alan_count = 0;
+ } else if (++ohci-alan_count ohci-alan_limit) {
+ ohci_info(ohci, Too many interrupts %d, disabling\n,
+ ohci-alan_limit);
+ ohci-alan_count = -1;
+ ohci_writel(ohci, ~0, regs-intrdisable);
+ ohci_writel(ohci, ~0, regs-intrstatus);
+ return IRQ_NOTMINE;
+ }
+
/* We only care about interrupts that are enabled */
ints = ohci_readl(ohci, regs-intrenable);
Index: usb-3.11/drivers/usb/host/ohci-hub.c
===
--- usb-3.11.orig/drivers/usb/host/ohci-hub.c
+++ usb-3.11/drivers/usb/host/ohci-hub.c
@@ -218,7 +218,8 @@ __acquires(ohci-lock)
skip_resume:
/* interrupts might have been disabled */
- ohci_writel (ohci, OHCI_INTR_INIT, ohci-regs-intrenable);
+ if (ohci-alan_count = 0)
+ ohci_writel (ohci, OHCI_INTR_INIT, ohci-regs-intrenable);
if (ohci-ed_rm_list)
ohci_writel (ohci, OHCI_INTR_SF, ohci-regs-intrenable);
Index: usb-3.11/drivers/usb/host/ohci.h
===
--- usb-3.11.orig/drivers/usb/host/ohci.h
+++ usb-3.11/drivers/usb/host/ohci.h
@@ -415,6 +415,10 @@ struct ohci_hcd {
struct ed *ed_to_check;
unsignedzf_delay;
+ unsigned long alan_time;
+ int alan_count;
+ int alan_limit;
+
#ifdef DEBUG
struct dentry *debug_dir;
struct dentry *debug_async;
--
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] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs
From: jwer...@google.com [mailto:jwer...@google.com] On Behalf Of Julius Werner Sent: Thursday, August 29, 2013 11:07 AM If you take a look at Table 13: BESL/HIRD Encoding from the xHCI spec version including errata to 08/14/2012 Could you please provide a link to that errata? I still cannot find it... but from your explanation, that design decision sounds pretty horrible. Why didn't they just choose not to set old HLC flag in BESL controllers? Seems like the only purpose it provides there is to make old drivers break. Anyway, looks like we are stuck with it now, and need to deal with those mislabeled DWC3 versions. I agree with you that we should blacklist instead of whitelist, but I don't think the device tree is the best place to put that... we would have to figure out the exact DWC3 version for every processor/SoC dtsi file to determine if they are affected, and remember to keep that up to date as we added more. I would instead propose to check for the revision register directly in the DWC3 stack. I think I could add a little check to dwc3_host_init() and hack the quirk bit into the newly created XHCI controller instance if required. However, I only have an old (unaffected) 1.85 controller for testing, so I would need Synopsys to provide me with the exact revision numbers affected (as read from the register) and to test the change for us. OK, I did a little more digging, and it turns out the 2.41a version _does_ set bit 20 in the protocol defined field if BESL support is enabled. It wasn't mentioned in the registers section of the databook, but there is a note to that effect in a different section. So it looks like we don't need to worry about this for the DWC3 controllers, anyway. Sorry for the noise. -- Paul -- 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: [alsa-devel] Buffer size for ALSA USB PCM audio
On Thu, 29 Aug 2013, James Stone wrote: At 32 frames/period (reported round-trip latency 1.33ms), it starts up but there are too many xruns for it to be usable. Interesting. Can you try doing the same thing with just playback, no recording? If that still gets underruns, please collect a usbmon trace. 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
[PATCH v2] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.
This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
Reported-by: Martin MOKREJS mmokr...@gmail.com
Tested-by: Martin MOKREJS mmokr...@gmail.com
Suggested-by: Alan Stern st...@rowland.harvard.edu
---
drivers/usb/core/hub.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..2455001 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
} /* usb_hub_cleanup() */
static int descriptors_changed(struct usb_device *udev,
- struct usb_device_descriptor *old_device_descriptor)
+ struct usb_device_descriptor *old_device_descriptor,
+ struct usb_host_bos *old_bos)
{
int changed = 0;
unsignedindex;
@@ -4953,6 +4954,16 @@ static int descriptors_changed(struct usb_device *udev,
sizeof(*old_device_descriptor)) != 0)
return 1;
+ if ((old_bos !udev-bos) || (!old_bos udev-bos))
+ return 1;
+ if (udev-bos) {
+ len = udev-bos-desc-wTotalLength;
+ if (len != old_bos-desc-wTotalLength)
+ return 1;
+ if (memcmp(udev-bos-desc, old_bos-desc, le16_to_cpu(len)))
+ return 1;
+ }
+
/* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
@@ -5049,6 +5060,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
struct usb_hub *parent_hub;
struct usb_hcd *hcd = bus_to_hcd(udev-bus);
struct usb_device_descriptordescriptor = udev-descriptor;
+ struct usb_host_bos *bos;
int i, ret = 0;
int port1 = udev-portnum;
@@ -5066,6 +5078,9 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
}
parent_hub = usb_hub_to_struct_hub(parent_hdev);
+ bos = udev-bos;
+ udev-bos = NULL;
+
/* Disable LPM and LTM while we reset the device and reinstall the alt
* settings. Device-initiated LPM settings, and system exit latency
* settings are cleared when the device is reset, so we have to set
@@ -5099,7 +5114,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
goto re_enumerate;
/* Device might have changed firmware (DFU or similar) */
- if (descriptors_changed(udev, descriptor)) {
+ if (descriptors_changed(udev, descriptor, bos)) {
dev_info(udev-dev, device firmware changed\n);
udev-descriptor = descriptor; /* for disconnect() calls */
goto re_enumerate;
@@ -5172,11 +5187,15 @@ done:
/* Now that the alt settings are re-installed, enable LTM and LPM. */
usb_unlocked_enable_lpm(udev);
usb_enable_ltm(udev);
+ usb_release_bos_descriptor(udev);
+ udev-bos = bos;
return 0;
re_enumerate:
/* LPM state doesn't matter when we're about to destroy the device. */
hub_port_logical_disconnect(parent_hub, port1);
+ usb_release_bos_descriptor(udev);
+ udev-bos = bos;
return -ENODEV;
}
--
1.8.3.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
Re: Include parent hub number in current warning message Parent hub missing LPM exit latency info
On Thu, Aug 29, 2013 at 08:36:51PM +0200, Martin MOKREJŠ wrote: Alan Stern wrote: On Thu, 29 Aug 2013, Martin MOKREJŠ wrote: Since this device is 4-2, the parent hub is usb4. Actually, even if that would be a another USB HUB and not a PCI device (root hub), I would be happy if it extracted something like: Bus 004 Device 006: ID 2109:0810 $iManufacturer and $iProduct for me. Bus 004 Device 006: ID 2109:0810 Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 3 bMaxPacketSize0 9 idVendor 0x2109 idProduct 0x0810 bcdDevice3.74 iManufacturer 1 VIA Labs, Inc. iProduct2 4-Port USB 3.0 Hub iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 31 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower2mA This really is asking too much of the kernel. That's why we have userspace utilities like lsusb. For example, it wouldn't be hard to write a shell script that would take a device name like 4-2 and print out the information you want. The problem is that I want the information to be logged automatically in syslog. Think of laptop-mode-tools or acpid or ACPI events from BIOS fiddling with my devices and causing those resets. Sometimes PCI restores their config space and it is way too late to run manually some utility hours later. Please log automatically whatever is doable. I just wanted to raise this up. I don't think usb core driver will call a shell script, so ... Just try to do something in this direction. I understand, the parent could be a PCI device or another USB device so it gets more complicated quickly but the relevant information must be gathered immediately. As the kernel logs the device that has the issue, it is up to the userspace tools to determine anything else it needs/wants from that device on its own. There are tools that to this today already (hint, look at journald), so it is possible, and already working quite well with those tools. Because of this, the kernel isn't going to be changed, sorry. thanks, 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
Re: Set SEL for device-initiated U1 failed. errors
On Thu, 29 Aug 2013, Sarah Sharp wrote:
On Thu, Aug 29, 2013 at 10:06:16AM -0700, Greg Kroah-Hartman wrote:
Hi Sarah,
I'm getting the following warnings from the 3.10.9 kernel all the time
when I unplug a USB 3 storage device from my laptop:
[203282.987687] usb 4-1: USB disconnect, device number 21
[203282.992904] usb 4-1: Set SEL for device-initiated U1 failed.
[203282.992909] usb 4-1: Set SEL for device-initiated U2 failed.
What can a normal user do with these failed messages? If nothing,
shouldn't we just turn them into debug messages instead?
Yes, those messages should probably be toned down to debug level instead
of warning level. If a device doesn't respond to the Set SEL request
when USB 3.0 LPM is enabled, the user has a buggy device. Of course, I
doubt anyone is going to return a drive based on those messages.
That error message happens because the USB core is attempting to disable
LPM for a disconnected device. The control transfer to set SEL fails,
resulting in those messages. The xHCI driver still needs to disable the
U1 and U2 timeouts for the port, so the core still needs to call into
usb_set_lpm_timeout. However, we could skip the control transfer to the
device.
The problem is that the USB core doesn't mark the device as DISCONNECTED
until after it attempts to disable LPM.
Are you certain? Look at the order of the lines in the log above.
The device is still marked as
being in the configured state, because we don't return early in this
function:
static int usb_set_device_initiated_lpm(struct usb_device *udev,
enum usb3_link_state state, bool enable)
{
...
}
So I don't know how the LPM code can know the device is disconnected, and thus
it should skip the control transfer. Do we get an -ENODEV in that case?
That doesn't sound right at all. This function is called from
usb_disable_link_state, which is called from usb_disable_lpm, which is
called from usb_unlocked_disable_lpm, which is called from
usb_disable_device, which is called from usb_disconnect.
The first thing usb_disconnect does is change udev-state to
STATE_NOTATTACHED. Therefore you can test for that in
usb_set_device_initiated_lpm, and avoid trying to send messages that
will never be received. Or if you prefer, avoid writing anything to
the log when the transfer fails with -ENODEV.
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: Include parent hub number in current warning message Parent hub missing LPM exit latency info
On Thu, 29 Aug 2013, Martin MOKREJŠ wrote: This really is asking too much of the kernel. That's why we have userspace utilities like lsusb. For example, it wouldn't be hard to write a shell script that would take a device name like 4-2 and print out the information you want. The problem is that I want the information to be logged automatically in syslog. Think of laptop-mode-tools or acpid or ACPI events from BIOS fiddling with my devices and causing those resets. Sometimes PCI restores their config space and it is way too late to run manually some utility hours later. Please log automatically whatever is doable. I just wanted to raise this up. I don't think usb core driver will call a shell script, so ... Just try to do something in this direction. This really isn't necessary. All the information you want is already in the system log. It's merely a matter of locating it. I can show you how. If you post a complete dmesg log, starting from boot and running up to one of these errors, I'll point out the messages containing the relevant information. I understand, the parent could be a PCI device or another USB device so it gets more complicated quickly but the relevant information must be gathered immediately. It is a mistake to put too much functionality in the kernel. Today you want device IDs to be printed. Tomorrow somebody will want something else. Before you know it, the kernel will end up printing out the complete state of the system whenever anything happens! :-) 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 v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
On 08/29/2013 05:48 AM, George Cherian wrote: On 8/29/2013 4:07 PM, Chanwoo Choi wrote: ... I tested various development board based on Samsung Exynos series SoC. Although some gpio of Exynos series SoC set high state(non zero, 1) as default value, this gpio state could mean off state, disconnected or un-powered state according to gpio. Of course, above explanation about specific gpio don't include all gpios. This is meaning that there is possibility. okay then how about adding a flag for inverted logic too. something like this if(of_property_read_bool(np,inverted_gpio)) gpio_usbvid-gpio_inv = 1; And always check on this before deciding? Don't do this. GPIO specifiers in DT typically include a flags cell that describes certain GPIO properties. One of those properties is signal inversion. You can use of_get_named_gpio_flags() to retrieve those flags. -- 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 v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
On 08/28/2013 11:33 AM, George Cherian wrote:
Add a generic USB VBUS/ID detection EXTCON driver. This driver expects
the ID/VBUS pin are connected via GPIOs. This driver is tested on
DRA7x board were the ID pin is routed via GPIOs. The driver supports
both VBUS and ID pin configuration and ID pin only configuration.
diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio-usbvid.txt
b/Documentation/devicetree/bindings/extcon/extcon-gpio-usbvid.txt
+EXTCON FOR USB VIA GPIO
Please write a brief explanation of the HW that this binding represents.
Extcon is a Linux-specific term. Binding documentation should be
written in an OS-agnostic manner. Bindings should not reference
OS-specific terminology, and should be a pure description of the HW.
+Required Properties:
+ - compatible : Should be ti,gpio-usb-vid for USB VBUS-ID detector
+ using gpios or ti,gpio-usb-id for USB ID pin detector
Those souond like two rather different things. Should they be separate
bindings, or at the least, separate sections in the document?
If this binding is meant to represent some generic hardware, the vendor
prefix probably isn't required. So, remove ti, from the compatible values.
+ - gpios : phandle and args ID pin gpio and VBUS gpio.
s/and args/and specifier/.
+The first gpio used for ID pin detection
+and the second used for VBUS detection.
+ID pin gpio is mandatory and VBUS is optional
+depending on implementation.
It'd be better to use named GPIO properties here, rather than having
multiple different kinds of GPIO in the same property. How about
id-gpios and vbus-gpios as the property names?
The semantics of each property should be specified. Are these input
GPIOs that reflect the ID and VBUS state? Do they directly represent the
signal state on the connector, or are they processed somehow? Does the
VBUS GPIO control the board's VBUS output, or is it an input? If it
controls VBUS output, it probably should be represented as a regulator
rather than a GPIO.
I think the following might work:
Required properties:
id-gpios: GPIO specifier for the connector's ID pin input. This directly
represents the value present on the ID pin at the connector.
Optional properties:
vbus-gpios: GPIO specifier for the connector's VBUS signal. This
directly represents whether VBUS being driven by the USB cable itself.
(although perhaps that isn't an optional property, but rather a required
property of the second compatible value?)
+Please refer to ../gpio/gpio.txt for details of the common GPIO bindings
+
+Example:
+
+ gpio_usbvid_extcon1 {
+ compatible = ti,gpio-usb-vid;
+ gpios = gpio1 1 0,
+ gpio2 2 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
Re: [PATCH v3 3/3] ARM: dts: dra7-evm: Add extcon nodes for USB ID pin detection
On 08/28/2013 11:33 AM, George Cherian wrote:
Add
-extcon nodes for USB ID pin detection.
-i2c nodes.
-pcf nodes to which USB ID pin is connected.
diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
dwc3_1 {
- dr_mode = otg;
+ dr_mode = host;
};
I wonder why one cares about ID/VBUS detection if the port doesn't
operate in OTG mode?
dwc3_2 {
dr_mode = host;
};
+
+usb1 {
+ extcon = extcon1;
+};
+
+usb2 {
+ extcon = extcon2;
+};
I assume the extcon property is already fully documented in the
binding for the USB controller? For some reason, extcon looks like an
odd property name; I would have expected something more HW-oriented that
Linux-subsystem-oriented, such as connector, or usb-connector.
--
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: Set SEL for device-initiated U1 failed. errors
On Thu, Aug 29, 2013 at 02:58:25PM -0400, Alan Stern wrote: On Thu, 29 Aug 2013, Sarah Sharp wrote: On Thu, Aug 29, 2013 at 10:06:16AM -0700, Greg Kroah-Hartman wrote: Hi Sarah, I'm getting the following warnings from the 3.10.9 kernel all the time when I unplug a USB 3 storage device from my laptop: [203282.987687] usb 4-1: USB disconnect, device number 21 [203282.992904] usb 4-1: Set SEL for device-initiated U1 failed. [203282.992909] usb 4-1: Set SEL for device-initiated U2 failed. What can a normal user do with these failed messages? If nothing, shouldn't we just turn them into debug messages instead? Yes, those messages should probably be toned down to debug level instead of warning level. If a device doesn't respond to the Set SEL request when USB 3.0 LPM is enabled, the user has a buggy device. Of course, I doubt anyone is going to return a drive based on those messages. That error message happens because the USB core is attempting to disable LPM for a disconnected device. The control transfer to set SEL fails, resulting in those messages. The xHCI driver still needs to disable the U1 and U2 timeouts for the port, so the core still needs to call into usb_set_lpm_timeout. However, we could skip the control transfer to the device. The problem is that the USB core doesn't mark the device as DISCONNECTED until after it attempts to disable LPM. Are you certain? Look at the order of the lines in the log above. Hmm, I'll have to look at the code more closely then. Sarah Sharp -- 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: Include parent hub number in current warning message Parent hub missing LPM exit latency info
On Thu, 29 Aug 2013, Martin MOKREJŠ wrote:
This really isn't necessary. All the information you want is already
in the system log. It's merely a matter of locating it.
I can show you how. If you post a complete dmesg log, starting from
boot and running up to one of these errors, I'll point out the messages
containing the relevant information.
I understand, the parent could be a PCI device or another USB device so it
gets
more complicated quickly but the relevant information must be gathered
immediately.
It is a mistake to put too much functionality in the kernel. Today you
want device IDs to be printed. Tomorrow somebody will want something
else. Before you know it, the kernel will end up printing out the
complete state of the system whenever anything happens! :-)
I understand your point but don't think this is the case. OK, please show me
why
is LPM not available. If USB is complaining about that being disabled it could
tell me based on what attribute at least. And printing the PCI device ID at
least
would be helpful. Yeah, will look what that journald does.
Actually, it looks like the missing LPM functionality is caused by a
bug in drivers/usb/core/hcd.c. The register_root_hub() routine does
this:
if (usb_dev-speed == USB_SPEED_SUPER) {
retval = usb_get_bos_descriptor(usb_dev);
if (retval 0) {
mutex_unlock(usb_bus_list_lock);
dev_dbg(parent_dev, can't read %s bos descriptor %d\n,
dev_name(usb_dev-dev), retval);
return retval;
}
}
Compare this to the code in hub.c:
if (udev-wusb == 0 le16_to_cpu(udev-descriptor.bcdUSB) = 0x0201) {
retval = usb_get_bos_descriptor(udev);
if (!retval) {
udev-lpm_capable = usb_device_supports_lpm(udev);
usb_set_lpm_parameters(udev);
}
}
You can see that register_root_hub() doesn't set udev-lpm_capable.
The missing call to usb_set_lpm_parameters() doesn't matter, because
root hubs don't have parent hubs.
Ksenia, can you send Martin patch to set udev-lpm_capable for root
hubs? Note that this will require you to fix usb_device_supports_lpm()
as well; that routine dereferences udev-parent, but for root hubs
udev-parent is NULL.
Martin, the information you asked about can be found as follows in your
log. Here are the parts to look at:
[4.228257] usb usb4: New USB device found, idVendor=1d6b, idProduct=0003
[4.228906] usb usb4: New USB device strings: Mfr=3, Product=2,
SerialNumber=1
[4.229571] usb usb4: Product: xHCI Host Controller
[4.230197] usb usb4: Manufacturer: Linux 3.10.9-default-pciehp xhci_hcd
[4.230823] usb usb4: SerialNumber: :0b:00.0
[5.291321] usb 4-2: new SuperSpeed USB device number 2 using xhci_hcd
[5.313219] usb 4-2: Parent hub missing LPM exit latency info. Power
management will be impacted.
[5.319428] usb 4-2: New USB device found, idVendor=2109, idProduct=0811
[5.321417] usb 4-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[5.323353] usb 4-2: Product: 4-Port USB 3.0 Hub
[5.325272] usb 4-2: Manufacturer: VIA Labs, Inc.
[14502.272269] usb 4-2.1: new SuperSpeed USB device number 3 using xhci_hcd
[14502.292302] usb 4-2.1: Parent hub missing LPM exit latency info. Power
management will be impacted.
[14502.301158] usb 4-2.1: New USB device found, idVendor=174c, idProduct=5106
[14502.301171] usb 4-2.1: New USB device strings: Mfr=2, Product=3,
SerialNumber=1
[14502.301173] usb 4-2.1: Product: AS2105
[14502.301174] usb 4-2.1: Manufacturer: ASMedia
[14502.301176] usb 4-2.1: SerialNumber: 3QD0AHHA
This tells you exactly what the usb4, 4-2, and 4-2.1 devices are
(including the PCI address of the controller for usb4; it is given as
the SerialNumber string). The parent-child relations are evident from
the device names.
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: Bug 60810 - Kernel oops with controller XHCI while wait usb packet
Hi Sarah, thank you for your patch, but not fix my issue. I attached new kernel oops log into bugzilla. https://bugzilla.kernel.org/attachment.cgi?id=107363 Regards, Giovanni On Thu, 8/29/13, Sarah Sharp sarah.a.sh...@linux.intel.com wrote: Subject: Re: Bug 60810 - Kernel oops with controller XHCI while wait usb packet To: Giovanni giovanni.ne...@yahoo.com Cc: linux-usb@vger.kernel.org, Mathias Nyman mathias.ny...@linux.intel.com Date: Thursday, August 29, 2013, 8:30 PM Hi Giovanni, Mathias mentioned that this patch might fix your issue: http://marc.info/?l=linux-usbm=136269803207465w=2 Can you please compile a custom 3.10 stable kernel with that patch applied and see if it fixes your issue? Sarah Sharp On Wed, Aug 28, 2013 at 09:46:42PM -0700, Giovanni wrote: https://bugzilla.kernel.org/show_bug.cgi?id=60810 Bug ID: 60810 Summary: Kernel oops with controller XHCI while wait usb packet Regards Giovanni -- 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 -- 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 -- 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: Include parent hub number in current warning message Parent hub missing LPM exit latency info
On 08/29/2013 11:32 PM, Alan Stern wrote:
On Thu, 29 Aug 2013, Martin MOKREJŠ wrote:
[snip]
I understand your point but don't think this is the case. OK, please show me why
is LPM not available. If USB is complaining about that being disabled it could
tell me based on what attribute at least. And printing the PCI device ID at
least
would be helpful. Yeah, will look what that journald does.
Actually, it looks like the missing LPM functionality is caused by a
bug in drivers/usb/core/hcd.c. The register_root_hub() routine does
this:
if (usb_dev-speed == USB_SPEED_SUPER) {
retval = usb_get_bos_descriptor(usb_dev);
if (retval 0) {
mutex_unlock(usb_bus_list_lock);
dev_dbg(parent_dev, can't read %s bos descriptor %d\n,
dev_name(usb_dev-dev), retval);
return retval;
}
}
Compare this to the code in hub.c:
if (udev-wusb == 0 le16_to_cpu(udev-descriptor.bcdUSB) = 0x0201) {
retval = usb_get_bos_descriptor(udev);
if (!retval) {
udev-lpm_capable = usb_device_supports_lpm(udev);
usb_set_lpm_parameters(udev);
}
}
You can see that register_root_hub() doesn't set udev-lpm_capable.
The missing call to usb_set_lpm_parameters() doesn't matter, because
root hubs don't have parent hubs.
Ksenia, can you send Martin patch to set udev-lpm_capable for root
hubs? Note that this will require you to fix usb_device_supports_lpm()
as well; that routine dereferences udev-parent, but for root hubs
udev-parent is NULL.
[snip]
Alan Stern
Yes, sure! I will work on this tomorrow.
regards,
ksenia
--
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] USB2NET : SR9700 : One Chip USB 1.1 USB2NET SR9700 Device Driver Support
Keep it small though literate.
---
drivers/net/usb/sr9700.c | 30 --
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
index f7f46e6..3f05b35 100644
--- a/drivers/net/usb/sr9700.c
+++ b/drivers/net/usb/sr9700.c
@@ -71,31 +71,25 @@ static void sr_write_reg_async(struct usbnet *dev, u8 reg,
u8 value)
static int wait_phy_eeprom_ready(struct usbnet *dev, int phy)
{
- int i, ret;
+ int i;
- ret = 0;
for (i = 0; i SR_SHARE_TIMEOUT; i++) {
u8 tmp = 0;
+ int ret;
udelay(1);
ret = sr_read_reg(dev, EPCR, tmp);
if (ret 0)
- goto out;
+ return ret;
/* ready */
- if ((tmp EPCR_ERRE) == 0)
- break;
+ if (!(tmp EPCR_ERRE))
+ return 0;
}
- if (i = SR_SHARE_TIMEOUT) {
- netdev_err(dev-net, %s write timed out!\n,
- phy ? phy : eeprom);
- ret = -EIO;
- goto out;
- }
+ netdev_err(dev-net, %s write timed out!\n, phy ? phy : eeprom);
-out:
- return ret;
+ return -EIO;
}
static int sr_share_read_word(struct usbnet *dev, int phy, u8 reg,
@@ -110,7 +104,7 @@ static int sr_share_read_word(struct usbnet *dev, int phy,
u8 reg,
ret = wait_phy_eeprom_ready(dev, phy);
if (ret 0)
- goto out;
+ goto out_unlock;
sr_write_reg(dev, EPCR, 0x0);
ret = sr_read(dev, EPDR, 2, value);
@@ -118,7 +112,7 @@ static int sr_share_read_word(struct usbnet *dev, int phy,
u8 reg,
netdev_dbg(dev-net, read shared %d 0x%02x returned 0x%04x, %d\n,
phy, reg, *value, ret);
-out:
+out_unlock:
mutex_unlock(dev-phy_mutex);
return ret;
}
@@ -132,18 +126,18 @@ static int sr_share_write_word(struct usbnet *dev, int
phy, u8 reg,
ret = sr_write(dev, EPDR, 2, value);
if (ret 0)
- goto out;
+ goto out_unlock;
sr_write_reg(dev, EPAR, phy ? (reg | 0x40) : reg);
sr_write_reg(dev, EPCR, phy ? 0x1a : 0x12);
ret = wait_phy_eeprom_ready(dev, phy);
if (ret 0)
- goto out;
+ goto out_unlock;
sr_write_reg(dev, EPCR, 0x0);
-out:
+out_unlock:
mutex_unlock(dev-phy_mutex);
return ret;
}
--
1.8.3.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] USB2NET : SR9700 : One Chip USB 1.1 USB2NET SR9700 Device Driver Support
Liu, please check those. --- drivers/net/usb/sr9700.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c index 3f05b35..3ae3caf 100644 --- a/drivers/net/usb/sr9700.c +++ b/drivers/net/usb/sr9700.c @@ -99,8 +99,9 @@ static int sr_share_read_word(struct usbnet *dev, int phy, u8 reg, mutex_lock(dev-phy_mutex); + // FIXME: the mistery 0x40 appears in sr_share_write_word as well. sr_write_reg(dev, EPAR, phy ? (reg | 0x40) : reg); - sr_write_reg(dev, EPCR, phy ? 0xc : 0x4); + sr_write_reg(dev, EPCR, phy ? (EPCR_EPOS | EPCR_ERPRR) : EPCR_ERPRR); ret = wait_phy_eeprom_ready(dev, phy); if (ret 0) @@ -128,7 +129,10 @@ static int sr_share_write_word(struct usbnet *dev, int phy, u8 reg, if (ret 0) goto out_unlock; + // FIXME: see sr_share_read_word above. sr_write_reg(dev, EPAR, phy ? (reg | 0x40) : reg); + // 0x1a - EPCR_WEP | EPCR_EPOS | EPCR_ERPRW ? + // 0x12 - EPCR_WEP | EPCR_ERPRW ? sr_write_reg(dev, EPCR, phy ? 0x1a : 0x12); ret = wait_phy_eeprom_ready(dev, phy); -- 1.8.3.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: Memory synchronization vs. interrupt handlers
On Wed, Aug 28, 2013 at 01:28:08PM -0700, H. Peter Anvin wrote: On 08/28/2013 12:16 PM, Alan Stern wrote: Russell, Peter, and Ingo: Can you folks enlighten us regarding this issue for some common architectures? On x86, IRET is a serializing instruction; it guarantees hard serialization of absolutely everything. So a second interrupt from this same device could not appear to happen before the IRET, no matter what device and/or I/O bus? Or is IRET defined to synchronize all the way out to the whatever device is generating the next interrupt? I would expect architectures that have weak memory ordering to put appropriate barriers in the IRQ entry/exit code. Adding a few on CC. Also restating the question as I understand it: Suppose that a given device generates an interrupt on CPU 0, but that before CPU 0's interrupt handler completes, this device wants to generate a second interrupt on CPU 1. This can happen as soon as CPU 0's handler does an EOI or equivalent. Can CPU 1's interrupt handler assume that all the in-memory effects of CPU 0's interrupt handler will be visible, even if neither interrupt handler uses locking or memory barriers? Thanx, Paul -- 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 v2] usb:gadget:dummy_hcd : Avoid infinite loop
When an error occurs adding a udc platform device there is a risk of an
infinite loop.
If more than one platform device was added i will remain = than 0. The
intention seems
to clean up all the different already added platform devices before the failure
occurs,
so fixed the code to actually do so. We need to decrement first because the
adding at the current
index of i is the one that failed.
At the same time the code is harmonized for hcd platform adding.
Found with coverity : CID 751073
Signed-off-by: Philippe De Swert philippe.desw...@jollamobile.com
---
drivers/usb/gadget/dummy_hcd.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
index c588e8e..2e16c9a 100644
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -2684,9 +2684,8 @@ static int __init init(void)
for (i = 0; i mod_data.num; i++) {
retval = platform_device_add(the_hcd_pdev[i]);
if (retval 0) {
- i--;
- while (i = 0)
- platform_device_del(the_hcd_pdev[i--]);
+ while (i-- = 0)
+ platform_device_del(the_hcd_pdev[i]);
goto err_add_hcd;
}
}
@@ -2705,8 +2704,7 @@ static int __init init(void)
for (i = 0; i mod_data.num; i++) {
retval = platform_device_add(the_udc_pdev[i]);
if (retval 0) {
- i--;
- while (i = 0)
+ while (i-- = 0)
platform_device_del(the_udc_pdev[i]);
goto err_add_udc;
}
--
1.8.1.2
--
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 v2] usb:gadget:dummy_hcd : Detect port and link state correctly to avoid unreachable code
Since USB_SS_PORT_LS_U0 is 0x the operation with the port state would always be 0. Thus the if would never be true. Moreover USB_PORT_STAT_ENABLE is 0x0002 and as such would never equal to 1. What we actually look for is a port that is in U0/link active state. Thanks to Alan Stern for suggesting the correct test in this case. Found with coverity: CID 744367, CID 145679 Signed-off-by: Philippe De Swert philippe.desw...@jollamobile.com --- drivers/usb/gadget/dummy_hcd.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c index 2e16c9a..f5c03dd 100644 --- a/drivers/usb/gadget/dummy_hcd.c +++ b/drivers/usb/gadget/dummy_hcd.c @@ -311,10 +311,8 @@ static void set_link_state_by_speed(struct dummy_hcd *dum_hcd) USB_PORT_STAT_CONNECTION) == 0) dum_hcd-port_status |= (USB_PORT_STAT_C_CONNECTION 16); - if ((dum_hcd-port_status -USB_PORT_STAT_ENABLE) == 1 - (dum_hcd-port_status -USB_SS_PORT_LS_U0) == 1 + if ((dum_hcd-port_status USB_PORT_STAT_LINK_STATE) + == USB_SS_PORT_LS_U0 dum_hcd-rh_state != DUMMY_RH_SUSPENDED) dum_hcd-active = 1; } -- 1.8.1.2 -- 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 v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
Hi George,
On 08/29/2013 10:45 PM, George Cherian wrote:
Hi Chanwoo,
On 8/29/2013 5:42 PM, Chanwoo Choi wrote:
[big snip ]
I tested various development board based on Samsung Exynos series SoC.
Although some gpio of Exynos series SoC set high state(non zero, 1) as
default value,
this gpio state could mean off state, disconnected or un-powered state
according to gpio.
Of course, above explanation about specific gpio don't include all gpios.
This is meaning that there is possibility.
okay then how about adding a flag for inverted logic too. something like
this
if(of_property_read_bool(np,inverted_gpio))
gpio_usbvid-gpio_inv = 1;
And always check on this before deciding?
Is this fine ?
OK.
But, as Stephen commented on other mail, you should use proper DT helper
function.
Second,
gpio_usbvid_set_initial_state() dertermine both USB-HOST and USB
cable state at same time
in 'case ID_DETCT' according to 'gpio_usbvid-id_gpio'. But, I think
that other extcon devices
would not control both USB-HOST and USB cable state at same time.
Other extcon devices would support either USB-HOST or USB cable.
The driver has 2 configurations.
1) supports implementations with both VBUS and ID pin are routed via
gpio's for swicthing roles(compatible gpio-usb-vid).
As you explained about case 1, it is only used on dra7x SoC.
No gpio-usb-id is used in dra7xx. dra7xx has got only ID pin routed via
gpio.
Other SoC could not wish to control both USB-HOST and USB cable at
same time.
I could'nt actually parse this. You meant since the id_irq_handler handles
both USB and USB-HOST cable
its not proper?
It's not proper in general case. The generic driver must guarantee all of
extcon device using gpio.
As far as I know, the generic driver cannot direclty control gpio pin and get
value from gpio pin.
Almost device driver including in kernel/drivers control gpio pin on specific
device driver
instead of generic driver.
I need your answer about above my opinion for other SoC.
So how about this, I have removed the dra7x specific stuffs (gpio-usb-id)
static void gpio_usbvid_set_initial_state(struct gpio_usbvid *gpio_usbvid)
{
int id_current, vbus_current;
id_current = gpio_get_value_cansleep(gpio_usbvid-id_gpio);
if (!!id_current == ID_FLOAT)
extcon_set_cable_state(gpio_usbvid-edev, USB-HOST, false);
else
extcon_set_cable_state(gpio_usbvid-edev, USB-HOST, true);
vbus_current = gpio_get_value_cansleep(gpio_usbvid-vbus_gpio);
if (!!vbus_current == VBUS_ON)
extcon_set_cable_state(gpio_usbvid-edev, USB, true);
else
extcon_set_cable_state(gpio_usbvid-edev, USB, false);
}
and the irq handlers like this
static irqreturn_t id_irq_handler(int irq, void *data)
{
struct gpio_usbvid *gpio_usbvid = (struct gpio_usbvid *)data;
int id_current;
id_current = gpio_get_value_cansleep(gpio_usbvid-id_gpio);
if (id_current == ID_GND)
extcon_set_cable_state(gpio_usbvid-edev, USB-HOST, true);
else
extcon_set_cable_state(gpio_usbvid-edev, USB-HOST, false);
return IRQ_HANDLED;
}
static irqreturn_t vbus_irq_handler(int irq, void *data)
{
struct gpio_usbvid *gpio_usbvid = (struct gpio_usbvid *)data;
int vbus_current;
vbus_current = gpio_get_value_cansleep(gpio_usbvid-vbus_gpio);
if (vbus_current == VBUS_OFF)
extcon_set_cable_state(gpio_usbvid-edev, USB, false);
else
extcon_set_cable_state(gpio_usbvid-edev, USB, true);
return IRQ_HANDLED;
}
I know your intention dividing interrupt handler for each cable.
But I don't think this driver must guarantee all of extcon device using gpio.
For example,
below three SoC wish to detect USB/USB-HOST cable with each different methods.
SoC A wish to detect USB/USB-HOST cable through only one gpio pin.
SoC B wish to detect USB/USB-HOST cable through ADC value instead of gpio pin.
SoC C wish to detect USB/USB-HOST cable through two gpio pin because USB
connected on gpio an USB-HOST connected on another.
In addition,
if SoC A/C wish to write some data to own specific registers for proper
opeating,
Could we write some data to register on generic driver?
Finally,
SoC D wish to detect USB/USB-HOST/JIG cable through two gpio pin
- one gpio may detect either USB or USB-HOST and another may detect JIG cable
or one gpio may detect either USb or JIG and another may detect USB-HOST cable.
That way, there are many cases we cannot guarantee all of extcon devices.
I think this driver could support dra7x series SoC but as I mentioned,
this driver may not guarantee all of cases.
[snip]
I have some confusion. I need additional your explanation.
Could this driver register only one interrupt handler either
id_irq_handler() or vbus_irq_handler()?
If compatible == ID_DETECT, only one handler --
Re: Memory synchronization vs. interrupt handlers
On Thu, 2013-08-29 at 16:51 -0700, Paul E. McKenney wrote: On Wed, Aug 28, 2013 at 01:28:08PM -0700, H. Peter Anvin wrote: On 08/28/2013 12:16 PM, Alan Stern wrote: Russell, Peter, and Ingo: Can you folks enlighten us regarding this issue for some common architectures? On x86, IRET is a serializing instruction; it guarantees hard serialization of absolutely everything. So a second interrupt from this same device could not appear to happen before the IRET, no matter what device and/or I/O bus? Or is IRET defined to synchronize all the way out to the whatever device is generating the next interrupt? I would expect architectures that have weak memory ordering to put appropriate barriers in the IRQ entry/exit code. Not sure why you would expect that, there is no reason to do such a thing. Adding a few on CC. Also restating the question as I understand it: Suppose that a given device generates an interrupt on CPU 0, but that before CPU 0's interrupt handler completes, this device wants to generate a second interrupt on CPU 1. This can happen as soon as CPU 0's handler does an EOI or equivalent. By interrupt handler are you talking about the general handling of all interrupts in the kernel or the device specific interrupt handler ? Typically the EOI is done after the later has run. Can CPU 1's interrupt handler assume that all the in-memory effects of CPU 0's interrupt handler will be visible, even if neither interrupt handler uses locking or memory barriers? We don't formally provide such a guarantee no. There is a spin_lock on the way back from the device handler and before the EOI but not an unlock so we don't have a full ordering here (see handle_irq_event). At least on powerpc, the EOI will generally be an MMIO which will however synchronize everything because we have a sync before the store in our MMIO accessors, but that might not be true when running under the pHyp hypervisor, as we do a hypercall there and I don't think that includes a sync instruction inside the hypervisor. So I'd say that as-is, no, we don't provide that guarantee. Cheers, Ben. Thanx, Paul -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- 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 RESEND 3/4] usb: phy-tegra-usb: use platform_{get,set}_drvdata()
Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with of-dev,
so we can directly pass a struct platform_device.
Signed-off-by: Libo Chen libo.c...@huawei.com
---
drivers/usb/phy/phy-tegra-usb.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
rebase on usb-next tree
diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 3bfb3d1..e9cb1cb 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -1064,7 +1064,7 @@ static int tegra_usb_phy_probe(struct platform_device
*pdev)
tegra_phy-u_phy.shutdown = tegra_usb_phy_close;
tegra_phy-u_phy.set_suspend = tegra_usb_phy_suspend;
- dev_set_drvdata(pdev-dev, tegra_phy);
+ platform_set_drvdata(pdev, tegra_phy);
err = usb_add_phy_dev(tegra_phy-u_phy);
if (err 0) {
--
1.7.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
[PATCH RESEND 2/4] usb: r8a66597-hcd: use platform_{get,set}_drvdata()
Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with of-dev,
so we can directly pass a struct platform_device.
Signed-off-by: Libo Chen libo.c...@huawei.com
---
drivers/usb/host/r8a66597-hcd.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
rebase on usb-next tree
diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
index a9eef68..2ad004a 100644
--- a/drivers/usb/host/r8a66597-hcd.c
+++ b/drivers/usb/host/r8a66597-hcd.c
@@ -2393,7 +2393,7 @@ static const struct dev_pm_ops r8a66597_dev_pm_ops = {
static int r8a66597_remove(struct platform_device *pdev)
{
- struct r8a66597 *r8a66597 = dev_get_drvdata(pdev-dev);
+ struct r8a66597 *r8a66597 = platform_get_drvdata(pdev);
struct usb_hcd *hcd = r8a66597_to_hcd(r8a66597);
del_timer_sync(r8a66597-rh_timer);
@@ -2466,7 +2466,7 @@ static int r8a66597_probe(struct platform_device *pdev)
}
r8a66597 = hcd_to_r8a66597(hcd);
memset(r8a66597, 0, sizeof(struct r8a66597));
- dev_set_drvdata(pdev-dev, r8a66597);
+ platform_set_drvdata(pdev, r8a66597);
r8a66597-pdata = dev_get_platdata(pdev-dev);
r8a66597-irq_sense_low = irq_trigger == IRQF_TRIGGER_LOW;
--
1.7.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: Memory synchronization vs. interrupt handlers
On 08/29/2013 04:51 PM, Paul E. McKenney wrote: On Wed, Aug 28, 2013 at 01:28:08PM -0700, H. Peter Anvin wrote: On 08/28/2013 12:16 PM, Alan Stern wrote: Russell, Peter, and Ingo: Can you folks enlighten us regarding this issue for some common architectures? On x86, IRET is a serializing instruction; it guarantees hard serialization of absolutely everything. So a second interrupt from this same device could not appear to happen before the IRET, no matter what device and/or I/O bus? Or is IRET defined to synchronize all the way out to the whatever device is generating the next interrupt? The second interrupt from this same device can occur as soon as the EOI cycle is done, which happens before the IRET. The EOI cycle is an I/O operation and since integer operations to memory are strongly ordered that implies all other effects are globally visible. In addition, there is usually synchronization that happens due to reading an interrupt status register or something else. I would expect architectures that have weak memory ordering to put appropriate barriers in the IRQ entry/exit code. Adding a few on CC. Also restating the question as I understand it: Suppose that a given device generates an interrupt on CPU 0, but that before CPU 0's interrupt handler completes, this device wants to generate a second interrupt on CPU 1. This can happen as soon as CPU 0's handler does an EOI or equivalent. Can CPU 1's interrupt handler assume that all the in-memory effects of CPU 0's interrupt handler will be visible, even if neither interrupt handler uses locking or memory barriers? On x86 it certainly can. -hpa -- 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] USB2NET : SR9700 : One Chip USB 1.1 USB2NET SR9700 Device Driver Support
On Fri, 2013-08-30 at 01:06 +0200, Francois Romieu wrote: just trivia: diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c [] @@ -288,19 +291,19 @@ static void sr9700_set_multicast(struct net_device *net) [] +static int sr9700_set_mac_address(struct net_device *netdev, void *p) [] - memcpy(net-dev_addr, addr-sa_data, net-addr_len); - sr_write_async(dev, PAR, 6, dev-net-dev_addr); + memcpy(netdev-dev_addr, addr-sa_data, netdev-addr_len); + sr_write_async(dev, PAR, 6, netdev-dev_addr); s/6/ETH_ALEN/ -- 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
