Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq
On Wed, Sep 02, 2015 at 09:43:38AM -0500, Felipe Balbi wrote: > Hi, > > > + > > +static irqreturn_t dwc3_otg_irq(int irq, void *_dwc) > > +{ > > + struct dwc3 *dwc = _dwc; > > + irqreturn_t ret = IRQ_NONE; > > + u32 reg; > > + > > + spin_lock(&dwc->lock); > > this seems unnecessary, we're already in hardirq with IRQs disabled. > What sort of race could we have ? (in fact, this also needs change in > dwc3/gadget.c). > Is it possible the kernel process is accessing the content you will access? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/9] usb: dwc3: add dual-role support
On Wed, Sep 02, 2015 at 05:24:16PM +0300, Roger Quadros wrote: > Register with the USB OTG core. Since we don't support > OTG yet we just work as a dual-role device even > if device tree says "otg". > > + > +static int dwc3_drd_init(struct dwc3 *dwc) > +{ > + int ret, id, vbus; > + struct usb_otg_caps *otgcaps = &dwc->otg_config.otg_caps; > + > + otgcaps->otg_rev = 0; > + otgcaps->hnp_support = false; > + otgcaps->srp_support = false; > + otgcaps->adp_support = false; > + dwc->otg_config.fsm_ops = &dwc3_drd_ops; > + > + if (!dwc->edev) { > + dev_err(dwc->dev, "No extcon device found for OTG mode\n"); > + return -ENODEV; > + } > + Do All dwc3 platforms id/vbus need to get through extcon? Do the SoCs have id/vbus pin? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/1] usb: misc: usbtest: add bulk queue test
On Wed, Sep 02, 2015 at 10:23:39AM -0400, Alan Stern wrote: > On Wed, 2 Sep 2015, Peter Chen wrote: > > > The bulk queue tests are used to show 'best performance' for bulk > > transfer, we are often asked this question by users. > > > > It's result should be very close to IC simulation, in order > > to get that, the device side should also need to prepare enough > > queue. > > > > We have got the 'best performance' (IN: ~41MB, OUT: ~39MB) at i.mx platform > > (USB2, ARM Cortex A9) with below command: > > > > Host side: > > modprobe usbtest > > ./testusb -a -t 27 -g 64 -s 16384 > > ./testusb -a -t 28 -g 64 -s 16384 > > Gadget side: > > modprobe g_zero loopdefault=1 qlen=64 buflen=16384 > > > > Signed-off-by: Peter Chen > > --- > > > > I am not sure if it is good to reuse iso structure, so I take > > it as RFC. If reuse is accepted, I will rename the iso > > structures to common one. > > > > drivers/usb/misc/usbtest.c | 88 > > -- > > 1 file changed, 69 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c > > index ad6dd4a..f1faeb9 100644 > > --- a/drivers/usb/misc/usbtest.c > > +++ b/drivers/usb/misc/usbtest.c > > @@ -17,6 +17,7 @@ > > static int override_alt = -1; > > module_param_named(alt, override_alt, int, 0644); > > MODULE_PARM_DESC(alt, ">= 0 to override altsetting selection"); > > +static void iso_callback(struct urb *urb); > > > > > > /*-*/ > > > > @@ -239,7 +240,8 @@ static struct urb *usbtest_alloc_urb( > > unsigned long bytes, > > unsignedtransfer_flags, > > unsignedoffset, > > - u8 bInterval) > > + u8 bInterval, > > + boolqueue_callback) > > Should this be named "is_iso" instead? It would be be more clear. > ISO transfer uses different function iso_alloc_urb to allocate urb, the reason we need one more parameter at usbtest_alloc_urb is we use different callback for simple polling bulk transfer and interrupt-triggered bulk transfer. So, do you agree I reuse the iso structures to implement it? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: amd5536udc: fix error handling in udc_pci_probe()
If a failure happens early in udc_pci_probe(), error handling code just kfree(dev) and returns. The patch adds proper resource deallocations in udc_pci_probe() itself, since udc_pci_remove() is not suitabe to be called so early in initialization process. By the way, iounmap(dev->regs) is replaced by iounmap(dev->virt_addr) in udc_pci_remove() for clarity. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov --- drivers/usb/gadget/udc/amd5536udc.c | 43 + 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c index fdacddb18c00..175ca93fe5e2 100644 --- a/drivers/usb/gadget/udc/amd5536udc.c +++ b/drivers/usb/gadget/udc/amd5536udc.c @@ -3138,8 +3138,8 @@ static void udc_pci_remove(struct pci_dev *pdev) writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), &dev->regs->cfg); if (dev->irq_registered) free_irq(pdev->irq, dev); - if (dev->regs) - iounmap(dev->regs); + if (dev->virt_addr) + iounmap(dev->virt_addr); if (dev->mem_region) release_mem_region(pci_resource_start(pdev, 0), pci_resource_len(pdev, 0)); @@ -3226,17 +3226,13 @@ static int udc_pci_probe( /* init */ dev = kzalloc(sizeof(struct udc), GFP_KERNEL); - if (!dev) { - retval = -ENOMEM; - goto finished; - } + if (!dev) + return -ENOMEM; /* pci setup */ if (pci_enable_device(pdev) < 0) { - kfree(dev); - dev = NULL; retval = -ENODEV; - goto finished; + goto err_pcidev; } dev->active = 1; @@ -3246,28 +3242,22 @@ static int udc_pci_probe( if (!request_mem_region(resource, len, name)) { dev_dbg(&pdev->dev, "pci device used already\n"); - kfree(dev); - dev = NULL; retval = -EBUSY; - goto finished; + goto err_memreg; } dev->mem_region = 1; dev->virt_addr = ioremap_nocache(resource, len); if (dev->virt_addr == NULL) { dev_dbg(&pdev->dev, "start address cannot be mapped\n"); - kfree(dev); - dev = NULL; retval = -EFAULT; - goto finished; + goto err_ioremap; } if (!pdev->irq) { dev_err(&pdev->dev, "irq not set\n"); - kfree(dev); - dev = NULL; retval = -ENODEV; - goto finished; + goto err_irq; } spin_lock_init(&dev->lock); @@ -3283,10 +3273,8 @@ static int udc_pci_probe( if (request_irq(pdev->irq, udc_irq, IRQF_SHARED, name, dev) != 0) { dev_dbg(&pdev->dev, "request_irq(%d) fail\n", pdev->irq); - kfree(dev); - dev = NULL; retval = -EBUSY; - goto finished; + goto err_irq; } dev->irq_registered = 1; @@ -3314,8 +3302,17 @@ static int udc_pci_probe( return 0; finished: - if (dev) - udc_pci_remove(pdev); + udc_pci_remove(pdev); + return retval; + +err_irq: + iounmap(dev->virt_addr); +err_ioremap: + release_mem_region(resource, len); +err_memreg: + pci_disable_device(pdev); +err_pcidev: + kfree(dev); return retval; } -- 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