Re: Huawei E3131 wwan interface
Am Montag, 1. Juli 2013, 00:37:14 schrieben Sie: Hi! I'm trying to investigate on the Huawei E3131 wwan interface - Is the E3131 an HILINK-Modem? Have you tried simply starting dhcp/automatic-configuration? Open a Browser to get connected. If dns-redirection doesn't work, try the router or dns/resolver-address. http://192.168.x.x. ip route | grep default cat /etc/resolv.conf Regards, Thomas -- 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
EHCI debug device gadgets and host
Hi Is someone actively working on or maintaining the EHCI debug device gadget driver? I would like to see some fixes and/or improvements to it but do not know the gadget framework and some of the required hardware is not yet on my desk. So the incomplete report here is from a friends' experiment. The behaviour was experienced on both BeagleBone Black and GTA04 used as the gadget side. These and other recent ARM boards could be used for kernel debugging of x86 platforms where EHCI debug port is available and there is no classic serial port. The problem is with CONFIG_G_DBGP_SERIAL disconnecting the TTY on USB resets. While this may be correct behaviour for most use cases, it means when connected to a debug target with EHCI debug port in kernel, the TTY link does not survive over target board USB reset signalling. Also in my use case (debugging coreboot) we may need one or two USB resets even before kernel. For the debug target side, I have previously written some patches to get compatibility with USB FX2 chips to be used as a replacement debug device now that the Net20DC part is hard to get. I hope we could discuss if some patches here are relevant for upstreaming: https://bitbucket.org/kmalkki/linux-kernel/commits/branch/ehci-dbgp-test2 Regards, KM -- 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: Huawei E3131 wwan interface
Enrico Mioso mrkiko...@gmail.com writes:
Hi!
I'm trying to investigate on the Huawei E3131 wwan interface - like the E398,
this device ignores the at^ndisdup command.
I wanted to verify if my device was running a jungo firmware or not, since I
found contraddictory informations on the network.
To this end, I patched the cdc_ncm driver with a function was responsible for
the initializzation of jungo devices:
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..79770ed 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -39,6 +39,7 @@
*/
#include linux/module.h
+#include linux/usbdevice_fs.h
#include linux/init.h
#include linux/netdevice.h
#include linux/ctype.h
@@ -352,6 +353,37 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
.nway_reset = usbnet_nway_reset,
};
+int hw_send_tlp_download_request(struct usb_interface *intf){
+ printk(Entering hw_send_tlp_download_request...\n);
+ struct usb_device *udev = interface_to_usbdev(intf);
+ struct usb_host_interface *interface = intf-cur_altsetting;
+ struct usbdevfs_ctrltransfer req = {0};
+//memset?
+ unsigned char buf[256] = {0};
+ int retval = 0;
+ req.bRequestType = 0xC0;
+ req.bRequest = 0x02;
+ req.wIndex = interface-desc.bInterfaceNumber;
+ req.wValue = 1;
+ req.wLength = 1;
+ req.data = buf;
+ req.timeout = 1000;
+ retval = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), req.bRequest,
req.bRequestType, req.wValue, req.wIndex, req.data, req.wLength, req.timeout);
+ /*
+ Check the TLP feature is activated or not, response value 0x01
+ indicates success
+ */
+ if (0 retval 0x01 == buf[0]){
+ printk(Success!\n);
+ return retval; /* succeeded */
+ }
+ else
+ {
+ printk(Failed...\n);
+ return 0; /* failed */
+ }
+}
+
int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8
data_altsetting)
{
struct cdc_ncm_ctx *ctx;
@@ -1142,7 +1174,10 @@ static int cdc_ncm_check_connect(struct usbnet *dev)
static int
cdc_ncm_probe(struct usb_interface *udev, const struct usb_device_id *prod)
{
- return usbnet_probe(udev, prod);
+ int retval;
+ retval = usbnet_probe(udev, prod);
+ hw_send_tlp_download_request(udev);
+ return retval;
}
static void cdc_ncm_disconnect(struct usb_interface *intf)
and I'm hitting the failed case. So, can I say I'm not running a
jungo-based
firmware?
This code enables the Qualcomm Thin Layer Protocol, which is a
proprietary and undocumented (as far as open source goes, at least)
protocol for frame bundling. I don't think it's related to Jungo or not
at all. If this protocol is implemented, then it is at a lower firmware
level (the Qualcomm firmware).
Enabling both NCM and TLP simultaneously does not make sense because
they do pretty much the same. So this could be the reason your request
fails. Or it could be becuase the device isn't a Qualcomm device at
all?
In any case, as far as I have been able to figure out, the TLP support
is entirely optional. If you don't enable it then the device will work
just fine, without any frame bundling. This is one of the reasons I have
decided not to implement support for it. The others being that the
protocol is undocumented, provides no measurable gain, and there are
standardized alternatives like NCM and MBIM available. Even RNDIS is
better documented. TLP is dead because the vendor chose to not make it
public. Let it rest in peace.
The Huawei driver does Jungo detection simply by looking at the
bcdDevice value. You'll find a few tests like this:
HW_JUNGO_BCDDEVICE_VALUE != dev-udev-descriptor.bcdDevice
where the constant is defined as 0x0102.
But you should note that we don't care unless there is a real quirk
needed. We try to make the drivers generic enough to handle any device
with a specific device ID or class code.
Some of the tests in the Huawei driver is only to know whether to expect
one or two interfaces for the function. We already deal with this by
looking at the Union descriptor. And most (all?) of the other tests are
there to know whether or not to use QMI. Which we figure out just fine
based on class codes for Huawei devices.
So I don't think you need to know where Huawei got the firmware for the
device. Focus on the features in that firmware instead, and try to
detect and support those features.
And - another note.
I never used ncm devices, so I don't know if there is a way to communicate
with
the device directly via ncm protocol (i.e.: not using AT).
Not any standardized way, but NCM has the same support for embedded
vendor specific management protocols as ECM. So it could have something
like QMI embedded the same way we have seen on devices supported by
qmi_wwan. Or RNDIS.
When I try to bring up
[PATCH 2/5] usb: phy: protect against NULL phy pointers
In order to decrease the amount of work done
by PHY users, allow NULL phy pointers to be
passed.
Signed-off-by: Felipe Balbi ba...@ti.com
---
include/linux/usb/phy.h | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index 4403680..6c0b1c5 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -142,7 +142,7 @@ extern void usb_remove_phy(struct usb_phy *);
/* helpers for direct access thru low-level io interface */
static inline int usb_phy_io_read(struct usb_phy *x, u32 reg)
{
- if (x-io_ops x-io_ops-read)
+ if (x x-io_ops x-io_ops-read)
return x-io_ops-read(x, reg);
return -EINVAL;
@@ -150,7 +150,7 @@ static inline int usb_phy_io_read(struct usb_phy *x, u32
reg)
static inline int usb_phy_io_write(struct usb_phy *x, u32 val, u32 reg)
{
- if (x-io_ops x-io_ops-write)
+ if (x x-io_ops x-io_ops-write)
return x-io_ops-write(x, val, reg);
return -EINVAL;
@@ -159,7 +159,7 @@ static inline int usb_phy_io_write(struct usb_phy *x, u32
val, u32 reg)
static inline int
usb_phy_init(struct usb_phy *x)
{
- if (x-init)
+ if (x x-init)
return x-init(x);
return 0;
@@ -168,14 +168,14 @@ usb_phy_init(struct usb_phy *x)
static inline void
usb_phy_shutdown(struct usb_phy *x)
{
- if (x-shutdown)
+ if (x x-shutdown)
x-shutdown(x);
}
static inline int
usb_phy_vbus_on(struct usb_phy *x)
{
- if (!x-set_vbus)
+ if (!x || !x-set_vbus)
return 0;
return x-set_vbus(x, true);
@@ -184,7 +184,7 @@ usb_phy_vbus_on(struct usb_phy *x)
static inline int
usb_phy_vbus_off(struct usb_phy *x)
{
- if (!x-set_vbus)
+ if (!x || !x-set_vbus)
return 0;
return x-set_vbus(x, false);
@@ -258,7 +258,7 @@ usb_phy_set_power(struct usb_phy *x, unsigned mA)
static inline int
usb_phy_set_suspend(struct usb_phy *x, int suspend)
{
- if (x-set_suspend != NULL)
+ if (x x-set_suspend != NULL)
return x-set_suspend(x, suspend);
else
return 0;
@@ -267,7 +267,7 @@ usb_phy_set_suspend(struct usb_phy *x, int suspend)
static inline int
usb_phy_notify_connect(struct usb_phy *x, enum usb_device_speed speed)
{
- if (x-notify_connect)
+ if (x x-notify_connect)
return x-notify_connect(x, speed);
else
return 0;
@@ -276,7 +276,7 @@ usb_phy_notify_connect(struct usb_phy *x, enum
usb_device_speed speed)
static inline int
usb_phy_notify_disconnect(struct usb_phy *x, enum usb_device_speed speed)
{
- if (x-notify_disconnect)
+ if (x x-notify_disconnect)
return x-notify_disconnect(x, speed);
else
return 0;
--
1.8.2.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 1/5] usb: dwc3: switch to GPL v2 only
This is a Linux-only driver which makes use of GPL-only symbols. It makes no sense to maintain Dual BSD/GPL licensing for this driver. Considering that the amount of work to use this driver in any different operating system would likely be as large as developing the driver from scratch and considering that we depend on GPL-only symbols, we will switch over to a GPL v2-only license. Cc: Sebastian Andrzej Siewior bige...@linutronix.de Cc: Anton Tikhomirov av.tikhomi...@samsung.com Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/usb/dwc3/core.c| 37 ++--- drivers/usb/dwc3/core.h| 34 +++--- drivers/usb/dwc3/debug.h | 34 +++--- drivers/usb/dwc3/debugfs.c | 34 +++--- drivers/usb/dwc3/dwc3-exynos.c | 14 +- drivers/usb/dwc3/dwc3-omap.c | 36 drivers/usb/dwc3/dwc3-pci.c| 36 drivers/usb/dwc3/ep0.c | 34 +++--- drivers/usb/dwc3/gadget.c | 34 +++--- drivers/usb/dwc3/gadget.h | 34 +++--- drivers/usb/dwc3/host.c| 34 +++--- drivers/usb/dwc3/io.h | 34 +++--- 12 files changed, 91 insertions(+), 304 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index c35d49d..41feedb 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -6,34 +6,17 @@ * Authors: Felipe Balbi ba...@ti.com, * Sebastian Andrzej Siewior bige...@linutronix.de * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - *notice, this list of conditions, and the following disclaimer, - *without modification. - * 2. Redistributions in binary form must reproduce the above copyright - *notice, this list of conditions and the following disclaimer in the - *documentation and/or other materials provided with the distribution. - * 3. The names of the above-listed copyright holders may not be used - *to endorse or promote products derived from this software without - *specific prior written permission. + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 of + * the License as published by the Free Software Foundation. * - * ALTERNATIVELY, this software may be distributed under the terms of the - * GNU General Public License (GPL) version 2, as published by the Free - * Software Foundation. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS AS - * IS AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, - * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR - * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR - * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, - * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, - * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR - * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF - * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING - * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. */ #include linux/module.h @@ -775,5 +758,5 @@ module_platform_driver(dwc3_driver); MODULE_ALIAS(platform:dwc3); MODULE_AUTHOR(Felipe Balbi ba...@ti.com); -MODULE_LICENSE(Dual BSD/GPL); +MODULE_LICENSE(GPL v2); MODULE_DESCRIPTION(DesignWare USB3 DRD Controller Driver); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index c4b5127..b847728 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -6,34 +6,14 @@ * Authors: Felipe Balbi ba...@ti.com, * Sebastian Andrzej Siewior bige...@linutronix.de * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - *notice, this list of conditions, and the following disclaimer, - *without modification. - * 2. Redistributions in binary form must reproduce the above copyright - *notice, this list
[PATCH 5/5] usb: dwc3: make maximum-speed a per-instance attribute
in order to allow different instances of the
core work in different maximum speeds, we will
move the maximum_speed module_parameter to
both DeviceTree (making use the new maximum-speed
DT property) and platform_data.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/core.c | 24 +---
drivers/usb/dwc3/gadget.c| 21 ++---
drivers/usb/dwc3/platform_data.h | 3 +++
3 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5d6372e..9893301 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -36,6 +36,7 @@
#include linux/usb/otg.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
+#include linux/usb/of.h
#include platform_data.h
#include core.h
@@ -44,10 +45,6 @@
#include debug.h
-static char *maximum_speed = super;
-module_param(maximum_speed, charp, 0);
-MODULE_PARM_DESC(maximum_speed, Maximum supported speed.);
-
/* --
*/
void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
@@ -412,17 +409,25 @@ static int dwc3_probe(struct platform_device *pdev)
}
if (node) {
+ dwc-maximum_speed = of_usb_get_maximum_speed(node);
+
dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
dwc-needs_fifo_resize = of_property_read_bool(node,
tx-fifo-resize);
} else {
+ dwc-maximum_speed = pdata-maximum_speed;
+
dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
}
+ /* default to superspeed if no maximum_speed passed */
+ if (dwc-maximum_speed == USB_SPEED_UNKNOWN)
+ dwc-maximum_speed = USB_SPEED_SUPER;
+
if (IS_ERR(dwc-usb2_phy)) {
ret = PTR_ERR(dwc-usb2_phy);
@@ -467,17 +472,6 @@ static int dwc3_probe(struct platform_device *pdev)
dev-dma_parms = dev-parent-dma_parms;
dma_set_coherent_mask(dev, dev-parent-coherent_dma_mask);
- if (!strncmp(super, maximum_speed, 5))
- dwc-maximum_speed = DWC3_DCFG_SUPERSPEED;
- else if (!strncmp(high, maximum_speed, 4))
- dwc-maximum_speed = DWC3_DCFG_HIGHSPEED;
- else if (!strncmp(full, maximum_speed, 4))
- dwc-maximum_speed = DWC3_DCFG_FULLSPEED1;
- else if (!strncmp(low, maximum_speed, 3))
- dwc-maximum_speed = DWC3_DCFG_LOWSPEED;
- else
- dwc-maximum_speed = DWC3_DCFG_SUPERSPEED;
-
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);
pm_runtime_forbid(dev);
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0c89338..0d50147 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1525,10 +1525,25 @@ static int dwc3_gadget_start(struct usb_gadget *g,
* STAR#9000525659: Clock Domain Crossing on DCTL in
* USB 2.0 Mode
*/
- if (dwc-revision DWC3_REVISION_220A)
+ if (dwc-revision DWC3_REVISION_220A) {
reg |= DWC3_DCFG_SUPERSPEED;
- else
- reg |= dwc-maximum_speed;
+ } else {
+ switch (dwc-maximum_speed) {
+ case USB_SPEED_LOW:
+ reg |= DWC3_DSTS_LOWSPEED;
+ break;
+ case USB_SPEED_FULL:
+ reg |= DWC3_DSTS_FULLSPEED1;
+ break;
+ case USB_SPEED_HIGH:
+ reg |= DWC3_DSTS_HIGHSPEED;
+ break;
+ case USB_SPEED_SUPER: /* FALLTHROUGH */
+ case USB_SPEED_UNKNOWN: /* FALTHROUGH */
+ default:
+ reg |= DWC3_DSTS_SUPERSPEED;
+ }
+ }
dwc3_writel(dwc-regs, DWC3_DCFG, reg);
dwc-start_config_issued = false;
diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
index 0385164..16ffe19 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -17,6 +17,9 @@
* along with this program. If not, see http://www.gnu.org/licenses/.
*/
+#include linux/usb/ch9.h
+
struct dwc3_platform_data {
+ enum usb_device_speed maximum_speed;
bool tx_fifo_resize;
};
--
1.8.2.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 0/5] DWC3 and PHY patches
Hi folks, here's a small series of patches. We're switching dwc3 to GPL v2 only, protecting our USB PHY layer against NULL pointers, introducing of_usb_get_maximum_speed() and making sure DWC3's maximum-speed is a per-instance attribute. While at that, we also make sure that tx-fifo-resize can be used by non-DT platforms through platform_data. A small cleanup could be added here which would be to refactor DT and non-DT initialization to their own functions. cheers Felipe Balbi (5): usb: dwc3: switch to GPL v2 only usb: phy: protect against NULL phy pointers usb: common: introduce of_usb_get_maximum_speed() usb: dwc3: let non-DT platforms pass tx-fifo-resize flag; usb: dwc3: make maximum-speed a per-instance attribute drivers/usb/dwc3/core.c | 69 +++- drivers/usb/dwc3/core.h | 34 drivers/usb/dwc3/debug.h | 34 drivers/usb/dwc3/debugfs.c | 34 drivers/usb/dwc3/dwc3-exynos.c | 14 +--- drivers/usb/dwc3/dwc3-omap.c | 36 + drivers/usb/dwc3/dwc3-pci.c | 36 + drivers/usb/dwc3/ep0.c | 34 drivers/usb/dwc3/gadget.c| 55 +++- drivers/usb/dwc3/gadget.h| 34 drivers/usb/dwc3/host.c | 34 drivers/usb/dwc3/io.h| 34 drivers/usb/dwc3/platform_data.h | 25 +++ drivers/usb/usb-common.c | 35 include/linux/usb/of.h | 8 + include/linux/usb/phy.h | 18 +-- 16 files changed, 201 insertions(+), 333 deletions(-) create mode 100644 drivers/usb/dwc3/platform_data.h -- 1.8.2.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 4/5] usb: dwc3: let non-DT platforms pass tx-fifo-resize flag;
in case we're not in a DT boot, we should
still be able to tell the driver how to behave.
In order to be able to pass flags to the driver,
we introduce platform_data structure which the
core driver should use.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/core.c | 8 ++--
drivers/usb/dwc3/platform_data.h | 22 ++
2 files changed, 28 insertions(+), 2 deletions(-)
create mode 100644 drivers/usb/dwc3/platform_data.h
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 41feedb..5d6372e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -37,6 +37,7 @@
#include linux/usb/ch9.h
#include linux/usb/gadget.h
+#include platform_data.h
#include core.h
#include gadget.h
#include io.h
@@ -350,6 +351,7 @@ static void dwc3_core_exit(struct dwc3 *dwc)
static int dwc3_probe(struct platform_device *pdev)
{
+ struct dwc3_platform_data *pdata = pdev-dev.platform_data;
struct device_node *node = pdev-dev.of_node;
struct resource *res;
struct dwc3 *dwc;
@@ -412,9 +414,13 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+
+ dwc-needs_fifo_resize = of_property_read_bool(node,
tx-fifo-resize);
} else {
dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+
+ dwc-needs_fifo_resize = pdata-tx_fifo_resize;
}
if (IS_ERR(dwc-usb2_phy)) {
@@ -472,8 +478,6 @@ static int dwc3_probe(struct platform_device *pdev)
else
dwc-maximum_speed = DWC3_DCFG_SUPERSPEED;
- dwc-needs_fifo_resize = of_property_read_bool(node, tx-fifo-resize);
-
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);
pm_runtime_forbid(dev);
diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
new file mode 100644
index 000..0385164
--- /dev/null
+++ b/drivers/usb/dwc3/platform_data.h
@@ -0,0 +1,22 @@
+/**
+ * platform_data.h - USB DWC3 Platform Data Support
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Felipe Balbi ba...@ti.com
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see http://www.gnu.org/licenses/.
+ */
+
+struct dwc3_platform_data {
+ bool tx_fifo_resize;
+};
--
1.8.2.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 3/5] usb: common: introduce of_usb_get_maximum_speed()
this helper will be used for controllers which
want to work at a lower speed even though they
support higher USB transfer rates.
One such case is Texas Instruments' AM437x
SoC where it uses a USB3 controller without
a USB3 PHY, rendering the controller USB2-only.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/usb-common.c | 35 +++
include/linux/usb/of.h | 8
2 files changed, 43 insertions(+)
diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
index 675384d..b87f202 100644
--- a/drivers/usb/usb-common.c
+++ b/drivers/usb/usb-common.c
@@ -112,6 +112,41 @@ enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
return USB_DR_MODE_UNKNOWN;
}
EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
+
+static const char *const usb_maximum_speed[] = {
+ [USB_SPEED_UNKNOWN] = ,
+ [USB_SPEED_LOW] = lowspeed,
+ [USB_SPEED_FULL]= fullspeed,
+ [USB_SPEED_HIGH]= highspeed,
+ [USB_SPEED_SUPER] = superspeed,
+};
+
+/**
+ * of_usb_get_maximum_speed - Get maximum requested speed for a given USB
+ * controller.
+ * @np: Pointer to the given device_node
+ *
+ * The function gets the maximum speed string from property maximum-speed,
+ * and returns the corresponding enum usb_device_speed.
+ */
+enum usb_device_speed of_usb_get_maximum_speed(struct device_node *np)
+{
+ const char *maximum_speed;
+ int err;
+ int i;
+
+ err = of_property_read_string(np, maximum-speed, maximum_speed);
+ if (err 0)
+ return USB_SPEED_UNKNOWN;
+
+ for (i = 0; i ARRAY_SIZE(usb_maximum_speed); i++)
+ if (strcmp(maximum_speed, usb_maximum_speed[i]) == 0)
+ return i;
+
+ return USB_SPEED_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_maximum_speed);
+
#endif
MODULE_LICENSE(GPL);
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index a0ef405..5a7cb9e 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -7,16 +7,24 @@
#ifndef __LINUX_USB_OF_H
#define __LINUX_USB_OF_H
+#include linux/usb/ch9.h
#include linux/usb/otg.h
#include linux/usb/phy.h
#if IS_ENABLED(CONFIG_OF)
enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np);
+enum usb_device_speed of_usb_get_maximum_speed(struct device_node *np);
#else
static inline enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
{
return USB_DR_MODE_UNKNOWN;
}
+
+static inline enum usb_device_speed
+of_usb_get_maximum_speed(struct device_node *np)
+{
+ return USB_SPEED_UNKNOWN;
+}
#endif
#if IS_ENABLED(CONFIG_OF) IS_ENABLED(CONFIG_USB_PHY)
--
1.8.2.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 03/14] USB: usbatm: don't rely on CONFIG_USB_DEBUG
On Fri, Jun 28, 2013 at 11:32:52AM -0700, Greg Kroah-Hartman wrote: Enable the USB atm drivers to use the dynamic debugging subsystem, and not rely on if CONFIG_USB_DEBUG is enabled or not for debugging messages. This also provides a saner debug message, pointing out the exact device the message is coming from. This also means the drivers do not have to be rebuilt to get debugging messages, important for getting information from users who can not rebuild their kernels. Cc: Duncan Sands duncan.sa...@free.fr Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 01/14] USB: usbatm: remove unused UDSL_ASSERT macro
On Fri, Jun 28, 2013 at 11:32:50AM -0700, Greg Kroah-Hartman wrote: If this code isn't triggering this assert by now, it never will, so just remove it, it's pointless. Cc: Duncan Sands duncan.sa...@free.fr Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org compile tested and reviewed all patches, thus: Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 07/14] USB: phy: remove CONFIG_USB_DEBUG usage
On Fri, Jun 28, 2013 at 11:32:56AM -0700, Greg Kroah-Hartman wrote: Now that no USB phy driver is using CONFIG_USB_DEBUG, remove it from the Makefile. Cc: Felipe Balbi ba...@ti.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 06/14] USB: phy: remove custom DBG macro
On Fri, Jun 28, 2013 at 11:32:55AM -0700, Greg Kroah-Hartman wrote: Use the in-kernel pr_debug() calls instead of trying to roll your own DBG macro. This means the dynamic debugging calls now work here, and there is no dependency on CONFIG_USB_DEBUG for the phy code anymore. Cc: Felipe Balbi ba...@ti.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 04/14] USB: usbatm: move the atm_dbg() call to use dynamic debug
On Fri, Jun 28, 2013 at 11:32:53AM -0700, Greg Kroah-Hartman wrote: Move the atm_dbg() call to use the dynamic debug subsystem, and not rely on CONFIG_USB_DEBUG for if things should be printed out or not. This also means the drivers do not have to be rebuilt to get debugging messages, important for getting information from users who can not rebuild their kernels. Cc: Duncan Sands duncan.sa...@free.fr Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 02/14] USB: usbatm: remove unneeded trace printk calls
On Fri, Jun 28, 2013 at 11:32:51AM -0700, Greg Kroah-Hartman wrote: We have an in-kernel trace subsystem, so use that instead of printk for trying to figure out what functions are being called. Cc: Duncan Sands duncan.sa...@free.fr Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 09/14] USB: isp1362: remove unused _BUG_ON() calls
On Fri, Jun 28, 2013 at 11:32:58AM -0700, Greg Kroah-Hartman wrote: We shouldn't ever panic in a driver, and these calls were never being used, so just delete them, as obviously the driver is working properly now (right?) Cc: Felipe Balbi ba...@ti.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 08/14] USB: gadget: fix up comment
On Fri, Jun 28, 2013 at 11:32:57AM -0700, Greg Kroah-Hartman wrote: This endif is for CONFIG_USB_GADGET_DEBUG_FILES, not CONFIG_USB_DEBUG, so document it properly. Cc: Felipe Balbi ba...@ti.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 10/14] USB: isp1362: remove unused _WARN_ON() calls
On Fri, Jun 28, 2013 at 11:32:59AM -0700, Greg Kroah-Hartman wrote: Like _BUG_ON(), _WARN_ON() wasn't ever being used, so just delete it, as obviously things are working properly now (if not, we have bigger problems...) Cc: Felipe Balbi ba...@ti.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 13/14] USB: isp116x: remove dependency on CONFIG_USB_DEBUG
On Fri, Jun 28, 2013 at 11:33:02AM -0700, Greg Kroah-Hartman wrote: Move all debugging messages in the driver to use the dynamic debug subsystem, and not rely on CONFIG_USB_DEBUG to turn them on or off. This lets debugging be enabled without having to rebuild the driver, an important thing for users that can not do it. It also removes the pointless IRQ_TEST() macro, as that was totally useless and obviously never used. Cc: Olav Kongas o...@artecdesign.ee Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 11/14] USB: isp1362: remove _DBG() usage
On Fri, Jun 28, 2013 at 11:33:00AM -0700, Greg Kroah-Hartman wrote: If you want a debug call, just make it, so move to using the already-there DBG() call. No need to make things more complex than they really need to be. Cc: Felipe Balbi ba...@ti.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 14/14] USB: sl811: remove CONFIG_USB_DEBUG dependency
On Fri, Jun 28, 2013 at 11:33:03AM -0700, Greg Kroah-Hartman wrote: This removes the dependency of the driver on CONFIG_USB_DEBUG and moves it to us the dynamic debug subsystem instead. Bonus is the fact that we can now properly determine the exact hardware that is spitting out the messages. This lets debugging be enabled without having to rebuild the driver, an important thing for users that can not do it. Cc: Felipe Balbi ba...@ti.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 12/14] USB: isp1362: remove CONFIG_USB_DEBUG dependency
On Fri, Jun 28, 2013 at 11:33:01AM -0700, Greg Kroah-Hartman wrote: Now that the debugging macros are cleaned up, just rely on the dynamic debug code in the kernel to do the debug messages for the driver. This lets debugging be enabled without having to rebuild the driver, an important thing for users that can not do it. Cc: Felipe Balbi ba...@ti.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 4/6] USB: ehci-omap: Suspend the controller during bus suspend
On 06/28/2013 10:06 PM, Alan Stern wrote: On Fri, 28 Jun 2013, Roger Quadros wrote: That's not what I meant. Never mind the pinctrl; I was asking about the EHCI controller itself. Under what circumstances does the controller assert its wakeup signal? And how do you tell it to stop asserting that signal? I believe this would be through the EHCI Interrupt enable register (USBINTR). I'm not aware of any other mechanism. That's strange, because ehci_suspend() sets the intr_enable register to 0. So how do you ever get any wakeup interrupts at all? Because after ehci_suspend() for OMAP, we solely rely on the out of band wake up mechanism. i.e. Pad wakeup. Right. It seems the external hub has signaled remote wakeup but the kernel doesn't resume the root hub's port it is connected to. By observing the detailed logs below you can see that the root hub does not generate an INTerrupt transaction to notify the port status change event. I've captured the pstatus and GetPortStatus info as well. We don't need an interrupt. The driver is supposed to detect the remote wakeup sent by the external hub all by itself. OK. Then it could point to a bug in our stack. Failing case [ 16.108032] usb usb1: usb auto-resume [ 16.108062] ehci-omap 48064800.ehci: resume root hub [ 16.108154] hub 1-0:1.0: hub_resume [ 16.108398] ehci_hub_control GetPortStatus, port 1 temp = 0x1000 [ 16.108459] ehci_hub_control GetPortStatus, port 2 temp = 0x14c5 Here's where we should detect it. Look at the GetPortStatus case in ehci_hub_control(); the PORT_RESUME bit (0x0040) is set in temp, so the Remote Wakeup received? code should run. In particular, these lines should run: /* resume signaling for 20 msec */ ehci-reset_done[wIndex] = jiffies + msecs_to_jiffies(20); usb_hcd_start_port_resume(hcd-self, wIndex); /* check the port again */ mod_timer(ehci_to_hcd(ehci)-rh_timer, ehci-reset_done[wIndex]); Therefore 20 ms later, around timestamp 16.128459, ehci_hub_status_data() should have been called. At that time, the root-hub port should have been fully resumed. OK. right. [ 16.108551] hub 1-0:1.0: port 2: status 0507 change [ 16.108612] ehci_hub_control GetPortStatus, port 3 temp = 0x1000 [ 16.108642] hub 1-0:1.0: hub_activate submitting urb [ 16.109222] ehci_irq port 3 pstatus 0x1000 [ 16.109222] ehci_irq port 2 pstatus 0x14c5 [ 16.109252] ehci_irq port 1 pstatus 0x1000 [ 16.109374] hub 1-0:1.0: state 7 ports 3 chg evt But apparently nothing happened. Why not? Did the rh_timer get reset? Maybe you can find out what went wrong. Sure. I'll investigate. (Hmmm, we seem to be missing a set_bit(wIndex, ehci-resuming_ports); line in there...) Also, why do you need omap-initialized? Do you think you might get a wakeup interrupt before the controller has been fully set up? I don't see how you could, given the pm_runtime_get_sync() call in the probe routine. During probe we need to runtime_resume the device before usb_add_hcd() since the controller clocks must be enabled before any registers are accessed. However, we cannot call ehci_resume() before usb_add_hcd(). So to prevent this chicken egg situation, I've used the omap-initialized flag. It only indicates that the ehci structures are initialized and we can call ehci_resume/suspend(). Ah, yes. Other subsystems, such as PCI, face exactly the same problem. You probably shouldn't call it initialized, though, because the same issue arises in ehci_hcd_omap_remove() -- the pm_runtime_put_sync() there would end up calling ehci_suspend() after usb_remove_hcd(). bound or started would be better names. OK. Started seems fine. cheers, -roger -- 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/3] usb: atm: speedtch: be careful with bInterval
bInterval must be on the range 1 - 16, if we want to pass the maximum allowed, we should be passing 16. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/usb/atm/speedtch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/atm/speedtch.c b/drivers/usb/atm/speedtch.c index 807627b..69461d6 100644 --- a/drivers/usb/atm/speedtch.c +++ b/drivers/usb/atm/speedtch.c @@ -888,7 +888,7 @@ static int speedtch_bind(struct usbatm_data *usbatm, usb_fill_int_urb(instance-int_urb, usb_dev, usb_rcvintpipe(usb_dev, ENDPOINT_INT), instance-int_data, sizeof(instance-int_data), -speedtch_handle_int, instance, 50); +speedtch_handle_int, instance, 16); else usb_dbg(usbatm, %s: no memory for interrupt urb!\n, __func__); -- 1.8.2.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 2/3] usb: class: cdc-acm: be careful with bInterval
bInterval must be on the range 1 - 16, if we want to pass the maximum allowed, we should be passing 16 Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/usb/class/cdc-acm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 9f49bfe..3e7560f 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1295,7 +1295,7 @@ skip_countries: usb_rcvintpipe(usb_dev, epctrl-bEndpointAddress), acm-ctrl_buffer, ctrlsize, acm_ctrl_irq, acm, /* works around buggy devices */ -epctrl-bInterval ? epctrl-bInterval : 0xff); +epctrl-bInterval ? epctrl-bInterval : 16); acm-ctrlurb-transfer_flags |= URB_NO_TRANSFER_DMA_MAP; acm-ctrlurb-transfer_dma = acm-ctrl_dma; -- 1.8.2.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 0/3] usb: be careful with bInterval
Hi Greg, I found this issue when build testing your latest cleanup series with Sparse enabled. Clearly it was not caused by your series, it just helped me expose the problem since I built drivers I don't normally build. Anyway, a too large bInterval will end up zeroing urb-interval and in turn might confuse HCD's transfer scheduling routines. I wonder if these should be backported to stable tree, but so far nobody has complained, so perhaps not; your call cheers Felipe Balbi (3): usb: atm: speedtch: be careful with bInterval usb: class: cdc-acm: be careful with bInterval usb: clamp bInterval to allowed range drivers/usb/atm/speedtch.c | 2 +- drivers/usb/class/cdc-acm.c | 2 +- include/linux/usb.h | 4 3 files changed, 6 insertions(+), 2 deletions(-) -- 1.8.2.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 3/3] usb: clamp bInterval to allowed range
bInterval must be within the range 1 - 16, in order to catch drivers passing a too large bInterval (thus zeroing urb-interval), let's clamp() the argument to the allowed range. Signed-off-by: Felipe Balbi ba...@ti.com --- include/linux/usb.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index a232b7e..0883e3a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb, urb-transfer_buffer_length = buffer_length; urb-complete = complete_fn; urb-context = context; + + /* make sure interval is within allowed range */ + interval = clamp(interval, 1, 16); + if (dev-speed == USB_SPEED_HIGH || dev-speed == USB_SPEED_SUPER) urb-interval = 1 (interval - 1); else -- 1.8.2.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: [RFC] [PATCH] Debugfs support for EHCI testing modes
Hi Am Samstag, 29. Juni 2013, 15:39:52 schrieb Alan Stern: On Fri, 28 Jun 2013, Jack Pham wrote: Sorry to jump into this conversation just now but I saw this thread and noticed the link to ehset.c. This file was authored by us at Qualcomm, and had been added to the out-of-tree MSM port of the kernel here: https://www.codeaurora.org/cgit/quic/la/kernel/msm/commit?id=073c9409b9cf4 11552314d4fa887ebc8c23942a1 Somehow it must have made its way to the tree hosted on code.google.com. I agree it should be upstreamed, and good to know you think it looks reasonable. I will submit a patch after cleaning it up so that it applies on a recent kernel. Okay, great! This is great news :-). I just finished looking through the EHSET PDF file from www.usb.org. The description of the SINGLE_STEP_SET_FEATURE test appears to depend on a hardware feature of the host controller that is not present in the EHCI 1.0 spec -- maybe it was added as part of the OTG spec. The SINGLE_STEP_SET_FEATURE test is also mentioned in several of the Host High-speed Electrical Test Procedure (not Embedded Host) PDFs here: http://www.usb.org/developers/docs/. From what I gather, they require the HSET tool installed on Windows hosts in order to activate this feature and it looks like this tool is using a modified proprietary EHCI driver. Thus it appears that this is a software-controlled test feature, and hence why it doesn't appear in the EHCI spec as a hardware test feature. That makes sense -- except for one thing. The EHSET device is intended for testing host controllers in embedded systems, right? Is an embedded system likely to be running a modified proprietary EHCI driver? In our case that is definetly not the case but we are missing the testmode driver. The testing functionality is triggered mainly by the magic usb id's reserved for the testing device. As for EHSET we were able to implement SINGLE_STEP_SET_FEATURE in the kernel by modifying the EHCI driver. This involved adding a hook function in ehci-q.c that calls submit_async() two separate times: first qtd_fill() is called for the initial SETUP transaction and submitted on its own; then a 15-second delay; and finally qtd_fill() is called for each of the DATA and STATUS stages and submit_async() is called again for these two to complete the transfer. In essence it is a deconstruction of ehci_urb_enqueue() for a GetDescriptor control URB, for the sole purpose of inserting a delay between the SETUP and DATA stages. I see. To me that seems to be digging a little too deep into the EHCI HCD innards, and welcome any better ideas on how to do it. In the meantime I can send what we did as an RFC. It does sound like a lot of mucking around inside ehci-hcd for a relatively small benefit. But if people want it, I guess it can be added. Well Linux is big nowadays in embedded devices and the less time is needed to bring up the hardware the more time is available for creating good hardware. So having a good testing infrastructure helps in new HW development and quality assurance for these devices. Having a sound hardware is paramount for a good user experience. Or the other way round: making testing the hardware harder is a receipe for crappy hardware in the end. By the way, if this is the appropriate approach, then we may have to do something similar with the xHCI driver also in order to allow USB 3.0 hosts the SINGLE_STEP_SET_FEATURE test when in high speed mode. According to our electrical engineer escpecially these single step mode matter for analyzing the hardware. Especially when the other testmodes can also be triggered by an ioctl as Alan kindly pointed out. Best regards Tim -- 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 3/3] usb: clamp bInterval to allowed range
Hi,
On Mon, Jul 01, 2013 at 11:23:25AM +0300, Felipe Balbi wrote:
bInterval must be within the range 1 - 16,
in order to catch drivers passing a too
large bInterval (thus zeroing urb-interval),
let's clamp() the argument to the allowed
range.
Signed-off-by: Felipe Balbi ba...@ti.com
---
include/linux/usb.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/linux/usb.h b/include/linux/usb.h
index a232b7e..0883e3a 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb,
urb-transfer_buffer_length = buffer_length;
urb-complete = complete_fn;
urb-context = context;
+
+ /* make sure interval is within allowed range */
+ interval = clamp(interval, 1, 16);
hmmm, there is one problem with this patch. It doesn't cope with the
fact that users could be running at full speed which would make the
allowed range 1 - 255 instead.
perhaps below is slightly better ?
commit 6d2f29734d901e483793c25dc34464eecf67b4e0
Author: Felipe Balbi ba...@ti.com
Date: Mon Jul 1 11:09:34 2013 +0300
usb: clamp bInterval to allowed range
bInterval must be within the range 1 - 16
when running at High/Super speed, or 1 - 255
when running at Full/Low speed.
In order to catch drivers passing a too
large bInterval (thus zeroing urb-interval),
let's clamp() the argument to the allowed
ranges.
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/include/linux/usb.h b/include/linux/usb.h
index a232b7e..fcf8110 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1545,10 +1545,20 @@ static inline void usb_fill_int_urb(struct urb *urb,
urb-transfer_buffer_length = buffer_length;
urb-complete = complete_fn;
urb-context = context;
- if (dev-speed == USB_SPEED_HIGH || dev-speed == USB_SPEED_SUPER)
+
+ if (dev-speed == USB_SPEED_HIGH || dev-speed == USB_SPEED_SUPER) {
+ /* make sure interval is within allowed range */
+ interval = clamp(interval, 1, 16);
+
urb-interval = 1 (interval - 1);
- else
+ } else if (dev-speed == USB_SPEED_FULL || dev-speed == USB_SPEED_LOW)
{
+ /* make sure interval is within allowed range */
+ interval = clamp(interval, 1, 255);
+
urb-interval = interval;
+ } else { /* WIRELESS */
+ urb-interval = interval;
+ }
urb-start_frame = -1;
}
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCH 4/6] USB: ehci-omap: Suspend the controller during bus suspend
On 06/28/2013 10:18 PM, Alan Stern wrote: On Fri, 28 Jun 2013, Roger Quadros wrote: Just found the problem. It seems that enabling the ehci_irq _after_ the root hub is resumed is the root cause of the problem. Doing so will miss events from the root hub. This sounds like a bug in the IRQ setup. It's the sort of thing you see when a level-triggered IRQ is treated as though it were edge-triggered. In any case, the wakeup should have worked whether the IRQ was issued or not. OK. I appreciate the symmetry of putting the enable_irq call in ehci-hcd, seeing as how the disable_irq is there too. On the other hand, every HCD using this mechanism is going to have to do the same thing, which argues for putting the enable call in the core. Perhaps at the start OK. of hcd_resume_work() instead of the end. We can't enable_irq at the start as the controller will only be resumed after usb_remote_wakeup(). cheers, -roger -- 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 USB file storage gadget with new UDC
Hi, No, i don't see that (Set-Config request with a config value of 0) Well, then I don't know where the problem is, but obviously the problem occurred before the gadget driver was involved. Either the host sent a wrong packet, or more likely the UDC received the packet incorrectly. Yes, UDC driver has bug. After modifying it, it can receive Set-Config request with a config value of 0. However, the device descriptor test - addressed state still fails. Please see the attached log. The Set-Config request with a config value of 0 is the second last USB request sent from the host. The last USB request is Get-Config, which still returns config value of 1. Thanks, victor # dmesg -c g_file_storage gadget: disconnect or port reset handle_exception begin handle_exception wait until handle_exception old_state 5 g_file_storage gadget: in handle_exception loop g_file_storage gadget: in fsg-running loop g_file_storage gadget: disconnect or port reset handle_exception begin handle_exception wait until handle_exception old_state 5 g_file_storage gadget: in handle_exception loop g_file_storage gadget: in fsg-running loop g_file_storage gadget: ep0-setup, length 8: : 80 06 00 01 00 00 12 00 g_file_storage gadget: get device descriptor exit C ept0 in queue len 0x12, buffer 0xc1289800 ep0_complete g_file_storage gadget: ep0-in, length 18: : 12 01 00 02 00 00 00 40 25 05 a5 a4 33 03 01 02 0010: 00 01 USB_RECIP_DEVICE 0x2 fa is 0x2 exit A g_file_storage gadget: ep0-setup, length 8: : 80 06 00 01 00 00 12 00 g_file_storage gadget: get device descriptor ept0 in queue len 0x12, buffer 0xc1289800 ep0_complete g_file_storage gadget: ep0-in, length 18: : 12 01 00 02 00 00 00 40 25 05 a5 a4 33 03 01 02 0010: 00 01 g_file_storage gadget: ep0-setup, length 8: : 80 06 00 02 00 00 09 00 g_file_storage gadget: get configuration descriptor ept0 in queue len 0x9, buffer 0xc1289800 ep0_complete g_file_storage gadget: ep0-in, length 9: : 09 02 20 00 01 01 04 c0 01 g_file_storage gadget: ep0-setup, length 8: : 00 09 01 00 00 00 00 00 g_file_storage gadget: set configuration g_file_storage gadget: ep0-setup, length 8: : 80 06 00 02 00 00 09 00 g_file_storage gadget: get configuration descriptor ept0 in queue len 0x9, buffer 0xc1289800 ep0_complete g_file_storage gadget: ep0-in, length 9: : 09 02 20 00 01 01 04 c0 01 handle_exception begin handle_exception wait until handle_exception old_state 4 g_file_storage gadget: ep0-setup, length 8: : 80 06 00 02 00 00 20 00 g_file_storage gadget: get configuration descriptor ept0 in queue len 0x20, buffer 0xc1289800 ep0_complete g_file_storage gadget: ep0-in, length 32: : 09 02 20 00 01 01 04 c0 01 09 04 00 00 02 08 06 0010: 50 05 07 05 81 02 40 00 00 07 05 01 02 40 00 00 g_file_storage gadget: set interface 0 g_file_storage gadget: full-speed config #1 g_file_storage gadget: in handle_exception loop [start_transfer] 43425355 899e1008 ept1 out queue len 0x40, buffer 0xc0c44000 before kagen2_ep_queue g_file_storage gadget: disconnect or port reset after kagen2_ep_queue kagen2_ep_queue 31 64 31 [kagen2_ep_queue] 43425355 899e1008 g_file_storage gadget: bulk-out, length 31: : 55 53 42 43 08 10 9e 89 00 00 00 00 00 00 0a 35 0010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 g_file_storage gadget: SCSI command: SYNCHRONIZE CACHE; Dc=10, Dn=0; Hc=10, Hn=0 attention condition g_file_storage gadget: after calling do_scsi_command handle_exception begin handle_exception wait until handle_exception old_state 5 g_file_storage gadget: reset config g_file_storage gadget: reset interface g_file_storage gadget: in handle_exception loop g_file_storage gadget: in fsg-running loop g_file_storage gadget: in fsg-running loop g_file_storage gadget: disconnect or port reset handle_exception begin handle_exception wait until handle_exception old_state 5 g_file_storage gadget: in handle_exception loop g_file_storage gadget: in fsg-running loop g_file_storage gadget: ep0-setup, length 8: : 80 06 00 01 00 00 12 00 g_file_storage gadget: get device descriptor exit C ept0 in queue len 0x12, buffer 0xc1289800 ep0_complete g_file_storage gadget: ep0-in, length 18: : 12 01 00 02 00 00 00 40 25 05 a5 a4 33 03 01 02 0010: 00 01 USB_RECIP_DEVICE 0x2 fa is 0x2 exit A g_file_storage gadget: ep0-setup, length 8: : 80 06 00 01 00 00 12 00 g_file_storage gadget: get device descriptor ept0 in queue len 0x12, buffer 0xc1289800 ep0_complete g_file_storage gadget: ep0-in, length 18: : 12 01 00 02 00 00 00 40 25 05 a5 a4 33 03 01 02 0010: 00 01 g_file_storage gadget: ep0-setup, length 8: : 80 06 00 02 00 00 09 00 g_file_storage gadget: get configuration descriptor ept0 in queue len 0x9, buffer 0xc1289800 ep0_complete g_file_storage gadget: ep0-in, length 9: : 09 02 20 00 01 01 04 c0 01 g_file_storage gadget: ep0-setup, length 8: : 00 09 01 00 00 00 00 00
Re: Scanner fails on USB3 port
On Mon, Jul 1, 2013 at 12:17 AM, Greg KH gre...@linuxfoundation.org wrote: On Sun, Jun 30, 2013 at 09:35:59PM +0200, Martin van Es wrote: 3.9.8 brought a tiny improvement! scanimage -L now succesfully reports the scanner, but then hangs. I still can not scan with xsane however (no scanner device found) $ scanimage -L device `plustek:libusb:001:004' is a Canon CanoScan N670U/N676U/LiDE20 flatbed scanner (hang, but eventually returns prompt after couple of minutes) Any chance you can try 3.10-rc7 to see if anything improved there? 3.10.0 is back to zero: no result on scanimage -L 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: Linux USB file storage gadget with new UDC
Hi, No, i don't see that (Set-Config request with a config value of 0) Well, then I don't know where the problem is, but obviously the problem occurred before the gadget driver was involved. Either the host sent a wrong packet, or more likely the UDC received the packet incorrectly. Yes, UDC driver has bug. After modifying it, it can receive Set-Config request with a config value of 0. However, the device descriptor test - addressed state still fails. Please see the attached log. The Set-Config request with a config value of 0 is the second last USB request sent from the host. The last USB request is Get-Config, which still returns config value of 1. In gadget driver, do_set_config(), if new_config is 0, the new_config is not processed. So config value of zero will never be saved by gadget driver. Isn't it? Thanks, victor -- 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: Huawei E3131 wwan interface
thank you very much for the hint! Unfortunately, this is not a Hi-Link modem. But I read about it, maybe your investigations? Thank again for the hint! On Mon, 1 Jul 2013, Thomas Sch?fer wrote: ==Date: Mon, 01 Jul 2013 08:16:34 +0200 ==From: Thomas Sch?fer tschae...@t-online.de ==To: Enrico Mioso mrkiko...@gmail.com, linux-usb@vger.kernel.org ==Subject: Re: Huawei E3131 wwan interface == ==Am Montag, 1. Juli 2013, 00:37:14 schrieben Sie: == Hi! == I'm trying to investigate on the Huawei E3131 wwan interface - == ==Is the E3131 an HILINK-Modem? == ==Have you tried simply starting dhcp/automatic-configuration? == ==Open a Browser to get connected. == ==If dns-redirection doesn't work, try the router or dns/resolver-address. == ==http://192.168.x.x. == ==ip route | grep default == ==cat /etc/resolv.conf == ==Regards, == ==Thomas == == == == -- 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: How should we handle isochronous underruns?
On 6/30/2013 8:32 PM, Alan Stern wrote: There are several possible things the HCD could do when an underrun occurs: I do not have much experience in working with ISOC host.But by the experience of device side I can say that, There could be problem irrespective of tasklet or irq context, only that tasklet would be more prone to it. Should not we have same approach in handling isoc packet in either of the cases? It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. IMO, this approach is better. But, what should we call as first time slot known to be available. Current code calculates it as now (current time slot) + delta microframe, and delta is kept as fixed. However, different system works with different values of delta. IMO, This delta should be dynamic and software should update it on the basis of initial value and feedback (past failure/missed isoc experience). It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. There is no meaning of submitting an URB for expired time slot. In anyway, it is not going to be transferred and will further result in under-run/missed packet. Something in between... Making this offset dynamic will cause software to be further complex, but that would be the best way to handle such situation. Regards Pratyush What would be best for your purposes? Or do you have any different suggestions? -- 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
Announcing libusbx-1.0.16-rc2
Hi All, I'm very happy to announce libusbx-1.0.16-rc2. Changes since 1.0.16-rc1: - * Fix crash on exit on openbsd, netbsd wince * Logging now use a single write call per log-message, avoiding log-message interlacing when using multiple threads. * Do not use uninitialized memory for auto_detach_kernel_driver Highlights of changes since 1.0.15: --- * As Nathan Hjelm already announced in his libusb and libusbx merging mail, Nathan has taken over libusb maintenance and this release is a combined effort between the libusb and libusbx projects! * Hotplug support for Darwin and Linux * Superspeed endpoint companion and BOS descriptor support * We finally have a libusb_strerror API You can find 1.0.16-rc2 docs including all the new API-s here: http://people.fedoraproject.org/~jwrdegoede/libusb-reference/ You can download the 1.0.16-rc2 tarbal here: http://downloads.sourceforge.net/libusbx/libusbx-1.0.16-rc2.tar.bz2 Please give it a thorough testing, and report any issues you find. For those interested the full ChangeLog is below. Regards, Hans 2013-07-01: v1.0.16-rc2 * Add hotplug support for Darwin and Linux (#9) * Add superspeed endpoint companion descriptor support (#15) * Add BOS descriptor support (#15) * Make descriptor parsing code more robust * New libusb_get_port_numbers API, this is libusb_get_port_path without the unnecessary context parameter, libusb_get_port_path is now deprecated * New libusb_strerror API (#14) * New libusb_set_auto_detach_kernel_driver API (#17) * Logging now use a single write call per log-message, avoiding log-message interlacing when using multiple threads. * Android: use Android logging when building on Android (#101) * Darwin: make libusb_reset reenumerate device on descriptors change (#89) * Darwin: add support for the LIBUSB_TRANSFER_ADD_ZERO_PACKET flag (#91) * Darwin: add a device cache (#112, #114) * Examples: Add sam3u_benchmark isochronous example by Harald Welte (#109) * Many other bug fixes and improvements The (#xx) numbers are libusbx issue numbers, see ie: https://github.com/libusbx/libusbx/issues/9 -- 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: How should we handle isochronous underruns?
Pratyush Anand wrote: On 6/30/2013 8:32 PM, Alan Stern wrote: It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. IMO, this approach is better. To quote the USB spec: | Isochronous Data | A stream of data whose timing is implied by its delivery rate. Isochronous transfers are usually used for real-time applications where it is better to drop a single packet than to delay *all* following packets. It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. There is no meaning of submitting an URB for expired time slot. Of course there is. The HCD cannot exactly know whether the current slot will expire before the URB is enqueued, so it is not always possible to label the slot as expired or not during submission. The meaning of an isochronous URB submission is try to transmit this data in that frame; whether the URB actually was transferred will be reported to the completion callback. Regards, Clemens -- 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: How should we handle isochronous underruns?
Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. [...] But I don't know how USB audio driver uses URBs, and could it submit only one isoc URB to playback/record audio data? The audio drivers uses at least two URBs. (The actual number and length of URBs depends on how the application configures its buffer.) Regards, Clemens -- 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: How should we handle isochronous underruns?
On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, so looks we still can ease the problem by submitting more URBs concurrently, and at the same time make less packets per URB if guys care latency. Correct me if it is wrong... On Mon, Jul 1, 2013 at 7:36 PM, Clemens Ladisch clem...@ladisch.de wrote: The audio drivers uses at least two URBs. (The actual number and length of URBs depends on how the application configures its buffer.) Clemens, thanks for your input. Also, another important data about the problem is that how much time one isoc URB may span, which depends on the endpoint interval and packet number of URB. 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: sleeping while atomic in dwc3_gadget_start
On Fri, Jun 28, 2013 at 07:02:32PM -0700, Stephen Boyd wrote: On 06/28/13 03:58, Felipe Balbi wrote: Hi, On Thu, Jun 27, 2013 at 09:57:52AM -0700, Stephen Boyd wrote: On 06/26/13 23:58, Felipe Balbi wrote: On Wed, Jun 26, 2013 at 02:52:56PM -0700, Stephen Boyd wrote: Hi, I'm getting the folllowing BUG message on bootup with 3.10-rc5 BUG: sleeping function called from invalid context at mm/slub.c:926 in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: swapper/0 CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.0-rc5-gee3e35b-09316-ge78f3b35 #643 [c0014220] (unwind_backtrace+0x0/0x120) from [c001212c] (show_stack+0x10/0x14) [c001212c] (show_stack+0x10/0x14) from [c0143750] (kmem_cache_alloc_trace+0x3c/0x210) [c0143750] (kmem_cache_alloc_trace+0x3c/0x210) from [c00e0c60] (request_threaded_irq+0x88/0x11c) [c00e0c60] (request_threaded_irq+0x88/0x11c) from [c03bf53c] (dwc3_gadget_start+0x198/0x200) [c03bf53c] (dwc3_gadget_start+0x198/0x200) from [c03f7a5c] (udc_bind_to_driver+0x70/0xd8) [c03f7a5c] (udc_bind_to_driver+0x70/0xd8) from [c03f7b50] (usb_gadget_probe_driver+0x8c/0xb8) and I suspect this problem was introduced in commit 8698e2acf (usb: dwc3: gadget: introduce and use enable/disable irq methods). Is there a fix for this problem? Can we just move the irq request outside the spinlock? nice :-) how about this ? If start fails do you call stop? I believe the answer is no, so we'll need to free_irq() somewhere along the error path. Or we can request it after the spin_unlock()? good point here's v2: Ok looks good to me. I hope that platform_get_irq() doesn't fail, otherwise we're in for a nasty surprise. Maybe we should add a check in request_irq() for that case. I don't think we would ever fall into that situation. And if we do, our data (struct resource or DT) is f-ed up anyway :-) -- balbi signature.asc Description: Digital signature
Re: Huawei E3131 wwan interface
Hi guys!
It's my first experience, and util now I acquired a null pointer dereference,
which kicks my kernel off! :)
This is the patch ... I know for sure I'm doing something horrible!
I'm a newbie !! XD
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4709fa3..725d892 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -52,7 +52,7 @@
#include linux/usb/usbnet.h
#include linux/usb/cdc.h
#include linux/usb/cdc_ncm.h
-
+#include linux/usb/cdc-wdm.h
#defineDRIVER_VERSION 14-Mar-2012
#if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
@@ -62,12 +62,41 @@ static bool prefer_mbim;
#endif
module_param(prefer_mbim, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(prefer_mbim, Prefer MBIM setting on dual NCM/MBIM
functions);
-
+atomic_t pmcount;
static void cdc_ncm_txpath_bh(unsigned long param);
static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
static struct usb_driver cdc_ncm_driver;
+static int cdc_mbim_manage_power(struct usbnet *dev, int on)
+{
+ int rv = 0;
+
+
+ if ((on atomic_add_return(1, pmcount) == 1) || (!on
atomic_dec_and_test(pmcount))) {
+ /* need autopm_get/put here to ensure the usbcore sees the new
value */
+ rv = usb_autopm_get_interface(dev-intf);
+ if (rv 0)
+ goto err;
+ dev-intf-needs_remote_wakeup = on;
+ usb_autopm_put_interface(dev-intf);
+ }
+err:
+ return rv;
+}
+
+static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status)
+{
+ struct usbnet *dev = usb_get_intfdata(intf);
+
+ /* can be called while disconnecting */
+ if (!dev)
+ return 0;
+
+ return cdc_mbim_manage_power(dev, status);
+}
+
+
static void
cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
{
@@ -355,6 +384,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8
data_altsetting)
{
struct cdc_ncm_ctx *ctx;
+ struct usb_driver *subdriver;
struct usb_driver *driver;
u8 *buf;
int len;
@@ -507,6 +537,20 @@ advance:
dev-rx_urb_size = ctx-rx_max;
ctx-tx_speed = ctx-rx_speed = 0;
+
+ usb_driver_release_interface(driver, ctx-control);
+ if (ctx-control == NULL){
+ printk(ctx-control is NULL!\n);
+ return -ENODEV;
+ }
+ if (dev-status-desc == NULL){
+ printk(ctx-control is NULL!\n);
+ return -ENODEV;
+ }
+ subdriver = usb_cdc_wdm_register(ctx-control,
+dev-status-desc,
+
le16_to_cpu(ctx-mbim_desc-wMaxControlMessage),
+cdc_mbim_wdm_manage_power);
return 0;
error2:
@@ -603,13 +647,14 @@ static int cdc_ncm_bind(struct usbnet *dev, struct
usb_interface *intf)
/* NCM data altsetting is always 1 */
ret = cdc_ncm_bind_common(dev, intf, 1);
+
/*
* We should get an event when network connection is connected or
* disconnected. Set network connection in disconnected state
* (carrier is OFF) during attach, so the IP network stack does not
* start IPv6 negotiation and more.
-*/
+ */
netif_carrier_off(dev-net);
return ret;
}
thank you for your help!
--
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: How should we handle isochronous underruns?
On 7/1/2013 4:48 PM, Clemens Ladisch wrote: It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. There is no meaning of submitting an URB for expired time slot. Of course there is. The HCD cannot exactly know whether the current slot will expire before the URB is enqueued, so it is not always possible to label the slot as expired or not during submission. The meaning of an isochronous URB submission is try to transmit this data in that frame; whether the URB actually was transferred will be reported to the completion callback. Correct, I understand it. Let me explain with an example what I wanted to convey: Lets assume that URB1-8 has been submitted to HCD driver from upper layer. HCD driver has also further submitted them to the controller hardware for execution in n to (n+7)th uf respectively. Now URB3 was not able to be transmitted in time and resulted in missed isoc. This failure will cause mainly because of two reasons: 1. SW was slow enough in submission of URB to HW. It actually submitted when timing was already expired. In this case, it is most likely that following URBs (4-8) will also result in missed isoc. So, if we add further URB9 for transmission in (n + 8) uf, most likely it may also result in missed isoc. 2. SW submitted well in time, but HW was slow enough and could not fetch data in time. (In case HS it may not occur, but in case of SS it may occur sometime, so this case may not be valid for ehci-hcd). In such situation, HW will flush current descriptor and most likely will be able to transmit next URB correctly. So, What I was proposing, to go for dynamic, something like this... When an HCD gets an URB request for any isoc pipe, and if there is any pending missed isoc flag then wait.. do not submit it to controller. May be just keep them in received list(submitted_list). If during any of the pending completion callback, SW observes success then submit all from submitted_list to be scheduled in very next uf, and clear pending missed isoc flag. If none of the submitted URB results in success, then do not submit them in very next frame, rather submit for the first time slot known to be available. Regards Pratyush -- 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: How should we handle isochronous underruns?
Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. Regards, Clemens -- 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: How should we handle isochronous underruns?
Hello, On Monday 01 July 2013 13:19:05 Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern wrote: Clement and Laurent: The two of you seem to be the people who make the most use of isochronous USB transfers. Since the ehci-hcd driver is being changed to use a tasklet for URB completion callbacks, it looks like I will need to reconsider how isochronous underruns get handled. Without using tasklet, the hard interrupt handler still can be delayed for some time, and switching to tasklet doesn't change the fact of the probable delay of URB handling. The basic prolem is simple enough: What should the HCD do when an URB is submitted after an isochronous pipeline has emptied out? I think some time-related data should be helpful for the discussion, for example, how long the isochronous pipeline may become empty in current audio/video driver(application) without any URB resubmit during the period? The problem will be more acute than in the past, because URB resubmissions will no longer be synchronous with URB completions, thanks to the tasklet. That is, in the current code, if the completion handler resubmits the URB, the resubmission occurs before the HCD finishes the completion callback. But in the new code, the HCD will simply hand the URB over to the tasklet, and the resubmission won't occur until some time later (when the tasklet wakes up and invokes the completion handler). As far as the HCD is concerned, the completion The delay should be very small(generally several microseconds) since isochronous URBs are completed in high priority tasklet. I don't think the introduced tasklet delay is a problem for EHCI since per EHCI spec the maximum rate at which the host controller will issue interrupts is one microframe(125us), which means isochronous transfer completion can be reported to CPU with about 125us delay in hardware level. will already be finished. Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. Single URB may not work well too when running complete() in hard irq context. In UVC driver, looks it may submit at most 5 URBs which can include max 32 packets, so it will take about 5*32*125us(20ms) to make isoc pipeline empty suppose the interval is 1uF. That's correct. Unlike the UAC driver, the UVC driver doesn't care too much about exact timings. Avoiding packet loss is my main concern. As video frames are not delivered to userspace before they are fully transmitted, and given the large number of URBs required to transmit a video frame, I can submit more URBs (without reasonable limits) if that can help avoiding underruns. But I don't know how USB audio driver uses URBs, and could it submit only one isoc URB to playback/record audio data? This means that the HCD will have trouble telling the difference between an underrun and a normal restart of a stopped I/O stream. In both cases it will see an URB being submitted to an empty queue. IMO, we should try to avoid the situation, and in UVC cases looks it is impossible to see an URB being submitted to an empty queue except for at the moment of starting streaming or interrupt disabled for extremely long. A couple of users have experienced underruns in the past due to a rogue driver disabling interrupts for tens of milliseconds. I don't think switching to tasklets will have a strong adverse effect, the 20ms buffer time should be large enough to avoid underruns in sane systems. Here's where the URB_ISO_ASAP flag will be useful; if that flag is set then the URB is restarting a stopped stream, but if it isn't set then the pipeline experienced an underrun. Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. There are several possible things the HCD could do when an underrun occurs: It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. Something in between... What would be best for your purposes? Or do you have any different suggestions? -- Regards, Laurent Pinchart -- 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: How should we handle isochronous underruns?
On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. For playback, every URB submitted is added into hw table immediately, then the data will be played to speaker. I don't understand why the latency is the entire pipeline. If you submitted a bit more URBs, underruns shouldn't happen at all. Could you explain it in a bit detail? 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: How should we handle isochronous underruns?
Ming Lei wrote: On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. For playback, every URB submitted is added into hw table immediately, then the data will be played to speaker. I don't understand why the latency is the entire pipeline. A submitted packet will be transmitted only after all the other packets in the pipeline have been transmitted. Regards, Clemens -- 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: Huawei E3131 wwan interface
Enrico Mioso mrkiko...@gmail.com writes:
Hi guys!
It's my first experience, and util now I acquired a null pointer dereference,
which kicks my kernel off! :)
This is the patch ... I know for sure I'm doing something horrible!
I'm a newbie !! XD
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4709fa3..725d892 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -52,7 +52,7 @@
#include linux/usb/usbnet.h
#include linux/usb/cdc.h
#include linux/usb/cdc_ncm.h
-
+#include linux/usb/cdc-wdm.h
#define DRIVER_VERSION 14-Mar-2012
#if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
@@ -62,12 +62,41 @@ static bool prefer_mbim;
#endif
module_param(prefer_mbim, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(prefer_mbim, Prefer MBIM setting on dual NCM/MBIM
functions);
-
+atomic_t pmcount;
static void cdc_ncm_txpath_bh(unsigned long param);
static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
static struct usb_driver cdc_ncm_driver;
+static int cdc_mbim_manage_power(struct usbnet *dev, int on)
+{
+ int rv = 0;
+
+
+ if ((on atomic_add_return(1, pmcount) == 1) || (!on
atomic_dec_and_test(pmcount))) {
+ /* need autopm_get/put here to ensure the usbcore sees the new
value */
+ rv = usb_autopm_get_interface(dev-intf);
+ if (rv 0)
+ goto err;
+ dev-intf-needs_remote_wakeup = on;
+ usb_autopm_put_interface(dev-intf);
+ }
+err:
+ return rv;
+}
+
+static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status)
+{
+ struct usbnet *dev = usb_get_intfdata(intf);
+
+ /* can be called while disconnecting */
+ if (!dev)
+ return 0;
+
+ return cdc_mbim_manage_power(dev, status);
+}
+
+
For a simple test, I'd suggest ignoring PM for now. You can do
usb_autopm_get_interface() on bind and a usb_autopm_put_interface() to
make sure autosuspend doesn't complicate the testing.
Just make a dummy manage_power function to make the subdriver
registration happy.
static void
cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
{
@@ -355,6 +384,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8
data_altsetting)
{
struct cdc_ncm_ctx *ctx;
+ struct usb_driver *subdriver;
struct usb_driver *driver;
u8 *buf;
int len;
@@ -507,6 +537,20 @@ advance:
dev-rx_urb_size = ctx-rx_max;
ctx-tx_speed = ctx-rx_speed = 0;
+
+ usb_driver_release_interface(driver, ctx-control);
This does not make any sense to me...
+ if (ctx-control == NULL){
+ printk(ctx-control is NULL!\n);
+ return -ENODEV;
+ }
+ if (dev-status-desc == NULL){
+ printk(ctx-control is NULL!\n);
+ return -ENODEV;
+ }
+ subdriver = usb_cdc_wdm_register(ctx-control,
+ dev-status-desc,
+
le16_to_cpu(ctx-mbim_desc-wMaxControlMessage),
+ cdc_mbim_wdm_manage_power);
You cannot use ctx-mbim_desc here. It is NULL for any non MBIM
device. Just use some static max message constant instead, like qmi_wwan
does. We don't know how to guess the the correct value until we know
what the protocol is.
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
Re: How should we handle isochronous underruns?
On Mon, Jul 1, 2013 at 9:27 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. For playback, every URB submitted is added into hw table immediately, then the data will be played to speaker. I don't understand why the latency is the entire pipeline. A submitted packet will be transmitted only after all the other packets in the pipeline have been transmitted. Yes, that is always true since EHCI HW will send out data(packet) to device one by one at the scheduled frame/uframe according to the order of URB submitting , so the USB audio driver can submit URBs in advance, can't it? Also could you provide the typical time one URB in audio driver may span? 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: How should we handle isochronous underruns?
Pratyush Anand wrote: On 7/1/2013 4:48 PM, Clemens Ladisch wrote: There is no meaning of submitting an URB for expired time slot. Of course there is. The HCD cannot exactly know whether the current slot will expire before the URB is enqueued, so it is not always possible to label the slot as expired or not during submission. The meaning of an isochronous URB submission is try to transmit this data in that frame; whether the URB actually was transferred will be reported to the completion callback. Correct, I understand it. Let me explain with an example what I wanted to convey: Lets assume that URB1-8 has been submitted to HCD driver from upper layer. HCD driver has also further submitted them to the controller hardware for execution in n to (n+7)th uf respectively. Now URB3 was not able to be transmitted in time and resulted in missed isoc. This failure will cause mainly because of two reasons: 1. SW was slow enough in submission of URB to HW. It actually submitted when timing was already expired. In this case, it is most likely that following URBs (4-8) will also result in missed isoc. So, if we add further URB9 for transmission in (n + 8) uf, most likely it may also result in missed isoc. But there is _some_ uf that has not yet expired. If the HCD delays enqueuing URBs, it just increases the risk that that uf will expire too. 2. SW submitted well in time, but HW was slow enough and could not fetch data in time. In such situation, HW will flush current descriptor and most likely will be able to transmit next URB correctly. So, What I was proposing, to go for dynamic, something like this... When an HCD gets an URB request for any isoc pipe, and if there is any pending missed isoc flag then wait.. do not submit it to controller. May be just keep them in received list(submitted_list). If during any of the pending completion callback, SW observes success then submit all from submitted_list to be scheduled in very next uf This is not possible with a very short pipeline where there is no other packet that could complete. And even if it is possible in some cases: why should submitted packets be delayed at all? The UAC driver wants all packets at their correct position in time (or else to be dropped), and the UVC driver does not want delays which could make it lose some data sent by the device. Regards, Clemens -- 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 3/3] usb: clamp bInterval to allowed range
On Mon, 1 Jul 2013, Felipe Balbi wrote: bInterval must be within the range 1 - 16, That's true only for high speed and SuperSpeed. For low speed and full speed, bInterval is allowed to range from 1 to 255. See p. 271 in the USB-2 spec. in order to catch drivers passing a too large bInterval (thus zeroing urb-interval), let's clamp() the argument to the allowed range. Signed-off-by: Felipe Balbi ba...@ti.com --- include/linux/usb.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index a232b7e..0883e3a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb, urb-transfer_buffer_length = buffer_length; urb-complete = complete_fn; urb-context = context; + + /* make sure interval is within allowed range */ + interval = clamp(interval, 1, 16); These lines should go within the first branch of the if statement below. + if (dev-speed == USB_SPEED_HIGH || dev-speed == USB_SPEED_SUPER) urb-interval = 1 (interval - 1); else 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: How should we handle isochronous underruns?
Ming Lei wrote: On Mon, Jul 1, 2013 at 9:27 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. For playback, every URB submitted is added into hw table immediately, then the data will be played to speaker. I don't understand why the latency is the entire pipeline. A submitted packet will be transmitted only after all the other packets in the pipeline have been transmitted. Yes, that is always true since EHCI HW will send out data(packet) to device one by one at the scheduled frame/uframe according to the order of URB submitting , so the USB audio driver can submit URBs in advance, can't it? Latency is defined as the time interval between the software generating the sample data, and the data actually being played. When the driver submits URBs in advance, latency increases accordingly. (The Linux USB API does not allow changing a URB's buffer after it has been submitted.) Also could you provide the typical time one URB in audio driver may span? Anything between one microframe and twenty milliseconds. Most software ends up using eight milliseconds. Regards, Clemens -- 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 3/3] usb: clamp bInterval to allowed range
Hi, On Mon, Jul 01, 2013 at 09:56:37AM -0400, Alan Stern wrote: On Mon, 1 Jul 2013, Felipe Balbi wrote: bInterval must be within the range 1 - 16, That's true only for high speed and SuperSpeed. For low speed and full speed, bInterval is allowed to range from 1 to 255. See p. 271 in the USB-2 spec. in order to catch drivers passing a too large bInterval (thus zeroing urb-interval), let's clamp() the argument to the allowed range. Signed-off-by: Felipe Balbi ba...@ti.com --- include/linux/usb.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index a232b7e..0883e3a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb, urb-transfer_buffer_length = buffer_length; urb-complete = complete_fn; urb-context = context; + + /* make sure interval is within allowed range */ + interval = clamp(interval, 1, 16); These lines should go within the first branch of the if statement below. could you look at the newer version I sent as a reply to this and tell me what you think of it ? thanks -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/3] usb: clamp bInterval to allowed range
On Mon, 1 Jul 2013, Felipe Balbi wrote: Hi, On Mon, Jul 01, 2013 at 09:56:37AM -0400, Alan Stern wrote: On Mon, 1 Jul 2013, Felipe Balbi wrote: bInterval must be within the range 1 - 16, That's true only for high speed and SuperSpeed. For low speed and full speed, bInterval is allowed to range from 1 to 255. See p. 271 in the USB-2 spec. in order to catch drivers passing a too large bInterval (thus zeroing urb-interval), let's clamp() the argument to the allowed range. Signed-off-by: Felipe Balbi ba...@ti.com --- include/linux/usb.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index a232b7e..0883e3a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb, urb-transfer_buffer_length = buffer_length; urb-complete = complete_fn; urb-context = context; + + /* make sure interval is within allowed range */ + interval = clamp(interval, 1, 16); These lines should go within the first branch of the if statement below. could you look at the newer version I sent as a reply to this and tell me what you think of it ? I shouldn't have replied so quickly to the original post -- it was a violation of my normal policy to read through all the accumulated inbox messages before replying to any of them. :-( Yeah, the updated version is better. I don't think we need to test the full/low-speed case, though. There already are checks in usb_submit_urb() to handle unreasonable values of urb-interval. The only reason for checking in the high/SuperSpeed case is that the stored value is computed from the input value, and you don't want the computation to overflow. 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: How should we handle isochronous underruns?
On Sun, 30 Jun 2013, Clemens Ladisch wrote: Alan Stern wrote: [...] This means that the HCD will have trouble telling the difference between an underrun and a normal restart of a stopped I/O stream. In both cases it will see an URB being submitted to an empty queue. Here's where the URB_ISO_ASAP flag will be useful; if that flag is set then the URB is restarting a stopped stream, but if it isn't set then the pipeline experienced an underrun. Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. There are several possible things the HCD could do when an underrun occurs: It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. The word might shows an important point: due to software and hardware racing against each other, it is _not possible_ to know whether some URB to be submitted will actually still be on time or not. That's a very important point. On the other hand, sometimes it _is_ possible to know that a particular slot has already expired (for example, if a register read indicates the hardware has already reached frame N+1 then we know that frame N is over). It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. For audio, it is important to _try_ to transmit the packet at the _correct_ time; a guarantee that the packet will be transmitted at _some_ time would be worthless. (This is what isochronous is all about.) That the error reporting is delayed from submission to the completion callback is no problem. Should the behavior change if the submission is _extremely_ late? The rogue driver leaving interrupts disabled for over 50 ms is a good example (and one that I actually encountered!). Even when that happens, would you prefer to require the audio driver to submit 50-ms worth of URBs before doing any real I/O? 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/1] usb,uhci: add a new tag for virtual uhci devices
On Mon, 1 Jul 2013, ZhenHua wrote: Let's make it clear: I said I don't have a machine that this makes action different, it does not mean my patch did not make the machine act different . Of course. There are many kinds of machines, I have never said my patch does not make ALL of them act different. But does it make a difference on _any_ machine? There is a good reason for asking this question. The auto-stopp mechanism in uhci-hcd was intended to be a temporary measure. It isn't needed now that we have runtime PM. Adding a patch just to make it work on a few systems doesn't seem worthwhile -- we would be better off removing it entirely. 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 3/3] usb: clamp bInterval to allowed range
Hi, On Mon, Jul 01, 2013 at 10:19:28AM -0400, Alan Stern wrote: On Mon, Jul 01, 2013 at 09:56:37AM -0400, Alan Stern wrote: On Mon, 1 Jul 2013, Felipe Balbi wrote: bInterval must be within the range 1 - 16, That's true only for high speed and SuperSpeed. For low speed and full speed, bInterval is allowed to range from 1 to 255. See p. 271 in the USB-2 spec. in order to catch drivers passing a too large bInterval (thus zeroing urb-interval), let's clamp() the argument to the allowed range. Signed-off-by: Felipe Balbi ba...@ti.com --- include/linux/usb.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index a232b7e..0883e3a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb, urb-transfer_buffer_length = buffer_length; urb-complete = complete_fn; urb-context = context; + + /* make sure interval is within allowed range */ + interval = clamp(interval, 1, 16); These lines should go within the first branch of the if statement below. could you look at the newer version I sent as a reply to this and tell me what you think of it ? I shouldn't have replied so quickly to the original post -- it was a violation of my normal policy to read through all the accumulated inbox messages before replying to any of them. :-( Yeah, the updated version is better. I don't think we need to test the full/low-speed case, though. There already are checks in usb_submit_urb() to handle unreasonable values of urb-interval. The only reason for checking in the high/SuperSpeed case is that the stored value is computed from the input value, and you don't want the computation to overflow. right, which is what happens on two drivers already (see patches 1 and 2). It makes sense to drop the Full/Low speed check, I'll do that and resend :-) Should I add your Reviewed-by or Acked-by ? cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink
On Mon, 1 Jul 2013, Ming Lei wrote: Currently, URB might be probably submitted to HCD too even after usb_hcd_flush_endpoint() completes since both accesses to dev-ep_in[epnum] and ep-enabled aren't protected by effective locks. The urb_list_lock in hcd.c serves to synchronize changes to ep-enabled. An URB might be submitted after usb_hcd_flush_endpoint() completes, but the submission will fail in usb_hcd_link_urb_to_ep(). But the lock isn't held when setting and clearing ep-enabled in usb_enable_endpoint and usb_disable_endpoint, is it? No. But it is held in usb_hcd_flush_endpoint() and usb_hcd_link_urb_to_ep(). Therefore, if a thread clears ep-enabled and then flushes the endpoint, further submissions will fail. However, this does still leave one possible race: On the first submission to an isochronous endpoint, itd_submit() and sitd_submit() will allocate a new ehci_iso_stream before calling usb_hcd_link_urb_to_ep(). If the endpoint has been flushed and disabled, the enqueue will fail but the new iso_stream will not be destroyed -- it will leak. But this is an old failure mode, not related to the new changes. So how about not adding the WARN_ON(!list_empty(qh-qtd_list) now when qh-qh_state is QH_STATE_LINKED? I think we should add the WARN_ON. Or jump to the default case in that switch statement -- maybe the error message there should include a WARN_ON. How about just adding WARN_ON(!list_empty(qh-qtd_list) but continue unlinking the qh like current code, which should be harmless and avoid the QH leak? I think the end result would be the same. It wouldn't hurt and it wouldn't help. We'd still end up at the default case. 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: How should we handle isochronous underruns?
On Mon, 1 Jul 2013, Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Clement and Laurent: The two of you seem to be the people who make the most use of isochronous USB transfers. Since the ehci-hcd driver is being changed to use a tasklet for URB completion callbacks, it looks like I will need to reconsider how isochronous underruns get handled. Without using tasklet, the hard interrupt handler still can be delayed for some time, and switching to tasklet doesn't change the fact of the probable delay of URB handling. Yes. That wasn't the point. The problem will be more acute than in the past, because URB resubmissions will no longer be synchronous with URB completions, thanks to the tasklet. That is, in the current code, if the completion handler resubmits the URB, the resubmission occurs before the HCD finishes the completion callback. But in the new code, the HCD will simply hand the URB over to the tasklet, and the resubmission won't occur until some time later (when the tasklet wakes up and invokes the completion handler). As far as the HCD is concerned, the completion The delay should be very small(generally several microseconds) since isochronous URBs are completed in high priority tasklet. The fact that the delay is small doesn't matter -- what's important is that it will be 0 whereas now there is no delay between completion and resubmission (in fact, the resubmission occurs before the completion ends). I don't think the introduced tasklet delay is a problem for EHCI since per EHCI spec the maximum rate at which the host controller will issue interrupts is one microframe(125us), which means isochronous transfer completion can be reported to CPU with about 125us delay in hardware level. Irrelevant. Besides, the delay can be longer than 125 us if another interrupt is being handled. Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. Single URB may not work well too when running complete() in hard irq context. It could work very well indeed, if the interval between URBs was larger than 1 ms. 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: Linux USB file storage gadget with new UDC
On Mon, 1 Jul 2013, Victor Yeo wrote:
Yes, UDC driver has bug. After modifying it, it can receive Set-Config
request with a config value of 0. However, the device descriptor test
- addressed state still fails.
Please see the attached log. The Set-Config request with a config
value of 0 is the second last USB request sent from the host. The last
USB request is Get-Config, which still returns config value of 1.
This looks like another bug in the UDC driver. It performs the Status
stage of the Set-Config request before the gadget driver has finished
carrying out the request.
Notice that the USB_REQ_SET_CONFIGURATION case in standard_setup_req()
returns DELAYED_STATUS. As a result, fsg_setup() does not call
ep0_queue(), and so usb_ep_queue() doesn't get called. The UDC driver
is not supposed to ACK the Status stage of an OUT control transfer
until usb_ep_queue() is called.
In gadget driver, do_set_config(), if new_config is 0, the new_config
is not processed. So config value of zero will never be saved by
gadget driver. Isn't it?
Look at do_set_config():
/* Disable the single interface */
if (fsg-config != 0) {
DBG(fsg, reset config\n);
fsg-config = 0;
rc = do_set_interface(fsg, -1);
}
/* Enable the interface */
if (new_config != 0) {
...
}
return rc;
So if new_config is 0, fsg-config remains set to 0 and the
deconfiguration is processed by the do_set_interface() call.
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: How should we handle isochronous underruns?
On Mon, 1 Jul 2013, Pratyush Anand wrote: On 6/30/2013 8:32 PM, Alan Stern wrote: There are several possible things the HCD could do when an underrun occurs: I do not have much experience in working with ISOC host.But by the experience of device side I can say that, There could be problem irrespective of tasklet or irq context, only that tasklet would be more prone to it. Should not we have same approach in handling isoc packet in either of the cases? Of course we should. It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. IMO, this approach is better. But, what should we call as first time slot known to be available. Current code calculates it as now (current time slot) + delta microframe, and delta is kept as fixed. However, different system works with different values of delta. IMO, This delta should be dynamic and software should update it on the basis of initial value and feedback (past failure/missed isoc experience). Clemens disagrees, and I accept his recommendation. It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. There is no meaning of submitting an URB for expired time slot. In anyway, it is not going to be transferred and will further result in under-run/missed packet. That is not true. For example, suppose 2 time slots have already expired, but the submitted URB contains 5 packets. Even though it's too late for the first two packets, the last three can still be transferred. 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
Private Message
I use this medium to inform you that you are a beneficiary of 3.8 million pounds willed to you by my late client Mr James Campbell. i advise you get in touch with me. Barr Colin Lee -- 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: How should we handle isochronous underruns?
On Mon, 1 Jul 2013, Laurent Pinchart wrote: That's correct. Unlike the UAC driver, the UVC driver doesn't care too much about exact timings. Avoiding packet loss is my main concern. As video frames are not delivered to userspace before they are fully transmitted, and given the large number of URBs required to transmit a video frame, I can submit more URBs (without reasonable limits) if that can help avoiding underruns. A couple of users have experienced underruns in the past due to a rogue driver disabling interrupts for tens of milliseconds. I don't think switching to tasklets will have a strong adverse effect, the 20ms buffer time should be large enough to avoid underruns in sane systems. What about error recovery for insane systems? If we do get a 50-ms gap in the data stream, what's the best way for the UVC driver to learn this has happens and attempt to carry on? 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 3/3] usb: clamp bInterval to allowed range
On Mon, 1 Jul 2013, Felipe Balbi wrote: It makes sense to drop the Full/Low speed check, I'll do that and resend :-) Should I add your Reviewed-by or Acked-by ? If you want to, sure... This isn't a big deal. 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: Huawei E3131 wwan interface, partial success?
Hi all guys! I discovered osmething very funny, something I wasn't expecting. And when you'll read the story, for sure you will be in my same position! Let me know. Relying on the immense knowledge and kindness of Bjorn and you all, I gathered some infos about the device who made my life extremely interesting, and not easy. so - what protocol do you think it's embedded over ncm? QMI? whatever? ... the protocol seems to be AT. Yes guys - AT! And I could never imagine it looking at the .c driver from Huawei right now. Yes, I'm not extremely acute. But, let's go to the technical thing: ndisdup works. 1 - Detecting connection I could not look at device leds, so I brought an AM radio near it. when I give the ndisdup commad on the standard serial ports, nothing happen. 2 - Connecting to the network To connect to the network you use the normal ndisdup command over WDM. At least in my case, the firmware became unstable, extremely unstable, after some minutes. The normal serial ports, and the at^statqry commands are not aware of what's happening, but the connection is activve. to confirm this, I can say that the modem emits ndis^stats:... unsolicited messages, and becomes hot after a while. Here is what happens: echo is never activated on this at port, even with ATE1 command. Script started on Mon Jul 1 17:08:31 2013 KDGKBLED: Inappropriate ioctl for device ## don't worry, caused by setled Error reading current flags setting. Maybe you are not on the console? mrkiko@eeeadesso:/tmp$ sudo modprobe -r cdc_ncm mrkiko@eeeadesso:/tmp$ sudo modprobe -f cdj mrkiko@eeeadesso:/tmp$ sudo dmesg | tail [ 949.084335] usbcore: deregistering interface driver cdc_ncm [ 949.084405] cdc_ncm 3-2:1.1 wwan0: unregister 'cdc_ncm' usb-:00:1d.7-2, Mobile Broadband Network Device [ 975.807854] mii: module_layout: kernel tainted. [ 975.807873] Disabling lock debugging due to kernel taint [ 975.824301] usb 3-2: MAC-Address: ...not reporting [ 975.828761] cdc_ncm 3-2:1.1: cdc-wdm0: USB WDM device [ 975.829481] cdc_ncm 3-2:1.1 wwan0: register 'cdc_ncm' at usb-:00:1d.7-2, Mobile Broadband Network Device, not reporting [ 975.829616] usbcore: registered new interface driver cdc_ncm And then, minicom mrkiko@eeeadesso:/tmp$ sudo minicom -D /dev/cdc-wdm0 ^RSSI: 19 ^RSSI: 21 ^RSSI: 22 ^RSSI: 19 ^RSSI: 17 ^RSSI: 15 ^RSSI: 17 ^RSSI: 18 ^RSSI: 17 ^RSSI: 15 ^RSSI: 14 ^RSSI: 9 ^RSSI: 15 ^RSSI: 16 ^RSSI: 13 ^RSSI: 11 ^RSSI: 18 ^RSSI: 17 ^RSSI: 16 ^RSSI: 14 ^RSSI: 17 ^RSSI: 17 ^RSSI: 17 ^RSSI: 17 ^RSSI: 18 ^RSSI: 15 ^RSSI: 17 ^RSSI: 16 ^RSSI: 17 ^RSSI: 22 ^RSSI: 20 ^RSSI: 19 ^RSSI: 17 ^RSSI: 19 ^RSSI: 20 ^RSSI: 22 ^MODE: 5,4 ^RSSI: 8 I wrote at+csq +CSQ: 8,99 OK I wrote at^ndisdup=1,1,web.coopvoce.it OK ^NDISSTAT:0,33,,IPV4 It failed because of network low coverage; I retrired.. OK Same story, I retried ^NDISSTAT:0,33,,IPV4 Same Story, tried again OK ^NDISSTAT:1,,,IPV4 ^DSFLOWRPT:0002,,,,,, ^DSFLOWRPT:0004,,,,,, ^DSFLOWRPT:0006,,,,,, ^DSFLOWRPT:0008,,,,,, ^DSFLOWRPT:000A,,,,,, ^DSFLOWRPT:000C,,,,,, ^DSFLOWRPT:000E,,,,,, ^DSFLOWRPT:0010,,,,,, ^DSFLOWRPT:0012,,,,,, ^DSFLOWRPT:0014,,,,,, ^DSFLOWRPT:0016,,,,,, ^DSFLOWRPT:0018,,,,,, ^DSFLOWRPT:001A,,,,,, ^DSFLOWRPT:001C,,,,,, ^DSFLOWRPT:001E,,,,,, ^DSFLOWRPT:0020,,,,,, ^DSFLOWRPT:0022,,,,,, ^DSFLOWRPT:0024,,,,,, ^DSFLOWRPT:0026,,,,,, ^DSFLOWRPT:0028,,,,,, ^DSFLOWRPT:002A,,,,,, ^DSFLOWRPT:002C,,,,,,
Re: [PATCH 3/3] usb: clamp bInterval to allowed range
On Mon, Jul 01, 2013 at 05:24:18PM +0300, Felipe Balbi wrote: Hi, On Mon, Jul 01, 2013 at 10:19:28AM -0400, Alan Stern wrote: On Mon, Jul 01, 2013 at 09:56:37AM -0400, Alan Stern wrote: On Mon, 1 Jul 2013, Felipe Balbi wrote: bInterval must be within the range 1 - 16, That's true only for high speed and SuperSpeed. For low speed and full speed, bInterval is allowed to range from 1 to 255. See p. 271 in the USB-2 spec. in order to catch drivers passing a too large bInterval (thus zeroing urb-interval), let's clamp() the argument to the allowed range. Signed-off-by: Felipe Balbi ba...@ti.com --- include/linux/usb.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index a232b7e..0883e3a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb, urb-transfer_buffer_length = buffer_length; urb-complete = complete_fn; urb-context = context; + + /* make sure interval is within allowed range */ + interval = clamp(interval, 1, 16); These lines should go within the first branch of the if statement below. could you look at the newer version I sent as a reply to this and tell me what you think of it ? I shouldn't have replied so quickly to the original post -- it was a violation of my normal policy to read through all the accumulated inbox messages before replying to any of them. :-( Yeah, the updated version is better. I don't think we need to test the full/low-speed case, though. There already are checks in usb_submit_urb() to handle unreasonable values of urb-interval. The only reason for checking in the high/SuperSpeed case is that the stored value is computed from the input value, and you don't want the computation to overflow. right, which is what happens on two drivers already (see patches 1 and 2). Based on the age of those drivers, I would think that those drivers are correct, right? Those were probably only tested on full speed devices (like the ATM one.) thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink
On Mon, Jul 1, 2013 at 10:44 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: Currently, URB might be probably submitted to HCD too even after usb_hcd_flush_endpoint() completes since both accesses to dev-ep_in[epnum] and ep-enabled aren't protected by effective locks. The urb_list_lock in hcd.c serves to synchronize changes to ep-enabled. An URB might be submitted after usb_hcd_flush_endpoint() completes, but the submission will fail in usb_hcd_link_urb_to_ep(). But the lock isn't held when setting and clearing ep-enabled in usb_enable_endpoint and usb_disable_endpoint, is it? No. But it is held in usb_hcd_flush_endpoint() and usb_hcd_link_urb_to_ep(). Therefore, if a thread clears ep-enabled and then flushes the endpoint, further submissions will fail. I am wondering if holding the lock in usb_hcd_flush_endpoint() can avoid the race completely. Suppose usb_hcd_link_urb_to_ep() in submit path runs on CPU1 just when usb_hcd_flush_endpoint()(called from usb_disable_endpoint()) completes on CPU0, there is no any guarantee that CPU1 can see the up-to-date value of ep-enabled, so the urb might be submitted to HCD successfully. However, this does still leave one possible race: On the first submission to an isochronous endpoint, itd_submit() and sitd_submit() will allocate a new ehci_iso_stream before calling usb_hcd_link_urb_to_ep(). If the endpoint has been flushed and disabled, the enqueue will fail but the new iso_stream will not be destroyed -- it will leak. But this is an old failure mode, not related to the new changes. So how about not adding the WARN_ON(!list_empty(qh-qtd_list) now when qh-qh_state is QH_STATE_LINKED? I think we should add the WARN_ON. Or jump to the default case in that switch statement -- maybe the error message there should include a WARN_ON. How about just adding WARN_ON(!list_empty(qh-qtd_list) but continue unlinking the qh like current code, which should be harmless and avoid the QH leak? I think the end result would be the same. It wouldn't hurt and it wouldn't help. We'd still end up at the default case. It may help to avoid one QH leak, even though the problem is caused by usbcore, so I plan to do that if you have no objection. 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: Huawei E3131 wwan interface
A sighted person confirmed me that: - the device has a blue intermittent led when in stand-by mode - when i connect it, with ndisdup, the led changes to blue, allways on. And dsflowrpt messages starts. So - yes, it works. Now the hard part - wwan interfacing. -- 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
Audio I/O parameters
Clemens: I'm working with James Stone on a bug report, and maybe you can help. The main problem seems to be connected with how snd-usb-audio decides to package its USB transfers. The details are all available at https://bugs.launchpad.net/bugs/1191603 Here's the quick summary. James is using JACK to do audio I/O. The data values are 24 bits (transmitted across the USB bus as 4-byte values). The audio-in stream has two data values per sample (stereo), the audio-out stream has four, and the sampling rate is 44100 Hz. I'm not at all familiar with JACK, but James explained that it uses two parameters to control the data flow: frames/period and periods/buffer. In case you don't already know what these mean, as far as I can tell a frame is a sample, and frames/period is the number of samples that JACK sends to/from the kernel in each system call. Periods/buffer evidently describes the total size of JACK's ring buffer. James has frames/period set to 256 and periods/buffer set to 2. This means the duration of a period is: 256 samples/period / (44100 samples/second) * 8000 microframes/second = 46.44 microframes/period. The bInterval values for the isochronous OUT and IN endpoints are both 1 (this is a high-speed device, so the intervals are in microframes). Therefore I would expect to see snd-usb-audio submitting isochronous URBs with 46 or 47 packets, with a pipeline depth of 2 URBs. This would yield an output latency of 11.6 ms, which is perhaps larger than James would like, but could be adjusted. However, that's not what actually happens. The audio-out stream uses 8 URBs each containing about 7 packets on average, for a latency of 7 ms. The audio-in stream uses 8 URBs each containing 1 packet, for a latency of 0.125 ms and a pipeline duration of 1 ms -- which is too small for the current version of ehci-hcd to accept. Why is there such a large discrepancy between the expected URB characteristics and the actual ones? Why are the audio-out URBs so different from the audio-in (7 packets per URB vs. 1 packet)? How can James adjust his settings to get something more reasonable? For example, URBs containing 11 or 12 packets (64 samples on average) would yield an input latency of 1.5 ms, and a pipeline depth of 4 URBs would yield an output latency of 5.8 ms. These numbers would be equivalent to 64 frames/period and 4 periods/buffer in JACK's terminology. 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 3/4] USB: EHCI: improve interrupt qh unlink
On Mon, 1 Jul 2013, Ming Lei wrote: On Mon, Jul 1, 2013 at 10:44 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: Currently, URB might be probably submitted to HCD too even after usb_hcd_flush_endpoint() completes since both accesses to dev-ep_in[epnum] and ep-enabled aren't protected by effective locks. The urb_list_lock in hcd.c serves to synchronize changes to ep-enabled. An URB might be submitted after usb_hcd_flush_endpoint() completes, but the submission will fail in usb_hcd_link_urb_to_ep(). But the lock isn't held when setting and clearing ep-enabled in usb_enable_endpoint and usb_disable_endpoint, is it? No. But it is held in usb_hcd_flush_endpoint() and usb_hcd_link_urb_to_ep(). Therefore, if a thread clears ep-enabled and then flushes the endpoint, further submissions will fail. I am wondering if holding the lock in usb_hcd_flush_endpoint() can avoid the race completely. Suppose usb_hcd_link_urb_to_ep() in submit path runs on CPU1 just when usb_hcd_flush_endpoint()(called from usb_disable_endpoint()) completes on CPU0, there is no any guarantee that CPU1 can see the up-to-date value of ep-enabled, so the urb might be submitted to HCD successfully. There is indeed such a guarantee, provided by the spinlock. CPU1 acquires urb_list_lock after CPU0 releases it, right? Therefore, all stores to memory performed by CPU0 before releasing the lock will be visible to CPU1 after it acquires the lock. Therefore CPU1 will see that ep-enabled is 0. How about just adding WARN_ON(!list_empty(qh-qtd_list) but continue unlinking the qh like current code, which should be harmless and avoid the QH leak? I think the end result would be the same. It wouldn't hurt and it wouldn't help. We'd still end up at the default case. It may help to avoid one QH leak, even though the problem is caused by usbcore, so I plan to do that if you have no objection. I don't mind -- but I don't see how it will 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: [PATCH 3/3] usb: clamp bInterval to allowed range
On Mon, Jul 01, 2013 at 08:31:38AM -0700, Greg KH wrote: On Mon, Jul 01, 2013 at 05:24:18PM +0300, Felipe Balbi wrote: Hi, On Mon, Jul 01, 2013 at 10:19:28AM -0400, Alan Stern wrote: On Mon, Jul 01, 2013 at 09:56:37AM -0400, Alan Stern wrote: On Mon, 1 Jul 2013, Felipe Balbi wrote: bInterval must be within the range 1 - 16, That's true only for high speed and SuperSpeed. For low speed and full speed, bInterval is allowed to range from 1 to 255. See p. 271 in the USB-2 spec. in order to catch drivers passing a too large bInterval (thus zeroing urb-interval), let's clamp() the argument to the allowed range. Signed-off-by: Felipe Balbi ba...@ti.com --- include/linux/usb.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index a232b7e..0883e3a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb, urb-transfer_buffer_length = buffer_length; urb-complete = complete_fn; urb-context = context; + + /* make sure interval is within allowed range */ + interval = clamp(interval, 1, 16); These lines should go within the first branch of the if statement below. could you look at the newer version I sent as a reply to this and tell me what you think of it ? I shouldn't have replied so quickly to the original post -- it was a violation of my normal policy to read through all the accumulated inbox messages before replying to any of them. :-( Yeah, the updated version is better. I don't think we need to test the full/low-speed case, though. There already are checks in usb_submit_urb() to handle unreasonable values of urb-interval. The only reason for checking in the high/SuperSpeed case is that the stored value is computed from the input value, and you don't want the computation to overflow. right, which is what happens on two drivers already (see patches 1 and 2). Based on the age of those drivers, I would think that those drivers are correct, right? Those were probably only tested on full speed devices (like the ATM one.) sparse still complains, should we then patch usb_fill_int_urb() for highspeed and leave those drivers alone ? -- balbi signature.asc Description: Digital signature
Re: Linux USB file storage gadget with new UDC
Hi,
Yes, UDC driver has bug. After modifying it, it can receive Set-Config
request with a config value of 0. However, the device descriptor test
- addressed state still fails.
Please see the attached log. The Set-Config request with a config
value of 0 is the second last USB request sent from the host. The last
USB request is Get-Config, which still returns config value of 1.
This looks like another bug in the UDC driver. It performs the Status
stage of the Set-Config request before the gadget driver has finished
carrying out the request.
Notice that the USB_REQ_SET_CONFIGURATION case in standard_setup_req()
returns DELAYED_STATUS. As a result, fsg_setup() does not call
ep0_queue(), and so usb_ep_queue() doesn't get called. The UDC driver
is not supposed to ACK the Status stage of an OUT control transfer
until usb_ep_queue() is called.
May i verify my understanding of Set-Config request packet flow?
Host Device
--Setup Packet
--Data0 Packet
-Ack Packet-
-- In Packet
Data1 Packet
- Ack Packet -
- Out Packet
- Data1 Packet
Ack Packet -
ACK the Status stage of an OUT control transfer, is it referring to
the Third ACK packet? So UDC driver should ACK only after Data1 packet
is sent via the usb_ep_queue()?
In gadget driver, do_set_config(), if new_config is 0, the new_config
is not processed. So config value of zero will never be saved by
gadget driver. Isn't it?
Look at do_set_config():
/* Disable the single interface */
if (fsg-config != 0) {
DBG(fsg, reset config\n);
fsg-config = 0;
rc = do_set_interface(fsg, -1);
}
/* Enable the interface */
if (new_config != 0) {
...
}
return rc;
So if new_config is 0, fsg-config remains set to 0 and the
deconfiguration is processed by the do_set_interface() call.
Understand now. Thanks.
victor
--
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 PATCH 4/6] USB: ehci-omap: Suspend the controller during bus suspend
On Mon, 1 Jul 2013, Roger Quadros wrote: On 06/28/2013 10:06 PM, Alan Stern wrote: On Fri, 28 Jun 2013, Roger Quadros wrote: That's not what I meant. Never mind the pinctrl; I was asking about the EHCI controller itself. Under what circumstances does the controller assert its wakeup signal? And how do you tell it to stop asserting that signal? I believe this would be through the EHCI Interrupt enable register (USBINTR). I'm not aware of any other mechanism. That's strange, because ehci_suspend() sets the intr_enable register to 0. So how do you ever get any wakeup interrupts at all? Because after ehci_suspend() for OMAP, we solely rely on the out of band wake up mechanism. i.e. Pad wakeup. I don't know what Pad wakeup is. The wakeup signal has to originate from the EHCI controller, doesn't it? If not, how does the Pad know when a wakeup is needed? We can't enable_irq at the start as the controller will only be resumed after usb_remote_wakeup(). Hmmm, yes, I had realized that at one point and then forgot it. You don't want an IRQ to arrive before you're prepared to handle it. This suggests that you really want to call enable_irq() call at the end of ehci_resume() instead of the beginning. (Convert the return 0 to a jump to the end of the routine.) 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: Huawei E3131 wwan interface, partial success?
Enrico Mioso mrkiko...@gmail.com writes: Hi all guys! I discovered osmething very funny, something I wasn't expecting. And when you'll read the story, for sure you will be in my same position! Let me know. Relying on the immense knowledge and kindness of Bjorn and you all, I gathered some infos about the device who made my life extremely interesting, and not easy. so - what protocol do you think it's embedded over ncm? QMI? whatever? ... the protocol seems to be AT. Yes guys - AT! And I could never imagine it looking at the .c driver from Huawei right now. Yes, I'm not extremely acute. But, let's go to the technical thing: ndisdup works. Ahhh! So when Aleksander wrote supposed to support NDISDUP through the NCM port with endpoint 0 he really meant NDISDUP through endpoint 0... Great work! This is encouraging. Now we know that the protocol is supportable, and we can even most likely reuse some existing userspace support once we have the driver parts in place. 1 - Detecting connection I could not look at device leds, so I brought an AM radio near it. That's a smart buzzer solution :) when I give the ndisdup commad on the standard serial ports, nothing happen. 2 - Connecting to the network To connect to the network you use the normal ndisdup command over WDM. At least in my case, the firmware became unstable, extremely unstable, after some minutes. The normal serial ports, and the at^statqry commands are not aware of what's happening, but the connection is activve. to confirm this, I can say that the modem emits ndis^stats:... unsolicited messages, and becomes hot after a while. Here is what happens: echo is never activated on this at port, even with ATE1 command. That's similar to the E367 MBIM firmware, which also has an embedded AT command channel. Script started on Mon Jul 1 17:08:31 2013 KDGKBLED: Inappropriate ioctl for device ## don't worry, caused by setled Error reading current flags setting. Maybe you are not on the console? mrkiko@eeeadesso:/tmp$ sudo modprobe -r cdc_ncm mrkiko@eeeadesso:/tmp$ sudo modprobe -f cdj mrkiko@eeeadesso:/tmp$ sudo dmesg | tail [ 949.084335] usbcore: deregistering interface driver cdc_ncm [ 949.084405] cdc_ncm 3-2:1.1 wwan0: unregister 'cdc_ncm' usb-:00:1d.7-2, Mobile Broadband Network Device [ 975.807854] mii: module_layout: kernel tainted. [ 975.807873] Disabling lock debugging due to kernel taint [ 975.824301] usb 3-2: MAC-Address: ...not reporting [ 975.828761] cdc_ncm 3-2:1.1: cdc-wdm0: USB WDM device [ 975.829481] cdc_ncm 3-2:1.1 wwan0: register 'cdc_ncm' at usb-:00:1d.7-2, Mobile Broadband Network Device, not reporting [ 975.829616] usbcore: registered new interface driver cdc_ncm And then, minicom mrkiko@eeeadesso:/tmp$ sudo minicom -D /dev/cdc-wdm0 [..] ^RSSI: 22 ^MODE: 5,4 ^RSSI: 8 I wrote at+csq +CSQ: 8,99 OK I wrote at^ndisdup=1,1,web.coopvoce.it OK ^NDISSTAT:0,33,,IPV4 It failed because of network low coverage; I retrired.. OK Same story, I retried ^NDISSTAT:0,33,,IPV4 Same Story, tried again OK ^NDISSTAT:1,,,IPV4 ^DSFLOWRPT:0002,,,,,, Nice. You are certainly connected here. [..] [ 1587.967595] cdc_ncm 3-2:1.1: unknown notification 42 received: index 1 len 8 OK, so that's unknown to cdc-wdm which is now handling all notifications. cdc-wdm logs the number as decimal, so that's most likely a proper (and expected for NCM) USB_CDC_NOTIFY_SPEED_CHANGE. We should probably handle that if we're going this route. Which means that the cdc-wdm subdriver interface needs a small extension. that said - I'm not able to bring up the wwan0 interface right now, but I think it's a problem with the drivers itself. From here on - I need help... Note that cdc_ncm by default requires a link up notification, and you've effectively filtered that out. Could the problem be that simple? If you haven't already, then remove the usbnet_link_change(dev, 0, 0); from your cdc_ncm_bind(). 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
Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink
On Tue, Jul 2, 2013 at 12:02 AM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: On Mon, Jul 1, 2013 at 10:44 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: Currently, URB might be probably submitted to HCD too even after usb_hcd_flush_endpoint() completes since both accesses to dev-ep_in[epnum] and ep-enabled aren't protected by effective locks. The urb_list_lock in hcd.c serves to synchronize changes to ep-enabled. An URB might be submitted after usb_hcd_flush_endpoint() completes, but the submission will fail in usb_hcd_link_urb_to_ep(). But the lock isn't held when setting and clearing ep-enabled in usb_enable_endpoint and usb_disable_endpoint, is it? No. But it is held in usb_hcd_flush_endpoint() and usb_hcd_link_urb_to_ep(). Therefore, if a thread clears ep-enabled and then flushes the endpoint, further submissions will fail. I am wondering if holding the lock in usb_hcd_flush_endpoint() can avoid the race completely. Suppose usb_hcd_link_urb_to_ep() in submit path runs on CPU1 just when usb_hcd_flush_endpoint()(called from usb_disable_endpoint()) completes on CPU0, there is no any guarantee that CPU1 can see the up-to-date value of ep-enabled, so the urb might be submitted to HCD successfully. There is indeed such a guarantee, provided by the spinlock. CPU1 acquires urb_list_lock after CPU0 releases it, right? Therefore, all stores to memory performed by CPU0 before releasing the lock will be visible to CPU1 after it acquires the lock. Therefore CPU1 will see that ep-enabled is 0. OK, I got it, but looks it is a bit tricky and maybe some comment is helpful, thanks for your explanation. How about just adding WARN_ON(!list_empty(qh-qtd_list) but continue unlinking the qh like current code, which should be harmless and avoid the QH leak? I think the end result would be the same. It wouldn't hurt and it wouldn't help. We'd still end up at the default case. It may help to avoid one QH leak, even though the problem is caused by usbcore, so I plan to do that if you have no objection. I don't mind -- but I don't see how it will help. Alan Stern 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: Linux USB file storage gadget with new UDC
On Tue, 2 Jul 2013, Victor Yeo wrote: This looks like another bug in the UDC driver. It performs the Status stage of the Set-Config request before the gadget driver has finished carrying out the request. Notice that the USB_REQ_SET_CONFIGURATION case in standard_setup_req() returns DELAYED_STATUS. As a result, fsg_setup() does not call ep0_queue(), and so usb_ep_queue() doesn't get called. The UDC driver is not supposed to ACK the Status stage of an OUT control transfer until usb_ep_queue() is called. May i verify my understanding of Set-Config request packet flow? Host Device --Setup Packet --Data0 Packet -Ack Packet- -- In Packet Data1 Packet - Ack Packet - - Out Packet - Data1 Packet Ack Packet - No, that's not right. Set-Config has only two stages, Setup and Status; there is no Data stage. The flow is: Host Device - Setup Packet --- | - Data0 Packet --- |== Setup stage Ack Packet -- | - In Packet -- | Data1 Packet |== Status stage - Ack Packet - | ACK the Status stage of an OUT control transfer, is it referring to the Third ACK packet? So UDC driver should ACK only after Data1 packet is sent via the usb_ep_queue()? I meant the Data1 packet above. The UDC driver should not send this packet until the gadget driver tells it to (by calling usb_ep_queue). Until then, it should send NAK in respond to the In packet. 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 3/4] USB: EHCI: improve interrupt qh unlink
On Tue, 2 Jul 2013, Ming Lei wrote: I am wondering if holding the lock in usb_hcd_flush_endpoint() can avoid the race completely. Suppose usb_hcd_link_urb_to_ep() in submit path runs on CPU1 just when usb_hcd_flush_endpoint()(called from usb_disable_endpoint()) completes on CPU0, there is no any guarantee that CPU1 can see the up-to-date value of ep-enabled, so the urb might be submitted to HCD successfully. There is indeed such a guarantee, provided by the spinlock. CPU1 acquires urb_list_lock after CPU0 releases it, right? Therefore, all stores to memory performed by CPU0 before releasing the lock will be visible to CPU1 after it acquires the lock. Therefore CPU1 will see that ep-enabled is 0. OK, I got it, but looks it is a bit tricky and maybe some comment is helpful, thanks for your explanation. Adding a comment would be okay. 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 3/3] usb: clamp bInterval to allowed range
On Mon, Jul 01, 2013 at 07:12:07PM +0300, Felipe Balbi wrote: On Mon, Jul 01, 2013 at 08:31:38AM -0700, Greg KH wrote: On Mon, Jul 01, 2013 at 05:24:18PM +0300, Felipe Balbi wrote: Hi, On Mon, Jul 01, 2013 at 10:19:28AM -0400, Alan Stern wrote: On Mon, Jul 01, 2013 at 09:56:37AM -0400, Alan Stern wrote: On Mon, 1 Jul 2013, Felipe Balbi wrote: bInterval must be within the range 1 - 16, That's true only for high speed and SuperSpeed. For low speed and full speed, bInterval is allowed to range from 1 to 255. See p. 271 in the USB-2 spec. in order to catch drivers passing a too large bInterval (thus zeroing urb-interval), let's clamp() the argument to the allowed range. Signed-off-by: Felipe Balbi ba...@ti.com --- include/linux/usb.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index a232b7e..0883e3a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb, urb-transfer_buffer_length = buffer_length; urb-complete = complete_fn; urb-context = context; + + /* make sure interval is within allowed range */ + interval = clamp(interval, 1, 16); These lines should go within the first branch of the if statement below. could you look at the newer version I sent as a reply to this and tell me what you think of it ? I shouldn't have replied so quickly to the original post -- it was a violation of my normal policy to read through all the accumulated inbox messages before replying to any of them. :-( Yeah, the updated version is better. I don't think we need to test the full/low-speed case, though. There already are checks in usb_submit_urb() to handle unreasonable values of urb-interval. The only reason for checking in the high/SuperSpeed case is that the stored value is computed from the input value, and you don't want the computation to overflow. right, which is what happens on two drivers already (see patches 1 and 2). Based on the age of those drivers, I would think that those drivers are correct, right? Those were probably only tested on full speed devices (like the ATM one.) sparse still complains, should we then patch usb_fill_int_urb() for highspeed and leave those drivers alone ? Nah, the drivers are probably incorrect, it's just that no one has noticed all of these years (I doubt many people use the ATM driver, and the cdc-acm patch was just for broken devices.) So fixing the api up would be good. 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: [RFC PATCH 4/6] USB: ehci-omap: Suspend the controller during bus suspend
On Mon, Jul 01, 2013 at 12:24:07PM -0400, Alan Stern wrote: On Mon, 1 Jul 2013, Roger Quadros wrote: On 06/28/2013 10:06 PM, Alan Stern wrote: On Fri, 28 Jun 2013, Roger Quadros wrote: That's not what I meant. Never mind the pinctrl; I was asking about the EHCI controller itself. Under what circumstances does the controller assert its wakeup signal? And how do you tell it to stop asserting that signal? I believe this would be through the EHCI Interrupt enable register (USBINTR). I'm not aware of any other mechanism. That's strange, because ehci_suspend() sets the intr_enable register to 0. So how do you ever get any wakeup interrupts at all? Because after ehci_suspend() for OMAP, we solely rely on the out of band wake up mechanism. i.e. Pad wakeup. I don't know what Pad wakeup is. The wakeup signal has to originate from the EHCI controller, doesn't it? If not, how does the Pad know when a wakeup is needed? That's really an OMAP thing, I guess. Pad wakeup sits in the PRCM (IIRC) inside and always on power domain. EHCI sits in another power domain which be turned off. When it's turned off (power gated and clock gated) the EHCI block has no means to wakeup whatsoever. That's when pad wakeup comes into play. It is generated when PRCM sees a change in the actual pad of the die. To check who should 'own' the wakeup, it checks the muxing settings to verify whose pad is that. -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink
On Mon, Jul 1, 2013 at 10:44 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: However, this does still leave one possible race: On the first submission to an isochronous endpoint, itd_submit() and sitd_submit() will allocate a new ehci_iso_stream before calling usb_hcd_link_urb_to_ep(). If the endpoint has been flushed and disabled, the enqueue will fail but the new iso_stream will not be destroyed -- it will leak. But this is an old failure mode, not related to the new changes. Yes, not only the stream, but also ehci_iso_sched instance. 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
[GIT PATCH] USB patches for 3.11-rc1
The following changes since commit 9e895ace5d82df8929b16f58e9f515f6d54ab82d: Linux 3.10-rc7 (2013-06-22 09:47:31 -1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-3.11-rc1 for you to fetch changes up to 3a0ddc714a1b8fcbff24c135a1332a28b4668d78: usb: musb: omap2430: make it compile again (2013-06-25 16:41:07 -0700) USB 3.11-rc1 merge Here's the big USB 3.11-rc1 merge request. Lots of gadget and finally, chipidea driver updates (they were much needed), along with a new host controller driver, lots of little serial driver fixes, the removal of the 255 usb-serial device limitation, and a variety of other minor things. All of these have been in the linux-next releases for a while. Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Alan Stern (3): USB: MUSB: upgrade the isochronous API USB: IMX21: upgrade the isochronous API USB: FHCI: upgrade the isochronous API Alex Shi (1): usb/xhci: unify parameter of xhci_msi_irq Alexander Shishkin (2): usb: chipidea: get rid of camelcase names usb: chipidea: drop 13xxx infix Alexandre Peixoto Ferreira (5): USB: usbtmc: Add flag rigol_quirk to usbtmc_device_data USB: usbtmc: TMC request code segregated from usbtmc_read USB: usbtmc: Set rigol_quirk if device is listed USB: usbtmc: usbtmc_read sends multiple TMC header based on rigol_quirk USB: usbtmc: Change magic number to constant Alexey Khoroshilov (2): USB: fix PTR_ERR translation in init_usb_class() usb: gadget: r8a66597-udc: do not unlock unheld spinlock in r8a66597_sudmac_irq() Andrzej Pietrasiewicz (43): usb: gadget: u_ether: convert into module usb: gadget: rndis: convert into module usb: gadget: u_ether: construct with default values and add setters/getters usb: gadget: f_ncm: convert to new function interface with backward compatibility usb: gadget: ncm: convert to new function interface usb: gadget: f_ncm: remove compatibility layer usb: gadget: f_ncm: use usb_gstrings_attach usb: gadget: f_ncm: add configfs support usb: gadget: add helpers for configfs support for USB Ethernet usb: gadget: f_ecm: convert to new function interface with backward compatibility usb: gadget: cdc2: convert to new interface of f_ecm usb: gadget: f_ecm: use usb_gstrings_attach usb: gadget: f_ecm: add configfs support usb: gadget: f_obex: use usb_gstrings_attach usb: gadget: nokia: convert to new interface of f_obex usb: gadget: f_obex: remove compatibility layer usb: gadget: phonet: move global dev variable to its user usb: gadget: f_phonet: convert to new function interface with backward compatibility usb: gadget: nokia: convert to new interface of f_phonet usb: gadget: f_phonet: remove compatibility layer usb: gadget: nokia: convert to new interface of f_ecm usb: gadget: f_phonet: add configfs support usb: gadget: u_ether: allow getting binary-form host address usb: gadget: ether: convert to new interface of f_ecm usb: gadget: f_eem: convert to new function interface with backward compatibility usb: gadget: ether: convert to new interface of f_eem usb: gadget: f_eem: remove compatibility layer usb: gadget: f_eem: use usb_gstrings_attach usb: gadget: f_eem: add configfs support usb: gadget: multi: Remove unused include usb: gadget: f_subset: convert to new function interface with backward compatibility usb: gadget: ether: convert to new interface of f_subset usb: gadget: f_subset: use usb_gstrings_attach usb: gadget: f_subset: add configfs support usb: gadget: f_rndis: convert to new function interface with backward compatibility usb: gadget: ether: convert to new interface of f_rndis usb: gadget: rndis: init exit rndis at module load/unload usb: gadget: f_rndis: use usb_gstrings_attach usb: gadget: f_rndis: add configfs support usb: gadget: f_mass_storage: fix default product name usb/gadget: Kconfig: fix separate building of configfs-enabled functions Documentation/usb: gadget_configfs Documentation/ABI/testing: configfs-based usb gadgets Andy Shevchenko (4): usb: serial: dump small buffers with help of %*ph uwb: use %*ph specifier to dump buffer usb: chipidea: remove superfluous pci_set_drvdata(pci, NULL) usb: chipidea: move to pcim_* functions Arnaud Patard (1): usbmisc_imx: allow autoloading on according to dt ids Arnd Bergmann (2): USB: OHCI: remove bogus #error usb: host: make USB_ARCH_HAS_?HCI obsolete Boris BREZILLON (2): USB: ohci-at91: prepare clk before calling enable ehci-atmel.c: prepare clk before
Re: EHCI debug device gadgets and host
On Mon, Jul 01, 2013 at 09:32:13AM +0300, Kyösti Mälkki wrote: Hi Is someone actively working on or maintaining the EHCI debug device gadget driver? I would like to see some fixes and/or improvements to it but do not know the gadget framework and some of the required hardware is not yet on my desk. Patches are always gladly accepted, so feel free to send them based on the issues you have found. 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: Scanner fails on USB3 port
On Mon, Jul 01, 2013 at 10:45:27AM +0200, Martin van Es wrote: On Mon, Jul 1, 2013 at 12:17 AM, Greg KH gre...@linuxfoundation.org wrote: On Sun, Jun 30, 2013 at 09:35:59PM +0200, Martin van Es wrote: 3.9.8 brought a tiny improvement! scanimage -L now succesfully reports the scanner, but then hangs. I still can not scan with xsane however (no scanner device found) $ scanimage -L device `plustek:libusb:001:004' is a Canon CanoScan N670U/N676U/LiDE20 flatbed scanner (hang, but eventually returns prompt after couple of minutes) Any chance you can try 3.10-rc7 to see if anything improved there? 3.10.0 is back to zero: no result on scanimage -L Ugh. Can you run 'usbmon' and get a dump of what is happening here? 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: Huawei E3131 wwan interface, partial success?
Ah - I didn't knew there where other devices with an embedded AT channel - I really don't imagine what could hide on those devices! :) I can't find the function call you're referrint to, sorry. :( And - another question. how can the cdc_ncm driver handle notifications if it has no control over the -control endpoint anymore? And - why I'm using a function called *_release_interface_* when I'm not talking about an interface, but an endpoint? But guess this is off-topic :) !! On Mon, 1 Jul 2013, Bj?rn Mork wrote: ==Date: Mon, 01 Jul 2013 18:28:20 +0200 ==From: Bj?rn Mork bj...@mork.no ==To: Enrico Mioso mrkiko...@gmail.com ==Cc: linux-usb@vger.kernel.org ==Subject: Re: Huawei E3131 wwan interface, partial success? == ==Enrico Mioso mrkiko...@gmail.com writes: == == Hi all guys! == I discovered osmething very funny, something I wasn't expecting. == And when you'll read the story, for sure you will be in my same position! Let == me know. == == Relying on the immense knowledge and kindness of Bjorn and you all, I gathered == some infos about the device who made my life extremely interesting, and not == easy. == == so - what protocol do you think it's embedded over ncm? == QMI? == whatever? == ... the protocol seems to be AT. == Yes guys - AT! And I could never imagine it looking at the .c driver from == Huawei right now. Yes, I'm not extremely acute. But, let's go to the technical == thing: ndisdup works. == ==Ahhh! So when Aleksander wrote supposed to support NDISDUP through the ==NCM port with endpoint 0 he really meant NDISDUP through endpoint 0... == ==Great work! This is encouraging. Now we know that the protocol is ==supportable, and we can even most likely reuse some existing userspace ==support once we have the driver parts in place. == == == 1 - Detecting connection == I could not look at device leds, so I brought an AM radio near it. == ==That's a smart buzzer solution :) == == when I give the ndisdup commad on the standard serial ports, nothing == happen. == == 2 - Connecting to the network == To connect to the network you use the normal ndisdup command over WDM. == == At least in my case, the firmware became unstable, extremely unstable, after == some minutes. The normal serial ports, and the at^statqry commands are not == aware of what's happening, but the connection is activve. to confirm this, I == can say that the modem emits == ndis^stats:... == unsolicited messages, and becomes hot after a while. == == Here is what happens: echo is never activated on this at port, even with ATE1 == command. == ==That's similar to the E367 MBIM firmware, which also has an embedded AT ==command channel. == == Script started on Mon Jul 1 17:08:31 2013 == KDGKBLED: Inappropriate ioctl for device ## don't worry, caused by setled == Error reading current flags setting. Maybe you are not on the console? == mrkiko@eeeadesso:/tmp$ sudo modprobe -r cdc_ncm == mrkiko@eeeadesso:/tmp$ sudo modprobe -f cdj == mrkiko@eeeadesso:/tmp$ sudo dmesg | tail == [ 949.084335] usbcore: deregistering interface driver cdc_ncm == [ 949.084405] cdc_ncm 3-2:1.1 wwan0: unregister 'cdc_ncm' usb-:00:1d.7-2, Mobile Broadband Network Device == [ 975.807854] mii: module_layout: kernel tainted. == [ 975.807873] Disabling lock debugging due to kernel taint == [ 975.824301] usb 3-2: MAC-Address: ...not reporting == [ 975.828761] cdc_ncm 3-2:1.1: cdc-wdm0: USB WDM device == [ 975.829481] cdc_ncm 3-2:1.1 wwan0: register 'cdc_ncm' at usb-:00:1d.7-2, Mobile Broadband Network Device, not reporting == [ 975.829616] usbcore: registered new interface driver cdc_ncm == == And then, minicom == mrkiko@eeeadesso:/tmp$ sudo minicom -D /dev/cdc-wdm0 ==[..] == == ^RSSI: 22 == == ^MODE: 5,4 == == ^RSSI: 8 == I wrote at+csq == == == +CSQ: 8,99 == == OK == I wrote at^ndisdup=1,1,web.coopvoce.it == OK == == ^NDISSTAT:0,33,,IPV4 == It failed because of network low coverage; I retrired.. == OK == Same story, I retried == ^NDISSTAT:0,33,,IPV4 == Same Story, tried again == OK == == ^NDISSTAT:1,,,IPV4 == == ^DSFLOWRPT:0002,,,,,, == ==Nice. You are certainly connected here. ==[..] == == [ 1587.967595] cdc_ncm 3-2:1.1: unknown notification 42 received: index 1 len 8 == ==OK, so that's unknown to cdc-wdm which is now handling all ==notifications. cdc-wdm logs the number as decimal, so that's most ==likely a proper (and expected for NCM) USB_CDC_NOTIFY_SPEED_CHANGE. == ==We should probably handle that if we're going this route. Which means ==that the cdc-wdm subdriver interface needs a small extension. == == that said - I'm not able to bring up the wwan0 interface right now, but I think == it's a problem with the drivers itself. == == From here on - I need help... == ==Note that cdc_ncm by default requires a link up notification, and you've ==effectively filtered that out. Could the problem be that simple? If ==you haven't already, then remove the ==
Re: Huawei E3131 wwan interface, partial success?
Enrico Mioso mrkiko...@gmail.com writes: Ah - I didn't knew there where other devices with an embedded AT channel - I really don't imagine what could hide on those devices! :) Yes, but I did not think of it because in the case of the E367 this is a highly optional feature. MBIM has a standard management protocol, and the AT channel doesn't add anything useful. I can't find the function call you're referrint to, sorry. :( Sorry, I should have noticed this was recently changed. It used to be netif_carrier_off(dev-net); before commit 8a34b0ae (usbnet: cdc_ncm: apply usbnet_link_change). I guess you are working in a v3.9 based tree? Remove that and see if you can get the network interface going. And - another question. how can the cdc_ncm driver handle notifications if it has no control over the -control endpoint anymore? We'd need to extend the cdc-wdm subdriver interface with additional callbacks, so that cdc-wdm can call the appropriate network driver functions when it receives notifications it doesn't handle. But I believe implementing this can wait. Better focus on getting the basic functionality in place for now. And - why I'm using a function called *_release_interface_* when I'm not talking about an interface, but an endpoint? But guess this is off-topic :) !! I don't know... 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
Re: Huawei E3131 wwan interface, partial success?
Hi! I read your mail from the wwan0 interface :) . This time I was able to understand the netif thing by re-reading the code. The device works in ncm mode - the wdm device has became inaccessible, and any attempt to write to it results in a LOT of unknown notifications, obviously. My dmesg: ... [ 124.508433] cdc_ncm 1-2:1.1: unknown notification 42 received: index 1 len 8 [ 137.673907] IPv6: ADDRCONF(NETDEV_UP): wwan0: link is not ready [ 137.674431] usb 1-2: NCM: unexpected notification 0x01! [ 137.674594] IPv6: ADDRCONF(NETDEV_CHANGE): wwan0: link becomes ready [ 215.212011] usb 1-2: NCM: unexpected notification 0x01! [ 215.338349] usb 1-2: NCM: unexpected notification 0x01! [ 215.556161] usb 1-2: NCM: unexpected notification 0x01! [ 215.732430] usb 1-2: NCM: unexpected notification 0x01! [ 215.868418] usb 1-2: NCM: unexpected notification 0x01! [ 216.108429] usb 1-2: NCM: unexpected notification 0x01! [ 216.204367] usb 1-2: NCM: unexpected notification 0x01! [ 216.364424] usb 1-2: NCM: unexpected notification 0x01! [ 216.452420] usb 1-2: NCM: unexpected notification 0x01! [ 216.676428] usb 1-2: NCM: unexpected notification 0x01! [ 216.772163] usb 1-2: NCM: unexpected notification 0x01! [ 216.876425] usb 1-2: NCM: unexpected notification 0x01! [ 217.100426] usb 1-2: NCM: unexpected notification 0x01! [ 217.244420] usb 1-2: NCM: unexpected notification 0x01! [ 217.332432] usb 1-2: NCM: unexpected notification 0x01! But now - no problems, the thing is quiet and all seems fine! On Mon, 1 Jul 2013, Bj?rn Mork wrote: ==Date: Mon, 01 Jul 2013 19:59:51 +0200 ==From: Bj?rn Mork bj...@mork.no ==To: Enrico Mioso mrkiko...@gmail.com ==Cc: linux-usb@vger.kernel.org ==Subject: Re: Huawei E3131 wwan interface, partial success? == ==Enrico Mioso mrkiko...@gmail.com writes: == == Ah - I didn't knew there where other devices with an embedded AT channel - I == really don't imagine what could hide on those devices! :) == ==Yes, but I did not think of it because in the case of the E367 this is a ==highly optional feature. MBIM has a standard management protocol, and ==the AT channel doesn't add anything useful. == == I can't find the function call you're referrint to, sorry. :( == ==Sorry, I should have noticed this was recently changed. It used to be == == netif_carrier_off(dev-net); == ==before commit 8a34b0ae (usbnet: cdc_ncm: apply usbnet_link_change). I ==guess you are working in a v3.9 based tree? == ==Remove that and see if you can get the network interface going. == == And - another question. how can the cdc_ncm driver handle notifications if it == has no control over the -control endpoint anymore? == ==We'd need to extend the cdc-wdm subdriver interface with additional ==callbacks, so that cdc-wdm can call the appropriate network driver ==functions when it receives notifications it doesn't handle. But I ==believe implementing this can wait. Better focus on getting the basic ==functionality in place for now. == == And - why I'm using a function called *_release_interface_* when I'm not == talking about an interface, but an endpoint? But guess this is off-topic :) !! == ==I don't know... == == == ==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
Re: How should we handle isochronous underruns?
Hi Alan, On Monday 01 July 2013 11:08:33 Alan Stern wrote: On Mon, 1 Jul 2013, Laurent Pinchart wrote: That's correct. Unlike the UAC driver, the UVC driver doesn't care too much about exact timings. Avoiding packet loss is my main concern. As video frames are not delivered to userspace before they are fully transmitted, and given the large number of URBs required to transmit a video frame, I can submit more URBs (without reasonable limits) if that can help avoiding underruns. A couple of users have experienced underruns in the past due to a rogue driver disabling interrupts for tens of milliseconds. I don't think switching to tasklets will have a strong adverse effect, the 20ms buffer time should be large enough to avoid underruns in sane systems. What about error recovery for insane systems? If we do get a 50-ms gap in the data stream, what's the best way for the UVC driver to learn this has happens and attempt to carry on? When isochronous packets are lost video frames get corrupted. UVC doesn't provide sequence numbers for packets, so there's no way to know exactly how many packets have been lost. However, several synchronization methods are available for the driver to detect frame boundaries (namely an End-Of-Frame bit and a Frame ID bit that toggles for every frame in isochronous packet headers). The driver already handles those bits and marks video frames as complete when the EOF bit or an FID transition is detected. For uncompressed formats the driver then checks whether the received data size matches the frame size, and marks the frame as bad if it doesn't. That check can't be performed for compressed formats as the frame size is then variable. It would thus be helpful to receive a notification when a gap in the data stream is detected, through the URB status field for instance. The driver could then mark the frame being received as faulty and reset its state machine to resynchronize to the next frame boundary. -- Regards, Laurent Pinchart -- 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: Scanner fails on USB3 port
Here's the usbmon log on bus 01 worflow: 1. cat /sys/kernel/debug/usb/usbmon/1u 1u.mon.out 2. plug scanner 3. scanimage -L 4. unplug scanner 5. stop cat Regards, Martin On Mon, Jul 1, 2013 at 7:34 PM, Greg KH gre...@linuxfoundation.org wrote: On Mon, Jul 01, 2013 at 10:45:27AM +0200, Martin van Es wrote: On Mon, Jul 1, 2013 at 12:17 AM, Greg KH gre...@linuxfoundation.org wrote: On Sun, Jun 30, 2013 at 09:35:59PM +0200, Martin van Es wrote: 3.9.8 brought a tiny improvement! scanimage -L now succesfully reports the scanner, but then hangs. I still can not scan with xsane however (no scanner device found) $ scanimage -L device `plustek:libusb:001:004' is a Canon CanoScan N670U/N676U/LiDE20 flatbed scanner (hang, but eventually returns prompt after couple of minutes) Any chance you can try 3.10-rc7 to see if anything improved there? 3.10.0 is back to zero: no result on scanimage -L Ugh. Can you run 'usbmon' and get a dump of what is happening here? thanks, greg k-h -- If 'but' was any useful, it would be a logic operator 1u.mon.out Description: Binary data
Re: Huawei E3131 wwan interface, partial success?
Enrico Mioso mrkiko...@gmail.com writes: Hi! I read your mail from the wwan0 interface :) . This time I was able to understand the netif thing by re-reading the code. The device works in ncm mode - the wdm device has became inaccessible, and any attempt to write to it results in a LOT of unknown notifications, obviously. My dmesg: ... [ 124.508433] cdc_ncm 1-2:1.1: unknown notification 42 received: index 1 len 8 [ 137.673907] IPv6: ADDRCONF(NETDEV_UP): wwan0: link is not ready [ 137.674431] usb 1-2: NCM: unexpected notification 0x01! [ 137.674594] IPv6: ADDRCONF(NETDEV_CHANGE): wwan0: link becomes ready [ 215.212011] usb 1-2: NCM: unexpected notification 0x01! [ 215.338349] usb 1-2: NCM: unexpected notification 0x01! [ 215.556161] usb 1-2: NCM: unexpected notification 0x01! Ok, you've obviously ended up with cdc_ncm listening on the notification endpoint instead of cdc-wdm here. Expect a crash coming up :) The simple test approach does of course need some cleanup before it is ready for production. But you have proved the important parts: - the network function is standard CDC NCM, except that there doesn't seem to be any USB_CDC_NOTIFY_NETWORK_CONNECTION notification (which is mandatory in NCM) - there is a USB_CDC_NOTIFY_SPEED_CHANGE notification - the embedded management protocol is Huawei specific AT commands From here, I believe you should start looking at how to create a driver supporting this. Factoring out the parts of cdc_ncm you need and creating a separate driver for these devices seems cleanest to me, but that's only my opinion before trying it. I could be wrong. It is also possible to add the support to cdc_ncm as a new driver template with the necessary functions supporting it. From the look of it, I believe you only need cdc_ncm_rx_fixup and cdc_ncm_tx_fixup in addition to the functions already exported. And maybe also factor out and export the handling of the two NCM notifications from cdc_ncm_status so they can be shared. Other than that, the driver can be simple bind/unbind/suspend/resume/ manage_power boiler-plate code from cdc_mbim and/or qmi_wwan. The hardest part is probably extending the cdc-wdm API. If you google for the pre-cdc_mbim discussions, I believe Oliver Neukum had some nice thoughts on that then. We originally thought that we had to extend the API with network function notification support for cdc_mbim, but it turned out we didn't. Still, the discussions from then are relevant to this case. There were some code examples circulating too. 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
Re: How should we handle isochronous underruns?
On Mon, 1 Jul 2013, Laurent Pinchart wrote: What about error recovery for insane systems? If we do get a 50-ms gap in the data stream, what's the best way for the UVC driver to learn this has happens and attempt to carry on? When isochronous packets are lost video frames get corrupted. UVC doesn't provide sequence numbers for packets, so there's no way to know exactly how many packets have been lost. However, several synchronization methods are available for the driver to detect frame boundaries (namely an End-Of-Frame bit and a Frame ID bit that toggles for every frame in isochronous packet headers). The driver already handles those bits and marks video frames as complete when the EOF bit or an FID transition is detected. For uncompressed formats the driver then checks whether the received data size matches the frame size, and marks the frame as bad if it doesn't. That check can't be performed for compressed formats as the frame size is then variable. It would thus be helpful to receive a notification when a gap in the data stream is detected, through the URB status field for instance. The driver could then mark the frame being received as faulty and reset its state machine to resynchronize to the next frame boundary. One possibility is to set urb-status to -EXDEV if an underrun has caused the entire URB to be too late. Currently, under those conditions the HCD rejects the URB submission with a -EXDEV error code, which doesn't seem to be the right approach. Drivers don't expect isochronous _submissions_ to fail, although they are prepared to see failure statuses for isochronous _completions_. Of course, it has been true all along that the status fields in the URB's individual usb_iso_packet_descriptor structures indicate any errors. But HCDs tend to set urb-status to 0 always, for isochronous URBs. Is there any advantage to setting urb-status to -EXDEV, given that we already set urb-iso_frame_desc[i].status to -EXDEV for each i? (There's also an urb-error_count field, which reports how many of the isochronous packets got an error. It is hardly used; I think we could remove it.) This boils down to three possibilities for how to handle URBs that were submitted too late -- that is, so late that all the time slots for all the URB's packets are known to have expired already: (1) Reject the submission with -EXDEV -- this is what we do now. (2) Accept the submission, but have the URB complete immediately with urb-status and all the packet statuses set to -EXDEV. (3) Accept the submission, but have the URB complete immediately with urb-status set to 0 and all the packet statuses set to -EXDEV. For (1), the only way to recover is to submit an URB with URB_ISO_ASAP set. This is essentially the same as shutting down the stream and restarting it. For (2) and (3), the stream's next time slot value would be updated in the usual way. For example, if 10 slots had expired and the driver was submitting URBs containing 4 packets each, then the first and second URBs would complete right away with errors, and the first two packets of the third URB would get errors, but the last two packets of the third URB would be assigned to the two upcoming time slots and would be treated normally. Thus recovery would be automatic, at the cost of wasting two URBs. Since we expect underruns to be rare, maybe this is acceptable. With (2) or (3), the driver could also recover right away by setting the URB_ISO_ASAP flag on its next URB, but then synchronization would be lost. You wouldn't want to do this if synchronization matters. But if it doesn't, the driver could simply set URB_ISO_ASAP on every URB to avoid worrying about the problem -- the flag would have no effect in the absence of underruns. (The main reason the current code uses (1) is because (2) and (3) require completion to occur _during_ submission. Resubmissions would thus be nested, consuming excessive stack space, and there would be a deadlock if the completion handler tried to acquire a lock that was held during submission. When a tasklet is used for completions, none of these problems arise.) Any preference? Clemens, what do you think? 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: Scanner fails on USB3 port
On Mon, 1 Jul 2013, Martin van Es wrote: Here's the usbmon log on bus 01 worflow: 1. cat /sys/kernel/debug/usb/usbmon/1u 1u.mon.out 2. plug scanner 3. scanimage -L 4. unplug scanner 5. stop cat The problems start here: 88023423db40 1844894429 S Bo:1:004:3 -115 4 = 01070001 88023423db40 1844894484 C Bo:1:004:3 -71 0 88023423db40 1844894509 S Co:1:004:0 s 02 01 0003 0 88023423db40 1844894571 C Co:1:004:0 -71 0 Two transfers failed, with low-level errors. The second transfer was a Clear-Halt, even though the endpoint wasn't halted. 880231da76c0 1845894772 S Bo:1:004:3 -115 4 = 01070001 880231da76c0 1845894861 C Bo:1:004:3 0 4 880232b38b40 1845894959 S Bi:1:004:2 -115 1 880232b38b40 1868423627 C Bi:1:004:2 -108 0 Then one transfer worked and another one hung. Next, for some unknown reason there was a logical disconnect. Maybe the scanimage program quit. 880232df6900 1877247948 C Ii:1:001:1 0:2048 1 = 04 880232df6900 1877247970 S Ii:1:001:1 -115:2048 4 88020fef2840 1877248025 S Ci:1:001:0 s a3 00 0002 0004 4 88020fef2840 1877248032 C Ci:1:001:0 0 4 = 00010100 This is the physical disconnect -- when the cable was unplugged. It happened 9 seconds after the logical disconnect. Martin, can you provide a similar usbmon trace with the scanner plugged into a USB-2 port? 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: How should we handle isochronous underruns?
Alan Stern wrote: (1) Reject the submission with -EXDEV -- this is what we do now. (2) Accept the submission, but have the URB complete immediately with urb-status and all the packet statuses set to -EXDEV. (3) Accept the submission, but have the URB complete immediately with urb-status set to 0 and all the packet statuses set to -EXDEV. The audio driver currently ignores submission errors (because of a bug: the obvious way to stop the PCM stream would result in a deadlock; the tasklet completion might change this), and checks urb-status only to detect if the stream should be stopped (because of a 'normal' stop or unplugging.) So (2) or (3) (no matter which) is preferrable over (1). For (1), the only way to recover is to submit an URB with URB_ISO_ASAP set. This is essentially the same as shutting down the stream and restarting it. For (2) and (3), the stream's next time slot value would be updated in the usual way. For example, if 10 slots had expired and the driver was submitting URBs containing 4 packets each, then the first and second URBs would complete right away with errors, and the first two packets of the third URB would get errors, but the last two packets of the third URB would be assigned to the two upcoming time slots and would be treated normally. Thus recovery would be automatic, This is certainly preferrable over (1). at the cost of wasting two URBs. Since we expect underruns to be rare, maybe this is acceptable. And it's the only recovery mechanism that can preserves the timing/ synchronization of the overall stream. Regards, Clemens -- 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: Clear both buffers when clearing a control transfer TT buffer.
On Thu, 27 Jun 2013, William Gulland wrote:
Control transfers have both IN and OUT (or SETUP) packets, so when
clearing TT buffers for a control transfer it's necessary to send
two HUB_CLEAR_TT_BUFFER requests to the hub.
Signed-off-by: William Gulland wgull...@google.com
---
drivers/usb/core/hub.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index feef935..868ad74 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -668,6 +668,15 @@ resubmit:
static inline int
hub_clear_tt_buffer (struct usb_device *hdev, u16 devinfo, u16 tt)
{
+ /* Need to clear both directions for control ep */
+ if (((devinfo 11) USB_ENDPOINT_XFERTYPE_MASK) ==
+ USB_ENDPOINT_XFER_CONTROL) {
+ int status = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+ HUB_CLEAR_TT_BUFFER, USB_RT_PORT,
+ devinfo ^ 0x8000, tt, NULL, 0, 1000);
+ if (status)
+ return status;
+ }
return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
HUB_CLEAR_TT_BUFFER, USB_RT_PORT, devinfo,
tt, NULL, 0, 1000);
Well, this won't hurt much. And since it does seem to fix a hardware
problem,
Acked-by: Alan Stern st...@rowland.harvard.edu
--
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/2] ARM: dts: tegra20: Rename USB UTMI parameters according to new definitions
On 06/26/2013 05:58 AM, Mikko Perttunen wrote: Signed-off-by: Mikko Perttunen mperttu...@nvidia.com The series, Tested-by: Stephen Warren swar...@nvidia.com I'll at least apply the *.dtsi changes once the merge window is over, and prepare a branch so they can be pulled into the USB tree as a basis for the code changes. -- 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 PATCH 4/6] USB: ehci-omap: Suspend the controller during bus suspend
On Mon, 1 Jul 2013, Felipe Balbi wrote: I don't know what Pad wakeup is. The wakeup signal has to originate from the EHCI controller, doesn't it? If not, how does the Pad know when a wakeup is needed? That's really an OMAP thing, I guess. Pad wakeup sits in the PRCM (IIRC) inside and always on power domain. EHCI sits in another power domain which be turned off. When it's turned off (power gated and clock gated) the EHCI block has no means to wakeup whatsoever. That's when pad wakeup comes into play. It is generated when PRCM sees a change in the actual pad of the die. To check who should 'own' the wakeup, it checks the muxing settings to verify whose pad is that. How does the PRCM know which changes should generate wakeup events? In the EHCI controller, this is managed by the settings of the WKOC_E, WKDSCNNT_E, and WKCNNT_E bits in the PORTSC registers. But if EHCI is powered off, those bits can't be used. Also, once the wakeup signal has been turned on, how does it get turned off again? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] usb: tegra: Replace nvidia,vbus-gpio property with vbus-supply
On 06/28/2013 06:33 AM, Mikko Perttunen wrote: This patchset removes the ehci-tegra device tree property nvidia,vbus-gpio and adds support for the vbus-supply property to phy-tegra-usb. With bug-fix patch ARM: tegra: fix VBUS regulator GPIO polarity in DT (which I just sent) applied first, this series, Tested-by: Stephen Warren swar...@nvidia.com -- 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 4/9] ARM: tegra: Remove USB platform data
On 06/28/2013 03:37 PM, Tuomas Tynkkynen wrote: USB-related platform data is not used anymore in the Tegra USB drivers, so remove all of it. Acked-by: Stephen Warren swar...@nvidia.com -- 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 5/9] usb: phy: tegra: Register as an USB PHY.
On 06/28/2013 03:37 PM, Tuomas Tynkkynen wrote:
Register the Tegra PHY device instances with the PHY subsystem so that
the Tegra EHCI driver can locate a PHY via the standard APIs.
diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
+ err = usb_add_phy_dev(tegra_phy-u_phy);
+ if (err 0) {
+ tegra_usb_phy_close(tegra_phy-u_phy);
+ return err;
+ }
Don't you want to do that a bit later? In particular ...
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);
... that all happens *after* the call to usb_add_phy_dev() call, yet I
think the USB core could legally call any of the PHY methods after the
call to usb_add_phy_dev(), and that probably requires all those methods
pointers and drvdata to be set up? I suggest moving the call to
usb_add_phy_dev() to the very end of the function.
+static int tegra_usb_phy_remove(struct platform_device *pdev)
+{
+ struct tegra_usb_phy *tegra_phy = platform_get_drvdata(pdev);
+
+ usb_remove_phy(tegra_phy-u_phy);
+ return 0;
+}
Nit: I'd typically expect to see a blank line before the return statement.
--
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 9/9] usb: phy: tegra: Use DT helpers for dr_mode
On 06/28/2013 03:37 PM, Tuomas Tynkkynen wrote:
Use the new of_usb_get_dr_mode helper function for parsing dr_mode
from the device tree. Also replace the usage of the custom
tegra_usb_phy_mode enum with the standard enum.
diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
+ tegra_phy-mode = of_usb_get_dr_mode(np);
+ if (tegra_phy-mode == USB_DR_MODE_UNKNOWN) {
+ dev_err(pdev-dev, dr_mode is invalid\n);
+ return -EINVAL;
+ }
Unfortunately, of_usb_get_dr_mode() returns USB_DR_MODE_UNKNOWN if the
property is missing, rather than defaulting to host mode as the original
code here did. I would suggest solving this by:
diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
index 675384d..6391de5 100644
--- a/drivers/usb/usb-common.c
+++ b/drivers/usb/usb-common.c
@@ -103,7 +103,7 @@ enum usb_dr_mode of_usb_get_dr_mode(struct device_node
*np)
err = of_property_read_string(np, dr_mode, dr_mode);
if (err 0)
- return USB_DR_MODE_UNKNOWN;
+ return USB_DR_MODE_HOST;
for (i = 0; i ARRAY_SIZE(usb_dr_modes); i++)
if (!strcmp(dr_mode, usb_dr_modes[i]))
This can't be done by the caller, since of_usb_get_dr_mode() returns
UNKNOWN in two cases, which the caller can't distinguish without
manually checking whether the property exists first:
a) Property is not present (which should default to HOST mode at least
for the Tegra binding, as in the patch I show above).
b) Property is present, but contains an invalid value, which probably
should cause the driver to error-out.
This is only a problem for dr_mode in the Tegra binding: dr_mode is an
optional property, whereas the phy_type property as parsed by patch 8/9
is mandatory, so this issue doesn't come up.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] Tegra USB cleanup series
On 06/28/2013 03:36 PM, Tuomas Tynkkynen wrote: Hi, Here's a few cleanup patches for the Tegra USB drivers, to be applied on top of Mikko's two patch sets. It mostly deals with removing all usage of platform data and using standard helpers and enums instead of Tegra-specific ones. With the issues in patches 5/9 and 9/9 fixed as I mentioned, this series, Tested-by: Stephen Warren swar...@nvidia.com -- 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] net: ipheth: Add USB ID for iPad mini
Adds the USB device ID (0x12ab) to the ipheth network-over-USB-tethering
driver for iOS devices. Applied and tested against mainline tag v3.10
(as well as 3.8.x and 3.6.y kernel for Raspbian on Raspberry pi)
Signed-off-by: Aaron Marburg amarb...@notetofutureself.org
---
diff -upr -X linux-3.10.pristine/Documentation/dontdiff
linux-3.10.pristine/drivers/net/usb/ipheth.c
linux-3.10.ipheth/drivers/net/usb/ipheth.c
--- linux-3.10.pristine/drivers/net/usb/ipheth.c2013-07-01
21:28:56.577106633 +1200
+++ linux-3.10.ipheth/drivers/net/usb/ipheth.c 2013-07-01 21:31:31.021102411
+1200
@@ -60,6 +60,7 @@
#define USB_PRODUCT_IPHONE_3GS 0x1294
#define USB_PRODUCT_IPHONE_4 0x1297
#define USB_PRODUCT_IPAD 0x129a
+#define USB_PRODUCT_IPAD_MINI0x12ab
#define USB_PRODUCT_IPHONE_4_VZW 0x129c
#define USB_PRODUCT_IPHONE_4S 0x12a0
#define USB_PRODUCT_IPHONE_5 0x12a8
@@ -107,6 +108,10 @@ static struct usb_device_id ipheth_table
IPHETH_USBINTF_CLASS, IPHETH_USBINTF_SUBCLASS,
IPHETH_USBINTF_PROTO) },
{ USB_DEVICE_AND_INTERFACE_INFO(
+ USB_VENDOR_APPLE, USB_PRODUCT_IPAD_MINI,
+ IPHETH_USBINTF_CLASS, IPHETH_USBINTF_SUBCLASS,
+ IPHETH_USBINTF_PROTO) },
+ { USB_DEVICE_AND_INTERFACE_INFO(
USB_VENDOR_APPLE, USB_PRODUCT_IPHONE_4_VZW,
IPHETH_USBINTF_CLASS, IPHETH_USBINTF_SUBCLASS,
IPHETH_USBINTF_PROTO) },
--
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
