Re: [PATCH 26/37] xhci: use the trb_to_noop() helper for command trbs
On 20.01.2017 20:18, Felipe Balbi wrote: Mathias Nyman writes: Remove duplicate code by using trb_to_noop() when handling Aborted commads Signed-off-by: Mathias Nyman isn't this just [1] https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com A simplified version of [1]. There are only two types of no-op trbs, command and transfer no-ops. We always know to which one of these we want to turn the trb so just give it as a parameter. Solution [1] takes any trb and turns it into no-op, it's a more generic solution but it comes with the expense of code complexity, like the 20-case switch statement to check trb type to narrow it down to two options. I want xhci code to be as simple as possible. -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: [RESEND PATCH] usb: chipidea: delete an useless header include
On Mon, Jan 23, 2017 at 03:05:45PM +0800, Jisheng Zhang wrote: > is for net phy drivers, we don't need it. > > Signed-off-by: Jisheng Zhang > --- > drivers/usb/chipidea/core.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > index 3dbb4a21ab44..77078083e9fb 100644 > --- a/drivers/usb/chipidea/core.c > +++ b/drivers/usb/chipidea/core.c > @@ -62,7 +62,6 @@ > #include > #include > #include > -#include > #include > #include > Applied, thanks. -- 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: [RESEND PATCH] usb: chipidea: usb2: delete the redundant setting default DMA mask code
On Mon, Jan 23, 2017 at 03:09:23PM +0800, Jisheng Zhang wrote: > Similar as commit 2b2fe36def08 ("usb: chipidea: imx: delete the > redundant setting default DMA mask code"), the ci_hdrc_usb2 platform > device is also created by device tree, the default DMA mask should be > already set by of_dma_configure when the device are created. So delete > the redundant code at driver. > > Signed-off-by: Jisheng Zhang > --- > drivers/usb/chipidea/ci_hdrc_usb2.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c > b/drivers/usb/chipidea/ci_hdrc_usb2.c > index 4456d2cf80ff..d162cc0bb8ce 100644 > --- a/drivers/usb/chipidea/ci_hdrc_usb2.c > +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c > @@ -74,10 +74,6 @@ static int ci_hdrc_usb2_probe(struct platform_device *pdev) > } > } > > - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); > - if (ret) > - goto clk_err; > - > ci_pdata->name = dev_name(dev); > > priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource, Applied, thanks. -- 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 26/37] xhci: use the trb_to_noop() helper for command trbs
On 23.01.2017 10:12, Mathias Nyman wrote: On 20.01.2017 20:18, Felipe Balbi wrote: Mathias Nyman writes: Remove duplicate code by using trb_to_noop() when handling Aborted commads Signed-off-by: Mathias Nyman isn't this just [1] https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com A simplified version of [1]. There are only two types of no-op trbs, command and transfer no-ops. We always know to which one of these we want to turn the trb so just give it as a parameter. Solution [1] takes any trb and turns it into no-op, it's a more generic solution but it comes with the expense of code complexity, like the 20-case switch statement to check trb type to narrow it down to two options. I want xhci code to be as simple as possible. I'm resending the series anyway to fix the extra empty lines and a typo, want me to add a Suggested-by, or a "based on code by comment" to patch 25 and 26? -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 26/37] xhci: use the trb_to_noop() helper for command trbs
Hi, Mathias Nyman writes: > On 20.01.2017 20:18, Felipe Balbi wrote: >> >> Mathias Nyman writes: >> >>> Remove duplicate code by using trb_to_noop() when >>> handling Aborted commads >>> >>> Signed-off-by: Mathias Nyman >> >> isn't this just [1] >> >> https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com >> > > A simplified version of [1]. > There are only two types of no-op trbs, command and transfer no-ops. > We always know to which one of these we want to turn the trb so just > give it as a parameter. Right, the only difference is that you're passing NOOP type as argument instead of figuring it out inside trb_to_noop(). I didn't do it like that because I find it to be error prone. User has to remember if s/he is currently dealing with transfer or command TRBs when the function (totally not a critical path) can easily figure that out. > Solution [1] takes any trb and turns it into no-op, it's a more generic > solution but it comes with the expense of code complexity, like the 20-case > switch statement to check trb type to narrow it down to two options. yeah, that 20-case switch statement is largely optimized by the compiler, though. All cases are sequential integer values, so your switch() gets optimized to something like: if (type > X && type < Y) noop = Z; else if (type > W && type < V) noop = T; else error(); Well, compiler could optimize this ever further if TRB types were held in an enum instead of just several macros. Compiler would know for sure which are allowed values for that argument... but let's not go into that. > I want xhci code to be as simple as possible. a very honorable goal; common courtesy, however, would have you commenting on the original patch and my explanation above could be given to you. Then we would come to an agreement and I would, gladly, update original patch. In any case, it's just a patch ;-) No reason to stop your changes from going upstream. Just, next time, please have the courtesy of discussing changes in the list to avoid surprises ;-) cheers -- balbi signature.asc Description: PGP signature
Re: [PATCH 26/37] xhci: use the trb_to_noop() helper for command trbs
Hi, Mathias Nyman writes: > On 23.01.2017 10:12, Mathias Nyman wrote: >> On 20.01.2017 20:18, Felipe Balbi wrote: >>> >>> Mathias Nyman writes: >>> Remove duplicate code by using trb_to_noop() when handling Aborted commads Signed-off-by: Mathias Nyman >>> >>> isn't this just [1] >>> >>> https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com >>> >> >> A simplified version of [1]. >> There are only two types of no-op trbs, command and transfer no-ops. >> We always know to which one of these we want to turn the trb so just >> give it as a parameter. >> >> Solution [1] takes any trb and turns it into no-op, it's a more generic >> solution but it comes with the expense of code complexity, like the 20-case >> switch statement to check trb type to narrow it down to two options. >> >> I want xhci code to be as simple as possible. >> > > I'm resending the series anyway to fix the extra empty lines and a > typo, want me to add a Suggested-by, or a "based on code by comment" > to patch 25 and 26? I'll leave the decision to you. Either way is fine. -- balbi signature.asc Description: PGP signature
Re: [PATCH 26/37] xhci: use the trb_to_noop() helper for command trbs
On 23.01.2017 11:21, Felipe Balbi wrote: Hi, Mathias Nyman writes: On 20.01.2017 20:18, Felipe Balbi wrote: Mathias Nyman writes: Remove duplicate code by using trb_to_noop() when handling Aborted commads Signed-off-by: Mathias Nyman isn't this just [1] https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com A simplified version of [1]. There are only two types of no-op trbs, command and transfer no-ops. We always know to which one of these we want to turn the trb so just give it as a parameter. Right, the only difference is that you're passing NOOP type as argument instead of figuring it out inside trb_to_noop(). I didn't do it like that because I find it to be error prone. User has to remember if s/he is currently dealing with transfer or command TRBs when the function (totally not a critical path) can easily figure that out. Solution [1] takes any trb and turns it into no-op, it's a more generic solution but it comes with the expense of code complexity, like the 20-case switch statement to check trb type to narrow it down to two options. yeah, that 20-case switch statement is largely optimized by the compiler, though. All cases are sequential integer values, so your switch() gets optimized to something like: if (type > X && type < Y) noop = Z; else if (type > W && type < V) noop = T; else error(); Well, compiler could optimize this ever further if TRB types were held in an enum instead of just several macros. Compiler would know for sure which are allowed values for that argument... but let's not go into that. I want xhci code to be as simple as possible. a very honorable goal; common courtesy, however, would have you commenting on the original patch and my explanation above could be given to you. Then we would come to an agreement and I would, gladly, update original patch. In any case, it's just a patch ;-) No reason to stop your changes from going upstream. Just, next time, please have the courtesy of discussing changes in the list to avoid surprises ;-) Sure, will remember that. I wanted to get things forward to 4.11 before rc5 was out, and moving those few lines under their own helper my way was faster. Didn't think it mattered. -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 v3 3/3] usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps
On 01/20/2017 09:17 PM, Bin Liu wrote: > On Thu, Jan 19, 2017 at 11:06:59AM +0100, Alexandre Bailon wrote: >> Despite the CPPI 4.1 is a generic DMA, it is tied to USB. >> On the dsps, CPPI 4.1 interrupt's registers are in USBSS (the MUSB glue). >> Currently, to enable / disable and clear interrupts, the CPPI 4.1 driver >> maps and accesses to USBSS's register, which making CPPI 4.1 driver not >> really generic. >> Move the interrupt management to dsps driver. >> >> Signed-off-by: Alexandre Bailon >> --- >> drivers/dma/cppi41.c | 28 >> drivers/usb/musb/musb_dsps.c | 77 >> ++-- >> 2 files changed, 82 insertions(+), 23 deletions(-) > > This patch touches both dma and musb modules, I know it makes review > easier, but how we get it merged? One maintainer ACK it and the other > pick it up? Sorry for the dumb question, I am new as a maintainer... > >> >> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c >> index d5ba43a..4999e7d 100644 >> --- a/drivers/dma/cppi41.c >> +++ b/drivers/dma/cppi41.c > > [...] > >> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c >> index 9f125e1..9dad3a6 100644 >> --- a/drivers/usb/musb/musb_dsps.c >> +++ b/drivers/usb/musb/musb_dsps.c >> @@ -121,6 +121,7 @@ struct dsps_glue { >> struct timer_list timer;/* otg_workaround timer */ >> unsigned long last_timer;/* last timer data for each instance */ >> bool sw_babble_enabled; >> +void __iomem *usbss_base; >> >> struct dsps_context context; >> struct debugfs_regset32 regset; >> @@ -145,6 +146,13 @@ static const struct debugfs_reg32 dsps_musb_regs[] = { >> { "mode", 0xe8 }, >> }; >> >> +/* USBSS / USB AM335x */ >> +#define USBSS_IRQ_STATUS0x28 >> +#define USBSS_IRQ_ENABLER 0x2c >> +#define USBSS_IRQ_CLEARR0x30 >> + >> +#define USBSS_IRQ_PD_COMP (1 << 2) > > Please fix the double white spaces bwteen '<' and '2' this time. > >> + >> /** >> * dsps_musb_enable - enable interrupts >> */ >> @@ -619,14 +627,72 @@ static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, >> u16 len, u8 *dst) >> } >> } >> >> +#ifdef CONFIG_USB_TI_CPPI41_DMA >> +static void dsps_dma_controller_callback(struct dma_controller *c) >> +{ >> +struct musb *musb = c->musb; >> +struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent); >> +void __iomem *usbss_base = glue->usbss_base; >> +u32 status; >> + >> +status = musb_readl(usbss_base, USBSS_IRQ_STATUS); >> +if (status & USBSS_IRQ_PD_COMP) >> +musb_writel(usbss_base, USBSS_IRQ_STATUS, USBSS_IRQ_PD_COMP); >> +} >> + >> +static struct dma_controller * >> +dsps_dma_controller_create(struct musb *musb, void __iomem *base) >> +{ >> +struct dma_controller *controller; >> +struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent); >> +void __iomem *usbss_base = glue->usbss_base; >> + >> +controller = cppi41_dma_controller_create(musb, base); >> +if (IS_ERR_OR_NULL(controller)) >> +return controller; >> + >> +musb_writel(usbss_base, USBSS_IRQ_ENABLER, USBSS_IRQ_PD_COMP); >> +controller->dma_callback = dsps_dma_controller_callback; >> + >> +return controller; >> +} >> + >> +static void dsps_dma_controller_destroy(struct dma_controller *c) >> +{ >> +struct musb *musb = c->musb; >> +struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent); >> +void __iomem *usbss_base = glue->usbss_base; >> + >> +musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP); >> +cppi41_dma_controller_destroy(c); >> +} >> + >> +static void dsps_dma_controller_suspend(struct dsps_glue *glue) >> +{ >> +void __iomem *usbss_base = glue->usbss_base; >> + >> +musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP); >> +} >> + >> +static void dsps_dma_controller_resume(struct dsps_glue *glue) >> +{ >> +void __iomem *usbss_base = glue->usbss_base; >> + >> +musb_writel(usbss_base, USBSS_IRQ_ENABLER, USBSS_IRQ_PD_COMP); >> +} > > The two functions above need to be wrapped in CONFIG_PM_SLEEP. > >> +#else >> +static void dsps_dma_controller_suspend(struct dsps_glue *glue) {} >> +static void dsps_dma_controller_resume(struct dsps_glue *glue) {} >> +#endif >> + >> static struct musb_platform_ops dsps_ops = { >> .quirks = MUSB_DMA_CPPI41 | MUSB_INDEXED_EP, >> .init = dsps_musb_init, >> .exit = dsps_musb_exit, >> >> #ifdef CONFIG_USB_TI_CPPI41_DMA >> -.dma_init = cppi41_dma_controller_create, >> -.dma_exit = cppi41_dma_controller_destroy, >> +.dma_init = dsps_dma_controller_create, >> +.dma_exit = dsps_dma_controller_destroy, >> #endif >> .enable = dsps_musb_enable, >> .disable= dsps_musb_disable, >> @@ -792,6 +858,9 @@ static int dsps_probe(struct platform_device *pdev) >> >> glue->dev = &pdev->dev; >>
[RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
Hello, It seems the USB product ID for g_webcam usb device gadget is incorrectly used from EEM gadget "Ethernet Emulation Model". So "webcam" device has a confusing description in lsusb: 1d6b:0102 Linux Foundation EEM Gadget I would change it to 0x0106, which is a next unassigned value (according to http://www.linux-usb.org/usb.ids). Does this step require some official communication with the holder of VID (Linux Foundation)? Or all it takes is just to send patch to the kernel and to the usb.ids database? I know it is only a cosmetic change on a legacy driver, but I assume it would be better to have some default value for configfs API than to borrow a PID from a whole different gadget. Change in question would be something like this (I will send a proper patch after RFC): Fixing USB Product ID for UVC gadget (used from Ethernet Emulation Model) and changing it to new one. Signed-off-by: Petr Cvek --- drivers/usb/gadget/legacy/webcam.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/legacy/webcam.c b/drivers/usb/gadget/legacy/webcam.c index f9661cd627c8..988814f2e1c7 100644 --- a/drivers/usb/gadget/legacy/webcam.c +++ b/drivers/usb/gadget/legacy/webcam.c @@ -42,7 +42,7 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); */ #define WEBCAM_VENDOR_ID 0x1d6b /* Linux Foundation */ -#define WEBCAM_PRODUCT_ID 0x0102 /* Webcam A/V gadget */ +#define WEBCAM_PRODUCT_ID 0x0106 /* UVC video gadget */ #define WEBCAM_DEVICE_BCD 0x0010 /* 0.10 */ static char webcam_vendor_label[] = "Linux Foundation"; -- 2.11.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: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
Hi, Petr Cvek writes: > It seems the USB product ID for g_webcam usb device gadget is > incorrectly used from EEM gadget "Ethernet Emulation Model". So > "webcam" device has a confusing description in lsusb: > > 1d6b:0102 Linux Foundation EEM Gadget > > I would change it to 0x0106, which is a next unassigned value > (according to http://www.linux-usb.org/usb.ids). Does this step > require some official communication with the holder of VID (Linux > Foundation)? Or all it takes is just to send patch to the kernel and > to the usb.ids database? > > I know it is only a cosmetic change on a legacy driver, but I assume > it would be better to have some default value for configfs API than to > borrow a PID from a whole different gadget. > > Change in question would be something like this (I will send a proper > patch after RFC): > > Fixing USB Product ID for UVC gadget (used from Ethernet Emulation > Model) and changing it to new one. Greg, do we still have an official Vendor ID? If so, any chance we can allocate a unique default ID for the webcam gadget? > Signed-off-by: Petr Cvek > --- > drivers/usb/gadget/legacy/webcam.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/legacy/webcam.c > b/drivers/usb/gadget/legacy/webcam.c > index f9661cd627c8..988814f2e1c7 100644 > --- a/drivers/usb/gadget/legacy/webcam.c > +++ b/drivers/usb/gadget/legacy/webcam.c > @@ -42,7 +42,7 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); > */ > > #define WEBCAM_VENDOR_ID 0x1d6b /* Linux Foundation */ > -#define WEBCAM_PRODUCT_ID0x0102 /* Webcam A/V gadget */ > +#define WEBCAM_PRODUCT_ID0x0106 /* UVC video gadget */ > #define WEBCAM_DEVICE_BCD0x0010 /* 0.10 */ > > static char webcam_vendor_label[] = "Linux Foundation"; > -- > 2.11.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 -- balbi signature.asc Description: PGP signature
Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
On Mon, Jan 23, 2017 at 12:59:33PM +0200, Felipe Balbi wrote: > > Hi, > > Petr Cvek writes: > > It seems the USB product ID for g_webcam usb device gadget is > > incorrectly used from EEM gadget "Ethernet Emulation Model". So > > "webcam" device has a confusing description in lsusb: > > > > 1d6b:0102 Linux Foundation EEM Gadget > > > > I would change it to 0x0106, which is a next unassigned value > > (according to http://www.linux-usb.org/usb.ids). Does this step > > require some official communication with the holder of VID (Linux > > Foundation)? Or all it takes is just to send patch to the kernel and > > to the usb.ids database? > > > > I know it is only a cosmetic change on a legacy driver, but I assume > > it would be better to have some default value for configfs API than to > > borrow a PID from a whole different gadget. > > > > Change in question would be something like this (I will send a proper > > patch after RFC): > > > > Fixing USB Product ID for UVC gadget (used from Ethernet Emulation > > Model) and changing it to new one. > > Greg, do we still have an official Vendor ID? Well, as official as we are allowed to get :) > If so, any chance we can > allocate a unique default ID for the webcam gadget? Hm, I thought we wanted to get rid of the "legacy" gadget stuff. Who is using it these days? It's not Android... If it's really needed, then yes, you can use 0106, just let me know. But do you really need a new id? What's wrong with 0102? 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
[PATCH 2/8] usb: dwc3-omap: Fix missing break in dwc3_omap_set_mailbox()
We need to break from all cases if we want to treat each one of them separately. Fixes: d2728fb3e01f ("usb: dwc3: omap: Pass VBUS and ID events transparently") Signed-off-by: Roger Quadros --- drivers/usb/dwc3/dwc3-omap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index eb1b9cb..35b6351 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -250,6 +250,7 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap, val = dwc3_omap_read_utmi_ctrl(omap); val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG; dwc3_omap_write_utmi_ctrl(omap, val); + break; case OMAP_DWC3_VBUS_OFF: val = dwc3_omap_read_utmi_ctrl(omap); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] ARM: dts: dra7x-evm: Enable dual-role mode for USB1
USB1 port is micro-AB type and can function as peripheral as well as host. Enable dual-role mode for USB1. Signed-off-by: Roger Quadros --- arch/arm/boot/dts/dra7-evm.dts | 2 +- arch/arm/boot/dts/dra72-evm-common.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts index 132f2be..d64e72d 100644 --- a/arch/arm/boot/dts/dra7-evm.dts +++ b/arch/arm/boot/dts/dra7-evm.dts @@ -731,7 +731,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; pinctrl-names = "default"; pinctrl-0 = <&usb1_pins>; }; diff --git a/arch/arm/boot/dts/dra72-evm-common.dtsi b/arch/arm/boot/dts/dra72-evm-common.dtsi index e50fbee..dc2c328 100644 --- a/arch/arm/boot/dts/dra72-evm-common.dtsi +++ b/arch/arm/boot/dts/dra72-evm-common.dtsi @@ -374,7 +374,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; }; &usb2 { -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] usb: dwc3: add dual-role support
If dr_mode is "otg" then support dual role mode of operation. Get ID and VBUS information from the OTG controller and put the controller in the appropriate state. This is our dual-role state table. ID VBUSdual-role state -- --- 0 x A_HOST - Host controller active 1 0 B_IDLE - Both Host and Gadget controllers inactive 1 1 B_PERIPHERAL - Gadget controller active Signed-off-by: Roger Quadros --- drivers/usb/dwc3/core.c | 583 -- drivers/usb/dwc3/core.h | 38 ++- drivers/usb/dwc3/gadget.c | 18 +- 3 files changed, 615 insertions(+), 24 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 369bab1..ca8b814 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -107,6 +108,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode) reg = dwc3_readl(dwc->regs, DWC3_GCTL); reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG)); reg |= DWC3_GCTL_PRTCAPDIR(mode); + dwc->current_mode = mode; dwc3_writel(dwc->regs, DWC3_GCTL, reg); } @@ -839,6 +841,495 @@ static int dwc3_core_get_phy(struct dwc3 *dwc) return 0; } +static int dwc3_drd_start_host(struct dwc3 *dwc, int on); +static int dwc3_drd_start_gadget(struct dwc3 *dwc, int on); + +/* dwc->lock must be held */ +static void dwc3_drd_statemachine(struct dwc3 *dwc, int id, int vbus) +{ + enum usb_otg_state new_state; + int protocol; + + if (id == dwc->otg_fsm.id && vbus == dwc->otg_fsm.b_sess_vld) + return; + + dwc->otg_fsm.id = id; + dwc->otg_fsm.b_sess_vld = vbus; + + if (!id) { + new_state = OTG_STATE_A_HOST; + } else{ + if (vbus) + new_state = OTG_STATE_B_PERIPHERAL; + else + new_state = OTG_STATE_B_IDLE; + } + + if (dwc->otg.state == new_state) + return; + + protocol = dwc->otg_fsm.protocol; + switch (new_state) { + case OTG_STATE_B_IDLE: + if (protocol == PROTO_GADGET) + dwc3_drd_start_gadget(dwc, 0); + else if (protocol == PROTO_HOST) + dwc3_drd_start_host(dwc, 0); + dwc->otg_fsm.protocol = PROTO_UNDEF; + break; + case OTG_STATE_B_PERIPHERAL: + if (protocol == PROTO_HOST) + dwc3_drd_start_host(dwc, 0); + + if (protocol != PROTO_GADGET) { + dwc->otg_fsm.protocol = PROTO_GADGET; + dwc3_drd_start_gadget(dwc, 1); + } + break; + case OTG_STATE_A_HOST: + if (protocol == PROTO_GADGET) + dwc3_drd_start_gadget(dwc, 0); + + if (protocol != PROTO_HOST) { + dwc->otg_fsm.protocol = PROTO_HOST; + dwc3_drd_start_host(dwc, 1); + } + break; + default: + dev_err(dwc->dev, "drd: invalid usb-drd state: %s\n", + usb_otg_state_string(new_state)); + return; + } + + dwc->otg.state = new_state; +} + +/* dwc->lock must be held */ +static void dwc3_otg_fsm_sync(struct dwc3 *dwc) +{ + u32 reg, osts; + int id, vbus; + + /* +* calling dwc3_otg_fsm_sync() during resume breaks host +* if adapter was removed during suspend as xhci driver +* is not prepared to see hcd removal before xhci_resume. +*/ + if (dwc->otg_prevent_sync) + return; + + do { + reg = dwc3_readl(dwc->regs, DWC3_OSTS); + + id = !!(reg & DWC3_OSTS_CONIDSTS); + vbus = !!(reg & DWC3_OSTS_BSESVLD); + + dwc3_drd_statemachine(dwc, id, vbus); + osts = dwc3_readl(dwc->regs, DWC3_OSTS); + /* +* OTG status might have changed and if we don't re-check +* here we will loose events as OTG events have been +* temporarily disabled. +*/ + /* FIXME: there is still a small time window where we can +* miss an OTG event. i.e. from here till where we enable the +* OEVTEN in OTG IRQ thread handler. Try another solution +* where OEVTEN can be enabled but otg irq in GIC is disabled. +* so any OTG changes can re-trigger the OTG IRQ after current +* ISR ends. +*/ + } while (osts != reg); +} + +static void dwc3_otg_mask_irq(struct dwc3 *dwc) +{ + dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN); + dwc3_writel(dwc->regs, DWC3_OEVTEN, 0); +} + +static void dwc3_otg_unmask_irq(struct dwc3 *dwc) +{ + dwc3_wri
[PATCH 3/8] usb: dwc3: use BIT() macro where possible
To avoid checkpatch warnings with new patches let's start using the BIT() macro wherever possible. Signed-off-by: Roger Quadros --- drivers/usb/dwc3/core.h | 192 +-- drivers/usb/dwc3/dwc3-omap.c | 48 +-- drivers/usb/dwc3/gadget.h| 20 ++--- 3 files changed, 130 insertions(+), 130 deletions(-) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 14b7602..d514dca 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -65,7 +65,7 @@ #define DWC3_DEVICE_EVENT_OVERFLOW 11 #define DWC3_GEVNTCOUNT_MASK 0xfffc -#define DWC3_GEVNTCOUNT_EHB(1 << 31) +#define DWC3_GEVNTCOUNT_EHBBIT(31) #define DWC3_GSNPSID_MASK 0x #define DWC3_GSNPSREV_MASK 0x @@ -175,11 +175,11 @@ /* Global RX Threshold Configuration Register */ #define DWC3_GRXTHRCFG_MAXRXBURSTSIZE(n) (((n) & 0x1f) << 19) #define DWC3_GRXTHRCFG_RXPKTCNT(n) (((n) & 0xf) << 24) -#define DWC3_GRXTHRCFG_PKTCNTSEL (1 << 29) +#define DWC3_GRXTHRCFG_PKTCNTSEL BIT(29) /* Global Configuration Register */ #define DWC3_GCTL_PWRDNSCALE(n)((n) << 19) -#define DWC3_GCTL_U2RSTECN (1 << 16) +#define DWC3_GCTL_U2RSTECN BIT(16) #define DWC3_GCTL_RAMCLKSEL(x) (((x) & DWC3_GCTL_CLK_MASK) << 6) #define DWC3_GCTL_CLK_BUS (0) #define DWC3_GCTL_CLK_PIPE (1) @@ -192,24 +192,24 @@ #define DWC3_GCTL_PRTCAP_DEVICE2 #define DWC3_GCTL_PRTCAP_OTG 3 -#define DWC3_GCTL_CORESOFTRESET(1 << 11) -#define DWC3_GCTL_SOFITPSYNC (1 << 10) +#define DWC3_GCTL_CORESOFTRESETBIT(11) +#define DWC3_GCTL_SOFITPSYNC BIT(10) #define DWC3_GCTL_SCALEDOWN(n) ((n) << 4) #define DWC3_GCTL_SCALEDOWN_MASK DWC3_GCTL_SCALEDOWN(3) -#define DWC3_GCTL_DISSCRAMBLE (1 << 3) -#define DWC3_GCTL_U2EXIT_LFPS (1 << 2) -#define DWC3_GCTL_GBLHIBERNATIONEN (1 << 1) -#define DWC3_GCTL_DSBLCLKGTNG (1 << 0) +#define DWC3_GCTL_DISSCRAMBLE BIT(3) +#define DWC3_GCTL_U2EXIT_LFPS BIT(2) +#define DWC3_GCTL_GBLHIBERNATIONEN BIT(1) +#define DWC3_GCTL_DSBLCLKGTNG BIT(0) /* Global User Control 1 Register */ -#define DWC3_GUCTL1_DEV_L1_EXIT_BY_HW (1 << 24) +#define DWC3_GUCTL1_DEV_L1_EXIT_BY_HW BIT(24) /* Global USB2 PHY Configuration Register */ -#define DWC3_GUSB2PHYCFG_PHYSOFTRST(1 << 31) -#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS (1 << 30) -#define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6) -#define DWC3_GUSB2PHYCFG_ULPI_UTMI (1 << 4) -#define DWC3_GUSB2PHYCFG_ENBLSLPM (1 << 8) +#define DWC3_GUSB2PHYCFG_PHYSOFTRSTBIT(31) +#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS BIT(30) +#define DWC3_GUSB2PHYCFG_SUSPHYBIT(6) +#define DWC3_GUSB2PHYCFG_ULPI_UTMI BIT(4) +#define DWC3_GUSB2PHYCFG_ENBLSLPM BIT(8) #define DWC3_GUSB2PHYCFG_PHYIF(n) (n << 3) #define DWC3_GUSB2PHYCFG_PHYIF_MASKDWC3_GUSB2PHYCFG_PHYIF(1) #define DWC3_GUSB2PHYCFG_USBTRDTIM(n) (n << 10) @@ -220,25 +220,25 @@ #define UTMI_PHYIF_8_BIT 0 /* Global USB2 PHY Vendor Control Register */ -#define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25) -#define DWC3_GUSB2PHYACC_BUSY (1 << 23) -#define DWC3_GUSB2PHYACC_WRITE (1 << 22) +#define DWC3_GUSB2PHYACC_NEWREGREQ BIT(25) +#define DWC3_GUSB2PHYACC_BUSY BIT(23) +#define DWC3_GUSB2PHYACC_WRITE BIT(22) #define DWC3_GUSB2PHYACC_ADDR(n) (n << 16) #define DWC3_GUSB2PHYACC_EXTEND_ADDR(n)(n << 8) #define DWC3_GUSB2PHYACC_DATA(n) (n & 0xff) /* Global USB3 PIPE Control Register */ -#define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) -#define DWC3_GUSB3PIPECTL_U2SSINP3OK (1 << 29) -#define DWC3_GUSB3PIPECTL_DISRXDETINP3 (1 << 28) -#define DWC3_GUSB3PIPECTL_REQP1P2P3(1 << 24) +#define DWC3_GUSB3PIPECTL_PHYSOFTRST BIT(31) +#define DWC3_GUSB3PIPECTL_U2SSINP3OK BIT(29) +#define DWC3_GUSB3PIPECTL_DISRXDETINP3 BIT(28) +#define DWC3_GUSB3PIPECTL_REQP1P2P3BIT(24) #define DWC3_GUSB3PIPECTL_DEP1P2P3(n) ((n) << 19) #define DWC3_GUSB3PIPECTL_DEP1P2P3_MASKDWC3_GUSB3PIPECTL_DEP1P2P3(7) #define DWC3_GUSB3PIPECTL_DEP1P2P3_EN DWC3_GUSB3PIPECTL_DEP1P2P3(1) -#define DWC3_GUSB3PIPECTL_DEPOCHANGE (1 << 18) -#define DWC3_GUSB3PIPECTL_SUSPHY (1 << 17) -#define DWC3_GUSB3PIPECTL_LFPSFILT (1 << 9) -#define DWC3_GUSB3PIPECTL_RX_DETOPOLL (1 << 8) +#define DWC3_GUSB3PIPECTL_DEPOCHANGE BIT(18) +#define DWC3_GUSB3PIPECTL_SUSPHY BIT(17) +#define DWC3_GUSB3PIPECTL_LFPSFILT BIT(9) +#define DWC3_GUSB3PIPECTL_RX_DETOPOLL BIT(8) #define DWC3_GUSB3PIPECTL_TX_DEEPH_MASKDWC3_GUSB3PIPECTL_TX_DEEPH(3) #define DWC3_GUSB3PIPECTL_TX_DEEPH(n) ((n) << 1) @@ -247,7 +247,7 @@ #define DWC3_GTXFIFOSIZ_TXFSTADDR(n) ((n) & 0x) /* Global Event Size Registers */ -#define DWC3_GEVNTSIZ_INTMASK (1 << 31) +#define DWC3_GEVNTSIZ_INTMASK BIT(31) #define DWC3
[PATCH 0/8] usb: dwc3: add dual-role support
Hi, We rely on the OTG controller block to provide us with VBUS and ID line status via an interrupt. This is then used to switch the controller between host, peripheral and idle roles based on the following table. ID VBUSdual-role state -- --- 0 x A_HOST - Host controller active 1 0 B_IDLE - Both Host and Gadget controllers inactive 1 1 B_PERIPHERAL - Gadget controller active Couple of things to clarify: - There is a small window where we can potentially miss an event related to OTG. I've added a comment in the code where this could happen. How can we prevent this? Is it better to just leave the OTG events unmasked (but keep otg_irq on ARM GIC disabled) so that any new events can be captured by the OTG event register and interrupt re-triggered if it has not been serviced by the previous interrupt. - I'm running the entire dual-role state change logic inside the threaded interrupt handler with dwc->lock (spinlock) held but IRQs enabled. OTG events are very rare i.e. manual intervention so I don't see this as a problem. Just wanted to double check. - Some SoC's (e.g. Qualcomm MSM) do not have the OTG controller block but do have both host and peripheral controllers and so can operate in dual role mode. Current series does not address this case. We can get dual-role to work with such SoCs if core.c can get information about ID and VBUS somehow (private interface from parent or directly read extcon?). cheers, -roger Roger Quadros (8): usb: otg-fsm: Prevent build warning "VDBG" redefined usb: dwc3-omap: Fix missing break in dwc3_omap_set_mailbox() usb: dwc3: use BIT() macro where possible usb: dwc3: core.h: add some register definitions usb: dwc3: add dual-role support ARM: dts: dra7x-evm: Enable dual-role mode for USB1 ARM: dts: am43xx: Enable dual-role mode for USB1 ARM: dts: am57xx-idk: Enable dual-role mode for USB2 arch/arm/boot/dts/am437x-gp-evm.dts | 2 +- arch/arm/boot/dts/am437x-sk-evm.dts | 2 +- arch/arm/boot/dts/am43x-epos-evm.dts | 2 +- arch/arm/boot/dts/am57xx-idk-common.dtsi | 2 +- arch/arm/boot/dts/dra7-evm.dts | 2 +- arch/arm/boot/dts/dra72-evm-common.dtsi | 2 +- drivers/usb/common/usb-otg-fsm.c | 7 + drivers/usb/dwc3/core.c | 583 ++- drivers/usb/dwc3/core.h | 312 - drivers/usb/dwc3/dwc3-omap.c | 49 +-- drivers/usb/dwc3/gadget.c| 18 +- drivers/usb/dwc3/gadget.h| 20 +- drivers/usb/phy/phy-fsl-usb.c| 7 + include/linux/usb/otg-fsm.h | 15 - 14 files changed, 848 insertions(+), 175 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
On Mon, Jan 23, 2017 at 12:20:03PM +0100, Greg Kroah-Hartman wrote: > On Mon, Jan 23, 2017 at 12:59:33PM +0200, Felipe Balbi wrote: > > > > Hi, > > > > Petr Cvek writes: > > > It seems the USB product ID for g_webcam usb device gadget is > > > incorrectly used from EEM gadget "Ethernet Emulation Model". So > > > "webcam" device has a confusing description in lsusb: > > > > > > 1d6b:0102 Linux Foundation EEM Gadget > > > > > > I would change it to 0x0106, which is a next unassigned value > > > (according to http://www.linux-usb.org/usb.ids). Does this step > > > require some official communication with the holder of VID (Linux > > > Foundation)? Or all it takes is just to send patch to the kernel and > > > to the usb.ids database? > > > > > > I know it is only a cosmetic change on a legacy driver, but I assume > > > it would be better to have some default value for configfs API than to > > > borrow a PID from a whole different gadget. > > > > > > Change in question would be something like this (I will send a proper > > > patch after RFC): > > > > > > Fixing USB Product ID for UVC gadget (used from Ethernet Emulation > > > Model) and changing it to new one. > > > > Greg, do we still have an official Vendor ID? > > Well, as official as we are allowed to get :) > > > If so, any chance we can > > allocate a unique default ID for the webcam gadget? > > Hm, I thought we wanted to get rid of the "legacy" gadget stuff. Who is > using it these days? It's not Android... > > If it's really needed, then yes, you can use 0106, just let me know. > But do you really need a new id? What's wrong with 0102? Oops, no, 0106 is already reserved. 0107 is the next "free" number. 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
[PATCH 1/8] usb: otg-fsm: Prevent build warning "VDBG" redefined
If usb/otg-fsm.h and usb/composite.h are included together then it results in the build warning [1]. Prevent that by defining VDBG locally. Also get rid of MPC_LOC which doesn't seem to be used by anyone. [1] - warning fixed by this patch: In file included from drivers/usb/dwc3/core.h:33, from drivers/usb/dwc3/ep0.c:33: include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined In file included from drivers/usb/dwc3/ep0.c:31: include/linux/usb/composite.h:615:1: warning: this is the location of the previous definition Signed-off-by: Roger Quadros Reviewed-by: Jun Li Acked-by: Peter Chen --- drivers/usb/common/usb-otg-fsm.c | 7 +++ drivers/usb/phy/phy-fsl-usb.c| 7 +++ include/linux/usb/otg-fsm.h | 15 --- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 2f537bb..b8fe31e 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -31,6 +31,13 @@ #include #include +#ifdef VERBOSE +#define VDBG(fmt, args...) pr_debug("[%s] " fmt, \ +__func__, ## args) +#else +#define VDBG(stuff...) do {} while (0) +#endif + /* Change USB protocol when there is a protocol change */ static int otg_set_protocol(struct otg_fsm *fsm, int protocol) { diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c index 94eb292..a8784ec 100644 --- a/drivers/usb/phy/phy-fsl-usb.c +++ b/drivers/usb/phy/phy-fsl-usb.c @@ -44,6 +44,13 @@ #include "phy-fsl-usb.h" +#ifdef VERBOSE +#define VDBG(fmt, args...) pr_debug("[%s] " fmt, \ +__func__, ## args) +#else +#define VDBG(stuff...) do {} while (0) +#endif + #define DRIVER_VERSION "Rev. 1.55" #define DRIVER_AUTHOR "Jerry Huang/Li Yang" #define DRIVER_DESC "Freescale USB OTG Transceiver Driver" diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index 7a03505..a0a8f87 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -21,21 +21,6 @@ #include #include -#undef VERBOSE - -#ifdef VERBOSE -#define VDBG(fmt, args...) pr_debug("[%s] " fmt , \ -__func__, ## args) -#else -#define VDBG(stuff...) do {} while (0) -#endif - -#ifdef VERBOSE -#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__) -#else -#define MPC_LOC do {} while (0) -#endif - #define PROTO_UNDEF(0) #define PROTO_HOST (1) #define PROTO_GADGET (2) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] ARM: dts: am43xx: Enable dual-role mode for USB1
USB1 port is micro-AB type and can function as peripheral as well as host. Enable dual-role mode for USB1. Signed-off-by: Roger Quadros --- arch/arm/boot/dts/am437x-gp-evm.dts | 2 +- arch/arm/boot/dts/am437x-sk-evm.dts | 2 +- arch/arm/boot/dts/am43x-epos-evm.dts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts index 957840c..74728cc 100644 --- a/arch/arm/boot/dts/am437x-gp-evm.dts +++ b/arch/arm/boot/dts/am437x-gp-evm.dts @@ -772,7 +772,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; status = "okay"; }; diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts index 319d942..cfffce8 100644 --- a/arch/arm/boot/dts/am437x-sk-evm.dts +++ b/arch/arm/boot/dts/am437x-sk-evm.dts @@ -596,7 +596,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; status = "okay"; pinctrl-names = "default"; pinctrl-0 = <&usb1_pins>; diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts index 9d35c3f..8a48d9b 100644 --- a/arch/arm/boot/dts/am43x-epos-evm.dts +++ b/arch/arm/boot/dts/am43x-epos-evm.dts @@ -669,7 +669,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; status = "okay"; }; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] usb: dwc3: core.h: add some register definitions
Add OTG and GHWPARAMS6 register definitions Signed-off-by: Roger Quadros --- drivers/usb/dwc3/core.h | 82 + 1 file changed, 82 insertions(+) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index d514dca..fc82d2e 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -204,6 +204,15 @@ /* Global User Control 1 Register */ #define DWC3_GUCTL1_DEV_L1_EXIT_BY_HW BIT(24) +/* Global Status Register */ +#define DWC3_GSTS_OTG_IP BIT(10) +#define DWC3_GSTS_BC_IPBIT(9) +#define DWC3_GSTS_ADP_IP BIT(8) +#define DWC3_GSTS_HOST_IP BIT(7) +#define DWC3_GSTS_DEVICE_IPBIT(6) +#define DWC3_GSTS_CSR_TIMEOUT BIT(5) +#define DWC3_GSTS_BUS_ERR_ADDR_VLD BIT(4) + /* Global USB2 PHY Configuration Register */ #define DWC3_GUSB2PHYCFG_PHYSOFTRSTBIT(31) #define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS BIT(30) @@ -288,6 +297,11 @@ #define DWC3_MAX_HIBER_SCRATCHBUFS 15 /* Global HWPARAMS6 Register */ +#define DWC3_GHWPARAMS6_BCSUPPORT BIT(14) +#define DWC3_GHWPARAMS6_OTG3SUPPORTBIT(13) +#define DWC3_GHWPARAMS6_ADPSUPPORT BIT(12) +#define DWC3_GHWPARAMS6_HNPSUPPORT BIT(11) +#define DWC3_GHWPARAMS6_SRPSUPPORT BIT(10) #define DWC3_GHWPARAMS6_EN_FPGABIT(7) /* Global HWPARAMS7 Register */ @@ -469,6 +483,74 @@ #define DWC3_DEV_IMOD_INTERVAL_SHIFT 0 #define DWC3_DEV_IMOD_INTERVAL_MASK(0x << 0) +/* OTG Configuration Register */ +#define DWC3_OCFG_DISPWRCUTTOFFBIT(5) +#define DWC3_OCFG_HIBDISMASK BIT(4) +#define DWC3_OCFG_SFTRSTMASK BIT(3) +#define DWC3_OCFG_OTGVERSION BIT(2) +#define DWC3_OCFG_HNPCAP BIT(1) +#define DWC3_OCFG_SRPCAP BIT(0) + +/* OTG CTL Register */ +#define DWC3_OCTL_OTG3GOERRBIT(7) +#define DWC3_OCTL_PERIMODE BIT(6) +#define DWC3_OCTL_PRTPWRCTLBIT(5) +#define DWC3_OCTL_HNPREQ BIT(4) +#define DWC3_OCTL_SESREQ BIT(3) +#define DWC3_OCTL_TERMSELIDPULSE BIT(2) +#define DWC3_OCTL_DEVSETHNPEN BIT(1) +#define DWC3_OCTL_HSTSETHNPEN BIT(0) + +/* OTG Event Register */ +#define DWC3_OEVT_DEVICEMODE BIT(31) +#define DWC3_OEVT_XHCIRUNSTPSETBIT(27) +#define DWC3_OEVT_DEVRUNSTPSET BIT(26) +#define DWC3_OEVT_HIBENTRY BIT(25) +#define DWC3_OEVT_CONIDSTSCHNG BIT(24) +#define DWC3_OEVT_HRRCONFNOTIF BIT(23) +#define DWC3_OEVT_HRRINITNOTIF BIT(22) +#define DWC3_OEVT_ADEVIDLE BIT(21) +#define DWC3_OEVT_ADEVBHOSTEND BIT(20) +#define DWC3_OEVT_ADEVHOST BIT(19) +#define DWC3_OEVT_ADEVHNPCHNG BIT(18) +#define DWC3_OEVT_ADEVSRPDET BIT(17) +#define DWC3_OEVT_ADEVSESSENDDET BIT(16) +#define DWC3_OEVT_BDEVBHOSTEND BIT(11) +#define DWC3_OEVT_BDEVHNPCHNG BIT(10) +#define DWC3_OEVT_BDEVSESSVLDDET BIT(9) +#define DWC3_OEVT_BDEVVBUSCHNG BIT(8) +#define DWC3_OEVT_BSESSVLD BIT(3) +#define DWC3_OEVT_HSTNEGSTSBIT(2) +#define DWC3_OEVT_SESREQSTSBIT(1) +#define DWC3_OEVT_ERRORBIT(0) + +/* OTG Event Enable Register */ +#define DWC3_OEVTEN_XHCIRUNSTPSETENBIT(27) +#define DWC3_OEVTEN_DEVRUNSTPSETEN BIT(26) +#define DWC3_OEVTEN_HIBENTRYEN BIT(25) +#define DWC3_OEVTEN_CONIDSTSCHNGEN BIT(24) +#define DWC3_OEVTEN_HRRCONFNOTIFEN BIT(23) +#define DWC3_OEVTEN_HRRINITNOTIFEN BIT(22) +#define DWC3_OEVTEN_ADEVIDLEEN BIT(21) +#define DWC3_OEVTEN_ADEVBHOSTENDEN BIT(20) +#define DWC3_OEVTEN_ADEVHOSTEN BIT(19) +#define DWC3_OEVTEN_ADEVHNPCHNGEN BIT(18) +#define DWC3_OEVTEN_ADEVSRPDETEN BIT(17) +#define DWC3_OEVTEN_ADEVSESSENDDETEN BIT(16) +#define DWC3_OEVTEN_BDEVHOSTENDEN BIT(11) +#define DWC3_OEVTEN_BDEVHNPCHNGEN BIT(10) +#define DWC3_OEVTEN_BDEVSESSVLDDETEN BIT(9) +#define DWC3_OEVTEN_BDEVVBUSCHNGE BIT(8) + +/* OTG Status Register */ +#define DWC3_OSTS_DEVRUNSTPBIT(13) +#define DWC3_OSTS_XHCIRUNSTP BIT(12) +#define DWC3_OSTS_PERIPHERALSTATE BIT(4) +#define DWC3_OSTS_XHCIPRTPOWER BIT(3) +#define DWC3_OSTS_BSESVLD BIT(2) +#define DWC3_OSTS_VBUSVLD BIT(1) +#define DWC3_OSTS_CONIDSTS BIT(0) + /* Structures */ struct dwc3_trb; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] ARM: dts: am57xx-idk: Enable dual-role mode for USB2
USB port is micro-AB type and can function as peripheral as well as host. Enable dual-role mode for USB2. Signed-off-by: Roger Quadros --- arch/arm/boot/dts/am57xx-idk-common.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/am57xx-idk-common.dtsi b/arch/arm/boot/dts/am57xx-idk-common.dtsi index 814a720..14bdb24 100644 --- a/arch/arm/boot/dts/am57xx-idk-common.dtsi +++ b/arch/arm/boot/dts/am57xx-idk-common.dtsi @@ -376,7 +376,7 @@ }; &usb2 { - dr_mode = "peripheral"; + dr_mode = "otg"; }; &mmc2 { -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
On Mon, Jan 23, 2017 at 11:30:01AM +0100, Petr Cvek wrote: > Hello, > > It seems the USB product ID for g_webcam usb device gadget is incorrectly > used from EEM gadget "Ethernet Emulation Model". So "webcam" device has a > confusing description in lsusb: > > 1d6b:0102 Linux Foundation EEM Gadget > > I would change it to 0x0106, which is a next unassigned value > (according to http://www.linux-usb.org/usb.ids). Does this step > require some official communication with the holder of VID (Linux > Foundation)? Or all it takes is just to send patch to the kernel and > to the usb.ids database? Note, usb.ids doesn't seem to have picked up the last few changes I asked to reserve there, here's the latest reserved id list from the LF that I am in charge of: # Linux Foundation USB id list. 1d6b Linux Foundation 0001 1.1 root hub 0002 2.0 root hub 0003 3.0 root hub 0100 PTP Gadget 0101 Audio Gadget 0102 EEM Gadget 0103 NCM (Ethernet) Gadget 0104 Multifunction Composite Gadget 0105 FunctionFS Gadget 0106 Composite Gadget: ACM + Mass Storage 0200 Qemu Audio Device 0246 BlueZ Host Stack 0247 BlueZ for Android > I know it is only a cosmetic change on a legacy driver, but I assume > it would be better to have some default value for configfs API than to > borrow a PID from a whole different gadget. For class devices, they really don't need a new id, we should just use only one of them as it doesn't matter :) 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
Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
Hi, Greg KH writes: > On Mon, Jan 23, 2017 at 11:30:01AM +0100, Petr Cvek wrote: >> Hello, >> >> It seems the USB product ID for g_webcam usb device gadget is incorrectly >> used from EEM gadget "Ethernet Emulation Model". So "webcam" device has a >> confusing description in lsusb: >> >> 1d6b:0102 Linux Foundation EEM Gadget >> >> I would change it to 0x0106, which is a next unassigned value >> (according to http://www.linux-usb.org/usb.ids). Does this step >> require some official communication with the holder of VID (Linux >> Foundation)? Or all it takes is just to send patch to the kernel and >> to the usb.ids database? > > Note, usb.ids doesn't seem to have picked up the last few changes I > asked to reserve there, here's the latest reserved id list from the LF > that I am in charge of: > > # Linux Foundation USB id list. > 1d6b Linux Foundation > 0001 1.1 root hub > 0002 2.0 root hub > 0003 3.0 root hub > 0100 PTP Gadget > 0101 Audio Gadget > 0102 EEM Gadget > 0103 NCM (Ethernet) Gadget > 0104 Multifunction Composite Gadget > 0105 FunctionFS Gadget > 0106 Composite Gadget: ACM + Mass Storage > 0200 Qemu Audio Device > 0246 BlueZ Host Stack > 0247 BlueZ for Android > >> I know it is only a cosmetic change on a legacy driver, but I assume >> it would be better to have some default value for configfs API than to >> borrow a PID from a whole different gadget. > > For class devices, they really don't need a new id, we should just use > only one of them as it doesn't matter :) fine by me. Just lsusb will look funky ;-) -- balbi signature.asc Description: PGP signature
Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
On Mon, Jan 23, 2017 at 01:27:59PM +0200, Felipe Balbi wrote: > > Hi, > > Greg KH writes: > > On Mon, Jan 23, 2017 at 11:30:01AM +0100, Petr Cvek wrote: > >> Hello, > >> > >> It seems the USB product ID for g_webcam usb device gadget is incorrectly > >> used from EEM gadget "Ethernet Emulation Model". So "webcam" device has a > >> confusing description in lsusb: > >> > >>1d6b:0102 Linux Foundation EEM Gadget > >> > >> I would change it to 0x0106, which is a next unassigned value > >> (according to http://www.linux-usb.org/usb.ids). Does this step > >> require some official communication with the holder of VID (Linux > >> Foundation)? Or all it takes is just to send patch to the kernel and > >> to the usb.ids database? > > > > Note, usb.ids doesn't seem to have picked up the last few changes I > > asked to reserve there, here's the latest reserved id list from the LF > > that I am in charge of: > > > > # Linux Foundation USB id list. > > 1d6b Linux Foundation > > 0001 1.1 root hub > > 0002 2.0 root hub > > 0003 3.0 root hub > > 0100 PTP Gadget > > 0101 Audio Gadget > > 0102 EEM Gadget > > 0103 NCM (Ethernet) Gadget > > 0104 Multifunction Composite Gadget > > 0105 FunctionFS Gadget > > 0106 Composite Gadget: ACM + Mass Storage > > 0200 Qemu Audio Device > > 0246 BlueZ Host Stack > > 0247 BlueZ for Android > > > >> I know it is only a cosmetic change on a legacy driver, but I assume > >> it would be better to have some default value for configfs API than to > >> borrow a PID from a whole different gadget. > > > > For class devices, they really don't need a new id, we should just use > > only one of them as it doesn't matter :) > > fine by me. Just lsusb will look funky ;-) Heh, true, but I thought lsusb would use a string if the device provided it. Haven't looked at that portion of the code in a very long time... -- 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 4/4] usb: gadget: f_hid: fix: Move IN request allocation to set_alt()
Krzysztof Opasiak writes: > Since commit ba1582f22231 ("usb: gadget: f_hid: use alloc_ep_req()") > we cannot allocate any requests in bind() as we check if we should > align request buffer based on endpoint descriptor which is assigned > in set_alt(). > > Allocating request in bind() function causes a NULL pointer > dereference. > > This commit moves allocation of IN request from bind() to set_alt() > to prevent this issue. > > Fixes: ba1582f22231 ("usb: gadget: f_hid: use alloc_ep_req()") > Cc: sta...@vger.kernel.org > Signed-off-by: Krzysztof Opasiak please rebase on testing/next: checking file drivers/usb/gadget/function/f_hid.c Hunk #1 succeeded at 338 (offset 54 lines). Hunk #2 succeeded at 348 (offset 54 lines). Hunk #3 succeeded at 363 (offset 54 lines). Hunk #4 succeeded at 376 (offset 54 lines). Hunk #5 succeeded at 611 (offset 54 lines). Hunk #6 succeeded at 648 (offset 54 lines). Hunk #7 FAILED at 611. Hunk #8 succeeded at 686 (offset 54 lines). Hunk #9 succeeded at 765 (offset 54 lines). Hunk #10 succeeded at 802 (offset 65 lines). Hunk #11 succeeded at 1068 (offset 65 lines). 1 out of 11 hunks FAILED previous 3 patches applied fine, though. -- balbi signature.asc Description: PGP signature
Re: [PATCH 0/8] usb: dwc3: add dual-role support
Hi Roger, On 2017년 01월 23일 20:19, Roger Quadros wrote: > - Some SoC's (e.g. Qualcomm MSM) do not have the OTG controller block > but do have both host and peripheral controllers and so can operate > in dual role mode. Current series does not address this case. > We can get dual-role to work with such SoCs if core.c can get > information about ID and VBUS somehow (private interface > from parent or directly read extcon?). Also, Exynos SoC doesn't include the physical otg block to detect the type of external connector(HOST or Peripheral). So, our team member and me are preparing the patches to identify the type of external connector with extcon This approach is not dependent on any specific SoC. Maybe, we will send the RFC patches within this month. -- Best Regards, Chanwoo Choi S/W R&D Center Samsung Electronics -- 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 v2 10/28] usb: dwc2: Remove unused otg_ver parameter
John Youn writes: > The otg_ver parameter only controls the SRP pulsing method and defaults > to the 1.3 behavior. It is unused and can be removed. > > Signed-off-by: John Youn this patch fails to apply: checking file drivers/usb/dwc2/core.c checking file drivers/usb/dwc2/core.h Hunk #2 succeeded at 467 (offset 6 lines). Hunk #3 succeeded at 1180 (offset 9 lines). checking file drivers/usb/dwc2/hcd.c Hunk #1 succeeded at 2258 (offset -2 lines). checking file drivers/usb/dwc2/params.c Hunk #3 succeeded at 106 (offset 7 lines). Hunk #4 succeeded at 136 (offset 7 lines). Hunk #5 succeeded at 166 (offset 7 lines). Hunk #6 succeeded at 196 (offset 7 lines). Hunk #7 succeeded at 970 with fuzz 1 (offset -2 lines). Hunk #8 FAILED at 1132. 1 out of 8 hunks FAILED please rebase on testing/next -- balbi signature.asc Description: PGP signature
Re: [PATCH 2/2] usb: gadget: uac2: add req_number as parameter
Hi, Peter Chen writes: > On Tue, Jan 17, 2017 at 05:57:50PM +0800, Peter Chen wrote: >> On Tue, Jan 17, 2017 at 11:23:55AM +0200, Felipe Balbi wrote: >> > >> > Hi, >> > >> > Peter Chen writes: >> > > On Mon, Jan 16, 2017 at 12:40:06PM +0200, Felipe Balbi wrote: >> > >> >> > >> Hi, >> > >> >> > >> Peter Chen writes: >> > >> > There are only two requests for uac2, it may not be enough at high >> > >> > loading system which usb interrupt handler can't be serviced on >> > >> > time, then the data will be lost since it is isoc transfer for audio. >> > >> > >> > >> > In this patch, we introduce a parameter for the number for usb >> > >> > request, >> > >> > and the user can override it if current number for request is not >> > >> > enough >> > >> > for his/her use case. >> > >> > >> > >> > Besides, update this parameter for legacy audio gadget and >> > >> > documentation. >> > >> >> > >> I would rather just drop pre-allocation of requests. Every time Audio >> > >> wants transmit data you allocate and queue a request there and free() >> > >> the request on completion. >> > >> >> > >> All of these functions already have a ton of parameters :-s >> > >> >> > > >> > > At high loading system, how can we make sure the interrupt can be >> > > serviced in one isoc time slice? We ran out this problem at one >> > > customer project, and even at default mainline, the aplay will >> > > meet underrun using UAC2. >> > >> > well, if you don't have any limits to how deep your queue can be, that >> > should cover it, no? Another way to look at this is that 2 requests >> > really _is_ too little, and that can be calculated. How much data do you >> > want to send per interval and what is the interval? Currently, uac2 is >> > using 1024-byte requests with bInterval of 4, which means an interval of >> > 1ms (2^(4-1) * 125 us = 1000 us). >> > >> > I can see how we would miss an interval if our CPU is hogged by another >> > task and our interrupt handler can't run for over 1ms. Maybe, as a fix, >> > we should just increase USB_XFERS to a value that would work. Doubling >> > it will make us allocate exactly one PAGE (4096) for rbuf. What are you >> > passing as req_number on this customer project? >> > >> > -- >> > balbi >> >> Set req_number as 4 can let aplay work well using mainline kernel. >> For the customer project, the number is 20 since they don't care >> the memory. The system latency and memory requirement are different >> per projects, why not let user choose it? >> > > Any more comments, Felipe? If you don't like extra parameter, I can > have a patch to change USB_XFERS as 4. I've applied with the extra parameters. You're right, if we just change USB_XFERS to 4 we're probably gonna have that parameter being changed every few months or so. -- balbi signature.asc Description: PGP signature
Re: [BISSECTED]: BUG: usb: dwc3: usb ports not working anymore on odroid-XU4
Hi, Richard Genoud writes: > On 19/01/2017 09:03, Felipe Balbi wrote: >> >> Hi, >> >> Richard Genoud writes: >>> Hi, >>> Since commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit >>> paths") >>> (merged in 4.8), the usb ports on odroid-XU4 don't work anymore. >>> >>> [ Actually, it's commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all >>> cores"), cf below ] >>> >>> Inserting an usb key (USB2.0) on the USB3.0 port result in: >>> [ 64.488264] xhci-hcd xhci-hcd.2.auto: Port resume took longer than 2 >>> msec, port status = 0xc400fe3 >>> [ 74.568156] xhci-hcd xhci-hcd.2.auto: xHCI host not responding to stop >>> endpoint command. >>> [ 74.574806] xhci-hcd xhci-hcd.2.auto: Assuming host is dying, halting >>> host. >>> [ 74.601970] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up >>> [ 74.606276] usb 3-1: USB disconnect, device number 2 >>> [ 74.613565] usb 4-1: USB disconnect, device number 2 >>> [ 74.621208] usb usb3-port1: couldn't allocate usb_device >>> NB: it's not related to USB2.0 devices, I get the same result with an >>> USB3.0 device (SATA to USB3 for instance). >>> NB2: it doesn't happen on an odriod-XU3 board, that doesn't have the >>> realtek RTL8153 chip. >>> >>> I instrumented what was read/written in the registers before and after this >>> patch, and I found that the culprit is: >>> if (dwc->revision > DWC3_REVISION_194A) >>> reg |= DWC3_GUSB3PIPECTL_SUSPHY; >>> Before commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit >>> paths"), dwc3_phy_setup() was >>> done early in dwc3_probe() and thus, dwc->revision wasn't set yet (==0) >>> After this commit, dwc3_phy_setup() is done in dwc3_core_init(), after >>> setting dwc->revision (it's done in dwc3_core_is_valid()). >>> >>> (The dwc3->revision on odroid-XU4 is 5533200a.) >>> >>> If I comment out the 2 lines: >>> if (dwc->revision > DWC3_REVISION_194A) >>> reg |= DWC3_GUSB3PIPECTL_SUSPHY; >>> The usb key is recognized right away: >>> [ 38.008158] usb 3-1.2: new high-speed USB device number 3 using xhci-hcd >>> [ 38.138924] usb 3-1.2: New USB device found, idVendor=abcd, >>> idProduct=1234 >>> [ 38.144399] usb 3-1.2: New USB device strings: Mfr=1, Product=2, >>> SerialNumber=3 >>> >>> I took a look at the history behind those 2 lines, and they've been >>> introduced by: >>> commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores") in 3.19. >>> In 3.19, the dwc->revision dwc3_phy_setup() was set in dwc3_core_init(). >>> And, booting a 3.19 kernel on XU4 gives the same error (whereas booting a >>> 3.18 kernel is ok) >>> >>> [ So, I should have started this email with: >>> Since commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"), >>> the usb ports on odroid-XU4 don't work anymore. ] >>> >>> The funny thing is that commit 45bb7de213d8("usb: dwc3: setup phys >>> earlier") (merged in 4.2) >>> moved dwc3_phy_setup() in dwc3_probe(), way before dwc->revision was set, >>> acting like a revert >>> on commit 2164a476205c("usb: dwc3: set SUSPHY bit for all cores") >>> That's why on 4.2, inserting an USB key on odroid-XU4 worked again, >>> >>> So, to make a resume: >>> xxx-> 3.18: usb ok >>> 3.19->4.1: usb error (due to commit 2164a476205ccc ("usb: dwc3: set SUSPHY >>> bit for all cores")) >>> 4.2->4.7: usb ok (due to commit 45bb7de213d8("usb: dwc3: setup phys >>> earlier")) >>> 4.8->now: usb error (due to commit c499ff71ff2a2 ("usb: dwc3: core: >>> re-factor init and exit paths")) >>> >>> >>> any idea on this ? >> >> Thanks for bisecting. This could be caused by a similar case as >> described at [1]. Can you check if adding quirk >> "snps,dis_u3_susphy_quirk" helps ? >> >> [1] >> https://marc.info/?i=2b3535c5ece8b5419e3ecbe300772909021b3fd...@us01wembx2.internal.synopsys.com >> > Thanks for your quick answer ! > > yes, adding this patch: > > diff --git a/arch/arm/boot/dts/exynos5422-odroidxu4.dts > b/arch/arm/boot/dts/exynos5422-odroidxu4.dts > index 2faf88627a48..c76e76bc9ade 100644 > --- a/arch/arm/boot/dts/exynos5422-odroidxu4.dts > +++ b/arch/arm/boot/dts/exynos5422-odroidxu4.dts > @@ -43,6 +43,9 @@ > status = "okay"; > }; > > +&usbdrd_dwc3_0 { > + snps,dis_u3_susphy_quirk; > +}; > &usbdrd_dwc3_1 { > dr_mode = "host"; > }; > > removes the error "Port resume took longer than 2 msec, port > status = 0xc400fe3" and the USB devices are detected. okay great. Temporarily, you can probably push a patch doing that to your DTS. At least you're gonna have things working until we understand what's going on with DWC3. -- balbi signature.asc Description: PGP signature
Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
Dne 23.1.2017 v 12:32 Greg KH napsal(a): I know it is only a cosmetic change on a legacy driver, but I assume it would be better to have some default value for configfs API than to borrow a PID from a whole different gadget. >>> >>> For class devices, they really don't need a new id, we should just use >>> only one of them as it doesn't matter :) >> So using 0106 "Composite gadget: ACM + Mass Storage" or 0104 "Multifunction Composite Gadget" should be fine? (my actual setup is not multifunction though). I'm actually trying to catch bug, when using configfs access webcam does not work or kernel oopses (and webcam does not work) :-D. >> fine by me. Just lsusb will look funky ;-) > > Heh, true, but I thought lsusb would use a string if the device provided > it. Haven't looked at that portion of the code in a very long time... > My lsusb shows separate strings (using usbutils from slackware64-current): Bus 004 Device 003: ID 1d6b:0102 Linux Foundation EEM Gadget ... idVendor 0x1d6b Linux Foundation idProduct 0x0102 EEM Gadget bcdDevice4.07 iManufacturer 1 Linux Foundation iProduct2 Webcam gadget ... Best regards, Petr -- 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: dwc2: host: Correct snpsid checking for GDFIFOCFG
John Youn writes: > From: Sevak Arakelyan > > GDFIFOCFG is available from IP version 2.91a. Fix the code to reflect > this. > > Signed-off-by: Sevak Arakelyan > Signed-off-by: John Youn doesn't apply :-( please rebase -- balbi signature.asc Description: PGP signature
Re: [BISSECTED]: BUG: usb: dwc3: usb ports not working anymore on odroid-XU4
2017-01-23 12:45 GMT+01:00 Felipe Balbi : > > Hi, > > Richard Genoud writes: >> On 19/01/2017 09:03, Felipe Balbi wrote: >>> >>> Hi, >>> >>> Richard Genoud writes: Hi, Since commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit paths") (merged in 4.8), the usb ports on odroid-XU4 don't work anymore. [ Actually, it's commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"), cf below ] Inserting an usb key (USB2.0) on the USB3.0 port result in: [ 64.488264] xhci-hcd xhci-hcd.2.auto: Port resume took longer than 2 msec, port status = 0xc400fe3 [ 74.568156] xhci-hcd xhci-hcd.2.auto: xHCI host not responding to stop endpoint command. [ 74.574806] xhci-hcd xhci-hcd.2.auto: Assuming host is dying, halting host. [ 74.601970] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up [ 74.606276] usb 3-1: USB disconnect, device number 2 [ 74.613565] usb 4-1: USB disconnect, device number 2 [ 74.621208] usb usb3-port1: couldn't allocate usb_device NB: it's not related to USB2.0 devices, I get the same result with an USB3.0 device (SATA to USB3 for instance). NB2: it doesn't happen on an odriod-XU3 board, that doesn't have the realtek RTL8153 chip. I instrumented what was read/written in the registers before and after this patch, and I found that the culprit is: if (dwc->revision > DWC3_REVISION_194A) reg |= DWC3_GUSB3PIPECTL_SUSPHY; Before commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit paths"), dwc3_phy_setup() was done early in dwc3_probe() and thus, dwc->revision wasn't set yet (==0) After this commit, dwc3_phy_setup() is done in dwc3_core_init(), after setting dwc->revision (it's done in dwc3_core_is_valid()). (The dwc3->revision on odroid-XU4 is 5533200a.) If I comment out the 2 lines: if (dwc->revision > DWC3_REVISION_194A) reg |= DWC3_GUSB3PIPECTL_SUSPHY; The usb key is recognized right away: [ 38.008158] usb 3-1.2: new high-speed USB device number 3 using xhci-hcd [ 38.138924] usb 3-1.2: New USB device found, idVendor=abcd, idProduct=1234 [ 38.144399] usb 3-1.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3 I took a look at the history behind those 2 lines, and they've been introduced by: commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores") in 3.19. In 3.19, the dwc->revision dwc3_phy_setup() was set in dwc3_core_init(). And, booting a 3.19 kernel on XU4 gives the same error (whereas booting a 3.18 kernel is ok) [ So, I should have started this email with: Since commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"), the usb ports on odroid-XU4 don't work anymore. ] The funny thing is that commit 45bb7de213d8("usb: dwc3: setup phys earlier") (merged in 4.2) moved dwc3_phy_setup() in dwc3_probe(), way before dwc->revision was set, acting like a revert on commit 2164a476205c("usb: dwc3: set SUSPHY bit for all cores") That's why on 4.2, inserting an USB key on odroid-XU4 worked again, So, to make a resume: xxx-> 3.18: usb ok 3.19->4.1: usb error (due to commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores")) 4.2->4.7: usb ok (due to commit 45bb7de213d8("usb: dwc3: setup phys earlier")) 4.8->now: usb error (due to commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit paths")) any idea on this ? >>> >>> Thanks for bisecting. This could be caused by a similar case as >>> described at [1]. Can you check if adding quirk >>> "snps,dis_u3_susphy_quirk" helps ? >>> >>> [1] >>> https://marc.info/?i=2b3535c5ece8b5419e3ecbe300772909021b3fd...@us01wembx2.internal.synopsys.com >>> >> Thanks for your quick answer ! >> >> yes, adding this patch: >> >> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu4.dts >> b/arch/arm/boot/dts/exynos5422-odroidxu4.dts >> index 2faf88627a48..c76e76bc9ade 100644 >> --- a/arch/arm/boot/dts/exynos5422-odroidxu4.dts >> +++ b/arch/arm/boot/dts/exynos5422-odroidxu4.dts >> @@ -43,6 +43,9 @@ >> status = "okay"; >> }; >> >> +&usbdrd_dwc3_0 { >> + snps,dis_u3_susphy_quirk; >> +}; >> &usbdrd_dwc3_1 { >> dr_mode = "host"; >> }; >> >> removes the error "Port resume took longer than 2 msec, port >> status = 0xc400fe3" and the USB devices are detected. > > okay great. Temporarily, you can probably push a patch doing that to > your DTS. At least you're gonna have things working until we understand > what's going on with DWC3. Ok. Thanks ! Richard. -- 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: dwc2: fix "iomem 0x00000000" message by setting iomem parameters in usb_hcd
Hi, John Youn writes: > @@ -1229,7 +1229,8 @@ static inline void dwc2_hcd_connect(struct > dwc2_hsotg *hsotg) {} > static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool > force) {} > static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {} > static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {} > -static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq) > +static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, > + struct resource *res) > { return 0; } > static inline int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg) > { return 0; } > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index 911c3b36..2cfbd10e 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -4964,7 +4964,7 @@ static void dwc2_hcd_release(struct dwc2_hsotg > *hsotg) > * USB bus with the core and calls the hc_driver->start() function. It > returns > * a negative error on failure. > */ > -int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq) > +int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, struct resource > *res) > { > struct usb_hcd *hcd; > struct dwc2_host_chan *channel; > @@ -5021,6 +5021,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq) > > hcd->has_tt = 1; > > + hcd->rsrc_start = res->start; > + hcd->rsrc_len = resource_size(res); > + > ((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg; > hsotg->priv = hcd; > > diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h > index 1ed5fa2b..2305b5fb 100644 > --- a/drivers/usb/dwc2/hcd.h > +++ b/drivers/usb/dwc2/hcd.h > @@ -521,7 +521,8 @@ static inline u8 dwc2_hcd_is_pipe_out(struct > dwc2_hcd_pipe_info *pipe) > return !dwc2_hcd_is_pipe_in(pipe); > } > > -extern int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq); > +extern int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, > + struct resource *res); > extern void dwc2_hcd_remove(struct dwc2_hsotg *hsotg); > > /* Transaction Execution Functions */ > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > index 4fc8c603..5ddc8860 100644 > --- a/drivers/usb/dwc2/platform.c > +++ b/drivers/usb/dwc2/platform.c > @@ -445,7 +445,7 @@ static int dwc2_driver_probe(struct platform_device > *dev) > } > > if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) { > - retval = dwc2_hcd_init(hsotg, hsotg->irq); > + retval = dwc2_hcd_init(hsotg, hsotg->irq, res); This is a good idea, but there's more work that can come out of this, if you're interested: i) devm_ioremap_resource() could be called by the generic layer ii) devm_request_irq() could be move to the generic layer iii) dwc2_hcd_init() could also be called generically as long as dr_mode is set properly. iv) dwc2_debugfs_init() could be called generically as well IOW, dwc2_driver_probe() could be as minimal as: static int dwc2_driver_probe(struct platform_device *dev) { struct dwc2_hsotg *hsotg; struct resource *res; int retval; hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL); if (!hsotg) return -ENOMEM; hsotg->dev = &dev->dev; if (!dev->dev.dma_mask) dev->dev.dma_mask = &dev->dev.coherent_dma_mask; retval = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32)); if (retval) return retval; hsotg->res = platform_get_resource(dev, IORESOURCE_MEM, 0); hsotg->irq = platform_get_irq(dev, 0); retval = dwc2_get_dr_mode(hsotg); if (retval) return retval; retval = dwc2_get_hwparams(hsotg); if (retval) return retval; platform_set_drvdata(dev, hsotg); retval = dwc2_core_init(hsotg); if (retval) return retval; return 0; } dwc2_core_init() needs to implemented, of course, but it could hide all details about what to do with IRQs and resources and what not. Assuming you can properly initialize clocks, it could even handle clocks generically for you. >>> I see what you mean. I'm just a little confused about the term "generic" >>> here: >>> For me something generic has more than one user. Here we seem to speak just >>> about factoring out some code from the probe function, or? >> >> :-) by generic, I mean moving some of that code to >> drivers/usb/dwc2/core.c so it can be used by both PCI and non-PCI >> systems. >> >>> Your proposal for reducing the probe function would mean to reorder some >>> calls >>> like the ones to dwc2_
Re: usb: gadget: Kernel panic (NULL pointer dereference) when using fsl_udc2_core on i.MX31 PDK
Hi, Magnus Lilja writes: > Hi > > I tried the fsl_udc_core gadget driver on the i.MX31 PDK board and got a > kernel panic (NULL pointer dereference) when connecting the USB cable. I > had the g_serial module loaded as well. > > The NULL pointer panic comes from gadget/udc/core.c > usb_gadget_giveback_request() which calls req->complete() and in some > cases req->complete is NULL. > > Commit 304f7e5e1d08 ("usb: gadget: Refactor request completion") changed > fsl_udc2_core.c (and several other files) and in fsl_udc2_core.c a check > that req->complete is non-NULL was removed: > > --- a/drivers/usb/gadget/udc/fsl_udc_core.c > +++ b/drivers/usb/gadget/udc/fsl_udc_core.c > @@ -197,10 +197,8 @@ __acquires(ep->udc->lock) > ep->stopped = 1; > > spin_unlock(&ep->udc->lock); > - /* complete() is from gadget layer, > -* eg fsg->bulk_in_complete() */ > - if (req->req.complete) > - req->req.complete(&ep->ep, &req->req); > + > + usb_gadget_giveback_request(&ep->ep, &req->req); > > spin_lock(&ep->udc->lock); > ep->stopped = stopped; > > If I re-introduce the check (either in fsl_udc_core.c or core.c) at > least USB gadget operation using g_serial seems to work just fine. > > I don't know the logic in detail to understand whether this is a proper > fix or if there is some other more problem with the fls_udc_core driver. > Does anyone have input in this matter? > > I can produce a proper patch that fixes this problem by re-introducing > the check (in either fsl_udc_core.c or core.c) if that is a proper > solution and I can also assist in testing other fixes to the problem. ->complete() is supposed to be mandatory. Which gadget do you have that ->doesn't set ->complete() to a valid function pointer? -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase
Hi, Alan Stern writes: > On Mon, 16 Jan 2017, Felipe Balbi wrote: > >> > The gadget driver never calls usb_ep_queue in order to receive the next >> > SETUP packet; the UDC driver takes care of SETUP handling >> > automatically. >> >> yeah, that's another thing I'd like to change. Currently, we have no >> means to either try to implement device-initiated LPM without adding a >> ton of hacks to UDC drivers. If we require upper layers (composite.c, >> most of the time) to usb_ep_queue() separate requests for all 3 phases >> of a ctrl transfer, we can actually rely on the fact that a new SETUP >> phase hasn't been queued yet to trigger U3 entry. > > I haven't given any thought to LPM. okay > However, requiring gadget drivers to request SETUP packets seems rather > questionable. It flies against the USB spec, which requires right, maybe SETUP is a bit of an overkill. DATA and STATUS, however, should be doable. > peripherals to accept SETUP packets at any time -- a device is not > allowed to NAK or STALL a SETUP packet (see 8.4.6.4 in the USB-2 spec). > In fact, the hardware in UDCs probably isn't capable of doing it. > > This means that to do what you want, the UDC driver would have to > accept SETUP packets at any time, and store the most recent packet > contents. Then, when the gadget driver submits a request, the UDC > driver would give it this stored data. It would also have to detect that's right, I missed that part. > and prevent a nasty race where the gadget driver tries to queue a > request on ep0 that is a response to an old SETUP, one that has already > been overwritten. I'm not even sure preventing this race would be > possible in your scheme. > > The advantage to invoking the gadget driver's setup callback directly > from the UDC driver's interrupt handler is that the gadget driver will > know immediately when an old SETUP has become stale. (That's what > ep0_req_tag is for in f_mass_storage.) It also provides a concurrency > guarantee, because the driver does not re-enable UDC SETUP interrupts > until the handler is finished. > >> Another detail that this helps is that PM (overall) becomes simpler as, >> most likely, we won't need to mess with transfer cancellation, for >> example. > > System PM on a gadget is always troublesome. Even if the USB > connection is a wakeup source, it may not be possible to guarantee that > the gadget can wake up quickly enough to handle an incoming packet. that's true. >> > You are suggesting that status stage requests should not be queued >> > automatically by UDC drivers but instead queued explicitly by gadget >> > drivers. This would mean changing every UDC driver and every gadget >> > driver. >> >> yes, a bit of work but has been done before. One example that comes to >> mind is when I added ->udc_start() and ->udc_stop(). It's totally >> doable. We can, for instance, add a temporary >> "wants_explicit_ctrl_phases" flag to struct usb_gadget which, if set, >> will tell composite.c (or whatever) that the UDC wants explicitly queued >> ctrl phases. > > The term used in the USB spec is "stage", not "phase". "Phase" refers > to the packets making up a single transaction: token, data, and > handshake. > > Also, data stages are already explicit. So your temporary flag might > better be called "wants_explicit_status_stages". I stand corrected ;-) >> Then add support for that to each UDC and set the flag. Once all are >> converted, add one extra patch to remove the flag and the legacy >> code. This has, of course, the draw back of increasing complexity until >> everything is converted over; but if it's all done in a single series, I >> can't see any problems with that. >> >> > Also, it won't fix the race that Baolin Wang found. The setup routine >> >> well, it will help... see below. >> >> > is always called in interrupt context, so it can't sleep. Doing >> > anything non-trivial will require a separate task, and it's possible >> > that this task will try to enqueue the data-stage or status-stage >> > request before the UDC driver is ready to handle it (for example, >> > before or shortly after the setup routine returns). >> > >> > To work properly, the UDC driver must be able to accept a request for >> > ep0 any time after it invokes the setup callback -- either before the >> > callback returns or after. >> >> Right, all UDCs are *already* required to support this case anyway >> because of USB_GADGET_DELAYED_STATUS. There was a bug in DWC3, sure, but >> it was already required to support this case. >> >> By removing USB_GADGET_DELAYED_STATUS altogether and making phases more >> explict, we enforce this requirement and it'll be much easier to test >> for it IMO. > > Okay, I can see the point of requiring explicit status requests. > Implementing it will be a little tricky, because right now some status > requests already are explicit (those for length-0 OUT transfers) while > others are implicit. exactly. And that's source o
Re: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?
Hi, Vincent Pelletier writes: > On Mon, 16 Jan 2017 14:48:31 +, Vincent Pelletier > wrote: >> 3) I declared 4 endpoints (2 IN, 2 OUT). And I went one level deeper >>down the rabbit hole: now enumeration fails with this message on >>host: > > Aaand... I dug this part of the hole myself. In the library part of my > python module, and I was looking at the test program. Shame on me. > > Next tests: After a bit of patching f_fs so it accepts creating 30 > endpoints, more endpoints seem to have issues above the 7th (ignoring > direction). > > 1IN bandwidth: 0 B/s (0.20s) > 1OUTbandwidth: 30230053 B/s (0.21s) > 2IN bandwidth: 29541962 B/s (0.22s) > 2OUTbandwidth: 29995671 B/s (0.22s) > 3IN bandwidth: 28787881 B/s (0.22s) > 3OUTbandwidth: 30428794 B/s (0.22s) > 4IN bandwidth: 29768777 B/s (0.22s) > 4OUTbandwidth: 30377747 B/s (0.22s) > 5IN bandwidth: 29521290 B/s (0.22s) > 5OUTbandwidth: 30133012 B/s (0.22s) > 6IN bandwidth: 29662245 B/s (0.22s) > 6OUTbandwidth: 30335271 B/s (0.22s) > 7IN bandwidth: 29467489 B/s (0.22s) > 7OUTbandwidth: 30387586 B/s (0.22s) > 8IN bandwidth: 0 B/s (0.20s) > 8OUTbandwidth: 0 B/s (0.20s) > 9IN bandwidth: 0 B/s (0.20s) > 9OUTbandwidth: 30174377 B/s (0.22s) > 10INbandwidth: 0 B/s (0.20s) > 10OUT bandwidth: 30389703 B/s (0.22s) > 11INbandwidth: 0 B/s (0.20s) > 11OUT bandwidth: 29947720 B/s (0.21s) > 12INbandwidth: 0 B/s (0.20s) > 12OUT bandwidth: 29386298 B/s (0.21s) > 13INbandwidth: 0 B/s (0.20s) > 13OUT bandwidth: 30097119 B/s (0.22s) > 14INbandwidth: 0 B/s (0.20s) > 14OUT bandwidth: 30099782 B/s (0.22s) > 15INbandwidth: 0 B/s (0.20s) > 15OUT bandwidth: 31122053 B/s (0.21s) > > Would this pattern ring a bell ? it could be that we're ran out of IN endpoints. There's a maximum number to how many IN endpoints we can have enabled at one time and, currently, dwc3 is not enforcing that in any way (I'll get that sorted out for v4.12, v4.11 is already too late) > I did the stall test again (on all endpoints and on 8IN + 9IN), and the > halted endpoints behave as expected. > I also verified the right data reaches/comes from the right endpoint > (for those which work), without error. > I did not check with my protocol analyser yet. > > I attach the patch against f_fs.c as an RFC. > > Regards, > -- > Vincent Pelletier > From fc77bb86dfbf088ce4a63510867210c2f2d8b0af Mon Sep 17 00:00:00 2001 > Message-Id: > > From: Vincent Pelletier > Date: Tue, 17 Jan 2017 12:57:39 + > Subject: usb: gadget: f_fs: Accept up to 30 endpoints. > > It is allowed by the USB specification to enabled same-address, opposite- > direction endpoints simultaneously, which means 30 non-zero endpoints > are allowed. So double eps_addrmap length to 30. > The original code only accepted 14 descriptors, because the first > eps_addrmap entry is unused (it is a placeholder for endpoint zero). > So increase eps_addrmap length by one to 31. > > Signed-off-by: Vincent Pelletier > --- > drivers/usb/gadget/function/f_fs.c | 2 +- > drivers/usb/gadget/function/u_fs.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_fs.c > b/drivers/usb/gadget/function/f_fs.c > index 5c91a6f4613b..206184b561b7 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -2101,7 +2101,7 @@ static int __ffs_data_do_entity(enum ffs_entity_type > type, > case FFS_ENDPOINT: > d = (void *)desc; > helper->eps_count++; > - if (helper->eps_count >= 15) > + if (helper->eps_count >= 31) > return -EINVAL; > /* Check if descriptors for any speed were already parsed */ > if (!helper->ffs->eps_count && !helper->ffs->interfaces_count) > diff --git a/drivers/usb/gadget/function/u_fs.h > b/drivers/usb/gadget/function/u_fs.h > index 60139854e0b1..8eaf473d1ac0 100644 > --- a/drivers/usb/gadget/function/u_fs.h > +++ b/drivers/usb/gadget/function/u_fs.h > @@ -247,7 +247,7 @@ struct ffs_data { > > unsigneduser_flags; > > - u8 eps_addrmap[15]; > + u8 eps_addrmap[31]; please send this as a proper patch using git send-email. Also, please define a macro for this constant. If we ever need to change it again, it'll change in one place. thanks -- balbi signature.asc Description: PGP signature
Re: [PATCH v2] usb: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS
Hi, Bryan O'Donoghue writes: > - DWC_USB3_NUM indicates the number of Device mode single directional > endpoints, including OUT and IN endpoint 0. > > - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN > endpoints active at any time, including control endpoint 0. > > It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to > DWC_USB3_NUM_IN_EPS. > > dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus > DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS > equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT > endpoints as zero. > > For example a from dwc3_core_num_eps() shows: > [1.565000] /usb0@f01d: found 8 IN and 0 OUT endpoints > > This patch works around this case by detecting when DWC_USB3_NUM_EPS is > equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number > of OUT and IN endpoints to make dwc->num_in_eps equal to half > DWC_USB3_NUM_EPS. > > If DWC_USB3_NUM_EPS is equal to DWC3_USB3_NUM_IN_EPS and the endpoint count > is an odd number then dwc->num_out_eps will be assigned the extra endpoint. sorry, now that I spent some more time with this. Isn't something like below solving all problems? diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 369bab16a824..68c9c84b7216 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -397,8 +397,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc) { struct dwc3_hwparams*parms = &dwc->hwparams; - dwc->num_in_eps = DWC3_NUM_IN_EPS(parms); - dwc->num_out_eps = DWC3_NUM_EPS(parms) - dwc->num_in_eps; + dwc->num_eps = DWC3_NUM_EPS(parms); } static void dwc3_cache_hwparams(struct dwc3 *dwc) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 0a664d8eba3f..8c187df0aa42 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2001,23 +2002,7 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 *dwc, static int dwc3_gadget_init_endpoints(struct dwc3 *dwc) { - int ret; - - INIT_LIST_HEAD(&dwc->gadget.ep_list); - - ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_out_eps, 0); - if (ret < 0) { - dev_err(dwc->dev, "failed to initialize OUT endpoints\n"); - return ret; - } - - ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_in_eps, 1); - if (ret < 0) { - dev_err(dwc->dev, "failed to initialize IN endpoints\n"); - return ret; - } - - return 0; + return dwc3_gadget_init_hw_endpoints(dwc, dwc->num_eps); } static void dwc3_gadget_free_endpoints(struct dwc3 *dwc) (clearly this won't compile... It's just to illustrate) The HW actually already tells us total number of endpoints and according to John, they can all behave in either direction. Can you test it out and finish it up as a proper patch? I'll make sure to fix up the "maximum number of IN endpoints enabled at one time" for v4.12. thanks -- balbi signature.asc Description: PGP signature
Re: [PATCH 36/37] xhci: simplify how we store TDs in urb private data
On 23.01.2017 13:57, David Laight wrote: From: Mathias Nyman Sent: 20 January 2017 14:47 Instead of storing a zero length array of td pointers, and then allocate memory both for the td pointer array and the td's, just use a zero length array of actual td's in urb private data. This reminds me of an old patch that got reverted because things broke because the lifetimes/accesses of the data wasn't the obvious one. Can you recall more details about that case? I'd hate to revert this later. I looked at the history how we ended up with this type of allocation for urb priv. I couldn't find any reverts or reasons why it's the way it is. That might have been a different structure. FWIW it is a shame that things like USB3 ethernet can't transmit and receive without all these intermediate and dynamically allocated structures. Well, there's one less allocation after this patch :) -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
[PATCH v2 02/37] xhci: rename EP_HALT_PENDING to EP_STOP_CMD_PENDING
We don't want to confuse halted and stalled endpoint states with a flag indicating we are waiting for a stop endpoint command to finish or timeout Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 6 +++--- drivers/usb/host/xhci.c | 6 +++--- drivers/usb/host/xhci.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 46df89e..213cb02 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -410,7 +410,7 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, * pointer command pending because the device can choose to start any * stream once the endpoint is on the HW schedule. */ - if ((ep_state & EP_HALT_PENDING) || (ep_state & SET_DEQ_PENDING) || + if ((ep_state & EP_STOP_CMD_PENDING) || (ep_state & SET_DEQ_PENDING) || (ep_state & EP_HALTED)) return; writel(DB_VALUE(ep_index, stream_id), db_addr); @@ -626,7 +626,7 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd *xhci, struct xhci_virt_ep *ep) { - ep->ep_state &= ~EP_HALT_PENDING; + ep->ep_state &= ~EP_STOP_CMD_PENDING; /* Can't del_timer_sync in interrupt, so we attempt to cancel. If the * timer is running on another CPU, we don't decrement stop_cmds_pending * (since we didn't successfully stop the watchdog timer). @@ -914,7 +914,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) ep->stop_cmds_pending--; - if (ep->stop_cmds_pending || !(ep->ep_state & EP_HALT_PENDING)) { + if (ep->stop_cmds_pending || !(ep->ep_state & EP_STOP_CMD_PENDING)) { xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Stop EP timer ran, but no command pending, " "exiting."); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 9a0ec11..fcb3fa4 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1563,13 +1563,13 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) /* Queue a stop endpoint command, but only if this is * the first cancellation to be handled. */ - if (!(ep->ep_state & EP_HALT_PENDING)) { + if (!(ep->ep_state & EP_STOP_CMD_PENDING)) { command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); if (!command) { ret = -ENOMEM; goto done; } - ep->ep_state |= EP_HALT_PENDING; + ep->ep_state |= EP_STOP_CMD_PENDING; ep->stop_cmds_pending++; ep->stop_cmd_timer.expires = jiffies + XHCI_STOP_EP_CMD_TIMEOUT * HZ; @@ -3610,7 +3610,7 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev) /* Stop any wayward timer functions (which may grab the lock) */ for (i = 0; i < 31; ++i) { - virt_dev->eps[i].ep_state &= ~EP_HALT_PENDING; + virt_dev->eps[i].ep_state &= ~EP_STOP_CMD_PENDING; del_timer_sync(&virt_dev->eps[i].stop_cmd_timer); } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 2d7b637..198f403 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -912,7 +912,7 @@ struct xhci_virt_ep { unsigned intep_state; #define SET_DEQ_PENDING(1 << 0) #define EP_HALTED (1 << 1)/* For stall handling */ -#define EP_HALT_PENDING(1 << 2)/* For URB cancellation */ +#define EP_STOP_CMD_PENDING(1 << 2)/* For URB cancellation */ /* Transitioning the endpoint to using streams, don't enqueue URBs */ #define EP_GETTING_STREAMS (1 << 3) #define EP_HAS_STREAMS (1 << 4) -- 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
[PATCH v2 05/37] usb: xhci: remove unnecessary second abort try
From: Lu Baolu The second try was a workaround for (what we thought was) command ring failing to stop in the first place. But this turns out to be due to the race that we have fixed(see "xhci: Fix race related to abort operation"). With that fix, it is time to remove the second try. Signed-off-by: Lu Baolu Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 18 +- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 8ccd24e..bcc0894 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -362,19 +362,11 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags) ret = xhci_handshake(&xhci->op_regs->cmd_ring, CMD_RING_RUNNING, 0, 5 * 1000 * 1000); if (ret < 0) { - /* we are about to kill xhci, give it one more chance */ - xhci_write_64(xhci, temp_64 | CMD_RING_ABORT, - &xhci->op_regs->cmd_ring); - udelay(1000); - ret = xhci_handshake(&xhci->op_regs->cmd_ring, -CMD_RING_RUNNING, 0, 3 * 1000 * 1000); - if (ret < 0) { - xhci_err(xhci, "Stopped the command ring failed, " -"maybe the host is dead\n"); - xhci->xhc_state |= XHCI_STATE_DYING; - xhci_halt(xhci); - return -ESHUTDOWN; - } + xhci_err(xhci, +"Stop command ring failed, maybe the host is dead\n"); + xhci->xhc_state |= XHCI_STATE_DYING; + xhci_halt(xhci); + return -ESHUTDOWN; } /* * Writing the CMD_RING_ABORT bit should cause a cmd completion event, -- 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
[PATCH v2 01/37] xhci: simplify if statement to make it more readable
No functional change, De Morgan !(A && B) = (!A || !B) Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index e32029a..46df89e 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -913,7 +913,8 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) spin_lock_irqsave(&xhci->lock, flags); ep->stop_cmds_pending--; - if (!(ep->stop_cmds_pending == 0 && (ep->ep_state & EP_HALT_PENDING))) { + + if (ep->stop_cmds_pending || !(ep->ep_state & EP_HALT_PENDING)) { xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Stop EP timer ran, but no command pending, " "exiting."); -- 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
[PATCH v2 00/37] xhci features for usb-next 4.11
Hi Greg version 2. A lot of xhci cleanups, refactoring and changes for 4.11 Including major tracing rework by Felipe Changes since v1: Removed extra empty lines in patch 25/37 Added "based on" entry to patch 25/37 and 26/37 commit messages fixed typo in patch 32/37 commit message -Mathias Alexander Stein (1): xhci: Put warning message on a single line Baolin Wang (1): usb: host: xhci: Remove unused 'addr_64' variable in xhci_hcd structure Felipe Balbi (20): usb: xhci: add quirk flag for broken PED bits usb: host: xhci-plat: enable BROKEN_PED quirk if platform requested usb: host: xhci: change pre-increments to post-increments usb: host: xhci: print HCIVERSION on debug usb: host: xhci: rename completion codes to match spec usb: host: xhci: simplify irq handler return usb: host: xhci: remove unneded semicolon usb: host: xhci: use slightly better list helpers usb: host: xhci: reorder variable definitions usb: host: xhci: introduce xhci_td_cleanup() usb: host: xhci: remove bogus __releases()/__acquires() annotation usb: host: xhci: check for a valid ring when unmapping bounce buffer usb: host: xhci: unconditionally call xhci_unmap_td_bounce_buffer() usb: host: xhci: convert to list_for_each_entry_safe() usb: host: xhci: combine event TRB completion debugging messages usb: host: xhci: make a generic TRB tracer usb: host: xhci: add urb_enqueue/dequeue/giveback tracers usb: host: xhci: convert several if() to a single switch statement usb: host: xhci: remove newline from tracer usb: host: xhci: add xhci_virt_device tracer Lu Baolu (5): usb: xhci: remove unnecessary second abort try usb: xhci: remove unnecessary assignment usb: xhci: avoid unnecessary calculation usb: xhci: use list_is_singular for cmd_list usb: xhci: remove unnecessary return in xhci_pci_setup() Mathias Nyman (10): xhci: simplify if statement to make it more readable xhci: rename EP_HALT_PENDING to EP_STOP_CMD_PENDING xhci: detect stop endpoint race using pending timer instead of counter. xhci: remove unnecessary check for pending timer xhci: Introduce helper to turn one TRB into a no-op xhci: use the trb_to_noop() helper for command trbs xhci: rename size variable to num_tds xhci: Rename variables related to transfer descritpors xhci: simplify how we store TDs in urb private data xhci: refactor xhci_urb_enqueue Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 + drivers/usb/host/xhci-dbg.c| 22 +- drivers/usb/host/xhci-ext-caps.h | 2 +- drivers/usb/host/xhci-hub.c| 14 +- drivers/usb/host/xhci-mem.c| 30 +- drivers/usb/host/xhci-pci.c| 6 +- drivers/usb/host/xhci-plat.c | 3 + drivers/usb/host/xhci-ring.c | 463 +- drivers/usb/host/xhci-trace.h | 184 ++- drivers/usb/host/xhci.c| 212 - drivers/usb/host/xhci.h| 528 ++--- 11 files changed, 982 insertions(+), 483 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
[PATCH v2 04/37] xhci: remove unnecessary check for pending timer
Checking if the command timeout timer is pending when queueing the first command to the command ring is not really useful, remove it. Suggested-by: Lu Baolu Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 2ce132b..8ccd24e 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -3815,8 +3815,7 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd, list_add_tail(&cmd->cmd_list, &xhci->cmd_list); /* if there are no other commands queued we start the timeout timer */ - if (xhci->cmd_list.next == &cmd->cmd_list && - !delayed_work_pending(&xhci->cmd_timer)) { + if (xhci->cmd_list.next == &cmd->cmd_list) { xhci->current_cmd = cmd; xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT); } -- 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
[PATCH v2 03/37] xhci: detect stop endpoint race using pending timer instead of counter.
A counter was used to find out if the stop endpoint completion raced with the stop endpoint timeout timer. This was needed in case the stop ep completion failed to delete the timer as it was running on anoter cpu. The EP_STOP_CMD_PENDING flag was not enough as a new stop endpoint command may be queued between the command completion and timeout function, which would set the flag back. Instead of the separate counter that was used we can detect the race by checking both the STOP_EP_PENDING flag and timer_pending in the timeout function. Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 27 +++ drivers/usb/host/xhci.c | 1 - drivers/usb/host/xhci.h | 1 - 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 213cb02..2ce132b 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -627,12 +627,8 @@ static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd *xhci, struct xhci_virt_ep *ep) { ep->ep_state &= ~EP_STOP_CMD_PENDING; - /* Can't del_timer_sync in interrupt, so we attempt to cancel. If the -* timer is running on another CPU, we don't decrement stop_cmds_pending -* (since we didn't successfully stop the watchdog timer). -*/ - if (del_timer(&ep->stop_cmd_timer)) - ep->stop_cmds_pending--; + /* Can't del_timer_sync in interrupt */ + del_timer(&ep->stop_cmd_timer); } /* @@ -895,10 +891,8 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, * simple flag to say whether there is a pending stop endpoint command for a * particular endpoint. * - * Instead we use a combination of that flag and a counter for the number of - * pending stop endpoint commands. If the timer is the tail end of the last - * stop endpoint command, and the endpoint's command is still pending, we assume - * the host is dying. + * Instead we use a combination of that flag and checking if a new timer is + * pending. */ void xhci_stop_endpoint_command_watchdog(unsigned long arg) { @@ -912,13 +906,11 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) spin_lock_irqsave(&xhci->lock, flags); - ep->stop_cmds_pending--; - - if (ep->stop_cmds_pending || !(ep->ep_state & EP_STOP_CMD_PENDING)) { - xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, - "Stop EP timer ran, but no command pending, " - "exiting."); + /* bail out if cmd completed but raced with stop ep watchdog timer.*/ + if (!(ep->ep_state & EP_STOP_CMD_PENDING) || + timer_pending(&ep->stop_cmd_timer)) { spin_unlock_irqrestore(&xhci->lock, flags); + xhci_dbg(xhci, "Stop EP timer raced with cmd completion, exit"); return; } @@ -927,7 +919,10 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) /* Oops, HC is dead or dying or at least not responding to the stop * endpoint command. */ + xhci->xhc_state |= XHCI_STATE_DYING; + ep->ep_state &= ~EP_STOP_CMD_PENDING; + /* Disable interrupts from the host controller and start halting it */ xhci_quiesce(xhci); spin_unlock_irqrestore(&xhci->lock, flags); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index fcb3fa4..fb7a6dc 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1570,7 +1570,6 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) goto done; } ep->ep_state |= EP_STOP_CMD_PENDING; - ep->stop_cmds_pending++; ep->stop_cmd_timer.expires = jiffies + XHCI_STOP_EP_CMD_TIMEOUT * HZ; add_timer(&ep->stop_cmd_timer); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 198f403..cdf8c03 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -924,7 +924,6 @@ struct xhci_virt_ep { unsigned intstopped_stream; /* Watchdog timer for stop endpoint command to cancel URBs */ struct timer_list stop_cmd_timer; - int stop_cmds_pending; struct xhci_hcd *xhci; /* Dequeue pointer and dequeue segment for a submitted Set TR Dequeue * command. We'll need to update the ring's dequeue segment and dequeue -- 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
[PATCH v2 07/37] xhci: Put warning message on a single line
From: Alexander Stein This allows someone to grep for the complete warning message as in; xhci-hcd xhci-hcd.0.auto: USB core suspending device not in U0/U1/U2. Signed-off-by: Alexander Stein Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-hub.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 0ef1690..2d154e2 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -990,8 +990,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, temp = readl(port_array[wIndex]); if ((temp & PORT_PE) == 0 || (temp & PORT_RESET) || (temp & PORT_PLS_MASK) >= XDEV_U3) { - xhci_warn(xhci, "USB core suspending device " - "not in U0/U1/U2.\n"); + xhci_warn(xhci, "USB core suspending device not in U0/U1/U2.\n"); goto error; } -- 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
[PATCH v2 11/37] usb: xhci: avoid unnecessary calculation
From: Lu Baolu No need to calculate remainder and length_field, if there is no data phase of a control transfer. Signed-off-by: Lu Baolu Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 61b5fea..2374a13 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -3237,7 +3237,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags, struct usb_ctrlrequest *setup; struct xhci_generic_trb *start_trb; int start_cycle; - u32 field, length_field, remainder; + u32 field; struct urb_priv *urb_priv; struct xhci_td *td; @@ -3310,16 +3310,16 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags, else field = TRB_TYPE(TRB_DATA); - remainder = xhci_td_remainder(xhci, 0, - urb->transfer_buffer_length, - urb->transfer_buffer_length, - urb, 1); - - length_field = TRB_LEN(urb->transfer_buffer_length) | - TRB_TD_SIZE(remainder) | - TRB_INTR_TARGET(0); - if (urb->transfer_buffer_length > 0) { + u32 length_field, remainder; + + remainder = xhci_td_remainder(xhci, 0, + urb->transfer_buffer_length, + urb->transfer_buffer_length, + urb, 1); + length_field = TRB_LEN(urb->transfer_buffer_length) | + TRB_TD_SIZE(remainder) | + TRB_INTR_TARGET(0); if (setup->bRequestType & USB_DIR_IN) field |= TRB_DIR_IN; queue_trb(xhci, ep_ring, true, -- 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
[PATCH v2 12/37] usb: xhci: use list_is_singular for cmd_list
From: Lu Baolu Use list_is_singular() to check if cmd_list has only one entry. [use list_empty() in queue command instead -Mathias] Signed-off-by: Lu Baolu Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 2374a13..b11f479 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1425,7 +1425,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, } /* restart timer if this wasn't the last command */ - if (cmd->cmd_list.next != &xhci->cmd_list) { + if (!list_is_singular(&xhci->cmd_list)) { xhci->current_cmd = list_entry(cmd->cmd_list.next, struct xhci_command, cmd_list); xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT); @@ -3802,14 +3802,15 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd, } cmd->command_trb = xhci->cmd_ring->enqueue; - list_add_tail(&cmd->cmd_list, &xhci->cmd_list); /* if there are no other commands queued we start the timeout timer */ - if (xhci->cmd_list.next == &cmd->cmd_list) { + if (list_empty(&xhci->cmd_list)) { xhci->current_cmd = cmd; xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT); } + list_add_tail(&cmd->cmd_list, &xhci->cmd_list); + queue_trb(xhci, xhci->cmd_ring, false, field1, field2, field3, field4 | xhci->cmd_ring->cycle_state); return 0; -- 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
[PATCH v2 06/37] usb: host: xhci: Remove unused 'addr_64' variable in xhci_hcd structure
From: Baolin Wang Since the 'addr_64' variable as legacy is unused now, then remove it from xhci_hcd structure. Signed-off-by: Baolin Wang Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index cdf8c03..5bf9df2 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1548,7 +1548,6 @@ struct xhci_hcd { u8 max_ports; u8 isoc_threshold; int event_ring_max; - int addr_64; /* 4KB min, 128MB max */ int page_size; /* Valid values are 12 to 20, inclusive */ -- 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
[PATCH v2 08/37] usb: xhci: add quirk flag for broken PED bits
From: Felipe Balbi Some devices from Texas Instruments [1] suffer from a silicon bug where Port Enabled/Disabled bit should not be used to silence an erroneous device. The bug is so that if port is disabled with PED bit, an IRQ for device removal (or attachment) will never fire. Just for the sake of completeness, the actual problem lies with SNPS USB IP and this affects all known versions up to 3.00a. A separate patch will be added to dwc3 to enabled this quirk flag if version is <= 3.00a. [1] - AM572x Silicon Errata http://www.ti.com/lit/er/sprz429j/sprz429j.pdf Section i896— USB xHCI Port Disable Feature Does Not Work Signed-off-by: Felipe Balbi Signed-off-by: Roger Quadros Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-hub.c | 6 ++ drivers/usb/host/xhci.h | 3 +++ 2 files changed, 9 insertions(+) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 2d154e2..8b906c3 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -458,6 +458,12 @@ static void xhci_disable_port(struct usb_hcd *hcd, struct xhci_hcd *xhci, return; } + if (xhci->quirks & XHCI_BROKEN_PORT_PED) { + xhci_dbg(xhci, +"Broken Port Enabled/Disabled, ignoring port disable request.\n"); + return; + } + /* Write 1 to disable the port */ writel(port_status | PORT_PE, addr); port_status = readl(addr); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 5bf9df2..b8474a2 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1648,6 +1648,9 @@ struct xhci_hcd { #define XHCI_SSIC_PORT_UNUSED (1 << 22) #define XHCI_NO_64BIT_SUPPORT (1 << 23) #define XHCI_MISSING_CAS (1 << 24) +/* For controller with a broken Port Disable implementation */ +#define XHCI_BROKEN_PORT_PED (1 << 25) + unsigned intnum_active_eps; unsigned intlimit_active_eps; /* There are two roothubs to keep track of bus suspend info for */ -- 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
[PATCH v2 15/37] usb: host: xhci: print HCIVERSION on debug
From: Felipe Balbi When calling xhci_dbg_regs() we actually _do_ want to know XHCI's version. This might help figure out why certain problems only happen in some cases. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-dbg.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c index a3b67f3..363d125 100644 --- a/drivers/usb/host/xhci-dbg.c +++ b/drivers/usb/host/xhci-dbg.c @@ -37,10 +37,8 @@ void xhci_dbg_regs(struct xhci_hcd *xhci) &xhci->cap_regs->hc_capbase, temp); xhci_dbg(xhci, "// CAPLENGTH: 0x%x\n", (unsigned int) HC_LENGTH(temp)); -#if 0 xhci_dbg(xhci, "// HCIVERSION: 0x%x\n", (unsigned int) HC_VERSION(temp)); -#endif xhci_dbg(xhci, "// xHCI operational registers at %p:\n", xhci->op_regs); -- 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
[PATCH v2 13/37] usb: xhci: remove unnecessary return in xhci_pci_setup()
From: Lu Baolu Remove the unnecessary return line in xhci_pci_setup(). Signed-off-by: Lu Baolu Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-pci.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 954abfd..fc99f51 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -242,11 +242,7 @@ static int xhci_pci_setup(struct usb_hcd *hcd) xhci_dbg(xhci, "Got SBRN %u\n", (unsigned int) xhci->sbrn); /* Find any debug ports */ - retval = xhci_pci_reinit(xhci, pdev); - if (!retval) - return retval; - - return retval; + return xhci_pci_reinit(xhci, pdev); } /* -- 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
[PATCH v2 16/37] usb: host: xhci: rename completion codes to match spec
From: Felipe Balbi Cleanup only. This patch is a mechaninal rename to make sure our macros for TRB completion codes match what the specification uses to refer to such errors. The idea behind this is that it makes it far easier to grep the specification and match it with implementation. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-hub.c | 3 +- drivers/usb/host/xhci-ring.c | 124 +-- drivers/usb/host/xhci.c | 48 - drivers/usb/host/xhci.h | 106 +--- 4 files changed, 124 insertions(+), 157 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 8b906c3..50d086b 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -418,7 +418,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) /* Wait for last stop endpoint command to finish */ wait_for_completion(cmd->completion); - if (cmd->status == COMP_CMD_ABORT || cmd->status == COMP_CMD_STOP) { + if (cmd->status == COMP_COMMAND_ABORTED || + cmd->status == COMP_STOPPED) { xhci_warn(xhci, "Timeout while waiting for stop endpoint command\n"); ret = -ETIME; } diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index b11f479..f71f4dd 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -304,10 +304,10 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci, /* Turn all aborted commands in list to no-ops, then restart */ list_for_each_entry(i_cmd, &xhci->cmd_list, cmd_list) { - if (i_cmd->status != COMP_CMD_ABORT) + if (i_cmd->status != COMP_COMMAND_ABORTED) continue; - i_cmd->status = COMP_CMD_STOP; + i_cmd->status = COMP_STOPPED; xhci_dbg(xhci, "Turn aborted command %p to no-op\n", i_cmd->command_trb); @@ -1038,10 +1038,10 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, unsigned int slot_state; switch (cmd_comp_code) { - case COMP_TRB_ERR: + case COMP_TRB_ERROR: xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid because of stream ID configuration\n"); break; - case COMP_CTX_STATE: + case COMP_CONTEXT_STATE_ERROR: xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state.\n"); ep_state = GET_EP_CTX_STATE(ep_ctx); slot_state = le32_to_cpu(slot_ctx->dev_state); @@ -1050,7 +1050,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, "Slot state = %u, EP state = %u", slot_state, ep_state); break; - case COMP_EBADSLT: + case COMP_SLOT_NOT_ENABLED_ERROR: xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed because slot %u was not enabled.\n", slot_id); break; @@ -1247,7 +1247,7 @@ void xhci_cleanup_command_queue(struct xhci_hcd *xhci) { struct xhci_command *cur_cmd, *tmp_cmd; list_for_each_entry_safe(cur_cmd, tmp_cmd, &xhci->cmd_list, cmd_list) - xhci_complete_del_and_free_cmd(cur_cmd, COMP_CMD_ABORT); + xhci_complete_del_and_free_cmd(cur_cmd, COMP_COMMAND_ABORTED); } void xhci_handle_command_timeout(struct work_struct *work) @@ -1270,7 +1270,7 @@ void xhci_handle_command_timeout(struct work_struct *work) return; } /* mark this command to be cancelled */ - xhci->current_cmd->status = COMP_CMD_ABORT; + xhci->current_cmd->status = COMP_COMMAND_ABORTED; /* Make sure command ring is running before aborting it */ hw_ring_state = xhci_read_64(xhci, &xhci->op_regs->cmd_ring); @@ -1344,7 +1344,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status)); /* If CMD ring stopped we own the trbs between enqueue and dequeue */ - if (cmd_comp_code == COMP_CMD_STOP) { + if (cmd_comp_code == COMP_STOPPED) { complete_all(&xhci->cmd_ring_stop_completion); return; } @@ -1361,9 +1361,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, * The command ring is stopped now, but the xHC will issue a Command * Ring Stopped event which will cause us to restart it. */ - if (cmd_comp_code == COMP_CMD_ABORT) { + if (cmd_comp_code == COMP_COMMAND_ABORTED) { xhci->cmd_ring_state = CMD_RING_STATE_ST
[PATCH v2 25/37] xhci: Introduce helper to turn one TRB into a no-op
Useful for turning both transfer and command trbs into no-ops. Based on earlier code by Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 8b78eee..bd00ada 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -129,6 +129,21 @@ static void inc_td_cnt(struct urb *urb) urb_priv->td_cnt++; } +static void trb_to_noop(union xhci_trb *trb, u32 noop_type) +{ + if (trb_is_link(trb)) { + /* unchain chained link TRBs */ + trb->link.control &= cpu_to_le32(~TRB_CHAIN); + } else { + trb->generic.field[0] = 0; + trb->generic.field[1] = 0; + trb->generic.field[2] = 0; + /* Preserve only the cycle bit of this TRB */ + trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); + trb->generic.field[3] |= cpu_to_le32(TRB_TYPE(noop_type)); + } +} + /* Updates trb to point to the next TRB in the ring, and updates seg if the next * TRB is in a new segment. This does not skip over link TRBs, and it does not * effect the ring dequeue or enqueue pointers. @@ -592,18 +607,8 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, union xhci_trb *trb = td->first_trb; while (1) { - if (trb_is_link(trb)) { - /* unchain chained link TRBs */ - trb->link.control &= cpu_to_le32(~TRB_CHAIN); - } else { - trb->generic.field[0] = 0; - trb->generic.field[1] = 0; - trb->generic.field[2] = 0; - /* Preserve only the cycle bit of this TRB */ - trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); - trb->generic.field[3] |= cpu_to_le32( - TRB_TYPE(TRB_TR_NOOP)); - } + trb_to_noop(trb, TRB_TR_NOOP); + /* flip cycle if asked to */ if (flip_cycle && trb != td->first_trb && trb != td->last_trb) trb->generic.field[3] ^= cpu_to_le32(TRB_CYCLE); -- 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
[PATCH v2 18/37] usb: host: xhci: remove unneded semicolon
From: Felipe Balbi it does no good, let's remove it. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ext-caps.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h index e0244fb..28deea5 100644 --- a/drivers/usb/host/xhci-ext-caps.h +++ b/drivers/usb/host/xhci-ext-caps.h @@ -117,7 +117,7 @@ static inline int xhci_find_next_ext_cap(void __iomem *base, u32 start, int id) offset = XHCI_HCC_EXT_CAPS(val) << 2; if (!offset) return 0; - }; + } do { val = readl(base + offset); if (val == ~0) -- 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
[PATCH v2 37/37] xhci: refactor xhci_urb_enqueue
Use switch instead of several if statements Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 93 - 1 file changed, 37 insertions(+), 56 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dde5c2d..6d6c460 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1334,7 +1334,7 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) struct xhci_hcd *xhci = hcd_to_xhci(hcd); unsigned long flags; int ret = 0; - unsigned int slot_id, ep_index; + unsigned int slot_id, ep_index, ep_state; struct urb_priv *urb_priv; int num_tds; @@ -1348,8 +1348,7 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) if (!HCD_HW_ACCESSIBLE(hcd)) { if (!in_interrupt()) xhci_dbg(xhci, "urb submitted during PCI suspend\n"); - ret = -ESHUTDOWN; - goto exit; + return -ESHUTDOWN; } if (usb_endpoint_xfer_isoc(&urb->ep->desc)) @@ -1386,69 +1385,51 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) return ret; } } + } - /* We have a spinlock and interrupts disabled, so we must pass -* atomic context to this function, which may allocate memory. -*/ - spin_lock_irqsave(&xhci->lock, flags); - if (xhci->xhc_state & XHCI_STATE_DYING) - goto dying; + spin_lock_irqsave(&xhci->lock, flags); + + if (xhci->xhc_state & XHCI_STATE_DYING) { + xhci_dbg(xhci, "Ep 0x%x: URB %p submitted for non-responsive xHCI host.\n", +urb->ep->desc.bEndpointAddress, urb); + ret = -ESHUTDOWN; + goto free_priv; + } + + switch (usb_endpoint_type(&urb->ep->desc)) { + + case USB_ENDPOINT_XFER_CONTROL: ret = xhci_queue_ctrl_tx(xhci, GFP_ATOMIC, urb, - slot_id, ep_index); - if (ret) - goto free_priv; - spin_unlock_irqrestore(&xhci->lock, flags); - } else if (usb_endpoint_xfer_bulk(&urb->ep->desc)) { - spin_lock_irqsave(&xhci->lock, flags); - if (xhci->xhc_state & XHCI_STATE_DYING) - goto dying; - if (xhci->devs[slot_id]->eps[ep_index].ep_state & - EP_GETTING_STREAMS) { - xhci_warn(xhci, "WARN: Can't enqueue URB while bulk ep " - "is transitioning to using streams.\n"); - ret = -EINVAL; - } else if (xhci->devs[slot_id]->eps[ep_index].ep_state & - EP_GETTING_NO_STREAMS) { - xhci_warn(xhci, "WARN: Can't enqueue URB while bulk ep " - "is transitioning to " - "not having streams.\n"); +slot_id, ep_index); + break; + case USB_ENDPOINT_XFER_BULK: + ep_state = xhci->devs[slot_id]->eps[ep_index].ep_state; + if (ep_state & (EP_GETTING_STREAMS | EP_GETTING_NO_STREAMS)) { + xhci_warn(xhci, "WARN: Can't enqueue URB, ep in streams transition state %x\n", + ep_state); ret = -EINVAL; - } else { - ret = xhci_queue_bulk_tx(xhci, GFP_ATOMIC, urb, - slot_id, ep_index); + break; } - if (ret) - goto free_priv; - spin_unlock_irqrestore(&xhci->lock, flags); - } else if (usb_endpoint_xfer_int(&urb->ep->desc)) { - spin_lock_irqsave(&xhci->lock, flags); - if (xhci->xhc_state & XHCI_STATE_DYING) - goto dying; + ret = xhci_queue_bulk_tx(xhci, GFP_ATOMIC, urb, +slot_id, ep_index); + break; + + + case USB_ENDPOINT_XFER_INT: ret = xhci_queue_intr_tx(xhci, GFP_ATOMIC, urb, slot_id, ep_index); - if (ret) - goto free_priv; - spin_unlock_irqrestore(&xhci->lock, flags); - } else { - spin_lock_irqsave(&xhci->lock, flags); - if (xhci->xhc_state & XHCI_STATE_DYING) - goto dying; + break; + + case USB_ENDPOINT_XFER_ISOC: ret = xhci_queue_isoc_tx_prepare(xhci, GFP_ATOMIC, urb, slot_id, ep_index); - if (ret) -
[PATCH v2 30/37] usb: host: xhci: add urb_enqueue/dequeue/giveback tracers
From: Felipe Balbi These three new tracers will help us tie TRBs into URBs by *also* looking into URB lifetime. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 1 + drivers/usb/host/xhci-trace.h | 70 +++ drivers/usb/host/xhci.c | 5 3 files changed, 76 insertions(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 4316273..e0a49cb 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -642,6 +642,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, usb_hcd_unlink_urb_from_ep(hcd, urb); spin_unlock(&xhci->lock); usb_hcd_giveback_urb(hcd, urb, status); + trace_xhci_urb_giveback(urb); spin_lock(&xhci->lock); } diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index d01524b..4bad0d6 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -158,6 +158,76 @@ TP_ARGS(ring, trb) ); +DECLARE_EVENT_CLASS(xhci_log_urb, + TP_PROTO(struct urb *urb), + TP_ARGS(urb), + TP_STRUCT__entry( + __field(void *, urb) + __field(unsigned int, pipe) + __field(unsigned int, stream) + __field(int, status) + __field(unsigned int, flags) + __field(int, num_mapped_sgs) + __field(int, num_sgs) + __field(int, length) + __field(int, actual) + __field(int, epnum) + __field(int, dir_in) + __field(int, type) + ), + TP_fast_assign( + __entry->urb = urb; + __entry->pipe = urb->pipe; + __entry->stream = urb->stream_id; + __entry->status = urb->status; + __entry->flags = urb->transfer_flags; + __entry->num_mapped_sgs = urb->num_mapped_sgs; + __entry->num_sgs = urb->num_sgs; + __entry->length = urb->transfer_buffer_length; + __entry->actual = urb->actual_length; + __entry->epnum = usb_endpoint_num(&urb->ep->desc); + __entry->dir_in = usb_endpoint_dir_in(&urb->ep->desc); + __entry->type = usb_endpoint_type(&urb->ep->desc); + ), + TP_printk("ep%d%s-%s: urb %p pipe %u length %d/%d sgs %d/%d stream %d flags %08x", + __entry->epnum, __entry->dir_in ? "in" : "out", + ({ char *s; + switch (__entry->type) { + case USB_ENDPOINT_XFER_INT: + s = "intr"; + break; + case USB_ENDPOINT_XFER_CONTROL: + s = "control"; + break; + case USB_ENDPOINT_XFER_BULK: + s = "bulk"; + break; + case USB_ENDPOINT_XFER_ISOC: + s = "isoc"; + break; + default: + s = "UNKNOWN"; + } s; }), __entry->urb, __entry->pipe, __entry->actual, + __entry->length, __entry->num_mapped_sgs, + __entry->num_sgs, __entry->stream, __entry->flags + ) +); + +DEFINE_EVENT(xhci_log_urb, xhci_urb_enqueue, + TP_PROTO(struct urb *urb), + TP_ARGS(urb) +); + +DEFINE_EVENT(xhci_log_urb, xhci_urb_giveback, + TP_PROTO(struct urb *urb), + TP_ARGS(urb) +); + +DEFINE_EVENT(xhci_log_urb, xhci_urb_dequeue, + TP_PROTO(struct urb *urb), + TP_ARGS(urb) +); + #endif /* __XHCI_TRACE_H */ /* this part must be outside header guard */ diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 5d6b5a2..958c92b 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1383,6 +1383,8 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) urb_priv->td_cnt = 0; urb->hcpriv = urb_priv; + trace_xhci_urb_enqueue(urb); + if (usb_endpoint_xfer_control(&urb->ep->desc)) { /* Check to see if the max packet size for the default control * endpoint changed during FS device enumeration @@ -1509,6 +1511,9 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) xhci = hcd_to_xhci(hcd); spin_lock_irqsave(&xhci->lock, flags); + + trace_xhci_urb_dequeue(urb); + /* Make sure the URB hasn't completed or been unlinked already */ ret = usb_hcd_check_unlink_urb(hcd, urb, status); if (ret || !urb->hcpriv) -- 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.ker
[PATCH v2 09/37] usb: host: xhci-plat: enable BROKEN_PED quirk if platform requested
From: Felipe Balbi In case 'quirk-broken-port-ped' property is passed in via device property, we should enable the corresponding BROKEN_PED quirk flag for XHCI core. [rog...@ti.com] Updated code from platform data to device property and added DT binding. Signed-off-by: Felipe Balbi Signed-off-by: Roger Quadros Signed-off-by: Mathias Nyman --- Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 + drivers/usb/host/xhci-plat.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt index 0b7d857..2d80b60 100644 --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt @@ -27,6 +27,7 @@ Required properties: Optional properties: - clocks: reference to a clock - usb3-lpm-capable: determines if platform is USB3 LPM capable + - quirk-broken-port-ped: set if the controller has broken port disable mechanism Example: usb@f0931000 { diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index f96caeb..e1939de 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -232,6 +232,9 @@ static int xhci_plat_probe(struct platform_device *pdev) if (device_property_read_bool(&pdev->dev, "usb3-lpm-capable")) xhci->quirks |= XHCI_LPM_SUPPORT; + if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped")) + xhci->quirks |= XHCI_BROKEN_PORT_PED; + hcd->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0); if (IS_ERR(hcd->usb_phy)) { ret = PTR_ERR(hcd->usb_phy); -- 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
[PATCH v2 10/37] usb: xhci: remove unnecessary assignment
From: Lu Baolu Drop an unnecessary assignment in prepare_transfer(). Signed-off-by: Lu Baolu Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index bcc0894..61b5fea 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2844,8 +2844,6 @@ static int prepare_transfer(struct xhci_hcd *xhci, td->start_seg = ep_ring->enq_seg; td->first_trb = ep_ring->enqueue; - urb_priv->td[td_index] = td; - return 0; } -- 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
[PATCH v2 14/37] usb: host: xhci: change pre-increments to post-increments
From: Felipe Balbi This is a cleanup patch only, no functional changes. The idea is just to make sure for loops look the same all over the driver. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-dbg.c | 20 ++-- drivers/usb/host/xhci-mem.c | 8 drivers/usb/host/xhci.c | 14 +++--- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c index 74c42f7..a3b67f3 100644 --- a/drivers/usb/host/xhci-dbg.c +++ b/drivers/usb/host/xhci-dbg.c @@ -177,7 +177,7 @@ static void xhci_print_ports(struct xhci_hcd *xhci) ports = HCS_MAX_PORTS(xhci->hcs_params1); addr = &xhci->op_regs->port_status_base; for (i = 0; i < ports; i++) { - for (j = 0; j < NUM_PORT_REGS; ++j) { + for (j = 0; j < NUM_PORT_REGS; j++) { xhci_dbg(xhci, "%p port %s reg = 0x%x\n", addr, names[j], (unsigned int) readl(addr)); @@ -240,7 +240,7 @@ void xhci_print_run_regs(struct xhci_hcd *xhci) xhci_dbg(xhci, " %p: Microframe index = 0x%x\n", &xhci->run_regs->microframe_index, (unsigned int) temp); - for (i = 0; i < 7; ++i) { + for (i = 0; i < 7; i++) { temp = readl(&xhci->run_regs->rsvd[i]); if (temp != XHCI_INIT_VALUE) xhci_dbg(xhci, " WARN: %p: Rsvd[%i] = 0x%x\n", @@ -259,7 +259,7 @@ void xhci_print_registers(struct xhci_hcd *xhci) void xhci_print_trb_offsets(struct xhci_hcd *xhci, union xhci_trb *trb) { int i; - for (i = 0; i < 4; ++i) + for (i = 0; i < 4; i++) xhci_dbg(xhci, "Offset 0x%x = 0x%x\n", i*4, trb->generic.field[i]); } @@ -332,7 +332,7 @@ void xhci_debug_segment(struct xhci_hcd *xhci, struct xhci_segment *seg) u64 addr = seg->dma; union xhci_trb *trb = seg->trbs; - for (i = 0; i < TRBS_PER_SEGMENT; ++i) { + for (i = 0; i < TRBS_PER_SEGMENT; i++) { trb = &seg->trbs[i]; xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n", addr, lower_32_bits(le64_to_cpu(trb->link.segment_ptr)), @@ -413,7 +413,7 @@ void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst *erst) int i; struct xhci_erst_entry *entry; - for (i = 0; i < erst->num_entries; ++i) { + for (i = 0; i < erst->num_entries; i++) { entry = &erst->entries[i]; xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n", addr, @@ -440,7 +440,7 @@ void xhci_dbg_cmd_ptrs(struct xhci_hcd *xhci) static void dbg_rsvd64(struct xhci_hcd *xhci, u64 *ctx, dma_addr_t dma) { int i; - for (i = 0; i < 4; ++i) { + for (i = 0; i < 4; i++) { xhci_dbg(xhci, "@%p (virt) @%08llx " "(dma) %#08llx - rsvd64[%d]\n", &ctx[4 + i], (unsigned long long)dma, @@ -496,7 +496,7 @@ static void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx * &slot_ctx->dev_state, (unsigned long long)dma, slot_ctx->dev_state); dma += field_size; - for (i = 0; i < 4; ++i) { + for (i = 0; i < 4; i++) { xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd[%d]\n", &slot_ctx->reserved[i], (unsigned long long)dma, slot_ctx->reserved[i], i); @@ -519,7 +519,7 @@ static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci, if (last_ep < 31) last_ep_ctx = last_ep + 1; - for (i = 0; i < last_ep_ctx; ++i) { + for (i = 0; i < last_ep_ctx; i++) { unsigned int epaddr = xhci_get_endpoint_address(i); struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, ctx, i); dma_addr_t dma = ctx->dma + @@ -544,7 +544,7 @@ static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci, &ep_ctx->tx_info, (unsigned long long)dma, ep_ctx->tx_info); dma += field_size; - for (j = 0; j < 3; ++j) { + for (j = 0; j < 3; j++) { xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd[%d]\n", &ep_ctx->reserved[j], (unsigned long long)dma, @@ -583,7 +583,7 @@ void xhci_dbg_ctx(struct xhci_hcd *xhci, &ctrl_ctx->add_flags, (unsigned long long)dma, ctrl_ctx->add_flags); dma += field_size; - for (i = 0; i < 6; ++i) { + for (i = 0; i < 6; i++) { xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd2[%d]\n",
[PATCH v2 17/37] usb: host: xhci: simplify irq handler return
From: Felipe Balbi Instead of having several return points, let's use a local variable and a single place to return. This makes the code slightly easier to read. [set ret = IRQ_HANDLED in default working case -Mathias] Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 32 +--- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index f71f4dd..efc4657 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2605,27 +2605,28 @@ static int xhci_handle_event(struct xhci_hcd *xhci) irqreturn_t xhci_irq(struct usb_hcd *hcd) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); - u32 status; - u64 temp_64; union xhci_trb *event_ring_deq; + irqreturn_t ret = IRQ_NONE; dma_addr_t deq; + u64 temp_64; + u32 status; spin_lock(&xhci->lock); /* Check if the xHC generated the interrupt, or the irq is shared */ status = readl(&xhci->op_regs->status); - if (status == 0x) - goto hw_died; - - if (!(status & STS_EINT)) { - spin_unlock(&xhci->lock); - return IRQ_NONE; + if (status == 0x) { + ret = IRQ_HANDLED; + goto out; } + + if (!(status & STS_EINT)) + goto out; + if (status & STS_FATAL) { xhci_warn(xhci, "WARNING: Host System Error\n"); xhci_halt(xhci); -hw_died: - spin_unlock(&xhci->lock); - return IRQ_HANDLED; + ret = IRQ_HANDLED; + goto out; } /* @@ -2656,9 +2657,8 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd) temp_64 = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue); xhci_write_64(xhci, temp_64 | ERST_EHB, &xhci->ir_set->erst_dequeue); - spin_unlock(&xhci->lock); - - return IRQ_HANDLED; + ret = IRQ_HANDLED; + goto out; } event_ring_deq = xhci->event_ring->dequeue; @@ -2683,10 +2683,12 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd) /* Clear the event handler busy flag (RW1C); event ring is empty. */ temp_64 |= ERST_EHB; xhci_write_64(xhci, temp_64, &xhci->ir_set->erst_dequeue); + ret = IRQ_HANDLED; +out: spin_unlock(&xhci->lock); - return IRQ_HANDLED; + return ret; } irqreturn_t xhci_msi_irq(int irq, void *hcd) -- 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
[PATCH v2 24/37] usb: host: xhci: unconditionally call xhci_unmap_td_bounce_buffer()
From: Felipe Balbi xhci_unmap_td_bounce_buffer() already checks for a valid td->bounce_seg and bails out early if that's invalid. There's no need to check for this twice. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0de9966..8b78eee 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -793,8 +793,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, * just overwrite it (because the URB has been unlinked). */ ep_ring = xhci_urb_to_transfer_ring(xhci, cur_td->urb); - if (ep_ring && cur_td->bounce_seg) - xhci_unmap_td_bounce_buffer(xhci, ep_ring, cur_td); + xhci_unmap_td_bounce_buffer(xhci, ep_ring, cur_td); inc_td_cnt(cur_td->urb); if (last_td_in_urb(cur_td)) xhci_giveback_urb_in_irq(xhci, cur_td, 0); @@ -820,8 +819,7 @@ static void xhci_kill_ring_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring) if (!list_empty(&cur_td->cancelled_td_list)) list_del_init(&cur_td->cancelled_td_list); - if (cur_td->bounce_seg) - xhci_unmap_td_bounce_buffer(xhci, ring, cur_td); + xhci_unmap_td_bounce_buffer(xhci, ring, cur_td); inc_td_cnt(cur_td->urb); if (last_td_in_urb(cur_td)) @@ -1833,8 +1831,7 @@ static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td, urb_priv = urb->hcpriv; /* if a bounce buffer was used to align this td then unmap it */ - if (td->bounce_seg) - xhci_unmap_td_bounce_buffer(xhci, ep_ring, td); + xhci_unmap_td_bounce_buffer(xhci, ep_ring, td); /* Do one last check of the actual transfer length. * If the host controller said we transferred more data than the buffer -- 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
[PATCH v2 20/37] usb: host: xhci: reorder variable definitions
From: Felipe Balbi no functional changes. Simple cleanup to make sure variables are ordered in a 'reverse christmas tree' fashion. While at that, also remove an obsolete comment which doesn't apply anymore. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 4e7a6c2..627518d 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1822,22 +1822,18 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code) return 0; } -/* - * Finish the td processing, remove the td from td list; - * Return 1 if the urb can be given back. - */ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, union xhci_trb *ep_trb, struct xhci_transfer_event *event, struct xhci_virt_ep *ep, int *status, bool skip) { struct xhci_virt_device *xdev; - struct xhci_ring *ep_ring; - unsigned int slot_id; - int ep_index; - struct urb *urb = NULL; struct xhci_ep_ctx *ep_ctx; + struct xhci_ring *ep_ring; struct urb_priv *urb_priv; + struct urb *urb = NULL; + unsigned int slot_id; u32 trb_comp_code; + int ep_index; slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags)); xdev = xhci->devs[slot_id]; -- 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
[PATCH v2 28/37] usb: host: xhci: combine event TRB completion debugging messages
From: Felipe Balbi If we just provide a helper to convert completion code to string, we can combine all debugging messages into a single print. [keep the old debug messages, for warn and grep -Mathias] Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.h | 80 + 1 file changed, 80 insertions(+) diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index aa63e38..ebdd920 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1097,6 +1097,86 @@ struct xhci_transfer_event { #define COMP_SECONDARY_BANDWIDTH_ERROR 35 #define COMP_SPLIT_TRANSACTION_ERROR 36 +static inline const char *xhci_trb_comp_code_string(u8 status) +{ + switch (status) { + case COMP_INVALID: + return "Invalid"; + case COMP_SUCCESS: + return "Success"; + case COMP_DATA_BUFFER_ERROR: + return "Data Buffer Error"; + case COMP_BABBLE_DETECTED_ERROR: + return "Babble Detected"; + case COMP_USB_TRANSACTION_ERROR: + return "USB Transaction Error"; + case COMP_TRB_ERROR: + return "TRB Error"; + case COMP_STALL_ERROR: + return "Stall Error"; + case COMP_RESOURCE_ERROR: + return "Resource Error"; + case COMP_BANDWIDTH_ERROR: + return "Bandwidth Error"; + case COMP_NO_SLOTS_AVAILABLE_ERROR: + return "No Slots Available Error"; + case COMP_INVALID_STREAM_TYPE_ERROR: + return "Invalid Stream Type Error"; + case COMP_SLOT_NOT_ENABLED_ERROR: + return "Slot Not Enabled Error"; + case COMP_ENDPOINT_NOT_ENABLED_ERROR: + return "Endpoint Not Enabled Error"; + case COMP_SHORT_PACKET: + return "Short Packet"; + case COMP_RING_UNDERRUN: + return "Ring Underrun"; + case COMP_RING_OVERRUN: + return "Ring Overrun"; + case COMP_VF_EVENT_RING_FULL_ERROR: + return "VF Event Ring Full Error"; + case COMP_PARAMETER_ERROR: + return "Parameter Error"; + case COMP_BANDWIDTH_OVERRUN_ERROR: + return "Bandwidth Overrun Error"; + case COMP_CONTEXT_STATE_ERROR: + return "Context State Error"; + case COMP_NO_PING_RESPONSE_ERROR: + return "No Ping Response Error"; + case COMP_EVENT_RING_FULL_ERROR: + return "Event Ring Full Error"; + case COMP_INCOMPATIBLE_DEVICE_ERROR: + return "Incompatible Device Error"; + case COMP_MISSED_SERVICE_ERROR: + return "Missed Service Error"; + case COMP_COMMAND_RING_STOPPED: + return "Command Ring Stopped"; + case COMP_COMMAND_ABORTED: + return "Command Aborted"; + case COMP_STOPPED: + return "Stopped"; + case COMP_STOPPED_LENGTH_INVALID: + return "Stopped - Length Invalid"; + case COMP_STOPPED_SHORT_PACKET: + return "Stopped - Short Packet"; + case COMP_MAX_EXIT_LATENCY_TOO_LARGE_ERROR: + return "Max Exit Latency Too Large Error"; + case COMP_ISOCH_BUFFER_OVERRUN: + return "Isoch Buffer Overrun"; + case COMP_EVENT_LOST_ERROR: + return "Event Lost Error"; + case COMP_UNDEFINED_ERROR: + return "Undefined Error"; + case COMP_INVALID_STREAM_ID_ERROR: + return "Invalid Stream ID Error"; + case COMP_SECONDARY_BANDWIDTH_ERROR: + return "Secondary Bandwidth Error"; + case COMP_SPLIT_TRANSACTION_ERROR: + return "Split Transaction Error"; + default: + return "Unknown!!"; + } +} + struct xhci_link_trb { /* 64-bit segment pointer*/ __le64 segment_ptr; -- 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
[PATCH v2 32/37] usb: host: xhci: remove newline from tracer
From: Felipe Balbi If we add that newline, the output will look like the following: kworker/2:1-42[002] 169.811435: xhci_address_ctx: ctx_64=0, ctx_type=2, ctx_dma=@153fbd000, ctx_va=@880153fbd000 We would rather have that in a single line. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 4bad0d6..08bed2f 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -103,7 +103,7 @@ ((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) * ((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1)); ), - TP_printk("\nctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p", + TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p", __entry->ctx_64, __entry->ctx_type, (unsigned long long) __entry->ctx_dma, __entry->ctx_va ) -- 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
[PATCH v2 29/37] usb: host: xhci: make a generic TRB tracer
From: Felipe Balbi instead of having a tracer that can only trace command completions, let's promote this tracer so it can trace and decode any TRB. With that, it will be easier to extrapolate the lifetime of any TRB which might help debugging certain issues. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 14 +- drivers/usb/host/xhci-trace.h | 55 --- drivers/usb/host/xhci.h | 329 ++ 3 files changed, 375 insertions(+), 23 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 059825b..4316273 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1319,6 +1319,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, cmd_dma = le64_to_cpu(event->cmd_trb); cmd_trb = xhci->cmd_ring->dequeue; + + trace_xhci_handle_command(xhci->cmd_ring, &cmd_trb->generic); + cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg, cmd_trb); /* @@ -1335,8 +1338,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, cancel_delayed_work(&xhci->cmd_timer); - trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event); - cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status)); /* If CMD ring stopped we own the trbs between enqueue and dequeue */ @@ -2479,6 +2480,10 @@ static int handle_tx_event(struct xhci_hcd *xhci, ep_trb = &ep_seg->trbs[(ep_trb_dma - ep_seg->dma) / sizeof(*ep_trb)]; + + trace_xhci_handle_transfer(ep_ring, + (struct xhci_generic_trb *) ep_trb); + /* * No-op TRB should not trigger interrupts. * If ep_trb is a no-op TRB, it means the @@ -2545,6 +2550,8 @@ static int xhci_handle_event(struct xhci_hcd *xhci) xhci->event_ring->cycle_state) return 0; + trace_xhci_handle_event(xhci->event_ring, &event->generic); + /* * Barrier between reading the TRB_CYCLE (valid) flag above and any * speculative reads of the event's flags/data below. @@ -2714,6 +2721,9 @@ static void queue_trb(struct xhci_hcd *xhci, struct xhci_ring *ring, trb->field[1] = cpu_to_le32(field2); trb->field[2] = cpu_to_le32(field3); trb->field[3] = cpu_to_le32(field4); + + trace_xhci_queue_trb(ring, trb); + inc_enq(xhci, ring, more_trbs_coming); } diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 59c0565..d01524b 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -115,34 +115,47 @@ TP_ARGS(xhci, ctx, ep_num) ); -DECLARE_EVENT_CLASS(xhci_log_event, - TP_PROTO(void *trb_va, struct xhci_generic_trb *ev), - TP_ARGS(trb_va, ev), +DECLARE_EVENT_CLASS(xhci_log_trb, + TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb), + TP_ARGS(ring, trb), TP_STRUCT__entry( - __field(void *, va) - __field(u64, dma) - __field(u32, status) - __field(u32, flags) - __dynamic_array(u8, trb, sizeof(struct xhci_generic_trb)) + __field(u32, type) + __field(u32, field0) + __field(u32, field1) + __field(u32, field2) + __field(u32, field3) ), TP_fast_assign( - __entry->va = trb_va; - __entry->dma = ((u64)le32_to_cpu(ev->field[1])) << 32 | - le32_to_cpu(ev->field[0]); - __entry->status = le32_to_cpu(ev->field[2]); - __entry->flags = le32_to_cpu(ev->field[3]); - memcpy(__get_dynamic_array(trb), trb_va, - sizeof(struct xhci_generic_trb)); + __entry->type = ring->type; + __entry->field0 = le32_to_cpu(trb->field[0]); + __entry->field1 = le32_to_cpu(trb->field[1]); + __entry->field2 = le32_to_cpu(trb->field[2]); + __entry->field3 = le32_to_cpu(trb->field[3]); ), - TP_printk("\ntrb_dma=@%llx, trb_va=@%p, status=%08x, flags=%08x", - (unsigned long long) __entry->dma, __entry->va, - __entry->status, __entry->flags + TP_printk("%s: %s", xhci_ring_type_string(__entry->type), + xhci_decode_trb(__entry->field0, __entry->field1, + __entry->field2, __entry->field3) ) ); -DEFINE_EVENT(xhci_log_event, xhci_cmd_completion, - TP_PROTO(void *trb_va, struct xhci_generic_trb *ev), - TP_ARGS(trb_va, ev) +DEFINE_EVENT(xhci_log_trb, xhci_handle_event, + TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb), + TP_ARGS(ring, trb) +);
[PATCH v2 19/37] usb: host: xhci: use slightly better list helpers
From: Felipe Balbi Replace list_entry() with list_first_entry() and list_for_each() with list_for_each_entry(). This makes the code slightly more readable. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index efc4657..4e7a6c2 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -689,7 +689,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, unsigned int ep_index; struct xhci_ring *ep_ring; struct xhci_virt_ep *ep; - struct list_head *entry; struct xhci_td *cur_td = NULL; struct xhci_td *last_unlinked_td; @@ -706,6 +705,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, memset(&deq_state, 0, sizeof(deq_state)); ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); ep = &xhci->devs[slot_id]->eps[ep_index]; + last_unlinked_td = list_last_entry(&ep->cancelled_td_list, + struct xhci_td, cancelled_td_list); if (list_empty(&ep->cancelled_td_list)) { xhci_stop_watchdog_timer_in_irq(xhci, ep); @@ -719,8 +720,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, * it. We're also in the event handler, so we can't get re-interrupted * if another Stop Endpoint command completes */ - list_for_each(entry, &ep->cancelled_td_list) { - cur_td = list_entry(entry, struct xhci_td, cancelled_td_list); + list_for_each_entry(cur_td, &ep->cancelled_td_list, cancelled_td_list) { xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Removing canceled TD starting at 0x%llx (dma).", (unsigned long long)xhci_trb_virt_to_dma( @@ -762,7 +762,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, */ list_del_init(&cur_td->td_list); } - last_unlinked_td = cur_td; + xhci_stop_watchdog_timer_in_irq(xhci, ep); /* If necessary, queue a Set Transfer Ring Dequeue Pointer command */ @@ -784,7 +784,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, * So stop when we've completed the URB for the last TD we unlinked. */ do { - cur_td = list_entry(ep->cancelled_td_list.next, + cur_td = list_first_entry(&ep->cancelled_td_list, struct xhci_td, cancelled_td_list); list_del_init(&cur_td->cancelled_td_list); @@ -1335,7 +1335,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, return; } - cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list); + cmd = list_first_entry(&xhci->cmd_list, struct xhci_command, cmd_list); cancel_delayed_work(&xhci->cmd_timer); @@ -1426,8 +1426,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, /* restart timer if this wasn't the last command */ if (!list_is_singular(&xhci->cmd_list)) { - xhci->current_cmd = list_entry(cmd->cmd_list.next, - struct xhci_command, cmd_list); + xhci->current_cmd = list_first_entry(&cmd->cmd_list, + struct xhci_command, cmd_list); xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT); } else if (xhci->current_cmd == cmd) { xhci->current_cmd = NULL; @@ -2421,7 +2421,8 @@ static int handle_tx_event(struct xhci_hcd *xhci, goto cleanup; } - td = list_entry(ep_ring->td_list.next, struct xhci_td, td_list); + td = list_first_entry(&ep_ring->td_list, struct xhci_td, + td_list); if (ep->skip) td_num--; -- 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
[PATCH v2 26/37] xhci: use the trb_to_noop() helper for command trbs
Remove duplicate code by using trb_to_noop() when handling Aborted commads Based on earlier code by Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index bd00ada..92715b9 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -314,7 +314,6 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci, struct xhci_command *cur_cmd) { struct xhci_command *i_cmd; - u32 cycle_state; /* Turn all aborted commands in list to no-ops, then restart */ list_for_each_entry(i_cmd, &xhci->cmd_list, cmd_list) { @@ -326,15 +325,8 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci, xhci_dbg(xhci, "Turn aborted command %p to no-op\n", i_cmd->command_trb); - /* get cycle state from the original cmd trb */ - cycle_state = le32_to_cpu( - i_cmd->command_trb->generic.field[3]) & TRB_CYCLE; - /* modify the command trb to no-op command */ - i_cmd->command_trb->generic.field[0] = 0; - i_cmd->command_trb->generic.field[1] = 0; - i_cmd->command_trb->generic.field[2] = 0; - i_cmd->command_trb->generic.field[3] = cpu_to_le32( - TRB_TYPE(TRB_CMD_NOOP) | cycle_state); + + trb_to_noop(i_cmd->command_trb, TRB_CMD_NOOP); /* * caller waiting for completion is called when command -- 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
[PATCH v2 23/37] usb: host: xhci: check for a valid ring when unmapping bounce buffer
From: Felipe Balbi This way we can remove checks for valid ring from call sites of xhci_unmap_td_bounce_buffer() Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 9084afb..0de9966 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -655,7 +655,7 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci, struct xhci_segment *seg = td->bounce_seg; struct urb *urb = td->urb; - if (!seg || !urb) + if (!ring || !seg || !urb) return; if (usb_urb_dir_out(urb)) { -- 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
[PATCH v2 22/37] usb: host: xhci: remove bogus __releases()/__acquires() annotation
From: Felipe Balbi handle_tx_event() is not releasing xhci->lock nor reacquiring it, remove the bogus annotation. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 31518dd..9084afb 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2223,8 +2223,6 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td, */ static int handle_tx_event(struct xhci_hcd *xhci, struct xhci_transfer_event *event) - __releases(&xhci->lock) - __acquires(&xhci->lock) { struct xhci_virt_device *xdev; struct xhci_virt_ep *ep; -- 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
[PATCH v2 33/37] usb: host: xhci: add xhci_virt_device tracer
From: Felipe Balbi Let's start tracing at least part of an xhci_virt_device lifetime. We might want to extend this tracepoint class later, but for now it already exposes quite a bit of valuable information. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-hub.c | 2 ++ drivers/usb/host/xhci-mem.c | 7 ++ drivers/usb/host/xhci-trace.h | 57 +++ drivers/usb/host/xhci.c | 1 + 4 files changed, 67 insertions(+) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 50d086b..3bddeaa 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -389,6 +389,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) if (!virt_dev) return -ENODEV; + trace_xhci_stop_device(virt_dev); + cmd = xhci_alloc_command(xhci, false, true, GFP_NOIO); if (!cmd) { xhci_dbg(xhci, "Couldn't allocate command structure.\n"); diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 0019094..e452492 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -936,6 +936,9 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id) return; dev = xhci->devs[slot_id]; + + trace_xhci_free_virt_device(dev); + xhci->dcbaa->dev_context_ptrs[slot_id] = 0; if (!dev) return; @@ -1075,6 +1078,8 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id, &xhci->dcbaa->dev_context_ptrs[slot_id], le64_to_cpu(xhci->dcbaa->dev_context_ptrs[slot_id])); + trace_xhci_alloc_virt_device(dev); + return 1; fail: xhci_free_virt_device(xhci, slot_id); @@ -1249,6 +1254,8 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud ep0_ctx->deq = cpu_to_le64(dev->eps[0].ring->first_seg->dma | dev->eps[0].ring->cycle_state); + trace_xhci_setup_addressable_virt_device(dev); + /* Steps 7 and 8 were done in xhci_alloc_virt_device() */ return 0; diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 08bed2f..1ac2cdf 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -158,6 +158,63 @@ TP_ARGS(ring, trb) ); +DECLARE_EVENT_CLASS(xhci_log_virt_dev, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev), + TP_STRUCT__entry( + __field(void *, vdev) + __field(unsigned long long, out_ctx) + __field(unsigned long long, in_ctx) + __field(int, devnum) + __field(int, state) + __field(int, speed) + __field(u8, portnum) + __field(u8, level) + __field(int, slot_id) + ), + TP_fast_assign( + __entry->vdev = vdev; + __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma; + __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma; + __entry->devnum = vdev->udev->devnum; + __entry->state = vdev->udev->state; + __entry->speed = vdev->udev->speed; + __entry->portnum = vdev->udev->portnum; + __entry->level = vdev->udev->level; + __entry->slot_id = vdev->udev->slot_id; + ), + TP_printk("vdev %p ctx %llx | %llx num %d state %d speed %d port %d level %d slot %d", + __entry->vdev, __entry->in_ctx, __entry->out_ctx, + __entry->devnum, __entry->state, __entry->speed, + __entry->portnum, __entry->level, __entry->slot_id + ) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_alloc_virt_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_free_virt_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_setup_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_setup_addressable_virt_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_stop_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + DECLARE_EVENT_CLASS(xhci_log_urb, TP_PROTO(struct urb *urb), TP_ARGS(urb), diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 958c92b..4968e9a 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3848,6 +3848,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, le32_to_cpu(slot_ctx->dev_info) >> 27); spin_lock_irqsave(&xhci->lock, flags); + trace_xhci_setup_device(virt_dev); ret = xhci
[PATCH v2 21/37] usb: host: xhci: introduce xhci_td_cleanup()
From: Felipe Balbi By extracting xhci_td_cleanup() from finish_td(), code before clearer and easier to follow. There are no functional changes with this patch. It's merely a cleanup. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 92 1 file changed, 50 insertions(+), 42 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 627518d..31518dd 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1822,6 +1822,55 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code) return 0; } +static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td, + struct xhci_ring *ep_ring, int *status) +{ + struct urb_priv *urb_priv; + struct urb *urb = NULL; + + /* Clean up the endpoint's TD list */ + urb = td->urb; + urb_priv = urb->hcpriv; + + /* if a bounce buffer was used to align this td then unmap it */ + if (td->bounce_seg) + xhci_unmap_td_bounce_buffer(xhci, ep_ring, td); + + /* Do one last check of the actual transfer length. +* If the host controller said we transferred more data than the buffer +* length, urb->actual_length will be a very big number (since it's +* unsigned). Play it safe and say we didn't transfer anything. +*/ + if (urb->actual_length > urb->transfer_buffer_length) { + xhci_warn(xhci, "URB req %u and actual %u transfer length mismatch\n", + urb->transfer_buffer_length, urb->actual_length); + urb->actual_length = 0; + *status = 0; + } + list_del_init(&td->td_list); + /* Was this TD slated to be cancelled but completed anyway? */ + if (!list_empty(&td->cancelled_td_list)) + list_del_init(&td->cancelled_td_list); + + inc_td_cnt(urb); + /* Giveback the urb when all the tds are completed */ + if (last_td_in_urb(td)) { + if ((urb->actual_length != urb->transfer_buffer_length && +(urb->transfer_flags & URB_SHORT_NOT_OK)) || + (*status != 0 && !usb_endpoint_xfer_isoc(&urb->ep->desc))) + xhci_dbg(xhci, "Giveback URB %p, len = %d, expected = %d, status = %d\n", +urb, urb->actual_length, +urb->transfer_buffer_length, *status); + + /* set isoc urb status to 0 just as EHCI, UHCI, and OHCI */ + if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS) + *status = 0; + xhci_giveback_urb_in_irq(xhci, td, *status); + } + + return 0; +} + static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, union xhci_trb *ep_trb, struct xhci_transfer_event *event, struct xhci_virt_ep *ep, int *status, bool skip) @@ -1829,8 +1878,6 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_virt_device *xdev; struct xhci_ep_ctx *ep_ctx; struct xhci_ring *ep_ring; - struct urb_priv *urb_priv; - struct urb *urb = NULL; unsigned int slot_id; u32 trb_comp_code; int ep_index; @@ -1873,46 +1920,7 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, } td_cleanup: - /* Clean up the endpoint's TD list */ - urb = td->urb; - urb_priv = urb->hcpriv; - - /* if a bounce buffer was used to align this td then unmap it */ - if (td->bounce_seg) - xhci_unmap_td_bounce_buffer(xhci, ep_ring, td); - - /* Do one last check of the actual transfer length. -* If the host controller said we transferred more data than the buffer -* length, urb->actual_length will be a very big number (since it's -* unsigned). Play it safe and say we didn't transfer anything. -*/ - if (urb->actual_length > urb->transfer_buffer_length) { - xhci_warn(xhci, "URB req %u and actual %u transfer length mismatch\n", - urb->transfer_buffer_length, urb->actual_length); - urb->actual_length = 0; - *status = 0; - } - list_del_init(&td->td_list); - /* Was this TD slated to be cancelled but completed anyway? */ - if (!list_empty(&td->cancelled_td_list)) - list_del_init(&td->cancelled_td_list); - - inc_td_cnt(urb); - /* Giveback the urb when all the tds are completed */ - if (last_td_in_urb(td)) { - if ((urb->actual_length != urb->transfer_buffer_length && -(urb->transfer_flags & URB_SHORT_NOT_OK)) || - (*status != 0 && !usb_endpoint_xfer_isoc(&urb->ep->desc))) - xhci_dbg(xhci, "Giveback URB %p, len = %d, expected = %d, status = %d\n", -
[PATCH v2 36/37] xhci: simplify how we store TDs in urb private data
Instead of storing a zero length array of td pointers, and then allocate memory both for the td pointer array and the td's, just use a zero length array of actual td's in urb private data. old: struct urb_priv { struct xhci_td *td[0] } new: struct urb_priv { struct xhci_td td[0] } Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-mem.c | 5 + drivers/usb/host/xhci-ring.c | 20 ++-- drivers/usb/host/xhci.c | 24 ++-- drivers/usb/host/xhci.h | 2 +- 4 files changed, 18 insertions(+), 33 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index e452492..ba1853f4 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1817,10 +1817,7 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci, void xhci_urb_free_priv(struct urb_priv *urb_priv) { - if (urb_priv) { - kfree(urb_priv->td[0]); - kfree(urb_priv); - } + kfree(urb_priv); } void xhci_free_command(struct xhci_hcd *xhci, diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 7a70406..d9936c7 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2838,7 +2838,7 @@ static int prepare_transfer(struct xhci_hcd *xhci, return ret; urb_priv = urb->hcpriv; - td = urb_priv->td[td_index]; + td = &urb_priv->td[td_index]; INIT_LIST_HEAD(&td->td_list); INIT_LIST_HEAD(&td->cancelled_td_list); @@ -3134,7 +3134,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags, if (urb->transfer_flags & URB_ZERO_PACKET && urb_priv->num_tds > 1) need_zero_pkt = true; - td = urb_priv->td[0]; + td = &urb_priv->td[0]; /* * Don't give the first TRB to the hardware (by toggling the cycle bit) @@ -3227,7 +3227,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags, ret = prepare_transfer(xhci, xhci->devs[slot_id], ep_index, urb->stream_id, 1, urb, 1, mem_flags); - urb_priv->td[1]->last_trb = ring->enqueue; + urb_priv->td[1].last_trb = ring->enqueue; field = TRB_TYPE(TRB_NORMAL) | ring->cycle_state | TRB_IOC; queue_trb(xhci, ring, 0, 0, 0, TRB_INTR_TARGET(0), field); } @@ -3279,7 +3279,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags, return ret; urb_priv = urb->hcpriv; - td = urb_priv->td[0]; + td = &urb_priv->td[0]; /* * Don't give the first TRB to the hardware (by toggling the cycle bit) @@ -3567,7 +3567,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags, return ret; goto cleanup; } - td = urb_priv->td[i]; + td = &urb_priv->td[i]; /* use SIA as default, if frame id is used overwrite it */ sia_frame_id = TRB_SIA; @@ -3674,20 +3674,20 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags, /* Clean up a partially enqueued isoc transfer. */ for (i--; i >= 0; i--) - list_del_init(&urb_priv->td[i]->td_list); + list_del_init(&urb_priv->td[i].td_list); /* Use the first TD as a temporary variable to turn the TDs we've queued * into No-ops with a software-owned cycle bit. That way the hardware * won't accidentally start executing bogus TDs when we partially * overwrite them. td->first_trb and td->start_seg are already set. */ - urb_priv->td[0]->last_trb = ep_ring->enqueue; + urb_priv->td[0].last_trb = ep_ring->enqueue; /* Every TRB except the first & last will have its cycle bit flipped. */ - td_to_noop(xhci, ep_ring, urb_priv->td[0], true); + td_to_noop(xhci, ep_ring, &urb_priv->td[0], true); /* Reset the ring enqueue back to the first TRB and its cycle bit. */ - ep_ring->enqueue = urb_priv->td[0]->first_trb; - ep_ring->enq_seg = urb_priv->td[0]->start_seg; + ep_ring->enqueue = urb_priv->td[0].first_trb; + ep_ring->enq_seg = urb_priv->td[0].start_seg; ep_ring->cycle_state = start_cycle; ep_ring->num_trbs_free = ep_ring->num_trbs_free_temp; usb_hcd_unlink_urb_from_ep(bus_to_hcd(urb->dev->bus), urb); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index bee6272..dde5c2d 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1332,12 +1332,11 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id, int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); - struct xhci_td *buffer; unsigned long fl
[PATCH v2 35/37] xhci: Rename variables related to transfer descritpors
urb_priv structure has a count on how many TDs the URB contains, and how many of those TD's we have handled. rename: length -> num_tds td_cnt -> num_tds_done No functional changes Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 10 +- drivers/usb/host/xhci.c | 14 +++--- drivers/usb/host/xhci.h | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index e0a49cb..7a70406 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -119,14 +119,14 @@ static bool last_td_in_urb(struct xhci_td *td) { struct urb_priv *urb_priv = td->urb->hcpriv; - return urb_priv->td_cnt == urb_priv->length; + return urb_priv->num_tds_done == urb_priv->num_tds; } static void inc_td_cnt(struct urb *urb) { struct urb_priv *urb_priv = urb->hcpriv; - urb_priv->td_cnt++; + urb_priv->num_tds_done++; } static void trb_to_noop(union xhci_trb *trb, u32 noop_type) @@ -2055,7 +2055,7 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td, ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer)); trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len)); urb_priv = td->urb->hcpriv; - idx = urb_priv->td_cnt; + idx = urb_priv->num_tds_done; frame = &td->urb->iso_frame_desc[idx]; requested = frame->length; remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); @@ -2134,7 +2134,7 @@ static int skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td, ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer)); urb_priv = td->urb->hcpriv; - idx = urb_priv->td_cnt; + idx = urb_priv->num_tds_done; frame = &td->urb->iso_frame_desc[idx]; /* The transfer is partly done. */ @@ -3131,7 +3131,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags, urb_priv = urb->hcpriv; /* Deal with URB_ZERO_PACKET - need one more td/trb */ - if (urb->transfer_flags & URB_ZERO_PACKET && urb_priv->length > 1) + if (urb->transfer_flags & URB_ZERO_PACKET && urb_priv->num_tds > 1) need_zero_pkt = true; td = urb_priv->td[0]; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 40b1486..bee6272 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1379,8 +1379,8 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) buffer++; } - urb_priv->length = num_tds; - urb_priv->td_cnt = 0; + urb_priv->num_tds = num_tds; + urb_priv->num_tds_done = 0; urb->hcpriv = urb_priv; trace_xhci_urb_enqueue(urb); @@ -1523,8 +1523,8 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "HW died, freeing TD."); urb_priv = urb->hcpriv; - for (i = urb_priv->td_cnt; -i < urb_priv->length && xhci->devs[urb->dev->slot_id]; + for (i = urb_priv->num_tds_done; +i < urb_priv->num_tds && xhci->devs[urb->dev->slot_id]; i++) { td = urb_priv->td[i]; if (!list_empty(&td->td_list)) @@ -1549,8 +1549,8 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) } urb_priv = urb->hcpriv; - i = urb_priv->td_cnt; - if (i < urb_priv->length) + i = urb_priv->num_tds_done; + if (i < urb_priv->num_tds) xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Cancel URB %p, dev %s, ep 0x%x, " "starting at offset 0x%llx", @@ -1560,7 +1560,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) urb_priv->td[i]->start_seg, urb_priv->td[i]->first_trb)); - for (; i < urb_priv->length; i++) { + for (; i < urb_priv->num_tds; i++) { td = urb_priv->td[i]; list_add_tail(&td->cancelled_td_list, &ep->cancelled_td_list); } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 9193a42..dab2719 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1608,8 +1608,8 @@ struct xhci_scratchpad { }; struct urb_priv { - int length; - int td_cnt; + int num_tds; + int num_tds_done; struct xhci_td *td[0]; }; -- 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: [PATCH 36/37] xhci: simplify how we store TDs in urb private data
From: Mathias Nyman > Sent: 20 January 2017 14:47 > Instead of storing a zero length array of td pointers, and then > allocate memory both for the td pointer array and the td's, just > use a zero length array of actual td's in urb private data. This reminds me of an old patch that got reverted because things broke because the lifetimes/accesses of the data wasn't the obvious one. That might have been a different structure. FWIW it is a shame that things like USB3 ethernet can't transmit and receive without all these intermediate and dynamically allocated structures. 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 v2 31/37] usb: host: xhci: convert several if() to a single switch statement
From: Felipe Balbi when getting endpoint type, a switch statement looks better than a series of if () branches. There are no functional changes with this patch, cleanup only. Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-mem.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 0640e762..0019094 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1414,14 +1414,16 @@ static u32 xhci_get_endpoint_type(struct usb_host_endpoint *ep) in = usb_endpoint_dir_in(&ep->desc); - if (usb_endpoint_xfer_control(&ep->desc)) + switch (usb_endpoint_type(&ep->desc)) { + case USB_ENDPOINT_XFER_CONTROL: return CTRL_EP; - if (usb_endpoint_xfer_bulk(&ep->desc)) + case USB_ENDPOINT_XFER_BULK: return in ? BULK_IN_EP : BULK_OUT_EP; - if (usb_endpoint_xfer_isoc(&ep->desc)) + case USB_ENDPOINT_XFER_ISOC: return in ? ISOC_IN_EP : ISOC_OUT_EP; - if (usb_endpoint_xfer_int(&ep->desc)) + case USB_ENDPOINT_XFER_INT: return in ? INT_IN_EP : INT_OUT_EP; + } return 0; } -- 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
[PATCH v2 34/37] xhci: rename size variable to num_tds
No functinal changes. num_tds describes the number of transfer descriptor better than "size" Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 4968e9a..40b1486 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1337,7 +1337,7 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) int ret = 0; unsigned int slot_id, ep_index; struct urb_priv *urb_priv; - int size, i; + int num_tds, i; if (!urb || xhci_check_args(hcd, urb->dev, urb->ep, true, true, __func__) <= 0) @@ -1354,32 +1354,32 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) } if (usb_endpoint_xfer_isoc(&urb->ep->desc)) - size = urb->number_of_packets; + num_tds = urb->number_of_packets; else if (usb_endpoint_is_bulk_out(&urb->ep->desc) && urb->transfer_buffer_length > 0 && urb->transfer_flags & URB_ZERO_PACKET && !(urb->transfer_buffer_length % usb_endpoint_maxp(&urb->ep->desc))) - size = 2; + num_tds = 2; else - size = 1; + num_tds = 1; urb_priv = kzalloc(sizeof(struct urb_priv) + - size * sizeof(struct xhci_td *), mem_flags); + num_tds * sizeof(struct xhci_td *), mem_flags); if (!urb_priv) return -ENOMEM; - buffer = kzalloc(size * sizeof(struct xhci_td), mem_flags); + buffer = kzalloc(num_tds * sizeof(struct xhci_td), mem_flags); if (!buffer) { kfree(urb_priv); return -ENOMEM; } - for (i = 0; i < size; i++) { + for (i = 0; i < num_tds; i++) { urb_priv->td[i] = buffer; buffer++; } - urb_priv->length = size; + urb_priv->length = num_tds; urb_priv->td_cnt = 0; urb->hcpriv = urb_priv; -- 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
[PATCH v2 27/37] usb: host: xhci: convert to list_for_each_entry_safe()
From: Felipe Balbi instead of using while(!list_empty()) followed by list_first_entry(), we can actually use list_for_each_entry_safe(). Signed-off-by: Felipe Balbi Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 92715b9..059825b 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -808,11 +808,11 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, static void xhci_kill_ring_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring) { struct xhci_td *cur_td; + struct xhci_td *tmp; - while (!list_empty(&ring->td_list)) { - cur_td = list_first_entry(&ring->td_list, - struct xhci_td, td_list); + list_for_each_entry_safe(cur_td, tmp, &ring->td_list, td_list) { list_del_init(&cur_td->td_list); + if (!list_empty(&cur_td->cancelled_td_list)) list_del_init(&cur_td->cancelled_td_list); @@ -828,6 +828,7 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, int slot_id, int ep_index) { struct xhci_td *cur_td; + struct xhci_td *tmp; struct xhci_virt_ep *ep; struct xhci_ring *ring; @@ -853,12 +854,12 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, slot_id, ep_index); xhci_kill_ring_urbs(xhci, ring); } - while (!list_empty(&ep->cancelled_td_list)) { - cur_td = list_first_entry(&ep->cancelled_td_list, - struct xhci_td, cancelled_td_list); - list_del_init(&cur_td->cancelled_td_list); + list_for_each_entry_safe(cur_td, tmp, &ep->cancelled_td_list, + cancelled_td_list) { + list_del_init(&cur_td->cancelled_td_list); inc_td_cnt(cur_td->urb); + if (last_td_in_urb(cur_td)) xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN); } -- 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: [PATCH 32/37] usb: host: xhci: remove newline from tracer
Hi, David Laight writes: > From: Mathias Nyman >> Sent: 20 January 2017 14:47 >> From: Felipe Balbi >> >> If we add that newline, the output will like like the following: >> >> kworker/2:1-42[002] 169.811435: xhci_address_ctx: >> ctx_64=0, ctx_type=2, ctx_dma=@153fbd000, ctx_va=@880153fbd000 >> >> We would rather have that in a single line. > ... >> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h >> index 4bad0d6..08bed2f 100644 >> --- a/drivers/usb/host/xhci-trace.h >> +++ b/drivers/usb/host/xhci-trace.h >> @@ -103,7 +103,7 @@ >> ((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) * >> ((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1)); >> ), >> -TP_printk("\nctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p", >> +TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p", >> __entry->ctx_64, __entry->ctx_type, >> (unsigned long long) __entry->ctx_dma, __entry->ctx_va > > I suspect the '\n' needs to go at the end of the previous trace? > Attempts to generate single lines from multiple printk() calls > are doomed to give unreadable output. > > As are attempts to generate readable multi-line output. > (Ever had multiple cpus splat stack traces at the same time??) TP_printk() adds \n to the end of every single tracepoint. Tracepoints shouldn't have \n ever ;-) -- balbi signature.asc Description: PGP signature
RE: [PATCH 32/37] usb: host: xhci: remove newline from tracer
From: Mathias Nyman > Sent: 20 January 2017 14:47 > From: Felipe Balbi > > If we add that newline, the output will like like the following: > > kworker/2:1-42[002] 169.811435: xhci_address_ctx: > ctx_64=0, ctx_type=2, ctx_dma=@153fbd000, ctx_va=@880153fbd000 > > We would rather have that in a single line. ... > diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h > index 4bad0d6..08bed2f 100644 > --- a/drivers/usb/host/xhci-trace.h > +++ b/drivers/usb/host/xhci-trace.h > @@ -103,7 +103,7 @@ > ((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) * > ((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1)); > ), > - TP_printk("\nctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p", > + TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p", > __entry->ctx_64, __entry->ctx_type, > (unsigned long long) __entry->ctx_dma, __entry->ctx_va I suspect the '\n' needs to go at the end of the previous trace? Attempts to generate single lines from multiple printk() calls are doomed to give unreadable output. As are attempts to generate readable multi-line output. (Ever had multiple cpus splat stack traces at the same time??) 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
Re: [RFC v2 1/5] UDC: Split the driver into amd (pci) and Synopsys core driver
Hi Florian, On Fri, Jan 20, 2017 at 12:58 AM, Florian Fainelli wrote: > On 01/19/2017 02:44 AM, Raviteja Garimella wrote: >> Hi, >> >> On Thu, Jan 19, 2017 at 12:15 AM, Florian Fainelli >> wrote: >>> On 01/17/2017 12:05 AM, Raviteja Garimella wrote: This patch splits the amd5536udc driver into two -- one that does pci device registration and the other file that does the rest of the driver tasks like the gadget/ep ops etc for Synopsys UDC. This way of splitting helps in exporting core driver symbols which can be used by any other platform/pci driver that is written for the same Synopsys USB device controller. The current patch also includes a change in the Kconfig and Makefile. A new config option USB_SNP_CORE will be selected automatically when any one of the platform or pci driver for the same UDC is selected. Signed-off-by: Raviteja Garimella >>> >>> Although the changes you have done make sense and it is most certainly a >>> good idea to split udc core from bus specific glue logic, it is really >>> hard to review the changes per-se because of the file rename, could that >>> happen at a later time? >> >> If you start looking at this specific patch from the header file >> (amd5536udc.h), >> the additions in there comprise of: >> - 9 function declarations >> - some module parameter variable declarations that's moved out from the older >> common file amd5536udc.c >> - 2 #includes that are needed by all files. > > Well, I don't really question the changes themselves, rather how this is > presented as a patch series to reviewers. > > What I would do, to help introduce both the rename, and the splitting of > core vs. bus-glue specific changes is: > > - have an initial patch which extracts the core functionality of the > driver and the PCI bus glue logic into adm5536udc_pci.c and left > adm5536udc.c intact (that would be a small delta to review) > > - have a second patch that performs the file rename from adm5536udc.c > into snps_udc_core.c and updates adm5536udc_pci.c eventually as a result > of that, then again, a reviewer can ignore the rename part (don't format > to generate your patches with git format-patch -M in that case) and just > focus on the conversion part for adm5536udc_pci.c > Just waited for any more comments coming in. I will submit the next version as PATCH like the way you suggested. >> >> So, basically what's done for this split is that: >> 1. the static keyword is removed from those 9 functions in the new file >> snps_udc_core.c and are exported. >> 2. The module parameters declarations (since they are used in both core >> and pci file) are moved to the header file now. > > These should really be part of the commit messages for each commit doing > the changes, this is meant to help a reviewer understand what you are > doing, and to some degree, will help him/her make an educated decision > as to what part of the code the focus should be put on. > Will do. Thanks, Ravi > Thanks > -- > Florian -- 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 v2] usb: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS
On 23/01/17 12:08, Felipe Balbi wrote: > > Hi, > > Bryan O'Donoghue writes: >> - DWC_USB3_NUM indicates the number of Device mode single directional >> endpoints, including OUT and IN endpoint 0. >> >> - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN >> endpoints active at any time, including control endpoint 0. >> >> It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to >> DWC_USB3_NUM_IN_EPS. >> >> dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus >> DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS >> equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT >> endpoints as zero. >> >> For example a from dwc3_core_num_eps() shows: >> [1.565000] /usb0@f01d: found 8 IN and 0 OUT endpoints >> >> This patch works around this case by detecting when DWC_USB3_NUM_EPS is >> equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number >> of OUT and IN endpoints to make dwc->num_in_eps equal to half >> DWC_USB3_NUM_EPS. >> >> If DWC_USB3_NUM_EPS is equal to DWC3_USB3_NUM_IN_EPS and the endpoint count >> is an odd number then dwc->num_out_eps will be assigned the extra endpoint. > > sorry, now that I spent some more time with this. Isn't something like > below solving all problems? > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 369bab16a824..68c9c84b7216 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -397,8 +397,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc) > { > struct dwc3_hwparams*parms = &dwc->hwparams; > > - dwc->num_in_eps = DWC3_NUM_IN_EPS(parms); > - dwc->num_out_eps = DWC3_NUM_EPS(parms) - dwc->num_in_eps; > + dwc->num_eps = DWC3_NUM_EPS(parms); > } > > static void dwc3_cache_hwparams(struct dwc3 *dwc) > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 0a664d8eba3f..8c187df0aa42 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2001,23 +2002,7 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 > *dwc, > > static int dwc3_gadget_init_endpoints(struct dwc3 *dwc) > { > - int ret; > - > - INIT_LIST_HEAD(&dwc->gadget.ep_list); > - > - ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_out_eps, 0); > - if (ret < 0) { > - dev_err(dwc->dev, "failed to initialize OUT endpoints\n"); > - return ret; > - } > - > - ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_in_eps, 1); > - if (ret < 0) { > - dev_err(dwc->dev, "failed to initialize IN endpoints\n"); > - return ret; > - } > - > - return 0; > + return dwc3_gadget_init_hw_endpoints(dwc, dwc->num_eps); Well I hadn't considered that level of change myself but, it should work. > > static void dwc3_gadget_free_endpoints(struct dwc3 *dwc) > > (clearly this won't compile... It's just to illustrate) > > The HW actually already tells us total number of endpoints and according > to John, they can all behave in either direction. Can you test it out > and finish it up as a proper patch? Sure no problem. > > I'll make sure to fix up the "maximum number of IN endpoints enabled at > one time" for v4.12. > > thanks > -- 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 1/3] usb: musb: dma: Add a DMA completion platform callback
On 01/20/2017 09:00 PM, Bin Liu wrote: > On Thu, Jan 19, 2017 at 11:06:57AM +0100, Alexandre Bailon wrote: >> Currently, the CPPI 4.1 driver is not completely generic and >> only work on dsps. This is because of IRQ management. >> Add a callback to dma_controller that could be invoked on DMA completion >> to acknodlege the IRQ. >> >> Signed-off-by: Alexandre Bailon >> --- >> drivers/usb/musb/musb_cppi41.c | 7 +-- >> drivers/usb/musb/musb_dma.h| 4 >> 2 files changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c >> index 1636385..f7d3d27 100644 >> --- a/drivers/usb/musb/musb_cppi41.c >> +++ b/drivers/usb/musb/musb_cppi41.c >> @@ -217,6 +217,10 @@ static void cppi41_dma_callback(void *private_data) >> int is_hs = 0; >> bool empty; >> >> +controller = cppi41_channel->controller; >> +if (controller->controller.dma_callback) >> +controller->controller.dma_callback(&controller->controller); >> + >> spin_lock_irqsave(&musb->lock, flags); >> >> dmaengine_tx_status(cppi41_channel->dc, cppi41_channel->cookie, >> @@ -249,8 +253,6 @@ static void cppi41_dma_callback(void *private_data) >> * We spin on HS (no longer than than 25us and setup a timer on >> * FS to check for the bit and complete the transfer. >> */ >> -controller = cppi41_channel->controller; >> - >> if (is_host_active(musb)) { >> if (musb->port1_status & USB_PORT_STAT_HIGH_SPEED) >> is_hs = 1; >> @@ -695,6 +697,7 @@ cppi41_dma_controller_create(struct musb *musb, void >> __iomem *base) >> controller->controller.channel_program = cppi41_dma_channel_program; >> controller->controller.channel_abort = cppi41_dma_channel_abort; >> controller->controller.is_compatible = cppi41_is_compatible; >> +controller->controller.musb = musb; >> >> ret = cppi41_dma_controller_start(controller); >> if (ret) >> diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h >> index 46357e1..8bea0cd 100644 >> --- a/drivers/usb/musb/musb_dma.h >> +++ b/drivers/usb/musb/musb_dma.h >> @@ -181,10 +181,13 @@ dma_channel_status(struct dma_channel *c) >> * @channel_release: call this to release a DMA channel >> * @channel_abort: call this to abort a pending DMA transaction, >> * returning it to FREE (but allocated) state >> + * @platform_dma_callback: invoked on DMA completion, useful to run platform >> + * code such IRQ acknowledgment. >> * >> * Controllers manage dma channels. >> */ >> struct dma_controller { >> +struct musb *musb; > > Since musb is added here, can we clean up the corresponding one in > struct cppi41_dma_controller and struct cppi? Yes, and also for the one in tusb_omap_dma. > > Regards, > -Bin. > Regards, Alexandre -- 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 6/6] dt-bindings: mt8173-mtu3: add reference clock
On Sat, Jan 21, 2017 at 7:49 PM, Chunfeng Yun wrote: > Hi, > > On Sat, 2017-01-21 at 14:11 -0600, Rob Herring wrote: >> On Wed, Jan 18, 2017 at 02:08:27PM +0800, Chunfeng Yun wrote: >> > add a reference clock for compatibility >> >> Why? This block suddenly has 2 clocks instead of 1? > In fact, there is a reference clock which comes from 26M oscillator > directly. I ignore it because it is a fixed-clock in DTS, and always > turned on for mt8173. But later, I find that I made a mistake before > when I bring up it on a new platform whose reference clock comes from > PLL, and need control it. So here add it, no matter it is a fixed-clock > or not. Add this explanation to the changelog. Rob -- 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: [RESEND PATCH 6/6] dt-bindings: phy-mt65xx-usb: add support for mt2712 platform
On Sat, Jan 21, 2017 at 8:50 PM, Chunfeng Yun wrote: > On Sat, 2017-01-21 at 14:08 -0600, Rob Herring wrote: >> On Wed, Jan 18, 2017 at 02:00:14PM +0800, Chunfeng Yun wrote: >> > add a new compatible string for "mt2712", and a new reference clock >> > for SuperSpeed analog phy; >> > >> > Signed-off-by: Chunfeng Yun >> > --- >> > .../devicetree/bindings/phy/phy-mt65xx-usb.txt | 81 >> > +--- >> > 1 file changed, 70 insertions(+), 11 deletions(-) [...] >> > @@ -31,21 +37,27 @@ Example: >> > u3phy: usb-phy@1129 { >> > compatible = "mediatek,mt8173-u3phy"; >> > reg = <0 0x1129 0 0x800>; >> > - clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>; >> > - clock-names = "u3phya_ref"; >> > + clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>, <&clk26m>; >> > + clock-names = "u2ref_clk", "u3ref_clk"; >> > #address-cells = <2>; >> > #size-cells = <2>; >> > ranges; >> > status = "okay"; >> > >> > - phy_port0: port@11290800 { >> > - reg = <0 0x11290800 0 0x800>; >> > + u2port0: port@11290800 { >> >> port is for OF graph. This should be usb-phy@... instead. > Is there any problems if u2port0's name@addr is the same as its parent's > (u3phy)? as following: > u3phy: usb-phy@1129 { > compatible = ...; > // no reg property > clocks = ...; > u2port0: usb-phy@1129 { > reg = ...; > } No problem, that is fine. Rob -- 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] HID: get rid of HID_QUIRK_NO_INIT_REPORTS
On Thu, 5 Jan 2017, Benjamin Tissoires wrote: > I don't know what to do about hiddev too. I don't know if we have actual > users besides some debugging tools. And hidraw is much better than > hiddev, so ideally, I'd like to remove it some way. First, I'd love to get rid of HID_QUIRK_NO_INIT_REPORTS; I am whole-heartedly convinced that initializing reports was a big mistake to start with, but changing history is hard :) Regarding hiddev -- there actually are 'legitimate' non-debug users; for example NUT (UPS management software) has been using it last time I looked. I am pretty sure there are more. So we'd rather be careful not to break hiddev by default. I'll think about your RFC a little bit more. Thanks, -- 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: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
On Mon, Jan 23, 2017 at 12:47:31PM +0100, Petr Cvek wrote: > Dne 23.1.2017 v 12:32 Greg KH napsal(a): > I know it is only a cosmetic change on a legacy driver, but I assume > it would be better to have some default value for configfs API than to > borrow a PID from a whole different gadget. > >>> > >>> For class devices, they really don't need a new id, we should just use > >>> only one of them as it doesn't matter :) > >> > > So using 0106 "Composite gadget: ACM + Mass Storage" or 0104 > "Multifunction Composite Gadget" should be fine? (my actual setup is > not multifunction though). > > I'm actually trying to catch bug, when using configfs access webcam > does not work or kernel oopses (and webcam does not work) :-D. The product id should never be an issue for a bug, if so, then something a lot worse is going on. > >> fine by me. Just lsusb will look funky ;-) > > > > Heh, true, but I thought lsusb would use a string if the device provided > > it. Haven't looked at that portion of the code in a very long time... > > > > My lsusb shows separate strings (using usbutils from slackware64-current): > > Bus 004 Device 003: ID 1d6b:0102 Linux Foundation EEM Gadget > ... > idVendor 0x1d6b Linux Foundation > idProduct 0x0102 EEM Gadget > bcdDevice4.07 > iManufacturer 1 Linux Foundation > iProduct2 Webcam gadget > ... Ah, I guess it doesn't, but who knows how old that version of usbutils is, considering the last release I did was well over a year ago. I should do a new one one of these days... Anyway, I'd like to not assign a product id to a chunk of code that is going to be eventually deleted. Felipe, what's the plan for the "legacy" gadget code. Is it ever going away? 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
Re: [PATCHv15 3/3] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY
Hi, Heikki Krogerus writes: > +static int wcove_typec_probe(struct platform_device *pdev) > +{ [snip] > + wcove->cap.prefer_role = TYPEC_NO_PREFERRED_ROLE; we have a slight problem here that affects users of this particular driver. Well, more specifically, it affects Intel Joule. Because of the way ASL was written and the way Intel's DRD mux works, we don't have a state which means "don't route USB signals to either Host or Peripheral". Because of that, when we plug the type-c cable either XHCI or dwc3 will have noise coming into the IP. If default mode ends up being peripheral we have two possible problems here: i) device-to-device cable assembly this won't be a big problem because we will just negotiate who's Sink and who's Source then change mux on one side. ii) lack of disconnect IRQ on dwc3 Because of how ASL was written, when we unplug the cable, mux's VBUS_VALID bit won't be cleared which means dwc3 won't generate disconnect interrupt. This means that upon cable reconnect (!!) we will run ->disconnect() callback. The result is that dwc3 will never runtime suspend. If default mode ends up being host we're possibly going to end up with a host-to-host cable assembly. Now this can cause 2 issues: i) port config error host-to-host is not a supported cable assembly. While we see errors on dmesg, eventually type-c negotiation will happen and nothing will actually break. ii) DbC can start on the other end of the cable Now this was really surprising to me. When testing this on Intel Joule and plugging Intel Joule straight to my PC's roothub port, I noticed Joule ended up being host and my Desktop () became a peripheral enumerated by Joule. I can only assume this is DbC somehow being started. The only way to have Joule become a peripheral is to connect it through an external hub to my PC. Odd, ain't it? I'm not sure how to solve this problem apart from modifying BIOS :-( Any ideas? -- balbi signature.asc Description: PGP signature
Re: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?
Hello, On Mon, 23 Jan 2017 14:00:40 +0200, Felipe Balbi wrote: > it could be that we're ran out of IN endpoints. There's a maximum number > to how many IN endpoints we can have enabled at one time and, currently, > dwc3 is not enforcing that in any way (I'll get that sorted out for > v4.12, v4.11 is already too late) 8OUT failing also caught my eye, which is the only OUT endpoint failing. And it happens when ep_addr & 0x7 == 0. Could something (hardware ?) be confused and handle this as a kind of EP0 ? Then, there would be 3 patterns to my uneducated eye: - 15IN and several downward would fail because of the dwc3 active IN endpoint limit you mention - 8IN and 8OUT would fail for some aliasing-with-EP0 reason - 1IN and 9IN may fail for a related reason (depending on where the IN endpoint limit exactly stands - 8 or 9 I guess - and whether EP0 counts towards the limit) BTW, I also checked what my protocol analyser says in this 30-endpoints version, but the result is quite boring: - all (failing) IN endpoints NAK - the one failing OUT endpoint NAKs the first attempt, then follows the PING protocol (MAK'ing all PINGs until the 0.2s timeout I set in the host test app). Addresses are properly transmitted (ie, it's not on bus level that EP8 would get aliased). I still could not find the time to rebuild my machine's kernel on your xhci-cleanup branch, so I'm still on 4.8.11 from a bit outdated Debian sid package). I also noticed your commit about dwc3 having problems when clearing halt of an already non-halted endpoint (and reciprocally), and I do clear halts in the device test program. I tried commenting that out in my program and merging your testing/next branch in my working copy, without any improvement. Regards, -- Vincent Pelletier pgpvG5cslVx5L.pgp Description: Signature digitale OpenPGP
Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget
Hi, Greg KH writes: >> >> fine by me. Just lsusb will look funky ;-) >> > >> > Heh, true, but I thought lsusb would use a string if the device provided >> > it. Haven't looked at that portion of the code in a very long time... >> > >> >> My lsusb shows separate strings (using usbutils from slackware64-current): >> >> Bus 004 Device 003: ID 1d6b:0102 Linux Foundation EEM Gadget >> ... >> idVendor 0x1d6b Linux Foundation >> idProduct 0x0102 EEM Gadget >> bcdDevice4.07 >> iManufacturer 1 Linux Foundation >> iProduct2 Webcam gadget >> ... > > Ah, I guess it doesn't, but who knows how old that version of usbutils > is, considering the last release I did was well over a year ago. I > should do a new one one of these days... > > Anyway, I'd like to not assign a product id to a chunk of code that is > going to be eventually deleted. Felipe, what's the plan for the > "legacy" gadget code. Is it ever going away? Well, I wasn't really planning on deleting them just stopped accepting any new one. I wanted to avoid angry mobs complaining about not having a g_mass_storage.ko anymore. Personally, I don't feel strongly about the legacy gadget drivers. They're not really needed anymore as everything they do can be done with configfs already. Perhaps we could schedule their removal for v5.0? -- balbi signature.asc Description: PGP signature
Re: [PATCH v3 2/3] dmaengine: cppi41: Add support of DA8xx to CPPI 4.1
On 01/19/2017 07:15 PM, Sergei Shtylyov wrote: > On 01/19/2017 02:13 PM, Alexandre Bailon wrote: > > > The DA8xx has a CPPI 4.1 DMA controller. > > This is add the glue layer required to make it work on DA8xx. > > > > Signed-off-by: Alexandre Bailon > > --- > > drivers/dma/Kconfig | 6 +++--- > > drivers/dma/cppi41.c | 24 > > 2 files changed, 27 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig > > index 0d6a96e..03ae2a1 100644 > > --- a/drivers/dma/Kconfig > > +++ b/drivers/dma/Kconfig > > @@ -514,12 +514,12 @@ config TIMB_DMA > >Enable support for the Timberdale FPGA DMA engine. > > > > config TI_CPPI41 > > -tristate "AM33xx CPPI41 DMA support" > > -depends on ARCH_OMAP > > +tristate "CPPI41 DMA support" > >Grr... CPPI 4.1, please. > > > +depends on (ARCH_OMAP || ARCH_DAVINCI_DA8XX) > > select DMA_ENGINE > > help > >The Communications Port Programming Interface (CPPI) 4.1 DMA > > engine > > - is currently used by the USB driver on AM335x platforms. > > + is currently used by the USB driver on AM335x and DA8xx platforms. > > > > config TI_DMA_CROSSBAR > > bool > > diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c > > index 9767b97..f6f2d84 100644 > > --- a/drivers/dma/cppi41.c > > +++ b/drivers/dma/cppi41.c > [...] > > @@ -951,8 +965,18 @@ static const struct cppi_glue_infos > > am335x_usb_infos = { > > .qmgr_num_pend = 5, > > }; > > > > +static const struct cppi_glue_infos da8xx_usb_infos = { > > +.isr = cppi41_irq, > >Isn't the ISR the same for all glues now? True. I should remove isr callback. > > [...] > > MBR, Sergei > -- 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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?
Hi, Vincent Pelletier writes: > On Mon, 23 Jan 2017 14:00:40 +0200, Felipe Balbi > wrote: >> it could be that we're ran out of IN endpoints. There's a maximum number >> to how many IN endpoints we can have enabled at one time and, currently, >> dwc3 is not enforcing that in any way (I'll get that sorted out for >> v4.12, v4.11 is already too late) > > 8OUT failing also caught my eye, which is the only OUT endpoint > failing. And it happens when ep_addr & 0x7 == 0. Could something > (hardware ?) be confused and handle this as a kind of EP0 ? hmmm, what does the following show? # mkdir -p /d # mount -t debugfs none /d # grep HWPARAMS3 /d/*dwc3*/regdump > Then, there would be 3 patterns to my uneducated eye: > - 15IN and several downward would fail because of the dwc3 active IN > endpoint limit you mention > - 8IN and 8OUT would fail for some aliasing-with-EP0 reason > - 1IN and 9IN may fail for a related reason (depending on where the IN > endpoint limit exactly stands - 8 or 9 I guess - and whether EP0 > counts towards the limit) well, the IN limit can be lower than what HW reports depending on how your TX FIFO space is setup. > BTW, I also checked what my protocol analyser says in this 30-endpoints > version, but the result is quite boring: > - all (failing) IN endpoints NAK > - the one failing OUT endpoint NAKs the first attempt, then follows the > PING protocol (MAK'ing all PINGs until the 0.2s timeout I set in the > host test app). > Addresses are properly transmitted (ie, it's not on bus level that EP8 > would get aliased). I still could not find the time to rebuild my > machine's kernel on your xhci-cleanup branch, so I'm still on 4.8.11 > from a bit outdated Debian sid package). > > I also noticed your commit about dwc3 having problems when clearing > halt of an already non-halted endpoint (and reciprocally), and I do > clear halts in the device test program. I tried commenting that out in > my program and merging your testing/next branch in my working copy, > without any improvement. thorough testing, thanks :-) -- balbi signature.asc Description: PGP signature
Re: [PATCH 36/37] xhci: simplify how we store TDs in urb private data
On 23.01.2017 14:15, Mathias Nyman wrote: On 23.01.2017 13:57, David Laight wrote: From: Mathias Nyman Sent: 20 January 2017 14:47 Instead of storing a zero length array of td pointers, and then allocate memory both for the td pointer array and the td's, just use a zero length array of actual td's in urb private data. This reminds me of an old patch that got reverted because things broke because the lifetimes/accesses of the data wasn't the obvious one. Can you recall more details about that case? I'd hate to revert this later. Thanks for the (off list) info. You submitted a similar patch in 2013, but it caused a regression and was not applied: https://www.spinics.net/lists/linux-usb/msg99748.html Reason it regressed back then was because we handled stalled endpoints a bit differently. We saved a pointer to the current TD of a stalled endpoint, then gave back the URB, and freed urb_priv, but _not_ the TD that urb_priv->td[x] pointed to. This TD was later used in the endpoint reset callback to figure out where we should set the dequeue pointer of that ring. Handling stalled endpoints has changed since then. We don't store the TD pointer in STALL cases, and we free the urb_priv completely when giving back the URB see: commit 8e71a322fdb127814bcba423a512914ca5bc6cf5 USB: xhci: Reset a halted endpoint immediately when we encounter a stall. So now it should work -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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?
On Mon, 23 Jan 2017 16:30:57 +0200, Felipe Balbi wrote: > hmmm, what does the following show? > > # mkdir -p /d > # mount -t debugfs none /d > # grep HWPARAMS3 /d/*dwc3*/regdump # grep HWPARAMS3 /sys/kernel/debug/*dwc3*/regdump GHWPARAMS3 = 0x10420089 Regards, -- Vincent Pelletier pgp9ZJv9GNzfE.pgp Description: Signature digitale OpenPGP
usb: gadget: f_fs: Accept up to 30 endpoints.
It is allowed by the USB specification to enabled same-address, opposite- direction endpoints simultaneously, which means 30 non-zero endpoints are allowed. So double eps_addrmap length to 30. The original code only accepted 14 descriptors out of a likely intended 15 (as there are 15 endpoint addresses, ignoring direction), because the first eps_addrmap entry is unused (it is a placeholder for endpoint zero). So increase eps_addrmap length by one to 31. Signed-off-by: Vincent Pelletier --- drivers/usb/gadget/function/f_fs.c | 2 +- drivers/usb/gadget/function/u_fs.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 5c91a6f4613b..c4364c87 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -2101,7 +2101,7 @@ static int __ffs_data_do_entity(enum ffs_entity_type type, case FFS_ENDPOINT: d = (void *)desc; helper->eps_count++; - if (helper->eps_count >= 15) + if (helper->eps_count >= FFS_MAX_EPS_COUNT) return -EINVAL; /* Check if descriptors for any speed were already parsed */ if (!helper->ffs->eps_count && !helper->ffs->interfaces_count) diff --git a/drivers/usb/gadget/function/u_fs.h b/drivers/usb/gadget/function/u_fs.h index 60139854e0b1..4b6969451cdc 100644 --- a/drivers/usb/gadget/function/u_fs.h +++ b/drivers/usb/gadget/function/u_fs.h @@ -247,7 +247,8 @@ struct ffs_data { unsigneduser_flags; - u8 eps_addrmap[15]; +#define FFS_MAX_EPS_COUNT 31 + u8 eps_addrmap[FFS_MAX_EPS_COUNT]; unsigned short strings_count; unsigned short interfaces_count; -- 2.11.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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?
Changes since try 1: - impove commit message a bit - #declare array size to avoid duplication -- 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 linux-next v2 1/1] usb: gadget: udc: atmel: Update endpoint allocation scheme
From: Cristian Birsan Update atmel udc driver with a new enpoint allocation scheme. The data sheet requires that all endpoints are allocated in order. Signed-off-by: Cristian Birsan --- drivers/usb/gadget/udc/Kconfig | 14 ++ drivers/usb/gadget/udc/atmel_usba_udc.c | 236 +++- drivers/usb/gadget/udc/atmel_usba_udc.h | 10 +- 3 files changed, 227 insertions(+), 33 deletions(-) diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index 658b8da..4b69f28 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -60,6 +60,20 @@ config USB_ATMEL_USBA USBA is the integrated high-speed USB Device controller on the AT32AP700x, some AT91SAM9 and AT91CAP9 processors from Atmel. + The fifo_mode parameter is used to select endpoint allocation mode. + fifo_mode = 0 is used to let the driver autoconfigure the endpoints. + In this case 2 banks are allocated for isochronous endpoints and + only one bank is allocated for the rest of the endpoints. + + fifo_mode = 1 is a generic maximum fifo size (1024 bytes) configuration + allowing the usage of ep1 - ep6 + + fifo_mode = 2 is a generic performance maximum fifo size (1024 bytes) + configuration allowing the usage of ep1 - ep3 + + fifo_mode = 3 is a balanced performance configuration allowing the + the usage of ep1 - ep8 + config USB_BCM63XX_UDC tristate "Broadcom BCM63xx Peripheral Controller" depends on BCM63XX diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c index 12c7687..11bbce2 100644 --- a/drivers/usb/gadget/udc/atmel_usba_udc.c +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -318,6 +319,91 @@ static inline void usba_cleanup_debugfs(struct usba_udc *udc) } #endif +static ushort fifo_mode; + +/* "modprobe ... fifo_mode=1" etc */ +module_param(fifo_mode, ushort, 0x0); +MODULE_PARM_DESC(fifo_mode, "Endpoint configuration mode"); + +/* mode 0 - uses autoconfig */ + +/* mode 1 - fits in 8KB, generic max fifo configuration */ +static struct usba_fifo_cfg mode_1_cfg[] = { +{ .hw_ep_num = 0, .fifo_size = 64, .nr_banks = 1, }, +{ .hw_ep_num = 1, .fifo_size = 1024, .nr_banks = 2, }, +{ .hw_ep_num = 2, .fifo_size = 1024, .nr_banks = 1, }, +{ .hw_ep_num = 3, .fifo_size = 1024, .nr_banks = 1, }, +{ .hw_ep_num = 4, .fifo_size = 1024, .nr_banks = 1, }, +{ .hw_ep_num = 5, .fifo_size = 1024, .nr_banks = 1, }, +{ .hw_ep_num = 6, .fifo_size = 1024, .nr_banks = 1, }, +}; + +/* mode 2 - fits in 8KB, performance max fifo configuration */ +static struct usba_fifo_cfg mode_2_cfg[] = { +{ .hw_ep_num = 0, .fifo_size = 64, .nr_banks = 1, }, +{ .hw_ep_num = 1, .fifo_size = 1024, .nr_banks = 3, }, +{ .hw_ep_num = 2, .fifo_size = 1024, .nr_banks = 2, }, +{ .hw_ep_num = 3, .fifo_size = 1024, .nr_banks = 2, }, +}; + +/* mode 3 - fits in 8KB, mixed fifo configuration */ +static struct usba_fifo_cfg mode_3_cfg[] = { +{ .hw_ep_num = 0, .fifo_size = 64, .nr_banks = 1, }, +{ .hw_ep_num = 1, .fifo_size = 1024, .nr_banks = 2, }, +{ .hw_ep_num = 2, .fifo_size = 512,.nr_banks = 2, }, +{ .hw_ep_num = 3, .fifo_size = 512,.nr_banks = 2, }, +{ .hw_ep_num = 4, .fifo_size = 512,.nr_banks = 2, }, +{ .hw_ep_num = 5, .fifo_size = 512,.nr_banks = 2, }, +{ .hw_ep_num = 6, .fifo_size = 512,.nr_banks = 2, }, +}; + +/* mode 4 - fits in 8KB, custom fifo configuration */ +static struct usba_fifo_cfg mode_4_cfg[] = { +{ .hw_ep_num = 0, .fifo_size = 64, .nr_banks = 1, }, +{ .hw_ep_num = 1, .fifo_size = 512,.nr_banks = 2, }, +{ .hw_ep_num = 2, .fifo_size = 512,.nr_banks = 2, }, +{ .hw_ep_num = 3, .fifo_size = 8, .nr_banks = 2, }, +{ .hw_ep_num = 4, .fifo_size = 512,.nr_banks = 2, }, +{ .hw_ep_num = 5, .fifo_size = 512,.nr_banks = 2, }, +{ .hw_ep_num = 6, .fifo_size = 16, .nr_banks = 2, }, +{ .hw_ep_num = 7, .fifo_size = 8, .nr_banks = 2, }, +{ .hw_ep_num = 8, .fifo_size = 8, .nr_banks = 2, }, +}; +/* Add additional configurations here */ + +int usba_config_fifo_table(struct usba_udc *udc) +{ + int n; + + switch (fifo_mode) { + default: + fifo_mode = 0; + case 0: + udc->fifo_cfg = NULL; + n = 0; + break; + case 1: + udc->fifo_cfg = mode_1_cfg; + n = ARRAY_SIZE(mode_1_cfg); + break; + case 2: + udc->fifo_cfg = mode_2_cfg; + n = ARRAY_SIZE(mode_2_cfg); + break; + case 3: + udc->fifo_cfg = mode_3_cfg; + n = ARRAY_SIZE(mode_3_cfg); + break; + case 4: + udc->fifo_cfg = mode_4_cfg; + n = ARRAY_SIZE(mode_4_cfg); + break; +
Re: [PATCH v5] USB: Add uPD78F0730 USB to Serial Adaptor Driver
On Sun, Jan 22, 2017 at 12:32:16AM +0300, Maksim Salau wrote: > The adaptor can be found on development boards for 78k, RL78 and V850 > microcontrollers produced by Renesas Electronics Corporation. > > This is not a full-featured USB to serial converter, however it allows > basic communication and simple control which is enough for programming of > on-board flash and debugging through a debug monitor. > > uPD78F0730 is a USB-enabled microcontroller with USB-to-UART conversion > implemented in firmware. > > This chip is also present in some debugging adaptors which use it for > USB-to-SPI conversion as well. The present driver doesn't cover SPI, > only USB-to-UART conversion is supported. > > Signed-off-by: Maksim Salau > --- > Changes in v5: > Fixed a typo in assignment of opcode of the SET_DTR_RTS request > > Changes in v4: > Addressed comments from Johan You made some further changes than what I suggested but forgot to document those. Often better to explicitly list the changes made rather than refer to review comments this way. This already looks really good; just a few minor things noted below. > drivers/usb/serial/Kconfig | 9 + > drivers/usb/serial/Makefile | 1 + > drivers/usb/serial/upd78f0730.c | 458 > > 3 files changed, 468 insertions(+) > create mode 100644 drivers/usb/serial/upd78f0730.c > +static int upd78f0730_send_ctl(struct usb_serial_port *port, > + void *data, int size) > +{ > + struct device *dev = &port->dev; > + struct usb_device *usbdev = port->serial->dev; > + void *buf; > + int res; > + > + if (!size || !data) > + return -EINVAL; > + > + buf = kmemdup(data, size, GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > + > + res = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x00, > + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, > + 0x, 0x, buf, size, USB_CTRL_SET_TIMEOUT); > + > + kfree(buf); > + > + if (res < 0) > + return res; > + > + if (res != size) { > + dev_err(dev, "%s - send failed: opcode=%02x, size=%d, res=%d\n", > + __func__, *(u8 *)data, size, res); You still want an error message in case res < 0 (as in v3), but you can return that errno instead of -EIO then. You can drop the __func__ prefixes you added to error messages in v5 throughout, better to spell out what went wrong (e.g. "failed to send control request %02x: %d\n", *(u8 *)data, res). > + /* The maximum expected length of a transfer is 6 bytes */ > + return -EIO; > + } > + > + return 0; > +} > + > +static int upd78f0730_attach(struct usb_serial *serial) > +{ > + const char num_ports = serial->num_ports; > + > + if (serial->num_bulk_in < num_ports || > + serial->num_bulk_out < num_ports) { > + dev_err(&serial->interface->dev, "%s - missing endpoints\n", > + __func__); > + return -ENODEV; > + } > + > + return 0; > +} This is new in v5, and should have been mentioned in the changelog above. Note that this is not strictly necessary, though. Some drivers dereferenced pointers without the appropriate sanity checks, but this driver and the generic callbacks it uses, does not suffer from such problems. I suggest you just drop this check (i.e. the attach callback). > +static int upd78f0730_tiocmget(struct tty_struct *tty) > +{ > + struct device *dev = tty->dev; > + struct upd78f0730_port_private *private; > + struct usb_serial_port *port = tty->driver_data; > + int signals; > + int res; > + > + private = usb_get_serial_port_data(port); > + > + mutex_lock(&private->lock); > + signals = private->line_signals; > + mutex_unlock(&private->lock); > + > + res = ((signals & UPD78F0730_DTR) ? TIOCM_DTR : 0) | > + ((signals & UPD78F0730_RTS) ? TIOCM_RTS : 0); > + > + dev_dbg(dev, "%s - res = %x\n", __func__, res); Note that __func__ is fine for concise debug messages like this one (that users generally won't see). But please drop it from all ERR and WARN messages. > + > + return res; > +} Thanks, 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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?
Hi, Vincent Pelletier writes: > On Mon, 23 Jan 2017 16:30:57 +0200, Felipe Balbi > wrote: >> hmmm, what does the following show? >> >> # mkdir -p /d >> # mount -t debugfs none /d >> # grep HWPARAMS3 /d/*dwc3*/regdump > > # grep HWPARAMS3 /sys/kernel/debug/*dwc3*/regdump > GHWPARAMS3 = 0x10420089 odd, this board was configured with support for all 32 endpoints. You should not be running out of FIFO space and you can have all 16 IN endpoints enabled at the same time. This all shouldn't matter for you. Unless total TX FIFO size isn't large enough for all 16 endpoints, which I kinda doubt. -- balbi signature.asc Description: PGP signature
Re: [PATCHv15 2/3] usb: USB Type-C connector class
Hi, Heikki Krogerus writes: > +static void typec_report_identity(struct device *dev) > +{ > + sysfs_notify(&dev->kobj, "identity", "id_header"); > + sysfs_notify(&dev->kobj, "identity", "cert_stat"); > + sysfs_notify(&dev->kobj, "identity", "product"); if you sysfs_notify() all three everytime this might cause issues for userspace pollers. What will happen is that you're gonna change e.g. id_header and threads polling id_header, cert_stat and product will be notified of what's supposed to be new data. Maybe this should know what it's notifying and only notify what actually changed. Seems like just passing one extra char * argument is enough: static void typec_report_identity(struct device *dev, const char *prop) { sysfs_notify(&dev->kobj, "identity", prop); } > +static ssize_t power_role_show(struct device *dev, > +struct device_attribute *attr, char *buf) > +{ > + struct typec_port *port = to_typec_port(dev); > + > + if (port->cap->type == TYPEC_PORT_DRP) > + return sprintf(buf, "%s\n", port->pwr_role == TYPEC_SOURCE ? > +"[source] sink" : "source [sink]"); > + > + return sprintf(buf, "[%s]\n", typec_roles[port->pwr_role]); > +} > +static DEVICE_ATTR_RW(power_role); > + > +static const char * const typec_pwr_opmodes[] = { > + [TYPEC_PWR_MODE_USB]= "default", > + [TYPEC_PWR_MODE_1_5A] = "1.5A", > + [TYPEC_PWR_MODE_3_0A] = "3.0A", > + [TYPEC_PWR_MODE_PD] = "usb_power_delivery", do you need this to have underscores? How about "USB Power Delivery", instead? Userland could use the string untreated as part of GUI. > +static ssize_t vconn_source_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + struct typec_port *port = to_typec_port(dev); > + enum typec_role role; > + int ret; > + > + if (!port->cap->pd_revision) { > + dev_dbg(dev, "VCONN swap depends on USB Power Delivery\n"); > + return -EOPNOTSUPP; > + } > + > + if (!port->cap->vconn_set) { > + dev_dbg(dev, "VCONN swapping not supported\n"); > + return -EOPNOTSUPP; > + } > + > + if (sysfs_streq(buf, "1")) > + role = TYPEC_SOURCE; > + else if (sysfs_streq(buf, "0")) > + role = TYPEC_SINK; > + else > + return -EINVAL; seems to me this could be stringified (both store and show) to "source" and "sink". > + > + ret = port->cap->vconn_set(port->cap, role); > + if (ret) > + return ret; > + > + return size; > +} > + > +static ssize_t vconn_source_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct typec_port *port = to_typec_port(dev); > + > + return sprintf(buf, "%d\n", port->vconn_role == TYPEC_SOURCE ? 1 : 0); > +} > +static DEVICE_ATTR_RW(vconn_source); > + > +static ssize_t supported_accessory_modes_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct typec_port *port = to_typec_port(dev); > + ssize_t ret = 0; > + int i; > + > + if (!port->cap->accessory) > + return 0; > + > + for (i = 0; port->cap->accessory[i]; i++) > + ret += sprintf(buf + ret, "%s ", > +typec_accessory_modes[port->cap->accessory[i]]); possible buffer overflow here? -- balbi signature.asc Description: PGP signature
[PATCH linux-next v2 0/1] usb: gadget: udc: atmel: Update endpoint allocation scheme
From: Cristian Birsan Hi, This patch updates the usb endpoint allocation scheme for atmel usba driver to make sure all endpoints are allocated in order. This requirement comes from the datasheet of the controller. The allocation scheme is decided by fifo_mode parameter. For fifo_mode = 0 the driver tries to autoconfigure the endpoints fifo size. All other modes contain fixed configurations optimized for different purposes. The idea is somehow similar with the approach used on musb driver. Please let me know if you have any comments or suggestions. Changes since v1: - Minor reworks based on received fedback Kind regards, Cristian Cristian Birsan (1): usb: gadget: udc: atmel: Update endpoint allocation scheme drivers/usb/gadget/udc/Kconfig | 14 ++ drivers/usb/gadget/udc/atmel_usba_udc.c | 236 +++- drivers/usb/gadget/udc/atmel_usba_udc.h | 10 +- 3 files changed, 227 insertions(+), 33 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html