RE: [Intel-linux-usb] RE: One question about Linux xHCI driver

2014-06-18 Thread Wang, Yu Y
> On Wed, 18 Jun 2014, Mathias Nyman wrote:
> 
> > On 06/17/2014 05:17 AM, Wang, Yu Y wrote:
> > >
> > > Hi All,
> > >
> > > I have one question about Linux xHCI driver. Need your help to introduce
> more backgrounds.
> > > About the S3 flow:
> > > 1, Freeze all user processes.
> > > 2, Freeze all kernel threads (including workqueue thread).
> > > 3, Trying to suspend all devices.
> > > 3.1, if pci device in RPM suspended, then try to resume it to D0. Call
> pm_runtime_resume API in prepare stage.
> > > 3.2, xhci_resume callback will call usb_hcd_resume_root_hub to queue two
> workqueue items to resume USB2&USB3 roothub devices.
> > > 3.3, call suspend callback of devices.
> > > 3.3.1, All suspend callback be called of hcd's children included roothub.
> > > 3.3.2, Finally, hcd_pci_suspend callback will be called.
> > >
> > > For above S3 enter flow. Due to workqueue thread were already frozen
> > > in step 2. So the two workqueue items can't be scheduled in step
> > > 3.2. But step 3.2 will set HCD_FLAG_WAKEUP_PENDING flag to hcd and
> > > shared_hcd which expecting clear in bus_resume which will not be
> > > executed in this scenario. It will cause step 3.3.2 return -EBUSY
> > > due to HCD_FLAG_WAKEUP_PENDING flag is set. Finally, S3 enter
> > > failed.
> > >
> > > As my debugging, I see sometimes, the HCD_FLAG_WAKEUP_PENDING flag
> > > will be clear in step 3.3.1 due to udev->do_remote_wakeup is not
> > > equal device_may_wakeup(&udev->dev) in choose_wakeup function.
> > >
> > > Is it by design behavior? Or it is just one lucky hit?
> 
> It is a bug.  And you are right; the only reason it doesn't show up very 
> often is
> because of luck.

[Yu:] Thanks for help confirm.

> 
> > > Another question, step 3.2, why xhci_resume try to positive to
> > > resume its roothub devices?
> > >
> >
> > Hi
> >
> > This is a good question, I'm not that familiar with the internal details of 
> > pm
> framework or the usb core power management.
> >
> > The powermanagemnt workqueue (pm_wq) which is used by the PM
> framework
> > is used here as well to resume the root hub by calling
> > hcd_bus_resume() which clears WAKEUP_PENDING flags. I have a hard time
> believing this workqueue can't be scheduled in the middle of PM transitions
> activity.
> 
> It can't, and for a very good reason: We don't want workqueue actions
> interfering with a system suspend by waking devices up after the PM core has
> powered them down.
> 
> > I think that it should be possible to suspend a device already in runtime
> suspend without resuming it first.
> > This would at least make sure the runtime resume won't cause the system
> suspend to fail.
> 
> It may or may not be possible, depending on the circumstances.
> Generally, if a device is in runtime suspend then it is enabled for wakeup 
> (if it
> supports wakeup).  But during system sleep, a device is supposed to be
> enabled for wakeup only if the user wants it to be.
> Root hubs in particular usually aren't wakeup-enabled during system sleep.
> You can imagine the trouble that would result if unplugging a USB mouse from a
> sleeping computer caused the computer to wake up.
> 
> This means that for root hubs and various other devices, the wakeup settings
> have to be changed if the device is already in runtime suspend when a system
> sleep starts.  And in general, you can't change a device's wakeup settings
> while it is powered down; you have to resume it first.
> 
> That's why the PCI core does a runtime resume on every PCI device at the start
> of system sleep.  It's overkill, but it allows wakeup settings to be adjusted
> properly.
> 
> The problem here is that xhci_resume() assumes that it gets called _only_
> when the root hubs are about to be resumed -- either because the whole
> system is waking up from sleep or because a USB device has sent a runtime
> wakeup request.  But this isn't true during the initial stages of system
> suspend.  When the PCI core does its runtime resume of the controller at this
> time, this does not have to cause the root hubs to resume.
> 
> I'm not sure of the right way to solve this problem.  Probably
> xhci_resume() should check the root-hub statuses to see if either root hub
> really needs to be resumed before calling usb_hcd_resume_root_hub(); I think
> that will work.

[Yu:] I understand your point. From the big picture, to my understanding, there 
have
three scenarios.

The first one is USB device trigger remote wakeup. 

The second one is user space trying to resume xHC which will queue URBs.

The third one is PCI driver try to resume pci device during S3 entering if the 
device
under suspended state.

Your patch is focus on the first case. Avoid xHCI re-enter RPM suspended in the 
gap
between HCD resumed and activate the IRQ, right?

But your patch will cause this BUG for the third case. And the second case 
should be fine.
So my understanding is to find one way to distinguish the first one and second 
one.
We need to confirm if RH need to resum

[PATCH] usb: host: max3421-hcd: Use atomic bitops in lieu of bit fields

2014-06-18 Thread David Mosberger
From: David Mosberger-Tang 

Bit fields are not MP-safe.

Signed-off-by: Davidm Mosberger 
---
 drivers/usb/host/max3421-hcd.c |   45 ++--
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 858efcf..f8ecd7d 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -102,6 +102,15 @@ enum scheduling_pass {
SCHED_PASS_DONE
 };
 
+/* Bit numbers for max3421_hcd->todo: */
+enum {
+   ENABLE_IRQ = 0,
+   RESET_HCD,
+   RESET_PORT,
+   CHECK_UNLINK,
+   IOPIN_UPDATE
+};
+
 struct max3421_dma_buf {
u8 data[2];
 };
@@ -146,11 +155,7 @@ struct max3421_hcd {
u8 hien;
u8 mode;
u8 iopins[2];
-   unsigned int do_enable_irq:1;
-   unsigned int do_reset_hcd:1;
-   unsigned int do_reset_port:1;
-   unsigned int do_check_unlink:1;
-   unsigned int do_iopin_update:1;
+   unsigned long todo;
 #ifdef DEBUG
unsigned long err_stat[16];
 #endif
@@ -1165,10 +1170,8 @@ max3421_irq_handler(int irq, void *dev_id)
if (max3421_hcd->spi_thread &&
max3421_hcd->spi_thread->state != TASK_RUNNING)
wake_up_process(max3421_hcd->spi_thread);
-   if (!max3421_hcd->do_enable_irq) {
-   max3421_hcd->do_enable_irq = 1;
+   if (!test_and_set_bit(ENABLE_IRQ, &max3421_hcd->todo))
disable_irq_nosync(spi->irq);
-   }
return IRQ_HANDLED;
 }
 
@@ -1423,10 +1426,8 @@ max3421_spi_thread(void *dev_id)
spi_wr8(hcd, MAX3421_REG_HIEN, max3421_hcd->hien);
 
set_current_state(TASK_INTERRUPTIBLE);
-   if (max3421_hcd->do_enable_irq) {
-   max3421_hcd->do_enable_irq = 0;
+   if (test_and_clear_bit(ENABLE_IRQ, &max3421_hcd->todo))
enable_irq(spi->irq);
-   }
schedule();
__set_current_state(TASK_RUNNING);
}
@@ -1440,23 +1441,18 @@ max3421_spi_thread(void *dev_id)
else if (!max3421_hcd->curr_urb)
i_worked |= max3421_select_and_start_urb(hcd);
 
-   if (max3421_hcd->do_reset_hcd) {
+   if (test_and_clear_bit(RESET_HCD, &max3421_hcd->todo))
/* reset the HCD: */
-   max3421_hcd->do_reset_hcd = 0;
i_worked |= max3421_reset_hcd(hcd);
-   }
-   if (max3421_hcd->do_reset_port) {
+   if (test_and_clear_bit(RESET_PORT, &max3421_hcd->todo)) {
/* perform a USB bus reset: */
-   max3421_hcd->do_reset_port = 0;
spi_wr8(hcd, MAX3421_REG_HCTL,
BIT(MAX3421_HCTL_BUSRST_BIT));
i_worked = 1;
}
-   if (max3421_hcd->do_check_unlink) {
-   max3421_hcd->do_check_unlink = 0;
+   if (test_and_clear_bit(CHECK_UNLINK, &max3421_hcd->todo))
i_worked |= max3421_check_unlink(hcd);
-   }
-   if (max3421_hcd->do_iopin_update) {
+   if (test_and_clear_bit(IOPIN_UPDATE, &max3421_hcd->todo)) {
/*
 * IOPINS1/IOPINS2 do not auto-increment, so we can't
 * use spi_wr_buf().
@@ -1469,7 +1465,6 @@ max3421_spi_thread(void *dev_id)
spi_wr8(hcd, MAX3421_REG_IOPINS1 + i, val);
max3421_hcd->iopins[i] = val;
}
-   max3421_hcd->do_iopin_update = 0;
i_worked = 1;
}
}
@@ -1485,7 +1480,7 @@ max3421_reset_port(struct usb_hcd *hcd)
 
max3421_hcd->port_status &= ~(USB_PORT_STAT_ENABLE |
  USB_PORT_STAT_LOW_SPEED);
-   max3421_hcd->do_reset_port = 1;
+   set_bit(RESET_PORT, &max3421_hcd->todo);
wake_up_process(max3421_hcd->spi_thread);
return 0;
 }
@@ -1498,7 +1493,7 @@ max3421_reset(struct usb_hcd *hcd)
hcd->self.sg_tablesize = 0;
hcd->speed = HCD_USB2;
hcd->self.root_hub->speed = USB_SPEED_FULL;
-   max3421_hcd->do_reset_hcd = 1;
+   set_bit(RESET_HCD, &max3421_hcd->todo);
wake_up_process(max3421_hcd->spi_thread);
return 0;
 }
@@ -1590,7 +1585,7 @@ max3421_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, 
int status)
 */
retval = usb_hcd_check_unlink_urb(hcd, urb, status);
if (retval == 0) {
-   max3421_hcd->do_check_unlink = 1;
+   set_bit(CHECK_UNLINK, &max3421_hcd->todo);
wake_up_process(max3421_hcd->spi_thread);
}
spin_unlock_irqrestore(&max3421_hcd->l

[PATCH] usb: host: max3421-hcd: Fix max3421_reset_port() to set USB_PORT_STAT_RESET

2014-06-18 Thread David Mosberger
From: David Mosberger-Tang 

Signed-off-by: Davidm Mosberger 
---
 drivers/usb/host/max3421-hcd.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index f8ecd7d..6dbf1e9 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1480,6 +1480,7 @@ max3421_reset_port(struct usb_hcd *hcd)
 
max3421_hcd->port_status &= ~(USB_PORT_STAT_ENABLE |
  USB_PORT_STAT_LOW_SPEED);
+   max3421_hcd->port_status |= USB_PORT_STAT_RESET;
set_bit(RESET_PORT, &max3421_hcd->todo);
wake_up_process(max3421_hcd->spi_thread);
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


Re: [PATCH] USB:gadget: Fix a warning while loading g_mass_storage

2014-06-18 Thread Yang,Wei

On 06/18/2014 07:44 PM, Michal Nazarewicz wrote:

On Wed, Jun 18 2014, Yang,Wei wrote:

On 06/17/2014 10:18 PM, Alan Stern wrote:

That is a strange question to ask.  If you did not know that I approved
the patch, why did you insert my Acked-By:?

I added your Acked-By, as when you reviewed V3, you mentioned that I
*may* add your Acked-by in this patch. If I misunderstood your point, I
am so sorry for that.

Alan's point is that if you have any doubts whether someone approves
your patch you should *not* add their Acked-by.  So adding someone's
Acked-by and then asking if they are fine with the patch is
contradictory -- the former indicates that you believe the person is
fine with the patch, while the latter shows that you aren't.

Alan wrote:


With that change, you may add

Acked-by: Alan Stern 

so after adding “that change” you are in your right to add Alan's
Acked-by, but what that also means is that Alan will be fine with the
patch if you make the requested change, so you don't need to ask for an
opinion again.

PS. Note that “having any doubts” also means that if someone gave you
 their Acked-by, but then you substantially rewrite the patch, you
 probably should consider removing their Acked-by.


Michal, Thank you for your detailed explanation, it is very helpful to me.

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


Re: [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels

2014-06-18 Thread Jiri Kosina
On Wed, 18 Jun 2014, Johan Hovold wrote:

> > This driver adds support for USB controlled led panels that exists in
> > MSI GT683R laptop
> > 
> > Signed-off-by: Janne Kanniainen 
> 
> Reviewed-by: Johan Hovold 
> 
> Thanks, Janne!

Now applied. Thanks Janne for the driver, and thanks a lot Johan for a 
very careful review, I really appreciate it.

-- 
Jiri Kosina
SUSE Labs
--
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: [bisected][regression] USB Ethernet Gadget Support - Freescale 8308

2014-06-18 Thread Fabio Estevam
On Wed, Jun 18, 2014 at 5:31 PM, Barry G  wrote:
> On Wed, Jun 18, 2014 at 1:00 PM, Fabio Estevam  wrote:
>> Can't you use the chipidea driver instead of 
>> drivers/usb/gadget/fsl_udc_core.c?
>
> I was under the impression that the chipidea stuff was for iMX series
> processors.

MPC does not use it only because no one has converted it yet :-)

> The comments in fsl_udc_core.c say it is for the MPC8349E and friends and
> it worked fine for us until eb65796e.

i.MX used fsl_udc_core.c in the past. Now we have moved all of the
i.MX SoCs into chipidea driver.

It would make things a lot simpler if MPC could be moved to chipidea
driver as well so that fsl_udc_core.c could go away.

>
> I checked out the arch/powerpc/boot/dts/mpc8308rdb.dts for the RDB and it is
> claiming a compatibility of fsl-usb2-dr.
>
> What would you recommend I use as a new device tree node/compatible string?

Take a look at the existing bindings of i.MX. You probably only needs
to add the drivers/usb/chipidea/ci_hdrc_imx.c equivalent for MPC.
--
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: [bisected][regression] USB Ethernet Gadget Support - Freescale 8308

2014-06-18 Thread Barry G
On Wed, Jun 18, 2014 at 1:00 PM, Fabio Estevam  wrote:
> Can't you use the chipidea driver instead of 
> drivers/usb/gadget/fsl_udc_core.c?

I was under the impression that the chipidea stuff was for iMX series
processors.
The comments in fsl_udc_core.c say it is for the MPC8349E and friends and
it worked fine for us until eb65796e.

I checked out the arch/powerpc/boot/dts/mpc8308rdb.dts for the RDB and it is
claiming a compatibility of fsl-usb2-dr.

What would you recommend I use as a new device tree node/compatible string?

Thanks,

Barry
--
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: [bisected][regression] USB Ethernet Gadget Support - Freescale 8308

2014-06-18 Thread Fabio Estevam
On Wed, Jun 18, 2014 at 4:47 PM, Barry G  wrote:

> Seems to me the right solution is making a patch to add the 83XX stuff
> to the id_table
> and finding the right place to set_dma_ops?  I am fine doing the leg work of
> creating/testing/submitting the patch providing that sounds right and people
> can point me in the right direction :-)

Can't you use the chipidea driver instead of drivers/usb/gadget/fsl_udc_core.c?
--
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: [bisected][regression] USB Ethernet Gadget Support - Freescale 8308

2014-06-18 Thread Barry G
On Wed, Jun 18, 2014 at 9:17 AM, Barry G  wrote:

> NIP [c02b3f58] usb_gadget_map_request+0x118/0x1a4
> LR [c02bc68c] fsl_ep_queue+0xc4/0x19c
> Call Trace:
> [dfff7ea8] [c02bc68c] fsl_ep_queue+0xc4/0x19c
> [dfff7ec8] [c02b7514] composite_setup+0x1324/0x13e8
> [dfff7f20] [c02bd070] fsl_udc_irq+0x5cc/0xcbc
> [dfff7f78] [c004b42c] handle_irq_event_percpu+0x4c/0x150
> [dfff7fa8] [c004b594] handle_irq_event+0x64/0x94
> [dfff7fc0] [c004e7bc] handle_level_irq+0x138/0x15c
> [dfff7fd8] [c004b118] generic_handle_irq+0x38/0x50
> [dfff7fe8] [c000530c] __do_irq+0x44/0x58
> [dfff7ff0] [c000c510] call_do_irq+0x24/0x3c
> [c054fe98] [c0005510] do_IRQ+0x94/0xe0
> [c054fec0] [c000dca4] ret_from_except+0x0/0x14
> --- Exception: 501 at arch_cpu_idle+0x24/0x68
> LR = arch_cpu_idle+0x24/0x68
> [c054ff80] [c054e000] 0xc054e000 (unreliable)
> [c054ff88] [c0042dec] cpu_startup_entry+0x100/0x180
> [c054ffa8] [c0004068] rest_init+0x84/0x9c
> [c054ffc0] [c04edd18] start_kernel+0x334/0x348
> [c054fff0] [3438] 0x3438
> Instruction dump:
> 7fff0034 57ffd97f 41a2000c 3960 4808 817e00bc 20070002 7c000110
> 7cd0 0f00 3d20c056 3c854000 <816b0010> 8009f200 5484c9f4 54a5053e
> ---[ end trace 1e1b78a0b4f63fb8 ]---


Did some more digging into this.  I found out that
usb_gadget_map_request is failing
in the dma_map_single call because get_dma_ops is returning NULL (and map_page
is offset 16 into dma_maps_ops hence the 0x10 offset).

Looks like nothing on the Freescale 83XX is calling set_dma_ops.

The following complete hacks give me USB gadget support:
diff --git a/arch/powerpc/include/asm/dma-mapping.h
b/arch/powerpc/include/asm/dma-mapping.h
index 150866b..50db4f7 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -87,6 +87,12 @@ static inline struct dma_map_ops
*get_dma_ops(struct device *dev)
if (unlikely(dev == NULL))
return NULL;

+   if (dev->archdata.dma_ops == NULL)
+   {
+   printk(KERN_ERR "Barry's Complete Hack triggered
%s\n", __func__);
+   return &dma_direct_ops;
+   }
+
return dev->archdata.dma_ops;
 }

diff --git a/drivers/usb/gadget/fsl_udc_core.c
b/drivers/usb/gadget/fsl_udc_core.c
index 28e4fc9..3385e8a 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2662,7 +2662,7 @@ MODULE_DEVICE_TABLE(platform, fsl_udc_devtype);
 static struct platform_driver udc_driver = {
.remove = __exit_p(fsl_udc_remove),
/* Just for FSL i.mx SoC currently */
-   .id_table   = fsl_udc_devtype,
+   /* .id_table= fsl_udc_devtype, */
/* these suspend and resume are not usb suspend and resume */
.suspend= fsl_udc_suspend,
.resume = fsl_udc_resume,

Seems to me the right solution is making a patch to add the 83XX stuff
to the id_table
and finding the right place to set_dma_ops?  I am fine doing the leg work of
creating/testing/submitting the patch providing that sounds right and people
can point me in the right direction :-)

Thanks,

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


Re: [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels

2014-06-18 Thread Johan Hovold
On Wed, Jun 18, 2014 at 09:41:35PM +0300, Janne Kanniainen wrote:
> >> This driver adds support for USB controlled led panels that exists in
> >> MSI GT683R laptop
> >>
> >> Signed-off-by: Janne Kanniainen 
> >
> > Reviewed-by: Johan Hovold 
> >
> > Thanks, Janne!
> >
> > Johan
> 
> Thank you for reviewing my patch :) I sure learnt a
> lot from you and if I ever send patch in future, it is for sure much
> easier (Maybe I get it trough in 8th time ;))

My pleasure. It's indeed a very good first patch you have created.

Good luck with the next 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 v9] leds: USB: HID: Add support for MSI GT683R led panels

2014-06-18 Thread Janne Kanniainen
>> This driver adds support for USB controlled led panels that exists in
>> MSI GT683R laptop
>>
>> Signed-off-by: Janne Kanniainen 
>
> Reviewed-by: Johan Hovold 
>
> Thanks, Janne!
>
> Johan

Thank you for reviewing my patch :) I sure learnt a
lot from you and if I ever send patch in future, it is for sure much
easier (Maybe I get it trough in 8th time ;))

Janne
--
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] staging: usbip: fixed a coding-style warning

2014-06-18 Thread Greg Kroah-Hartman
On Wed, Jun 18, 2014 at 12:17:55AM -0700, Joe Perches wrote:
> On Wed, 2014-06-18 at 00:06 -0700, Greg Kroah-Hartman wrote:
> > On Wed, Jun 18, 2014 at 09:49:39AM +0300, Alexey Tulia wrote:
> > > On Tue, Jun 17, 2014 at 03:44:58PM -0700, Greg Kroah-Hartman wrote:
> > > > On Fri, Jun 13, 2014 at 11:35:13AM +0300, Alexey Tulia wrote:
> > > > > This fixes the following warning:
> > > > >   - WARNING: __constant_cpu_to_le32 should be cpu_to_le32
> > > > 
> > > > What produces this warning?
> > > > 
> > > 
> > > This warning was found by checkpatch.pl -f
> > > 
> > > > > 
> > > > > Signed-off-by: Alexey Tulia 
> > > > > ---
> > > > >  drivers/staging/usbip/vhci_hcd.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/staging/usbip/vhci_hcd.c 
> > > > > b/drivers/staging/usbip/vhci_hcd.c
> > > > > index 0007d30..e21c1b4 100644
> > > > > --- a/drivers/staging/usbip/vhci_hcd.c
> > > > > +++ b/drivers/staging/usbip/vhci_hcd.c
> > > > > @@ -304,7 +304,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, 
> > > > > u16 typeReq, u16 wValue,
> > > > >   break;
> > > > >   case GetHubStatus:
> > > > >   usbip_dbg_vhci_rh(" GetHubStatus\n");
> > > > > - *(__le32 *) buf = __constant_cpu_to_le32(0);
> > > > > + *(__le32 *) buf = cpu_to_le32(0);
> > > > 
> > > > How is this correct?  Why shouldn't __constant_cpu_to_le32() be used
> > > > here?  Heck, why can't we just use 0 given that it doesn't matter the
> > > > endianness of that specific value :)
> > > 
> > > It may be so, but anyway the __constant_cpu_to_le32 produced a warning
> > > and it was obvious to clean up this part of code a bit.
> > > However, the 0 value possibly can change to other value in future and
> > > this macro acts as a safety net here. 
> > 
> > Dragging Joe in here as he wrote that checkpatch feature.
> > 
> > Joe, how is not using __constant_cpu_to_* a good thing?  If I have a
> > constant value, cpu_to_* will be a function call if the bits have to be
> > switched around (no-op if not).  I don't see the code path that detects
> > a constant value and calls the swap-bits-as-a-constant macro instead,
> > unless the __constant_cpu_to* function is called.
> > 
> > Am I just missing it somewhere in the .h include chain?
> 
> Probably.
> 
> There a __builtin_constant_p check in there via the
> include/uapi/linux/swab.h chain.
> 
> for instance:
> 
> htonl ->
> ___htonl ->
> __cpu_to_be32
> __constant_htonl ->
> __swab32 ->
> 
> include/uapi/linux/swab.h:#define __swab32(x)   \
> include/uapi/linux/swab.h-  (__builtin_constant_p((__u32)(x)) ? \
> include/uapi/linux/swab.h-  ___constant_swab32(x) : \
> include/uapi/linux/swab.h-  __fswab32(x))

Ah, ok, my fault, sorry for the noise.  I'll go queue this patch up in a
bit...

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: [Intel-linux-usb] RE: One question about Linux xHCI driver

2014-06-18 Thread Alan Stern
On Wed, 18 Jun 2014, Mathias Nyman wrote:

> On 06/17/2014 05:17 AM, Wang, Yu Y wrote:
> > 
> > Hi All,
> > 
> > I have one question about Linux xHCI driver. Need your help to introduce 
> > more backgrounds.
> > About the S3 flow:
> > 1, Freeze all user processes.
> > 2, Freeze all kernel threads (including workqueue thread).
> > 3, Trying to suspend all devices.
> > 3.1, if pci device in RPM suspended, then try to resume it to D0. Call 
> > pm_runtime_resume API in prepare stage.
> > 3.2, xhci_resume callback will call usb_hcd_resume_root_hub to queue two 
> > workqueue items to resume USB2&USB3 roothub devices.
> > 3.3, call suspend callback of devices.
> > 3.3.1, All suspend callback be called of hcd's children included roothub.
> > 3.3.2, Finally, hcd_pci_suspend callback will be called.
> > 
> > For above S3 enter flow. Due to workqueue thread were already
> > frozen in step 2. So the two workqueue items can't be scheduled in
> > step 3.2. But step 3.2 will set HCD_FLAG_WAKEUP_PENDING flag to hcd
> > and shared_hcd which expecting clear in bus_resume which will not
> > be executed in this scenario. It will cause step 3.3.2 return
> > -EBUSY due to HCD_FLAG_WAKEUP_PENDING flag is set. Finally, S3
> > enter failed.
> > 
> > As my debugging, I see sometimes, the HCD_FLAG_WAKEUP_PENDING flag
> > will be clear in step 3.3.1 due to udev->do_remote_wakeup is not
> > equal device_may_wakeup(&udev->dev) in choose_wakeup function.
> > 
> > Is it by design behavior? Or it is just one lucky hit?

It is a bug.  And you are right; the only reason it doesn't show up 
very often is because of luck.

> > Another question, step 3.2, why xhci_resume try to positive to
> > resume its roothub devices?
> > 
> 
> Hi
> 
> This is a good question, I'm not that familiar with the internal details of 
> pm framework or the usb core power management.
> 
> The powermanagemnt workqueue (pm_wq) which is used by the PM framework is 
> used here as well to resume the root hub
> by calling hcd_bus_resume() which clears WAKEUP_PENDING flags. I have a hard 
> time believing this workqueue can't be scheduled in the middle of
> PM transitions activity.

It can't, and for a very good reason: We don't want workqueue actions 
interfering with a system suspend by waking devices up after the PM 
core has powered them down.

> I think that it should be possible to suspend a device already in runtime 
> suspend without resuming it first.
> This would at least make sure the runtime resume won't cause the system 
> suspend to fail.

It may or may not be possible, depending on the circumstances.  
Generally, if a device is in runtime suspend then it is enabled for
wakeup (if it supports wakeup).  But during system sleep, a device is
supposed to be enabled for wakeup only if the user wants it to be.  
Root hubs in particular usually aren't wakeup-enabled during system
sleep.  You can imagine the trouble that would result if unplugging a
USB mouse from a sleeping computer caused the computer to wake up.

This means that for root hubs and various other devices, the wakeup
settings have to be changed if the device is already in runtime suspend
when a system sleep starts.  And in general, you can't change a
device's wakeup settings while it is powered down; you have to resume 
it first.

That's why the PCI core does a runtime resume on every PCI device at
the start of system sleep.  It's overkill, but it allows wakeup
settings to be adjusted properly.

The problem here is that xhci_resume() assumes that it gets called
_only_ when the root hubs are about to be resumed -- either because the
whole system is waking up from sleep or because a USB device has sent a
runtime wakeup request.  But this isn't true during the initial stages
of system suspend.  When the PCI core does its runtime resume of the
controller at this time, this does not have to cause the root hubs to
resume.

I'm not sure of the right way to solve this problem.  Probably
xhci_resume() should check the root-hub statuses to see if either root
hub really needs to be resumed before calling
usb_hcd_resume_root_hub(); I think that will work.

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 net,stable] net: huawei_cdc_ncm: increase command buffer size

2014-06-18 Thread Bjørn Mork
No problem. I think Dan's testing is sufficient verification that this doesn't 
have unexpected side effects.


Bjørn

On 18 June 2014 18:32:58 CEST, Enrico Mioso  wrote:
>I am sorry - I am not in good conditions to perform the testing. But -
>I think 
>it sould be fine anyway.
>Sorry Bjorn. I might be able to do it in the weekend if you would like.
>Enrico
>
>
>On Wed, 18 Jun 2014, Dan Williams wrote:
>
>==Date: Wed, 18 Jun 2014 16:03:17
>==From: Dan Williams 
>==To: Bjørn Mork 
>==Cc: net...@vger.kernel.org, linux-usb@vger.kernel.org,
>==Enrico Mioso 
>==Subject: Re: [PATCH net,
>==stable] net: huawei_cdc_ncm: increase command buffer size
>==
>==On Wed, 2014-06-18 at 14:21 +0200, Bjørn Mork wrote:
>==> Messages from the modem exceeding 256 bytes cause communication
>==> failure.
>==> 
>==> The WDM protocol is strictly "read on demand", meaning that we only
>==> poll for unread data after receiving a notification from the modem.
>==> Since we have no way to know how much data the modem has to send,
>==> we must make sure that the buffer we provide is "big enough".
>==> Message truncation does not work. Truncated messages are left
>unread
>==> until the modem has another message to send.  Which often won't
>==> happen until the userspace application has given up waiting for the
>==> final part of the last message, and therefore sends another
>command.
>==> 
>==> With a proper CDC WDM function there is a descriptor telling us
>==> which buffer size the modem uses. But with this vendor specific
>==> implementation there is no known way to calculate the exact "big
>==> enough" number.  It is an unknown property of the modem firmware.
>==> Experience has shown that 256 is too small.  The discussion of
>==> this failure ended up concluding that 512 might be too small as
>==> well. So 1024 seems like a reasonable value for now.
>==> 
>==> Fixes: 41c47d8cfd68 ("net: huawei_cdc_ncm: Introduce the
>huawei_cdc_ncm driver")
>==> Cc: Enrico Mioso 
>==> Reported-by: Dan Williams 
>==> Signed-off-by: Bjørn Mork 
>==
>==Tested-by: Dan Williams 
>==
>=='^SYSCFGEX:
>==("00","01","02","03","99"),((400380,"GSM900/GSM1800/WCDMA2100"),(6a8,"GSM850/GSM1900/WCDMA850/AWS/WCDMA1900"),(3fff,"All
>bands")),(0-2),(0-4),((1081b,"LTE_B1/LTE_B2/LTE_B4/LTE_B5/LTE_B12/LTE_B17"),(7fff,"All
>bands"))OK'
>==
>==I get the last "" now :)
>==
>==> ---
>==> 
>==> The problem is a showstopper for anyone hitting it, so I believe
>this
>==> fix should go into all maintained stable kernels with this driver.
>==> That is anything based on v3.13 or newer.
>==> 
>==> Thanks,
>==> Bjørn
>==> 
>==> 
>==>  drivers/net/usb/huawei_cdc_ncm.c | 7 ---
>==>  1 file changed, 4 insertions(+), 3 deletions(-)
>==> 
>==> diff --git a/drivers/net/usb/huawei_cdc_ncm.c
>b/drivers/net/usb/huawei_cdc_ncm.c
>==> index f9822bc75425..5d95a13dbe2a 100644
>==> --- a/drivers/net/usb/huawei_cdc_ncm.c
>==> +++ b/drivers/net/usb/huawei_cdc_ncm.c
>==> @@ -84,12 +84,13 @@ static int huawei_cdc_ncm_bind(struct usbnet
>*usbnet_dev,
>==>ctx = drvstate->ctx;
>==>  
>==>if (usbnet_dev->status)
>==> -  /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256
>==> -   * decimal (0x100)"
>==> +  /* The wMaxCommand buffer must be big enough to hold
>==> +   * any message from the modem. Experience has shown
>==> +   * that some replies are more than 256 bytes long
>==> */
>==>subdriver = usb_cdc_wdm_register(ctx->control,
>==> &usbnet_dev->status->desc,
>==> -   256, /* wMaxCommand */
>==> +   1024, /* wMaxCommand */
>==> 
>huawei_cdc_ncm_wdm_manage_power);
>==>if (IS_ERR(subdriver)) {
>==>ret = PTR_ERR(subdriver);
>==
>==
>==

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


[RFC PATCH 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle

2014-06-18 Thread Srinivas Kandagatla
Use case is when the phy is configured in host mode and a usb device is
attached to board before bootup. On bootup, with the existing code and
runtime pm enabled, the driver would decrement the pm usage count
without checking the current state of the phy. This pm usage count
decrement would trigger the runtime pm which than would abort the
usb enumeration which was in progress. In my case a usb stick gets
detected and then immediatly the driver goes to low power mode which is
not correct.

log:
[1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller
[1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned 
bus number 1
[1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252
[1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00
[1.659473] hub 1-0:1.0: USB hub found
[1.663415] hub 1-0:1.0: 1 port detected
...
[1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host
[2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected
[2.108993] scsi0 : usb-storage 1-1:1.0
[2.678341] msm_otg 1252.phy: USB in low power mode
[3.168977] usb 1-1: USB disconnect, device number 2

This issue was detected on IFC6410 board.

This patch fixes the intial runtime pm trigger by checking the phy
state and decrementing the pm use count only when the phy state is IDLE.

Signed-off-by: Srinivas Kandagatla 
---
 drivers/usb/phy/phy-msm-usb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 3bb559d..78cc870 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w)
motg->chg_state = USB_CHG_STATE_UNDEFINED;
motg->chg_type = USB_INVALID_CHARGER;
}
-   pm_runtime_put_sync(otg->phy->dev);
+
+   if (otg->phy->state == OTG_STATE_B_IDLE)
+   pm_runtime_put_sync(otg->phy->dev);
break;
case OTG_STATE_B_PERIPHERAL:
dev_dbg(otg->phy->dev, "OTG_STATE_B_PERIPHERAL state\n");
-- 
1.9.1

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


[RFC PATCH 1/3] usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs

2014-06-18 Thread Srinivas Kandagatla
This patch makes the msm ehci driver available to use on QCOM SOCs,
which have the same IP.

Signed-off-by: Srinivas Kandagatla 
---
 drivers/usb/host/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 61b7817..03314f8 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -176,7 +176,7 @@ config USB_EHCI_HCD_AT91
 
 config USB_EHCI_MSM
tristate "Support for Qualcomm QSD/MSM on-chip EHCI USB controller"
-   depends on ARCH_MSM
+   depends on ARCH_MSM || ARCH_QCOM
select USB_EHCI_ROOT_HUB_TT
---help---
  Enables support for the USB Host controller present on the
-- 
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


[RFC PATCH 2/3] usb: phy: msm: Make phy_reset clk and reset line optional.

2014-06-18 Thread Srinivas Kandagatla
This patch makes the phy reset clk and reset line optional as this clk
is not available on boards like IFC6410 with APQ8064.

Signed-off-by: Srinivas Kandagatla 
---
 drivers/usb/phy/phy-msm-usb.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index ced34f3..3bb559d 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -279,11 +279,11 @@ static int msm_otg_link_clk_reset(struct msm_otg *motg, 
bool assert)
 
 static int msm_otg_phy_clk_reset(struct msm_otg *motg)
 {
-   int ret;
+   int ret = 0;
 
-   if (motg->pdata->phy_clk_reset)
+   if (motg->pdata->phy_clk_reset && motg->phy_reset_clk)
ret = motg->pdata->phy_clk_reset(motg->phy_reset_clk);
-   else
+   else if (motg->phy_rst)
ret = reset_control_reset(motg->phy_rst);
 
if (ret)
@@ -1464,7 +1464,7 @@ static int msm_otg_read_dt(struct platform_device *pdev, 
struct msm_otg *motg)
 
motg->phy_rst = devm_reset_control_get(&pdev->dev, "phy");
if (IS_ERR(motg->phy_rst))
-   return PTR_ERR(motg->phy_rst);
+   motg->phy_rst = NULL;
 
pdata->mode = of_usb_get_dr_mode(node);
if (pdata->mode == USB_DR_MODE_UNKNOWN)
@@ -1556,7 +1556,7 @@ static int msm_otg_probe(struct platform_device *pdev)
   np ? "phy" : "usb_phy_clk");
if (IS_ERR(motg->phy_reset_clk)) {
dev_err(&pdev->dev, "failed to get usb_phy_clk\n");
-   return PTR_ERR(motg->phy_reset_clk);
+   motg->phy_reset_clk = NULL;
}
 
motg->clk = devm_clk_get(&pdev->dev, np ? "core" : "usb_hs_clk");
-- 
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


[RFC PATCH 0/3] ehci_msm fixes for APQ8064 USB host support.

2014-06-18 Thread Srinivas Kandagatla
While testing usb host on Qualcomm APQ8064, I encountered few issues.
These patches fixes those issues.

Without these patches USB is not functional on AQ8064.
All the patches are tested on IFC6410.

Thanks,
srini

Srinivas Kandagatla (3):
  usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs
  usb: phy: msm: Make phy_reset clk and reset line optional.
  usb: phy: msm: Do not do runtime pm if the phy is not idle

 drivers/usb/host/Kconfig  |  2 +-
 drivers/usb/phy/phy-msm-usb.c | 14 --
 2 files changed, 9 insertions(+), 7 deletions(-)

-- 
1.9.1

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


Re: [Intel-linux-usb] RE: One question about Linux xHCI driver

2014-06-18 Thread Mathias Nyman


On 06/17/2014 05:17 AM, Wang, Yu Y wrote:
> 
> Hi All,
> 
> I have one question about Linux xHCI driver. Need your help to introduce more 
> backgrounds.
> About the S3 flow:
> 1, Freeze all user processes.
> 2, Freeze all kernel threads (including workqueue thread).
> 3, Trying to suspend all devices.
> 3.1, if pci device in RPM suspended, then try to resume it to D0. Call 
> pm_runtime_resume API in prepare stage.
> 3.2, xhci_resume callback will call usb_hcd_resume_root_hub to queue two 
> workqueue items to resume USB2&USB3 roothub devices.
> 3.3, call suspend callback of devices.
> 3.3.1, All suspend callback be called of hcd's children included roothub.
> 3.3.2, Finally, hcd_pci_suspend callback will be called.
> 
> For above S3 enter flow. Due to workqueue thread were already frozen in step 
> 2. So the two workqueue items can't be scheduled in step 3.2. But step 3.2 
> will set HCD_FLAG_WAKEUP_PENDING flag to hcd and shared_hcd which expecting 
> clear in bus_resume which will not be executed in this scenario. It will 
> cause step 3.3.2 return -EBUSY due to HCD_FLAG_WAKEUP_PENDING flag is set. 
> Finally, S3 enter failed.
> 
> As my debugging, I see sometimes, the HCD_FLAG_WAKEUP_PENDING flag will be 
> clear in step 3.3.1 due to udev->do_remote_wakeup is not equal 
> device_may_wakeup(&udev->dev) in choose_wakeup function.
> 
> Is it by design behavior? Or it is just one lucky hit?
> Another question, step 3.2, why xhci_resume try to positive to resume its 
> roothub devices?
> 

Hi

This is a good question, I'm not that familiar with the internal details of pm 
framework or the usb core power management.

The powermanagemnt workqueue (pm_wq) which is used by the PM framework is used 
here as well to resume the root hub
by calling hcd_bus_resume() which clears WAKEUP_PENDING flags. I have a hard 
time believing this workqueue can't be scheduled in the middle of
PM transitions activity.

I think that it should be possible to suspend a device already in runtime 
suspend without resuming it first.
This would at least make sure the runtime resume won't cause the system suspend 
to fail.

The patch that adds roothub resume on controller resume explains the reasons 
why:

commit f69e3120df82391a0ee8118e0a156239a06b2afb
Author: Alan Stern 
Date:   Thu Nov 3 11:37:10 2011 -0400

USB: XHCI: resume root hubs when the controller resumes

This patch (as1494) fixes a problem in xhci-hcd's resume routine.
When the controller is runtime-resumed, this can only mean that one of
the two root hubs has made a wakeup request and therefore needs to be
resumed as well.  Rather than try to determine which root hub requires
attention (which might be difficult in the case where a new
non-SuperSpeed device has been plugged in), the patch simply resumes
both root hubs.

Without this change, there is a race: The controller might be put back
to sleep before it can activate its IRQ line, and the wakeup condition
might never get handled.

The patch also simplifies the logic in xhci_resume a little, combining
some repeated flag settings into a single pair of statements.

Signed-off-by: Alan Stern 
CC: Sarah Sharp 
Cc: stable 
Tested-by: Linus Torvalds 
Signed-off-by: Greg Kroah-Hartman 

I'm adding Alan Stern and the public usb list to this thread,
Alan in particular has far better insight in usb PM 

-Mathias  


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


Re: [PATCH net,stable] net: huawei_cdc_ncm: increase command buffer size

2014-06-18 Thread Enrico Mioso
I am sorry - I am not in good conditions to perform the testing. But - I think 
it sould be fine anyway.
Sorry Bjorn. I might be able to do it in the weekend if you would like.
Enrico


On Wed, 18 Jun 2014, Dan Williams wrote:

==Date: Wed, 18 Jun 2014 16:03:17
==From: Dan Williams 
==To: Bjørn Mork 
==Cc: net...@vger.kernel.org, linux-usb@vger.kernel.org,
==Enrico Mioso 
==Subject: Re: [PATCH net,
==stable] net: huawei_cdc_ncm: increase command buffer size
==
==On Wed, 2014-06-18 at 14:21 +0200, Bjørn Mork wrote:
==> Messages from the modem exceeding 256 bytes cause communication
==> failure.
==> 
==> The WDM protocol is strictly "read on demand", meaning that we only
==> poll for unread data after receiving a notification from the modem.
==> Since we have no way to know how much data the modem has to send,
==> we must make sure that the buffer we provide is "big enough".
==> Message truncation does not work. Truncated messages are left unread
==> until the modem has another message to send.  Which often won't
==> happen until the userspace application has given up waiting for the
==> final part of the last message, and therefore sends another command.
==> 
==> With a proper CDC WDM function there is a descriptor telling us
==> which buffer size the modem uses. But with this vendor specific
==> implementation there is no known way to calculate the exact "big
==> enough" number.  It is an unknown property of the modem firmware.
==> Experience has shown that 256 is too small.  The discussion of
==> this failure ended up concluding that 512 might be too small as
==> well. So 1024 seems like a reasonable value for now.
==> 
==> Fixes: 41c47d8cfd68 ("net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm 
driver")
==> Cc: Enrico Mioso 
==> Reported-by: Dan Williams 
==> Signed-off-by: Bjørn Mork 
==
==Tested-by: Dan Williams 
==
=='^SYSCFGEX:
==("00","01","02","03","99"),((400380,"GSM900/GSM1800/WCDMA2100"),(6a8,"GSM850/GSM1900/WCDMA850/AWS/WCDMA1900"),(3fff,"All
 
bands")),(0-2),(0-4),((1081b,"LTE_B1/LTE_B2/LTE_B4/LTE_B5/LTE_B12/LTE_B17"),(7fff,"All
 bands"))OK'
==
==I get the last "" now :)
==
==> ---
==> 
==> The problem is a showstopper for anyone hitting it, so I believe this
==> fix should go into all maintained stable kernels with this driver.
==> That is anything based on v3.13 or newer.
==> 
==> Thanks,
==> Bjørn
==> 
==> 
==>  drivers/net/usb/huawei_cdc_ncm.c | 7 ---
==>  1 file changed, 4 insertions(+), 3 deletions(-)
==> 
==> diff --git a/drivers/net/usb/huawei_cdc_ncm.c 
b/drivers/net/usb/huawei_cdc_ncm.c
==> index f9822bc75425..5d95a13dbe2a 100644
==> --- a/drivers/net/usb/huawei_cdc_ncm.c
==> +++ b/drivers/net/usb/huawei_cdc_ncm.c
==> @@ -84,12 +84,13 @@ static int huawei_cdc_ncm_bind(struct usbnet 
*usbnet_dev,
==> ctx = drvstate->ctx;
==>  
==> if (usbnet_dev->status)
==> -   /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256
==> -* decimal (0x100)"
==> +   /* The wMaxCommand buffer must be big enough to hold
==> +* any message from the modem. Experience has shown
==> +* that some replies are more than 256 bytes long
==>  */
==> subdriver = usb_cdc_wdm_register(ctx->control,
==>  &usbnet_dev->status->desc,
==> -256, /* wMaxCommand */
==> +1024, /* wMaxCommand */
==>  
huawei_cdc_ncm_wdm_manage_power);
==> if (IS_ERR(subdriver)) {
==> ret = PTR_ERR(subdriver);
==
==
==

Re: [bisected][regression] USB Ethernet Gadget Support - Freescale 8308

2014-06-18 Thread Barry G
On Tue, Jun 17, 2014 at 8:47 PM, Felipe Balbi  wrote:
> Hi,
> 3.10 is a pretty old kernel, you need to ask support from whoever gave
> you that kernel, unless you can try v3.16-rc1 on your board.
>

Thanks for responding.

We are just running Vanilla 3.10 from kernel.org without any "vendor" per se.

I did a quick port to 3.16-rc1 and booted off NFS so I didn't have
to port our NAND drivers.  Here is the dmesg:
Using Custom Platform machine description
Initializing cgroup subsys cpu
Initializing cgroup subsys cpuacct
Linux version 3.16.0-rc1+ (barrgr@zoidberg) (gcc version 4.2.4) #2
PREEMPT Wed Jun 18 08:39:02 PDT 2014
Found legacy serial port 0 for /immr@e000/serial@4500
  mem=e0004500, taddr=e0004500, irq=0, clk=13200, speed=0
Found legacy serial port 1 for /immr@e000/serial@4600
  mem=e0004600, taddr=e0004600, irq=0, clk=13200, speed=0
bootconsole [udbg0] enabled
Top of RAM: 0x2000, Total RAM: 0x2000
Memory hole size: 0MB
Zone ranges:
  DMA  [mem 0x-0x1fff]
  Normal   empty
Movable zone start for each node
Early memory node ranges
  node   0: [mem 0x-0x1fff]
On node 0 totalpages: 131072
free_area_init_node: node 0, pgdat c054c700, node_mem_map c07fd000
  DMA zone: 1024 pages used for memmap
  DMA zone: 0 pages reserved
  DMA zone: 131072 pages, LIFO batch:31
pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
pcpu-alloc: [0] 0
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 130048
Kernel command line: root=/dev/nfs
nfsroot=192.168.1.1:/srv/nfs_powerpc
ip=192.168.1.4::192.168.1.1:255.255.255.0::eth0:off panic=10
console=ttyS0,115200 selinux=0
PID hash table entries: 2048 (order: 1, 8192 bytes)
Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
Sorting __ex_table...
Memory: 514080K/524288K available (4156K kernel code, 268K rwdata,
880K rodata, 188K init, 116K bss, 10208K reserved)
Kernel virtual memory layout:
  * 0xfffdf000..0xf000  : fixmap
  * 0xfdffc000..0xfe00  : early ioremap
  * 0xe100..0xfdffc000  : vmalloc & ioremap
Preemptible hierarchical RCU implementation.
NR_IRQS:512 nr_irqs:512 16
IPIC (128 IRQ sources) at e1000700
time_init: decrementer frequency = 33.00 MHz
time_init: processor frequency   = 330.00 MHz
clocksource: timebase mult[1e4d9365] shift[24] registered
clockevent: decrementer mult[872b021] shift[32] cpu[0]
pid_max: default: 32768 minimum: 301
Security Framework initialized
SELinux:  Disabled at boot.
Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
NET: Registered protocol family 16
Registering ipic system core operations
Freescale Elo series DMA driver
SCSI subsystem initialized
usbcore: registered new interface driver usbfs
usbcore: registered new interface driver hub
usbcore: registered new device driver usb
pps_core: LinuxPPS API ver. 1 registered
pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti

PTP clock support registered
EDAC MC: Ver: 3.0.0
Switched to clocksource timebase
NET: Registered protocol family 2
TCP established hash table entries: 4096 (order: 2, 16384 bytes)
TCP bind hash table entries: 4096 (order: 2, 16384 bytes)
TCP: Hash tables configured (established 4096 bind 4096)
TCP: reno registered
UDP hash table entries: 256 (order: 0, 4096 bytes)
UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
NET: Registered protocol family 1
RPC: Registered named UNIX socket transport module.
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
futex hash table entries: 256 (order: -1, 3072 bytes)
audit: initializing netlink subsys (disabled)
audit: type=2000 audit(0.217:1): initialized
msgmni has been set to 1004
alg: No test for stdrng (krng)
io scheduler noop registered (default)
Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
console [ttyS0] disabled
serial8250.0: ttyS0 at MMIO 0xe0004500 (irq = 16, base_baud = 825)
is a 16550A
console [ttyS0] enabled
bootconsole [udbg0] disabled
serial8250.0: ttyS1 at MMIO 0xe0004600 (irq = 17, base_baud = 825)
is a 16550A
fe00.flash: Found 1 x16 devices at 0x0 in 16-bit bank.
Manufacturer ID 0x01 Chip ID 0x002201
Amd/Fujitsu Extended Query Table at 0x0040
  Amd/Fujitsu Extended Query version 1.3.
number of CFI chips: 1
fsl_spi e0007000.spi: master is unqueued, this is deprecated
fsl_spi e0007000.spi: at 0xe10a6000 (irq = 21), CPU mode
bonding: Ethernet Channel Bonding Driver: v3.7.1 (April 27, 2011)
libphy: Freescale PowerQUICC MII Bus: probed
fsl-gianfar e0024000.ethernet: enabled errata workarounds, flags: 0x4
fsl-gianfar e0024000.ethernet eth0: mac: 00:30:a7:07:15:f6
fsl-gianfar e0024000.ethernet eth0: Running with NAPI enabled
fsl-gianfar e0024000.ethernet eth0: RX BD ring size for Q[0]: 256
fsl-gianfar e0024000.ethernet eth0: TX BD ring size for Q[0]: 256
fsl-gianfar e00

Re: [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels

2014-06-18 Thread Johan Hovold
On Wed, Jun 18, 2014 at 07:05:02PM +0300, Janne Kanniainen wrote:
> This driver adds support for USB controlled led panels that exists in
> MSI GT683R laptop
> 
> Signed-off-by: Janne Kanniainen 

Reviewed-by: Johan Hovold 

Thanks, Janne!

Johan

> ---
> Changes in v2:
>   - sorted headers to alphabetic order
>   - using devm_kzalloc
>   - using BIT(n)
>   - using usb_control_msg instead of usb_submit_urb
>   - removing unneeded code
> Changes in v3:
>   - implemented as HID device
>   - some cleanups and bug fixes
> 
> Changes in v4:
>   - more cleanups
>   - support for selecting leds
>   - suppport for selecting status
> 
> Changes in v5:
>   - mode attribute documented under Documentation/ABI
>   - made array for led_classdev
>   - led devices uses now recommended naming scheme
> 
> Changes in v6:
>   - flush_work added
>   - using hid device name instead of hard coded gt683r
>   - allocating name buffers with devm_kzalloc
> 
> Changes in v7:
>   - buf with for fixed
> 
> Changes in v8:
>   - some cleanups and bugs fixed
> 
> Changes in v9:
>   - few style issues fixed
> 
>  .../ABI/testing/sysfs-class-hid-driver-gt683r  |  14 +
>  drivers/hid/Kconfig|  14 +
>  drivers/hid/Makefile   |   1 +
>  drivers/hid/hid-core.c |   1 +
>  drivers/hid/hid-gt683r.c   | 309 
> +
>  drivers/hid/hid-ids.h  |   2 +-
>  drivers/hid/usbhid/hid-quirks.c|   2 +-
>  7 files changed, 341 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>  create mode 100644 drivers/hid/hid-gt683r.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r 
> b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> new file mode 100644
> index 000..317e9d5
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> @@ -0,0 +1,14 @@
> +What:/sys/class/hidraw//device/leds_mode
> +Date:Jun 2014
> +KernelVersion:   3.17
> +Contact: Janne Kanniainen 
> +Description:
> + Set the mode of LEDs
> +
> + 0 - normal
> + 1 - audio
> + 2 - breathing
> +
> + Normal: LEDs are fully on when enabled
> + Audio:  LEDs brightness depends on sound level
> + Breathing: LEDs brightness varies at human breathing rate
> \ No newline at end of file
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 7af9d0b..e2f4590 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -261,6 +261,20 @@ config HOLTEK_FF
> Say Y here if you have a Holtek On Line Grip based game controller
> and want to have force feedback support for it.
>  
> +config HID_GT683R
> + tristate "MSI GT68xR LED support"
> + depends on LEDS_CLASS && USB_HID
> + ---help---
> + Say Y here if you want to enable support for the three MSI GT68xR LEDs
> +
> + This driver support following modes:
> +   - Normal: LEDs are fully on when enabled
> +   - Audio:  LEDs brightness depends on sound level
> +   - Breathing: LEDs brightness varies at human breathing rate
> +
> + Currently the following devices are know to be supported:
> +   - MSI GT683R
> +
>  config HID_HUION
>   tristate "Huion tablets"
>   depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index fc712dd..7129311 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF)+= hid-emsff.o
>  obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
>  obj-$(CONFIG_HID_ELO)+= hid-elo.o
>  obj-$(CONFIG_HID_EZKEY)  += hid-ezkey.o
> +obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
>  obj-$(CONFIG_HID_GYRATION)   += hid-gyration.o
>  obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
>  obj-$(CONFIG_HID_HOLTEK) += hid-holtek-mouse.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index da52279..ec88fdb 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1827,6 +1827,7 @@ static const struct hid_device_id 
> hid_have_special_driver[] = {
>   { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, 
> USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) 
> },
>   { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) 
> },
>   { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, 
> USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, 
> USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
> diff --git a/drivers/hid

[PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels

2014-06-18 Thread Janne Kanniainen
This driver adds support for USB controlled led panels that exists in
MSI GT683R laptop

Signed-off-by: Janne Kanniainen 
---
Changes in v2:
- sorted headers to alphabetic order
- using devm_kzalloc
- using BIT(n)
- using usb_control_msg instead of usb_submit_urb
- removing unneeded code
Changes in v3:
- implemented as HID device
- some cleanups and bug fixes

Changes in v4:
- more cleanups
- support for selecting leds
- suppport for selecting status

Changes in v5:
- mode attribute documented under Documentation/ABI
- made array for led_classdev
- led devices uses now recommended naming scheme

Changes in v6:
- flush_work added
- using hid device name instead of hard coded gt683r
- allocating name buffers with devm_kzalloc

Changes in v7:
- buf with for fixed

Changes in v8:
- some cleanups and bugs fixed

Changes in v9:
- few style issues fixed

 .../ABI/testing/sysfs-class-hid-driver-gt683r  |  14 +
 drivers/hid/Kconfig|  14 +
 drivers/hid/Makefile   |   1 +
 drivers/hid/hid-core.c |   1 +
 drivers/hid/hid-gt683r.c   | 309 +
 drivers/hid/hid-ids.h  |   2 +-
 drivers/hid/usbhid/hid-quirks.c|   2 +-
 7 files changed, 341 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
 create mode 100644 drivers/hid/hid-gt683r.c

diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r 
b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
new file mode 100644
index 000..317e9d5
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
@@ -0,0 +1,14 @@
+What:  /sys/class/hidraw//device/leds_mode
+Date:  Jun 2014
+KernelVersion: 3.17
+Contact:   Janne Kanniainen 
+Description:
+   Set the mode of LEDs
+
+   0 - normal
+   1 - audio
+   2 - breathing
+
+   Normal: LEDs are fully on when enabled
+   Audio:  LEDs brightness depends on sound level
+   Breathing: LEDs brightness varies at human breathing rate
\ No newline at end of file
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 7af9d0b..e2f4590 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -261,6 +261,20 @@ config HOLTEK_FF
  Say Y here if you have a Holtek On Line Grip based game controller
  and want to have force feedback support for it.
 
+config HID_GT683R
+   tristate "MSI GT68xR LED support"
+   depends on LEDS_CLASS && USB_HID
+   ---help---
+   Say Y here if you want to enable support for the three MSI GT68xR LEDs
+
+   This driver support following modes:
+ - Normal: LEDs are fully on when enabled
+ - Audio:  LEDs brightness depends on sound level
+ - Breathing: LEDs brightness varies at human breathing rate
+
+   Currently the following devices are know to be supported:
+ - MSI GT683R
+
 config HID_HUION
tristate "Huion tablets"
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index fc712dd..7129311 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF)  += hid-emsff.o
 obj-$(CONFIG_HID_ELECOM)   += hid-elecom.o
 obj-$(CONFIG_HID_ELO)  += hid-elo.o
 obj-$(CONFIG_HID_EZKEY)+= hid-ezkey.o
+obj-$(CONFIG_HID_GT683R)   += hid-gt683r.o
 obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
 obj-$(CONFIG_HID_HOLTEK)   += hid-holtek-kbd.o
 obj-$(CONFIG_HID_HOLTEK)   += hid-holtek-mouse.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index da52279..ec88fdb 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1827,6 +1827,7 @@ static const struct hid_device_id 
hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, 
USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
+   { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) 
},
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) 
},
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, 
USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, 
USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
new file mode 100644
index 000..077f7a1
--- /dev/null
+++ b/drivers/hid/hid-gt683r.c
@@ -0,0 +1,309 @@
+/*
+ * MSI GT683R led driver
+ *
+ * Copyright (c) 2014 Janne Kanniainen 
+ *
+ * This program is free software; you can redistribute it and/o

Re: [PATCH 3/3] usb: fix hub-port pm_runtime_enable() vs runtime pm transitions

2014-06-18 Thread Alan Stern
On Wed, 18 Jun 2014, Dan Williams wrote:

> On Wed, Jun 18, 2014 at 7:54 AM, Alan Stern  wrote:
> > On Tue, 17 Jun 2014, Dan Williams wrote:
> >
> >> Commit 9262c19d14c4 "usb: disable port power control if not supported in
> >> wHubCharacteristics" gated enabling runtime pm for usb_port devices on
> >> whether the parent hub supports power control, which causes a
> >> regression.  The port must still be allowed to carry out runtime pm
> >> callbacks and receive a -EAGAIN or -EBUSY result.  Otherwise the
> >> usb_port device will transition to the pm error state and trigger the
> >> same for the child usb_device.
> >>
> >> Prior to the offending commit usb_hub_create_port_device() arranged for
> >> runtime pm to be disabled is dev_pm_qos_expose_flags() failed.  Instead,
> >> force the default state of PM_QOS_FLAG_NO_POWER_OFF flag to be set prior
> >> to enabling runtime pm.  If that policy can not be set then fail
> >> registration.
> >
> > This whole thing seems much more complicated than necessary.
> >
> > Instead of messing around with PM-QOS settings, why not just do an
> > extra pm_runtime_get_noresume() if the hub doesn't support power
> > control?
> 
> In that scenario userspace has to fishing for why the port is not
> powering off vs no "pm_qos_no_power_off" == no capability.

Can't the user change that flag back again?

> > Or, what about leaving this code the way it was and changing the PM
> > core (see http://marc.info/?l=linux-pm&m=140303690820354&w=2)?
> 
> I like this approach too, but was not sure a core change like that
> would be accepted now that -rc1 is out.

That's up to Rafael.  He'd probably agree if we explained that it would
fix a bug.  Alternatively, the code following the relevant
pm_runtime_get_sync() could explicitly check for -EACCES -- but I don't
think that's such a good solution.

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 3/3] usb: fix hub-port pm_runtime_enable() vs runtime pm transitions

2014-06-18 Thread Dan Williams
On Wed, Jun 18, 2014 at 7:54 AM, Alan Stern  wrote:
> On Tue, 17 Jun 2014, Dan Williams wrote:
>
>> Commit 9262c19d14c4 "usb: disable port power control if not supported in
>> wHubCharacteristics" gated enabling runtime pm for usb_port devices on
>> whether the parent hub supports power control, which causes a
>> regression.  The port must still be allowed to carry out runtime pm
>> callbacks and receive a -EAGAIN or -EBUSY result.  Otherwise the
>> usb_port device will transition to the pm error state and trigger the
>> same for the child usb_device.
>>
>> Prior to the offending commit usb_hub_create_port_device() arranged for
>> runtime pm to be disabled is dev_pm_qos_expose_flags() failed.  Instead,
>> force the default state of PM_QOS_FLAG_NO_POWER_OFF flag to be set prior
>> to enabling runtime pm.  If that policy can not be set then fail
>> registration.
>
> This whole thing seems much more complicated than necessary.
>
> Instead of messing around with PM-QOS settings, why not just do an
> extra pm_runtime_get_noresume() if the hub doesn't support power
> control?

In that scenario userspace has to fishing for why the port is not
powering off vs no "pm_qos_no_power_off" == no capability.

> Or, what about leaving this code the way it was and changing the PM
> core (see http://marc.info/?l=linux-pm&m=140303690820354&w=2)?

I like this approach too, but was not sure a core change like that
would be accepted now that -rc1 is out.
--
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: Fix a warning while loading g_mass_storage

2014-06-18 Thread Alan Stern
On Wed, 18 Jun 2014, Michal Nazarewicz wrote:

> On Wed, Jun 18 2014, Yang,Wei wrote:
> > On 06/17/2014 10:18 PM, Alan Stern wrote:
> >> That is a strange question to ask.  If you did not know that I approved
> >> the patch, why did you insert my Acked-By:?
> 
> > I added your Acked-By, as when you reviewed V3, you mentioned that I 
> > *may* add your Acked-by in this patch. If I misunderstood your point, I 
> > am so sorry for that.
> 
> Alan's point is that if you have any doubts whether someone approves
> your patch you should *not* add their Acked-by.  So adding someone's
> Acked-by and then asking if they are fine with the patch is
> contradictory -- the former indicates that you believe the person is
> fine with the patch, while the latter shows that you aren't.
> 
> Alan wrote:
> 
> >>> With that change, you may add
> >>>
> >>> Acked-by: Alan Stern 
> 
> so after adding “that change” you are in your right to add Alan's
> Acked-by, but what that also means is that Alan will be fine with the
> patch if you make the requested change, so you don't need to ask for an
> opinion again.
> 
> PS. Note that “having any doubts” also means that if someone gave you
> their Acked-by, but then you substantially rewrite the patch, you
> probably should consider removing their Acked-by.

Exactly so.

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 3/3] usb: fix hub-port pm_runtime_enable() vs runtime pm transitions

2014-06-18 Thread Alan Stern
On Tue, 17 Jun 2014, Dan Williams wrote:

> Commit 9262c19d14c4 "usb: disable port power control if not supported in
> wHubCharacteristics" gated enabling runtime pm for usb_port devices on
> whether the parent hub supports power control, which causes a
> regression.  The port must still be allowed to carry out runtime pm
> callbacks and receive a -EAGAIN or -EBUSY result.  Otherwise the
> usb_port device will transition to the pm error state and trigger the
> same for the child usb_device.
> 
> Prior to the offending commit usb_hub_create_port_device() arranged for
> runtime pm to be disabled is dev_pm_qos_expose_flags() failed.  Instead,
> force the default state of PM_QOS_FLAG_NO_POWER_OFF flag to be set prior
> to enabling runtime pm.  If that policy can not be set then fail
> registration.

This whole thing seems much more complicated than necessary.

Instead of messing around with PM-QOS settings, why not just do an 
extra pm_runtime_get_noresume() if the hub doesn't support power 
control?

Or, what about leaving this code the way it was and changing the PM
core (see http://marc.info/?l=linux-pm&m=140303690820354&w=2)?

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 2/3] usb: quiet peer failure warning, disable poweroff

2014-06-18 Thread Alan Stern
On Tue, 17 Jun 2014, Dan Williams wrote:

> In the case where platform firmware has specified conflicting values for
> port locations it is confusing and otherwise not helpful to throw a
> backtrace.  Instead, include enough information to determine that
> firmware has done something wrong and globally disable port poweroff.


> @@ -251,6 +264,7 @@ static void link_peers_report(struct usb_port *left, 
> struct usb_port *right)
>   dev_warn(&left->dev, "failed to peer to %s (%d)\n",
>   dev_name(&right->dev), rc);
>   pr_warn_once("usb: port power management may be unreliable\n");
> + usb_port_block_power_off = 1;
>   }
>  }

Ironically, since you've got usb_port_block_power_off, you don't need 
to make this pr_warn_once().  Instead, you can do

if (!usb_port_block_power_off) {
usb_port_block_power_off = 1;
pr_warn(...);
}

which is pretty much what pr_warn_once() does.

Furthermore, since power poweroff is now globally disabled, you can say 
something stronger than "may be unreliable", such as "has been 
disabled".

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 0/3] port power control fixes for 3.16-rc2

2014-06-18 Thread Dan Williams
On Wed, Jun 18, 2014 at 1:18 AM, Bjørn Mork  wrote:
> Dan Williams  writes:
>
>> I put patch 3 "usb: fix hub-port pm_runtime_enable() vs runtime pm
>> transitions" through it's paces, but I'd still like to see a positive
>> test report from Bjørn... even if it's too late to add a "Tested-by".
>
> Fixes the problem for me.  Thanks
>

Thanks for the report and the test.  Much appreciated!
--
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: xhci handling ring expansion

2014-06-18 Thread David Laight
From: vichy
...
> and I get error message as
> platform-xhci: ERROR Transfer event TRB DMA ptr 483702160 not part of current 
> TD
> platform-xhci: trb_comp_code = 0x1, event status -115, buffer
> 1cd4b590, len 100, flags 1038001
> 
> that seems wired.
> complete code is success and not short package.
> Why we still get TRB DMA will not be part of current TD?

I would trace all the setup requests and responses and then work
out what is expected and then missing.

David



Re: [PATCH net,stable] net: huawei_cdc_ncm: increase command buffer size

2014-06-18 Thread Dan Williams
On Wed, 2014-06-18 at 14:21 +0200, Bjørn Mork wrote:
> Messages from the modem exceeding 256 bytes cause communication
> failure.
> 
> The WDM protocol is strictly "read on demand", meaning that we only
> poll for unread data after receiving a notification from the modem.
> Since we have no way to know how much data the modem has to send,
> we must make sure that the buffer we provide is "big enough".
> Message truncation does not work. Truncated messages are left unread
> until the modem has another message to send.  Which often won't
> happen until the userspace application has given up waiting for the
> final part of the last message, and therefore sends another command.
> 
> With a proper CDC WDM function there is a descriptor telling us
> which buffer size the modem uses. But with this vendor specific
> implementation there is no known way to calculate the exact "big
> enough" number.  It is an unknown property of the modem firmware.
> Experience has shown that 256 is too small.  The discussion of
> this failure ended up concluding that 512 might be too small as
> well. So 1024 seems like a reasonable value for now.
> 
> Fixes: 41c47d8cfd68 ("net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm 
> driver")
> Cc: Enrico Mioso 
> Reported-by: Dan Williams 
> Signed-off-by: Bjørn Mork 

Tested-by: Dan Williams 

'^SYSCFGEX:
("00","01","02","03","99"),((400380,"GSM900/GSM1800/WCDMA2100"),(6a8,"GSM850/GSM1900/WCDMA850/AWS/WCDMA1900"),(3fff,"All
 
bands")),(0-2),(0-4),((1081b,"LTE_B1/LTE_B2/LTE_B4/LTE_B5/LTE_B12/LTE_B17"),(7fff,"All
 bands"))OK'

I get the last "" now :)

> ---
> 
> The problem is a showstopper for anyone hitting it, so I believe this
> fix should go into all maintained stable kernels with this driver.
> That is anything based on v3.13 or newer.
> 
> Thanks,
> Bjørn
> 
> 
>  drivers/net/usb/huawei_cdc_ncm.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/usb/huawei_cdc_ncm.c 
> b/drivers/net/usb/huawei_cdc_ncm.c
> index f9822bc75425..5d95a13dbe2a 100644
> --- a/drivers/net/usb/huawei_cdc_ncm.c
> +++ b/drivers/net/usb/huawei_cdc_ncm.c
> @@ -84,12 +84,13 @@ static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
>   ctx = drvstate->ctx;
>  
>   if (usbnet_dev->status)
> - /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256
> -  * decimal (0x100)"
> + /* The wMaxCommand buffer must be big enough to hold
> +  * any message from the modem. Experience has shown
> +  * that some replies are more than 256 bytes long
>*/
>   subdriver = usb_cdc_wdm_register(ctx->control,
>&usbnet_dev->status->desc,
> -  256, /* wMaxCommand */
> +  1024, /* wMaxCommand */
>
> huawei_cdc_ncm_wdm_manage_power);
>   if (IS_ERR(subdriver)) {
>   ret = PTR_ERR(subdriver);


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


Re: [PATCH net,stable] net: huawei_cdc_ncm: increase command buffer size

2014-06-18 Thread Enrico Mioso
Oh - I forgot to say: I can provide a tested-by in some hours. I have my E3131 
at something like 2.1  KM going by feets, from here at University. 

On Wed, 18 Jun 2014, Bjørn Mork wrote:

==Date: Wed, 18 Jun 2014 14:21:24
==From: Bjørn Mork 
==To: net...@vger.kernel.org
==Cc: linux-usb@vger.kernel.org, Bjørn Mork ,
==Enrico Mioso 
==Subject: [PATCH net,stable] net: huawei_cdc_ncm: increase command buffer size
==
==Messages from the modem exceeding 256 bytes cause communication
==failure.
==
==The WDM protocol is strictly "read on demand", meaning that we only
==poll for unread data after receiving a notification from the modem.
==Since we have no way to know how much data the modem has to send,
==we must make sure that the buffer we provide is "big enough".
==Message truncation does not work. Truncated messages are left unread
==until the modem has another message to send.  Which often won't
==happen until the userspace application has given up waiting for the
==final part of the last message, and therefore sends another command.
==
==With a proper CDC WDM function there is a descriptor telling us
==which buffer size the modem uses. But with this vendor specific
==implementation there is no known way to calculate the exact "big
==enough" number.  It is an unknown property of the modem firmware.
==Experience has shown that 256 is too small.  The discussion of
==this failure ended up concluding that 512 might be too small as
==well. So 1024 seems like a reasonable value for now.
==
==Fixes: 41c47d8cfd68 ("net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm 
driver")
==Cc: Enrico Mioso 
==Reported-by: Dan Williams 
==Signed-off-by: Bjørn Mork 
==---
==
==The problem is a showstopper for anyone hitting it, so I believe this
==fix should go into all maintained stable kernels with this driver.
==That is anything based on v3.13 or newer.
==
==Thanks,
==Bjørn
==
==
== drivers/net/usb/huawei_cdc_ncm.c | 7 ---
== 1 file changed, 4 insertions(+), 3 deletions(-)
==
==diff --git a/drivers/net/usb/huawei_cdc_ncm.c 
b/drivers/net/usb/huawei_cdc_ncm.c
==index f9822bc75425..5d95a13dbe2a 100644
==--- a/drivers/net/usb/huawei_cdc_ncm.c
==+++ b/drivers/net/usb/huawei_cdc_ncm.c
==@@ -84,12 +84,13 @@ static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
==  ctx = drvstate->ctx;
== 
==  if (usbnet_dev->status)
==- /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256
==-  * decimal (0x100)"
==+ /* The wMaxCommand buffer must be big enough to hold
==+  * any message from the modem. Experience has shown
==+  * that some replies are more than 256 bytes long
==   */
==  subdriver = usb_cdc_wdm_register(ctx->control,
==   &usbnet_dev->status->desc,
==-  256, /* wMaxCommand */
==+  1024, /* wMaxCommand */
==   
huawei_cdc_ncm_wdm_manage_power);
==  if (IS_ERR(subdriver)) {
==  ret = PTR_ERR(subdriver);
==-- 
==2.0.0
==
==

Re: [PATCH net,stable] net: huawei_cdc_ncm: increase command buffer size

2014-06-18 Thread Enrico Mioso
I think this is good, even if I admit I am starting to feel pretty 
unconfortable with this situation where a firmware is using an unknown 
combination of two protocols, not respecting both AT spec and CDC WDM one.
That may explain why the device refuses some commands sometimes, such as
at+clac
.

I hope I'll be able to come back at this driver some day and try to figure out 
something more.
I can not promise anything - but I would like to modify cdc-wdm as per our last 
discussion (and I might ask you to point me to the right message from Oliver 
again).
Regarding the buffer size - I do not know if there is a way to obtain it. Might 
USB sniffing help in this, to see if we are receiving the data in some 
unexpected location?
I am asking myself how this thing is implemented in Windows Huawei drivers: 
just out of curiosity.
i can tell also - i experienced some crash with E3251 where I had to close and 
re-open the seiral port, but this usually happened after simple
AT
commands, and it seemd a problem with minicom, but it was only an impression, 
not confirmed by anything, so it can very easily wrong. I like to play and try 
to understand.

So - I acknoledge the patch.
Please can you CC Oliver so that he knows?


Acked-By: Enrico Mioso 

On Wed, 18 Jun 2014, Bjørn Mork wrote:

==Date: Wed, 18 Jun 2014 14:21:24
==From: Bjørn Mork 
==To: net...@vger.kernel.org
==Cc: linux-usb@vger.kernel.org, Bjørn Mork ,
==Enrico Mioso 
==Subject: [PATCH net,stable] net: huawei_cdc_ncm: increase command buffer size
==
==Messages from the modem exceeding 256 bytes cause communication
==failure.
==
==The WDM protocol is strictly "read on demand", meaning that we only
==poll for unread data after receiving a notification from the modem.
==Since we have no way to know how much data the modem has to send,
==we must make sure that the buffer we provide is "big enough".
==Message truncation does not work. Truncated messages are left unread
==until the modem has another message to send.  Which often won't
==happen until the userspace application has given up waiting for the
==final part of the last message, and therefore sends another command.
==
==With a proper CDC WDM function there is a descriptor telling us
==which buffer size the modem uses. But with this vendor specific
==implementation there is no known way to calculate the exact "big
==enough" number.  It is an unknown property of the modem firmware.
==Experience has shown that 256 is too small.  The discussion of
==this failure ended up concluding that 512 might be too small as
==well. So 1024 seems like a reasonable value for now.
==
==Fixes: 41c47d8cfd68 ("net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm 
driver")
==Cc: Enrico Mioso 
==Reported-by: Dan Williams 
==Signed-off-by: Bjørn Mork 
==---
==
==The problem is a showstopper for anyone hitting it, so I believe this
==fix should go into all maintained stable kernels with this driver.
==That is anything based on v3.13 or newer.
==
==Thanks,
==Bjørn
==
==
== drivers/net/usb/huawei_cdc_ncm.c | 7 ---
== 1 file changed, 4 insertions(+), 3 deletions(-)
==
==diff --git a/drivers/net/usb/huawei_cdc_ncm.c 
b/drivers/net/usb/huawei_cdc_ncm.c
==index f9822bc75425..5d95a13dbe2a 100644
==--- a/drivers/net/usb/huawei_cdc_ncm.c
==+++ b/drivers/net/usb/huawei_cdc_ncm.c
==@@ -84,12 +84,13 @@ static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
==  ctx = drvstate->ctx;
== 
==  if (usbnet_dev->status)
==- /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256
==-  * decimal (0x100)"
==+ /* The wMaxCommand buffer must be big enough to hold
==+  * any message from the modem. Experience has shown
==+  * that some replies are more than 256 bytes long
==   */
==  subdriver = usb_cdc_wdm_register(ctx->control,
==   &usbnet_dev->status->desc,
==-  256, /* wMaxCommand */
==+  1024, /* wMaxCommand */
==   
huawei_cdc_ncm_wdm_manage_power);
==  if (IS_ERR(subdriver)) {
==  ret = PTR_ERR(subdriver);
==-- 
==2.0.0
==
==

Re: xhci handling ring expansion

2014-06-18 Thread vichy
hi David:

2014-06-18 16:45 GMT+08:00 David Laight :
> From: vichy
>> hi david:
> ...
>> > From one of the patches (not applied) I sent last November ...
>> > Include the unknown address when the DMA pointer from an event isn't part
>> > of the current TD. This will happen if the error code is "TRB error".
>
>> do you mean below patch?
>> http://markmail.org/message/74qvwz7fhjxqeih3
>> it only add debug message instead of fixing it.
>>
>> appreciate your help,
>
> In my case there was a coding error in one of the other xhci
> patches that caused a 'TRB error' report.
> The patch to change the tracing was an attempt to make things
> less confusing.
>
> If you modify the tracing you might find out where the pointer from
> the event came from.
I modify your patch like below
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7f76a49..17d5ad0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2456,6 +2456,11 @@ static int handle_tx_event(struct xhci_hcd *xhci,
goto cleanup;
}

+   if (status && status != -EINPROGRESS)
+   xhci_dbg(xhci, "event status %d, buffer %llx,
len %x, flags %x\n",
+   status,
event->buffer, event->transfer_len,
+   event->flags);
+
do {
/* This TRB should be in the TD at the head of this ring's
 * TD list.
@@ -2522,9 +2527,10 @@ static int handle_tx_event(struct xhci_hcd *xhci,
goto cleanup;
}
/* HC is busted, give up! */
-   xhci_err(xhci,
-   "ERROR Transfer event TRB DMA ptr not "
-   "part of current TD\n");
+   xhci_err(xhci, "ERROR Transfer event
TRB DMA ptr %lld not part of current TD\n",event->buffer);
+   xhci_err(xhci, "trb_comp_code = 0x%x,
event status %d, buffer %llx, len %x, flags %x\n",trb_comp_code,
+
status, event->buffer, event->transfer_len,
+   event->flags);
return -ESHUTDOWN;
}

and I get error message as
platform-xhci: ERROR Transfer event TRB DMA ptr 483702160 not part of current TD
platform-xhci: trb_comp_code = 0x1, event status -115, buffer
1cd4b590, len 100, flags 1038001

that seems wired.
complete code is success and not short package.
Why we still get TRB DMA will not be part of current TD?

appreciate your help,
--
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][fixes 1/2] usb: gadget: OS descriptors configfs cleanup

2014-06-18 Thread Andrzej Pietrasiewicz
A number of variables serve a generic purpose of handling
"compatible id" and "subcompatible id", but the names suggest they
are for rndis only. Rename to reflect variables' purpose.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/configfs.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 2ddcd63..fadd6be 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1145,15 +1145,15 @@ static struct configfs_item_operations interf_item_ops 
= {
.store_attribute= usb_os_desc_attr_store,
 };
 
-static ssize_t rndis_grp_compatible_id_show(struct usb_os_desc *desc,
-   char *page)
+static ssize_t interf_grp_compatible_id_show(struct usb_os_desc *desc,
+char *page)
 {
memcpy(page, desc->ext_compat_id, 8);
return 8;
 }
 
-static ssize_t rndis_grp_compatible_id_store(struct usb_os_desc *desc,
-const char *page, size_t len)
+static ssize_t interf_grp_compatible_id_store(struct usb_os_desc *desc,
+ const char *page, size_t len)
 {
int l;
 
@@ -1171,20 +1171,20 @@ static ssize_t rndis_grp_compatible_id_store(struct 
usb_os_desc *desc,
return len;
 }
 
-static struct usb_os_desc_attribute rndis_grp_attr_compatible_id =
+static struct usb_os_desc_attribute interf_grp_attr_compatible_id =
__CONFIGFS_ATTR(compatible_id, S_IRUGO | S_IWUSR,
-   rndis_grp_compatible_id_show,
-   rndis_grp_compatible_id_store);
+   interf_grp_compatible_id_show,
+   interf_grp_compatible_id_store);
 
-static ssize_t rndis_grp_sub_compatible_id_show(struct usb_os_desc *desc,
-   char *page)
+static ssize_t interf_grp_sub_compatible_id_show(struct usb_os_desc *desc,
+char *page)
 {
memcpy(page, desc->ext_compat_id + 8, 8);
return 8;
 }
 
-static ssize_t rndis_grp_sub_compatible_id_store(struct usb_os_desc *desc,
-const char *page, size_t len)
+static ssize_t interf_grp_sub_compatible_id_store(struct usb_os_desc *desc,
+ const char *page, size_t len)
 {
int l;
 
@@ -1202,14 +1202,14 @@ static ssize_t rndis_grp_sub_compatible_id_store(struct 
usb_os_desc *desc,
return len;
 }
 
-static struct usb_os_desc_attribute rndis_grp_attr_sub_compatible_id =
+static struct usb_os_desc_attribute interf_grp_attr_sub_compatible_id =
__CONFIGFS_ATTR(sub_compatible_id, S_IRUGO | S_IWUSR,
-   rndis_grp_sub_compatible_id_show,
-   rndis_grp_sub_compatible_id_store);
+   interf_grp_sub_compatible_id_show,
+   interf_grp_sub_compatible_id_store);
 
 static struct configfs_attribute *interf_grp_attrs[] = {
-   &rndis_grp_attr_compatible_id.attr,
-   &rndis_grp_attr_sub_compatible_id.attr,
+   &interf_grp_attr_compatible_id.attr,
+   &interf_grp_attr_sub_compatible_id.attr,
NULL
 };
 
-- 
1.8.3.2

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


[PATCH][fixes 0/2] OS descriptors fixes for 3.16

2014-06-18 Thread Andrzej Pietrasiewicz
This short series contains two patches intended for 3.16:

1/2: change some variable (and function) names to reflect their actual purpose
2/2: eliminate dependency on yet unknown interface numbers

Rebased onto Greg's usb-next with this series:

http://www.spinics.net/lists/linux-usb/msg109256.html

applied first.

Andrzej Pietrasiewicz (2):
  usb: gadget: OS descriptors configfs cleanup
  usb: gadget: OS descriptors: provide interface directory names

 drivers/usb/gadget/configfs.c | 37 ++-
 drivers/usb/gadget/configfs.h |  1 +
 drivers/usb/gadget/function/f_rndis.c |  4 +++-
 3 files changed, 23 insertions(+), 19 deletions(-)

-- 
1.8.3.2

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


[PATCH][fixes 2/2] usb: gadget: OS descriptors: provide interface directory names

2014-06-18 Thread Andrzej Pietrasiewicz
Function's interface directories need to be created when the function
directory is created, but interface numbers are not known until
the gadget is ready and bound to udc, so we cannot use numbers
as part of interface directory names.
Let the client decide what names to use.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/configfs.c | 5 +++--
 drivers/usb/gadget/configfs.h | 1 +
 drivers/usb/gadget/function/f_rndis.c | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index fadd6be..9714214 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1216,6 +1216,7 @@ static struct configfs_attribute *interf_grp_attrs[] = {
 int usb_os_desc_prepare_interf_dir(struct config_group *parent,
   int n_interf,
   struct usb_os_desc **desc,
+  char **names,
   struct module *owner)
 {
struct config_group **f_default_groups, *os_desc_group,
@@ -1257,8 +1258,8 @@ int usb_os_desc_prepare_interf_dir(struct config_group 
*parent,
d = desc[n_interf];
d->owner = owner;
config_group_init_type_name(&d->group, "", interface_type);
-   config_item_set_name(&d->group.cg_item, "interface.%d",
-n_interf);
+   config_item_set_name(&d->group.cg_item, "interface.%s",
+names[n_interf]);
interface_groups[n_interf] = &d->group;
}
 
diff --git a/drivers/usb/gadget/configfs.h b/drivers/usb/gadget/configfs.h
index a14ac79..36c468c 100644
--- a/drivers/usb/gadget/configfs.h
+++ b/drivers/usb/gadget/configfs.h
@@ -8,6 +8,7 @@ void unregister_gadget_item(struct config_item *item);
 int usb_os_desc_prepare_interf_dir(struct config_group *parent,
   int n_interf,
   struct usb_os_desc **desc,
+  char **names,
   struct module *owner);
 
 static inline struct usb_os_desc *to_usb_os_desc(struct config_item *item)
diff --git a/drivers/usb/gadget/function/f_rndis.c 
b/drivers/usb/gadget/function/f_rndis.c
index eed3ad8..1d1806a 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -905,6 +905,7 @@ static struct usb_function_instance *rndis_alloc_inst(void)
 {
struct f_rndis_opts *opts;
struct usb_os_desc *descs[1];
+   char *names[1];
 
opts = kzalloc(sizeof(*opts), GFP_KERNEL);
if (!opts)
@@ -922,8 +923,9 @@ static struct usb_function_instance *rndis_alloc_inst(void)
INIT_LIST_HEAD(&opts->rndis_os_desc.ext_prop);
 
descs[0] = &opts->rndis_os_desc;
+   names[0] = "rndis";
usb_os_desc_prepare_interf_dir(&opts->func_inst.group, 1, descs,
-  THIS_MODULE);
+  names, THIS_MODULE);
config_group_init_type_name(&opts->func_inst.group, "",
&rndis_func_type);
 
-- 
1.8.3.2

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


[PATCH net,stable] net: huawei_cdc_ncm: increase command buffer size

2014-06-18 Thread Bjørn Mork
Messages from the modem exceeding 256 bytes cause communication
failure.

The WDM protocol is strictly "read on demand", meaning that we only
poll for unread data after receiving a notification from the modem.
Since we have no way to know how much data the modem has to send,
we must make sure that the buffer we provide is "big enough".
Message truncation does not work. Truncated messages are left unread
until the modem has another message to send.  Which often won't
happen until the userspace application has given up waiting for the
final part of the last message, and therefore sends another command.

With a proper CDC WDM function there is a descriptor telling us
which buffer size the modem uses. But with this vendor specific
implementation there is no known way to calculate the exact "big
enough" number.  It is an unknown property of the modem firmware.
Experience has shown that 256 is too small.  The discussion of
this failure ended up concluding that 512 might be too small as
well. So 1024 seems like a reasonable value for now.

Fixes: 41c47d8cfd68 ("net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver")
Cc: Enrico Mioso 
Reported-by: Dan Williams 
Signed-off-by: Bjørn Mork 
---

The problem is a showstopper for anyone hitting it, so I believe this
fix should go into all maintained stable kernels with this driver.
That is anything based on v3.13 or newer.

Thanks,
Bjørn


 drivers/net/usb/huawei_cdc_ncm.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index f9822bc75425..5d95a13dbe2a 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -84,12 +84,13 @@ static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
ctx = drvstate->ctx;
 
if (usbnet_dev->status)
-   /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256
-* decimal (0x100)"
+   /* The wMaxCommand buffer must be big enough to hold
+* any message from the modem. Experience has shown
+* that some replies are more than 256 bytes long
 */
subdriver = usb_cdc_wdm_register(ctx->control,
 &usbnet_dev->status->desc,
-256, /* wMaxCommand */
+1024, /* wMaxCommand */
 
huawei_cdc_ncm_wdm_manage_power);
if (IS_ERR(subdriver)) {
ret = PTR_ERR(subdriver);
-- 
2.0.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] USB:gadget: Fix a warning while loading g_mass_storage

2014-06-18 Thread Michal Nazarewicz
On Wed, Jun 18 2014, Yang,Wei wrote:
> On 06/17/2014 10:18 PM, Alan Stern wrote:
>> That is a strange question to ask.  If you did not know that I approved
>> the patch, why did you insert my Acked-By:?

> I added your Acked-By, as when you reviewed V3, you mentioned that I 
> *may* add your Acked-by in this patch. If I misunderstood your point, I 
> am so sorry for that.

Alan's point is that if you have any doubts whether someone approves
your patch you should *not* add their Acked-by.  So adding someone's
Acked-by and then asking if they are fine with the patch is
contradictory -- the former indicates that you believe the person is
fine with the patch, while the latter shows that you aren't.

Alan wrote:

>>> With that change, you may add
>>>
>>> Acked-by: Alan Stern 

so after adding “that change” you are in your right to add Alan's
Acked-by, but what that also means is that Alan will be fine with the
patch if you make the requested change, so you don't need to ask for an
opinion again.

PS. Note that “having any doubts” also means that if someone gave you
their Acked-by, but then you substantially rewrite the patch, you
probably should consider removing their Acked-by.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
--
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


move ZTE CDMA device pid from zte_ev.c back to option.c

2014-06-18 Thread 刘磊

dear linuxfoundation:
Because of the usb driver parameters error that leads to failed in 
reconnect. now i want to modify the error parameter and 
move device pid fffe from zte_ev.c back to option.c for our company.


modify reason:
1. Move device pid fffe from zte_ev.c back to option.c for our company.
2. Modify the parameter from 0x0003 to 0x. the problem may cause the device 
can not be close. 

these two points are in the patch, please submit it for me. thanks.





Signed-off-by:lei liu
diff -u -r drivers-old/usb/serial/option.c drivers/usb/serial/option.c
--- drivers-old/usb/serial/option.c 2014-06-17 04:44:27.0 +0800
+++ drivers/usb/serial/option.c 2014-06-18 15:19:21.936561280 +0800
@@ -1542,6 +1542,7 @@
{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x02, 0x05) },
{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x86, 0x10) },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AC2726, 
0xff, 0xff, 0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xfffe, 0xff, 0xff, 
0xff) },
 
{ USB_DEVICE(BENQ_VENDOR_ID, BENQ_PRODUCT_H10) },
{ USB_DEVICE(DLINK_VENDOR_ID, DLINK_PRODUCT_DWM_652) },
Only in drivers/usb/serial: option.c~
diff -u -r drivers-old/usb/serial/zte_ev.c drivers/usb/serial/zte_ev.c
--- drivers-old/usb/serial/zte_ev.c 2014-06-17 04:44:27.0 +0800
+++ drivers/usb/serial/zte_ev.c 2014-06-18 15:03:47.176597158 +0800
@@ -257,12 +257,12 @@
 
/* send 8th cmd */
/*
-* 16.0 CTL21 22 03 00  00 00 00 00
+* 16.0 CTL21 22 00 00  00 00 00 00
 */
len = 0;
result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 0x22, 0x21,
-0x0003, 0x, NULL, len,
+0x, 0x, NULL, len,
 USB_CTRL_GET_TIMEOUT);
dev_dbg(dev, "result = %d\n", result);
 
@@ -274,8 +274,6 @@
 static const struct usb_device_id id_table[] = {
/* AC8710, AC8710T */
{ USB_DEVICE_AND_INTERFACE_INFO(0x19d2, 0x, 0xff, 0xff, 0xff) },
-/* AC8700 */
-   { USB_DEVICE_AND_INTERFACE_INFO(0x19d2, 0xfffe, 0xff, 0xff, 0xff) },
/* MG880 */
{ USB_DEVICE(0x19d2, 0xfffd) },
{ USB_DEVICE(0x19d2, 0xfffc) },
Only in drivers/usb/serial: zte_ev.c~





N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: [PATCH v3 5/7] ARM: DRA7: hwmod: Add SYSCONFIG for usb_otg_ss

2014-06-18 Thread Roger Quadros
On 06/18/2014 02:19 PM, Rajendra Nayak wrote:
> On Wednesday 18 June 2014 04:40 PM, Roger Quadros wrote:
>> + Nishant and Rajendra for review.
>>
>> On 05/05/2014 12:54 PM, Roger Quadros wrote:
>>> Add the sysconfig class bits for the Super Speed USB
>>> controllers
>>>
>>> CC: Paul Walmsley 
>>> Signed-off-by: Roger Quadros 
> 
> verified against TRM version vP, looks good to me.
> Reviewed-by: Rajendra Nayak 

Tested-by: Roger Quadros 

against 3.16-rc1. no dependency patches.

cheers,
-roger

> 
>>> ---
>>>  arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 12 
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c 
>>> b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> index 810c205..067d322 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> @@ -1731,8 +1731,20 @@ static struct omap_hwmod dra7xx_uart6_hwmod = {
>>>   *
>>>   */
>>>  
>>> +static struct omap_hwmod_class_sysconfig dra7xx_usb_otg_ss_sysc = {
>>> +   .rev_offs   = 0x,
>>> +   .sysc_offs  = 0x0010,
>>> +   .sysc_flags = (SYSC_HAS_DMADISABLE | SYSC_HAS_MIDLEMODE |
>>> +  SYSC_HAS_SIDLEMODE),
>>> +   .idlemodes  = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
>>> +  SIDLE_SMART_WKUP | MSTANDBY_FORCE | MSTANDBY_NO |
>>> +  MSTANDBY_SMART | MSTANDBY_SMART_WKUP),
>>> +   .sysc_fields= &omap_hwmod_sysc_type2,
>>> +};
>>> +
>>>  static struct omap_hwmod_class dra7xx_usb_otg_ss_hwmod_class = {
>>> .name   = "usb_otg_ss",
>>> +   .sysc   = &dra7xx_usb_otg_ss_sysc,
>>>  };
>>>  
>>>  /* usb_otg_ss1 */
>>>
>>
> 

--
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 5/7] ARM: DRA7: hwmod: Add SYSCONFIG for usb_otg_ss

2014-06-18 Thread Rajendra Nayak
On Wednesday 18 June 2014 04:40 PM, Roger Quadros wrote:
> + Nishant and Rajendra for review.
> 
> On 05/05/2014 12:54 PM, Roger Quadros wrote:
>> Add the sysconfig class bits for the Super Speed USB
>> controllers
>>
>> CC: Paul Walmsley 
>> Signed-off-by: Roger Quadros 

verified against TRM version vP, looks good to me.
Reviewed-by: Rajendra Nayak 

>> ---
>>  arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c 
>> b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> index 810c205..067d322 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> @@ -1731,8 +1731,20 @@ static struct omap_hwmod dra7xx_uart6_hwmod = {
>>   *
>>   */
>>  
>> +static struct omap_hwmod_class_sysconfig dra7xx_usb_otg_ss_sysc = {
>> +.rev_offs   = 0x,
>> +.sysc_offs  = 0x0010,
>> +.sysc_flags = (SYSC_HAS_DMADISABLE | SYSC_HAS_MIDLEMODE |
>> +   SYSC_HAS_SIDLEMODE),
>> +.idlemodes  = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
>> +   SIDLE_SMART_WKUP | MSTANDBY_FORCE | MSTANDBY_NO |
>> +   MSTANDBY_SMART | MSTANDBY_SMART_WKUP),
>> +.sysc_fields= &omap_hwmod_sysc_type2,
>> +};
>> +
>>  static struct omap_hwmod_class dra7xx_usb_otg_ss_hwmod_class = {
>>  .name   = "usb_otg_ss",
>> +.sysc   = &dra7xx_usb_otg_ss_sysc,
>>  };
>>  
>>  /* usb_otg_ss1 */
>>
> 

--
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 5/7] ARM: DRA7: hwmod: Add SYSCONFIG for usb_otg_ss

2014-06-18 Thread Roger Quadros
+ Nishant and Rajendra for review.

On 05/05/2014 12:54 PM, Roger Quadros wrote:
> Add the sysconfig class bits for the Super Speed USB
> controllers
> 
> CC: Paul Walmsley 
> Signed-off-by: Roger Quadros 
> ---
>  arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 810c205..067d322 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1731,8 +1731,20 @@ static struct omap_hwmod dra7xx_uart6_hwmod = {
>   *
>   */
>  
> +static struct omap_hwmod_class_sysconfig dra7xx_usb_otg_ss_sysc = {
> + .rev_offs   = 0x,
> + .sysc_offs  = 0x0010,
> + .sysc_flags = (SYSC_HAS_DMADISABLE | SYSC_HAS_MIDLEMODE |
> +SYSC_HAS_SIDLEMODE),
> + .idlemodes  = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
> +SIDLE_SMART_WKUP | MSTANDBY_FORCE | MSTANDBY_NO |
> +MSTANDBY_SMART | MSTANDBY_SMART_WKUP),
> + .sysc_fields= &omap_hwmod_sysc_type2,
> +};
> +
>  static struct omap_hwmod_class dra7xx_usb_otg_ss_hwmod_class = {
>   .name   = "usb_otg_ss",
> + .sysc   = &dra7xx_usb_otg_ss_sysc,
>  };
>  
>  /* usb_otg_ss1 */
> 

--
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: musb: cppi41: fire hrtimer according to programmed channel length

2014-06-18 Thread Daniel Mack
On 06/18/2014 11:32 AM, David Laight wrote:
> From: Of Daniel Mack
>> Sent: 18 June 2014 10:28
>> To: ba...@ti.com; george.cher...@ti.com; bige...@linutronix.de
>> Cc: sebastian.reim...@googlemail.com; linux-usb@vger.kernel.org; Daniel Mack
>> Subject: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed 
>> channel length
>>
>> The musb/cppi41 code installs a hrtimer to work around DMA completion
>> interrupts that have fired too early on AM335x hardware. This timer
>> is currently programmed to first fire 140 nanoseconds after the DMA
>> completion callback. According to the commit which introduced it
>> (a655f481d83, "usb: musb: musb_cppi41: handle pre-mature TX complete
>> interrupt"), that value is is considered a 'rule of thumb' that worked
>> well with the test case described in the commit log.
>>
>> Test show, however, that for USB audio devices and much smaller packet
>> sizes, the timer has to fire earlier in order to correctly handle the audio
>> stream. The original test case had output transfer sizes of 1514 bytes, and
>> a delay of 140 nanoseconds. For audio devices with 24 bytes channel size, 3
>> nanoseconds seem to work well.
> 
> You can't really mean nanoseconds?

Microseconds of course. Thanks for the heads-up. Fixed locally.


Daniel

--
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: musb: cppi41: fire hrtimer according to programmed channel length

2014-06-18 Thread David Laight
From: Of Daniel Mack
> Sent: 18 June 2014 10:28
> To: ba...@ti.com; george.cher...@ti.com; bige...@linutronix.de
> Cc: sebastian.reim...@googlemail.com; linux-usb@vger.kernel.org; Daniel Mack
> Subject: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed 
> channel length
> 
> The musb/cppi41 code installs a hrtimer to work around DMA completion
> interrupts that have fired too early on AM335x hardware. This timer
> is currently programmed to first fire 140 nanoseconds after the DMA
> completion callback. According to the commit which introduced it
> (a655f481d83, "usb: musb: musb_cppi41: handle pre-mature TX complete
> interrupt"), that value is is considered a 'rule of thumb' that worked
> well with the test case described in the commit log.
> 
> Test show, however, that for USB audio devices and much smaller packet
> sizes, the timer has to fire earlier in order to correctly handle the audio
> stream. The original test case had output transfer sizes of 1514 bytes, and
> a delay of 140 nanoseconds. For audio devices with 24 bytes channel size, 3
> nanoseconds seem to work well.

You can't really mean nanoseconds?

David



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


[PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed channel length

2014-06-18 Thread Daniel Mack
The musb/cppi41 code installs a hrtimer to work around DMA completion
interrupts that have fired too early on AM335x hardware. This timer
is currently programmed to first fire 140 nanoseconds after the DMA
completion callback. According to the commit which introduced it
(a655f481d83, "usb: musb: musb_cppi41: handle pre-mature TX complete
interrupt"), that value is is considered a 'rule of thumb' that worked
well with the test case described in the commit log.

Test show, however, that for USB audio devices and much smaller packet
sizes, the timer has to fire earlier in order to correctly handle the audio
stream. The original test case had output transfer sizes of 1514 bytes, and
a delay of 140 nanoseconds. For audio devices with 24 bytes channel size, 3
nanoseconds seem to work well.

Hence, let's assume that the time it takes to clear the bit correlates with
the number of bytes transferred. The referenced commit log mentions such a
suspicion as well. Let the timer fire in cppi41_channel->total_len/10
nanoseconds to correctly handle both cases.

Also, shorten the interval in which the timer fires again in case of
a non-empty early_tx list.

With these changes in place, both FS and HS audio devices appear to work
well on AM335x hardware.

Signed-off-by: Daniel Mack 
Reported-by: Sebastian Reimers 
---
 drivers/usb/musb/musb_cppi41.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index a2c4456..ce918eb 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -200,7 +200,7 @@ static enum hrtimer_restart cppi41_recheck_tx_req(struct 
hrtimer *timer)
if (!list_empty(&controller->early_tx_list)) {
ret = HRTIMER_RESTART;
hrtimer_forward_now(&controller->early_tx,
-   ktime_set(0, 150 * NSEC_PER_USEC));
+   ktime_set(0, 50 * NSEC_PER_USEC));
}
 
spin_unlock_irqrestore(&musb->lock, flags);
@@ -274,8 +274,10 @@ static void cppi41_dma_callback(void *private_data)
list_add_tail(&cppi41_channel->tx_check,
&controller->early_tx_list);
if (!hrtimer_active(&controller->early_tx)) {
+   unsigned long nsecs = cppi41_channel->total_len / 10;
+
hrtimer_start_range_ns(&controller->early_tx,
-   ktime_set(0, 140 * NSEC_PER_USEC),
+   ktime_set(0, nsecs * NSEC_PER_USEC),
40 * NSEC_PER_USEC,
HRTIMER_MODE_REL);
}
-- 
1.9.3

--
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] usb musb/cppi41: Address issues with isochronous audio endpoints

2014-06-18 Thread Daniel Mack
Hi,

I've been debugging issues with musb in host mode and both full-speed
and high-speed USB audio devices with cppi41 DMA mode enabled.

The effect that was observed with full-speed devices was that CPU load
went up to 100% due to the dma channels dma_completion work struct.
For FS devices, the MUSB_TXCSR_TXPKTRDY bit that signals the FIFO's
emptyness takes a long time to be cleared, and if the worker function
determines that it's still set, it will re-queue the work immediately,
which effectively results in a busy poll that renders the system
unusable. There are audible crackles on the output, and every bit of
extra load will distort the audio stream even more.

The work struct was introduced by 1af54b7a40 ("usb: musb: musb_cppi41:
Handle ISOCH differently and not use the hrtimer."), apparantly in
order to mitigate an "unreliable" behaviour of the driver.

Geroge, do you recall which problems you saw, which device you tested
with and what the effect of this patch was for you? I'm asking because
I suspect the issue in fact lies in the hrtimer interval setup and can
hence be fixed well without the extra worker.

Sebastian's patch to implement that timer (a655f481d: "usb: musb:
musb_cppi41: handle pre-mature TX complete interrupt") expressed some
uncertainty about the chosen value of 140us, and suspected that there's
a relation between the dma burst size and the minimum time value.

Hence, I'm sending two patches. The first one reverts 1af54b7a40 as it
causes trouble with FS audio devices, and the seconds addresses the
issue by tweaking the hrtimer settings instead. This seems to work fine
with both FS and HS audio devices now.

George, could you give this a try with your original test case?

Note that these patches are to be applied on top of my musb/cppi41
cleanup series that I sent ~3 weeks ago.


Thanks,
Daniel



Daniel Mack (2):
  Revert "usb: musb: musb_cppi41: Handle ISOCH differently and not use
the hrtimer."
  usb: musb: cppi41: fire hrtimer according to programmed channel length

 drivers/usb/musb/musb_cppi41.c | 59 +++---
 1 file changed, 4 insertions(+), 55 deletions(-)

-- 
1.9.3

--
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] Revert "usb: musb: musb_cppi41: Handle ISOCH differently and not use the hrtimer."

2014-06-18 Thread Daniel Mack
This reverts commit 1af54b7a4.

The commit tried to address cases in which isochronous transfers are 'not
reliable', most probably in the tests conducted, polling for the
MUSB_TXCSR_TXPKTRDY bit in MUSB_TXCSR is done too late.

Hence, it installs a work struct which basically busy-polls for the bit in a
rather agressive way by rescheduling the work if the FIFO is not empty. With
USB audio devices, tests have shown that it takes approximately 100
iterations of the asynchronous worker until the FIFO signals completion,
which leads to 100% CPU loads when streaming audio.

The issue the patch tried to address can be handled differently, which is
what the next patch does.

Signed-off-by: Daniel Mack 
Reported-by: Sebastian Reimers 
---
 drivers/usb/musb/musb_cppi41.c | 53 --
 1 file changed, 53 deletions(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index 932464f..a2c4456 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -39,7 +39,6 @@ struct cppi41_dma_channel {
u32 transferred;
u32 packet_sz;
struct list_head tx_check;
-   struct work_struct dma_completion;
 };
 
 #define MUSB_DMA_NUM_CHANNELS 15
@@ -117,18 +116,6 @@ static bool musb_is_tx_fifo_empty(struct musb_hw_ep *hw_ep)
return true;
 }
 
-static bool is_isoc(struct musb_hw_ep *hw_ep, bool in)
-{
-   if (in && hw_ep->in_qh) {
-   if (hw_ep->in_qh->type == USB_ENDPOINT_XFER_ISOC)
-   return true;
-   } else if (hw_ep->out_qh) {
-   if (hw_ep->out_qh->type == USB_ENDPOINT_XFER_ISOC)
-   return true;
-   }
-   return false;
-}
-
 static void cppi41_dma_callback(void *private_data);
 
 static void cppi41_trans_done(struct cppi41_dma_channel *cppi41_channel)
@@ -185,32 +172,6 @@ static void cppi41_trans_done(struct cppi41_dma_channel 
*cppi41_channel)
}
 }
 
-static void cppi_trans_done_work(struct work_struct *work)
-{
-   unsigned long flags;
-   struct cppi41_dma_channel *cppi41_channel =
-   container_of(work, struct cppi41_dma_channel, dma_completion);
-   struct cppi41_dma_controller *controller = cppi41_channel->controller;
-   struct musb *musb = controller->musb;
-   struct musb_hw_ep *hw_ep = cppi41_channel->hw_ep;
-   bool empty;
-
-   if (!cppi41_channel->is_tx && is_isoc(hw_ep, 1)) {
-   spin_lock_irqsave(&musb->lock, flags);
-   cppi41_trans_done(cppi41_channel);
-   spin_unlock_irqrestore(&musb->lock, flags);
-   } else {
-   empty = musb_is_tx_fifo_empty(hw_ep);
-   if (empty) {
-   spin_lock_irqsave(&musb->lock, flags);
-   cppi41_trans_done(cppi41_channel);
-   spin_unlock_irqrestore(&musb->lock, flags);
-   } else {
-   schedule_work(&cppi41_channel->dma_completion);
-   }
-   }
-}
-
 static enum hrtimer_restart cppi41_recheck_tx_req(struct hrtimer *timer)
 {
struct cppi41_dma_controller *controller;
@@ -274,14 +235,6 @@ static void cppi41_dma_callback(void *private_data)
transferred < cppi41_channel->packet_sz)
cppi41_channel->prog_len = 0;
 
-   if (!cppi41_channel->is_tx) {
-   if (is_isoc(hw_ep, 1))
-   schedule_work(&cppi41_channel->dma_completion);
-   else
-   cppi41_trans_done(cppi41_channel);
-   goto out;
-   }
-
empty = musb_is_tx_fifo_empty(hw_ep);
if (empty) {
cppi41_trans_done(cppi41_channel);
@@ -318,10 +271,6 @@ static void cppi41_dma_callback(void *private_data)
goto out;
}
}
-   if (is_isoc(hw_ep, 0)) {
-   schedule_work(&cppi41_channel->dma_completion);
-   goto out;
-   }
list_add_tail(&cppi41_channel->tx_check,
&controller->early_tx_list);
if (!hrtimer_active(&controller->early_tx)) {
@@ -679,8 +628,6 @@ static int cppi41_dma_controller_start(struct 
cppi41_dma_controller *controller)
cppi41_channel->port_num = port;
cppi41_channel->is_tx = is_tx;
INIT_LIST_HEAD(&cppi41_channel->tx_check);
-   INIT_WORK(&cppi41_channel->dma_completion,
- cppi_trans_done_work);
 
musb_dma = &cppi41_channel->channel;
musb_dma->private_data = cppi41_channel;
-- 
1.9.3

--
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: xhci handling ring expansion

2014-06-18 Thread David Laight
From: vichy 
> hi david:
...
> > From one of the patches (not applied) I sent last November ...
> > Include the unknown address when the DMA pointer from an event isn't part
> > of the current TD. This will happen if the error code is "TRB error".

> do you mean below patch?
> http://markmail.org/message/74qvwz7fhjxqeih3
> it only add debug message instead of fixing it.
> 
> appreciate your help,

In my case there was a coding error in one of the other xhci
patches that caused a 'TRB error' report.
The patch to change the tracing was an attempt to make things
less confusing.

If you modify the tracing you might find out where the pointer from
the event came from.

David



Re: [PATCH 0/3] port power control fixes for 3.16-rc2

2014-06-18 Thread Bjørn Mork
Dan Williams  writes:

> I put patch 3 "usb: fix hub-port pm_runtime_enable() vs runtime pm
> transitions" through it's paces, but I'd still like to see a positive
> test report from Bjørn... even if it's too late to add a "Tested-by".

Fixes the problem for me.  Thanks


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


Re: [PATCH] usb: renesas: gadget: fixup: complete STATUS stage after receiving

2014-06-18 Thread Kuninori Morimoto

ping ?

> From: Kuninori Morimoto 
> 
> Current usbhs gadget driver didn't complete STATUS stage after receiving.
> It wasn't problem for us before, because some USB class doesn't use
> DATA OUT stage in control transfer.
> But, it is required on some device.
> 
> Signed-off-by: Yoshihiro Shimoda 
> Signed-off-by: Kuninori Morimoto 
> ---
>  drivers/usb/renesas_usbhs/fifo.c |8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/usb/renesas_usbhs/fifo.c 
> b/drivers/usb/renesas_usbhs/fifo.c
> index d49f9c3..4fd3653 100644
> --- a/drivers/usb/renesas_usbhs/fifo.c
> +++ b/drivers/usb/renesas_usbhs/fifo.c
> @@ -681,6 +681,14 @@ usbhs_fifo_read_end:
>   usbhs_pipe_number(pipe),
>   pkt->length, pkt->actual, *is_done, pkt->zero);
>  
> + /*
> +  * Transmission end
> +  */
> + if (*is_done) {
> + if (usbhs_pipe_is_dcp(pipe))
> + usbhs_dcp_control_transfer_done(pipe);
> + }
> +
>  usbhs_fifo_read_busy:
>   usbhsf_fifo_unselect(pipe, fifo);
>  
> -- 
> 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 v8] leds: USB: HID: Add support for MSI GT683R led panels

2014-06-18 Thread Johan Hovold
On Tue, Jun 17, 2014 at 07:41:44PM +0300, Janne Kanniainen wrote:
> This driver adds support for USB controlled led panels that exists in
> MSI GT683R laptop
> 
> Changes in v2:
>   - sorted headers to alphabetic order
>   - using devm_kzalloc
>   - using BIT(n)
>   - using usb_control_msg instead of usb_submit_urb
>   - removing unneeded code
> Changes in v3:
>   - implemented as HID device
>   - some cleanups and bug fixes
> 
> Changes in v4:
>   - more cleanups
>   - support for selecting leds
>   - suppport for selecting status
> 
> Changes in v5:
>   - mode attribute documented under Documentation/ABI
>   - made array for led_classdev
>   - led devices uses now recommended naming scheme
> 
> Changes in v6:
>   - flush_work added
>   - using hid device name instead of hard coded gt683r
>   - allocating name buffers with devm_kzalloc
> 
> Changes in v7:
>   - buf with for fixed
> 
> Changes in v8:
>   - some cleanups and bugs fixed

Great job, Janne. I have a few really minor style issues I just noted
and that I point out below, but that's it.

> Signed-off-by: Janne Kanniainen 
> ---
>  .../ABI/testing/sysfs-class-hid-driver-gt683r  |  14 +
>  drivers/hid/Kconfig|  14 +
>  drivers/hid/Makefile   |   1 +
>  drivers/hid/hid-core.c |   1 +
>  drivers/hid/hid-gt683r.c   | 308 
> +
>  drivers/hid/hid-ids.h  |   2 +-
>  drivers/hid/usbhid/hid-quirks.c|   2 +-
>  7 files changed, 340 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>  create mode 100644 drivers/hid/hid-gt683r.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r 
> b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> new file mode 100644
> index 000..317e9d5
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> @@ -0,0 +1,14 @@
> +What:/sys/class/hidraw//device/leds_mode
> +Date:Jun 2014
> +KernelVersion:   3.17
> +Contact: Janne Kanniainen 
> +Description:
> + Set the mode of LEDs
> +
> + 0 - normal
> + 1 - audio
> + 2 - breathing
> +
> + Normal: LEDs are fully on when enabled
> + Audio:  LEDs brightness depends on sound level
> + Breathing: LEDs brightness varies at human breathing rate
> \ No newline at end of file
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 7af9d0b..e2f4590 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -261,6 +261,20 @@ config HOLTEK_FF
> Say Y here if you have a Holtek On Line Grip based game controller
> and want to have force feedback support for it.
>  
> +config HID_GT683R
> + tristate "MSI GT68xR LED support"
> + depends on LEDS_CLASS && USB_HID
> + ---help---
> + Say Y here if you want to enable support for the three MSI GT68xR LEDs
> +
> + This driver support following modes:
> +   - Normal: LEDs are fully on when enabled
> +   - Audio:  LEDs brightness depends on sound level
> +   - Breathing: LEDs brightness varies at human breathing rate
> +
> + Currently the following devices are know to be supported:
> +   - MSI GT683R
> +
>  config HID_HUION
>   tristate "Huion tablets"
>   depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index fc712dd..7129311 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF)+= hid-emsff.o
>  obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
>  obj-$(CONFIG_HID_ELO)+= hid-elo.o
>  obj-$(CONFIG_HID_EZKEY)  += hid-ezkey.o
> +obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
>  obj-$(CONFIG_HID_GYRATION)   += hid-gyration.o
>  obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
>  obj-$(CONFIG_HID_HOLTEK) += hid-holtek-mouse.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index da52279..ec88fdb 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1827,6 +1827,7 @@ static const struct hid_device_id 
> hid_have_special_driver[] = {
>   { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, 
> USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) 
> },
>   { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) 
> },
>   { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, 
> USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, 
> USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
> diff --git a/driv

Re: [PATCH 4/4] USB: ohci-spear: Make of_device_id array const

2014-06-18 Thread Viresh Kumar
On 18 June 2014 10:08, Jingoo Han  wrote:
> Make of_device_id array const, because all OF functions handle
> it as const.
>
> Signed-off-by: Jingoo Han 
> ---
>  drivers/usb/host/ohci-spear.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ohci-spear.c b/drivers/usb/host/ohci-spear.c
> index 8b29a0c..8d58766 100644
> --- a/drivers/usb/host/ohci-spear.c
> +++ b/drivers/usb/host/ohci-spear.c
> @@ -162,7 +162,7 @@ static int spear_ohci_hcd_drv_resume(struct 
> platform_device *dev)
>  }
>  #endif
>
> -static struct of_device_id spear_ohci_id_table[] = {
> +static const struct of_device_id spear_ohci_id_table[] = {
> { .compatible = "st,spear600-ohci", },
> { },
>  };

Acked-by: Viresh Kumar 
--
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/4] USB: ehci-spear: Make of_device_id array const

2014-06-18 Thread Viresh Kumar
On 18 June 2014 10:05, Jingoo Han  wrote:
> Make of_device_id array const, because all OF functions handle
> it as const.
>
> Signed-off-by: Jingoo Han 
> ---
>  drivers/usb/host/ehci-spear.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ehci-spear.c b/drivers/usb/host/ehci-spear.c
> index 1d59958..1355ff0 100644
> --- a/drivers/usb/host/ehci-spear.c
> +++ b/drivers/usb/host/ehci-spear.c
> @@ -150,7 +150,7 @@ static int spear_ehci_hcd_drv_remove(struct 
> platform_device *pdev)
> return 0;
>  }
>
> -static struct of_device_id spear_ehci_id_table[] = {
> +static const struct of_device_id spear_ehci_id_table[] = {
> { .compatible = "st,spear600-ehci", },
> { },
>  };

Acked-by: Viresh Kumar 
--
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] staging: usbip: fixed a coding-style warning

2014-06-18 Thread Joe Perches
On Wed, 2014-06-18 at 00:06 -0700, Greg Kroah-Hartman wrote:
> On Wed, Jun 18, 2014 at 09:49:39AM +0300, Alexey Tulia wrote:
> > On Tue, Jun 17, 2014 at 03:44:58PM -0700, Greg Kroah-Hartman wrote:
> > > On Fri, Jun 13, 2014 at 11:35:13AM +0300, Alexey Tulia wrote:
> > > > This fixes the following warning:
> > > > - WARNING: __constant_cpu_to_le32 should be cpu_to_le32
> > > 
> > > What produces this warning?
> > > 
> > 
> > This warning was found by checkpatch.pl -f
> > 
> > > > 
> > > > Signed-off-by: Alexey Tulia 
> > > > ---
> > > >  drivers/staging/usbip/vhci_hcd.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/staging/usbip/vhci_hcd.c 
> > > > b/drivers/staging/usbip/vhci_hcd.c
> > > > index 0007d30..e21c1b4 100644
> > > > --- a/drivers/staging/usbip/vhci_hcd.c
> > > > +++ b/drivers/staging/usbip/vhci_hcd.c
> > > > @@ -304,7 +304,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, 
> > > > u16 typeReq, u16 wValue,
> > > > break;
> > > > case GetHubStatus:
> > > > usbip_dbg_vhci_rh(" GetHubStatus\n");
> > > > -   *(__le32 *) buf = __constant_cpu_to_le32(0);
> > > > +   *(__le32 *) buf = cpu_to_le32(0);
> > > 
> > > How is this correct?  Why shouldn't __constant_cpu_to_le32() be used
> > > here?  Heck, why can't we just use 0 given that it doesn't matter the
> > > endianness of that specific value :)
> > 
> > It may be so, but anyway the __constant_cpu_to_le32 produced a warning
> > and it was obvious to clean up this part of code a bit.
> > However, the 0 value possibly can change to other value in future and
> > this macro acts as a safety net here. 
> 
> Dragging Joe in here as he wrote that checkpatch feature.
> 
> Joe, how is not using __constant_cpu_to_* a good thing?  If I have a
> constant value, cpu_to_* will be a function call if the bits have to be
> switched around (no-op if not).  I don't see the code path that detects
> a constant value and calls the swap-bits-as-a-constant macro instead,
> unless the __constant_cpu_to* function is called.
> 
> Am I just missing it somewhere in the .h include chain?

Probably.

There a __builtin_constant_p check in there via the
include/uapi/linux/swab.h chain.

for instance:

htonl ->
___htonl ->
__cpu_to_be32
__constant_htonl ->
__swab32 ->

include/uapi/linux/swab.h:#define __swab32(x)   \
include/uapi/linux/swab.h-  (__builtin_constant_p((__u32)(x)) ? \
include/uapi/linux/swab.h-  ___constant_swab32(x) : \
include/uapi/linux/swab.h-  __fswab32(x))



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


Re: [PATCH 1/2] usb: gadget: gr_udc: Make of_device_id array const

2014-06-18 Thread Andreas Larsson

On 2014-06-18 06:40, Jingoo Han wrote:

Make of_device_id array const, because all OF functions handle
it as const.

Signed-off-by: Jingoo Han 


Acked-by: Andreas Larsson 

Cheers,
Andreas


---
  drivers/usb/gadget/gr_udc.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index 99a37ed..5d93f2b 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -2212,7 +2212,7 @@ out:
return retval;
  }

-static struct of_device_id gr_match[] = {
+static const struct of_device_id gr_match[] = {
{.name = "GAISLER_USBDC"},
{.name = "01_021"},
{},


--
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] staging: usbip: fixed a coding-style warning

2014-06-18 Thread Greg Kroah-Hartman
On Wed, Jun 18, 2014 at 09:49:39AM +0300, Alexey Tulia wrote:
> On Tue, Jun 17, 2014 at 03:44:58PM -0700, Greg Kroah-Hartman wrote:
> > On Fri, Jun 13, 2014 at 11:35:13AM +0300, Alexey Tulia wrote:
> > > This fixes the following warning:
> > >   - WARNING: __constant_cpu_to_le32 should be cpu_to_le32
> > 
> > What produces this warning?
> > 
> 
> This warning was found by checkpatch.pl -f
> 
> > > 
> > > Signed-off-by: Alexey Tulia 
> > > ---
> > >  drivers/staging/usbip/vhci_hcd.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/usbip/vhci_hcd.c 
> > > b/drivers/staging/usbip/vhci_hcd.c
> > > index 0007d30..e21c1b4 100644
> > > --- a/drivers/staging/usbip/vhci_hcd.c
> > > +++ b/drivers/staging/usbip/vhci_hcd.c
> > > @@ -304,7 +304,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 
> > > typeReq, u16 wValue,
> > >   break;
> > >   case GetHubStatus:
> > >   usbip_dbg_vhci_rh(" GetHubStatus\n");
> > > - *(__le32 *) buf = __constant_cpu_to_le32(0);
> > > + *(__le32 *) buf = cpu_to_le32(0);
> > 
> > How is this correct?  Why shouldn't __constant_cpu_to_le32() be used
> > here?  Heck, why can't we just use 0 given that it doesn't matter the
> > endianness of that specific value :)
> 
> It may be so, but anyway the __constant_cpu_to_le32 produced a warning
> and it was obvious to clean up this part of code a bit.
> However, the 0 value possibly can change to other value in future and
> this macro acts as a safety net here. 

Dragging Joe in here as he wrote that checkpatch feature.

Joe, how is not using __constant_cpu_to_* a good thing?  If I have a
constant value, cpu_to_* will be a function call if the bits have to be
switched around (no-op if not).  I don't see the code path that detects
a constant value and calls the swap-bits-as-a-constant macro instead,
unless the __constant_cpu_to* function is called.

Am I just missing it somewhere in the .h include chain?

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