Re: [PATCH V2 1/2] dt-bindings: leds: document new led-triggers property
Hi Rafał, On 01/20/2017 10:56 PM, Rafał Miłecki wrote: > From: Rafał Miłecki> > Some LEDs can be related to particular devices described in DT. This > property allows specifying such relations. E.g. USB LED should usually > be used to indicate some USB port(s) state. > > Signed-off-by: Rafał Miłecki > --- > V2: Replace "usb-ports" with "led-triggers" property which is more generic and > allows specifying other devices as well. > > When bindings patch is related to some followup implementation, they usually > go > through the same tree. > > Greg: this patch is based on top of e64b8cc72bf9 ("DT: leds: Improve examples > by > adding some context") from kernel/git/j.anaszewski/linux-leds.git . Is there > any > way to solve this dependency issue? Or should this patch wait until 3.11 is > released? > --- > Documentation/devicetree/bindings/leds/common.txt | 16 > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/leds/common.txt > b/Documentation/devicetree/bindings/leds/common.txt > index 24b656014089..17632a041196 100644 > --- a/Documentation/devicetree/bindings/leds/common.txt > +++ b/Documentation/devicetree/bindings/leds/common.txt > @@ -49,6 +49,17 @@ Optional properties for child nodes: > - panic-indicator : This property specifies that the LED should be used, > if at all possible, as a panic indicator. > > +- led-triggers : List of devices that should trigger this LED activity. Some > + LEDs can be related to a specific device and should somehow > + indicate its state. E.g. USB 2.0 LED may react to device(s) in > + a USB 2.0 port(s). Another common example is switch or router > + with multiple Ethernet ports each of them having its own LED > + assigned (assuminled-trigger-usbportg they are not hardwired). > + In such cases this property should contain phandle(s) of > + related device(s). In many cases LED can be related to more > + than one device (e.g. one USB LED vs. multiple USB ports) so a > + list of entries can be specified. > + This implies that it is possible to define multiple triggers for a LED class device but it is not supported by LED Trigger core. There is linux,default-trigger property which allows to define one trigger that will be initially assigned. I am aware that this is renamed usb-ports property from v1, that attempts to address Rob's comment, but we can't do that this way. Maybe usb-ports property could be documented in led-trigger-usbport's specific bindings and a reference to it could be added next to the related entry on the list of the available LED triggers (which is actually missing) in Documentation/devicetree/bindings/leds/common.txt. > Required properties for flash LED child nodes: > - flash-max-microamp : Maximum flash LED supply current in microamperes. > - flash-max-timeout-us : Maximum timeout in microseconds after which the > flash > @@ -69,6 +80,11 @@ gpio-leds { > linux,default-trigger = "heartbeat"; > gpios = < 0 GPIO_ACTIVE_HIGH>; > }; > + > + usb { > + gpios = < 1 GPIO_ACTIVE_HIGH>; > + led-triggers = <_port1>, <_port1>; > + }; > }; > > max77693-led { > -- Best regards, Jacek Anaszewski -- 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 2/2] usb: core: read USB ports from DT in the usbport LED trigger driver
From: Rafał MiłeckiThis adds support for using description of relation between LEDs and USB ports from device tree. If DT has properly described LEDs, trigger will know when to turn them on. Signed-off-by: Rafał Miłecki --- V2: Update to use "led-triggers" --- drivers/usb/core/ledtrig-usbport.c | 54 ++ 1 file changed, 54 insertions(+) diff --git a/drivers/usb/core/ledtrig-usbport.c b/drivers/usb/core/ledtrig-usbport.c index 1713248ab15a..aee80748eb7a 100644 --- a/drivers/usb/core/ledtrig-usbport.c +++ b/drivers/usb/core/ledtrig-usbport.c @@ -11,8 +11,10 @@ #include #include #include +#include #include #include +#include struct usbport_trig_data { struct led_classdev *led_cdev; @@ -123,6 +125,55 @@ static const struct attribute_group ports_group = { * Adding & removing ports ***/ +/** + * usbport_trig_port_observed - Check if port should be observed + */ +static bool usbport_trig_port_observed(struct usbport_trig_data *usbport_data, + struct usb_device *usb_dev, int port1) +{ + struct device *dev = usbport_data->led_cdev->dev; + struct device_node *led_np = dev->of_node; + struct of_phandle_args args; + struct device_node *port_np; + int count, i; + + if (!led_np) + return false; + + /* Get node of port being added */ + port_np = usb_of_get_child_node(usb_dev->dev.of_node, port1); + if (!port_np) + return false; + + /* Amount of ports this LED references */ + count = of_count_phandle_with_args(led_np, "led-triggers", NULL); + if (count < 0) { + dev_warn(dev, "Failed to get USB ports for %s\n", +led_np->full_name); + return false; + } + + /* Check if port is on this LED's list */ + for (i = 0; i < count; i++) { + int err; + + err = of_parse_phandle_with_args(led_np, "led-triggers", NULL, +i, ); + if (err) { + dev_err(dev, "Failed to get USB port phandle at index %d: %d\n", + i, err); + continue; + } + + of_node_put(args.np); + + if (args.np == port_np) + return true; + } + + return false; +} + static int usbport_trig_add_port(struct usbport_trig_data *usbport_data, struct usb_device *usb_dev, const char *hub_name, int portnum) @@ -141,6 +192,8 @@ static int usbport_trig_add_port(struct usbport_trig_data *usbport_data, port->data = usbport_data; port->hub = usb_dev; port->portnum = portnum; + port->observed = usbport_trig_port_observed(usbport_data, usb_dev, + portnum); len = strlen(hub_name) + 8; port->port_name = kzalloc(len, GFP_KERNEL); @@ -255,6 +308,7 @@ static void usbport_trig_activate(struct led_classdev *led_cdev) if (err) goto err_free; usb_for_each_dev(usbport_data, usbport_trig_add_usb_dev_ports); + usbport_trig_update_count(usbport_data); /* Notifications */ usbport_data->nb.notifier_call = usbport_trig_notify, -- 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
[PATCH V2 1/2] dt-bindings: leds: document new led-triggers property
From: Rafał MiłeckiSome LEDs can be related to particular devices described in DT. This property allows specifying such relations. E.g. USB LED should usually be used to indicate some USB port(s) state. Signed-off-by: Rafał Miłecki --- V2: Replace "usb-ports" with "led-triggers" property which is more generic and allows specifying other devices as well. When bindings patch is related to some followup implementation, they usually go through the same tree. Greg: this patch is based on top of e64b8cc72bf9 ("DT: leds: Improve examples by adding some context") from kernel/git/j.anaszewski/linux-leds.git . Is there any way to solve this dependency issue? Or should this patch wait until 3.11 is released? --- Documentation/devicetree/bindings/leds/common.txt | 16 1 file changed, 16 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt index 24b656014089..17632a041196 100644 --- a/Documentation/devicetree/bindings/leds/common.txt +++ b/Documentation/devicetree/bindings/leds/common.txt @@ -49,6 +49,17 @@ Optional properties for child nodes: - panic-indicator : This property specifies that the LED should be used, if at all possible, as a panic indicator. +- led-triggers : List of devices that should trigger this LED activity. Some +LEDs can be related to a specific device and should somehow +indicate its state. E.g. USB 2.0 LED may react to device(s) in +a USB 2.0 port(s). Another common example is switch or router +with multiple Ethernet ports each of them having its own LED +assigned (assuming they are not hardwired). +In such cases this property should contain phandle(s) of +related device(s). In many cases LED can be related to more +than one device (e.g. one USB LED vs. multiple USB ports) so a +list of entries can be specified. + Required properties for flash LED child nodes: - flash-max-microamp : Maximum flash LED supply current in microamperes. - flash-max-timeout-us : Maximum timeout in microseconds after which the flash @@ -69,6 +80,11 @@ gpio-leds { linux,default-trigger = "heartbeat"; gpios = < 0 GPIO_ACTIVE_HIGH>; }; + + usb { + gpios = < 1 GPIO_ACTIVE_HIGH>; + led-triggers = <_port1>, <_port1>; + }; }; max77693-led { -- 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
[EXAMPLE V2 3/2] ARM: BCM53573: Specify ports for USB LED for Tenda AC9
From: Rafał MiłeckiSigned-off-by: Rafał Miłecki --- This patch *should not* be applied. It's only an EXAMPLE and that's why it uses weird 3/2 number. It's a proof of concept, it was tested & will be submitted through ARM tree if previous patches get accepted. --- arch/arm/boot/dts/bcm47189-tenda-ac9.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts index 4403ae8790c2..f484d17a2270 100644 --- a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts +++ b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts @@ -24,6 +24,7 @@ compatible = "gpio-leds"; usb { + led-triggers = <_port1>, <_port1>; label = "bcm53xx:blue:usb"; gpios = < 1 GPIO_ACTIVE_HIGH>; linux,default-trigger = "default-off"; -- 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: [PATCH 4/4] usb: gadget: f_hid: fix: Move IN request allocation to set_alt()
On 01/19/2017 11:55 AM, Krzysztof Opasiak wrote: 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--- This series fixes a crash I was experiencing in the v4.9.4 kernel caused by "usb: gadget: f_hid: use alloc_ep_req()"[1]. Thank you! [1]: http://lkml.iu.edu/hypermail/linux/kernel/1701.0/00575.html Tested-by: David Lechner -- 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: phy: Implement am335x advisory 1.0.34
On Fri, Jan 20, 2017 at 12:01:04PM -0800, Tony Lindgren wrote: > I noticed in sprz360i.pdf there's "Advisory 1.0.34 USB2PHY: Register > Accesses After a USB Subsystem Soft Reset May Lock Up the Entire System" > that seems to affect am335x revisions 1.0, 2.0 and 2.1: > > "The synchronization bridge connecting the USB2PHY register interface > to the L3S interconnect may hang and lock up the entire system. When > there is a sequence of any USB2PHY register access, followed by a USB > subsystem soft reset initiated with the SOFT_RESET bit in the SYSCONFIG > register, followed by any USB2PHY register access, the L3S interconnect > may hang on the second USB2PHY register access." I understand this patch adds runtime PM in the am335x phy driver, but the message above is irrelevant. This advisory is meant for the PHY registers, not the phy-ctrl registers in the USBSS wrapper. Currently we don't touch any PHY register in any driver. Regards, -Bin. -- 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: [PATCHv4 3/2] dmaengine: cppi41: Clean up pointless warnings
* Bin Liu[170120 13:08]: > On Fri, Jan 20, 2017 at 12:07:53PM -0800, Tony Lindgren wrote: > > With patches "dmaengine: cppi41: Fix runtime PM timeouts with USB mass > > storage", and "dmaengine: cppi41: Fix oops in cppi41_runtime_resume", > > the pm_runtime_get/put() in cppi41_irq() is no longer needed. We now > > guarantee that cppi41 is enabled when dma is in use. > > > > We can still get pointless error -115 when musb is configured as a > > usb peripheral. That's because we should now check for the state of > > is_suspended instead. > > I am not sure I understand this paragraph. Do you mean we still get > harmless -115 in peripheral mode? If so how is it caused by is_suspended > check? And the comment below for the check implies the WARN_ON() never > happens... Yes I noticed we can still get it in peripheral mode. And it's a bogus warning now because we should now be using the new cdd->is_suspended instead. It happens because cppi41_runtime_resume() has not yet completed and is calling cppi41_run_queue() that produces the interrupt. So that that point we have cppi41 active with !cdd->is_suspended, but pm_runtime_get() still returns -EINPROGRESS (115). Regards, Tony -- 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: phy: Implement am335x advisory 1.0.34
* Bin Liu[170120 12:32]: > On Fri, Jan 20, 2017 at 12:01:04PM -0800, Tony Lindgren wrote: > > I noticed in sprz360i.pdf there's "Advisory 1.0.34 USB2PHY: Register > > Accesses After a USB Subsystem Soft Reset May Lock Up the Entire System" > > that seems to affect am335x revisions 1.0, 2.0 and 2.1: > > > > "The synchronization bridge connecting the USB2PHY register interface > > to the L3S interconnect may hang and lock up the entire system. When > > there is a sequence of any USB2PHY register access, followed by a USB > > subsystem soft reset initiated with the SOFT_RESET bit in the SYSCONFIG > > register, followed by any USB2PHY register access, the L3S interconnect > > may hang on the second USB2PHY register access." > > I understand this patch adds runtime PM in the am335x phy driver, but the > message above is irrelevant. This advisory is meant for the PHY > registers, not the phy-ctrl registers in the USBSS wrapper. Currently we > don't touch any PHY register in any driver. OK so we don't currently need this then. If all phy-am335x.c is doing is calling phy_ctrl_power() in another driver then yeah no point with this patch. Regards, Tony -- 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: [PATCHv4 3/2] dmaengine: cppi41: Clean up pointless warnings
On Fri, Jan 20, 2017 at 12:07:53PM -0800, Tony Lindgren wrote: > With patches "dmaengine: cppi41: Fix runtime PM timeouts with USB mass > storage", and "dmaengine: cppi41: Fix oops in cppi41_runtime_resume", > the pm_runtime_get/put() in cppi41_irq() is no longer needed. We now > guarantee that cppi41 is enabled when dma is in use. > > We can still get pointless error -115 when musb is configured as a > usb peripheral. That's because we should now check for the state of > is_suspended instead. I am not sure I understand this paragraph. Do you mean we still get harmless -115 in peripheral mode? If so how is it caused by is_suspended check? And the comment below for the check implies the WARN_ON() never happens... Regards, -Bin. > > Let's just remove the now useless code and replace it with a WARN(). > > Signed-off-by: Tony Lindgren> --- > > Found one more cosmetic issue. With patches 1/2 and 2/2 fixing the > problems, this can now wait if considered not suitable for the -rc > cycle. > > --- > drivers/dma/cppi41.c | 13 + > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c > --- a/drivers/dma/cppi41.c > +++ b/drivers/dma/cppi41.c > @@ -323,12 +323,12 @@ static irqreturn_t cppi41_irq(int irq, void *data) > > while (val) { > u32 desc, len; > - int error; > > - error = pm_runtime_get(cdd->ddev.dev); > - if (error < 0) > - dev_err(cdd->ddev.dev, "%s pm runtime get: > %i\n", > - __func__, error); > + /* > + * This should never trigger, see the comments in > + * push_desc_queue() > + */ > + WARN_ON(cdd->is_suspended); > > q_num = __fls(val); > val &= ~(1 << q_num); > @@ -349,9 +349,6 @@ static irqreturn_t cppi41_irq(int irq, void *data) > c->residue = pd_trans_len(c->desc->pd6) - len; > dma_cookie_complete(>txd); > dmaengine_desc_get_callback_invoke(>txd, NULL); > - > - pm_runtime_mark_last_busy(cdd->ddev.dev); > - pm_runtime_put_autosuspend(cdd->ddev.dev); > } > } > return IRQ_HANDLED; > -- > 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: [PATCH v3 3/3] usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps
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_STATUS 0x28 > +#define USBSS_IRQ_ENABLER0x2c > +#define USBSS_IRQ_CLEARR 0x30 > + > +#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 = >dev; > glue->wrp = wrp; > + glue->usbss_base = of_iomap(pdev->dev.parent->of_node, 0); > + if (!glue->usbss_base) use
[PATCHv4 3/2] dmaengine: cppi41: Clean up pointless warnings
With patches "dmaengine: cppi41: Fix runtime PM timeouts with USB mass storage", and "dmaengine: cppi41: Fix oops in cppi41_runtime_resume", the pm_runtime_get/put() in cppi41_irq() is no longer needed. We now guarantee that cppi41 is enabled when dma is in use. We can still get pointless error -115 when musb is configured as a usb peripheral. That's because we should now check for the state of is_suspended instead. Let's just remove the now useless code and replace it with a WARN(). Signed-off-by: Tony Lindgren--- Found one more cosmetic issue. With patches 1/2 and 2/2 fixing the problems, this can now wait if considered not suitable for the -rc cycle. --- drivers/dma/cppi41.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c --- a/drivers/dma/cppi41.c +++ b/drivers/dma/cppi41.c @@ -323,12 +323,12 @@ static irqreturn_t cppi41_irq(int irq, void *data) while (val) { u32 desc, len; - int error; - error = pm_runtime_get(cdd->ddev.dev); - if (error < 0) - dev_err(cdd->ddev.dev, "%s pm runtime get: %i\n", - __func__, error); + /* +* This should never trigger, see the comments in +* push_desc_queue() +*/ + WARN_ON(cdd->is_suspended); q_num = __fls(val); val &= ~(1 << q_num); @@ -349,9 +349,6 @@ static irqreturn_t cppi41_irq(int irq, void *data) c->residue = pd_trans_len(c->desc->pd6) - len; dma_cookie_complete(>txd); dmaengine_desc_get_callback_invoke(>txd, NULL); - - pm_runtime_mark_last_busy(cdd->ddev.dev); - pm_runtime_put_autosuspend(cdd->ddev.dev); } } return IRQ_HANDLED; -- 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
[PATCH] usb: phy: Implement am335x advisory 1.0.34
I noticed in sprz360i.pdf there's "Advisory 1.0.34 USB2PHY: Register Accesses After a USB Subsystem Soft Reset May Lock Up the Entire System" that seems to affect am335x revisions 1.0, 2.0 and 2.1: "The synchronization bridge connecting the USB2PHY register interface to the L3S interconnect may hang and lock up the entire system. When there is a sequence of any USB2PHY register access, followed by a USB subsystem soft reset initiated with the SOFT_RESET bit in the SYSCONFIG register, followed by any USB2PHY register access, the L3S interconnect may hang on the second USB2PHY register access." As the USB2PHY is a child of the otg interconnect target module, I don't think we can easily hit this as we have now musb doing runtime PM that keeps the module enabled. But as we now have musb and cppi41 components in the same interonnect target module use runtime PM, let's also add runtime PM to the phy driver. This way we have have them all behave the same way. Signed-off-by: Tony Lindgren--- drivers/usb/phy/phy-am335x.c | 61 +--- 1 file changed, 58 insertions(+), 3 deletions(-) diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c --- a/drivers/usb/phy/phy-am335x.c +++ b/drivers/usb/phy/phy-am335x.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include "phy-am335x-control.h" @@ -22,16 +23,40 @@ struct am335x_phy { static int am335x_init(struct usb_phy *phy) { struct am335x_phy *am_phy = dev_get_drvdata(phy->dev); + int error; + + error = pm_runtime_get_sync(phy->dev); + if (error < 0) { + dev_err(phy->dev, "%s pm_runtime_get: %i\n", __func__, error); + + return error; + } phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, am_phy->dr_mode, true); + + pm_runtime_mark_last_busy(phy->dev); + pm_runtime_put_autosuspend(phy->dev); + return 0; } static void am335x_shutdown(struct usb_phy *phy) { struct am335x_phy *am_phy = dev_get_drvdata(phy->dev); + int error; + + error = pm_runtime_get_sync(phy->dev); + if (error < 0) { + dev_err(phy->dev, "%s pm_runtime_get: %i\n", __func__, error); + + return; + } phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, am_phy->dr_mode, false); + + pm_runtime_mark_last_busy(phy->dev); + pm_runtime_put_autosuspend(phy->dev); + } static int am335x_phy_probe(struct platform_device *pdev) @@ -56,13 +81,23 @@ static int am335x_phy_probe(struct platform_device *pdev) am_phy->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node, -1); + pm_runtime_enable(dev); + pm_runtime_set_autosuspend_delay(dev, 100); + pm_runtime_use_autosuspend(dev); + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + dev_err(>dev, "%s pm_runtime_get: %i\n", __func__, ret); + + return ret; + } + ret = usb_phy_gen_create_phy(dev, _phy->usb_phy_gen, NULL); if (ret) - return ret; + goto disable; ret = usb_add_phy_dev(_phy->usb_phy_gen.phy); if (ret) - return ret; + goto disable; am_phy->usb_phy_gen.phy.init = am335x_init; am_phy->usb_phy_gen.phy.shutdown = am335x_shutdown; @@ -81,14 +116,34 @@ static int am335x_phy_probe(struct platform_device *pdev) device_set_wakeup_enable(dev, false); phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, am_phy->dr_mode, false); - return 0; + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + + return ret; + +disable: + pm_runtime_dont_use_autosuspend(dev); + pm_runtime_put_sync(dev); + pm_runtime_disable(dev); + + return ret; } static int am335x_phy_remove(struct platform_device *pdev) { struct am335x_phy *am_phy = platform_get_drvdata(pdev); + int error; + + error = pm_runtime_get_sync(>dev); + if (error < 0) + dev_err(>dev, "%s pm_runtime_get: %i\n", __func__, error); usb_remove_phy(_phy->usb_phy_gen.phy); + + pm_runtime_dont_use_autosuspend(>dev); + pm_runtime_put_sync(>dev); + pm_runtime_disable(>dev); + return 0; } -- 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: [PATCH v3 1/3] usb: musb: dma: Add a DMA completion platform callback
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); > + > spin_lock_irqsave(>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? Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: musb: Fix external abort on non-linefetch for musb_irq_work()
While testing musb host mode cable plugging on a BeagleBone, I came across this error: Unhandled fault: external abort on non-linefetch (0x1008) at 0xd1dcfc60 ... [] (musb_default_readb [musb_hdrc]) from [] (musb_irq_work+0x1c/0x180 [musb_hdrc]) [] (musb_irq_work [musb_hdrc]) from [] (process_one_work+0x2b4/0x808) [] (process_one_work) from [] (worker_thread+0x3c/0x550) [] (worker_thread) from [] (kthread+0x104/0x148) [] (kthread) from [] (ret_from_fork+0x14/0x24) Signed-off-by: Tony Lindgren--- I found one more corner case issue in my usb cable plugfest yesterday. drivers/usb/musb/musb_core.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1925,6 +1925,14 @@ static void musb_pm_runtime_check_session(struct musb *musb) static void musb_irq_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, irq_work.work); + int error; + + error = pm_runtime_get_sync(musb->controller); + if (error < 0) { + dev_err(musb->controller, "Could not enable: %i\n", error); + + return; + } musb_pm_runtime_check_session(musb); @@ -1932,6 +1940,9 @@ static void musb_irq_work(struct work_struct *data) musb->xceiv_old_state = musb->xceiv->otg->state; sysfs_notify(>controller->kobj, NULL, "mode"); } + + pm_runtime_mark_last_busy(musb->controller); + pm_runtime_put_autosuspend(musb->controller); } static void musb_recover_from_babble(struct musb *musb) -- 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: [PATCH 1/2] usb: musb: Fix host mode error -71 regression
On Wed, Jan 18, 2017 at 06:29:58PM -0800, Tony Lindgren wrote: > Commit 467d5c980709 ("usb: musb: Implement session bit based runtime PM for > musb-core") started implementing musb generic runtime PM support by > introducing devctl register session bit based state control. > > This caused a regression where if a USB mass storage device is connected > to a USB hub, we can get: > > usb 1-1: reset high-speed USB device number 2 using musb-hdrc > usb 1-1: device descriptor read/64, error -71 > usb 1-1.1: new high-speed USB device number 4 using musb-hdrc > > This is because before the USB storage device is connected, musb is > in OTG_STATE_A_SUSPEND. And we currently only set need_finish_resume > in musb_stage0_irq() and the related code calling finish_resume_work > in musb_resume() and musb_runtime_resume() never gets called. > > To fix the issue, we can call schedule_delayed_work() directly in > musb_stage0_irq() to have finish_resume_work run. > > And we should no longer never get interrupts when when suspended. > We have changed musb to no longer need pm_runtime_irqsafe(). > The need_finish_resume flag was added in commit 9298b4aad37e ("usb: > musb: fix device hotplug behind hub") and no longer applies as far > as I can tell. So let's just remove the earlier code that no longer > is needed. > > Fixes: 467d5c980709 ("usb: musb: Implement session bit based > runtime PM for musb-core") > Reported-by: Bin Liu> Signed-off-by: Tony Lindgren Applied. Thanks. -Bin. > --- > drivers/usb/musb/musb_core.c | 15 ++- > drivers/usb/musb/musb_core.h | 1 - > 2 files changed, 2 insertions(+), 14 deletions(-) > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > --- a/drivers/usb/musb/musb_core.c > +++ b/drivers/usb/musb/musb_core.c > @@ -594,11 +594,11 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, > u8 int_usb, > | MUSB_PORT_STAT_RESUME; > musb->rh_timer = jiffies > + msecs_to_jiffies(USB_RESUME_TIMEOUT); > - musb->need_finish_resume = 1; > - > musb->xceiv->otg->state = OTG_STATE_A_HOST; > musb->is_active = 1; > musb_host_resume_root_hub(musb); > + schedule_delayed_work(>finish_resume_work, > + msecs_to_jiffies(USB_RESUME_TIMEOUT)); > break; > case OTG_STATE_B_WAIT_ACON: > musb->xceiv->otg->state = > OTG_STATE_B_PERIPHERAL; > @@ -2710,11 +2710,6 @@ static int musb_resume(struct device *dev) > mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV; > if ((devctl & mask) != (musb->context.devctl & mask)) > musb->port1_status = 0; > - if (musb->need_finish_resume) { > - musb->need_finish_resume = 0; > - schedule_delayed_work(>finish_resume_work, > - msecs_to_jiffies(USB_RESUME_TIMEOUT)); > - } > > /* >* The USB HUB code expects the device to be in RPM_ACTIVE once it came > @@ -2766,12 +2761,6 @@ static int musb_runtime_resume(struct device *dev) > > musb_restore_context(musb); > > - if (musb->need_finish_resume) { > - musb->need_finish_resume = 0; > - schedule_delayed_work(>finish_resume_work, > - msecs_to_jiffies(USB_RESUME_TIMEOUT)); > - } > - > spin_lock_irqsave(>lock, flags); > error = musb_run_resume_work(musb); > if (error) > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > --- a/drivers/usb/musb/musb_core.h > +++ b/drivers/usb/musb/musb_core.h > @@ -410,7 +410,6 @@ struct musb { > > /* is_suspended means USB B_PERIPHERAL suspend */ > unsignedis_suspended:1; > - unsignedneed_finish_resume :1; > > /* may_wakeup means remote wakeup is enabled */ > unsignedmay_wakeup:1; > -- > 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
[PATCH v7 1/5] phy: Add support for Qualcomm's USB HSIC phy
The HSIC USB controller on qcom SoCs has an integrated all digital phy controlled via the ULPI viewport. Acked-by: Rob HerringCc: Signed-off-by: Stephen Boyd --- No changes. .../devicetree/bindings/phy/qcom,usb-hsic-phy.txt | 65 + drivers/phy/Kconfig| 7 + drivers/phy/Makefile | 1 + drivers/phy/phy-qcom-usb-hsic.c| 160 + 4 files changed, 233 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt create mode 100644 drivers/phy/phy-qcom-usb-hsic.c diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt b/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt new file mode 100644 index ..3c7cb2be4b12 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt @@ -0,0 +1,65 @@ +Qualcomm's USB HSIC PHY + +PROPERTIES + +- compatible: +Usage: required +Value type: +Definition: Should contain "qcom,usb-hsic-phy" and more specifically one of the + following: + + "qcom,usb-hsic-phy-mdm9615" + "qcom,usb-hsic-phy-msm8974" + +- #phy-cells: +Usage: required +Value type: +Definition: Should contain 0 + +- clocks: +Usage: required +Value type: +Definition: Should contain clock specifier for phy, calibration and +a calibration sleep clock + +- clock-names: +Usage: required +Value type: +Definition: Should contain "phy, "cal" and "cal_sleep" + +- pinctrl-names: +Usage: required +Value type: +Definition: Should contain "init" and "default" in that order + +- pinctrl-0: +Usage: required +Value type: +Definition: List of pinctrl settings to apply to keep HSIC pins in a glitch +free state + +- pinctrl-1: +Usage: required +Value type: +Definition: List of pinctrl settings to apply to mux out the HSIC pins + +EXAMPLE + +usb-controller { + ulpi { + phy { + compatible = "qcom,usb-hsic-phy-msm8974", +"qcom,usb-hsic-phy"; + #phy-cells = <0>; + pinctrl-names = "init", "default"; + pinctrl-0 = <_sleep>; + pinctrl-1 = <_default>; + clocks = < GCC_USB_HSIC_CLK>, +< GCC_USB_HSIC_IO_CAL_CLK>, +< GCC_USB_HSIC_IO_CAL_SLEEP_CLK>; + clock-names = "phy", "cal", "cal_sleep"; + assigned-clocks = < GCC_USB_HSIC_IO_CAL_CLK>; + assigned-clock-rates = <96>; + }; + }; +}; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index e8eb7f225a88..a430a64981d5 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -437,6 +437,13 @@ config PHY_QCOM_UFS help Support for UFS PHY on QCOM chipsets. +config PHY_QCOM_USB_HSIC + tristate "Qualcomm USB HSIC ULPI PHY module" + depends on USB_ULPI_BUS + select GENERIC_PHY + help + Support for the USB HSIC ULPI compliant PHY on QCOM chipsets. + config PHY_TUSB1210 tristate "TI TUSB1210 ULPI PHY module" depends on USB_ULPI_BUS diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 65eb2f436a41..c43c9df5d301 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -52,6 +52,7 @@ obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o +obj-$(CONFIG_PHY_QCOM_USB_HSIC)+= phy-qcom-usb-hsic.o obj-$(CONFIG_PHY_TUSB1210) += phy-tusb1210.o obj-$(CONFIG_PHY_BRCM_SATA)+= phy-brcm-sata.o obj-$(CONFIG_PHY_PISTACHIO_USB)+= phy-pistachio-usb.o diff --git a/drivers/phy/phy-qcom-usb-hsic.c b/drivers/phy/phy-qcom-usb-hsic.c new file mode 100644 index ..47690f9945b9 --- /dev/null +++ b/drivers/phy/phy-qcom-usb-hsic.c @@ -0,0 +1,160 @@ +/** + * Copyright (C) 2016 Linaro Ltd + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include +#include +#include +#include +#include +#include +#include + +#include "ulpi_phy.h" + +#define ULPI_HSIC_CFG 0x30 +#define ULPI_HSIC_IO_CAL 0x33 + +struct qcom_usb_hsic_phy { + struct ulpi *ulpi; + struct phy *phy; + struct pinctrl *pctl; + struct clk *phy_clk; + struct clk *cal_clk; + struct clk *cal_sleep_clk; +}; + +static int qcom_usb_hsic_phy_power_on(struct phy *phy) +{ +
[PATCH v7 0/5] Support qcom's HSIC USB and rewrite USB2 HS support
This patch series continues the usb chipidea rewrite for Qualcommm platforms. I've dropped the patches that were applied to Peter's tree for chipidea. Now the phy drivers are left, along with a new hook to set the vbus in the phy and changes to chipidea to call the new hook. I've left the HSIC phy driver here, because it wasn't merged in the last round. Nothing has changed in that driver, so I believe it is ready to be merged now. The real changes are the new set_vbus() hook and how we're supposed to handle that in the HS phy driver. The only dependencies between subsystems is the set_vbus callback. If that can go into some stable branch in the phy tree, then Kishon can apply the phy patches on top of that and Peter can apply the small chipidea patch on top of that. Or everything can go in parallel, except for the chipidea patch that adds the set_vbus hook. That hook can be added after rc1 when dependencies merge. Patches based on v4.10-rc1 + first 22 patches from v5. Full branch is here[1]. Changes from v6: * Dropped first 22 applied patches * Rewrote phy_set_mode() patch to be msm specific * New set_vbus() callback in phy framework * Updated HS phy driver for set_vbus() callback Changes from v5: * Replaced "Emulate OTGSC interrupt enable path" patch with a patch from Peter * Updated HS phy driver to support set_mode callback to handle pullup * New patch to set the mode to device or host in chipidea udc pullup function to toggle the pullup for HS mode * New patch to drop lock around event_notify callback to avoid lockdep issues * Removal of extcon usage from HS phy driver * Picked up acks from Heikki and Peter on ULPI core patch Changes from v4: * Picked up Acks from Rob * Updated HS phy init sequence DT property to restrict it to offsets Changes from v3: * Picked up Acks from Peter * Updated extcon consolidation patch per Peter's comments * Folded in simplification from Heikki for ULPI DT matching Changes from v2: * Added SoC specific compatibles in phy bindings * Dropped AVVIS patch for OTG statemachine * New patch to consolidate extcon handlers * Picked up Acks from Peter * Rebased onto v4.8-rc1 * Reworked ULPI OF code to look at vid == 0 instead of pid == 0 * Dropped ULPI bindings for vid and pid overrides Changes from v1: * Reworked ULPI device probing to keep using vendor/product ids that come from DT if needed and falls back to OF style match when product id is 0 * PHY init later patch was rejected so that moved to a quirk flag and the msm wrapper started managing the phy on/off * Updated clk requirements for HSIC phy in binding doc * Added optional clk in wrapper for "housekeeping" found on older qcom platforms * Bug fix to OTGSC polling function * Changed runtime PM patch to set as active instead of get/put TODO: * DMA fails on arm64 so we need something like [2] to make it work. * The db410c needs a driver to toggle the onboard switch to connect the usb hub instead of micro port when the usb cable is disconnected. I've sent a patch set for this[3], which needs some further discussion/development. The current plan is to reintroduce the usb mux framework. * apq8064 platforms need a vbus regulator to really use otg and I haven't tried out the RPM based regulators yet * The HSIC phy on the apq8074 dragonboard is connected to a usb4604 device which requires the i2c driver to probe and send an i2c sequence before the HSIC controller enumerates or HSIC doesn't work. Right now I have a hack to force the controller to probe defer once so that usb4604 probes first. This needs a more proper solution like having the DT describe a linkage between the controller and the usb device so we can enforce probe ordering. My current plan is to use DT graphs/endpoints for this. [1] https://git.linaro.org/people/stephen.boyd/linux.git/log/?h=usb-hsic-8074 [2] https://patchwork.kernel.org/patch/9319527/ [3] https://lkml.kernel.org/r/20160914014246.31847-1-stephen.b...@linaro.org Stephen Boyd (5): phy: Add support for Qualcomm's USB HSIC phy usb: chipidea: msm: Configure phy for appropriate mode phy: Add set_vbus callback usb: chipidea: Signal vbus state to phy phy: Add support for Qualcomm's USB HS phy .../devicetree/bindings/phy/qcom,usb-hs-phy.txt| 78 +++ .../devicetree/bindings/phy/qcom,usb-hsic-phy.txt | 65 ++ drivers/phy/Kconfig| 15 ++ drivers/phy/Makefile | 2 + drivers/phy/phy-core.c | 15 ++ drivers/phy/phy-qcom-usb-hs.c | 256 + drivers/phy/phy-qcom-usb-hsic.c| 160 + drivers/usb/chipidea/ci.h | 7 +- drivers/usb/chipidea/ci_hdrc_msm.c | 4 + drivers/usb/chipidea/otg.c | 7 +- include/linux/phy/phy.h| 10 + 11 files
[PATCH v7 2/5] usb: chipidea: msm: Configure phy for appropriate mode
When the qcom chipidea controller is used with an extcon, we need to signal device mode or host mode to the phy so it can configure itself for the correct mode. This should be done after the phy is powered up, so that the register writes work correctly. Add in the appropriate phy_set_mode() call here. To signal the correct state to the qcom glue driver we need to change the ci->role before we do the role switch. Make sure to undo the change if the role switch fails, but otherwise update the state before calling the role start function so that the glue driver knows what state to configure for. Cc: Greg Kroah-HartmanSigned-off-by: Stephen Boyd --- Made this msm specific because of how msm handles phy powerup. drivers/usb/chipidea/ci.h | 7 +-- drivers/usb/chipidea/ci_hdrc_msm.c | 4 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index 59e22389c10b..18348b0529af 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -268,6 +268,7 @@ static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci) static inline int ci_role_start(struct ci_hdrc *ci, enum ci_role role) { int ret; + enum ci_role prev_role; if (role >= CI_ROLE_END) return -EINVAL; @@ -275,9 +276,11 @@ static inline int ci_role_start(struct ci_hdrc *ci, enum ci_role role) if (!ci->roles[role]) return -ENXIO; + prev_role = ci->role; + ci->role = role; ret = ci->roles[role]->start(ci); - if (!ret) - ci->role = role; + if (ret) + ci->role = prev_role; return ret; } diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index f1ede7909f54..9c58d13970ca 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -125,6 +125,10 @@ static int ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event) hw_write(ci, OP_USBCMD, HSPHY_SESS_VLD_CTRL, HSPHY_SESS_VLD_CTRL); + if (ci->role == CI_ROLE_GADGET) + phy_set_mode(ci->phy, PHY_MODE_USB_DEVICE); + else if (ci->role == CI_ROLE_HOST) + phy_set_mode(ci->phy, PHY_MODE_USB_HOST); } break; case CI_HDRC_CONTROLLER_STOPPED_EVENT: -- 2.10.0.297.gf6727b0 -- 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 v7 4/5] usb: chipidea: Signal vbus state to phy
Some USB PHYs need to be told about vbus changing state explicitly. For example the qcom USB HS PHY needs to toggle a bit when vbus goes from low to high (VBUSVLDEXT) to cause the "session valid" signal to toggle. This signal will pull up D+ when the phy starts running. Add the appropriate phy_set_vbus() call here to signal vbus state changes to the phy. Cc: Greg Kroah-HartmanSigned-off-by: Stephen Boyd --- New patch drivers/usb/chipidea/otg.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index 10236fe71522..6ea702beed48 100644 --- a/drivers/usb/chipidea/otg.c +++ b/drivers/usb/chipidea/otg.c @@ -134,10 +134,13 @@ void ci_handle_vbus_change(struct ci_hdrc *ci) if (!ci->is_otg) return; - if (hw_read_otgsc(ci, OTGSC_BSV) && !ci->vbus_active) + if (hw_read_otgsc(ci, OTGSC_BSV) && !ci->vbus_active) { usb_gadget_vbus_connect(>gadget); - else if (!hw_read_otgsc(ci, OTGSC_BSV) && ci->vbus_active) + phy_set_vbus(ci->phy, 1); + } else if (!hw_read_otgsc(ci, OTGSC_BSV) && ci->vbus_active) { + phy_set_vbus(ci->phy, 0); usb_gadget_vbus_disconnect(>gadget); + } } /** -- 2.10.0.297.gf6727b0 -- 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 v7 5/5] phy: Add support for Qualcomm's USB HS phy
The high-speed phy on qcom SoCs is controlled via the ULPI viewport. Cc:Acked-by: Rob Herring Signed-off-by: Stephen Boyd --- phy_set_mode() hook split up to toggle two bits independently with new set_vbus() hook. .../devicetree/bindings/phy/qcom,usb-hs-phy.txt| 78 +++ drivers/phy/Kconfig| 8 + drivers/phy/Makefile | 1 + drivers/phy/phy-qcom-usb-hs.c | 256 + 4 files changed, 343 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt create mode 100644 drivers/phy/phy-qcom-usb-hs.c diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt new file mode 100644 index ..bec77a74bd39 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt @@ -0,0 +1,78 @@ +Qualcomm's USB HS PHY + +PROPERTIES + +- compatible: +Usage: required +Value type: +Definition: Should contain "qcom,usb-hs-phy" and more specifically one of the +following: + +"qcom,usb-hs-phy-apq8064" +"qcom,usb-hs-phy-msm8916" +"qcom,usb-hs-phy-msm8974" + +- #phy-cells: +Usage: required +Value type: +Definition: Should contain 0 + +- clocks: +Usage: required +Value type: +Definition: Should contain clock specifier for the reference and sleep +clocks + +- clock-names: +Usage: required +Value type: +Definition: Should contain "ref" and "sleep" for the reference and sleep +clocks respectively + +- resets: +Usage: required +Value type: +Definition: Should contain the phy and POR resets + +- reset-names: +Usage: required +Value type: +Definition: Should contain "phy" and "por" for the phy and POR resets +respectively + +- v3p3-supply: +Usage: required +Value type: +Definition: Should contain a reference to the 3.3V supply + +- v1p8-supply: +Usage: required +Value type: +Definition: Should contain a reference to the 1.8V supply + +- qcom,init-seq: +Usage: optional +Value type: +Definition: Should contain a sequence of ULPI address and value pairs to +program into the ULPI_EXT_VENDOR_SPECIFIC area. This is related +to Device Mode Eye Diagram test. The addresses are offsets + from the ULPI_EXT_VENDOR_SPECIFIC address, for example, + <0x1 0x53> would mean "write the value 0x53 to address 0x81". + +EXAMPLE + +otg: usb-controller { + ulpi { + phy { + compatible = "qcom,usb-hs-phy-msm8974", "qcom,usb-hs-phy"; + #phy-cells = <0>; + clocks = <_board>, < GCC_USB2A_PHY_SLEEP_CLK>; + clock-names = "ref", "sleep"; + resets = < GCC_USB2A_PHY_BCR>, < 0>; + reset-names = "phy", "por"; + v3p3-supply = <_l24>; + v1p8-supply = <_l6>; + qcom,init-seq = /bits/ 8 <0x1 0x63>; + }; + }; +}; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index a430a64981d5..61a22e985831 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -437,6 +437,14 @@ config PHY_QCOM_UFS help Support for UFS PHY on QCOM chipsets. +config PHY_QCOM_USB_HS + tristate "Qualcomm USB HS PHY module" + depends on USB_ULPI_BUS + select GENERIC_PHY + help + Support for the USB high-speed ULPI compliant phy on Qualcomm + chipsets. + config PHY_QCOM_USB_HSIC tristate "Qualcomm USB HSIC ULPI PHY module" depends on USB_ULPI_BUS diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index c43c9df5d301..0e4259473d28 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -52,6 +52,7 @@ obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o +obj-$(CONFIG_PHY_QCOM_USB_HS) += phy-qcom-usb-hs.o obj-$(CONFIG_PHY_QCOM_USB_HSIC)+= phy-qcom-usb-hsic.o obj-$(CONFIG_PHY_TUSB1210) += phy-tusb1210.o obj-$(CONFIG_PHY_BRCM_SATA)+= phy-brcm-sata.o diff --git a/drivers/phy/phy-qcom-usb-hs.c b/drivers/phy/phy-qcom-usb-hs.c new file mode 100644 index ..50cb40977737 --- /dev/null +++ b/drivers/phy/phy-qcom-usb-hs.c @@ -0,0 +1,256 @@ +/** + * Copyright (C) 2016 Linaro Ltd + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free
[PATCH v7 3/5] phy: Add set_vbus callback
Some USB PHYs need to be told about vbus changing state explicitly. For example the qcom USB HS PHY needs to toggle a bit when vbus goes from low to high (VBUSVLDEXT) to cause the "session valid" signal to toggle. This signal will pull up D+ when the phy starts running. If the vbus signal isn't routed to the PHY this "session valid" signal won't ever toggle, so we have to toggle it explicitly. This callback is used to do that. Cc: Peter ChenSigned-off-by: Stephen Boyd --- New patch drivers/phy/phy-core.c | 15 +++ include/linux/phy/phy.h | 10 ++ 2 files changed, 25 insertions(+) diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index a268f4d6f3e9..8b1a6bfa5133 100644 --- a/drivers/phy/phy-core.c +++ b/drivers/phy/phy-core.c @@ -357,6 +357,21 @@ int phy_set_mode(struct phy *phy, enum phy_mode mode) } EXPORT_SYMBOL_GPL(phy_set_mode); +int phy_set_vbus(struct phy *phy, int on) +{ + int ret; + + if (!phy || !phy->ops->set_vbus) + return 0; + + mutex_lock(>mutex); + ret = phy->ops->set_vbus(phy, on); + mutex_unlock(>mutex); + + return ret; +} +EXPORT_SYMBOL_GPL(phy_set_vbus); + int phy_reset(struct phy *phy) { int ret; diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index 78bb0d7f6b11..4d1ebde7fb14 100644 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -36,6 +36,7 @@ enum phy_mode { * @power_on: powering on the phy * @power_off: powering off the phy * @set_mode: set the mode of the phy + * @set_vbus: enable/disable vbus in the phy (USB) * @reset: resetting the phy * @owner: the module owner containing the ops */ @@ -45,6 +46,7 @@ struct phy_ops { int (*power_on)(struct phy *phy); int (*power_off)(struct phy *phy); int (*set_mode)(struct phy *phy, enum phy_mode mode); + int (*set_vbus)(struct phy *phy, int on); int (*reset)(struct phy *phy); struct module *owner; }; @@ -138,6 +140,7 @@ int phy_exit(struct phy *phy); int phy_power_on(struct phy *phy); int phy_power_off(struct phy *phy); int phy_set_mode(struct phy *phy, enum phy_mode mode); +int phy_set_vbus(struct phy *phy, int on); int phy_reset(struct phy *phy); static inline int phy_get_bus_width(struct phy *phy) { @@ -253,6 +256,13 @@ static inline int phy_set_mode(struct phy *phy, enum phy_mode mode) return -ENOSYS; } +static inline int phy_set_vbus(struct phy *phy, int on) +{ + if (!phy) + return 0; + return -ENOSYS; +} + static inline int phy_reset(struct phy *phy) { if (!phy) -- 2.10.0.297.gf6727b0 -- 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 4/5] usb: musb: musb_cppi41: Workaround DMA stall issue during teardown
* Bin Liu[170120 09:24]: > On Thu, Jan 19, 2017 at 08:56:46AM -0800, Tony Lindgren wrote: > > * Alexandre Bailon [170119 06:09]: > > > The DMA may hung up if a teardown is initiated while an endpoint is still > > > active (Advisory 2.3.27 of DA8xx errata). > > > To workaround this issue, add a delay before to initiate the teardown. > > > > > > Signed-off-by: Alexandre Bailon > > > --- > > > drivers/usb/musb/da8xx.c | 2 +- > > > drivers/usb/musb/musb_core.h | 1 + > > > drivers/usb/musb/musb_cppi41.c | 4 > > > 3 files changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c > > > index 5f50a78..8c9850c 100644 > > > --- a/drivers/usb/musb/da8xx.c > > > +++ b/drivers/usb/musb/da8xx.c > > > @@ -483,7 +483,7 @@ da8xx_dma_controller_create(struct musb *musb, void > > > __iomem *base) > > > #endif > > > > > > static const struct musb_platform_ops da8xx_ops = { > > > - .quirks = MUSB_INDEXED_EP | MUSB_DMA_CPPI41, > > > + .quirks = MUSB_INDEXED_EP | MUSB_DMA_CPPI41 | MUSB_DA8XX, > > > .init = da8xx_musb_init, > > > .exit = da8xx_musb_exit, > > > > > > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > > > index ade902e..d129278 100644 > > > --- a/drivers/usb/musb/musb_core.h > > > +++ b/drivers/usb/musb/musb_core.h > > > @@ -172,6 +172,7 @@ struct musb_io; > > > */ > > > struct musb_platform_ops { > > > > > > +#define MUSB_DA8XX BIT(7) > > > #define MUSB_DMA_UX500 BIT(6) > > > #define MUSB_DMA_CPPI41 BIT(5) > > > #define MUSB_DMA_CPPIBIT(4) > > > diff --git a/drivers/usb/musb/musb_cppi41.c > > > b/drivers/usb/musb/musb_cppi41.c > > > index 1fe7eae..d371d05 100644 > > > --- a/drivers/usb/musb/musb_cppi41.c > > > +++ b/drivers/usb/musb/musb_cppi41.c > > > @@ -554,6 +554,10 @@ static int cppi41_dma_channel_abort(struct > > > dma_channel *channel) > > > } > > > } > > > > > > + /* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */ > > > + if (musb->io.quirks & MUSB_DA8XX) > > > + mdelay(250); > > > + > > > tdbit = 1 << cppi41_channel->port_num; > > > if (is_tx) > > > tdbit <<= 16; > > > > How about replace the do while loop calling dmaengine_terminate_all() with > > deferred work doing that? > > It would be great, but it seems a little hard to implement. If the > _abort is initiated by application due to transfer timeout, the > application might immediately retry the transfer, then the driver would > have to ensure the deferred work is done and the channel is free by that > time. > > I guess we have to evaluate the impact of the do-while loop and the > performance impact of the deferred work to know if it is worth it. > > > > > That way it's more generic as it seems there's no hurry doing that > > after the musb registers are cleared. That would also allow getting rid of > > the two current udelay() calls there that also seems nasty. > > I don't think replacing the do-while loop will allow to get rid of the > two udelay() calls, which are there as barriers for the register changes > - clearing MUSB_RXCSR_DMAENAB and flushing FIFO. OK fine with me. Tony -- 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
Mathias Nymanwrites: > Remove duplicate code by using trb_to_noop() when > handling Aborted commads > > Signed-off-by: Mathias Nyman https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com ? -- balbi -- 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
Mathias Nymanwrites: > 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 -- balbi -- 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 25/37] xhci: Introduce helper to turn one TRB into a no-op
Mathias Nymanwrites: > Useful for turning both transfer and command trbs > into no-ops > > Signed-off-by: Mathias Nyman isn't this just [1] [1] https://marc.info/?i=20161229110109.26372-24-felipe.ba...@linux.intel.com -- balbi -- 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: unable to handle EVENT RING FULL error in xhci
Hi Mathias, > -Original Message- > From: Mathias Nyman [mailto:mathias.ny...@linux.intel.com] > Sent: Friday, January 20, 2017 10:38 PM > To: gre...@linuxfoundation.org; Anurag Kumar Vulisha >> Cc: linux-usb@vger.kernel.org; mathias.ny...@intel.com; Punnaiah > Choudary Kalluri > Subject: Re: unable to handle EVENT RING FULL error in xhci > > On 20.01.2017 10:41, gre...@linuxfoundation.org wrote: > > On Fri, Jan 20, 2017 at 07:59:48AM +, Anurag Kumar Vulisha wrote: > >> Hi, > > > > Hi! > > > > Minor nit, vger.kernel.org rejects html emails, so this didn't make it > > through to that list. Now it should with my response :) > > > >> I have a usecase where I want to stream a stereo video from the ZED > Stereo > >> Camera (www.stereolabs.com) by connecting it to our ZynqMP SOC > based platform > >> using the USB 3.0 interface. This camera keeps on streaming 48k chunks of > BULK > >> data to the HOST. > > Any idea what size chunks the driver is asking for? if URB asks for > 48k then > we would have a lot of short packet events on the event ring. > Driver is asking for 48K and the Synopsys dwc3 controller is supporting only Burst length of 16. So, on lecroy trace we see > 3 burst transactions for 48K Chunk. Randomly we are observing stale data from (based on the uvc header info) the camera and during that time we see that the ACK from the host is delayed by >4 milli seconds for one of the data packet with in a burst. We are unable to figure out why this much delay. Any pointers or help to probe further ? > Initially the data is received correctly, but after some time > >> I noticed frame corruption on the BULK packets received by HOST > controller. On > >> debugging I found that HOST event ring is full and it issues “EVENT RING > FULL > >> ERROR” and this error seems to be not handled in XHCI. I am using linux > 4.6 > >> kernel for testing . Please help me in by giving any suggestions on how to > >> handle this issue? Currently I increased the TRB_PER_SEGMENT from 256 > to 512 > >> but the issue seems to delayed and gets reproduced after some more > time. > > We are currently not handling EVEN_RING_FULL_ERRORS at all, that should > be fixed. > > 256 is a nice number because with 16byte TRBs we end up with ring > segments of size > 4096 Bytes. Correct but the URB processing is happening in interrupt context and since the uvc Layer is copying the 48k chunk of data to the upper layer buffer using memcpy. So, we changed the URB processing context to bottom half and now we didn’t Observe any EVENT_RING_FULL message but our original issue of receiving the Corrupted frames from the camera still there. > > xhci driver is only using one event ring, the primary one, for all transfer, > command completion and port status events. > If I remember correctly that event ring only has one segment. > > >> > >> To increase the TRB_PER_SEGMENT from 256 to 512 I made the below > changes > >> > >> xhci->segment_pool = dma_pool_create("xHCI ring segments", dev, > >> TRB_SEGMENT_SIZE, TRB_SEGMENT_SIZE, xhci->page_size * 4); > >> > >> Is this the correct way ? or I am doing wrong. > > Probably best to check what the event ring is full of first, maybe dumping it. > > It could be just about the ring size, or then it could be filled with > unnecessary > events (short transfers), or then we are handling some other slow event that > with interrupts disabled. > Or then we just don't generate interrupts, or they are disabled, preventing > us from > handling events on the ring. > > A 2k USB 3 stereo camera seems like a devie that generates a lot of traffic > and > events. > > >> > >> It would be very helpful if you can help me in solving this issue > > > > 4.6 is very old and obsolete and behind in a lot of xhci fixes. Can you > > test 4.9.4 or even better yet, Linus's 4.10-rc4 tree to see if the issue > > is still there? > > > > A more recent kernel would be preferred, dumping the event ring could be > done Ok. > using something like this: > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index 5e51109..3e6a816 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -2575,6 +2575,9 @@ static int xhci_handle_event(struct xhci_hcd *xhci) > if (ret >= 0) > update_ptrs = 0; > break; > + case TRB_TYPE(TRB_HC_EVENT): > + xhci_debug_segment(xhci, xhci->event_ring->deq_seg); > + break; > case TRB_TYPE(TRB_DEV_NOTE): > handle_device_notification(xhci, event); > break; > > And enable xhci debugging, but only for xhci_debug_segment() function > Ok Thanks. Regards, Punnaiah > -Mathias
Re: [PATCHv4 0/2] cppi41 dma fixes for v4.10-rc cycle
On Fri, Jan 20, 2017 at 10:50:03PM +0530, Vinod Koul wrote: > On Thu, Jan 19, 2017 at 08:49:06AM -0800, Tony Lindgren wrote: > > Hi all, > > > > I'm using v4 naming here as the earlier patch "dmaengine: cppi41: Add dma > > support to da8xx" has been separated from the error -115 issue. We've > > identified so far three musb and cppi41 regressions: > > Hi, > > Can we get some tested-by for this series? Tested-by: Bin Liu, or Acked-by: Bin Liu , whichever you prefer ;) Regards, -Bin. > > > > > 1. Error -71 regression with musb > > > >This is not dma related, and fixed with recently posted patch > >"[PATCH 1/2] usb: musb: Fix host mode error -71 regression". > > > > 2. Error -115 regression with cppi41 dma with musb > > > >This regression was caused by commit 098de42ad670 ("dmaengine: > >cppi41: Fix unpaired pm runtime when only a USB hub is connected") > >and causes runtime PM autosuspend delay to time out on active dma > >transfers when connecting a mass storage device to a usb hub. > >This is fixed in the first patch in this series. > > > > 3. Race with runtime PM and cppi41_dma_issue_pending() > > > >This is really a separate issue from the error -115 problem, and > >fixed with the second patch in this series. That's minimal v4 > >version of the "dmaengine: cppi41: Fix oops in cppi41_runtime_resume" > >patch. > > > > Regards, > > > > Tony > > > > > > Tony Lindgren (2): > > dmaengine: cppi41: Fix runtime PM timeouts with USB mass storage > > dmaengine: cppi41: Fix oops in cppi41_runtime_resume > > > > drivers/dma/cppi41.c | 56 > > ++-- > > 1 file changed, 41 insertions(+), 15 deletions(-) > > > > -- > > 2.11.0 > > -- > ~Vinod -- 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 4/5] usb: musb: musb_cppi41: Workaround DMA stall issue during teardown
On Thu, Jan 19, 2017 at 08:56:46AM -0800, Tony Lindgren wrote: > * Alexandre Bailon[170119 06:09]: > > The DMA may hung up if a teardown is initiated while an endpoint is still > > active (Advisory 2.3.27 of DA8xx errata). > > To workaround this issue, add a delay before to initiate the teardown. > > > > Signed-off-by: Alexandre Bailon > > --- > > drivers/usb/musb/da8xx.c | 2 +- > > drivers/usb/musb/musb_core.h | 1 + > > drivers/usb/musb/musb_cppi41.c | 4 > > 3 files changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c > > index 5f50a78..8c9850c 100644 > > --- a/drivers/usb/musb/da8xx.c > > +++ b/drivers/usb/musb/da8xx.c > > @@ -483,7 +483,7 @@ da8xx_dma_controller_create(struct musb *musb, void > > __iomem *base) > > #endif > > > > static const struct musb_platform_ops da8xx_ops = { > > - .quirks = MUSB_INDEXED_EP | MUSB_DMA_CPPI41, > > + .quirks = MUSB_INDEXED_EP | MUSB_DMA_CPPI41 | MUSB_DA8XX, > > .init = da8xx_musb_init, > > .exit = da8xx_musb_exit, > > > > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > > index ade902e..d129278 100644 > > --- a/drivers/usb/musb/musb_core.h > > +++ b/drivers/usb/musb/musb_core.h > > @@ -172,6 +172,7 @@ struct musb_io; > > */ > > struct musb_platform_ops { > > > > +#define MUSB_DA8XX BIT(7) > > #define MUSB_DMA_UX500 BIT(6) > > #define MUSB_DMA_CPPI41BIT(5) > > #define MUSB_DMA_CPPI BIT(4) > > diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c > > index 1fe7eae..d371d05 100644 > > --- a/drivers/usb/musb/musb_cppi41.c > > +++ b/drivers/usb/musb/musb_cppi41.c > > @@ -554,6 +554,10 @@ static int cppi41_dma_channel_abort(struct dma_channel > > *channel) > > } > > } > > > > + /* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */ > > + if (musb->io.quirks & MUSB_DA8XX) > > + mdelay(250); > > + > > tdbit = 1 << cppi41_channel->port_num; > > if (is_tx) > > tdbit <<= 16; > > How about replace the do while loop calling dmaengine_terminate_all() with > deferred work doing that? It would be great, but it seems a little hard to implement. If the _abort is initiated by application due to transfer timeout, the application might immediately retry the transfer, then the driver would have to ensure the deferred work is done and the channel is free by that time. I guess we have to evaluate the impact of the do-while loop and the performance impact of the deferred work to know if it is worth it. > > That way it's more generic as it seems there's no hurry doing that > after the musb registers are cleared. That would also allow getting rid of > the two current udelay() calls there that also seems nasty. I don't think replacing the do-while loop will allow to get rid of the two udelay() calls, which are there as barriers for the register changes - clearing MUSB_RXCSR_DMAENAB and flushing FIFO. Regards, -Bin. -- 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: [PATCHv4 0/2] cppi41 dma fixes for v4.10-rc cycle
On Thu, Jan 19, 2017 at 08:49:06AM -0800, Tony Lindgren wrote: > Hi all, > > I'm using v4 naming here as the earlier patch "dmaengine: cppi41: Add dma > support to da8xx" has been separated from the error -115 issue. We've > identified so far three musb and cppi41 regressions: Hi, Can we get some tested-by for this series? > > 1. Error -71 regression with musb > >This is not dma related, and fixed with recently posted patch >"[PATCH 1/2] usb: musb: Fix host mode error -71 regression". > > 2. Error -115 regression with cppi41 dma with musb > >This regression was caused by commit 098de42ad670 ("dmaengine: >cppi41: Fix unpaired pm runtime when only a USB hub is connected") >and causes runtime PM autosuspend delay to time out on active dma >transfers when connecting a mass storage device to a usb hub. >This is fixed in the first patch in this series. > > 3. Race with runtime PM and cppi41_dma_issue_pending() > >This is really a separate issue from the error -115 problem, and >fixed with the second patch in this series. That's minimal v4 >version of the "dmaengine: cppi41: Fix oops in cppi41_runtime_resume" >patch. > > Regards, > > Tony > > > Tony Lindgren (2): > dmaengine: cppi41: Fix runtime PM timeouts with USB mass storage > dmaengine: cppi41: Fix oops in cppi41_runtime_resume > > drivers/dma/cppi41.c | 56 > ++-- > 1 file changed, 41 insertions(+), 15 deletions(-) > > -- > 2.11.0 -- ~Vinod -- 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: unable to handle EVENT RING FULL error in xhci
On 20.01.2017 10:41, gre...@linuxfoundation.org wrote: On Fri, Jan 20, 2017 at 07:59:48AM +, Anurag Kumar Vulisha wrote: Hi, Hi! Minor nit, vger.kernel.org rejects html emails, so this didn't make it through to that list. Now it should with my response :) I have a usecase where I want to stream a stereo video from the ZED Stereo Camera (www.stereolabs.com) by connecting it to our ZynqMP SOC based platform using the USB 3.0 interface. This camera keeps on streaming 48k chunks of BULK data to the HOST. Any idea what size chunks the driver is asking for? if URB asks for > 48k then we would have a lot of short packet events on the event ring. Initially the data is received correctly, but after some time I noticed frame corruption on the BULK packets received by HOST controller. On debugging I found that HOST event ring is full and it issues “EVENT RING FULL ERROR” and this error seems to be not handled in XHCI. I am using linux 4.6 kernel for testing . Please help me in by giving any suggestions on how to handle this issue? Currently I increased the TRB_PER_SEGMENT from 256 to 512 but the issue seems to delayed and gets reproduced after some more time. We are currently not handling EVEN_RING_FULL_ERRORS at all, that should be fixed. 256 is a nice number because with 16byte TRBs we end up with ring segments of size 4096 Bytes. xhci driver is only using one event ring, the primary one, for all transfer, command completion and port status events. If I remember correctly that event ring only has one segment. To increase the TRB_PER_SEGMENT from 256 to 512 I made the below changes xhci->segment_pool = dma_pool_create("xHCI ring segments", dev, TRB_SEGMENT_SIZE, TRB_SEGMENT_SIZE, xhci->page_size * 4); Is this the correct way ? or I am doing wrong. Probably best to check what the event ring is full of first, maybe dumping it. It could be just about the ring size, or then it could be filled with unnecessary events (short transfers), or then we are handling some other slow event that with interrupts disabled. Or then we just don't generate interrupts, or they are disabled, preventing us from handling events on the ring. A 2k USB 3 stereo camera seems like a devie that generates a lot of traffic and events. It would be very helpful if you can help me in solving this issue 4.6 is very old and obsolete and behind in a lot of xhci fixes. Can you test 4.9.4 or even better yet, Linus's 4.10-rc4 tree to see if the issue is still there? A more recent kernel would be preferred, dumping the event ring could be done using something like this: diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 5e51109..3e6a816 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2575,6 +2575,9 @@ static int xhci_handle_event(struct xhci_hcd *xhci) if (ret >= 0) update_ptrs = 0; break; + case TRB_TYPE(TRB_HC_EVENT): + xhci_debug_segment(xhci, xhci->event_ring->deq_seg); + break; case TRB_TYPE(TRB_DEV_NOTE): handle_device_notification(xhci, event); break; And enable xhci debugging, but only for xhci_debug_segment() function -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 25/37] xhci: Introduce helper to turn one TRB into a no-op
On 01/20/2017 05:47 PM, Mathias Nyman wrote: Useful for turning both transfer and command trbs into no-ops Signed-off-by: Mathias Nyman--- drivers/usb/host/xhci-ring.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 8b78eee..699fc2d 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c [...] @@ -290,6 +305,9 @@ static struct xhci_command *xhci_next_queued_cmd(struct xhci_hcd *xhci) cmd_list); } + + + ??? /* * Turn all commands on command ring with status set to "aborted" to no-op trbs. * If there are other commands waiting then restart the ring and kick the timer. [...] 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: [PATCH 32/37] usb: host: xhci: remove newline from tracer
Hello! On 01/20/2017 05:47 PM, Mathias Nyman wrote: From: Felipe BalbiIf we add that newline, the output will like like the following: Look like. :-) 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 [...] 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: [PATCH net] r8152: fix rtl8152_post_reset function
From: Hayes WangDate: Fri, 20 Jan 2017 14:33:55 +0800 > The rtl8152_post_reset() should sumbit rx urb and interrupt transfer, > otherwise the rx wouldn't work and the linking change couldn't be > detected. > > Signed-off-by: Hayes Wang Applied, thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XHCI regression on v4.10-rc1
On 20.01.2017 16:49, Patrice CHOTARD wrote: On 01/13/2017 05:12 PM, Patrice Chotard wrote: On 01/12/2017 06:17 PM, Mathias Nyman wrote: On 09.01.2017 18:42, Mathias Nyman wrote: On 09.01.2017 16:23, Patrice Chotard wrote: On 01/09/2017 01:30 PM, Mathias Nyman wrote: On 09.01.2017 11:51, Patrice Chotard wrote: Hi Mathias, Greg Hi I am working on ARM STi platform, since v4.10-rc1, when booting B2260 or B2120 STi boards platform with nothing plugged on USB3 connector, i observed the following kernel logs : [ 801.953836] usb usb6-port1: Cannot enable. Maybe the USB cable is bad? That's odd, why does it try to reset the port if there's nothing there? [ 801.960455] xhci-hcd xhci-hcd.0.auto: Cannot set link state. This makes sense, nothing is connected, and we try to set link to U3 (PORT_PE == 0) [ 801.966611] usb usb6-port1: cannot disable (err = -32) xhci reutns -EPIPE as we try to set link state to U3 while port is not enabled (PORT_PE == 0) [ 806.083772] usb usb6-port1: Cannot enable. Maybe the USB cable is bad? Again, about 5 seconds later the same port is reset, while nothing is connected. Odd [ 806.090370] xhci-hcd xhci-hcd.0.auto: Cannot set link state. [ 806.096494] usb usb6-port1: cannot disable (err = -32) [ 810.208766] usb usb6-port1: Cannot enable. Maybe the USB cable is bad? [ 810.215374] xhci-hcd xhci-hcd.0.auto: Cannot set link state. [ 810.221478] usb usb6-port1: cannot disable (err = -32) [ 814.333767] usb usb6-port1: Cannot enable. Maybe the USB cable is bad? [ 814.340364] xhci-hcd xhci-hcd.0.auto: Cannot set link state. . Another interesting thing is even if i plugged a device (a mass storage device in my case) on the USB3 connector, the above logs continue to appear. This is due to commit 37be66767e3ca "usb: hub: Fix auto-remount of safely removed or ejected USB-3 devices". I don't know if STi platforms are the only impacted by this issue. On v4.9 everything was ok. Was there not a single "Cannot enable. Maybe USB cable is bad" message with 4.9? There is no "Cannot enable. Maybe USB cable is bad" message with 4.9 Before commit 37be66767e3ca we forced the port to RxDetect state via Disabled state, and cleared port change flags when a usb3_port_disable() was called. after we just set the port to U3. Maybe those STi platforms depend on that cycle somehow? I really don't know :-( Could you take logs (dmesg) of both 4.9 and 4.10-rc1 with both usb core and xhci debugging enabled: echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control You will find below the 2 requested logs, in both case , no USB cable are plugged. v4.9 dmesg logs : . [ 31.368022] hub 6-0:1.0: hub_resume [ 31.368064] xhci-hcd xhci-hcd.0.auto: get port status, actual port 0 status = 0x400340 [ 31.368071] xhci-hcd xhci-hcd.0.auto: Get port status returned 0x400340 [ 31.368224] hub 6-0:1.0: state 7 ports 1 chg evt Looks like port is stuck in Compliance mode, this should only happen if a connect was detected, moving the port from RxDetect to Polling, and then timeout on polling to get to compliance. 4.9 is then stuck in a hub_suspend/resume loop with a port in compliance mode. v4.10-rc1 dmesg logs : [ 269.436617] usb usb6-port1: cannot disable (err = -32) [ 269.464728] hub 6-0:1.0: state 7 ports 1 chg evt 0002 [ 269.464756] xhci-hcd xhci-hcd.0.auto: get port status, actual port 0 status = 0x340 I 4.10-rc1 we are again stuck in Compliance mode, but this time there was a event (2) visible, maybe because the flags were not cleared or some other reason. Now are stuck in a port reset loop with the port in compliance mode. So the real question is why is the port in compliance mode when there are no devices connected.? I'll continue looking at this more tomorrow So with both 4.9 and 4.10-rc the ports are in Compliance mode state, even if nothing is connected To me this looks like the real underlying issue. For some reason it didn't try to reset the port in 4.9, and never got to the loop of port reset. In 4.9 the hub->events_bits[0] are not set, so the hub thread won't even ask for port status on any ports. In 4.10-rc the hub->events_bits[0] is set for port 1, It will start handling port events, see the compliance state and try to warm reset the port, and fail as it never sees port Any chance you could add extra debugging to xhci to show why the change bit is set? Something like this: diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 0ef1690..e425d82 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -1271,6 +1271,10 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf) jiffies, bus_state->resume_done[i]))) { buf[(i + 1) / 8] |= 1 << (i + 1) % 8; status = 1; +
[PATCH linux-next 1/1] usb: gadget: udc: atmel: Update endpoint allocation scheme
From: Cristian BirsanUpdate 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..3417bb9 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; +
[PATCH linux-next 0/1] usb: gadget: udc: atmel: Update endpoint allocation scheme
From: Cristian BirsanHi, 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. 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
Re: [PATCH v3 4/5] usb: musb: musb_cppi41: Workaround DMA stall issue during teardown
On 01/19/2017 05:56 PM, Tony Lindgren wrote: > * Alexandre Bailon[170119 06:09]: >> The DMA may hung up if a teardown is initiated while an endpoint is still >> active (Advisory 2.3.27 of DA8xx errata). >> To workaround this issue, add a delay before to initiate the teardown. >> >> Signed-off-by: Alexandre Bailon >> --- >> drivers/usb/musb/da8xx.c | 2 +- >> drivers/usb/musb/musb_core.h | 1 + >> drivers/usb/musb/musb_cppi41.c | 4 >> 3 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c >> index 5f50a78..8c9850c 100644 >> --- a/drivers/usb/musb/da8xx.c >> +++ b/drivers/usb/musb/da8xx.c >> @@ -483,7 +483,7 @@ da8xx_dma_controller_create(struct musb *musb, void >> __iomem *base) >> #endif >> >> static const struct musb_platform_ops da8xx_ops = { >> -.quirks = MUSB_INDEXED_EP | MUSB_DMA_CPPI41, >> +.quirks = MUSB_INDEXED_EP | MUSB_DMA_CPPI41 | MUSB_DA8XX, >> .init = da8xx_musb_init, >> .exit = da8xx_musb_exit, >> >> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h >> index ade902e..d129278 100644 >> --- a/drivers/usb/musb/musb_core.h >> +++ b/drivers/usb/musb/musb_core.h >> @@ -172,6 +172,7 @@ struct musb_io; >> */ >> struct musb_platform_ops { >> >> +#define MUSB_DA8XX BIT(7) >> #define MUSB_DMA_UX500 BIT(6) >> #define MUSB_DMA_CPPI41 BIT(5) >> #define MUSB_DMA_CPPI BIT(4) >> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c >> index 1fe7eae..d371d05 100644 >> --- a/drivers/usb/musb/musb_cppi41.c >> +++ b/drivers/usb/musb/musb_cppi41.c >> @@ -554,6 +554,10 @@ static int cppi41_dma_channel_abort(struct dma_channel >> *channel) >> } >> } >> >> +/* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */ >> +if (musb->io.quirks & MUSB_DA8XX) >> +mdelay(250); >> + >> tdbit = 1 << cppi41_channel->port_num; >> if (is_tx) >> tdbit <<= 16; > > How about replace the do while loop calling dmaengine_terminate_all() with > deferred work doing that? That would be great but I don't know how hard to would be to do it. Right after the teardown, we may have to queue a new request. But we must be sure that the teardown has completed before to start a new transfer. And because it happen in atomic section, we must also defer it. > > That way it's more generic as it seems there's no hurry doing that > after the musb registers are cleared. That would also allow getting rid of > the two current udelay() calls there that also seems nasty. > > Regards, > > Tony > 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
[PATCH 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 60e8bf9..dbbf8c2 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) @@ -2058,7 +2058,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 = >urb->iso_frame_desc[idx]; requested = frame->length; remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); @@ -2137,7 +2137,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 = >urb->iso_frame_desc[idx]; /* The transfer is partly done. */ @@ -3134,7 +3134,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_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(>cancelled_td_list, >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
[PATCH 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 dbbf8c2..5e51109 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2841,7 +2841,7 @@ static int prepare_transfer(struct xhci_hcd *xhci, return ret; urb_priv = urb->hcpriv; - td = urb_priv->td[td_index]; + td = _priv->td[td_index]; INIT_LIST_HEAD(>td_list); INIT_LIST_HEAD(>cancelled_td_list); @@ -3137,7 +3137,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 = _priv->td[0]; /* * Don't give the first TRB to the hardware (by toggling the cycle bit) @@ -3230,7 +3230,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); } @@ -3282,7 +3282,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 = _priv->td[0]; /* * Don't give the first TRB to the hardware (by toggling the cycle bit) @@ -3570,7 +3570,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 = _priv->td[i]; /* use SIA as default, if frame id is used overwrite it */ sia_frame_id = TRB_SIA; @@ -3677,20 +3677,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(_priv->td[i]->td_list); + list_del_init(_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, _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 flags;
[PATCH 32/37] usb: host: xhci: remove newline from tracer
From: Felipe BalbiIf 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. 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 31/37] usb: host: xhci: convert several if() to a single switch statement
From: Felipe Balbiwhen 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(>desc); - if (usb_endpoint_xfer_control(>desc)) + switch (usb_endpoint_type(>desc)) { + case USB_ENDPOINT_XFER_CONTROL: return CTRL_EP; - if (usb_endpoint_xfer_bulk(>desc)) + case USB_ENDPOINT_XFER_BULK: return in ? BULK_IN_EP : BULK_OUT_EP; - if (usb_endpoint_xfer_isoc(>desc)) + case USB_ENDPOINT_XFER_ISOC: return in ? ISOC_IN_EP : ISOC_OUT_EP; - if (usb_endpoint_xfer_int(>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 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(>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(>lock, flags); - if (xhci->xhc_state & XHCI_STATE_DYING) - goto dying; + spin_lock_irqsave(>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(>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(>lock, flags); - } else if (usb_endpoint_xfer_bulk(>ep->desc)) { - spin_lock_irqsave(>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(>lock, flags); - } else if (usb_endpoint_xfer_int(>ep->desc)) { - spin_lock_irqsave(>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(>lock, flags); - } else { - spin_lock_irqsave(>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) - goto free_priv; -
[PATCH 28/37] usb: host: xhci: combine event TRB completion debugging messages
From: Felipe BalbiIf 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 33/37] usb: host: xhci: add xhci_virt_device tracer
From: Felipe BalbiLet'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, >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);
[PATCH 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(>ep->desc)) - size = urb->number_of_packets; + num_tds = urb->number_of_packets; else if (usb_endpoint_is_bulk_out(>ep->desc) && urb->transfer_buffer_length > 0 && urb->transfer_flags & URB_ZERO_PACKET && !(urb->transfer_buffer_length % usb_endpoint_maxp(>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 20/37] usb: host: xhci: reorder variable definitions
From: Felipe Balbino 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 15/37] usb: host: xhci: print HCIVERSION on debug
From: Felipe BalbiWhen 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) >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 26/37] xhci: use the trb_to_noop() helper for command trbs
Remove duplicate code by using trb_to_noop() when handling Aborted commads 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 699fc2d..b372c61 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -317,7 +317,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, >cmd_list, cmd_list) { @@ -329,15 +328,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 25/37] xhci: Introduce helper to turn one TRB into a no-op
Useful for turning both transfer and command trbs into no-ops Signed-off-by: Mathias Nyman--- drivers/usb/host/xhci-ring.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 8b78eee..699fc2d 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. @@ -290,6 +305,9 @@ static struct xhci_command *xhci_next_queued_cmd(struct xhci_hcd *xhci) cmd_list); } + + + /* * Turn all commands on command ring with status set to "aborted" to no-op trbs. * If there are other commands waiting then restart the ring and kick the timer. @@ -592,18 +610,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 19/37] usb: host: xhci: use slightly better list helpers
From: Felipe BalbiReplace 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(_state, 0, sizeof(deq_state)); ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); ep = >devs[slot_id]->eps[ep_index]; + last_unlinked_td = list_last_entry(>cancelled_td_list, + struct xhci_td, cancelled_td_list); if (list_empty(>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, >cancelled_td_list) { - cur_td = list_entry(entry, struct xhci_td, cancelled_td_list); + list_for_each_entry(cur_td, >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(_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(>cancelled_td_list, struct xhci_td, cancelled_td_list); list_del_init(_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(>cmd_list, struct xhci_command, cmd_list); cancel_delayed_work(>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(>cmd_list)) { - xhci->current_cmd = list_entry(cmd->cmd_list.next, - struct xhci_command, cmd_list); + xhci->current_cmd = list_first_entry(>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(_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 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(>stop_cmd_timer)) - ep->stop_cmds_pending--; + /* Can't del_timer_sync in interrupt */ + del_timer(>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(>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(>stop_cmd_timer)) { spin_unlock_irqrestore(>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(>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(>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 22/37] usb: host: xhci: remove bogus __releases()/__acquires() annotation
From: Felipe Balbihandle_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(>lock) - __acquires(>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 23/37] usb: host: xhci: check for a valid ring when unmapping bounce buffer
From: Felipe BalbiThis 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 09/37] usb: host: xhci-plat: enable BROKEN_PED quirk if platform requested
From: Felipe BalbiIn 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(>dev, "usb3-lpm-capable")) xhci->quirks |= XHCI_LPM_SUPPORT; + if (device_property_read_bool(>dev, "quirk-broken-port-ped")) + xhci->quirks |= XHCI_BROKEN_PORT_PED; + hcd->usb_phy = devm_usb_get_phy_by_phandle(>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 00/37] xhci features for usb-next 4.11
Hi Greg A lot of xhci cleanups, refactoring and changes for 4.11 Including major tracing rework by Felipe -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 | 466 +- drivers/usb/host/xhci-trace.h | 184 ++- drivers/usb/host/xhci.c| 212 - drivers/usb/host/xhci.h| 528 ++--- 11 files changed, 985 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 13/37] usb: xhci: remove unnecessary return in xhci_pci_setup()
From: Lu BaoluRemove 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 07/37] xhci: Put warning message on a single line
From: Alexander SteinThis 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 11/37] usb: xhci: avoid unnecessary calculation
From: Lu BaoluNo 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 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(_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 10/37] usb: xhci: remove unnecessary assignment
From: Lu BaoluDrop 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 21/37] usb: host: xhci: introduce xhci_td_cleanup()
From: Felipe BalbiBy 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_list); + /* Was this TD slated to be cancelled but completed anyway? */ + if (!list_empty(>cancelled_td_list)) + list_del_init(>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(>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_list); - /* Was this TD slated to be cancelled but completed anyway? */ - if (!list_empty(>cancelled_td_list)) - list_del_init(>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(>ep->desc))) - xhci_dbg(xhci,
[PATCH 08/37] usb: xhci: add quirk flag for broken PED bits
From: Felipe BalbiSome 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 24/37] usb: host: xhci: unconditionally call xhci_unmap_td_bounce_buffer()
From: Felipe Balbixhci_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(_td->cancelled_td_list)) list_del_init(_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 12/37] usb: xhci: use list_is_singular for cmd_list
From: Lu BaoluUse 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 != >cmd_list) { + if (!list_is_singular(>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_list, >cmd_list); /* if there are no other commands queued we start the timeout timer */ - if (xhci->cmd_list.next == >cmd_list) { + if (list_empty(>cmd_list)) { xhci->current_cmd = cmd; xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT); } + list_add_tail(>cmd_list, >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 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(>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
Re: XHCI regression on v4.10-rc1
On 01/13/2017 05:12 PM, Patrice Chotard wrote: > On 01/12/2017 06:17 PM, Mathias Nyman wrote: >> On 09.01.2017 18:42, Mathias Nyman wrote: >>> On 09.01.2017 16:23, Patrice Chotard wrote: On 01/09/2017 01:30 PM, Mathias Nyman wrote: > > On 09.01.2017 11:51, Patrice Chotard wrote: >> Hi Mathias, Greg > > Hi > >> >> I am working on ARM STi platform, since v4.10-rc1, when booting B2260 or >> B2120 STi boards platform >> with nothing plugged on USB3 connector, i observed the following kernel >> logs : >> >> >> [ 801.953836] usb usb6-port1: Cannot enable. Maybe the USB cable is bad? > > That's odd, why does it try to reset the port if there's nothing there? > >> [ 801.960455] xhci-hcd xhci-hcd.0.auto: Cannot set link state. > > This makes sense, nothing is connected, and we try to set link to U3 > (PORT_PE == 0) > >> [ 801.966611] usb usb6-port1: cannot disable (err = -32) > xhci reutns -EPIPE as we try to set link state to U3 while port is not > enabled (PORT_PE == 0) > >> [ 806.083772] usb usb6-port1: Cannot enable. Maybe the USB cable is bad? > > Again, about 5 seconds later the same port is reset, while nothing is > connected. > Odd > >> [ 806.090370] xhci-hcd xhci-hcd.0.auto: Cannot set link state. >> [ 806.096494] usb usb6-port1: cannot disable (err = -32) >> [ 810.208766] usb usb6-port1: Cannot enable. Maybe the USB cable is bad? >> [ 810.215374] xhci-hcd xhci-hcd.0.auto: Cannot set link state. >> [ 810.221478] usb usb6-port1: cannot disable (err = -32) >> [ 814.333767] usb usb6-port1: Cannot enable. Maybe the USB cable is bad? >> [ 814.340364] xhci-hcd xhci-hcd.0.auto: Cannot set link state. >> . >> >> Another interesting thing is even if i plugged a device (a mass storage >> device in my case) on the USB3 connector, >> the above logs continue to appear. >> >> This is due to commit 37be66767e3ca "usb: hub: Fix auto-remount of >> safely removed or ejected USB-3 devices". >> >> I don't know if STi platforms are the only impacted by this issue. >> >> On v4.9 everything was ok. >> > > Was there not a single "Cannot enable. Maybe USB cable is bad" message > with 4.9? There is no "Cannot enable. Maybe USB cable is bad" message with 4.9 > > Before commit 37be66767e3ca we forced the port to RxDetect state via > Disabled state, > and cleared port change flags when a usb3_port_disable() was called. > > after we just set the port to U3. Maybe those STi platforms depend on > that cycle somehow? I really don't know :-( > > Could you take logs (dmesg) of both 4.9 and 4.10-rc1 with both usb core > and xhci debugging enabled: > > echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control > echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control You will find below the 2 requested logs, in both case , no USB cable are plugged. v4.9 dmesg logs : . [ 31.368022] hub 6-0:1.0: hub_resume [ 31.368064] xhci-hcd xhci-hcd.0.auto: get port status, actual port 0 status = 0x400340 [ 31.368071] xhci-hcd xhci-hcd.0.auto: Get port status returned 0x400340 [ 31.368224] hub 6-0:1.0: state 7 ports 1 chg evt >>> >>> Looks like port is stuck in Compliance mode, this should only happen if a >>> connect was detected, >>> moving the port from RxDetect to Polling, and then timeout on polling to >>> get to compliance. >>> >>> 4.9 is then stuck in a hub_suspend/resume loop with a port in compliance >>> mode. v4.10-rc1 dmesg logs : [ 269.436617] usb usb6-port1: cannot disable (err = -32) [ 269.464728] hub 6-0:1.0: state 7 ports 1 chg evt 0002 [ 269.464756] xhci-hcd xhci-hcd.0.auto: get port status, actual port 0 status = 0x340 >>> >>> I 4.10-rc1 we are again stuck in Compliance mode, but this time there was >>> a event (2) visible, maybe because >>> the flags were not cleared or some other reason. Now are stuck in a port >>> reset loop with the >>> port in compliance mode. >>> So the real question is why is the port in compliance mode when there are >>> no devices connected.? >>> >>> I'll continue looking at this more tomorrow >> >> So with both 4.9 and 4.10-rc the ports are in Compliance mode state, even if >> nothing is connected >> To me this looks like the real underlying issue. For some reason it didn't >> try to reset the port in 4.9, >> and never got to the loop of port reset. >> >> In 4.9 the hub->events_bits[0] are not set, so the hub thread won't even ask >> for port status >> on any ports. >> >> In 4.10-rc the hub->events_bits[0] is set for port 1, It will start handling >> port events, see the >> compliance state and
[PATCH 06/37] usb: host: xhci: Remove unused 'addr_64' variable in xhci_hcd structure
From: Baolin WangSince 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 05/37] usb: xhci: remove unnecessary second abort try
From: Lu BaoluThe 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(>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, - >op_regs->cmd_ring); - udelay(1000); - ret = xhci_handshake(>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 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 BaoluSigned-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_list, >cmd_list); /* if there are no other commands queued we start the timeout timer */ - if (xhci->cmd_list.next == >cmd_list && - !delayed_work_pending(>cmd_timer)) { + if (xhci->cmd_list.next == >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 17/37] usb: host: xhci: simplify irq handler return
From: Felipe BalbiInstead 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(>lock); /* Check if the xHC generated the interrupt, or the irq is shared */ status = readl(>op_regs->status); - if (status == 0x) - goto hw_died; - - if (!(status & STS_EINT)) { - spin_unlock(>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(>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, >ir_set->erst_dequeue); xhci_write_64(xhci, temp_64 | ERST_EHB, >ir_set->erst_dequeue); - spin_unlock(>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, >ir_set->erst_dequeue); + ret = IRQ_HANDLED; +out: spin_unlock(>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 27/37] usb: host: xhci: convert to list_for_each_entry_safe()
From: Felipe Balbiinstead 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 b372c61..f65487c 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -811,11 +811,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(>td_list)) { - cur_td = list_first_entry(>td_list, - struct xhci_td, td_list); + list_for_each_entry_safe(cur_td, tmp, >td_list, td_list) { list_del_init(_td->td_list); + if (!list_empty(_td->cancelled_td_list)) list_del_init(_td->cancelled_td_list); @@ -831,6 +831,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; @@ -856,12 +857,12 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, slot_id, ep_index); xhci_kill_ring_urbs(xhci, ring); } - while (!list_empty(>cancelled_td_list)) { - cur_td = list_first_entry(>cancelled_td_list, - struct xhci_td, cancelled_td_list); - list_del_init(_td->cancelled_td_list); + list_for_each_entry_safe(cur_td, tmp, >cancelled_td_list, + cancelled_td_list) { + list_del_init(_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
[PATCH 14/37] usb: host: xhci: change pre-increments to post-increments
From: Felipe BalbiThis 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 = >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", >run_regs->microframe_index, (unsigned int) temp); - for (i = 0; i < 7; ++i) { + for (i = 0; i < 7; i++) { temp = readl(>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 = >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 = >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", [4 + i], (unsigned long long)dma, @@ -496,7 +496,7 @@ static void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx * _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", _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, _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", _ctx->reserved[j], (unsigned long long)dma, @@ -583,7 +583,7 @@ void xhci_dbg_ctx(struct xhci_hcd *xhci, _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)
[PATCH 16/37] usb: host: xhci: rename completion codes to match spec
From: Felipe BalbiCleanup 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, >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, >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, >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(>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 ==
[PATCH 29/37] usb: host: xhci: make a generic TRB tracer
From: Felipe Balbiinstead 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 f65487c..299cdd8 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1322,6 +1322,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, _trb->generic); + cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg, cmd_trb); /* @@ -1338,8 +1341,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, cancel_delayed_work(>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 */ @@ -2482,6 +2483,10 @@ static int handle_tx_event(struct xhci_hcd *xhci, ep_trb = _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 @@ -2548,6 +2553,8 @@ static int xhci_handle_event(struct xhci_hcd *xhci) xhci->event_ring->cycle_state) return 0; + trace_xhci_handle_event(xhci->event_ring, >generic); + /* * Barrier between reading the TRB_CYCLE (valid) flag above and any * speculative reads of the event's flags/data below. @@ -2717,6 +2724,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
[PATCH 18/37] usb: host: xhci: remove unneded semicolon
From: Felipe Balbiit 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 30/37] usb: host: xhci: add urb_enqueue/dequeue/giveback tracers
From: Felipe BalbiThese 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 299cdd8..60e8bf9 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -645,6 +645,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, usb_hcd_unlink_urb_from_ep(hcd, urb); spin_unlock(>lock); usb_hcd_giveback_urb(hcd, urb, status); + trace_xhci_urb_giveback(urb); spin_lock(>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(>ep->desc); + __entry->dir_in = usb_endpoint_dir_in(>ep->desc); + __entry->type = usb_endpoint_type(>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(>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(>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
Re: [PATCH] usb: dwc3: add quirk to handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS
On 20/01/17 12:10, Felipe Balbi wrote: >>> And unfortunately, whether this is set or not is not visible to the >>> software so it will require a quirk. >> >> but arrived at this conclusion because I couldn't think of a reasonable >> guess value for IN/OUT endpoint numbers that would work if >> DWC_USB3_NUM_EPS == DWC_USB3_NUM_IN_EPS > > Well, we can, for now at least, take the simple approach of half IN, > half OUT. If DWC3_USB3_NUM_EPS is odd, then OUT should take the extra > endpoint. If anybody complains, we can fix it later ;-) Sure, works just as well for me either way. --- bod -- 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 4/5] DT bindings documentation for Synopsys UDC platform driver
On Thu, Jan 19, 2017 at 4:56 PM, Florian Fainelliwrote: > On 01/19/2017 02:36 PM, Scott Branden wrote: > The driver stands alone from the SoC and does not need compatibility > strings per SoC. New SoCs will use the exact same block. Even if you take the exact same block and put it in a different SoC, that's still an integration work that 99% of the time goes just fine because the validation worked great, and the 1% of the time where you need to capture an integration bug, you are glad this SoC-specific compatible string exists such that you can work around it in the driver. >>> >>> That's a very conservative estimate. Based on my experience, it's more >>> like 50/50, i.e., roughly half of the time we found SoC integration >>> specific quirks or workaround are needed. >>> >> 50% is an exaggeration for sure. Maybe a driver you are has that issue >> but that is not the case with most drivers. We have many IP blocks in >> the SoC - both internal and externally sourced IP. We integrate SP805 >> timer driver into many SoCs and never specify a SoC specific >> compatibility string with it (nor should we). Even if it was only 10%, that's still reason to do it. > Well, that's a good example where in premise, each SoC vendor > integrating such a peripheral from a third party should actually have > defined its own SoC/vendor compatible string to document the > integration. And you can sometimes see some vendors having to workaround > such essential peripherals and ending-up documenting compatible strings > (or close enough in the example at [1]). ARM peripherals are a bit unique because they have ID registers and vendors tend to change them if they change the IP. And we can also set the ID in the DT. It's also a huge difference between a timer and a USB controller. There's very little in a timer that vendors can f*ck up as well as few revisions and config options. Experience has shown that USB always gets integrated in different ways. We can't even get the number of clocks right on licensed IP blocks. Like many things, there is a judgement call here. > [1]: http://www.spinics.net/lists/devicetree/msg159585.html > > It's a bad example though in that it's an IP that came from ARM, so the > confidence level in getting the integration right is just higher > (typically above level 9000), because ripping apart a third party is > governed by strict architecture licensing agreements that usually > prevents people suffering from the Not Invented Here syndrome from > making damage. > >> >> That being said - if your driver needs to know SoC specifics is should >> not need to have an SoC specific compatibility string added per driver. >> Why can your driver just not query that information from the upper level >> SoC specific info already present in device tree? > > You could do that, but that just does not happen to be a common or > recommended practice AFAICT, although I could be just wrong here of course. We did a lot of work to get rid machine_is_X(). Let's say you have 10 SoCs and 5 have a quirk in a device and 5 don't. You need 2 device compatible strings to match in that case. If you check at the top level you may have to check 5 strings because you can't claim all 5 SoCs to be compatible with each other (that only works for sub/supersets). You would also have to update the driver for new SoCs depending if they had the quirk or not. >> Each SoC is already specified in device tree at the upper level already. >> Example: >> arch/arm/boot/dts/bcm7445.dtsi has this compatibility info already >> present in its device tree: >> >> compatible = "brcm,bcm7445", "brcm,brcmstb"; >> >> If needed, a driver should query this info rather than adding SoC >> specific compatibility strings to every single device tree entry. > > Or you could just put it in the compatible string list for a given > peripheral, and yes, this is a repetition of information that is already > there at a higher level from that particular node, but, it has the > advantage of making all this information self contained within that > node's context, and that's a good design goal. > >> >> We should only add driver revision numbers as needed, not SoC specific >> names. That way drivers don't change when the (same revision) of the IP >> block is added to a new SoCs. And then if a SoC specific workaround is >> needed the upper level compatibility string can be queried should be >> utilized. It already exists today and is available for use to all drivers. > > The point is to plan ahead for information that you *may*, but *wish* > you did not need. > > Quite frankly, I don't think you are going to win any argument where you > don't add a SoC compatible string to the binding, because there are tons > of precedents and good practices that suggest doing it. You might as > well just do it, it's documented, it's there, if you end up using it or > not, that's totally up to the driver author.
Re: [PATCH 1/1] xhci: remove WARN_ON if dma mask is not set for platform devices
On Fri, Jan 20, 2017 at 3:38 PM, Mathias Nyman <mathias.ny...@linux.intel.com> wrote: > The warn on is a bit too much, we will anyway set the dma mask if not set > previously. > > The main reason for this fix is that 4.10-rc1 has a dwc3 change that > pass a parent sysdev dev pointer instead of setting the dma mask of > its xhci platform device. xhci platform driver can then get more > attributes from the sysdev than just the dma mask. > > The usb core and xhci changes are not yet in 4.10, and a fix like > this was preferred instead of taking those big changes this late in > the rc-cycle. > 100% reproducible on Intel Edison BEFORE # uname -a Linux buildroot 4.10.0-rc4-next-20170120+ #245 SMP Fri Jan 20 15:49:08 EET 2017 x86_64 GNU/Linux # dmesg | grep WARN [ 19.25] WARNING: CPU: 0 PID: 1 at /home/andy/prj/linux-topic-mfld/drivers/usb/host/xhci-plat.c:168 xhci_pla t_probe+0x4a9/0x550 AFTER # uname -a Linux buildroot 4.10.0-rc4-next-20170120+ #246 SMP Fri Jan 20 15:52:10 EET 2017 x86_64 GNU/Linux # dmesg | grep WARN # Tested-by: Andy Shevchenko <andy.shevche...@gmail.com> > Signed-off-by: Mathias Nyman <mathias.ny...@linux.intel.com> > --- > drivers/usb/host/xhci-plat.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index ddfab30..e5834dd 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -165,7 +165,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > return -ENODEV; > > /* Try to set 64-bit DMA first */ > - if (WARN_ON(!pdev->dev.dma_mask)) > + if (!pdev->dev.dma_mask) > /* Platform did not initialize dma_mask */ > ret = dma_coerce_mask_and_coherent(>dev, >DMA_BIT_MASK(64)); -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/1] xhci fix for usb-linus
Hi Greg Here's the patch that removes the scary WARN_ON in 4.10 xhci platform driver -Mathias Mathias Nyman (1): xhci: remove WARN_ON if dma mask is not set for platform devices drivers/usb/host/xhci-plat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 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 1/1] xhci: remove WARN_ON if dma mask is not set for platform devices
The warn on is a bit too much, we will anyway set the dma mask if not set previously. The main reason for this fix is that 4.10-rc1 has a dwc3 change that pass a parent sysdev dev pointer instead of setting the dma mask of its xhci platform device. xhci platform driver can then get more attributes from the sysdev than just the dma mask. The usb core and xhci changes are not yet in 4.10, and a fix like this was preferred instead of taking those big changes this late in the rc-cycle. Signed-off-by: Mathias Nyman--- drivers/usb/host/xhci-plat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index ddfab30..e5834dd 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -165,7 +165,7 @@ static int xhci_plat_probe(struct platform_device *pdev) return -ENODEV; /* Try to set 64-bit DMA first */ - if (WARN_ON(!pdev->dev.dma_mask)) + if (!pdev->dev.dma_mask) /* Platform did not initialize dma_mask */ ret = dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64)); -- 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 resend v5 0/4] xhci fixes for usb-linus
Hi, Greg KHwrites: >> >> > > > > This series by Arnd Bergmann was originally six patches, but last >> >> > > > > two of >> >> > > > > them were already taken to 4.10. Without the rest of them there >> >> > > > > will >> >> > > > > be a regression in 4.10. >> >> > > > >> >> > > > Is it really a regression? I thought this had never worked before >> >> > > > in >> >> > > > older kernels, right? >> >> > > > >> >> > > >> >> > > Regression when xhci hosts in dwc3 controllers are used. >> >> > >> >> > So that worked in 4.9? >> >> > >> >> > > For example patch 5/6 removed setting dma mask for xhci in dwc3 host >> >> > > init: >> >> > > >> >> > > +++ b/drivers/usb/dwc3/host.c >> >> > > @@ -84,11 +84,7 @@ int dwc3_host_init(struct dwc3 *dwc) >> >> > > return -ENOMEM; >> >> > > } >> >> > > - dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask); >> >> > > - >> >> > > xhci->dev.parent= dwc->dev; >> >> > > - xhci->dev.dma_mask = dwc->dev->dma_mask; >> >> > > - xhci->dev.dma_parms = dwc->dev->dma_parms; >> >> > > >> >> > > So now xhci platform driver prints a scary warning because of the >> >> > > missing dma mask: >> >> > > >> >> > > static int xhci_plat_probe(struct platform_device *pdev) >> >> > > /* Try to set 64-bit DMA first */ >> >> > > if (WARN_ON(!pdev->dev.dma_mask)) >> >> > > /* Platform did not initialize dma_mask */ >> >> > > ret = dma_coerce_mask_and_coherent(>dev, >> >> > > DMA_BIT_MASK(64)); >> >> > > else >> >> > > ... >> >> > > This is fixed in the first 4 patches. >> >> > > >> >> > > There might be other other issues as well caused by having only the >> >> > > dwc3 >> >> > > changed applied of this series, but not the core and xhci parts >> >> > >> >> > Should we just fix the "scary warning" instead, by removing it? :) >> >> > >> >> > I say all of this because this seems like some very big changes so late >> >> > in the -rc cycle. >> >> > >> >> >> >> I guess that would work, or at least get us to the same stage as 4.9. >> >> I'll send a patch for it. >> > >> > Great. >> > >> >> Gives more time to look at the usb core changes. I'm not really >> >> myself running or testing the dwc3 host side. >> > >> > Me either. Any hints on some hardware that would allow me to do that? >> >> Intel Edison. > > Ah nice, if only I knew someone at Intel who could get me one of > those... :) :-) >> Or any recent TI board (AM437x SK, for instance). > > Will that run a mainline kernel? yes :-) >> Google Pixel Phone (but good luck running a mainline kernel there ;-) > > I have a Pixel phone here, but haven't been paying attention to the dwc3 > interface. Is dwc3 a block in the SoC on the Pixel? I have another yeah. Qcom, intel, TI, ST, Xilinx... they're all using this IP from Synopsys. > device here with the same SoC as the Pixel (OnePlus 3T) that I can run > my own kernel on, but it's a bit older version, due to SoC issues (same > one the Pixel has at the moment...) However some of us have a crazy > idea to drag that platform up to mainline over the next few months. yeah, I'd totally buy that OnePlus 3T if it starts running mainline. I've been looking for an Android device to do that. It would actually be much easier to get dwc3 stuff properly tested :-p -- balbi signature.asc Description: PGP signature
Re: [PATCH resend v5 0/4] xhci fixes for usb-linus
On Fri, Jan 20, 2017 at 02:08:50PM +0200, Felipe Balbi wrote: > > Hi, > > Greg KHwrites: > > On Fri, Jan 20, 2017 at 01:35:39PM +0200, Mathias Nyman wrote: > >> On 20.01.2017 12:22, Greg KH wrote: > >> > On Fri, Jan 20, 2017 at 11:23:36AM +0200, Mathias Nyman wrote: > >> > > On 19.01.2017 20:48, Greg KH wrote: > >> > > > On Thu, Jan 19, 2017 at 02:21:26PM +0200, Mathias Nyman wrote: > >> > > > > Hi Greg > >> > > > > > >> > > > > This series by Arnd Bergmann was originally six patches, but last > >> > > > > two of > >> > > > > them were already taken to 4.10. Without the rest of them there > >> > > > > will > >> > > > > be a regression in 4.10. > >> > > > > >> > > > Is it really a regression? I thought this had never worked before in > >> > > > older kernels, right? > >> > > > > >> > > > >> > > Regression when xhci hosts in dwc3 controllers are used. > >> > > >> > So that worked in 4.9? > >> > > >> > > For example patch 5/6 removed setting dma mask for xhci in dwc3 host > >> > > init: > >> > > > >> > > +++ b/drivers/usb/dwc3/host.c > >> > > @@ -84,11 +84,7 @@ int dwc3_host_init(struct dwc3 *dwc) > >> > >return -ENOMEM; > >> > >} > >> > > - dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask); > >> > > - > >> > >xhci->dev.parent= dwc->dev; > >> > > - xhci->dev.dma_mask = dwc->dev->dma_mask; > >> > > - xhci->dev.dma_parms = dwc->dev->dma_parms; > >> > > > >> > > So now xhci platform driver prints a scary warning because of the > >> > > missing dma mask: > >> > > > >> > > static int xhci_plat_probe(struct platform_device *pdev) > >> > > /* Try to set 64-bit DMA first */ > >> > >if (WARN_ON(!pdev->dev.dma_mask)) > >> > > /* Platform did not initialize dma_mask */ > >> > > ret = dma_coerce_mask_and_coherent(>dev, > >> > > DMA_BIT_MASK(64)); > >> > > else > >> > >... > >> > > This is fixed in the first 4 patches. > >> > > > >> > > There might be other other issues as well caused by having only the > >> > > dwc3 > >> > > changed applied of this series, but not the core and xhci parts > >> > > >> > Should we just fix the "scary warning" instead, by removing it? :) > >> > > >> > I say all of this because this seems like some very big changes so late > >> > in the -rc cycle. > >> > > >> > >> I guess that would work, or at least get us to the same stage as 4.9. > >> I'll send a patch for it. > > > > Great. > > > >> Gives more time to look at the usb core changes. I'm not really > >> myself running or testing the dwc3 host side. > > > > Me either. Any hints on some hardware that would allow me to do that? > > Intel Edison. Ah nice, if only I knew someone at Intel who could get me one of those... :) > Or any recent TI board (AM437x SK, for instance). Will that run a mainline kernel? > Google Pixel Phone (but good luck running a mainline kernel there ;-) I have a Pixel phone here, but haven't been paying attention to the dwc3 interface. Is dwc3 a block in the SoC on the Pixel? I have another device here with the same SoC as the Pixel (OnePlus 3T) that I can run my own kernel on, but it's a bit older version, due to SoC issues (same one the Pixel has at the moment...) However some of us have a crazy idea to drag that platform up to mainline over the next few months. 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: [PATCH] usb: dwc3: add quirk to handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS
Hi, Bryan O'Donoghuewrites: > On 19/01/17 22:49, John Youn wrote: > >> So it is valid to have say, DWC_USB3_NUM_EPS=8 and >> DWC_USB3_NUM_IN_EPS=8. Even though it is not possible to use all 8 EPs >> as IN since you need at least one of them to be a control OUT. So you >> could have a configuration of EP0 IN and OUT plus 6 IN EPs (total 1 >> OUT, 7 IN). Or EP0 IN and OUT plus any other combination of IN/OUT for >> the remaining 6 EPs. >> >> With the above in mind, you can probably just redo the endpoint number >> logic in dwc3 to handle all cases without any quirks at all. > > I thought of that. > >> If it is disabled, it will fix the physical and logical EP number and >> direction of the available EPs to meet timings for FPGA designs. This >> shouldn't be used in final designs for ASICS but we don't know for >> sure whether it has made it to any final designs or not. >> >> And unfortunately, whether this is set or not is not visible to the >> software so it will require a quirk. > > but arrived at this conclusion because I couldn't think of a reasonable > guess value for IN/OUT endpoint numbers that would work if > DWC_USB3_NUM_EPS == DWC_USB3_NUM_IN_EPS Well, we can, for now at least, take the simple approach of half IN, half OUT. If DWC3_USB3_NUM_EPS is odd, then OUT should take the extra endpoint. If anybody complains, we can fix it later ;-) -- balbi signature.asc Description: PGP signature
Re: [PATCH resend v5 0/4] xhci fixes for usb-linus
Hi, Greg KHwrites: > On Fri, Jan 20, 2017 at 01:35:39PM +0200, Mathias Nyman wrote: >> On 20.01.2017 12:22, Greg KH wrote: >> > On Fri, Jan 20, 2017 at 11:23:36AM +0200, Mathias Nyman wrote: >> > > On 19.01.2017 20:48, Greg KH wrote: >> > > > On Thu, Jan 19, 2017 at 02:21:26PM +0200, Mathias Nyman wrote: >> > > > > Hi Greg >> > > > > >> > > > > This series by Arnd Bergmann was originally six patches, but last >> > > > > two of >> > > > > them were already taken to 4.10. Without the rest of them there will >> > > > > be a regression in 4.10. >> > > > >> > > > Is it really a regression? I thought this had never worked before in >> > > > older kernels, right? >> > > > >> > > >> > > Regression when xhci hosts in dwc3 controllers are used. >> > >> > So that worked in 4.9? >> > >> > > For example patch 5/6 removed setting dma mask for xhci in dwc3 host >> > > init: >> > > >> > > +++ b/drivers/usb/dwc3/host.c >> > > @@ -84,11 +84,7 @@ int dwc3_host_init(struct dwc3 *dwc) >> > > return -ENOMEM; >> > > } >> > > -dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask); >> > > - >> > > xhci->dev.parent= dwc->dev; >> > > -xhci->dev.dma_mask = dwc->dev->dma_mask; >> > > -xhci->dev.dma_parms = dwc->dev->dma_parms; >> > > >> > > So now xhci platform driver prints a scary warning because of the >> > > missing dma mask: >> > > >> > > static int xhci_plat_probe(struct platform_device *pdev) >> > > /* Try to set 64-bit DMA first */ >> > > if (WARN_ON(!pdev->dev.dma_mask)) >> > > /* Platform did not initialize dma_mask */ >> > > ret = dma_coerce_mask_and_coherent(>dev, >> > > DMA_BIT_MASK(64)); >> > > else >> > > ... >> > > This is fixed in the first 4 patches. >> > > >> > > There might be other other issues as well caused by having only the dwc3 >> > > changed applied of this series, but not the core and xhci parts >> > >> > Should we just fix the "scary warning" instead, by removing it? :) >> > >> > I say all of this because this seems like some very big changes so late >> > in the -rc cycle. >> > >> >> I guess that would work, or at least get us to the same stage as 4.9. >> I'll send a patch for it. > > Great. > >> Gives more time to look at the usb core changes. I'm not really >> myself running or testing the dwc3 host side. > > Me either. Any hints on some hardware that would allow me to do that? Intel Edison. Or any recent TI board (AM437x SK, for instance). Google Pixel Phone (but good luck running a mainline kernel there ;-) -- balbi signature.asc Description: PGP signature
Re: [RFC v2 4/5] DT bindings documentation for Synopsys UDC platform driver
Hi Rob, On Thu, Jan 19, 2017 at 11:06 PM, Rob Herringwrote: > On Tue, Jan 17, 2017 at 01:35:07PM +0530, Raviteja Garimella wrote: >> This patch adds device tree bindings documentation for Synopsys >> USB device controller platform driver. > > Bindings describe h/w, not drivers. Will correct the commit message. >> >> Signed-off-by: Raviteja Garimella >> --- >> .../devicetree/bindings/usb/snps,dw-ahb-udc.txt| 27 >> ++ >> 1 file changed, 27 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt >> >> diff --git a/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt >> b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt >> new file mode 100644 >> index 000..0c18327 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt >> @@ -0,0 +1,27 @@ >> +Synopsys USB Device controller. >> + >> +The device node is used for Synopsys Designware Cores AHB >> +Subsystem Device Controller (UDC). >> + >> +This device node is used by UDCs integrated it Broadcom's >> +Northstar2 and Cygnus SoC's. > > You need compatible strings for these in addition. Is it fine to have "brcm,iproc-udc"? iProc refers to a Broadcom family of processors that includes above mentioned SoCs. I see there are some compatible strings that are based on the IP, and some based on the SoCs. I chose to have the IP based string. Please let me know which one would be agreeable in this case. I will also correct the typo in the above notes -- it meant to be UDCs integrated into Broadcom's Northstar2 and Cygnus SoC's. > >> + >> +Required properties: >> + - compatible: should be "snps,dw-ahb-udc" > > This is a different IP than DWC2? Yes, this is different IP. DWC2 is HS OTG. > >> + - reg: Offset and length of UDC register set >> + - interrupts: description of interrupt line >> + - phys: phandle to phy node. >> + - extcon: phandle to the extcon device. This is optional and >> + not required for those that don't require extcon support. >> + Extcon support will be required if the UDC is connected to >> + a Dual Role Device Phy that supports both Host and Device >> + mode based on the external cable. > > Drop this. It should be a part of the phy. Also, I don't care to see new > users of extcon binding because it needs redoing. Currently we can't get the extcon node from Phy. "extcon_get_edev_by_phandle" requires "extcon" property, else would fail. As Scott said in one of the comments, we can drop this when we get that support in kernel. Is it fine? Thanks, Ravi -- 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 resend v5 0/4] xhci fixes for usb-linus
On Fri, Jan 20, 2017 at 01:35:39PM +0200, Mathias Nyman wrote: > On 20.01.2017 12:22, Greg KH wrote: > > On Fri, Jan 20, 2017 at 11:23:36AM +0200, Mathias Nyman wrote: > > > On 19.01.2017 20:48, Greg KH wrote: > > > > On Thu, Jan 19, 2017 at 02:21:26PM +0200, Mathias Nyman wrote: > > > > > Hi Greg > > > > > > > > > > This series by Arnd Bergmann was originally six patches, but last two > > > > > of > > > > > them were already taken to 4.10. Without the rest of them there will > > > > > be a regression in 4.10. > > > > > > > > Is it really a regression? I thought this had never worked before in > > > > older kernels, right? > > > > > > > > > > Regression when xhci hosts in dwc3 controllers are used. > > > > So that worked in 4.9? > > > > > For example patch 5/6 removed setting dma mask for xhci in dwc3 host init: > > > > > > +++ b/drivers/usb/dwc3/host.c > > > @@ -84,11 +84,7 @@ int dwc3_host_init(struct dwc3 *dwc) > > > return -ENOMEM; > > > } > > > - dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask); > > > - > > > xhci->dev.parent= dwc->dev; > > > - xhci->dev.dma_mask = dwc->dev->dma_mask; > > > - xhci->dev.dma_parms = dwc->dev->dma_parms; > > > > > > So now xhci platform driver prints a scary warning because of the missing > > > dma mask: > > > > > > static int xhci_plat_probe(struct platform_device *pdev) > > > /* Try to set 64-bit DMA first */ > > > if (WARN_ON(!pdev->dev.dma_mask)) > > > /* Platform did not initialize dma_mask */ > > > ret = dma_coerce_mask_and_coherent(>dev, > > > DMA_BIT_MASK(64)); > > > else > > > ... > > > This is fixed in the first 4 patches. > > > > > > There might be other other issues as well caused by having only the dwc3 > > > changed applied of this series, but not the core and xhci parts > > > > Should we just fix the "scary warning" instead, by removing it? :) > > > > I say all of this because this seems like some very big changes so late > > in the -rc cycle. > > > > I guess that would work, or at least get us to the same stage as 4.9. > I'll send a patch for it. Great. > Gives more time to look at the usb core changes. I'm not really > myself running or testing the dwc3 host side. Me either. Any hints on some hardware that would allow me to do that? 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: [PATCH v11 08/14] usb: otg: add OTG/dual-role core
On 01/20/2017 02:00 PM, Roger Quadros wrote: Vivek, On 19/01/17 17:15, vivek.gau...@codeaurora.org wrote: Hi Roger, On 2017-01-19 17:45, Roger Quadros wrote: Vivek, On 19/01/17 13:56, Vivek Gautam wrote: Hi, On Wed, Jun 22, 2016 at 2:00 PM, Roger Quadroswrote: Luckily hit this thread while checking about DRD role functionality for DWC3. On 22/06/16 11:14, Felipe Balbi wrote: Hi, Roger Quadros writes: For the real use case, some Carplay platforms need it. Carplay does *NOT* rely on OTG. Apple has its own proprietary and closed specification which is not OTG-compliant. Yes, it is not OTG-compliant, but it can co-work with some standard OTG FSM states to finish role swap. What are you referring to as "finish role swap"? I don't get that. Change current role from host to peripheral. Okay, we have two scenarios here: 1. You need full OTG compliance For this, granted, you need the state machine if your HW doesn't track it. This is a given. With only one user, however, perhaps we don't need a generic layer. There are not enough different setups to design a good enough generic layer. We will end up with a pseudo-generic framework which is coupled with its only user. 2. Dual-role support, without OTG compliance In this case, you don't need a stack. All you need is a signal to tell you state of ID pin and another to tell you state of VBUS level. If you have those, you don't need to walk an OTG state machine at all. You don't need any of those quirky OTG timers, agreed? Given the above, why would you even want to use a subset of OTG state machine to implement something that's _usually_ as simple as: 8<-- vbus = read(VBUS_STATE); /* could be a gpio_get_value() */ id = read(ID_STATE); /* could be a gpio_get_value() */ set_role(id); set_vbus(vbus); In fact, the individual driver can do it by itself. The chipidea driver handles OTG and dual-role well currently. By considering this OTG/DRD framework is worthwhile or not, we would like to see if it can simplify DRD design for each driver, and can benefit the platforms which has different drivers for host and peripheral to finish the role switch well. simplify how? By adding unnecessary workqueues and a level indirection that just goes back to the same driver? What do you mean by same driver? dwc3 registers to OTG layer. dwc3 also registers as UDC to UDC layer. When dwc3 OTG IRQ fires, dwc3 tells OTG layer about it and OTG layer jumps to a callback that goes back to dwc3 to e.g. start peripheral side. See ?!? Starts on dwc3, goes to OTG layer, goes back to DWC3. Gadget driver, host driver and PHY (or MUX) driver (for ID/VBUS) can be 3 totally independent drivers unlike dwc3 where you have a single driver in control of both host and gadget. That's a totally different issue and one not being tackled by OTG layer, because there are no such users yet. We can't design anything based solely on speculation of what might happen. If there aren't enough users, there is no way to design a good generic layer. Questions not clear to me are: 1) Which driver handles ID/VBUS events and makes a decision to do the role swap? Probably the PHY/MUX driver? This is implementation dependent. For TI's USB subsystem, we have PMIC sampling VBUS/ID that and using EXTCON to tell dwc3-omap to program UTMI mailbox. The same mailbox can be used in HW-mode (see AM437x) where SW has no intervention. For Intel's USB subsystem, we have PMIC sampling VBUS/ID with an internal mux (much like TI's UTMI mailbox, but slightly different) to switch between a separate XHCI or a separate dwc3. The same mux can be put in HW-mode where SW has no intervention. In any case, for Intel's stuff most of the magic happens in ASL. Our PHY driver just detects role (at least for Type-C based plats) and executes _DSM with correct arguments [1]. _DSM will program internal MUX, toggle VBUS and, for type-C, toggle VCONN when needed. 2) How does it perform the role swap? Probably a register write to the PHY/MUX without needing to stop/start controllers? Easy case is both controllers can run in co-existence without interference. Is there any platform other than dwc3 where this is not the case? Again speculation. But to answer your question, only dwc3 is in such a case today. But even for dwc3 we can have DRD with a much, much simpler setup as I have already explained. 3) Even if host and gadget controllers can operate in coexistence, there is no need for both to be running for embedded applications which are usually power conservative. How can we achieve that? Now you're also speculating that you're running on embedded applications and that we _can_ power off parts of the IP. I happen to know that we can't power off XHCI part of dwc3 in
Re: [PATCH resend v5 0/4] xhci fixes for usb-linus
On 20.01.2017 12:22, Greg KH wrote: On Fri, Jan 20, 2017 at 11:23:36AM +0200, Mathias Nyman wrote: On 19.01.2017 20:48, Greg KH wrote: On Thu, Jan 19, 2017 at 02:21:26PM +0200, Mathias Nyman wrote: Hi Greg This series by Arnd Bergmann was originally six patches, but last two of them were already taken to 4.10. Without the rest of them there will be a regression in 4.10. Is it really a regression? I thought this had never worked before in older kernels, right? Regression when xhci hosts in dwc3 controllers are used. So that worked in 4.9? For example patch 5/6 removed setting dma mask for xhci in dwc3 host init: +++ b/drivers/usb/dwc3/host.c @@ -84,11 +84,7 @@ int dwc3_host_init(struct dwc3 *dwc) return -ENOMEM; } - dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask); - xhci->dev.parent = dwc->dev; - xhci->dev.dma_mask = dwc->dev->dma_mask; - xhci->dev.dma_parms = dwc->dev->dma_parms; So now xhci platform driver prints a scary warning because of the missing dma mask: static int xhci_plat_probe(struct platform_device *pdev) /* Try to set 64-bit DMA first */ if (WARN_ON(!pdev->dev.dma_mask)) /* Platform did not initialize dma_mask */ ret = dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64)); else ... This is fixed in the first 4 patches. There might be other other issues as well caused by having only the dwc3 changed applied of this series, but not the core and xhci parts Should we just fix the "scary warning" instead, by removing it? :) I say all of this because this seems like some very big changes so late in the -rc cycle. I guess that would work, or at least get us to the same stage as 4.9. I'll send a patch for it. Gives more time to look at the usb core changes. I'm not really myself running or testing the dwc3 host side. -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 resend v5 0/4] xhci fixes for usb-linus
On Fri, Jan 20, 2017 at 11:23:36AM +0200, Mathias Nyman wrote: > On 19.01.2017 20:48, Greg KH wrote: > > On Thu, Jan 19, 2017 at 02:21:26PM +0200, Mathias Nyman wrote: > > > Hi Greg > > > > > > This series by Arnd Bergmann was originally six patches, but last two of > > > them were already taken to 4.10. Without the rest of them there will > > > be a regression in 4.10. > > > > Is it really a regression? I thought this had never worked before in > > older kernels, right? > > > > Regression when xhci hosts in dwc3 controllers are used. So that worked in 4.9? > For example patch 5/6 removed setting dma mask for xhci in dwc3 host init: > > +++ b/drivers/usb/dwc3/host.c > @@ -84,11 +84,7 @@ int dwc3_host_init(struct dwc3 *dwc) > return -ENOMEM; > } > - dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask); > - > xhci->dev.parent= dwc->dev; > - xhci->dev.dma_mask = dwc->dev->dma_mask; > - xhci->dev.dma_parms = dwc->dev->dma_parms; > > So now xhci platform driver prints a scary warning because of the missing dma > mask: > > static int xhci_plat_probe(struct platform_device *pdev) > /* Try to set 64-bit DMA first */ > if (WARN_ON(!pdev->dev.dma_mask)) > /* Platform did not initialize dma_mask */ > ret = dma_coerce_mask_and_coherent(>dev, >DMA_BIT_MASK(64)); > else > ... > This is fixed in the first 4 patches. > > There might be other other issues as well caused by having only the dwc3 > changed applied of this series, but not the core and xhci parts Should we just fix the "scary warning" instead, by removing it? :) I say all of this because this seems like some very big changes so late in the -rc cycle. 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: [PATCH v11 2/8] power: add power sequence library
On Fri, Jan 20, 2017 at 8:52 AM, Peter Chenwrote: > On Tue, Jan 10, 2017 at 03:02:41PM +0800, Peter Chen wrote: >> On Sat, Jan 07, 2017 at 10:54:56AM +0200, Krzysztof Kozlowski wrote: >> > On Thu, Jan 05, 2017 at 02:01:53PM +0800, Peter Chen wrote: >> > > We have an well-known problem that the device needs to do some power >> > > sequence before it can be recognized by related host, the typical >> > > example like hard-wired mmc devices and usb devices. >> > > >> > > This power sequence is hard to be described at device tree and handled by >> > > related host driver, so we have created a common power sequence >> > > library to cover this requirement. The core code has supplied >> > > some common helpers for host driver, and individual power sequence >> > > libraries handle kinds of power sequence for devices. The pwrseq >> > > librares always need to allocate extra instance for compatible >> > > string match. >> > > >> > > pwrseq_generic is intended for general purpose of power sequence, which >> > > handles gpios and clocks currently, and can cover other controls in >> > > future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off >> > > if only one power sequence is needed, else call of_pwrseq_on_list >> > > /of_pwrseq_off_list instead (eg, USB hub driver). >> > > >> > > For new power sequence library, it can add its compatible string >> > > to pwrseq_of_match_table, then the pwrseq core will match it with >> > > DT's, and choose this library at runtime. >> > > >> > > Signed-off-by: Peter Chen >> > > Tested-by: Maciej S. Szmigiero >> > > Tested-by Joshua Clayton >> > > Reviewed-by: Matthias Kaehlcke >> > > Tested-by: Matthias Kaehlcke >> > >> > Acked-by: Krzysztof Kozlowski >> > Tested on Odroid U3 (reset sequence for LAN9730): >> > Tested-by: Krzysztof Kozlowski >> > >> >> A nice ping... >> > > Rafael, would you please review it? This series was discussed about > half a year, and many people need it, I hope it can be in v4.11-rc1, > thanks. I'm travelling now (http://marc.info/?l=linux-pm=148410629024194=2) and (as stated in this message) I'll get to the patches when I'm back home. There is a good chance for your code to go into 4.11-rc1 if the review comments so far have been addressed. Thanks, Rafael -- 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] arm: davinci: Make the usb20 clock available to PM runtime
On 01/20/2017 10:24 AM, Sekhar Nori wrote: > On Thursday 19 January 2017 11:01 PM, Alexandre Bailon wrote: >> On 01/19/2017 05:49 PM, Grygorii Strashko wrote: >>> On 01/19/2017 09:08 AM, Alexandre Bailon wrote: On 01/19/2017 03:48 PM, Sekhar Nori wrote: > On Thursday 19 January 2017 07:39 PM, Alexandre Bailon wrote: >> Add usb20 to the list of clock supported by PM runtime. >> >> Signed-off-by: Alexandre Bailon>> --- >> arch/arm/mach-davinci/pm_domain.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/mach-davinci/pm_domain.c >> b/arch/arm/mach-davinci/pm_domain.c >> index 78eac2c..66471f2 100644 >> --- a/arch/arm/mach-davinci/pm_domain.c >> +++ b/arch/arm/mach-davinci/pm_domain.c >> @@ -23,7 +23,7 @@ static struct dev_pm_domain davinci_pm_domain = { >> >> static struct pm_clk_notifier_block platform_bus_notifier = { >> .pm_domain = _pm_domain, >> -.con_ids = { "fck", "master", "slave", NULL }, >> +.con_ids = { "fck", "master", "slave", "usb20", NULL }, > > Instead of doing this, can we drop the con_id from musb clock? Looking > at the USB clocking diagram in the TRM. There is a single clock input to > the USB 2.0 subsystem. There is no real need for a con_id at all. Currently, the con_id is required to get the usb20 clock from usb-da8xx.c I will try to figure out which changes are required remove con_id. >>> >>> It most probably should be renamed to "fck" then it should work with your >>> patch "[PATCH v3 5/5] usb: musb: da8xx: Add a primary support of PM >>> runtime". > >> Actually, because of the USB phy, more changes are required. >> Something like that works for me. > > There are too many things going on in your patch and I am not clear why > all of those changes are needed. The following diff worked for me on top > of master branch of my tree. Can you check if this works for you? If not, > I need some more explanation of why you are making all the changes that > you are. Your patch works. I guess I have forgotten something when I have tried to do it by myself because the phy was not working. Thanks, Alexandre > > Thanks, > Sekhar > > ---8<--- > diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c > index 073c458d0c67..2cfd9d70a818 100644 > --- a/arch/arm/mach-davinci/da830.c > +++ b/arch/arm/mach-davinci/da830.c > @@ -412,7 +412,7 @@ static struct clk_lookup da830_clks[] = { > CLK("davinci-mcasp.0", NULL, _clk), > CLK("davinci-mcasp.1", NULL, _clk), > CLK("davinci-mcasp.2", NULL, _clk), > - CLK("musb-da8xx", "usb20",_clk), > + CLK("musb-da8xx", NULL, _clk), > CLK(NULL, "aemif",_clk), > CLK(NULL, "aintc",_clk), > CLK(NULL, "secu_mgr", _mgr_clk), > diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c > index 55f6e1172517..946cd06d8c05 100644 > --- a/arch/arm/mach-davinci/da850.c > +++ b/arch/arm/mach-davinci/da850.c > @@ -567,7 +567,7 @@ static struct clk_lookup da850_clks[] = { >*/ > CLK(NULL, "aemif",_nand_clk), > CLK("ohci-da8xx", "usb11",_clk), > - CLK("musb-da8xx", "usb20",_clk), > + CLK("musb-da8xx", NULL, _clk), > CLK("spi_davinci.0",NULL, _clk), > CLK("spi_davinci.1",NULL, _clk), > CLK("vpif", NULL, _clk), > diff --git a/arch/arm/mach-davinci/usb-da8xx.c > b/arch/arm/mach-davinci/usb-da8xx.c > index 9a6af0bd5dc3..9b667689b665 100644 > --- a/arch/arm/mach-davinci/usb-da8xx.c > +++ b/arch/arm/mach-davinci/usb-da8xx.c > @@ -275,7 +275,7 @@ int __init da8xx_register_usb20_phy_clk(bool > use_usb_refclkin) > struct clk *parent; > int ret; > > - usb20_clk = clk_get(_usb20_dev.dev, "usb20"); > + usb20_clk = clk_get(_usb20_dev.dev, NULL); > ret = PTR_ERR_OR_ZERO(usb20_clk); > if (ret) > return ret; > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c > index e89708d839e5..8f6f0efd978e 100644 > --- a/drivers/usb/musb/da8xx.c > +++ b/drivers/usb/musb/da8xx.c > @@ -502,7 +502,7 @@ static int da8xx_probe(struct platform_device *pdev) > if (!glue) > return -ENOMEM; > > - clk = devm_clk_get(>dev, "usb20"); > + clk = devm_clk_get(>dev, NULL); > if (IS_ERR(clk)) { > dev_err(>dev, "failed to get clock\n"); > return PTR_ERR(clk); > -- 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: dwc3: add quirk to handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS
On 19/01/17 22:49, John Youn wrote: > So it is valid to have say, DWC_USB3_NUM_EPS=8 and > DWC_USB3_NUM_IN_EPS=8. Even though it is not possible to use all 8 EPs > as IN since you need at least one of them to be a control OUT. So you > could have a configuration of EP0 IN and OUT plus 6 IN EPs (total 1 > OUT, 7 IN). Or EP0 IN and OUT plus any other combination of IN/OUT for > the remaining 6 EPs. > > With the above in mind, you can probably just redo the endpoint number > logic in dwc3 to handle all cases without any quirks at all. I thought of that. > If it is disabled, it will fix the physical and logical EP number and > direction of the available EPs to meet timings for FPGA designs. This > shouldn't be used in final designs for ASICS but we don't know for > sure whether it has made it to any final designs or not. > > And unfortunately, whether this is set or not is not visible to the > software so it will require a quirk. but arrived at this conclusion because I couldn't think of a reasonable guess value for IN/OUT endpoint numbers that would work if DWC_USB3_NUM_EPS == DWC_USB3_NUM_IN_EPS thanks --- bod -- 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] arm: davinci: Make the usb20 clock available to PM runtime
On Thursday 19 January 2017 11:01 PM, Alexandre Bailon wrote: > On 01/19/2017 05:49 PM, Grygorii Strashko wrote: >> On 01/19/2017 09:08 AM, Alexandre Bailon wrote: >>> On 01/19/2017 03:48 PM, Sekhar Nori wrote: On Thursday 19 January 2017 07:39 PM, Alexandre Bailon wrote: > Add usb20 to the list of clock supported by PM runtime. > > Signed-off-by: Alexandre Bailon> --- > arch/arm/mach-davinci/pm_domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mach-davinci/pm_domain.c > b/arch/arm/mach-davinci/pm_domain.c > index 78eac2c..66471f2 100644 > --- a/arch/arm/mach-davinci/pm_domain.c > +++ b/arch/arm/mach-davinci/pm_domain.c > @@ -23,7 +23,7 @@ static struct dev_pm_domain davinci_pm_domain = { > > static struct pm_clk_notifier_block platform_bus_notifier = { > .pm_domain = _pm_domain, > - .con_ids = { "fck", "master", "slave", NULL }, > + .con_ids = { "fck", "master", "slave", "usb20", NULL }, Instead of doing this, can we drop the con_id from musb clock? Looking at the USB clocking diagram in the TRM. There is a single clock input to the USB 2.0 subsystem. There is no real need for a con_id at all. >>> Currently, the con_id is required to get the usb20 clock from usb-da8xx.c >>> I will try to figure out which changes are required remove con_id. >> >> It most probably should be renamed to "fck" then it should work with your >> patch "[PATCH v3 5/5] usb: musb: da8xx: Add a primary support of PM runtime". > Actually, because of the USB phy, more changes are required. > Something like that works for me. There are too many things going on in your patch and I am not clear why all of those changes are needed. The following diff worked for me on top of master branch of my tree. Can you check if this works for you? If not, I need some more explanation of why you are making all the changes that you are. Thanks, Sekhar ---8<--- diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c index 073c458d0c67..2cfd9d70a818 100644 --- a/arch/arm/mach-davinci/da830.c +++ b/arch/arm/mach-davinci/da830.c @@ -412,7 +412,7 @@ static struct clk_lookup da830_clks[] = { CLK("davinci-mcasp.0", NULL, _clk), CLK("davinci-mcasp.1", NULL, _clk), CLK("davinci-mcasp.2", NULL, _clk), - CLK("musb-da8xx", "usb20",_clk), + CLK("musb-da8xx", NULL, _clk), CLK(NULL, "aemif",_clk), CLK(NULL, "aintc",_clk), CLK(NULL, "secu_mgr", _mgr_clk), diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c index 55f6e1172517..946cd06d8c05 100644 --- a/arch/arm/mach-davinci/da850.c +++ b/arch/arm/mach-davinci/da850.c @@ -567,7 +567,7 @@ static struct clk_lookup da850_clks[] = { */ CLK(NULL, "aemif",_nand_clk), CLK("ohci-da8xx", "usb11",_clk), - CLK("musb-da8xx", "usb20",_clk), + CLK("musb-da8xx", NULL, _clk), CLK("spi_davinci.0",NULL, _clk), CLK("spi_davinci.1",NULL, _clk), CLK("vpif", NULL, _clk), diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c index 9a6af0bd5dc3..9b667689b665 100644 --- a/arch/arm/mach-davinci/usb-da8xx.c +++ b/arch/arm/mach-davinci/usb-da8xx.c @@ -275,7 +275,7 @@ int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin) struct clk *parent; int ret; - usb20_clk = clk_get(_usb20_dev.dev, "usb20"); + usb20_clk = clk_get(_usb20_dev.dev, NULL); ret = PTR_ERR_OR_ZERO(usb20_clk); if (ret) return ret; diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c index e89708d839e5..8f6f0efd978e 100644 --- a/drivers/usb/musb/da8xx.c +++ b/drivers/usb/musb/da8xx.c @@ -502,7 +502,7 @@ static int da8xx_probe(struct platform_device *pdev) if (!glue) return -ENOMEM; - clk = devm_clk_get(>dev, "usb20"); + clk = devm_clk_get(>dev, NULL); if (IS_ERR(clk)) { dev_err(>dev, "failed to get clock\n"); return PTR_ERR(clk); -- 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 resend v5 0/4] xhci fixes for usb-linus
On 19.01.2017 20:48, Greg KH wrote: On Thu, Jan 19, 2017 at 02:21:26PM +0200, Mathias Nyman wrote: Hi Greg This series by Arnd Bergmann was originally six patches, but last two of them were already taken to 4.10. Without the rest of them there will be a regression in 4.10. Is it really a regression? I thought this had never worked before in older kernels, right? Regression when xhci hosts in dwc3 controllers are used. For example patch 5/6 removed setting dma mask for xhci in dwc3 host init: +++ b/drivers/usb/dwc3/host.c @@ -84,11 +84,7 @@ int dwc3_host_init(struct dwc3 *dwc) return -ENOMEM; } - dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask); - xhci->dev.parent = dwc->dev; - xhci->dev.dma_mask = dwc->dev->dma_mask; - xhci->dev.dma_parms = dwc->dev->dma_parms; So now xhci platform driver prints a scary warning because of the missing dma mask: static int xhci_plat_probe(struct platform_device *pdev) /* Try to set 64-bit DMA first */ if (WARN_ON(!pdev->dev.dma_mask)) /* Platform did not initialize dma_mask */ ret = dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64)); else ... This is fixed in the first 4 patches. There might be other other issues as well caused by having only the dwc3 changed applied of this series, but not the core and xhci parts -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 v11 08/14] usb: otg: add OTG/dual-role core
Vivek, On 19/01/17 17:15, vivek.gau...@codeaurora.org wrote: > Hi Roger, > > On 2017-01-19 17:45, Roger Quadros wrote: >> Vivek, >> >> On 19/01/17 13:56, Vivek Gautam wrote: >>> Hi, >>> >>> >>> On Wed, Jun 22, 2016 at 2:00 PM, Roger Quadroswrote: >>> >>> Luckily hit this thread while checking about DRD role functionality for >>> DWC3. >>> On 22/06/16 11:14, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >> For the real use case, some Carplay platforms need it. > > Carplay does *NOT* rely on OTG. Apple has its own proprietary and > closed > specification which is not OTG-compliant. > Yes, it is not OTG-compliant, but it can co-work with some standard OTG FSM states to finish role swap. >>> >>> What are you referring to as "finish role swap"? I don't get that. >> >> Change current role from host to peripheral. > > Okay, we have two scenarios here: > > 1. You need full OTG compliance > > For this, granted, you need the state machine if your HW doesn't > track it. This is a given. With only one user, however, perhaps > we don't need a generic layer. There are not enough different > setups to design a good enough generic layer. We will end up > with a pseudo-generic framework which is coupled with its only > user. > > 2. Dual-role support, without OTG compliance > > In this case, you don't need a stack. All you need is a signal > to tell you state of ID pin and another to tell you state of > VBUS level. If you have those, you don't need to walk an OTG > state machine at all. You don't need any of those quirky OTG > timers, agreed? > > Given the above, why would you even want to use a subset of OTG > state machine to implement something that's _usually_ as simple > as: > > 8<-- > vbus = read(VBUS_STATE); /* could be a gpio_get_value() */ > id = read(ID_STATE); /* could be a gpio_get_value() */ > > set_role(id); > set_vbus(vbus); > > In fact, the individual driver can do it by itself. The chipidea driver handles OTG and dual-role well currently. By considering this OTG/DRD framework is worthwhile or not, we would like to see if it can simplify DRD design for each driver, and can benefit the platforms which has different drivers for host and peripheral to finish the role switch well. >>> >>> simplify how? By adding unnecessary workqueues and a level indirection >>> that just goes back to the same driver? >> >> What do you mean by same driver? > > dwc3 registers to OTG layer. dwc3 also registers as UDC to UDC > layer. When dwc3 OTG IRQ fires, dwc3 tells OTG layer about it and OTG > layer jumps to a callback that goes back to dwc3 to e.g. start > peripheral side. > > See ?!? Starts on dwc3, goes to OTG layer, goes back to DWC3. > >> Gadget driver, host driver and PHY (or MUX) driver (for ID/VBUS) can >> be 3 totally independent drivers unlike dwc3 where you have a single >> driver in control of both host and gadget. > > That's a totally different issue and one not being tackled by OTG > layer, because there are no such users yet. We can't design anything > based solely on speculation of what might happen. > > If there aren't enough users, there is no way to design a good generic > layer. > >> Questions not clear to me are: >> >> 1) Which driver handles ID/VBUS events and makes a decision to do the >> role swap? Probably the PHY/MUX driver? > > This is implementation dependent. For TI's USB subsystem, we have PMIC > sampling VBUS/ID that and using EXTCON to tell dwc3-omap to program UTMI > mailbox. The same mailbox can be used in HW-mode (see AM437x) where SW > has no intervention. > > For Intel's USB subsystem, we have PMIC sampling VBUS/ID with an > internal mux (much like TI's UTMI mailbox, but slightly different) to > switch between a separate XHCI or a separate dwc3. The same mux can be > put in HW-mode where SW has no intervention. > > In any case, for Intel's stuff most of the magic happens in ASL. Our PHY > driver just detects role (at least for Type-C based plats) and executes > _DSM with correct arguments [1]. _DSM will program internal MUX, toggle > VBUS and, for type-C, toggle VCONN when needed. >
Re: unable to handle EVENT RING FULL error in xhci
On Fri, Jan 20, 2017 at 07:59:48AM +, Anurag Kumar Vulisha wrote: > Hi, Hi! Minor nit, vger.kernel.org rejects html emails, so this didn't make it through to that list. Now it should with my response :) > I have a usecase where I want to stream a stereo video from the ZED Stereo > Camera (www.stereolabs.com) by connecting it to our ZynqMP SOC based > platform > using the USB 3.0 interface. This camera keeps on streaming 48k chunks of BULK > data to the HOST. Initially the data is received correctly, but after some > time > I noticed frame corruption on the BULK packets received by HOST controller. On > debugging I found that HOST event ring is full and it issues “EVENT RING FULL > ERROR” and this error seems to be not handled in XHCI. I am using linux 4.6 > kernel for testing . Please help me in by giving any suggestions on how to > handle this issue? Currently I increased the TRB_PER_SEGMENT from 256 to 512 > but the issue seems to delayed and gets reproduced after some more time. > > To increase the TRB_PER_SEGMENT from 256 to 512 I made the below changes > > xhci->segment_pool = dma_pool_create("xHCI ring segments", dev, > TRB_SEGMENT_SIZE, TRB_SEGMENT_SIZE, xhci->page_size * 4); > > Is this the correct way ? or I am doing wrong. > > It would be very helpful if you can help me in solving this issue 4.6 is very old and obsolete and behind in a lot of xhci fixes. Can you test 4.9.4 or even better yet, Linus's 4.10-rc4 tree to see if the issue is still there? 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 v2, 4/6] arm64: dts: mt8173: split usb SuperSpeed port into two ports
split the old SuperSpeed port node into a HighSpeed one and a new SuperSpeed one. Signed-off-by: Chunfeng Yun--- arch/arm64/boot/dts/mediatek/mt8173.dtsi | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi index 12e7027..1074ed2 100644 --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi @@ -724,8 +724,9 @@ <0 0x11280700 0 0x0100>; reg-names = "mac", "ippc"; interrupts = ; - phys = <_port0 PHY_TYPE_USB3>, - <_port1 PHY_TYPE_USB2>; + phys = < PHY_TYPE_USB2>, + < PHY_TYPE_USB3>, + < PHY_TYPE_USB2>; power-domains = < MT8173_POWER_DOMAIN_USB>; clocks = < CLK_TOP_USB30_SEL>, < CLK_PERI_USB0>, @@ -761,14 +762,20 @@ ranges; status = "okay"; - phy_port0: port@11290800 { - reg = <0 0x11290800 0 0x800>; + u2port0: port@11290800 { + reg = <0 0x11290800 0 0x100>; #phy-cells = <1>; status = "okay"; }; - phy_port1: port@11291000 { - reg = <0 0x11291000 0 0x800>; + u3port0: port@11290900 { + reg = <0 0x11290900 0 0x700>; + #phy-cells = <1>; + status = "okay"; + }; + + u2port1: port@11291000 { + reg = <0 0x11291000 0 0x100>; #phy-cells = <1>; status = "okay"; }; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2, 2/6] phy: phy-mt65xx-usb3: move clock from phy node into port nodes
the reference clock of HighSpeed port is 48M which comes from PLL; the reference clock of SuperSpeed port is 26M which usually comes from 26M oscillator directly, but some SoCs are not, add it for compatibility, and put them into port node for flexibility. Signed-off-by: Chunfeng Yun--- drivers/phy/phy-mt65xx-usb3.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c index 93f57d9..0995433 100644 --- a/drivers/phy/phy-mt65xx-usb3.c +++ b/drivers/phy/phy-mt65xx-usb3.c @@ -141,6 +141,7 @@ struct mt65xx_phy_pdata { struct mt65xx_phy_instance { struct phy *phy; void __iomem *port_base; + struct clk *ref_clk;/* reference clock of anolog phy */ u32 index; u8 type; }; @@ -148,7 +149,6 @@ struct mt65xx_phy_instance { struct mt65xx_u3phy { struct device *dev; void __iomem *sif_base; /* only shared sif */ - struct clk *u3phya_ref; /* reference clock of usb3 anolog phy */ const struct mt65xx_phy_pdata *pdata; struct mt65xx_phy_instance **phys; int nphys; @@ -422,9 +422,9 @@ static int mt65xx_phy_init(struct phy *phy) struct mt65xx_u3phy *u3phy = dev_get_drvdata(phy->dev.parent); int ret; - ret = clk_prepare_enable(u3phy->u3phya_ref); + ret = clk_prepare_enable(instance->ref_clk); if (ret) { - dev_err(u3phy->dev, "failed to enable u3phya_ref\n"); + dev_err(u3phy->dev, "failed to enable ref_clk\n"); return ret; } @@ -467,7 +467,7 @@ static int mt65xx_phy_exit(struct phy *phy) if (instance->type == PHY_TYPE_USB2) phy_instance_exit(u3phy, instance); - clk_disable_unprepare(u3phy->u3phya_ref); + clk_disable_unprepare(instance->ref_clk); return 0; } @@ -567,12 +567,6 @@ static int mt65xx_u3phy_probe(struct platform_device *pdev) return PTR_ERR(u3phy->sif_base); } - u3phy->u3phya_ref = devm_clk_get(dev, "u3phya_ref"); - if (IS_ERR(u3phy->u3phya_ref)) { - dev_err(dev, "error to get u3phya_ref\n"); - return PTR_ERR(u3phy->u3phya_ref); - } - port = 0; for_each_child_of_node(np, child_np) { struct mt65xx_phy_instance *instance; @@ -607,6 +601,13 @@ static int mt65xx_u3phy_probe(struct platform_device *pdev) goto put_child; } + instance->ref_clk = devm_clk_get(>dev, "ref_clk"); + if (IS_ERR(instance->ref_clk)) { + dev_err(dev, "failed to get ref_clk(id-%d)\n", port); + retval = PTR_ERR(instance->ref_clk); + goto put_child; + } + instance->phy = phy; instance->index = port; phy_set_drvdata(phy, instance); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2, 3/6] phy: phy-mt65xx-usb3: add support for new version phy
There are some variations from mt2701 to mt2712: 1. banks shared by multiple ports are put back into each port, such as SPLLC and U2FREQ; 2. add a new bank MISC for u2port, and CHIP for u3port; 3. bank's offset in each port are also rearranged; Signed-off-by: Chunfeng Yun--- drivers/phy/phy-mt65xx-usb3.c | 326 ++--- 1 file changed, 208 insertions(+), 118 deletions(-) diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c index 0995433..462e02b 100644 --- a/drivers/phy/phy-mt65xx-usb3.c +++ b/drivers/phy/phy-mt65xx-usb3.c @@ -23,46 +23,54 @@ #include #include -/* - * for sifslv2 register, but exclude port's; - * relative to USB3_SIF2_BASE base address - */ -#define SSUSB_SIFSLV_SPLLC 0x -#define SSUSB_SIFSLV_U2FREQ0x0100 - -/* offsets of banks in each u2phy registers */ -#define SSUSB_SIFSLV_U2PHY_COM_BASE0x -/* offsets of banks in each u3phy registers */ -#define SSUSB_SIFSLV_U3PHYD_BASE 0x -#define SSUSB_SIFSLV_U3PHYA_BASE 0x0200 - -#define U3P_USBPHYACR0 (SSUSB_SIFSLV_U2PHY_COM_BASE + 0x) +/* version V1 sub-banks offset base address */ +/* banks shared by multiple phys */ +#define SSUSB_SIFSLV_V1_SPLLC 0x000 /* shared by u3 phys */ +#define SSUSB_SIFSLV_V1_U2FREQ 0x100 /* shared by u2 phys */ +/* u2 phy bank */ +#define SSUSB_SIFSLV_V1_U2PHY_COM 0x000 +/* u3 phy banks */ +#define SSUSB_SIFSLV_V1_U3PHYD 0x000 +#define SSUSB_SIFSLV_V1_U3PHYA 0x200 + +/* version V2 sub-banks offset base address */ +/* u2 phy banks */ +#define SSUSB_SIFSLV_V2_MISC 0x000 +#define SSUSB_SIFSLV_V2_U2FREQ 0x100 +#define SSUSB_SIFSLV_V2_U2PHY_COM 0x300 +/* u3 phy banks */ +#define SSUSB_SIFSLV_V2_SPLLC 0x000 +#define SSUSB_SIFSLV_V2_CHIP 0x100 +#define SSUSB_SIFSLV_V2_U3PHYD 0x200 +#define SSUSB_SIFSLV_V2_U3PHYA 0x400 + +#define U3P_USBPHYACR0 0x000 #define PA0_RG_U2PLL_FORCE_ON BIT(15) -#define U3P_USBPHYACR2 (SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0008) +#define U3P_USBPHYACR2 0x008 #define PA2_RG_SIF_U2PLL_FORCE_EN BIT(18) -#define U3P_USBPHYACR5 (SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0014) +#define U3P_USBPHYACR5 0x014 #define PA5_RG_U2_HSTX_SRCAL_ENBIT(15) #define PA5_RG_U2_HSTX_SRCTRL GENMASK(14, 12) #define PA5_RG_U2_HSTX_SRCTRL_VAL(x) ((0x7 & (x)) << 12) #define PA5_RG_U2_HS_100U_U3_ENBIT(11) -#define U3P_USBPHYACR6 (SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0018) +#define U3P_USBPHYACR6 0x018 #define PA6_RG_U2_BC11_SW_EN BIT(23) #define PA6_RG_U2_OTG_VBUSCMP_EN BIT(20) #define PA6_RG_U2_SQTH GENMASK(3, 0) #define PA6_RG_U2_SQTH_VAL(x) (0xf & (x)) -#define U3P_U2PHYACR4 (SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0020) +#define U3P_U2PHYACR4 0x020 #define P2C_RG_USB20_GPIO_CTL BIT(9) #define P2C_USB20_GPIO_MODEBIT(8) #define P2C_U2_GPIO_CTR_MSK(P2C_RG_USB20_GPIO_CTL | P2C_USB20_GPIO_MODE) -#define U3D_U2PHYDCR0 (SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0060) +#define U3D_U2PHYDCR0 0x060 #define P2C_RG_SIF_U2PLL_FORCE_ON BIT(24) -#define U3P_U2PHYDTM0 (SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0068) +#define U3P_U2PHYDTM0 0x068 #define P2C_FORCE_UART_EN BIT(26) #define P2C_FORCE_DATAIN BIT(23) #define P2C_FORCE_DM_PULLDOWN BIT(21) @@ -84,47 +92,44 @@ P2C_FORCE_TERMSEL | P2C_RG_DMPULLDOWN | \ P2C_RG_DPPULLDOWN | P2C_RG_TERMSEL) -#define U3P_U2PHYDTM1 (SSUSB_SIFSLV_U2PHY_COM_BASE + 0x006C) +#define U3P_U2PHYDTM1 0x06C #define P2C_RG_UART_EN BIT(16) #define P2C_RG_VBUSVALID BIT(5) #define P2C_RG_SESSEND BIT(4) #define P2C_RG_AVALID BIT(2) -#define U3P_U3_PHYA_REG0 (SSUSB_SIFSLV_U3PHYA_BASE + 0x) -#define P3A_RG_U3_VUSB10_ONBIT(5) - -#define U3P_U3_PHYA_REG6 (SSUSB_SIFSLV_U3PHYA_BASE + 0x0018) +#define U3P_U3_PHYA_REG6 0x018 #define P3A_RG_TX_EIDLE_CM GENMASK(31, 28) #define P3A_RG_TX_EIDLE_CM_VAL(x) ((0xf & (x)) << 28) -#define U3P_U3_PHYA_REG9 (SSUSB_SIFSLV_U3PHYA_BASE + 0x0024) +#define U3P_U3_PHYA_REG9 0x024 #define P3A_RG_RX_DAC_MUX GENMASK(5, 1) #define P3A_RG_RX_DAC_MUX_VAL(x) ((0x1f & (x)) << 1) -#define U3P_U3PHYA_DA_REG0 (SSUSB_SIFSLV_U3PHYA_BASE + 0x0100) +#define U3P_U3_PHYA_DA_REG00x100 #define P3A_RG_XTAL_EXT_EN_U3 GENMASK(11, 10) #define P3A_RG_XTAL_EXT_EN_U3_VAL(x) ((0x3 & (x)) << 10) -#define U3P_PHYD_CDR1 (SSUSB_SIFSLV_U3PHYD_BASE + 0x005c) +#define U3P_U3_PHYD_CDR1 0x05c #define P3D_RG_CDR_BIR_LTD1GENMASK(28, 24) #define P3D_RG_CDR_BIR_LTD1_VAL(x) ((0x1f & (x)) <<