Re: [Libusbx-devel] Linux sysfs usb descriptors file has broken configuration length handling

2013-05-19 Thread Hans de Goede 

Hi, Greg, Alan, All,

On 05/18/2013 06:17 PM, Greg KH wrote:

On Sat, May 18, 2013 at 12:14:08PM -0400, Alan Stern wrote:

On Sat, 18 May 2013, Hans de Goede wrote:

But the sysfs descriptors file will just packs the
rawdescriptors one behind the other, using
usb_device-config[x].desc.wTotalLength, where as
userspace only sees the length advertised by
the rawdescriptors, which may be different, and when
it is userspace will this have no idea where the next
descriptor starts.

I believe the proper way to fix this is to make the
sysfs code deal with this the same way the usbfs code
does (filling the holes with 0 to avoid leaking kmem),
if people agree I can write a patch for this.


That's okay with me.


Sounds good to me as well.


Thanks for the input. I've been thinking about this some-more,
and I see a possible other solution which I think we need to
consider.

Although the config.wTotalLength value in /sys/.../descriptors
can not be trusted, the kernel does sanitize things to such a
degree that the standard descriptor header bLength can be
trusted to always be valid inside /sys/.../descriptors, so
alternatively to using config.wTotalLength userspace could
simple parse things one descriptor at a time, until it
encounters another config descriptor or EOF, and reliable figure
out where the next config descriptor starts that way.

I'm sort of tending towards using that solution instead

Advantages:

1) If libusb is modified to do thing this way it will work with
older kernels, without needing to switch to using usbfs (which
in some rough benchmarks I've run turns out to be 7 times as slow
when it comes to open file, read desc, close file).

2) We're not breaking the kernel ABI, which technically my
other proposal does. I know devices with multiple configs are
rare, and one could argue the old behavior is a bug, but we
may have something out there depending on it.

Disadvantages:

1) It means that there will be some inconsistencies to how this
is handled between usbfs and sysfs (note there already is such
an inconsistency with regards to the endian-ness of the device
descriptor).

As said I tend towards using the alternative proposal I've
just suggested, input very much welcome!

Either way I noticed that descriptors is absent from
Documentation/ABI/testing/sysfs-bus-usb

Once we've a decision on which way to go, I'll document the
behavior there.


Regards,

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

Re: Question about generic HID devices

2013-05-19 Thread Alan Stern 
On Sat, 18 May 2013, Daniel Santos wrote:

 Yes, my apologies, I didn't state that very clearly.  I meant that I 
 submit a request URB to the in interrupt endpoint as well a response 
 URB to the out interrupt endpoint.This is where I'm currently submitting 
 these (for now just assume can_sleep is always zero, it's broken otherwise):
 
 static int ctl_submit_req(struct mcp2210_command *cmd, int can_sleep)
 {
  struct mcp2210_ctl_command *ctl_cmd = (struct mcp2210_ctl_command 
 *)cmd;
  struct mcp2210_device *dev;
  const gfp_t gfp_flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
  unsigned long irqflags;
  int ret = 0;
 
  if (!cmd || !cmd-dev || cmd-type != mcp2210_cmd_type_ctrl)
  return -EINVAL;
 
  dev = cmd-dev;
  memcpy(dev-in.buffer, ctl_cmd-req, sizeof(ctl_cmd-req));
 
  dev-in.int_urb-complete = cmd-type-complete_req;
  dev-out.int_urb-complete = cmd-type-complete_rep;
 
  /* lock command in case we get preempted here and a completion handler
   * is called */
  /* FIXME: this makes no sense when can_sleep != 0 */
  spin_lock_irqsave(cmd-spinlock, irqflags);
 
  ret = usb_submit_urb(dev-in.int_urb, gfp_flags);
  if (ret) {
  dev_err(dev-udev-dev,
  usb_submit_urb failed for request URB %d, ret);
  goto exit_unlock;
  }
 
  ret = usb_submit_urb(dev-out.int_urb, gfp_flags);
  if (ret) {
  dev_err(dev-udev-dev,
  usb_submit_urb failed for response URB %d, ret);
  usb_kill_urb(dev-in.int_urb);

Note that this call won't work if you are holding a spinlock.

  goto exit_unlock;
  }
 
  cmd-state = MCP2210_CMD_STATE_SUBMITTED;
 
 exit_unlock:
  spin_unlock_irqrestore(cmd-spinlock, irqflags);
 
  return ret;
 }
 
  But that doesn't make much sense either; it would require an extremely
  unusual HID device to send 64-byte responses.  For example, a mouse or
  a keyboard generally sends responses in the 3- to 5-byte range.
 
 Yes, I know.  This isn't even a device which interfaces with humans!  I 
 presume Microchip implemented it as such so that it could be interfaced 
 from userspace with the generic HID drivers available on all modern 
 platforms.  I think I understand their design decision, but from my 
 standpoint, it seems introduce a lot of needless complexity.  Section 
 3.0 (page 11) of the datasheet 
 (http://ww1.microchip.com/downloads/en/DeviceDoc/22288A.pdf) describes 
 the USB command/response pairs it supports and all of them are 64 bytes, 
 most padded with lots of zeros.  Then again, it's a $2.10 IC.

Using interrupt transfers for commands and responses is another bad
design.  No doubt it flows from the decision to support the HID
interface.

 I guess this is the reason I've shied away from Mathew King's basic 
 alpha driver (https://github.com/MathewKing/mcp2210-linux), with 64 byte 
 messages.  For some reason, I originally thought that it was allocated 
 6k of memory for each HID report, 
 (https://github.com/MathewKing/mcp2210-linux/blob/master/mcp2210-core.c#L102) 
 but after actually testing it (on x86_64), it's only 2456 bytes.
 
 I guess this introduces a new (maybe better) question.  If I instead 
 convert this back into a proper usbhid driver (instead of a plane USB 
 interface driver as it is now), can I allocate my struct hid_report and 
 struct hid_field just once and reuse them instead of allocating new ones 
 for each command/response pair? The original point of doing this as a 
 USB interface driver was to reduce memory requirements.

I don't know enough about the HID stack to answer this question.  
However, I suspect that the HID stack won't be a very good match to 
something which isn't really an HID device.

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: [Libusbx-devel] Linux sysfs usb descriptors file has broken configuration length handling

2013-05-19 Thread Alan Stern 
On Sun, 19 May 2013, Hans de Goede wrote:

 Hi, Greg, Alan, All,
 
 On 05/18/2013 06:17 PM, Greg KH wrote:
  On Sat, May 18, 2013 at 12:14:08PM -0400, Alan Stern wrote:
  On Sat, 18 May 2013, Hans de Goede wrote:
  But the sysfs descriptors file will just packs the
  rawdescriptors one behind the other, using
  usb_device-config[x].desc.wTotalLength, where as
  userspace only sees the length advertised by
  the rawdescriptors, which may be different, and when
  it is userspace will this have no idea where the next
  descriptor starts.
 
  I believe the proper way to fix this is to make the
  sysfs code deal with this the same way the usbfs code
  does (filling the holes with 0 to avoid leaking kmem),
  if people agree I can write a patch for this.
 
  That's okay with me.
 
  Sounds good to me as well.
 
 Thanks for the input. I've been thinking about this some-more,
 and I see a possible other solution which I think we need to
 consider.
 
 Although the config.wTotalLength value in /sys/.../descriptors
 can not be trusted, the kernel does sanitize things to such a
 degree that the standard descriptor header bLength can be
 trusted to always be valid inside /sys/.../descriptors, so
 alternatively to using config.wTotalLength userspace could
 simple parse things one descriptor at a time, until it
 encounters another config descriptor or EOF, and reliable figure
 out where the next config descriptor starts that way.
 
 I'm sort of tending towards using that solution instead
 
 Advantages:
 
 1) If libusb is modified to do thing this way it will work with
 older kernels, without needing to switch to using usbfs (which
 in some rough benchmarks I've run turns out to be 7 times as slow
 when it comes to open file, read desc, close file).

Furthermore, usbfs is not guaranteed to be present in all systems, 
whereas sysfs pretty much is.

 2) We're not breaking the kernel ABI, which technically my
 other proposal does. I know devices with multiple configs are
 rare, and one could argue the old behavior is a bug, but we
 may have something out there depending on it.

That's a very good point.  In fact, I'd say it trumps the other 
considerations.

 Disadvantages:
 
 1) It means that there will be some inconsistencies to how this
 is handled between usbfs and sysfs (note there already is such
 an inconsistency with regards to the endian-ness of the device
 descriptor).

In light of the existing inconsistency, this doesn't bother me.  
People shouldn't be using the usbfs interface for cached descriptors,
if they can possibly avoid it.

 As said I tend towards using the alternative proposal I've
 just suggested, input very much welcome!
 
 Either way I noticed that descriptors is absent from
 Documentation/ABI/testing/sysfs-bus-usb
 
 Once we've a decision on which way to go, I'll document the
 behavior there.

I'd say keep it the way it is and add the Documentation file.  If I 
were designing it from scratch, I might add a true length field just 
before the start of each descriptor.  But that's not an option now.

By the way, Greg, at what point does it make sense to move things from 
Documenation/ABI/testing to Documentation/ABI/stable?  I get the 
impression that nothing ever makes this jump.

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: Regression in 3735ba8db8e6ea22ad3ff524328926d8d780a884 with Freescale P1020

2013-05-19 Thread Michael Braun 
Hi,

I've got hardware here to test with, so if there any changes to test, I'm 
willing to support.
Meanwhile, might it be a good idea to make that check optional - i.e. add a 
module parameter or something like this around it?

Regards,
 M. Braun

On Thu, Apr 18, 2013 at 05:13:39PM +0200, michael-dev wrote:
 Hi,
 
 thanks for the quick reply.
 
 Please review the patch http://patchwork.ozlabs.org/patch/237201/
 I applied it, but it does not make any difference on my platform.
 
 Regards,
  M. Braun
 
 Am 17.04.2013 12:53, schrieb Liu Shengzhou-B36685:
 Hi Braun,
 
 It seems the duplicated tdi_reset caused the PHY_CLK_VALID bit
 unstable,
 introduced by patch EHCI: centralize controller initialization.
 I submitted a patch to fix it.
 Please review the patch http://patchwork.ozlabs.org/patch/237201/
 
 Regards,
 Shengzhou
 
 
 -Original Message-
 From: Michael Braun [mailto:michael-...@fami-braun.de]
 Sent: Wednesday, April 17, 2013 6:08 PM
 To: Liu Shengzhou-B36685
 Cc: Alan Stern; projekt-w...@fem.tu-ilmenau.de; Greg Kroah-Hartman;
 linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org
 Subject: Regression in 3735ba8db8e6ea22ad3ff524328926d8d780a884 with
 Freescale P1020
 
 Hi,
 
 I'm running OpenWRT Kernel 3.8.3 (which already has
 f66dea709cd9309b2ee9f715697818001fb518de and
 5ed338778f917a035f0f0a52327fc4f72e36f7a1 applied) on a P1020WLAN
 (QorlQ,
 PPC) device.
 Before updating the kernel from 3.3.0, USB host support was
 working fine.
 Now I get fsl-ehci: USB PHY clock invalid messages in dmesg and the
 lsusb output is empty, so USB host support is not working. When
 I apply
 the following patch, USB host support starts working again, so I
 guess
 3735ba8db8e6ea22ad3ff524328926d8d780a884 is the cause.
 Do you have an idea how to fix it more appropriately?
 
 Thanks,
   M. Braun
 
 --- a/drivers/usb/host/ehci-fsl.c   2013-04-15 21:13:52.924403077
 +0200
 +++ b/drivers/usb/host/ehci-fsl.c   2013-04-15 21:13:57.572410838
 +0200
 @@ -273,7 +273,6 @@ static int ehci_fsl_setup_phy(struct usb
  if (!spin_event_timeout(in_be32(non_ehci +
 FSL_SOC_USB_CTRL) 
  PHY_CLK_VALID,
 FSL_USB_PHY_CLK_TIMEOUT,
 0)) {
  printk(KERN_WARNING fsl-ehci: USB PHY clock
 invalid\n);
 -   return -EINVAL;
  }
  }
 
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Libusbx-devel] Linux sysfs usb descriptors file has broken configuration length handling

2013-05-19 Thread Greg KH 
On Sun, May 19, 2013 at 11:20:59AM -0400, Alan Stern wrote:
 By the way, Greg, at what point does it make sense to move things from 
 Documenation/ABI/testing to Documentation/ABI/stable?  I get the 
 impression that nothing ever makes this jump.

One people start relying on the file, it should be moved to stable, but
you are right, almost no one ever does.

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

[patch] USB: cxacru: potential underflow in cxacru_cm_get_array()

2013-05-19 Thread Dan Carpenter 
The value of offd comes off the instance-rcv_buf[] and we used it as
the offset into an array.  The problem is that we check the upper bound
but not for negative values.

Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index b7eb86a..8a7eb77 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -686,7 +686,8 @@ static int cxacru_cm_get_array(struct cxacru_data 
*instance, enum cxacru_cm_requ
 {
int ret, len;
__le32 *buf;
-   int offb, offd;
+   int offb;
+   unsigned int offd;
const int stride = CMD_PACKET_SIZE / (4 * 2) - 1;
int buflen =  ((size - 1) / stride + 1 + size * 2) * 4;
 
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] usb: dwc3: pci: PHY should be deleted later than dwc3 core

2013-05-19 Thread Peter Chen 
If the glue layer is removed first (core layer later),
it deletes the phy device first, then the core device.
But at core's removal, it still uses PHY's resources, it may
cause kernel's oops. It is much like the problem
Paul Zimmerman reported at:
http://marc.info/?l=linux-usbm=136547502011472w=2.

Besides, it is reasonable the PHY is deleted at last as
the controller is the PHY's user.

Signed-off-by: Peter Chen peter.c...@freescale.com
---
 drivers/usb/dwc3/dwc3-pci.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 227d4a7..eba9e2b 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -196,9 +196,9 @@ static void dwc3_pci_remove(struct pci_dev *pci)
 {
struct dwc3_pci *glue = pci_get_drvdata(pci);
 
+   platform_device_unregister(glue-dwc3);
platform_device_unregister(glue-usb2_phy);
platform_device_unregister(glue-usb3_phy);
-   platform_device_unregister(glue-dwc3);
pci_set_drvdata(pci, NULL);
pci_disable_device(pci);
 }
-- 
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

[PATCH 2/2] usb: dwc3: exynos: PHY should be deleted later than dwc3 core

2013-05-19 Thread Peter Chen 
If the glue layer is removed first (core layer later),
it deletes the phy device first, then the core device.
But at core's removal, it still uses PHY's resources, it may
cause kernel's oops. It is much like the problem
Paul Zimmerman reported at:
http://marc.info/?l=linux-usbm=136547502011472w=2.

Besides, it is reasonable the PHY is deleted at last as
the controller is the PHY's user.

Signed-off-by: Peter Chen peter.c...@freescale.com
---
 drivers/usb/dwc3/dwc3-exynos.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index a8afe6e..df6bd5c 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -164,9 +164,9 @@ static int dwc3_exynos_remove(struct platform_device *pdev)
 {
struct dwc3_exynos  *exynos = platform_get_drvdata(pdev);
 
+   device_for_each_child(pdev-dev, NULL, dwc3_exynos_remove_child);
platform_device_unregister(exynos-usb2_phy);
platform_device_unregister(exynos-usb3_phy);
-   device_for_each_child(pdev-dev, NULL, dwc3_exynos_remove_child);
 
clk_disable_unprepare(exynos-clk);
 
-- 
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: Crash in dwc3 driver on module unload

2013-05-19 Thread Peter Chen 
On Fri, May 17, 2013 at 10:27 PM, Alan Stern st...@rowland.harvard.edu wrote:
 On Fri, 17 May 2013, Peter Chen wrote:

  I found out a couple more things about this.
 
  If I do rmmod dwc3 first, and then do rmmod dwc3-pci, it doesn't
  crash.
 

 dwc3-pci will release dwc3 and its resources at its removal.
 So, dwc3 core must be released first, then glue layer.

 That makes no sense.  Obviously dwc3 can't be removed until all its
 resources have been released.  And you said that dwc3-pci doesn't
 release the dwc3 resources until dwc3-pci has been removed.

 Therefore dwc3 can't be removed until dwc3-pci has been removed.  This
 is exactly the opposite of what you wrote above.


After checking the code more, the things should like below:

At dwc3 core's removal:
602 static int dwc3_remove(struct platform_device *pdev)
603 {
604 struct dwc3 *dwc = platform_get_drvdata(pdev);
605
606 usb_phy_set_suspend(dwc-usb2_phy, 1);
607 usb_phy_set_suspend(dwc-usb3_phy, 1);
608
609 pm_runtime_put(pdev-dev);
610 pm_runtime_disable(pdev-dev);

At dwc3-pci's removal:

195 static void dwc3_pci_remove(struct pci_dev *pci)
196 {
197 struct dwc3_pci *glue = pci_get_drvdata(pci);
198
199 platform_device_unregister(glue-usb2_phy);
200 platform_device_unregister(glue-usb3_phy);
201 platform_device_unregister(glue-dwc3);
202 pci_set_drvdata(pci, NULL);
203 pci_disable_device(pci);
204 }

I am afraid I did not mention platform_device_unregister(glue-dwc3)
will call dwc3 core's removal.

If dwc3 core is removed first, then dwc3-pci. It is ok.
If dwc3-pci is removed first, the PHY's device will be removed,
when platform_device_unregister(glue-dwc3) -dwc3_remove
-usb_phy_set_suspend, the oops will be occurred.
I think it was the problem Paul met.


 Since there are no relationship between core and glue
 layer, If user unload module in incorrect order, the oops
 may occur, the phy driver is the same situation.

 For such kinds of case, do we need to fix at kernel layer?
 or using user space method to fix?

 There _should_ be a relationship between the core and the glue layer.
 The glue layer uses the core's resources; therefore the glue layer
 should contain references to symbols that are defined in the core.
 This will cause the kernel to prevent the core from being removed
 before the glue layer.

Yes, that is the hcd/gadget driver do.

 PHY drivers will need different treatment.  I suspect that the right
 approach is to increment the phy driver's module count each time an HCD
 claims a PHY.


I think it is ok, one more thing is we need to return -EPROBE_DEFER
if PHY driver does not load before the HCD.


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

Re: [PATCH 1/5] usb: add Atmel USBA UDC DT support

2013-05-19 Thread Bo Shen 

Hi Jean-Christophe,

On 5/17/2013 22:42, Jean-Christophe PLAGNIOL-VILLARD wrote:

Allow to compile the driver all the time if AT91 enabled.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
Cc: Nicolas Ferre nicolas.fe...@atmel.com
Cc: linux-usb@vger.kernel.org
---
  .../devicetree/bindings/usb/atmel-usb.txt  |   82 
  drivers/usb/gadget/Kconfig |2 +-
  drivers/usb/gadget/atmel_usba_udc.c|  214 +++-
  drivers/usb/gadget/atmel_usba_udc.h|1 +
  4 files changed, 244 insertions(+), 55 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/atmel-usb.txt 
b/Documentation/devicetree/bindings/usb/atmel-usb.txt
index 60bd215..878556b2 100644
--- a/Documentation/devicetree/bindings/usb/atmel-usb.txt
+++ b/Documentation/devicetree/bindings/usb/atmel-usb.txt
@@ -47,3 +47,85 @@ usb1: gadget@fffa4000 {
interrupts = 10 4;
atmel,vbus-gpio = pioC 5 0;
  };
+
+Atmel High-Speed USB device controller
+
+Required properties:
+ - compatible: Should be atmel,at91sam9rl-udc
+ - reg: Address and length of the register set for the device
+ - interrupts: Should contain macb interrupt


s/macb/udphs


+ - ep childnode: To specifiy the number of endpoints and their properties.


s/specifiy/specify


+
+Optional properties:
+ - atmel,vbus-gpio: If present, specifies a gpio that needs to be
+   activated for the bus to be powered.
+
+Required child node properties:
+ - name: Name of the endpoint.
+ - reg: Num of the endpoint.
+ - atmel,fifo-size: Size of the fifo.
+ - atmel,nb-banks: Number of banks.
+ - atmel,can-dma: Boolean to specify if the endpoint support DMA.
+ - atmel,can-isoc: Boolean to specify if the endpoint support ISOC.
+
+usb2: gadget at fff78000 {
+   #address-cells = 1;
+   #size-cells = 0;
+   compatible = atmel,at91sam9rl-udc;
+   reg = 0x0060 0x8
+  0xfff78000 0x400;
+   interrupts = 27 4;


s/interrupts = 27 4/interrupts = 27 IRQ_TYPE_LEVEL_HIGH 0;


+   atmel,vbus-gpio = pioB 19 0;
+
+   ep0 {
+   reg = 0;
+   atmel,fifo-size = 64;
+   atmel,nb-banks = 1;
+   };
+
+   ep1 {
+   reg = 1;
+   atmel,fifo-size = 1024;
+   atmel,nb-banks = 2;
+   atmel,can-dma;
+   atmel,can-isoc;
+   };
+
+   ep2 {
+   reg = 2;
+   atmel,fifo-size = 1024;
+   atmel,nb-banks = 2;
+   atmel,can-dma;
+   atmel,can-isoc;
+   };
+
+   ep3 {
+   reg = 3;
+   atmel,fifo-size = 1024;
+   atmel,nb-banks = 3;
+   atmel,can-dma;
+   };
+
+   ep4 {
+   reg = 4;
+   atmel,fifo-size = 1024;
+   atmel,nb-banks = 3;
+   atmel,can-dma;
+   };
+
+   ep5 {
+   reg = 5;
+   atmel,fifo-size = 1024;
+   atmel,nb-banks = 3;
+   atmel,can-dma;
+   atmel,can-isoc;
+   };
+
+   ep6 {
+   reg = 6;
+   atmel,fifo-size = 1024;
+   atmel,nb-banks = 3;
+   atmel,can-dma;
+   atmel,can-isoc;
+   };
+};
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 83300d9..5e47d50 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -156,7 +156,7 @@ config USB_LPC32XX

  config USB_ATMEL_USBA
tristate Atmel USBA
-   depends on AVR32 || ARCH_AT91SAM9RL || ARCH_AT91SAM9G45
+   depends on AVR32 || ARCH_AT91
help
  USBA is the integrated high-speed USB Device controller on
  the AT32AP700x, some AT91SAM9 and AT91CAP9 processors from Atmel.
diff --git a/drivers/usb/gadget/atmel_usba_udc.c 
b/drivers/usb/gadget/atmel_usba_udc.c
index f2a970f..b3084b9 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -22,6 +22,8 @@
  #include linux/usb/atmel_usba_udc.h
  #include linux/delay.h
  #include linux/platform_data/atmel.h
+#include linux/of.h
+#include linux/of_gpio.h

  #include asm/gpio.h

@@ -1835,9 +1837,143 @@ static int atmel_usba_stop(struct usb_gadget *gadget,
return 0;
  }

-static int __init usba_udc_probe(struct platform_device *pdev)
+#ifdef CONFIG_OF
+static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
+   struct usba_udc *udc)
+{
+   u32 val;
+   const char *name;
+   enum of_gpio_flags flags;
+   struct device_node *np = pdev-dev.of_node;
+   struct device_node *pp;
+   int i, ret;
+   struct usba_ep *eps, *ep;
+
+   udc-num_ep = 0;
+
+   udc-vbus_pin = of_get_named_gpio_flags(np, atmel,vbus-gpio, 0,
+   flags);
+   udc-vbus_pin_inverted = (flags  OF_GPIO_ACTIVE_LOW) ? 1 : 0;
+
+

RE: Regression in 3735ba8db8e6ea22ad3ff524328926d8d780a884 with Freescale P1020

2013-05-19 Thread Liu Shengzhou-B36685 
Hi Braun,

At that time I had an P4080DS board which had the same issue and had been fixed 
with this patch.
I didn't test it on P1020 due to the absence of P1020. I think P1020 will need 
a new patch besides this one.
Later Ramneek took this issue on P1020 for more investigation.

Hello Ramneek, any update for the PHY_CLK_VALID issue?

Regards,
Shengzhou


 -Original Message-
 From: Michael Braun [mailto:michael.br...@fem.tu-ilmenau.de]
 Sent: Sunday, May 19, 2013 11:23 PM
 To: Liu Shengzhou-B36685
 Cc: projekt-w...@fem.tu-ilmenau.de; Greg Kroah-Hartman; linux-
 u...@vger.kernel.org; Alan Stern; linux-ker...@vger.kernel.org
 Subject: Re: Regression in 3735ba8db8e6ea22ad3ff524328926d8d780a884 with
 Freescale P1020
 
 Hi,
 
 I've got hardware here to test with, so if there any changes to test, I'm
 willing to support.
 Meanwhile, might it be a good idea to make that check optional - i.e. add
 a module parameter or something like this around it?
 
 Regards,
  M. Braun
 
 On Thu, Apr 18, 2013 at 05:13:39PM +0200, michael-dev wrote:
  Hi,
 
  thanks for the quick reply.
 
  Please review the patch http://patchwork.ozlabs.org/patch/237201/
  I applied it, but it does not make any difference on my platform.
 
  Regards,
   M. Braun
 
  Am 17.04.2013 12:53, schrieb Liu Shengzhou-B36685:
  Hi Braun,
  
  It seems the duplicated tdi_reset caused the PHY_CLK_VALID bit
  unstable, introduced by patch EHCI: centralize controller
  initialization.
  I submitted a patch to fix it.
  Please review the patch http://patchwork.ozlabs.org/patch/237201/
  
  Regards,
  Shengzhou
  
  
  -Original Message-
  From: Michael Braun [mailto:michael-...@fami-braun.de]
  Sent: Wednesday, April 17, 2013 6:08 PM
  To: Liu Shengzhou-B36685
  Cc: Alan Stern; projekt-w...@fem.tu-ilmenau.de; Greg Kroah-Hartman;
  linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org
  Subject: Regression in 3735ba8db8e6ea22ad3ff524328926d8d780a884 with
  Freescale P1020
  
  Hi,
  
  I'm running OpenWRT Kernel 3.8.3 (which already has
  f66dea709cd9309b2ee9f715697818001fb518de and
  5ed338778f917a035f0f0a52327fc4f72e36f7a1 applied) on a P1020WLAN
  (QorlQ,
  PPC) device.
  Before updating the kernel from 3.3.0, USB host support was working
  fine.
  Now I get fsl-ehci: USB PHY clock invalid messages in dmesg and
  the lsusb output is empty, so USB host support is not working. When
  I apply the following patch, USB host support starts working again,
  so I guess
  3735ba8db8e6ea22ad3ff524328926d8d780a884 is the cause.
  Do you have an idea how to fix it more appropriately?
  
  Thanks,
M. Braun
  
  --- a/drivers/usb/host/ehci-fsl.c   2013-04-15 21:13:52.924403077
  +0200
  +++ b/drivers/usb/host/ehci-fsl.c   2013-04-15 21:13:57.572410838
  +0200
  @@ -273,7 +273,6 @@ static int ehci_fsl_setup_phy(struct usb
   if (!spin_event_timeout(in_be32(non_ehci +
  FSL_SOC_USB_CTRL) 
   PHY_CLK_VALID,
  FSL_USB_PHY_CLK_TIMEOUT,
  0)) {
   printk(KERN_WARNING fsl-ehci: USB PHY
  clock invalid\n);
  -   return -EINVAL;
   }
   }
  


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

RE: Regression in 3735ba8db8e6ea22ad3ff524328926d8d780a884 with Freescale P1020

2013-05-19 Thread Mehresh Ramneek-B31383 
Hi Shengzhou/Braun,

We changed the controller init sequence to make this work. I'll submit the 
patch upstream soon.

Regards,
Ramneek

-Original Message-
From: Liu Shengzhou-B36685 
Sent: Monday, May 20, 2013 9:07 AM
To: Michael Braun; Mehresh Ramneek-B31383
Cc: projekt-w...@fem.tu-ilmenau.de; Greg Kroah-Hartman; 
linux-usb@vger.kernel.org; Alan Stern; linux-ker...@vger.kernel.org
Subject: RE: Regression in 3735ba8db8e6ea22ad3ff524328926d8d780a884 with 
Freescale P1020

Hi Braun,

At that time I had an P4080DS board which had the same issue and had been fixed 
with this patch.
I didn't test it on P1020 due to the absence of P1020. I think P1020 will need 
a new patch besides this one.
Later Ramneek took this issue on P1020 for more investigation.

Hello Ramneek, any update for the PHY_CLK_VALID issue?

Regards,
Shengzhou


 -Original Message-
 From: Michael Braun [mailto:michael.br...@fem.tu-ilmenau.de]
 Sent: Sunday, May 19, 2013 11:23 PM
 To: Liu Shengzhou-B36685
 Cc: projekt-w...@fem.tu-ilmenau.de; Greg Kroah-Hartman; linux- 
 u...@vger.kernel.org; Alan Stern; linux-ker...@vger.kernel.org
 Subject: Re: Regression in 3735ba8db8e6ea22ad3ff524328926d8d780a884 
 with Freescale P1020
 
 Hi,
 
 I've got hardware here to test with, so if there any changes to test, 
 I'm willing to support.
 Meanwhile, might it be a good idea to make that check optional - i.e. 
 add a module parameter or something like this around it?
 
 Regards,
  M. Braun
 
 On Thu, Apr 18, 2013 at 05:13:39PM +0200, michael-dev wrote:
  Hi,
 
  thanks for the quick reply.
 
  Please review the patch http://patchwork.ozlabs.org/patch/237201/
  I applied it, but it does not make any difference on my platform.
 
  Regards,
   M. Braun
 
  Am 17.04.2013 12:53, schrieb Liu Shengzhou-B36685:
  Hi Braun,
  
  It seems the duplicated tdi_reset caused the PHY_CLK_VALID bit 
  unstable, introduced by patch EHCI: centralize controller 
  initialization.
  I submitted a patch to fix it.
  Please review the patch http://patchwork.ozlabs.org/patch/237201/
  
  Regards,
  Shengzhou
  
  
  -Original Message-
  From: Michael Braun [mailto:michael-...@fami-braun.de]
  Sent: Wednesday, April 17, 2013 6:08 PM
  To: Liu Shengzhou-B36685
  Cc: Alan Stern; projekt-w...@fem.tu-ilmenau.de; Greg 
  Kroah-Hartman; linux-usb@vger.kernel.org; 
  linux-ker...@vger.kernel.org
  Subject: Regression in 3735ba8db8e6ea22ad3ff524328926d8d780a884 
  with Freescale P1020
  
  Hi,
  
  I'm running OpenWRT Kernel 3.8.3 (which already has 
  f66dea709cd9309b2ee9f715697818001fb518de and
  5ed338778f917a035f0f0a52327fc4f72e36f7a1 applied) on a P1020WLAN 
  (QorlQ,
  PPC) device.
  Before updating the kernel from 3.3.0, USB host support was 
  working fine.
  Now I get fsl-ehci: USB PHY clock invalid messages in dmesg and 
  the lsusb output is empty, so USB host support is not working. 
  When I apply the following patch, USB host support starts working 
  again, so I guess
  3735ba8db8e6ea22ad3ff524328926d8d780a884 is the cause.
  Do you have an idea how to fix it more appropriately?
  
  Thanks,
M. Braun
  
  --- a/drivers/usb/host/ehci-fsl.c   2013-04-15 21:13:52.924403077
  +0200
  +++ b/drivers/usb/host/ehci-fsl.c   2013-04-15 21:13:57.572410838
  +0200
  @@ -273,7 +273,6 @@ static int ehci_fsl_setup_phy(struct usb
   if (!spin_event_timeout(in_be32(non_ehci +
  FSL_SOC_USB_CTRL) 
   PHY_CLK_VALID, 
  FSL_USB_PHY_CLK_TIMEOUT,
  0)) {
   printk(KERN_WARNING fsl-ehci: USB PHY 
  clock invalid\n);
  -   return -EINVAL;
   }
   }
  


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

usb:is that a bug?

2013-05-19 Thread linux fddl 
Hi,

When I used mass_storage as a gadget(use a linux-3.0.77 kernel),
After the following operations:

1.Turn my board into hibernation.
2.Plug the usb into host before resume.
3.Resume my board.

I saw some dump message like this, after some digging,
I think the other devices' retore time is too long to make the gadget
fail to response
the command(GET_MAX_LUN) from the host.
What I hope to know is:
  Is it a bug?
  or I need some operations to cooperate them?

[12254.335994] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[12254.353797] PM: freeze of devices complete after 18.095 msecs
[12254.354397] PM: late freeze of devices complete after 0.565 msecs
[12254.354416] Disabling non-boot CPUs ...
[12254.365973] CPU1: shutdown
[12254.367032] PM: Creating hibernation image:
[12254.934979] PM: Need to copy 77295 pages
[12249.081603] Enabling non-boot CPUs ...
[12249.092911] CPU1: Booted secondary processor
[12249.097392] Switched to NOHz mode on CPU #1
[12249.168129] CPU1 is up
[12249.168498] PM: early restore of devices complete after 0.335 msecs
[12249.399814] usb usb1: root hub lost power or was reset
[12249.400290] sd 0:0:0:0: [sda] Starting disk
[12250.976563] g_mass_storage gadget: super speed config #1: Linux
File-Backed Storage
[12251.007422] g_mass_storage gadget: common-fsg is NULL in fsg_setup at 614
[12251.007441] [ cut here ]
[12251.007504] WARNING: at
/work1/share/new/linux_aa9_fddl/drivers/usb/gadget/f_mass_storage.c:460
fsg_setup+0x50/0x118 [g_mass_storage]()
[12251.007524] Modules linked in: g_mass_storage f_usb30_udc [last
unloaded: g_mass_storage]
[12251.007551] Backtrace:
[12251.007616] [80031d78] (dump_backtrace+0x0/0x10c) from
[802c4358] (dump_stack+0x18/0x1c)
[12251.007634]  r6:7f18e8bf r5:01cc r4: r3:8038a000
[12251.007699] [802c4340] (dump_stack+0x0/0x1c) from [80049840]
(warn_slowpath_common+0x54/0x6c)
[12251.007735] [800497ec] (warn_slowpath_common+0x0/0x6c) from
[8004987c] (warn_slowpath_null+0x24/0x2c)
[12251.007752]  r8: r7:7f18cfa4 r6: r5:0001 r4:
[12251.007778] r3:0009
[12251.007814] [80049858] (warn_slowpath_null+0x0/0x2c) from
[7f189cac] (fsg_setup+0x50/0x118 [g_mass_storage])
[12251.007871] [7f189c5c] (fsg_setup+0x0/0x118 [g_mass_storage])
from [7f18db70] (composite_setup+0xbcc/0xce8 [g_mass_storage])
[12251.007891]  r6:8038be30 r5:f021ad60 r4:f09dc6c0 r3:7f189c5c
[12251.007985] [7f18cfa4] (composite_setup+0x0/0xce8
[g_mass_storage]) from [7f00547c]
(on_end_control_setup_transfer+0x17c/0x298 [f_usb30_udc])
[12251.008045] [7f005300] (on_end_control_setup_transfer+0x0/0x298
[f_usb30_udc]) from [7f00b6bc]
(on_usb_ss_function_controller+0x634/0x10b4 [f_usb30_udc])
[12251.008067]  r8:fc42 r7:0002 r6:f027754c r5:f027750c r4:f0277400
[12251.008138] [7f00b088] (on_usb_ss_function_controller+0x0/0x10b4
[f_usb30_udc]) from [80082a20] (handle_irq_event_percpu+0x34/0x160)
[12251.008175] [800829ec] (handle_irq_event_percpu+0x0/0x160) from
[80082b90] (handle_irq_event+0x44/0x64)
[12251.008214] [80082b4c] (handle_irq_event+0x0/0x64) from
[8008501c] (handle_fasteoi_irq+0xd0/0x11c)
[12251.008230]  r6:0061 r5:803901c8 r4:80390180 r3:
[12251.008269] [80084f4c] (handle_fasteoi_irq+0x0/0x11c) from
[800824bc] (generic_handle_irq+0x28/0x38)
[12251.008285]  r5: r4:0061
[12251.008337] [80082494] (generic_handle_irq+0x0/0x38) from
[80029080] (asm_do_IRQ+0x80/0xc0)
[12251.008353]  r4:0061 r3:0100
[12251.008385] [80029000] (asm_do_IRQ+0x0/0xc0) from [8002e6cc]
(__irq_svc+0x4c/0xe0)
[12251.008403] Exception stack(0x8038bf40 to 0x8038bf88)
[12251.008427] bf40: 8038a000 8038a008 8038bf88  80395334
803ad3a0 800218d4 813cd1e0
[12251.008453] bf60: 4000406a 412fc092  8038bf94 8038bf98
8038bf88 8002f698 8002f69c
[12251.008470] bf80: 6013 
[12251.008480]  r5:fd100100 r4:
[12251.008513] [8002f670] (default_idle+0x0/0x30) from [8002f878]
(cpu_idle+0x78/0xe0)
[12251.008564] [8002f800] (cpu_idle+0x0/0xe0) from [802be948]
(rest_init+0x8c/0xa4)
[12251.008579]  r4:8038a000 r3:0001
[12251.008612] [802be8bc] (rest_init+0x0/0xa4) from [80008928]
(start_kernel+0x24c/0x28c)
[12251.008627]  r4:8039539c r3:8038a000
[12251.008652] [800086dc] (start_kernel+0x0/0x28c) from [40008040]
(0x40008040)
[12251.008667]  r7:80398154 r6:800218d0 r5:803952ec r4:10c5387d
[12251.008695] ---[ end trace de8deac85e8d982e ]---
[12251.008723] f_usb30_udc f_usb30_udc: SETUP transfer to gadget
driver is failed at -95.
[12251.012030] g_mass_storage gadget: common-fsg is NULL in fsg_setup at 614
[12251.012046] [ cut here ]
[12251.012089] WARNING: at
/work1/share/new/linux_aa9_fddl/drivers/usb/gadget/f_mass_storage.c:460
fsg_setup+0x50/0x118 [g_mass_storage]()
[12251.012108] Modules linked in: g_mass_storage f_usb30_udc [last
unloaded: g_mass_storage]
[12251.012133] Backtrace:
[12251.012172] [80031d78] (dump_backtrace+0x0/0x10c) from
[802c4358] (dump_stack+0x18/0x1c)
[12251.012190]

Re: Linux USB file storage gadget with new UDC

2013-05-19 Thread victor yeo 
Hi,

 Another question from the bulk_out_complete() printout which is shown
 below. The req-actual is 512 byte. The bh-bulk_out_intended_length
 is 31. Is this a bug?

 g_file_storage gadget: get_next_command
 [start_transfer] 6f007442 7000
 ept1 out queue len 0x200, buffer 0xc133
 kagen2_ep_queue 512 512 512
 g_file_storage gadget: bulk_out_complete -- 0, 512/31
 .

 Well, it's a mistake.  It might be a bug.

 If the host really did send a 13-byte packet then it's definitely a
 bug.  But if the host sent a 512-byte packet then something else is
 wrong; it would mean the gadget was expecting a CBW packet but the host
 sent something else.

 Alan Stern

When copying a file to the USB gadget, sometimes the USB gadget will
hang, sometimes the USB gadget will crash, sometimes the copy is ok.

From the UDC driver log, when the USB gadget crashes, the host is
sending 16384 bytes of data. It seems that bulk_out_complete() is not
able to handle it.

[c03282ec] (dev_printk+0x0/0x3c) from [bf035924]
(bulk_out_complete+0xc4/0x1a8 [g_file_storage])
 r3:152a0e00 r2:a020d0e5
[bf02fac4] (kagen2_ep_queue+0x0/0x680 [kagen2_udc]) from
[bf035f9c] (bulk_in_complete+0x24c/0x1010 [g_file_storage])

The meaning of printk of kagen2_ep_queue 512 16384 512 in UDC driver log:
ka_req-req.actual is 512
ka_req-req.length is 16384
length from hardware FIFO is 512

Please see the attached UDC driver log and corresponding usbmon trace.

Thanks,
victor
bulk_in_complete -- 0, 512/512
bulk_in_complete -- 0, 13/13
kagen2_ep_queue 31 512 31
bulk_in_complete -- 0, 512/512
bulk_in_complete -- 0, 13/13
kagen2_ep_queue 31 512 31
EP1 OUT IRQ 0x28
ep1_out: RX DMA done : NULL REQ on OUT EP-1
bulk_in_complete -- 0, 512/512
bulk_in_complete -- 0, 13/13
kagen2_ep_queue 31 512 31
EP1 OUT IRQ 0x28
ep1_out: RX DMA done : NULL REQ on OUT EP-1
kagen2_ep_queue 512 16384 512
kagen2_ep_queue 1024 16384 512
kagen2_ep_queue 1536 16384 512
kagen2_ep_queue 2048 16384 512
kagen2_ep_queue 2560 16384 512
kagen2_ep_queue 3072 16384 512
kagen2_ep_queue 3584 16384 512
kagen2_ep_queue 4096 16384 512
kagen2_ep_queue 4608 16384 512
kagen2_ep_queue 5120 16384 512
kagen2_ep_queue 5632 16384 512
kagen2_ep_queue 6144 16384 512
kagen2_ep_queue 6656 16384 512
kagen2_ep_queue 7168 16384 512
kagen2_ep_queue 7680 16384 512
kagen2_ep_queue 8192 16384 512
kagen2_ep_queue 8704 16384 512
kagen2_ep_queue 9216 16384 512
kagen2_ep_queue 9728 16384 512
kagen2_ep_queue 10240 16384 512
kagen2_ep_queue 10752 16384 512
kagen2_ep_queue 11264 16384 512
kagen2_ep_queue 11776 16384 512
kagen2_ep_queue 12288 16384 512
kagen2_ep_queue 12800 16384 512
kagen2_ep_queue 13312 16384 512
kagen2_ep_queue 13824 16384 512
kagen2_ep_queue 14336 16384 512
kagen2_ep_queue 14848 16384 512
kagen2_ep_queue 15360 16384 512
kagen2_ep_queue 15872 16384 512
kagen2_ep_queue 16384 16384 512
Unable to handle kernel NULL pointer dereference at virtual address 0004
pgd = c0204000
[0004] *pgd=
Internal error: Oops - BUG: 17 [#1] PREEMPT ARM
Modules linked in: g_file_storage kagen2_udc ath6kl_sdio ath6kl_core 
ka2000_sdio ka2000_sdhc
CPU: 0Not tainted  (3.4.4+ #43)
PC is at dev_driver_string+0x30/0x44
LR is at __dev_printk+0x38/0x68
pc : [c0327ef8]lr : [c03280c4]psr: 2093
sp : c1333c08  ip : c1333c18  fp : c1333c14
r10: c0c38000  r9 : c12a0e34  r8 : 0001
r7 : c1289600  r6 : c129ec00  r5 : c1333c44  r4 : c129edd0
r3 : 0004  r2 : c1333c44  r1 : c129ec00  r0 : c129ec00
Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 0005717f  Table: 01308000  DAC: 0017
Process file-storage-ga (pid: 123, stack limit = 0xc1332270)
Stack: (0xc1333c08 to 0xc1334000)
3c00:   c1333c3c c1333c18 c03280c4 c0327ed8 c0208eb8 c0208564
3c20: c129edd0 c12a0e00 c12896bc 0200 c1333c5c c1333c40 c0328320 c032809c
3c40: 0001 a020d0e5 c1333c4c c1333c64 c1333eb4 c1333c68 bf035924 c0328300
3c60: a020d0e5 152a0e00 152a0e00 c1333c78   000a 6013
3c80: c0c38000 c12a0e34 20313320 34660a3e 32613231 32203034 32323434 31363639
3ca0: 20532033 323a6942 3435303a 2d20313a 20353131 36393034 660a3c20 61323134
3cc0: 20303432 32343432 37313435 43203534 3a694220 35303a32 20313a34 30342030
3ce0: 3d203639 30303020 30303030 30302030 30303030 30203030 30303030 20303030
3d00: 30303030 30303030 30303020 30303030 30302030 30303030 30203030 30303030
3d20: 20303030 30303030 30303030 6166640a 34313936 34322030 34353234 34323831
3d40: 42205320 3a323a69 3a343530 312d2031 31203531 0a3c2033 36616664 30343139
3d60: 34343220 38343532 20323934 69422043 303a323a 313a3435 31203020 203d2033
3d80: 33353535 33353234 30613520 30303030 30302030 30303030 30203030 66640a30
3da0: 31393661 32203034 35323434 38353834 20532037 323a6f42 3435303a 2d20313a
3dc0: 20353131 3d203133 35353520 34323433 62352033 30303030 30203030 30303130
3de0: 20303030 30303038 38326130 30303020 30303030 30382033 30303030 30203830
3e00: 30303030 20303030 30303030 640a3030 39366166 20303431 32343432 37383435