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