[PATCH] usb: gadget: f_rndis: fix usb_interface_descriptor for rndis

2014-09-24 Thread Heiko Schocher
use the values for RNDIS over Ethernet as defined in
http://www.usb.org/developers/defined_class
(search for RDNIS):

- baseclass: 0xef (miscellaneous)
- subclass: 0x04
- protocol: 0x01

with this setings the file in Documentation/usb/linux.inf is
obsolete.

Signed-off-by: Heiko Schocher h...@denx.de

---

Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@suse.de
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Oliver Neukum oli...@neukum.name
Cc: net...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: Andrzej Pietrasiewicz andrze...@samsung.com
Cc: Michal Nazarewicz min...@mina86.com
Cc: Kyungmin Park kyungmin.p...@samsung.com
Cc: Dan Carpenter dan.carpen...@oracle.com
Cc: Macpaul Lin macp...@gmail.com

Tested with the USB Compliance test suite which runs Windows, see:
http://www.usb.org/developers/tools/usb20_tools/#usb20cv

 drivers/net/usb/cdc_ether.c   | 6 +++---
 drivers/usb/core/generic.c| 6 +++---
 drivers/usb/gadget/function/f_rndis.c | 6 +++---
 include/uapi/linux/usb/cdc.h  | 3 +++
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 2a32d91..9c216c2 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -35,9 +35,9 @@
 
 static int is_rndis(struct usb_interface_descriptor *desc)
 {
-   return (desc-bInterfaceClass == USB_CLASS_COMM 
-   desc-bInterfaceSubClass == 2 
-   desc-bInterfaceProtocol == 0xff);
+   return (desc-bInterfaceClass == USB_CLASS_MISC 
+   desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS 
+   desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH);
 }
 
 static int is_activesync(struct usb_interface_descriptor *desc)
diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index 358ca8d..a2a4e05 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -28,9 +28,9 @@ static inline const char *plural(int n)
 
 static int is_rndis(struct usb_interface_descriptor *desc)
 {
-   return desc-bInterfaceClass == USB_CLASS_COMM
-desc-bInterfaceSubClass == 2
-desc-bInterfaceProtocol == 0xff;
+   return desc-bInterfaceClass == USB_CLASS_MISC
+desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS
+desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH;
 }
 
 static int is_activesync(struct usb_interface_descriptor *desc)
diff --git a/drivers/usb/gadget/function/f_rndis.c 
b/drivers/usb/gadget/function/f_rndis.c
index ddb09dc..cc06046 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -117,9 +117,9 @@ static struct usb_interface_descriptor rndis_control_intf = 
{
/* .bInterfaceNumber = DYNAMIC */
/* status endpoint is optional; this could be patched later */
.bNumEndpoints =1,
-   .bInterfaceClass =  USB_CLASS_COMM,
-   .bInterfaceSubClass =   USB_CDC_SUBCLASS_ACM,
-   .bInterfaceProtocol =   USB_CDC_ACM_PROTO_VENDOR,
+   .bInterfaceClass =  USB_CLASS_MISC,
+   .bInterfaceSubClass =   USB_CDC_SUBCLASS_RNDIS,
+   .bInterfaceProtocol =   USB_CDC_RNDIS_PROTO_ETH,
/* .iInterface = DYNAMIC */
 };
 
diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index b6a9cdd..8e8fc85 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -12,6 +12,7 @@
 #include linux/types.h
 
 #define USB_CDC_SUBCLASS_ACM   0x02
+#define USB_CDC_SUBCLASS_RNDIS 0x04
 #define USB_CDC_SUBCLASS_ETHERNET  0x06
 #define USB_CDC_SUBCLASS_WHCM  0x08
 #define USB_CDC_SUBCLASS_DMM   0x09
@@ -31,6 +32,8 @@
 #define USB_CDC_ACM_PROTO_AT_CDMA  6
 #define USB_CDC_ACM_PROTO_VENDOR   0xff
 
+#define USB_CDC_RNDIS_PROTO_ETH1
+
 #define USB_CDC_PROTO_EEM  7
 
 #define USB_CDC_NCM_PROTO_NTB  1
-- 
1.8.3.1

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


Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

2014-09-24 Thread Arnd Bergmann
On Wednesday 24 September 2014 10:27:52 Peter Chen wrote:
 
 Antoine is adding a generic chipdea glue layer driver, which like ehci generic
 platform driver: drivers/usb/host/ehci-platform.c, since other architectures
 like MIPS (Someone submitted mips chipidea driver before) may not have device
 tree support, I think non-dt support is also needed.

Right.

 It is a good suggestion for adding DT support for core driver, Since we did
 not do it at the first, it is a little embarrass at current situation.
 
 - For the new chipidea glue drivers, it is ok we can have a child node
 for core device at glue device node, and some common entries can be there
 like: phy, vbus, dr_mode, etc. We need to add support for getting
 these common things for both through device tree and platform data
 (parent is DT support and parent is non-DT support) at core driver.

I don't really want to see the child devices show up in DT, as this is
really an implementation detail of the way that the driver currently works,
and IMHO we should change that.

I know that Felipe disagrees with me on this, but almost every other
driver that has soc-specific specializations does not use parent/child
nodes, neither in DT nor in Linux. Instead you have a single device
node that gets distinguished by compatible string.

 - For the existing glue drivers, since we can't change existed dts, we can
 only do it from future SoC support. Then, in this kinds of glue drivers,
 we need to support for both create core driver by node and by current
 calling platform_device_add way.

We should be able to easily change the ci_hdrc_add_device call into
a different exported function that calls a version of the probe()
code directly, as we do in all sorts of other drivers (ehci, ohci,
dwc2, ..., but not dwc3 and musb as they are maintained by Felipe).

We can also gradually move in some of the other glue drivers into
the main driver if the differences are small enough.

Moving the PCI driver over to this model requires a little more
work but is absolutely doable (again, see ehci, ahci, sdhci examples)
by moving the platform_device handling out of core.c and changing
it just operate on the plain 'struct device', with an exported
probe function that gets called on either the platform device
or the pci device.

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


[resend PATCH] usb: dwc3: pci: Add PCI ID for Intel Braswell

2014-09-24 Thread Heikki Krogerus
From: Alan Cox a...@linux.intel.com

The device controller is the same but it has different PCI ID. Add this new
ID to the driver's list of supported IDs.

Signed-off-by: Alan Cox a...@linux.intel.com
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
 drivers/usb/dwc3/dwc3-pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 436fb08..a36cf66 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -30,6 +30,7 @@
 #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB30xabcd
 #define PCI_DEVICE_ID_INTEL_BYT0x0f37
 #define PCI_DEVICE_ID_INTEL_MRFLD  0x119e
+#define PCI_DEVICE_ID_INTEL_BSW0x22B7
 
 struct dwc3_pci {
struct device   *dev;
@@ -181,6 +182,7 @@ static const struct pci_device_id dwc3_pci_id_table[] = {
PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS,
PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3),
},
+   { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW), },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT), },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRFLD), },
{  }/* Terminating Entry */
-- 
2.1.0

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


Re: [PATCH 0/2] USB: core: add add device-qualifier quirk

2014-09-24 Thread Greg Kroah-Hartman
On Tue, Sep 23, 2014 at 05:27:22PM +0200, Johan Hovold wrote:
 On Tue, Sep 23, 2014 at 08:21:51AM -0700, Greg Kroah-Hartman wrote:
  On Tue, Sep 23, 2014 at 04:56:24PM +0200, Johan Hovold wrote:
   On Mon, Aug 25, 2014 at 05:51:25PM +0200, Johan Hovold wrote:
This is quirk is indeed needed to get the Elan Touchscreen found on some
Samsung laptops to enumerate reliably.
   
   [...]
   
Johan Hovold (2):
  USB: core: add device-qualifier quirk
  USB: quirks: enable device-qualifier quirk for Elan Touchscreen
   
   Are these patches still in your queue, Greg? Just checking now that the
   merge window is around the corner.
  
  My queue is huge and I hope to tackle it tonight...
 
 Yeah, that's the impression I got.
 
 Just a heads up: There was another quirk submitted after this one that
 uses the same quirk mask.

Ah, thanks for the warning, I'll watch out for it.

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


[resend PATCH 2/3] usb: dwc3: core: only setting the dma_mask when needed

2014-09-24 Thread Heikki Krogerus
If the probe drivers have already set the dma_mask, not
replacing the value.

Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
 drivers/usb/dwc3/core.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b0f4d52..d08cac5 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -708,9 +708,11 @@ static int dwc3_probe(struct platform_device *pdev)
spin_lock_init(dwc-lock);
platform_set_drvdata(pdev, dwc);
 
-   dev-dma_mask   = dev-parent-dma_mask;
-   dev-dma_parms  = dev-parent-dma_parms;
-   dma_set_coherent_mask(dev, dev-parent-coherent_dma_mask);
+   if (!dev-dma_mask) {
+   dev-dma_mask = dev-parent-dma_mask;
+   dev-dma_parms = dev-parent-dma_parms;
+   dma_set_coherent_mask(dev, dev-parent-coherent_dma_mask);
+   }
 
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);
-- 
2.1.0

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


[resend PATCH 1/3] ACPI / platform: provide default DMA mask

2014-09-24 Thread Heikki Krogerus
Most devices are configured for 32-bit DMA addresses.
Setting the mask to 32-bit here removes the need for the
drivers to do it separately.

Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Cc: Rafael J. Wysocki r...@rjwysocki.net
---
 drivers/acpi/acpi_platform.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index 2bf9082..8d099e6 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -16,6 +16,7 @@
 #include linux/err.h
 #include linux/kernel.h
 #include linux/module.h
+#include linux/dma-mapping.h
 #include linux/platform_device.h
 
 #include internal.h
@@ -102,6 +103,7 @@ struct platform_device *acpi_create_platform_device(struct 
acpi_device *adev)
pdevinfo.res = resources;
pdevinfo.num_res = count;
pdevinfo.acpi_node.companion = adev;
+   pdevinfo.dma_mask = DMA_BIT_MASK(32);
pdev = platform_device_register_full(pdevinfo);
if (IS_ERR(pdev))
dev_err(adev-dev, platform device creation failed: %ld\n,
-- 
2.1.0

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


[resend PATCH 0/3] usb: dwc3: ACPI support

2014-09-24 Thread Heikki Krogerus
Hi,

The original series included patch that adds Braswell PCI ID, but I
send it separately.

The DMA mask caused a problem as our acpi platform code does not
provide anything for us. Instead of trying to fix it in dwc3 I
decided to suggest the first patch in this series where I provide
default DMA mask for all the ACPI platform devices.


Heikki Krogerus (3):
  ACPI / platform: provide default DMA mask
  usb: dwc3: core: only setting the dma_mask when needed
  usb: dwc3: add ACPI support

 drivers/acpi/acpi_platform.c |  2 ++
 drivers/usb/dwc3/core.c  | 18 +++---
 2 files changed, 17 insertions(+), 3 deletions(-)

-- 
2.1.0

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


[resend PATCH 3/3] usb: dwc3: add ACPI support

2014-09-24 Thread Heikki Krogerus
Adds ACPI ID used on newer Intel SoCs.

Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
 drivers/usb/dwc3/core.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index d08cac5..c2cf2d8 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -32,6 +32,7 @@
 #include linux/delay.h
 #include linux/dma-mapping.h
 #include linux/of.h
+#include linux/acpi.h
 
 #include linux/usb/ch9.h
 #include linux/usb/gadget.h
@@ -960,12 +961,21 @@ static const struct of_device_id of_dwc3_match[] = {
 MODULE_DEVICE_TABLE(of, of_dwc3_match);
 #endif
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id dwc3_acpi_match[] = {
+   { 808622B7, 0 },
+   { },
+};
+MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
+#endif
+
 static struct platform_driver dwc3_driver = {
.probe  = dwc3_probe,
.remove = dwc3_remove,
.driver = {
.name   = dwc3,
.of_match_table = of_match_ptr(of_dwc3_match),
+   .acpi_match_table = ACPI_PTR(dwc3_acpi_match),
.pm = DWC3_PM_OPS,
},
 };
-- 
2.1.0

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


Re: [PATCH] drivers: usb: fsl: Check IP version 2.4 for mph USB controller

2014-09-24 Thread Greg KH
On Thu, Aug 21, 2014 at 12:43:30PM +0530, Nikhil Badola wrote:
 Check 2.4 IP version for multi port host USB controller and
 return FSL_USB_VER_2_4 macro
 
 Signed-off-by: Nikhil Badola nikhil.bad...@freescale.com
 ---
  drivers/usb/host/fsl-mph-dr-of.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/usb/host/fsl-mph-dr-of.c 
 b/drivers/usb/host/fsl-mph-dr-of.c
 index 9162d1b..e0315de 100644
 --- a/drivers/usb/host/fsl-mph-dr-of.c
 +++ b/drivers/usb/host/fsl-mph-dr-of.c
 @@ -126,6 +126,7 @@ static int usb_get_ver_info(struct device_node *np)
   /*
* returns 1 for usb controller version 1.6
* returns 2 for usb controller version 2.2
 +  * returns 3 for usb controller version 2.4
* returns 0 otherwise
*/
   if (of_device_is_compatible(np, fsl-usb2-dr)) {
 @@ -150,6 +151,8 @@ static int usb_get_ver_info(struct device_node *np)
   ver = FSL_USB_VER_1_6;
   else if (of_device_is_compatible(np, fsl-usb2-mph-v2.2))
   ver = FSL_USB_VER_2_2;
 + else if (of_device_is_compatible(np, fsl-usb2-mph-v2.4))
 + ver = FSL_USB_VER_2_4;

Why do we care?
--
To unsubscribe from this list: send the line unsubscribe linux-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] drivers: usb :fsl: Add support for USB controller version-2.5

2014-09-24 Thread Greg KH
On Thu, Aug 21, 2014 at 12:56:22PM +0530, Nikhil Badola wrote:
 Add support for USB controller version-2.5 used in
 T4240 rev2.0, T1024, B3421, T1040, T2080, LS1021A.
 
 Signed-off-by: Nikhil Badola nikhil.bad...@freescale.com
 ---
   - Depends on commit 990c2c7829d98517228f2b2ff14919c83b75e124
 drivers: usb: fsl: Check IP version 2.4 for mph USB controller

That ocmmit isn't in Linus's tree, how can you use the git commit id for
it?

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] drivers: usb :fsl: Add support for USB controller version-2.5

2014-09-24 Thread Greg KH
On Thu, Aug 21, 2014 at 12:56:22PM +0530, Nikhil Badola wrote:
 Add support for USB controller version-2.5 used in
 T4240 rev2.0, T1024, B3421, T1040, T2080, LS1021A.
 
 Signed-off-by: Nikhil Badola nikhil.bad...@freescale.com
 ---
   - Depends on commit 990c2c7829d98517228f2b2ff14919c83b75e124
 drivers: usb: fsl: Check IP version 2.4 for mph USB controller
 
  drivers/usb/host/fsl-mph-dr-of.c | 5 +
  include/linux/fsl_devices.h  | 1 +
  2 files changed, 6 insertions(+)
 
 diff --git a/drivers/usb/host/fsl-mph-dr-of.c 
 b/drivers/usb/host/fsl-mph-dr-of.c
 index e0315de..45b9e36 100644
 --- a/drivers/usb/host/fsl-mph-dr-of.c
 +++ b/drivers/usb/host/fsl-mph-dr-of.c
 @@ -127,6 +127,7 @@ static int usb_get_ver_info(struct device_node *np)
* returns 1 for usb controller version 1.6
* returns 2 for usb controller version 2.2
* returns 3 for usb controller version 2.4
 +  * returns 4 for usb controller version 2.5
* returns 0 otherwise
*/
   if (of_device_is_compatible(np, fsl-usb2-dr)) {
 @@ -136,6 +137,8 @@ static int usb_get_ver_info(struct device_node *np)
   ver = FSL_USB_VER_2_2;
   else if (of_device_is_compatible(np, fsl-usb2-dr-v2.4))
   ver = FSL_USB_VER_2_4;
 + else if (of_device_is_compatible(np, fsl-usb2-dr-v2.5))
 + ver = FSL_USB_VER_2_5;
   else /* for previous controller versions */
   ver = FSL_USB_VER_OLD;
  
 @@ -153,6 +156,8 @@ static int usb_get_ver_info(struct device_node *np)
   ver = FSL_USB_VER_2_2;
   else if (of_device_is_compatible(np, fsl-usb2-mph-v2.4))
   ver = FSL_USB_VER_2_4;
 + else if (of_device_is_compatible(np, fsl-usb2-mph-v2.5))
 + ver = FSL_USB_VER_2_5;
   else /* for previous controller versions */
   ver = FSL_USB_VER_OLD;
   }
 diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
 index a82296a..2a2f56b 100644
 --- a/include/linux/fsl_devices.h
 +++ b/include/linux/fsl_devices.h
 @@ -24,6 +24,7 @@
  #define FSL_USB_VER_1_6  1
  #define FSL_USB_VER_2_2  2
  #define FSL_USB_VER_2_4  3
 +#define FSL_USB_VER_2_5  4

Why not just keep going and add the rest of the numbers while you are at
it?

Seriously, what are these two patches doing?  You just set the value,
never do anything with it, what good is it?

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 fix for 3.17 v5] uas: Add a quirk for rejecting ATA_12 and ATA_16 commands

2014-09-24 Thread Greg Kroah-Hartman
On Mon, Sep 15, 2014 at 04:13:52PM +0200, Hans de Goede wrote:
 Hi,
 
 On 09/15/2014 04:08 PM, Greg Kroah-Hartman wrote:
  On Mon, Sep 15, 2014 at 04:04:12PM +0200, Hans de Goede wrote:
  And set this quirk for the Seagate Expansion Desk (0bc2:2312), as that one
  seems to hang upon receiving an ATA_12 or ATA_16 command.
 
  https://bugzilla.kernel.org/show_bug.cgi?id=79511
  https://bbs.archlinux.org/viewtopic.php?id=183190
 
  While at it also add missing documentation for the u value for usb-storage
  quirks.
 
  Cc: sta...@vger.kernel.org # 3.16
  Signed-off-by: Hans de Goede hdego...@redhat.com
 
  --
  Changes in v2: Add documentation for new t and u usb-storage.quirks flags
  Changes in v3: Fix typo in documentation
  Changes in v4: Also apply the quirk to (0bc2:3312)
  Changes in v5: Rebased on 3.17-rc5, drop u documentation, already upstream
  
  So I should wait another day for v6?  :)
 
 Hehe, no I certainly hope not.
 
 Chances are I there will eventually be more seagate models needing the quirk,
 but I've none in the pipeline atm, and we can always add those with an
 incremental patch.
 
 So if this is not too late for 3.17 and you can pick up v5 that would be 
 great.

It's too late for 3.17, but I'll mark it for stable for 3.17 and 3.16.

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 1/1] Added support for Seluxit USB dongle to cp210x driver.

2014-09-24 Thread Greg KH
On Mon, Sep 22, 2014 at 10:52:56AM +0200, Johan Hovold wrote:
 On Mon, Sep 22, 2014 at 09:50:43AM +0200, Andreas Bomholtz wrote:
  Added the Seluxit ApS USB Serial Dongle to cp210x driver.
  
  Signed-off-by: Andreas Bomholtz andr...@seluxit.com
  ---
 
 Thanks for resending. Applied just fine now.

Was this in the last pull request you made to me?  I don't see it
there...

thanks,

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


Re: [RFC PATCH 2/2] usb-core: Remove the local_irq_save/local_irq_restore around complete

2014-09-24 Thread Greg KH
On Mon, Sep 15, 2014 at 10:22:49AM -0500, dingu...@opensource.altera.com wrote:
 From: Dinh Nguyen dingu...@opensource.altera.com
 
 When enabling HCD_BH for the DWC2 HCD, these local_irq_save/local_irq_restore
 was causing a timeout with a webcam.
 
 Signed-off-by: Dinh Nguyen dingu...@opensource.altera.com
 ---
  drivers/usb/core/hcd.c | 2 --
  1 file changed, 2 deletions(-)
 
 diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
 index 487abcf..463dc44 100644
 --- a/drivers/usb/core/hcd.c
 +++ b/drivers/usb/core/hcd.c
 @@ -1678,9 +1678,7 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
* and no one may trigger the above deadlock situation when
* running complete() in tasklet.
*/
 - local_irq_save(flags);
   urb-complete(urb);
 - local_irq_restore(flags);
  
   usb_anchor_resume_wakeups(anchor);
   atomic_dec(urb-use_count);

That really looks like an incorrect patch, are you _sure_ you can do
this?

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 v5 1/2] usb: rename phy to usb_phy in HCD

2014-09-24 Thread Greg KH
On Fri, Sep 05, 2014 at 01:42:09AM +0400, Sergei Shtylyov wrote:
 From: Antoine Tenart antoine.ten...@free-electrons.com
 
 The USB PHY member of the HCD structure is renamed to 'usb_phy' and
 modifications are done in all drivers accessing it.
 This is in preparation to adding the generic PHY support.
 
 Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com
 [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects
 caused by patch reordering, updated changelog.]
 Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 Acked-by: Felipe Balbi ba...@ti.com
 
 ---
 Changes in version 5:
 - imported the patch from Antoine Tenart's series;
 - added missing 'drivers/usb/misc/lvstest.c' file;
 - resolved rejects caused by patch reordering;
 - refreshed patch;
 - updated changelog.
 
  drivers/usb/chipidea/host.c   |2 +-
  drivers/usb/core/hcd.c|   20 ++--
  drivers/usb/core/hub.c|8 
  drivers/usb/host/ehci-fsl.c   |   16 
  drivers/usb/host/ehci-hub.c   |2 +-
  drivers/usb/host/ehci-msm.c   |4 ++--
  drivers/usb/host/ehci-tegra.c |   16 
  drivers/usb/host/ohci-omap.c  |   20 ++--
  drivers/usb/misc/lvstest.c|8 
  include/linux/usb/hcd.h   |2 +-
  10 files changed, 49 insertions(+), 49 deletions(-)

This doesn't apply to my tree at all anymore, can you refresh it and
resend?

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] Replace a single-value sscanf() call with a call to kstrtou32(), as recommended by checkpatch.pl.

2014-09-24 Thread Greg KH
On Sat, Sep 06, 2014 at 11:12:27PM -0400, Lars R. Damerow wrote:
 Signed-off-by: Lars R. Damerow l...@grandstreet.us

No changelog body?

Also, the subject should have usb and vhci_sysfs somewhere in there,
right?

thanks,

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


Re: [PATCH] storage: Replace magic number with define in usb_stor_euscsi_init()

2014-09-24 Thread Greg KH
On Sun, Sep 21, 2014 at 07:59:42PM +0100, Mark wrote:
 Hi,
 
 usb_stor_euscsi_init() calls usb_stor_control_msg() with timeout argument
 5000. USB_CTRL_SET_TIMEOUT is defined to be 5000 in usb.h, so would it make
 sense to use that instead? Patch below if it would.
 
 Signed-off-by: Mark Knibbs ma...@clara.co.uk

Note, your email client doesn't show your full name in the From: line,
can you fix it?  I don't like having to hand-edit patches to add it back
in...

thanks,

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


Re: [PATCH v3 1/1] USB: Add device quirk for ASUS T100 Base Station keyboard

2014-09-24 Thread Greg Kroah-Hartman
On Fri, Sep 19, 2014 at 10:13:50AM +0800, Lu Baolu wrote:
 This full-speed USB device generates spurious remote wakeup event
 as soon as USB_DEVICE_REMOTE_WAKEUP feature is set. As the result,
 Linux can't enter system suspend and S0ix power saving modes once
 this keyboard is used.
 
 This patch tries to introduce USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk.
 With this quirk set, wakeup capability will be ignored during
 device configure.
 
 This patch could be back-ported to kernels as old as 2.6.39.

I had to renumber the quirk as a different patch grabbed that number,
but all should be fine now.

thanks,

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


Re: [PATCH v3 0/6] usb: hub: convert khubd into workqueue

2014-09-24 Thread Greg Kroah-Hartman
On Sat, Sep 20, 2014 at 08:41:21PM +, Paul Zimmerman wrote:
  From: linux-usb-ow...@vger.kernel.org 
  [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Alan Stern
  Sent: Friday, September 19, 2014 12:39 PM
  
  On Fri, 19 Sep 2014, Petr Mladek wrote:
  
   The 3rd version of the patchset is slightly reordered and refactored
   as suggested earlier. See below for more details.
  
   IMHO, the result is more clean. But feel free to ask me to revert it.
   I do not want to make the review more complicated. Well, I double checked
   the diff of patches files between v2 and v3. It is pretty small and only
   the expected changes are there.
  
   Changes in:
   + v3:
 + split the optimization of hub-hdev reference counting
   into separate patch; usable standalone (1st patch)
  
 + switch order of the two patches that convert the kthread
   and that remove the while cycle in hub_event(); it is
   cleaner (2nd and 3rd patch)
  
 + use ordered workqueue by default; it makes it compatible
   with the original kthread solution (3rd patch)
  
   + added optional patch that allows to process events
   in parallel (6th patch)
  
 + fixed the suggested stylistic problems (2nd and 3rd patch)
  
  
   + v2:
 + solved potential races:
  + get hub-kref in kick_hub_wq()
  + call usb_get_dev(hdev) in hub_probe()
and  usb_put_dev(hdev) in hub_release()
  + INIT_WORK only once in hub_probe()
  
  + do not call cancel_work_sync() in hub_disconnect()
to keep it fast
  
  + rename kick_khubd() to kick_hub_wq() already
in the first patch; IMHO, it is cleaner while
adding only very few changes
  
  
   The workqueue API is well defined and tested. It has many options
   that could be used to tune the scheduling. The code is usually
   easier and thus more safe. It allows to avoid the extra thread
   in most cases. It has has clearly defined behavior vrt. system suspend.
  
   This patchset converts khubd into the workqueue. It saves one thread,
   lock, and list.
  
   It  looks huge but the main change is in the first patch. The rest is
   simple renaming of functions, comments and documentation.
  
  
   Thanks a lot Alan Stern and Tejun Heo for hints and guidance.
  
   The patches can be applied either against Linus' tree or linux-next.
  
  Very nice work.  I haven't tried it out, but it all looks good.  No
  doubt we will be able to sort out unforeseen troubles if any arise,
  without much difficulty.
 
 Hmm. What are the odds that some old kernel monitoring program, in an
 ancient Fedora distribution let's say, will be checking for 'khubd' in
 the kernel process list? Pretty high I would think.

I really doubt it, what would that tell them?

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 v2] usb: core: downgrade log severity to info when descriptor unavailable

2014-09-24 Thread Greg Kroah-Hartman
On Tue, Sep 23, 2014 at 07:12:40PM +, Scot Doyle wrote:
 According to commit 0cce2eda19923e5e5ccc8b042dec5af87b3ffad0
 USB: fix LANGID=0 regression
 
 usb devices are not required to report string descriptors. Since they are
 optional, log an info message instead of an error message.
 
 Signed-off-by: Scot Doyle lkm...@scotdoyle.com
 ---
  drivers/usb/core/message.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
 index 0c8a7fc..da2f1f2 100644
 --- a/drivers/usb/core/message.c
 +++ b/drivers/usb/core/message.c
 @@ -780,8 +780,12 @@ static int usb_get_langid(struct usb_device *dev, 
 unsigned char *tbuf)
* deal with strings at all. Set string_langid to -1 in order to
* prevent any string to be retrieved from the device */
   if (err  0) {
 - dev_err(dev-dev, string descriptor 0 read error: %d\n,
 - err);
 + if (err == -EPIPE)
 + dev_info(dev-dev,
 +  string descriptor 0 read error: -EPIPE\n);
 + else
 + dev_err(dev-dev,
 + string descriptor 0 read error: %d\n, err);

What real difference does this patch make?  What would a user do with
-EPIPE?

And USB devices _are_ required to provide a string descriptor if they
provide a string id, so your changelog body doesn't make much sense.  I
suggest rewriting it to say they aren't required to provide a string
descriptor for string 0.

But even then, I don't understand the goal of this patch.

thanks,

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


Re: [RFC PATCH] usb: core: log more general message on malformed LANGID descriptor

2014-09-24 Thread Greg Kroah-Hartman
On Tue, Sep 23, 2014 at 10:28:49PM +, Scot Doyle wrote:
 I'd like to change this error message:
 [3.325837] usb 1-4: string descriptor 0 malformed (err = -61), defaulting 
 to 0x0409
 
 into an error message followed by a debug message:
 [3.324726] usb 1-4: malformed string descriptor; unknown language, 
 defaulting to English
 [3.327514] usb 1-4: string descriptor 0 malformed (err = -61), defaulting 
 to 0x0409
 
 in order to communicate more information from the log itself. Are there 
 any problems with this approach? Would it be better to put all the 
 information on a single line? Something else?

How about just one line, why two?

And something like device not implementing langid specifier, defaulting
to English, but cleaned up to sound a bit better?

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 v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

2014-09-24 Thread Arnd Bergmann
On Wednesday 24 September 2014 09:44:19 Arnd Bergmann wrote:
 
 We can also gradually move in some of the other glue drivers into
 the main driver if the differences are small enough.
 

FWIW, I've just looked at the other glue drivers that already
exist:

- zevio can just get merged into the common driver, all that seems
  to be needed for that is the additional compatible string, and
  keying off the ci_default_zevio_platdata on the .data field of
  the of_device_id table.

- msm has a custom notifier, which justifies leaving it in a separate
  driver, but it's also small enough that it wouldn't hurt to have
  that merged into the main driver too.

- imx requires a lot of other things, in particular the dependency
  on the usbmisc driver means we don't want to have that in the
  core anyway, so we can't really merge that in.

- the proposed ar933x driver again looks almost trivial, so no reason
  for a separate glue driver for that.

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


Re: [PATCH 1/1] Added support for Seluxit USB dongle to cp210x driver.

2014-09-24 Thread Johan Hovold
On Tue, Sep 23, 2014 at 09:44:48PM -0700, Greg Kroah-Hartman wrote:
 On Mon, Sep 22, 2014 at 10:52:56AM +0200, Johan Hovold wrote:
  On Mon, Sep 22, 2014 at 09:50:43AM +0200, Andreas Bomholtz wrote:
   Added the Seluxit ApS USB Serial Dongle to cp210x driver.
   
   Signed-off-by: Andreas Bomholtz andr...@seluxit.com
   ---
  
  Thanks for resending. Applied just fine now.
 
 Was this in the last pull request you made to me?  I don't see it
 there...

I just sent you a final pull request for 3.17. Just two device ids,
including this one.

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 3/4] gpiolib: add irq_not_threaded flag to gpio_chip

2014-09-24 Thread Linus Walleij
On Fri, Sep 19, 2014 at 10:22 PM, Octavian Purdila
octavian.purd...@intel.com wrote:

 Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set
 operation but do not need a threaded irq handler.

 Signed-off-by: Octavian Purdila octavian.purd...@intel.com

Usually I don't apply patches adding interfaces with no users, but
this seems very useful, so patch applied. I guess your driver will
appear on v3.19+ so then you can rely on this having been merged
for v3.18.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-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: f_rndis: fix usb_interface_descriptor for rndis

2014-09-24 Thread Michal Nazarewicz
On Wed, Sep 24 2014, Heiko Schocher h...@denx.de wrote:
 use the values for RNDIS over Ethernet as defined in
 http://www.usb.org/developers/defined_class
 (search for RDNIS):

 - baseclass: 0xef (miscellaneous)
 - subclass: 0x04
 - protocol: 0x01

 with this setings the file in Documentation/usb/linux.inf is
 obsolete.

 Signed-off-by: Heiko Schocher h...@denx.de

 ---

 Cc: Felipe Balbi ba...@ti.com
 Cc: Greg Kroah-Hartman gre...@suse.de
 Cc: linux-usb@vger.kernel.org
 Cc: linux-ker...@vger.kernel.org
 Cc: Oliver Neukum oli...@neukum.name
 Cc: net...@vger.kernel.org
 Cc: linux-...@vger.kernel.org
 Cc: Andrzej Pietrasiewicz andrze...@samsung.com
 Cc: Michal Nazarewicz min...@mina86.com
 Cc: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Dan Carpenter dan.carpen...@oracle.com
 Cc: Macpaul Lin macp...@gmail.com

 Tested with the USB Compliance test suite which runs Windows, see:
 http://www.usb.org/developers/tools/usb20_tools/#usb20cv

  drivers/net/usb/cdc_ether.c   | 6 +++---
  drivers/usb/core/generic.c| 6 +++---
  drivers/usb/gadget/function/f_rndis.c | 6 +++---
  include/uapi/linux/usb/cdc.h  | 3 +++
  4 files changed, 12 insertions(+), 9 deletions(-)

 diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
 index 2a32d91..9c216c2 100644
 --- a/drivers/net/usb/cdc_ether.c
 +++ b/drivers/net/usb/cdc_ether.c
 @@ -35,9 +35,9 @@
  
  static int is_rndis(struct usb_interface_descriptor *desc)
  {
 - return (desc-bInterfaceClass == USB_CLASS_COMM 
 - desc-bInterfaceSubClass == 2 
 - desc-bInterfaceProtocol == 0xff);
 + return (desc-bInterfaceClass == USB_CLASS_MISC 
 + desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS 
 + desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH);
  }

Does that mean that new kernels will stop working with old RNDIs
gadgets because they stop recognising them as RNDIS?  I feel like this
function should accept both, i.e.:

return (desc-bInterfaceClass == USB_CLASS_COMM 
desc-bInterfaceSubClass == 2 
desc-bInterfaceProtocol == 0xff) ||
   (desc-bInterfaceClass == USB_CLASS_MISC 
desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS 
desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH);

  
  static int is_activesync(struct usb_interface_descriptor *desc)
 diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
 index 358ca8d..a2a4e05 100644
 --- a/drivers/usb/core/generic.c
 +++ b/drivers/usb/core/generic.c
 @@ -28,9 +28,9 @@ static inline const char *plural(int n)
  
  static int is_rndis(struct usb_interface_descriptor *desc)
  {
 - return desc-bInterfaceClass == USB_CLASS_COMM
 -  desc-bInterfaceSubClass == 2
 -  desc-bInterfaceProtocol == 0xff;
 + return desc-bInterfaceClass == USB_CLASS_MISC
 +  desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS
 +  desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH;
  }

Ditto.

  
  static int is_activesync(struct usb_interface_descriptor *desc)
 diff --git a/drivers/usb/gadget/function/f_rndis.c 
 b/drivers/usb/gadget/function/f_rndis.c
 index ddb09dc..cc06046 100644
 --- a/drivers/usb/gadget/function/f_rndis.c
 +++ b/drivers/usb/gadget/function/f_rndis.c
 @@ -117,9 +117,9 @@ static struct usb_interface_descriptor rndis_control_intf 
 = {
   /* .bInterfaceNumber = DYNAMIC */
   /* status endpoint is optional; this could be patched later */
   .bNumEndpoints =1,
 - .bInterfaceClass =  USB_CLASS_COMM,
 - .bInterfaceSubClass =   USB_CDC_SUBCLASS_ACM,
 - .bInterfaceProtocol =   USB_CDC_ACM_PROTO_VENDOR,
 + .bInterfaceClass =  USB_CLASS_MISC,
 + .bInterfaceSubClass =   USB_CDC_SUBCLASS_RNDIS,
 + .bInterfaceProtocol =   USB_CDC_RNDIS_PROTO_ETH,
   /* .iInterface = DYNAMIC */
  };
  
 diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
 index b6a9cdd..8e8fc85 100644
 --- a/include/uapi/linux/usb/cdc.h
 +++ b/include/uapi/linux/usb/cdc.h
 @@ -12,6 +12,7 @@
  #include linux/types.h
  
  #define USB_CDC_SUBCLASS_ACM 0x02
 +#define USB_CDC_SUBCLASS_RNDIS   0x04
  #define USB_CDC_SUBCLASS_ETHERNET0x06
  #define USB_CDC_SUBCLASS_WHCM0x08
  #define USB_CDC_SUBCLASS_DMM 0x09
 @@ -31,6 +32,8 @@
  #define USB_CDC_ACM_PROTO_AT_CDMA6
  #define USB_CDC_ACM_PROTO_VENDOR 0xff
  
 +#define USB_CDC_RNDIS_PROTO_ETH  1
 +
  #define USB_CDC_PROTO_EEM7
  
  #define USB_CDC_NCM_PROTO_NTB1

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the 

Re: [PATCH 2/6] phy: improved lookup method

2014-09-24 Thread Kishon Vijay Abraham I
Hi,

On Tuesday 23 September 2014 05:13 PM, Heikki Krogerus wrote:
 On Tue, Sep 23, 2014 at 04:33:09PM +0530, Kishon Vijay Abraham I wrote:
 Hi,

 On Tuesday 23 September 2014 04:23 PM, Heikki Krogerus wrote:
 On Mon, Sep 22, 2014 at 05:07:55PM +0530, Kishon Vijay Abraham I wrote:
 On Thursday 18 September 2014 03:55 PM, Heikki Krogerus wrote:
 On Mon, Sep 15, 2014 at 03:35:08PM +0300, Heikki Krogerus wrote:
 On Fri, Sep 12, 2014 at 08:16:01PM +0530, Kishon Vijay Abraham I wrote:
 Assume you have 2 phys in your system..
 static struct phy_lookup usb_lookup = {
 .phy_name   = phy-usb.0,
 .dev_id = usb.0,
 .con_id = usb,
 };

 static struct phy_lookup sata_lookup = {
 .phy_name   = sata-usb.1,
 .dev_id = sata.0,
 .con_id = sata,
 };

 First you do modprobe phy-usb, the probe of USB PHY driver gets invoked 
 and it
 creates the PHY. The phy-core will find a free id (now it will be 0) 
 and then
 name the phy as phy-usb.0.
 Then with modprobe phy-sata, the phy-core will create phy-sata.1.

 This is an ideal case where the .phy_name in phy_lookup matches.

 Consider if the order is flipped and the user does modprobe phy-sata 
 first. The
 phy_names won't match anymore (the sata phy device name would be 
 sata-usb.0).

 Actually, I don't think there would be this problem if we used the
 name of the actual device which is the parent of phy devices, right?

 hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY 
 driver), we
 might end up with the same problem.

 I'm not completely sure what you mean? If you are talking about
 platforms with multiple instances of a single phy, I don't see how
 there could ever be a scenario where we did not know the order in
 which they were enumerated. Can you give an example again?

 If a single IP implements multiple PHYs (phy-miphy365x.c in linux-phy next),
 the parent for all the phy devices would be the same.
 
 OK, got it. So I guess we need to match to the phy dev and to the phy
 name. First to the dev and then in case the phy name is defined in the
 lookup, to that as well. That should cover both cases.

So what would be the phy name? I mean it's completely user-defined or it's
derived from device name?

Isn't making the PHY to be aware of it's user much simpler?

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


Re: [PATCH v2 1/2] mfd: viperboard: allocate I/O buffer separately

2014-09-24 Thread Lee Jones
On Mon, 22 Sep 2014, Octavian Purdila wrote:

 Currently the I/O buffer is allocated part of the device status
 structure, potentially sharing the same cache line with other members
 in this structure.
 
 Allocate the buffer separately, to avoid the I/O operations corrupting
 the device status structure due to cache line sharing.
 
 Compiled tested only as I don't have access to hardware.
 
 Signed-off-by: Octavian Purdila octavian.purd...@intel.com
 ---
  drivers/mfd/viperboard.c   | 8 
  include/linux/mfd/viperboard.h | 2 +-
  2 files changed, 9 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
 index e00f534..5f62f4e 100644
 --- a/drivers/mfd/viperboard.c
 +++ b/drivers/mfd/viperboard.c
 @@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface,
   return -ENOMEM;
   }
  
 + vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);

Can you obtain the 'struct device' first then use managed resources
(devm_*)?

 + if (vb-buf == NULL) {
 + ret = -ENOMEM;
 + goto error;
 + }
 +
   mutex_init(vb-lock);
  
   vb-usb_dev = usb_get_dev(interface_to_usbdev(interface));
 @@ -105,6 +111,7 @@ static int vprbrd_probe(struct usb_interface *interface,
  error:
   if (vb) {
   usb_put_dev(vb-usb_dev);
 + kfree(vb-buf);

Then you don't need this.

   kfree(vb);
   }
  
 @@ -118,6 +125,7 @@ static void vprbrd_disconnect(struct usb_interface 
 *interface)
   mfd_remove_devices(interface-dev);
   usb_set_intfdata(interface, NULL);
   usb_put_dev(vb-usb_dev);
 + kfree(vb-buf);

Or this.

   kfree(vb);
  
   dev_dbg(interface-dev, disconnected\n);
 diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h
 index 1934528..af928d0 100644
 --- a/include/linux/mfd/viperboard.h
 +++ b/include/linux/mfd/viperboard.h
 @@ -103,7 +103,7 @@ struct vprbrd_i2c_addr_msg {
  struct vprbrd {
   struct usb_device *usb_dev; /* the usb device for this device */
   struct mutex lock;
 - u8 buf[sizeof(struct vprbrd_i2c_write_msg)];
 + u8 *buf;
   struct platform_device pdev;
  };
  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] mfd: viperboard: remove redundant OOM message and NULL pointer check

2014-09-24 Thread Lee Jones
On Mon, 22 Sep 2014, Octavian Purdila wrote:

 Signed-off-by: Octavian Purdila octavian.purd...@intel.com
 ---
  drivers/mfd/viperboard.c | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)

Looks fine, but won't apply without patch 1.

 diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
 index 5f62f4e..d27c131 100644
 --- a/drivers/mfd/viperboard.c
 +++ b/drivers/mfd/viperboard.c
 @@ -59,10 +59,8 @@ static int vprbrd_probe(struct usb_interface *interface,
  
   /* allocate memory for our device state and initialize it */
   vb = kzalloc(sizeof(*vb), GFP_KERNEL);
 - if (vb == NULL) {
 - dev_err(interface-dev, Out of memory\n);
 + if (vb == NULL)
   return -ENOMEM;
 - }
  
   vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
   if (vb-buf == NULL) {
 @@ -109,11 +107,9 @@ static int vprbrd_probe(struct usb_interface *interface,
   return 0;
  
  error:
 - if (vb) {
 - usb_put_dev(vb-usb_dev);
 - kfree(vb-buf);
 - kfree(vb);
 - }
 + usb_put_dev(vb-usb_dev);
 + kfree(vb-buf);
 + kfree(vb);
  
   return ret;
  }

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] mfd: viperboard: allocate I/O buffer separately

2014-09-24 Thread Johan Hovold
On Wed, Sep 24, 2014 at 11:12:06AM +0100, Lee Jones wrote:
 On Mon, 22 Sep 2014, Octavian Purdila wrote:
 
  Currently the I/O buffer is allocated part of the device status
  structure, potentially sharing the same cache line with other members
  in this structure.
  
  Allocate the buffer separately, to avoid the I/O operations corrupting
  the device status structure due to cache line sharing.
  
  Compiled tested only as I don't have access to hardware.
  
  Signed-off-by: Octavian Purdila octavian.purd...@intel.com
  ---
   drivers/mfd/viperboard.c   | 8 
   include/linux/mfd/viperboard.h | 2 +-
   2 files changed, 9 insertions(+), 1 deletion(-)
  
  diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
  index e00f534..5f62f4e 100644
  --- a/drivers/mfd/viperboard.c
  +++ b/drivers/mfd/viperboard.c
  @@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface,
  return -ENOMEM;
  }
   
  +   vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
 
 Can you obtain the 'struct device' first then use managed resources
 (devm_*)?

I think any devres conversion should be done in a follow-up patch and
not be included in the fix (e.g. in order to facilitate backporting). We
also don't want to mix allocation schemes.

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/4] mfd: add support for Diolan DLN-2 devices

2014-09-24 Thread Johan Hovold
On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote:
 This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
 Master Adapter DLN-2. Details about the device can be found here:
 
 https://www.diolan.com/i2c/i2c_interface.html.
 
 Information about the USB protocol can be found in the Programmer's
 Reference Manual [1], see section 1.7.
 
 Because the hardware has a single transmit endpoint and a single
 receive endpoint the communication between the various DLN2 drivers
 and the hardware will be muxed/demuxed by this driver.
 
 Each DLN2 module will be identified by the handle field within the DLN2
 message header. If a DLN2 module issues multiple commands in parallel
 they will be identified by the echo counter field in the message header.
 
 The DLN2 modules can use the dln2_transfer() function to issue a
 command and wait for its response. They can also register a callback
 that is going to be called when a specific event id is generated by
 the device (e.g. GPIO interrupts). The device uses handle 0 for
 sending events.
 
 [1] https://www.diolan.com/downloads/dln-api-manual.pdf
 
 Signed-off-by: Octavian Purdila octavian.purd...@intel.com
 ---
  drivers/mfd/Kconfig  |   9 +
  drivers/mfd/Makefile |   1 +
  drivers/mfd/dln2.c   | 758 
 +++
  include/linux/mfd/dln2.h |  67 +
  4 files changed, 835 insertions(+)
  create mode 100644 drivers/mfd/dln2.c
  create mode 100644 include/linux/mfd/dln2.h
 
 diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
 index de5abf2..7bcf895 100644
 --- a/drivers/mfd/Kconfig
 +++ b/drivers/mfd/Kconfig
 @@ -183,6 +183,15 @@ config MFD_DA9063
 Additional drivers must be enabled in order to use the functionality
 of the device.
  
 +config MFD_DLN2
 + tristate Diolan DLN2 support
 + select MFD_CORE
 + depends on USB
 + help
 +   This adds support for Diolan USB-I2C/SPI/GPIO Master Adapter DLN-2.
 +   Additional drivers must be enabled in order to use the functionality
 +   of the device.
 +
  config MFD_MC13XXX
   tristate
   depends on (SPI_MASTER || I2C)
 diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
 index f001487..591988d 100644
 --- a/drivers/mfd/Makefile
 +++ b/drivers/mfd/Makefile
 @@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711)  += as3711.o
  obj-$(CONFIG_MFD_AS3722) += as3722.o
  obj-$(CONFIG_MFD_STW481X)+= stw481x.o
  obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
 +obj-$(CONFIG_MFD_DLN2)   += dln2.o
  
  intel-soc-pmic-objs  := intel_soc_pmic_core.o intel_soc_pmic_crc.o
  obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
 diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
 new file mode 100644
 index 000..36c53cd
 --- /dev/null
 +++ b/drivers/mfd/dln2.c
 @@ -0,0 +1,758 @@
 +/*
 + * Driver for the Diolan DLN-2 USB adapter
 + *
 + * Copyright (c) 2014 Intel Corporation
 + *
 + * Derived from:
 + *  i2c-diolan-u2c.c
 + *  Copyright (c) 2010-2011 Ericsson AB
 + *
 + * 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, version 2.
 + */
 +
 +#include linux/kernel.h
 +#include linux/module.h
 +#include linux/types.h
 +#include linux/slab.h
 +#include linux/usb.h
 +#include linux/i2c.h
 +#include linux/mutex.h
 +#include linux/platform_device.h
 +#include linux/mfd/core.h
 +#include linux/mfd/dln2.h
 +#include linux/rculist.h
 +
 +struct dln2_header {
 + __le16 size;
 + __le16 id;
 + __le16 echo;
 + __le16 handle;
 +} __packed;
 +
 +struct dln2_response {
 + struct dln2_header hdr;
 + __le16 result;
 +} __packed;
 +
 +#define DLN2_GENERIC_MODULE_ID   0x00
 +#define DLN2_GENERIC_CMD(cmd)DLN2_CMD(cmd, 
 DLN2_GENERIC_MODULE_ID)
 +#define CMD_GET_DEVICE_VER   DLN2_GENERIC_CMD(0x30)
 +#define CMD_GET_DEVICE_SNDLN2_GENERIC_CMD(0x31)
 +
 +#define DLN2_HW_ID   0x200
 +#define DLN2_USB_TIMEOUT 200 /* in ms */
 +#define DLN2_MAX_RX_SLOTS16
 +#define DLN2_MAX_URBS16
 +#define DLN2_RX_BUF_SIZE 512
 +
 +#define DLN2_HANDLE_EVENT0
 +#define DLN2_HANDLE_CTRL 1
 +#define DLN2_HANDLE_GPIO 2
 +#define DLN2_HANDLE_I2C  3
 +#define DLN2_HANDLES 4

This should be an enum.

 +
 +
 +/*
 + * Receive context used between the receive demultiplexer and the
 + * transfer routine. While sending a request the transfer routine
 + * will look for a free receive context and use it to wait for a
 + * response and to receive the URB and thus the response data.
 + */
 +struct dln2_rx_context {
 + /* completion used to wait a response */

wait for

 + struct completion done;
 +
 + /* if non-NULL the URB contains the response */
 + struct urb *urb;
 +
 + /* if true then 

Re: [PATCH v5 2/4] i2c: add support for Diolan DLN-2 USB-I2C adapter

2014-09-24 Thread Johan Hovold
On Fri, Sep 19, 2014 at 11:22:43PM +0300, Octavian Purdila wrote:

 +struct dln2_i2c {
 + struct platform_device *pdev;
 + struct i2c_adapter adapter;
 + u32 freq;
 + u32 min_freq;
 + u32 max_freq;
 + /*
 +  * Buffer to hold the packet for read or write transfers. One
 +  * is enough since we can't have multiple transfers in
 +  * parallel on the i2c adapter.
 +  */
 + union {
 + struct {
 + u8 port;
 + u8 addr;
 + u8 mem_addr_len;
 + __le32 mem_addr;
 + __le16 buf_len;
 + u8 buf[DLN2_I2C_MAX_XFER_SIZE];
 + } __packed tx;
 + struct {
 + __le16 buf_len;
 + u8 buf[DLN2_I2C_MAX_XFER_SIZE];
 + } __packed rx;
 + } *buf;

Please declare these structs separately from the dln2_i2c struct so you
can use temporary pointers below.

Perhaps just allocate a large enough buffer (or page) here and keep you
transfer-buffer declarations in the two functions where they are used if
you prefer.

 +};

 +static int dln2_i2c_enable(struct dln2_i2c *dln2, bool enable)
 +{
 + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev);
 + int ret;
 + u8 port;
 + u16 cmd;
 +
 + port = pdata-port;

Store the port number in the dln2_i2c struct rather than access it
through the platform data for every I/O operation.

 +
 + if (enable)
 + cmd = DLN2_I2C_ENABLE;
 + else
 + cmd = DLN2_I2C_DISABLE;
 +
 + ret = dln2_transfer(dln2-pdev, cmd, port, sizeof(port), NULL, NULL);
 + if (ret  0)
 + return ret;
 +
 + return 0;
 +}
 +
 +static int dln2_i2c_set_frequency(struct dln2_i2c *dln2, u32 freq)
 +{
 + int ret;
 + struct tx_data {
 + u8 port;
 + __le32 freq;
 + } __packed tx;
 + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev);
 +
 + tx.port = pdata-port;
 + tx.freq = cpu_to_le32(freq);
 +
 + ret = dln2_transfer(dln2-pdev, DLN2_I2C_SET_FREQUENCY, tx, sizeof(tx),
 + NULL, NULL);
 + if (ret  0)
 + return ret;
 +
 + dln2-freq = freq;
 +
 + return 0;
 +}
 +
 +static int dln2_i2c_get_freq(struct dln2_i2c *dln2, u16 cmd, u32 *res)
 +{
 + int ret;
 + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev);
 + u8 port = pdata-port;
 + __le32 freq;
 + unsigned len = sizeof(freq);
 +
 + ret = dln2_transfer(dln2-pdev, cmd, port, sizeof(port),
 + freq, len);
 + if (ret  0)
 + return ret;
 + if (len  sizeof(freq))
 + return -EPROTO;
 +
 + *res = le32_to_cpu(freq);
 +
 + return 0;
 +}
 +
 +static int dln2_i2c_setup(struct dln2_i2c *dln2)
 +{
 + int ret;
 +
 + ret = dln2_i2c_enable(dln2, false);
 + if (ret  0)
 + return ret;
 +
 + ret = dln2_i2c_get_freq(dln2, DLN2_I2C_GET_MIN_FREQUENCY,
 + dln2-min_freq);
 + if (ret  0)
 + return 0;

return ret

 +
 + ret = dln2_i2c_get_freq(dln2, DLN2_I2C_GET_MAX_FREQUENCY,
 + dln2-max_freq);
 + if (ret  0)
 + return 0;

return ret

 +
 + ret = dln2_i2c_set_frequency(dln2, DLN2_I2C_FREQ_STD);
 + if (ret  0)
 + return ret;
 +
 + ret = dln2_i2c_enable(dln2, true);
 +
 + return ret;
 +}
 +
 +static int dln2_i2c_write(struct dln2_i2c *dln2, u8 addr,
 +   u8 *data, u16 data_len)
 +{
 + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev);
 + int ret;
 + unsigned len;
 +
 + dln2-buf-tx.port = pdata-port;
 + dln2-buf-tx.addr = addr;
 + dln2-buf-tx.mem_addr_len = 0;
 + dln2-buf-tx.mem_addr = 0;
 + dln2-buf-tx.buf_len = cpu_to_le16(data_len);
 + memcpy(dln2-buf-tx.buf, data, data_len);
 +
 + len = sizeof(dln2-buf-tx) + data_len - DLN2_I2C_MAX_XFER_SIZE;
 + ret = dln2_transfer(dln2-pdev, DLN2_I2C_WRITE, dln2-buf-tx, len,
 + NULL, NULL);

Use a temporary pointer to the tx struct above for readability.

 + if (ret  0)
 + return ret;
 +
 + return data_len;
 +}
 +
 +static int dln2_i2c_read(struct dln2_i2c *dln2, u16 addr, u8 *data,
 +  u16 data_len)
 +{
 + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev);
 + int ret;
 + struct {
 + u8 port;
 + u8 addr;
 + u8 mem_addr_len;
 + __le32 mem_addr;
 + __le16 buf_len;
 + } __packed tx;
 + unsigned rx_len;
 +
 + tx.port = pdata-port;
 + tx.addr = addr;
 + tx.mem_addr_len = 0;
 + tx.mem_addr = 0;
 + tx.buf_len = cpu_to_le16(data_len);
 +
 + rx_len = sizeof(dln2-buf-rx);
 +
 + ret = dln2_transfer(dln2-pdev, DLN2_I2C_READ, tx, sizeof(tx),
 + 

Re: [PATCH v5 3/4] gpiolib: add irq_not_threaded flag to gpio_chip

2014-09-24 Thread Johan Hovold
On Wed, Sep 24, 2014 at 10:54:51AM +0200, Linus Walleij wrote:
 On Fri, Sep 19, 2014 at 10:22 PM, Octavian Purdila
 octavian.purd...@intel.com wrote:
 
  Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set
  operation but do not need a threaded irq handler.
 
  Signed-off-by: Octavian Purdila octavian.purd...@intel.com
 
 Usually I don't apply patches adding interfaces with no users, but
 this seems very useful, so patch applied. I guess your driver will
 appear on v3.19+ so then you can rely on this having been merged
 for v3.18.

Octavian, please include this one in any future revision of you series
so that it is self-contained (at least until v3.18-rc1 is out)
nonetheless.

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


[PATCH] USB: Remove .owner field for driver

2014-09-24 Thread Kiran Padwal
There is no need to init .owner field.

Based on the patch from Peter Griffin peter.grif...@linaro.org
mmc: remove .owner field for drivers using module_platform_driver

This patch removes the superflous .owner field for drivers which
use the module_platform_driver API, as this is overriden in
platform_driver_register anyway.

Signed-off-by: Kiran Padwal kiran.pad...@smartplayin.com
---
 drivers/usb/dwc3/dwc3-keystone.c |1 -
 drivers/usb/dwc3/dwc3-qcom.c |1 -
 drivers/usb/phy/phy-msm-usb.c|1 -
 3 files changed, 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
index 1fad161..7ec8495 100644
--- a/drivers/usb/dwc3/dwc3-keystone.c
+++ b/drivers/usb/dwc3/dwc3-keystone.c
@@ -189,7 +189,6 @@ static struct platform_driver kdwc3_driver = {
.remove = kdwc3_remove,
.driver = {
.name   = keystone-dwc3,
-   .owner  = THIS_MODULE,
.of_match_table = kdwc3_of_match,
},
 };
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 611f8e7..8c2e8ee 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -118,7 +118,6 @@ static struct platform_driver dwc3_qcom_driver = {
.remove = dwc3_qcom_remove,
.driver = {
.name   = qcom-dwc3,
-   .owner  = THIS_MODULE,
.of_match_table = of_dwc3_match,
},
 };
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 7bb48af..7843ef7 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1841,7 +1841,6 @@ static struct platform_driver msm_otg_driver = {
.remove = msm_otg_remove,
.driver = {
.name = DRIVER_NAME,
-   .owner = THIS_MODULE,
.pm = msm_otg_dev_pm_ops,
.of_match_table = msm_otg_dt_match,
},
-- 
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: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

2014-09-24 Thread Peter Chen
On Wed, Sep 24, 2014 at 10:30:41AM +0200, Arnd Bergmann wrote:
 On Wednesday 24 September 2014 09:44:19 Arnd Bergmann wrote:
  
  We can also gradually move in some of the other glue drivers into
  the main driver if the differences are small enough.
  
 
 FWIW, I've just looked at the other glue drivers that already
 exist:
 
 - zevio can just get merged into the common driver, all that seems
   to be needed for that is the additional compatible string, and
   keying off the ci_default_zevio_platdata on the .data field of
   the of_device_id table.
 
 - msm has a custom notifier, which justifies leaving it in a separate
   driver, but it's also small enough that it wouldn't hurt to have
   that merged into the main driver too.
 
 - imx requires a lot of other things, in particular the dependency
   on the usbmisc driver means we don't want to have that in the
   core anyway, so we can't really merge that in.
 
 - the proposed ar933x driver again looks almost trivial, so no reason
   for a separate glue driver for that.
 
   Arnd

Thanks, Arnd.

So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver
(dwc3, musb, chipidea) you are talking about, right? Except for
creating another platform driver as well as related DT node (optional),
are there any advantages compared to current IP core driver structure?

In this thread, we are talking about creating common platform driver for glue
layer, its design purpose (adapt it for as many as platforms) should be the
same, no matter the IP core part is a LIB or platform driver, am I missing
something?

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


Poor performance with USB 1.1 drive connected to USB 3.0 port

2014-09-24 Thread Mark Knibbs
Hi,

I did some benchmarks to check the maximum transfer rate of a USB-to-SCSI
converter. The converter is USB 1.1, so limited to the 12Mbps full speed
rate. The performance when connected to a USB 3.0 port is significantly
worse than when connected to a USB 2.0 port, about 26.5% slower (0.63MB/sec
vs 0.86MB/sec).

The USB 3.0 port is provided by an ExpressCard which has a Renesas
controller. It doesn't seem to be defective because I can get 138MB/sec
from a USB 3.0 hard disk. lspci shows
  05:00.0 USB Controller: Renesas Technology Corp. Device 0015 (rev 02)

I didn't test with an unmodified mainline kernel, but the issue is present
with both Ubuntu kernel 3.0 and Debian 3.14 (booted from Kali Linux live
DVD).

I used sg_rbuf from the sg3-utils package. That issues READ BUFFER commands
instead of reading data from disk, so the result should be closer to the
maximum achievable. The USB-SCSI converter was a Newer Technology uSCSI
connected to an HP 2600fx MO drive.

In a USB 2.0 port:

# sg_rbuf --buffer=524288 -v -t -q /dev/sg3
Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 
READ BUFFER reports: buffer capacity=983040, offset boundary=0
time to read data from buffer was 242.800241 secs, 0.86 MB/sec
Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes)

# sg_rbuf --buffer=524288 -v -t -q /dev/sg3
Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 
READ BUFFER reports: buffer capacity=983040, offset boundary=0
time to read data from buffer was 242.800230 secs, 0.86 MB/sec
Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes)


In a USB 3.0 port:

# sg_rbuf --buffer=524288 -v -t -q /dev/sg3
Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 
READ BUFFER reports: buffer capacity=983040, offset boundary=0
time to read data from buffer was 330.311337 secs, 0.63 MB/sec
Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes)

# sg_rbuf --buffer=524288 -v -t -q /dev/sg3
Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 
READ BUFFER reports: buffer capacity=983040, offset boundary=0
time to read data from buffer was 330.388556 secs, 0.63 MB/sec
Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes)


By comparison, I checked the transfer rate in native Windows (Vista) with
the WinSAT program. That reported 0.66MB/sec for both USB 2.0 and 3.0
ports. (WinSAT reads data from disk, probably in smaller chunks than 512KB.
There's no sg_rbuf executable in the Windows sg3_utils archive.)

Any ideas what the reason for the discrepancy might be? Can anyone else
reproduce it? I guess USB 1.1-only devices aren't that common nowadays.


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


Re: [PATCH v2 1/2] mfd: viperboard: allocate I/O buffer separately

2014-09-24 Thread Lee Jones
On Wed, 24 Sep 2014, Johan Hovold wrote:

 On Wed, Sep 24, 2014 at 11:12:06AM +0100, Lee Jones wrote:
  On Mon, 22 Sep 2014, Octavian Purdila wrote:
  
   Currently the I/O buffer is allocated part of the device status
   structure, potentially sharing the same cache line with other members
   in this structure.
   
   Allocate the buffer separately, to avoid the I/O operations corrupting
   the device status structure due to cache line sharing.
   
   Compiled tested only as I don't have access to hardware.
   
   Signed-off-by: Octavian Purdila octavian.purd...@intel.com
   ---
drivers/mfd/viperboard.c   | 8 
include/linux/mfd/viperboard.h | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
   
   diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
   index e00f534..5f62f4e 100644
   --- a/drivers/mfd/viperboard.c
   +++ b/drivers/mfd/viperboard.c
   @@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface 
   *interface,
 return -ENOMEM;
 }

   + vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
  
  Can you obtain the 'struct device' first then use managed resources
  (devm_*)?
 
 I think any devres conversion should be done in a follow-up patch and
 not be included in the fix (e.g. in order to facilitate backporting). We
 also don't want to mix allocation schemes.

I agree, but equally I'm not keen on accepting this patch as I believe
it could be done better.

Please submit two patches, one converting to shared resources and this
being the subsequent one, fixed up to do the right thing.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-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 v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

2014-09-24 Thread Arnd Bergmann
On Wednesday 24 September 2014 19:29:05 Peter Chen wrote:
 
 So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver
 (dwc3, musb, chipidea) you are talking about, right? Except for
 creating another platform driver as well as related DT node (optional),
 are there any advantages compared to current IP core driver structure?

Having a library module usually requires less code, and is more consistent
with other drivers, which helps new developers understand what the
driver is doing.

The most important aspect though is the DT binding: once the structure
with separate kernel drivers leaks out into the DT format, you can't
easily change the driver any more, e.g. to make a property that was
introduced for one glue driver more general so it can get handled by
the common part. Having a single node lets us convert to the library
model later, so that would be a strong reason to keep the DT binding
simple, without child nodes.

Following that logic, it turns into another reason for using the library
model to start with, because otherwise the child device does not have
any DT properties itself and has to rely on the parent device properties,
which somewhat complicates the driver structure.

 In this thread, we are talking about creating common platform driver for glue
 layer, its design purpose (adapt it for as many as platforms) should be the
 same, no matter the IP core part is a LIB or platform driver, am I missing
 something?

No, you are absolutely right with that, introducing the generic glue
driver is orthogonal to the structure of the core driver in principle.

I mainly brought it up because I noticed how this patch could be done
in a simpler way by combining the new generic glue with the move to
a library driver model.

Arnd
--
To unsubscribe from this list: send the line unsubscribe 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][RFC] storage: Simplify condition to reject bad target in usb_stor_control_thread()

2014-09-24 Thread Mark Knibbs
Hi,

This follows on from something I mentioned in my recent post [PATCH]
storage: Don't scan target 7 for SCM USB-SCSI converters.

In drivers/usb/storage/usb.c, usb_stor_control_thread():
/* reject if target != 0 or if LUN is higher than
 * the maximum known LUN
 */
else if (us-srb-device-id 
!(us-fflags  US_FL_SCM_MULT_TARG)) {
usb_stor_dbg(us, Bad target number (%d:%llu)\n,
 us-srb-device-id,
 us-srb-device-lun);
us-srb-result = DID_BAD_TARGET  16;
}

else if (us-srb-device-lun  us-max_lun) {
usb_stor_dbg(us, Bad LUN (%d:%llu)\n,
...

The condition (us-srb-device-id  !(us-fflags  US_FL_SCM_MULT_TARG))
means:
 - If not SCM_MULT_TARG, reject if us-srb-device-id is non-zero. That
   works for the usual case.
 - If SCM_MULT_TARG, never reject. Hmm...

Would it be better/clearer to change the condition to simply
  (us-srb-device-id = host-max_id)

Then the command is rejected if us-srb-device-id is too large, and works
for the SCM_MULT_TARG case too. (host-max_id is 1 for the non-SCM_MULT_TARG
case.) Plus the condition matches nicely with the later LUN check. Untested
patch below.


Signed-off-by: Mark Knibbs ma...@clara.co.uk

diff -up linux-3.17-rc6/drivers/usb/storage/usb.c.orig 
linux-3.17-rc6/drivers/usb/storage/usb.c 
--- linux-3.17-rc6/drivers/usb/storage/usb.c.orig   2014-09-21 
23:43:02.0 +0100
+++ linux-3.17-rc6/drivers/usb/storage/usb.c2014-09-24 13:09:55.0 
+0100
@@ -342,11 +342,10 @@ static int usb_stor_control_thread(void
us-srb-result = DID_ERROR  16;
}
 
-   /* reject if target != 0 or if LUN is higher than
+   /* reject if target is invalid or if LUN is higher than
 * the maximum known LUN
 */
-   else if (us-srb-device-id 
-   !(us-fflags  US_FL_SCM_MULT_TARG)) {
+   else if (us-srb-device-id = host-max_id) {
usb_stor_dbg(us, Bad target number (%d:%llu)\n,
 us-srb-device-id,
 us-srb-device-lun);
--
To unsubscribe from this list: send the line unsubscribe linux-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: f_rndis: fix usb_interface_descriptor for rndis

2014-09-24 Thread Lars Melin

On 2014-09-24 13:48, Heiko Schocher wrote:

use the values for RNDIS over Ethernet as defined in
http://www.usb.org/developers/defined_class
(search for RDNIS):

- baseclass: 0xef (miscellaneous)
- subclass: 0x04
- protocol: 0x01


That is usb class, it is not the same thing as communication device class.

--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -12,6 +12,7 @@
  #include linux/types.h
  
  #define USB_CDC_SUBCLASS_ACM			0x02

+#define USB_CDC_SUBCLASS_RNDIS 0x04

No, no, no.
There is no CDC_SUBCLASS_RNDIS and you can not define one over an 
already used cdc subclass number,  0x04 is  Multi-Channel Control Model


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


Re: [PATCH v2 1/2] mfd: viperboard: allocate I/O buffer separately

2014-09-24 Thread Johan Hovold
On Wed, Sep 24, 2014 at 01:00:02PM +0100, Lee Jones wrote:
 On Wed, 24 Sep 2014, Johan Hovold wrote:
 
  On Wed, Sep 24, 2014 at 11:12:06AM +0100, Lee Jones wrote:
   On Mon, 22 Sep 2014, Octavian Purdila wrote:
   
Currently the I/O buffer is allocated part of the device status
structure, potentially sharing the same cache line with other members
in this structure.

Allocate the buffer separately, to avoid the I/O operations corrupting
the device status structure due to cache line sharing.

Compiled tested only as I don't have access to hardware.

Signed-off-by: Octavian Purdila octavian.purd...@intel.com
---
 drivers/mfd/viperboard.c   | 8 
 include/linux/mfd/viperboard.h | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index e00f534..5f62f4e 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface 
*interface,
return -ENOMEM;
}
 
+   vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), 
GFP_KERNEL);
   
   Can you obtain the 'struct device' first then use managed resources
   (devm_*)?
  
  I think any devres conversion should be done in a follow-up patch and
  not be included in the fix (e.g. in order to facilitate backporting). We
  also don't want to mix allocation schemes.
 
 I agree, but equally I'm not keen on accepting this patch as I believe
 it could be done better.

 Please submit two patches, one converting to shared resources and this
 being the subsequent one, fixed up to do the right thing.

A buffer-corruption fix is a candidate for stable, whereas a devres
conversion (clean up) is not. Hence the former should not depend on the
latter.

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 v2 1/2] mfd: viperboard: allocate I/O buffer separately

2014-09-24 Thread Octavian Purdila
On Wed, Sep 24, 2014 at 3:26 PM, Johan Hovold jo...@kernel.org wrote:
 On Wed, Sep 24, 2014 at 01:00:02PM +0100, Lee Jones wrote:
 On Wed, 24 Sep 2014, Johan Hovold wrote:

  On Wed, Sep 24, 2014 at 11:12:06AM +0100, Lee Jones wrote:
   On Mon, 22 Sep 2014, Octavian Purdila wrote:
  
Currently the I/O buffer is allocated part of the device status
structure, potentially sharing the same cache line with other members
in this structure.
   
Allocate the buffer separately, to avoid the I/O operations corrupting
the device status structure due to cache line sharing.
   
Compiled tested only as I don't have access to hardware.
   
Signed-off-by: Octavian Purdila octavian.purd...@intel.com
---
 drivers/mfd/viperboard.c   | 8 
 include/linux/mfd/viperboard.h | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)
   
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index e00f534..5f62f4e 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface 
*interface,
return -ENOMEM;
}
   
+   vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), 
GFP_KERNEL);
  
   Can you obtain the 'struct device' first then use managed resources
   (devm_*)?
 
  I think any devres conversion should be done in a follow-up patch and
  not be included in the fix (e.g. in order to facilitate backporting). We
  also don't want to mix allocation schemes.

 I agree, but equally I'm not keen on accepting this patch as I believe
 it could be done better.

 Please submit two patches, one converting to shared resources and this
 being the subsequent one, fixed up to do the right thing.

 A buffer-corruption fix is a candidate for stable, whereas a devres
 conversion (clean up) is not. Hence the former should not depend on the
 latter.


I can follow-up with a v3 3 patch series: first for the fix, second
for the OOM  error path cleanup, third for devm conversion.

I think this will address both issues (backport and better final result).
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] mfd: viperboard: allocate I/O buffer separately

2014-09-24 Thread Johan Hovold
On Wed, Sep 24, 2014 at 03:34:08PM +0300, Octavian Purdila wrote:

 I can follow-up with a v3 3 patch series: first for the fix, second
 for the OOM  error path cleanup, third for devm conversion.

I'd include the error-path clean up bit in the devres conversion as that
is what it's really all about.

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 4/4] gpio: add support for the Diolan DLN-2 USB GPIO driver

2014-09-24 Thread Johan Hovold
On Fri, Sep 19, 2014 at 11:22:45PM +0300, Octavian Purdila wrote:

 +struct dln2_gpio {
 + struct platform_device *pdev;
 + struct gpio_chip gpio;
 +
 + /*
 +  * Cache pin direction to save us one transfer, since the
 +  * hardware has separate commands to read the in and out
 +  * values. Bit set for out, bit clear for in.
 +  */
 + DECLARE_BITMAP(pin_dir, DLN2_GPIO_MAX_PINS);

Using the more common name output_enabled might be preferred? Then you
can even get rid of (part of) the comment.

 +
 + DECLARE_BITMAP(irqs_masked, DLN2_GPIO_MAX_PINS);
 + DECLARE_BITMAP(irqs_enabled, DLN2_GPIO_MAX_PINS);
 + DECLARE_BITMAP(irqs_pending, DLN2_GPIO_MAX_PINS);
 + struct dln2_irq_work *irq_work;
 +};

[...]

 +#define DLN2_GPIO_DIRECTION_IN   0
 +#define DLN2_GPIO_DIRECTION_OUT  1
 +
 +static int dln2_gpio_request(struct gpio_chip *chip, unsigned offset)
 +{
 + struct dln2_gpio *dln2 = container_of(chip, struct dln2_gpio, gpio);
 + struct dln2_gpio_pin req = {
 + .pin = cpu_to_le16(offset),
 + };
 + struct dln2_gpio_pin_val rsp;
 + int len = sizeof(rsp);
 + int ret;
 +
 + ret = dln2_gpio_pin_cmd(dln2, DLN2_GPIO_PIN_ENABLE, offset);
 + if (ret  0)
 + return ret;
 +
 + /* cache the pin direction */
 + ret = dln2_transfer(dln2-pdev, DLN2_GPIO_PIN_GET_DIRECTION,
 + req, sizeof(req), rsp, len);
 + if (ret  0)
 + return ret;
 + if (len  sizeof(rsp) || req.pin != rsp.pin)
 + return -EPROTO;

Disable the pin?

 +
 + switch (rsp.value) {
 + case DLN2_GPIO_DIRECTION_IN:
 + clear_bit(offset, dln2-pin_dir);
 + return 0;
 + case DLN2_GPIO_DIRECTION_OUT:
 + set_bit(offset, dln2-pin_dir);
 + return 0;
 + default:
 + return -EPROTO;

Disable the pin?

 + }
 +}

[...]

 +static struct platform_driver dln2_gpio_driver = {
 + .driver.name= dln2-gpio,
 + .driver.owner   = THIS_MODULE,

No need to set the owner field when using the module_platform_driver
macro.

 + .probe  = dln2_gpio_probe,
 + .remove = dln2_gpio_remove,
 +};
 +
 +module_platform_driver(dln2_gpio_driver);
 +
 +MODULE_AUTHOR(Daniel Baluta daniel.bal...@intel.com);
 +MODULE_DESCRIPTION(Driver for the Diolan DLN2 GPIO interface);
 +MODULE_LICENSE(GPL v2);
 +MODULE_ALIAS(platform:dln2-gpio);

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: f_rndis: fix usb_interface_descriptor for rndis

2014-09-24 Thread Heiko Schocher

Hello Michal,

Am 24.09.2014 11:38, schrieb Michal Nazarewicz:

On Wed, Sep 24 2014, Heiko Schocherh...@denx.de  wrote:

use the values for RNDIS over Ethernet as defined in
http://www.usb.org/developers/defined_class
(search for RDNIS):

- baseclass: 0xef (miscellaneous)
- subclass: 0x04
- protocol: 0x01

with this setings the file in Documentation/usb/linux.inf is
obsolete.

Signed-off-by: Heiko Schocherh...@denx.de

---

Cc: Felipe Balbiba...@ti.com
Cc: Greg Kroah-Hartmangre...@suse.de
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Oliver Neukumoli...@neukum.name
Cc: net...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: Andrzej Pietrasiewiczandrze...@samsung.com
Cc: Michal Nazarewiczmin...@mina86.com
Cc: Kyungmin Parkkyungmin.p...@samsung.com
Cc: Dan Carpenterdan.carpen...@oracle.com
Cc: Macpaul Linmacp...@gmail.com

Tested with the USB Compliance test suite which runs Windows, see:
http://www.usb.org/developers/tools/usb20_tools/#usb20cv

  drivers/net/usb/cdc_ether.c   | 6 +++---
  drivers/usb/core/generic.c| 6 +++---
  drivers/usb/gadget/function/f_rndis.c | 6 +++---
  include/uapi/linux/usb/cdc.h  | 3 +++
  4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 2a32d91..9c216c2 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -35,9 +35,9 @@

  static int is_rndis(struct usb_interface_descriptor *desc)
  {
-   return (desc-bInterfaceClass == USB_CLASS_COMM
-   desc-bInterfaceSubClass == 2
-   desc-bInterfaceProtocol == 0xff);
+   return (desc-bInterfaceClass == USB_CLASS_MISC
+   desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS
+   desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH);
  }


Does that mean that new kernels will stop working with old RNDIs
gadgets because they stop recognising them as RNDIS?  I feel like this
function should accept both, i.e.:


Hmm.. I am not a usb guru ... but I think, yes, you are right.
I add this to a v2 (if this patch has a chance to go in mainline).

Thanks!

bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
To unsubscribe from this list: send the line unsubscribe linux-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: f_rndis: fix usb_interface_descriptor for rndis

2014-09-24 Thread Heiko Schocher

Hello Lars,

Am 24.09.2014 14:25, schrieb Lars Melin:

On 2014-09-24 13:48, Heiko Schocher wrote:

use the values for RNDIS over Ethernet as defined in
http://www.usb.org/developers/defined_class
(search for RDNIS):

- baseclass: 0xef (miscellaneous)
- subclass: 0x04
- protocol: 0x01


That is usb class, it is not the same thing as communication device class.

--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -12,6 +12,7 @@
#include linux/types.h
#define USB_CDC_SUBCLASS_ACM 0x02
+#define USB_CDC_SUBCLASS_RNDIS 0x04

No, no, no.
There is no CDC_SUBCLASS_RNDIS and you can not define one over an already used 
cdc subclass number, 0x04 is Multi-Channel Control Model


Ah, ok, so I have to define this values in a new header file, as there
is no current file for the USB_CLASS_MISC defines? Or is there a proper
place for them?

BTW: where do I find the cdc subclass number, 0x04 is Multi-Channel
Control Model define?

bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
To unsubscribe from this list: send the line unsubscribe 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] Equivalent of g_webcam with configfs

2014-09-24 Thread Andrzej Pietrasiewicz
This series aims at integrating configfs into uvc, the way
it has been done for acm, ncm, ecm, eem, ecm subset, rndis, obex, phonet,
mass_storage, FunctionFS, loopback, sourcesink, uac1 and uac2.

The preparation series is already on Felipe's next, so this series
in fact consists of a small preparation patch and the configfs support
proper.

Rebased onto Felipe's next.

@Felipe: I know you asked for not sending patches until 3.18-rc1 is out.
I'm sending this short series anyway, so that e.g. Laurent can have
a look at it.

@Laurent: Can you have a look at this series?

Below is an example listing of gadget's function directory. The attributes
whose access permissions are -r--r--r-- are there only for inspection
by the user but not for modifications. For color matching, camera
terminal, processing unit and output terminal only default descriptors
are provided, that is, creating user's own is not supported at this
moment.

drwxr-xr-x .  
drwxr-xr-x ./streaming  
drwxr-xr-x ./streaming/class  
drwxr-xr-x ./streaming/class/ss  
drwxr-xr-x ./streaming/class/hs  
lrwxrwxrwx ./streaming/class/hs/h - \
../../../../../../../usb_gadget/g1/functions/uvc.usb0/streaming/header/h
drwxr-xr-x ./streaming/class/fs  
lrwxrwxrwx ./streaming/class/fs/h - \
../../../../../../../usb_gadget/g1/functions/uvc.usb0/streaming/header/h
drwxr-xr-x ./streaming/color_matching  
drwxr-xr-x ./streaming/color_matching/default  
-r--r--r-- ./streaming/color_matching/default/bMatrixCoefficients  
-r--r--r-- ./streaming/color_matching/default/bTransferCharacteristics  
-r--r--r-- ./streaming/color_matching/default/bColorPrimaries  
drwxr-xr-x ./streaming/uncompressed  
drwxr-xr-x ./streaming/uncompressed/u  
drwxr-xr-x ./streaming/uncompressed/u/360p  
-rw-r--r-- ./streaming/uncompressed/u/360p/dwFrameInterval  
-rw-r--r-- ./streaming/uncompressed/u/360p/dwDefaultFrameInterval  
-rw-r--r-- ./streaming/uncompressed/u/360p/dwMaxVideoFrameBufferSize  
-rw-r--r-- ./streaming/uncompressed/u/360p/dwMaxBitRate  
-rw-r--r-- ./streaming/uncompressed/u/360p/dwMinBitRate  
-rw-r--r-- ./streaming/uncompressed/u/360p/wHeight  
-rw-r--r-- ./streaming/uncompressed/u/360p/wWidth  
-rw-r--r-- ./streaming/uncompressed/u/360p/bmCapabilities  
-rw-r--r-- ./streaming/uncompressed/u/bmaControls  
-r--r--r-- ./streaming/uncompressed/u/bmInterfaceFlags  
-r--r--r-- ./streaming/uncompressed/u/bAspectRatioY  
-r--r--r-- ./streaming/uncompressed/u/bAspectRatioX  
-rw-r--r-- ./streaming/uncompressed/u/bDefaultFrameIndex  
-rw-r--r-- ./streaming/uncompressed/u/bBitsPerPixel  
-rw-r--r-- ./streaming/uncompressed/u/guidFormat  
drwxr-xr-x ./streaming/header  
drwxr-xr-x ./streaming/header/h  
lrwxrwxrwx ./streaming/header/h/u - \
../../../../../../../usb_gadget/g1/functions/uvc.usb0/streaming/uncompressed/u
-r--r--r-- ./streaming/header/h/bTriggerUsage  
-r--r--r-- ./streaming/header/h/bTriggerSupport  
-r--r--r-- ./streaming/header/h/bStillCaptureMethod  
-r--r--r-- ./streaming/header/h/bTerminalLink  
-r--r--r-- ./streaming/header/h/bmInfo  
drwxr-xr-x ./control  
drwxr-xr-x ./control/class  
drwxr-xr-x ./control/class/ss  
lrwxrwxrwx ./control/class/ss/h - \
../../../../../../../usb_gadget/g1/functions/uvc.usb0/control/header/h
drwxr-xr-x ./control/class/fs  
lrwxrwxrwx ./control/class/fs/h - \
../../../../../../../usb_gadget/g1/functions/uvc.usb0/control/header/h
drwxr-xr-x ./control/terminal  
drwxr-xr-x ./control/terminal/output  
drwxr-xr-x ./control/terminal/output/default  
-r--r--r-- ./control/terminal/output/default/iTerminal  
-r--r--r-- ./control/terminal/output/default/bSourceID  
-r--r--r-- ./control/terminal/output/default/bAssocTerminal  
-r--r--r-- ./control/terminal/output/default/wTerminalType  
-r--r--r-- ./control/terminal/output/default/bTerminalID  
drwxr-xr-x ./control/terminal/camera  
drwxr-xr-x ./control/terminal/camera/default  
-r--r--r-- ./control/terminal/camera/default/bmControls  
-r--r--r-- ./control/terminal/camera/default/wOcularFocalLength  
-r--r--r-- ./control/terminal/camera/default/wObjectiveFocalLengthMax  
-r--r--r-- ./control/terminal/camera/default/wObjectiveFocalLengthMin  
-r--r--r-- ./control/terminal/camera/default/iTerminal  
-r--r--r-- ./control/terminal/camera/default/bAssocTerminal  
-r--r--r-- ./control/terminal/camera/default/wTerminalType  
-r--r--r-- ./control/terminal/camera/default/bTerminalID  
drwxr-xr-x ./control/processing  
drwxr-xr-x ./control/processing/default  
-r--r--r-- ./control/processing/default/iProcessing  
-r--r--r-- ./control/processing/default/bmControls  
-r--r--r-- ./control/processing/default/wMaxMultiplier  
-r--r--r-- ./control/processing/default/bSourceID  
-r--r--r-- ./control/processing/default/bUnitID  
drwxr-xr-x ./control/header  
drwxr-xr-x ./control/header/h  
-rw-r--r-- ./control/header/h/dwClockFrequency  
-rw-r--r-- ./control/header/h/bcdUVC  
-rw-r--r-- ./streaming_maxburst  
-rw-r--r-- ./streaming_maxpacket  
-rw-r--r-- ./streaming_interval  

BACKWARD COMPATIBILITY

[PATCH 1/2] usb: gadget: f_uvc: rename a macro to avoid conflicts

2014-09-24 Thread Andrzej Pietrasiewicz
When configfs is integrated, CONFIGFS_ATTR_STRUCT and CONFIGFS_ATTR_OPS
macros should be used, but the latter expects that tere is a to_f_uvc_opts
function accepting a config_item, whereas the macro being changed
can be applied to a different type of argument.

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
---
 drivers/usb/gadget/function/f_uvc.c | 6 +++---
 drivers/usb/gadget/function/u_uvc.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c 
b/drivers/usb/gadget/function/f_uvc.c
index e126439..71e5935 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -571,7 +571,7 @@ uvc_function_bind(struct usb_configuration *c, struct 
usb_function *f)
 
INFO(cdev, uvc_function_bind\n);
 
-   opts = to_f_uvc_opts(f-fi);
+   opts = fi_to_f_uvc_opts(f-fi);
/* Sanity check the streaming endpoint module parameters.
 */
opts-streaming_interval = clamp(opts-streaming_interval, 1U, 16U);
@@ -732,7 +732,7 @@ error:
 
 static void uvc_free_inst(struct usb_function_instance *f)
 {
-   struct f_uvc_opts *opts = to_f_uvc_opts(f);
+   struct f_uvc_opts *opts = fi_to_f_uvc_opts(f);
 
kfree(opts);
 }
@@ -784,7 +784,7 @@ static struct usb_function *uvc_alloc(struct 
usb_function_instance *fi)
return ERR_PTR(-ENOMEM);
 
uvc-state = UVC_STATE_DISCONNECTED;
-   opts = to_f_uvc_opts(fi);
+   opts = fi_to_f_uvc_opts(fi);
 
uvc-desc.fs_control = opts-fs_control;
uvc-desc.ss_control = opts-ss_control;
diff --git a/drivers/usb/gadget/function/u_uvc.h 
b/drivers/usb/gadget/function/u_uvc.h
index 2a8dfdf..c0706a3 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -18,7 +18,7 @@
 
 #include linux/usb/composite.h
 
-#define to_f_uvc_opts(f)   container_of(f, struct f_uvc_opts, func_inst)
+#define fi_to_f_uvc_opts(f)container_of(f, struct f_uvc_opts, func_inst)
 
 struct f_uvc_opts {
struct usb_function_instancefunc_inst;
-- 
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/2] usb: gadget: uvc: configfs support in uvc function

2014-09-24 Thread Andrzej Pietrasiewicz
Add support for using the uvc function as a component of USB gadgets composed
with configfs.

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
---
 Documentation/ABI/testing/configfs-usb-gadget-uvc |  264 +++
 drivers/usb/gadget/Kconfig|   11 +
 drivers/usb/gadget/function/Makefile  |2 +-
 drivers/usb/gadget/function/f_uvc.c   |  122 +-
 drivers/usb/gadget/function/u_uvc.h   |   23 +
 drivers/usb/gadget/function/uvc_configfs.c| 2439 +
 drivers/usb/gadget/function/uvc_configfs.h|   23 +
 7 files changed, 2879 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-uvc
 create mode 100644 drivers/usb/gadget/function/uvc_configfs.c
 create mode 100644 drivers/usb/gadget/function/uvc_configfs.h

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc 
b/Documentation/ABI/testing/configfs-usb-gadget-uvc
new file mode 100644
index 000..8f2825d
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -0,0 +1,264 @@
+What:  /config/usb-gadget/gadget/functions/uvc.name
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   UVC function directory
+
+   streaming_maxburst  - 0..15 (ss only)
+   streaming_maxpacket - 1..1023 (fs), 1..3072 (hs/ss)
+   streaming_interval  - 1..16
+
+What:  /config/usb-gadget/gadget/functions/uvc.name/control
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Control descriptors
+
+What:  /config/usb-gadget/gadget/functions/uvc.name/control/class
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Class descriptors
+
+What:  /config/usb-gadget/gadget/functions/uvc.name/control/class/ss
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Super speed control class descriptors
+
+What:  /config/usb-gadget/gadget/functions/uvc.name/control/class/fs
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Full speed control class descriptors
+
+What:  /config/usb-gadget/gadget/functions/uvc.name/control/terminal
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Terminal descriptors
+
+What:  
/config/usb-gadget/gadget/functions/uvc.name/control/terminal/output
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Output terminal descriptors
+
+What:  
/config/usb-gadget/gadget/functions/uvc.name/control/terminal/output/default
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Default output terminal descriptors
+
+   All attributes read only:
+   iTerminal   - index of string descriptor
+   bSourceID   - id of the terminal to which this terminal
+   is connected
+   bAssocTerminal  - id of the input terminal to which this output
+   terminal is associated
+   wTerminalType   - terminal type
+   bTerminalID - a non-zero id of this terminal
+
+What:  
/config/usb-gadget/gadget/functions/uvc.name/control/terminal/camera
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Camera terminal descriptors
+
+What:  
/config/usb-gadget/gadget/functions/uvc.name/control/terminal/camera/default
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Default camera terminal descriptors
+
+   All attributes read only:
+   bmControls  - bitmap specifying which controls are
+   supported for the video stream
+   wOcularFocalLength  - the value of Locular
+   wObjectiveFocalLengthMax- the value of Lmin
+   wObjectiveFocalLengthMin- the value of Lmax
+   iTerminal   - index of string descriptor
+   bAssocTerminal  - id of the output terminal to which
+   this terminal is connected
+   wTerminalType   - terminal type
+   bTerminalID - a non-zero id of this terminal
+
+What:  /config/usb-gadget/gadget/functions/uvc.name/control/processing
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Processing unit descriptors
+
+What:  
/config/usb-gadget/gadget/functions/uvc.name/control/processing/default
+Date:  Nov 2014
+KernelVersion: 3.18
+Description:   Default processing unit descriptors
+
+   All attributes read only:
+   iProcessing - index of string descriptor
+   bmControls  - bitmap specifying which controls are
+   supported for the video stream
+   wMaxMultiplier  - maximum digital magnification x100
+   bSourceID   - id of the terminal to which this unit is
+   connected
+   

Re: [resend PATCH 1/3] ACPI / platform: provide default DMA mask

2014-09-24 Thread Rafael J. Wysocki
On Wednesday, September 24, 2014 11:00:37 AM Heikki Krogerus wrote:
 Most devices are configured for 32-bit DMA addresses.
 Setting the mask to 32-bit here removes the need for the
 drivers to do it separately.
 
 Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
 Cc: Rafael J. Wysocki r...@rjwysocki.net

ACK

 ---
  drivers/acpi/acpi_platform.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
 index 2bf9082..8d099e6 100644
 --- a/drivers/acpi/acpi_platform.c
 +++ b/drivers/acpi/acpi_platform.c
 @@ -16,6 +16,7 @@
  #include linux/err.h
  #include linux/kernel.h
  #include linux/module.h
 +#include linux/dma-mapping.h
  #include linux/platform_device.h
  
  #include internal.h
 @@ -102,6 +103,7 @@ struct platform_device 
 *acpi_create_platform_device(struct acpi_device *adev)
   pdevinfo.res = resources;
   pdevinfo.num_res = count;
   pdevinfo.acpi_node.companion = adev;
 + pdevinfo.dma_mask = DMA_BIT_MASK(32);
   pdev = platform_device_register_full(pdevinfo);
   if (IS_ERR(pdev))
   dev_err(adev-dev, platform device creation failed: %ld\n,
 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/4] mfd: add support for Diolan DLN-2 devices

2014-09-24 Thread Octavian Purdila
On Wed, Sep 24, 2014 at 1:48 PM, Johan Hovold jo...@kernel.org wrote:
 On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote:

snip

 + * dln2_dev.mod_rx_slots and then the echo header field to index the
 + * slots field and find the receive context for a particular
 + * request.
 + */
 +struct dln2_mod_rx_slots {
 + /* RX slots bitmap */
 + unsigned long bmap;
 +
 + /* used to wait for a free RX slot */
 + wait_queue_head_t wq;
 +
 + /* used to wait for an RX operation to complete */
 + struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS];
 +
 + /* device has been disconnected */
 + bool disconnected;

 This belongs in the dln2_dev struct.

 I think you're overcomplicating the disconnect handling by intertwining
 it with your slots.

 Add a lock, an active-transfer counter, a disconnected flag, and a wait
 queue to struct dln2_dev.


I agree that disconnected is better suited in dln2_dev.

However, I don't think that we need the active-transfer counter and a
new wait queue. We can simply use the existing waiting queues and the
implicit alloc_rx_slot()/free_rx_slot() calls to see if we are still
waiting for I/O.

snip

 +
 +static int _dln2_transfer(struct dln2_dev *dln2, u16 handle, u16 cmd,
 +   const void *obuf, unsigned obuf_len,
 +   void *ibuf, unsigned *ibuf_len)
 +{
 + int ret = 0;
 + u16 result;
 + int rx_slot;
 + unsigned long flags;
 + struct dln2_response *rsp;
 + struct dln2_rx_context *rxc;
 + struct device *dev;
 + const int timeout = DLN2_USB_TIMEOUT * HZ / 1000;
 + struct dln2_mod_rx_slots *rxs = dln2-mod_rx_slots[handle];
 +

 Check the disconnected flag before incrementing the transfer count
 (while holding the device lock) here. Then decrement counter before
 returning and wake up an waiters if disconnected below.

 That is sufficient to implement wait-until-io-has-completed. Anything
 else you do below and in the helper functions is only to speed things
 up at disconnect (and can be done without locking the device).

 + rx_slot = alloc_rx_slot(rxs);
 + if (rx_slot  0)
 + return rx_slot;
 +
 + dev = dln2-interface-dev;
 +
 + ret = dln2_send_wait(dln2, handle, cmd, rx_slot, obuf, obuf_len);
 + if (ret  0) {
 + free_rx_slot(dln2, rxs, rx_slot);

 goto out_free_rx_slot

 + dev_err(dev, USB write failed: %d, ret);
 + return ret;
 + }
 +
 + rxc = rxs-slots[rx_slot];
 +
 + ret = wait_for_completion_interruptible_timeout(rxc-done, timeout);
 + if (ret = 0) {
 + if (!ret)
 + ret = -ETIMEDOUT;
 + goto out_free_rx_slot;
 + }
 +
 + spin_lock_irqsave(rxs-lock, flags);
 +
 + if (!rxc-urb) {

 Just check the disconnected flag directly here. Locking not needed (see
 below).


I think we need the check here for the case when we cancel the
completion and no response has been received yet. In that case rx-urb
will be NULL (even if we remove the rx-urb = NULL statement in
dln2_stop).

 + ret = -ECONNRESET;

 -ENODEV

OK.


 + spin_unlock_irqrestore(rxs-lock, flags);
 + goto out_free_rx_slot;
 + }
 +
 + /* if we got here we know that the response header has been checked */
 + rsp = rxc-urb-transfer_buffer;
 +
 + spin_unlock_irqrestore(rxs-lock, flags);
 +
 +
 +static void dln2_stop(struct dln2_dev *dln2)
 +{
 + int i, j;
 +
 + for (i = 0; i  DLN2_HANDLES; i++) {
 + struct dln2_mod_rx_slots *rxs = dln2-mod_rx_slots[i];
 + unsigned long flags;
 +
 + spin_lock_irqsave(rxs-lock, flags);
 +
 + /* fail further alloc_rx_slot calls */
 + rxs-disconnected = true;
 +
 + /* cancel all response waiters */
 + for (j = 0; j  DLN2_MAX_RX_SLOTS; j++) {
 + struct dln2_rx_context *rxc = rxs-slots[j];
 +
 + if (rxc-connected) {
 + rxc-urb = NULL;

 Don't overload the meaning of urb. Check the disconnected flag were
 needed.

 Also what would prevent a racing completion handler from setting
 rxc-urb again when you release the lock below?


That should not be an issue, in that case _dln2_transfer will complete
successfully. But you are right, we don't need to set rxc-urb to NULL
here.

 + complete(rxc-done);
 + }
 + }
 + spin_unlock_irqrestore(rxs-lock, flags);
 +
 + /* wait for callers to release all rx_slots */
 + wait_event(rxs-wq, dln2_all_rx_slots_free(rxs));
 + }
 +}
 +
 +static void dln2_disconnect(struct usb_interface *interface)
 +{
 + struct dln2_dev *dln2 = usb_get_intfdata(interface);
 +

 First set the disconnected flag directly here to prevent any new
 transfers from starting.

 + dln2_stop(dln2);

 Then do all the completions (to speed things up 

Re: [PATCH v5 1/4] mfd: add support for Diolan DLN-2 devices

2014-09-24 Thread Johan Hovold
On Wed, Sep 24, 2014 at 04:36:22PM +0300, Octavian Purdila wrote:
 On Wed, Sep 24, 2014 at 1:48 PM, Johan Hovold jo...@kernel.org wrote:
  On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote:
 
 snip
 
  + * dln2_dev.mod_rx_slots and then the echo header field to index the
  + * slots field and find the receive context for a particular
  + * request.
  + */
  +struct dln2_mod_rx_slots {
  + /* RX slots bitmap */
  + unsigned long bmap;
  +
  + /* used to wait for a free RX slot */
  + wait_queue_head_t wq;
  +
  + /* used to wait for an RX operation to complete */
  + struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS];
  +
  + /* device has been disconnected */
  + bool disconnected;
 
  This belongs in the dln2_dev struct.
 
  I think you're overcomplicating the disconnect handling by intertwining
  it with your slots.
 
  Add a lock, an active-transfer counter, a disconnected flag, and a wait
  queue to struct dln2_dev.
 
 
 I agree that disconnected is better suited in dln2_dev.
 
 However, I don't think that we need the active-transfer counter and a
 new wait queue. We can simply use the existing waiting queues and the
 implicit alloc_rx_slot()/free_rx_slot() calls to see if we are still
 waiting for I/O.

Just because you can reuse them doesn't mean it's a good idea. By
separating a generic disconnect solution from your custom slot
implementation we get something that is way easier to verify for
correctness and that could also be reused in other drivers.

 snip
 
  +
  +static int _dln2_transfer(struct dln2_dev *dln2, u16 handle, u16 cmd,
  +   const void *obuf, unsigned obuf_len,
  +   void *ibuf, unsigned *ibuf_len)
  +{
  + int ret = 0;
  + u16 result;
  + int rx_slot;
  + unsigned long flags;
  + struct dln2_response *rsp;
  + struct dln2_rx_context *rxc;
  + struct device *dev;
  + const int timeout = DLN2_USB_TIMEOUT * HZ / 1000;
  + struct dln2_mod_rx_slots *rxs = dln2-mod_rx_slots[handle];
  +
 
  Check the disconnected flag before incrementing the transfer count
  (while holding the device lock) here. Then decrement counter before
  returning and wake up an waiters if disconnected below.
 
  That is sufficient to implement wait-until-io-has-completed. Anything
  else you do below and in the helper functions is only to speed things
  up at disconnect (and can be done without locking the device).
 
  + rx_slot = alloc_rx_slot(rxs);
  + if (rx_slot  0)
  + return rx_slot;
  +
  + dev = dln2-interface-dev;
  +
  + ret = dln2_send_wait(dln2, handle, cmd, rx_slot, obuf, obuf_len);
  + if (ret  0) {
  + free_rx_slot(dln2, rxs, rx_slot);
 
  goto out_free_rx_slot
 
  + dev_err(dev, USB write failed: %d, ret);
  + return ret;
  + }
  +
  + rxc = rxs-slots[rx_slot];
  +
  + ret = wait_for_completion_interruptible_timeout(rxc-done, timeout);
  + if (ret = 0) {
  + if (!ret)
  + ret = -ETIMEDOUT;
  + goto out_free_rx_slot;
  + }
  +
  + spin_lock_irqsave(rxs-lock, flags);
  +
  + if (!rxc-urb) {
 
  Just check the disconnected flag directly here. Locking not needed (see
  below).
 
 
 I think we need the check here for the case when we cancel the
 completion and no response has been received yet. In that case rx-urb
 will be NULL (even if we remove the rx-urb = NULL statement in
 dln2_stop).

But the disconnect flag will also be set and makes it more obvious what
is going on.

[...]

  +static void dln2_disconnect(struct usb_interface *interface)
  +{
  + struct dln2_dev *dln2 = usb_get_intfdata(interface);
  +
 
  First set the disconnected flag directly here to prevent any new
  transfers from starting.
 
  + dln2_stop(dln2);
 
  Then do all the completions (to speed things up somewhat).
 
  Then wait for the transfer counter to reach 0.
 
  + mfd_remove_devices(interface-dev);
  + dln2_free(dln2);
  +}
  +
 
 As I mentioned above, I don't think we need the transfer counter, we
 can rely on the slots bitmap. Yes, a counter is simpler but it also
 requires adding a new waiting queue and a new lock.

That's really not a big deal. See above.

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: f_rndis: fix usb_interface_descriptor for rndis

2014-09-24 Thread Lars Melin

On 2014-09-24 20:12, Heiko Schocher wrote:

Hello Lars,

Am 24.09.2014 14:25, schrieb Lars Melin:

On 2014-09-24 13:48, Heiko Schocher wrote:

use the values for RNDIS over Ethernet as defined in
http://www.usb.org/developers/defined_class
(search for RDNIS):

- baseclass: 0xef (miscellaneous)
- subclass: 0x04
- protocol: 0x01

That is usb class, it is not the same thing as communication device 
class.

--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -12,6 +12,7 @@
#include linux/types.h
#define USB_CDC_SUBCLASS_ACM 0x02
+#define USB_CDC_SUBCLASS_RNDIS 0x04

No, no, no.
There is no CDC_SUBCLASS_RNDIS and you can not define one over an 
already used cdc subclass number, 0x04 is Multi-Channel Control Model


Ah, ok, so I have to define this values in a new header file, as there
is no current file for the USB_CLASS_MISC defines? Or is there a proper
place for them?

BTW: where do I find the cdc subclass number, 0x04 is Multi-Channel
Control Model define?

bye,
Heiko


You can still find the original specification usbcdc11.pdf on the net if 
you google for it, it has been pulled from usb.org where you could 
download it until a few years ago.

It is old but covers a lot of what you need to know.

Linux has afaik only the cdc.h definition file, everything else is coded 
by class/subclass in respectively drivers when needed.
02/02/ff  or  e0/01/03  are the most common interface attribute for 
rndis,  both of them together with a data interface with attributes 
0a/00/00.
Please check the whitelisting in drivers/net/usb/rndis_host.c  and also 
blacklistings in other net drivers under the same path, it should give 
you an idea how to bind an interface to a specific driver by interface 
attributes and/or usb vid:pid.

You should be able to do the same for your particular device.

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


Re: [resend PATCH 3/3] usb: dwc3: add ACPI support

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 11:00:39AM +0300, Heikki Krogerus wrote:
 Adds ACPI ID used on newer Intel SoCs.
 
 Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
 ---
  drivers/usb/dwc3/core.c | 10 ++
  1 file changed, 10 insertions(+)
 
 diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index d08cac5..c2cf2d8 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
 @@ -32,6 +32,7 @@
  #include linux/delay.h
  #include linux/dma-mapping.h
  #include linux/of.h
 +#include linux/acpi.h
  
  #include linux/usb/ch9.h
  #include linux/usb/gadget.h
 @@ -960,12 +961,21 @@ static const struct of_device_id of_dwc3_match[] = {
  MODULE_DEVICE_TABLE(of, of_dwc3_match);
  #endif
  
 +#ifdef CONFIG_ACPI
 +static const struct acpi_device_id dwc3_acpi_match[] = {
 + { 808622B7, 0 },

can you just provide a macro for this string ?

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v5 1/3] usb: gadget: Refactor request completion

2014-09-24 Thread Felipe Balbi
Hi,

On Tue, Sep 23, 2014 at 10:09:22AM +0200, Michal Sojka wrote:
  +/**
  + * usb_gadget_giveback_request - give the request back to the gadget layer
  + * Context: in_interrupt()
  + *
  + * This is called by device controller drivers in order to return the
  + * completed request back to the gadget layer.
  + */
  +void usb_gadget_giveback_request(struct usb_ep *ep,
  +  struct usb_request *req)
  +{
  +  if (likely(req-complete))
  +  req-complete(ep, req);
  +  else
  +  pr_err(%s : req-complete must not be NULL\n, __func__);
 
  let it Oops. We require -complete to be valid, if there's any gadget
  driver not setting -complete, it deserves to oops so we can the
  error.
 
 The Oops was there before, but I removed it because greg k-h didn't want
 it. See http://marc.info/?l=linux-usbm=140917381611947w=2. Do you
 still want the oops here?

you don't need a BUG_ON(), just call req-complete() directly without
any checks, if it's not valid, it _will_ oops. -complete() is mandatory
and anybody sending requests without complete deserve to oops.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v5 1/3] usb: gadget: Refactor request completion

2014-09-24 Thread Alan Stern
On Tue, 23 Sep 2014, Michal Sojka wrote:

  +/**
  + * usb_gadget_giveback_request - give the request back to the gadget layer
  + * Context: in_interrupt()
  + *
  + * This is called by device controller drivers in order to return the
  + * completed request back to the gadget layer.
  + */
  +void usb_gadget_giveback_request(struct usb_ep *ep,
  +  struct usb_request *req)
  +{
  +  if (likely(req-complete))
  +  req-complete(ep, req);
  +  else
  +  pr_err(%s : req-complete must not be NULL\n, __func__);
 
  let it Oops. We require -complete to be valid, if there's any gadget
  driver not setting -complete, it deserves to oops so we can the
  error.
 
 The Oops was there before, but I removed it because greg k-h didn't want
 it. See http://marc.info/?l=linux-usbm=140917381611947w=2. Do you
 still want the oops here?

Greg didn't want you to add a BUG() statement, whereas Felipe wants you
to leave out the if test and pr_err().  Just omit both, and everyone
will be satisfied.

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: g_mass_storage bug ?

2014-09-24 Thread Alan Stern
On Tue, 23 Sep 2014, Felipe Balbi wrote:

 Hi Alan,
 
 Need your help looking over this detail here. When I run g_mass_storage
 with stall=0 everything works fine. As soon as I remove it, things go
 bonkers.
 
 Looking at the bulk-only spec, I see:
 
 6.7.2 Hi - Host expects to receive data from the device
 
 [ ... ]
 
 The specific device requirements are:
 
 1. The device shall receive a CBW.
 2. When the CBW is valid and meaningful, then:
   . The device shall attempt the command.
   . [Case (6)]
   If the device intends to send dCBWDataTransferLength, then:
   The device shall send dCBWDataTransferLength bytes of
   data.
 
   The device shall set bCSWStatus to 00h or 01h.
 
   The device shall set dCSWDataResidue to zero.
 
 
 Case (6) is when Hi == Di, looking at my logs, I have:
 
 720 [  286.843965] SCSI CDB: 1a 00 3f 00 c0 00
 721 [  286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); Dc=6, 
 Di=192;  Hc=6, Hi=192
 722 [  286.844018] g_mass_storage gadget: bulk-in set halt
 723 [  286.844034] g_mass_storage gadget: sending command-failure status
 724 [  286.844045] g_mass_storage gadget:   sense data: SK x06, ASC x29, ASCQ 
 x00;  info x0
 
 Isn't it wrong to halt in this condition ?

No, it's correct.  SK = 6 and ASC = 0x29 means Unit Attention, Reset
Occurred.  It occurs because this is the first command the gadget has
received since starting up, which certainly is a form of reset.  In
effect, this is how the device tells the host that it was just powered
on.

So the case you should be looking at is Case 5: Hi  Di.  I realize the 
debugging output indicates Di = 192, but that line gets written before 
the driver checks for a Unit Attention condition.  (The line is written 
near the start of check_command() in f_mass_storage.c, and the Unit 
Attention check is near the end of the same function.)  The printed Di 
value indicates how much data the gadget thinks the command is asking 
for, not the amount of data the gadget actually intends to send.

Therefore stalling is appropriate.  Why it causes it problem for your 
system is a different matter.  Is your UDC hardware capable of halting 
bulk endpoints?

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][RFC] storage: Simplify condition to reject bad target in usb_stor_control_thread()

2014-09-24 Thread Alan Stern
On Wed, 24 Sep 2014, Mark Knibbs wrote:

 Hi,
 
 This follows on from something I mentioned in my recent post [PATCH]
 storage: Don't scan target 7 for SCM USB-SCSI converters.
 
 In drivers/usb/storage/usb.c, usb_stor_control_thread():
   /* reject if target != 0 or if LUN is higher than
* the maximum known LUN
*/
   else if (us-srb-device-id 
   !(us-fflags  US_FL_SCM_MULT_TARG)) {
   usb_stor_dbg(us, Bad target number (%d:%llu)\n,
us-srb-device-id,
us-srb-device-lun);
   us-srb-result = DID_BAD_TARGET  16;
   }
 
   else if (us-srb-device-lun  us-max_lun) {
   usb_stor_dbg(us, Bad LUN (%d:%llu)\n,
   ...
 
 The condition (us-srb-device-id  !(us-fflags  US_FL_SCM_MULT_TARG))
 means:
  - If not SCM_MULT_TARG, reject if us-srb-device-id is non-zero. That
works for the usual case.
  - If SCM_MULT_TARG, never reject. Hmm...
 
 Would it be better/clearer to change the condition to simply
   (us-srb-device-id = host-max_id)
 
 Then the command is rejected if us-srb-device-id is too large, and works
 for the SCM_MULT_TARG case too. (host-max_id is 1 for the non-SCM_MULT_TARG
 case.) Plus the condition matches nicely with the later LUN check. Untested
 patch below.
 
 
 Signed-off-by: Mark Knibbs ma...@clara.co.uk
 
 diff -up linux-3.17-rc6/drivers/usb/storage/usb.c.orig 
 linux-3.17-rc6/drivers/usb/storage/usb.c 
 --- linux-3.17-rc6/drivers/usb/storage/usb.c.orig 2014-09-21 
 23:43:02.0 +0100
 +++ linux-3.17-rc6/drivers/usb/storage/usb.c  2014-09-24 13:09:55.0 
 +0100
 @@ -342,11 +342,10 @@ static int usb_stor_control_thread(void
   us-srb-result = DID_ERROR  16;
   }
  
 - /* reject if target != 0 or if LUN is higher than
 + /* reject if target is invalid or if LUN is higher than
* the maximum known LUN
*/
 - else if (us-srb-device-id 
 - !(us-fflags  US_FL_SCM_MULT_TARG)) {
 + else if (us-srb-device-id = host-max_id) {
   usb_stor_dbg(us, Bad target number (%d:%llu)\n,
us-srb-device-id,
us-srb-device-lun);

That's probably okay.  As far as I know, the SCSI layer doesn't 
generate commands for targets with id = host-max_id, so this entire 
test may be unnecessary.

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 v5 1/4] mfd: add support for Diolan DLN-2 devices

2014-09-24 Thread Johan Hovold
On Wed, Sep 24, 2014 at 05:54:15PM +0300, Octavian Purdila wrote:
 On Wed, Sep 24, 2014 at 4:54 PM, Johan Hovold jo...@kernel.org wrote:
  On Wed, Sep 24, 2014 at 04:36:22PM +0300, Octavian Purdila wrote:
  On Wed, Sep 24, 2014 at 1:48 PM, Johan Hovold jo...@kernel.org wrote:
   On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote:
 
  snip
 
   + * dln2_dev.mod_rx_slots and then the echo header field to index the
   + * slots field and find the receive context for a particular
   + * request.
   + */
   +struct dln2_mod_rx_slots {
   + /* RX slots bitmap */
   + unsigned long bmap;
   +
   + /* used to wait for a free RX slot */
   + wait_queue_head_t wq;
   +
   + /* used to wait for an RX operation to complete */
   + struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS];
   +
   + /* device has been disconnected */
   + bool disconnected;
  
   This belongs in the dln2_dev struct.
  
   I think you're overcomplicating the disconnect handling by intertwining
   it with your slots.
  
   Add a lock, an active-transfer counter, a disconnected flag, and a wait
   queue to struct dln2_dev.
  
 
  I agree that disconnected is better suited in dln2_dev.
 
  However, I don't think that we need the active-transfer counter and a
  new wait queue. We can simply use the existing waiting queues and the
  implicit alloc_rx_slot()/free_rx_slot() calls to see if we are still
  waiting for I/O.
 
  Just because you can reuse them doesn't mean it's a good idea. By
  separating a generic disconnect solution from your custom slot
  implementation we get something that is way easier to verify for
  correctness and that could also be reused in other drivers.
 
 Maybe I miss-understood what you are proposing, let me try to
 summarize it to see if I got it right.
 
 You are suggesting to add a counter, increment it in alloc_rx_slot(),
 decrement it in free_rx_slot().

No increment it at the start of _dln2_transfer, and decrement it before
returning from that function.

 Then add a new waitqueue in dln2_dev
 and in free_rx_slot() wake it up while in disconnect do a wait_event()
 on it and check for the counter.

Where you also wake the disconnect (or wait-until-sent) wait queue.

 Also, alloc_rx_slot() should fail if
 the disconnect flag is set.

That is not required, but you can bail out early after alloc_rx_slot if
the disconnect flag is set (no locking).

 In this case we are still coupled to the slots implementation, in the
 sense that you would need to understand the slots implementation to
 understand how the disconnect works. We are also doing two wake-up
 operations which I find redundant and which does not add much value in
 clarity (since we still need to wake-up all completions for each
 handle).

 I do agree that using a counter instead of checking the bitmaps is
 cleaner though.

You only need to the wake up if disconnected is set when returning from
_dln2_transfer.

Sure, the optimisation bit -- to abort any ongoing transfer -- still
requires some insight into the slot implementation.

But this way everything disconnect related (correctness-wise) is
isolated to _dln2_transfer and dln2_disconnect.

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: g_mass_storage bug ?

2014-09-24 Thread Alan Stern
On Wed, 24 Sep 2014, Alan Stern wrote:

  Case (6) is when Hi == Di, looking at my logs, I have:
  
  720 [  286.843965] SCSI CDB: 1a 00 3f 00 c0 00
  721 [  286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); 
  Dc=6, Di=192;  Hc=6, Hi=192
  722 [  286.844018] g_mass_storage gadget: bulk-in set halt
  723 [  286.844034] g_mass_storage gadget: sending command-failure status
  724 [  286.844045] g_mass_storage gadget:   sense data: SK x06, ASC x29, 
  ASCQ x00;  info x0
  
  Isn't it wrong to halt in this condition ?
 
 No, it's correct.  SK = 6 and ASC = 0x29 means Unit Attention, Reset
 Occurred.  It occurs because this is the first command the gadget has
 received since starting up, which certainly is a form of reset.  In
 effect, this is how the device tells the host that it was just powered
 on.

Actually, looking at your log again, it seems more like this followed a 
genuine reset, not a power-on.  Regardless, it's still appropriate.

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: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
Hi,

On Wed, Sep 24, 2014 at 11:05:58AM -0400, Alan Stern wrote:
 On Tue, 23 Sep 2014, Felipe Balbi wrote:
 
  Hi Alan,
  
  Need your help looking over this detail here. When I run g_mass_storage
  with stall=0 everything works fine. As soon as I remove it, things go
  bonkers.
  
  Looking at the bulk-only spec, I see:
  
  6.7.2 Hi - Host expects to receive data from the device
  
  [ ... ]
  
  The specific device requirements are:
  
  1. The device shall receive a CBW.
  2. When the CBW is valid and meaningful, then:
  . The device shall attempt the command.
  . [Case (6)]
  If the device intends to send dCBWDataTransferLength, then:
  The device shall send dCBWDataTransferLength bytes of
  data.
  
  The device shall set bCSWStatus to 00h or 01h.
  
  The device shall set dCSWDataResidue to zero.
  
  
  Case (6) is when Hi == Di, looking at my logs, I have:
  
  720 [  286.843965] SCSI CDB: 1a 00 3f 00 c0 00
  721 [  286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); 
  Dc=6, Di=192;  Hc=6, Hi=192
  722 [  286.844018] g_mass_storage gadget: bulk-in set halt
  723 [  286.844034] g_mass_storage gadget: sending command-failure status
  724 [  286.844045] g_mass_storage gadget:   sense data: SK x06, ASC x29, 
  ASCQ x00;  info x0
  
  Isn't it wrong to halt in this condition ?
 
 No, it's correct.  SK = 6 and ASC = 0x29 means Unit Attention, Reset
 Occurred.  It occurs because this is the first command the gadget has
 received since starting up, which certainly is a form of reset.  In
 effect, this is how the device tells the host that it was just powered
 on.
 
 So the case you should be looking at is Case 5: Hi  Di.  I realize the 
 debugging output indicates Di = 192, but that line gets written before 
 the driver checks for a Unit Attention condition.  (The line is written 
 near the start of check_command() in f_mass_storage.c, and the Unit 
 Attention check is near the end of the same function.)  The printed Di 
 value indicates how much data the gadget thinks the command is asking 
 for, not the amount of data the gadget actually intends to send.
 
 Therefore stalling is appropriate.  Why it causes it problem for your 
 system is a different matter.  Is your UDC hardware capable of halting 
 bulk endpoints?

yeah, that part is just fine; I also verified with my sniffer that bulk
halt is happening as it should. The problem, however, is that after that
halt condition happens, host (same board has xhci too, Linux 3.17-rc5)
issues a reset recovery and it all happens again. I stay in that loop
for a while until it finally enumerates correctly, but when I try to
write to the block device with dd, it resets again.

I'll try the same test against my desktop (3.16.1) and a Mac OS X I have
here, and see if the same behavior shows up.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v5 1/4] mfd: add support for Diolan DLN-2 devices

2014-09-24 Thread Octavian Purdila
On Wed, Sep 24, 2014 at 6:07 PM, Johan Hovold jo...@kernel.org wrote:
 On Wed, Sep 24, 2014 at 05:54:15PM +0300, Octavian Purdila wrote:
 On Wed, Sep 24, 2014 at 4:54 PM, Johan Hovold jo...@kernel.org wrote:
  On Wed, Sep 24, 2014 at 04:36:22PM +0300, Octavian Purdila wrote:
  On Wed, Sep 24, 2014 at 1:48 PM, Johan Hovold jo...@kernel.org wrote:
   On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote:
 
  snip
 
   + * dln2_dev.mod_rx_slots and then the echo header field to index the
   + * slots field and find the receive context for a particular
   + * request.
   + */
   +struct dln2_mod_rx_slots {
   + /* RX slots bitmap */
   + unsigned long bmap;
   +
   + /* used to wait for a free RX slot */
   + wait_queue_head_t wq;
   +
   + /* used to wait for an RX operation to complete */
   + struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS];
   +
   + /* device has been disconnected */
   + bool disconnected;
  
   This belongs in the dln2_dev struct.
  
   I think you're overcomplicating the disconnect handling by intertwining
   it with your slots.
  
   Add a lock, an active-transfer counter, a disconnected flag, and a wait
   queue to struct dln2_dev.
  
 
  I agree that disconnected is better suited in dln2_dev.
 
  However, I don't think that we need the active-transfer counter and a
  new wait queue. We can simply use the existing waiting queues and the
  implicit alloc_rx_slot()/free_rx_slot() calls to see if we are still
  waiting for I/O.
 
  Just because you can reuse them doesn't mean it's a good idea. By
  separating a generic disconnect solution from your custom slot
  implementation we get something that is way easier to verify for
  correctness and that could also be reused in other drivers.

 Maybe I miss-understood what you are proposing, let me try to
 summarize it to see if I got it right.

 You are suggesting to add a counter, increment it in alloc_rx_slot(),
 decrement it in free_rx_slot().

 No increment it at the start of _dln2_transfer, and decrement it before
 returning from that function.

 Then add a new waitqueue in dln2_dev
 and in free_rx_slot() wake it up while in disconnect do a wait_event()
 on it and check for the counter.

 Where you also wake the disconnect (or wait-until-sent) wait queue.

 Also, alloc_rx_slot() should fail if
 the disconnect flag is set.

 That is not required, but you can bail out early after alloc_rx_slot if
 the disconnect flag is set (no locking).

 In this case we are still coupled to the slots implementation, in the
 sense that you would need to understand the slots implementation to
 understand how the disconnect works. We are also doing two wake-up
 operations which I find redundant and which does not add much value in
 clarity (since we still need to wake-up all completions for each
 handle).

 I do agree that using a counter instead of checking the bitmaps is
 cleaner though.

 You only need to the wake up if disconnected is set when returning from
 _dln2_transfer.

 Sure, the optimisation bit -- to abort any ongoing transfer -- still
 requires some insight into the slot implementation.

 But this way everything disconnect related (correctness-wise) is
 isolated to _dln2_transfer and dln2_disconnect.


OK, I see what you mean now. I'll give it a try and will follow up
with a new patch set.

Oh, and thanks for yet another review, it has been very educational to
me just like the rest  :)
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 11:17:18AM -0400, Alan Stern wrote:
 On Wed, 24 Sep 2014, Alan Stern wrote:
 
   Case (6) is when Hi == Di, looking at my logs, I have:
   
   720 [  286.843965] SCSI CDB: 1a 00 3f 00 c0 00
   721 [  286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); 
   Dc=6, Di=192;  Hc=6, Hi=192
   722 [  286.844018] g_mass_storage gadget: bulk-in set halt
   723 [  286.844034] g_mass_storage gadget: sending command-failure status
   724 [  286.844045] g_mass_storage gadget:   sense data: SK x06, ASC x29, 
   ASCQ x00;  info x0
   
   Isn't it wrong to halt in this condition ?
  
  No, it's correct.  SK = 6 and ASC = 0x29 means Unit Attention, Reset
  Occurred.  It occurs because this is the first command the gadget has
  received since starting up, which certainly is a form of reset.  In
  effect, this is how the device tells the host that it was just powered
  on.
 
 Actually, looking at your log again, it seems more like this followed a 
 genuine reset, not a power-on.  Regardless, it's still appropriate.

It seems to work better against my v3.16.1 desktop and Mac OS X then it
does against v3.17-rc5 (running on the development board).

I'll try using a USB stick attached to the board.

-- 
balbi


signature.asc
Description: Digital signature


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 10:30:17AM -0500, Felipe Balbi wrote:
 On Wed, Sep 24, 2014 at 11:17:18AM -0400, Alan Stern wrote:
  On Wed, 24 Sep 2014, Alan Stern wrote:
  
Case (6) is when Hi == Di, looking at my logs, I have:

720 [  286.843965] SCSI CDB: 1a 00 3f 00 c0 00
721 [  286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); 
Dc=6, Di=192;  Hc=6, Hi=192
722 [  286.844018] g_mass_storage gadget: bulk-in set halt
723 [  286.844034] g_mass_storage gadget: sending command-failure status
724 [  286.844045] g_mass_storage gadget:   sense data: SK x06, ASC 
x29, ASCQ x00;  info x0

Isn't it wrong to halt in this condition ?
   
   No, it's correct.  SK = 6 and ASC = 0x29 means Unit Attention, Reset
   Occurred.  It occurs because this is the first command the gadget has
   received since starting up, which certainly is a form of reset.  In
   effect, this is how the device tells the host that it was just powered
   on.
  
  Actually, looking at your log again, it seems more like this followed a 
  genuine reset, not a power-on.  Regardless, it's still appropriate.
 
 It seems to work better against my v3.16.1 desktop and Mac OS X then it
 does against v3.17-rc5 (running on the development board).
 
 I'll try using a USB stick attached to the board.

alright, USB stick attached to the board behaves perfectly, but then
again I don't see any stalls from that device. Any chance I can persuade
you into running a similar test with your net2272/2280 UDC ? I'm using a
500MiB tmpfs as storage backend, then just connecting to an XHCI host
and looking at dmesg. A dd also triggers a few resets with v3.17-rc5
which don't happen on 3.16.1.

cheers

-- 
balbi


signature.asc
Description: Digital signature


Re: Poor performance with USB 1.1 drive connected to USB 3.0 port

2014-09-24 Thread Alan Stern
On Wed, 24 Sep 2014, Mark Knibbs wrote:

 Hi,
 
 I did some benchmarks to check the maximum transfer rate of a USB-to-SCSI
 converter. The converter is USB 1.1, so limited to the 12Mbps full speed
 rate. The performance when connected to a USB 3.0 port is significantly
 worse than when connected to a USB 2.0 port, about 26.5% slower (0.63MB/sec
 vs 0.86MB/sec).
 
 The USB 3.0 port is provided by an ExpressCard which has a Renesas
 controller. It doesn't seem to be defective because I can get 138MB/sec
 from a USB 3.0 hard disk. lspci shows
   05:00.0 USB Controller: Renesas Technology Corp. Device 0015 (rev 02)
 
 I didn't test with an unmodified mainline kernel, but the issue is present
 with both Ubuntu kernel 3.0 and Debian 3.14 (booted from Kali Linux live
 DVD).
 
 I used sg_rbuf from the sg3-utils package. That issues READ BUFFER commands
 instead of reading data from disk, so the result should be closer to the
 maximum achievable. The USB-SCSI converter was a Newer Technology uSCSI
 connected to an HP 2600fx MO drive.
 
 In a USB 2.0 port:
 
 # sg_rbuf --buffer=524288 -v -t -q /dev/sg3
 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 
 READ BUFFER reports: buffer capacity=983040, offset boundary=0
 time to read data from buffer was 242.800241 secs, 0.86 MB/sec
 Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes)
 
 # sg_rbuf --buffer=524288 -v -t -q /dev/sg3
 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 
 READ BUFFER reports: buffer capacity=983040, offset boundary=0
 time to read data from buffer was 242.800230 secs, 0.86 MB/sec
 Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes)
 
 
 In a USB 3.0 port:
 
 # sg_rbuf --buffer=524288 -v -t -q /dev/sg3
 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 
 READ BUFFER reports: buffer capacity=983040, offset boundary=0
 time to read data from buffer was 330.311337 secs, 0.63 MB/sec
 Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes)
 
 # sg_rbuf --buffer=524288 -v -t -q /dev/sg3
 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 
 READ BUFFER reports: buffer capacity=983040, offset boundary=0
 time to read data from buffer was 330.388556 secs, 0.63 MB/sec
 Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes)
 
 
 By comparison, I checked the transfer rate in native Windows (Vista) with
 the WinSAT program. That reported 0.66MB/sec for both USB 2.0 and 3.0
 ports. (WinSAT reads data from disk, probably in smaller chunks than 512KB.
 There's no sg_rbuf executable in the Windows sg3_utils archive.)
 
 Any ideas what the reason for the discrepancy might be? Can anyone else
 reproduce it? I guess USB 1.1-only devices aren't that common nowadays.

You might be able to get more detailed timings if you collect usbmon
traces of the two tests.  They won't answer your question but they may 
help point in a particular direction.

The xhci-hcd driver has been under active development.  For the best 
results, you really should use the most up-to-date version of the 
kernel.

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] drivers/usb/host/ehci-xilinx-of.c: Include linux/of_irq.h to avoid compiling error

2014-09-24 Thread Alan Stern
On Mon, 22 Sep 2014, Michal Simek wrote:

 Hi Alan and Greg,
 
 On 09/20/2014 06:19 AM, Chen Gang wrote:
  Hello Maintainers:
  
  Please help check this patch, when you have time.
  
  Thanks.
  
  On 09/08/2014 01:20 PM, Michal Simek wrote:
  On 09/03/2014 05:50 PM, Chen Gang wrote:
  Need include it for irq_of_parse_and_map(), the related error with
  allmodconfig under microblaze:
 
drivers/usb/host/ehci-xilinx-of.c: In function 
  �ehci_hcd_xilinx_of_probe�:
drivers/usb/host/ehci-xilinx-of.c:156:2: error: implicit declaration of 
  function �irq_of_parse_and_map� [-Werror=implicit-function-declaration]
  irq = irq_of_parse_and_map(dn, 0);
  ^
 
  Signed-off-by: Chen Gang gang.chen.5...@gmail.com
  ---
   drivers/usb/host/ehci-xilinx-of.c | 1 +
   1 file changed, 1 insertion(+)
 
  diff --git a/drivers/usb/host/ehci-xilinx-of.c 
  b/drivers/usb/host/ehci-xilinx-of.c
  index fe57710..a232836 100644
  --- a/drivers/usb/host/ehci-xilinx-of.c
  +++ b/drivers/usb/host/ehci-xilinx-of.c
  @@ -31,6 +31,7 @@
   #include linux/of.h
   #include linux/of_platform.h
   #include linux/of_address.h
  +#include linux/of_irq.h
   
   /**
* ehci_xilinx_port_handed_over - hand the port out if failed to enable 
  it
 
 
  Acked-by: Michal Simek mon...@monstr.eu
 
 Alan: Can you please add this patch to your queue?
 Greg: If Alan is not maintaining this part of kernel, is this patch in your 
 queue?
 
 I have also not a problem to add this patch through my microblaze tree
 but I think it will be much better to use any USB tree.

Greg should be able to accept a trivial patch like this one directly.

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: g_mass_storage bug ?

2014-09-24 Thread Alan Stern
On Wed, 24 Sep 2014, Felipe Balbi wrote:

  Therefore stalling is appropriate.  Why it causes it problem for your 
  system is a different matter.  Is your UDC hardware capable of halting 
  bulk endpoints?
 
 yeah, that part is just fine; I also verified with my sniffer that bulk
 halt is happening as it should. The problem, however, is that after that
 halt condition happens, host (same board has xhci too, Linux 3.17-rc5)
 issues a reset recovery

It shouldn't; there's no reason for it to do so.  Unless something 
else is going wrong on the host side.  Have you tried capturing a 
usbmon trace on the host?

 and it all happens again. I stay in that loop
 for a while until it finally enumerates correctly, but when I try to
 write to the block device with dd, it resets again.
 
 I'll try the same test against my desktop (3.16.1) and a Mac OS X I have
 here, and see if the same behavior shows up.

 It seems to work better against my v3.16.1 desktop and Mac OS X then it
 does against v3.17-rc5 (running on the development board).

Indicating that this really is a host-side problem.

 I'll try using a USB stick attached to the board.

USB sticks probably won't generate the Unit Attention condition in 
response to a reset.  They tend not to adhere terribly closely to the 
SCSI standard.

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: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
Hi,

On Wed, Sep 24, 2014 at 11:40:54AM -0400, Alan Stern wrote:
 On Wed, 24 Sep 2014, Felipe Balbi wrote:
 
   Therefore stalling is appropriate.  Why it causes it problem for your 
   system is a different matter.  Is your UDC hardware capable of halting 
   bulk endpoints?
  
  yeah, that part is just fine; I also verified with my sniffer that bulk
  halt is happening as it should. The problem, however, is that after that
  halt condition happens, host (same board has xhci too, Linux 3.17-rc5)
  issues a reset recovery
 
 It shouldn't; there's no reason for it to do so.  Unless something 
 else is going wrong on the host side.  Have you tried capturing a 
 usbmon trace on the host?

I'll capture usbmon and send here shortly.

  and it all happens again. I stay in that loop
  for a while until it finally enumerates correctly, but when I try to
  write to the block device with dd, it resets again.
  
  I'll try the same test against my desktop (3.16.1) and a Mac OS X I have
  here, and see if the same behavior shows up.
 
  It seems to work better against my v3.16.1 desktop and Mac OS X then it
  does against v3.17-rc5 (running on the development board).
 
 Indicating that this really is a host-side problem.

right.

  I'll try using a USB stick attached to the board.
 
 USB sticks probably won't generate the Unit Attention condition in 
 response to a reset.  They tend not to adhere terribly closely to the 
 SCSI standard.

yeah, I figured :-s

-- 
balbi


signature.asc
Description: Digital signature


[PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper

2014-09-24 Thread Vinod Koul
This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
sequentially. Then we do a tree wide update of current patterns which are
present. As evident from log below this pattern is frequent in the
kernel.

This series can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
topic/pm_runtime_last_busy_and_autosuspend

Fengguang's kbuild has tested it so it shouldn't break things for anyone.
Barring one patch (explictyly mentioned in its changelog) rest are simple
replacements.

If all are okay, this should be merged thru PM tree as it depends on macro
addition.

Subhransu S. Prusty (1):
  PM: Add helper pm_runtime_last_busy_and_autosuspend()

Vinod Koul (26):
  dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
  extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
  drm/i915: use pm_runtime_last_busy_and_autosuspend helper
  drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
  drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
  vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
  i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
  i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
  i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
  mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
  mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
  mei: use pm_runtime_last_busy_and_autosuspend helper
  mmc: use pm_runtime_last_busy_and_autosuspend helper
  mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
  mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
  mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
  mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
  NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
  pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
  spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
  spi: orion: use pm_runtime_last_busy_and_autosuspend helper
  spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
  spi: core: use pm_runtime_last_busy_and_autosuspend helper
  tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
  usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
  video: fbdev: use pm_runtime_last_busy_and_autosuspend helper

 Documentation/power/runtime_pm.txt  |4 ++
 drivers/dma/ste_dma40.c |   30 -
 drivers/extcon/extcon-arizona.c |6 +--
 drivers/gpu/drm/i915/intel_pm.c |3 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c |3 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c   |9 +---
 drivers/gpu/drm/radeon/radeon_connectors.c  |   15 ++
 drivers/gpu/drm/radeon/radeon_drv.c |5 +-
 drivers/gpu/drm/radeon/radeon_kms.c |6 +--
 drivers/gpu/vga/vga_switcheroo.c|7 +--
 drivers/i2c/busses/i2c-designware-core.c|3 +-
 drivers/i2c/busses/i2c-omap.c   |6 +--
 drivers/i2c/busses/i2c-qup.c|3 +-
 drivers/mfd/ab8500-gpadc.c  |6 +--
 drivers/mfd/arizona-irq.c   |3 +-
 drivers/misc/mei/client.c   |   12 ++
 drivers/mmc/core/core.c |3 +-
 drivers/mmc/host/mmci.c |   12 ++
 drivers/mmc/host/omap_hsmmc.c   |   19 ++---
 drivers/mmc/host/sdhci-pxav3.c  |6 +--
 drivers/mmc/host/sdhci.c|3 +-
 drivers/nfc/trf7970a.c  |3 +-
 drivers/power/pm2301_charger.c  |3 +-
 drivers/spi/spi-omap2-mcspi.c   |9 +---
 drivers/spi/spi-orion.c |3 +-
 drivers/spi/spi-ti-qspi.c   |5 +-
 drivers/spi/spi.c   |6 +--
 drivers/tty/serial/omap-serial.c|   60 +--
 drivers/usb/musb/omap2430.c |6 +--
 drivers/video/fbdev/auo_k190x.c |9 +---
 include/linux/pm_runtime.h  |6 +++
 31 files changed, 97 insertions(+), 177 deletions(-)


Thanks
-- 
~Vinod
--
To unsubscribe from this list: send the line unsubscribe 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 26/27] usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper

2014-09-24 Thread Vinod Koul
Use the new pm_runtime_last_busy_and_autosuspend helper instead of open
coding the same code

Signed-off-by: Vinod Koul vinod.k...@intel.com
---
 drivers/usb/musb/omap2430.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index d369bf1..17c6c49 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -293,8 +293,7 @@ static void omap_musb_set_mailbox(struct omap2430_glue 
*glue)
musb-xceiv-last_event = USB_EVENT_NONE;
if (musb-gadget_driver) {
omap2430_musb_set_vbus(musb, 0);
-   pm_runtime_mark_last_busy(dev);
-   pm_runtime_put_autosuspend(dev);
+   pm_runtime_last_busy_and_autosuspend(dev);
}
 
if (data-interface_type == MUSB_INTERFACE_UTMI)
@@ -321,8 +320,7 @@ static void omap_musb_mailbox_work(struct work_struct 
*mailbox_work)
 
pm_runtime_get_sync(dev);
omap_musb_set_mailbox(glue);
-   pm_runtime_mark_last_busy(dev);
-   pm_runtime_put_autosuspend(dev);
+   pm_runtime_last_busy_and_autosuspend(dev);
 }
 
 static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci)
-- 
1.7.0.4

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


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
Hi,

On Wed, Sep 24, 2014 at 11:20:31AM -0500, Felipe Balbi wrote:
 Therefore stalling is appropriate.  Why it causes it problem for your 
 system is a different matter.  Is your UDC hardware capable of 
 halting 
 bulk endpoints?

yeah, that part is just fine; I also verified with my sniffer that bulk
halt is happening as it should. The problem, however, is that after that
halt condition happens, host (same board has xhci too, Linux 3.17-rc5)
issues a reset recovery
   
   It shouldn't; there's no reason for it to do so.  Unless something 
   else is going wrong on the host side.  Have you tried capturing a 
   usbmon trace on the host?
  
  I'll capture usbmon and send here shortly.
 
 here it is... Interesting part starts at line 73 (114 on this email)
 where the data transport received EPIPE (due to Stall). This time
 however, I was eventually able to talk to the device and managed to
 issue quite a few writes to it.

so here's sequence of events so far:

- Enumration goes fine
- Get Max Lun   - 0 (single lun)
- Inquiry   - Passed
- Test Unit Ready   - Failed
- Request Sense (Unit Attention)- Passed
- Test Unit Ready   - Passed
- Mode Sense- Stall of Data transport.
- Clear Endpoint Feature (HALT) EP1 IN
- After clear feature, a 16 bulk in completes. Shouldn't gadget
  driver have cancelled that ?
- Bus reset

This remains for a few iterations. One thing is very interesting ...

[ snip ]

 ed2541c0 1239906485 S Bo:003:01 -115 31 = 55534243 1e00 1200 8603 
 0012   00
 ed2541c0 1239906590 C Bo:003:01 0 31 
 ec1a8740 1239906770 S Bi:003:01 -115 18 
 ec1a8740 1239906871 C Bi:003:01 0 18 = 7600 000a  2900 
 
 ed2541c0 1239906975 S Bi:003:01 -115 13 
 ed2541c0 1239907026 C Bi:003:01 0 13 = 55534253 1e00  00
 ed2541c0 1239907803 S Bo:003:01 -115 31 = 55534243 1f00 0002 8ca1 
 082e0001  ec00 00

0xa1 ? What is this ? Looks like XHCI corrupted the packet ? I can see
the same SCSI opcode (0xa1) with my sniffer.

-- 
balbi


signature.asc
Description: Digital signature


[PATCH v2] storage: Don't scan target 7 for SCM USB-SCSI converters

2014-09-24 Thread Mark Knibbs
Hi,

Regarding the patch I posted to prevent the bus scan of SCM USB-SCSI
converters attempting to scan the converter itself, there is probably a
neater way.

In drivers/scsi/scsi_scan.c, function __scsi_scan_target(), there is:
if (shost-this_id == id)
/*
 * Don't scan the host adapter
 */
return;

Since __scsi_scan_target() takes care to avoid scanning the host adapter, we
just need to set this_id to 7. Untested patch below.

Signed-off-by: Mark Knibbs ma...@clara.co.uk

---
diff -up linux-3.17-rc6/drivers/usb/storage/usb.c.orig 
linux-3.17-rc6/drivers/usb/storage/usb.c
--- linux-3.17-rc6/drivers/usb/storage/usb.c.orig   2014-09-21 
23:43:02.0 +0100
+++ linux-3.17-rc6/drivers/usb/storage/usb.c2014-09-24 18:21:24.0 
+0100
@@ -980,8 +980,12 @@ int usb_stor_probe2(struct us_data *us)
if (us-fflags  US_FL_SINGLE_LUN)
us-max_lun = 0;
 
-   if (!(us-fflags  US_FL_SCM_MULT_TARG))
+   if (!(us-fflags  US_FL_SCM_MULT_TARG)) {
us_to_host(us)-max_id = 1;
+   } else {
+   /* SCM USB-SCSI converter ID is 7 */
+   us_to_host(us)-this_id = 7;
+   }
 
/* Find the endpoints and calculate pipe values */
result = get_pipes(us);
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] usb: core: log more general message on malformed LANGID descriptor

2014-09-24 Thread Scot Doyle
On Tue, 23 Sep 2014, Greg Kroah-Hartman wrote:

 On Tue, Sep 23, 2014 at 10:28:49PM +, Scot Doyle wrote:
 I'd like to change this error message:
 [3.325837] usb 1-4: string descriptor 0 malformed (err = -61), 
 defaulting to 0x0409

 into an error message followed by a debug message:
 [3.324726] usb 1-4: malformed string descriptor; unknown language, 
 defaulting to English
 [3.327514] usb 1-4: string descriptor 0 malformed (err = -61), 
 defaulting to 0x0409

 in order to communicate more information from the log itself. Are there
 any problems with this approach? Would it be better to put all the
 information on a single line? Something else?

 How about just one line, why two?

 And something like device not implementing langid specifier, defaulting
 to English, but cleaned up to sound a bit better?

That sounds good, I plan to submit this patch tomorrow unless anyone 
prefers different wording.

---
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 0c8a7fc..5317081 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -770,9 +770,7 @@ static int usb_get_langid(struct usb_device *dev, unsigned 
char *tbuf)
dev-string_langid = 0x0409;
dev-have_langid = 1;
dev_err(dev-dev,
-   string descriptor 0 malformed (err = %d), 
-   defaulting to 0x%04x\n,
-   err, dev-string_langid);
+   language id specifier not provided by device, 
defaulting to English);
return 0;
}

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


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 12:19:13PM -0500, Felipe Balbi wrote:
 Hi,
 
 On Wed, Sep 24, 2014 at 11:20:31AM -0500, Felipe Balbi wrote:
  Therefore stalling is appropriate.  Why it causes it problem for 
  your 
  system is a different matter.  Is your UDC hardware capable of 
  halting 
  bulk endpoints?
 
 yeah, that part is just fine; I also verified with my sniffer that 
 bulk
 halt is happening as it should. The problem, however, is that after 
 that
 halt condition happens, host (same board has xhci too, Linux 3.17-rc5)
 issues a reset recovery

It shouldn't; there's no reason for it to do so.  Unless something 
else is going wrong on the host side.  Have you tried capturing a 
usbmon trace on the host?
   
   I'll capture usbmon and send here shortly.
  
  here it is... Interesting part starts at line 73 (114 on this email)
  where the data transport received EPIPE (due to Stall). This time
  however, I was eventually able to talk to the device and managed to
  issue quite a few writes to it.
 
 so here's sequence of events so far:
 
 - Enumration goes fine
 - Get Max Lun - 0 (single lun)
 - Inquiry - Passed
 - Test Unit Ready - Failed
 - Request Sense (Unit Attention)  - Passed
 - Test Unit Ready - Passed
 - Mode Sense  - Stall of Data transport.
   - Clear Endpoint Feature (HALT) EP1 IN
   - After clear feature, a 16 bulk in completes. Shouldn't gadget
 driver have cancelled that ?
 - Bus reset

looking into the SCSI glue, it seems like scsi is calling
-eh_bus_reset_handler() instead of -eh_device_reset_handler(). Digging
a little more.

-- 
balbi


signature.asc
Description: Digital signature


Re: g_mass_storage bug ?

2014-09-24 Thread Alan Stern
On Wed, 24 Sep 2014, Felipe Balbi wrote:

  I'll capture usbmon and send here shortly.
 
 here it is... Interesting part starts at line 73 (114 on this email)
 where the data transport received EPIPE (due to Stall). This time
 however, I was eventually able to talk to the device and managed to
 issue quite a few writes to it.

Here's where the unexpected stuff begins:

 ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 0600 c000 861a 
 003f00c0   00
 ed2541c0 1237463431 C Bo:003:01 0 31 
 ec1a8540 1237463873 S Bi:003:01 -115 192 
 ec1a8540 1237464053 C Bi:003:01 -32 0
 ed2541c0 1237464158 S Co:003:00 s 02 01  0081  0
 ed2541c0 1237464359 C Co:003:00 0 0
 ed2541c0 1237468607 S Bi:003:01 -115 13 
 ed2541c0 1237468802 C Bi:003:01 -75 0

This is the first MODE SENSE command.  The gadget should send as much
data as it can before halting the bulk-IN endpoint.  Instead, the
endpoint was halted first.

Then, after the host cleared the halt, the gadget apparently sent the
data that _should_ have been sent previously.  The host was expecting
to receive the CSW at this point, so there was an overflow error.
That's what caused the host to perform a reset.

Evidently this UDC implements the set_halt method incorrectly.  
According to the kerneldoc for usb_ep_set_halt:

 * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any
 * transfer requests are still queued, or if the controller hardware
 * (usually a FIFO) still holds bytes that the host hasn't collected.

This didn't happen; the endpoint was halted before the host collected 
the pending data.

Incidentally, even though the URB completed with -EOVERFLOW status, we
still should see the first 13 bytes of data (i.e., the portion that
could fit into the data buffer).  But the actual_length value is 0.  I
don't know if this is a quirk of the xHCI hardware or a bug in
xhci-hcd.

 ec1a8540 1237469361 S Co:001:00 s 23 03 0004 0001  0
 ec1a8540 1237471551 C Co:001:00 0 0
 ed2209c0 1237534064 S Ci:001:00 s a3 00  0001 0004 4 
 ed2209c0 1237537012 C Ci:001:00 0 4 = 03051000
 ed2209c0 1237594113 S Co:001:00 s 23 01 0014 0001  0
 ed2209c0 1237595037 C Co:001:00 0 0
 ed2209c0 1237595434 S Co:001:00 s 23 01 0001 0001  0
 ed2209c0 1237597480 C Co:001:00 0 0

Immediately after resetting the port, the host disabled it.  No
indication of why.

 ed2209c0 1237597823 S Co:001:00 s 23 03 0004 0001  0
 ed2209c0 1237597890 C Co:001:00 0 0
 ed2209c0 1237654005 S Ci:001:00 s a3 00  0001 0004 4 
 ed2209c0 1237654098 C Ci:001:00 0 4 = 03051000
 ed2209c0 1237714084 S Co:001:00 s 23 01 0014 0001  0
 ed2209c0 1237714151 C Co:001:00 0 0
 ed2209c0 1237715894 S Co:001:00 s 23 01 0001 0001  0
 ed2209c0 1237715985 C Co:001:00 0 0

Another reset followed by another port disable.

 ed2209c0 1237716244 S Co:001:00 s 23 03 0004 0001  0
 ed2209c0 1237716308 C Co:001:00 0 0
 ed2209c0 1237774094 S Ci:001:00 s a3 00  0001 0004 4 
 ed2209c0 1237775327 C Ci:001:00 0 4 = 03051000
 ed2209c0 1237834107 S Co:001:00 s 23 01 0014 0001  0
 ed2209c0 1237834183 C Co:001:00 0 0
 ed2209c0 1237854094 S Ci:003:00 s 80 06 0100  0008 8 
 ed2209c0 1237854455 C Ci:003:00 0 8 = 12011002 0040
 ed2209c0 1237854963 S Ci:003:00 s 80 06 0100  0012 18 
 ed2209c0 1237855219 C Ci:003:00 0 18 = 12011002 0040 2505a5a4 17030304 
 0001
 ed2209c0 1237855544 S Ci:003:00 s 80 06 0f00  0005 5 
 ed2209c0 1237855771 C Ci:003:00 0 5 = 050f1600 02
 ed2209c0 1237856062 S Ci:003:00 s 80 06 0f00  0016 22 
 ed2209c0 1237856265 C Ci:003:00 0 22 = 050f1600 02071002 0200 0a100300 
 0f000101 f401
 ed2209c0 1237856548 S Ci:003:00 s 80 06 0200  0020 32 
 ed2209c0 1237858430 C Ci:003:00 0 32 = 09022000 010100c0 01090400 00020806 
 50010705 81020002 00070501 02000201
 ed2200c0 1237860245 S Co:003:00 s 00 09 0001   0
 ed2200c0 1237861785 C Co:003:00 0 0

Then a normal reset, like we should have seen originally.  I have no
idea what was going on.  Maybe something involving warm vs. cold 
resets?

 ed2541c0 1237875505 S Bo:003:01 -115 31 = 55534243 0700 c000 861a 
 003f00c0   00
 ed2541c0 1237875778 C Bo:003:01 0 31 
 ed2200c0 1237876448 S Bi:003:01 -115 192 
 ed2200c0 1237876534 C Bi:003:01 -32 0
 ed2541c0 1237876703 S Co:003:00 s 02 01  0081  0
 ed2541c0 1237876883 C Co:003:00 0 0
 ed2541c0 1237876987 S Bi:003:01 -115 13 
 ed2541c0 1237877114 C Bi:003:01 0 0
 ed2541c0 1237877486 S Bi:003:01 -115 13 
 ed2541c0 1237877572 C Bi:003:01 0 13 = 55534253 0700 c000 01

Here the MODE SENSE command was sent again, and this time the gadget
responded in a way that the host could accept.  I think it still
wasn't _right_, because it appears the gadget tried to send a 0-length
reply after the reset.  But at least it didn't provoke another reset.

 ed2541c0 1237877915 S Bo:003:01 -115 31 = 55534243 0800 1200 8603 
 0012   00
 ed2541c0 1237878041 C 

Re: g_mass_storage bug ?

2014-09-24 Thread Alan Stern
On Wed, 24 Sep 2014, Felipe Balbi wrote:

 so here's sequence of events so far:
 
 - Enumration goes fine
 - Get Max Lun - 0 (single lun)
 - Inquiry - Passed
 - Test Unit Ready - Failed
 - Request Sense (Unit Attention)  - Passed
 - Test Unit Ready - Passed
 - Mode Sense  - Stall of Data transport.
   - Clear Endpoint Feature (HALT) EP1 IN
   - After clear feature, a 16 bulk in completes. Shouldn't gadget
 driver have cancelled that ?

No.  The 16-byte transfer (which I presume was the response to the MODE
SENSE) should have completed _before_ the halt feature was set.  The
UDC driver is buggy.

 - Bus reset
 
 This remains for a few iterations. One thing is very interesting ...
 
 [ snip ]
 
  ed2541c0 1239906485 S Bo:003:01 -115 31 = 55534243 1e00 1200 
  8603 0012   00
  ed2541c0 1239906590 C Bo:003:01 0 31 
  ec1a8740 1239906770 S Bi:003:01 -115 18 
  ec1a8740 1239906871 C Bi:003:01 0 18 = 7600 000a  2900 
  
  ed2541c0 1239906975 S Bi:003:01 -115 13 
  ed2541c0 1239907026 C Bi:003:01 0 13 = 55534253 1e00  00
  ed2541c0 1239907803 S Bo:003:01 -115 31 = 55534243 1f00 0002 
  8ca1 082e0001  ec00 00
 
 0xa1 ? What is this ? Looks like XHCI corrupted the packet ? I can see
 the same SCSI opcode (0xa1) with my sniffer.

0xa1 is an ATA pass-through command.

Alan Stern

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


Re: [PATCH v2] usb: core: downgrade log severity to info when descriptor unavailable

2014-09-24 Thread Scot Doyle

On Tue, 23 Sep 2014, Greg Kroah-Hartman wrote:

On Tue, Sep 23, 2014 at 07:12:40PM +, Scot Doyle wrote:

According to commit 0cce2eda19923e5e5ccc8b042dec5af87b3ffad0
USB: fix LANGID=0 regression

usb devices are not required to report string descriptors. Since they are
optional, log an info message instead of an error message.

Signed-off-by: Scot Doyle lkm...@scotdoyle.com
---
 drivers/usb/core/message.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 0c8a7fc..da2f1f2 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -780,8 +780,12 @@ static int usb_get_langid(struct usb_device *dev, unsigned 
char *tbuf)
 * deal with strings at all. Set string_langid to -1 in order to
 * prevent any string to be retrieved from the device */
if (err  0) {
-   dev_err(dev-dev, string descriptor 0 read error: %d\n,
-   err);
+   if (err == -EPIPE)
+   dev_info(dev-dev,
+string descriptor 0 read error: -EPIPE\n);
+   else
+   dev_err(dev-dev,
+   string descriptor 0 read error: %d\n, err);


What real difference does this patch make?  What would a user do with
-EPIPE?


Not much difference, except e.g. a log line that isn't red.


And USB devices _are_ required to provide a string descriptor if they
provide a string id, so your changelog body doesn't make much sense.  I
suggest rewriting it to say they aren't required to provide a string
descriptor for string 0.

But even then, I don't understand the goal of this patch.


I'm happy to drop this patch request, since in my case the return value is 
-ENODATA, which is covered by the other patch: [RFC PATCH] usb: core: log 
more general message on malformed LANGID descriptor


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


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
Hi,

On Wed, Sep 24, 2014 at 01:56:21PM -0400, Alan Stern wrote:
 On Wed, 24 Sep 2014, Felipe Balbi wrote:
 
  so here's sequence of events so far:
  
  - Enumration goes fine
  - Get Max Lun   - 0 (single lun)
  - Inquiry   - Passed
  - Test Unit Ready   - Failed
  - Request Sense (Unit Attention)- Passed
  - Test Unit Ready   - Passed
  - Mode Sense- Stall of Data transport.
  - Clear Endpoint Feature (HALT) EP1 IN
  - After clear feature, a 16 bulk in completes. Shouldn't gadget
driver have cancelled that ?
 
 No.  The 16-byte transfer (which I presume was the response to the MODE
 SENSE) should have completed _before_ the halt feature was set.  The
 UDC driver is buggy.

ahaaa, now to figure out how to synchronize that.

  - Bus reset
  
  This remains for a few iterations. One thing is very interesting ...
  
  [ snip ]
  
   ed2541c0 1239906485 S Bo:003:01 -115 31 = 55534243 1e00 1200 
   8603 0012   00
   ed2541c0 1239906590 C Bo:003:01 0 31 
   ec1a8740 1239906770 S Bi:003:01 -115 18 
   ec1a8740 1239906871 C Bi:003:01 0 18 = 7600 000a  
   2900 
   ed2541c0 1239906975 S Bi:003:01 -115 13 
   ed2541c0 1239907026 C Bi:003:01 0 13 = 55534253 1e00  00
   ed2541c0 1239907803 S Bo:003:01 -115 31 = 55534243 1f00 0002 
   8ca1 082e0001  ec00 00
  
  0xa1 ? What is this ? Looks like XHCI corrupted the packet ? I can see
  the same SCSI opcode (0xa1) with my sniffer.
 
 0xa1 is an ATA pass-through command.

ok, thanks.

-- 
balbi


signature.asc
Description: Digital signature


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 01:53:31PM -0400, Alan Stern wrote:
 On Wed, 24 Sep 2014, Felipe Balbi wrote:
 
   I'll capture usbmon and send here shortly.
  
  here it is... Interesting part starts at line 73 (114 on this email)
  where the data transport received EPIPE (due to Stall). This time
  however, I was eventually able to talk to the device and managed to
  issue quite a few writes to it.
 
 Here's where the unexpected stuff begins:
 
  ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 0600 c000 
  861a 003f00c0   00
  ed2541c0 1237463431 C Bo:003:01 0 31 
  ec1a8540 1237463873 S Bi:003:01 -115 192 
  ec1a8540 1237464053 C Bi:003:01 -32 0
  ed2541c0 1237464158 S Co:003:00 s 02 01  0081  0
  ed2541c0 1237464359 C Co:003:00 0 0
  ed2541c0 1237468607 S Bi:003:01 -115 13 
  ed2541c0 1237468802 C Bi:003:01 -75 0
 
 This is the first MODE SENSE command.  The gadget should send as much
 data as it can before halting the bulk-IN endpoint.  Instead, the
 endpoint was halted first.
 
 Then, after the host cleared the halt, the gadget apparently sent the
 data that _should_ have been sent previously.  The host was expecting
 to receive the CSW at this point, so there was an overflow error.
 That's what caused the host to perform a reset.
 
 Evidently this UDC implements the set_halt method incorrectly.  
 According to the kerneldoc for usb_ep_set_halt:
 
  * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any
  * transfer requests are still queued, or if the controller hardware
  * (usually a FIFO) still holds bytes that the host hasn't collected.

damn old bugs :-) I'll fix that up and Cc stable.

 This didn't happen; the endpoint was halted before the host collected 
 the pending data.
 
 Incidentally, even though the URB completed with -EOVERFLOW status, we
 still should see the first 13 bytes of data (i.e., the portion that
 could fit into the data buffer).  But the actual_length value is 0.  I
 don't know if this is a quirk of the xHCI hardware or a bug in
 xhci-hcd.
 
  ec1a8540 1237469361 S Co:001:00 s 23 03 0004 0001  0
  ec1a8540 1237471551 C Co:001:00 0 0
  ed2209c0 1237534064 S Ci:001:00 s a3 00  0001 0004 4 
  ed2209c0 1237537012 C Ci:001:00 0 4 = 03051000
  ed2209c0 1237594113 S Co:001:00 s 23 01 0014 0001  0
  ed2209c0 1237595037 C Co:001:00 0 0
  ed2209c0 1237595434 S Co:001:00 s 23 01 0001 0001  0
  ed2209c0 1237597480 C Co:001:00 0 0
 
 Immediately after resetting the port, the host disabled it.  No
 indication of why.
 
  ed2209c0 1237597823 S Co:001:00 s 23 03 0004 0001  0
  ed2209c0 1237597890 C Co:001:00 0 0
  ed2209c0 1237654005 S Ci:001:00 s a3 00  0001 0004 4 
  ed2209c0 1237654098 C Ci:001:00 0 4 = 03051000
  ed2209c0 1237714084 S Co:001:00 s 23 01 0014 0001  0
  ed2209c0 1237714151 C Co:001:00 0 0
  ed2209c0 1237715894 S Co:001:00 s 23 01 0001 0001  0
  ed2209c0 1237715985 C Co:001:00 0 0
 
 Another reset followed by another port disable.
 
  ed2209c0 1237716244 S Co:001:00 s 23 03 0004 0001  0
  ed2209c0 1237716308 C Co:001:00 0 0
  ed2209c0 1237774094 S Ci:001:00 s a3 00  0001 0004 4 
  ed2209c0 1237775327 C Ci:001:00 0 4 = 03051000
  ed2209c0 1237834107 S Co:001:00 s 23 01 0014 0001  0
  ed2209c0 1237834183 C Co:001:00 0 0
  ed2209c0 1237854094 S Ci:003:00 s 80 06 0100  0008 8 
  ed2209c0 1237854455 C Ci:003:00 0 8 = 12011002 0040
  ed2209c0 1237854963 S Ci:003:00 s 80 06 0100  0012 18 
  ed2209c0 1237855219 C Ci:003:00 0 18 = 12011002 0040 2505a5a4 17030304 
  0001
  ed2209c0 1237855544 S Ci:003:00 s 80 06 0f00  0005 5 
  ed2209c0 1237855771 C Ci:003:00 0 5 = 050f1600 02
  ed2209c0 1237856062 S Ci:003:00 s 80 06 0f00  0016 22 
  ed2209c0 1237856265 C Ci:003:00 0 22 = 050f1600 02071002 0200 0a100300 
  0f000101 f401
  ed2209c0 1237856548 S Ci:003:00 s 80 06 0200  0020 32 
  ed2209c0 1237858430 C Ci:003:00 0 32 = 09022000 010100c0 01090400 00020806 
  50010705 81020002 00070501 02000201
  ed2200c0 1237860245 S Co:003:00 s 00 09 0001   0
  ed2200c0 1237861785 C Co:003:00 0 0
 
 Then a normal reset, like we should have seen originally.  I have no
 idea what was going on.  Maybe something involving warm vs. cold 
 resets?
 
  ed2541c0 1237875505 S Bo:003:01 -115 31 = 55534243 0700 c000 
  861a 003f00c0   00
  ed2541c0 1237875778 C Bo:003:01 0 31 
  ed2200c0 1237876448 S Bi:003:01 -115 192 
  ed2200c0 1237876534 C Bi:003:01 -32 0
  ed2541c0 1237876703 S Co:003:00 s 02 01  0081  0
  ed2541c0 1237876883 C Co:003:00 0 0
  ed2541c0 1237876987 S Bi:003:01 -115 13 
  ed2541c0 1237877114 C Bi:003:01 0 0
  ed2541c0 1237877486 S Bi:003:01 -115 13 
  ed2541c0 1237877572 C Bi:003:01 0 13 = 55534253 0700 c000 01
 
 Here the MODE SENSE command was sent again, and this time the gadget
 responded in a way that the host could accept.  I think it still
 wasn't _right_, because it appears the gadget tried to send a 

MUSB: extra cppi irq?

2014-09-24 Thread Bin Liu
Hi Felipe and all,

The musb driver musb_host_tx() has the following:

1244 /* with CPPI, DMA sometimes triggers extra irqs */
1245 if (!urb) {
1246 dev_dbg(musb-controller, extra TX%d ready, csr
%04x\n, epnum, tx_csr);
1247 return;
1248 }

and

1321 /* second cppi case */
1322 if (dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) {
1323 dev_dbg(musb-controller, extra TX%d ready, csr
%04x\n, epnum, tx_csr);
1324 return;
1325 }

which come with the very first commit of musb driver and never got
changed. Is there any more information about the 'extra' irqs? and
what is the 'second cppi case'?

I ran into this problem and musb stops working after hits here.

[13542.933563] musb-hdrc musb-hdrc.1: qh c261ad80 urb eec92bc0 dev4
ep7in-bulk, hw_ep 2, c2599000/4096
[13542.953552] musb-hdrc musb-hdrc.1: usbintr (0) epintr(4)
[13542.953582] musb-hdrc musb-hdrc.1: extra TX2 ready, csr 0004

I bet this 'extra TX2 ready' log is printed by the first case, because
only RX2 is used in this test, and TX2 is never got used, so there
should not be any urb in TX2 queue. I am still waiting for another
test failure to confirm this, but I am trying to understand why TX2
interrupt happened...which seems to be related RX2 request.

Thanks for any help.

-Bin.
--
To unsubscribe from this list: send the line unsubscribe linux-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/2] usb: rename phy to usb_phy in HCD

2014-09-24 Thread Sergei Shtylyov

hello.

On 09/24/2014 09:11 AM, Greg KH wrote:


From: Antoine Tenart antoine.ten...@free-electrons.com



The USB PHY member of the HCD structure is renamed to 'usb_phy' and
modifications are done in all drivers accessing it.
This is in preparation to adding the generic PHY support.



Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com
[Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects
caused by patch reordering, updated changelog.]
Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
Acked-by: Alan Stern st...@rowland.harvard.edu
Acked-by: Felipe Balbi ba...@ti.com



---
Changes in version 5:
- imported the patch from Antoine Tenart's series;
- added missing 'drivers/usb/misc/lvstest.c' file;
- resolved rejects caused by patch reordering;
- refreshed patch;
- updated changelog.



  drivers/usb/chipidea/host.c   |2 +-
  drivers/usb/core/hcd.c|   20 ++--
  drivers/usb/core/hub.c|8 
  drivers/usb/host/ehci-fsl.c   |   16 
  drivers/usb/host/ehci-hub.c   |2 +-
  drivers/usb/host/ehci-msm.c   |4 ++--
  drivers/usb/host/ehci-tegra.c |   16 
  drivers/usb/host/ohci-omap.c  |   20 ++--
  drivers/usb/misc/lvstest.c|8 
  include/linux/usb/hcd.h   |2 +-
  10 files changed, 49 insertions(+), 49 deletions(-)



This doesn't apply to my tree at all anymore,


   Well, I'm seeing only a minor reject in the first file, easily fixable.


can you refresh it and resend?


   OK, will re-post now.


thanks,



greg k-h


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 v6 0/2] Add generic PHY support to USB HCD

2014-09-24 Thread Sergei Shtylyov
Hello.

   This patchset is against the usb-next' branch of Greg KH's 'usb.git' repo.
Here I add support for the generic PHY to the 'struct usb_hcd' (having to
rename the existing 'phy' field to 'usb_phy' beforehand). This was mainly
intended to be used with the PCI OHCI/EHCI drivers and also xHCI driver;
however there are several more dependent patchsets now posted to 'linux-usb'.

[1/2] usb: rename phy to usb_phy in HCD
[2/2] usb: hcd: add generic PHY support

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 v6 1/2] usb: rename phy to usb_phy in HCD

2014-09-24 Thread Sergei Shtylyov
From: Antoine Tenart antoine.ten...@free-electrons.com

The USB PHY member of the HCD structure is renamed to 'usb_phy' and
modifications are done in all drivers accessing it.
This is in preparation to adding the generic PHY support.

Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com
[Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects,
updated changelog.]
Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
Acked-by: Alan Stern st...@rowland.harvard.edu

---
Changes in version 6:
- resolved reject, refreshed the patch.

Changes in version 5:
- imported the patch from Antoine Tenart's series;
- added missing 'drivers/usb/misc/lvstest.c' file;
- resolved rejects caused by patch reordering;
- refreshed patch;
- updated changelog.

 drivers/usb/chipidea/host.c   |2 +-
 drivers/usb/core/hcd.c|   20 ++--
 drivers/usb/core/hub.c|8 
 drivers/usb/host/ehci-fsl.c   |   16 
 drivers/usb/host/ehci-hub.c   |2 +-
 drivers/usb/host/ehci-msm.c   |4 ++--
 drivers/usb/host/ehci-tegra.c |   16 
 drivers/usb/host/ohci-omap.c  |   20 ++--
 drivers/usb/misc/lvstest.c|8 
 include/linux/usb/hcd.h   |2 +-
 10 files changed, 49 insertions(+), 49 deletions(-)

Index: usb/drivers/usb/chipidea/host.c
===
--- usb.orig/drivers/usb/chipidea/host.c
+++ usb/drivers/usb/chipidea/host.c
@@ -59,7 +59,7 @@ static int host_start(struct ci_hdrc *ci
hcd-has_tt = 1;
 
hcd-power_budget = ci-platdata-power_budget;
-   hcd-phy = ci-transceiver;
+   hcd-usb_phy = ci-transceiver;
hcd-tpl_support = ci-platdata-tpl_support;
 
ehci = hcd_to_ehci(hcd);
Index: usb/drivers/usb/core/hcd.c
===
--- usb.orig/drivers/usb/core/hcd.c
+++ usb/drivers/usb/core/hcd.c
@@ -2627,7 +2627,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
int retval;
struct usb_device *rhdev;
 
-   if (IS_ENABLED(CONFIG_USB_PHY)  !hcd-phy) {
+   if (IS_ENABLED(CONFIG_USB_PHY)  !hcd-usb_phy) {
struct usb_phy *phy = usb_get_phy_dev(hcd-self.controller, 0);
 
if (IS_ERR(phy)) {
@@ -2640,7 +2640,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
usb_put_phy(phy);
return retval;
}
-   hcd-phy = phy;
+   hcd-usb_phy = phy;
hcd-remove_phy = 1;
}
}
@@ -2788,10 +2788,10 @@ err_allocate_root_hub:
 err_register_bus:
hcd_buffer_destroy(hcd);
 err_remove_phy:
-   if (hcd-remove_phy  hcd-phy) {
-   usb_phy_shutdown(hcd-phy);
-   usb_put_phy(hcd-phy);
-   hcd-phy = NULL;
+   if (hcd-remove_phy  hcd-usb_phy) {
+   usb_phy_shutdown(hcd-usb_phy);
+   usb_put_phy(hcd-usb_phy);
+   hcd-usb_phy = NULL;
}
return retval;
 }
@@ -2864,10 +2864,10 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 
usb_deregister_bus(hcd-self);
hcd_buffer_destroy(hcd);
-   if (hcd-remove_phy  hcd-phy) {
-   usb_phy_shutdown(hcd-phy);
-   usb_put_phy(hcd-phy);
-   hcd-phy = NULL;
+   if (hcd-remove_phy  hcd-usb_phy) {
+   usb_phy_shutdown(hcd-usb_phy);
+   usb_put_phy(hcd-usb_phy);
+   hcd-usb_phy = NULL;
}
 
usb_put_invalidate_rhdev(hcd);
Index: usb/drivers/usb/core/hub.c
===
--- usb.orig/drivers/usb/core/hub.c
+++ usb/drivers/usb/core/hub.c
@@ -4467,8 +4467,8 @@ hub_port_init (struct usb_hub *hub, stru
if (retval)
goto fail;
 
-   if (hcd-phy  !hdev-parent)
-   usb_phy_notify_connect(hcd-phy, udev-speed);
+   if (hcd-usb_phy  !hdev-parent)
+   usb_phy_notify_connect(hcd-usb_phy, udev-speed);
 
/*
 * Some superspeed devices have finished the link training process
@@ -4626,9 +4626,9 @@ static void hub_port_connect(struct usb_
 
/* Disconnect any existing devices under this port */
if (udev) {
-   if (hcd-phy  !hdev-parent 
+   if (hcd-usb_phy  !hdev-parent 
!(portstatus  USB_PORT_STAT_CONNECTION))
-   usb_phy_notify_disconnect(hcd-phy, udev-speed);
+   usb_phy_notify_disconnect(hcd-usb_phy, udev-speed);
usb_disconnect(port_dev-child);
}
 
Index: usb/drivers/usb/host/ehci-fsl.c
===
--- usb.orig/drivers/usb/host/ehci-fsl.c
+++ usb/drivers/usb/host/ehci-fsl.c
@@ -136,15 +136,15 @@ static int usb_hcd_fsl_probe(const struc
if 

[PATCH v6 2/2] usb: hcd: add generic PHY support

2014-09-24 Thread Sergei Shtylyov
Add the generic PHY support, analogous to the USB PHY support. Intended it to be
used with the PCI EHCI/OHCI drivers and the xHCI platform driver.

Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com

---
Changes in version 5:
- renamed the new 'gen_phy' field of 'struct usb_phy' back to 'phy';
- resolved rejects occured due to a newly added patch.

Changes in version 4:
- refreshed the patch.

Changes in version 3:
- refreshed the patch.

Changes in version 2:
- renamed the new 'phy' field of 'struct usb_phy' to 'gen_phy';
- resolved rejects due to removal of the first patch in the series.

 drivers/usb/core/hcd.c  |   42 --
 include/linux/usb/hcd.h |1 +
 2 files changed, 41 insertions(+), 2 deletions(-)

Index: usb/drivers/usb/core/hcd.c
===
--- usb.orig/drivers/usb/core/hcd.c
+++ usb/drivers/usb/core/hcd.c
@@ -42,6 +42,7 @@
 #include linux/pm_runtime.h
 #include linux/types.h
 
+#include linux/phy/phy.h
 #include linux/usb.h
 #include linux/usb/hcd.h
 #include linux/usb/phy.h
@@ -2645,6 +2646,29 @@ int usb_add_hcd(struct usb_hcd *hcd,
}
}
 
+   if (IS_ENABLED(CONFIG_GENERIC_PHY)) {
+   struct phy *phy = phy_get(hcd-self.controller, usb);
+
+   if (IS_ERR(phy)) {
+   retval = PTR_ERR(phy);
+   if (retval == -EPROBE_DEFER)
+   goto err_phy;
+   } else {
+   retval = phy_init(phy);
+   if (retval) {
+   phy_put(phy);
+   goto err_phy;
+   }
+   retval = phy_power_on(phy);
+   if (retval) {
+   phy_exit(phy);
+   phy_put(phy);
+   goto err_phy;
+   }
+   hcd-phy = phy;
+   }
+   }
+
dev_info(hcd-self.controller, %s\n, hcd-product_desc);
 
/* Keep old behaviour if authorized_default is not in [0, 1]. */
@@ -2660,7 +2684,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
 */
if ((retval = hcd_buffer_create(hcd)) != 0) {
dev_dbg(hcd-self.controller, pool alloc failed\n);
-   goto err_remove_phy;
+   goto err_create_buf;
}
 
if ((retval = usb_register_bus(hcd-self))  0)
@@ -2787,7 +2811,14 @@ err_allocate_root_hub:
usb_deregister_bus(hcd-self);
 err_register_bus:
hcd_buffer_destroy(hcd);
-err_remove_phy:
+err_create_buf:
+   if (IS_ENABLED(CONFIG_GENERIC_PHY)  hcd-phy) {
+   phy_power_off(hcd-phy);
+   phy_exit(hcd-phy);
+   phy_put(hcd-phy);
+   hcd-phy = NULL;
+   }
+err_phy:
if (hcd-remove_phy  hcd-usb_phy) {
usb_phy_shutdown(hcd-usb_phy);
usb_put_phy(hcd-usb_phy);
@@ -2864,6 +2895,13 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 
usb_deregister_bus(hcd-self);
hcd_buffer_destroy(hcd);
+
+   if (IS_ENABLED(CONFIG_GENERIC_PHY)  hcd-phy) {
+   phy_power_off(hcd-phy);
+   phy_exit(hcd-phy);
+   phy_put(hcd-phy);
+   hcd-phy = NULL;
+   }
if (hcd-remove_phy  hcd-usb_phy) {
usb_phy_shutdown(hcd-usb_phy);
usb_put_phy(hcd-usb_phy);
Index: usb/include/linux/usb/hcd.h
===
--- usb.orig/include/linux/usb/hcd.h
+++ usb/include/linux/usb/hcd.h
@@ -107,6 +107,7 @@ struct usb_hcd {
 * other external phys should be software-transparent
 */
struct usb_phy  *usb_phy;
+   struct phy  *phy;
 
/* Flags that need to be manipulated atomically because they can
 * change while the host controller is running.  Always use

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


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
Hi,

On Wed, Sep 24, 2014 at 01:08:15PM -0500, Felipe Balbi wrote:
 On Wed, Sep 24, 2014 at 01:53:31PM -0400, Alan Stern wrote:
  On Wed, 24 Sep 2014, Felipe Balbi wrote:
  
I'll capture usbmon and send here shortly.
   
   here it is... Interesting part starts at line 73 (114 on this email)
   where the data transport received EPIPE (due to Stall). This time
   however, I was eventually able to talk to the device and managed to
   issue quite a few writes to it.
  
  Here's where the unexpected stuff begins:
  
   ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 0600 c000 
   861a 003f00c0   00
   ed2541c0 1237463431 C Bo:003:01 0 31 
   ec1a8540 1237463873 S Bi:003:01 -115 192 
   ec1a8540 1237464053 C Bi:003:01 -32 0
   ed2541c0 1237464158 S Co:003:00 s 02 01  0081  0
   ed2541c0 1237464359 C Co:003:00 0 0
   ed2541c0 1237468607 S Bi:003:01 -115 13 
   ed2541c0 1237468802 C Bi:003:01 -75 0
  
  This is the first MODE SENSE command.  The gadget should send as much
  data as it can before halting the bulk-IN endpoint.  Instead, the
  endpoint was halted first.
  
  Then, after the host cleared the halt, the gadget apparently sent the
  data that _should_ have been sent previously.  The host was expecting
  to receive the CSW at this point, so there was an overflow error.
  That's what caused the host to perform a reset.
  
  Evidently this UDC implements the set_halt method incorrectly.  
  According to the kerneldoc for usb_ep_set_halt:
  
   * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any
   * transfer requests are still queued, or if the controller hardware
   * (usually a FIFO) still holds bytes that the host hasn't collected.
 
 damn old bugs :-) I'll fix that up and Cc stable.

alright fixed. Below you can see a combined diff which I'll still split
into patches so they can be applied properly.

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 66f6256..834f524 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -210,6 +210,14 @@
 #define DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(n)   (((n)  (0x0f  13))  13)
 #define DWC3_MAX_HIBER_SCRATCHBUFS 15
 
+/* Global FIFO Space Register */
+#define DWC3_GDBGFIFOSPACE_TXFIFO  (0  5)
+#define DWC3_GDBGFIFOSPACE_RXFIFO  (1  5)
+#define DWC3_GDBGFIFOSPACE_TXREQ_Q (2  5)
+#define DWC3_GDBGFIFOSPACE_RXREQ_Q (3  5)
+
+#define DWC3_GDBGFIFOSPACE_SPACE_AVAIL(num)(((num)  0x)  16)
+
 /* Device Configuration Register */
 #define DWC3_DCFG_DEVADDR(addr)((addr)  3)
 #define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index de53361..5e89913 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -145,6 +145,75 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum 
dwc3_link_state state)
 }
 
 /**
+ * dwc3_gadget_ep_fifo_space - returns currently available space on FIFO
+ * @dwc: pointer to our context struct
+ * @dep: the endpoint to fetch FIFO space
+ *
+ * This function will return the currently available FIFO space
+ */
+static int dwc3_gadget_ep_fifo_space(struct dwc3 *dwc, struct dwc3_ep *dep)
+{
+   u32 current_fifo;
+   u32 reg;
+
+   if (dep-direction)
+   reg = DWC3_GDBGFIFOSPACE_TXFIFO;
+   else
+   reg = DWC3_GDBGFIFOSPACE_RXFIFO;
+
+   /* remove direction bit */
+   reg |= dep-number  1;
+
+   dwc3_writel(dwc-regs, DWC3_GDBGFIFOSPACE, reg);
+   reg = dwc3_readl(dwc-regs, DWC3_GDBGFIFOSPACE);
+   current_fifo = DWC3_GDBGFIFOSPACE_SPACE_AVAIL(reg);
+
+   return current_fifo;
+}
+
+/**
+ * dwc3_gadget_ep_fifo_size - returns allocated FIFO size
+ * @dwc: pointer to our context struct
+ * @dep: TX endpoint to fetch allocated FIFO size
+ *
+ * This function will read the correct TX FIFO register and
+ * return the FIFO size
+ */
+static int dwc3_gadget_ep_fifo_size(struct dwc3 *dwc, struct dwc3_ep *dep)
+{
+   if (WARN_ON(!dep-direction))
+   return 0;
+
+   return dwc3_readl(dwc-regs, DWC3_GTXFIFOSIZ(dep-number))  0x;
+}
+
+/**
+ * dwc3_gadget_ep_fifo_empty - returns true when FIFO is empty
+ * @dwc: pointer to our context struct
+ * @dep: endpoint to request FIFO space
+ *
+ * This function will return a TRUE when FIFO is empty and FALSE
+ * otherwise.
+ */
+static int dwc3_gadget_ep_fifo_empty(struct dwc3 *dwc, struct dwc3_ep *dep)
+{
+   u32 allocated_fifo;
+   u32 current_fifo;
+
+   /* should not be called for RX endpoints */
+   if (WARN_ON(!dep-direction))
+   return false;
+
+   current_fifo = dwc3_gadget_ep_fifo_space(dwc, dep);
+   allocated_fifo = dwc3_gadget_ep_fifo_size(dwc, dep);
+
+   dev_vdbg(dwc-dev, %s: FIFO space %u/%u\n, dep-name,
+   current_fifo, allocated_fifo);
+
+   return current_fifo == allocated_fifo;
+}
+
+/**
  * 

Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
Hi,

On Wed, Sep 24, 2014 at 02:18:14PM -0500, Felipe Balbi wrote:
 On Wed, Sep 24, 2014 at 01:08:15PM -0500, Felipe Balbi wrote:
  On Wed, Sep 24, 2014 at 01:53:31PM -0400, Alan Stern wrote:
   On Wed, 24 Sep 2014, Felipe Balbi wrote:
   
 I'll capture usbmon and send here shortly.

here it is... Interesting part starts at line 73 (114 on this email)
where the data transport received EPIPE (due to Stall). This time
however, I was eventually able to talk to the device and managed to
issue quite a few writes to it.
   
   Here's where the unexpected stuff begins:
   
ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 0600 c000 
861a 003f00c0   00
ed2541c0 1237463431 C Bo:003:01 0 31 
ec1a8540 1237463873 S Bi:003:01 -115 192 
ec1a8540 1237464053 C Bi:003:01 -32 0
ed2541c0 1237464158 S Co:003:00 s 02 01  0081  0
ed2541c0 1237464359 C Co:003:00 0 0
ed2541c0 1237468607 S Bi:003:01 -115 13 
ed2541c0 1237468802 C Bi:003:01 -75 0
   
   This is the first MODE SENSE command.  The gadget should send as much
   data as it can before halting the bulk-IN endpoint.  Instead, the
   endpoint was halted first.
   
   Then, after the host cleared the halt, the gadget apparently sent the
   data that _should_ have been sent previously.  The host was expecting
   to receive the CSW at this point, so there was an overflow error.
   That's what caused the host to perform a reset.
   
   Evidently this UDC implements the set_halt method incorrectly.  
   According to the kerneldoc for usb_ep_set_halt:
   
* Attempts to halt IN endpoints will fail (returning -EAGAIN) if any
* transfer requests are still queued, or if the controller hardware
* (usually a FIFO) still holds bytes that the host hasn't collected.
  
  damn old bugs :-) I'll fix that up and Cc stable.
 
 alright fixed. Below you can see a combined diff which I'll still split
 into patches so they can be applied properly.

OTOH, there's really no way to split it. It's all needed to fix a single
bug. Do you want me to add Reported-by: Alan Stern ?

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v6 1/2] usb: rename phy to usb_phy in HCD

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 11:05:50PM +0400, Sergei Shtylyov wrote:
 From: Antoine Tenart antoine.ten...@free-electrons.com
 
 The USB PHY member of the HCD structure is renamed to 'usb_phy' and
 modifications are done in all drivers accessing it.
 This is in preparation to adding the generic PHY support.
 
 Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com
 [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects,
 updated changelog.]
 Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
 Acked-by: Alan Stern st...@rowland.harvard.edu

Acked-by: Felipe Balbi ba...@ti.com

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v6 1/2] usb: rename phy to usb_phy in HCD

2014-09-24 Thread Sergei Shtylyov

Hello.

On 09/24/2014 11:28 PM, Felipe Balbi wrote:


From: Antoine Tenart antoine.ten...@free-electrons.com



The USB PHY member of the HCD structure is renamed to 'usb_phy' and
modifications are done in all drivers accessing it.
This is in preparation to adding the generic PHY support.



Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com
[Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects,
updated changelog.]
Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
Acked-by: Alan Stern st...@rowland.harvard.edu



Acked-by: Felipe Balbi ba...@ti.com


   Sorry, forgot to collect your ACK from the previous posting. :-/

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


Re: g_mass_storage bug ?

2014-09-24 Thread Alan Stern
On Wed, 24 Sep 2014, Felipe Balbi wrote:

   According to the kerneldoc for usb_ep_set_halt:
   
* Attempts to halt IN endpoints will fail (returning -EAGAIN) if any
* transfer requests are still queued, or if the controller hardware
* (usually a FIFO) still holds bytes that the host hasn't collected.
  
  damn old bugs :-) I'll fix that up and Cc stable.
 
 alright fixed. Below you can see a combined diff which I'll still split
 into patches so they can be applied properly.

And this eliminates the problems you saw with g_mass_storage?

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: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 03:31:13PM -0400, Alan Stern wrote:
 On Wed, 24 Sep 2014, Felipe Balbi wrote:
 
According to the kerneldoc for usb_ep_set_halt:

 * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any
 * transfer requests are still queued, or if the controller hardware
 * (usually a FIFO) still holds bytes that the host hasn't collected.
   
   damn old bugs :-) I'll fix that up and Cc stable.
  
  alright fixed. Below you can see a combined diff which I'll still split
  into patches so they can be applied properly.
 
 And this eliminates the problems you saw with g_mass_storage?

yup, working with and without stall=0, with and without debugging on. On
all three systems I tested before ;-)

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v6 1/2] usb: rename phy to usb_phy in HCD

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 11:31:11PM +0400, Sergei Shtylyov wrote:
 Hello.
 
 On 09/24/2014 11:28 PM, Felipe Balbi wrote:
 
 From: Antoine Tenart antoine.ten...@free-electrons.com
 
 The USB PHY member of the HCD structure is renamed to 'usb_phy' and
 modifications are done in all drivers accessing it.
 This is in preparation to adding the generic PHY support.
 
 Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com
 [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects,
 updated changelog.]
 Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 
 Acked-by: Felipe Balbi ba...@ti.com
 
Sorry, forgot to collect your ACK from the previous posting. :-/

don't worry ;-)

-- 
balbi


signature.asc
Description: Digital signature


[PATCH v6 partial 0/2] pxa27x_udc port to device-tree and gpio_desc

2014-09-24 Thread Robert Jarzmik
Hi Felipe and Linus,

As you requested, Felipe, I made the translation for the driver's
probing from gpio to gpio_desc. Even if the code is functional, I'm
pretty unhappy about my patch 1, because I lost the active_low
notion in the transformation.

I have not found in the gpiolib anything for a driver to set that
active_low value, only for machine code. The legacy behaviour was that
that information was handed over to the driver.

As transforming the different pxa27x_udc users will be not part of
this serie, ie. because the mach info should be kept ISO for this
serie, Linus, do you have an idea so that I still can port to
gpio_desc this driver, and keep the gpio_inverted notion in it ?

Ah, and I sent only 2 out of the 6 patches of the serie, mainly
because Linus doesn't care about the remaining stuff, which remained
constant from last version (at rebase conflicts resolution exception
of course).

Cheers.

--
Robert

Robert Jarzmik (2):
  usb: gadget: pxa27x_udc: prepare device-tree support
  usb: gadget: pxa27x_udc: add devicetree support

 drivers/usb/gadget/udc/pxa27x_udc.c | 61 +
 drivers/usb/gadget/udc/pxa27x_udc.h |  6 ++--
 2 files changed, 38 insertions(+), 29 deletions(-)

-- 
2.0.0.rc2

PS: For Felipe, the diff of the complete serie, for a one quick glance
overview, and not for review is attached to this mail.
 drivers/usb/gadget/udc/pxa27x_udc.c | 96 +
 drivers/usb/gadget/udc/pxa27x_udc.h | 10 ++--
 2 files changed, 26 insertions(+), 80 deletions(-)

diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c 
b/drivers/usb/gadget/udc/pxa27x_udc.c
index 9506da8..adbe74a 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -22,6 +22,7 @@
 #include linux/clk.h
 #include linux/irq.h
 #include linux/gpio.h
+#include linux/gpio/consumer.h
 #include linux/slab.h
 #include linux/prefetch.h
 #include linux/byteorder/generic.h
@@ -1509,18 +1510,13 @@ static struct usb_ep_ops pxa_ep_ops = {
  */
 static void dplus_pullup(struct pxa_udc *udc, int on)
 {
-   if (on) {
-   if (gpio_is_valid(udc-gpio_pullup))
-   gpio_set_value(udc-gpio_pullup,
-  !udc-gpio_pullup_inverted);
-   if (udc-mach  udc-mach-udc_command)
-   udc-mach-udc_command(PXA2XX_UDC_CMD_CONNECT);
-   } else {
-   if (gpio_is_valid(udc-gpio_pullup))
-   gpio_set_value(udc-gpio_pullup,
-  udc-gpio_pullup_inverted);
-   if (udc-mach  udc-mach-udc_command)
-   udc-mach-udc_command(PXA2XX_UDC_CMD_DISCONNECT);
+   if (udc-gpiod) {
+   gpiod_set_value(udc-gpiod, on);
+   } else if (udc-udc_command) {
+   if (on)
+   udc-udc_command(PXA2XX_UDC_CMD_CONNECT);
+   else
+   udc-udc_command(PXA2XX_UDC_CMD_DISCONNECT);
}
udc-pullup_on = on;
 }
@@ -1611,8 +1607,7 @@ static int pxa_udc_pullup(struct usb_gadget *_gadget, int 
is_active)
 {
struct pxa_udc *udc = to_gadget_udc(_gadget);
 
-   if (!gpio_is_valid(udc-gpio_pullup)
-!(udc-mach  udc-mach-udc_command))
+   if (!udc-gpiod  !udc-udc_command)
return -EOPNOTSUPP;
 
dplus_pullup(udc, is_active);
@@ -2414,50 +2409,6 @@ static struct of_device_id udc_pxa_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, udc_pxa_dt_ids);
 
 /**
- * pxa_udc_probe_dt - device tree specific probe
- * @pdev: platform data
- * @udc: pxa_udc structure to fill
- *
- * Fills udc from platform data out of device tree.
- *
- * Returns 0 if DT found, 1 if DT not found, and 0 on error
- */
-static int pxa_udc_probe_dt(struct platform_device *pdev, struct pxa_udc *udc)
-{
-   struct device_node *np = pdev-dev.of_node;
-   const struct of_device_id *of_id =
-   of_match_device(udc_pxa_dt_ids, pdev-dev);
-   u32 gpio_pullup;
-   enum of_gpio_flags flags;
-
-   if (!np || !of_id)
-   return 1;
-   pdev-id = -1;
-
-   gpio_pullup = of_get_gpio_flags(np, 0, flags);
-   if (gpio_pullup = 0) {
-   udc-gpio_pullup = gpio_pullup;
-   udc-gpio_pullup_inverted = (flags  OF_GPIO_ACTIVE_LOW);
-   }
-   return 0;
-}
-
-/**
- * pxa_udc_probe_pdata - legacy platform data probe
- * @pdev: platform device
- * @udc: pxa_udc structure to fill
- *
- * Simple copy of data from platform_data to udc control structure
- */
-static void pxa_udc_probe_pdata(struct platform_device *pdev,
-  struct pxa_udc *udc)
-{
-   udc-mach = dev_get_platdata(pdev-dev);
-   udc-gpio_pullup = udc-mach-gpio_pullup;
-   udc-gpio_pullup_inverted = udc-mach-gpio_pullup_inverted;
-}
-
-/**
  * pxa_udc_probe - probes the udc device
  * @_dev: platform device
  *
@@ -2468,13 +2419,15 @@ static int 

[PATCH v6 1/2] usb: gadget: pxa27x_udc: prepare device-tree support

2014-09-24 Thread Robert Jarzmik
For this preparation, a preliminary cleanup is done :
  - convert the probing of pxa27x_udc to gpio_desc.
The conversion is partial because :
 - the platform data still provides a gpio number, not a gpio desc
 - the invert attribute is lost, hence a loss in the translation
  - convert the mach info, and store the udc_command in the pxa_udc
control structure
  - loose the udc_is_connected() in mach info
This was not really used, as mioa701 machine doesn't need it,
balloon3 doesn't really use it, and most importantly the current
driver never uses it.

The drawback with the gpio_desc conversion is that the inverted gpio
attribute is lost, as no gpiod_*() function exists to set the
active_low state of a gpio, and that attribute was at driver's
creation forecast to be set up by the driver and not the machine
specific code.

Signed-off-by: Robert Jarzmik robert.jarz...@free.fr
---
 drivers/usb/gadget/udc/pxa27x_udc.c | 51 -
 drivers/usb/gadget/udc/pxa27x_udc.h |  6 +++--
 2 files changed, 26 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c 
b/drivers/usb/gadget/udc/pxa27x_udc.c
index 597d39f..e11f9c5 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -22,6 +22,7 @@
 #include linux/clk.h
 #include linux/irq.h
 #include linux/gpio.h
+#include linux/gpio/consumer.h
 #include linux/slab.h
 #include linux/prefetch.h
 #include linux/byteorder/generic.h
@@ -1507,18 +1508,13 @@ static struct usb_ep_ops pxa_ep_ops = {
  */
 static void dplus_pullup(struct pxa_udc *udc, int on)
 {
-   if (on) {
-   if (gpio_is_valid(udc-mach-gpio_pullup))
-   gpio_set_value(udc-mach-gpio_pullup,
-  !udc-mach-gpio_pullup_inverted);
-   if (udc-mach-udc_command)
-   udc-mach-udc_command(PXA2XX_UDC_CMD_CONNECT);
-   } else {
-   if (gpio_is_valid(udc-mach-gpio_pullup))
-   gpio_set_value(udc-mach-gpio_pullup,
-  udc-mach-gpio_pullup_inverted);
-   if (udc-mach-udc_command)
-   udc-mach-udc_command(PXA2XX_UDC_CMD_DISCONNECT);
+   if (udc-gpiod) {
+   gpiod_set_value(udc-gpiod, on);
+   } else if (udc-udc_command) {
+   if (on)
+   udc-udc_command(PXA2XX_UDC_CMD_CONNECT);
+   else
+   udc-udc_command(PXA2XX_UDC_CMD_DISCONNECT);
}
udc-pullup_on = on;
 }
@@ -1609,7 +1605,7 @@ static int pxa_udc_pullup(struct usb_gadget *_gadget, int 
is_active)
 {
struct pxa_udc *udc = to_gadget_udc(_gadget);
 
-   if (!gpio_is_valid(udc-mach-gpio_pullup)  !udc-mach-udc_command)
+   if (!udc-gpiod  !udc-udc_command)
return -EOPNOTSUPP;
 
dplus_pullup(udc, is_active);
@@ -2415,7 +2411,13 @@ static int pxa_udc_probe(struct platform_device *pdev)
 {
struct resource *regs;
struct pxa_udc *udc = memory;
-   int retval = 0, gpio;
+   struct pxa2xx_udc_mach_info *mach = dev_get_platdata(pdev-dev);
+   int retval = 0;
+
+   if (mach) {
+   udc-gpiod = gpio_to_desc(mach-gpio_pullup);
+   udc-udc_command = mach-udc_command;
+   }
 
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs)
@@ -2425,21 +2427,15 @@ static int pxa_udc_probe(struct platform_device *pdev)
return udc-irq;
 
udc-dev = pdev-dev;
-   udc-mach = dev_get_platdata(pdev-dev);
udc-transceiver = usb_get_phy(USB_PHY_TYPE_USB2);
 
-   gpio = udc-mach-gpio_pullup;
-   if (gpio_is_valid(gpio)) {
-   retval = gpio_request(gpio, USB D+ pullup);
-   if (retval == 0)
-   gpio_direction_output(gpio,
-  udc-mach-gpio_pullup_inverted);
-   }
-   if (retval) {
-   dev_err(pdev-dev, Couldn't request gpio %d : %d\n,
-   gpio, retval);
-   return retval;
+   if (IS_ERR(udc-gpiod)) {
+   dev_err(pdev-dev, Couldn't find or request D+ gpio : %ld\n,
+   PTR_ERR(udc-gpiod));
+   return PTR_ERR(udc-gpiod);
}
+   if (udc-gpiod)
+   gpiod_direction_output(udc-gpiod, 0);
 
udc-clk = clk_get(pdev-dev, NULL);
if (IS_ERR(udc-clk)) {
@@ -2501,14 +2497,11 @@ err_clk:
 static int pxa_udc_remove(struct platform_device *_dev)
 {
struct pxa_udc *udc = platform_get_drvdata(_dev);
-   int gpio = udc-mach-gpio_pullup;
 
usb_del_gadget_udc(udc-gadget);
usb_gadget_unregister_driver(udc-driver);
free_irq(udc-irq, udc);
pxa_cleanup_debugfs(udc);
-   if (gpio_is_valid(gpio))
-   gpio_free(gpio);
 
usb_put_phy(udc-transceiver);
 
diff --git 

[PATCH v6 2/2] usb: gadget: pxa27x_udc: add devicetree support

2014-09-24 Thread Robert Jarzmik
Add support for device-tree device discovery. If devicetree is not
provided, fallback to legacy platform data discovery.

Signed-off-by: Robert Jarzmik robert.jarz...@free.fr
Cc: devicet...@vger.kernel.org

---
Since V1: change OF id mrvl,pxa27x_udc - marvell,pxa27x-udc
  This is a consequence of other DT reviews on the marvell
  namings.
Since V2: address Mark's comments:
- wildcard pxa27x becomes pxa270
- pullup gpio is described as standard dt gpio
- bool XXX_probe_dt becomes int XXX_probe_dt
Since v5: split out into 2 patches, this one being the true device-tree port
---
 drivers/usb/gadget/udc/pxa27x_udc.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c 
b/drivers/usb/gadget/udc/pxa27x_udc.c
index e11f9c5..8961530 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -27,6 +27,8 @@
 #include linux/prefetch.h
 #include linux/byteorder/generic.h
 #include linux/platform_data/pxa2xx_udc.h
+#include linux/of_device.h
+#include linux/of_gpio.h
 
 #include linux/usb.h
 #include linux/usb/ch9.h
@@ -2400,6 +2402,12 @@ static struct pxa_udc memory = {
}
 };
 
+static struct of_device_id udc_pxa_dt_ids[] = {
+   { .compatible = marvell,pxa270-udc },
+   {}
+};
+MODULE_DEVICE_TABLE(of, udc_pxa_dt_ids);
+
 /**
  * pxa_udc_probe - probes the udc device
  * @_dev: platform device
@@ -2417,6 +2425,8 @@ static int pxa_udc_probe(struct platform_device *pdev)
if (mach) {
udc-gpiod = gpio_to_desc(mach-gpio_pullup);
udc-udc_command = mach-udc_command;
+   } else {
+   udc-gpiod = devm_gpiod_get(pdev-dev, NULL);
}
 
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -2608,6 +2618,7 @@ static struct platform_driver udc_driver = {
.driver = {
.name   = pxa27x-udc,
.owner  = THIS_MODULE,
+   .of_match_table = of_match_ptr(udc_pxa_dt_ids),
},
.probe  = pxa_udc_probe,
.remove = pxa_udc_remove,
-- 
2.0.0.rc2

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


Re: g_mass_storage bug ?

2014-09-24 Thread Alan Stern
On Wed, 24 Sep 2014, Felipe Balbi wrote:

  alright fixed. Below you can see a combined diff which I'll still split
  into patches so they can be applied properly.
 
 OTOH, there's really no way to split it. It's all needed to fix a single
 bug. Do you want me to add Reported-by: Alan Stern ?

What we really need is a Diagnosed-by: tag.  :-)

I'll settle for Suggested-by:.

Alan Stern

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


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 03:48:29PM -0400, Alan Stern wrote:
 On Wed, 24 Sep 2014, Felipe Balbi wrote:
 
   alright fixed. Below you can see a combined diff which I'll still split
   into patches so they can be applied properly.
  
  OTOH, there's really no way to split it. It's all needed to fix a single
  bug. Do you want me to add Reported-by: Alan Stern ?
 
 What we really need is a Diagnosed-by: tag.  :-)

heh, that's right.

 I'll settle for Suggested-by:.

alright, I'll add that :-)

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v6 1/2] usb: gadget: pxa27x_udc: prepare device-tree support

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 09:41:12PM +0200, Robert Jarzmik wrote:
 For this preparation, a preliminary cleanup is done :
   - convert the probing of pxa27x_udc to gpio_desc.
 The conversion is partial because :
  - the platform data still provides a gpio number, not a gpio desc
  - the invert attribute is lost, hence a loss in the translation

I asked for gpio to gpiod conversion to be a separate patch. It'll make
things a lot easier to review.

   - convert the mach info, and store the udc_command in the pxa_udc
 control structure
   - loose the udc_is_connected() in mach info
 This was not really used, as mioa701 machine doesn't need it,
 balloon3 doesn't really use it, and most importantly the current
 driver never uses it.
 
 The drawback with the gpio_desc conversion is that the inverted gpio
 attribute is lost, as no gpiod_*() function exists to set the

it's not lost, it's handled for you by the gpio library. Look at how
GPIO_ACTIVE_LOW is used.

-- 
balbi


signature.asc
Description: Digital signature


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 02:40:12PM -0500, Felipe Balbi wrote:
 On Wed, Sep 24, 2014 at 03:31:13PM -0400, Alan Stern wrote:
  On Wed, 24 Sep 2014, Felipe Balbi wrote:
  
 According to the kerneldoc for usb_ep_set_halt:
 
  * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any
  * transfer requests are still queued, or if the controller hardware
  * (usually a FIFO) still holds bytes that the host hasn't collected.

damn old bugs :-) I'll fix that up and Cc stable.
   
   alright fixed. Below you can see a combined diff which I'll still split
   into patches so they can be applied properly.
  
  And this eliminates the problems you saw with g_mass_storage?
 
 yup, working with and without stall=0, with and without debugging on. On
 all three systems I tested before ;-)

there is still one detail which I just caught and not sure if it's
something we should care. When I run my msc.sh/msc.c tests [1], after
each test I see a new sdX: unknown partition table. This doesn't
happen with my USB stick.

I'll fire up my sniffer again and see if I find anything peculiar.

[1] 
https://gitorious.org/usb/usb-tools/source/7eb7ef21de6cd124e0e0d0e7df9ddfff0e2f548e:msc.sh

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper

2014-09-24 Thread Rafael J. Wysocki
On Wednesday, September 24, 2014 09:44:50 PM Vinod Koul wrote:
 This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
 which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
 sequentially. Then we do a tree wide update of current patterns which are
 present. As evident from log below this pattern is frequent in the
 kernel.
 
 This series can be found at
 git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
 topic/pm_runtime_last_busy_and_autosuspend
 
 Fengguang's kbuild has tested it so it shouldn't break things for anyone.
 Barring one patch (explictyly mentioned in its changelog) rest are simple
 replacements.
 
 If all are okay, this should be merged thru PM tree as it depends on macro
 addition.
 
 Subhransu S. Prusty (1):
   PM: Add helper pm_runtime_last_busy_and_autosuspend()
 
 Vinod Koul (26):
   dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
   extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
   drm/i915: use pm_runtime_last_busy_and_autosuspend helper
   drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
   drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
   vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
   i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
   i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
   i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
   mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
   mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
   mei: use pm_runtime_last_busy_and_autosuspend helper
   mmc: use pm_runtime_last_busy_and_autosuspend helper
   mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
   mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
   mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
   mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
   NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
   pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
   spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
   spi: orion: use pm_runtime_last_busy_and_autosuspend helper
   spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
   spi: core: use pm_runtime_last_busy_and_autosuspend helper
   tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
   usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
   video: fbdev: use pm_runtime_last_busy_and_autosuspend helper
 
  Documentation/power/runtime_pm.txt  |4 ++
  drivers/dma/ste_dma40.c |   30 -
  drivers/extcon/extcon-arizona.c |6 +--
  drivers/gpu/drm/i915/intel_pm.c |3 +-
  drivers/gpu/drm/nouveau/nouveau_connector.c |3 +-
  drivers/gpu/drm/nouveau/nouveau_drm.c   |9 +---
  drivers/gpu/drm/radeon/radeon_connectors.c  |   15 ++
  drivers/gpu/drm/radeon/radeon_drv.c |5 +-
  drivers/gpu/drm/radeon/radeon_kms.c |6 +--
  drivers/gpu/vga/vga_switcheroo.c|7 +--
  drivers/i2c/busses/i2c-designware-core.c|3 +-
  drivers/i2c/busses/i2c-omap.c   |6 +--
  drivers/i2c/busses/i2c-qup.c|3 +-
  drivers/mfd/ab8500-gpadc.c  |6 +--
  drivers/mfd/arizona-irq.c   |3 +-
  drivers/misc/mei/client.c   |   12 ++
  drivers/mmc/core/core.c |3 +-
  drivers/mmc/host/mmci.c |   12 ++
  drivers/mmc/host/omap_hsmmc.c   |   19 ++---
  drivers/mmc/host/sdhci-pxav3.c  |6 +--
  drivers/mmc/host/sdhci.c|3 +-
  drivers/nfc/trf7970a.c  |3 +-
  drivers/power/pm2301_charger.c  |3 +-
  drivers/spi/spi-omap2-mcspi.c   |9 +---
  drivers/spi/spi-orion.c |3 +-
  drivers/spi/spi-ti-qspi.c   |5 +-
  drivers/spi/spi.c   |6 +--
  drivers/tty/serial/omap-serial.c|   60 
 +--
  drivers/usb/musb/omap2430.c |6 +--
  drivers/video/fbdev/auo_k190x.c |9 +---
  include/linux/pm_runtime.h  |6 +++
  31 files changed, 97 insertions(+), 177 deletions(-)

OK, I guess this is as good as it gets.

What tree would you like it go through?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: g_mass_storage bug ?

2014-09-24 Thread Felipe Balbi
On Wed, Sep 24, 2014 at 03:06:15PM -0500, Felipe Balbi wrote:
 On Wed, Sep 24, 2014 at 02:40:12PM -0500, Felipe Balbi wrote:
  On Wed, Sep 24, 2014 at 03:31:13PM -0400, Alan Stern wrote:
   On Wed, 24 Sep 2014, Felipe Balbi wrote:
   
  According to the kerneldoc for usb_ep_set_halt:
  
   * Attempts to halt IN endpoints will fail (returning -EAGAIN) if 
  any
   * transfer requests are still queued, or if the controller hardware
   * (usually a FIFO) still holds bytes that the host hasn't 
  collected.
 
 damn old bugs :-) I'll fix that up and Cc stable.

alright fixed. Below you can see a combined diff which I'll still split
into patches so they can be applied properly.
   
   And this eliminates the problems you saw with g_mass_storage?
  
  yup, working with and without stall=0, with and without debugging on. On
  all three systems I tested before ;-)
 
 there is still one detail which I just caught and not sure if it's
 something we should care. When I run my msc.sh/msc.c tests [1], after
 each test I see a new sdX: unknown partition table. This doesn't
 happen with my USB stick.

it happens with the USB stick after I destroy the partition table. So I
guess that's normal.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v6 1/2] usb: gadget: pxa27x_udc: prepare device-tree support

2014-09-24 Thread Robert Jarzmik
Felipe Balbi ba...@ti.com writes:

 On Wed, Sep 24, 2014 at 09:41:12PM +0200, Robert Jarzmik wrote:
 For this preparation, a preliminary cleanup is done :
   - convert the probing of pxa27x_udc to gpio_desc.
 The conversion is partial because :
  - the platform data still provides a gpio number, not a gpio desc
  - the invert attribute is lost, hence a loss in the translation

 I asked for gpio to gpiod conversion to be a separate patch. It'll make
 things a lot easier to review.

What does patch 1/2 do then ? There are 2 lines for udc_command (1 in .h and
1.c), and all the remaining is the gpiod conversion.

Is it difficult for you to review a 51 lines changed patch you asked for ? Do
you want me to ask other people to help you ?

   - convert the mach info, and store the udc_command in the pxa_udc
 control structure
   - loose the udc_is_connected() in mach info
 This was not really used, as mioa701 machine doesn't need it,
 balloon3 doesn't really use it, and most importantly the current
 driver never uses it.
 
 The drawback with the gpio_desc conversion is that the inverted gpio
 attribute is lost, as no gpiod_*() function exists to set the

 it's not lost, it's handled for you by the gpio library. Look at how
 GPIO_ACTIVE_LOW is used.
It is so, the above assertion no gpiod_*() function exists is
false. Therefore, which is the right function in the gpio library accessible to
a driver ?

Cheers.

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


  1   2   >