Re: [PATCH] usb: dwc2: gadget: Update for new usb_endpoint_maxp()
Hi, John Youn writes: >>> @@ -1812,17 +1812,17 @@ static u32 dwc2_hsotg_ep0_mps(unsigned int mps) >>> * @hsotg: The driver state. >>> * @ep: The index number of the endpoint >>> * @mps: The maximum packet size in bytes >>> + * @mc: The multicount value >>> * >>> * Configure the maximum packet size for the given endpoint, updating >>> * the hardware control registers to reflect this. >>> */ >>> static void dwc2_hsotg_set_ep_maxpacket(struct dwc2_hsotg *hsotg, >>> - unsigned int ep, unsigned int mps, unsigned int dir_in) >>> + unsigned int ep, unsigned int mps, >>> + unsigned int mc, unsigned int dir_in) >> >> this has an odd set of arguments. You pass the ep index, mps, direction >> and mult value, when you could just pass hsotg_ep and descriptor instead. > > Yes looks like we can do some simplification here. And you probably > don't need to pass a descriptor either since it must be set in the > usb_ep before enable. > > However this is also called in some contexts where a descriptor is not > available (initialization and ep0). So we have to think about this a > bit. > > I think dwc3 can make similar simplification on the > __dwc3_gadget_ep_enable(). __dwc3_gadget_ep_enable() takes the actual descriptors as arguments: static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, const struct usb_endpoint_descriptor *desc, const struct usb_ss_ep_comp_descriptor *comp_desc, bool modify, bool restore) I fail to see how much simpler we can make this. Perhaps we can turn bool and restore into a single argument if we use a bitfield instead of a bool. -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] usb: xhci: move slot_id from xhci_hcd to xhci_command structure
xhci->slot_id is used for providing a way to pass slot id from the
command completion handler to the funtion waiting for completion.
It's shared by enumerations of all USB devices connected to an
xhci host. Hence, it's a source for possible races. Since we've
introduced command structure and the command queue to xhci driver.
It's better to move slot_id from xhci_hcd structure to xhci_command
structure. Hence the race source is removed.
Signed-off-by: Lu Baolu
---
drivers/usb/host/xhci-ring.c | 8
drivers/usb/host/xhci.c | 2 +-
drivers/usb/host/xhci.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 797137e..ddfe2e2 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1096,12 +1096,12 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd
*xhci, int slot_id,
}
static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci, int slot_id,
- u32 cmd_comp_code)
+ struct xhci_command *command, u32 cmd_comp_code)
{
if (cmd_comp_code == COMP_SUCCESS)
- xhci->slot_id = slot_id;
+ command->slot_id = slot_id;
else
- xhci->slot_id = 0;
+ command->slot_id = 0;
}
static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id)
@@ -1371,7 +1371,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
cmd_type = TRB_FIELD_TO_TYPE(le32_to_cpu(cmd_trb->generic.field[3]));
switch (cmd_type) {
case TRB_ENABLE_SLOT:
- xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
+ xhci_handle_cmd_enable_slot(xhci, slot_id, cmd, cmd_comp_code);
break;
case TRB_DISABLE_SLOT:
xhci_handle_cmd_disable_slot(xhci, slot_id);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 42ea9ac..8ab5ed6 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3702,7 +3702,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device
*udev)
spin_unlock_irqrestore(&xhci->lock, flags);
wait_for_completion(command->completion);
- slot_id = xhci->slot_id;
+ slot_id = command->slot_id;
mutex_unlock(&xhci->mutex);
if (!slot_id || command->status != COMP_SUCCESS) {
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 60f254a..1468353 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -789,6 +789,7 @@ struct xhci_command {
/* Input context for changing device state */
struct xhci_container_ctx *in_ctx;
u32 status;
+ int slot_id;
/* If completion is null, no one is waiting on this command
* and the structure can be freed after the command completes.
*/
@@ -1583,7 +1584,6 @@ struct xhci_hcd {
/* slot enabling and address device helpers */
/* these are not thread safe so use mutex */
struct mutex mutex;
- int slot_id;
/* For USB 3.0 LPM enable/disable. */
struct xhci_command *lpm_command;
/* Internal mirror of the HW's dcbaa */
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Multiple chatty devices on Intel 5 Series/3400 USB2 EHCI controller act erratic
On Thu, 3 Nov 2016 sonofa...@openmailbox.org wrote: > Hello guys! Did you have any progress on this? In a word: No. > Do you still have the > affected machine and devices? I never had them in the first place. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
On Mon, Nov 7, 2016 at 9:29 AM, Peter Chen wrote:
> On Fri, Nov 04, 2016 at 01:51:34PM -0700, Stephen Boyd wrote:
>> Quoting Peter Chen (2016-10-24 18:16:32)
>> > On Mon, Oct 24, 2016 at 12:48:24PM -0700, Stephen Boyd wrote:
>> > > Quoting Chen-Yu Tsai (2016-10-24 05:19:05)
>> > > > Hi,
>> > > >
>> > > > On Tue, Oct 18, 2016 at 9:56 AM, Stephen Boyd
>> > > > wrote:
>> > > > > The ULPI bus can be built as a module, and it will soon be
>> > > > > calling these functions when it supports probing devices from DT.
>> > > > > Export them so they can be used by the ULPI module.
>> > > > >
>> > > > > Acked-by: Rob Herring
>> > > > > Cc:
>> > > > > Signed-off-by: Stephen Boyd
>> > > > > ---
>> > > > > drivers/of/device.c | 2 ++
>> > > > > 1 file changed, 2 insertions(+)
>> > > > >
>> > > > > diff --git a/drivers/of/device.c b/drivers/of/device.c
>> > > > > index 8a22a253a830..6719ab35b62e 100644
>> > > > > --- a/drivers/of/device.c
>> > > > > +++ b/drivers/of/device.c
>> > > > > @@ -225,6 +225,7 @@ ssize_t of_device_get_modalias(struct device
>> > > > > *dev, char *str, ssize_t len)
>> > > > >
>> > > > > return tsize;
>> > > > > }
>> > > > > +EXPORT_SYMBOL_GPL(of_device_get_modalias);
>> > > > >
>> > > > > int of_device_request_module(struct device *dev)
>> > > > > {
>> > > > > @@ -290,6 +291,7 @@ void of_device_uevent(struct device *dev, struct
>> > > > > kobj_uevent_env *env)
>> > > > > }
>> > > > > mutex_unlock(&of_mutex);
>> > > > > }
>> > > > > +EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
>> > > >
>> > > > This is trailing the wrong function.
>> > > >
>> > >
>> > > Good catch. Must have been some bad rebase.
>> > >
>> > > Peter, can you fix it while applying or should I resend this patch?
>> > >
>> >
>> > But, this is device tree patch. I can only get chipidea part and other
>> > USB patches if Greg agrees.
>> >
>>
>> Were you expecting Rob to take the drivers/of/* patches? Sorry I thought
>> Rob acked them so they could go through usb with the other changes.
>
> I am just worried about possible merge error when linus pulls both OF
> and USB tree. Greg, is it ok the OF patches through USB tree with OF
> maintainer's ack?
May I suggest putting the OF patches on an immutable branch so other
subsystems can pull them in without pulling in the USB patches? At
least I want to use them in the I2C subsystem, and in the sunxi-rsb
driver.
Regards
ChenYu
--
To unsubscribe from this list: send the line "unsubscribe 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 v5 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
On Fri, Nov 04, 2016 at 01:51:34PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-10-24 18:16:32)
> > On Mon, Oct 24, 2016 at 12:48:24PM -0700, Stephen Boyd wrote:
> > > Quoting Chen-Yu Tsai (2016-10-24 05:19:05)
> > > > Hi,
> > > >
> > > > On Tue, Oct 18, 2016 at 9:56 AM, Stephen Boyd
> > > > wrote:
> > > > > The ULPI bus can be built as a module, and it will soon be
> > > > > calling these functions when it supports probing devices from DT.
> > > > > Export them so they can be used by the ULPI module.
> > > > >
> > > > > Acked-by: Rob Herring
> > > > > Cc:
> > > > > Signed-off-by: Stephen Boyd
> > > > > ---
> > > > > drivers/of/device.c | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > > > > index 8a22a253a830..6719ab35b62e 100644
> > > > > --- a/drivers/of/device.c
> > > > > +++ b/drivers/of/device.c
> > > > > @@ -225,6 +225,7 @@ ssize_t of_device_get_modalias(struct device
> > > > > *dev, char *str, ssize_t len)
> > > > >
> > > > > return tsize;
> > > > > }
> > > > > +EXPORT_SYMBOL_GPL(of_device_get_modalias);
> > > > >
> > > > > int of_device_request_module(struct device *dev)
> > > > > {
> > > > > @@ -290,6 +291,7 @@ void of_device_uevent(struct device *dev, struct
> > > > > kobj_uevent_env *env)
> > > > > }
> > > > > mutex_unlock(&of_mutex);
> > > > > }
> > > > > +EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
> > > >
> > > > This is trailing the wrong function.
> > > >
> > >
> > > Good catch. Must have been some bad rebase.
> > >
> > > Peter, can you fix it while applying or should I resend this patch?
> > >
> >
> > But, this is device tree patch. I can only get chipidea part and other
> > USB patches if Greg agrees.
> >
>
> Were you expecting Rob to take the drivers/of/* patches? Sorry I thought
> Rob acked them so they could go through usb with the other changes.
I am just worried about possible merge error when linus pulls both OF
and USB tree. Greg, is it ok the OF patches through USB tree with OF
maintainer's ack?
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: usbip device reset
Sorry. I missed the mail archive URL. Merged patch is : http://www.mail-archive.com/linux-usb@vger.kernel.org/msg73658.html nobuo.iwata // > -Original Message- > From: fx IWATA NOBUO > Sent: Monday, November 07, 2016 10:02 AM > To: 'lars.taeu...@web.de'; linux-usb@vger.kernel.org > Subject: RE: usbip device reset > > Hello, > > I'm not clear but my patch is related to the server (stub) side situation. > http://www.mail-archive.com/linux-usb@vger.kernel.org/msg58989.html > > It was merged at 4.7-rc1. > > nobuo.iwata > // > > -Original Message- > > From: linux-usb-ow...@vger.kernel.org > > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Lars Tauber > > Sent: Monday, October 31, 2016 3:48 AM > > To: linux-usb@vger.kernel.org > > Subject: usbip device reset > > > > Hi there, > > > > I struggle with a problem of broken usbip connections. > > The situation is the following: > > > > usbip server: Openwrt (LEDE - kernel 4.4.12) device powered by a > > battery in the woods connected via wifi to a local network. > > usbip client: ubuntu 16.04 64bit with self compiled kernel 4.6.2. > > > > The usbip server serves a "C-Media Electronics Inc. USB PnP Sound Device". > > The usbip client uses this device via ALSA for a ffmpeg service. > > The ffmpeg service gets (re)started by systemd service unit. > > The ffmpeg writes one vorbis file each day and additionally serves the > > stream to ffserver. > > > > Every now and then (I couldn't find a reason yet) the usbip connection > > gets lost. > > Last time the connection was running for nearly 23 days. > > > > uptime usbip server: 105 days > > uptime usbip client: 30 days > > > > dmesg on client when the connection gets lost: > > > > [2051897.472372] usb usb5: Not yet implemented [2051897.473483] usb usb5: > > Not yet implemented [2051897.474407] usb usb5: Not yet implemented > > [2051897.475386] usb usb5: Not yet implemented [2059111.396128] vhci_hcd: > > connection reset by peer [2059111.396233] vhci_hcd: stop threads > > [2059111.396242] vhci_hcd: release socket [2059111.396249] vhci_hcd: > > disconnect device [2059111.396294] usb 5-1: USB disconnect, device > > number > > 2 [2059111.396638] vhci_hcd: dequeue a urb 8800b74e4500 > > [2059111.396644] vhci_hcd: device 8800d6e6a290 seems to be > > disconnected [2059111.396646] vhci_hcd: gives back urb > > 8800b74e4500 [2059111.396650] vhci_hcd: dequeue a urb > > 8800b74e4600 [2059111.396652] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396654] vhci_hcd: gives back urb > > 8800b74e4600 [2059111.396657] vhci_hcd: dequeue a urb > > 8800b74e4700 [2059111.396659] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396661] vhci_hcd: gives back urb > > 8800b74e4700 [2059111.396664] vhci_hcd: dequeue a urb > > 8800b74e4800 [2059111.39] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396668] vhci_hcd: gives back urb > > 8800b74e4800 [2059111.396671] vhci_hcd: dequeue a urb > > 8800b74e4900 [2059111.396673] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396675] vhci_hcd: gives back urb > > 8800b74e4900 [2059111.396678] vhci_hcd: dequeue a urb > > 8800b74e4c00 [2059111.396680] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396682] vhci_hcd: gives back urb > > 8800b74e4c00 [2059111.396685] vhci_hcd: dequeue a urb > > 8800b74e4d00 [2059111.396687] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396689] vhci_hcd: gives back urb > > 8800b74e4d00 [2059111.396692] vhci_hcd: dequeue a urb > > 8800b74e4e00 [2059111.396694] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396696] vhci_hcd: gives back urb > > 8800b74e4e00 [2059111.396699] vhci_hcd: dequeue a urb > > 8800b74e4f00 [2059111.396701] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396703] vhci_hcd: gives back urb > > 8800b74e4f00 [2059111.396706] vhci_hcd: dequeue a urb > > 8800b74e4a00 [2059111.396708] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396710] vhci_hcd: gives back urb > > 8800b74e4a00 [2059111.396713] vhci_hcd: dequeue a urb > > 8800b74e4b00 [2059111.396715] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396717] vhci_hcd: gives back urb > > 8800b74e4b00 [2059111.396720] vhci_hcd: dequeue a urb > > 880024029b00 [2059111.396722] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396724] vhci_hcd: gives back urb > > 880024029b00 [2059111.396871] vhci_hcd: dequeue a urb > > 88009e1a4a80 [2059111.396873] vhci_hcd: device 8800d6e6a290 > > seems to be disconnected [2059111.396876] vhci_hcd: gives back urb > > 88009e1a4a80 > > > > > > > > > > usbip server: > > > > [5242788.550105] wlan0: authenticate with 70:62:b8:2f:4f:9e > > [5242788.565693] wlan0: send auth to 70:62:b8:2f:4f:9e
RE: usbip device reset
Hello, I'm not clear but my patch is related to the server (stub) side situation. http://www.mail-archive.com/linux-usb@vger.kernel.org/msg58989.html It was merged at 4.7-rc1. nobuo.iwata // > -Original Message- > From: linux-usb-ow...@vger.kernel.org > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Lars Tauber > Sent: Monday, October 31, 2016 3:48 AM > To: linux-usb@vger.kernel.org > Subject: usbip device reset > > Hi there, > > I struggle with a problem of broken usbip connections. > The situation is the following: > > usbip server: Openwrt (LEDE - kernel 4.4.12) device powered by a battery > in the woods connected via wifi to a local network. > usbip client: ubuntu 16.04 64bit with self compiled kernel 4.6.2. > > The usbip server serves a "C-Media Electronics Inc. USB PnP Sound Device". > The usbip client uses this device via ALSA for a ffmpeg service. > The ffmpeg service gets (re)started by systemd service unit. > The ffmpeg writes one vorbis file each day and additionally serves the stream > to ffserver. > > Every now and then (I couldn't find a reason yet) the usbip connection gets > lost. > Last time the connection was running for nearly 23 days. > > uptime usbip server: 105 days > uptime usbip client: 30 days > > dmesg on client when the connection gets lost: > > [2051897.472372] usb usb5: Not yet implemented [2051897.473483] usb usb5: > Not yet implemented [2051897.474407] usb usb5: Not yet implemented > [2051897.475386] usb usb5: Not yet implemented [2059111.396128] vhci_hcd: > connection reset by peer [2059111.396233] vhci_hcd: stop threads > [2059111.396242] vhci_hcd: release socket [2059111.396249] vhci_hcd: > disconnect device [2059111.396294] usb 5-1: USB disconnect, device number > 2 [2059111.396638] vhci_hcd: dequeue a urb 8800b74e4500 > [2059111.396644] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396646] vhci_hcd: gives back urb 8800b74e4500 > [2059111.396650] vhci_hcd: dequeue a urb 8800b74e4600 > [2059111.396652] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396654] vhci_hcd: gives back urb 8800b74e4600 > [2059111.396657] vhci_hcd: dequeue a urb 8800b74e4700 > [2059111.396659] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396661] vhci_hcd: gives back urb 8800b74e4700 > [2059111.396664] vhci_hcd: dequeue a urb 8800b74e4800 > [2059111.39] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396668] vhci_hcd: gives back urb 8800b74e4800 > [2059111.396671] vhci_hcd: dequeue a urb 8800b74e4900 > [2059111.396673] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396675] vhci_hcd: gives back urb 8800b74e4900 > [2059111.396678] vhci_hcd: dequeue a urb 8800b74e4c00 > [2059111.396680] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396682] vhci_hcd: gives back urb 8800b74e4c00 > [2059111.396685] vhci_hcd: dequeue a urb 8800b74e4d00 > [2059111.396687] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396689] vhci_hcd: gives back urb 8800b74e4d00 > [2059111.396692] vhci_hcd: dequeue a urb 8800b74e4e00 > [2059111.396694] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396696] vhci_hcd: gives back urb 8800b74e4e00 > [2059111.396699] vhci_hcd: dequeue a urb 8800b74e4f00 > [2059111.396701] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396703] vhci_hcd: gives back urb 8800b74e4f00 > [2059111.396706] vhci_hcd: dequeue a urb 8800b74e4a00 > [2059111.396708] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396710] vhci_hcd: gives back urb 8800b74e4a00 > [2059111.396713] vhci_hcd: dequeue a urb 8800b74e4b00 > [2059111.396715] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396717] vhci_hcd: gives back urb 8800b74e4b00 > [2059111.396720] vhci_hcd: dequeue a urb 880024029b00 > [2059111.396722] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396724] vhci_hcd: gives back urb 880024029b00 > [2059111.396871] vhci_hcd: dequeue a urb 88009e1a4a80 > [2059111.396873] vhci_hcd: device 8800d6e6a290 seems to be > disconnected [2059111.396876] vhci_hcd: gives back urb 88009e1a4a80 > > > > > usbip server: > > [5242788.550105] wlan0: authenticate with 70:62:b8:2f:4f:9e > [5242788.565693] wlan0: send auth to 70:62:b8:2f:4f:9e (try 1/3) > [5242788.572561] wlan0: authenticated [5242788.577715] wlan0: associate > with 70:62:b8:2f:4f:9e (try 1/3) [5242788.586829] wlan0: RX AssocResp from > 70:62:b8:2f:4f:9e (capab=0x431 status=0 aid=1) [5242788.593625] wlan0: > associated [5382075.114791] usbip-host 1-1: unlink urb 81a37f00 > [5382075.118279] usbip-host 1-1: failed to unlink a urb 81a37f00, ret -36 > [5560060.290463] usbip-host 1-1: recv a header, 0 [5560060.296580] > usbip-host 1-1: stopped by a call to usb_kill_urb() because o
Re: [PATCH 4/4] cdc-acm: clear halt condition
On Sun, Nov 06, 2016 at 10:30:02PM +0100, Oliver Neukum wrote:
> Hi,
>
> almost. Two issues left with this section.
>
> 1. It makes no sense to check the results of usb_clear_halt()
> If it fails, we cannot do anything sensible. We have to restart
> IO and hope for the best. Not doing it that way risks introducing
> a regression.
>
> 2. usb_autopm_put_interface() must be after acm_submit_read_urbs()
> because the URBs are to be killed when the device is suspended.
>
> And on a related note:
>
> 3. The device may be reset externally before the work queue
> is executed. pre_reset() needs to clear the flag EVENT_RX_STALL.
Hi,
thanks for feedback. Here's an update:
-- >8 --
Subject: [PATCHv3 4/4] cdc-acm: clear halt condition
Signed-off-by: Ladislav Michl
---
drivers/usb/class/cdc-acm.c | 59 +++--
drivers/usb/class/cdc-acm.h | 3 +++
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b76c95c..711d680 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -424,15 +424,29 @@ static void acm_read_bulk_callback(struct urb *urb)
return;
}
- if (status) {
- set_bit(rb->index, &acm->read_urbs_free);
- if ((status != -ENOENT) || (urb->actual_length == 0))
- return;
+ switch (status) {
+ case 0:
+ usb_mark_last_busy(acm->dev);
+ acm_process_read_urb(acm, urb);
+ break;
+ case -EPIPE:
+ set_bit(EVENT_RX_STALL, &acm->flags);
+ schedule_work(&acm->work);
+ return;
+ case -ECONNRESET:
+ case -ENOENT:
+ case -ESHUTDOWN:
+ dev_dbg(&acm->data->dev,
+ "%s - urb shutting down with status: %d\n",
+ __func__, status);
+ return;
+ default:
+ dev_dbg(&acm->data->dev,
+ "%s - nonzero urb status received: %d\n",
+ __func__, status);
+ break;
}
- usb_mark_last_busy(acm->dev);
-
- acm_process_read_urb(acm, urb);
/*
* Unthrottle may run on another CPU which needs to see events
* in the same order. Submission has an implict barrier
@@ -468,14 +482,33 @@ static void acm_write_bulk(struct urb *urb)
spin_lock_irqsave(&acm->write_lock, flags);
acm_write_done(acm, wb);
spin_unlock_irqrestore(&acm->write_lock, flags);
+ set_bit(EVENT_TTY_WAKEUP, &acm->flags);
schedule_work(&acm->work);
}
static void acm_softint(struct work_struct *work)
{
+ int i;
struct acm *acm = container_of(work, struct acm, work);
- tty_port_tty_wakeup(&acm->port);
+ if (test_bit(EVENT_RX_STALL, &acm->flags)) {
+ if (!(usb_autopm_get_interface(acm->data))) {
+ for (i = 0; i < acm->rx_buflimit; i++) {
+ usb_kill_urb(acm->read_urbs[i]);
+ set_bit(i, &acm->read_urbs_free);
+ }
+ usb_clear_halt(acm->dev, acm->in);
+ acm_submit_read_urbs(acm, GFP_KERNEL);
+ usb_autopm_put_interface(acm->data);
+
+ }
+ clear_bit(EVENT_RX_STALL, &acm->flags);
+ }
+
+ if (test_bit(EVENT_TTY_WAKEUP, &acm->flags)) {
+ tty_port_tty_wakeup(&acm->port);
+ clear_bit(EVENT_TTY_WAKEUP, &acm->flags);
+ }
}
/*
@@ -1654,6 +1687,15 @@ static int acm_reset_resume(struct usb_interface *intf)
#endif /* CONFIG_PM */
+static int acm_pre_reset(struct usb_interface *intf)
+{
+ struct acm *acm = usb_get_intfdata(intf);
+
+ clear_bit(EVENT_RX_STALL, &acm->flags);
+
+ return 0;
+}
+
#define NOKIA_PCSUITE_ACM_INFO(x) \
USB_DEVICE_AND_INTERFACE_INFO(0x0421, x, \
USB_CLASS_COMM, USB_CDC_SUBCLASS_ACM, \
@@ -1895,6 +1937,7 @@ static struct usb_driver acm_driver = {
.resume = acm_resume,
.reset_resume = acm_reset_resume,
#endif
+ .pre_reset =acm_pre_reset,
.id_table = acm_ids,
#ifdef CONFIG_PM
.supports_autosuspend = 1,
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index 58ddd25..1db974d 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -103,6 +103,9 @@ struct acm {
spinlock_t write_lock;
struct mutex mutex;
bool disconnected;
+ unsigned long flags;
+# define EVENT_TTY_WAKEUP 0
+# define EVENT_RX_STALL 1
struct usb_cdc_line_coding line;/* bits, stop, parity */
struct work_struct work;/* work queue entry for
line discipline waking up */
unsigned int ctrlin;
Re: [PATCH 4/4] cdc-acm: clear halt condition
On Sun, 2016-11-06 at 10:07 +0100, Ladislav Michl wrote:
> + if (test_bit(EVENT_RX_STALL, &acm->flags)) {
> + status = usb_autopm_get_interface(acm->data);
> + if (!status) {
> + for (i = 0; i < acm->rx_buflimit; i++) {
> + usb_kill_urb(acm->read_urbs[i]);
> + set_bit(i, &acm->read_urbs_free);
> + }
> + status = usb_clear_halt(acm->dev, acm->in);
> + usb_autopm_put_interface(acm->data);
> + if (!status)
> + acm_submit_read_urbs(acm, GFP_KERNEL);
Hi,
almost. Two issues left with this section.
1. It makes no sense to check the results of usb_clear_halt()
If it fails, we cannot do anything sensible. We have to restart
IO and hope for the best. Not doing it that way risks introducing
a regression.
2. usb_autopm_put_interface() must be after acm_submit_read_urbs()
because the URBs are to be killed when the device is suspended.
And on a related note:
3. The device may be reset externally before the work queue
is executed. pre_reset() needs to clear the flag EVENT_RX_STALL.
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 4/4] cdc-acm: clear halt condition
On Sun, Nov 06, 2016 at 07:25:19AM +0100, Oliver Neukum wrote:
> On Sun, 2016-11-06 at 01:36 +0100, Ladislav Michl wrote:
> >
> > @@ -475,7 +490,30 @@ static void acm_softint(struct work_struct *work)
> > {
> > struct acm *acm = container_of(work, struct acm, work);
> >
> > - tty_port_tty_wakeup(&acm->port);
> > + dev_vdbg(&acm->data->dev, "scheduled work\n");
> > +
> > + if (test_bit(EVENT_RX_STALL, &acm->flags)) {
> > + int i, status;
> > +
> > + for (i = 0; i < acm->rx_buflimit; i++) {
> > + usb_kill_urb(acm->read_urbs[i]);
> > + set_bit(i, &acm->read_urbs_free);
> > + }
> > +
> > + status = usb_autopm_get_interface(acm->data);
>
> No. If you really resume the device here, you reanimate the URBs
> you just killed. The order must be inverted.
>
> > + if (!status) {
> > + status = usb_clear_halt(acm->dev, acm->in);
> > + usb_autopm_put_interface(acm->data);
> > + }
> > + if (!status)
> > + acm_submit_read_urbs(acm, GFP_KERNEL);
>
> No, you would kill the device. Either do it all conditionally
> or nothing.
I do not follow. Did you mean something like this?
-- >8 --
Subject: [PATCHv2 4/4] cdc-acm: clear halt condition
Signed-off-by: Ladislav Michl
---
drivers/usb/class/cdc-acm.c | 53 ++---
drivers/usb/class/cdc-acm.h | 3 +++
2 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b76c95c..c122fdd 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -424,15 +424,29 @@ static void acm_read_bulk_callback(struct urb *urb)
return;
}
- if (status) {
- set_bit(rb->index, &acm->read_urbs_free);
- if ((status != -ENOENT) || (urb->actual_length == 0))
- return;
+ switch (status) {
+ case 0:
+ usb_mark_last_busy(acm->dev);
+ acm_process_read_urb(acm, urb);
+ break;
+ case -EPIPE:
+ set_bit(EVENT_RX_STALL, &acm->flags);
+ schedule_work(&acm->work);
+ return;
+ case -ECONNRESET:
+ case -ENOENT:
+ case -ESHUTDOWN:
+ dev_dbg(&acm->data->dev,
+ "%s - urb shutting down with status: %d\n",
+ __func__, status);
+ return;
+ default:
+ dev_dbg(&acm->data->dev,
+ "%s - nonzero urb status received: %d\n",
+ __func__, status);
+ break;
}
- usb_mark_last_busy(acm->dev);
-
- acm_process_read_urb(acm, urb);
/*
* Unthrottle may run on another CPU which needs to see events
* in the same order. Submission has an implict barrier
@@ -468,14 +482,37 @@ static void acm_write_bulk(struct urb *urb)
spin_lock_irqsave(&acm->write_lock, flags);
acm_write_done(acm, wb);
spin_unlock_irqrestore(&acm->write_lock, flags);
+ set_bit(EVENT_TTY_WAKEUP, &acm->flags);
schedule_work(&acm->work);
}
static void acm_softint(struct work_struct *work)
{
+ int i, status;
struct acm *acm = container_of(work, struct acm, work);
- tty_port_tty_wakeup(&acm->port);
+ dev_vdbg(&acm->data->dev, "scheduled work\n");
+
+ if (test_bit(EVENT_RX_STALL, &acm->flags)) {
+ status = usb_autopm_get_interface(acm->data);
+ if (!status) {
+ for (i = 0; i < acm->rx_buflimit; i++) {
+ usb_kill_urb(acm->read_urbs[i]);
+ set_bit(i, &acm->read_urbs_free);
+ }
+ status = usb_clear_halt(acm->dev, acm->in);
+ usb_autopm_put_interface(acm->data);
+ if (!status)
+ acm_submit_read_urbs(acm, GFP_KERNEL);
+
+ }
+ clear_bit(EVENT_RX_STALL, &acm->flags);
+ }
+
+ if (test_bit(EVENT_TTY_WAKEUP, &acm->flags)) {
+ tty_port_tty_wakeup(&acm->port);
+ clear_bit(EVENT_TTY_WAKEUP, &acm->flags);
+ }
}
/*
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index 58ddd25..1db974d 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -103,6 +103,9 @@ struct acm {
spinlock_t write_lock;
struct mutex mutex;
bool disconnected;
+ unsigned long flags;
+# define EVENT_TTY_WAKEUP 0
+# define EVENT_RX_STALL 1
struct usb_cdc_line_coding line;/* bits, stop, parity */
struct work_struct work;/* work queue entry for
line discipline waking up */
unsigned int ctrlin;
