Re: Query regarding USB gadget driver

2014-12-15 Thread Peter Chen
On Tue, Dec 16, 2014 at 10:50:59AM +0530, Sanchayan Maity wrote:
> On 12/16/2014 06:16 AM, Peter Chen wrote:
> > On Mon, Dec 15, 2014 at 02:59:31PM +0530, Sanchayan Maity wrote:
> >> Hello,
> >>
> >> On 12/15/2014 07:42 AM, Peter Chen wrote:
> >>> On Fri, Dec 12, 2014 at 06:55:36PM +0530, Sanchayan Maity wrote:
>  Hello,
> 
>  On 12/12/2014 07:21 AM, Peter Chen wrote:
> > On Thu, Dec 11, 2014 at 08:34:45AM -0600, Felipe Balbi wrote:
> >> Hi,
> >>
> >> On Thu, Dec 11, 2014 at 04:08:43PM +0530, Sanchayan Maity wrote:
> >>> Hello,
> >>>
> >>> I am working on a Freescale Cortex-A5 Vybrid Processor. The chip core
> >>> is clocked at 500MHz and the USB IP core for this is by Chip-idea. I
> >>> am running a 3.18-rc5 kernel on it and trying to use the USB gadget
> >>> functionality. To be more specific the CDC ECM class. Currently, I
> >>> cannot use this properly. If I use just "ping" to check, it works
> >>> fine, but, after running iperf, even one transaction doesn't complete
> >>> or completes rarely. Checking the CDC Ether interface with Wireshark
> >>> shows, TCP Dup Ack messages and checking the USB bus with Wireshark,
> >>> shows packets with USB Protocol Error -71 at one point and after that
> >>> packets with USB connection Reset -104 error. If it's of any
> >>> significance, I have Arch Linux with the 3.18 kernel running on my
> >>> laptop with which the Vybrid connects. On the host side, the only
> >>> error dmesg shows is "kevent 12 may have been dropped". I guess this
> >>> is connected to the "TCP Previous Segment not captured" and "TCP Dup
> >>> ACK" messages.
> >>>
> >>> My script for the gadget configuration is as below:
> >>>
> >>> /bin/mount none /mnt -t configfs
> >>> /bin/mkdir /mnt/usb_gadget/g1
> >>> cd /mnt/usb_gadget/g1
> >>> /bin/mkdir configs/c.1
> >>> /bin/mkdir functions/ecm.0
> >>> /bin/mkdir strings/0x409
> >>> /bin/mkdir configs/c.1/strings/0x409
> >>> echo 0xa4a2 > idProduct
> >>> echo 0x0525 > idVendor
> >>> echo Freescale123 > strings/0x409/serialnumber
> >>> echo Freescale > strings/0x409/manufacturer
> >>> echo "USB Serial Gadget" > strings/0x409/product
> >>> echo "Conf 1" > configs/c.1/strings/0x409/configuration
> >>> echo 200 > configs/c.1/MaxPower
> >>> ln -s functions/ecm.0 configs/c.1
> >>> echo ci_hdrc.0 > UDC
> >>> /sbin/ifconfig usb0 up
> >>> /sbin/ifconfig usb0 192.168.1.10
> >>>
> >>> I have debug prints in the udc.c and u_ether.c using pr_debug and
> >>
> >> just a little hint, use any of the dev_*() macros next time, they'll
> >> print the device name which helps figuring out which UDC you're using.
> >>
> >> Based on ci_hdrc.0 above, I suppose it's chipidea and Peter Chen
> >> maintains that one, it really helps adding maintainers to Cc list.
> >>
> >>> enable them when required using dynamic debug. Without running iperf,
> >>> using ping gives me a sequence of prints as below:
> >>>
> >>> [  277.434409] In eth_start_xmit
> >>> [  277.434517] In UDC irq
> >>> [  277.434553] In usb_gadget_giveback_request
> >>> [  277.434567] In tx_complete
> >>> [  277.435443] In UDC irq
> >>> [  277.435477] In usb_gadget_giveback_request
> >>> [  277.435491] In rx_complete
> >>> [  277.435517] In rx_submit
> >>> [  277.435601] In eth_start_xmit
> >>> [  277.436441] In UDC irq
> >>> [  277.436465] In usb_gadget_giveback_request
> >>> [  277.436478] In rx_complete
> >>> [  277.436493] In rx_submit
> >>> [  277.436520] In usb_gadget_giveback_request
> >>> [  277.436533] In tx_complete
> >>> [  278.434865] In eth_start_xmit
> >>> [  278.434959] In UDC irq
> >>> [  278.434993] In usb_gadget_giveback_request
> >>> [  278.435006] In tx_complete
> >>> [  278.435881] In UDC irq
> >>> [  278.435910] In usb_gadget_giveback_request
> >>> [  278.435923] In rx_complete
> >>> [  278.435946] In rx_submit
> >>>
> >>> After running iperf without debug prints and then enabling before
> >>> using ping gives me a sequence of prints as below
> >>> [   81.989827] In UDC irq
> >>> [   81.989871] In usb_gadget_giveback_request
> >>> [   81.989886] In rx_complete
> >>> [   81.989905] In rx_submit
> >>> [   82.989892] In UDC irq
> >>> [   82.989951] In usb_gadget_giveback_request
> >>> [   82.989967] In rx_complete
> >>> [   82.989992] In rx_submit
> >>> [   83.990064] In UDC irq
> >>> [   83.990126] In usb_gadget_giveback_request
> >>> [   83.990142] In rx_complete
> >>> [   83.990167] In rx_submit
> >>> [   84.990007] In UDC irq
> >>> [   84.990049] In usb_gadget_giveback_request
> >>> [   84.990064] In rx_complete
> >>> [   84.990083] In rx_submit
> >>> [   85.990085] In UDC irq
> >>> [   85.990147] In us

Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

2014-12-15 Thread Jeremiah Mahler
Greg,

On Mon, Dec 15, 2014 at 08:38:01AM -0800, Greg Kroah-Hartman wrote:
> On Mon, Dec 15, 2014 at 04:53:05AM -0800, Jeremiah Mahler wrote:
> > Johan,
> > 
> > On Mon, Dec 15, 2014 at 11:23:21AM +0100, Johan Hovold wrote:
> > > On Thu, Dec 11, 2014 at 03:29:52PM -0800, Jeremiah Mahler wrote:
> > > > If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> > > > unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
> > > > logs.  This patch set quiets these messages without changing the
> > > > original behavior.
> > > 
> > > Don't unplug devices that are in use then. ;)
> > > 
> > I knew someone was going to say that :-)
> > 
> > > > This change is beneficial when using daemons such as slcand, which is
> > > > similar to pppd or slip, that cannot determine whether they should exit
> > > > until after the USB serial device is unplugged.  Producing these error
> > > > messages for a normal use case is not helpful.
> > > 
> > > Your patches would hide these errors when they occur during normal use
> > > (e.g. EPROTO).
> > > 
> > > Receiving an error message when unplugging an active device should not
> > > surprise anyone. And at least you know where it came from (and it's
> > > right there in the logs as well).
> > > 
> > > Johan
> > 
> > Hmm.  Yes, I can see why quieting -EPROTO would be bad because it would
> > hide protocol errors which we want to know about.
> 
> Do you really want to "know about" them?  What can a user do with them?
> Nothing, so just resubmit and you should be fine.
> 
> > I need to re-think this patch.
> > Nack.
> 
> I like this patch, putting crud in the kernel log that no one can do
> anything with for a "normal" operation like yanking a USB device out
> while it is open should not happen.
> 
> thanks,
> 
> greg k-h

Perhaps something at a lower level could return a more apt error number
such as -ENODEV.  Then there would be no conflict with -EPROTO.

I will look in to it again and re-submit the patches.

-- 
- Jeremiah Mahler
--
To unsubscribe from this list: send the line "unsubscribe 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: Query regarding USB gadget driver

2014-12-15 Thread Sanchayan Maity
On 12/16/2014 06:16 AM, Peter Chen wrote:
> On Mon, Dec 15, 2014 at 02:59:31PM +0530, Sanchayan Maity wrote:
>> Hello,
>>
>> On 12/15/2014 07:42 AM, Peter Chen wrote:
>>> On Fri, Dec 12, 2014 at 06:55:36PM +0530, Sanchayan Maity wrote:
 Hello,

 On 12/12/2014 07:21 AM, Peter Chen wrote:
> On Thu, Dec 11, 2014 at 08:34:45AM -0600, Felipe Balbi wrote:
>> Hi,
>>
>> On Thu, Dec 11, 2014 at 04:08:43PM +0530, Sanchayan Maity wrote:
>>> Hello,
>>>
>>> I am working on a Freescale Cortex-A5 Vybrid Processor. The chip core
>>> is clocked at 500MHz and the USB IP core for this is by Chip-idea. I
>>> am running a 3.18-rc5 kernel on it and trying to use the USB gadget
>>> functionality. To be more specific the CDC ECM class. Currently, I
>>> cannot use this properly. If I use just "ping" to check, it works
>>> fine, but, after running iperf, even one transaction doesn't complete
>>> or completes rarely. Checking the CDC Ether interface with Wireshark
>>> shows, TCP Dup Ack messages and checking the USB bus with Wireshark,
>>> shows packets with USB Protocol Error -71 at one point and after that
>>> packets with USB connection Reset -104 error. If it's of any
>>> significance, I have Arch Linux with the 3.18 kernel running on my
>>> laptop with which the Vybrid connects. On the host side, the only
>>> error dmesg shows is "kevent 12 may have been dropped". I guess this
>>> is connected to the "TCP Previous Segment not captured" and "TCP Dup
>>> ACK" messages.
>>>
>>> My script for the gadget configuration is as below:
>>>
>>> /bin/mount none /mnt -t configfs
>>> /bin/mkdir /mnt/usb_gadget/g1
>>> cd /mnt/usb_gadget/g1
>>> /bin/mkdir configs/c.1
>>> /bin/mkdir functions/ecm.0
>>> /bin/mkdir strings/0x409
>>> /bin/mkdir configs/c.1/strings/0x409
>>> echo 0xa4a2 > idProduct
>>> echo 0x0525 > idVendor
>>> echo Freescale123 > strings/0x409/serialnumber
>>> echo Freescale > strings/0x409/manufacturer
>>> echo "USB Serial Gadget" > strings/0x409/product
>>> echo "Conf 1" > configs/c.1/strings/0x409/configuration
>>> echo 200 > configs/c.1/MaxPower
>>> ln -s functions/ecm.0 configs/c.1
>>> echo ci_hdrc.0 > UDC
>>> /sbin/ifconfig usb0 up
>>> /sbin/ifconfig usb0 192.168.1.10
>>>
>>> I have debug prints in the udc.c and u_ether.c using pr_debug and
>>
>> just a little hint, use any of the dev_*() macros next time, they'll
>> print the device name which helps figuring out which UDC you're using.
>>
>> Based on ci_hdrc.0 above, I suppose it's chipidea and Peter Chen
>> maintains that one, it really helps adding maintainers to Cc list.
>>
>>> enable them when required using dynamic debug. Without running iperf,
>>> using ping gives me a sequence of prints as below:
>>>
>>> [  277.434409] In eth_start_xmit
>>> [  277.434517] In UDC irq
>>> [  277.434553] In usb_gadget_giveback_request
>>> [  277.434567] In tx_complete
>>> [  277.435443] In UDC irq
>>> [  277.435477] In usb_gadget_giveback_request
>>> [  277.435491] In rx_complete
>>> [  277.435517] In rx_submit
>>> [  277.435601] In eth_start_xmit
>>> [  277.436441] In UDC irq
>>> [  277.436465] In usb_gadget_giveback_request
>>> [  277.436478] In rx_complete
>>> [  277.436493] In rx_submit
>>> [  277.436520] In usb_gadget_giveback_request
>>> [  277.436533] In tx_complete
>>> [  278.434865] In eth_start_xmit
>>> [  278.434959] In UDC irq
>>> [  278.434993] In usb_gadget_giveback_request
>>> [  278.435006] In tx_complete
>>> [  278.435881] In UDC irq
>>> [  278.435910] In usb_gadget_giveback_request
>>> [  278.435923] In rx_complete
>>> [  278.435946] In rx_submit
>>>
>>> After running iperf without debug prints and then enabling before
>>> using ping gives me a sequence of prints as below
>>> [   81.989827] In UDC irq
>>> [   81.989871] In usb_gadget_giveback_request
>>> [   81.989886] In rx_complete
>>> [   81.989905] In rx_submit
>>> [   82.989892] In UDC irq
>>> [   82.989951] In usb_gadget_giveback_request
>>> [   82.989967] In rx_complete
>>> [   82.989992] In rx_submit
>>> [   83.990064] In UDC irq
>>> [   83.990126] In usb_gadget_giveback_request
>>> [   83.990142] In rx_complete
>>> [   83.990167] In rx_submit
>>> [   84.990007] In UDC irq
>>> [   84.990049] In usb_gadget_giveback_request
>>> [   84.990064] In rx_complete
>>> [   84.990083] In rx_submit
>>> [   85.990085] In UDC irq
>>> [   85.990147] In usb_gadget_giveback_request
>>> [   85.990163] In rx_complete
>>> [   85.990188] In rx_submit
>>>
>>> If I force a full speed configuration for this USB client port, I get
>>> a slightly more reliable operation where iperf can run for may be half
>>> an hou

[PATCH 0/3] Add support for Fujitsu USB host controller

2014-12-15 Thread Sneeker Yeh
These patches add support for EHCI and XHCI compliant Host controller found
on Fujitsu Socs, and are based on
http://www.spinics.net/lists/arm-kernel/msg385573.html

First patch is EHCI platform glue driver. Patch 2 introduces Fujitsu
Specific Glue layer in Synopsis DesignWare USB3 IP driver. Patch 3
introduces a quirk with device disconnection management necessary for IPs
using the Synopsys Designware dwc3 core. It solves a problem where without
the quirk, that host controller will die after a usb device is disconnected
from port of root hub, which happened in Synopsis core with Fujitsu
config-free usb phy integrated.

Successfully tested on Fujitsu mb86s7x board.

Sneeker Yeh (3):
  usb: host: f_usb20ho: add support for Fujitsu ehci/ohci USB 2.0 host
controller
  usb: dwc3: add Fujitsu Specific Glue layer
  usb: dwc3: add a quirk for device disconnection issue in Synopsis
dwc3 core

 .../devicetree/bindings/usb/fujitsu-dwc3.txt   |   25 ++
 .../devicetree/bindings/usb/fujitsu-ehci.txt   |   22 ++
 drivers/usb/dwc3/Kconfig   |   11 +
 drivers/usb/dwc3/Makefile  |1 +
 drivers/usb/dwc3/dwc3-mb86s70.c|  194 +
 drivers/usb/host/Kconfig   |   11 +
 drivers/usb/host/Makefile  |1 +
 drivers/usb/host/f_usb20ho_hcd.c   |  306 
 drivers/usb/host/f_usb20ho_hcd.h   |   35 +++
 drivers/usb/host/xhci-hub.c|4 +
 drivers/usb/host/xhci-plat.c   |5 +
 drivers/usb/host/xhci.c|   29 ++
 drivers/usb/host/xhci.h|7 +
 13 files changed, 651 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
 create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
 create mode 100644 drivers/usb/dwc3/dwc3-mb86s70.c
 create mode 100644 drivers/usb/host/f_usb20ho_hcd.c
 create mode 100644 drivers/usb/host/f_usb20ho_hcd.h

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


[PATCH 1/3] usb: host: f_usb20ho: add support for Fujitsu ehci/ohci USB 2.0 host controller

2014-12-15 Thread Sneeker Yeh
This patch adds support for EHCI compliant Host controller found
on Fujitsu Socs.

Signed-off-by: Sneeker Yeh 
---
 .../devicetree/bindings/usb/fujitsu-ehci.txt   |   22 ++
 drivers/usb/host/Kconfig   |   11 +
 drivers/usb/host/Makefile  |1 +
 drivers/usb/host/f_usb20ho_hcd.c   |  306 
 drivers/usb/host/f_usb20ho_hcd.h   |   35 +++
 5 files changed, 375 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
 create mode 100644 drivers/usb/host/f_usb20ho_hcd.c
 create mode 100644 drivers/usb/host/f_usb20ho_hcd.h

diff --git a/Documentation/devicetree/bindings/usb/fujitsu-ehci.txt 
b/Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
new file mode 100644
index 000..e180860
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
@@ -0,0 +1,22 @@
+FUJITSU GLUE COMPONENTS
+
+MB86S7x EHCI GLUE
+ - compatible : Should be "fujitsu,f_usb20ho_hcd"
+ - reg : Address and length of the register set for the device.
+ - interrupts : The irq number of this device that is used to interrupt the
+   CPU
+ - clocks: from common clock binding, handle to usb clock.
+ - clock-names: from common clock binding.
+ - #stream-id-cells : handle to use "arm,mmu-400" ARM IOMMU driver
+ - fujitsu,power-domain : pd_usb2h node has to be builded, details can be
+   found in:
+   Documentation/devicetree/bindings/
+
+hcd21: f_usb20ho_hcd {
+   compatible = "fujitsu,f_usb20ho_hcd";
+   reg = <0 0x3420 0x8>;
+   interrupts = <0 419 0x4>;
+   clocks = <&clk_main_2_4>, <&clk_main_4_5>, <&clk_usb_0_0>;
+   clock-names = "h_clk", "p_clk", "p_cryclk";
+   #stream-id-cells = <0>;
+};
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index fafc628..9482140 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -786,6 +786,17 @@ config USB_HCD_SSB
 
  If unsure, say N.
 
+config USB_F_USB20HO_HCD
+   tristate "F_USB20HO USB Host Controller"
+   depends on USB && ARCH_MB86S7X
+   default y
+   help
+ This driver enables support for USB20HO USB Host Controller,
+ the driver supports High, Full and Low Speed USB.
+
+ To compile this driver a module, choose M here: the module
+ will be called "f_usb20ho-hcd".
+
 config USB_HCD_TEST_MODE
bool "HCD test mode support"
---help---
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index d6216a4..b89e536 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -78,3 +78,4 @@ obj-$(CONFIG_USB_HCD_SSB) += ssb-hcd.o
 obj-$(CONFIG_USB_FUSBH200_HCD) += fusbh200-hcd.o
 obj-$(CONFIG_USB_FOTG210_HCD)  += fotg210-hcd.o
 obj-$(CONFIG_USB_MAX3421_HCD)  += max3421-hcd.o
+obj-$(CONFIG_USB_F_USB20HO_HCD)+= f_usb20ho_hcd.o
diff --git a/drivers/usb/host/f_usb20ho_hcd.c b/drivers/usb/host/f_usb20ho_hcd.c
new file mode 100644
index 000..d665487
--- /dev/null
+++ b/drivers/usb/host/f_usb20ho_hcd.c
@@ -0,0 +1,306 @@
+/**
+ * f_usb20ho_hcd.c - Fujitsu EHCI platform driver
+ *
+ * Copyright (c) 2013 - 2014 FUJITSU SEMICONDUCTOR LIMITED
+ * http://jp.fujitsu.com/group/fsl
+ *
+ * based on bcma-hcd.c
+ *
+ * Author: Sneeker Yeh 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "f_usb20ho_hcd.h"
+
+static int f_usb20ho_clk_control(struct device *dev, bool on)
+{
+   int ret, i;
+   struct clk *clk;
+
+   if (!on)
+   goto clock_off;
+
+   for (i = 0;; i++) {
+   clk = of_clk_get(dev->of_node, i);
+   if (IS_ERR(clk))
+   break;
+
+   ret = clk_prepare_enable(clk);
+   if (ret) {
+   dev_err(dev, "failed to enable clock[%d]\n", i);
+   goto clock_off;
+   }
+   }
+
+   return 0;
+
+clock_off:
+   for (i = 0;; i++) {
+   clk = of_clk_get(dev->of_node, i);
+   if (IS_ERR(clk))
+   break;
+
+   clk_disable_unprepare(clk);
+   }
+
+   return on;
+}
+
+static struct platform_device *f_usb20ho_hcd_create_pdev(
+   struct platform_device *pdev, bool ohci)
+{
+   struct resource *resource;
+   struct platform_device *hci_dev;
+   struct resource hci_res[2];
+   int ret = -ENOMEM;
+   int irq;
+   resource_size_t resource_size;
+
+   memset(hci_res, 0, sizeof(hci_res));
+
+   resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   if

[PATCH 2/3] usb: dwc3: add Fujitsu Specific Glue layer

2014-12-15 Thread Sneeker Yeh
This patch adds support for Synopsis DesignWare USB3 IP Core found
on Fujitsu Socs.

Signed-off-by: Sneeker Yeh 
---
 .../devicetree/bindings/usb/fujitsu-dwc3.txt   |   25 +++
 drivers/usb/dwc3/Kconfig   |   11 ++
 drivers/usb/dwc3/Makefile  |1 +
 drivers/usb/dwc3/dwc3-mb86s70.c|  194 
 4 files changed, 231 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
 create mode 100644 drivers/usb/dwc3/dwc3-mb86s70.c

diff --git a/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt 
b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
new file mode 100644
index 000..df77f91
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
@@ -0,0 +1,25 @@
+FUJITSU GLUE COMPONENTS
+
+MB86S7x DWC3 GLUE
+ - compatible : Should be "fujitsu,mb86s70-dwc3"
+ - clocks: from common clock binding, handle to usb clock.
+ - clock-names: from common clock binding.
+ - #address-cells, #size-cells : Must be present if the device has sub-nodes
+ - ranges: the child address space are mapped 1:1 onto the parent address space
+ - #stream-id-cells : handle to use "arm,mmu-400" ARM IOMMU driver
+
+Sub-nodes:
+The dwc3 core should be added as subnode to MB86S7x dwc3 glue.
+- dwc3 :
+   The binding details of dwc3 can be found in:
+   Documentation/devicetree/bindings/usb/dwc3.txt
+
+usb3host: mb86s70_usb3host {
+   compatible = "fujitsu,mb86s70-dwc3";
+   clocks = <&clk_alw_1_1>;
+   clock-names = "bus_clk";
+   #address-cells = <2>;
+   #size-cells = <1>;
+   ranges;
+   #stream-id-cells = <0>;
+};
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 58b5b2c..3390d42 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -61,6 +61,17 @@ config USB_DWC3_EXYNOS
  Recent Exynos5 SoCs ship with one DesignWare Core USB3 IP inside,
  say 'Y' or 'M' if you have one such device.
 
+config USB_DWC3_MB86S70
+   tristate "MB86S70 Designware USB3 Platform code"
+   default USB_DWC3
+   help
+ MB86S7X SOC ship with DesignWare Core USB3 IP inside,
+ this implementation also integrated Fujitsu USB PHY inside
+ this Core USB3 IP.
+
+ say 'Y' or 'M' if you have one such device.
+
+
 config USB_DWC3_PCI
tristate "PCIe-based Platforms"
depends on PCI
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index bb34fbc..05d1de2 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -38,3 +38,4 @@ obj-$(CONFIG_USB_DWC3_PCI)+= dwc3-pci.o
 obj-$(CONFIG_USB_DWC3_KEYSTONE)+= dwc3-keystone.o
 obj-$(CONFIG_USB_DWC3_QCOM)+= dwc3-qcom.o
 obj-$(CONFIG_USB_DWC3_ST)  += dwc3-st.o
+obj-$(CONFIG_USB_DWC3_MB86S70) += dwc3-mb86s70.o
diff --git a/drivers/usb/dwc3/dwc3-mb86s70.c b/drivers/usb/dwc3/dwc3-mb86s70.c
new file mode 100644
index 000..241c5bd
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-mb86s70.c
@@ -0,0 +1,194 @@
+/**
+ * dwc3-mb86s70.c - Fujitsu mb86s70 DWC3 Specific Glue layer
+ *
+ * Copyright (c) 2013 - 2014 FUJITSU SEMICONDUCTOR LIMITED
+ * http://jp.fujitsu.com/group/fsl
+ *
+ * Author: Alice Chan 
+ * based on dwc3-exynos.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct dwc3_mb86s70 {
+   struct device   *dev;
+   struct clk  **clk_table;
+};
+
+/* return 0 means successful */
+static int dwc3_mb86s70_clk_control(struct device *dev, bool on)
+{
+   int ret, i;
+   struct clk *clk;
+
+   if (!on)
+   goto clock_off;
+
+   for (i = 0;; i++) {
+   clk = of_clk_get(dev->of_node, i);
+   if (IS_ERR(clk))
+   break;
+
+   ret = clk_prepare_enable(clk);
+   if (ret) {
+   dev_err(dev, "failed to enable clock[%d]\n", i);
+   goto clock_off;
+   }
+   }
+
+   return 0;
+
+clock_off:
+   for (i = 0;; i++) {
+   clk = of_clk_get(dev->of_node, i);
+   if (IS_ERR(clk))
+   break;
+
+   clk_disable_unprepare(clk);
+   }
+
+   return on;
+}
+
+static int dwc3_mb86s70_remove_child(struct device *dev, void *unused)
+{
+   struct platform_device *pdev = to_platform_device(dev);
+
+   of_device_unregister(pdev);
+
+   return 0;
+}
+
+static u64 dwc3_mb86s70_dma_mask = DMA_BIT_MASK(32);
+
+static int dwc3_mb86s70_probe(struct platform_device *pdev)
+{
+   struct dwc3_mb86s70 *mb86s70;
+   s

[PATCH 3/3] usb: dwc3: add a quirk for device disconnection issue in Synopsis dwc3 core

2014-12-15 Thread Sneeker Yeh
Synopsis DesignWare USB3 IP Core integrated with a config-free
phy needs special handling during device disconnection to avoid
the host controller dying.

This quirk makes sure PORT_CSC is cleared after the disable slot
command when usb device is disconnected from internal root hub,
otherwise, Synopsis core would fall into a state that cannot use
any endpoint command. Consequently, device disconnection procedure
might not be finished because sometimes endpoint need to be stop
by endpoint stop command issuing.

Symptom usually happens when disconnected device is keyboard,
mouse, and hub.

Signed-off-by: Sneeker Yeh 
---
 drivers/usb/host/xhci-hub.c  |4 
 drivers/usb/host/xhci-plat.c |5 +
 drivers/usb/host/xhci.c  |   29 +
 drivers/usb/host/xhci.h  |7 +++
 4 files changed, 45 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a7865c4..3b8f7fc 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -368,6 +368,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd 
*xhci, u16 wValue,
port_change_bit = "warm(BH) reset";
break;
case USB_PORT_FEAT_C_CONNECTION:
+   if ((xhci->quirks & XHCI_DISCONNECT_QUIRK) &&
+   !(readl(addr) & PORT_CONNECT))
+   return;
+
status = PORT_CSC;
port_change_bit = "connect";
break;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 08d402b..3ede6b4 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -33,6 +33,11 @@ static void xhci_plat_quirks(struct device *dev, struct 
xhci_hcd *xhci)
 * dev struct in order to setup MSI
 */
xhci->quirks |= XHCI_PLAT;
+
+   if (dev->parent && dev->parent->parent &&
+   of_device_is_compatible(dev->parent->parent->of_node,
+   "fujitsu,mb86s70-dwc3"))
+   xhci->quirks |= XHCI_DISCONNECT_QUIRK;
 }
 
 /* called during probe() after chip reset completes */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 01fcbb5..50d757b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3584,6 +3584,33 @@ command_cleanup:
return ret;
 }
 
+static void xhci_try_to_clear_csc(struct usb_hcd *hcd, int dev_port_num)
+{
+   int max_ports;
+   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+   __le32 __iomem **port_array;
+   u32 status;
+
+   /* print debug info */
+   if (hcd->speed == HCD_USB3) {
+   max_ports = xhci->num_usb3_ports;
+   port_array = xhci->usb3_ports;
+   } else {
+   max_ports = xhci->num_usb2_ports;
+   port_array = xhci->usb2_ports;
+   }
+
+   if (dev_port_num > max_ports) {
+   xhci_err(xhci, "%s() port number invalid", __func__);
+   return;
+   }
+   status = readl(port_array[dev_port_num - 1]);
+
+   /* write 1 to clear */
+   if (!(status & PORT_CONNECT) && (status & PORT_CSC))
+   writel(status & PORT_CSC, port_array[0]);
+}
+
 /*
  * At this point, the struct usb_device is about to go away, the device has
  * disconnected, and all traffic has been stopped and the endpoints have been
@@ -3649,6 +3676,8 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device 
*udev)
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
 
+   if (xhci->quirks & XHCI_DISCONNECT_QUIRK)
+   xhci_try_to_clear_csc(hcd, udev->portnum);
/*
 * Event command completion handler will free any data structures
 * associated with the slot.  XXX Can free sleep?
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index cc7c5bb..e6c820c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1560,6 +1560,13 @@ struct xhci_hcd {
 #define XHCI_SPURIOUS_WAKEUP   (1 << 18)
 /* For controllers with a broken beyond repair streams implementation */
 #define XHCI_BROKEN_STREAMS(1 << 19)
+/*
+ * Synopsis dwc3 core integrated with a config-free phy needs special
+ * handling during device disconnection to avoid the host controller
+ * dying. This quirk makes sure PORT_CSC is cleared after the disable
+ * slot command.
+ */
+#define XHCI_DISCONNECT_QUIRK  (1 << 20)
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
/* There are two roothubs to keep track of bus suspend info for */
-- 
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: Query regarding USB gadget driver

2014-12-15 Thread Peter Chen
On Mon, Dec 15, 2014 at 02:59:31PM +0530, Sanchayan Maity wrote:
> Hello,
> 
> On 12/15/2014 07:42 AM, Peter Chen wrote:
> > On Fri, Dec 12, 2014 at 06:55:36PM +0530, Sanchayan Maity wrote:
> >> Hello,
> >>
> >> On 12/12/2014 07:21 AM, Peter Chen wrote:
> >>> On Thu, Dec 11, 2014 at 08:34:45AM -0600, Felipe Balbi wrote:
>  Hi,
> 
>  On Thu, Dec 11, 2014 at 04:08:43PM +0530, Sanchayan Maity wrote:
> > Hello,
> >
> > I am working on a Freescale Cortex-A5 Vybrid Processor. The chip core
> > is clocked at 500MHz and the USB IP core for this is by Chip-idea. I
> > am running a 3.18-rc5 kernel on it and trying to use the USB gadget
> > functionality. To be more specific the CDC ECM class. Currently, I
> > cannot use this properly. If I use just "ping" to check, it works
> > fine, but, after running iperf, even one transaction doesn't complete
> > or completes rarely. Checking the CDC Ether interface with Wireshark
> > shows, TCP Dup Ack messages and checking the USB bus with Wireshark,
> > shows packets with USB Protocol Error -71 at one point and after that
> > packets with USB connection Reset -104 error. If it's of any
> > significance, I have Arch Linux with the 3.18 kernel running on my
> > laptop with which the Vybrid connects. On the host side, the only
> > error dmesg shows is "kevent 12 may have been dropped". I guess this
> > is connected to the "TCP Previous Segment not captured" and "TCP Dup
> > ACK" messages.
> >
> > My script for the gadget configuration is as below:
> >
> > /bin/mount none /mnt -t configfs
> > /bin/mkdir /mnt/usb_gadget/g1
> > cd /mnt/usb_gadget/g1
> > /bin/mkdir configs/c.1
> > /bin/mkdir functions/ecm.0
> > /bin/mkdir strings/0x409
> > /bin/mkdir configs/c.1/strings/0x409
> > echo 0xa4a2 > idProduct
> > echo 0x0525 > idVendor
> > echo Freescale123 > strings/0x409/serialnumber
> > echo Freescale > strings/0x409/manufacturer
> > echo "USB Serial Gadget" > strings/0x409/product
> > echo "Conf 1" > configs/c.1/strings/0x409/configuration
> > echo 200 > configs/c.1/MaxPower
> > ln -s functions/ecm.0 configs/c.1
> > echo ci_hdrc.0 > UDC
> > /sbin/ifconfig usb0 up
> > /sbin/ifconfig usb0 192.168.1.10
> >
> > I have debug prints in the udc.c and u_ether.c using pr_debug and
> 
>  just a little hint, use any of the dev_*() macros next time, they'll
>  print the device name which helps figuring out which UDC you're using.
> 
>  Based on ci_hdrc.0 above, I suppose it's chipidea and Peter Chen
>  maintains that one, it really helps adding maintainers to Cc list.
> 
> > enable them when required using dynamic debug. Without running iperf,
> > using ping gives me a sequence of prints as below:
> >
> > [  277.434409] In eth_start_xmit
> > [  277.434517] In UDC irq
> > [  277.434553] In usb_gadget_giveback_request
> > [  277.434567] In tx_complete
> > [  277.435443] In UDC irq
> > [  277.435477] In usb_gadget_giveback_request
> > [  277.435491] In rx_complete
> > [  277.435517] In rx_submit
> > [  277.435601] In eth_start_xmit
> > [  277.436441] In UDC irq
> > [  277.436465] In usb_gadget_giveback_request
> > [  277.436478] In rx_complete
> > [  277.436493] In rx_submit
> > [  277.436520] In usb_gadget_giveback_request
> > [  277.436533] In tx_complete
> > [  278.434865] In eth_start_xmit
> > [  278.434959] In UDC irq
> > [  278.434993] In usb_gadget_giveback_request
> > [  278.435006] In tx_complete
> > [  278.435881] In UDC irq
> > [  278.435910] In usb_gadget_giveback_request
> > [  278.435923] In rx_complete
> > [  278.435946] In rx_submit
> >
> > After running iperf without debug prints and then enabling before
> > using ping gives me a sequence of prints as below
> > [   81.989827] In UDC irq
> > [   81.989871] In usb_gadget_giveback_request
> > [   81.989886] In rx_complete
> > [   81.989905] In rx_submit
> > [   82.989892] In UDC irq
> > [   82.989951] In usb_gadget_giveback_request
> > [   82.989967] In rx_complete
> > [   82.989992] In rx_submit
> > [   83.990064] In UDC irq
> > [   83.990126] In usb_gadget_giveback_request
> > [   83.990142] In rx_complete
> > [   83.990167] In rx_submit
> > [   84.990007] In UDC irq
> > [   84.990049] In usb_gadget_giveback_request
> > [   84.990064] In rx_complete
> > [   84.990083] In rx_submit
> > [   85.990085] In UDC irq
> > [   85.990147] In usb_gadget_giveback_request
> > [   85.990163] In rx_complete
> > [   85.990188] In rx_submit
> >
> > If I force a full speed configuration for this USB client port, I get
> > a slightly more reliable operation where iperf can run for may be half
> > an hour or so or almost an hour before it falls thro

Re: PROBLEM: USB isochronous urb leak on EHCI driver

2014-12-15 Thread Fabio Estevam
On Mon, Dec 15, 2014 at 6:53 PM, Michael Tessier
 wrote:

> Before attempting to upgrade to an earlier kernel driver (this is
> a fairly big amount of work), I would really like to know if this
> problem would still be in the 3.x kernels. Has anyone seen that
> issue in 3.x kernels?

In fact it is very simple to test USB on a 3.18 kernel with your hardware.

You only need a minimal dts file with the usb/serial nodes enabled.

Take a loot at arch/arm/boot/dts/imx51-babbage.dts for a reference.

I tested USB earlier today on this board with 3.18 and it worked fine.
You need this patch:
http://www.spinics.net/lists/arm-kernel/msg385739.html

Regards,

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


USB 3.0 and xHCI Host-Controller

2014-12-15 Thread Gustavo Duarte
Hi Guys,



*
Issue description
*

I'm  having troubles for communication  between a  NXT - Lego Brick
device (http://www.lego.com/)  and a Notebook with USB 3.0.

I'm using a Java development SDK, to build apps and flash the Brick:
http://www.lejos.org

We are working with two very similar models of Notebooks:

a) Called Positivo-1: Has  3 USB 2.0 ports,
b) Called Positivo-2: Has  2 USB 2.0 and 1 USB 3.0 port.

The name of positivo comes from: (making a reference to the brand
name:http://www.positivobgh.com.ar/hogar/spa/productos/c-501)


On Postivo-1 all work fine, but on the Positivo-2 the communication
with the brick isn't working.


I did some tests, getting some treces, on a Notebook where all work
fine and on the other one.

Both notebook are running Ubuntu 12.04 with 64 bits.


Positivo-1, has 3 physical interfaces USB 2.0

Positivo-2, has 2 physical interfaces USB 2.0 y one USB 3.0

On Positivo-1, as it has USB 2.0, linux attach to this hardware the
driver "ehci-pci"

On Positivo-2, as it has one USB 3.0, linux attach to the 3 ports the
driver, "xhci_hcd".


On Positivo-1 all work, fine, I make a HelloWorld example,
following http://www.lejos.org/nxt/nxj/tutorial/Preliminaries/FirstProgram.htm
Tutorial, and i could upload the app to the brick without any problem.

But on Positivo-2, I couldn't, because the upload procedure is hangup,
like is mentioned on the
http://www.lejos.org/forum/viewtopic.php?f=7&t=4307&hilit=linux Topic.



Making a research to try find out, what is the problem, i added printf
to the library JNI code, which is used to send/receive message to/from
the Brick. I modified the file: main_jlibnxt.c.

With these printf I can see what messages are sent, when the PC try to
open the communication with the BRICK and what is the response.

I created two traces files during uploading a HelloWorld app, once for
Positivo-1 and other one on Positivo-2 (USB 3.0)

Here is the JNI code i modified,  and whole traces,
https://github.com/gusDuarte/enchanting-usb3



*
Traces
*


POSITIVO-1:
=

Found NXT: NXT 001653105DC5
* starting FIND ...
** Created dev(139821856403248), addr: USB0::0x0694::0x0002::001653105DC5::RAW
* ending FIND ...

start OPEN 
** Created dev(139821856403248), addr: USB0::0x0694::0x0002::001653105DC5::RAW
end OPEN ===

SEND> 01:9B:12:D0:2A:7F:00:00
READ< 02:9B:00:4E:58:54:00:00

start OPEN 
** Created dev(139821856403248), addr: USB0::0x0694::0x0002::001653105DC5::RAW
end OPEN ===

SEND> 01:FF:00:00:00:00:00:00
READ< 02:FF:FD:00:00:00:00:00

SEND> 01:81:48:65:6C:6C:6F:57
READ< 02:81:00:00:00:00:00:00
.
.
more messages
.
.




POSITIVO-2:
=

Found NXT: NXT 001653105DC5
* starting FIND ...
** Created dev(139675089352512), addr: USB0::0x0694::0x0002::001653105DC5::RAW
* ending FIND ...

start OPEN 
** Created dev(139675089352512), addr: USB0::0x0694::0x0002::001653105DC5::RAW
end OPEN ===

SEND> 01:9B:15:A4:08:7F:00:00
READ< 02:9B:00:4E:58:54:00:00

start OPEN 
** Created dev(139675089352512), addr: USB0::0x0694::0x0002::001653105DC5::RAW
end OPEN ===


SEND> 01:FF:00:00:00:00:00:00
READ< invalid data len: -110

Here the communication hangs.





*
Notebooks System information
*

PC POSITIVO-1
=

Kernel version:

$ uname -r
3.8.0-33-generic

Libusb:
ceibal@ceibal:~$ dpkg -l | grep libusb

ii  libusb-0.1-4  2:0.1.12-20
 userspace USB programming library
ii  libusb-1.0-0  2:1.0.9~rc3-2ubuntu1
  userspace USB programming library
ic  libusb-1.0-0:i386 2:1.0.9~rc3-2ubuntu1
  userspace USB programming library
ii  libusb-dev2:0.1.12-20
 userspace USB programming library development
files
ii  libusbmuxd1




$ lsusb

Bus 001 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub
Bus 002 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 003: ID 04f2:b340 Chicony Electronics Co., Ltd


$lspci -vvv

00:1a.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
Family USB Enhanced Host Controller #2 (rev 04) (prog-if 20 [EHCI])
Subsystem: CLEVO/KAPOK Computer Device 3110
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
SERR- 
Kernel driver in use: ehci-pci



PC POSITIVO-2

kernel version:

$ una

[PATCH] renesas_usbhs: add OTG ID signal sensing

2014-12-15 Thread Sergei Shtylyov
On the Renesas R8A7791 SoC based boards there's MAX3355 USB OTG chip and mini-AB
USB connector corresponding to USB port 0 driven either by EHCI/OHCI or  Renesas
USBHS gadget controller. And we'd like the host/gadget  drivers to work based on
the cable type connected. An 'extcon' driver for MAX3355 has been written, so we
only need to bind  to it via device tree which I'm doing in this patch.

(Perhaps, it would also make sense to use OTG HNP when the USBHS host mode is
active and a B-cable is connected but I don't have access to host-capable USBHS,
so  wouldn't be able to test it.)

Signed-off-by: Sergei Shtylyov 

---
This patch is against the 'next'  branch of Felipe Balbi's 'usb.git' repo.
It needs the recent 'extcon' core in  order to properly handle probe deferral.  

 drivers/usb/renesas_usbhs/common.c |   17 +
 drivers/usb/renesas_usbhs/common.h |3 +++
 2 files changed, 20 insertions(+)

Index: usb/drivers/usb/renesas_usbhs/common.c
===
--- usb.orig/drivers/usb/renesas_usbhs/common.c
+++ usb/drivers/usb/renesas_usbhs/common.c
@@ -363,6 +363,7 @@ static void usbhsc_hotplug(struct usbhs_
struct usbhs_mod *mod = usbhs_mod_get_current(priv);
int id;
int enable;
+   int cable;
int ret;
 
/*
@@ -376,6 +377,16 @@ static void usbhsc_hotplug(struct usbhs_
id = usbhs_platform_call(priv, get_id, pdev);
 
if (enable && !mod) {
+   if (priv->edev) {
+   cable = extcon_get_cable_state(priv->edev, "USB-HOST");
+   if ((cable > 0 && id != USBHS_HOST) ||
+   (!cable && id != USBHS_GADGET)) {
+   dev_info(&pdev->dev,
+"USB cable plugged in doesn't match 
the selected role!\n");
+   return;
+   }
+   }
+
ret = usbhs_mod_change(priv, id);
if (ret < 0)
return;
@@ -514,6 +525,12 @@ static int usbhs_probe(struct platform_d
if (IS_ERR(priv->base))
return PTR_ERR(priv->base);
 
+   if (of_property_read_bool(pdev->dev.of_node, "extcon")) {
+   priv->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
+   if (IS_ERR(priv->edev))
+   return PTR_ERR(priv->edev);
+   }
+
/*
 * care platform info
 */
Index: usb/drivers/usb/renesas_usbhs/common.h
===
--- usb.orig/drivers/usb/renesas_usbhs/common.h
+++ usb/drivers/usb/renesas_usbhs/common.h
@@ -17,6 +17,7 @@
 #ifndef RENESAS_USB_DRIVER_H
 #define RENESAS_USB_DRIVER_H
 
+#include 
 #include 
 #include 
 
@@ -254,6 +255,8 @@ struct usbhs_priv {
struct delayed_work notify_hotplug_work;
struct platform_device *pdev;
 
+   struct extcon_dev *edev;
+
spinlock_t  lock;
 
u32 flags;

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


Re: PROBLEM: USB isochronous urb leak on EHCI driver

2014-12-15 Thread Greg KH
On Mon, Dec 15, 2014 at 08:53:20PM +, Michael Tessier wrote:
> <5>Linux version 2.6.31-770-g0e46b52-0897 
> (michael.tess...@vsvr-compile-01.pocatec.com) (gcc version 4.1.2) #12 PREEMPT 
> Mon Nov 24 18:34:19 EST 2014

That is a _very_ old and obsolete kernel version, you need to get
support from the company that is forcing you to stay on that version, as
you are paying them for this.

There is nothing the community can do with a kernel version like this,
sorry.

best of luck,

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


PROBLEM: USB isochronous urb leak on EHCI driver

2014-12-15 Thread Michael Tessier
Hi,

I am dealing with a USB EHCI driver bug. Here is the info:

My configuration:
-

Host: Freescale i.MX512 with ARM Cortex A8 (USB 2.0 host controller)
Linux kernel: 2.6.31, using EHCI USB driver
Hub: 4-PORT USB 1.1 HUB (Texas Instruments PN: tusb2046b)
Devices: 4 USB 1.1 audio codecs (Texas Instruments PN: pcm2901)

Note: each codec is being used in R/W access, so with 4 codecs, I have
4 playback and 4 capture streams.

My problem:
---

I have usb urb leaks when connecting more than 1 codec to the USB 1.1
Hub. (the result is that some of the audio data is not transferred,
part of the sound is simply missing) No problem when using only 1
of the 4 codecs connected to the hub; When I connect a second codec,
the sound quality starts to degrade. With 3 codecs, we just cannot
recognize a speach.

Tests and observations:
---

Since I have 3 usb ports available on the i.MX512, I tried to connect
3 codecs directly on USB ports: the sound is perfect on each of the
three ports.

I bought a consumer USB 2.0 Hub: no problem when using 3 codecs
connected to that Hub, however, the audio will completly stop on all
channels when connecting the 4th codec.

I checked the communication between the Hub (USB 1.1) and the Host
controller (USB 2.0) with a scope and concluded that the
communication speed is 1.5 MBytes/s has expected (so the 
communication is downgraded to USB 1.1, since codecs and hub are USB
1.1 devices).

Also, I know that there is physically enough bandwidth to
transfer the data for two reasons:
1) I have an older CPU with a USB 1.1 host controller (using the OHCI
driver), using the same hub and the same codecs: works like a champ,
using less than 50% of the available bandwidth (observed with a
scope)
2) 1 audio stream is 32khz-mono, 16 bits = 64 kB/s,
4 codecs = 8 streams(R/W) x 64 kB/s = 512 kB/s (out of 1.5MB/s)

I noticed that my sound problem starts happening with only 2 codecs
(4 streams, 256 kB/s). I first thought that it was a bandwidth
limitation, so I decided to connect only 1 codec using more bandwidth.
I configured it to 48khz-stereo (16-bits), using 384 kB/s for both
read and write streams: no problem. With that configuration, the 
scope shows about 30% of total bandwidth usage (300us used out of 1ms
periods). Then, I added a second codec (48khz-stereo-16bits): very
strange, now the total bandwidth usage felt down to about 200us, which
seems to keep the same, whatever the number of codec I add (I also
tried 3 and 4...). So it looks like the scheduler is not able to
properly allocate Isochronous time slots when more than one device is
connected to the hub. However, without the hub, it works perfectly.

Another interresting fact is that at application level, the Read and
Write operations are returning the good amount of bytes read/written.
This is not the case at kernel level: I noticed that function
"usb_submit_urb" (from /drivers/usb/core/urb.c) will only tranfer
part of the "urbs" when the sound is degraded. I tried to figure out
where the leak comes from without success. Also, there are no error
messages from kernel so everything appears to work well, excepted
that part of the sound is missing!

I can't change my hardware (this is in the hand of customers), so
the only possible solution for me is to correct the software.

I tried to change my ehci driver with the one from kernel 2.6.39.4
but did not work, same problem.

Question:
-

Before attempting to upgrade to an earlier kernel driver (this is
a fairly big amount of work), I would really like to know if this
problem would still be in the 3.x kernels. Has anyone seen that
issue in 3.x kernels?

I am pretty new to USB driver debugging, so any ideas of where/how
to find solutions will be appreciated. Thank you very much in advance
for the support. Also don't hesitate to redirect me if I'm not at the
right place to ask these questions. I can also provide some code if
someone need it to help.

Attached is a dump of my "dmesg" after startup.

Michael Tessier







<5>Linux version 2.6.31-770-g0e46b52-0897 
(michael.tess...@vsvr-compile-01.pocatec.com) (gcc version 4.1.2) #12 PREEMPT 
Mon Nov 24 18:34:19 EST 2014
<4>CPU: ARMv7 Processor [412fc085] revision 5 (ARMv7), cr=10c53c7f
<4>CPU: VIPT nonaliasing data cache, VIPT nonaliasing instruction cache
<4>Machine: MX51 Xanthus Board
<4>Memory policy: ECC disabled, Data cache writeback
<7>On node 0 totalpages: 65536
<7>free_area_init_node: node 0, pgdat c02f137c, node_mem_map c0307000
<7>  DMA zone: 128 pages used for memmap
<7>  DMA zone: 0 pages reserved
<7>  DMA zone: 16256 pages, LIFO batch:3
<7>  Normal zone: 384 pages used for memmap
<7>  Normal zone: 48768 pages, LIFO batch:15
<4>Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 65024
<5>Kernel command line: console=ttymxc2,115200 CNFGmagic=XATS 
CNFGversion=1.0.0.0 vendorID=EXPT platformID=LINX productID=XATS 
mac1=00:1D:9E:00:17:2e mac2=00:1D:9E:00:17:2f XanthusPart=810

Re: chipidea hangs on mx51 running linux-next

2014-12-15 Thread Fabio Estevam
Hi Vladimir,

On Mon, Dec 15, 2014 at 6:23 PM, Vladimir Zapolskiy  wrote:

> Recently I was told by Mark Brown that this is an incorrect hack --- you
> should either create a regulator consumer or pass "regulator-always-on"
> property.

Exactly, I passed "regulator-always-on" in the official patch submission ;-)
http://www.spinics.net/lists/arm-kernel/msg385739.html

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: chipidea hangs on mx51 running linux-next

2014-12-15 Thread Vladimir Zapolskiy
Hi Fabio,

On 15.12.2014 17:05, Fabio Estevam wrote:
> On Tue, Dec 9, 2014 at 11:48 AM, Matthieu CASTET
>  wrote:
> 
>> Is your usb phy ok ?
>>
>> I saw freeze like that when there was no ulpi communication with the
>> usb phy.
> 
> Thanks. I fixed it by passing 'regulator-boot-on' to the USB regulators
> 
> --- a/arch/arm/boot/dts/imx51-babbage.dts
> +++ b/arch/arm/boot/dts/imx51-babbage.dts
> @@ -137,6 +137,7 @@
>  regulator-max-microvolt = <500>;
>  gpio = <&gpio2 5 GPIO_ACTIVE_HIGH>;
>  enable-active-high;
> +regulator-boot-on;
>  };
> 
>  reg_usbotg_vbus: regulator@1 {
> @@ -149,6 +150,7 @@
>  regulator-max-microvolt = <500>;
>  gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
>  enable-active-high;
> +regulator-boot-on;
>  };
>  };
> 

Recently I was told by Mark Brown that this is an incorrect hack --- you
should either create a regulator consumer or pass "regulator-always-on"
property.

>From Documentation/devicetree/bindings/regulator/regulator.txt:

  [snip]
  - regulator-boot-on: bootloader/firmware enabled regulator

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe 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: [GIT PULL] USB driver patches for 3.19-rc1

2014-12-15 Thread Linus Torvalds
On Mon, Dec 15, 2014 at 3:32 AM, Hans de Goede  wrote:
>
> Code wise this looks good and I've just given:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>
> a spin with an uas disk enclosure, and verified that tcq is being used,
> and everything works fine.

Thanks for verifying,

  Linus
--
To unsubscribe from this list: send the line "unsubscribe 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 v7 3/5] usb: dwc2: add generic PHY framework support for dwc2 usb controler platform driver.

2014-12-15 Thread Doug Anderson
Yunzhi,

On Fri, Dec 12, 2014 at 7:09 AM, Yunzhi Li  wrote:
> Get PHY parameters from devicetree and power off usb PHY during
> system suspend.
>
> Signed-off-by: Yunzhi Li 
> Acked-by: Paul Zimmerman 
>
> ---
>
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3:
> - Fix coding style: both branches of the if() which only one
>   branch of the conditional statement is a single statement
>   should have braces.
> - No need to test dwc2->phy for NULL before calling generic phy
>   APIs.
>
>  drivers/usb/dwc2/gadget.c   | 33 -
>  drivers/usb/dwc2/platform.c | 36 ++--
>  2 files changed, 46 insertions(+), 23 deletions(-)

On rk3288-pinky (on a 3.14 tree with backports), I can confirm that
this properly gets us into low power at suspend time.

Tested-by: Doug Anderson 
--
To unsubscribe from this list: send the line "unsubscribe 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 v7 2/5] phy: add a driver for the Rockchip SoC internal USB2.0 PHY

2014-12-15 Thread Doug Anderson
Yunzhi,

On Fri, Dec 12, 2014 at 7:07 AM, Yunzhi Li  wrote:
> This patch to add a generic PHY driver for ROCKCHIP usb PHYs,
> currently this driver can support RK3288. The RK3288 SoC have
> three independent USB PHY IPs which are all configured through a
> set of registers located in the GRF (general register files)
> module.
>
> Signed-off-by: Yunzhi Li 
>
> ---
>
> Changes in v7:
> - Accept Kishon's comments to use phandle args to find a phy
>   struct directly and get rid of using a custom of_xlate
>   function.
>
> Changes in v6:
> - Rename SIDDQ_MSK to SIDDQ_WRITE_ENA.
>
> Changes in v5: None
> Changes in v4:
> - Get number of PHYs from device tree.
> - Model each PHY as subnode of the phy provider node.
>
> Changes in v3:
> - Use BIT macro instead of bit shift ops.
> - Rename the config entry to PHY_ROCKCHIP_USB.
>
>  drivers/phy/Kconfig|   7 ++
>  drivers/phy/Makefile   |   1 +
>  drivers/phy/phy-rockchip-usb.c | 158 
> +
>  3 files changed, 166 insertions(+)
>  create mode 100644 drivers/phy/phy-rockchip-usb.c

On rk3288-pinky (on a 3.14 tree with backports), I can confirm that
this properly gets us into low power at suspend time.

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


Re: [PATCH 4/4] usb: atmel_usba_udc: mask status with enabled irqs

2014-12-15 Thread Boris Brezillon
On Mon, 15 Dec 2014 17:22:04 +
David Laight  wrote:

> From: Boris Brezillon
> > Hi David,
> > 
> > On Mon, 15 Dec 2014 13:34:56 +
> > David Laight  wrote:
> > 
> > > From: Sergei Shtylyov
> > > > Hello.
> > > >
> > > > On 12/15/2014 4:03 PM, Boris Brezillon wrote:
> > > >
> > > > > Avoid interpreting useless status flags when we're not waiting for 
> > > > > such
> > > > > events by masking the status variable with the interrupt enabled 
> > > > > register
> > > > > value.
> > > >
> > > > > Reported-by: Patrice VILCHEZ 
> > > > > Signed-off-by: Boris Brezillon 
> > > > > ---
> > > > >   drivers/usb/gadget/udc/atmel_usba_udc.c | 6 +-
> > > > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
> > > > > b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > > index 55c8dde..bc3a532 100644
> > > > > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > > @@ -1612,12 +1612,14 @@ static irqreturn_t usba_udc_irq(int irq, void 
> > > > > *devid)
> > > > >
> > > > >   spin_lock(&udc->lock);
> > > > >
> > > > > - status = usba_readl(udc, INT_STA);
> > > > > + status = usba_readl(udc, INT_STA) & usba_readl(udc, INT_ENB);
> ...
> > > > Looks like t make sense to read the INT_ENB register into a separate
> > > > variable, to save on extra reads?
> > >
> > >
> > > Better still remember the written value in one of the structures so
> > > that it doesn't have to be read at all.
> > 
> > Hmm, I'm getting back to this suggestion.
> > While I definitely understand why I should use a local variable to
> > store INT_ENB value in usba_udc_irq, I don't see the point of mirroring
> > INT_EN status in an udc struct field (after all, INT_EN will always
> > contain the value we previously set).
> 
> This is exactly why it makes sense to mirror it locally.

Absolutely.

> 
> > Is this a performance concern ?
> 
> Absolutely, you really don't want to know how many cpu cycles it is
> likely to take to do a read from an io device.
> At best it is a uncached read of a fast on-chip peripheral.
> If you are reading from a PCIe device then you are looking at hundreds
> (if not thousands) of cpu clock cycles.

I know there is a perf penalty when accessing IO memory regions (in this
case an uncached memory access) compared to standard memory accesses (in
other words a cached accesses), just don't know the exact numbers.
My point was, is the performance improvement worth the addition of this
new field and the code modification (addition of a wrapper function to
modify the interrupt register) ?

I take your answer as a yes ;-).

Thanks,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[no subject]

2014-12-15 Thread steve

1) Summary pasted
 summary: scanimage -L finds fitjitsu 04c5:11fc but if repeated does 
not find it

+ 04c5:11fc [System76 gazp9b] Excuting scanimage -L finds Fujitsu 6110
+ Scanner but if repeated does not


2) full description:
i plug a fitjitsu 04c5:11fc into a usb2.0 plug
lsusb shows the equipment
command line simple-scan, scanimage -L or sane-find_scanner does not find it

3) Keywords

4) kernel version 3.18-vivid

5) syslog:
Dec 14 12:53:48 skc kernel: [ 219.200113] usb 1-2: new high-speed USB 
device number 5 using xhci_hcd
Dec 14 12:53:48 skc kernel: [ 219.390380] usb 1-2: New USB device found, 
idVendor=04c5, idProduct=11fc
Dec 14 12:53:48 skc kernel: [ 219.390384] usb 1-2: New USB device 
strings: Mfr=0, Product=0, SerialNumber=0
Dec 14 12:53:48 skc kernel: [ 219.390563] usb 1-2: ep 0x81 - rounding 
interval to 128 microframes, ep desc says 255 microframes
Dec 14 12:53:48 skc kernel: [ 219.390567] usb 1-2: ep 0x2 - rounding 
interval to 128 microframes, ep desc says 255 microframes

Dec 14 12:53:48 skc colord: Device added: sysfs-04c5-11fc

6)
scanimage -L
simple-scan
sane-find-scanner
do not find any scanner

7) lsb_release -rd
Description:Ubuntu 14.04.1 LTS
Release:14.04

7.1) ver_linux
If some fields are empty or look unusual you may have an old version.
Compare to the current minimal requirements in Documentation/Changes.

Linux skc 3.18.0-031800-generic #201412071935 SMP Mon Dec 8 00:36:34 UTC 
2014 x86_64 x86_64 x86_64 GNU/Linux


Gnu C  4.8
Gnu make   3.81
binutils   2.24
util-linux 2.20.1
mount  support
module-init-tools  15
e2fsprogs  1.42.9
pcmciautils018
PPP2.4.5
Linux C Library2.19
Dynamic linker (ldd)   2.19
Procps 3.3.9
Net-tools  1.60
Kbd1.15.5
Sh-utils   8.21
wireless-tools 30
Modules Loaded xt_conntrack ipt_REJECT nf_reject_ipv4 
ip6table_filter ip6_tables ebtable_nat ebtables xt_CHECKSUM 
iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 ctr iptable_nat ccm 
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack 
xt_tcpudp bridge stp llc iptable_filter ip_tables x_tables rfcomm bnep 
arc4 intel_rapl x86_pkg_temp_thermal intel_powerclamp iwlmvm coretemp 
mac80211 kvm_intel kvm uvcvideo snd_hda_codec_via videobuf2_vmalloc 
snd_hda_codec_generic snd_hda_codec_hdmi crct10dif_pclmul 
videobuf2_memops iwlwifi videobuf2_core crc32_pclmul v4l2_common 
ghash_clmulni_intel videodev aesni_intel media aes_x86_64 cfg80211 
snd_hda_intel lrw btusb gf128mul joydev glue_helper bluetooth 
ablk_helper snd_hda_controller serio_raw cryptd rtsx_pci_ms 
snd_hda_codec memstick lpc_ich snd_hwdep mei_me i915 wmi mei snd_pcm 
tpm_infineon mac_hid snd_seq_midi snd_seq_midi_event snd_rawmidi video 
snd_seq drm_kms_helper snd_se
q_device drm snd_timer snd i2c_algo_bit soundcore ie31200_edac 
parport_pc edac_core ppdev lp parport rtsx_pci_sdmmc psmouse r8169 ahci 
libahci mii rtsx_pci



7,2) /proc/cpuinfo
processor: 0
vendor_id: GenuineIntel
cpu family: 6
model: 60
model name: Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz
stepping: 3
microcode: 0x17
cpu MHz: 2515.917
cache size: 6144 KB
physical id: 0
siblings: 8
core id: 0
cpu cores: 4
apicid: 0
initial apicid: 0
fpu: yes
fpu_exception: yes
cpuid level: 13
wp: yes
flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall 
nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl 
xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor 
ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 movbe 
popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm ida arat 
epb pln pts dtherm tpr_shadow vnmi flexpriority ept vpid fsgsbase 
tsc_adjust bmi1 avx2 smep bmi2 erms invpcid xsaveopt

bugs:
bogomips: 4988.89
clflush size: 64
cache_alignment: 64
address sizes: 39 bits physical, 48 bits virtual
power management:

processor: 1
vendor_id: GenuineIntel
cpu family: 6
model: 60
model name: Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz
stepping: 3
microcode: 0x17
cpu MHz: 2500.000
cache size: 6144 KB
physical id: 0
siblings: 8
core id: 1
cpu cores: 4
apicid: 2
initial apicid: 2
fpu: yes
fpu_exception: yes
cpuid level: 13
wp: yes
flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall 
nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl 
xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor 
ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 movbe 
popcnt tsc_deadline_timer aes xsa

RE: [PATCH 4/4] usb: atmel_usba_udc: mask status with enabled irqs

2014-12-15 Thread David Laight
From: Boris Brezillon
> Hi David,
> 
> On Mon, 15 Dec 2014 13:34:56 +
> David Laight  wrote:
> 
> > From: Sergei Shtylyov
> > > Hello.
> > >
> > > On 12/15/2014 4:03 PM, Boris Brezillon wrote:
> > >
> > > > Avoid interpreting useless status flags when we're not waiting for such
> > > > events by masking the status variable with the interrupt enabled 
> > > > register
> > > > value.
> > >
> > > > Reported-by: Patrice VILCHEZ 
> > > > Signed-off-by: Boris Brezillon 
> > > > ---
> > > >   drivers/usb/gadget/udc/atmel_usba_udc.c | 6 +-
> > > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
> > > > b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > index 55c8dde..bc3a532 100644
> > > > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > @@ -1612,12 +1612,14 @@ static irqreturn_t usba_udc_irq(int irq, void 
> > > > *devid)
> > > >
> > > > spin_lock(&udc->lock);
> > > >
> > > > -   status = usba_readl(udc, INT_STA);
> > > > +   status = usba_readl(udc, INT_STA) & usba_readl(udc, INT_ENB);
...
> > > Looks like t make sense to read the INT_ENB register into a separate
> > > variable, to save on extra reads?
> >
> >
> > Better still remember the written value in one of the structures so
> > that it doesn't have to be read at all.
> 
> Hmm, I'm getting back to this suggestion.
> While I definitely understand why I should use a local variable to
> store INT_ENB value in usba_udc_irq, I don't see the point of mirroring
> INT_EN status in an udc struct field (after all, INT_EN will always
> contain the value we previously set).

This is exactly why it makes sense to mirror it locally.

> Is this a performance concern ?

Absolutely, you really don't want to know how many cpu cycles it is
likely to take to do a read from an io device.
At best it is a uncached read of a fast on-chip peripheral.
If you are reading from a PCIe device then you are looking at hundreds
(if not thousands) of cpu clock cycles.

David



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


Re: [PATCH 4/4] usb: atmel_usba_udc: mask status with enabled irqs

2014-12-15 Thread Boris Brezillon
Hi David,

On Mon, 15 Dec 2014 13:34:56 +
David Laight  wrote:

> From: Sergei Shtylyov
> > Hello.
> > 
> > On 12/15/2014 4:03 PM, Boris Brezillon wrote:
> > 
> > > Avoid interpreting useless status flags when we're not waiting for such
> > > events by masking the status variable with the interrupt enabled register
> > > value.
> > 
> > > Reported-by: Patrice VILCHEZ 
> > > Signed-off-by: Boris Brezillon 
> > > ---
> > >   drivers/usb/gadget/udc/atmel_usba_udc.c | 6 +-
> > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
> > > b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > index 55c8dde..bc3a532 100644
> > > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > @@ -1612,12 +1612,14 @@ static irqreturn_t usba_udc_irq(int irq, void 
> > > *devid)
> > >
> > >   spin_lock(&udc->lock);
> > >
> > > - status = usba_readl(udc, INT_STA);
> > > + status = usba_readl(udc, INT_STA) & usba_readl(udc, INT_ENB);
> > >   DBG(DBG_INT, "irq, status=%#08x\n", status);
> > >
> > >   if (status & USBA_DET_SUSPEND) {
> > >   toggle_bias(udc, 0);
> > >   usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
> > > + usba_writel(udc, INT_ENB,
> > > + usba_readl(udc, INT_ENB) | USBA_WAKE_UP);
> > >   udc->bias_pulse_needed = true;
> > >   DBG(DBG_BUS, "Suspend detected\n");
> > >   if (udc->gadget.speed != USB_SPEED_UNKNOWN
> > > @@ -1631,6 +1633,8 @@ static irqreturn_t usba_udc_irq(int irq, void 
> > > *devid)
> > >   if (status & USBA_WAKE_UP) {
> > >   toggle_bias(udc, 1);
> > >   usba_writel(udc, INT_CLR, USBA_WAKE_UP);
> > > + usba_writel(udc, INT_ENB,
> > > + usba_readl(udc, INT_ENB) & ~USBA_WAKE_UP);
> > >   DBG(DBG_BUS, "Wake Up CPU detected\n");
> > >   }
> > 
> > Looks like t make sense to read the INT_ENB register into a separate
> > variable, to save on extra reads?
> 
> 
> Better still remember the written value in one of the structures so
> that it doesn't have to be read at all.

Hmm, I'm getting back to this suggestion.
While I definitely understand why I should use a local variable to
store INT_ENB value in usba_udc_irq, I don't see the point of mirroring
INT_EN status in an udc struct field (after all, INT_EN will always
contain the value we previously set).
Is this a performance concern ?

Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

2014-12-15 Thread Greg Kroah-Hartman
On Mon, Dec 15, 2014 at 04:53:05AM -0800, Jeremiah Mahler wrote:
> Johan,
> 
> On Mon, Dec 15, 2014 at 11:23:21AM +0100, Johan Hovold wrote:
> > On Thu, Dec 11, 2014 at 03:29:52PM -0800, Jeremiah Mahler wrote:
> > > If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> > > unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
> > > logs.  This patch set quiets these messages without changing the
> > > original behavior.
> > 
> > Don't unplug devices that are in use then. ;)
> > 
> I knew someone was going to say that :-)
> 
> > > This change is beneficial when using daemons such as slcand, which is
> > > similar to pppd or slip, that cannot determine whether they should exit
> > > until after the USB serial device is unplugged.  Producing these error
> > > messages for a normal use case is not helpful.
> > 
> > Your patches would hide these errors when they occur during normal use
> > (e.g. EPROTO).
> > 
> > Receiving an error message when unplugging an active device should not
> > surprise anyone. And at least you know where it came from (and it's
> > right there in the logs as well).
> > 
> > Johan
> 
> Hmm.  Yes, I can see why quieting -EPROTO would be bad because it would
> hide protocol errors which we want to know about.

Do you really want to "know about" them?  What can a user do with them?
Nothing, so just resubmit and you should be fine.

> I need to re-think this patch.
> Nack.

I like this patch, putting crud in the kernel log that no one can do
anything with for a "normal" operation like yanking a USB device out
while it is open should not happen.

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 0/2 resend] USB: host: Misc patches to remove hard-coded octeon platform information

2014-12-15 Thread Greg KH
On Mon, Dec 15, 2014 at 02:26:29PM +0100, Andreas Herrmann wrote:
> This is a re-submission of patches 2 and 3 from
> http://marc.info/?i=1415914590-31647-1-git-send-email-andreas.herrm...@caviumnetworks.com
> (Only patch 1/3 made it into usb-next and meanwhile is in mainline.)
> 
> Please apply.

I'll get to patches after 3.19-rc1 is out, can't do anything with my
trees until then, sorry.

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


Re: [PATCH] usb: gadget: udc-core: call udc_stop() before gadget unbind

2014-12-15 Thread Alan Stern
On Fri, 12 Dec 2014, Robert Baldyga wrote:

> As usb function drivers assumes that all usb request will be completed
> before function unbind call, we should supply such behavior. In some
> cases ep_disable() won't kill all request effectively, because some
> IN requests can be in running state. In such situation it's possible
> to have unbind function called before last request completion, which
> can cause problems.
> 
> For example unbinding f_ecm function while request on 'notify' endpoint
> is not completed, ends up NULL pointer dereference in unbind() function.
> 
> usb_gadget_udc_stop() call causes completion of all requests so if it's
> called before gadget unbind there is no risk that some of requests will
> stay uncompleted.
> 
> Signed-off-by: Robert Baldyga 
> ---
>  drivers/usb/gadget/udc/udc-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/udc-core.c 
> b/drivers/usb/gadget/udc/udc-core.c
> index e31d574..6f0d233 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -331,8 +331,8 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
>  
>   usb_gadget_disconnect(udc->gadget);
>   udc->driver->disconnect(udc->gadget);
> - udc->driver->unbind(udc->gadget);
>   usb_gadget_udc_stop(udc);
> + udc->driver->unbind(udc->gadget);
>  
>   udc->driver = NULL;
>   udc->dev.driver = NULL;

There has been a lot of churn and a lot of bug fixes involving those
lines of code.  Have you checked the git log for this function?  It's
quite possible that interchanging those two statements will recreate a
bug that has already been fixed.

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: serial: add nt124 usb to serial driver

2014-12-15 Thread George McCollister
On Mon, Dec 15, 2014 at 3:52 AM, Johan Hovold  wrote:
> On Sun, Dec 14, 2014 at 11:51:11AM -0600, George McCollister wrote:
>> Johan,
>>
>> While working on the tx_empty changes you suggested it occurred to me
>> that it might not be obvious to others that the firmware doesn't send
>> a packet with the NT124_CTRL_TXEMPTY flag cleared when it begins
>> transmitting. The practical implication is that if the driver sets
>> tx_empty = true when it sees NT124_CTRL_TXEMPTY, tx_empty must be
>> reset to false somewhere when more data is transmitted. Perhaps I
>> could add prepare_write_buffer and do it in there before calling
>> usb_serial_generic_prepare_write_buffer(). Does that sound acceptable?
>
> Hmm. There's no way to query that flag? And the status is sent (as bulk
> in data) periodically or only when data has been received? And not when
> the actual status changes?
The bulk in packets are not sent periodically only on TXEMPTY, other
line change or received data. There's no way to query the flag, though
we're still at the stage we can make modifications to the firmware if
there's justification. One of the design goals is to minimize
unnecessary USB traffic so if there's a place to clear the flag in the
driver without querying it would be preferable.
>
> A potential problem with using prepare_write_buffer would be on failures
> to submit the write urb, in which case this flag might never be cleared.
Yes, this could be a problem though I wonder how many (if any)
recoverable situations this would happen in that didn't eventually
lead to a transmission. Adding a timer with a long timeout that set
tx_empty  = true crossed my mind but that doesn't really seem any less
error prone than the original issue. I could of course duplicate a
bunch of code from generic.c and make a few minor changes to set
tx_empty = true but that obviously isn't desirable either.

If none of the above solutions sound acceptable, let me know I and I
guess I'll change the firmware to allow polling of TXEMPTY with a
control message and remove it from the bulk in packet.

Thanks again,
George

> Johan
--
To unsubscribe from this list: send the line "unsubscribe 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 resend] USB: host: Remove hard-coded octeon platform information for ehci/ohci

2014-12-15 Thread Ralf Baechle
On Mon, Dec 15, 2014 at 02:28:41PM +0100, Andreas Herrmann wrote:

> Instead rely on device tree information for ehci and ohci.
> 
> This was suggested with
> http://www.linux-mips.org/archives/linux-mips/2014-05/msg00307.html

Please use the permanent link from that page:

  
http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=1401358203-60225-4-git-send-email-alex.smith%40imgtec.com

The non-permanent links might change.

  Ralf
--
To unsubscribe from this list: send the line "unsubscribe 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: dwc2: gadget: kill requests with 'force' in s3c_hsotg_udc_stop()

2014-12-15 Thread Robert Baldyga
On 12/11/2014 08:34 PM, Paul Zimmerman wrote:
>> From: Robert Baldyga [mailto:r.bald...@samsung.com]
>> Sent: Tuesday, December 09, 2014 5:42 AM
>>
>> This makes us sure that all requests are completed before we unbind
>> gadget. There are assumptions in gadget API that all requests have to
>> be completed and leak of complete can break some usb function drivers.
>>
>> For example unbind of ECM function can cause NULL pointer dereference:
>>
>> [   26.396595] configfs-gadget gadget: unbind function
>> 'cdc_ethernet'/e79c4c00
>> [   26.414999] Unable to handle kernel NULL pointer dereference at
>> virtual address 
>> (...)
>> [   26.452223] PC is at ecm_unbind+0x6c/0x9c
>> [   26.456209] LR is at ecm_unbind+0x68/0x9c
>> (...)
>> [   26.603696] [] (ecm_unbind) from []
>> (purge_configs_funcs+0x94/0xd8)
>> [   26.611674] [] (purge_configs_funcs) from []
>> (configfs_composite_unbind+0x14/0x34)
>> [   26.620961] [] (configfs_composite_unbind) from
>> [] (usb_gadget_remove_driver+0x68/0x9c)
>> [   26.630683] [] (usb_gadget_remove_driver) from []
>> (usb_gadget_unregister_driver+0x64/0x94)
>> [   26.640664] [] (usb_gadget_unregister_driver) from
>> [] (unregister_gadget+0x20/0x3c)
>> [   26.650038] [] (unregister_gadget) from []
>> (gadget_dev_desc_UDC_store+0x80/0xb8)
>> [   26.659152] [] (gadget_dev_desc_UDC_store) from
>> [] (gadget_info_attr_store+0x1c/0x28)
>> [   26.668703] [] (gadget_info_attr_store) from []
>> (configfs_write_file+0xe8/0x148)
>> [   26.677818] [] (configfs_write_file) from []
>> (vfs_write+0xb0/0x1a0)
>> [   26.685801] [] (vfs_write) from []
>> (SyS_write+0x44/0x84)
>> [   26.692834] [] (SyS_write) from []
>> (ret_fast_syscall+0x0/0x30)
>> [   26.700381] Code: e30409f8 e34c0069 eb07b88d e59430a8 (e593)
>> [   26.706485] ---[ end trace f62a082b323838a2 ]---
>>
>> It's because in some cases request is still running on endpoint during
>> unbind and kill_all_requests() called from s3c_hsotg_udc_stop() function
>> doesn't cause call of complete() of request. Missing complete() call
>> causes ecm->notify_req equals NULL in ecm_unbind() function, and this
>> is reason of this bug.
>>
>> Similar breaks can be observed in another usb function drivers.
>>
>> This patch fixes this bug forcing usb request completion in when
>> s3c_hsotg_ep_disable() is called from s3c_hsotg_udc_stop().
>>
>> Signed-off-by: Robert Baldyga 
>> ---
>>  drivers/usb/dwc2/gadget.c | 10 +++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 8b5c079..cb4c925 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2590,7 +2590,7 @@ error:
>>   * s3c_hsotg_ep_disable - disable given endpoint
>>   * @ep: The endpoint to disable.
>>   */
>> -static int s3c_hsotg_ep_disable(struct usb_ep *ep)
>> +static int s3c_hsotg_ep_disable_force(struct usb_ep *ep, bool force)
>>  {
>>  struct s3c_hsotg_ep *hs_ep = our_ep(ep);
>>  struct s3c_hsotg *hsotg = hs_ep->parent;
>> @@ -2611,7 +2611,7 @@ static int s3c_hsotg_ep_disable(struct usb_ep *ep)
>>
>>  spin_lock_irqsave(&hsotg->lock, flags);
>>  /* terminate all requests with shutdown */
>> -kill_all_requests(hsotg, hs_ep, -ESHUTDOWN, false);
>> +kill_all_requests(hsotg, hs_ep, -ESHUTDOWN, force);
>>
>>  hsotg->fifo_map &= ~(1>  hs_ep->fifo_index = 0;
>> @@ -2632,6 +2632,10 @@ static int s3c_hsotg_ep_disable(struct usb_ep *ep)
>>  return 0;
>>  }
>>
>> +static int s3c_hsotg_ep_disable(struct usb_ep *ep)
>> +{
>> +return s3c_hsotg_ep_disable_force(ep, false);
>> +}
>>  /**
>>   * on_list - check request is on the given endpoint
>>   * @ep: The endpoint to check.
>> @@ -2933,7 +2937,7 @@ static int s3c_hsotg_udc_stop(struct usb_gadget 
>> *gadget,
>>
>>  /* all endpoints should be shutdown */
>>  for (ep = 1; ep < hsotg->num_of_eps; ep++)
>> -s3c_hsotg_ep_disable(&hsotg->eps[ep].ep);
>> +s3c_hsotg_ep_disable_force(&hsotg->eps[ep].ep, true);
>>
>>  spin_lock_irqsave(&hsotg->lock, flags);
>>  
> 
> Acked-by: Paul Zimmerman 
> 

One more thing which I have forgot to write: This patch makes sense only
with my patch to udc-core [1] which modifies order of calls in
usb_gadget_remove_driver() function. Hence this patch shouldn't be
applied before [1] will be accepted.

[1] https://lkml.org/lkml/2014/12/12/360

Best regards,
Robert Baldyga
--
To unsubscribe from this list: send the line "unsubscribe 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: chipidea hangs on mx51 running linux-next

2014-12-15 Thread Fabio Estevam
On Tue, Dec 9, 2014 at 11:48 AM, Matthieu CASTET
 wrote:

> Is your usb phy ok ?
>
> I saw freeze like that when there was no ulpi communication with the
> usb phy.

Thanks. I fixed it by passing 'regulator-boot-on' to the USB regulators

--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -137,6 +137,7 @@
 regulator-max-microvolt = <500>;
 gpio = <&gpio2 5 GPIO_ACTIVE_HIGH>;
 enable-active-high;
+regulator-boot-on;
 };

 reg_usbotg_vbus: regulator@1 {
@@ -149,6 +150,7 @@
 regulator-max-microvolt = <500>;
 gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
 enable-active-high;
+regulator-boot-on;
 };
 };

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


Re: [PATCH 4/4] usb: atmel_usba_udc: mask status with enabled irqs

2014-12-15 Thread Boris Brezillon
On Mon, 15 Dec 2014 13:34:56 +
David Laight  wrote:

> From: Sergei Shtylyov
> > Hello.
> > 
> > On 12/15/2014 4:03 PM, Boris Brezillon wrote:
> > 
> > > Avoid interpreting useless status flags when we're not waiting for such
> > > events by masking the status variable with the interrupt enabled register
> > > value.
> > 
> > > Reported-by: Patrice VILCHEZ 
> > > Signed-off-by: Boris Brezillon 
> > > ---
> > >   drivers/usb/gadget/udc/atmel_usba_udc.c | 6 +-
> > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
> > > b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > index 55c8dde..bc3a532 100644
> > > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > @@ -1612,12 +1612,14 @@ static irqreturn_t usba_udc_irq(int irq, void 
> > > *devid)
> > >
> > >   spin_lock(&udc->lock);
> > >
> > > - status = usba_readl(udc, INT_STA);
> > > + status = usba_readl(udc, INT_STA) & usba_readl(udc, INT_ENB);
> > >   DBG(DBG_INT, "irq, status=%#08x\n", status);
> > >
> > >   if (status & USBA_DET_SUSPEND) {
> > >   toggle_bias(udc, 0);
> > >   usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
> > > + usba_writel(udc, INT_ENB,
> > > + usba_readl(udc, INT_ENB) | USBA_WAKE_UP);
> > >   udc->bias_pulse_needed = true;
> > >   DBG(DBG_BUS, "Suspend detected\n");
> > >   if (udc->gadget.speed != USB_SPEED_UNKNOWN
> > > @@ -1631,6 +1633,8 @@ static irqreturn_t usba_udc_irq(int irq, void 
> > > *devid)
> > >   if (status & USBA_WAKE_UP) {
> > >   toggle_bias(udc, 1);
> > >   usba_writel(udc, INT_CLR, USBA_WAKE_UP);
> > > + usba_writel(udc, INT_ENB,
> > > + usba_readl(udc, INT_ENB) & ~USBA_WAKE_UP);
> > >   DBG(DBG_BUS, "Wake Up CPU detected\n");
> > >   }
> > 
> > Looks like t make sense to read the INT_ENB register into a separate
> > variable, to save on extra reads?
> 
> 
> Better still remember the written value in one of the structures so
> that it doesn't have to be read at all.

Sure, I'll modify the code accordingly.

Thanks,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2 resend] USB: host: Remove hard-coded octeon platform information for ehci/ohci

2014-12-15 Thread Andreas Herrmann
Instead rely on device tree information for ehci and ohci.

This was suggested with
http://www.linux-mips.org/archives/linux-mips/2014-05/msg00307.html

  "The device tree will *always* have correct ehci/ohci clock
  configuration, so use it.  This allows us to remove a big chunk of
  platform configuration code from octeon-platform.c."

More or less I rebased that patch on Alan's work to remove ehci-octeon
and ohci-octeon drivers.

Cc: David Daney 
Cc: Alex Smith 
Cc: Alan Stern 
Signed-off-by: Andreas Herrmann 
Acked-by: Ralf Baechle 
Tested-by: Aaro Koskinen 
---
 arch/mips/cavium-octeon/octeon-platform.c |  148 -
 drivers/usb/host/ehci-platform.c  |1 +
 drivers/usb/host/ohci-platform.c  |1 +
 3 files changed, 64 insertions(+), 86 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-platform.c 
b/arch/mips/cavium-octeon/octeon-platform.c
index b67ddf0..eea60b6 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -77,7 +77,7 @@ static DEFINE_MUTEX(octeon2_usb_clocks_mutex);
 
 static int octeon2_usb_clock_start_cnt;
 
-static void octeon2_usb_clocks_start(void)
+static void octeon2_usb_clocks_start(struct device *dev)
 {
u64 div;
union cvmx_uctlx_if_ena if_ena;
@@ -86,6 +86,8 @@ static void octeon2_usb_clocks_start(void)
union cvmx_uctlx_uphy_portx_ctl_status port_ctl_status;
int i;
unsigned long io_clk_64_to_ns;
+   u32 clock_rate = 1200;
+   bool is_crystal_clock = false;
 
 
mutex_lock(&octeon2_usb_clocks_mutex);
@@ -96,6 +98,28 @@ static void octeon2_usb_clocks_start(void)
 
io_clk_64_to_ns = 640ull / octeon_get_io_clock_rate();
 
+   if (dev->of_node) {
+   struct device_node *uctl_node;
+   const char *clock_type;
+
+   uctl_node = of_get_parent(dev->of_node);
+   if (!uctl_node) {
+   dev_err(dev, "No UCTL device node\n");
+   goto exit;
+   }
+   i = of_property_read_u32(uctl_node,
+"refclk-frequency", &clock_rate);
+   if (i) {
+   dev_err(dev, "No UCTL \"refclk-frequency\"\n");
+   goto exit;
+   }
+   i = of_property_read_string(uctl_node,
+   "refclk-type", &clock_type);
+
+   if (!i && strcmp("crystal", clock_type) == 0)
+   is_crystal_clock = true;
+   }
+
/*
 * Step 1: Wait for voltages stable.  That surely happened
 * before starting the kernel.
@@ -126,9 +150,22 @@ static void octeon2_usb_clocks_start(void)
cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk_rst_ctl.u64);
 
/* 3b */
-   /* 12MHz crystal. */
-   clk_rst_ctl.s.p_refclk_sel = 0;
-   clk_rst_ctl.s.p_refclk_div = 0;
+   clk_rst_ctl.s.p_refclk_sel = is_crystal_clock ? 0 : 1;
+   switch (clock_rate) {
+   default:
+   pr_err("Invalid UCTL clock rate of %u, using 1200 
instead\n",
+   clock_rate);
+   /* Fall through */
+   case 1200:
+   clk_rst_ctl.s.p_refclk_div = 0;
+   break;
+   case 2400:
+   clk_rst_ctl.s.p_refclk_div = 1;
+   break;
+   case 4800:
+   clk_rst_ctl.s.p_refclk_div = 2;
+   break;
+   }
cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk_rst_ctl.u64);
 
/* 3c */
@@ -259,7 +296,7 @@ static void octeon2_usb_clocks_stop(void)
 
 static int octeon_ehci_power_on(struct platform_device *pdev)
 {
-   octeon2_usb_clocks_start();
+   octeon2_usb_clocks_start(&pdev->dev);
return 0;
 }
 
@@ -277,11 +314,11 @@ static struct usb_ehci_pdata octeon_ehci_pdata = {
.power_off  = octeon_ehci_power_off,
 };
 
-static void __init octeon_ehci_hw_start(void)
+static void __init octeon_ehci_hw_start(struct device *dev)
 {
union cvmx_uctlx_ehci_ctl ehci_ctl;
 
-   octeon2_usb_clocks_start();
+   octeon2_usb_clocks_start(dev);
 
ehci_ctl.u64 = cvmx_read_csr(CVMX_UCTLX_EHCI_CTL(0));
/* Use 64-bit addressing. */
@@ -299,59 +336,28 @@ static u64 octeon_ehci_dma_mask = DMA_BIT_MASK(64);
 static int __init octeon_ehci_device_init(void)
 {
struct platform_device *pd;
+   struct device_node *ehci_node;
int ret = 0;
 
-   struct resource usb_resources[] = {
-   {
-   .flags  = IORESOURCE_MEM,
-   }, {
-   .flags  = IORESOURCE_IRQ,
-   }
-   };
-
-   /* Only Octeon2 has ehci/ohci */
-   if (!OCTEON_IS_MODEL(OCTEON_CN63XX))
+   ehci_node = of_find_node_by_name(NULL, "ehci");
+   if (!ehci_node)
return 0;
 
-   if (octeon_is_simulation() || usb_disab

RE: [PATCH 4/4] usb: atmel_usba_udc: mask status with enabled irqs

2014-12-15 Thread David Laight
From: Sergei Shtylyov
> Hello.
> 
> On 12/15/2014 4:03 PM, Boris Brezillon wrote:
> 
> > Avoid interpreting useless status flags when we're not waiting for such
> > events by masking the status variable with the interrupt enabled register
> > value.
> 
> > Reported-by: Patrice VILCHEZ 
> > Signed-off-by: Boris Brezillon 
> > ---
> >   drivers/usb/gadget/udc/atmel_usba_udc.c | 6 +-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
> > b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > index 55c8dde..bc3a532 100644
> > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > @@ -1612,12 +1612,14 @@ static irqreturn_t usba_udc_irq(int irq, void 
> > *devid)
> >
> > spin_lock(&udc->lock);
> >
> > -   status = usba_readl(udc, INT_STA);
> > +   status = usba_readl(udc, INT_STA) & usba_readl(udc, INT_ENB);
> > DBG(DBG_INT, "irq, status=%#08x\n", status);
> >
> > if (status & USBA_DET_SUSPEND) {
> > toggle_bias(udc, 0);
> > usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
> > +   usba_writel(udc, INT_ENB,
> > +   usba_readl(udc, INT_ENB) | USBA_WAKE_UP);
> > udc->bias_pulse_needed = true;
> > DBG(DBG_BUS, "Suspend detected\n");
> > if (udc->gadget.speed != USB_SPEED_UNKNOWN
> > @@ -1631,6 +1633,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> > if (status & USBA_WAKE_UP) {
> > toggle_bias(udc, 1);
> > usba_writel(udc, INT_CLR, USBA_WAKE_UP);
> > +   usba_writel(udc, INT_ENB,
> > +   usba_readl(udc, INT_ENB) & ~USBA_WAKE_UP);
> > DBG(DBG_BUS, "Wake Up CPU detected\n");
> > }
> 
> Looks like t make sense to read the INT_ENB register into a separate
> variable, to save on extra reads?


Better still remember the written value in one of the structures so
that it doesn't have to be read at all.

David



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


Re: [PATCH 4/4] usb: atmel_usba_udc: mask status with enabled irqs

2014-12-15 Thread Sergei Shtylyov

Hello.

On 12/15/2014 4:03 PM, Boris Brezillon wrote:


Avoid interpreting useless status flags when we're not waiting for such
events by masking the status variable with the interrupt enabled register
value.



Reported-by: Patrice VILCHEZ 
Signed-off-by: Boris Brezillon 
---
  drivers/usb/gadget/udc/atmel_usba_udc.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)



diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 55c8dde..bc3a532 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1612,12 +1612,14 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)

spin_lock(&udc->lock);

-   status = usba_readl(udc, INT_STA);
+   status = usba_readl(udc, INT_STA) & usba_readl(udc, INT_ENB);
DBG(DBG_INT, "irq, status=%#08x\n", status);

if (status & USBA_DET_SUSPEND) {
toggle_bias(udc, 0);
usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
+   usba_writel(udc, INT_ENB,
+   usba_readl(udc, INT_ENB) | USBA_WAKE_UP);
udc->bias_pulse_needed = true;
DBG(DBG_BUS, "Suspend detected\n");
if (udc->gadget.speed != USB_SPEED_UNKNOWN
@@ -1631,6 +1633,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (status & USBA_WAKE_UP) {
toggle_bias(udc, 1);
usba_writel(udc, INT_CLR, USBA_WAKE_UP);
+   usba_writel(udc, INT_ENB,
+   usba_readl(udc, INT_ENB) & ~USBA_WAKE_UP);
DBG(DBG_BUS, "Wake Up CPU detected\n");
}


   Looks like t make sense to read the INT_ENB register into a separate 
variable, to save on extra reads?


WBR, Sergei

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


[PATCH 2/2 resend] USB: host: Introduce flag to enable use of 64-bit dma_mask for ehci-platform

2014-12-15 Thread Andreas Herrmann

ehci-octeon driver used a 64-bit dma_mask. With removal of ehci-octeon
and usage of ehci-platform ehci dma_mask is now limited to 32 bits
(coerced in ehci_platform_probe).

Provide a flag in ehci platform data to allow use of 64 bits for
dma_mask.

Cc: David Daney 
Cc: Alex Smith 
Cc: Alan Stern 
Signed-off-by: Andreas Herrmann 
Tested-by: Aaro Koskinen 
---
 arch/mips/cavium-octeon/octeon-platform.c |4 +---
 drivers/usb/host/ehci-platform.c  |3 ++-
 include/linux/usb/ehci_pdriver.h  |1 +
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-platform.c 
b/arch/mips/cavium-octeon/octeon-platform.c
index eea60b6..12410a2 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -310,6 +310,7 @@ static struct usb_ehci_pdata octeon_ehci_pdata = {
 #ifdef __BIG_ENDIAN
.big_endian_mmio= 1,
 #endif
+   .dma_mask_64= 1,
.power_on   = octeon_ehci_power_on,
.power_off  = octeon_ehci_power_off,
 };
@@ -331,8 +332,6 @@ static void __init octeon_ehci_hw_start(struct device *dev)
octeon2_usb_clocks_stop();
 }
 
-static u64 octeon_ehci_dma_mask = DMA_BIT_MASK(64);
-
 static int __init octeon_ehci_device_init(void)
 {
struct platform_device *pd;
@@ -347,7 +346,6 @@ static int __init octeon_ehci_device_init(void)
if (!pd)
return 0;
 
-   pd->dev.dma_mask = &octeon_ehci_dma_mask;
pd->dev.platform_data = &octeon_ehci_pdata;
octeon_ehci_hw_start(&pd->dev);
 
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 5b8533f..37abbe2 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -155,7 +155,8 @@ static int ehci_platform_probe(struct platform_device *dev)
if (!pdata)
pdata = &ehci_platform_defaults;
 
-   err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
+   err = dma_coerce_mask_and_coherent(&dev->dev,
+   pdata->dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
if (err)
return err;
 
diff --git a/include/linux/usb/ehci_pdriver.h b/include/linux/usb/ehci_pdriver.h
index 7eb4dcd..f69529e 100644
--- a/include/linux/usb/ehci_pdriver.h
+++ b/include/linux/usb/ehci_pdriver.h
@@ -45,6 +45,7 @@ struct usb_ehci_pdata {
unsignedbig_endian_desc:1;
unsignedbig_endian_mmio:1;
unsignedno_io_watchdog:1;
+   unsigneddma_mask_64:1;
 
/* Turn on all power and clocks */
int (*power_on)(struct platform_device *pdev);
-- 
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


[PATCH 0/2 resend] USB: host: Misc patches to remove hard-coded octeon platform information

2014-12-15 Thread Andreas Herrmann
This is a re-submission of patches 2 and 3 from
http://marc.info/?i=1415914590-31647-1-git-send-email-andreas.herrm...@caviumnetworks.com
(Only patch 1/3 made it into usb-next and meanwhile is in mainline.)

Please apply.


Thanks,

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


[PATCH 3/4] ARM: at91/dt: update udc compatible strings

2014-12-15 Thread Boris Brezillon
at91sam9g45, at91sam9x5 and sama5 SoCs should not use
"atmel,at91sam9rl-udc" for their USB device compatible property since
this compatible is attached to a specific hardware bug fix.

Signed-off-by: Boris Brezillon 
---
 arch/arm/boot/dts/at91sam9g45.dtsi | 2 +-
 arch/arm/boot/dts/at91sam9x5.dtsi  | 2 +-
 arch/arm/boot/dts/sama5d3.dtsi | 2 +-
 arch/arm/boot/dts/sama5d4.dtsi | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi 
b/arch/arm/boot/dts/at91sam9g45.dtsi
index 6c0637a..cf53155 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -1112,7 +1112,7 @@
usb2: gadget@fff78000 {
#address-cells = <1>;
#size-cells = <0>;
-   compatible = "atmel,at91sam9rl-udc";
+   compatible = "atmel,at91sam9g45-udc";
reg = <0x0060 0x8
   0xfff78000 0x400>;
interrupts = <27 IRQ_TYPE_LEVEL_HIGH 0>;
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi 
b/arch/arm/boot/dts/at91sam9x5.dtsi
index bbb3ba6..d213147 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -1057,7 +1057,7 @@
usb2: gadget@f803c000 {
#address-cells = <1>;
#size-cells = <0>;
-   compatible = "atmel,at91sam9rl-udc";
+   compatible = "atmel,at91sam9g45-udc";
reg = <0x0050 0x8
   0xf803c000 0x400>;
interrupts = <23 IRQ_TYPE_LEVEL_HIGH 0>;
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 5f4144d..2b407a4 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -1264,7 +1264,7 @@
usb0: gadget@0050 {
#address-cells = <1>;
#size-cells = <0>;
-   compatible = "atmel,at91sam9rl-udc";
+   compatible = "atmel,sama5d3-udc";
reg = <0x0050 0x10
   0xf803 0x4000>;
interrupts = <33 IRQ_TYPE_LEVEL_HIGH 2>;
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index e0157b0..0896efe 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -111,7 +111,7 @@
usb0: gadget@0040 {
#address-cells = <1>;
#size-cells = <0>;
-   compatible = "atmel,at91sam9rl-udc";
+   compatible = "atmel,sama5d3-udc";
reg = <0x0040 0x10
   0xfc02c000 0x4000>;
interrupts = <47 IRQ_TYPE_LEVEL_HIGH 2>;
-- 
1.9.1

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


[PATCH 4/4] usb: atmel_usba_udc: mask status with enabled irqs

2014-12-15 Thread Boris Brezillon
Avoid interpreting useless status flags when we're not waiting for such
events by masking the status variable with the interrupt enabled register
value.

Reported-by: Patrice VILCHEZ 
Signed-off-by: Boris Brezillon 
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 55c8dde..bc3a532 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1612,12 +1612,14 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 
spin_lock(&udc->lock);
 
-   status = usba_readl(udc, INT_STA);
+   status = usba_readl(udc, INT_STA) & usba_readl(udc, INT_ENB);
DBG(DBG_INT, "irq, status=%#08x\n", status);
 
if (status & USBA_DET_SUSPEND) {
toggle_bias(udc, 0);
usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
+   usba_writel(udc, INT_ENB,
+   usba_readl(udc, INT_ENB) | USBA_WAKE_UP);
udc->bias_pulse_needed = true;
DBG(DBG_BUS, "Suspend detected\n");
if (udc->gadget.speed != USB_SPEED_UNKNOWN
@@ -1631,6 +1633,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (status & USBA_WAKE_UP) {
toggle_bias(udc, 1);
usba_writel(udc, INT_CLR, USBA_WAKE_UP);
+   usba_writel(udc, INT_ENB,
+   usba_readl(udc, INT_ENB) & ~USBA_WAKE_UP);
DBG(DBG_BUS, "Wake Up CPU detected\n");
}
 
-- 
1.9.1

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


[PATCH 2/4] usb: atmel_usba_udc: Add at91sam9g45 and at91sam9x5 errata handling

2014-12-15 Thread Boris Brezillon
at91sam9g45 and at91sam9x5 SoCs have an hardware bug forcing us to
generate a pulse on the BIAS signal on "USB end of reset” and
“USB end of resume" events.

Reported-by: Patrice VILCHEZ 
Signed-off-by: Boris Brezillon 
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 28 +++-
 drivers/usb/gadget/udc/atmel_usba_udc.h |  2 ++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 36fd34b..55c8dde 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -331,6 +331,17 @@ static void toggle_bias(struct usba_udc *udc, int is_on)
udc->errata->toggle_bias(udc, is_on);
 }
 
+static void generate_bias_pulse(struct usba_udc *udc)
+{
+   if (!udc->bias_pulse_needed)
+   return;
+
+   if (udc->errata && udc->errata->pulse_bias)
+   udc->errata->pulse_bias(udc);
+
+   udc->bias_pulse_needed = false;
+}
+
 static void next_fifo_transaction(struct usba_ep *ep, struct usba_request *req)
 {
unsigned int transaction_len;
@@ -1607,6 +1618,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (status & USBA_DET_SUSPEND) {
toggle_bias(udc, 0);
usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
+   udc->bias_pulse_needed = true;
DBG(DBG_BUS, "Suspend detected\n");
if (udc->gadget.speed != USB_SPEED_UNKNOWN
&& udc->driver && udc->driver->suspend) {
@@ -1624,6 +1636,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 
if (status & USBA_END_OF_RESUME) {
usba_writel(udc, INT_CLR, USBA_END_OF_RESUME);
+   generate_bias_pulse(udc);
DBG(DBG_BUS, "Resume detected\n");
if (udc->gadget.speed != USB_SPEED_UNKNOWN
&& udc->driver && udc->driver->resume) {
@@ -1659,6 +1672,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
struct usba_ep *ep0;
 
usba_writel(udc, INT_CLR, USBA_END_OF_RESET);
+   generate_bias_pulse(udc);
reset_all_endpoints(udc);
 
if (udc->gadget.speed != USB_SPEED_UNKNOWN && udc->driver) {
@@ -1818,13 +1832,25 @@ static void at91sam9rl_toggle_bias(struct usba_udc 
*udc, int is_on)
at91_pmc_write(AT91_CKGR_UCKR, uckr & ~(AT91_PMC_BIASEN));
 }
 
+static void at91sam9g45_pulse_bias(struct usba_udc *udc)
+{
+   unsigned int uckr = at91_pmc_read(AT91_CKGR_UCKR);
+
+   at91_pmc_write(AT91_CKGR_UCKR, uckr & ~(AT91_PMC_BIASEN));
+   at91_pmc_write(AT91_CKGR_UCKR, uckr | AT91_PMC_BIASEN);
+}
+
 static const struct usba_udc_errata at91sam9rl_errata = {
.toggle_bias = at91sam9rl_toggle_bias,
 };
 
+static const struct usba_udc_errata at91sam9g45_errata = {
+   .pulse_bias = at91sam9g45_pulse_bias,
+};
+
 static const struct of_device_id atmel_udc_dt_ids[] = {
{ .compatible = "atmel,at91sam9rl-udc", .data = &at91sam9rl_errata },
-   { .compatible = "atmel,at91sam9g45-udc" },
+   { .compatible = "atmel,at91sam9g45-udc", .data = &at91sam9g45_errata },
{ .compatible = "atmel,sama5d3-udc" },
{ /* sentinel */ }
 };
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h 
b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 456899e..72b3537 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -306,6 +306,7 @@ struct usba_request {
 
 struct usba_udc_errata {
void (*toggle_bias)(struct usba_udc *udc, int is_on);
+   void (*pulse_bias)(struct usba_udc *udc);
 };
 
 struct usba_udc {
@@ -326,6 +327,7 @@ struct usba_udc {
struct clk *pclk;
struct clk *hclk;
struct usba_ep *usba_ep;
+   bool bias_pulse_needed;
 
u16 devstatus;
 
-- 
1.9.1

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


[PATCH 0/4] usb: atmel_usba_udc: Rework errata handling

2014-12-15 Thread Boris Brezillon
Hello,

Here is a set of patches porting existing at91sam9rl erratum handling to
DT and adding new code to handle at91sam9g45/9x5 erratum.
It also adds several compatible strings to differentiate those errata.

These patches should be backported to 3.17 and 3.18 stable releases but
they do not apply cleanly on those stable branches.
I'll send a backport of this series once it is merged in mainline.

Regards,

Boris


Boris Brezillon (4):
  usb: atmel_usba_udc: Rework at91sam9rl errata handling
  usb: atmel_usba_udc: Add at91sam9g45 and at91sam9x5 errata handling
  ARM: at91/dt: update udc compatible strings
  usb: atmel_usba_udc: mask status with enabled irqs

 .../devicetree/bindings/usb/atmel-usb.txt  |   5 +-
 arch/arm/boot/dts/at91sam9g45.dtsi |   2 +-
 arch/arm/boot/dts/at91sam9x5.dtsi  |   2 +-
 arch/arm/boot/dts/sama5d3.dtsi |   2 +-
 arch/arm/boot/dts/sama5d4.dtsi |   2 +-
 drivers/usb/gadget/udc/atmel_usba_udc.c| 104 ++---
 drivers/usb/gadget/udc/atmel_usba_udc.h|   7 ++
 7 files changed, 86 insertions(+), 38 deletions(-)

-- 
1.9.1

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


[PATCH 1/4] usb: atmel_usba_udc: Rework at91sam9rl errata handling

2014-12-15 Thread Boris Brezillon
at91sam9rl SoC has an erratum forcing us to toggle the BIAS on USB
suspend/resume events.

This specific handling is only activated when CONFIG_ARCH_AT91SAM9RL is
set and this option is only set when building a non-DT kernel, which is
problematic since non-DT support for at91sam9rl SoC has been removed.

Rework the toggle_bias implementation to attach it to the "at91sam9rl-udc"
compatible string.

Add new compatible strings to avoid executing at91sam9rl erratum handling
on other SoCs.

Signed-off-by: Boris Brezillon 
---
 .../devicetree/bindings/usb/atmel-usb.txt  |  5 +-
 drivers/usb/gadget/udc/atmel_usba_udc.c| 78 --
 drivers/usb/gadget/udc/atmel_usba_udc.h|  5 ++
 3 files changed, 52 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/atmel-usb.txt 
b/Documentation/devicetree/bindings/usb/atmel-usb.txt
index bcc..38fee0f 100644
--- a/Documentation/devicetree/bindings/usb/atmel-usb.txt
+++ b/Documentation/devicetree/bindings/usb/atmel-usb.txt
@@ -51,7 +51,10 @@ usb1: gadget@fffa4000 {
 Atmel High-Speed USB device controller
 
 Required properties:
- - compatible: Should be "atmel,at91sam9rl-udc"
+ - compatible: Should be one of the following
+  "at91sam9rl-udc"
+  "at91sam9g45-udc"
+  "sama5d3-udc"
  - reg: Address and length of the register set for the device
  - interrupts: Should contain usba interrupt
  - ep childnode: To specify the number of endpoints and their properties.
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index ce88237..36fd34b 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -8,6 +8,7 @@
  * published by the Free Software Foundation.
  */
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -324,28 +325,12 @@ static int vbus_is_present(struct usba_udc *udc)
return 1;
 }
 
-#if defined(CONFIG_ARCH_AT91SAM9RL)
-
-#include 
-
-static void toggle_bias(int is_on)
-{
-   unsigned int uckr = at91_pmc_read(AT91_CKGR_UCKR);
-
-   if (is_on)
-   at91_pmc_write(AT91_CKGR_UCKR, uckr | AT91_PMC_BIASEN);
-   else
-   at91_pmc_write(AT91_CKGR_UCKR, uckr & ~(AT91_PMC_BIASEN));
-}
-
-#else
-
-static void toggle_bias(int is_on)
+static void toggle_bias(struct usba_udc *udc, int is_on)
 {
+   if (udc->errata && udc->errata->toggle_bias)
+   udc->errata->toggle_bias(udc, is_on);
 }
 
-#endif /* CONFIG_ARCH_AT91SAM9RL */
-
 static void next_fifo_transaction(struct usba_ep *ep, struct usba_request *req)
 {
unsigned int transaction_len;
@@ -1620,7 +1605,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
DBG(DBG_INT, "irq, status=%#08x\n", status);
 
if (status & USBA_DET_SUSPEND) {
-   toggle_bias(0);
+   toggle_bias(udc, 0);
usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
DBG(DBG_BUS, "Suspend detected\n");
if (udc->gadget.speed != USB_SPEED_UNKNOWN
@@ -1632,7 +1617,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
}
 
if (status & USBA_WAKE_UP) {
-   toggle_bias(1);
+   toggle_bias(udc, 1);
usba_writel(udc, INT_CLR, USBA_WAKE_UP);
DBG(DBG_BUS, "Wake Up CPU detected\n");
}
@@ -1736,13 +1721,13 @@ static irqreturn_t usba_vbus_irq(int irq, void *devid)
vbus = vbus_is_present(udc);
if (vbus != udc->vbus_prev) {
if (vbus) {
-   toggle_bias(1);
+   toggle_bias(udc, 1);
usba_writel(udc, CTRL, USBA_ENABLE_MASK);
usba_writel(udc, INT_ENB, USBA_END_OF_RESET);
} else {
udc->gadget.speed = USB_SPEED_UNKNOWN;
reset_all_endpoints(udc);
-   toggle_bias(0);
+   toggle_bias(udc, 0);
usba_writel(udc, CTRL, USBA_DISABLE_MASK);
if (udc->driver->disconnect) {
spin_unlock(&udc->lock);
@@ -1788,7 +1773,7 @@ static int atmel_usba_start(struct usb_gadget *gadget,
/* If Vbus is present, enable the controller and wait for reset */
spin_lock_irqsave(&udc->lock, flags);
if (vbus_is_present(udc) && udc->vbus_prev == 0) {
-   toggle_bias(1);
+   toggle_bias(udc, 1);
usba_writel(udc, CTRL, USBA_ENABLE_MASK);
usba_writel(udc, INT_ENB, USBA_END_OF_RESET);
}
@@ -1811,7 +1796,7 @@ static int atmel_usba_stop(struct usb_gadget *gadget)
spin_unlock_irqrestore(&udc->lock, flags);
 
/* This will also disable the DP pullup */
-   toggle_bias(0);
+   toggle_bias(udc, 0);
usba_writel(udc, CTRL, USBA_DISABLE_MASK);
 
clk_disable_unprepare(udc->hclk);
@@ 

Re: [PATCH] usb: gadget: hid: consistently use 2^n - 1 for max values

2014-12-15 Thread Geert Uytterhoeven
On Mon, Dec 15, 2014 at 1:50 PM, Andrzej Pietrasiewicz
 wrote:
> A maximum value which fits in 16 bits, unsigned, is 65535.
>
> Signed-off-by: Andrzej Pietrasiewicz 

Acked-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

2014-12-15 Thread Jeremiah Mahler
Johan,

On Mon, Dec 15, 2014 at 11:23:21AM +0100, Johan Hovold wrote:
> On Thu, Dec 11, 2014 at 03:29:52PM -0800, Jeremiah Mahler wrote:
> > If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> > unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
> > logs.  This patch set quiets these messages without changing the
> > original behavior.
> 
> Don't unplug devices that are in use then. ;)
> 
I knew someone was going to say that :-)

> > This change is beneficial when using daemons such as slcand, which is
> > similar to pppd or slip, that cannot determine whether they should exit
> > until after the USB serial device is unplugged.  Producing these error
> > messages for a normal use case is not helpful.
> 
> Your patches would hide these errors when they occur during normal use
> (e.g. EPROTO).
> 
> Receiving an error message when unplugging an active device should not
> surprise anyone. And at least you know where it came from (and it's
> right there in the logs as well).
> 
> Johan

Hmm.  Yes, I can see why quieting -EPROTO would be bad because it would
hide protocol errors which we want to know about.

I need to re-think this patch.
Nack.

Thanks for the review,
-- 
- Jeremiah Mahler
--
To unsubscribe from this list: send the line "unsubscribe 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: hid: consistently use 2^n - 1 for max values

2014-12-15 Thread Andrzej Pietrasiewicz
A maximum value which fits in 16 bits, unsigned, is 65535.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/f_hid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index 488ac66..005c7d2 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -758,7 +758,7 @@ static struct f_hid_opts_attribute f_hid_opts_##name =  
\
 
 F_HID_OPT(subclass, 8, 255);
 F_HID_OPT(protocol, 8, 255);
-F_HID_OPT(report_length, 16, 65536);
+F_HID_OPT(report_length, 16, 65535);
 
 static ssize_t f_hid_opts_report_desc_show(struct f_hid_opts *opts, char *page)
 {
-- 
1.9.1

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


Re: [PATCH 3/3] mfd: dln2: add suspend/resume functionality

2014-12-15 Thread Johan Hovold
On Thu, Dec 11, 2014 at 03:02:31PM +0200, Octavian Purdila wrote:
> Without suspend/resume functionality in the USB driver the USB core
> will disconnect and reconnect the DLN2 port and because the GPIO
> framework does not yet support removal of an in-use controller a
> suspend/resume operation will result in a crash.
> 
> This patch provides suspend and resume functions for the DLN2 driver
> so that the above scenario is avoided.
>
> Signed-off-by: Octavian Purdila 
> ---
>  drivers/mfd/dln2.c | 41 ++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
> index 6d49685..08c403c 100644
> --- a/drivers/mfd/dln2.c
> +++ b/drivers/mfd/dln2.c
> @@ -587,7 +587,6 @@ static void dln2_free_rx_urbs(struct dln2_dev *dln2)
>   int i;
>  
>   for (i = 0; i < DLN2_MAX_URBS; i++) {
> - usb_kill_urb(dln2->rx_urb[i]);
>   usb_free_urb(dln2->rx_urb[i]);
>   kfree(dln2->rx_buf[i]);
>   }

Now dln2_free will no longer stop the urbs before releasing them and
hence you can get a use after free in the error path of probe where
dln2_free is called after you have submitted the urbs.

Please be more careful.

Splitting allocation and submission (and reuse that helper in resume)
and adding a stop_rx_urbs helper might be a good idea.

> @@ -665,9 +664,8 @@ static const struct mfd_cell dln2_devs[] = {
>   },
>  };
>  
> -static void dln2_disconnect(struct usb_interface *interface)
> +static void dln2_stop(struct dln2_dev *dln2)
>  {
> - struct dln2_dev *dln2 = usb_get_intfdata(interface);
>   int i, j;
>  
>   /* don't allow starting new transfers */
> @@ -696,6 +694,16 @@ static void dln2_disconnect(struct usb_interface 
> *interface)
>   /* wait for transfers to end */
>   wait_event(dln2->disconnect_wq, !dln2->active_transfers);
>  
> + for (i = 0; i < DLN2_MAX_URBS; i++)
> + usb_kill_urb(dln2->rx_urb[i]);
> +}
> +
> +static void dln2_disconnect(struct usb_interface *interface)
> +{
> + struct dln2_dev *dln2 = usb_get_intfdata(interface);
> +
> + dln2_stop(dln2);
> +
>   mfd_remove_devices(&interface->dev);
>  
>   dln2_free(dln2);
> @@ -767,11 +775,38 @@ static const struct usb_device_id dln2_table[] = {
>  
>  MODULE_DEVICE_TABLE(usb, dln2_table);

I believe I already asked you to place the new callbacks above the id
table.

> +static int dln2_suspend(struct usb_interface *iface, pm_message_t message)
> +{
> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
> +
> + dln2_stop(dln2);
> + return 0;
> +}
> +
> +static int dln2_resume(struct usb_interface *iface)
> +{
> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
> + int i;
> + int ret = 0;
> +
> + dln2->disconnect = false;
> +
> + for (i = 0; i < DLN2_MAX_URBS; i++) {
> + ret = usb_submit_urb(dln2->rx_urb[i], GFP_KERNEL);

You cannot use GFP_KERNEL in resume, use GFP_NOIO.

> + if (ret)

Add a dev_err here.

> + break;
> + }
> +
> + return ret;
> +}

Johan
--
To unsubscribe from this list: send the line "unsubscribe 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: [GIT PULL] USB driver patches for 3.19-rc1

2014-12-15 Thread Hans de Goede

Hi Linus,

On 15-12-14 00:06, Linus Torvalds wrote:

On Sun, Dec 14, 2014 at 2:35 PM, Greg KH  wrote:


Hans de Goede (1):
   uas: Make uas work with blk-mq


So I got some fairly trivial conflicts on this one (conflicting with
the scsi cleanups mainly by Christoph Hellwig.

I resolved the conflict easily, and it all *looks* fine, but quite
frankly, I'd be a lot happier about it if somebody who has the
hardware were to actually test the end result.   The whole interaction
with ".use_blk_tags" etc should be verified by somebody who knows the
code.


Code wise this looks good and I've just given:

git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

a spin with an uas disk enclosure, and verified that tcq is being used,
and everything works fine.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe 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: gadget: hid: add configfs support

2014-12-15 Thread Geert Uytterhoeven
On Mon, Dec 15, 2014 at 12:06 AM, Linux Kernel Mailing List
 wrote:
> Gitweb: 
> http://git.kernel.org/linus/;a=commit;h=21a9476a7ba847e413bf1c144d7c614532aed6dd
> Commit: 21a9476a7ba847e413bf1c144d7c614532aed6dd
> Parent: 5ca8d3ec9970f4798e68bd21a9d44db3d0ff4da7
> Refname:refs/heads/master
> Author: Andrzej Pietrasiewicz 
> AuthorDate: Thu Nov 6 11:12:03 2014 +0100
> Committer:  Felipe Balbi 
> CommitDate: Thu Nov 6 16:18:19 2014 -0600
>
> usb: gadget: hid: add configfs support
>
> Make the hid function available for gadgets composed with configfs.
>

> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c

> +
> +#define F_HID_OPT(name, prec, limit)   \
> +static ssize_t f_hid_opts_##name##_show(struct f_hid_opts *opts, char *page)\
> +{  \
> +   int result; \
> +   \
> +   mutex_lock(&opts->lock);\
> +   result = sprintf(page, "%d\n", opts->name); \
> +   mutex_unlock(&opts->lock);  \
> +   \
> +   return result;  \
> +}  \
> +   \
> +static ssize_t f_hid_opts_##name##_store(struct f_hid_opts *opts,  \
> +const char *page, size_t len)  \
> +{  \
> +   int ret;\
> +   u##prec num;\
> +   \
> +   mutex_lock(&opts->lock);\
> +   if (opts->refcnt) { \
> +   ret = -EBUSY;   \
> +   goto end;   \
> +   }   \
> +   \
> +   ret = kstrtou##prec(page, 0, &num); \
> +   if (ret)\
> +   goto end;   \
> +   \
> +   if (num > limit) {  \
> +   ret = -EINVAL;  \
> +   goto end;   \
> +   }   \
> +   opts->name = num;   \
> +   ret = len;  \
> +   \
> +end:   \
> +   mutex_unlock(&opts->lock);  \
> +   return ret; \
> +}  \
> +   \
> +static struct f_hid_opts_attribute f_hid_opts_##name = \
> +   __CONFIGFS_ATTR(name, S_IRUGO | S_IWUSR, f_hid_opts_##name##_show,\
> +   f_hid_opts_##name##_store)
> +
> +F_HID_OPT(subclass, 8, 255);
> +F_HID_OPT(protocol, 8, 255);
> +F_HID_OPT(report_length, 16, 65536);

While all three of the above lines trigger a (false positive) compiler warning:

warning: comparison is always false due to limited range of data type

it's a bit sloppy to use "2^8-1" for the first two limits, and "2^16"
(without -1) for
the third limit.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

2014-12-15 Thread Johan Hovold
On Thu, Dec 11, 2014 at 03:29:52PM -0800, Jeremiah Mahler wrote:
> If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
> logs.  This patch set quiets these messages without changing the
> original behavior.

Don't unplug devices that are in use then. ;)

> This change is beneficial when using daemons such as slcand, which is
> similar to pppd or slip, that cannot determine whether they should exit
> until after the USB serial device is unplugged.  Producing these error
> messages for a normal use case is not helpful.

Your patches would hide these errors when they occur during normal use
(e.g. EPROTO).

Receiving an error message when unplugging an active device should not
surprise anyone. And at least you know where it came from (and it's
right there in the logs as well).

Johan
--
To unsubscribe from this list: send the line "unsubscribe 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 v5 1/3] mfd: add support for Cypress CYUSBS234 USB Serial Bridge controller

2014-12-15 Thread Johan Hovold
On Sat, Dec 13, 2014 at 05:17:42PM +0530, Muthu Mani wrote:
> Adds support for USB-I2C/GPIO interfaces of Cypress Semiconductor
> CYUSBS234 USB-Serial Bridge controller.
> 
> Details about the device can be found at:
> http://www.cypress.com/?rID=84126
> 
> Separate cell drivers are available for I2C and GPIO. SPI and UART
> are not supported yet.
> 
> Signed-off-by: Muthu Mani 
> Signed-off-by: Rajaram Regupathy 
> ---

The patches in this series are still white-space damaged.

Try sending it to yourself before reposting to the mailings lists again.

And as I already asked you, make sure to remove your corporate
confidentiality footers from mails you post to the lists.

Johan
--
To unsubscribe from this list: send the line "unsubscribe 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: udc-core: call udc_stop() before gadget unbind

2014-12-15 Thread Robert Baldyga
On 12/15/2014 06:13 AM, Peter Chen wrote:
> On Fri, Dec 12, 2014 at 02:17:28PM +0100, Robert Baldyga wrote:
>> As usb function drivers assumes that all usb request will be completed
>> before function unbind call, we should supply such behavior. In some
>> cases ep_disable() won't kill all request effectively, because some
>> IN requests can be in running state. In such situation it's possible
>> to have unbind function called before last request completion, which
>> can cause problems.
> 
> Doesn't the function's disable/unbind should call usb_ep_dequeue to make
> sure the transfer has ended?

USB function drivers like ECM or HID surely doesn't. It looks like
there's assumption that all requests will be completed by UDC driver.

Function ep_disable() should complete requests in hardware driver, but
at least in DWC2 driver not all requests are completed at this stage
(request which are in hardware FIFO are omitted to give them chance to
be transferred). Those requests are forced to complete in udc_stop()
function, and that's why it's needed to be called before unbind.

> 
> .udc_stop may stop the controller, and .unbind may still visit hardware.

Hmm, indeed it can be problem.

Thanks,
Robert Baldyga

> 
>>
>> For example unbinding f_ecm function while request on 'notify' endpoint
>> is not completed, ends up NULL pointer dereference in unbind() function.
>>
>> usb_gadget_udc_stop() call causes completion of all requests so if it's
>> called before gadget unbind there is no risk that some of requests will
>> stay uncompleted.
>>
>> Signed-off-by: Robert Baldyga 
>> ---
>>  drivers/usb/gadget/udc/udc-core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/udc/udc-core.c 
>> b/drivers/usb/gadget/udc/udc-core.c
>> index e31d574..6f0d233 100644
>> --- a/drivers/usb/gadget/udc/udc-core.c
>> +++ b/drivers/usb/gadget/udc/udc-core.c
>> @@ -331,8 +331,8 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
>>  
>>  usb_gadget_disconnect(udc->gadget);
>>  udc->driver->disconnect(udc->gadget);
>> -udc->driver->unbind(udc->gadget);
>>  usb_gadget_udc_stop(udc);
>> +udc->driver->unbind(udc->gadget);
>>  
>>  udc->driver = NULL;
>>  udc->dev.driver = NULL;
>> -- 
>> 1.9.1
>>
> 

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


Re: [PATCH] USB: serial: add nt124 usb to serial driver

2014-12-15 Thread Johan Hovold
On Sun, Dec 14, 2014 at 11:51:11AM -0600, George McCollister wrote:
> Johan,
> 
> While working on the tx_empty changes you suggested it occurred to me
> that it might not be obvious to others that the firmware doesn't send
> a packet with the NT124_CTRL_TXEMPTY flag cleared when it begins
> transmitting. The practical implication is that if the driver sets
> tx_empty = true when it sees NT124_CTRL_TXEMPTY, tx_empty must be
> reset to false somewhere when more data is transmitted. Perhaps I
> could add prepare_write_buffer and do it in there before calling
> usb_serial_generic_prepare_write_buffer(). Does that sound acceptable?

Hmm. There's no way to query that flag? And the status is sent (as bulk
in data) periodically or only when data has been received? And not when
the actual status changes?

A potential problem with using prepare_write_buffer would be on failures
to submit the write urb, in which case this flag might never be cleared.

Johan
--
To unsubscribe from this list: send the line "unsubscribe 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: usb-nop-xceiv.txt: Fix the description of 'vcc-supply'

2014-12-15 Thread Roger Quadros
On 12/12/14 17:08, Fabio Estevam wrote:
> From: Fabio Estevam 
> 
> Since bd27fa44e13830d2b ("usb: phy: generic: Don't use regulator framework for
> RESET line") we no longer model the reset line as a regulator supply, so 
> adapt the documentation accordingly.

It seems that the description of vcc-supply and reset-supply was swapped in the 
original commit 0eba387973f5 that introduced this file.

> 
> Signed-off-by: Fabio Estevam 

Acked-by: Roger Quadros 

cheers,
-roger

> ---
>  Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt 
> b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> index 1bd37fa..8db5b33 100644
> --- a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> @@ -13,7 +13,7 @@ Optional properties:
>  - clock-frequency: the clock frequency (in Hz) that the PHY clock must
>be configured to.
>  
> -- vcc-supply: phandle to the regulator that provides RESET to the PHY.
> +- vcc-supply: phandle to the regulator that provides power to the PHY.
>  
>  - reset-gpios: Should specify the GPIO for reset.
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe 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: serial: add nt124 usb to serial driver

2014-12-15 Thread Johan Hovold
On Fri, Dec 12, 2014 at 09:01:03AM -0600, George McCollister wrote:
> On Wed, Dec 10, 2014 at 7:04 AM, Johan Hovold  wrote:
> > On Mon, Dec 08, 2014 at 05:24:17PM -0600, George McCollister wrote:

> >> + switch (termios->c_cflag & CSIZE) {
> >
> > C_CSIZE(tty)
> Okay
> >
> >> + case CS5:
> >> + newline.bDataBits = 5;
> >> + break;
> >> + case CS6:
> >> + newline.bDataBits = 6;
> >> + break;
> I should probably just remove CS5 and CS6 since I don't think they
> will work anyway.

Ok. Remember to update the passed termios with the settings that you
actually end up using (i.e. settings from old_termios).

> >> +static int nt124_open(struct tty_struct *tty,
> >> +   struct usb_serial_port *port)
> >> +{
> >> + struct nt124_private *priv = usb_get_serial_port_data(port);
> >> + int result = 0;
> >> + unsigned long flags;
> >> +
> >> + spin_lock_irqsave(&port->lock, flags);
> >> + port->throttled = 0;
> >> + port->throttle_req = 0;
> >> + spin_unlock_irqrestore(&port->lock, flags);
> >> +
> >> + priv->flowctrl = 0;
> >
> > Drop this and keep the current settings.
> Okay
> >
> >> + nt124_set_termios(tty, port, NULL);
> >> + nt124_set_flowctrl(port, priv->flowctrl);
> >
> > Drop this. This is handled by tty and dtr_rts().
> Okay
> >
> >> +
> >> + if (port->bulk_in_size)
> >> + result = usb_serial_generic_submit_read_urbs(port, 
> >> GFP_KERNEL);
> >
> > Call usb_serial_generic_open() instead (and skip the throttle-flags bit
> > above).
> Okay, so looks like the only thing I will do in this function is call
> nt124_set_termios() followed by usb_serial_generic_open().

Yes, that should do it.

> >> +static struct usb_serial_driver nt124_device = {
> >> + .driver = {
> >> + .owner =THIS_MODULE,
> >> + .name = "nt124",
> >> + },
> >> + .id_table = id_table,
> >> + .num_ports =1,
> >> + .bulk_in_size = 32,
> >> + .bulk_out_size =32,
> >
> > Why do you reduce these buffer sizes? They can be bigger than the
> > endpoint size for increased throughput.
> I'll leave them out like most of the other drivers (and retest) unless
> you have another suggestion.

That's perfectly fine, and means that the buffer sizes will match the
endpoint sizes (they will in fact never be smaller than that).

Johan
--
To unsubscribe from this list: send the line "unsubscribe 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: Query regarding USB gadget driver

2014-12-15 Thread Sanchayan Maity
Hello,

On 12/15/2014 07:42 AM, Peter Chen wrote:
> On Fri, Dec 12, 2014 at 06:55:36PM +0530, Sanchayan Maity wrote:
>> Hello,
>>
>> On 12/12/2014 07:21 AM, Peter Chen wrote:
>>> On Thu, Dec 11, 2014 at 08:34:45AM -0600, Felipe Balbi wrote:
 Hi,

 On Thu, Dec 11, 2014 at 04:08:43PM +0530, Sanchayan Maity wrote:
> Hello,
>
> I am working on a Freescale Cortex-A5 Vybrid Processor. The chip core
> is clocked at 500MHz and the USB IP core for this is by Chip-idea. I
> am running a 3.18-rc5 kernel on it and trying to use the USB gadget
> functionality. To be more specific the CDC ECM class. Currently, I
> cannot use this properly. If I use just "ping" to check, it works
> fine, but, after running iperf, even one transaction doesn't complete
> or completes rarely. Checking the CDC Ether interface with Wireshark
> shows, TCP Dup Ack messages and checking the USB bus with Wireshark,
> shows packets with USB Protocol Error -71 at one point and after that
> packets with USB connection Reset -104 error. If it's of any
> significance, I have Arch Linux with the 3.18 kernel running on my
> laptop with which the Vybrid connects. On the host side, the only
> error dmesg shows is "kevent 12 may have been dropped". I guess this
> is connected to the "TCP Previous Segment not captured" and "TCP Dup
> ACK" messages.
>
> My script for the gadget configuration is as below:
>
> /bin/mount none /mnt -t configfs
> /bin/mkdir /mnt/usb_gadget/g1
> cd /mnt/usb_gadget/g1
> /bin/mkdir configs/c.1
> /bin/mkdir functions/ecm.0
> /bin/mkdir strings/0x409
> /bin/mkdir configs/c.1/strings/0x409
> echo 0xa4a2 > idProduct
> echo 0x0525 > idVendor
> echo Freescale123 > strings/0x409/serialnumber
> echo Freescale > strings/0x409/manufacturer
> echo "USB Serial Gadget" > strings/0x409/product
> echo "Conf 1" > configs/c.1/strings/0x409/configuration
> echo 200 > configs/c.1/MaxPower
> ln -s functions/ecm.0 configs/c.1
> echo ci_hdrc.0 > UDC
> /sbin/ifconfig usb0 up
> /sbin/ifconfig usb0 192.168.1.10
>
> I have debug prints in the udc.c and u_ether.c using pr_debug and

 just a little hint, use any of the dev_*() macros next time, they'll
 print the device name which helps figuring out which UDC you're using.

 Based on ci_hdrc.0 above, I suppose it's chipidea and Peter Chen
 maintains that one, it really helps adding maintainers to Cc list.

> enable them when required using dynamic debug. Without running iperf,
> using ping gives me a sequence of prints as below:
>
> [  277.434409] In eth_start_xmit
> [  277.434517] In UDC irq
> [  277.434553] In usb_gadget_giveback_request
> [  277.434567] In tx_complete
> [  277.435443] In UDC irq
> [  277.435477] In usb_gadget_giveback_request
> [  277.435491] In rx_complete
> [  277.435517] In rx_submit
> [  277.435601] In eth_start_xmit
> [  277.436441] In UDC irq
> [  277.436465] In usb_gadget_giveback_request
> [  277.436478] In rx_complete
> [  277.436493] In rx_submit
> [  277.436520] In usb_gadget_giveback_request
> [  277.436533] In tx_complete
> [  278.434865] In eth_start_xmit
> [  278.434959] In UDC irq
> [  278.434993] In usb_gadget_giveback_request
> [  278.435006] In tx_complete
> [  278.435881] In UDC irq
> [  278.435910] In usb_gadget_giveback_request
> [  278.435923] In rx_complete
> [  278.435946] In rx_submit
>
> After running iperf without debug prints and then enabling before
> using ping gives me a sequence of prints as below
> [   81.989827] In UDC irq
> [   81.989871] In usb_gadget_giveback_request
> [   81.989886] In rx_complete
> [   81.989905] In rx_submit
> [   82.989892] In UDC irq
> [   82.989951] In usb_gadget_giveback_request
> [   82.989967] In rx_complete
> [   82.989992] In rx_submit
> [   83.990064] In UDC irq
> [   83.990126] In usb_gadget_giveback_request
> [   83.990142] In rx_complete
> [   83.990167] In rx_submit
> [   84.990007] In UDC irq
> [   84.990049] In usb_gadget_giveback_request
> [   84.990064] In rx_complete
> [   84.990083] In rx_submit
> [   85.990085] In UDC irq
> [   85.990147] In usb_gadget_giveback_request
> [   85.990163] In rx_complete
> [   85.990188] In rx_submit
>
> If I force a full speed configuration for this USB client port, I get
> a slightly more reliable operation where iperf can run for may be half
> an hour or so or almost an hour before it falls through. Putting in a
> delay of 100-150 microseconds in eth_start_xmit also improves it like
> full speed, but, still not reliable. If I run iperf with debug prints
> enable, this gives similar results to full speed config. After the
> failure of iperf test, even ping doesn