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 

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-usb&m=136547502011472&w=2.

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

Signed-off-by: Peter Chen 
---
 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-usb&m=136547502011472&w=2.

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

Signed-off-by: Peter Chen 
---
 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  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 
Cc: Nicolas Ferre 
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 
  #include 
  #include 
+#include 
+#include 

  #include 

@@ -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;
+
+   pp = NULL;
+   while ((pp = of_get_next_child(np, pp)))
+  

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_backtra

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.

[] (dev_printk+0x0/0x3c) from []
(bulk_out_complete+0xc4/0x1a8 [g_file_storage])
 r3:152a0e00 r2:a020d0e5
[] (kagen2_ep_queue+0x0/0x680 [kagen2_udc]) from
[] (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 : []lr : []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
3e20: 4

[PATCH 1/2] usb: host: fusbh200-hcd: Remove redundant platform_set_drvdata()

2013-05-19 Thread Sachin Kamat
Commit 0998d06310 (device-core: Ensure drvdata = NULL when no
driver is bound) removes the need to set driver data field to
NULL.

Signed-off-by: Sachin Kamat 
Cc: Yuan-Hsin Chen 
---
 drivers/usb/host/fusbh200-hcd.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c
index 79ce799..b17dd3f 100644
--- a/drivers/usb/host/fusbh200-hcd.c
+++ b/drivers/usb/host/fusbh200-hcd.c
@@ -5907,7 +5907,6 @@ int fusbh200_hcd_fusbh200_remove(struct platform_device 
*pdev)
iounmap(hcd->regs);
release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
-   platform_set_drvdata(pdev, NULL);
 
return 0;
 }
-- 
1.7.9.5

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


[PATCH 2/2] usb: host: fusbh200-hcd: Staticize local symbols

2013-05-19 Thread Sachin Kamat
Local symbols referenced only in this file are made static.

Signed-off-by: Sachin Kamat 
---
 drivers/usb/host/fusbh200-hcd.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c
index b17dd3f..0855ca4 100644
--- a/drivers/usb/host/fusbh200-hcd.c
+++ b/drivers/usb/host/fusbh200-hcd.c
@@ -5769,7 +5769,7 @@ static const struct hc_driver fusbh200_fusbh200_hc_driver 
= {
.clear_tt_buffer_complete = fusbh200_clear_tt_buffer_complete,
 };
 
-void fusbh200_init(struct fusbh200_hcd *fusbh200)
+static void fusbh200_init(struct fusbh200_hcd *fusbh200)
 {
u32 reg;
 
@@ -5895,7 +5895,7 @@ fail_create_hcd:
  * the HCD's stop() method.  It is always called from a thread
  * context, normally "rmmod", "apmd", or something similar.
  */
-int fusbh200_hcd_fusbh200_remove(struct platform_device *pdev)
+static int fusbh200_hcd_fusbh200_remove(struct platform_device *pdev)
 {
struct device *dev  = &pdev->dev;
struct usb_hcd *hcd = dev_get_drvdata(dev);
@@ -5911,7 +5911,7 @@ int fusbh200_hcd_fusbh200_remove(struct platform_device 
*pdev)
return 0;
 }
 
-struct platform_driver fusbh200_hcd_fusbh200_driver = {
+static struct platform_driver fusbh200_hcd_fusbh200_driver = {
.driver = {
.name   = "fusbh200",
},
-- 
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 2/2] usb: host: fusbh200-hcd: Staticize local symbols

2013-05-19 Thread Yuan-Hsin Chen
Hi,

On Mon, May 20, 2013 at 1:51 PM, Sachin Kamat  wrote:
> Local symbols referenced only in this file are made static.
>
> Signed-off-by: Sachin Kamat 

Thank you.

> ---
>  drivers/usb/host/fusbh200-hcd.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c
> index b17dd3f..0855ca4 100644
> --- a/drivers/usb/host/fusbh200-hcd.c
> +++ b/drivers/usb/host/fusbh200-hcd.c
> @@ -5769,7 +5769,7 @@ static const struct hc_driver 
> fusbh200_fusbh200_hc_driver = {
> .clear_tt_buffer_complete = fusbh200_clear_tt_buffer_complete,
>  };
>
> -void fusbh200_init(struct fusbh200_hcd *fusbh200)
> +static void fusbh200_init(struct fusbh200_hcd *fusbh200)
>  {
> u32 reg;
>
> @@ -5895,7 +5895,7 @@ fail_create_hcd:
>   * the HCD's stop() method.  It is always called from a thread
>   * context, normally "rmmod", "apmd", or something similar.
>   */
> -int fusbh200_hcd_fusbh200_remove(struct platform_device *pdev)
> +static int fusbh200_hcd_fusbh200_remove(struct platform_device *pdev)
>  {
> struct device *dev  = &pdev->dev;
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> @@ -5911,7 +5911,7 @@ int fusbh200_hcd_fusbh200_remove(struct platform_device 
> *pdev)
> return 0;
>  }
>
> -struct platform_driver fusbh200_hcd_fusbh200_driver = {
> +static struct platform_driver fusbh200_hcd_fusbh200_driver = {
> .driver = {
> .name   = "fusbh200",
> },
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] usb: host: fusbh200-hcd: Staticize local symbols

2013-05-19 Thread Sachin Kamat
Hi,

On 20 May 2013 12:13, Yuan-Hsin Chen  wrote:
> Hi,
>
> On Mon, May 20, 2013 at 1:51 PM, Sachin Kamat  wrote:
>> Local symbols referenced only in this file are made static.
>>
>> Signed-off-by: Sachin Kamat 
>
> Thank you.

I believe you probably meant to add your Ack to this series?


>
>> ---
>>  drivers/usb/host/fusbh200-hcd.c |6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/host/fusbh200-hcd.c 
>> b/drivers/usb/host/fusbh200-hcd.c
>> index b17dd3f..0855ca4 100644
>> --- a/drivers/usb/host/fusbh200-hcd.c
>> +++ b/drivers/usb/host/fusbh200-hcd.c
>> @@ -5769,7 +5769,7 @@ static const struct hc_driver 
>> fusbh200_fusbh200_hc_driver = {
>> .clear_tt_buffer_complete = fusbh200_clear_tt_buffer_complete,
>>  };
>>
>> -void fusbh200_init(struct fusbh200_hcd *fusbh200)
>> +static void fusbh200_init(struct fusbh200_hcd *fusbh200)
>>  {
>> u32 reg;
>>
>> @@ -5895,7 +5895,7 @@ fail_create_hcd:
>>   * the HCD's stop() method.  It is always called from a thread
>>   * context, normally "rmmod", "apmd", or something similar.
>>   */
>> -int fusbh200_hcd_fusbh200_remove(struct platform_device *pdev)
>> +static int fusbh200_hcd_fusbh200_remove(struct platform_device *pdev)
>>  {
>> struct device *dev  = &pdev->dev;
>> struct usb_hcd *hcd = dev_get_drvdata(dev);
>> @@ -5911,7 +5911,7 @@ int fusbh200_hcd_fusbh200_remove(struct 
>> platform_device *pdev)
>> return 0;
>>  }
>>
>> -struct platform_driver fusbh200_hcd_fusbh200_driver = {
>> +static struct platform_driver fusbh200_hcd_fusbh200_driver = {
>> .driver = {
>> .name   = "fusbh200",
>> },
>> --
>> 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



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


Re: usb sysfs file handling functions don't take

2013-05-19 Thread Hans de Goede

Hi,

On 05/18/2013 06:03 PM, Alan Stern wrote:

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


Hi,

As written in my mail titled: "Linux sysfs usb descriptors
file has broken configuration length handling"

I've been taking a close look at the usb sysfs handling
code, specifically for the descriptors sysfs file.

One other difference I've noticed is that the usbfs
code for reading the descriptors does a usb_lock_device,
whereas read_descriptors for the sysfs descriptors file
does not. Unless I'm mistaken that means the sysfs
code can race with (re)-enumeration and bad things could
happen.

Similar concerns apply to the other usb sysfs files.


It's true that several of the attribute routines in sysfs.c don't
include proper locking.  I started to work on this some time ago (back
around the 3.5 time frame, or maybe earlier) but never finished up.
Maybe I should go back and complete the work.


If you could I think that would be great.

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