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 a
[no subject]
My wife and I won the Euro Millions Lottery of 41 Million British Pounds and we have decided to donate 1.5 million British Pounds to 6 individuals worldwide as our own charity project. Your email address was among the emails which were submitted to us by the Google, Inc as a web user, which was used for the draw with an electronic balloting system your email address came out as the 4th lucky beneficiary world wide. To verify, please see our interview by visiting the web page below: http://www.dailymail.co.uk/news/article-2091124/EuroMillions-winners-Gareth-Catherine-Bull-scoop-41MILLION-lotto-jackpot.html Send the below information for urgent processing. Full Name: Mobile No: Age: Country: Send your response to Congratulations once again, Best Regards, Gareth & Catherine Bull gcb.foundat...@postafiok.hu -- 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,
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: [PATCH v2 0/5] Chipidea Misc patchset
Peter Chen 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. Regards, -- Alex -- 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 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 >> wrote: >> Still noone answered me why "drivers/usb/host/xhci-ring.c" does NOT >> include (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 >> 68:#include >> 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 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 writes: > From: Rob Gardner > 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
---
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",
-
[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
---
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 sche
[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
---
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 --- 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
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 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
> ---
>
> 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 simila
Re: [PATCH v2 0/5] Chipidea Misc patchset
On Thu, Aug 29, 2013 at 10:53:50AM +0300, Alexander Shishkin wrote: > Peter Chen 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 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 m
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"
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 c
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- > SERR- 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- > SERR- 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- > SERR- 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: 0 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.o
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 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 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 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 Signed-off-by: Alan Stern 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: [RFC 2/3] EHCI: convert the IRQ handler to a tasklet
On Thu, 29 Aug 2013, Ming Lei wrote: > > 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. Good. At this point, I don't think anybody is planning to do it. But just in case somebody does, I would prefer not to remove the code that handles races between processing and submission. 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: 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: > >> 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 ... -- 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 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
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: 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: > >> 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. 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 -- 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 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-next&id=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" > > >>> > > >>> 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 > > >>> --- > > >>> 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: 3.4.4: disabling irq
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? > 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. > 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. Udo -- 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] 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
---
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: [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 wrote: > On Thu, 29 Aug 2013, James Stone wrote: > >> On Wed, Aug 28, 2013 at 7:46 PM, Alan Stern >> 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 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 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Š Tested-by: Martin MOKREJŠ and resend as PATCH v2 or just PATCH? Also does it need a Reviewed-by: Alan Stern ? 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 > > 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: [PATCH] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On Thu, Aug 29, 2013 at 09:16:56PM +0300, Xenia Ragiadakou wrote: > 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 > > 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Š > Tested-by: Martin MOKREJŠ Yes. > and resend as PATCH v2 or just PATCH? v2 please. > Also does it need a > Reviewed-by: Alan Stern ? If he said that in a review message, yes, please add it. 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, Aug 29, 2013 at 11:18:21AM -0700, 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.
Which device is "buggy" the USB 3 device (in this case, a USB flash
drive), or the USB host controller in the laptop?
> 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?
We should, but as I don't see that error message, is that what is really
happening here?
Shouldn't everyone with a USB3 device be seing these messages if it's a
disconnect issue?
> The least we could do is change that dev_warn to dev_dbg instead.
Ok, I can do that :)
But, given that this should happen for all USB3 devices that are
removed, I think the root cause should be fixed, otherwise it will just
be annoying debug messages that we can't do anything about, right?
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: 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-usb&m=136269803207465&w=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: [PATCH] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On Thu, Aug 29, 2013 at 11:22:46AM -0700, Sarah Sharp wrote: > 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 > > > > 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). It's in SubmittingPatches already :) -- 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, ®s->intrdisable);
+ ohci_writel(ohci, ~0, ®s->intrstatus);
+ return IRQ_NOTMINE;
+ }
+
/* We only care about interrupts that are enabled */
ints &= ohci_readl(ohci, ®s->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: Include parent hub number in current warning message "Parent hub missing LPM exit latency info"
Xenia Ragiadakou wrote: > 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. So is that a bug in pciutils or in xhci or something writing a value to procfs? I don't understand kernel at all so to me your answer sounds quite good. ;) 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: [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
Reported-by: Martin MOKREJS
Tested-by: Martin MOKREJS
Suggested-by: Alan Stern
---
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 wrote: Subject: Re: Bug 60810 - Kernel oops with controller XHCI while wait usb packet To: "Giovanni" Cc: linux-usb@vger.kernel.org, "Mathias Nyman" Date: Thursday, August 29, 2013, 8:30 PM Hi Giovanni, Mathias mentioned that this patch might fix your issue: http://marc.info/?l=linux-usb&m=136269803207465&w=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
- use all columns
- the driver uses both 'netdev' and 'net'. Let's use 'netdev' ('net' is
shorter but it tastes of network namespaces)
- don't dereference dev->net when there is a local netdev at hand
- use some existing #define (please check those)
- skb_set_tail_pointer to avoid compiler cast warnings
Pick whatever you want. skb_set_tail_pointer if nothing else.
---
drivers/net/usb/sr9700.c | 113 +--
1 file changed, 60 insertions(+), 53 deletions(-)
diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
index 76e11f5..f7f46e6 100644
--- a/drivers/net/usb/sr9700.c
+++ b/drivers/net/usb/sr9700.c
@@ -28,8 +28,8 @@ static int sr_read(struct usbnet *dev, u8 reg, u16 length,
void *data)
{
int err;
- err = usbnet_read_cmd(dev, SR_RD_REGS, SR_REQ_RD_REG,
- 0, reg, data, length);
+ err = usbnet_read_cmd(dev, SR_RD_REGS, SR_REQ_RD_REG, 0, reg, data,
+ length);
if ((err != length) && (err >= 0))
err = -EINVAL;
return err;
@@ -39,8 +39,8 @@ static int sr_write(struct usbnet *dev, u8 reg, u16 length,
void *data)
{
int err;
- err = usbnet_write_cmd(dev, SR_WR_REGS, SR_REQ_WR_REG,
- 0, reg, data, length);
+ err = usbnet_write_cmd(dev, SR_WR_REGS, SR_REQ_WR_REG, 0, reg, data,
+ length);
if ((err >= 0) && (err < length))
err = -EINVAL;
return err;
@@ -158,11 +158,11 @@ static int sr9700_get_eeprom_len(struct net_device *dev)
return SR_EEPROM_LEN;
}
-static int sr9700_get_eeprom(struct net_device *net,
+static int sr9700_get_eeprom(struct net_device *netdev,
struct ethtool_eeprom *eeprom, u8 *data)
{
- struct usbnet *dev = netdev_priv(net);
- __le16 *ebuf = (__le16 *)data;
+ struct usbnet *dev = netdev_priv(netdev);
+ __le16 *buf = (__le16 *)data;
int ret = 0;
int i;
@@ -170,9 +170,11 @@ static int sr9700_get_eeprom(struct net_device *net,
if ((eeprom->offset & 0x01) || (eeprom->len & 0x01))
return -EINVAL;
- for (i = 0; i < eeprom->len / 2; i++)
- ret = sr_read_eeprom_word(dev, eeprom->offset / 2 + i,
- &ebuf[i]);
+ for (i = 0; i < eeprom->len / 2; i++) {
+ ret = sr_read_eeprom_word(dev, eeprom->offset / 2 + i, buf + i);
+ if (ret < 0)
+ break;
+ }
return ret;
}
@@ -184,7 +186,7 @@ static int sr_mdio_read(struct net_device *netdev, int
phy_id, int loc)
int rc = 0;
if (phy_id) {
- netdev_dbg(dev->net, "Only internal phy supported\n");
+ netdev_dbg(netdev, "Only internal phy supported\n");
return 0;
}
@@ -202,7 +204,7 @@ static int sr_mdio_read(struct net_device *netdev, int
phy_id, int loc)
else
res = le16_to_cpu(res) & ~BMSR_LSTATUS;
- netdev_dbg(dev->net, "sr_mdio_read() phy_id=0x%02x, loc=0x%02x,
returns=0x%04x\n",
+ netdev_dbg(netdev, "sr_mdio_read() phy_id=0x%02x, loc=0x%02x,
returns=0x%04x\n",
phy_id, loc, res);
return res;
@@ -215,19 +217,19 @@ static void sr_mdio_write(struct net_device *netdev, int
phy_id, int loc,
__le16 res = cpu_to_le16(val);
if (phy_id) {
- netdev_dbg(dev->net, "Only internal phy supported\n");
+ netdev_dbg(netdev, "Only internal phy supported\n");
return;
}
- netdev_dbg(dev->net, "sr_mdio_write() phy_id=0x%02x, loc=0x%02x,
val=0x%04x\n",
+ netdev_dbg(netdev, "sr_mdio_write() phy_id=0x%02x, loc=0x%02x,
val=0x%04x\n",
phy_id, loc, val);
sr_share_write_word(dev, 1, loc, res);
}
-static u32 sr9700_get_link(struct net_device *net)
+static u32 sr9700_get_link(struct net_device *netdev)
{
- struct usbnet *dev = netdev_priv(net);
+ struct usbnet *dev = netdev_priv(netdev);
u8 value = 0;
int rc = 0;
@@ -239,9 +241,9 @@ static u32 sr9700_get_link(struct net_device *net)
return rc;
}
-static int sr9700_ioctl(struct net_device *net, struct ifreq *rq, int cmd)
+static int sr9700_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
{
- struct usbnet *dev = netdev_priv(net);
+ struct usbnet *dev = netdev_priv(netdev);
return generic_mii_ioctl(&dev->mii, if_mii(rq), cmd, NULL);
}
@@ -258,9 +260,9 @@ static const struct ethtool_ops sr9700_ethtool_ops = {
.nway_reset = usbnet_nway_reset,
};
-static void sr9700_set_multicast(struct net_device *net)
+static void sr9700_set_multicast(struct net_device *netdev)
{
- struct usbnet *dev = netdev_priv(net);
+ struct usbnet *dev = netdev_priv(netdev);
/* We use the 20 byte dev->data fo
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
---
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 0/2 v2] Various fixes for the dummy_hcd driver
These two patches fix some minor issues in the dummy_hcd driver. Both errors were detected with the help of coverity analysis. These patches have been updated based on Alan Sterns comments. For the first patch I have harmonized both occurences of the different clean-up routine as suggested. The second patch should now actually have the correct test. Since I did not find USB_SS_PORT_LINK_STATE defined I checked include/uapi/linux/usb/ch11.h and figured out that USB_PORT_STAT_LINK_STATE has the right value we look for to isolate bits 5-8. I left the dum_hcd->rh_state != DUMMY_RH_SUSPENDED check in, but it might not be needed. I can update that if needed. Thank you. Philippe De Swert (2): usb:gadget:dummy_hcd : Avoid infinite loop usb:gadget:dummy_hcd : Detect port and link state correctly to avoid unreachable code drivers/usb/gadget/dummy_hcd.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) -- 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 --- 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 guara
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
---
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
---
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
Re: [PATCH v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
Hi Chanwoo,
On 8/30/2013 5:41 AM, Chanwoo Choi wrote:
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.
okay
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.
But without reading the gpio value how can we set the cable states? for
this driver the assumption is the dedicated gpio
is always DIR_IN and in the driver we just read the value.
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.
You mean to say that both USB ID pin and VBUS are connected to same gpio?
If so that is a really bad h/w design and it will not fly.
Or, you meant that only USB ID pin is connected to single gpio and it
controls the state of the USB/USB-HOST cables?
If so thats what exactly the v3 driver did with compatible gpio-usb-id.
"SoC B" wish to detect USB/USB-HOST cable through ADC value instead of gpio pin.
Via ADC this driver should never be used , it should be
extcon-adc-usbvid driver and not extcon-gpio-usbvid driver.
"SoC C" wish to detect USB/USB-HOST cable through two gpio pin because USB
connected on gpio an USB-HOST connected on another.
Whatever modifications above should meet this need in combination with
named gpios (id_gpio and vbus_gp
Re: [PATCH v3 1/3] extcon: extcon-gpio-usbvid: Generic USB VBUS/ID detection via GPIO
Hi George,
On 08/30/2013 03:15 PM, George Cherian wrote:
> Hi Chanwoo,
>
> On 8/30/2013 5:41 AM, Chanwoo Choi wrote:
>> 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.
> okay
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.
> But without reading the gpio value how can we set the cable states? for this
> driver the assumption is the dedicated gpio
> is always DIR_IN and in the driver we just read the value.
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.
>
> You mean to say that both USB ID pin and VBUS are connected to same gpio?
> If so that is a really bad h/w design and it will not fly.
> Or, you meant that only USB ID pin is connected to single gpio and
