Re: [PATCH] usb: phy: add R-Car R8A7779 USB phy driver.

[ABRAHAM, KISHON VIJAY]

Hi,

On Wed, Sep 5, 2012 at 6:20 AM, Kuninori Morimoto
 wrote:
>
> Hi Abraham
>
> Thank you for checking patch
>
>
>> > +config USB_RCAR_PHY
>> > +   tristate "Renesas R-Car USB phy support"
>> > +   depends on (USB || USB_GADGET) && ARCH_R8A7779
>> > +   help
>> > + Say Y here to add support for the Renesas R-Car USB phy driver.
>>
>> Just out of curiosity, which USB contoller will/is using this PHY?
>
> EHCI/OHCI.
> Now, I'm using ohci/ehci-platform.c

Thats a generic ehci driver. I was interested in the specific usb
controller that uses this phy. Anyways now I know its a host only
driver :-)

>
>> > +   reg0 = ioremap_nocache(res0->start, resource_size(res0));
>> > +   reg1 = ioremap_nocache(res1->start, resource_size(res1));
>>
>> Instead use devm_ioremap_nocache?
>
> will do in v2 patch
>
>> > +   /* (1) USB-PHY standby release */
>> > +   iowrite32(0x0001, (reg0 + USBPCTRL1));
>>
>> Can we have some macros to define the above constant (and for the
>> constants used below)?
>
> will do in v2 patch
>
>> > +
>> > +   /* (2) start USB-PHY internal PLL */
>> > +   iowrite32(0x0003, (reg0 + USBPCTRL1));
>>
>> No power management stuff added?? Dont we need to stop this PLL during 
>> suspend??
> (snip)
>> I see you are doing one time initialization of the phy during probe.
>> But I think this phy will be pointless without the usb controller.
>> Instead how about using the library functions like
>> usb_add_phy/usb_get_phy/usb_phy_init?
>
> I'm not sure detail, but this usb_xxx_phy() is under otg.
> Our system don't support it, but is it normal ?

Yeah. We are in the process of cleaning it up. Indeed it's normal in
the sense quite a few host only drivers use it (ehci-fsl.c,
ehci-msm.c, ehci-mv.c, ehci-tegra.c, ohci-omap.c).
>
> Now, I'm using ehci/ohci-platform.
> When I support this usb_xxx_phy(), do I need to customize these drivers ?

You can't change ehci-platform.c because thats a generic driver. But
you have to do those modifications in your controller specific file.
>
> And, this is extra stuff, but I have 1 question.
> it is ${LINUX}/driver/usb/Makefile's order.
>
> obj-(xxx) += host/
> ...
> obj-(xxx) += phy/
>
> This means that usb phy driver probe function is called
> after host driver probe function().

AFAIK, the order in Makefile will just determine the order in which in
the files are compiled and not the order in which the modules are
loaded/probed. The module loading/probing is dependent on which
*initcall* section you've added your module to and also based on
device creation.
However if two modules add themselves to the same *initcall* section,
the order in which the files are compiled determine the order in which
the modules are probed (I think this is what is happening in your
case).

> Now, I'm using a technique which delays usb host driver registration,
> but why phy driver is called after host driver ?

Does changing the order in Makefile helped you?

Thanks
Kishon
--
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] USB: FHCI: use list_move_tail instead of list_del/list_add_tail

[Wei Yongjun]

From: Wei Yongjun 

Using list_move_tail() instead of list_del() + list_add_tail().

spatch with a semantic match is used to found this problem.
(http://coccinelle.lip6.fr/)

Signed-off-by: Wei Yongjun 
---
 drivers/usb/host/fhci-sched.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/fhci-sched.c b/drivers/usb/host/fhci-sched.c
index 2dc8a40..8f18538 100644
--- a/drivers/usb/host/fhci-sched.c
+++ b/drivers/usb/host/fhci-sched.c
@@ -261,8 +261,7 @@ static void move_head_to_tail(struct list_head *list)
struct list_head *node = list->next;
 
if (!list_empty(list)) {
-   list_del(node);
-   list_add_tail(node, list);
+   list_move_tail(node, list);
}
 }
 


--
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 resend2] Input: usbtouchscreen - initialize eGalax devices

[Dmitry Torokhov]

On Mon, Sep 03, 2012 at 01:33:50PM -0400, Forest Bond wrote:
> From: Forest Bond 
> 
> Certain eGalax devices expose an interface with class HID and protocol
> None.  Some work with usbhid and some work with usbtouchscreen, but
> there is no easy way to differentiate.  Sending an eGalax diagnostic
> packet seems to kick them all into using the right protocol for
> usbtouchscreen, so we can continue to bind them all there (as opposed to
> handing some off to usbhid).
> 
> This fixes a regression for devices that were claimed by (and worked
> with) usbhid prior to commit 139ebe8dc80dd74cb2ac9f5603d18fbf5cff049f
> ("Input: usbtouchscreen - fix eGalax HID ignoring"), which made
> usbtouchscreen claim them instead.  With this patch they will still be
> claimed by usbtouchscreen, but they will actually report events
> usbtouchscreen can understand.  Note that these devices will be limited
> to the usbtouchscreen feature set so e.g. dual touch features are not
> supported.
> 
> I have the distinct pleasure of needing to support devices of both types
> and have tested accordingly.
> 
> Signed-off-by: Forest Bond 

Applied, thank you Forest.

> ---
>  drivers/input/touchscreen/usbtouchscreen.c |   39 
> 
>  1 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/usbtouchscreen.c 
> b/drivers/input/touchscreen/usbtouchscreen.c
> index e32709e..c5f4dc0 100644
> --- a/drivers/input/touchscreen/usbtouchscreen.c
> +++ b/drivers/input/touchscreen/usbtouchscreen.c
> @@ -304,6 +304,44 @@ static int e2i_read_data(struct usbtouch_usb *dev, 
> unsigned char *pkt)
>  #define EGALAX_PKT_TYPE_REPT 0x80
>  #define EGALAX_PKT_TYPE_DIAG 0x0A
>  
> +static int egalax_init(struct usbtouch_usb *usbtouch)
> +{
> + int ret, i;
> + unsigned char *buf;
> + struct usb_device *udev = interface_to_usbdev(usbtouch->interface);
> +
> + /* An eGalax diagnostic packet kicks the device into using the right
> +  * protocol.  We send a "check active" packet.  The response will be
> +  * read later and ignored.
> +  */
> +
> + buf = kmalloc(3, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + buf[0] = EGALAX_PKT_TYPE_DIAG;
> + buf[1] = 1; /* length */
> + buf[2] = 'A';   /* command - check active */
> +
> + for (i = 0; i < 3; i++) {
> + ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +   0,
> +   USB_DIR_OUT | USB_TYPE_VENDOR | 
> USB_RECIP_DEVICE,
> +   0, 0, buf, 3,
> +   USB_CTRL_SET_TIMEOUT);
> + if (ret >= 0) {
> + ret = 0;
> + break;
> + }
> + if (ret != -EPIPE)
> + break;
> + }
> +
> + kfree(buf);
> +
> + return ret;
> +}
> +
>  static int egalax_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
>  {
>   if ((pkt[0] & EGALAX_PKT_TYPE_MASK) != EGALAX_PKT_TYPE_REPT)
> @@ -1056,6 +1094,7 @@ static struct usbtouch_device_info usbtouch_dev_info[] 
> = {
>   .process_pkt= usbtouch_process_multi,
>   .get_pkt_len= egalax_get_pkt_len,
>   .read_data  = egalax_read_data,
> + .init   = egalax_init,
>   },
>  #endif
>  
> -- 
> 1.7.0.4

-- 
Dmitry
--
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

[Resend PATCH V2 8/8] usb : Add sysfs files to control port power.

[Lan Tianyu]

This patch adds two sysfs files for each usb hub port to allow userspace
to control the port power policy.

For an upcoming Intel xHCI roothub, this will translate into ACPI calls
to completely power off or power on the port.  As a reminder, when these
ports are completely powered off, the USB host and device will see a
physical disconnect.  All future USB device connections will be lost,
and the device will not be able to signal a remote wakeup.

The control sysfs file can be written to with two options:
"on" - port power must be on.
"off" - port must be off.

The state sysfs file reports usb port's power state:
"on" - powered on
"off" - powered off
"error" - can't get power state

For now, let userspace dictate the port power off policy.  Future
patches may add an in-kernel policy.

Acked-by: Alan Stern 
Signed-off-by: Lan Tianyu 
Signed-off-by: Sarah Sharp 
---
 Documentation/ABI/testing/sysfs-bus-usb |   24 ++
 drivers/usb/core/hub.c  |  133 ++-
 2 files changed, 156 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb 
b/Documentation/ABI/testing/sysfs-bus-usb
index b6fbe51..13f3eee 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -227,3 +227,27 @@ Contact:   Lan Tianyu 
 Description:
The /sys/bus/usb/devices/.../(hub interface)/portX
is usb port device's sysfs directory.
+
+What:  /sys/bus/usb/devices/.../(hub interface)/portX/control
+Date:  August 2012
+Contact:   Lan Tianyu 
+Description:
+   The /sys/bus/usb/devices/.../(hub interface)/portX/control
+   attribute allows user space to control the power policy on
+   the usb port.
+
+   All ports have one of the following two values for control
+   "on" - port power must be on.
+   "off" - port power must be off.
+
+What:  /sys/bus/usb/devices/.../(hub interface)/portX/state
+Date:  August 2012
+Contact:   Lan Tianyu 
+Description:
+   The /sys/bus/usb/devices/.../(hub interface)/portX/state
+   attribute allows user space to check hub port's power state.
+
+   All ports have three following states
+   "on"  -port power on
+   "off" -port power off
+   "error"   -can't get the hub port's power state
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 4ffe837..3def91e 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -39,11 +39,17 @@
 #endif
 #endif
 
+enum port_power_policy {
+   USB_PORT_POWER_ON = 0,
+   USB_PORT_POWER_OFF,
+};
+
 struct usb_port {
struct usb_device *child;
struct device dev;
struct dev_state *port_owner;
enum usb_port_connect_type connect_type;
+   enum port_power_policy port_power_policy;
 };
 
 struct usb_hub {
@@ -93,6 +99,10 @@ struct usb_hub {
struct usb_port **ports;
 };
 
+static const char on_string[] = "on";
+static const char off_string[] = "off";
+static const struct attribute_group *port_dev_group[];
+
 static inline int hub_is_superspeed(struct usb_device *hdev)
 {
return (hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS);
@@ -845,7 +855,12 @@ static unsigned hub_power_on(struct usb_hub *hub, bool 
do_delay)
dev_dbg(hub->intfdev, "trying to enable port power on "
"non-switchable hub\n");
for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++)
-   set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
+   if (hub->ports[port1 - 1]->port_power_policy
+   == USB_PORT_POWER_ON)
+   set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
+   else
+   clear_port_feature(hub->hdev, port1,
+   USB_PORT_FEAT_POWER);
 
/* Wait at least 100 msec for power to become stable */
delay = max(pgood_delay, (unsigned) 100);
@@ -1263,6 +1278,7 @@ static int usb_hub_create_port_device(struct usb_hub *hub,
 
hub->ports[port1 - 1] = port_dev;
port_dev->dev.parent = hub->intfdev;
+   port_dev->dev.groups = port_dev_group;
port_dev->dev.type = &usb_port_device_type;
dev_set_name(&port_dev->dev, "port%d", port1);
 
@@ -2627,6 +2643,25 @@ static int port_is_power_on(struct usb_hub *hub, 
unsigned portstatus)
return ret;
 }
 
+static int usb_get_hub_port_power_state(struct usb_device *hdev, int port1)
+{
+   struct usb_hub *hub = hdev_to_hub(hdev);
+   struct usb_port_status data;
+   u16 portstatus;
+   int ret;
+
+   ret = get_port_status(hub->hdev, port1, &data);
+   if (ret < 4) {
+   dev_err(hub->intfdev,
+   "%s failed (err = %d)\n", __func__, ret);
+

[Resend PATCH V2 3/8] usb/acpi: Bind ACPI node to USB port, not usb_device.

[Lan Tianyu]

In the ACPI DSDT table, only usb root hub and usb ports are ACPI device
nodes.  Originally, we bound the usb port's ACPI node to the usb device
attached to the port.  However, we want to access those ACPI port
methods when the port is empty, and there's no usb_device associated
with that port.

Now that the usb port is a real device, we can bind the port's ACPI node
to struct usb_port instead.

Signed-off-by: Lan Tianyu 
Signed-off-by: Sarah Sharp 
---
-v2: remove #ifdef CONFIG_ACPI around "#include " in the drivers 
/usb/core/usb.h

 drivers/usb/core/hub.c  |   18 
 drivers/usb/core/usb-acpi.c |   67 +--
 drivers/usb/core/usb.h  |9 ++
 3 files changed, 79 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cdbade1..3341b55 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5077,3 +5077,21 @@ struct usb_device *usb_hub_find_child(struct usb_device 
*hdev,
return hub->ports[port1 - 1]->child;
 }
 EXPORT_SYMBOL_GPL(usb_hub_find_child);
+
+#ifdef CONFIG_ACPI
+/**
+ * usb_get_hub_port_acpi_handle - Get the usb port's acpi handle
+ * @hdev: USB device belonging to the usb hub
+ * @port1: port num of the port
+ *
+ * Return port's acpi handle if successful, NULL if params are
+ * invaild.
+ */
+acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
+   int port1)
+{
+   struct usb_hub *hub = hdev_to_hub(hdev);
+
+   return DEVICE_ACPI_HANDLE(&hub->ports[port1 - 1]->dev);
+}
+#endif
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 8947b20..47197bf 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -70,22 +70,59 @@ static int usb_acpi_check_pld(struct usb_device *udev, 
acpi_handle handle)
 static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
 {
struct usb_device *udev;
-   struct device *parent;
acpi_handle *parent_handle;
+   int port_num;
 
-   if (!is_usb_device(dev))
-   return -ENODEV;
-
-   udev = to_usb_device(dev);
-   parent = dev->parent;
-   parent_handle = DEVICE_ACPI_HANDLE(parent);
-
-   if (!parent_handle)
-   return -ENODEV;
-
-   *handle = acpi_get_child(parent_handle, udev->portnum);
-
-   if (!*handle)
+   /*
+* In the ACPI DSDT table, only usb root hub and usb ports are
+* acpi device nodes. The hierarchy like following.
+* Device (EHC1)
+*  Device (HUBN)
+*  Device (PR01)
+*  Device (PR11)
+*  Device (PR12)
+*  Device (PR13)
+*  ...
+* So all binding process is divided into two parts. binding
+* root hub and usb ports.
+*/
+   if (is_usb_device(dev)) {
+   udev = to_usb_device(dev);
+   if (udev->parent)
+   return -ENODEV;
+   /* root hub's parent is the usb hcd. */
+   parent_handle = DEVICE_ACPI_HANDLE(dev->parent);
+   *handle = acpi_get_child(parent_handle, udev->portnum);
+   if (!*handle)
+   return -ENODEV;
+   return 0;
+   } else if (is_usb_port(dev)) {
+   sscanf(dev_name(dev), "port%d", &port_num);
+   /* Get the struct usb_device point of port's hub */
+   udev = to_usb_device(dev->parent->parent);
+
+   /*
+* The root hub ports' parent is the root hub. The non-root-hub
+* ports' parent is the parent hub port which the hub is
+* connected to.
+*/
+   if (!udev->parent) {
+   *handle = acpi_get_child(DEVICE_ACPI_HANDLE(&udev->dev),
+   port_num);
+   if (!*handle)
+   return -ENODEV;
+   } else {
+   parent_handle =
+   usb_get_hub_port_acpi_handle(udev->parent,
+   udev->portnum);
+   if (!parent_handle)
+   return -ENODEV;
+
+   *handle = acpi_get_child(parent_handle, port_num);
+   if (!*handle)
+   return -ENODEV;
+   }
+   } else
return -ENODEV;
 
/*
@@ -102,7 +139,7 @@ static int usb_acpi_find_device(struct device *dev, 
acpi_handle *handle)
 
 static struct acpi_bus_type usb_acpi_bus = {
.bus = &usb_bus_type,
-   .find_bridge = NULL,
+   .find_bridge = usb_acpi_find_device,
.find_device = usb_acpi_find_device,
 };
 
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index acb103c..1633f6e 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@

[Resend PATCH V2 7/8] usb: Fail a get config when the port is powered off.

[Lan Tianyu]

Alan Stern pointed out that a USB port could potentially get powered off
when the attached USB device is in the middle of enumerating, due to
race conditions:
http://marc.info/?l=linux-usb&m=134130616707548&w=2

If that happens, we need to ensure the enumeration fails.  If a call to
usb_get_descriptor() fails for a reason other than a Stall, return an
error.  That should handle the case where the port is powered off.

Acked-by: Alan Stern 
Signed-off-by: Lan Tianyu 
Signed-off-by: Sarah Sharp 
---
 drivers/usb/core/config.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index f4bdd0c..7199adc 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -702,6 +702,8 @@ int usb_get_configuration(struct usb_device *dev)
if (result < 0) {
dev_err(ddev, "unable to read config index %d "
"descriptor/%s: %d\n", cfgno, "start", result);
+   if (result != -EPIPE)
+   goto err;
dev_err(ddev, "chopping to %d config(s)\n", cfgno);
dev->descriptor.bNumConfigurations = cfgno;
break;
-- 
1.7.9.5

--
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

[Resend PATCH V2 6/8] usb/acpi: Use ACPI methods to power off ports.

[Lan Tianyu]

Upcoming Intel systems will have an ACPI method to control whether a USB
port can be completely powered off.  The implication of powering off a
USB port is that the device and host sees a physical disconnect, and
subsequent port connections and remote wakeups will be lost.

Add a new function, usb_acpi_power_manageable(), that can be used to
find whether the usb port has ACPI power resources that can be used to
power on and off the port on these machines. Also add a new function
called usb_acpi_set_power_state() that controls the port power via these
ACPI methods.

When the USB core calls into the xHCI hub driver to power off a port,
check whether the port can be completely powered off via this new ACPI
mechanism.  If so, call into these new ACPI methods.  Also use the ACPI
methods when the USB core asks to power on a port.

Signed-off-by: Lan Tianyu 
Signed-off-by: Sarah Sharp 
---
 drivers/usb/core/usb-acpi.c |   62 +++
 drivers/usb/host/xhci-hub.c |   12 +
 include/linux/usb.h |   10 +++
 3 files changed, 84 insertions(+)

diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 404d86a..0ef7d42 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -19,6 +19,68 @@
 
 #include "usb.h"
 
+/**
+ * usb_acpi_power_manageable - check whether usb port has
+ * acpi power resource.
+ * @hdev: USB device belonging to the usb hub
+ * @index: port index based zero
+ *
+ * Return true if the port has acpi power resource and false if no.
+ */
+bool usb_acpi_power_manageable(struct usb_device *hdev, int index)
+{
+   acpi_handle port_handle;
+   int port1 = index + 1;
+
+   port_handle = usb_get_hub_port_acpi_handle(hdev,
+   port1);
+   if (port_handle)
+   return acpi_bus_power_manageable(port_handle);
+   else
+   return false;
+}
+EXPORT_SYMBOL_GPL(usb_acpi_power_manageable);
+
+/**
+ * usb_acpi_set_power_state - control usb port's power via acpi power
+ * resource
+ * @hdev: USB device belonging to the usb hub
+ * @index: port index based zero
+ * @enable: power state expected to be set
+ *
+ * Notice to use usb_acpi_power_manageable() to check whether the usb port
+ * has acpi power resource before invoking this function.
+ *
+ * Returns 0 on success, else negative errno.
+ */
+int usb_acpi_set_power_state(struct usb_device *hdev, int index, bool enable)
+{
+   acpi_handle port_handle;
+   unsigned char state;
+   int port1 = index + 1;
+   int error = -EINVAL;
+
+   port_handle = (acpi_handle)usb_get_hub_port_acpi_handle(hdev,
+   port1);
+   if (!port_handle)
+   return error;
+
+   if (enable)
+   state = ACPI_STATE_D0;
+   else
+   state = ACPI_STATE_D3_COLD;
+
+   error = acpi_bus_set_power(port_handle, state);
+   if (!error)
+   dev_dbg(&hdev->dev, "The power of hub port %d was set to %d\n",
+   port1, enable);
+   else
+   dev_dbg(&hdev->dev, "The power of hub port failed to be set\n");
+
+   return error;
+}
+EXPORT_SYMBOL_GPL(usb_acpi_set_power_state);
+
 static int usb_acpi_check_port_connect_type(struct usb_device *hdev,
acpi_handle handle, int port1)
 {
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 03032b3..630e9e6 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -766,6 +766,12 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 
wValue,
 
temp = xhci_readl(xhci, port_array[wIndex]);
xhci_dbg(xhci, "set port power, actual port %d status  
= 0x%x\n", wIndex, temp);
+
+   temp = usb_acpi_power_manageable(hcd->self.root_hub,
+   wIndex);
+   if (temp)
+   usb_acpi_set_power_state(hcd->self.root_hub,
+   wIndex, true);
break;
case USB_PORT_FEAT_RESET:
temp = (temp | PORT_RESET);
@@ -868,6 +874,12 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 
wValue,
case USB_PORT_FEAT_POWER:
xhci_writel(xhci, temp & ~PORT_POWER,
port_array[wIndex]);
+
+   temp = usb_acpi_power_manageable(hcd->self.root_hub,
+   wIndex);
+   if (temp)
+   usb_acpi_set_power_state(hcd->self.root_hub,
+   wIndex, false);
break;
default:
goto error;
diff --git a/include/linux/usb.h b/include/linux/usb.h
index e0084a1..07915a3 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -602,6 +602,16 @@ extern int usb_

[Resend PATCH V2 5/8] xhci: Handle clear PORT_POWER feature.

[Lan Tianyu]

This patch makes the xHCI roothub code handle the clear PORT_POWER
feature request.  Setting port power is already handled.

Signed-off-by: Lan Tianyu 
Signed-off-by: Sarah Sharp 
---
 drivers/usb/host/xhci-hub.c |4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 74bfc86..03032b3 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -865,6 +865,10 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 
wValue,
xhci_disable_port(hcd, xhci, wIndex,
port_array[wIndex], temp);
break;
+   case USB_PORT_FEAT_POWER:
+   xhci_writel(xhci, temp & ~PORT_POWER,
+   port_array[wIndex]);
+   break;
default:
goto error;
}
-- 
1.7.9.5

--
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

[Resend PATCH V2 4/8] usb/acpi: Store info on device removability.

[Lan Tianyu]

In the upcoming USB port power off patches, we need to know whether a
USB port can ever see a disconnect event.  Often USB ports are internal
to a system, and users can't disconnect USB devices from that port.
Sometimes those ports will remain empty, because the OEM chose not to
connect an internal USB device to that port.

According to ACPI Spec 9.13, PLD indicates whether USB port is
user visible and _UPC indicates whether a USB device can be connected to
the USB port (we'll call this "connectible").  Here's a matrix of the
possible combinations:

Visible Connectible
NameExample
-

Yes No  Unknown (Invalid state.)

Yes Yes Hot-plugUSB ports on the outside of a laptop.
A user could freely connect and disconnect
USB devices.

No  Yes Hard-wired  A USB modem hard-wired to a port on the
inside of a laptop.

No  No  Not usedThe port is internal to the system and
will remain empty.

Represent each of these four states with an enum usb_port_connect_type.
The four states are USB_PORT_CONNECT_TYPE_UNKNOWN,
USB_PORT_CONNECT_TYPE_HOT_PLUG, USB_PORT_CONNECT_TYPE_HARD_WIRED, and
USB_PORT_NOT_USED.  When we get the USB port's acpi_handle, store the
state in connect_type in struct usb_port.

Signed-off-by: Lan Tianyu 
Signed-off-by: Sarah Sharp 
---
 drivers/usb/core/hub.c  |   31 +
 drivers/usb/core/usb-acpi.c |   80 ---
 drivers/usb/core/usb.h  |4 +++
 include/linux/usb.h |7 
 4 files changed, 87 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3341b55..4ffe837 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -43,6 +43,7 @@ struct usb_port {
struct usb_device *child;
struct device dev;
struct dev_state *port_owner;
+   enum usb_port_connect_type connect_type;
 };
 
 struct usb_hub {
@@ -5078,6 +5079,36 @@ struct usb_device *usb_hub_find_child(struct usb_device 
*hdev,
 }
 EXPORT_SYMBOL_GPL(usb_hub_find_child);
 
+/**
+ * usb_set_hub_port_connect_type - set hub port connect type.
+ * @hdev: USB device belonging to the usb hub
+ * @port1: port num of the port
+ * @type: connect type of the port
+ */
+void usb_set_hub_port_connect_type(struct usb_device *hdev, int port1,
+   enum usb_port_connect_type type)
+{
+   struct usb_hub *hub = hdev_to_hub(hdev);
+
+   hub->ports[port1 - 1]->connect_type = type;
+}
+
+/**
+ * usb_get_hub_port_connect_type - Get the port's connect type
+ * @hdev: USB device belonging to the usb hub
+ * @port1: port num of the port
+ *
+ * Return connect type of the port and if input params are
+ * invalid, return USB_PORT_CONNECT_TYPE_UNKNOWN.
+ */
+enum usb_port_connect_type
+usb_get_hub_port_connect_type(struct usb_device *hdev, int port1)
+{
+   struct usb_hub *hub = hdev_to_hub(hdev);
+
+   return hub->ports[port1 - 1]->connect_type;
+}
+
 #ifdef CONFIG_ACPI
 /**
  * usb_get_hub_port_acpi_handle - Get the usb port's acpi handle
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 47197bf..404d86a 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -19,20 +19,29 @@
 
 #include "usb.h"
 
-static int usb_acpi_check_upc(struct usb_device *udev, acpi_handle handle)
+static int usb_acpi_check_port_connect_type(struct usb_device *hdev,
+   acpi_handle handle, int port1)
 {
acpi_status status;
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
union acpi_object *upc;
+   struct acpi_pld pld;
int ret = 0;
 
-   status = acpi_evaluate_object(handle, "_UPC", NULL, &buffer);
-
+   /*
+* Accoding to ACPI Spec 9.13. PLD indicates whether usb port is
+* user visible and _UPC indicates whether it is connectable. If
+* the port was visible and connectable, it could be freely connected
+* and disconnected with USB devices. If no visible and connectable,
+* a usb device is directly hard-wired to the port. If no visible and
+* no connectable, the port would be not used.
+*/
+   status = acpi_get_physical_device_location(handle, &pld);
if (ACPI_FAILURE(status))
return -ENODEV;
 
+   status = acpi_evaluate_object(handle, "_UPC", NULL, &buffer);
upc = buffer.pointer;
-
if (!upc || (upc->type != ACPI_TYPE_PACKAGE)
|| upc->package.count != 4) {
ret = -EINVAL;
@@ -40,33 +49,20 @@ static int usb_acpi_check_upc(struct usb_device *udev, 
acpi_handle handle)
}
 
if (upc->package.elements[0].integer.value)
-   udev->removable = USB_DEVICE_REMOVABLE;
-   else
-  

[Resend PATCH V2 2/8] usb: move children to struct usb_port

[Lan Tianyu]

The usb_device structure contains an array of usb_device "children".
This array is only valid if the usb_device is a hub, so it makes no
sense to store it there.  Instead, store the usb_device child
in its parent usb_port structure.

Since usb_port is an internal USB core structure, add a new function to
get the USB device child, usb_hub_find_child().  Add a new macro,
usb_hub_get_each_child(), to iterate over all the children attached to a
particular USB hub.

Remove the printing the USB children array pointer from the usb-ip
driver, since it's really not necessary.

Acked-by: Alan Stern 
Signed-off-by: Lan Tianyu 
Signed-off-by: Sarah Sharp 
---
 drivers/staging/usbip/usbip_common.c |3 +-
 drivers/usb/core/devices.c   |7 ++--
 drivers/usb/core/hub.c   |   73 +++---
 drivers/usb/host/r8a66597-hcd.c  |5 +--
 include/linux/usb.h  |   15 ++-
 5 files changed, 68 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/usbip/usbip_common.c 
b/drivers/staging/usbip/usbip_common.c
index 70f23026..95beb76 100644
--- a/drivers/staging/usbip/usbip_common.c
+++ b/drivers/staging/usbip/usbip_common.c
@@ -157,8 +157,7 @@ static void usbip_dump_usb_device(struct usb_device *udev)
dev_dbg(dev, "have_langid %d, string_langid %d\n",
udev->have_langid, udev->string_langid);
 
-   dev_dbg(dev, "maxchild %d, children %p\n",
-   udev->maxchild, udev->children);
+   dev_dbg(dev, "maxchild %d\n", udev->maxchild);
 }
 
 static void usbip_dump_request_type(__u8 rt)
diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
index d956965..f4ead12 100644
--- a/drivers/usb/core/devices.c
+++ b/drivers/usb/core/devices.c
@@ -496,6 +496,7 @@ static ssize_t usb_device_dump(char __user **buffer, size_t 
*nbytes,
char *pages_start, *data_end, *speed;
unsigned int length;
ssize_t total_written = 0;
+   struct usb_device *childdev = NULL;
 
/* don't bother with anything else if we're not writing any data */
if (*nbytes <= 0)
@@ -589,14 +590,12 @@ static ssize_t usb_device_dump(char __user **buffer, 
size_t *nbytes,
free_pages((unsigned long)pages_start, 1);
 
/* Now look at all of this device's children. */
-   for (chix = 0; chix < usbdev->maxchild; chix++) {
-   struct usb_device *childdev = usbdev->children[chix];
-
+   usb_hub_for_each_child(usbdev, chix, childdev) {
if (childdev) {
usb_lock_device(childdev);
ret = usb_device_dump(buffer, nbytes, skip_bytes,
  file_offset, childdev, bus,
- level + 1, chix, ++cnt);
+ level + 1, chix - 1, ++cnt);
usb_unlock_device(childdev);
if (ret == -EFAULT)
return total_written;
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 87df22e..cdbade1 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -40,6 +40,7 @@
 #endif
 
 struct usb_port {
+   struct usb_device *child;
struct device dev;
struct dev_state *port_owner;
 };
@@ -181,7 +182,7 @@ static inline char *portspeed(struct usb_hub *hub, int 
portstatus)
 /* Note that hdev or one of its children must be locked! */
 static struct usb_hub *hdev_to_hub(struct usb_device *hdev)
 {
-   if (!hdev || !hdev->actconfig)
+   if (!hdev || !hdev->actconfig || !hdev->maxchild)
return NULL;
return usb_get_intfdata(hdev->actconfig->interface[0]);
 }
@@ -876,8 +877,8 @@ static int hub_port_disable(struct usb_hub *hub, int port1, 
int set_state)
struct usb_device *hdev = hub->hdev;
int ret = 0;
 
-   if (hdev->children[port1-1] && set_state)
-   usb_set_device_state(hdev->children[port1-1],
+   if (hub->ports[port1 - 1]->child && set_state)
+   usb_set_device_state(hub->ports[port1 - 1]->child,
USB_STATE_NOTATTACHED);
if (!hub->error && !hub_is_superspeed(hub->hdev))
ret = clear_port_feature(hdev, port1, USB_PORT_FEAT_ENABLE);
@@ -1033,7 +1034,7 @@ static void hub_activate(struct usb_hub *hub, enum 
hub_activation_type type)
 * which ports need attention.
 */
for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
-   struct usb_device *udev = hdev->children[port1-1];
+   struct usb_device *udev = hub->ports[port1 - 1]->child;
u16 portstatus, portchange;
 
portstatus = portchange = 0;
@@ -1198,8 +1199,8 @@ static void hub_quiesce(struct usb_hub *hub, enum 
hub_quiescing_type type)
if (type != HUB_SUSPEND) {
/* Disconnect all the children */
for (i = 0; i < hdev->maxchild; ++i)

[Resend PATCH V2 1/8] usb: make usb port a real device

[Lan Tianyu]

This patch turns each USB port on a hub into a new struct device.  This
new device has the USB hub interface device as its parent.  The port
devices are stored in a new structure (usb_port), and an array of
usb_ports are dynamically allocated once we know how many ports the USB
hub has.

Move the port_owner variable out of usb_hub and into this new structure.

A new file will be created in the hub interface sysfs directory, so
add documentation.

Acked-by: Alan Stern 
Signed-off-by: Lan Tianyu 
Signed-off-by: Sarah Sharp 
---
-V2: Set release callback in the struct device_type instead of in the device 
itself directly.

This patchset is based on usb-next tree and last commit is
commit a7bdf7fa33127bf08eb0810698bca607a9462df4
Merge: 7fda953 fea7a08
Author: Greg Kroah-Hartman 
Date:   Mon Aug 27 07:15:30 2012 -0700

Merge v3.6-rc3 into usb-next

This picks up fixes that we need in this branch for testing.

Signed-off-by: Greg Kroah-Hartman 
---
 Documentation/ABI/testing/sysfs-bus-usb |7 +++
 drivers/usb/core/hub.c  |   91 +--
 2 files changed, 82 insertions(+), 16 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb 
b/Documentation/ABI/testing/sysfs-bus-usb
index 5f75f8f..b6fbe51 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -220,3 +220,10 @@ Description:
If the device doesn't support LTM, the file will read "no".
The file will be present for all speeds of USB devices, and will
always read "no" for USB 1.1 and USB 2.0 devices.
+
+What:  /sys/bus/usb/devices/.../(hub interface)/portX
+Date:  August 2012
+Contact:   Lan Tianyu 
+Description:
+   The /sys/bus/usb/devices/.../(hub interface)/portX
+   is usb port device's sysfs directory.
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 128a804..87df22e 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -39,6 +39,11 @@
 #endif
 #endif
 
+struct usb_port {
+   struct device dev;
+   struct dev_state *port_owner;
+};
+
 struct usb_hub {
struct device   *intfdev;   /* the "interface" device */
struct usb_device   *hdev;
@@ -83,7 +88,7 @@ struct usb_hub {
u8  indicator[USB_MAXCHILDREN];
struct delayed_work leds;
struct delayed_work init_work;
-   struct dev_state**port_owners;
+   struct usb_port **ports;
 };
 
 static inline int hub_is_superspeed(struct usb_device *hdev)
@@ -156,6 +161,8 @@ EXPORT_SYMBOL_GPL(ehci_cf_port_reset_rwsem);
 #define HUB_DEBOUNCE_STEP25
 #define HUB_DEBOUNCE_STABLE 100
 
+#define to_usb_port(_dev) \
+   container_of(_dev, struct usb_port, dev)
 
 static int usb_reset_and_verify_device(struct usb_device *udev);
 
@@ -1222,6 +1229,52 @@ static int hub_post_reset(struct usb_interface *intf)
return 0;
 }
 
+static void usb_port_device_release(struct device *dev)
+{
+   struct usb_port *port_dev = to_usb_port(dev);
+
+   kfree(port_dev);
+}
+
+static void usb_hub_remove_port_device(struct usb_hub *hub,
+  int port1)
+{
+   device_unregister(&hub->ports[port1 - 1]->dev);
+}
+
+struct device_type usb_port_device_type = {
+   .name = "usb_port",
+   .release =  usb_port_device_release,
+};
+
+static int usb_hub_create_port_device(struct usb_hub *hub,
+ int port1)
+{
+   struct usb_port *port_dev = NULL;
+   int retval;
+
+   port_dev = kzalloc(sizeof(*port_dev), GFP_KERNEL);
+   if (!port_dev) {
+   retval = -ENOMEM;
+   goto exit;
+   }
+
+   hub->ports[port1 - 1] = port_dev;
+   port_dev->dev.parent = hub->intfdev;
+   port_dev->dev.type = &usb_port_device_type;
+   dev_set_name(&port_dev->dev, "port%d", port1);
+
+   retval = device_register(&port_dev->dev);
+   if (retval)
+   goto error_register;
+   return 0;
+
+error_register:
+   put_device(&port_dev->dev);
+exit:
+   return retval;
+}
+
 static int hub_configure(struct usb_hub *hub,
struct usb_endpoint_descriptor *endpoint)
 {
@@ -1231,7 +1284,7 @@ static int hub_configure(struct usb_hub *hub,
u16 hubstatus, hubchange;
u16 wHubCharacteristics;
unsigned int pipe;
-   int maxp, ret;
+   int maxp, ret, i;
char *message = "out of memory";
 
hub->buffer = kmalloc(sizeof(*hub->buffer), GFP_KERNEL);
@@ -1273,9 +1326,9 @@ static int hub_configure(struct usb_hub *hub,
 
hdev->children = kzalloc(hdev->maxchild *
sizeof(struct usb_device *), GFP_KERNEL);
-   hub->port_owners = kzalloc(hdev->maxchild * sizeof(struct dev_state *),
-   GFP_KERNEL);
-   if (!hdev->children || !hub->port_ow

Re: [PATCH] usb: host: xhci-plat: use ioremap_nocache

[Felipe Balbi]

Hi,

On Tue, Sep 04, 2012 at 02:51:31PM -0700, Sarah Sharp wrote:
> Hi Felipe,
> 
> Should this be queued for stable as well?

it probably makes sense to Cc stable, indeed. Thanks a lot.

> Sarah Sharp
> 
> On Fri, Aug 10, 2012 at 09:58:30AM +0300, Felipe Balbi wrote:
> > From: Ruchika Kharwar 
> > 
> > Use the ioremap_nocache variant of the ioremap API in
> > order to make sure our memory will be marked uncachable.
> > 
> > Signed-off-by: Ruchika Kharwar 
> > Signed-off-by: Felipe Balbi 
> > ---
> >  drivers/usb/host/xhci-plat.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> > index 689bc18..df90fe5 100644
> > --- a/drivers/usb/host/xhci-plat.c
> > +++ b/drivers/usb/host/xhci-plat.c
> > @@ -118,7 +118,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
> > goto put_hcd;
> > }
> >  
> > -   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
> > +   hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
> > if (!hcd->regs) {
> > dev_dbg(&pdev->dev, "error mapping memory\n");
> > ret = -EFAULT;
> > -- 
> > 1.7.4.1
> > 

-- 
balbi


signature.asc
Description: Digital signature

[PATCH] usb: host: xhci: fix compilation error for non-PCI based stacks

[Felipe Balbi]

From: Moiz Sonasath 

For non PCI-based stacks, this function call
usb_disable_xhci_ports(to_pci_dev(hcd->self.controller));
made from xhci_shutdown is not applicable.

Ideally, we wouldn't have any PCI-specific code on
a generic driver such as the xHCI stack, but it looks
like we should just stub usb_disable_xhci_ports() out
for non-PCI devices.

[ ba...@ti.com: slight improvement to commit log ]

Signed-off-by: Moiz Sonasath
Signed-off-by: Ruchika Kharwar 
Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/pci-quirks.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h
index ef004a5..7f69a39 100644
--- a/drivers/usb/host/pci-quirks.h
+++ b/drivers/usb/host/pci-quirks.h
@@ -15,6 +15,7 @@ void usb_disable_xhci_ports(struct pci_dev *xhci_pdev);
 static inline void usb_amd_quirk_pll_disable(void) {}
 static inline void usb_amd_quirk_pll_enable(void) {}
 static inline void usb_amd_dev_put(void) {}
+static inline void usb_disable_xhci_ports(struct pci_dev *xhci_pdev) {}
 #endif  /* CONFIG_PCI */
 
 #endif  /*  __LINUX_USB_PCI_QUIRKS_H  */
-- 
1.7.12.rc3

--
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: otg: Move phy interface to separate file.

[Venu Byravarasu]

Hi Greg,

Initially I pushed patch to linux-next with https://lkml.org/lkml/2012/8/29/40 .
In this patch, my mail id for "Signed-off-by" is correctly shown.

However as Felipe wanted me to push the patch to his branch, I synced to his 
code base and pushed the patch.
Am still not sure, why my mail id against signed-off-by got corrupted that way 
in this patch.

Please let me know if the patch is to be resent.

Thanks,
Venu


> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, September 04, 2012 10:51 PM
> To: Venu Byravarasu
> Cc: ba...@ti.com; linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: Re: [PATCH] usb: otg: Move phy interface to separate file.
> 
> On Tue, Sep 04, 2012 at 02:25:58PM +0530, Venu Byravarasu wrote:
> > As otg.h is containing lots of phy interface related
> > stuff, moving all phy interface related stuff to new
> > file named phy.h
> >
> > Signed-off-by: Venu Byravarasu 
> 
> For some reason, I don't think that is a valid email address :(

--
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: otg: Move phy interface to separate file.

[Venu Byravarasu]

Not sure, why it appeared like that.
However my actual mail id is: vbyravar...@nvidia.com

Shall I resend the patch, or this mail id can be added in the patch?

Thanks,
Venu


> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, September 04, 2012 10:51 PM
> To: Venu Byravarasu
> Cc: ba...@ti.com; linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: Re: [PATCH] usb: otg: Move phy interface to separate file.
> 
> On Tue, Sep 04, 2012 at 02:25:58PM +0530, Venu Byravarasu wrote:
> > As otg.h is containing lots of phy interface related
> > stuff, moving all phy interface related stuff to new
> > file named phy.h
> >
> > Signed-off-by: Venu Byravarasu 
> 
> For some reason, I don't think that is a valid email address :(

--
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: gadget: fsl_udc_core: remove mapped flag

[Chen Peter-B29397]

 
> @@ -195,14 +195,13 @@ static void done(struct fsl_ep *ep, struct fsl_req
> *req, int status)
>   dma_pool_free(udc->td_pool, curr_td, curr_td->td_dma);
>   }
> 
> - if (req->mapped) {
> + if (req->req.dma != DMA_ADDR_INVALID) {
>   dma_unmap_single(ep->udc->gadget.dev.parent,
>   req->req.dma, req->req.length,
>   ep_is_in(ep)
>   ? DMA_TO_DEVICE
>   : DMA_FROM_DEVICE);
>   req->req.dma = DMA_ADDR_INVALID;
> - req->mapped = 0;
>   } else
>   dma_sync_single_for_cpu(ep->udc->gadget.dev.parent,
If the class driver has already mapped this address, the req->req.dma is not
DMA_ADDR_INVALID either, in this case, the dma_sync_single_for_cpu is enough.


Peter


--
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: gadget: fsl_udc_core: do not immediatly prime STATUS for IN xfer

[Chen Peter-B29397]

 
> 
> Because the fsl_udc_core driver shares one 'status_req' object for the
> complete ep0 control transfer, it is not possible to prime the final
> STATUS phase immediately after the IN transaction.  E.g. ch9getstatus()
> executed:
> 
> | req = udc->status_req;
> | ...
> | list_add_tail(&req->queue, &ep->queue);
> | if (ep0_prime_status(udc, EP_DIR_OUT))
> |   
> |   struct fsl_req *req = udc->status_req;
> |   list_add_tail(&req->queue, &ep->queue);
> 
> which corrupts the ep->queue list by inserting 'status_req' twice.  This
> causes a kernel oops e.g. when 'lsusb -v' is executed on the host.
> 
> Patch delays the final 'ep0_prime_status(udc, EP_DIR_OUT))' by moving it
> into the ep0 completion handler.
> 
Enrico, thanks for pointing this problem.

As "prime STATUS phase immediately after the IN transaction" is followed
USB 2.0 spec, to fix this problem, it is better to add data_req for ep0.
In fact, it is already at FSL i.mx internal code, just still not mainlined.

Peter
 


--
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] USB: add support to novatec wireless mouse

[Marco Biscaro]

This patch adds support to Novatec wireless mouse (device ID 0603:1602).
Fixes bug https://bugzilla.kernel.org/show_bug.cgi?id=47031

Reported-by: draekko1 
Signed-off-by: Marco Biscaro 
---
 drivers/hid/hid-ids.h   |3 +++
 drivers/hid/usbhid/hid-quirks.c |2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 244009a..d68400f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -760,4 +760,7 @@
 #define USB_VENDOR_ID_PRIMAX0x0461
 #define USB_DEVICE_ID_PRIMAX_KEYBOARD0x4e05

+#define USB_VENDOR_ID_NOVATEK0x0603
+#define USB_DEVICE_ID_NOVATEK_MOUSE0x1602
+
 #endif
diff --git a/drivers/hid/usbhid/hid-quirks.c 
b/drivers/hid/usbhid/hid-quirks.c

index d347ecd..01674be 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -89,6 +89,8 @@ static const struct hid_blacklist {

 { USB_VENDOR_ID_PI_ENGINEERING, 
USB_DEVICE_ID_PI_ENGINEERING_VEC_USB_FOOTPEDAL, HID_QUIRK_HIDINPUT_FORCE },


+{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, 
HID_QUIRK_NO_INIT_REPORTS },

+
 { USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_MULTI_TOUCH, 
HID_QUIRK_MULTI_INPUT },
 { USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_WIRELESS, 
HID_QUIRK_MULTI_INPUT },
 { USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD, 
HID_QUIRK_NO_INIT_REPORTS },


--
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: phy: add R-Car R8A7779 USB phy driver.

[Kuninori Morimoto]

Hi Abraham

Thank you for checking patch


> > +config USB_RCAR_PHY
> > +   tristate "Renesas R-Car USB phy support"
> > +   depends on (USB || USB_GADGET) && ARCH_R8A7779
> > +   help
> > + Say Y here to add support for the Renesas R-Car USB phy driver.
> 
> Just out of curiosity, which USB contoller will/is using this PHY?

EHCI/OHCI.
Now, I'm using ohci/ehci-platform.c

> > +   reg0 = ioremap_nocache(res0->start, resource_size(res0));
> > +   reg1 = ioremap_nocache(res1->start, resource_size(res1));
> 
> Instead use devm_ioremap_nocache?

will do in v2 patch

> > +   /* (1) USB-PHY standby release */
> > +   iowrite32(0x0001, (reg0 + USBPCTRL1));
> 
> Can we have some macros to define the above constant (and for the
> constants used below)?

will do in v2 patch

> > +
> > +   /* (2) start USB-PHY internal PLL */
> > +   iowrite32(0x0003, (reg0 + USBPCTRL1));
> 
> No power management stuff added?? Dont we need to stop this PLL during 
> suspend??
(snip)
> I see you are doing one time initialization of the phy during probe.
> But I think this phy will be pointless without the usb controller.
> Instead how about using the library functions like
> usb_add_phy/usb_get_phy/usb_phy_init?

I'm not sure detail, but this usb_xxx_phy() is under otg.
Our system don't support it, but is it normal ?

Now, I'm using ehci/ohci-platform.
When I support this usb_xxx_phy(), do I need to customize these drivers ?

And, this is extra stuff, but I have 1 question.
it is ${LINUX}/driver/usb/Makefile's order.

obj-(xxx) += host/
...
obj-(xxx) += phy/

This means that usb phy driver probe function is called
after host driver probe function().
Now, I'm using a technique which delays usb host driver registration,
but why phy driver is called after host driver ?

Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Ming Lei]

On Tue, Sep 4, 2012 at 6:36 PM, Bjørn Mork  wrote:
>
> I believe any Ericsson or Gobi modem would do, and most likely other USB
> networking devices too.
>
> I haven't explored the gadget.  Doesn't it support remote wakeup?  Well,

No.

> it doesn't really have to just for testing this.  You just have to fake
> the remote wakeup support, either in the gadget or in the device
> driver.  It doesn't matter whether it works or not.  The point is making
> the driver suspend the USB device while the network device is running.

The attachment patch is what I have been using to make it support remote
wakeup, and looks it works wrt. runtime PM.


>
>
>
> So the code trigging this seems to be
>
>
> static enum skb_state defer_bh(struct usbnet *dev, struct sk_buff *skb,
> struct sk_buff_head *list, enum skb_state state)
> {
> unsigned long   flags;
> enum skb_state  old_state;
> struct skb_data *entry = (struct skb_data *) skb->cb;
>
> spin_lock_irqsave(&list->lock, flags);
> old_state = entry->state;
> entry->state = state;
> __skb_unlink(skb, list);
> spin_unlock(&list->lock);
> spin_lock(&dev->done.lock);
> __skb_queue_tail(&dev->done, skb);
> if (dev->done.qlen == 1)
> tasklet_schedule(&dev->bh);
> spin_unlock_irqrestore(&dev->done.lock, flags);
> return old_state;
> }
>
>
> Hmm, I should probably dump stack here as well.. Anyway, it's a start.

Maybe a debug message in defer_bh is enough, :-)


Thanks,
--
Ming Lei


usb-gadget-remote-wakeup.patch
Description: Binary data

Re: generic wireless mouse not working

[Greg KH]

On Tue, Sep 04, 2012 at 08:01:33PM -0300, Marco Biscaro wrote:
> Hello all,
> 
> I've opened a bug report about a wireless mouse problem [1], and
> Greg Kroah-Hartman told me to post the link on this list. Could
> anyone take a look on the bug?
> 
> The problem was originally reported in Launchpad [2], and there is a
> patch [3], proposed by draekko (the original downstream reporter,
> which is CC'd).
> 
> Marco Biscaro
> 
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=47031
> [2] https://bugs.launchpad.net/linux/+bug/1021019
> [3] 
> https://bugs.launchpad.net/linux/+bug/1021019/+attachment/3213388/+files/novatech-mouse.patch

Care to take the patch and submit it properly through email as described
in the file Documentation/SubmittingPatches so we can apply it and get
this resolved?

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

generic wireless mouse not working

[Marco Biscaro]

Hello all,

I've opened a bug report about a wireless mouse problem [1], and Greg 
Kroah-Hartman told me to post the link on this list. Could anyone take a 
look on the bug?


The problem was originally reported in Launchpad [2], and there is a 
patch [3], proposed by draekko (the original downstream reporter, which 
is CC'd).


Marco Biscaro

[1] https://bugzilla.kernel.org/show_bug.cgi?id=47031
[2] https://bugs.launchpad.net/linux/+bug/1021019
[3] 
https://bugs.launchpad.net/linux/+bug/1021019/+attachment/3213388/+files/novatech-mouse.patch

--
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: Could you please help me out?

[Thomas Post]

Hi Sarah,

Guess what?  There is nothing wrong with your xHCI driver.  I picked
up my computer and when I brought it home, I fired it up and stuck in
a USB 3.0 device and all is well.  For the first time the drive lit
up, too.  The computer shop said they did a little research and
apparently Intel had made an announcement that the BIOS was not
handling the USB 3.0 ports correctly.  Apparently the BIOS date was
2/12 and now I have a newer BIOS and it works fine in Windows and
Linux Mint 13.  I am confident it will work now with all Linux
flavors.

Just in case you get another call or email from someone like me, the
problem was with the Intel DH77EB Motherboard BIOS.  Flash updating it
fixes the problem.

Thanks for all your help.  I'll save your message so I know how to
update a kernel should I ever need to.

Yours,

Tom

-- 
Dr. Thomas A. Post
Post Mixing Optimization and Solutions
(Bus.) +1-585-507-4318 (Fax) +1-585-383-8788
t...@postmixing.com - www.postmixing.com


On Tue, Sep 4, 2012 at 5:05 PM, Thomas Post  wrote:
> Hi Sarah,
>
> I installed Windows 7 with the Intel drivers and it still did not
> work.  So today I brought the PC back to where I bought it last
> Thursday and they looked it over.  They confirmed that they could not
> get it to work, either (Microsoft techs).  So the did a firmware and
> BIOS update on the motherboard and apparently it now works in Windows
> 7.  So I think that for whatever reason the Motherboard wasn't
> communicating properly to the USB 3.0 (the 2.0 ports were working
> fine).  I am going to pick it up now and shall see if this update
> makes it now work in Linux, too.  I will let you know, and thanks so
> much for your help.
>
> The last time you helped me, it is possible you told me to install a
> new kernel, but what I remember you saying and it worked was to use a
> newer version of Ubuntu.  When I did, everything worked fine.  The
> problem there was not USB on the Motherboard, but it was USB 3.0 on a
> PCIexpress card, and having one port connected to a 4 port USB 3.0
> hub.  In Ubuntu 10.10, it did not recognize the hub at all, but did
> recognize the 2 USB 3.0 ports on the pciExpress card.  Then upon your
> recommendation (or at least what I had interpreted was your
> recommendation) I installed Ubuntu 12.04 and voila it worked.
>
> Tom
>
> --
> Dr. Thomas A. Post
> Post Mixing Optimization and Solutions
> (Bus.) +1-585-507-4318 (Fax) +1-585-383-8788
> t...@postmixing.com - www.postmixing.com
>
> On Tue, Sep 4, 2012 at 3:39 PM, Sarah Sharp
>  wrote:
>>
>> Hi Tom,
>>
>> I'm sorry, I thought that since I had helped you with an issue in the
>> past that you would know how to install a custom kernel.  My bad!
>>
>> What I need you to do is test with the latest kernel from Linus.  I had
>> directions on how to do this on my blog, but apparently my blog is down
>> right now.  So here's what I need you to do:
>>
>> First, I need you to install a couple of packages:  git and
>> libncurses5-dev.
>>
>> Then, open a terminal, and run the following command:
>>
>> $ git clone
>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>
>> That will download the latest kernel from Linus into a directory called
>> "linux".  Now change into that directory:
>>
>> $ cd linux
>>
>> Next we need to compile the kernel.  We need to copy the configuration
>> from your distribution kernel.  That configuration is found in the /boot
>> directory.  You need to look for the latest file that starts with
>> "config" and copy that into the "linux" directory.
>>
>> For example, if I run this command, I'll see several config files:
>>
>> sarah@xanatos:/tmp$ ls /boot/config*
>> /boot/config-3.0.0-12-generic  /boot/config-3.0.0-17-generic
>> /boot/config-3.1.0   /boot/config-3.2.0-27-generic
>> /boot/config-3.2.0-rc2+  /boot/config-3.4.0
>> /boot/config-3.0.0-16-generic  /boot/config-3.0.0-20-generic
>> /boot/config-3.1.0+  /boot/config-3.2.0-29-generic
>> /boot/config-3.2.0-rc2+.old  /boot/config-3.5.0
>>
>> I want the config file with the biggest kernel number (in this case
>> /boot/config-3.5.0).  So I would copy it into the "linux" directory into
>> a new file called .config:
>>
>> $ cp /boot/config-3.5.0 ~/linux/.config
>>
>> Now I need you to modify the kernel configuration a bit.  We need to
>> turn on debugging in order to see where the kernel problem is.  You can
>> change the kernel configuration by running this command:
>>
>> $ make menuconfig
>>
>> That will open a graphical configuration manager.  You can use arrow
>> keys and enter to navigate into Device Drivers -> USB support.  Then you
>> need to turn on three options.  Turn on "USB verbose debug messages" by
>> moving to that option and hitting 'y'.  You'll need to turn
>> on "xHCI HCD (USB 3.0) support" by hitting 'm', and then hitting 'y' on
>> "Debugging for the xHCI host controller".  Then hit the right arrow key
>> to move the bottom cursor from "Select" to "Exit" and say 'y' to save
>> the configuration.
>>

Re: [PATCH] xhci: Make handover code more robust

[Sarah Sharp]

On Tue, Aug 14, 2012 at 04:44:49PM -0400, Matthew Garrett wrote:
> My test platform (Intel DX79SI) boots reliably under BIOS, but frequently
> crashes when booting via UEFI. I finally tracked this down to the xhci
> handoff code. It seems that reads from the device occasionally just return
> 0xff, resulting in xhci_find_next_cap_offset generating a value that's
> larger than the resource region. We then oops when attempting to read the
> value. Sanity checking that value lets us avoid the crash.
>
> I've no idea what's causing the underlying problem, and xhci still doesn't
> actually *work* even with this, but the machine at least boots which will
> probably make further debugging easier.

Is this an Ivy Bridge system?  xHCI may not work because you're skipping
the port switchover code just before the hc_init label.  Of course, if
the host controller continues to report all f's for register values,
there's no point in trying to initialize the host at all...

Sarah Sharp

> 
> Signed-off-by: Matthew Garrett 
> ---
>  drivers/usb/host/pci-quirks.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index df0828c..7cda232 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -815,12 +815,12 @@ static void __devinit quirk_usb_handoff_xhci(struct 
> pci_dev *pdev)
>   void __iomem *op_reg_base;
>   u32 val;
>   int timeout;
> + int len = pci_resource_len(pdev, 0);
>  
>   if (!mmio_resource_enabled(pdev, 0))
>   return;
>  
> - base = ioremap_nocache(pci_resource_start(pdev, 0),
> - pci_resource_len(pdev, 0));
> + base = ioremap_nocache(pci_resource_start(pdev, 0), len);
>   if (base == NULL)
>   return;
>  
> @@ -830,9 +830,17 @@ static void __devinit quirk_usb_handoff_xhci(struct 
> pci_dev *pdev)
>*/
>   ext_cap_offset = xhci_find_next_cap_offset(base, 
> XHCI_HCC_PARAMS_OFFSET);
>   do {
> + if ((ext_cap_offset + sizeof(val)) > len) {
> + /* We're reading garbage from the controller */
> + dev_warn(&pdev->dev,
> +  "xHCI controller failing to respond");
> + return;
> + }
> +
>   if (!ext_cap_offset)
>   /* We've reached the end of the extended capabilities */
>   goto hc_init;
> +
>   val = readl(base + ext_cap_offset);
>   if (XHCI_EXT_CAPS_ID(val) == XHCI_EXT_CAPS_LEGACY)
>   break;
> -- 
> 1.7.11.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb: host: xhci: Fix Compliance Mode on SN65LVPE502CP Hardware

[Greg KH]

On Tue, Sep 04, 2012 at 02:49:37PM -0700, Sarah Sharp wrote:
> Hi Alexis,
> 
> You caught me just before I left for vacation, which is why nothing has
> been done with this patch.  I think it's fine, and I'll try to apply it
> to my tree and send it to Greg tomorrow.
> 
> It is a larger patch, but it fixes a bug that's pretty user visible
> (dead USB ports), so my inclination is to queue it for stable as well.
> Greg, any objections?

None from me, it needs to get into older kernels as well.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: host: xhci-plat: use ioremap_nocache

[Sarah Sharp]

Hi Felipe,

Should this be queued for stable as well?

Sarah Sharp

On Fri, Aug 10, 2012 at 09:58:30AM +0300, Felipe Balbi wrote:
> From: Ruchika Kharwar 
> 
> Use the ioremap_nocache variant of the ioremap API in
> order to make sure our memory will be marked uncachable.
> 
> Signed-off-by: Ruchika Kharwar 
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/host/xhci-plat.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 689bc18..df90fe5 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -118,7 +118,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   goto put_hcd;
>   }
>  
> - hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
> + hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
>   if (!hcd->regs) {
>   dev_dbg(&pdev->dev, "error mapping memory\n");
>   ret = -EFAULT;
> -- 
> 1.7.4.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: usb: host: xhci: Fix Compliance Mode on SN65LVPE502CP Hardware

[Sarah Sharp]

Hi Alexis,

You caught me just before I left for vacation, which is why nothing has
been done with this patch.  I think it's fine, and I'll try to apply it
to my tree and send it to Greg tomorrow.

It is a larger patch, but it fixes a bug that's pretty user visible
(dead USB ports), so my inclination is to queue it for stable as well.
Greg, any objections?

Sarah Sharp

On Tue, Sep 04, 2012 at 01:40:42PM -0500, Alexis R. Cortes wrote:
> Hi Sarah/Greg,
> 
> Sorry to bother you, but I was wondering if you have some news concerning my 
> patch for the Compliance Mode issue of our re-driver. Should I submit the 
> patch again?
> 
> Thanks and Best Regards,
> Alexis Cortes.
> 
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org 
> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Alexis R. Cortes
> Sent: Tuesday, August 14, 2012 11:14 AM
> To: sarah.a.sh...@linux.intel.com
> Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; 
> linux-ker...@vger.kernel.org; brian.qu...@ti.com; jorge.lla...@ti.com
> Subject: Re: usb: host: xhci: Fix Compliance Mode on SN65LVPE502CP Hardware
> 
> Hi Sarah,
> 
> I was wondering if you have any news regarding this patch, or if there's 
> something else I need to change on code. I also noticed that I forgot to 
> write '[PATCH]' at the beginning of the patch's subject, Should I resend it?
> 
> Thanks and Best Regards,
> Alexis Cortes. 
> 
> On 8/3/2012 2:00 PM, Alexis R. Cortes wrote:
> > This patch is intended to work around a known issue on the 
> > SN65LVPE502CP USB3.0 re-driver that can delay the negotiation between 
> > a device and the host past the usual handshake timeout.
> > 
> > If that happens on the first insertion, the host controller port will 
> > enter in Compliance Mode and NO port status event will be generated 
> > (as per xHCI Spec) making impossible to detect this event by software. 
> > The port will remain in compliance mode until a warm reset is applied 
> > to it.
> > 
> > As a result of this, the port will seem "dead" to the user and no 
> > device connections or disconnections will be detected.
> > 
> > For solving this, the patch creates a timer which polls every 2 
> > seconds the link state of each host controller's port (this by reading 
> > the PORTSC register) and recovers the port by issuing a Warm reset 
> > every time Compliance mode is detected.
> > 
> > If a xHC USB3.0 port has previously entered to U0, the compliance mode 
> > issue will NOT occur only until system resumes from sleep/hibernate, 
> > therefore, the compliance mode timer is stopped when all xHC USB 3.0 
> > ports have entered U0. The timer is initialized again after each 
> > system resume.
> > 
> > Since the issue is being caused by a pice of hardware, the timer will 
> > be enabled ONLY on those systems that have the SN65LVPE502CP installed 
> > (this patch uses DMI strings for detecting those systems) therefore 
> > making this patch to act as a quirk (XHCI_COMP_MODE_QUIRK has been 
> > added to the xhci stack).
> > 
> > This patch applies for these systems:
> > Vendor: Hewlett-Packard. System Models: Z420, Z620 and Z820.
> > 
> > Signed-off-by: Alexis R. Cortes 
> > ---
> >  drivers/usb/host/xhci-hub.c |   42 +++
> >  drivers/usb/host/xhci.c |  121 
> > +++
> >  drivers/usb/host/xhci.h |6 ++
> >  3 files changed, 169 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c 
> > index 7b01094..32ca289 100644
> > --- a/drivers/usb/host/xhci-hub.c
> > +++ b/drivers/usb/host/xhci-hub.c
> > @@ -493,11 +493,48 @@ static void xhci_hub_report_link_state(u32 *status, 
> > u32 status_reg)
> >  * when this bit is set.
> >  */
> > pls |= USB_PORT_STAT_CONNECTION;
> > +   } else {
> > +   /*
> > +* If CAS bit isn't set but the Port is already at
> > +* Compliance Mode, fake a connection so the USB core
> > +* notices the Compliance state and resets the port.
> > +* This resolves an issue generated by the SN65LVPE502CP
> > +* in which sometimes the port enters compliance mode
> > +* caused by a delay on the host-device negotiation.
> > +*/
> > +   if (pls == USB_SS_PORT_LS_COMP_MOD)
> > +   pls |= USB_PORT_STAT_CONNECTION;
> > }
> > +
> > /* update status field */
> > *status |= pls;
> >  }
> >  
> > +/*
> > + * Function for Compliance Mode Quirk.
> > + *
> > + * This Function verifies if all xhc USB3 ports have entered U0, if 
> > +so,
> > + * the compliance mode timer is deleted. A port won't enter
> > + * compliance mode if it has previously entered U0.
> > + */
> > +void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32 status, u16 
> > +wIndex) {
> > +   u32 all_ports_seen_u0 = ((1 << xhci->num_usb3_ports)-1);
> > +   bool port_in_u0 = ((status & PORT_PLS_MASK) == XDEV_U0);
>

Re: [PATCH v2] usb: move phy driver from mach-tegra to drivers/usb

[Stephen Warren]

On 08/31/2012 12:45 AM, Venu Byravarasu wrote:
> As part of this patch:
>   1. Moved existing tegra phy driver to drivers/USB directory.
>   2. Added standard USB phy driver APIs to tegra phy driver.
> 
> Signed-off-by: Venu Byravarasu 

Tested-by: Stephen Warren 

Felipe, Would it be possible to apply this patch to a topic branch, so
that I can merge it into the Tegra tree? The reason is that I'd like to
remove Tegra's devices.[ch] in kernel 3.7, but to do that, I need to
move a tiny chunk of code out of that file somewhere else, and doing
that relies on adding a #include to usb_phy.h/tegra_usb_phy.h, which is
renamed in this patch.

Alternatively, if you don't think it'll cause any merge conflicts with
the USB PHY tree, I can just take this patch through the Tegra tree if
you want. Venu, do you plan any other patches within the next couple
weeks or so (i.e. before 3.6-rc6) that will depend on this patch? If so,
taking it only through the Tegra tree might not work so well.

Thanks.
--
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] USB: cdc-wdm: Extend and improve subdriver interface

[Oliver Neukum]

On Tuesday 04 September 2012 17:32:17 Bjørn Mork wrote:
> Oliver Neukum  writes:
> > On Tuesday 04 September 2012 15:45:36 Bjørn Mork wrote:

> >> USB_CDC_NOTIFY_NETWORK_CONNECTION and USB_CDC_NOTIFY_SPEED_CHANGE.
> >> cdc-wdm will just debug print USB_CDC_NOTIFY_NETWORK_CONNECTION and
> >> ignore it, but will log an error if it sees USB_CDC_NOTIFY_SPEED_CHANGE
> >
> > So provide callbacks for them.
> 
> It seems a littly overkill to provide a separate callback for each of
> these, so how about using the same callback but call it only for the
> notifications we know a main driver may be interested in? I.e. something
> along the lines

No. Once we've decided that multiple callbacks are needed, there's no use in
limiting their number. It is importantant that they get clear semantics and
reasonable names. You've already introduced a structure for them. So beef it up.

> Should I take it in again, and submit a new version for further
> comments?  Still won't have any sample code using the API, I'm afraid.

Please propose a version with the callbacks you'll need all separated.

Regards
Oliver

--
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: Could you please help me out?

[Thomas Post]

Hi Sarah,

I installed Windows 7 with the Intel drivers and it still did not
work.  So today I brought the PC back to where I bought it last
Thursday and they looked it over.  They confirmed that they could not
get it to work, either (Microsoft techs).  So the did a firmware and
BIOS update on the motherboard and apparently it now works in Windows
7.  So I think that for whatever reason the Motherboard wasn't
communicating properly to the USB 3.0 (the 2.0 ports were working
fine).  I am going to pick it up now and shall see if this update
makes it now work in Linux, too.  I will let you know, and thanks so
much for your help.

The last time you helped me, it is possible you told me to install a
new kernel, but what I remember you saying and it worked was to use a
newer version of Ubuntu.  When I did, everything worked fine.  The
problem there was not USB on the Motherboard, but it was USB 3.0 on a
PCIexpress card, and having one port connected to a 4 port USB 3.0
hub.  In Ubuntu 10.10, it did not recognize the hub at all, but did
recognize the 2 USB 3.0 ports on the pciExpress card.  Then upon your
recommendation (or at least what I had interpreted was your
recommendation) I installed Ubuntu 12.04 and voila it worked.

Tom

--
Dr. Thomas A. Post
Post Mixing Optimization and Solutions
(Bus.) +1-585-507-4318 (Fax) +1-585-383-8788
t...@postmixing.com - www.postmixing.com

On Tue, Sep 4, 2012 at 3:39 PM, Sarah Sharp
 wrote:
>
> Hi Tom,
>
> I'm sorry, I thought that since I had helped you with an issue in the
> past that you would know how to install a custom kernel.  My bad!
>
> What I need you to do is test with the latest kernel from Linus.  I had
> directions on how to do this on my blog, but apparently my blog is down
> right now.  So here's what I need you to do:
>
> First, I need you to install a couple of packages:  git and
> libncurses5-dev.
>
> Then, open a terminal, and run the following command:
>
> $ git clone
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>
> That will download the latest kernel from Linus into a directory called
> "linux".  Now change into that directory:
>
> $ cd linux
>
> Next we need to compile the kernel.  We need to copy the configuration
> from your distribution kernel.  That configuration is found in the /boot
> directory.  You need to look for the latest file that starts with
> "config" and copy that into the "linux" directory.
>
> For example, if I run this command, I'll see several config files:
>
> sarah@xanatos:/tmp$ ls /boot/config*
> /boot/config-3.0.0-12-generic  /boot/config-3.0.0-17-generic
> /boot/config-3.1.0   /boot/config-3.2.0-27-generic
> /boot/config-3.2.0-rc2+  /boot/config-3.4.0
> /boot/config-3.0.0-16-generic  /boot/config-3.0.0-20-generic
> /boot/config-3.1.0+  /boot/config-3.2.0-29-generic
> /boot/config-3.2.0-rc2+.old  /boot/config-3.5.0
>
> I want the config file with the biggest kernel number (in this case
> /boot/config-3.5.0).  So I would copy it into the "linux" directory into
> a new file called .config:
>
> $ cp /boot/config-3.5.0 ~/linux/.config
>
> Now I need you to modify the kernel configuration a bit.  We need to
> turn on debugging in order to see where the kernel problem is.  You can
> change the kernel configuration by running this command:
>
> $ make menuconfig
>
> That will open a graphical configuration manager.  You can use arrow
> keys and enter to navigate into Device Drivers -> USB support.  Then you
> need to turn on three options.  Turn on "USB verbose debug messages" by
> moving to that option and hitting 'y'.  You'll need to turn
> on "xHCI HCD (USB 3.0) support" by hitting 'm', and then hitting 'y' on
> "Debugging for the xHCI host controller".  Then hit the right arrow key
> to move the bottom cursor from "Select" to "Exit" and say 'y' to save
> the configuration.
>
> Now we need to build the kernel.  This may take a while, so you probably
> want to leave it for an hour or so.  Run these commands:
>
> $ make -j4
> $ sudo make modules_install install
>
> You'll need to be set up as a superuser to use "sudo".  If it gives you
> some sort of error when you try to use sudo, run these commands instead:
>
> $ su -i
>
> It will prompt for your root/superuser/admin password and the cursor
> will change to a # to indicate you are logged in as root.  Next I need
> you to change into the linux directory in your homedir.  I don't know
> what your username is, but the command will look something like:
>
> # cd /home/user/linux
>
> Then run the second make command, without the sudo prefix:
>
> # make modules_install install
>
> Ok, so now you should have the latest kernel version compiled and
> installed!  Next, reboot, and see if the xHCI driver loads for your host
> controller.  If not, we can discuss further debugging steps from there.
>
> Sarah Sharp
>
> On Sat, Sep 01, 2012 at 09:55:27PM +, Thomas Post wrote:
> > Hi Sarah,
> >
> > Thank you so much for the speed reply.  I wish I was as smart as you.  I
>

Re: Kconfig DRM_USB/DRM_UDL, and select vs. depends, and causing Tegra USB to be disabled

[Stephen Warren]

On 09/04/2012 02:00 PM, Guenter Roeck wrote:
> On Tue, Sep 04, 2012 at 01:19:12PM -0600, Stephen Warren wrote:
>> With respect to the following commits:
>>
>> df0b344 drm/usb: select USB_SUPPORT in Kconfig
>> 8f057d7 gpu/mfd/usb: Fix USB randconfig problems
>>
>> ... which end up with the following in next-20120904:
>>
>> config DRM_USB
>> depends on DRM
>> depends on USB_ARCH_HAS_HCD
>> select USB
>> select USB_SUPPORT
>>
>> config DRM_UDL
>> depends on DRM && EXPERIMENTAL
>> depends on USB_ARCH_HAS_HCD
>> select DRM_USB
>>
>> Surely this is backwards; these should be dependencies, not selects? In
>> other words:
>>
>> config DRM_USB
>> depends on DRM && USB
>>
>> config DRM_UDL
>> depends on DRM && EXPERIMENTAL && USB
>> select DRM_USB
>>
>> or perhaps:
>>
>> config DRM_USB
>> depends on DRM && USB
>>
>> config DRM_UDL
>> depends on DRM && EXPERIMENTAL && DRM_USB
>>
>> The problem here is that currently, the dependency logic for USB:
>>
>> config USB
>>  depends on USB_ARCH_HAS_HCD
>>
>> ... is duplicated into each of DRM_USB and DRM_UDL, thus requiring both
>> of those to be edited should the dependencies for USB ever change.
>
> This should be fixed with in https://patchwork.kernel.org/patch/1373371/ (drm:
> udl: usb: Fix recursive Kconfig dependency), which should make it into the 
> next
> iteration of linux-next.

Yes, this does appear to solve all the problems for me. Thanks.

I still tend to believe that drivers should probably depend on things
rather than select them, but given the common precedent for "select USB"
that exists here, others clearly don't agree!

Sorry; accidentally sent the email too early last time:-(
--
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] USB: cdc-wdm: Extend and improve subdriver interface

[Oliver Neukum]

On Tuesday 04 September 2012 16:09:13 Alexey Orishko wrote:

> MBIM CID handling should not be done in the driver, but in the user space 
> daemon
> or connection manager application. So the need is to add only support
> for encapsulated
> commands and either use a static amount of network interfaces or create them
> dynamically based on CID parser commands to the driver.
> It also requires updates for constructing headers, etc, but it's simple 
> change.

Yes, provided all of it can be done in user space.

> Why do we need cdc-wdm driver? It would be too complex to handle all
> required data.
> We already handling several control requests, so adding one more won't
> be a problem.
> And we have to expose some control interface towards user space from
> mbim driver.

Implementing this protocol is harder than it looks. If code already exists,
it is a good idea to reuse it.

Regards
Oliver

--
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: Kconfig DRM_USB/DRM_UDL, and select vs. depends, and causing Tegra USB to be disabled

[Guenter Roeck]

On Tue, Sep 04, 2012 at 01:19:12PM -0600, Stephen Warren wrote:
> With respect to the following commits:
> 
> df0b344 drm/usb: select USB_SUPPORT in Kconfig
> 8f057d7 gpu/mfd/usb: Fix USB randconfig problems
> 
> ... which end up with the following in next-20120904:
> 
> config DRM_USB
> depends on DRM
> depends on USB_ARCH_HAS_HCD
> select USB
> select USB_SUPPORT
> 
> config DRM_UDL
> depends on DRM && EXPERIMENTAL
> depends on USB_ARCH_HAS_HCD
> select DRM_USB
> 
> Surely this is backwards; these should be dependencies, not selects? In
> other words:
> 
> config DRM_USB
> depends on DRM && USB
> 
> config DRM_UDL
> depends on DRM && EXPERIMENTAL && USB
> select DRM_USB
> 
> or perhaps:
> 
> config DRM_USB
> depends on DRM && USB
> 
> config DRM_UDL
> depends on DRM && EXPERIMENTAL && DRM_USB
> 
> The problem here is that currently, the dependency logic for USB:
> 
> config USB
>   depends on USB_ARCH_HAS_HCD
> 
> ... is duplicated into each of DRM_USB and DRM_UDL, thus requiring both
> of those to be edited should the dependencies for USB ever change.
> 
This should be fixed with in https://patchwork.kernel.org/patch/1373371/ (drm:
udl: usb: Fix recursive Kconfig dependency), which should make it into the next
iteration of linux-next.

Guenter
--
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: Could you please help me out?

[Sarah Sharp]

Hi Tom,

I'm sorry, I thought that since I had helped you with an issue in the
past that you would know how to install a custom kernel.  My bad!

What I need you to do is test with the latest kernel from Linus.  I had
directions on how to do this on my blog, but apparently my blog is down
right now.  So here's what I need you to do:

First, I need you to install a couple of packages:  git and
libncurses5-dev.

Then, open a terminal, and run the following command:

$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

That will download the latest kernel from Linus into a directory called
"linux".  Now change into that directory:

$ cd linux

Next we need to compile the kernel.  We need to copy the configuration
from your distribution kernel.  That configuration is found in the /boot
directory.  You need to look for the latest file that starts with
"config" and copy that into the "linux" directory.

For example, if I run this command, I'll see several config files:

sarah@xanatos:/tmp$ ls /boot/config*
/boot/config-3.0.0-12-generic  /boot/config-3.0.0-17-generic
/boot/config-3.1.0   /boot/config-3.2.0-27-generic
/boot/config-3.2.0-rc2+  /boot/config-3.4.0
/boot/config-3.0.0-16-generic  /boot/config-3.0.0-20-generic
/boot/config-3.1.0+  /boot/config-3.2.0-29-generic
/boot/config-3.2.0-rc2+.old  /boot/config-3.5.0

I want the config file with the biggest kernel number (in this case
/boot/config-3.5.0).  So I would copy it into the "linux" directory into
a new file called .config:

$ cp /boot/config-3.5.0 ~/linux/.config

Now I need you to modify the kernel configuration a bit.  We need to
turn on debugging in order to see where the kernel problem is.  You can
change the kernel configuration by running this command:

$ make menuconfig

That will open a graphical configuration manager.  You can use arrow
keys and enter to navigate into Device Drivers -> USB support.  Then you
need to turn on three options.  Turn on "USB verbose debug messages" by
moving to that option and hitting 'y'.  You'll need to turn
on "xHCI HCD (USB 3.0) support" by hitting 'm', and then hitting 'y' on
"Debugging for the xHCI host controller".  Then hit the right arrow key
to move the bottom cursor from "Select" to "Exit" and say 'y' to save
the configuration.

Now we need to build the kernel.  This may take a while, so you probably
want to leave it for an hour or so.  Run these commands:

$ make -j4
$ sudo make modules_install install

You'll need to be set up as a superuser to use "sudo".  If it gives you
some sort of error when you try to use sudo, run these commands instead:

$ su -i

It will prompt for your root/superuser/admin password and the cursor
will change to a # to indicate you are logged in as root.  Next I need
you to change into the linux directory in your homedir.  I don't know
what your username is, but the command will look something like:

# cd /home/user/linux

Then run the second make command, without the sudo prefix:

# make modules_install install

Ok, so now you should have the latest kernel version compiled and
installed!  Next, reboot, and see if the xHCI driver loads for your host
controller.  If not, we can discuss further debugging steps from there.

Sarah Sharp

On Sat, Sep 01, 2012 at 09:55:27PM +, Thomas Post wrote:
> Hi Sarah,
> 
> Thank you so much for the speed reply.  I wish I was as smart as you.  I
> don't know what you mean by this:
> 
> Also, please retry with the latest -rc kernel from linus. We extended the
> reset command timeout, which helped another chipset that had problems
> during init. That was commit 22ceac1. From your logs, it looks like you
> might be running into a similar issue.
> 
> Does this mean I should boot up in Ubuntu 12.04 and edit the boot up line
> with a -rc parameter?  What is commit 22ceac1?  Is that another command
> line modifier?
> 
> Tom
> 
> PS:  Thanks again for all your help.
> 
> -- 
> Dr. Thomas A. Post
> Post Mixing Optimization and Solutions
> (Bus.) +1-585-507-4318 (Fax) +1-585-383-8788
> t...@postmixing.com - www.postmixing.com
> 
> On Sat, Sep 1, 2012 at 9:24 PM, Sarah A Sharp  wrote:
> 
> > Hi Tom,
> >
> > Can you resend this to my work address 
> > and cc the  mailing list?
> >
> > Also, please retry with the latest -rc kernel from linus. We extended the
> > reset command timeout, which helped another chipset that had problems
> > during init. That was commit 22ceac1. From your logs, it looks like you
> > might be running into a similar issue.
> >
> > Sarah Sharp
> > On Sep 1, 2012 1:43 PM, "Thomas Post"  wrote:
> >
> >> Hi Sarah,
> >>
> >> I found reference to you on the internet and you helped me last year with
> >> a USB 3.0 problem.  Thanks again for that.
> >>
> >> I just put together another PC.  I used the Intel Motherboard DH77EB.  I
> >> have an i7 3770 3.4 gHz chip and 16 MB.  I cannot get the onboard USB 3.0
> >> ports to work.
> >>
> >> I googled everything before I had this computer put together (Microworx,
> >

Re: [PATCH 2/3] usb: otg: add device tree support to otg library

[Marc Kleine-Budde]

On 09/04/2012 07:51 PM, ABRAHAM, KISHON VIJAY wrote:
>>> Since it's already a common function, we may give phandler property
>>> a common name too. So we will not need phandle argument.
>>> Please also don't forget to document the devm_xxx and dt binding.
>>
>> I don't think this is a good idea. If we hardcode the phandle name, 
>> we
>> introduce a limit of one phy per usb device. The usb3 controllers
>> alreadyt use two phys (one for usb2, the othere for usb3) for one
>> controller. So I think we should not make the same mistake again.
 That only means current binding is not good enough. Rather not, means
 it should not be in common code.
 Maybe something like:
 usbport0-phys = <&phy0>;
 usbport1-phys = <&phy1 &phy2>; /* usb2 & usb3 */
>>>
>>> Granted. Do we need strings that describe the phys, like in pinctrl or
>>> is a index enough? What about this?
>>>
>>> struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>>>   int index)
>>>
>>
>> Comments? The phandle_name string will be "usbphy".
>
> I don't think phandle_name should be usbphy. Eventually we want to turn
> this into a kernel-wide phy subsystem and if we use usbphy, we will just
> have to patch a bunch of dts files once we make the move.
>>> Coud you please give a link of "kernel-wide phy subsystem" discussion?

 Is just "phy" better?
>>> If the property name don't include port number, how do we know what
>>> port the phy is attached to?
> 
> We can use something like "-phy" as the phandle name. And the
> users can get the phy by using
> devm_usb_get_phy_by_phandle(dev, "").
> (So the frwrk appends *-phy* to the name and searches). Or we don't
> have any  restriction on the phandle naming conventions and search for
> the phandle by the name the user passes in devm_usb_get_phy_by_phandle
> directly.

Maintainer, I need a Maintainer. Can someone please decide what we want
to have here. I can code all that, but please someone has to make a
decision. Now, please.

> Btw the regulator framework uses something like "-supply" and it
> took me sometime to figure out the regulator phandle name should be
> appended with "-supply".
> 
> +   const char *phandle)
>>
>> Take this ci13xxx-imx dts snippet for example:
>>
>> usb@02184000 { /* USB OTG */
>> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
>> reg = <0x02184000 0x200>;
>> interrupts = <0 43 0x04>;
>> phy = <&usbphy1>;
>> };
>>
>> The existing "fsl,usbphy" will be renamed into just "phy". If a usb/otg
>> device needs more than one phy the dts will look like this:
>>
>> usb@02184000 { /* USB OTG */
>> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
>> reg = <0x02184000 0x200>;
>> interrupts = <0 43 0x04>;
>> phy = <&usbphy1 &usbphy2>;
> 
> Will this work? Can we return two phandles for a single phandle name?

Not two handles with one devm_usb_get_phy_by_phandle call :), but if you
want to get both you do:

phy0 = devm_usb_get_phy_by_phandle(dev, 0);
phy1 = devm_usb_get_phy_by_phandle(dev, 1);

The magic lies in the third parameter of the of_parse_phandle():

of_parse_phandle(dev->of_node, "phy", index);

This even fits into Ravi Babu's use case, two phys are attached to one
device (at least at the DT abstraction level).

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Kconfig DRM_USB/DRM_UDL, and select vs. depends, and causing Tegra USB to be disabled

[Stephen Warren]

With respect to the following commits:

df0b344 drm/usb: select USB_SUPPORT in Kconfig
8f057d7 gpu/mfd/usb: Fix USB randconfig problems

... which end up with the following in next-20120904:

config DRM_USB
depends on DRM
depends on USB_ARCH_HAS_HCD
select USB
select USB_SUPPORT

config DRM_UDL
depends on DRM && EXPERIMENTAL
depends on USB_ARCH_HAS_HCD
select DRM_USB

Surely this is backwards; these should be dependencies, not selects? In
other words:

config DRM_USB
depends on DRM && USB

config DRM_UDL
depends on DRM && EXPERIMENTAL && USB
select DRM_USB

or perhaps:

config DRM_USB
depends on DRM && USB

config DRM_UDL
depends on DRM && EXPERIMENTAL && DRM_USB

The problem here is that currently, the dependency logic for USB:

config USB
depends on USB_ARCH_HAS_HCD

... is duplicated into each of DRM_USB and DRM_UDL, thus requiring both
of those to be edited should the dependencies for USB ever change.

The current state of the code also causes some strange problem with
ARM's tegra_defconfig, whereby running "make tegra_defconfig" will
result in USB support fully enabled in .config as expected, yet
subsequently running "make oldconfig" will cause all USB support to be
removed from .config. For some reason, the above DRM logic is causing
CONFIG_USB_ARCH_HAS_HCD not to be selected (perhaps it isn't evaluated
because USB is selected, so there's no need to evaluate USB's
dependencies?). Arguably, this is a deficiency in Tegra's Kconfig, in
that it probably should say:

select USB_ARCH_HAS_EHCI

not:

select USB_ARCH_HAS_EHCI if USB_SUPPORT

... but it has contained the latter for quite some time, and it's always
worked before somehow.
--
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 v9 01/13] usb: musb: dsps: add phy control logic to glue

[B, Ravi]

Hi

> > > > >> AM335x uses NOP transceiver driver and need to enable
> > > builtin PHY
> > > > >> by writing into usb_ctrl register available in 
> system control 
> > > > >> module register space. This is being added at musb 
> glue driver 
> > > > >> layer untill a separate system control module driver is
> > > available.
> > > > >>
> > > > >> Signed-off-by: Ajay Kumar Gupta 
> > > > >> Signed-off-by: Santhapuri, Damodar 
> 
> > > > >> Signed-off-by: Ravi Babu 
> > > > >
> > > > > Kishon, you were adding a real phy driver for OMAP's internal 
> > > > > phy logic on one of your patches and I believe this will
> > > conflict with
> > > > > your changes, right ?
> > > > 
> > > > Indeed. My final patch of that series removes some of the 
> > > > functions from omap_phy_internal.c (which was taken 
> care in the phy driver).
> > > > >
> > > > > How does this look to you ? Is this at least correct ? I
> > > suppose the
> > > > > correct way would be to actually have the system 
> control module 
> > > > > driver which we have been waiting, right ?
> > > > 
> > > > Correct. I think once we have the system control module 
> driver in 
> > > > place, we'll have everything wrt control module register writes 
> > > > implemented in correct way.
> > > 
> > > So $SUBJECT will pretty much be thrown away once we have 
> SCM driver, 
> > > in that case it's best to wait a bit longer and apply this series 
> > > once SCM driver is available and after your series too... 
> you agree 
> > > ?
> > > 
> > 
> > Felipe, I am sure there are patches in this series[0/13], which are 
> > not dependent on this patch or control module, Can we pull in those 
> > patches (all dual instances support patches)? So that I can re-work 
> > and submit again?
> 
> sure, will do, don't worry :-)

Thanks.
Then shall I rework patches [3/13 to 13/13] and re-submit only musb dual 
instances patches which 
are independent of control module. 

> 
> --
> balbi
> --
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: surfstick K5006-Z in 3.6.0-rc4

[Bjørn Mork]

Thomas Schäfer  writes:

> T:  Bus=01 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  4 Spd=480  MxCh= 0
> D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> P:  Vendor=19d2 ProdID=1018 Rev= 0.00
> S:  Manufacturer=ZTE,Incorporated
> S:  Product=ZTE LTE Technologies MSM
> S:  SerialNumber=MF821VVDFS02
> C:* #Ifs= 5 Cfg#= 1 Atr=c0 MxPwr=500mA
> I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=86 Prot=10 Driver=(none)
> E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=02 Prot=05 Driver=(none)
> E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
> I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=02 Prot=01 Driver=(none)
> E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=2ms
> E:  Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
> I:* If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=06 Prot=00 Driver=qmi_wwan
> E:  Ad=85(I) Atr=03(Int.) MxPS=  64 Ivl=2ms
> E:  Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms


OK, so ZTE might have picked up a good idea from Huawei, using subclass
and protocol to differentiate between all the vendor specific functions.
It would have been even nicer if they documented it so we wouldn't havt
to guess...

You wouldn't happen to have a windows *.inf file for this device as
well? Would be interesting to see how ZTE intended these functions to be
matched.

Anyway, I think we should replace the device specific entry in the
option driver with 3 vendor+class matches for the 3 serial functions.

The qmi_wwan entry worked by luck because it matches on the interface
number.  But we might consider replacing that entry as well if ZTE are
going to use ff/06/00 for the QMI/wwan function on more of these
devices.  I'll keep my eyes open.




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 2/3] usb: otg: add device tree support to otg library

[ABRAHAM, KISHON VIJAY]

Hi,

On Tue, Sep 4, 2012 at 7:46 PM, Marc Kleine-Budde  wrote:
> On 09/04/2012 04:07 PM, Richard Zhao wrote:
>>> +struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>>> +const char *phandle)
>
>> Since it's already a common function, we may give phandler property
>> a common name too. So we will not need phandle argument.
>> Please also don't forget to document the devm_xxx and dt binding.
>
> I don't think this is a good idea. If we hardcode the phandle name, we
> introduce a limit of one phy per usb device. The usb3 controllers
> alreadyt use two phys (one for usb2, the othere for usb3) for one
> controller. So I think we should not make the same mistake again.
>>> That only means current binding is not good enough. Rather not, means
>>> it should not be in common code.
>>> Maybe something like:
>>> usbport0-phys = <&phy0>;
>>> usbport1-phys = <&phy1 &phy2>; /* usb2 & usb3 */
>>
>> Granted. Do we need strings that describe the phys, like in pinctrl or
>> is a index enough? What about this?
>>
>> struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>>   int index)
>>
>
> Comments? The phandle_name string will be "usbphy".

 I don't think phandle_name should be usbphy. Eventually we want to turn
 this into a kernel-wide phy subsystem and if we use usbphy, we will just
 have to patch a bunch of dts files once we make the move.
>> Coud you please give a link of "kernel-wide phy subsystem" discussion?
>>>
>>> Is just "phy" better?
>> If the property name don't include port number, how do we know what
>> port the phy is attached to?

We can use something like "-phy" as the phandle name. And the
users can get the phy by using
devm_usb_get_phy_by_phandle(dev, "").
(So the frwrk appends *-phy* to the name and searches). Or we don't
have any  restriction on the phandle naming conventions and search for
the phandle by the name the user passes in devm_usb_get_phy_by_phandle
directly.
Btw the regulator framework uses something like "-supply" and it
took me sometime to figure out the regulator phandle name should be
appended with "-supply".

+   const char *phandle)
>
> Take this ci13xxx-imx dts snippet for example:
>
> usb@02184000 { /* USB OTG */
> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> reg = <0x02184000 0x200>;
> interrupts = <0 43 0x04>;
> phy = <&usbphy1>;
> };
>
> The existing "fsl,usbphy" will be renamed into just "phy". If a usb/otg
> device needs more than one phy the dts will look like this:
>
> usb@02184000 { /* USB OTG */
> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> reg = <0x02184000 0x200>;
> interrupts = <0 43 0x04>;
> phy = <&usbphy1 &usbphy2>;

Will this work? Can we return two phandles for a single phandle name?

Thanks
Kishon
--
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: surfstick K5006-Z in 3.6.0-rc4

[Thomas Schäfer]

Am Dienstag, 4. September 2012, 11:21:44 schrieben Sie:

> Could you post the output of lsusb -v or /sys/kernel/debug/usb/devices
> (with debugfs mounted) for this device?



Here they are:


dmesg
lsusb 
 /sys/kernel/debug/usb/devices

[ 1080.264122] usb 1-4: new high-speed USB device number 3 using ehci_hcd
[ 1080.400198] usb 1-4: New USB device found, idVendor=19d2, idProduct=1017
[ 1080.400216] usb 1-4: New USB device strings: Mfr=3, Product=2, SerialNumber=4
[ 1080.400229] usb 1-4: Product: ZTE LTE Technologies MSM
[ 1080.400241] usb 1-4: Manufacturer: ZTE,Incorporated
[ 1080.400253] usb 1-4: SerialNumber: MF821VVDFS02
[ 1080.86] usbcore: registered new interface driver uas
[ 1080.452216] Initializing USB Mass Storage driver...
[ 1080.453840] scsi4 : usb-storage 1-4:1.0
[ 1080.454128] usbcore: registered new interface driver usb-storage
[ 1080.454135] USB Mass Storage support registered. 
 
[ 1083.599662] usb 1-4: USB disconnect, device number 3 
 
[ 1088.088111] usb 1-4: new high-speed USB device number 4 using ehci_hcd
[ 1088.223941] usb 1-4: New USB device found, idVendor=19d2, idProduct=1018 
  
[ 1088.223955] usb 1-4: New USB device strings: Mfr=3, Product=2, 
SerialNumber=4  
[ 1088.223965] usb 1-4: Product: ZTE LTE Technologies MSM   
  
[ 1088.223973] usb 1-4: Manufacturer: ZTE,Incorporated  
  
[ 1088.223981] usb 1-4: SerialNumber: MF821VVDFS02  
  
[ 1088.231065] scsi5 : usb-storage 1-4:1.4
[ 1088.282944] usbcore: registered new interface driver cdc_wdm 
 
[ 1088.292875] qmi_wwan 1-4:1.3: cdc-wdm0: USB WDM device
[ 1088.293426] qmi_wwan 1-4:1.3: wwan0: register 'qmi_wwan' at 
usb-:00:1d.7-4, WWAN/QMI device, 5a:8e:7f:57:b6:ca
[ 1088.293537] usbcore: registered new interface driver qmi_wwan
[ 1089.229793] scsi 5:0:0:0: CD-ROMVodafone USB SCSI CD-ROM  2.31 
PQ: 0 ANSI: 0
[ 1089.230464] scsi 5:0:0:0: Attached scsi generic sg1 type 5
[ 1089.231789] scsi 5:0:0:1: Direct-Access Vodafone Storage  2.31 
PQ: 0 ANSI: 0
[ 1089.235281] sd 5:0:0:1: Attached scsi generic sg2 type 0
[ 1089.239118] sd 5:0:0:1: [sdb] Attached SCSI removable disk
[ 1089.264303] sr0: scsi-1 drive
[ 1089.264313] cdrom: Uniform CD-ROM driver Revision: 3.20
[ 1089.265674] sr 5:0:0:0: Attached scsi CD-ROM sr0

lsusb -v -d 19d2:1018

Bus 001 Device 004: ID 19d2:1018 ONDA Communication S.p.A. 
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass0 (Defined at Interface level)
  bDeviceSubClass 0 
  bDeviceProtocol 0 
  bMaxPacketSize064
  idVendor   0x19d2 ONDA Communication S.p.A.
  idProduct  0x1018 
  bcdDevice0.00
  iManufacturer   3 ZTE,Incorporated
  iProduct2 ZTE LTE Technologies MSM
  iSerial 4 MF821VVDFS02
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength  138
bNumInterfaces  5
bConfigurationValue 1
iConfiguration  1 ZTE Configuration
bmAttributes 0xc0
  Self Powered
MaxPower  500mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   2
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass134 
  bInterfaceProtocol 16 
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval  32
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01  EP 1 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval  32
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber1
  bAlternateSetting   0
  bNumEndpoints   2
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass  2 
  bInterfaceProtocol  5 
  iInterface  0 
  Endpoint Descriptor:
bLength 7

[PATCH] usb: gadget: fsl_udc_core: do not immediatly prime STATUS for IN xfer

[Enrico Scholz]

Because the fsl_udc_core driver shares one 'status_req' object for the
complete ep0 control transfer, it is not possible to prime the final
STATUS phase immediately after the IN transaction.  E.g. ch9getstatus()
executed:

| req = udc->status_req;
| ...
| list_add_tail(&req->queue, &ep->queue);
| if (ep0_prime_status(udc, EP_DIR_OUT))
|   
|   struct fsl_req *req = udc->status_req;
|   list_add_tail(&req->queue, &ep->queue);

which corrupts the ep->queue list by inserting 'status_req' twice.  This
causes a kernel oops e.g. when 'lsusb -v' is executed on the host.

Patch delays the final 'ep0_prime_status(udc, EP_DIR_OUT))' by moving it
into the ep0 completion handler.

Signed-off-by: Enrico Scholz 
---
 drivers/usb/gadget/fsl_udc_core.c | 16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/fsl_udc_core.c 
b/drivers/usb/gadget/fsl_udc_core.c
index d7138cc..55c4a61 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -1294,8 +1294,7 @@ static int ep0_prime_status(struct fsl_udc *udc, int 
direction)
udc->ep0_dir = USB_DIR_OUT;
 
ep = &udc->eps[0];
-   if (udc->ep0_state != DATA_STATE_XMIT)
-   udc->ep0_state = WAIT_FOR_OUT_STATUS;
+   udc->ep0_state = WAIT_FOR_OUT_STATUS;
 
req->ep = ep;
req->req.length = 0;
@@ -1400,8 +1399,6 @@ static void ch9getstatus(struct fsl_udc *udc, u8 
request_type, u16 value,
 
list_add_tail(&req->queue, &ep->queue);
udc->ep0_state = DATA_STATE_XMIT;
-   if (ep0_prime_status(udc, EP_DIR_OUT))
-   ep0stall(udc);
 
return;
 stall:
@@ -1511,14 +1508,6 @@ static void setup_received_irq(struct fsl_udc *udc,
spin_lock(&udc->lock);
udc->ep0_state = (setup->bRequestType & USB_DIR_IN)
?  DATA_STATE_XMIT : DATA_STATE_RECV;
-   /*
-* If the data stage is IN, send status prime immediately.
-* See 2.0 Spec chapter 8.5.3.3 for detail.
-*/
-   if (udc->ep0_state == DATA_STATE_XMIT)
-   if (ep0_prime_status(udc, EP_DIR_OUT))
-   ep0stall(udc);
-
} else {
/* No data phase, IN status from gadget */
udc->ep0_dir = USB_DIR_IN;
@@ -1548,7 +1537,8 @@ static void ep0_req_complete(struct fsl_udc *udc, struct 
fsl_ep *ep0,
switch (udc->ep0_state) {
case DATA_STATE_XMIT:
/* already primed at setup_received_irq */
-   udc->ep0_state = WAIT_FOR_OUT_STATUS;
+   if (ep0_prime_status(udc, EP_DIR_OUT))
+   ep0stall(udc);
break;
case DATA_STATE_RECV:
/* send status phase */
-- 
1.7.11.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: fsl_udc_core: remove mapped flag

[Enrico Scholz]

The 'mapped' flag in 'struct fsl_req' flag is redundant with checking
for 'req.dma != DMA_ADDR_INVALID' and it was also set to a wrong value
(see 2nd hunk of patch).

Replacing it in the way described above saves 60 bytes:

  function old new   delta
  fsl_udc_irq 29522940 -12
  ep0_prime_status 380 368 -12
  done 448 432 -16
  fsl_ep_queue 668 648 -20

and has same (or less) runtime costs like evaluating 'req->mapped'.

Signed-off-by: Enrico Scholz 
---
 drivers/usb/gadget/fsl_udc_core.c | 10 ++
 drivers/usb/gadget/fsl_usb2_udc.h |  1 -
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/fsl_udc_core.c 
b/drivers/usb/gadget/fsl_udc_core.c
index 55c4a61..1282a11 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -195,14 +195,13 @@ static void done(struct fsl_ep *ep, struct fsl_req *req, 
int status)
dma_pool_free(udc->td_pool, curr_td, curr_td->td_dma);
}
 
-   if (req->mapped) {
+   if (req->req.dma != DMA_ADDR_INVALID) {
dma_unmap_single(ep->udc->gadget.dev.parent,
req->req.dma, req->req.length,
ep_is_in(ep)
? DMA_TO_DEVICE
: DMA_FROM_DEVICE);
req->req.dma = DMA_ADDR_INVALID;
-   req->mapped = 0;
} else
dma_sync_single_for_cpu(ep->udc->gadget.dev.parent,
req->req.dma, req->req.length,
@@ -915,15 +914,12 @@ fsl_ep_queue(struct usb_ep *_ep, struct usb_request 
*_req, gfp_t gfp_flags)
req->req.length, ep_is_in(ep)
? DMA_TO_DEVICE
: DMA_FROM_DEVICE);
-   req->mapped = 1;
-   } else {
+   } else
dma_sync_single_for_device(ep->udc->gadget.dev.parent,
req->req.dma, req->req.length,
ep_is_in(ep)
? DMA_TO_DEVICE
: DMA_FROM_DEVICE);
-   req->mapped = 0;
-   }
 
req->req.status = -EINPROGRESS;
req->req.actual = 0;
@@ -1306,7 +1302,6 @@ static int ep0_prime_status(struct fsl_udc *udc, int 
direction)
req->req.dma = dma_map_single(ep->udc->gadget.dev.parent,
req->req.buf, req->req.length,
ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-   req->mapped = 1;
 
if (fsl_req_to_dtd(req, GFP_ATOMIC) == 0)
fsl_queue_td(ep, req);
@@ -1389,7 +1384,6 @@ static void ch9getstatus(struct fsl_udc *udc, u8 
request_type, u16 value,
req->req.dma = dma_map_single(ep->udc->gadget.dev.parent,
req->req.buf, req->req.length,
ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-   req->mapped = 1;
 
/* prime the data phase */
if ((fsl_req_to_dtd(req, GFP_ATOMIC) == 0))
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h 
b/drivers/usb/gadget/fsl_usb2_udc.h
index fbd77ba..9aab166 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -436,7 +436,6 @@ struct fsl_req {
/* ep_queue() func will add
   a request->queue into a udc_ep->queue 'd tail */
struct fsl_ep *ep;
-   unsigned mapped:1;
 
struct ep_td_struct *head, *tail;   /* For dTD List
   cpu endian Virtual addr */
-- 
1.7.11.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: otg: Move phy interface to separate file.

[Greg KH]

On Tue, Sep 04, 2012 at 02:25:58PM +0530, Venu Byravarasu wrote:
> As otg.h is containing lots of phy interface related
> stuff, moving all phy interface related stuff to new
> file named phy.h
> 
> Signed-off-by: Venu Byravarasu 

For some reason, I don't think that is a valid email address :(

--
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 V2 2/2] USB: Set usb port's DevicerRemovable according acpi information in EHCI

[Alan Stern]

On Mon, 3 Sep 2012, Lan Tianyu wrote:

> ACPI provide "_PLD" and "_UPC" aml methods to describe usb port
> visibility and connectability. This patch is to use those information
> to set usb port's DeviceRemovable.
> 
> Signed-off-by: Lan Tianyu 
> ---
> v2: Set DeviceRemovable according acpi infomation in the hub_configure()
> instead of calling get_hub_descriptor().

> @@ -1566,6 +1566,25 @@ static int hub_configure(struct usb_hub *hub,
>   dev_err(hub->intfdev,
>   "couldn't create port%d device.\n", i + 1);
>  
> + if (hub_is_superspeed(hdev)) {
> + for (i = 1; i <= hdev->maxchild; i++)
> + if (hub->ports[i - 1]->connect_type
> + == USB_PORT_CONNECT_TYPE_HARD_WIRED)
> + hub->descriptor->u.hs.DeviceRemovable[i/8]
> + |= 1 << (i%8);
> + } else  {
> + u16 port_removable = 0;
> +
> + for (i = i; i <= hdev->maxchild; i++)
> + if (hub->ports[i - 1]->connect_type
> + == USB_PORT_CONNECT_TYPE_HARD_WIRED)
> + port_removable |= 1 << i;
> +
> + memset(&hub->descriptor->u.ss.DeviceRemovable,
> + (__force __u16) cpu_to_le16(port_removable),
> + sizeof(__u16));

Use put_unaligned_le16() instead of memset, if you're worried about 
alignment.

This isn't right, because you overwrite information provided by the hub 
with what ACPI says, even if ACPI doesn't say anything.  You should 
initialize port_removable to the original value of 
u.ss.DeviceRemovable, not to 0.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] USB: cdc-wdm: Extend and improve subdriver interface

[Bjørn Mork]

Alexey Orishko  writes:
> On Tue, Sep 4, 2012 at 3:45 PM, Bjørn Mork  wrote:
>
>> Yes.  But this time with some hope of multi-vendor support, given that
>> Microsoft points to it for Windows 8 Mobile Broadband device support:
>> http://msdn.microsoft.com/en-us/library/windows/hardware/mbim-based-mobile-broadband-requirements-for-windows.aspx
> It looks like mandatory feature for W8 logo tests, so vendors must adapt to it

That was my interpretation as well, but I never understand these
commercial issues so I wouldn't be surprised if some vendor found some
other solution.

Anyway, as things are looking right now we need to support this protocol
in Linux for the next (or next-next?) generation of mobile broadband
devices.

>> Yes, I expect that major changes to cdc_ncm will be necessary, and
>> pointing to it could be wrong from my side? It just seemed natural to
>> try to reuse any existing code.
>
> MBIM CID handling should not be done in the driver, but in the user space 
> daemon
> or connection manager application.

Exactly!  That is the main target of this excercise.

> So the need is to add only support
> for encapsulated
> commands and either use a static amount of network interfaces or create them
> dynamically based on CID parser commands to the driver.
> It also requires updates for constructing headers, etc, but it's simple 
> change.

Yes, something like that is required.  Greg S will probably come up with
some good ideas here :-)

 CDC MBIM features we are going to support, there is also a possibility
 that the main driver will have to intercept messages between the
 userspace and the cdc-wdm subdriver.
>>>
>>> Ouch.
> Why do we need cdc-wdm driver? It would be too complex to handle all
> required data.
> We already handling several control requests, so adding one more won't
> be a problem.
> And we have to expose some control interface towards user space from
> mbim driver.

That's what we use the cdc-wdm driver for.  It takes the CDC
encapsulated MBIM control protocol and export it as a character device
to the userspace application.  The ideal case is the one we have in
qmi_wwan, where cdc-wdm exports the QMI protocol without either qmi_wwan
or cdc-wdm knowing anything about the protocol at all.  I hope we can
get as close to that for MBIM too, only looking at one (or a limited
set of) specific messages from the device to know when to add or remove
a network device.

>> yes.  Reading the spec terrified me.  Among other issues, it allows
>> multiplexing several IP connections over a single set of bulk in/out
>> endpoints, where an "IP connection" probably will have to map logically
>> to a network device.
> Yes, it means we get more than one virtual network interface.

Right.  So the driver will not necessarily create network devices on
probe, but instead when it sees a MBIM message from the device
indicating a successful IP session start.


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: [RFC] USB: cdc-wdm: Extend and improve subdriver interface

[Bjørn Mork]

Oliver Neukum  writes:
> On Tuesday 04 September 2012 15:45:36 Bjørn Mork wrote:
>
>> Yes, I expect that major changes to cdc_ncm will be necessary, and
>> pointing to it could be wrong from my side? It just seemed natural to
>> try to reuse any existing code.
>
> No, that is perfectly all right. It seems to me that the API between usbnet
> and its subdrivers is not powerful enough to support CDC-NCM well.

Maybe not.  But I assume that any shortcomings found could be integrated
in usbnet, if that is found to be the best solution.

So what we are looking for is somehow for a usbnet minidriver to receive
and submit single packet skbs and have usbnet handle all the logic
related to packing and unpacking the multi-packet URBs.  Should not be
impossible.

>> >> found that the cdc-wdm subdriver interface must be extended somewhat for
>> >> this to be possible.  At the very least, the main driver need to handle
>> >> some notifications which are ignored by cdc-wdm.  And depending on which
>> >
>> > Which ones?
>> 
>> The ones which are already handled by cdc_ncm I believe:
>> 
>> USB_CDC_NOTIFY_NETWORK_CONNECTION and USB_CDC_NOTIFY_SPEED_CHANGE.
>> cdc-wdm will just debug print USB_CDC_NOTIFY_NETWORK_CONNECTION and
>> ignore it, but will log an error if it sees USB_CDC_NOTIFY_SPEED_CHANGE
>
> So provide callbacks for them.

It seems a littly overkill to provide a separate callback for each of
these, so how about using the same callback but call it only for the
notifications we know a main driver may be interested in? I.e. something
along the lines


switch (dr->bNotificationType) {
case USB_CDC_NOTIFY_RESPONSE_AVAILABLE:
dev_dbg(&desc->intf->dev,
"NOTIFY_RESPONSE_AVAILABLE received: index %d len %d",
dr->wIndex, dr->wLength);
break;
case USB_CDC_NOTIFY_NETWORK_CONNECTION:
if (desc->driver_info->status &&
!desc->driver_info->status(desc->intf, urb))
dev_dbg(&desc->intf->dev,
"NOTIFY_NETWORK_CONNECTION %s network",
dr->wValue ? "connected to" : "disconnected 
from");
goto exit;
case USB_CDC_NOTIFY_SPEED_CHANGE:
if (desc->driver_info->status &&
desc->driver_info->status(desc->intf, urb))
goto exit;
/* else fallthrough */
default:
clear_bit(WDM_POLL_RUNNING, &desc->flags);
dev_err(&desc->intf->dev,
"unknown notification %d received: index %d len %d\n",
dr->bNotificationType, dr->wIndex, dr->wLength);
goto exit;
}



>> yes.  Reading the spec terrified me.  Among other issues, it allows
>> multiplexing several IP connections over a single set of bulk in/out
>> endpoints, where an "IP connection" probably will have to map logically
>> to a network device.
>
> Yes, but given the capability of UMTS something like this would be needed.

I am not convinced about that..  Devices supporting more that 10
connections?  Well, the USB IF seems to think there will be, so you
are probably right...  I'd just like to see one of those devices :-)

>> I believe we can get away with the main driver only looking at messages
>> received from the device, without changing or filtering them.
>
> In hindsight, cdc-wdm should have used the tty layer. Then we'd now
> write a line discipline.

Too late to change that, I guess.  One of these days I'll have to find
out how all this tty magic works...
  
>> I see that I should have included the data interception hook, or not
>> have mentioned it at all. Sorry for any confusion.
>
> Very well.

Should I take it in again, and submit a new version for further
comments?  Still won't have any sample code using the API, I'm afraid.

Thanks for yet again giving very useful and constructive feedback.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: ftdi-sio: add support for more Physik Instrumente devices

[Éric Piel]

Commit b69cc672052540 added support for the E-861.  After acquiring a C-867, I
realised that every Physik Instrumente's device has a different PID. They are
listed in the Windows device driver's .inf file. So here are all PIDs for the
current (and probably future) USB devices from Physik Instrumente.

Compiled, but only actually tested on the E-861 and C-867.

Signed-off-by: Éric Piel 
Cc: stable 
---
 drivers/usb/serial/ftdi_sio.c |   17 +
 drivers/usb/serial/ftdi_sio_ids.h |   21 -
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 5620db6..1197d47 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -810,7 +810,24 @@ static struct usb_device_id id_table_combined [] = {
.driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
{ USB_DEVICE(LARSENBRUSGAARD_VID, LB_ALTITRACK_PID) },
{ USB_DEVICE(GN_OTOMETRICS_VID, AURICAL_USB_PID) },
+   { USB_DEVICE(FTDI_VID, PI_C865_PID) },
+   { USB_DEVICE(FTDI_VID, PI_C857_PID) },
+   { USB_DEVICE(PI_VID, PI_C866_PID) },
+   { USB_DEVICE(PI_VID, PI_C663_PID) },
+   { USB_DEVICE(PI_VID, PI_C725_PID) },
+   { USB_DEVICE(PI_VID, PI_E517_PID) },
+   { USB_DEVICE(PI_VID, PI_C863_PID) },
{ USB_DEVICE(PI_VID, PI_E861_PID) },
+   { USB_DEVICE(PI_VID, PI_C867_PID) },
+   { USB_DEVICE(PI_VID, PI_E609_PID) },
+   { USB_DEVICE(PI_VID, PI_E709_PID) },
+   { USB_DEVICE(PI_VID, PI_100F_PID) },
+   { USB_DEVICE(PI_VID, PI_1011_PID) },
+   { USB_DEVICE(PI_VID, PI_1012_PID) },
+   { USB_DEVICE(PI_VID, PI_1013_PID) },
+   { USB_DEVICE(PI_VID, PI_1014_PID) },
+   { USB_DEVICE(PI_VID, PI_1015_PID) },
+   { USB_DEVICE(PI_VID, PI_1016_PID) },
{ USB_DEVICE(KONDO_VID, KONDO_USB_SERIAL_PID) },
{ USB_DEVICE(BAYER_VID, BAYER_CONTOUR_CABLE_PID) },
{ USB_DEVICE(FTDI_VID, MARVELL_OPENRD_PID),
diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
b/drivers/usb/serial/ftdi_sio_ids.h
index 5dd96ca..080d158 100644
--- a/drivers/usb/serial/ftdi_sio_ids.h
+++ b/drivers/usb/serial/ftdi_sio_ids.h
@@ -791,8 +791,27 @@
  * Physik Instrumente
  * http://www.physikinstrumente.com/en/products/
  */
+/* These two devices use the VID of FTDI */
+#define PI_C865_PID0xe0a0  /* PI C-865 Piezomotor Controller */
+#define PI_C857_PID0xe0a1  /* PI Encoder Trigger Box */
+
 #define PI_VID  0x1a72  /* Vendor ID */
-#define PI_E861_PID 0x1008  /* E-861 piezo controller USB connection */
+#define PI_C866_PID0x1000  /* PI C-866 Piezomotor Controller */
+#define PI_C663_PID0x1001  /* PI C-663 Mercury-Step */
+#define PI_C725_PID0x1002  /* PI C-725 Piezomotor Controller */
+#define PI_E517_PID0x1005  /* PI E-517 Digital Piezo Controller Operation 
Module */
+#define PI_C863_PID0x1007  /* PI C-863 */
+#define PI_E861_PID0x1008  /* PI E-861 Piezomotor Controller */
+#define PI_C867_PID0x1009  /* PI C-867 Piezomotor Controller */
+#define PI_E609_PID0x100D  /* PI E-609 Digital Piezo Controller */
+#define PI_E709_PID0x100E  /* PI E-709 Digital Piezo Controller */
+#define PI_100F_PID0x100F  /* PI Digital Piezo Controller */
+#define PI_1011_PID0x1011  /* PI Digital Piezo Controller */
+#define PI_1012_PID0x1012  /* PI Motion Controller */
+#define PI_1013_PID0x1013  /* PI Motion Controller */
+#define PI_1014_PID0x1014  /* PI Device */
+#define PI_1015_PID0x1015  /* PI Device */
+#define PI_1016_PID0x1016  /* PI Digital Servo Module */
 
 /*
  * Kondo Kagaku Co.Ltd.
-- 
1.7.9.5

--
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 V2 1/2] usb: usb port power off mechanism add "auto" option

[Alan Stern]

On Mon, 3 Sep 2012, Lan Tianyu wrote:

> This patch is to add "auto" option to attribute portX/control.
> When echo "auto", the port's feature PORT_POWER would be clear
> if the port's connect type was mark not-used(connectability and
> visibility are both cleared) and with no device attached.
> 
> Signed-off-by: Lan Tianyu 

Acked-by: 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] USB/host: Cleanup unneccessary irq disable code

[Alan Stern]

On Sat, 1 Sep 2012, Liu, Chuansheng wrote:

> From: liu chuansheng 
> Subject: [PATCH] USB/host: Cleanup unneccessary irq disable code
> 
> Because the IRQF_DISABLED as the flag is now a NOOP and has been
> deprecated and in hardirq context the interrupt is disabled.
> 
> so in usb/host code:
> Removing the usage of flag IRQF_DISABLED;
> Removing the calling local_irq save/restore actions in irq
> handler usb_hcd_irq();
> 
> Signed-off-by: liu chuansheng 

Acked-by: 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/2] ARM: Kirkwood: ehci-orion: Add device tree binding

[Alan Stern]

On Sat, 1 Sep 2012, Andrew Lunn wrote:

> Based on previous work by Michael Walle and Jason Cooper.
> 
> Made their work actually work, which required added interrupt from DT
> and auxdata, along with setting the dma_mask, which DT does not
> currently do.
> 
> Signed-off-by: Andrew Lunn 

For the changes to ehci-orion.c:

Acked-by: Alan Stern 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: USB sound card device complains about "error -28: not enough bandwidth" only on specific PC hardware, seems not kernel specific

[Alan Stern]

On Sun, 26 Aug 2012, David Ranch wrote:

> > By the way, now that you've got CONFIG_USB_DEBUG enabled, there's 
> > something I'd like to see. I'm assuming you've got a debugfs 
> > filesystem mounted on /sys/kernel/debug. Go into 
> > /sys/kernel/debug/usb/ehci/:00:1d.0/ (that's the directory 
> > corresponding to the controller your device is plugged into, bus 2) 
> > and post a copy of the "periodic" file in that directory

...

> If you can help me figure out why the EHCI debug interface isn't showing 
> up (maybe the /proc line is a hint), I'll recompile and get the results 
> you asked for.

Were you able to make any progress on this?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: add device quirk for Joss Optical touchboard

[Alan Stern]

This patch (as1604) adds a CONFIG_INTF_STRINGS quirk for the Joss
infrared touchboard device.  The device doesn't like to be asked for
its interface strings.

Signed-off-by: Alan Stern 
Reported-by: adam ? 
CC: 

---

 drivers/usb/core/quirks.c |4 
 1 file changed, 4 insertions(+)

Index: usb-3.5/drivers/usb/core/quirks.c
===
--- usb-3.5.orig/drivers/usb/core/quirks.c
+++ usb-3.5/drivers/usb/core/quirks.c
@@ -71,6 +71,10 @@ static const struct usb_device_id usb_qu
{ USB_DEVICE(0x04b4, 0x0526), .driver_info =
USB_QUIRK_CONFIG_INTF_STRINGS },
 
+   /* Microchip Joss Optical infrared touchboard device */
+   { USB_DEVICE(0x04d8, 0x000c), .driver_info =
+   USB_QUIRK_CONFIG_INTF_STRINGS },
+
/* Samsung Android phone modem - ID conflict with SPH-I500 */
{ USB_DEVICE(0x04e8, 0x6601), .driver_info =
USB_QUIRK_CONFIG_INTF_STRINGS },

--
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 v9 01/13] usb: musb: dsps: add phy control logic to glue

[Felipe Balbi]

Hi,

On Tue, Sep 04, 2012 at 02:04:15PM +, B, Ravi wrote:
> > On Fri, Aug 31, 2012 at 06:51:04PM +0530, ABRAHAM, KISHON VIJAY wrote:
> > > Hi,
> > > 
> > > On Fri, Aug 31, 2012 at 5:53 PM, Felipe Balbi  wrote:
> > > > Hi,
> > > >
> > > > On Fri, Aug 31, 2012 at 04:39:47PM +0530, Ravi Babu wrote:
> > > >> From: Santhapuri, Damodar 
> > > >>
> > > >> AM335x uses NOP transceiver driver and need to enable 
> > builtin PHY 
> > > >> by writing into usb_ctrl register available in system control 
> > > >> module register space. This is being added at musb glue driver 
> > > >> layer untill a separate system control module driver is 
> > available.
> > > >>
> > > >> Signed-off-by: Ajay Kumar Gupta 
> > > >> Signed-off-by: Santhapuri, Damodar 
> > > >> Signed-off-by: Ravi Babu 
> > > >
> > > > Kishon, you were adding a real phy driver for OMAP's internal phy 
> > > > logic on one of your patches and I believe this will 
> > conflict with 
> > > > your changes, right ?
> > > 
> > > Indeed. My final patch of that series removes some of the functions 
> > > from omap_phy_internal.c (which was taken care in the phy driver).
> > > >
> > > > How does this look to you ? Is this at least correct ? I 
> > suppose the 
> > > > correct way would be to actually have the system control module 
> > > > driver which we have been waiting, right ?
> > > 
> > > Correct. I think once we have the system control module driver in 
> > > place, we'll have everything wrt control module register writes 
> > > implemented in correct way.
> > 
> > So $SUBJECT will pretty much be thrown away once we have SCM 
> > driver, in that case it's best to wait a bit longer and apply 
> > this series once SCM driver is available and after your 
> > series too... you agree ?
> > 
> 
> Felipe, I am sure there are patches in this series[0/13], which are
> not dependent on this patch or control module, Can we pull in those
> patches (all dual instances support patches)? So that I can re-work
> and submit again? 

sure, will do, don't worry :-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] USB: ohci-at91: fix PIO handling in relation with number of ports

[Alan Stern]

On Tue, 4 Sep 2012, Nicolas Ferre wrote:

> On 08/29/2012 11:49 AM, Nicolas Ferre :
> > If the number of ports present on the SoC/board is not the maximum
> > and that the platform data is not filled with all data, there is
> > an easy way to mess the PIO setup for this interface.
> > This quick fix addresses mis-configuration in USB host platform data
> > that is common in at91 boards since commit 0ee6d1e (USB: ohci-at91:
> > change maximum number of ports) that did not modified the associatd
> > board files.
> > 
> > Reported-by: Klaus Falkner 
> > Signed-off-by: Nicolas Ferre 
> > Cc: Alan Stern 
> > Cc: Greg Kroah-Hartman 
> > Cc: linux-usb@vger.kernel.org
> > Cc: Stable  [3.4+]
> 
> Alan, Greg, gentle ping...
> 
> I really would like to seen it landing in stable soon...
> 
> Thanks, best regards,

Sorry for the delay.  I feel a little uncomfortable judging board-level 
changes like this, since I know nothing about the hardware details.

Still, for what it's worth, this patch is okay as far as the core 
ohci-hcd driver components are concerned.  So...

Acked-by: 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: JMicron 20337 (152d:2338) and 3TB

[Alon Bar-Lev]

On Tue, Sep 4, 2012 at 5:25 PM, Alan Stern  wrote:
>
> On Sun, 2 Sep 2012, Alon Bar-Lev wrote:
>
> > I modified the patch to the following to make sure my main disk is
> > unaffected:
> > ---
> > --- drivers/scsi/sd.c   2012-07-21 23:58:29.0 +0300
> > +++ drivers/scsi/sd.c.new   2012-08-31 19:47:15.822632952 +0300
> > @@ -1899,13 +1899,17 @@ static int sd_try_rc16_first(struct scsi
> >  {
> > if (sdp->host->max_cmd_len < 16)
> > return 0;
> > -   if (sdp->try_rc_10_first)
> > -   return 0;
> > +   if (sdp->try_rc_10_first) {
> > +   printk("@ALON: apply workaround 1");
> > +   /*return 0;*/
> > +   }
> > if (sdp->scsi_level > SCSI_SPC_2)
> > return 1;
> > if (scsi_device_protection(sdp))
> > return 1;
> > -   return 0;
> > +   printk("@ALON: apply workaround 2");
> > +   /*return 0;*/
> > +   return 1;
> >  }
>
> If you still have the JMicron device, can you post a usbmon trace
> showing what happens without any patches at all?  I'm curious to see
> how the device gets handled.
>
> Alan Stern
>

Sorry... returned this device, looking for some workable device.
I very much appreciate your help!

Going to try out this one[1][2], but now I go to the store and test it
on site before buying.

Alon

[1] http://a.slickdeals.net/attachment.php?attachmentid=84784&d=...
[2] 
http://www.zhuzhuchina.com/store/esata_to_usb_2_0_external_adapter_spif225a_serial_ata.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] How to handle delays in isochronous transfers?

[Alan Stern]

On Tue, 4 Sep 2012, Jassi Brar wrote:

> >> How about effectively increasing the queue length from 10ms to 50ms
> >> (max anticipated latency) ?
> >
> > There are two problems with that approach.  First, 50 ms isn't really
> > the max anticipated latency; it's merely the largest that I've seen so
> > far.  (In fact, the max anticipated latency is probably < 10 ms; these
> > 50-ms delays were definitely exceptional.)
> >
> It doesn't have to be hardcoded - maybe USB audio/video code could get
> the hint via some module parameter?

That's even worse.  Fortunately it isn't necessary; ehci-hcd uses an 
I/O watchdog timer so the latency can never be much longer than 100 ms.

> > Second, people involved in real-time programming (such as audio or
> > video) generally want to keep latency to a minimum.
> >
> If we progress the h/w pointer of ALSA ring buffer at URB completion
> (and not at URB submission), this shouldn't affect the latency. And

I don't understand this comment.  As far as I can tell, the relevant 
pointer already is being advanced during URB completion.

> IIRC, USB isn't anyway recommended for real-time usage.

People still want to minimize latency, though.  I doubt that anybody 
would be happy with 100 ms latency.  Imagine trying to talk over a 
loudspeaker system if the sounds coming out of the speaker were 100 ms 
behind your voice.

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: JMicron 20337 (152d:2338) and 3TB

[Alan Stern]

On Sun, 2 Sep 2012, Alon Bar-Lev wrote:

> I modified the patch to the following to make sure my main disk is unaffected:
> ---
> --- drivers/scsi/sd.c   2012-07-21 23:58:29.0 +0300
> +++ drivers/scsi/sd.c.new   2012-08-31 19:47:15.822632952 +0300
> @@ -1899,13 +1899,17 @@ static int sd_try_rc16_first(struct scsi
>  {
> if (sdp->host->max_cmd_len < 16)
> return 0;
> -   if (sdp->try_rc_10_first)
> -   return 0;
> +   if (sdp->try_rc_10_first) {
> +   printk("@ALON: apply workaround 1");
> +   /*return 0;*/
> +   }
> if (sdp->scsi_level > SCSI_SPC_2)
> return 1;
> if (scsi_device_protection(sdp))
> return 1;
> -   return 0;
> +   printk("@ALON: apply workaround 2");
> +   /*return 0;*/
> +   return 1;
>  }

If you still have the JMicron device, can you post a usbmon trace
showing what happens without any patches at all?  I'm curious to see
how the device gets handled.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] How to handle delays in isochronous transfers?

[Alan Stern]

On Tue, 4 Sep 2012, Peter Chen wrote:

> > In such a situation, the delay is much bigger than the device's buffer,
> > so just sending more samples afterwards will not help.
> >
> It is ISO transfer, if the delay is too much, and the buffer at device side is
> empty, it is normal the screen is stopped like we watch movie on Internet
> (buffering).
> 
> So if the delay is too much, I don't think there is a way can deal with it as
> host does not send any data to device.

Yes, some data is lost.  We want to make sure that when the data starts
flowing again, it is properly synchronized.

> > Furthermore, if packets are lost, frequency feedback becomes impossible
> > because the device doesn't know how many samples were lost.
> Where the packets are lost?
> If the packets are lost at class driver/usb driver, class driver will
> know it and
> should take the responsible to re-send it.
> If the packets are lost on the USB bus (during the transfer), as it is
> ISO transfer,
> then the data should be lost, and host doesn't know the data is lost,  how can
> it re-sends the packet?

Data should not be resent in any case.  Isochronous data is never 
resent.

Here's the problem we face:  The class driver submits packets 1, 2, 3,
and 4.  They get sent properly, but the completion interrupt is
delayed.  As a result, the class driver's completion handler doesn't
get called until too late; the frames for packets 5 - 44 have already
expired.  The data that should have been sent during those frames is 
lost.

But the class driver doesn't know this, so when the completion handler
does get called, it submits packets 5, 6, 7, and 8.  They end up
getting sent in frames 45, 46, 47, and 48.  This means the data is now
out of synchronization by 40 frames.

That's the problem I want to solve.

There's an equivalent problem for data flowing into the host.  In this
case, the URB that should have been submitted for frame 5 would receive
data that the device sent during frame 45.  (Whether this data contains
the signals for frame 5 or for frame 45 would depend on the device; the
host can't do anything about it.)

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 05/11] USB: chipidea: don't let probe fail if otg controller start one role failed

[Richard Zhao]

On Wed, Aug 29, 2012 at 06:46:00PM +0800, Richard Zhao wrote:
> On Wed, Aug 29, 2012 at 12:48:15PM +0300, Alexander Shishkin wrote:
> > Richard Zhao  writes:
> > 
> > > On Wed, Aug 29, 2012 at 11:10:33AM +0300, Alexander Shishkin wrote:
> > >> Richard Zhao  writes:
> > >> 
> > >> > On Tue, Aug 28, 2012 at 11:38:23AM +0300, Alexander Shishkin wrote:
> > >> >> Richard Zhao  writes:
> > >> >> 
> > >> >> > One role failed, but the other role will possiblly still work.
> > >> >> > For example, when udc start failed, if plug in a host device,
> > >> >> > it works.
> > >> >> 
> > >> >> If you're a host device, ci->role should be HOST at this point and
> > >> >> ci_role_start() shouldn't fail. If ci->role is detected wrongly, the
> > >> >> role detection must be fixed. If ci_role_start() fails for host, but 
> > >> >> it
> > >> >> still works when it's called again after the id pin change is 
> > >> >> detected,
> > >> >> again, the problem is elsewhere.
> > >> > The check is only for OTG device. For single role device, it just fail
> > >> > if it start the role failed.
> > >> 
> > >> Sorry, can you rephrase?
> > >> 
> > >> >> 
> > >> >> >
> > >> >> > Signed-off-by: Richard Zhao 
> > >> >> > ---
> > >> >> >  drivers/usb/chipidea/core.c |7 +--
> > >> >> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >> >> >
> > >> >> > diff --git a/drivers/usb/chipidea/core.c 
> > >> >> > b/drivers/usb/chipidea/core.c
> > >> >> > index 19ef324..8fd390a 100644
> > >> >> > --- a/drivers/usb/chipidea/core.c
> > >> >> > +++ b/drivers/usb/chipidea/core.c
> > >> >> > @@ -473,8 +473,11 @@ static int __devinit ci_hdrc_probe(struct 
> > >> >> > platform_device *pdev)
> > >> >> > ret = ci_role_start(ci, ci->role);
> > >> >> > if (ret) {
> > >> >> > dev_err(dev, "can't start %s role\n", 
> > >> >> > ci_role(ci)->name);
> > >> >> > -   ret = -ENODEV;
> > >> >> > -   goto rm_wq;
> > >> >> > +   ci->role = CI_ROLE_END;
> > >> >> 
> > >> >> So are you relying on id pin interrupt for initializing the role after
> > >> >> this? Can you provide more details?
> > >> > Yes. The ID pin detect still works. My case is, gadget role failed,
> > >> > host role works. I was trying not to ruin host function.
> > >> 
> > >> But it shouldn't even try to start the gadget, because ci_otg_role()
> > >> should set ci->role to HOST before ci_role_start() happens.
> > > It depends on ID pin. If ID pin is gadget and gadget is not support well,
> > > ci_role_start will fail. See below example.
> > >> 
> > >> Another question is, why does gadget start fail?
> > > There's one example:
> > > If usb phy don't support otg yet, otg_set_peripheral will fail, and
> > > then cause udc_start fail.
> > 
> > So you should drop the CI13XXX_REQUIRE_TRANSCEIVER flag from imx
> > platform data till the phy is fully implemented.
> It's just an example. We can not assume udc_start always success. If it
> fails, isn't it better to continue support of host than say game over?
Hi Alex,

Is it persuadable? Could you please give comments of other patches too?

Thanks
Richard
> 
> Thanks
> Richard
> > 
> > Regards,
> > --
> > Alex
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: otg: add device tree support to otg library

[Marc Kleine-Budde]

On 09/04/2012 04:07 PM, Richard Zhao wrote:
>> +struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>> +const char *phandle)

> Since it's already a common function, we may give phandler property
> a common name too. So we will not need phandle argument.
> Please also don't forget to document the devm_xxx and dt binding.

 I don't think this is a good idea. If we hardcode the phandle name, we
 introduce a limit of one phy per usb device. The usb3 controllers
 alreadyt use two phys (one for usb2, the othere for usb3) for one
 controller. So I think we should not make the same mistake again.
>> That only means current binding is not good enough. Rather not, means
>> it should not be in common code.
>> Maybe something like:
>> usbport0-phys = <&phy0>;
>> usbport1-phys = <&phy1 &phy2>; /* usb2 & usb3 */
>
> Granted. Do we need strings that describe the phys, like in pinctrl or
> is a index enough? What about this?
>
> struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>   int index)
>

 Comments? The phandle_name string will be "usbphy".
>>>
>>> I don't think phandle_name should be usbphy. Eventually we want to turn
>>> this into a kernel-wide phy subsystem and if we use usbphy, we will just
>>> have to patch a bunch of dts files once we make the move.
> Coud you please give a link of "kernel-wide phy subsystem" discussion?
>>
>> Is just "phy" better?
> If the property name don't include port number, how do we know what
> port the phy is attached to?

Take this ci13xxx-imx dts snippet for example:

usb@02184000 { /* USB OTG */
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x02184000 0x200>;
interrupts = <0 43 0x04>;
phy = <&usbphy1>;
};

The existing "fsl,usbphy" will be renamed into just "phy". If a usb/otg
device needs more than one phy the dts will look like this:

usb@02184000 { /* USB OTG */
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x02184000 0x200>;
interrupts = <0 43 0x04>;
phy = <&usbphy1 &usbphy2>;
};

The driver will get a reference to the usb_phy with:

struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
int index)

Where the index specifies with (usb) phy it wants to use.

This covers TI's usecase with an USB2 and an USB3 phy for one otg device.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v6 1/3] USB: chipidea: add imx usbmisc support

[Richard Zhao]

On Wed, Aug 29, 2012 at 10:00:32PM +0200, Marc Kleine-Budde wrote:
> On 08/29/2012 01:01 PM, Sascha Hauer wrote:
> > On Wed, Aug 29, 2012 at 01:18:10PM +0300, Alexander Shishkin wrote:
> >> Sascha Hauer  writes:
> >>
> >>> On Wed, Aug 29, 2012 at 10:50:08AM +0300, Alexander Shishkin wrote:
>  Richard Zhao  writes:
> 
> > i.MX usb controllers shares non-core registers, which may include
> > SoC specific controls. We take it as a usbmisc device and usbmisc
> > driver set operations needed by ci13xxx_imx driver.
> >
> > For example, Sabrelite board has bad over-current design, we can
> > usbmisc to disable over-current detect.
> 
>  Why does this have to be part of the usb driver instead of SoC specific
>  code? It looks like you've created a whole new device/driver
>  infrastructure just to disable overcurrent for a specific board.
> >>>
> >>> Richards code indeed only handles overcurrent for a specific board, but
> >>> there are more bits to configure in the longer run: power pin
> >>> polarities, ULPI/serial mode select and some more.
> 
> We already have a patch adding a usbmisc_imx53_init() function to that
> driver.
> 
> >> Sounds to me like these things that should be taken care of by the phy
> >> driver, which will likely be simpler from both driver's and devicetree's
> >> perspective.
> > 
> > Most i.MX SoCs have three instances of the chipidea core. These cores
> > share a single register space for controlling the mentioned bits (the
> > usbmisc register space). The usbmisc looks different on the different
> > SoCs.
> > Indeed they control some phy specific aspects, but the phy itself may
> > also be an external ULPI or UTMI phy with a separate driver. So if we
> > integrate the usbmisc into the phy wouldn't that mean that it has to
> > be integrated into all possible phy drivers?
> > 
> > From a devicetrees perspective it makes sense to integrate the flags
> > into the chipidea nodes, because there is one node per chipidea core,
> > but only one usbmisc unit for all ports on the SoC. So we can do a:
> > 
> > chipidea@ofs {
> > disable-overcurrent = <1>;
> > };
> > 
> > instead of
> > 
> > usbmisc@ofs {
> > disable-overcurrent-port0 = <1>;
> > disable-overcurrent-port1 = <0>;
> > ...
> > };
> 
> +1
> 
> IMHO looks much cleaner.
So, Marc agree on the patch too. Maybe you can give a reviewed-by? :)

Hi Alex,

What do you think?

Thanks
Richard
> 
>  And the infrastructure boils down to a complex way of passing a callback
>  from imx driver to another imx driver, that only works if they are
>  probed in the right order. I don't see any point in doing it like this
>  other than inflating the device tree tables even further.
> 
>  Why can't this be part of the SoC code like it is done, for example in
>  arch/arm/mach-omap2/control.c?
> >>>
> >>> The settings are board specific, so there must be some way to configure
> >>> them in the devicetree.
> >>
> >> But I'm sure there's a way to control board-specific settings/kludges
> >> from devicetree?
> > 
> > Hm, yes. That's what Richard does, right? I may be misunderstanding you
> > here.
> > 
> > Sascha
> 
> Marc
> 
> 
> -- 
> Pengutronix e.K.  | Marc Kleine-Budde   |
> Industrial Linux Solutions| Phone: +49-231-2826-924 |
> Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 


--
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] USB: cdc-wdm: Extend and improve subdriver interface

[Alexey Orishko]

On Tue, Sep 4, 2012 at 3:45 PM, Bjørn Mork  wrote:

> Yes.  But this time with some hope of multi-vendor support, given that
> Microsoft points to it for Windows 8 Mobile Broadband device support:
> http://msdn.microsoft.com/en-us/library/windows/hardware/mbim-based-mobile-broadband-requirements-for-windows.aspx
It looks like mandatory feature for W8 logo tests, so vendors must adapt to it

> Yes, I expect that major changes to cdc_ncm will be necessary, and
> pointing to it could be wrong from my side? It just seemed natural to
> try to reuse any existing code.

MBIM CID handling should not be done in the driver, but in the user space daemon
or connection manager application. So the need is to add only support
for encapsulated
commands and either use a static amount of network interfaces or create them
dynamically based on CID parser commands to the driver.
It also requires updates for constructing headers, etc, but it's simple change.

>>> CDC MBIM features we are going to support, there is also a possibility
>>> that the main driver will have to intercept messages between the
>>> userspace and the cdc-wdm subdriver.
>>
>> Ouch.
Why do we need cdc-wdm driver? It would be too complex to handle all
required data.
We already handling several control requests, so adding one more won't
be a problem.
And we have to expose some control interface towards user space from
mbim driver.

>
> yes.  Reading the spec terrified me.  Among other issues, it allows
> multiplexing several IP connections over a single set of bulk in/out
> endpoints, where an "IP connection" probably will have to map logically
> to a network device.
Yes, it means we get more than one virtual network interface.

/alexey
--
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] USB: cdc-wdm: Extend and improve subdriver interface

[Oliver Neukum]

On Tuesday 04 September 2012 15:45:36 Bjørn Mork wrote:
> Oliver Neukum  writes:
> 
> > On Tuesday 04 September 2012 11:13:52 Bjørn Mork wrote:
> >
> > Hi,
> >
> >> there is some work going on trying to support CDC MBIM devices in
> >> Linux ( http://www.usb.org/developers/devclass_docs/MBIM10.zip )
> >
> > Yet another protocol.
> 
> Yes.  But this time with some hope of multi-vendor support, given that
> Microsoft points to it for Windows 8 Mobile Broadband device support:
> http://msdn.microsoft.com/en-us/library/windows/hardware/mbim-based-mobile-broadband-requirements-for-windows.aspx

It can't be helped. Support for this will be needed.

> Yes, I expect that major changes to cdc_ncm will be necessary, and
> pointing to it could be wrong from my side? It just seemed natural to
> try to reuse any existing code.

No, that is perfectly all right. It seems to me that the API between usbnet
and its subdrivers is not powerful enough to support CDC-NCM well.

> >> found that the cdc-wdm subdriver interface must be extended somewhat for
> >> this to be possible.  At the very least, the main driver need to handle
> >> some notifications which are ignored by cdc-wdm.  And depending on which
> >
> > Which ones?
> 
> The ones which are already handled by cdc_ncm I believe:
> 
> USB_CDC_NOTIFY_NETWORK_CONNECTION and USB_CDC_NOTIFY_SPEED_CHANGE.
> cdc-wdm will just debug print USB_CDC_NOTIFY_NETWORK_CONNECTION and
> ignore it, but will log an error if it sees USB_CDC_NOTIFY_SPEED_CHANGE

So provide callbacks for them.

> yes.  Reading the spec terrified me.  Among other issues, it allows
> multiplexing several IP connections over a single set of bulk in/out
> endpoints, where an "IP connection" probably will have to map logically
> to a network device.

Yes, but given the capability of UMTS something like this would be needed.

> I believe we can get away with the main driver only looking at messages
> received from the device, without changing or filtering them.

In hindsight, cdc-wdm should have used the tty layer. Then we'd now
write a line discipline.
 
> I see that I should have included the data interception hook, or not
> have mentioned it at all. Sorry for any confusion.

Very well.

Regards
Oliver

--
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 2/3] usb: otg: add device tree support to otg library

[Richard Zhao]

On Tue, Sep 04, 2012 at 03:58:50PM +0200, Marc Kleine-Budde wrote:
> On 09/04/2012 03:45 PM, Felipe Balbi wrote:
> > On Tue, Sep 04, 2012 at 12:31:14PM +0200, Marc Kleine-Budde wrote:
> >> On 08/29/2012 10:11 PM, Marc Kleine-Budde wrote:
> >> [...]
> >>
>  +struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>  +const char *phandle)
> >>
> >>> Since it's already a common function, we may give phandler property
> >>> a common name too. So we will not need phandle argument.
> >>> Please also don't forget to document the devm_xxx and dt binding.
> >>
> >> I don't think this is a good idea. If we hardcode the phandle name, we
> >> introduce a limit of one phy per usb device. The usb3 controllers
> >> alreadyt use two phys (one for usb2, the othere for usb3) for one
> >> controller. So I think we should not make the same mistake again.
>  That only means current binding is not good enough. Rather not, means
>  it should not be in common code.
>  Maybe something like:
>  usbport0-phys = <&phy0>;
>  usbport1-phys = <&phy1 &phy2>; /* usb2 & usb3 */
> >>>
> >>> Granted. Do we need strings that describe the phys, like in pinctrl or
> >>> is a index enough? What about this?
> >>>
> >>> struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
> >>>   int index)
> >>>
> >>
> >> Comments? The phandle_name string will be "usbphy".
> > 
> > I don't think phandle_name should be usbphy. Eventually we want to turn
> > this into a kernel-wide phy subsystem and if we use usbphy, we will just
> > have to patch a bunch of dts files once we make the move.
Coud you please give a link of "kernel-wide phy subsystem" discussion?
> 
> Is just "phy" better?
If the property name don't include port number, how do we know what
port the phy is attached to?

Thanks
Richard
> 
> Marc
> 
> -- 
> Pengutronix e.K.  | Marc Kleine-Budde   |
> Industrial Linux Solutions| Phone: +49-231-2826-924 |
> Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 


--
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 v9 01/13] usb: musb: dsps: add phy control logic to glue

[B, Ravi]

> On Fri, Aug 31, 2012 at 06:51:04PM +0530, ABRAHAM, KISHON VIJAY wrote:
> > Hi,
> > 
> > On Fri, Aug 31, 2012 at 5:53 PM, Felipe Balbi  wrote:
> > > Hi,
> > >
> > > On Fri, Aug 31, 2012 at 04:39:47PM +0530, Ravi Babu wrote:
> > >> From: Santhapuri, Damodar 
> > >>
> > >> AM335x uses NOP transceiver driver and need to enable 
> builtin PHY 
> > >> by writing into usb_ctrl register available in system control 
> > >> module register space. This is being added at musb glue driver 
> > >> layer untill a separate system control module driver is 
> available.
> > >>
> > >> Signed-off-by: Ajay Kumar Gupta 
> > >> Signed-off-by: Santhapuri, Damodar 
> > >> Signed-off-by: Ravi Babu 
> > >
> > > Kishon, you were adding a real phy driver for OMAP's internal phy 
> > > logic on one of your patches and I believe this will 
> conflict with 
> > > your changes, right ?
> > 
> > Indeed. My final patch of that series removes some of the functions 
> > from omap_phy_internal.c (which was taken care in the phy driver).
> > >
> > > How does this look to you ? Is this at least correct ? I 
> suppose the 
> > > correct way would be to actually have the system control module 
> > > driver which we have been waiting, right ?
> > 
> > Correct. I think once we have the system control module driver in 
> > place, we'll have everything wrt control module register writes 
> > implemented in correct way.
> 
> So $SUBJECT will pretty much be thrown away once we have SCM 
> driver, in that case it's best to wait a bit longer and apply 
> this series once SCM driver is available and after your 
> series too... you agree ?
> 

Felipe, I am sure there are patches in this series[0/13], which are not 
dependent on this patch or control module,
Can we pull in those patches (all dual instances support patches)? So that I 
can re-work and submit again? 

> --
> balbi
> --
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 2/3] usb: otg: add device tree support to otg library

[Marc Kleine-Budde]

On 09/04/2012 03:45 PM, Felipe Balbi wrote:
> On Tue, Sep 04, 2012 at 12:31:14PM +0200, Marc Kleine-Budde wrote:
>> On 08/29/2012 10:11 PM, Marc Kleine-Budde wrote:
>> [...]
>>
 +struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
 +const char *phandle)
>>
>>> Since it's already a common function, we may give phandler property
>>> a common name too. So we will not need phandle argument.
>>> Please also don't forget to document the devm_xxx and dt binding.
>>
>> I don't think this is a good idea. If we hardcode the phandle name, we
>> introduce a limit of one phy per usb device. The usb3 controllers
>> alreadyt use two phys (one for usb2, the othere for usb3) for one
>> controller. So I think we should not make the same mistake again.
 That only means current binding is not good enough. Rather not, means
 it should not be in common code.
 Maybe something like:
 usbport0-phys = <&phy0>;
 usbport1-phys = <&phy1 &phy2>; /* usb2 & usb3 */
>>>
>>> Granted. Do we need strings that describe the phys, like in pinctrl or
>>> is a index enough? What about this?
>>>
>>> struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>>> int index)
>>>
>>
>> Comments? The phandle_name string will be "usbphy".
> 
> I don't think phandle_name should be usbphy. Eventually we want to turn
> this into a kernel-wide phy subsystem and if we use usbphy, we will just
> have to patch a bunch of dts files once we make the move.

Is just "phy" better?

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 2/3] usb: otg: add device tree support to otg library

[Felipe Balbi]

On Tue, Sep 04, 2012 at 12:31:14PM +0200, Marc Kleine-Budde wrote:
> On 08/29/2012 10:11 PM, Marc Kleine-Budde wrote:
> [...]
> 
> >> +struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
> >> +const char *phandle)
> 
> > Since it's already a common function, we may give phandler property
> > a common name too. So we will not need phandle argument.
> > Please also don't forget to document the devm_xxx and dt binding.
> 
>  I don't think this is a good idea. If we hardcode the phandle name, we
>  introduce a limit of one phy per usb device. The usb3 controllers
>  alreadyt use two phys (one for usb2, the othere for usb3) for one
>  controller. So I think we should not make the same mistake again.
> >> That only means current binding is not good enough. Rather not, means
> >> it should not be in common code.
> >> Maybe something like:
> >> usbport0-phys = <&phy0>;
> >> usbport1-phys = <&phy1 &phy2>; /* usb2 & usb3 */
> > 
> > Granted. Do we need strings that describe the phys, like in pinctrl or
> > is a index enough? What about this?
> > 
> > struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
> > int index)
> >
> 
> Comments? The phandle_name string will be "usbphy".

I don't think phandle_name should be usbphy. Eventually we want to turn
this into a kernel-wide phy subsystem and if we use usbphy, we will just
have to patch a bunch of dts files once we make the move.

Kishon ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC] USB: cdc-wdm: Extend and improve subdriver interface

[Bjørn Mork]

Oliver Neukum  writes:

> On Tuesday 04 September 2012 11:13:52 Bjørn Mork wrote:
>
> Hi,
>
>> there is some work going on trying to support CDC MBIM devices in
>> Linux ( http://www.usb.org/developers/devclass_docs/MBIM10.zip )
>
> Yet another protocol.

Yes.  But this time with some hope of multi-vendor support, given that
Microsoft points to it for Windows 8 Mobile Broadband device support:
http://msdn.microsoft.com/en-us/library/windows/hardware/mbim-based-mobile-broadband-requirements-for-windows.aspx


>> The protocol is based on CDC NCM with a rather complex control protocol
>> ecapsulated in CDC using SendEncapsulatedCommand and
>> GetEncapsulatedResponse, similar to how QMI/wwan devices combine CDC ECM
>> with the Qualcomm QMI control protocol.
>> 
>> Based on these similarities, I proposed to reuse as much as possible of
>
> I am afraid cdc_ncm is not very clean at present. The bundling of network
> packets into larger units should be done so that usbnet is aware of what
> is happening. In particular starting a timer inside a subdriver is very dirty.

Yes, I expect that major changes to cdc_ncm will be necessary, and
pointing to it could be wrong from my side? It just seemed natural to
try to reuse any existing code.

>> the cdc-ncm and cdc-wdm drivers, creating a construct similar to that
>> provided by qmi_wwan: Exporting the control protocol as a /dev/cdc-wdmX
>> character device to some userspace application doing all the complex
>> policy decisions. Greg Suarez has started out on this track, but has
>
> Is code available to be seen?

Dunno how far Greg is on that.  I haven't seen it yet, in any case.

>> found that the cdc-wdm subdriver interface must be extended somewhat for
>> this to be possible.  At the very least, the main driver need to handle
>> some notifications which are ignored by cdc-wdm.  And depending on which
>
> Which ones?

The ones which are already handled by cdc_ncm I believe:

USB_CDC_NOTIFY_NETWORK_CONNECTION and USB_CDC_NOTIFY_SPEED_CHANGE.
cdc-wdm will just debug print USB_CDC_NOTIFY_NETWORK_CONNECTION and
ignore it, but will log an error if it sees USB_CDC_NOTIFY_SPEED_CHANGE

(This point should in theory apply to qmi_wwan as well, but I have not
yet seen a QMI device sending either of these notifications so the
theoretical problem is currently ignored)


>> CDC MBIM features we are going to support, there is also a possibility
>> that the main driver will have to intercept messages between the
>> userspace and the cdc-wdm subdriver.
>
> Ouch.

yes.  Reading the spec terrified me.  Among other issues, it allows
multiplexing several IP connections over a single set of bulk in/out
endpoints, where an "IP connection" probably will have to map logically
to a network device.

>> The following patch is meant as a RFC for a basic interface change,
>> adding the flexibility the new driver will need.  I would very much
>> welcome comments on
>>  - the whole concept of doing the same for MBIM as for QMI
>
> Sensible.
>
>>  - whether data interception may be acceptable at all
>
> in one direction yes, the other is very problematic

I believe we can get away with the main driver only looking at messages
received from the device, without changing or filtering them.

>>  - the proposed symbol change, including restricting it to GPL
>>drivers
>
> The changes are mainly sensible. Simply changing the license is not nice.
> It seems to me, once something has been allowed, it should stay allowed.
>
>>  - anything else that pops up in your head while reading this :-)
>> 
>> The RFC patch also includes the oneline change for qmi_wwan to use the
>> new API. If this approach is acceptable, I would like to push these
>> changes through net-next to reduce the cross-tree dependencies.
>
> This is the brute force approach:
>
> @@ -224,6 +224,11 @@ static void wdm_int_callback(struct urb *urb)
>   goto exit;
>   }
>  
> + /* check for callback function */
> + if (desc->driver_info->status &&
> + desc->driver_info->status(desc->intf, urb))
> + goto exit;
> +
>
> And it breaks the status model. This particular approach is not good.
> Even with MBIM you want the driver to retrieve the responses. So the
> correct approach would be to intercept the responses to 
> GET_ENCAPSULATED_RESPONSE
> and filter them, more or less like a line discipline.

I expected the desc->driver_info->status() to return true only for the
notifications it handled, which cannot include
USB_CDC_NOTIFY_RESPONSE_AVAILABLE as you note.

I am not sure I understand how else you would filter the notifications?

Note: This was not meant for the data interception I mentioned above.
That was just a pre-warning for things chich may be required.  Greg did
include it in an early patch, but I made him drop it for now as I wasn't
convinced it was needed.  Reading the MBIM standard made me more
unsure.  You are of course right that we want cdc_wdm to *always* deal
with USB_CDC_NOTIFY_RESPONSE

Re: create libcomposite, v3

[Felipe Balbi]

Hi,

On Tue, Sep 04, 2012 at 03:05:51PM +0200, Marek Szyprowski wrote:
> Hi Felibe,
> 
> On Friday, August 31, 2012 2:21 PM Felipe Balbi wrote:
> 
> > On Fri, Aug 31, 2012 at 01:39:21PM +0200, Andrzej Pietrasiewicz wrote:
> > > On Friday, August 31, 2012 12:12 PM Felipe Balbi wrote:
> > >
> > > 
> > >
> > > > Unless it can be proven that ccg has active users, we should get rid of 
> > > > it
> > > > for now before someone starts using it and we need to maintain the ABI 
> > > > for
> > > > the next 10 years.
> > >
> > > We need a configurable composite gadget in the Tizen platform to
> > > provide sdbd connectivity and mass storage function at the same time.
> > 
> > You don't need a "configurable composite gadget" if you always have
> > storage and sdbd (btw, what's sdbd ? is that provided by ccg ? I don't
> > think so). A static (temporarily out-of-tree) tizen gadget would
> > suffice, once we move to configfs Tizen can start using that.
> > 
> > IMHO, that'll be a much lower impact on Tizen's platform. I'm also
> > concerned that after you start using it and we try to change, it'll just
> > be an argument to keep CCG as is, since it is an interface to userland
> > after all, and I don't want to maintain ccg for the next 10 years as
> > it's clearly wrong approach.
> 
> CCG (as well as Android gadget) provides functionality that was not provided
> by any of the gadgets before. As You have noticed once, all existing gadget
> had static configuration and the only possibility to change configuration 
> (implied as a set of available usb functions) is to create, compile and load
> yet another gadget module. Such feature is really needed for our Linux 
> platform to replace developped-off-the-kernel old android gadget.
> 
> We were pleased when we saw that there was a will to develop an android-like
> gadget for mainline kernel and the patches posted to ml late in December 2011.
> Then, because of lack of progress we decided to continue those works on our
> own. We managed to resolve most of the issues in the Android gadget and
> extended it with functionfs support, which was crucial for implementing all
> functions that are needed for our platform (a replacement for adb-shell called
> sbsd and mtp protocol are implemented by respective daemons in userland). Our
> CCG got finally merged to staging.
> 
> The only missing piece was conversion to configfs. When we were posting ccg to
> staging tree, we thought that this can be done incrementally. In meantime we
> had something what was fully functional and demonstrated that it can fully 
> replace old Android gadget with some simple changes to userspace tools.
> 
> The conversion to configfs tuned out to be a complete rewrite from scratch of 
> the whole usb gadget subsystem, what is a huge task. Andrzej continues his 
> works in this area, but I don't expect it to be ready for v3.7 or even v3.8.
> In meanwhile CCG serves as a working solution for us. It lives in staging,
> so noone should expect that it's interface is stable, so I don't see any
> problem in replacing it with configfs driver solution in the future.

The problem is that ccg is completely wrong implementation WRT moving to
configfs. ccg decided that it was so special that it could overwrite
composite.c's setup function.

The whole idea behind configfs was to drop the need for mass_storage.c,
nokia.c, cdc2.c, etc, but ccg decided that it would create yet another
$whatever.c file for combining the functions into a gadget.

On top of all that, ccg is making it really difficult for Sebastian to
continue his big, highly needed, cleanup of the gadget framework and ccg
isn't even "part of the main kernel" (notice the quotation marks) since
it lies in staging tree. If you think you really need ccg, then we are
very happy to take patches from you fixing it and cleaning it up, but we
cannot delay the whole configfs part because ccg decided to be special.

I would still recommend dropping ccg so we can move faster with configfs
migration, though, as that would be something which should work for
Tizen, Android and anybody else.

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC] USB: cdc-wdm: Extend and improve subdriver interface

[Oliver Neukum]

On Tuesday 04 September 2012 11:13:52 Bjørn Mork wrote:

Hi,

> there is some work going on trying to support CDC MBIM devices in
> Linux ( http://www.usb.org/developers/devclass_docs/MBIM10.zip )

Yet another protocol.

> The protocol is based on CDC NCM with a rather complex control protocol
> ecapsulated in CDC using SendEncapsulatedCommand and
> GetEncapsulatedResponse, similar to how QMI/wwan devices combine CDC ECM
> with the Qualcomm QMI control protocol.
> 
> Based on these similarities, I proposed to reuse as much as possible of

I am afraid cdc_ncm is not very clean at present. The bundling of network
packets into larger units should be done so that usbnet is aware of what
is happening. In particular starting a timer inside a subdriver is very dirty.

> the cdc-ncm and cdc-wdm drivers, creating a construct similar to that
> provided by qmi_wwan: Exporting the control protocol as a /dev/cdc-wdmX
> character device to some userspace application doing all the complex
> policy decisions. Greg Suarez has started out on this track, but has

Is code available to be seen?

> found that the cdc-wdm subdriver interface must be extended somewhat for
> this to be possible.  At the very least, the main driver need to handle
> some notifications which are ignored by cdc-wdm.  And depending on which

Which ones?

> CDC MBIM features we are going to support, there is also a possibility
> that the main driver will have to intercept messages between the
> userspace and the cdc-wdm subdriver.

Ouch.
 
> The following patch is meant as a RFC for a basic interface change,
> adding the flexibility the new driver will need.  I would very much
> welcome comments on
>  - the whole concept of doing the same for MBIM as for QMI

Sensible.

>  - whether data interception may be acceptable at all

in one direction yes, the other is very problematic

>  - the proposed symbol change, including restricting it to GPL
>drivers

The changes are mainly sensible. Simply changing the license is not nice.
It seems to me, once something has been allowed, it should stay allowed.

>  - anything else that pops up in your head while reading this :-)
> 
> The RFC patch also includes the oneline change for qmi_wwan to use the
> new API. If this approach is acceptable, I would like to push these
> changes through net-next to reduce the cross-tree dependencies.

This is the brute force approach:

@@ -224,6 +224,11 @@ static void wdm_int_callback(struct urb *urb)
goto exit;
}
 
+   /* check for callback function */
+   if (desc->driver_info->status &&
+   desc->driver_info->status(desc->intf, urb))
+   goto exit;
+

And it breaks the status model. This particular approach is not good.
Even with MBIM you want the driver to retrieve the responses. So the
correct approach would be to intercept the responses to 
GET_ENCAPSULATED_RESPONSE
and filter them, more or less like a line discipline.

Regards
Oliver

--
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: create libcomposite, v3

[Marek Szyprowski]

Hi Felibe,

On Friday, August 31, 2012 2:21 PM Felipe Balbi wrote:

> On Fri, Aug 31, 2012 at 01:39:21PM +0200, Andrzej Pietrasiewicz wrote:
> > On Friday, August 31, 2012 12:12 PM Felipe Balbi wrote:
> >
> > 
> >
> > > Unless it can be proven that ccg has active users, we should get rid of it
> > > for now before someone starts using it and we need to maintain the ABI for
> > > the next 10 years.
> >
> > We need a configurable composite gadget in the Tizen platform to
> > provide sdbd connectivity and mass storage function at the same time.
> 
> You don't need a "configurable composite gadget" if you always have
> storage and sdbd (btw, what's sdbd ? is that provided by ccg ? I don't
> think so). A static (temporarily out-of-tree) tizen gadget would
> suffice, once we move to configfs Tizen can start using that.
> 
> IMHO, that'll be a much lower impact on Tizen's platform. I'm also
> concerned that after you start using it and we try to change, it'll just
> be an argument to keep CCG as is, since it is an interface to userland
> after all, and I don't want to maintain ccg for the next 10 years as
> it's clearly wrong approach.

CCG (as well as Android gadget) provides functionality that was not provided
by any of the gadgets before. As You have noticed once, all existing gadget
had static configuration and the only possibility to change configuration 
(implied as a set of available usb functions) is to create, compile and load
yet another gadget module. Such feature is really needed for our Linux 
platform to replace developped-off-the-kernel old android gadget.

We were pleased when we saw that there was a will to develop an android-like
gadget for mainline kernel and the patches posted to ml late in December 2011.
Then, because of lack of progress we decided to continue those works on our
own. We managed to resolve most of the issues in the Android gadget and
extended it with functionfs support, which was crucial for implementing all
functions that are needed for our platform (a replacement for adb-shell called
sbsd and mtp protocol are implemented by respective daemons in userland). Our
CCG got finally merged to staging.

The only missing piece was conversion to configfs. When we were posting ccg to
staging tree, we thought that this can be done incrementally. In meantime we
had something what was fully functional and demonstrated that it can fully 
replace old Android gadget with some simple changes to userspace tools.

The conversion to configfs tuned out to be a complete rewrite from scratch of 
the whole usb gadget subsystem, what is a huge task. Andrzej continues his 
works in this area, but I don't expect it to be ready for v3.7 or even v3.8.
In meanwhile CCG serves as a working solution for us. It lives in staging,
so noone should expect that it's interface is stable, so I don't see any
problem in replacing it with configfs driver solution in the future.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/5] usb: chipidea: fixes

[Felipe Balbi]

Hi,

On Tue, Sep 04, 2012 at 12:39:23PM +0200, Marc Kleine-Budde wrote:
> On 08/28/2012 03:57 PM, Marc Kleine-Budde wrote:
> > Hello,
> > 
> > this is a series fixes several problems in the chipidea udc driver. this
> > applies to v3.6-rc3.
> > 
> > These patches have been tested on mx28, mx53 and mx6q.
> > 
> > Changes since v1:
> > - reworded patch description for "fix setup of endpoint maxpacket size"
> > - added Acked-by and Reviewed-by
> > - added "cleanup dma_pool if udc_start() fails" patch
> > 
> > please review and test,
> > regards, Marc
> 
> Comments? Anyone?

take a look at the MAINTAINERS file every now and then. chipidea is
Alex's area ;-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 0/5] usb: chipidea: fixes

[Marc Kleine-Budde]

On 08/28/2012 03:57 PM, Marc Kleine-Budde wrote:
> Hello,
> 
> this is a series fixes several problems in the chipidea udc driver. this
> applies to v3.6-rc3.
> 
> These patches have been tested on mx28, mx53 and mx6q.
> 
> Changes since v1:
> - reworded patch description for "fix setup of endpoint maxpacket size"
> - added Acked-by and Reviewed-by
> - added "cleanup dma_pool if udc_start() fails" patch
> 
> please review and test,
> regards, Marc

Comments? Anyone?

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Bjørn Mork]

Ming Lei  writes:
> On Tue, Sep 4, 2012 at 3:49 PM, Bjørn Mork  wrote:
>>
>> That's easy:
>>
>> - Take any usbnet device supporting remote wakeup (and of course with a
>>   minidriver supporting it as well),
>> - enable autosuspend,
>> - ip link set dev ethX/usbX/wwanX up
>>
>> And watch ksoftirqd/X use 100% of one of your CPU cores.
>>
>> I'd very much like to hear the details if you are unable to reproduce
>> the bug using this simple test.
>
> I have no such kind of devices, and can't reproduce the problem with
> a cdc-ether gadget built by one omap3 beagleboard.

I believe any Ericsson or Gobi modem would do, and most likely other USB
networking devices too.

I haven't explored the gadget.  Doesn't it support remote wakeup?  Well,
it doesn't really have to just for testing this.  You just have to fake
the remote wakeup support, either in the gadget or in the device
driver.  It doesn't matter whether it works or not.  The point is making
the driver suspend the USB device while the network device is running.

>>> Once usbnet_terminate_urbs and netif_device_attach() are
>>> completed, who will schedule tasklet to trigger rx_alloc_submit?
>>
>> That's a good question.  It sure would be nice if that never happened
>> without waking the device first.  But there are just too many call sites
>> for me to follow:
>
> To be honest, your patch is correct, and I am just curious about
> the reason why tasklet is scheduled after the device is suspended.
>
>>
>> bjorn@nemi:/usr/local/src/git/linux$ grep tasklet_schedule 
>> drivers/net/usb/usbnet.c
>> tasklet_schedule(&dev->bh);
>>  * but tasklet_schedule() doesn't.  hope the failure is rare.
>> tasklet_schedule (&dev->bh);
>> tasklet_schedule(&dev->bh);
>> tasklet_schedule(&dev->bh);
>> tasklet_schedule (&dev->bh);
>> tasklet_schedule (&dev->bh);
>> tasklet_schedule (&dev->bh);
>> tasklet_schedule (&dev->bh);
>> tasklet_schedule (&dev->bh);
>> tasklet_schedule (&dev->bh);
>
> Could you add some debug info inside the caller to see which one
> cause the tasklet to be scheduled?


Goode idea.  I replaced the tasklet_schedule() calls with

#define DBG_TASKLET(bh) { \
 pr_err("%s scheduling tasklet %p\n", __func__, bh); \
 tasklet_schedule(bh); \
}

and added a useless WARN_ON around the test:

if (temp < RX_QLEN(dev)) {
if (WARN_ON_ONCE(rx_alloc_submit(dev, GFP_ATOMIC) == 
-ENOLINK))
return;


and got:

Sep  4 12:22:48 nemi kernel: [50270.964818] qmi_wwan 2-4:1.8: wdm1_resume
Sep  4 12:22:48 nemi kernel: [50270.964832] qmi_wwan 2-4:1.19: wdm2_resume
Sep  4 12:22:48 nemi kernel: [50270.964845] usbnet_open scheduling tasklet 
88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.964857] qmi_wwan 2-4:1.8: 
qmi_wwan_manage_power() pmcount=0, on=1
Sep  4 12:22:48 nemi kernel: [50270.964921] qmi_wwan 2-4:1.8: wwan1: rxqlen 0 
--> 10
Sep  4 12:22:48 nemi kernel: [50270.964926] usbnet_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.964962] qmi_wwan 2-4:1.8: wwan1: rxqlen 10 
--> 20
Sep  4 12:22:48 nemi kernel: [50270.964966] usbnet_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.965016] qmi_wwan 2-4:1.8: wwan1: rxqlen 20 
--> 30
Sep  4 12:22:48 nemi kernel: [50270.965020] usbnet_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.965057] qmi_wwan 2-4:1.8: wwan1: rxqlen 30 
--> 40
Sep  4 12:22:48 nemi kernel: [50270.965061] usbnet_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.965097] qmi_wwan 2-4:1.8: wwan1: rxqlen 40 
--> 50
Sep  4 12:22:48 nemi kernel: [50270.965101] usbnet_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.965136] qmi_wwan 2-4:1.8: wwan1: rxqlen 50 
--> 60
Sep  4 12:22:48 nemi kernel: [50270.972270] defer_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:48 nemi kernel: [50271.040290] defer_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:48 nemi kernel: [50271.240286] defer_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:49 nemi kernel: [50272.040536] defer_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:51 nemi kernel: [50274.040121] qmi_wwan 2-4:1.19: wdm2_suspend
Sep  4 12:22:51 nemi kernel: [50274.040234] qmi_wwan 2-4:1.8: wdm1_suspend
Sep  4 12:22:51 nemi kernel: [50274.040263] defer_bh scheduling tasklet 
88014a8529d0
Sep  4 12:22:51 nemi kernel: [50274.040599] [ cut here ]
Sep  4 12:22:51 nemi kernel: [50274.040612] WARNING: at 
/usr/local/src/git/linux/drivers/net/usb/usbnet.c:1271 
tasklet_action+0x76/0xc4()
Sep  4 12:22:51 nemi kernel: [50274.040617] Hardware name: 2776LEG
Sep  4 12:22:51 nemi kernel: [50274.040620] Modules linked in: qmi_wwan(O) 
usbnet(O) nfnetlink_log nfnetlink option nls_utf8 isofs usb_storage

Re: [PATCH 2/3] usb: otg: add device tree support to otg library

[Marc Kleine-Budde]

On 08/29/2012 10:11 PM, Marc Kleine-Budde wrote:
[...]

>> +struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>> +const char *phandle)

> Since it's already a common function, we may give phandler property
> a common name too. So we will not need phandle argument.
> Please also don't forget to document the devm_xxx and dt binding.

 I don't think this is a good idea. If we hardcode the phandle name, we
 introduce a limit of one phy per usb device. The usb3 controllers
 alreadyt use two phys (one for usb2, the othere for usb3) for one
 controller. So I think we should not make the same mistake again.
>> That only means current binding is not good enough. Rather not, means
>> it should not be in common code.
>> Maybe something like:
>> usbport0-phys = <&phy0>;
>> usbport1-phys = <&phy1 &phy2>; /* usb2 & usb3 */
> 
> Granted. Do we need strings that describe the phys, like in pinctrl or
> is a index enough? What about this?
> 
> struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>   int index)
>

Comments? The phandle_name string will be "usbphy".

>> +{
>> +struct usb_phy  *phy = NULL, **ptr;
>> +unsigned long   flags;
>> +struct device_node *node;
>> +
>> +if (!dev->of_node) {
>> +dev_dbg(dev, "device does not have a device node entry\n");
>> +return ERR_PTR(-EINVAL);
>> +}
>> +
>> +node = of_parse_phandle(dev->of_node, phandle, 0);
>> +if (!node) {
>> +dev_dbg(dev, "failed to get %s phandle in %s node\n", 
>> phandle,
>> +dev->of_node->full_name);
>> +return ERR_PTR(-ENODEV);
>> +}
>> +
>> +ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
>> +if (!ptr) {
>> +dev_dbg(dev, "failed to allocate memory for devres\n");
>> +return ERR_PTR(-ENOMEM);
>> +}
>> +
>> +spin_lock_irqsave(&phy_lock, flags);
>> +
>> +phy = __of_usb_find_phy(&phy_list, node);
>> +if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>> +phy = ERR_PTR(-EPROBE_DEFER);
>> +devres_free(ptr);
>> +goto err0;
>> +}
>> +
>> +*ptr = phy;
>> +devres_add(dev, ptr);
>> +
>> +get_device(phy->dev);
>> +
>> +err0:
>> +spin_unlock_irqrestore(&phy_lock, flags);
>> +
>> +return phy;
>> +}
>> +EXPORT_SYMBOL(devm_usb_get_phy_by_phandle);
>> +
>> +/**
>>   * devm_usb_put_phy - release the USB PHY
>>   * @dev - device that wants to release this phy
>>   * @phy - the phy returned by devm_usb_get_phy()
>> @@ -158,32 +234,24 @@ EXPORT_SYMBOL(usb_put_phy);
>>   */
>>  int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
>>  {
>> -int ret = 0;
>>  unsigned long   flags;
>>  struct usb_phy  *phy;
>>
>> -if (x && x->type != USB_PHY_TYPE_UNDEFINED) {
>> -dev_err(x->dev, "not accepting initialized PHY %s\n", 
>> x->label);
>> -return -EINVAL;
>>>
>>> how about having
>>> if (x && !x->dev.of_node && x->type != USB_PHY_TYPE_UNDEFINED) {
>>> dev_err(x->dev, "not accepting initialized PHY %s\n", x->label);
>>> return -EINVAL;
>>> }
>>>
>>> By using this we'll return error if the phy device does not have an
>>> of_node. (So it can't get back the phy by phandle).
>> Maybe it worth to create a new set functions. When using DT, the way to
>> add and get phy is totally different.
> 
> Getting already will be (get_by_phandle). Do we need a seperate List for
> DT and non DT phys? usb_add_of_phy()? usb_add_dt_phy()?

comments?

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Ming Lei]

On Tue, Sep 4, 2012 at 3:49 PM, Bjørn Mork  wrote:
>
> That's easy:
>
> - Take any usbnet device supporting remote wakeup (and of course with a
>   minidriver supporting it as well),
> - enable autosuspend,
> - ip link set dev ethX/usbX/wwanX up
>
> And watch ksoftirqd/X use 100% of one of your CPU cores.
>
> I'd very much like to hear the details if you are unable to reproduce
> the bug using this simple test.

I have no such kind of devices, and can't reproduce the problem with
a cdc-ether gadget built by one omap3 beagleboard.

>> Once usbnet_terminate_urbs and netif_device_attach() are
>> completed, who will schedule tasklet to trigger rx_alloc_submit?
>
> That's a good question.  It sure would be nice if that never happened
> without waking the device first.  But there are just too many call sites
> for me to follow:

To be honest, your patch is correct, and I am just curious about
the reason why tasklet is scheduled after the device is suspended.

>
> bjorn@nemi:/usr/local/src/git/linux$ grep tasklet_schedule 
> drivers/net/usb/usbnet.c
> tasklet_schedule(&dev->bh);
>  * but tasklet_schedule() doesn't.  hope the failure is rare.
> tasklet_schedule (&dev->bh);
> tasklet_schedule(&dev->bh);
> tasklet_schedule(&dev->bh);
> tasklet_schedule (&dev->bh);
> tasklet_schedule (&dev->bh);
> tasklet_schedule (&dev->bh);
> tasklet_schedule (&dev->bh);
> tasklet_schedule (&dev->bh);
> tasklet_schedule (&dev->bh);

Could you add some debug info inside the caller to see which one
cause the tasklet to be scheduled?


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: surfstick K5006-Z in 3.6.0-rc4

[Bjørn Mork]

Thomas Schäfer  writes:

> Despite the patch
>
> { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x1018, 0xff, 0xff, 
> 0xff),
>   .driver_info = (kernel_ulong_t)&net_intf3_blacklist },
>
> is included in option, it has no effect.

I quickly looked through the info you have provided on this device, but
cannot seem to find anything showing the class/subclass/protocol of
these interfaces.  I just assumed they were all 0xff, 0xff, 0xff like
other ZTE devices.  Guess that was wrong...

Could you post the output of lsusb -v or /sys/kernel/debug/usb/devices
(with debugfs mounted) for this device?


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 v2] usb: otg: Move phy interface to separate file.

[Venu Byravarasu]

Hi Felipe,

On Monday 03 September 2012 07:43 PM, Felipe Balbi wrote:

* PGP Signed by an unknown key

Hi,

On Wed, Aug 29, 2012 at 12:17:01PM +0530, Venu Byravarasu wrote:

As otg.h is containing lots of phy interface related
stuff, moving all phy interface related stuff to new
file named phy.h

Signed-off-by: Venu Byravarasu 

this doesn't apply to my "xceiv" branch. Please rebase there so I don't
cause any issues to your patch ;-)


Rebased the change in your tree and pushed a patch.
Plz merge.

Thanks,
Venu
--
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

[RFC] USB: cdc-wdm: Extend and improve subdriver interface

[Bjørn Mork]

Hello,

there is some work going on trying to support CDC MBIM devices in
Linux ( http://www.usb.org/developers/devclass_docs/MBIM10.zip )

The protocol is based on CDC NCM with a rather complex control protocol
ecapsulated in CDC using SendEncapsulatedCommand and
GetEncapsulatedResponse, similar to how QMI/wwan devices combine CDC ECM
with the Qualcomm QMI control protocol.

Based on these similarities, I proposed to reuse as much as possible of
the cdc-ncm and cdc-wdm drivers, creating a construct similar to that
provided by qmi_wwan: Exporting the control protocol as a /dev/cdc-wdmX
character device to some userspace application doing all the complex
policy decisions. Greg Suarez has started out on this track, but has
found that the cdc-wdm subdriver interface must be extended somewhat for
this to be possible.  At the very least, the main driver need to handle
some notifications which are ignored by cdc-wdm.  And depending on which
CDC MBIM features we are going to support, there is also a possibility
that the main driver will have to intercept messages between the
userspace and the cdc-wdm subdriver.

The following patch is meant as a RFC for a basic interface change,
adding the flexibility the new driver will need.  I would very much
welcome comments on
 - the whole concept of doing the same for MBIM as for QMI
 - whether data interception may be acceptable at all
 - the proposed symbol change, including restricting it to GPL
   drivers
 - anything else that pops up in your head while reading this :-)

The RFC patch also includes the oneline change for qmi_wwan to use the
new API. If this approach is acceptable, I would like to push these
changes through net-next to reduce the cross-tree dependencies.


>From fda3389d16e94ac358aff9f7af72b14320a3539d Mon Sep 17 00:00:00 2001
From: Greg Suarez 
Date: Thu, 16 Aug 2012 10:49:03 +0200
Subject: [RFC] USB: cdc-wdm: Extend and improve subdriver interface
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

New users of the subdriver interface need to
intercept the USB_CDC_NOTIFY_NETWORK_CONNECTION and
USB_CDC_NOTIFY_SPEED_CHANGE events.  Adding a status
callback to enable this.

There are use cases where data interception is required
as well, and we anticipate callbacks enabling this may be
added at a later stage.  Wrapping the subdriver callbacks
in a struct to reduce the number of arguments to the
register function and to enable such future extentions
without having to update all existing users.

The old subdriver interface provided no more than a full
cdc-wdm driver with few possibilities for the registering
driver to modify or inspect the data transmitted between
the device and the subdriver. Exporting this interface to
non GPL users made sense as it provided no more access to
the driver internals than any device with CDC WDM
descriptors would have.  The new interface is more
powerful, allowing user specific handling of some events,
and it may even be extended with data interception
callbacks. This makes the interface unsuitable for non
GPL users.  Tightening the symbol license now, before
there are any such users, prevents future problems.

Updating the only in-kernel user of this interface,
qmi_wwan, at the same time to make this a one step
transition.

Cc: Oliver Neukum 
Signed-off-by: Greg Suarez 
[bj...@mork.no: code style, qmi_wwan, and symbol license]
Signed-off-by: Bjørn Mork 
---
 drivers/net/usb/qmi_wwan.c  |6 -
 drivers/usb/class/cdc-wdm.c |   57 ++-
 include/linux/usb/cdc-wdm.h |9 ++-
 3 files changed, 47 insertions(+), 25 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 328397c..d9586b3 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -82,6 +82,10 @@ static int qmi_wwan_cdc_wdm_manage_power(struct usb_interface *intf, int on)
 	return qmi_wwan_manage_power(dev, on);
 }
 
+static const struct wdm_driver_info wdm_drv_info = {
+	.manage_power = &qmi_wwan_cdc_wdm_manage_power,
+};
+
 /* collect all three endpoints and register subdriver */
 static int qmi_wwan_register_subdriver(struct usbnet *dev)
 {
@@ -108,7 +112,7 @@ static int qmi_wwan_register_subdriver(struct usbnet *dev)
 	atomic_set(&info->pmcount, 0);
 
 	/* register subdriver */
-	subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc, 512, &qmi_wwan_cdc_wdm_manage_power);
+	subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc, 512, &wdm_drv_info);
 	if (IS_ERR(subdriver)) {
 		dev_err(&info->control->dev, "subdriver registration failed\n");
 		rv = PTR_ERR(subdriver);
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 65a55ab..627574b 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -101,7 +101,7 @@ struct wdm_device {
 	int			rerr;
 
 	struct list_head	device_list;
-	int			(*manage_power)(struct usb_interface *, int);
+	const struct wdm_driver_in

[PATCH] usb: otg: Move phy interface to separate file.

[Venu Byravarasu]

As otg.h is containing lots of phy interface related
stuff, moving all phy interface related stuff to new
file named phy.h

Signed-off-by: Venu Byravarasu 
---
 include/linux/usb/otg.h |  198 +
 include/linux/usb/phy.h |  208 +++
 2 files changed, 209 insertions(+), 197 deletions(-)
 create mode 100644 include/linux/usb/phy.h

diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
index 16dc520..65a2b6a 100644
--- a/include/linux/usb/otg.h
+++ b/include/linux/usb/otg.h
@@ -9,7 +9,7 @@
 #ifndef __LINUX_USB_OTG_H
 #define __LINUX_USB_OTG_H
 
-#include 
+#include 
 
 /* OTG defines lots of enumeration states before device reset */
 enum usb_otg_state {
@@ -35,31 +35,6 @@ enum usb_otg_state {
OTG_STATE_A_VBUS_ERR,
 };
 
-enum usb_phy_events {
-   USB_EVENT_NONE, /* no events or cable disconnected */
-   USB_EVENT_VBUS, /* vbus valid event */
-   USB_EVENT_ID,   /* id was grounded */
-   USB_EVENT_CHARGER,  /* usb dedicated charger */
-   USB_EVENT_ENUMERATED,   /* gadget driver enumerated */
-};
-
-/* associate a type with PHY */
-enum usb_phy_type {
-   USB_PHY_TYPE_UNDEFINED,
-   USB_PHY_TYPE_USB2,
-   USB_PHY_TYPE_USB3,
-};
-
-struct usb_phy;
-
-/* for transceivers connected thru an ULPI interface, the user must
- * provide access ops
- */
-struct usb_phy_io_ops {
-   int (*read)(struct usb_phy *x, u32 reg);
-   int (*write)(struct usb_phy *x, u32 val, u32 reg);
-};
-
 struct usb_otg {
u8  default_a;
 
@@ -85,120 +60,9 @@ struct usb_otg {
 
 };
 
-/*
- * the otg driver needs to interact with both device side and host side
- * usb controllers.  it decides which controller is active at a given
- * moment, using the transceiver, ID signal, HNP and sometimes static
- * configuration information (including "board isn't wired for otg").
- */
-struct usb_phy {
-   struct device   *dev;
-   const char  *label;
-   unsigned int flags;
-
-   enum usb_phy_type   type;
-   enum usb_otg_state  state;
-   enum usb_phy_events last_event;
-
-   struct usb_otg  *otg;
-
-   struct device   *io_dev;
-   struct usb_phy_io_ops   *io_ops;
-   void __iomem*io_priv;
-
-   /* for notification of usb_phy_events */
-   struct atomic_notifier_head notifier;
-
-   /* to pass extra port status to the root hub */
-   u16 port_status;
-   u16 port_change;
-
-   /* to support controllers that have multiple transceivers */
-   struct list_headhead;
-
-   /* initialize/shutdown the OTG controller */
-   int (*init)(struct usb_phy *x);
-   void(*shutdown)(struct usb_phy *x);
-
-   /* effective for B devices, ignored for A-peripheral */
-   int (*set_power)(struct usb_phy *x,
-   unsigned mA);
-
-   /* for non-OTG B devices: set transceiver into suspend mode */
-   int (*set_suspend)(struct usb_phy *x,
-   int suspend);
-
-   /* notify phy connect status change */
-   int (*notify_connect)(struct usb_phy *x, int port);
-   int (*notify_disconnect)(struct usb_phy *x, int port);
-};
-
-
-/* for board-specific init logic */
-extern int usb_add_phy(struct usb_phy *, enum usb_phy_type type);
-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)
-   return x->io_ops->read(x, reg);
-
-   return -EINVAL;
-}
-
-static inline int usb_phy_io_write(struct usb_phy *x, u32 val, u32 reg)
-{
-   if (x->io_ops && x->io_ops->write)
-   return x->io_ops->write(x, val, reg);
-
-   return -EINVAL;
-}
-
-static inline int
-usb_phy_init(struct usb_phy *x)
-{
-   if (x->init)
-   return x->init(x);
-
-   return 0;
-}
-
-static inline void
-usb_phy_shutdown(struct usb_phy *x)
-{
-   if (x->shutdown)
-   x->shutdown(x);
-}
-
-/* for usb host and peripheral controller drivers */
 #ifdef CONFIG_USB_OTG_UTILS
-extern struct usb_phy *usb_get_phy(enum usb_phy_type type);
-extern struct usb_phy *devm_usb_get_phy(struct device *dev,
-   enum usb_phy_type type);
-extern void usb_put_phy(struct usb_phy *);
-extern void devm_usb_put_phy(struct device *dev, struct usb_phy *x);
 extern const char *otg_state_string(enum usb_otg_state state);
 #else
-static inline struct usb_phy *usb_get_phy(enum usb_phy_type type)
-{
-   return NULL;
-}
-
-static inline struct usb_phy *devm_usb_get_phy(struct device *dev,
-   enum usb_phy_type type)
-{
-   return NULL;
-}
-
-static inline void usb_put_phy(struct usb_phy *x)
-{
-}
-
-static inline vo

Re: [PATCH] usb: phy: add R-Car R8A7779 USB phy driver.

[ABRAHAM, KISHON VIJAY]

Hi,

On Tue, Sep 4, 2012 at 10:13 AM, Kuninori Morimoto
 wrote:
> This patch adds Renesas R-Car USB phy driver.
> It supports R8A7779 chip at this point.
>
> Signed-off-by: Kuninori Morimoto 
> ---
>  drivers/usb/phy/Kconfig|   11 
>  drivers/usb/phy/Makefile   |1 +
>  drivers/usb/phy/rcar-phy.c |  137 
> 
>  3 files changed, 149 insertions(+)
>  create mode 100644 drivers/usb/phy/rcar-phy.c
>
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 2838adb..f55b6f6 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -23,3 +23,14 @@ config MV_U3D_PHY
> help
>   Enable this to support Marvell USB 3.0 phy controller for Marvell
>   SoC.
> +
> +config USB_RCAR_PHY
> +   tristate "Renesas R-Car USB phy support"
> +   depends on (USB || USB_GADGET) && ARCH_R8A7779
> +   help
> + Say Y here to add support for the Renesas R-Car USB phy driver.

Just out of curiosity, which USB contoller will/is using this PHY?

> + This chip is typically used as USB phy for USB host, gadget.
> + This driver supports: R8A7779
> +
> + To compile this driver as a module, choose M here: the
> + module will be called rcar-phy.
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index cf38f08..9af83f0 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -6,3 +6,4 @@ ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>
>  obj-$(CONFIG_USB_ISP1301)  += isp1301.o
>  obj-$(CONFIG_MV_U3D_PHY)   += mv_u3d_phy.o
> +obj-$(CONFIG_USB_RCAR_PHY) += rcar-phy.o
> diff --git a/drivers/usb/phy/rcar-phy.c b/drivers/usb/phy/rcar-phy.c
> new file mode 100644
> index 000..deec078
> --- /dev/null
> +++ b/drivers/usb/phy/rcar-phy.c
> @@ -0,0 +1,137 @@
> +/*
> + * Renesas R-Car USB phy driver
> + *
> + * Copyright (C) 2012 Renesas Solutions Corp.
> + * Kuninori Morimoto 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* USBH common register */
> +#define USBPCTRL0  0x0800
> +#define USBPCTRL1  0x0804
> +#define USBST  0x0808
> +#define USBEH0 0x080C
> +#define USBOH0 0x081C
> +#define USBCTL00x0858
> +#define EIIBC1 0x0094
> +#define EIIBC2 0x009C
> +
> +#ifdef CONFIG_ARCH_SUPPORTS_BIG_ENDIAN
> +# define PHY_ENDIAN "BIG"
> +# define PHY_NO_SWAP 0x0003
> +#else
> +# define PHY_ENDIAN "LITTLE"
> +# define PHY_NO_SWAP 0x
> +#endif
> +
> +/*
> + * USB initial/install operation.
> + *
> + * This function setup USB phy.
> + * The used value and setting order came from
> + * [USB :: Initial setting] on datasheet.
> + */
> +static int rcar_phy_probe(struct platform_device *pdev)
> +{
> +   struct resource *res0, *res1;
> +   void __iomem *reg0, *reg1;
> +   int i, ret = 0;
> +   u32 val;
> +
> +   res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +   res1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +   if (!res0 || !res1) {
> +   dev_err(&pdev->dev, "Not enough platform resources\n");
> +   return -ENODEV;
> +   }
> +
> +   reg0 = ioremap_nocache(res0->start, resource_size(res0));
> +   reg1 = ioremap_nocache(res1->start, resource_size(res1));

Instead use devm_ioremap_nocache?
> +   if (!reg0 || !reg1) {
> +   dev_err(&pdev->dev, "ioremap error.\n");
> +   ret = -ENOMEM;
> +   goto end;
> +   }
> +
> +   /**
> +* USB phy start-up
> +**/
> +
> +   /* (1) USB-PHY standby release */
> +   iowrite32(0x0001, (reg0 + USBPCTRL1));

Can we have some macros to define the above constant (and for the
constants used below)?

> +
> +   /* (2) start USB-PHY internal PLL */
> +   iowrite32(0x0003, (reg0 + USBPCTRL1));

No power management stuff added?? Dont we need to stop this PLL during suspend??
> +
> +   /* (3) USB module status check */
> +   for (i = 0; i < 1024; i++) {
> +   udelay(10);
> +   val = ioread32(reg0 + USBST);
> +   if (0xc000 == val)
> +   goto usb_module_is_working;
> +   }
> +   dev_err(&pdev->dev, "USB module not ready\n");
> +   ret = -EIO;
> +   goto end;
> +
> +usb_module_is_working:
> +   /* (4) USB-PHY reset clear */
> +   iowrite32(0x0007, (reg0 + USBPCTRL1));
> +
> +   /* set platform specific port settings */
> +   iowrite32(0x, (reg0 + USBPCTRL0));
> +
> +   /**
> +* EHCI IP Internal Buffer Setting
> +*-

Re: [PATCH] USB: ohci-at91: fix PIO handling in relation with number of ports

[Nicolas Ferre]

On 08/29/2012 11:49 AM, Nicolas Ferre :
> If the number of ports present on the SoC/board is not the maximum
> and that the platform data is not filled with all data, there is
> an easy way to mess the PIO setup for this interface.
> This quick fix addresses mis-configuration in USB host platform data
> that is common in at91 boards since commit 0ee6d1e (USB: ohci-at91:
> change maximum number of ports) that did not modified the associatd
> board files.
> 
> Reported-by: Klaus Falkner 
> Signed-off-by: Nicolas Ferre 
> Cc: Alan Stern 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Cc: Stable  [3.4+]

Alan, Greg, gentle ping...

I really would like to seen it landing in stable soon...

Thanks, best regards,

> ---
>  drivers/usb/host/ohci-at91.c |   10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index a665b3e..aaa8d2b 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -570,6 +570,16 @@ static int __devinit ohci_hcd_at91_drv_probe(struct 
> platform_device *pdev)
>  
>   if (pdata) {
>   at91_for_each_port(i) {
> + /*
> +  * do not configure PIO if not in relation with
> +  * real USB port on board
> +  */
> + if (i >= pdata->ports) {
> + pdata->vbus_pin[i] = -EINVAL;
> + pdata->overcurrent_pin[i] = -EINVAL;
> + break;
> + }
> +
>   if (!gpio_is_valid(pdata->vbus_pin[i]))
>   continue;
>   gpio = pdata->vbus_pin[i];
> 


-- 
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Bjørn Mork]

Bjørn Mork  writes:

> And I do believe the code before your change demonstrated that the
> original authors had the same view.  There was an explicit exception for
> just this case, and I do assume that was put there for a good
> reason. usbnet_bh() will be called while the device is suspended, and we
> must avoid making it reschedule itself in this case.
>
> Anyway, the ENOLINK test was there.  You removed it with no explanation
> whatsoever. It is *your* call to verify and explain to us why this test
> is unnecessary, not mine.

For your convienience, all the reasons why this code ended up like it
was are documented in the netdev patchwork:
http://patchwork.ozlabs.org/patch/59488/

There were different proposed solutions circulating, before the test for
ENOLINK was chosen.  No-one challenged the fact that some test for a
suspended device was needed here.


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 net] net: usbnet: fix softirq storm on suspend

[Bjørn Mork]

Ming Lei  writes:
> On Mon, Sep 3, 2012 at 4:26 PM, Bjørn Mork  wrote:
>
>> Sorry for not noticing this before, but commit 65841fd5
>> makes usbnet autosuspend completely unusable.  The device
>> is suspended fine, but burning one CPU core at full load
>> uses a tiny bit more power making the power saving
>> negative...
>
> I am wondering how you can reproduce the issue.

That's easy:

- Take any usbnet device supporting remote wakeup (and of course with a
  minidriver supporting it as well),
- enable autosuspend,
- ip link set dev ethX/usbX/wwanX up

And watch ksoftirqd/X use 100% of one of your CPU cores.

I'd very much like to hear the details if you are unable to reproduce
the bug using this simple test.

> usbnet_terminate_urbs is called inside usbnet_suspend to
> consume all URBs and SKBs, and rx_alloc_submit can't be
> called during the period because of !netif_device_present().

Huh We do support suspending a USB device while the network device
is up and running. That's the whole idea here, isn't it? I.e.
netif_device_present() is *expected* to be true while the USB device is
suspended.

> Once usbnet_terminate_urbs and netif_device_attach() are
> completed, who will schedule tasklet to trigger rx_alloc_submit?

That's a good question.  It sure would be nice if that never happened
without waking the device first.  But there are just too many call sites
for me to follow:

bjorn@nemi:/usr/local/src/git/linux$ grep tasklet_schedule 
drivers/net/usb/usbnet.c
tasklet_schedule(&dev->bh);
 * but tasklet_schedule() doesn't.  hope the failure is rare.
tasklet_schedule (&dev->bh);
tasklet_schedule(&dev->bh);
tasklet_schedule(&dev->bh);
tasklet_schedule (&dev->bh);
tasklet_schedule (&dev->bh);
tasklet_schedule (&dev->bh);
tasklet_schedule (&dev->bh);
tasklet_schedule (&dev->bh);
tasklet_schedule (&dev->bh);


And I do believe the code before your change demonstrated that the
original authors had the same view.  There was an explicit exception for
just this case, and I do assume that was put there for a good
reason. usbnet_bh() will be called while the device is suspended, and we
must avoid making it reschedule itself in this case.

Anyway, the ENOLINK test was there.  You removed it with no explanation
whatsoever. It is *your* call to verify and explain to us why this test
is unnecessary, not mine.


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