RE: [PATCH v3 2/4] libusbg: Fix buffer overrun issue. CID#56128
> -Original Message- > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > ow...@vger.kernel.org] On Behalf Of philippedesw...@gmail.com > Sent: Saturday, May 24, 2014 7:29 PM > To: philippedesw...@gmail.com; linux-usb@vger.kernel.org > Subject: [PATCH v3 2/4] libusbg: Fix buffer overrun issue. > CID#56128 > > From: Philippe De Swert > > Avoid calling usbg_read_string() with a 40 byte long buffer, which > in turn is filled in > by usbg_read_buf() which uses a maximum of 256 bytes > (USBG_MAX_STR_LENGTH). This adjusts > the buffer to be the right size. > > Signed-off-by: Philippe De Swert Reviewed-by: Krzysztof Opasiak > --- > src/usbg.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/usbg.c b/src/usbg.c > index 5d9c083..66aa435 100644 > --- a/src/usbg.c > +++ b/src/usbg.c > @@ -681,7 +681,7 @@ static int > usbg_parse_function_net_attrs(usbg_function *f, > usbg_function_attrs *f_attrs) > { > struct ether_addr *addr; > - char str_addr[40]; > + char str_addr[USBG_MAX_STR_LENGTH]; > int ret; > > ret = usbg_read_string(f->path, f->name, "dev_addr", > str_addr); > -- > 1.8.1.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux- > usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/4] libusbg: Fix readlink/buffer overrun issue. CID#56130, CID#56129
> -Original Message- > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > ow...@vger.kernel.org] On Behalf Of philippedesw...@gmail.com > Sent: Saturday, May 24, 2014 7:29 PM > To: philippedesw...@gmail.com; linux-usb@vger.kernel.org > Subject: [PATCH v3 1/4] libusbg: Fix readlink/buffer overrun issue. > CID#56130, CID#56129 > > From: Philippe De Swert > > Readlink() can return the total length of the buffer (here > 4096/USBG_MAX_PATH_LENGTH), > so we do not want to dereference target[4096] as that would give an > off by one error. > > Signed-off-by: Philippe De Swert Reviewed-by: Krzysztof Opasiak > --- > src/usbg.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/usbg.c b/src/usbg.c > index d73943c..5d9c083 100644 > --- a/src/usbg.c > +++ b/src/usbg.c > @@ -850,7 +850,7 @@ static int > usbg_parse_config_binding(usbg_config *c, char *bpath, > usbg_function *f; > usbg_binding *b; > > - nmb = readlink(bpath, target, sizeof(target)); > + nmb = readlink(bpath, target, sizeof(target) - 1 ); > if (nmb < 0) { > ret = usbg_translate_error(errno); > goto out; > -- > 1.8.1.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux- > usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/4] libusbg: Do not try to dereference func when it is NULL. CID#56127
> -Original Message- > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > ow...@vger.kernel.org] On Behalf Of philippedesw...@gmail.com > Sent: Saturday, May 24, 2014 7:29 PM > To: philippedesw...@gmail.com; linux-usb@vger.kernel.org > Subject: [PATCH v3 3/4] libusbg: Do not try to dereference func > when it is NULL. CID#56127 > > From: Philippe De Swert > > We check if func is NULL, so if the allocation function failed we > should > not dereference or handle it anymore, so we jump straight to the > end. > > Signed-off-by: Philippe De Swert Reviewed-by: Krzysztof Opasiak > --- > src/usbg.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/usbg.c b/src/usbg.c > index 66aa435..8ad6a9e 100644 > --- a/src/usbg.c > +++ b/src/usbg.c > @@ -1653,6 +1653,7 @@ int usbg_create_function(usbg_gadget *g, > usbg_function_type type, > if (!func) { > ERRORNO("allocating function\n"); > ret = USBG_ERROR_NO_MEM; > + goto out; > } > > free_space = sizeof(fpath) - n; > -- > 1.8.1.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux- > usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/4] libusbg: Do not dereference usb config attributes when they are NULL. CID#56126
> -Original Message- > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > ow...@vger.kernel.org] On Behalf Of philippedesw...@gmail.com > Sent: Saturday, May 24, 2014 7:29 PM > To: philippedesw...@gmail.com; linux-usb@vger.kernel.org > Subject: [PATCH v3 4/4] libusbg: Do not dereference usb config > attributes when they are NULL. CID#56126 > > From: Philippe De Swert > > We probably need to check if we get valid attributes passed. > Otherwise we will > try to dereference a NULL-pointer as the usb_config_attr will not > be valid. > > Signed-off-by: Philippe De Swert Reviewed-by: Krzysztof Opasiak > --- > src/usbg.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/usbg.c b/src/usbg.c > index 8ad6a9e..f7bd7aa 100644 > --- a/src/usbg.c > +++ b/src/usbg.c > @@ -1787,7 +1787,7 @@ int usbg_set_config_attrs(usbg_config *c, > usbg_config_attrs *c_attrs) > { > int ret = USBG_ERROR_INVALID_PARAM; > > - if (c && !c_attrs) { > + if (c && c_attrs) { > ret = usbg_write_dec(c->path, c->name, "MaxPower", > c_attrs->bMaxPower); > if (ret == USBG_SUCCESS) > ret = usbg_write_hex8(c->path, c->name, > "bmAttributes", > -- > 1.8.1.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux- > usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/7] usb: musb: introduce dma_channel.packet_done
On 05/26/2014 08:58 AM, George Cherian wrote: > On 5/25/2014 2:06 PM, Daniel Mack wrote: >> The musb/cppi41 glue layer is capable of handling transactions that span >> over more than one USB packet by reloading the DMA descriptors >> partially. An urb is considered completed when either its transfer >> buffer has been filled entirely (actual_length == >> transfer_buffer_length) or if a packet in the stream has less bytes than >> the endpoint's wMaxPacketSize. >> >> Once one of the above conditions is met, musb_dma_completion() is called >> from cppi41_trans_done(). However, the final decision whether or not to >> return the urb to its owner is made by the core and its determination of >> the variable 'done' in musb_host_rx(). This code has currently no way of >> knowing what the size of the last packet was, and whether or not to >> give back the urb due to a short read. >> >> Fix this by introducing a new boolean flag in 'struct dma_channel', and >> set it from musb_cppi41.c. If set, it will make the core do what the >> DMA layer decided and complete the urb. >> >> Signed-off-by: Daniel Mack > Since this is used only for rx, Can you name newly added dma flag as > rx_packet_done? Good idea. Will be part of v4, along with Sergei's s/CSR0/TXCSR/. Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] usb: musb: core: Handle Babble condition only in HOST mode
On 5/23/2014 2:12 AM, Bin Liu wrote: Hi George, On Mon, May 19, 2014 at 11:32 PM, George Cherian wrote: Hi Bin, On 5/19/2014 9:24 PM, Bin Liu wrote: Hi, On Mon, May 19, 2014 at 8:39 AM, George Cherian wrote: BABBLE and RESET share the same interrupt. The interrupt is considered to be RESET if MUSB is in peripheral mode and as a BABBLE if MUSB is in HOST mode. Handle babble condition iff MUSB is in HOST mode. Signed-off-by: George Cherian --- drivers/usb/musb/musb_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 61da471..eff3c5c 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -849,7 +849,7 @@ b_host: } /* handle babble condition */ - if (int_usb & MUSB_INTR_BABBLE) + if (int_usb & MUSB_INTR_BABBLE && is_host_active(musb)) schedule_work(&musb->recover_work); I guess my following comments are for Daniel's patch as while which initially added the babble work. Should this if statement be merged into the previous 'if(int_usb & MUSB_INTR_RESET)' one, which handles the same interrupt and already handles host and device mode respectively. Initially I too had the babble handling as part of 'if(int_usb & MUSB_INTR_RESET)' one. But during my tests I hit a corner case where in we hit a BABBLE condition on disconnect. In such case the babble interrupt can be handled only if we have a seperate check, else its considered as a BUS RESET. When all devices are disconnected MUSB_DEVCTL_HM = 0 and the code always enter the else path. In this path it treats the BABBLE as a BUS RESET. The code flow is a bit confusing, two if() handle the same interrupt. The second one implied using 'handled = IRQ_HANDLED;' from the first one. Also does the switch() in else{} in the first if() cause any side effect? No it doesn't. Since this babble handing is AM335x specific, how about handle it in dsps_interrupt() in musb_dsps.c, which already has an entry for babble interrupt? TI 3.2 kernel does this way. That the reason we have platform specific callbacks added from the main interrupt handler. Regards, -Bin. Regards, -Bin. #if 0 -- 1.8.3.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 -- -George -- -George -- 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 v6 3/5] usb: musb: dsps: Call usb_phy(_shutdown/_init) during musb_platform_reset()
For DSPS platform usb_phy_vbus(_off/_on) are NOPs. So during musb_platform_reset() call usb_phy(_shutdown/_init) Signed-off-by: George Cherian --- drivers/usb/musb/musb_dsps.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 51beb13..74c4193 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -543,7 +543,11 @@ static void dsps_musb_reset(struct musb *musb) const struct dsps_musb_wrapper *wrp = glue->wrp; dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset)); - udelay(100); + usleep_range(100, 200); + usb_phy_shutdown(musb->xceiv); + usleep_range(100, 200); + usb_phy_init(musb->xceiv); + } static struct musb_platform_ops dsps_ops = { -- 1.8.3.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 v6 4/5] usb: musb: core: Convert the musb_platform_reset to have a return value.
Currently musb_platform_reset() is only used by dsps. In case of BABBLE interrupt for other platforms the musb_platform_reset() is a NOP. In such situations no need to re-initialize the endpoints. Also in the latest silicon revision of AM335x, we do have a babble recovery mechanism without resetting the IP block. In preperation to add that support its better to have a rest_done return for musb_platform_reset(). Signed-off-by: George Cherian --- drivers/usb/musb/musb_core.c | 10 ++ drivers/usb/musb/musb_core.h | 10 ++ drivers/usb/musb/musb_dsps.c | 3 ++- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index c0ce09f..b841ee0 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1753,9 +1753,11 @@ static void musb_irq_work(struct work_struct *data) static void musb_recover_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, recover_work.work); - int status; + int status, ret; - musb_platform_reset(musb); + ret = musb_platform_reset(musb); + if (ret) + return; usb_phy_vbus_off(musb->xceiv); usleep_range(100, 200); @@ -1764,8 +1766,8 @@ static void musb_recover_work(struct work_struct *data) usleep_range(100, 200); /* -* When a babble condition occurs, the musb controller removes the -* session bit and the endpoint config is lost. +* When a babble condition occurs, the musb controller +* removes the session bit and the endpoint config is lost. */ if (musb->dyn_fifo) status = ep_config_from_table(musb); diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index 9241025..414e57a 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -192,7 +192,7 @@ struct musb_platform_ops { int (*set_mode)(struct musb *musb, u8 mode); void(*try_idle)(struct musb *musb, unsigned long timeout); - void(*reset)(struct musb *musb); + int (*reset)(struct musb *musb); int (*vbus_status)(struct musb *musb); void(*set_vbus)(struct musb *musb, int on); @@ -555,10 +555,12 @@ static inline void musb_platform_try_idle(struct musb *musb, musb->ops->try_idle(musb, timeout); } -static inline void musb_platform_reset(struct musb *musb) +static inline int musb_platform_reset(struct musb *musb) { - if (musb->ops->reset) - musb->ops->reset(musb); + if (!musb->ops->reset) + return -EINVAL; + + return musb->ops->reset(musb); } static inline int musb_platform_get_vbus_status(struct musb *musb) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 74c4193..f6f3087 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -536,7 +536,7 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode) return 0; } -static void dsps_musb_reset(struct musb *musb) +static int dsps_musb_reset(struct musb *musb) { struct device *dev = musb->controller; struct dsps_glue *glue = dev_get_drvdata(dev->parent); @@ -548,6 +548,7 @@ static void dsps_musb_reset(struct musb *musb) usleep_range(100, 200); usb_phy_init(musb->xceiv); + return 0; } static struct musb_platform_ops dsps_ops = { -- 1.8.3.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 v6 5/5] usb: musb: dsps: Add the sw_babble_control() and Enable for newer silicon
Add sw_babble_control() logic to differentiate between transient babble and real babble condition. Also add the SW babble control register definitions. Babble control register logic is implemented in the latest revision of AM335x. Find whether we are running on newer silicon. The babble control register reads 0x4 by default in newer silicon as opposed to 0 in old versions of AM335x. Based on this enable the sw babble control logic. Signed-off-by: George Cherian --- drivers/usb/musb/musb_dsps.c | 89 +--- drivers/usb/musb/musb_regs.h | 7 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index f6f3087..01543a9 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -136,6 +136,7 @@ struct dsps_glue { const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */ struct timer_list timer;/* otg_workaround timer */ unsigned long last_timer;/* last timer data for each instance */ + bool sw_babble_enabled; struct dsps_context context; struct debugfs_regset32 regset; @@ -469,6 +470,19 @@ static int dsps_musb_init(struct musb *musb) val &= ~(1 << wrp->otg_disable); dsps_writel(musb->ctrl_base, wrp->phy_utmi, val); + /* +* Check whether the dsps version has babble control enabled. +* In latest silicon revision the babble control logic is enabled. +* If MUSB_BABBLE_CTL returns 0x4 then we have the babble control +* logic enabled. +*/ + val = dsps_readb(musb->mregs, MUSB_BABBLE_CTL); + if (val == MUSB_BABBLE_RCV_DISABLE) { + glue->sw_babble_enabled = true; + val |= MUSB_BABBLE_SW_SESSION_CTRL; + dsps_writeb(musb->mregs, MUSB_BABBLE_CTL, val); + } + ret = dsps_musb_dbg_init(musb, glue); if (ret) return ret; @@ -536,19 +550,82 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode) return 0; } +static bool sw_babble_control(struct musb *musb) +{ + u8 babble_ctl; + bool session_restart = false; + + babble_ctl = dsps_readb(musb->mregs, MUSB_BABBLE_CTL); + dev_dbg(musb->controller, "babble: MUSB_BABBLE_CTL value %x\n", + babble_ctl); + /* +* check line monitor flag to check whether babble is +* due to noise +*/ + dev_dbg(musb->controller, "STUCK_J is %s\n", + babble_ctl & MUSB_BABBLE_STUCK_J ? "set" : "reset"); + + if (babble_ctl & MUSB_BABBLE_STUCK_J) { + int timeout = 10; + + /* +* babble is due to noise, then set transmit idle (d7 bit) +* to resume normal operation +*/ + babble_ctl = dsps_readb(musb->mregs, MUSB_BABBLE_CTL); + babble_ctl |= MUSB_BABBLE_FORCE_TXIDLE; + dsps_writeb(musb->mregs, MUSB_BABBLE_CTL, babble_ctl); + + /* wait till line monitor flag cleared */ + dev_dbg(musb->controller, "Set TXIDLE, wait J to clear\n"); + do { + babble_ctl = dsps_readb(musb->mregs, MUSB_BABBLE_CTL); + udelay(1); + } while ((babble_ctl & MUSB_BABBLE_STUCK_J) && timeout--); + + /* check whether stuck_at_j bit cleared */ + if (babble_ctl & MUSB_BABBLE_STUCK_J) { + /* +* real babble condition has occurred +* restart the controller to start the +* session again +*/ + dev_dbg(musb->controller, "J not cleared, misc (%x)\n", + babble_ctl); + session_restart = true; + } + } else { + session_restart = true; + } + + return session_restart; +} + static int dsps_musb_reset(struct musb *musb) { struct device *dev = musb->controller; struct dsps_glue *glue = dev_get_drvdata(dev->parent); const struct dsps_musb_wrapper *wrp = glue->wrp; + int session_restart = 0; - dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset)); - usleep_range(100, 200); - usb_phy_shutdown(musb->xceiv); - usleep_range(100, 200); - usb_phy_init(musb->xceiv); + if (glue->sw_babble_enabled) + session_restart = sw_babble_control(musb); + /* +* In case of new silicon version babble condition can be recovered +* without resetting the MUSB. But for older silicon versions, MUSB +* reset is needed +*/ + if (session_restart || !glue->sw_babble_enabled) { + dev_info(musb->controller, "Restarting MUSB to recover from Babble\n"); + dsps_writel(musb->ctrl_base, wrp->control
[PATCH v6 0/5] Add support for SW babble Control
Series add support for SW babble control logic found in new silicon versions of AM335x. Runtime differentiation of silicon version is done by checking the BABBLE_CTL register. For newer silicon the register default value read is 0x4 and for older versions its 0x0. Patch 1 -> Handle Babble only if MUSB is in HOST mode Patch 2 -> Convert recover work to delayed work. Patch 3 -> usb_phy_vbus_(off/_on) are NOPs for am335x PHY so use usb_phy(_shutdown/_init) in musb_platform_reset() Patch 4 -> Add return value for musb_platform_reset() in prepration to support SW babble_ctrl Patch 5 -> Add and Enable sw babble control for newer silicon v5 -> v6 : Squash patch 5 and 6 form v5 to avoid build warnings. v4 -> v5 : Added a debug print before resetting MUSB. changed a musb_readb to dsps_readb introduced in Patch#5 of v4. v3 -> v4 : Fixes an issue in gagdet mode - BUS RESET should not be handled as a BABBLE. Added a check for the same.(Patch #1) Enable sw babble control properly (Patch #6) v2 -> v3 : Modify musb_platform_reset() to return zero on success. George Cherian (5): usb: musb: core: Handle Babble condition only in HOST mode usb: musb: core: Convert babble recover work to delayed work usb: musb: dsps: Call usb_phy(_shutdown/_init) during musb_platform_reset() usb: musb: core: Convert the musb_platform_reset to have a return value. usb: musb: dsps: Add the sw_babble_control() and Enable for newer silicon drivers/usb/musb/musb_core.c | 27 -- drivers/usb/musb/musb_core.h | 12 +++--- drivers/usb/musb/musb_dsps.c | 88 ++-- drivers/usb/musb/musb_regs.h | 7 4 files changed, 114 insertions(+), 20 deletions(-) -- 1.8.3.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 v6 2/5] usb: musb: core: Convert babble recover work to delayed work
During babble condition both first disconnect of devices are initiated. Make sure MUSB controller is reset and re-initialized after all disconnects. To acheive this schedule a delayed work for babble recovery. While at that convert udelay to usleep_range. Refer Documentation/timers/timers-howto.txt Signed-off-by: George Cherian --- drivers/usb/musb/musb_core.c | 15 --- drivers/usb/musb/musb_core.h | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 0ad9551..c0ce09f 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -850,7 +850,8 @@ b_host: /* handle babble condition */ if (int_usb & MUSB_INTR_BABBLE && is_host_active(musb)) - schedule_work(&musb->recover_work); + schedule_delayed_work(&musb->recover_work, + msecs_to_jiffies(100)); #if 0 /* REVISIT ... this would be for multiplexing periodic endpoints, or @@ -1751,16 +1752,16 @@ static void musb_irq_work(struct work_struct *data) /* Recover from babble interrupt conditions */ static void musb_recover_work(struct work_struct *data) { - struct musb *musb = container_of(data, struct musb, recover_work); + struct musb *musb = container_of(data, struct musb, recover_work.work); int status; musb_platform_reset(musb); usb_phy_vbus_off(musb->xceiv); - udelay(100); + usleep_range(100, 200); usb_phy_vbus_on(musb->xceiv); - udelay(100); + usleep_range(100, 200); /* * When a babble condition occurs, the musb controller removes the @@ -1943,7 +1944,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) /* Init IRQ workqueue before request_irq */ INIT_WORK(&musb->irq_work, musb_irq_work); - INIT_WORK(&musb->recover_work, musb_recover_work); + INIT_DELAYED_WORK(&musb->recover_work, musb_recover_work); INIT_DELAYED_WORK(&musb->deassert_reset_work, musb_deassert_reset); INIT_DELAYED_WORK(&musb->finish_resume_work, musb_host_finish_resume); @@ -2039,7 +2040,7 @@ fail4: fail3: cancel_work_sync(&musb->irq_work); - cancel_work_sync(&musb->recover_work); + cancel_delayed_work_sync(&musb->recover_work); cancel_delayed_work_sync(&musb->finish_resume_work); cancel_delayed_work_sync(&musb->deassert_reset_work); if (musb->dma_controller) @@ -2105,7 +2106,7 @@ static int musb_remove(struct platform_device *pdev) dma_controller_destroy(musb->dma_controller); cancel_work_sync(&musb->irq_work); - cancel_work_sync(&musb->recover_work); + cancel_delayed_work_sync(&musb->recover_work); cancel_delayed_work_sync(&musb->finish_resume_work); cancel_delayed_work_sync(&musb->deassert_reset_work); musb_free(musb); diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index d155a15..9241025 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -297,7 +297,7 @@ struct musb { irqreturn_t (*isr)(int, void *); struct work_struct irq_work; - struct work_struct recover_work; + struct delayed_work recover_work; struct delayed_work deassert_reset_work; struct delayed_work finish_resume_work; u16 hwvers; -- 1.8.3.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 v6 1/5] usb: musb: core: Handle Babble condition only in HOST mode
BABBLE and RESET share the same interrupt. The interrupt is considered to be RESET if MUSB is in peripheral mode and as a BABBLE if MUSB is in HOST mode. Handle babble condition iff MUSB is in HOST mode. Signed-off-by: George Cherian --- drivers/usb/musb/musb_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 3c6043c..0ad9551 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -849,7 +849,7 @@ b_host: } /* handle babble condition */ - if (int_usb & MUSB_INTR_BABBLE) + if (int_usb & MUSB_INTR_BABBLE && is_host_active(musb)) schedule_work(&musb->recover_work); #if 0 -- 1.8.3.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 1/5] phy: add support for USB cluster on the Armada 375 SoC
hI, On Saturday 24 May 2014 03:20 AM, Gregory CLEMENT wrote: > On 23/05/2014 11:20, Kishon Vijay Abraham I wrote: >> Hi, >> >> On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote: >>> The Armada 375 SoC comes with an USB2 host and device controller and >>> an USB3 controller. The USB cluster control register allows to manage >>> common features of both USB controllers. >>> >>> This commit adds a driver integrated in the generic PHY framework to >>> control this USB cluster feature. >>> >>> Signed-off-by: Gregory CLEMENT >>> Signed-off-by: Thomas Petazzoni >>> --- >>> drivers/phy/Kconfig | 6 + >>> drivers/phy/Makefile | 1 + >>> drivers/phy/phy-armada375-usb2.c | 140 >>> +++ >>> include/dt-bindings/phy/armada-375-usb-cluster.h | 18 +++ > [...] > >>> +static struct phy_ops armada375_usb_phy_ops = { >>> + .init = armada375_usb_phy_init, >>> + .owner = THIS_MODULE, >> >> nitpick: unnecessary tab. > > OK > >>> +}; >>> + >>> +/* >>> + * Only one controller can use this PHY. We shouldn't have the case >>> + * when two controllers want to use this PHY. But if this case occurs >>> + * then we provide a phy to the first one and return an error for the >>> + * next one. This error has also to be an error returned by >>> + * devm_phy_optional_get() so different from ENODEV for USB2. In the >>> + * USB3 case it still optional and we use ENODEV. >>> + */ >>> +static struct phy *armada375_usb_phy_xlate(struct device *dev, >>> + struct of_phandle_args *args) >>> +{ >>> + if (WARN_ON(usb_cluster_phy.phy_provided)) { >>> + dev_err(dev, "This PHY has already been provided!\n"); >>> + dev_err(dev, "Check your device tree, only one controller can >>> use it\n."); >>> + if (args->args[0] == PHY_USB2) >>> + return ERR_PTR(-EBUSY); >>> + else >>> + return ERR_PTR(-ENODEV); >>> + } >>> + >>> + if (args->args[0] == PHY_USB2) >>> + usb_cluster_phy.use_usb3 = false; >>> + else if (args->args[0] == PHY_USB3) >>> + usb_cluster_phy.use_usb3 = true; >>> + else { >>> + dev_err(dev, "Invalid PHY mode\n"); >>> + return ERR_PTR(-ENODEV); >>> + } >> >> how will this behave if there is a phy_put and then a phy_get? > > Badly I think :( > > I have to think about a better solution then. > >>> + >>> + usb_cluster_phy.phy_provided = true; >>> + >>> + return usb_cluster_phy.phy; >>> +} >>> + >>> +static int armada375_usb_phy_probe(struct platform_device *pdev) >>> +{ >>> + struct device *dev = &pdev->dev; >>> + struct phy *phy; >>> + struct phy_provider *phy_provider; >>> + void __iomem *usb_cluster_base; >>> + struct resource *res; >>> + >>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> + usb_cluster_base = devm_ioremap_resource(&pdev->dev, res); >>> + if (!usb_cluster_base) >>> + return -ENOMEM; >>> + >>> + phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL); >>> + if (IS_ERR(phy)) { >>> + dev_err(dev, "failed to create PHY \n"); >>> + return PTR_ERR(phy); >>> + } >>> + >>> + usb_cluster_phy.phy = phy; >>> + usb_cluster_phy.reg = usb_cluster_base; >>> + phy_set_drvdata(phy, &usb_cluster_phy); >>> + >>> + phy_provider = devm_of_phy_provider_register(&pdev->dev, >>> +armada375_usb_phy_xlate); >>> + if (IS_ERR(phy_provider)) >>> + return PTR_ERR(phy_provider); >>> + >>> + return 0; >>> +} >>> + >>> +static const struct of_device_id of_usb_cluster_table[] = { >>> + { .compatible = "marvell,armada-375-usb-cluster", }, >>> + { /* end of list */ }, >>> +}; >>> +MODULE_DEVICE_TABLE(of, of_usb_cluster_table); >>> + >>> +static struct platform_driver armada375_usb_phy_driver = { >>> + .probe = armada375_usb_phy_probe, >>> + .driver = { >>> + .of_match_table = of_usb_cluster_table, >>> + .name = "armada-375-usb-cluster", >>> + .owner = THIS_MODULE, >>> + } >>> +}; >>> +module_platform_driver(armada375_usb_phy_driver); >>> + >>> +MODULE_DESCRIPTION("Armada 375 USB cluster driver"); >>> +MODULE_AUTHOR("Gregory CLEMENT "); >>> +MODULE_LICENSE("GPL"); >> >> GPL v2? > > See the header, I chose "GNU General Public License version 2 or later." > so "GPL" match it. Indeed! Just figured it's documented in include/linux/module.h. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 2/8] net: cdc_ncm: always reallocate tx_curr_skb when tx_max increases
We are calling usbnet_start_xmit() to flush any remaining data, depending on the side effect that tx_curr_skb is set to NULL, ensuring a new allocation using the updated tx_max. But this side effect will only happen if there were any cached data ready to transmit. If not, then an empty tx_curr_skb is still allocated using the old tx_max size. Free it to avoid a buffer overrun. Fixes: 68864abf08f0 ("net: cdc_ncm: support rx_max/tx_max updates when running") Signed-off-by: Bjørn Mork --- drivers/net/usb/cdc_ncm.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 2bbbd65591c7..ff5b3a854898 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -268,6 +268,11 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) if (netif_running(dev->net) && val > ctx->tx_max) { netif_tx_lock_bh(dev->net); usbnet_start_xmit(NULL, dev->net); + /* make sure tx_curr_skb is reallocated if it was empty */ + if (ctx->tx_curr_skb) { + dev_kfree_skb_any(ctx->tx_curr_skb); + ctx->tx_curr_skb = NULL; + } ctx->tx_max = val; netif_tx_unlock_bh(dev->net); } else { -- 2.0.0.rc4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 8/8] net: cdc_ncm: document the sysfs API
Adding documentation for all the driver specific sysfs attributes. Signed-off-by: Bjørn Mork --- Documentation/ABI/testing/sysfs-class-net-cdc_ncm | 143 ++ 1 file changed, 143 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-net-cdc_ncm diff --git a/Documentation/ABI/testing/sysfs-class-net-cdc_ncm b/Documentation/ABI/testing/sysfs-class-net-cdc_ncm new file mode 100644 index ..971c13297f06 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-net-cdc_ncm @@ -0,0 +1,143 @@ +What: /sys/class/net//cdc_ncm/min_tx_pkt +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + The driver will pad frames longer than this to tx_max, + allowing the device to receive tx_max sized frames + with no terminating short packet. This represents a + tradeoff between USB bus bandwidth and device DMA + optimization. + + Set to 0 to pad all frames. Set greater than tx_max to + disable all padding. + +What: /sys/class/net//cdc_ncm/rx_max +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + The maximum NCM Transfer Block (NTB) size for RX. + Cannot exceed the maximum value supported by the + device. Must allow at least one max sized datagram + plus headers. + + The actual limits are device dependent. See + dwNtbInMaxSize. + + Note: Some devices will silently ignore changes to + this value, resulting in oversized NTBs and + corresponding framing errors. + +What: /sys/class/net//cdc_ncm/tx_max +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + The maximum NTB size for TX. Cannot exceed the + maximum value supported by the device. Must allow at + least one max sized datagram plus headers. + + The actual limits are device dependent. See + dwNtbOutMaxSize. + +What: /sys/class/net//cdc_ncm/tx_timer_usecs +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + Datagram aggregation timeout in µs. The driver will + wait up to 3 times this timeout for more datagrams to + aggregate before transmitting a NTB frame. + + Valid range: 5 to 400 + + Set to 0 to disable aggregation. + +The following read only attributes all represent fields of the +structure defined in section 6.2.1 "GetNtbParameters" of "Universal +Serial Bus Communications Class Subclass Specifications for Network +Control Model Devices" (CDC NCM), Revision 1.0 (Errata 1), November +24, 2010 from USB Implementers Forum, Inc. The descriptions are +quoted from table 6-3 of CDC NCM: "NTB Parameter Structure". + +What: /sys/class/net//cdc_ncm/bmNtbFormatsSupported +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + Bit 0: 16-bit NTB supported (set to 1) + Bit 1: 32-bit NTB supported + Bits 2 – 15: reserved (reset to zero; must be ignored by host) + +What: /sys/class/net//cdc_ncm/dwNtbInMaxSize +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + IN NTB Maximum Size in bytes + +What: /sys/class/net//cdc_ncm/wNdpInDivisor +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + Divisor used for IN NTB Datagram payload alignment + +What: /sys/class/net//cdc_ncm/wNdpInPayloadRemainder +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + Remainder used to align input datagram payload within + the NTB: (Payload Offset) mod (wNdpInDivisor) = + wNdpInPayloadRemainder + +What: /sys/class/net//cdc_ncm/wNdpInAlignment +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + NDP alignment modulus for NTBs on the IN pipe. Shall + be a power of 2, and shall be at least 4. + +What: /sys/class/net//cdc_ncm/dwNtbOutMaxSize +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + OUT NTB Maximum Size + +What: /sys/class/net//cdc_ncm/wNdpOutDivisor +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + OUT NTB Datagram alignment modulus + +What: /sys/class/net//cdc_ncm/wNdpOutPayloadRemainder +Date: May 2014 +KernelVersion: 3.16 +Contact: Bjørn Mork +Description: + Remainder used to align output datagram payload + offsets within the NTB: Padding, shall be transmitted +
[PATCH net-next 1/8] net: cdc_ncm: reduce skb truesize in rx path
Cloning the big skbs we use for USB buffering chokes up TCP and SCTP because the socket memory limits are hitting earlier than they should. It is better to unconditionally copy the unwrapped packets to freshly allocated skbs. Reported-by: Jim Baxter Acked-by: Eric Dumazet Signed-off-by: Bjørn Mork --- drivers/net/usb/cdc_ncm.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 93c9ca9924eb..2bbbd65591c7 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -1289,12 +1289,11 @@ next_ndp: break; } else { - skb = skb_clone(skb_in, GFP_ATOMIC); + /* create a fresh copy to reduce truesize */ + skb = netdev_alloc_skb_ip_align(dev->net, len); if (!skb) goto error; - skb->len = len; - skb->data = ((u8 *)skb_in->data) + offset; - skb_set_tail_pointer(skb, len); + memcpy(skb_put(skb, len), skb_in->data + offset, len); usbnet_skb_return(dev, skb); payload += len; /* count payload bytes in this NTB */ } -- 2.0.0.rc4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 5/8] net: cdc_ncm: drop ethtool coalesce support
The ethtool coalesce API is not applicable for this driver. Forcing it to fit the NCM aggregation redefined the API in a driver specific way, which is much worse than defining a clean new API. These ethtool coalesce functions have therefore been replaced by a new sysfs API. Signed-off-by: Bjørn Mork --- drivers/net/usb/cdc_ncm.c | 48 --- 1 file changed, 48 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 631741c8ff22..aaa440d892b8 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -127,54 +127,8 @@ static void cdc_ncm_get_strings(struct net_device __always_unused *netdev, u32 s } } -static int cdc_ncm_get_coalesce(struct net_device *netdev, - struct ethtool_coalesce *ec) -{ - struct usbnet *dev = netdev_priv(netdev); - struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; - - /* assuming maximum sized dgrams and ignoring NDPs */ - ec->rx_max_coalesced_frames = ctx->rx_max / ctx->max_datagram_size; - ec->tx_max_coalesced_frames = ctx->tx_max / ctx->max_datagram_size; - - /* the timer will fire CDC_NCM_TIMER_PENDING_CNT times in a row */ - ec->tx_coalesce_usecs = ctx->timer_interval / (NSEC_PER_USEC / CDC_NCM_TIMER_PENDING_CNT); - return 0; -} - static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx); -static int cdc_ncm_set_coalesce(struct net_device *netdev, - struct ethtool_coalesce *ec) -{ - struct usbnet *dev = netdev_priv(netdev); - struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; - u32 new_rx_max = ctx->rx_max; - u32 new_tx_max = ctx->tx_max; - - /* assuming maximum sized dgrams and a single NDP */ - if (ec->rx_max_coalesced_frames) - new_rx_max = ec->rx_max_coalesced_frames * ctx->max_datagram_size; - if (ec->tx_max_coalesced_frames) - new_tx_max = ec->tx_max_coalesced_frames * ctx->max_datagram_size; - - if (ec->tx_coalesce_usecs && - (ec->tx_coalesce_usecs < CDC_NCM_TIMER_INTERVAL_MIN * CDC_NCM_TIMER_PENDING_CNT || -ec->tx_coalesce_usecs > CDC_NCM_TIMER_INTERVAL_MAX * CDC_NCM_TIMER_PENDING_CNT)) - return -EINVAL; - - spin_lock_bh(&ctx->mtx); - ctx->timer_interval = ec->tx_coalesce_usecs * (NSEC_PER_USEC / CDC_NCM_TIMER_PENDING_CNT); - if (!ctx->timer_interval) - ctx->tx_timer_pending = 0; - spin_unlock_bh(&ctx->mtx); - - /* inform device of new values */ - if (new_rx_max != ctx->rx_max || new_tx_max != ctx->tx_max) - cdc_ncm_update_rxtx_max(dev, new_rx_max, new_tx_max); - return 0; -} - static const struct ethtool_ops cdc_ncm_ethtool_ops = { .get_settings = usbnet_get_settings, .set_settings = usbnet_set_settings, @@ -187,8 +141,6 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = { .get_sset_count= cdc_ncm_get_sset_count, .get_strings = cdc_ncm_get_strings, .get_ethtool_stats = cdc_ncm_get_ethtool_stats, - .get_coalesce = cdc_ncm_get_coalesce, - .set_coalesce = cdc_ncm_set_coalesce, }; static u32 cdc_ncm_check_rx_max(struct usbnet *dev, u32 new_rx) -- 2.0.0.rc4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 4/8] net: cdc_ncm: use sysfs for rx/tx aggregation tuning
Attach a driver specific sysfs group to the netdev, and use it for the rx/tx aggregation variables. The datagram aggregation defined by the CDC NCM specification is specific to this device class (including CDC MBIM). Using the ethtool interrupt coalesce API as an interface to the aggregation parameters redefined that API in a driver specific and confusing way. A sysfs group - makes it clear that this is a driver specific userspace API, and - allows us to export the real values instead of some translated version, and - lets us include more aggregation variables which were impossible to force into the ethtool API. Additionally, using sysfs allows tuning the driver on space constrained hosts where userspace tools like ethtool are undesired. Suggested-by: Peter Stuge Signed-off-by: Bjørn Mork --- drivers/net/usb/cdc_ncm.c | 144 -- 1 file changed, 125 insertions(+), 19 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 5bce86a0d063..631741c8ff22 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -191,11 +191,9 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = { .set_coalesce = cdc_ncm_set_coalesce, }; -/* handle rx_max and tx_max changes */ -static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) +static u32 cdc_ncm_check_rx_max(struct usbnet *dev, u32 new_rx) { struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; - u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber; u32 val, max, min; /* clamp new_rx to sane values */ @@ -210,10 +208,126 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) } val = clamp_t(u32, new_rx, min, max); - if (val != new_rx) { - dev_dbg(&dev->intf->dev, "rx_max must be in the [%u, %u] range. Using %u\n", - min, max, val); - } + if (val != new_rx) + dev_dbg(&dev->intf->dev, "rx_max must be in the [%u, %u] range\n", min, max); + + return val; +} + +static u32 cdc_ncm_check_tx_max(struct usbnet *dev, u32 new_tx) +{ + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; + u32 val, max, min; + + /* clamp new_tx to sane values */ + min = ctx->max_datagram_size + ctx->max_ndp_size + sizeof(struct usb_cdc_ncm_nth16); + max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize)); + + /* some devices set dwNtbOutMaxSize too low for the above default */ + min = min(min, max); + + val = clamp_t(u32, new_tx, min, max); + if (val != new_tx) + dev_dbg(&dev->intf->dev, "tx_max must be in the [%u, %u] range\n", min, max); + + return val; +} + +static ssize_t cdc_ncm_show_rx_max(struct device *d, struct device_attribute *attr, char *buf) +{ + struct usbnet *dev = netdev_priv(to_net_dev(d)); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; + + return sprintf(buf, "%u\n", ctx->rx_max); +} + +static ssize_t cdc_ncm_show_tx_max(struct device *d, struct device_attribute *attr, char *buf) +{ + struct usbnet *dev = netdev_priv(to_net_dev(d)); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; + + return sprintf(buf, "%u\n", ctx->tx_max); +} + +static ssize_t cdc_ncm_show_tx_timer_usecs(struct device *d, struct device_attribute *attr, char *buf) +{ + struct usbnet *dev = netdev_priv(to_net_dev(d)); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; + + return sprintf(buf, "%u\n", ctx->timer_interval / (u32)NSEC_PER_USEC); +} + +static ssize_t cdc_ncm_store_rx_max(struct device *d, struct device_attribute *attr, const char *buf, size_t len) +{ + struct usbnet *dev = netdev_priv(to_net_dev(d)); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; + unsigned long val; + + if (kstrtoul(buf, 0, &val) || cdc_ncm_check_rx_max(dev, val) != val) + return -EINVAL; + + cdc_ncm_update_rxtx_max(dev, val, ctx->tx_max); + return len; +} + +static ssize_t cdc_ncm_store_tx_max(struct device *d, struct device_attribute *attr, const char *buf, size_t len) +{ + struct usbnet *dev = netdev_priv(to_net_dev(d)); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; + unsigned long val; + + if (kstrtoul(buf, 0, &val) || cdc_ncm_check_tx_max(dev, val) != val) + return -EINVAL; + + cdc_ncm_update_rxtx_max(dev, ctx->rx_max, val); + return len; +} + +static ssize_t cdc_ncm_store_tx_timer_usecs(struct device *d, struct device_attribute *attr, const char *buf, size_t len) +{ + struct usbnet *dev = netdev_priv(to_net_dev(d)); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; + ssize_t ret; + unsigned long val; + + ret = kstrt
[PATCH net-next 0/8] cdc_ncm: fixes and conversion to sysfs API
After considering the comments received after the ethtool coalesce support was commited, I have ended up concluding that we should remove it again, while we can, before it hits a release. The idea was not well enough thought through, and all comments received pointed to advantages of using a sysfs based API instead. This series removes the ethtool coalesce support and replaces it with sysfs attributes in a driver specific group under the netdev. The first 3 patches are unrelated fixes: patch 1: reducing truesize as discussed patch 2: fixing a potentional buffer overrun when changing tx_max patch 3: prevent framing errors when changing rx_max Bjørn Mork (8): net: cdc_ncm: reduce skb truesize in rx path net: cdc_ncm: always reallocate tx_curr_skb when tx_max increases net: cdc_ncm: inform usbnet when rx buffers are reduced net: cdc_ncm: use sysfs for rx/tx aggregation tuning net: cdc_ncm: drop ethtool coalesce support net: cdc_ncm: export NCM Transfer Block (NTB) parameters net: cdc_ncm: allow tuning min_tx_pkt net: cdc_ncm: document the sysfs API Documentation/ABI/testing/sysfs-class-net-cdc_ncm | 143 drivers/net/usb/cdc_ncm.c | 268 -- 2 files changed, 335 insertions(+), 76 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-class-net-cdc_ncm -- 2.0.0.rc4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 3/8] net: cdc_ncm: inform usbnet when rx buffers are reduced
It doesn't matter whether the buffer size goes up or down. We have to keep usbnet and device syncronized to be able to split transfers at the correct boundaries. The spec allow skipping short packets when using max sized transfers. If we don't tell usbnet about our new expected rx buffer size, then it will merge and/or split NTBs. The driver does not support this, and the result will be lots of framing errors. Fix by always reallocating usbnet rx buffers when the rx_max value changes. Fixes: 68864abf08f0 ("net: cdc_ncm: support rx_max/tx_max updates when running") Signed-off-by: Bjørn Mork --- drivers/net/usb/cdc_ncm.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index ff5b3a854898..5bce86a0d063 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -215,19 +215,12 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) min, max, val); } - /* usbnet use these values for sizing rx queues */ - dev->rx_urb_size = val; - /* inform device about NTB input size changes */ if (val != ctx->rx_max) { __le32 dwNtbInMaxSize = cpu_to_le32(val); dev_info(&dev->intf->dev, "setting rx_max = %u\n", val); - /* need to unlink rx urbs before increasing buffer size */ - if (netif_running(dev->net) && dev->rx_urb_size > ctx->rx_max) - usbnet_unlink_rx_urbs(dev); - /* tell device to use new size */ if (usbnet_write_cmd(dev, USB_CDC_SET_NTB_INPUT_SIZE, USB_TYPE_CLASS | USB_DIR_OUT @@ -238,6 +231,13 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) ctx->rx_max = val; } + /* usbnet use these values for sizing rx queues */ + if (dev->rx_urb_size != ctx->rx_max) { + dev->rx_urb_size = ctx->rx_max; + if (netif_running(dev->net)) + usbnet_unlink_rx_urbs(dev); + } + /* clamp new_tx to sane values */ min = ctx->max_datagram_size + ctx->max_ndp_size + sizeof(struct usb_cdc_ncm_nth16); max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize)); -- 2.0.0.rc4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 7/8] net: cdc_ncm: allow tuning min_tx_pkt
The min_tx_pkt variable decides the cutoff point where the driver will stop padding out NTBs to maximum size. The padding is a tradeoff where we use some USB bus bandwidth to allow the device to receive fixed size buffers. Different devices will have different optimal settings, spanning from no padding at all to padding every NTB. There is no way to automatically figure out which setting is best for a specific device. The default value is a reasonable tradeoff, calculated based on the USB packet size and out NTB max size. This may have to be changed along with any tx_max changes. Signed-off-by: Bjørn Mork --- drivers/net/usb/cdc_ncm.c | 24 1 file changed, 24 insertions(+) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 98c3adb5aea3..80a844e0ae03 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -185,6 +185,14 @@ static u32 cdc_ncm_check_tx_max(struct usbnet *dev, u32 new_tx) return val; } +static ssize_t cdc_ncm_show_min_tx_pkt(struct device *d, struct device_attribute *attr, char *buf) +{ + struct usbnet *dev = netdev_priv(to_net_dev(d)); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; + + return sprintf(buf, "%u\n", ctx->min_tx_pkt); +} + static ssize_t cdc_ncm_show_rx_max(struct device *d, struct device_attribute *attr, char *buf) { struct usbnet *dev = netdev_priv(to_net_dev(d)); @@ -209,6 +217,20 @@ static ssize_t cdc_ncm_show_tx_timer_usecs(struct device *d, struct device_attri return sprintf(buf, "%u\n", ctx->timer_interval / (u32)NSEC_PER_USEC); } +static ssize_t cdc_ncm_store_min_tx_pkt(struct device *d, struct device_attribute *attr, const char *buf, size_t len) +{ + struct usbnet *dev = netdev_priv(to_net_dev(d)); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; + unsigned long val; + + /* no need to restrict values - anything from 0 to infinity is OK */ + if (kstrtoul(buf, 0, &val)) + return -EINVAL; + + ctx->min_tx_pkt = val; + return len; +} + static ssize_t cdc_ncm_store_rx_max(struct device *d, struct device_attribute *attr, const char *buf, size_t len) { struct usbnet *dev = netdev_priv(to_net_dev(d)); @@ -256,6 +278,7 @@ static ssize_t cdc_ncm_store_tx_timer_usecs(struct device *d, struct device_att return len; } +static DEVICE_ATTR(min_tx_pkt, S_IRUGO | S_IWUSR, cdc_ncm_show_min_tx_pkt, cdc_ncm_store_min_tx_pkt); static DEVICE_ATTR(rx_max, S_IRUGO | S_IWUSR, cdc_ncm_show_rx_max, cdc_ncm_store_rx_max); static DEVICE_ATTR(tx_max, S_IRUGO | S_IWUSR, cdc_ncm_show_tx_max, cdc_ncm_store_tx_max); static DEVICE_ATTR(tx_timer_usecs, S_IRUGO | S_IWUSR, cdc_ncm_show_tx_timer_usecs, cdc_ncm_store_tx_timer_usecs); @@ -281,6 +304,7 @@ NCM_PARM_ATTR(wNdpOutAlignment, "%u", le16_to_cpu); NCM_PARM_ATTR(wNtbOutMaxDatagrams, "%u", le16_to_cpu); static struct attribute *cdc_ncm_sysfs_attrs[] = { + &dev_attr_min_tx_pkt.attr, &dev_attr_rx_max.attr, &dev_attr_tx_max.attr, &dev_attr_tx_timer_usecs.attr, -- 2.0.0.rc4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 6/8] net: cdc_ncm: export NCM Transfer Block (NTB) parameters
The mandatory GetNtbParameters control request is an important part of the host <-> device protocol negotiation in CDC NCM (and CDC MBIM). It gives device limits which the host must obey when configuring the protocol aggregation variables. The driver will enforce this by rejecting attempts to set any of the tunable variables to a value which is not supported by the device. Exporting the parameter block helps userspace decide which values are allowed without resorting to trial and error. Signed-off-by: Bjørn Mork --- drivers/net/usb/cdc_ncm.c | 30 ++ 1 file changed, 30 insertions(+) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index aaa440d892b8..98c3adb5aea3 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -260,10 +260,40 @@ static DEVICE_ATTR(rx_max, S_IRUGO | S_IWUSR, cdc_ncm_show_rx_max, cdc_ncm_store static DEVICE_ATTR(tx_max, S_IRUGO | S_IWUSR, cdc_ncm_show_tx_max, cdc_ncm_store_tx_max); static DEVICE_ATTR(tx_timer_usecs, S_IRUGO | S_IWUSR, cdc_ncm_show_tx_timer_usecs, cdc_ncm_store_tx_timer_usecs); +#define NCM_PARM_ATTR(name, format, tocpu) \ +static ssize_t cdc_ncm_show_##name(struct device *d, struct device_attribute *attr, char *buf) \ +{ \ + struct usbnet *dev = netdev_priv(to_net_dev(d)); \ + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; \ + return sprintf(buf, format "\n", tocpu(ctx->ncm_parm.name));\ +} \ +static DEVICE_ATTR(name, S_IRUGO, cdc_ncm_show_##name, NULL) + +NCM_PARM_ATTR(bmNtbFormatsSupported, "0x%04x", le16_to_cpu); +NCM_PARM_ATTR(dwNtbInMaxSize, "%u", le32_to_cpu); +NCM_PARM_ATTR(wNdpInDivisor, "%u", le16_to_cpu); +NCM_PARM_ATTR(wNdpInPayloadRemainder, "%u", le16_to_cpu); +NCM_PARM_ATTR(wNdpInAlignment, "%u", le16_to_cpu); +NCM_PARM_ATTR(dwNtbOutMaxSize, "%u", le32_to_cpu); +NCM_PARM_ATTR(wNdpOutDivisor, "%u", le16_to_cpu); +NCM_PARM_ATTR(wNdpOutPayloadRemainder, "%u", le16_to_cpu); +NCM_PARM_ATTR(wNdpOutAlignment, "%u", le16_to_cpu); +NCM_PARM_ATTR(wNtbOutMaxDatagrams, "%u", le16_to_cpu); + static struct attribute *cdc_ncm_sysfs_attrs[] = { &dev_attr_rx_max.attr, &dev_attr_tx_max.attr, &dev_attr_tx_timer_usecs.attr, + &dev_attr_bmNtbFormatsSupported.attr, + &dev_attr_dwNtbInMaxSize.attr, + &dev_attr_wNdpInDivisor.attr, + &dev_attr_wNdpInPayloadRemainder.attr, + &dev_attr_wNdpInAlignment.attr, + &dev_attr_dwNtbOutMaxSize.attr, + &dev_attr_wNdpOutDivisor.attr, + &dev_attr_wNdpOutPayloadRemainder.attr, + &dev_attr_wNdpOutAlignment.attr, + &dev_attr_wNtbOutMaxDatagrams.attr, NULL, }; -- 2.0.0.rc4 -- 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 usb 3.0 logitech c920 not working
Thank your for your reply, A summary of my Bugreport at Launchpad: My usb-Webcam Logitech c920 doesn't work when connected to my usb 3.0 Host Controller (Etron EJ168a). When connected to a usb 2.0 Host Controller it works perfectly. When I connect my usb 3.0 stick to the same port, it's recognized and also works - just not the Webcam. Disabling usb 3.0 Legacy Mode (xhci) in my Bios doesn't change anything, It's still used instead of ehci. Ubuntu 14.04 x64 3.13.0-24-generic Mainboard: Asrock Z68 Pro3 Gen3 Bios: P2.30 (latest) Here's what dmesg shows if I un/replug the webcam with the mainline/upstream kernel (3.15.0-031500rc6-generic amd64): dmesg | grep xhci [ 165.165101] usb 3-1: new high-speed USB device number 3 using xhci_hcd [ 166.411905] xhci_hcd :06:00.0: ERROR: unexpected command completion code 0x11. (multiple instances) dmesg | grep usb [ 24.168689] usb 2-1.2: cannot submit urb 0, error -28: not enough bandwidth (multiple instances) [ 161.917937] usb 3-1: USB disconnect, device number 2 [ 165.165101] usb 3-1: new high-speed USB device number 3 using xhci_hcd [ 166.409607] usb 3-1: New USB device found, idVendor=046d, idProduct=082d [ 166.409610] usb 3-1: New USB device strings: Mfr=0, Product=2, SerialNumber=1 [ 166.409611] usb 3-1: Product: HD Pro Webcam C920 [ 166.409612] usb 3-1: SerialNumber: 034DD99F [ 166.411467] input: HD Pro Webcam C920 as /devices/pci:00/:00:1c.6/:06:00.0/usb3/3-1/3-1:1.0/input/input17 [ 166.411909] usb 3-1: Not enough bandwidth for altsetting 1 [ 166.413653] usb 3-1: Not enough bandwidth for altsetting 2 [ 166.414394] usb 3-1: Not enough bandwidth for altsetting 3 [ 166.437340] usb 3-1: Not enough bandwidth for altsetting 3 [ 166.437343] usb 3-1: 3:3: usb_set_interface failed (-22) (multiple instances) [ 166.437381] usb 3-1: Not enough bandwidth for altsetting 3 (multiple instances) Here is a link to various relevant Logs/Infos/lsusb/lspci: http://pastebin.com/CFCqqfDG Inhibiting xhci for this specific port would be a solution in the interim ... how could I do that? Regards Martin Am 15.05.2014 10:23, schrieb Oliver Neukum: On Thu, 2014-05-15 at 10:03 +0200, Martin Rudhart wrote: Dear Sirs, I encountered a bug when trying to use my webcam in ubuntu 14.04 x64, which seems to be xhci/usb 3.0 related ... As I didn't know that it was a kernel-bug a filed a bug report at launchpad: https://bugs.launchpad.net/linux-kernel-headers/+bug/1319443 my google search turned up a few similar bugs: https://bugzilla.kernel.org/show_bug.cgi?id=60752 https://bugzilla.kernel.org/show_bug.cgi?id=60581 is there a workaround for this yet? (newer kernel version?) For some chipsets the newest kernels allow you to inhibit the switch to xhci for specific ports. But this is not a generally applicable solution, although it works for internal cameras. What exactly is the problem? Do you see exactly those messages as mentioned in the tracking tools? And it would be most helpful if you at least posted "lsusb -v" for your camera. Regards Oliver -- 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: add run-time dependencies to R-Car driver
Hi Jean, Thank you for the patch. On Friday 23 May 2014 10:36:22 Jean Delvare wrote: > The Renesas R-Car USB PHY driver only supports the R8A7778 and > R8A7779, it isn't useful on other systems unless build testing. > > Signed-off-by: Jean Delvare > Cc: Felipe Balbi > Cc: Greg Kroah-Hartman > Cc: Laurent Pinchart Could you please CC the linux-sh mailing list for similar patches in the future ? Sergei, any opinion on this ? > --- > drivers/usb/phy/Kconfig |1 + > 1 file changed, 1 insertion(+) > > --- linux-3.15-rc6.orig/drivers/usb/phy/Kconfig 2014-04-14 > 09:42:31.481112766 +0200 +++ > linux-3.15-rc6/drivers/usb/phy/Kconfig2014-05-23 10:11:59.979813625 > +0200 > @@ -208,6 +208,7 @@ config USB_MXS_PHY > config USB_RCAR_PHY > tristate "Renesas R-Car USB PHY support" > depends on USB || USB_GADGET > + depends on ARCH_R8A7778 || ARCH_R8A7779 || COMPILE_TEST > select USB_PHY > help > Say Y here to add support for the Renesas R-Car USB common PHY driver. -- Regards, Laurent Pinchart -- 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 v4 0/7] Multiple fixes for musb and cppi41
Here is v4 of my cleanup series for musb in CPPI DMA mode. v3 -> v4: * s/MUSB_CSR0_TXPKTRDY/MUSB_TXCSR_TXPKTRDY/ in patch #4 * s/packet_done/rx_packet_done/ in #5 v2 -> v3: * Dropped the RXSTALL bit modification from patch #4 * Droped some occasions where musb_ep_select() is not necessary, in patch #6 * Slightly reworded the commit log of #5 * Added a 7th patch to fix a wrongly indented block in musb_host.c v1 -> v2: * Some commit logs fixed and reworded * Squashed #6 and #7, as they are basically doing the same thing * Added some more occasions where musb_ep_select() was missing before CSR register access, and added those hunks to #6 as well. With this series applied, iperf works correctly with Wifi sticks connected via USB. Again, #1 can go via slave-dma.git, and #3 - #6 are potentially interesting for stable@. Thanks, Daniel Daniel Mack (7): dma: cppi41: handle 0-length packets usb: musb: remove unnecessary (void) prefix at function calls usb: musb: use is_host_active() to distinguish between host and gadget mode usb: musb: fix bit mask for CSR in musb_h_tx_flush_fifo() usb: musb: introduce dma_channel.rx_packet_done usb: musb/cppi41: call musb_ep_select() before accessing an endpoint's CSR usb: musb: fix wrong indentation in musb_host.c drivers/dma/cppi41.c | 13 ++--- drivers/usb/musb/musb_core.c | 16 +++- drivers/usb/musb/musb_cppi41.c | 11 +-- drivers/usb/musb/musb_dma.h| 1 + drivers/usb/musb/musb_host.c | 17 + 5 files changed, 36 insertions(+), 22 deletions(-) -- 1.9.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 v4 2/7] usb: musb: remove unnecessary (void) prefix at function calls
Just a little cleanup that removes unnecessary casts. Signed-off-by: Daniel Mack Acked-by: George Cherian --- drivers/usb/musb/musb_host.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index eb06291..f98a7c0 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -1295,7 +1295,7 @@ done: if (status) { if (dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) { dma->status = MUSB_DMA_STATUS_CORE_ABORT; - (void) musb->dma_controller->channel_abort(dma); + musb->dma_controller->channel_abort(dma); } /* do the proper sequence to abort the transfer in the @@ -1640,7 +1640,7 @@ void musb_host_rx(struct musb *musb, u8 epnum) /* clean up dma and collect transfer count */ if (dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) { dma->status = MUSB_DMA_STATUS_CORE_ABORT; - (void) musb->dma_controller->channel_abort(dma); + musb->dma_controller->channel_abort(dma); xfer_len = dma->actual_len; } musb_h_flush_rxfifo(hw_ep, MUSB_RXCSR_CLRDATATOG); @@ -1671,7 +1671,7 @@ void musb_host_rx(struct musb *musb, u8 epnum) */ if (dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) { dma->status = MUSB_DMA_STATUS_CORE_ABORT; - (void) musb->dma_controller->channel_abort(dma); + musb->dma_controller->channel_abort(dma); xfer_len = dma->actual_len; done = true; } -- 1.9.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 v4 3/7] usb: musb: use is_host_active() to distinguish between host and gadget mode
On AM33xx platforms, unplugging a device in the middle of an active transfer leads to a drop of MUSB_DEVCTL_HM in MUSB_DEVCTL before the system is informed about a disconnect. This consequently makes the musb core call the gadget code to handle the interrupt request, which then crashes the kernel because the relevant pointers haven't been set up for gadget mode. To fix this, use is_host_active() rather than (devctl & MUSB_DEVCTL_HM) in musb_interrupt() and musb_dma_completion() to detect whether the controller is in host or peripheral mode. This information is provided by the driver logic and does not rely on register contents. Signed-off-by: Daniel Mack --- drivers/usb/musb/musb_core.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 1f8b175..a496af6 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1518,7 +1518,7 @@ irqreturn_t musb_interrupt(struct musb *musb) devctl = musb_readb(musb->mregs, MUSB_DEVCTL); dev_dbg(musb->controller, "** IRQ %s usb%04x tx%04x rx%04x\n", - (devctl & MUSB_DEVCTL_HM) ? "host" : "peripheral", + is_host_active(musb) ? "host" : "peripheral", musb->int_usb, musb->int_tx, musb->int_rx); /* the core can interrupt us for multiple reasons; docs have @@ -1532,7 +1532,7 @@ irqreturn_t musb_interrupt(struct musb *musb) /* handle endpoint 0 first */ if (musb->int_tx & 1) { - if (devctl & MUSB_DEVCTL_HM) + if (is_host_active(musb)) retval |= musb_h_ep0_irq(musb); else retval |= musb_g_ep0_irq(musb); @@ -1546,7 +1546,7 @@ irqreturn_t musb_interrupt(struct musb *musb) /* musb_ep_select(musb->mregs, ep_num); */ /* REVISIT just retval = ep->rx_irq(...) */ retval = IRQ_HANDLED; - if (devctl & MUSB_DEVCTL_HM) + if (is_host_active(musb)) musb_host_rx(musb, ep_num); else musb_g_rx(musb, ep_num); @@ -1564,7 +1564,7 @@ irqreturn_t musb_interrupt(struct musb *musb) /* musb_ep_select(musb->mregs, ep_num); */ /* REVISIT just retval |= ep->tx_irq(...) */ retval = IRQ_HANDLED; - if (devctl & MUSB_DEVCTL_HM) + if (is_host_active(musb)) musb_host_tx(musb, ep_num); else musb_g_tx(musb, ep_num); @@ -1586,15 +1586,13 @@ MODULE_PARM_DESC(use_dma, "enable/disable use of DMA"); void musb_dma_completion(struct musb *musb, u8 epnum, u8 transmit) { - u8 devctl = musb_readb(musb->mregs, MUSB_DEVCTL); - /* called with controller lock already held */ if (!epnum) { #ifndef CONFIG_USB_TUSB_OMAP_DMA if (!is_cppi_enabled()) { /* endpoint 0 */ - if (devctl & MUSB_DEVCTL_HM) + if (is_host_active(musb)) musb_h_ep0_irq(musb); else musb_g_ep0_irq(musb); @@ -1603,13 +1601,13 @@ void musb_dma_completion(struct musb *musb, u8 epnum, u8 transmit) } else { /* endpoints 1..15 */ if (transmit) { - if (devctl & MUSB_DEVCTL_HM) + if (is_host_active(musb)) musb_host_tx(musb, epnum); else musb_g_tx(musb, epnum); } else { /* receive */ - if (devctl & MUSB_DEVCTL_HM) + if (is_host_active(musb)) musb_host_rx(musb, epnum); else musb_g_rx(musb, epnum); -- 1.9.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 v4 1/7] dma: cppi41: handle 0-length packets
When a 0-length packet is received on the bus, desc->pd0 yields 1, which confuses the driver's users. This information is clearly wrong and not in accordance to the datasheet, but it's been observed on an AM335x board, very reproducible. Fix this by looking at bit 19 in PD2 of the completed packet. This bit will tell us if a zero-length packet was received on a queue. If it's set, ignore the value in PD0 and report a total length of 0 instead. Signed-off-by: Daniel Mack --- drivers/dma/cppi41.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c index d028f36..8f8b0b6 100644 --- a/drivers/dma/cppi41.c +++ b/drivers/dma/cppi41.c @@ -86,6 +86,9 @@ #define USBSS_IRQ_PD_COMP (1 << 2) +/* Packet Descriptor */ +#define PD2_ZERO_LENGTH(1 << 19) + struct cppi41_channel { struct dma_chan chan; struct dma_async_tx_descriptor txd; @@ -307,7 +310,7 @@ static irqreturn_t cppi41_irq(int irq, void *data) __iormb(); while (val) { - u32 desc; + u32 desc, len; q_num = __fls(val); val &= ~(1 << q_num); @@ -319,9 +322,13 @@ static irqreturn_t cppi41_irq(int irq, void *data) q_num, desc); continue; } - c->residue = pd_trans_len(c->desc->pd6) - - pd_trans_len(c->desc->pd0); + if (c->desc->pd2 & PD2_ZERO_LENGTH) + len = 0; + else + len = pd_trans_len(c->desc->pd0); + + c->residue = pd_trans_len(c->desc->pd6) - len; dma_cookie_complete(&c->txd); c->txd.callback(c->txd.callback_param); } -- 1.9.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 v4 7/7] usb: musb: fix wrong indentation in musb_host.c
Just a cosmetic cleanup with no functional change. Signed-off-by: Daniel Mack --- drivers/usb/musb/musb_host.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index b00edd2..ac6dd92 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -1734,11 +1734,11 @@ void musb_host_rx(struct musb *musb, u8 epnum) } } else { - /* done if urb buffer is full or short packet is recd */ - done = (urb->actual_length + xfer_len >= - urb->transfer_buffer_length - || dma->actual_len < qh->maxpacket - || dma->rx_packet_done); + /* done if urb buffer is full or short packet is recd */ + done = (urb->actual_length + xfer_len >= + urb->transfer_buffer_length + || dma->actual_len < qh->maxpacket + || dma->rx_packet_done); } /* send IN token for next packet, without AUTOREQ */ -- 1.9.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 v4 5/7] usb: musb: introduce dma_channel.rx_packet_done
The musb/cppi41 glue layer is capable of handling transactions that span over more than one USB packet by reloading the DMA descriptors partially. An urb is considered completed when either its transfer buffer has been filled entirely (actual_length == transfer_buffer_length) or if a packet in the stream has less bytes than the endpoint's wMaxPacketSize. Once one of the above conditions is met, musb_dma_completion() is called from cppi41_trans_done(). However, the final decision whether or not to return the urb to its owner is made by the core and its determination of the variable 'done' in musb_host_rx(). This code has currently no way of knowing what the size of the last packet was, and whether or not to give back the urb due to a short read. Fix this by introducing a new boolean flag in 'struct dma_channel', and set it from musb_cppi41.c. If set, it will make the core do what the DMA layer decided and complete the urb. Signed-off-by: Daniel Mack Acked-by: George Cherian --- drivers/usb/musb/musb_cppi41.c | 2 ++ drivers/usb/musb/musb_dma.h| 1 + drivers/usb/musb/musb_host.c | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c index 7b8bbf5..4187ef1 100644 --- a/drivers/usb/musb/musb_cppi41.c +++ b/drivers/usb/musb/musb_cppi41.c @@ -139,6 +139,7 @@ static void cppi41_trans_done(struct cppi41_dma_channel *cppi41_channel) cppi41_channel->channel.actual_len = cppi41_channel->transferred; cppi41_channel->channel.status = MUSB_DMA_STATUS_FREE; + cppi41_channel->channel.rx_packet_done = true; musb_dma_completion(musb, hw_ep->epnum, cppi41_channel->is_tx); } else { /* next iteration, reload */ @@ -450,6 +451,7 @@ static bool cppi41_configure_channel(struct dma_channel *channel, dma_desc->callback = cppi41_dma_callback; dma_desc->callback_param = channel; cppi41_channel->cookie = dma_desc->tx_submit(dma_desc); + cppi41_channel->channel.rx_packet_done = false; save_rx_toggle(cppi41_channel); dma_async_issue_pending(dc); diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h index 1345a4f..1d44faa 100644 --- a/drivers/usb/musb/musb_dma.h +++ b/drivers/usb/musb/musb_dma.h @@ -129,6 +129,7 @@ struct dma_channel { size_t actual_len; enum dma_channel_status status; booldesired_mode; + boolrx_packet_done; }; /* diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index 73e8f1d..b00edd2 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -1737,7 +1737,8 @@ void musb_host_rx(struct musb *musb, u8 epnum) /* done if urb buffer is full or short packet is recd */ done = (urb->actual_length + xfer_len >= urb->transfer_buffer_length - || dma->actual_len < qh->maxpacket); + || dma->actual_len < qh->maxpacket + || dma->rx_packet_done); } /* send IN token for next packet, without AUTOREQ */ -- 1.9.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 v4 6/7] usb: musb/cppi41: call musb_ep_select() before accessing an endpoint's CSR
Before accessing any of an endpoint's CSR registers, make sure the correct endpoint is selected. Otherwise, data read from or written to the registers is likely to affect the wrong endpoint as long as the connected device has more than one endpoint. This, of course, leads to all sorts of strange effects such as stream starvation and driver internal state machine confusion due to spurious interrupts. Signed-off-by: Daniel Mack --- drivers/usb/musb/musb_cppi41.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c index 4187ef1..932464f 100644 --- a/drivers/usb/musb/musb_cppi41.c +++ b/drivers/usb/musb/musb_cppi41.c @@ -74,15 +74,18 @@ static void save_rx_toggle(struct cppi41_dma_channel *cppi41_channel) static void update_rx_toggle(struct cppi41_dma_channel *cppi41_channel) { + struct musb_hw_ep *hw_ep = cppi41_channel->hw_ep; + struct musb *musb = hw_ep->musb; u16 csr; u8 toggle; if (cppi41_channel->is_tx) return; - if (!is_host_active(cppi41_channel->controller->musb)) + if (!is_host_active(musb)) return; - csr = musb_readw(cppi41_channel->hw_ep->regs, MUSB_RXCSR); + musb_ep_select(musb->mregs, hw_ep->epnum); + csr = musb_readw(hw_ep->regs, MUSB_RXCSR); toggle = csr & MUSB_RXCSR_H_DATATOGGLE ? 1 : 0; /* @@ -107,6 +110,7 @@ static bool musb_is_tx_fifo_empty(struct musb_hw_ep *hw_ep) void __iomem*epio = musb->endpoints[epnum].regs; u16 csr; + musb_ep_select(musb->mregs, hw_ep->epnum); csr = musb_readw(epio, MUSB_TXCSR); if (csr & MUSB_TXCSR_TXPKTRDY) return false; @@ -173,6 +177,7 @@ static void cppi41_trans_done(struct cppi41_dma_channel *cppi41_channel) dma_async_issue_pending(dc); if (!cppi41_channel->is_tx) { + musb_ep_select(musb->mregs, hw_ep->epnum); csr = musb_readw(epio, MUSB_RXCSR); csr |= MUSB_RXCSR_H_REQPKT; musb_writew(epio, MUSB_RXCSR, csr); -- 1.9.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 v4 4/7] usb: musb: fix bit mask for CSR in musb_h_tx_flush_fifo()
The datasheet says that MUSB_TXCSR_FLUSHFIFO is only valid when MUSB_TXCSR_TXPKTRDY is set as well. With this patch applied, the warning in this function does no longer kick in when an USB soundcard is unplugged while the stream is active. Signed-off-by: Daniel Mack --- drivers/usb/musb/musb_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index f98a7c0..73e8f1d 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -120,7 +120,7 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep) if (csr != lastcsr) dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: %02x\n", csr); lastcsr = csr; - csr |= MUSB_TXCSR_FLUSHFIFO; + csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY; musb_writew(epio, MUSB_TXCSR, csr); csr = musb_readw(epio, MUSB_TXCSR); if (WARN(retries-- < 1, -- 1.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH usb: phy: add run-time dependencies to R-Car driver
Hello. On 05/26/2014 04:50 PM, Laurent Pinchart wrote: The Renesas R-Car USB PHY driver only supports the R8A7778 and R8A7779, it isn't useful on other systems unless build testing. Signed-off-by: Jean Delvare Cc: Felipe Balbi Cc: Greg Kroah-Hartman Cc: Laurent Pinchart Could you please CC the linux-sh mailing list for similar patches in the future ? Sergei, any opinion on this ? I wouldn't object. --- drivers/usb/phy/Kconfig |1 + 1 file changed, 1 insertion(+) --- linux-3.15-rc6.orig/drivers/usb/phy/Kconfig 2014-04-14 09:42:31.481112766 +0200 +++ linux-3.15-rc6/drivers/usb/phy/Kconfig 2014-05-23 10:11:59.979813625 +0200 @@ -208,6 +208,7 @@ config USB_MXS_PHY config USB_RCAR_PHY tristate "Renesas R-Car USB PHY support" depends on USB || USB_GADGET + depends on ARCH_R8A7778 || ARCH_R8A7779 || COMPILE_TEST select USB_PHY help Say Y here to add support for the Renesas R-Car USB common PHY driver. WBR, 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 v2 4/6] usb: musb: fix bit masks for CSR in musb_h_tx_flush_fifo()
Hello. On 05/24/2014 11:22 AM, Daniel Mack wrote: I I'd say it's not supposed to. I just wanted the bit to be 0 when writing back the CSR. You failed to achieve that since you forgot ~; you're clearing all bits but this one. Ouch, totally missed that, thanks! And I've missed CSR0 thing at first, so we have a tie. :-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index f98a7c0..9d3a5b2 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -120,7 +120,8 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep) if (csr != lastcsr) dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: %02x\n", csr); lastcsr = csr; - csr |= MUSB_TXCSR_FLUSHFIFO; + csr &= MUSB_TXCSR_H_RXSTALL; + csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_CSR0_TXPKTRDY; Yes, the manual tells us to set TxPktRdy along with FlushFIFO. This has been a long standing bug, perhaps because the old versions of the MUSBHDRC manuals didn't mention that. Ok. I'll drop the RXSTALL thing from the patch. It's also unrelated the issue that I've seen, so let's not touch the bit here at all. Thanks. musb_writew(epio, MUSB_TXCSR, csr); csr = musb_readw(epio, MUSB_TXCSR); Another possible bug here is not flushing the second FIFO in the double-buffered mode... Hmm, ok. I'll not make that part of this patch though, I'd say. Would you like to follow up with a patch for that? I don't have access to any MUSB hardware now (and when I had, it didn't have double buffered endpoints), so I wouldn't be able to test such a patch. Looking a bit more at the code, now it seems it already can handle that situation... Again, thanks for you review! Daniel WBR, 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: ASMedia AS2105 interface on USB 3.0 corrupts data
On 05/22/2014 05:10 PM, Alan Stern wrote: > On Thu, 22 May 2014, Heinz Diehl wrote: > >> On 21.05.2014, Alan Stern wrote: >> >>> In that case, I have no idea what else could be causing the problem. >> >> Thanks for your efforts! >> >> If somebody of the developers is interested in debugging this, I'm >> willing to provide/give away the device for free. Send me >> an email with your postal adress in this case, and I'll send you >> the controller mentioned in this thread. My GPG-key is available >> from the link given in the header. > > Mathias, this sounds like a pretty good opportunity for somebody at > Intel to use a USB-3 bus analyzer to solve some of the recurring > problems that have been affecting many people using Intel's xHCI > controllers. > This would be helpful, I've got a LeCroy protocol analyzer in Finland. Heinz, if you're willing to send it to Europe Finland I'll gladly take it, (I'll send the address details in a separate personal mail) Otherwise if you can tell me where exactly what device this is I might be able to purchase a similar one. Thanks -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: ASMedia AS2105 interface on USB 3.0 corrupts data
On 05/22/2014 05:22 PM, David Laight wrote: > From: Alan Stern >> On Thu, 22 May 2014, Heinz Diehl wrote: >> >>> On 21.05.2014, Alan Stern wrote: >>> In that case, I have no idea what else could be causing the problem. >>> >>> Thanks for your efforts! >>> >>> If somebody of the developers is interested in debugging this, I'm >>> willing to provide/give away the device for free. Send me >>> an email with your postal adress in this case, and I'll send you >>> the controller mentioned in this thread. My GPG-key is available >>> from the link given in the header. >> >> Mathias, this sounds like a pretty good opportunity for somebody at >> Intel to use a USB-3 bus analyzer to solve some of the recurring >> problems that have been affecting many people using Intel's xHCI >> controllers. > > I'd also suggest you look through the patches I've posted. > Some of the changes would make the code much easier to read. > Hi Seems that I lost track of which patches you refer to here, could you point out the relevant ones to me again? -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/18] USB: storage: ene_ub6250: Use kmemdup instead of kmalloc + memcpy
This issue was reported by coccicheck using the semantic patch at scripts/coccinelle/api/memdup.cocci Signed-off-by: Benoit Taine --- Tested by compilation without errors. drivers/usb/storage/ene_ub6250.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c index 1bfc9a6..ef6efb5 100644 --- a/drivers/usb/storage/ene_ub6250.c +++ b/drivers/usb/storage/ene_ub6250.c @@ -1928,11 +1928,10 @@ static int ene_load_bincode(struct us_data *us, unsigned char flag) usb_stor_dbg(us, "load firmware %s failed\n", fw_name); goto nofw; } - buf = kmalloc(sd_fw->size, GFP_KERNEL); + buf = kmemdup(sd_fw->data, sd_fw->size, GFP_KERNEL); if (buf == NULL) goto nofw; - memcpy(buf, sd_fw->data, sd_fw->size); memset(bcb, 0, sizeof(struct bulk_cb_wrap)); bcb->Signature = cpu_to_le32(US_BULK_CB_SIGN); bcb->DataTransferLength = sd_fw->size; -- 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/18] usb: gadget: Use kmemdup instead of kmalloc + memcpy
This issue was reported by coccicheck using the semantic patch at scripts/coccinelle/api/memdup.cocci Signed-off-by: Benoit Taine --- Tested by compilation without errors. drivers/usb/gadget/configfs.c|4 +--- drivers/usb/gadget/lpc32xx_udc.c |3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 2ddcd63..bcc2a62 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -1021,12 +1021,10 @@ static ssize_t ext_prop_data_store(struct usb_os_desc_ext_prop *ext_prop, if (page[len - 1] == '\n' || page[len - 1] == '\0') --len; - new_data = kzalloc(len, GFP_KERNEL); + new_data = kmemdup(page, len, GFP_KERNEL); if (!new_data) return -ENOMEM; - memcpy(new_data, page, len); - if (desc->opts_mutex) mutex_lock(desc->opts_mutex); kfree(ext_prop->data); diff --git a/drivers/usb/gadget/lpc32xx_udc.c b/drivers/usb/gadget/lpc32xx_udc.c index e471580..0588231 100644 --- a/drivers/usb/gadget/lpc32xx_udc.c +++ b/drivers/usb/gadget/lpc32xx_udc.c @@ -3045,11 +3045,10 @@ static int __init lpc32xx_udc_probe(struct platform_device *pdev) dma_addr_t dma_handle; struct device_node *isp1301_node; - udc = kzalloc(sizeof(*udc), GFP_KERNEL); + udc = kmemdup(&controller_template, sizeof(*udc), GFP_KERNEL); if (!udc) return -ENOMEM; - memcpy(udc, &controller_template, sizeof(*udc)); for (i = 0; i <= 15; i++) udc->ep[i].udc = udc; udc->gadget.ep0 = &udc->ep[0].ep; -- 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/18] Use kmemdup instead of kmalloc + memcpy
These patches enhance kernel style usage, and allows smaller code while preventing accidental code edits to produce overflows. The semantic patch at scripts/coccinelle/api/memdup.cocci was used to detect and edit this situation. -- 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/18] r8152: Use kmemdup instead of kmalloc + memcpy
This issue was reported by coccicheck using the semantic patch at scripts/coccinelle/api/memdup.cocci Signed-off-by: Benoit Taine --- Tested by compilation without errors. drivers/net/usb/r8152.c |4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 9f91c7a..2543196 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -630,12 +630,10 @@ int set_registers(struct r8152 *tp, u16 value, u16 index, u16 size, void *data) int ret; void *tmp; - tmp = kmalloc(size, GFP_KERNEL); + tmp = kmemdup(data, size, GFP_KERNEL); if (!tmp) return -ENOMEM; - memcpy(tmp, data, size); - ret = usb_control_msg(tp->udev, usb_sndctrlpipe(tp->udev, 0), RTL8152_REQ_SET_REGS, RTL8152_REQT_WRITE, value, index, tmp, size, 500); -- 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: ASMedia AS2105 interface on USB 3.0 corrupts data
On 26.05.2014, Mathias Nyman wrote: > This would be helpful, I've got a LeCroy protocol analyzer in Finland. > Heinz, if you're willing to send it to Europe Finland I'll gladly take it, > (I'll send the address details in a separate personal mail) Of course! Send me your adress, and I'll send you the controller. I've purchased it here in Norway, it's exactly this one: http://tinyurl.com/kuxcshn Thanks, Heinz -- 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/63] USB: sierra: fix use after free at suspend/resume
Fix use after free or NULL-pointer dereference during suspend and resume. The port data may never have been allocated (port probe failed) or may already have been released by port_remove (e.g. driver is unloaded) when suspend and resume are called. Fixes: e6929a9020ac ("USB: support for autosuspend in sierra while online") Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 07cbd985ed65..2c5c5a9330e2 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -933,6 +933,7 @@ static int sierra_port_remove(struct usb_serial_port *port) struct sierra_port_private *portdata; portdata = usb_get_serial_port_data(port); + usb_set_serial_port_data(port, NULL); kfree(portdata); return 0; @@ -949,6 +950,8 @@ static void stop_read_write_urbs(struct usb_serial *serial) for (i = 0; i < serial->num_ports; ++i) { port = serial->port[i]; portdata = usb_get_serial_port_data(port); + if (!portdata) + continue; sierra_stop_rx_urbs(port); usb_kill_anchored_urbs(&portdata->active); } @@ -991,6 +994,9 @@ static int sierra_resume(struct usb_serial *serial) port = serial->port[i]; portdata = usb_get_serial_port_data(port); + if (!portdata) + continue; + while ((urb = usb_get_from_anchor(&portdata->delayed))) { usb_anchor_urb(urb, &portdata->active); intfdata->in_flight++; -- 1.8.5.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 55/63] USB: cdc-acm: fix I/O after failed open
Make sure to kill any already submitted read urbs on read-urb submission failures in open in order to prevent doing I/O for a closed port. Fixes: 088c64f81284 ("USB: cdc-acm: re-write read processing") Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 6c6928a154bd..eddeba61a88c 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -506,6 +506,7 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) { struct acm *acm = container_of(port, struct acm, port); int retval = -ENODEV; + int i; dev_dbg(&acm->control->dev, "%s\n", __func__); @@ -556,6 +557,8 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) return 0; error_submit_read_urbs: + for (i = 0; i < acm->rx_buflimit; i++) + usb_kill_urb(acm->read_urbs[i]); acm->ctrlout = 0; acm_set_control(acm, acm->ctrlout); error_set_control: -- 1.8.5.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 04/63] USB: sierra: fix urb and memory leak on disconnect
The delayed-write queue was never emptied on disconnect, something which would lead to leaked urbs and transfer buffers if the device is disconnected before being runtime resumed due to a write. Fixes: e6929a9020ac ("USB: support for autosuspend in sierra while online") Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index dd9820d3fa03..1a42649253a5 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -759,6 +759,7 @@ static void sierra_close(struct usb_serial_port *port) struct usb_serial *serial = port->serial; struct sierra_port_private *portdata; struct sierra_intf_private *intfdata = port->serial->private; + struct urb *urb; portdata = usb_get_serial_port_data(port); @@ -780,6 +781,18 @@ static void sierra_close(struct usb_serial_port *port) portdata->opened = 0; spin_unlock_irq(&intfdata->susp_lock); + for (;;) { + urb = usb_get_from_anchor(&portdata->delayed); + if (!urb) + break; + kfree(urb->transfer_buffer); + usb_free_urb(urb); + usb_autopm_put_interface_async(serial->interface); + spin_lock(&portdata->lock); + portdata->outstanding_urbs--; + spin_unlock(&portdata->lock); + } + sierra_stop_rx_urbs(port); for (i = 0; i < portdata->num_in_urbs; i++) { sierra_release_urb(portdata->in_urbs[i]); -- 1.8.5.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 21/63] USB: option: fix runtime PM handling
Fix potential I/O while runtime suspended due to missing PM operations in send_setup. Fixes: 383cedc3bb43 ("USB: serial: full autosuspend support for the option driver") Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/option.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c index f213ee978516..802c0693e5c9 100644 --- a/drivers/usb/serial/option.c +++ b/drivers/usb/serial/option.c @@ -1923,6 +1923,7 @@ static int option_send_setup(struct usb_serial_port *port) struct option_private *priv = intfdata->private; struct usb_wwan_port_private *portdata; int val = 0; + int res; portdata = usb_get_serial_port_data(port); @@ -1931,9 +1932,17 @@ static int option_send_setup(struct usb_serial_port *port) if (portdata->rts_state) val |= 0x02; - return usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0), + res = usb_autopm_get_interface(serial->interface); + if (res) + return res; + + res = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0), 0x22, 0x21, val, priv->bInterfaceNumber, NULL, 0, USB_CTRL_SET_TIMEOUT); + + usb_autopm_put_interface(serial->interface); + + return res; } MODULE_AUTHOR(DRIVER_AUTHOR); -- 1.8.5.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 58/63] USB: cdc-acm: remove redundant disconnected test from shutdown
Remove redundant disconnect test from shutdown(), which is never called post disconnect() where we do synchronous hangup. Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 41 +++-- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index bc7a2a6fc4ac..91fdc293196f 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -594,30 +594,27 @@ static void acm_port_shutdown(struct tty_port *port) dev_dbg(&acm->control->dev, "%s\n", __func__); - mutex_lock(&acm->mutex); - if (!acm->disconnected) { - pm_err = usb_autopm_get_interface(acm->control); - acm_set_control(acm, acm->ctrlout = 0); - - for (;;) { - urb = usb_get_from_anchor(&acm->delayed); - if (!urb) - break; - wb = urb->context; - wb->use = 0; - usb_autopm_put_interface_async(acm->control); - } + pm_err = usb_autopm_get_interface(acm->control); + acm_set_control(acm, acm->ctrlout = 0); - usb_kill_urb(acm->ctrlurb); - for (i = 0; i < ACM_NW; i++) - usb_kill_urb(acm->wb[i].urb); - for (i = 0; i < acm->rx_buflimit; i++) - usb_kill_urb(acm->read_urbs[i]); - acm->control->needs_remote_wakeup = 0; - if (!pm_err) - usb_autopm_put_interface(acm->control); + for (;;) { + urb = usb_get_from_anchor(&acm->delayed); + if (!urb) + break; + wb = urb->context; + wb->use = 0; + usb_autopm_put_interface_async(acm->control); } - mutex_unlock(&acm->mutex); + + usb_kill_urb(acm->ctrlurb); + for (i = 0; i < ACM_NW; i++) + usb_kill_urb(acm->wb[i].urb); + for (i = 0; i < acm->rx_buflimit; i++) + usb_kill_urb(acm->read_urbs[i]); + + acm->control->needs_remote_wakeup = 0; + if (!pm_err) + usb_autopm_put_interface(acm->control); } static void acm_tty_cleanup(struct tty_struct *tty) -- 1.8.5.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 61/63] USB: cdc-acm: remove redundant usb_mark_last_busy
There's no need to call usb_mark_last_busy after having increased the PM counter in write(). The device will be marked busy by USB core when the PM counter is balanced in the completion handler. Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 3c7cfac48e30..8f654ce52570 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -690,7 +690,6 @@ static int acm_tty_write(struct tty_struct *tty, spin_unlock_irqrestore(&acm->write_lock, flags); return count; } - usb_mark_last_busy(acm->dev); stat = acm_start_wb(acm, wb); spin_unlock_irqrestore(&acm->write_lock, flags); -- 1.8.5.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 35/63] USB: usb_wwan: kill interrupt urb explicitly at suspend
As the port interrupt URB is submitted by the subdriver at open, we should also kill it explicitly at suspend (even though this will be taken care of by USB serial core otherwise). Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index c4a815c1da5b..b671d59e356e 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -562,7 +562,7 @@ int usb_wwan_port_remove(struct usb_serial_port *port) EXPORT_SYMBOL(usb_wwan_port_remove); #ifdef CONFIG_PM -static void stop_read_write_urbs(struct usb_serial *serial) +static void stop_urbs(struct usb_serial *serial) { int i, j; struct usb_serial_port *port; @@ -578,6 +578,7 @@ static void stop_read_write_urbs(struct usb_serial *serial) usb_kill_urb(portdata->in_urbs[j]); for (j = 0; j < N_OUT_URB; j++) usb_kill_urb(portdata->out_urbs[j]); + usb_kill_urb(port->interrupt_in_urb); } } @@ -595,7 +596,7 @@ int usb_wwan_suspend(struct usb_serial *serial, pm_message_t message) intfdata->suspended = 1; spin_unlock_irq(&intfdata->susp_lock); - stop_read_write_urbs(serial); + stop_urbs(serial); return 0; } -- 1.8.5.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 63/63] USB: wusbcore: fix control-pipe directions
Fix incorrect pipe directions in control requests (which has been silently fixed up by USB core). Signed-off-by: Johan Hovold --- drivers/usb/wusbcore/wa-rpipe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/wusbcore/wa-rpipe.c b/drivers/usb/wusbcore/wa-rpipe.c index 6d6da127f6de..a80c5d284b59 100644 --- a/drivers/usb/wusbcore/wa-rpipe.c +++ b/drivers/usb/wusbcore/wa-rpipe.c @@ -524,7 +524,7 @@ void rpipe_ep_disable(struct wahc *wa, struct usb_host_endpoint *ep) u16 index = le16_to_cpu(rpipe->descr.wRPipeIndex); usb_control_msg( - wa->usb_dev, usb_rcvctrlpipe(wa->usb_dev, 0), + wa->usb_dev, usb_sndctrlpipe(wa->usb_dev, 0), USB_REQ_RPIPE_ABORT, USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_RPIPE, 0, index, NULL, 0, USB_CTRL_SET_TIMEOUT); @@ -545,7 +545,7 @@ void rpipe_clear_feature_stalled(struct wahc *wa, struct usb_host_endpoint *ep) u16 index = le16_to_cpu(rpipe->descr.wRPipeIndex); usb_control_msg( - wa->usb_dev, usb_rcvctrlpipe(wa->usb_dev, 0), + wa->usb_dev, usb_sndctrlpipe(wa->usb_dev, 0), USB_REQ_CLEAR_FEATURE, USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_RPIPE, RPIPE_STALL, index, NULL, 0, USB_CTRL_SET_TIMEOUT); -- 1.8.5.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 34/63] USB: usb_wwan: remove redundant urb kill from port remove
Remove redundant usb_kill_urb from port remove, which is called post-shutdown (close). Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index aa45985520f7..c4a815c1da5b 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -546,20 +546,17 @@ int usb_wwan_port_remove(struct usb_serial_port *port) portdata = usb_get_serial_port_data(port); usb_set_serial_port_data(port, NULL); - /* Stop reading/writing urbs and free them */ for (i = 0; i < N_IN_URB; i++) { - usb_kill_urb(portdata->in_urbs[i]); usb_free_urb(portdata->in_urbs[i]); free_page((unsigned long)portdata->in_buffer[i]); } for (i = 0; i < N_OUT_URB; i++) { - usb_kill_urb(portdata->out_urbs[i]); usb_free_urb(portdata->out_urbs[i]); kfree(portdata->out_buffer[i]); } - /* Now free port private data */ kfree(portdata); + return 0; } EXPORT_SYMBOL(usb_wwan_port_remove); -- 1.8.5.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 57/63] USB: cdc-acm: simplify runtime PM locking
We can simply the runtime PM locking as there's no need to check the susp_count in the read path (at least not since killing the rx tasklet). Specifically, the read urbs will never be resubmitted by the completion handler when killed during suspend. Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 17 ++--- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 6bbd203f1861..bc7a2a6fc4ac 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -428,7 +428,7 @@ static void acm_read_bulk_callback(struct urb *urb) /* throttle device if requested by tty */ spin_lock_irqsave(&acm->read_lock, flags); acm->throttled = acm->throttle_req; - if (!acm->throttled && !acm->susp_count) { + if (!acm->throttled) { spin_unlock_irqrestore(&acm->read_lock, flags); acm_submit_read_urb(acm, rb->index, GFP_ATOMIC); } else { @@ -1546,18 +1546,15 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message) struct acm *acm = usb_get_intfdata(intf); int cnt; - spin_lock_irq(&acm->read_lock); - spin_lock(&acm->write_lock); + spin_lock_irq(&acm->write_lock); if (PMSG_IS_AUTO(message)) { if (acm->transmitting) { - spin_unlock(&acm->write_lock); - spin_unlock_irq(&acm->read_lock); + spin_unlock_irq(&acm->write_lock); return -EBUSY; } } cnt = acm->susp_count++; - spin_unlock(&acm->write_lock); - spin_unlock_irq(&acm->read_lock); + spin_unlock_irq(&acm->write_lock); if (cnt) return 0; @@ -1573,8 +1570,7 @@ static int acm_resume(struct usb_interface *intf) struct urb *urb; int rv = 0; - spin_lock_irq(&acm->read_lock); - spin_lock(&acm->write_lock); + spin_lock_irq(&acm->write_lock); if (--acm->susp_count) goto out; @@ -1600,8 +1596,7 @@ static int acm_resume(struct usb_interface *intf) rv = acm_submit_read_urbs(acm, GFP_ATOMIC); } out: - spin_unlock(&acm->write_lock); - spin_unlock_irq(&acm->read_lock); + spin_unlock_irq(&acm->write_lock); return rv; } -- 1.8.5.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 59/63] USB: cdc-acm: minimise no-suspend window during shutdown
Now that acm_set_control() handles runtime PM properly, the only remaining reason for the PM operations in shutdown is to clear the needs_remote_wakeup flag before the final put. Note that this also means that we now need to grab the write_lock to prevent racing with resume. Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 91fdc293196f..f038f390db97 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -590,13 +590,22 @@ static void acm_port_shutdown(struct tty_port *port) struct urb *urb; struct acm_wb *wb; int i; - int pm_err; dev_dbg(&acm->control->dev, "%s\n", __func__); - pm_err = usb_autopm_get_interface(acm->control); acm_set_control(acm, acm->ctrlout = 0); + /* +* Need to grab write_lock to prevent race with resume, but no need to +* hold it due to the tty-port initialised flag. +*/ + spin_lock_irq(&acm->write_lock); + spin_unlock_irq(&acm->write_lock); + + usb_autopm_get_interface_no_resume(acm->control); + acm->control->needs_remote_wakeup = 0; + usb_autopm_put_interface(acm->control); + for (;;) { urb = usb_get_from_anchor(&acm->delayed); if (!urb) @@ -611,10 +620,6 @@ static void acm_port_shutdown(struct tty_port *port) usb_kill_urb(acm->wb[i].urb); for (i = 0; i < acm->rx_buflimit; i++) usb_kill_urb(acm->read_urbs[i]); - - acm->control->needs_remote_wakeup = 0; - if (!pm_err) - usb_autopm_put_interface(acm->control); } static void acm_tty_cleanup(struct tty_struct *tty) -- 1.8.5.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 28/63] USB: usb_wwan: fix potential NULL-deref at resume
The interrupt urb was submitted unconditionally at resume, something which could lead to a NULL-pointer dereference in the urb completion handler as resume may be called after the port and port data is gone. Fix this by making sure the interrupt urb is only submitted and active when the port is open. Fixes: 383cedc3bb43 ("USB: serial: full autosuspend support for the option driver") Cc: # v2.6.32: 032129cb03df Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 43 +++ 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 2ab478b55759..c5b9deb2e04e 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -388,6 +388,14 @@ int usb_wwan_open(struct tty_struct *tty, struct usb_serial_port *port) portdata = usb_get_serial_port_data(port); intfdata = serial->private; + if (port->interrupt_in_urb) { + err = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); + if (err) { + dev_dbg(&port->dev, "%s: submit int urb failed: %d\n", + __func__, err); + } + } + /* Start reading from the IN endpoint */ for (i = 0; i < N_IN_URB; i++) { urb = portdata->in_urbs[i]; @@ -454,6 +462,7 @@ void usb_wwan_close(struct usb_serial_port *port) usb_kill_urb(portdata->in_urbs[i]); for (i = 0; i < N_OUT_URB; i++) usb_kill_urb(portdata->out_urbs[i]); + usb_kill_urb(port->interrupt_in_urb); /* balancing - important as an error cannot be handled*/ usb_autopm_get_interface_no_resume(serial->interface); @@ -487,7 +496,6 @@ int usb_wwan_port_probe(struct usb_serial_port *port) struct usb_wwan_port_private *portdata; struct urb *urb; u8 *buffer; - int err; int i; if (!port->bulk_in_size || !port->bulk_out_size) @@ -527,13 +535,6 @@ int usb_wwan_port_probe(struct usb_serial_port *port) usb_set_serial_port_data(port, portdata); - if (port->interrupt_in_urb) { - err = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); - if (err) - dev_dbg(&port->dev, "%s: submit irq_in urb failed %d\n", - __func__, err); - } - return 0; bail_out_error2: @@ -651,22 +652,6 @@ int usb_wwan_resume(struct usb_serial *serial) struct urb *urb; int err = 0; - /* get the interrupt URBs resubmitted unconditionally */ - for (i = 0; i < serial->num_ports; i++) { - port = serial->port[i]; - if (!port->interrupt_in_urb) { - dev_dbg(&port->dev, "%s: No interrupt URB for port\n", __func__); - continue; - } - err = usb_submit_urb(port->interrupt_in_urb, GFP_NOIO); - dev_dbg(&port->dev, "Submitted interrupt URB for port (result %d)\n", err); - if (err < 0) { - dev_err(&port->dev, "%s: Error %d for interrupt URB\n", - __func__, err); - goto err_out; - } - } - spin_lock_irq(&intfdata->susp_lock); for (i = 0; i < serial->num_ports; i++) { /* walk all ports */ @@ -677,6 +662,16 @@ int usb_wwan_resume(struct usb_serial *serial) if (!portdata || !portdata->opened) continue; + if (port->interrupt_in_urb) { + err = usb_submit_urb(port->interrupt_in_urb, + GFP_ATOMIC); + if (err) { + dev_err(&port->dev, + "%s: submit int urb failed: %d\n", + __func__, err); + } + } + for (j = 0; j < N_IN_URB; j++) { urb = portdata->in_urbs[j]; err = usb_submit_urb(urb, GFP_ATOMIC); -- 1.8.5.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 36/63] USB: usb_wwan: make resume error messages uniform
Make resume error messages uniform. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index b671d59e356e..3737006930c0 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -672,8 +672,9 @@ int usb_wwan_resume(struct usb_serial *serial) urb = portdata->in_urbs[j]; err = usb_submit_urb(urb, GFP_ATOMIC); if (err < 0) { - dev_err(&port->dev, "%s: Error %d for bulk URB %d\n", - __func__, err, i); + dev_err(&port->dev, + "%s: submit read urb %d failed: %d\n", + __func__, i, err); err_count++; } } -- 1.8.5.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 41/63] USB: usb_wwan: remove bogus function prototype
The usb_wwan_send_setup() function has never existed. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-wwan.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/serial/usb-wwan.h b/drivers/usb/serial/usb-wwan.h index dc379603c4b8..8502f9add334 100644 --- a/drivers/usb/serial/usb-wwan.h +++ b/drivers/usb/serial/usb-wwan.h @@ -16,7 +16,6 @@ extern int usb_wwan_tiocmset(struct tty_struct *tty, unsigned int set, unsigned int clear); extern int usb_wwan_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg); -extern int usb_wwan_send_setup(struct usb_serial_port *port); extern int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port, const unsigned char *buf, int count); extern int usb_wwan_chars_in_buffer(struct tty_struct *tty); -- 1.8.5.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 50/63] USB: cdc-acm: fix runtime PM for control messages
Fix runtime PM handling of control messages by adding the required PM counter operations. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Cc: # v2.6.27 Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 56419254ef75..2258827df084 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -122,13 +122,23 @@ static void acm_release_minor(struct acm *acm) static int acm_ctrl_msg(struct acm *acm, int request, int value, void *buf, int len) { - int retval = usb_control_msg(acm->dev, usb_sndctrlpipe(acm->dev, 0), + int retval; + + retval = usb_autopm_get_interface(acm->control); + if (retval) + return retval; + + retval = usb_control_msg(acm->dev, usb_sndctrlpipe(acm->dev, 0), request, USB_RT_ACM, value, acm->control->altsetting[0].desc.bInterfaceNumber, buf, len, 5000); + dev_dbg(&acm->control->dev, "%s - rq 0x%02x, val %#x, len %#x, result %d\n", __func__, request, value, len, retval); + + usb_autopm_put_interface(acm->control); + return retval < 0 ? retval : 0; } -- 1.8.5.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 37/63] USB: usb_wwan: use interface-data accessors
Use usb_get_serial_data() rather than accessing the private pointer directly. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 3737006930c0..b83aa60bbbdb 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -41,7 +41,7 @@ void usb_wwan_dtr_rts(struct usb_serial_port *port, int on) struct usb_wwan_port_private *portdata; struct usb_wwan_intf_private *intfdata; - intfdata = port->serial->private; + intfdata = usb_get_serial_data(port->serial); if (!intfdata->send_setup) return; @@ -82,7 +82,7 @@ int usb_wwan_tiocmset(struct tty_struct *tty, struct usb_wwan_intf_private *intfdata; portdata = usb_get_serial_port_data(port); - intfdata = port->serial->private; + intfdata = usb_get_serial_data(port->serial); if (!intfdata->send_setup) return -EINVAL; @@ -191,7 +191,7 @@ int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port, unsigned long flags; portdata = usb_get_serial_port_data(port); - intfdata = port->serial->private; + intfdata = usb_get_serial_data(port->serial); dev_dbg(&port->dev, "%s: write (%d chars)\n", __func__, count); @@ -302,7 +302,7 @@ static void usb_wwan_outdat_callback(struct urb *urb) int i; port = urb->context; - intfdata = port->serial->private; + intfdata = usb_get_serial_data(port->serial); usb_serial_port_softint(port); usb_autopm_put_interface_async(port->serial->interface); @@ -372,7 +372,7 @@ int usb_wwan_open(struct tty_struct *tty, struct usb_serial_port *port) struct urb *urb; portdata = usb_get_serial_port_data(port); - intfdata = serial->private; + intfdata = usb_get_serial_data(serial); if (port->interrupt_in_urb) { err = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); @@ -424,7 +424,7 @@ void usb_wwan_close(struct usb_serial_port *port) int i; struct usb_serial *serial = port->serial; struct usb_wwan_port_private *portdata; - struct usb_wwan_intf_private *intfdata = port->serial->private; + struct usb_wwan_intf_private *intfdata = usb_get_serial_data(serial); struct urb *urb; portdata = usb_get_serial_port_data(port); @@ -584,7 +584,7 @@ static void stop_urbs(struct usb_serial *serial) int usb_wwan_suspend(struct usb_serial *serial, pm_message_t message) { - struct usb_wwan_intf_private *intfdata = serial->private; + struct usb_wwan_intf_private *intfdata = usb_get_serial_data(serial); spin_lock_irq(&intfdata->susp_lock); if (PMSG_IS_AUTO(message)) { @@ -605,14 +605,14 @@ EXPORT_SYMBOL(usb_wwan_suspend); static int play_delayed(struct usb_serial_port *port) { struct usb_serial *serial = port->serial; - struct usb_wwan_intf_private *data; + struct usb_wwan_intf_private *data = usb_get_serial_data(serial); struct usb_wwan_port_private *portdata; struct urb *urb; int err_count = 0; int err; portdata = usb_get_serial_port_data(port); - data = port->serial->private; + while ((urb = usb_get_from_anchor(&portdata->delayed))) { err = usb_submit_urb(urb, GFP_ATOMIC); if (err) { @@ -637,7 +637,7 @@ int usb_wwan_resume(struct usb_serial *serial) { int i, j; struct usb_serial_port *port; - struct usb_wwan_intf_private *intfdata = serial->private; + struct usb_wwan_intf_private *intfdata = usb_get_serial_data(serial); struct usb_wwan_port_private *portdata; struct urb *urb; int err; -- 1.8.5.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 49/63] USB: cdc-acm: fix broken runtime suspend
The current ACM runtime-suspend implementation is broken in several ways: Firstly, it buffers only the first write request being made while suspended -- any further writes are silently dropped. Secondly, writes being dropped also leak write urbs, which are never reclaimed (until the device is unbound). Thirdly, even the single buffered write is not cleared at shutdown (which may happen before the device is resumed), something which can lead to another urb leak as well as a PM usage-counter leak. Fix this by implementing a delayed-write queue using urb anchors and making sure to discard the queue properly at shutdown. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Reported-by: Xiao Jin Cc: # v2.6.27 Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 32 ++-- drivers/usb/class/cdc-acm.h | 2 +- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index e72a657a6177..56419254ef75 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -573,6 +573,8 @@ static void acm_port_destruct(struct tty_port *port) static void acm_port_shutdown(struct tty_port *port) { struct acm *acm = container_of(port, struct acm, port); + struct urb *urb; + struct acm_wb *wb; int i; dev_dbg(&acm->control->dev, "%s\n", __func__); @@ -581,6 +583,16 @@ static void acm_port_shutdown(struct tty_port *port) if (!acm->disconnected) { usb_autopm_get_interface(acm->control); acm_set_control(acm, acm->ctrlout = 0); + + for (;;) { + urb = usb_get_from_anchor(&acm->delayed); + if (!urb) + break; + wb = urb->context; + wb->use = 0; + usb_autopm_put_interface_async(acm->control); + } + usb_kill_urb(acm->ctrlurb); for (i = 0; i < ACM_NW; i++) usb_kill_urb(acm->wb[i].urb); @@ -648,12 +660,9 @@ static int acm_tty_write(struct tty_struct *tty, usb_autopm_get_interface_async(acm->control); if (acm->susp_count) { - if (!acm->delayed_wb) - acm->delayed_wb = wb; - else - usb_autopm_put_interface_async(acm->control); + usb_anchor_urb(wb->urb, &acm->delayed); spin_unlock_irqrestore(&acm->write_lock, flags); - return count; /* A white lie */ + return count; } usb_mark_last_busy(acm->dev); @@ -1269,6 +1278,7 @@ made_compressed_probe: acm->bInterval = epread->bInterval; tty_port_init(&acm->port); acm->port.ops = &acm_port_ops; + init_usb_anchor(&acm->delayed); buf = usb_alloc_coherent(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma); if (!buf) { @@ -1539,7 +1549,7 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message) static int acm_resume(struct usb_interface *intf) { struct acm *acm = usb_get_intfdata(intf); - struct acm_wb *wb; + struct urb *urb; int rv = 0; spin_lock_irq(&acm->read_lock); @@ -1551,10 +1561,12 @@ static int acm_resume(struct usb_interface *intf) if (test_bit(ASYNCB_INITIALIZED, &acm->port.flags)) { rv = usb_submit_urb(acm->ctrlurb, GFP_ATOMIC); - if (acm->delayed_wb) { - wb = acm->delayed_wb; - acm->delayed_wb = NULL; - acm_start_wb(acm, wb); + for (;;) { + urb = usb_get_from_anchor(&acm->delayed); + if (!urb) + break; + + acm_start_wb(acm, urb->context); } /* diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h index e38dc785808f..80826f843e04 100644 --- a/drivers/usb/class/cdc-acm.h +++ b/drivers/usb/class/cdc-acm.h @@ -120,7 +120,7 @@ struct acm { unsigned int throttled:1; /* actually throttled */ unsigned int throttle_req:1;/* throttle requested */ u8 bInterval; - struct acm_wb *delayed_wb; /* write queued for a device about to be woken */ + struct usb_anchor delayed; /* writes queued for a device about to be woken */ }; #define CDC_DATA_INTERFACE_TYPE0x0a -- 1.8.5.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 13/63] USB: sierra: remove disconnected test from close
Remove no longer needed disconnected test from close, which is never called post disconnect (and drivers must handle failed I/O during disconnect anyway). Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 854ac61581ba..74b417c91e30 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -772,16 +772,12 @@ static void sierra_close(struct usb_serial_port *port) portdata->rts_state = 0; portdata->dtr_state = 0; - mutex_lock(&serial->disc_mutex); - if (!serial->disconnected) { - /* odd error handling due to pm counters */ - if (!usb_autopm_get_interface(serial->interface)) - sierra_send_setup(port); - else - usb_autopm_get_interface_no_resume(serial->interface); + /* odd error handling due to pm counters */ + if (!usb_autopm_get_interface(serial->interface)) + sierra_send_setup(port); + else + usb_autopm_get_interface_no_resume(serial->interface); - } - mutex_unlock(&serial->disc_mutex); spin_lock_irq(&intfdata->susp_lock); portdata->opened = 0; if (--intfdata->open_ports == 0) -- 1.8.5.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 51/63] USB: cdc-acm: fix shutdown and suspend race
We should stop I/O unconditionally at suspend rather than rely on the tty-port initialised flag (which is set prior to stopping I/O during shutdown) in order to prevent suspend returning with URBs still active. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Cc: # v2.6.27 Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 2258827df084..1ac6c5dda9f7 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1550,8 +1550,7 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message) if (cnt) return 0; - if (test_bit(ASYNCB_INITIALIZED, &acm->port.flags)) - stop_data_traffic(acm); + stop_data_traffic(acm); return 0; } -- 1.8.5.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 33/63] USB: usb_wwan: remove unimplemented set_termios
The driver does not implement set_termios so the operation can be left unset (tty will do the tty_termios_copy_hw for us). Note that the send_setup call is bogus as it really only sets DTR/RTS to their current values. Signed-off-by: Johan Hovold --- drivers/usb/serial/option.c | 1 - drivers/usb/serial/usb-wwan.h | 3 --- drivers/usb/serial/usb_wwan.c | 14 -- 3 files changed, 18 deletions(-) diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c index df91ea9243df..51e30740b2fe 100644 --- a/drivers/usb/serial/option.c +++ b/drivers/usb/serial/option.c @@ -1729,7 +1729,6 @@ static struct usb_serial_driver option_1port_device = { .write = usb_wwan_write, .write_room= usb_wwan_write_room, .chars_in_buffer = usb_wwan_chars_in_buffer, - .set_termios = usb_wwan_set_termios, .tiocmget = usb_wwan_tiocmget, .tiocmset = usb_wwan_tiocmset, .ioctl = usb_wwan_ioctl, diff --git a/drivers/usb/serial/usb-wwan.h b/drivers/usb/serial/usb-wwan.h index aca45efbd674..dc379603c4b8 100644 --- a/drivers/usb/serial/usb-wwan.h +++ b/drivers/usb/serial/usb-wwan.h @@ -11,9 +11,6 @@ extern void usb_wwan_close(struct usb_serial_port *port); extern int usb_wwan_port_probe(struct usb_serial_port *port); extern int usb_wwan_port_remove(struct usb_serial_port *port); extern int usb_wwan_write_room(struct tty_struct *tty); -extern void usb_wwan_set_termios(struct tty_struct *tty, -struct usb_serial_port *port, -struct ktermios *old); extern int usb_wwan_tiocmget(struct tty_struct *tty); extern int usb_wwan_tiocmset(struct tty_struct *tty, unsigned int set, unsigned int clear); diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 9aeaccfd4f74..aa45985520f7 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -55,20 +55,6 @@ void usb_wwan_dtr_rts(struct usb_serial_port *port, int on) } EXPORT_SYMBOL(usb_wwan_dtr_rts); -void usb_wwan_set_termios(struct tty_struct *tty, - struct usb_serial_port *port, - struct ktermios *old_termios) -{ - struct usb_wwan_intf_private *intfdata = port->serial->private; - - /* Doesn't support option setting */ - tty_termios_copy_hw(&tty->termios, old_termios); - - if (intfdata->send_setup) - intfdata->send_setup(port); -} -EXPORT_SYMBOL(usb_wwan_set_termios); - int usb_wwan_tiocmget(struct tty_struct *tty) { struct usb_serial_port *port = tty->driver_data; -- 1.8.5.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 60/63] USB: cdc-acm: do not update PM busy on read errors
There's no need to update the runtime PM last_busy field on read urb errors (e.g. when the urb is being killed on shutdown). Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index f038f390db97..3c7cfac48e30 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -416,13 +416,15 @@ static void acm_read_bulk_callback(struct urb *urb) dev_dbg(&acm->data->dev, "%s - disconnected\n", __func__); return; } - usb_mark_last_busy(acm->dev); if (urb->status) { dev_dbg(&acm->data->dev, "%s - non-zero urb status: %d\n", __func__, urb->status); return; } + + usb_mark_last_busy(acm->dev); + acm_process_read_urb(acm, urb); /* throttle device if requested by tty */ -- 1.8.5.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 42/63] USB: usb_wwan: report failed submissions as errors
Promote failed-submission messages in open() and write() to error log level. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index c951f75891c9..bbcbdaa16c17 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -231,9 +231,9 @@ int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port, spin_unlock_irqrestore(&intfdata->susp_lock, flags); err = usb_submit_urb(this_urb, GFP_ATOMIC); if (err) { - dev_dbg(&port->dev, - "usb_submit_urb %p (write bulk) failed (%d)\n", - this_urb, err); + dev_err(&port->dev, + "%s: submit urb %d failed: %d\n", + __func__, i, err); clear_bit(i, &portdata->out_busy); spin_lock_irqsave(&intfdata->susp_lock, flags); intfdata->in_flight--; @@ -376,7 +376,7 @@ int usb_wwan_open(struct tty_struct *tty, struct usb_serial_port *port) if (port->interrupt_in_urb) { err = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); if (err) { - dev_dbg(&port->dev, "%s: submit int urb failed: %d\n", + dev_err(&port->dev, "%s: submit int urb failed: %d\n", __func__, err); } } @@ -388,8 +388,9 @@ int usb_wwan_open(struct tty_struct *tty, struct usb_serial_port *port) continue; err = usb_submit_urb(urb, GFP_KERNEL); if (err) { - dev_dbg(&port->dev, "%s: submit urb %d failed (%d) %d\n", - __func__, i, err, urb->transfer_buffer_length); + dev_err(&port->dev, + "%s: submit read urb %d failed: %d\n", + __func__, i, err); } } -- 1.8.5.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 53/63] USB: cdc-acm: fix open and suspend race
We must not do the usb_autopm_put_interface() before submitting the read urbs or we might end up doing I/O to a suspended device. Fixes: 088c64f81284 ("USB: cdc-acm: re-write read processing") Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index c255e77282ad..74df311c8505 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -528,19 +528,15 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) if (usb_submit_urb(acm->ctrlurb, GFP_KERNEL)) { dev_err(&acm->control->dev, "%s - usb_submit_urb(ctrl irq) failed\n", __func__); - usb_autopm_put_interface(acm->control); goto error_submit_urb; } acm->ctrlout = ACM_CTRL_DTR | ACM_CTRL_RTS; if (acm_set_control(acm, acm->ctrlout) < 0 && (acm->ctrl_caps & USB_CDC_CAP_LINE)) { - usb_autopm_put_interface(acm->control); goto error_set_control; } - usb_autopm_put_interface(acm->control); - /* * Unthrottle device in case the TTY was closed while throttled. */ @@ -552,6 +548,8 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) if (acm_submit_read_urbs(acm, GFP_KERNEL)) goto error_submit_read_urbs; + usb_autopm_put_interface(acm->control); + mutex_unlock(&acm->mutex); return 0; @@ -562,6 +560,7 @@ error_submit_read_urbs: error_set_control: usb_kill_urb(acm->ctrlurb); error_submit_urb: + usb_autopm_put_interface(acm->control); error_get_interface: disconnected: mutex_unlock(&acm->mutex); -- 1.8.5.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 47/63] USB: cdc-acm: fix write and suspend race
Fix race between write() and suspend() which could lead to writes being dropped (or I/O while suspended) if the device is runtime suspended while a write request is being processed. Specifically, suspend() releases the write_lock after determining the device is idle but before incrementing the susp_count, thus leaving a window where a concurrent write() can submit an urb. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Cc: # v2.6.27 Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 904efb6035b0..3bd4226c13dc 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1514,18 +1514,15 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message) struct acm *acm = usb_get_intfdata(intf); int cnt; + spin_lock_irq(&acm->read_lock); + spin_lock(&acm->write_lock); if (PMSG_IS_AUTO(message)) { - int b; - - spin_lock_irq(&acm->write_lock); - b = acm->transmitting; - spin_unlock_irq(&acm->write_lock); - if (b) + if (acm->transmitting) { + spin_unlock(&acm->write_lock); + spin_unlock_irq(&acm->read_lock); return -EBUSY; + } } - - spin_lock_irq(&acm->read_lock); - spin_lock(&acm->write_lock); cnt = acm->susp_count++; spin_unlock(&acm->write_lock); spin_unlock_irq(&acm->read_lock); -- 1.8.5.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 38/63] USB: usb_wwan: clean up delayed-urb submission
Clean up and rename delay-urb submission function using a more descriptive name. Also add comment on locking assumptions. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index b83aa60bbbdb..45bc11b2d0ec 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -602,7 +602,8 @@ int usb_wwan_suspend(struct usb_serial *serial, pm_message_t message) } EXPORT_SYMBOL(usb_wwan_suspend); -static int play_delayed(struct usb_serial_port *port) +/* Caller must hold susp_lock. */ +static int usb_wwan_submit_delayed_urbs(struct usb_serial_port *port) { struct usb_serial *serial = port->serial; struct usb_wwan_intf_private *data = usb_get_serial_data(serial); @@ -613,11 +614,14 @@ static int play_delayed(struct usb_serial_port *port) portdata = usb_get_serial_port_data(port); - while ((urb = usb_get_from_anchor(&portdata->delayed))) { + for (;;) { + urb = usb_get_from_anchor(&portdata->delayed); + if (!urb) + break; + err = usb_submit_urb(urb, GFP_ATOMIC); if (err) { - dev_err(&port->dev, - "%s: submit write urb failed: %d\n", + dev_err(&port->dev, "%s: submit urb failed: %d\n", __func__, err); err_count++; unbusy_queued_urb(urb, portdata); @@ -664,7 +668,7 @@ int usb_wwan_resume(struct usb_serial *serial) } } - err = play_delayed(port); + err = usb_wwan_submit_delayed_urbs(port); if (err) err_count++; -- 1.8.5.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 10/63] USB: sierra: remove bogus endpoint test
Remove bogus endpoint-address test which is never true. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 4cb11b710b48..169899f5f2e6 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -747,9 +747,6 @@ static struct urb *sierra_setup_urb(struct usb_serial *serial, int endpoint, struct urb *urb; u8 *buf; - if (endpoint == -1) - return NULL; - urb = usb_alloc_urb(0, mem_flags); if (!urb) return NULL; -- 1.8.5.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 19/63] USB: sierra: minimise no-suspend window during close
Move usb_autopm_get_interface_no_resume to the end of close(). This makes the window during which suspend is prevented before the final put in USB serial core slightly smaller. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 967331964f36..be4a759b3aed 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -769,8 +769,6 @@ static void sierra_close(struct usb_serial_port *port) portdata = usb_get_serial_port_data(port); - usb_autopm_get_interface_no_resume(serial->interface); - spin_lock_irq(&intfdata->susp_lock); portdata->opened = 0; if (--intfdata->open_ports == 0) @@ -796,6 +794,8 @@ static void sierra_close(struct usb_serial_port *port) sierra_release_urb(portdata->in_urbs[i]); portdata->in_urbs[i] = NULL; } + + usb_autopm_get_interface_no_resume(serial->interface); } static int sierra_open(struct tty_struct *tty, struct usb_serial_port *port) -- 1.8.5.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 24/63] USB: usb_wwan: fix urb leak in write error path
From: xiao jin When enable usb serial for modem data, sometimes the tty is blocked in tty_wait_until_sent because portdata->out_busy always is set and have no chance to be cleared. We find a bug in write error path. usb_wwan_write set portdata->out_busy firstly, then try autopm async with error. No out urb submit and no usb_wwan_outdat_callback to this write, portdata->out_busy can't be cleared. This patch clear portdata->out_busy if usb_wwan_write try autopm async with error. Fixes: 383cedc3bb43 ("USB: serial: full autosuspend support for the option driver") Signed-off-by: xiao jin Signed-off-by: Zhang, Qi1 Reviewed-by: David Cohen Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index b078440e822f..47ad7550c5a6 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -228,8 +228,10 @@ int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port, usb_pipeendpoint(this_urb->pipe), i); err = usb_autopm_get_interface_async(port->serial->interface); - if (err < 0) + if (err < 0) { + clear_bit(i, &portdata->out_busy); break; + } /* send the data */ memcpy(this_urb->transfer_buffer, buf, todo); -- 1.8.5.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 11/63] USB: sierra: remove unused variable
Remove unused variable from sierra_release_urb. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 169899f5f2e6..fa0b78ad42d7 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -419,9 +419,7 @@ static int sierra_tiocmset(struct tty_struct *tty, static void sierra_release_urb(struct urb *urb) { - struct usb_serial_port *port; if (urb) { - port = urb->context; kfree(urb->transfer_buffer); usb_free_urb(urb); } -- 1.8.5.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 43/63] USB: usb_wwan: do not resume I/O on closing ports
Use tty-port initialised flag rather than private flag to determine when port is closing down. Since the tty-port flag is set prior to dropping DTR/RTS (when HUPCL is set) this avoid submitting the read urbs when resuming the interface in dtr_rts() only to immediately kill them again in shutdown(). Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-wwan.h | 1 - drivers/usb/serial/usb_wwan.c | 11 +++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/usb-wwan.h b/drivers/usb/serial/usb-wwan.h index 8502f9add334..f22dff58b587 100644 --- a/drivers/usb/serial/usb-wwan.h +++ b/drivers/usb/serial/usb-wwan.h @@ -48,7 +48,6 @@ struct usb_wwan_port_private { struct urb *out_urbs[N_OUT_URB]; u8 *out_buffer[N_OUT_URB]; unsigned long out_busy; /* Bit vector of URBs in use */ - int opened; struct usb_anchor delayed; /* Settings for the port */ diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index bbcbdaa16c17..2932d9cfb166 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -395,7 +395,6 @@ int usb_wwan_open(struct tty_struct *tty, struct usb_serial_port *port) } spin_lock_irq(&intfdata->susp_lock); - portdata->opened = 1; if (++intfdata->open_ports == 1) serial->interface->needs_remote_wakeup = 1; spin_unlock_irq(&intfdata->susp_lock); @@ -429,8 +428,11 @@ void usb_wwan_close(struct usb_serial_port *port) portdata = usb_get_serial_port_data(port); + /* +* Need to take susp_lock to make sure port is not already being +* resumed, but no need to hold it due to ASYNC_INITIALIZED. +*/ spin_lock_irq(&intfdata->susp_lock); - portdata->opened = 0; if (--intfdata->open_ports == 0) serial->interface->needs_remote_wakeup = 0; spin_unlock_irq(&intfdata->susp_lock); @@ -645,11 +647,12 @@ int usb_wwan_resume(struct usb_serial *serial) spin_lock_irq(&intfdata->susp_lock); for (i = 0; i < serial->num_ports; i++) { port = serial->port[i]; - portdata = usb_get_serial_port_data(port); - if (!portdata || !portdata->opened) + if (!test_bit(ASYNCB_INITIALIZED, &port->port.flags)) continue; + portdata = usb_get_serial_port_data(port); + if (port->interrupt_in_urb) { err = usb_submit_urb(port->interrupt_in_urb, GFP_ATOMIC); -- 1.8.5.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 12/63] USB: sierra: remove unimplemented set_termios
The driver does not implement set_termios so the operation can be left unset (tty will do the tty_termios_copy_hw for us). Note that the send_setup call is bogus as it really only sets DTR/RTS to their current values. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index fa0b78ad42d7..854ac61581ba 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -372,13 +372,6 @@ static int sierra_send_setup(struct usb_serial_port *port) return retval; } -static void sierra_set_termios(struct tty_struct *tty, - struct usb_serial_port *port, struct ktermios *old_termios) -{ - tty_termios_copy_hw(&tty->termios, old_termios); - sierra_send_setup(port); -} - static int sierra_tiocmget(struct tty_struct *tty) { struct usb_serial_port *port = tty->driver_data; @@ -1079,7 +1072,6 @@ static struct usb_serial_driver sierra_device = { .write = sierra_write, .write_room= sierra_write_room, .chars_in_buffer = sierra_chars_in_buffer, - .set_termios = sierra_set_termios, .tiocmget = sierra_tiocmget, .tiocmset = sierra_tiocmset, .attach= sierra_startup, -- 1.8.5.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 07/63] USB: sierra: fix urbs not being killed on shutdown
Make sure to stop all I/O, including any active write urbs, at shutdown. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index ed43b18ace78..96ad379a0681 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -813,6 +813,8 @@ static void sierra_close(struct usb_serial_port *port) } sierra_stop_rx_urbs(port); + usb_kill_anchored_urbs(&portdata->active); + for (i = 0; i < portdata->num_in_urbs; i++) { sierra_release_urb(portdata->in_urbs[i]); portdata->in_urbs[i] = NULL; -- 1.8.5.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 20/63] USB: sierra: do not resume I/O on closing ports
Use tty-port initialised flag rather than private flag to determine when port is closing down. Since the tty-port flag is set prior to dropping DTR/RTS (when HUPCL is set) this avoid submitting the read urbs when resuming the interface in dtr_rts() only to immediately kill them again in shutdown(). Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index be4a759b3aed..6f7f01eb556a 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -316,7 +316,6 @@ struct sierra_port_private { int dsr_state; int dcd_state; int ri_state; - unsigned int opened:1; }; static int sierra_send_setup(struct usb_serial_port *port) @@ -769,8 +768,11 @@ static void sierra_close(struct usb_serial_port *port) portdata = usb_get_serial_port_data(port); + /* +* Need to take susp_lock to make sure port is not already being +* resumed, but no need to hold it due to ASYNC_INITIALIZED. +*/ spin_lock_irq(&intfdata->susp_lock); - portdata->opened = 0; if (--intfdata->open_ports == 0) serial->interface->needs_remote_wakeup = 0; spin_unlock_irq(&intfdata->susp_lock); @@ -826,7 +828,6 @@ static int sierra_open(struct tty_struct *tty, struct usb_serial_port *port) goto err_submit; spin_lock_irq(&intfdata->susp_lock); - portdata->opened = 1; if (++intfdata->open_ports == 1) serial->interface->needs_remote_wakeup = 1; spin_unlock_irq(&intfdata->susp_lock); @@ -1025,16 +1026,14 @@ static int sierra_resume(struct usb_serial *serial) { struct usb_serial_port *port; struct sierra_intf_private *intfdata = usb_get_serial_data(serial); - struct sierra_port_private *portdata; int ec = 0; int i, err; spin_lock_irq(&intfdata->susp_lock); for (i = 0; i < serial->num_ports; i++) { port = serial->port[i]; - portdata = usb_get_serial_port_data(port); - if (!portdata || !portdata->opened) + if (!test_bit(ASYNCB_INITIALIZED, &port->port.flags)) continue; err = sierra_submit_delayed_urbs(port); -- 1.8.5.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 18/63] USB: sierra: refactor delayed-urb submission
Refactor and clean up delayed-urb submission at resume. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 63 +++-- 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 1d42e8305b8e..967331964f36 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -981,12 +981,51 @@ static int sierra_suspend(struct usb_serial *serial, pm_message_t message) return 0; } +/* Caller must hold susp_lock. */ +static int sierra_submit_delayed_urbs(struct usb_serial_port *port) +{ + struct sierra_port_private *portdata = usb_get_serial_port_data(port); + struct sierra_intf_private *intfdata; + struct urb *urb; + int ec = 0; + int err; + + intfdata = usb_get_serial_data(port->serial); + + for (;;) { + urb = usb_get_from_anchor(&portdata->delayed); + if (!urb) + break; + + usb_anchor_urb(urb, &portdata->active); + intfdata->in_flight++; + err = usb_submit_urb(urb, GFP_ATOMIC); + if (err) { + dev_err(&port->dev, "%s - submit urb failed: %d", + __func__, err); + ec++; + intfdata->in_flight--; + usb_unanchor_urb(urb); + kfree(urb->transfer_buffer); + usb_free_urb(urb); + + spin_lock(&portdata->lock); + portdata->outstanding_urbs--; + spin_unlock(&portdata->lock); + } + } + + if (ec) + return -EIO; + + return 0; +} + static int sierra_resume(struct usb_serial *serial) { struct usb_serial_port *port; struct sierra_intf_private *intfdata = usb_get_serial_data(serial); struct sierra_port_private *portdata; - struct urb *urb; int ec = 0; int i, err; @@ -998,25 +1037,9 @@ static int sierra_resume(struct usb_serial *serial) if (!portdata || !portdata->opened) continue; - while ((urb = usb_get_from_anchor(&portdata->delayed))) { - usb_anchor_urb(urb, &portdata->active); - intfdata->in_flight++; - err = usb_submit_urb(urb, GFP_ATOMIC); - if (err < 0) { - dev_err(&port->dev, - "%s - submit urb failed: %d", - __func__, err); - ec++; - intfdata->in_flight--; - usb_unanchor_urb(urb); - kfree(urb->transfer_buffer); - usb_free_urb(urb); - spin_lock(&portdata->lock); - portdata->outstanding_urbs--; - spin_unlock(&portdata->lock); - continue; - } - } + err = sierra_submit_delayed_urbs(port); + if (err) + ec++; err = sierra_submit_rx_urbs(port, GFP_ATOMIC); if (err) -- 1.8.5.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 08/63] USB: sierra: fix resume error reporting
Add error message to resume error path and make sure to also return an error when failing to submit a cached write. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 96ad379a0681..0254f6d6bc5d 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -1037,6 +1037,10 @@ static int sierra_resume(struct usb_serial *serial) intfdata->in_flight++; err = usb_submit_urb(urb, GFP_ATOMIC); if (err < 0) { + dev_err(&port->dev, + "%s - submit urb failed: %d", + __func__, err); + ec++; intfdata->in_flight--; usb_unanchor_urb(urb); kfree(urb->transfer_buffer); -- 1.8.5.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 45/63] USB: serial: remove overly defensive port tests
The only way a port pointer may be NULL is if probe() failed, and in that case neither disconnect(), resume(), or reset_resume() will be called. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-serial.c | 38 ++ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 6d40d56378d7..02de3110fe94 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -1060,6 +1060,7 @@ static void usb_serial_disconnect(struct usb_interface *interface) struct usb_serial *serial = usb_get_intfdata(interface); struct device *dev = &interface->dev; struct usb_serial_port *port; + struct tty_struct *tty; usb_serial_console_disconnect(serial); @@ -1070,18 +1071,16 @@ static void usb_serial_disconnect(struct usb_interface *interface) for (i = 0; i < serial->num_ports; ++i) { port = serial->port[i]; - if (port) { - struct tty_struct *tty = tty_port_tty_get(&port->port); - if (tty) { - tty_vhangup(tty); - tty_kref_put(tty); - } - usb_serial_port_poison_urbs(port); - wake_up_interruptible(&port->port.delta_msr_wait); - cancel_work_sync(&port->work); - if (device_is_registered(&port->dev)) - device_del(&port->dev); + tty = tty_port_tty_get(&port->port); + if (tty) { + tty_vhangup(tty); + tty_kref_put(tty); } + usb_serial_port_poison_urbs(port); + wake_up_interruptible(&port->port.delta_msr_wait); + cancel_work_sync(&port->work); + if (device_is_registered(&port->dev)) + device_del(&port->dev); } if (serial->type->disconnect) serial->type->disconnect(serial); @@ -1094,7 +1093,6 @@ static void usb_serial_disconnect(struct usb_interface *interface) int usb_serial_suspend(struct usb_interface *intf, pm_message_t message) { struct usb_serial *serial = usb_get_intfdata(intf); - struct usb_serial_port *port; int i, r = 0; serial->suspending = 1; @@ -1112,12 +1110,8 @@ int usb_serial_suspend(struct usb_interface *intf, pm_message_t message) } } - for (i = 0; i < serial->num_ports; ++i) { - port = serial->port[i]; - if (port) - usb_serial_port_poison_urbs(port); - } - + for (i = 0; i < serial->num_ports; ++i) + usb_serial_port_poison_urbs(serial->port[i]); err_out: return r; } @@ -1125,14 +1119,10 @@ EXPORT_SYMBOL(usb_serial_suspend); static void usb_serial_unpoison_port_urbs(struct usb_serial *serial) { - struct usb_serial_port *port; int i; - for (i = 0; i < serial->num_ports; ++i) { - port = serial->port[i]; - if (port) - usb_serial_port_unpoison_urbs(port); - } + for (i = 0; i < serial->num_ports; ++i) + usb_serial_port_unpoison_urbs(serial->port[i]); } int usb_serial_resume(struct usb_interface *intf) -- 1.8.5.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 01/63] USB: sierra: fix AA deadlock in open error path
Fix AA deadlock in open error path that would call close() and try to grab the already held disc_mutex. Fixes: b9a44bc19f48 ("sierra: driver urb handling improvements") Cc: # v2.6.31 Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 6b192e602ce0..07cbd985ed65 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -816,14 +816,9 @@ static int sierra_open(struct tty_struct *tty, struct usb_serial_port *port) usb_sndbulkpipe(serial->dev, endpoint) | USB_DIR_IN); err = sierra_submit_rx_urbs(port, GFP_KERNEL); - if (err) { - /* get rid of everything as in close */ - sierra_close(port); - /* restore balance for autopm */ - if (!serial->disconnected) - usb_autopm_put_interface(serial->interface); - return err; - } + if (err) + goto err_submit; + sierra_send_setup(port); serial->interface->needs_remote_wakeup = 1; @@ -833,6 +828,16 @@ static int sierra_open(struct tty_struct *tty, struct usb_serial_port *port) usb_autopm_put_interface(serial->interface); return 0; + +err_submit: + sierra_stop_rx_urbs(port); + + for (i = 0; i < portdata->num_in_urbs; i++) { + sierra_release_urb(portdata->in_urbs[i]); + portdata->in_urbs[i] = NULL; + } + + return err; } -- 1.8.5.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 31/63] USB: usb_wwan: fix remote wakeup
Make sure that needs_remote_wake up is always set when there are open ports. Currently close() would unconditionally set needs_remote_wakeup to 0 even though there might still be open ports. This could lead to blocked input and possibly dropped data on devices that do not support remote wakeup (and which must therefore not be runtime suspended while open). Add an open_ports counter (protected by the susp_lock) and only clear needs_remote_wakeup when the last port is closed. Note that there are currently no multi-port drivers using the usb_wwan implementation. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-wwan.h | 1 + drivers/usb/serial/usb_wwan.c | 6 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/usb-wwan.h b/drivers/usb/serial/usb-wwan.h index 684739b8efd0..aca45efbd674 100644 --- a/drivers/usb/serial/usb-wwan.h +++ b/drivers/usb/serial/usb-wwan.h @@ -39,6 +39,7 @@ struct usb_wwan_intf_private { spinlock_t susp_lock; unsigned int suspended:1; int in_flight; + unsigned int open_ports; int (*send_setup) (struct usb_serial_port *port); void *private; }; diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index ab8c17536534..daaa5d795946 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -411,9 +411,10 @@ int usb_wwan_open(struct tty_struct *tty, struct usb_serial_port *port) if (intfdata->send_setup) intfdata->send_setup(port); - serial->interface->needs_remote_wakeup = 1; spin_lock_irq(&intfdata->susp_lock); portdata->opened = 1; + if (++intfdata->open_ports == 1) + serial->interface->needs_remote_wakeup = 1; spin_unlock_irq(&intfdata->susp_lock); /* this balances a get in the generic USB serial code */ usb_autopm_put_interface(serial->interface); @@ -448,6 +449,8 @@ void usb_wwan_close(struct usb_serial_port *port) /* Stop reading/writing urbs */ spin_lock_irq(&intfdata->susp_lock); portdata->opened = 0; + if (--intfdata->open_ports == 0) + serial->interface->needs_remote_wakeup = 0; spin_unlock_irq(&intfdata->susp_lock); for (;;) { @@ -466,7 +469,6 @@ void usb_wwan_close(struct usb_serial_port *port) /* balancing - important as an error cannot be handled*/ usb_autopm_get_interface_no_resume(serial->interface); - serial->interface->needs_remote_wakeup = 0; } EXPORT_SYMBOL(usb_wwan_close); -- 1.8.5.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 14/63] USB: sierra: do not resume I/O on closed ports
Do not resume any I/O, including the delayed write queue, on closed ports. Note that this currently has no functional impact due to the usb_autopm_get_interface() in close(), but that call is about to be removed by a follow-up patch. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 74b417c91e30..ac5e20d9bd24 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -1013,7 +1013,7 @@ static int sierra_resume(struct usb_serial *serial) port = serial->port[i]; portdata = usb_get_serial_port_data(port); - if (!portdata) + if (!portdata || !portdata->opened) continue; while ((urb = usb_get_from_anchor(&portdata->delayed))) { @@ -1036,11 +1036,9 @@ static int sierra_resume(struct usb_serial *serial) } } - if (portdata->opened) { - err = sierra_submit_rx_urbs(port, GFP_ATOMIC); - if (err) - ec++; - } + err = sierra_submit_rx_urbs(port, GFP_ATOMIC); + if (err) + ec++; } intfdata->suspended = 0; spin_unlock_irq(&intfdata->susp_lock); -- 1.8.5.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 09/63] USB: sierra: fix line-control pipe direction
The sierra line-control request has been using the wrong pipe direction, while relying on USB core to fix it up. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 0254f6d6bc5d..4cb11b710b48 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -365,7 +365,7 @@ static int sierra_send_setup(struct usb_serial_port *port) if (retval < 0) return retval; - retval = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0), + retval = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0), 0x22, 0x21, val, interface, NULL, 0, USB_CTRL_SET_TIMEOUT); usb_autopm_put_interface(serial->interface); -- 1.8.5.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 00/63] USB: (mostly runtime PM) patches for v3.16-rc
Turns out that the three current implementations of aggressive runtime power management in USB-TTY drivers were quite badly broken (and has so been since first merged in v2.6.27 and v2.6.32). These patches fix this runtime PM mess, but also do some clean up and fix a few other issues. Note that all patches but the cdc-acm ones (and the single wusbcore one) are usb-serial, but I decided to include them all in one series as several bugs were reproduced in both cdc-acm and usb-serial. For reference, I'm responding to the thread with Xiao Jin's initial bug report, but the full thread is posted to linux-usb only. Johan Johan Hovold (61): USB: sierra: fix AA deadlock in open error path USB: sierra: fix use after free at suspend/resume USB: sierra: fix urb and memory leak in resume error path USB: sierra: fix urb and memory leak on disconnect USB: sierra: fix remote wakeup USB: sierra: fix characters being dropped at close USB: sierra: fix urbs not being killed on shutdown USB: sierra: fix resume error reporting USB: sierra: fix line-control pipe direction USB: sierra: remove bogus endpoint test USB: sierra: remove unused variable USB: sierra: remove unimplemented set_termios USB: sierra: remove disconnected test from close USB: sierra: do not resume I/O on closed ports USB: sierra: remove redundant modem-control requests USB: sierra: use interface-data accessors USB: sierra: clean up suspend USB: sierra: refactor delayed-urb submission USB: sierra: minimise no-suspend window during close USB: sierra: do not resume I/O on closing ports USB: option: fix runtime PM handling USB: option: fix line-control pipe direction USB: option: add missing usb_mark_last_busy USB: usb_wwan: fix write and suspend race USB: usb_wwan: fix urb leak at shutdown USB: usb_wwan: fix potential NULL-deref at resume USB: usb_wwan: fix potential blocked I/O after resume USB: usb_wwan: fix discarded writes on resume errors USB: usb_wwan: fix remote wakeup USB: usb_wwan: remove redundant modem-control request USB: usb_wwan: remove unimplemented set_termios USB: usb_wwan: remove redundant urb kill from port remove USB: usb_wwan: kill interrupt urb explicitly at suspend USB: usb_wwan: make resume error messages uniform USB: usb_wwan: use interface-data accessors USB: usb_wwan: clean up delayed-urb submission USB: usb_wwan: remove comment from close USB: usb_wwan: remove some superfluous comments USB: usb_wwan: remove bogus function prototype USB: usb_wwan: report failed submissions as errors USB: usb_wwan: do not resume I/O on closing ports USB: serial: fix potential runtime pm imbalance at device remove USB: serial: remove overly defensive port tests USB: kobil_sct: fix control requests without data stage USB: cdc-acm: fix write and suspend race USB: cdc-acm: fix write and resume race USB: cdc-acm: fix broken runtime suspend USB: cdc-acm: fix runtime PM for control messages USB: cdc-acm: fix shutdown and suspend race USB: cdc-acm: fix potential urb leak and PM imbalance in write USB: cdc-acm: fix open and suspend race USB: cdc-acm: fix failed open not being detected USB: cdc-acm: fix I/O after failed open USB: cdc-acm: fix runtime PM imbalance at shutdown USB: cdc-acm: simplify runtime PM locking USB: cdc-acm: remove redundant disconnected test from shutdown USB: cdc-acm: minimise no-suspend window during shutdown USB: cdc-acm: do not update PM busy on read errors USB: cdc-acm: remove redundant usb_mark_last_busy USB: cdc-acm: use tty-port dtr_rts USB: wusbcore: fix control-pipe directions xiao jin (2): USB: usb_wwan: fix urb leak in write error path USB: usb_wwan: fix race between write and resume drivers/usb/class/cdc-acm.c | 177 +- drivers/usb/class/cdc-acm.h | 2 +- drivers/usb/serial/bus.c| 14 ++- drivers/usb/serial/kobil_sct.c | 22 ++-- drivers/usb/serial/option.c | 13 ++- drivers/usb/serial/sierra.c | 198 -- drivers/usb/serial/usb-serial.c | 38 +++ drivers/usb/serial/usb-wwan.h | 6 +- drivers/usb/serial/usb_wwan.c | 234 drivers/usb/wusbcore/wa-rpipe.c | 4 +- 10 files changed, 390 insertions(+), 318 deletions(-) -- 1.8.5.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 05/63] USB: sierra: fix remote wakeup
Make sure that needs_remote_wake up is always set when there are open ports. Currently close() would unconditionally set needs_remote_wakeup to 0 even though there might still be open ports. This could lead to blocked input and possibly dropped data on devices that do not support remote wakeup (and which must therefore not be runtime suspended while open). Add an open_ports counter (protected by the susp_lock) and only clear needs_remote_wakeup when the last port is closed. Fixes: e6929a9020ac ("USB: support for autosuspend in sierra while online") Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 1a42649253a5..37480348e39b 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -58,6 +58,7 @@ struct sierra_intf_private { spinlock_t susp_lock; unsigned int suspended:1; int in_flight; + unsigned int open_ports; }; static int sierra_set_power_state(struct usb_device *udev, __u16 swiState) @@ -768,7 +769,6 @@ static void sierra_close(struct usb_serial_port *port) mutex_lock(&serial->disc_mutex); if (!serial->disconnected) { - serial->interface->needs_remote_wakeup = 0; /* odd error handling due to pm counters */ if (!usb_autopm_get_interface(serial->interface)) sierra_send_setup(port); @@ -779,6 +779,8 @@ static void sierra_close(struct usb_serial_port *port) mutex_unlock(&serial->disc_mutex); spin_lock_irq(&intfdata->susp_lock); portdata->opened = 0; + if (--intfdata->open_ports == 0) + serial->interface->needs_remote_wakeup = 0; spin_unlock_irq(&intfdata->susp_lock); for (;;) { @@ -834,9 +836,10 @@ static int sierra_open(struct tty_struct *tty, struct usb_serial_port *port) sierra_send_setup(port); - serial->interface->needs_remote_wakeup = 1; spin_lock_irq(&intfdata->susp_lock); portdata->opened = 1; + if (++intfdata->open_ports == 1) + serial->interface->needs_remote_wakeup = 1; spin_unlock_irq(&intfdata->susp_lock); usb_autopm_put_interface(serial->interface); -- 1.8.5.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 39/63] USB: usb_wwan: remove comment from close
Remove superfluous and cryptic comment from close. It should be obvious that we're balancing the autopm_put in open (and that operation already mentions the autopm_get done in the USB serial core). Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 45bc11b2d0ec..5042faa5d588 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -450,7 +450,6 @@ void usb_wwan_close(struct usb_serial_port *port) usb_kill_urb(portdata->out_urbs[i]); usb_kill_urb(port->interrupt_in_urb); - /* balancing - important as an error cannot be handled*/ usb_autopm_get_interface_no_resume(serial->interface); } EXPORT_SYMBOL(usb_wwan_close); -- 1.8.5.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 06/63] USB: sierra: fix characters being dropped at close
Fix characters potentially being dropped at close due to missing chars_in_buffer implementation. Note that currently the write urbs are not even killed at close (will be fixed separately), but this could still lead to dropped data since we have lowered DTR/RTS. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 37480348e39b..ed43b18ace78 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -675,6 +675,23 @@ static int sierra_write_room(struct tty_struct *tty) return 2048; } +static int sierra_chars_in_buffer(struct tty_struct *tty) +{ + struct usb_serial_port *port = tty->driver_data; + struct sierra_port_private *portdata = usb_get_serial_port_data(port); + unsigned long flags; + int chars; + + /* NOTE: This overcounts somewhat. */ + spin_lock_irqsave(&portdata->lock, flags); + chars = portdata->outstanding_urbs * MAX_TRANSFER; + spin_unlock_irqrestore(&portdata->lock, flags); + + dev_dbg(&port->dev, "%s - %d\n", __func__, chars); + + return chars; +} + static void sierra_stop_rx_urbs(struct usb_serial_port *port) { int i; @@ -1060,6 +1077,7 @@ static struct usb_serial_driver sierra_device = { .dtr_rts = sierra_dtr_rts, .write = sierra_write, .write_room= sierra_write_room, + .chars_in_buffer = sierra_chars_in_buffer, .set_termios = sierra_set_termios, .tiocmget = sierra_tiocmget, .tiocmset = sierra_tiocmset, -- 1.8.5.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 03/63] USB: sierra: fix urb and memory leak in resume error path
Neither the transfer buffer or the urb itself were released in the resume error path for delayed writes. Also on errors, the remainder of the queue was not even processed, which leads to further urb and buffer leaks. The same error path also failed to balance the outstanding-urb counter, something which results in degraded throughput or completely blocked writes. Fix this by releasing urb and buffer and balancing counters on errors, and by always processing the whole queue even when submission of one urb fails. Fixes: e6929a9020ac ("USB: support for autosuspend in sierra while online") Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 2c5c5a9330e2..dd9820d3fa03 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -1004,8 +1004,12 @@ static int sierra_resume(struct usb_serial *serial) if (err < 0) { intfdata->in_flight--; usb_unanchor_urb(urb); - usb_scuttle_anchored_urbs(&portdata->delayed); - break; + kfree(urb->transfer_buffer); + usb_free_urb(urb); + spin_lock(&portdata->lock); + portdata->outstanding_urbs--; + spin_unlock(&portdata->lock); + continue; } } -- 1.8.5.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 56/63] USB: cdc-acm: fix runtime PM imbalance at shutdown
Make sure only to decrement the PM counters if they were actually incremented. Note that the USB PM counter, but not necessarily the driver core PM counter, is reset when the interface is unbound. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index eddeba61a88c..6bbd203f1861 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -590,12 +590,13 @@ static void acm_port_shutdown(struct tty_port *port) struct urb *urb; struct acm_wb *wb; int i; + int pm_err; dev_dbg(&acm->control->dev, "%s\n", __func__); mutex_lock(&acm->mutex); if (!acm->disconnected) { - usb_autopm_get_interface(acm->control); + pm_err = usb_autopm_get_interface(acm->control); acm_set_control(acm, acm->ctrlout = 0); for (;;) { @@ -613,7 +614,8 @@ static void acm_port_shutdown(struct tty_port *port) for (i = 0; i < acm->rx_buflimit; i++) usb_kill_urb(acm->read_urbs[i]); acm->control->needs_remote_wakeup = 0; - usb_autopm_put_interface(acm->control); + if (!pm_err) + usb_autopm_put_interface(acm->control); } mutex_unlock(&acm->mutex); } -- 1.8.5.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 30/63] USB: usb_wwan: fix discarded writes on resume errors
There's no reason not to try sending off any further delayed write urbs, should one urb-submission fail. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 27 --- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index d91a9883e869..ab8c17536534 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -621,28 +621,33 @@ EXPORT_SYMBOL(usb_wwan_suspend); static int play_delayed(struct usb_serial_port *port) { + struct usb_serial *serial = port->serial; struct usb_wwan_intf_private *data; struct usb_wwan_port_private *portdata; struct urb *urb; - int err = 0; + int err_count = 0; + int err; portdata = usb_get_serial_port_data(port); data = port->serial->private; while ((urb = usb_get_from_anchor(&portdata->delayed))) { err = usb_submit_urb(urb, GFP_ATOMIC); - if (!err) { - data->in_flight++; - } else { - /* we have to throw away the rest */ - do { - unbusy_queued_urb(urb, portdata); - usb_autopm_put_interface_no_suspend(port->serial->interface); - } while ((urb = usb_get_from_anchor(&portdata->delayed))); - break; + if (err) { + dev_err(&port->dev, + "%s: submit write urb failed: %d\n", + __func__, err); + err_count++; + unbusy_queued_urb(urb, portdata); + usb_autopm_put_interface_async(serial->interface); + continue; } + data->in_flight++; } - return err; + if (err_count) + return -EIO; + + return 0; } int usb_wwan_resume(struct usb_serial *serial) -- 1.8.5.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 44/63] USB: serial: fix potential runtime pm imbalance at device remove
Only call usb_autopm_put_interface() if the corresponding usb_autopm_get_interface() was successful. This prevents a potential runtime PM counter imbalance should usb_autopm_get_interface() fail. Note that the USB PM usage counter is reset when the interface is unbound, but that the runtime PM counter may be left unbalanced. Also add comment on why we don't need to worry about racing resume/suspend on autopm_get failures. Fixes: d5fd650cfc7f ("usb: serial: prevent suspend/resume from racing against probe/remove") Signed-off-by: Johan Hovold --- drivers/usb/serial/bus.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c index 35a2373cde67..9374bd2aba20 100644 --- a/drivers/usb/serial/bus.c +++ b/drivers/usb/serial/bus.c @@ -97,13 +97,19 @@ static int usb_serial_device_remove(struct device *dev) struct usb_serial_port *port; int retval = 0; int minor; + int autopm_err; port = to_usb_serial_port(dev); if (!port) return -ENODEV; - /* make sure suspend/resume doesn't race against port_remove */ - usb_autopm_get_interface(port->serial->interface); + /* +* Make sure suspend/resume doesn't race against port_remove. +* +* Note that no further runtime PM callbacks will be made if +* autopm_get fails. +*/ + autopm_err = usb_autopm_get_interface(port->serial->interface); minor = port->minor; tty_unregister_device(usb_serial_tty_driver, minor); @@ -117,7 +123,9 @@ static int usb_serial_device_remove(struct device *dev) dev_info(dev, "%s converter now disconnected from ttyUSB%d\n", driver->description, minor); - usb_autopm_put_interface(port->serial->interface); + if (!autopm_err) + usb_autopm_put_interface(port->serial->interface); + return retval; } -- 1.8.5.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 40/63] USB: usb_wwan: remove some superfluous comments
Remove some more outdated or superfluous comments. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 5042faa5d588..c951f75891c9 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -178,7 +178,6 @@ int usb_wwan_ioctl(struct tty_struct *tty, } EXPORT_SYMBOL(usb_wwan_ioctl); -/* Write */ int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port, const unsigned char *buf, int count) { @@ -429,7 +428,6 @@ void usb_wwan_close(struct usb_serial_port *port) portdata = usb_get_serial_port_data(port); - /* Stop reading/writing urbs */ spin_lock_irq(&intfdata->susp_lock); portdata->opened = 0; if (--intfdata->open_ports == 0) @@ -454,7 +452,6 @@ void usb_wwan_close(struct usb_serial_port *port) } EXPORT_SYMBOL(usb_wwan_close); -/* Helper functions used by usb_wwan_setup_urbs */ static struct urb *usb_wwan_setup_urb(struct usb_serial_port *port, int endpoint, int dir, void *ctx, char *buf, int len, @@ -467,7 +464,6 @@ static struct urb *usb_wwan_setup_urb(struct usb_serial_port *port, if (!urb) return NULL; - /* Fill URB using supplied data. */ usb_fill_bulk_urb(urb, serial->dev, usb_sndbulkpipe(serial->dev, endpoint) | dir, buf, len, callback, ctx); @@ -567,7 +563,6 @@ static void stop_urbs(struct usb_serial *serial) struct usb_serial_port *port; struct usb_wwan_port_private *portdata; - /* Stop reading/writing urbs */ for (i = 0; i < serial->num_ports; ++i) { port = serial->port[i]; portdata = usb_get_serial_port_data(port); @@ -648,11 +643,9 @@ int usb_wwan_resume(struct usb_serial *serial) spin_lock_irq(&intfdata->susp_lock); for (i = 0; i < serial->num_ports; i++) { - /* walk all ports */ port = serial->port[i]; portdata = usb_get_serial_port_data(port); - /* skip closed ports */ if (!portdata || !portdata->opened) continue; -- 1.8.5.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 46/63] USB: kobil_sct: fix control requests without data stage
Fix incorrect pipe directions and remove bogus data buffer arguments from control requests without data stage. Signed-off-by: Johan Hovold --- drivers/usb/serial/kobil_sct.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c index fee242387f55..078f9ed419c8 100644 --- a/drivers/usb/serial/kobil_sct.c +++ b/drivers/usb/serial/kobil_sct.c @@ -215,13 +215,13 @@ static int kobil_open(struct tty_struct *tty, struct usb_serial_port *port) priv->device_type == KOBIL_ADAPTER_K_PRODUCT_ID) { /* Setting Baudrate, Parity and Stopbits */ result = usb_control_msg(port->serial->dev, - usb_rcvctrlpipe(port->serial->dev, 0), + usb_sndctrlpipe(port->serial->dev, 0), SUSBCRequest_SetBaudRateParityAndStopBits, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, SUSBCR_SBR_9600 | SUSBCR_SPASB_EvenParity | SUSBCR_SPASB_1StopBit, 0, - transfer_buffer, + NULL, 0, KOBIL_TIMEOUT ); @@ -229,12 +229,12 @@ static int kobil_open(struct tty_struct *tty, struct usb_serial_port *port) /* reset all queues */ result = usb_control_msg(port->serial->dev, - usb_rcvctrlpipe(port->serial->dev, 0), + usb_sndctrlpipe(port->serial->dev, 0), SUSBCRequest_Misc, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, SUSBCR_MSC_ResetAllQueues, 0, - transfer_buffer, + NULL, 0, KOBIL_TIMEOUT ); @@ -445,12 +445,12 @@ static int kobil_tiocmset(struct tty_struct *tty, else dev_dbg(dev, "%s - Clearing DTR\n", __func__); result = usb_control_msg(port->serial->dev, - usb_rcvctrlpipe(port->serial->dev, 0), + usb_sndctrlpipe(port->serial->dev, 0), SUSBCRequest_SetStatusLinesOrQueues, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, ((dtr != 0) ? SUSBCR_SSL_SETDTR : SUSBCR_SSL_CLRDTR), 0, - transfer_buffer, + NULL, 0, KOBIL_TIMEOUT); } else { @@ -459,12 +459,12 @@ static int kobil_tiocmset(struct tty_struct *tty, else dev_dbg(dev, "%s - Clearing RTS\n", __func__); result = usb_control_msg(port->serial->dev, - usb_rcvctrlpipe(port->serial->dev, 0), + usb_sndctrlpipe(port->serial->dev, 0), SUSBCRequest_SetStatusLinesOrQueues, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, ((rts != 0) ? SUSBCR_SSL_SETRTS : SUSBCR_SSL_CLRRTS), 0, - transfer_buffer, + NULL, 0, KOBIL_TIMEOUT); } @@ -514,7 +514,7 @@ static void kobil_set_termios(struct tty_struct *tty, tty_encode_baud_rate(tty, speed, speed); result = usb_control_msg(port->serial->dev, - usb_rcvctrlpipe(port->serial->dev, 0), + usb_sndctrlpipe(port->serial->dev, 0), SUSBCRequest_SetBaudRateParityAndStopBits, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, urb_val, @@ -546,12 +546,12 @@ static int kobil_ioctl(struct tty_struct *tty, return -ENOBUFS; result = usb_control_msg(port->serial->dev, - usb_rcvctrlpipe(port->serial->dev, 0), + usb_sndctrlpipe(port->serial->dev, 0), SUSBCRequest_Misc, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, SUSBCR_MSC_ResetAllQueues, 0, - NULL, /* transfer_buffer, */ + NULL, 0, KOBIL_TIMEOUT ); -- 1.8.5.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 23/63] USB: option: add missing usb_mark_last_busy
We should call usb_mark_last_busy in all input paths, including the interrupt completion handler. Signed-off-by: Johan Hovold --- drivers/usb/serial/option.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c index 2003a66c4807..df91ea9243df 100644 --- a/drivers/usb/serial/option.c +++ b/drivers/usb/serial/option.c @@ -1904,6 +1904,7 @@ static void option_instat_callback(struct urb *urb) /* Resubmit urb so we continue receiving IRQ data */ if (status != -ESHUTDOWN && status != -ENOENT) { + usb_mark_last_busy(port->serial->dev); err = usb_submit_urb(urb, GFP_ATOMIC); if (err) dev_dbg(dev, "%s: resubmit intr urb failed. (%d)\n", -- 1.8.5.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 27/63] USB: usb_wwan: fix urb leak at shutdown
The delayed-write queue was never emptied at shutdown (close), something which could lead to leaked urbs if the port is closed before being runtime resumed due to a write. When this happens the output buffer would not drain on close (closing_wait timeout), and after consecutive opens, writes could be corrupted with previously buffered data, transfered with reduced throughput or completely blocked. Note that unbusy_queued_urb() was simply moved out of CONFIG_PM. Fixes: 383cedc3bb43 ("USB: serial: full autosuspend support for the option driver") Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 34 ++ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 2b8f02696c00..2ab478b55759 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -414,12 +414,26 @@ int usb_wwan_open(struct tty_struct *tty, struct usb_serial_port *port) } EXPORT_SYMBOL(usb_wwan_open); +static void unbusy_queued_urb(struct urb *urb, + struct usb_wwan_port_private *portdata) +{ + int i; + + for (i = 0; i < N_OUT_URB; i++) { + if (urb == portdata->out_urbs[i]) { + clear_bit(i, &portdata->out_busy); + break; + } + } +} + void usb_wwan_close(struct usb_serial_port *port) { int i; struct usb_serial *serial = port->serial; struct usb_wwan_port_private *portdata; struct usb_wwan_intf_private *intfdata = port->serial->private; + struct urb *urb; portdata = usb_get_serial_port_data(port); @@ -428,6 +442,14 @@ void usb_wwan_close(struct usb_serial_port *port) portdata->opened = 0; spin_unlock_irq(&intfdata->susp_lock); + for (;;) { + urb = usb_get_from_anchor(&portdata->delayed); + if (!urb) + break; + unbusy_queued_urb(urb, portdata); + usb_autopm_put_interface_async(serial->interface); + } + for (i = 0; i < N_IN_URB; i++) usb_kill_urb(portdata->in_urbs[i]); for (i = 0; i < N_OUT_URB; i++) @@ -596,18 +618,6 @@ int usb_wwan_suspend(struct usb_serial *serial, pm_message_t message) } EXPORT_SYMBOL(usb_wwan_suspend); -static void unbusy_queued_urb(struct urb *urb, struct usb_wwan_port_private *portdata) -{ - int i; - - for (i = 0; i < N_OUT_URB; i++) { - if (urb == portdata->out_urbs[i]) { - clear_bit(i, &portdata->out_busy); - break; - } - } -} - static void play_delayed(struct usb_serial_port *port) { struct usb_wwan_intf_private *data; -- 1.8.5.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 62/63] USB: cdc-acm: use tty-port dtr_rts
Add dtr_rts tty-port operation which implements proper DTR/RTS handling (e.g. only lower DTR/RTS during shutdown if HUPCL is set). Note that modem-control locking still needs to be added throughout the driver. Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 8f654ce52570..e934e19f49f5 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -504,6 +504,25 @@ static int acm_tty_open(struct tty_struct *tty, struct file *filp) return tty_port_open(&acm->port, tty, filp); } +static void acm_port_dtr_rts(struct tty_port *port, int raise) +{ + struct acm *acm = container_of(port, struct acm, port); + int val; + int res; + + if (raise) + val = ACM_CTRL_DTR | ACM_CTRL_RTS; + else + val = 0; + + /* FIXME: add missing ctrlout locking throughout driver */ + acm->ctrlout = val; + + res = acm_set_control(acm, val); + if (res && (acm->ctrl_caps & USB_CDC_CAP_LINE)) + dev_err(&acm->control->dev, "failed to set dtr/rts\n"); +} + static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) { struct acm *acm = container_of(port, struct acm, port); @@ -535,11 +554,6 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) goto error_submit_urb; } - acm->ctrlout = ACM_CTRL_DTR | ACM_CTRL_RTS; - retval = acm_set_control(acm, acm->ctrlout); - if (retval < 0 && (acm->ctrl_caps & USB_CDC_CAP_LINE)) - goto error_set_control; - /* * Unthrottle device in case the TTY was closed while throttled. */ @@ -561,9 +575,6 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) error_submit_read_urbs: for (i = 0; i < acm->rx_buflimit; i++) usb_kill_urb(acm->read_urbs[i]); - acm->ctrlout = 0; - acm_set_control(acm, acm->ctrlout); -error_set_control: usb_kill_urb(acm->ctrlurb); error_submit_urb: usb_autopm_put_interface(acm->control); @@ -595,8 +606,6 @@ static void acm_port_shutdown(struct tty_port *port) dev_dbg(&acm->control->dev, "%s\n", __func__); - acm_set_control(acm, acm->ctrlout = 0); - /* * Need to grab write_lock to prevent race with resume, but no need to * hold it due to the tty-port initialised flag. @@ -992,6 +1001,7 @@ static void acm_tty_set_termios(struct tty_struct *tty, } static const struct tty_port_operations acm_port_ops = { + .dtr_rts = acm_port_dtr_rts, .shutdown = acm_port_shutdown, .activate = acm_port_activate, .destruct = acm_port_destruct, @@ -1429,8 +1439,6 @@ skip_countries: dev_info(&intf->dev, "ttyACM%d: USB ACM device\n", minor); - acm_set_control(acm, acm->ctrlout); - acm->line.dwDTERate = cpu_to_le32(9600); acm->line.bDataBits = 8; acm_set_line(acm, &acm->line); -- 1.8.5.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 48/63] USB: cdc-acm: fix write and resume race
Fix race between write() and resume() due to improper locking that could lead to writes being reordered. Resume must be done atomically and susp_count be protected by the write_lock in order to prevent racing with write(). This could otherwise lead to writes being reordered if write() grabs the write_lock after susp_count is decremented, but before the delayed urb is submitted. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Cc: # v2.6.27 Signed-off-by: Johan Hovold --- drivers/usb/class/cdc-acm.c | 23 +-- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 3bd4226c13dc..e72a657a6177 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1541,27 +1541,20 @@ static int acm_resume(struct usb_interface *intf) struct acm *acm = usb_get_intfdata(intf); struct acm_wb *wb; int rv = 0; - int cnt; spin_lock_irq(&acm->read_lock); - acm->susp_count -= 1; - cnt = acm->susp_count; - spin_unlock_irq(&acm->read_lock); + spin_lock(&acm->write_lock); - if (cnt) - return 0; + if (--acm->susp_count) + goto out; if (test_bit(ASYNCB_INITIALIZED, &acm->port.flags)) { - rv = usb_submit_urb(acm->ctrlurb, GFP_NOIO); + rv = usb_submit_urb(acm->ctrlurb, GFP_ATOMIC); - spin_lock_irq(&acm->write_lock); if (acm->delayed_wb) { wb = acm->delayed_wb; acm->delayed_wb = NULL; - spin_unlock_irq(&acm->write_lock); acm_start_wb(acm, wb); - } else { - spin_unlock_irq(&acm->write_lock); } /* @@ -1569,12 +1562,14 @@ static int acm_resume(struct usb_interface *intf) * do the write path at all cost */ if (rv < 0) - goto err_out; + goto out; - rv = acm_submit_read_urbs(acm, GFP_NOIO); + rv = acm_submit_read_urbs(acm, GFP_ATOMIC); } +out: + spin_unlock(&acm->write_lock); + spin_unlock_irq(&acm->read_lock); -err_out: return rv; } -- 1.8.5.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 25/63] USB: usb_wwan: fix race between write and resume
From: xiao jin We find a race between write and resume. usb_wwan_resume run play_delayed() and spin_unlock, but intfdata->suspended still is not set to zero. At this time usb_wwan_write is called and anchor the urb to delay list. Then resume keep running but the delayed urb have no chance to be commit until next resume. If the time of next resume is far away, tty will be blocked in tty_wait_until_sent during time. The race also can lead to writes being reordered. This patch put play_Delayed and intfdata->suspended together in the spinlock, it's to avoid the write race during resume. Fixes: 383cedc3bb43 ("USB: serial: full autosuspend support for the option driver") Signed-off-by: xiao jin Signed-off-by: Zhang, Qi1 Reviewed-by: David Cohen Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 47ad7550c5a6..112693a4100b 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -660,17 +660,15 @@ int usb_wwan_resume(struct usb_serial *serial) } } + spin_lock_irq(&intfdata->susp_lock); for (i = 0; i < serial->num_ports; i++) { /* walk all ports */ port = serial->port[i]; portdata = usb_get_serial_port_data(port); /* skip closed ports */ - spin_lock_irq(&intfdata->susp_lock); - if (!portdata || !portdata->opened) { - spin_unlock_irq(&intfdata->susp_lock); + if (!portdata || !portdata->opened) continue; - } for (j = 0; j < N_IN_URB; j++) { urb = portdata->in_urbs[j]; @@ -683,9 +681,7 @@ int usb_wwan_resume(struct usb_serial *serial) } } play_delayed(port); - spin_unlock_irq(&intfdata->susp_lock); } - spin_lock_irq(&intfdata->susp_lock); intfdata->suspended = 0; spin_unlock_irq(&intfdata->susp_lock); err_out: -- 1.8.5.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 16/63] USB: sierra: use interface-data accessors
Use usb_get_serial_data() rather than accessing the private pointer directly. Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 3a2f5b80d875..4b6d0ff07ddd 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -425,7 +425,7 @@ static void sierra_outdat_callback(struct urb *urb) struct sierra_intf_private *intfdata; int status = urb->status; - intfdata = port->serial->private; + intfdata = usb_get_serial_data(port->serial); /* free up the transfer buffer, as usb_free_urb() does not do this */ kfree(urb->transfer_buffer); @@ -462,7 +462,7 @@ static int sierra_write(struct tty_struct *tty, struct usb_serial_port *port, return 0; portdata = usb_get_serial_port_data(port); - intfdata = serial->private; + intfdata = usb_get_serial_data(serial); dev_dbg(&port->dev, "%s: write (%zd bytes)\n", __func__, writesize); spin_lock_irqsave(&portdata->lock, flags); @@ -764,7 +764,7 @@ static void sierra_close(struct usb_serial_port *port) int i; struct usb_serial *serial = port->serial; struct sierra_port_private *portdata; - struct sierra_intf_private *intfdata = port->serial->private; + struct sierra_intf_private *intfdata = usb_get_serial_data(serial); struct urb *urb; portdata = usb_get_serial_port_data(port); @@ -802,7 +802,7 @@ static int sierra_open(struct tty_struct *tty, struct usb_serial_port *port) { struct sierra_port_private *portdata; struct usb_serial *serial = port->serial; - struct sierra_intf_private *intfdata = serial->private; + struct sierra_intf_private *intfdata = usb_get_serial_data(serial); int i; int err; int endpoint; @@ -968,7 +968,7 @@ static int sierra_suspend(struct usb_serial *serial, pm_message_t message) int b; if (PMSG_IS_AUTO(message)) { - intfdata = serial->private; + intfdata = usb_get_serial_data(serial); spin_lock_irq(&intfdata->susp_lock); b = intfdata->in_flight; @@ -988,7 +988,7 @@ static int sierra_suspend(struct usb_serial *serial, pm_message_t message) static int sierra_resume(struct usb_serial *serial) { struct usb_serial_port *port; - struct sierra_intf_private *intfdata = serial->private; + struct sierra_intf_private *intfdata = usb_get_serial_data(serial); struct sierra_port_private *portdata; struct urb *urb; int ec = 0; -- 1.8.5.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 26/63] USB: usb_wwan: fix write and suspend race
Fix race between write() and suspend() which could lead to writes being dropped (or I/O while suspended) if the device is runtime suspended while a write request is being processed. Specifically, suspend() releases the susp_lock after determining the device is idle but before setting the suspended flag, thus leaving a window where a concurrent write() can submit an urb. Fixes: 383cedc3bb43 ("USB: serial: full autosuspend support for the option driver") Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 112693a4100b..2b8f02696c00 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -579,20 +579,17 @@ static void stop_read_write_urbs(struct usb_serial *serial) int usb_wwan_suspend(struct usb_serial *serial, pm_message_t message) { struct usb_wwan_intf_private *intfdata = serial->private; - int b; + spin_lock_irq(&intfdata->susp_lock); if (PMSG_IS_AUTO(message)) { - spin_lock_irq(&intfdata->susp_lock); - b = intfdata->in_flight; - spin_unlock_irq(&intfdata->susp_lock); - - if (b) + if (intfdata->in_flight) { + spin_unlock_irq(&intfdata->susp_lock); return -EBUSY; + } } - - spin_lock_irq(&intfdata->susp_lock); intfdata->suspended = 1; spin_unlock_irq(&intfdata->susp_lock); + stop_read_write_urbs(serial); return 0; -- 1.8.5.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 29/63] USB: usb_wwan: fix potential blocked I/O after resume
Keep trying to submit urbs rather than bail out on first read-urb submission error, which would also prevent I/O for any further ports from being resumed. Instead keep an error count, for all types of failed submissions, and let USB core know that something went wrong. Also make sure to always clear the suspended flag. Currently a failed read-urb submission would prevent cached writes as well as any subsequent writes from being submitted until next suspend-resume cycle, something which may not even necessarily happen. Note that USB core currently only logs an error if an interface resume failed. Fixes: 383cedc3bb43 ("USB: serial: full autosuspend support for the option driver") Cc: # v2.6.32 Signed-off-by: Johan Hovold --- drivers/usb/serial/usb_wwan.c | 25 + 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index c5b9deb2e04e..d91a9883e869 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -619,12 +619,12 @@ int usb_wwan_suspend(struct usb_serial *serial, pm_message_t message) } EXPORT_SYMBOL(usb_wwan_suspend); -static void play_delayed(struct usb_serial_port *port) +static int play_delayed(struct usb_serial_port *port) { struct usb_wwan_intf_private *data; struct usb_wwan_port_private *portdata; struct urb *urb; - int err; + int err = 0; portdata = usb_get_serial_port_data(port); data = port->serial->private; @@ -641,6 +641,8 @@ static void play_delayed(struct usb_serial_port *port) break; } } + + return err; } int usb_wwan_resume(struct usb_serial *serial) @@ -650,7 +652,8 @@ int usb_wwan_resume(struct usb_serial *serial) struct usb_wwan_intf_private *intfdata = serial->private; struct usb_wwan_port_private *portdata; struct urb *urb; - int err = 0; + int err; + int err_count = 0; spin_lock_irq(&intfdata->susp_lock); for (i = 0; i < serial->num_ports; i++) { @@ -669,25 +672,31 @@ int usb_wwan_resume(struct usb_serial *serial) dev_err(&port->dev, "%s: submit int urb failed: %d\n", __func__, err); + err_count++; } } + err = play_delayed(port); + if (err) + err_count++; + for (j = 0; j < N_IN_URB; j++) { urb = portdata->in_urbs[j]; err = usb_submit_urb(urb, GFP_ATOMIC); if (err < 0) { dev_err(&port->dev, "%s: Error %d for bulk URB %d\n", __func__, err, i); - spin_unlock_irq(&intfdata->susp_lock); - goto err_out; + err_count++; } } - play_delayed(port); } intfdata->suspended = 0; spin_unlock_irq(&intfdata->susp_lock); -err_out: - return err; + + if (err_count) + return -EIO; + + return 0; } EXPORT_SYMBOL(usb_wwan_resume); #endif -- 1.8.5.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 17/63] USB: sierra: clean up suspend
Clean up suspend() somewhat and make sure to always set the suspended flag (although it's only used for runtime PM) in order to match resume(). Signed-off-by: Johan Hovold --- drivers/usb/serial/sierra.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 4b6d0ff07ddd..1d42e8305b8e 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -964,22 +964,18 @@ static void stop_read_write_urbs(struct usb_serial *serial) static int sierra_suspend(struct usb_serial *serial, pm_message_t message) { - struct sierra_intf_private *intfdata; - int b; + struct sierra_intf_private *intfdata = usb_get_serial_data(serial); + spin_lock_irq(&intfdata->susp_lock); if (PMSG_IS_AUTO(message)) { - intfdata = usb_get_serial_data(serial); - spin_lock_irq(&intfdata->susp_lock); - b = intfdata->in_flight; - - if (b) { + if (intfdata->in_flight) { spin_unlock_irq(&intfdata->susp_lock); return -EBUSY; - } else { - intfdata->suspended = 1; - spin_unlock_irq(&intfdata->susp_lock); } } + intfdata->suspended = 1; + spin_unlock_irq(&intfdata->susp_lock); + stop_read_write_urbs(serial); return 0; -- 1.8.5.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