MUSB interrupt storm on device removal

MUSB interrupt storm on device removal

[Bin Liu]

Hi,

On Tue, Mar 05, 2019 at 11:30:28AM +0000, Måns Rullgård wrote:

[...]

> > Thanks for the info.
> > I will handle this case in musb driver.
> 
> What's happening to this?  There's no immediate urgency from my side,
> but I don't want it to get forgotten either.

no, it is not forgotten. it is on my todo. I have to spend some time to
think through should it simply return -ESHUTDOWN, or after a few
consecutive -EPROTO errors...

the former is sufficient for short-term workaround.

-Bin.
s Rullgård

MUSB interrupt storm on device removal

[Måns Rullgård]

Bin Liu <b-liu@ti.com> writes:

> On Wed, Jan 23, 2019 at 03:55:47PM +0100, Johan Hovold wrote:
>> On Wed, Jan 23, 2019 at 08:09:47AM -0600, Bin Liu wrote:
>> > On Wed, Jan 23, 2019 at 09:55:49AM +0100, Johan Hovold wrote:
>> > > On Wed, Jan 23, 2019 at 07:52:12AM +0100, Greg Kroah-Hartman wrote:
>> 
>> > > > That's not what any other host controller returns when a device is
>> > > > removed, so either you are going to have to fix all USB drives for this
>> > > > issue, or you need to fix the musb driver to not send this error for
>> > > > when a device is removed (hint, do the latter...)
>> > > 
>> > > Right, this needs to be handle at the HCD level.
>> > 
>> > Any reason usb_serial_generic_read_bulk_callback() doesn't handle
>> > -EPROTO in the same way as -EPIPE?
>> 
>> Since it is supposed to be intermittent unlike, for example, -ENOENT or
>> -EPIPE (the latter which the device driver can recover from if it cares
>> to implement clearing of halt).
>
> Okay, makes sense.
>
>> 
>> > > dwc2 fixed a similar lockup issue due to retried NAKed transaction by
>> > > not retrying immediately:
>> > > 
>> > > 	38d2b5fb75c1 ("usb: dwc2: host: Don't retry NAKed transactions right away")
>> > 
>> > Both cases are all about device removal, but this musb case is slightly
>> > different from this dwc2 case.
>> > 
>> > It is all about re-transmitting which causes interrupt storm, but in
>> > this dwc2 case, it is the dwc2 driver doing the re-transmitting, so it
>> > makes sense to delay it in the dwc2 driver as this referred patch does,
>> >
>> > but in this musb case, musb driver reports transaction error to the usb
>> > serial driver, the usb serial driver issues the re-transmitting not the
>> > musb driver, so I don't think the delay should be added in the musb
>> > driver.
>> 
>> I didn't say it was exactly the same.
>
> Yeah, I know. My point was the fix is in the place where re-transmitting
> happens, but
>
>> My point was that unless you fix this at the HCD level, you will need to
>> add complex recovery handling to every USB driver and completion handler
>> (~500 of those). But perhaps that is what it needed.
>
> okay, it probably make sense to handle the case in HCD because the
> number of HCD is much less.
>
>> I do see now that of all USB drivers we have two drivers that handles
>> -EPROTO by resubmitting after a delay, while a handful explicitly deals
>> with -EPROTO by simply stopping to resubmit (some probably bail out on
>> all errors, but the majority appear to resubmit on -EPROTO).
>
> Thanks for the info.
> I will handle this case in musb driver.

What's happening to this?  There's no immediate urgency from my side,
but I don't want it to get forgotten either.

MUSB interrupt storm on device removal

[Bin Liu]

On Thu, Jan 24, 2019 at 04:31:19PM +0000, Måns Rullgård wrote:
> Bin Liu <b-liu@ti.com> writes:
> 
> > On Thu, Jan 24, 2019 at 12:56:33PM +0000, Måns Rullgård wrote:
> >> Johan Hovold <johan@kernel.org> writes:
> >> 
> >> > On Wed, Jan 23, 2019 at 08:50:38PM +0000, Måns Rullgård wrote:
> >> >> Bin Liu <b-liu@ti.com> writes:
> >> >> 
> >> >> >> > > Why doesn't the same problem occur with other types of host controller?
> >> >> >> > 
> >> >> >> > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
> >> >> >> > giveback URBs with -EPROTO in such error case.
> >> >> >> 
> >> >> >> ehci-hcd also uses -EPROTO.
> >> >> >
> >> >> > Is it possible to test the use case on ehci?
> >> >> >
> >> >> > - connect a multi-ports usb serial device to a hub;
> >> >> > - open multiple ports with cat command;
> >> >> > - remove the usb serial device from the hub;
> >> >> > - console lockup happens?
> >> >> 
> >> >> It doesn't seem to happen using ehci or even musb on Allwinner A20.
> >> >> I have only seen the problem with musb on AM3358.
> >> >
> >> > The A20 being dual core may possible explain the difference.
> >> 
> >> Booting the A20 with nosmp it still works correctly.
> >
> > Can you please debug it to see how the hub disconnect event got a chance
> > to be processed?
> 
> Could you be a little more specific?  I'm happy to run some tests, but I
> need to know what information you're looking for.

Never mind.

We have already understood the root cause on Beaglbone - in device
disconnect musb generates interrupt storm which prevents the hub
disconnect event being processed.

The issue doesn't show up on A20, either because musb on A20 doesn't
generates the interrupts fast enough, or 'nosmp' parameter doesn't make
kernel running in non-SMP mode. In either way the issue on Beaglebone has
to be fixed.

error-code.txt says

-ESHUTDOWN              The device or host controller has been disabled due
                        to some problem that could not be worked around,
			such as a physical disconnect.

So instead -EPIPE as in the solution I proposed previously, I am going
to use -ESHUTDOWN when -EPROTO happens consequentially multiple times.

Regards,
-Bin.

MUSB interrupt storm on device removal

[Måns Rullgård]

Bin Liu <b-liu@ti.com> writes:

> On Thu, Jan 24, 2019 at 12:56:33PM +0000, Måns Rullgård wrote:
>> Johan Hovold <johan@kernel.org> writes:
>> 
>> > On Wed, Jan 23, 2019 at 08:50:38PM +0000, Måns Rullgård wrote:
>> >> Bin Liu <b-liu@ti.com> writes:
>> >> 
>> >> >> > > Why doesn't the same problem occur with other types of host controller?
>> >> >> > 
>> >> >> > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
>> >> >> > giveback URBs with -EPROTO in such error case.
>> >> >> 
>> >> >> ehci-hcd also uses -EPROTO.
>> >> >
>> >> > Is it possible to test the use case on ehci?
>> >> >
>> >> > - connect a multi-ports usb serial device to a hub;
>> >> > - open multiple ports with cat command;
>> >> > - remove the usb serial device from the hub;
>> >> > - console lockup happens?
>> >> 
>> >> It doesn't seem to happen using ehci or even musb on Allwinner A20.
>> >> I have only seen the problem with musb on AM3358.
>> >
>> > The A20 being dual core may possible explain the difference.
>> 
>> Booting the A20 with nosmp it still works correctly.
>
> Can you please debug it to see how the hub disconnect event got a chance
> to be processed?

Could you be a little more specific?  I'm happy to run some tests, but I
need to know what information you're looking for.

MUSB interrupt storm on device removal

[Bin Liu]

On Thu, Jan 24, 2019 at 10:49:30AM -0500, Alan Stern wrote:
> On Thu, 24 Jan 2019, Bin Liu wrote:
> 
> > > Perhaps it has something to do with the timing of the completion 
> > > interrupts.  I don't know anything about how musb works, though.  Some 
> > > low-level timing information would be good to see.
> > 
> > The musb controller driver itself doesn't have a isr BH, so when an ep
> > interrupt happened, the isr directly processes the urb and called
> > giveback. I tried to add HCD_BH to the musb hcd .flag, the issue still
> > happens.
> 
> I was thinking that maybe some controllers delay some types of
> completion interrupt until a frame boundary, while other controllers
> raise all completion interrupts immediately.

It is interesting that A20 which uses musb controller too doesn't show
the issue. We really have to compare the runtime on A20 and
BeagleboneBlack to understand the problem.

Regards,
-Bin.

MUSB interrupt storm on device removal

[Alan Stern]

On Thu, 24 Jan 2019, Bin Liu wrote:

> > Perhaps it has something to do with the timing of the completion 
> > interrupts.  I don't know anything about how musb works, though.  Some 
> > low-level timing information would be good to see.
> 
> The musb controller driver itself doesn't have a isr BH, so when an ep
> interrupt happened, the isr directly processes the urb and called
> giveback. I tried to add HCD_BH to the musb hcd .flag, the issue still
> happens.

I was thinking that maybe some controllers delay some types of
completion interrupt until a frame boundary, while other controllers
raise all completion interrupts immediately.

Alan Stern

MUSB interrupt storm on device removal

[Bin Liu]

On Thu, Jan 24, 2019 at 10:22:26AM -0500, Alan Stern wrote:
> On Thu, 24 Jan 2019, Johan Hovold wrote:
> 
> > > At least when -EPROTO errors are caused by device disconnect, we know 
> > > that they will eventually go away when the upstream hub reports the 
> > > port disconnect event.  But until then, an interrupt storm is certainly 
> > > possible.
> > 
> > Indeed, and this isn't the first time we've had this discussion either I
> > realised.
> > 
> > In fact, I've been hit by this myself on BBB and musb when disconnecting
> > a device connected through an external hub.
> > 
> > At the time, the immediate causes that were making the completion
> > handler take longer than usual (unaligned copy of a large buffer and a
> > printk respectively) could be fixed. The problem went away, but of
> > course not the underlying issue.
> > 
> > Note that the problem I was seeing also went away both when switching to
> > a different single-core SoC using ehci-omap, or when replacing the
> > external hub. IIRC it wasn't the hub workqueue that was starved as a I
> > first had thought, but the hub interrupt was never even received (or
> > processed at least).
> > 
> > Unfortunately, I never got around to investigating this further.
> 
> I guess now we have some motivation to look into this more closely.  :-)
> 
> > > > > My point was that unless you fix this at the HCD level, you will need to
> > > > > add complex recovery handling to every USB driver and completion handler
> > > > > (~500 of those). But perhaps that is what it needed.
> > > > 
> > > > okay, it probably make sense to handle the case in HCD because the
> > > > number of HCD is much less.
> > > 
> > > One possibility is to giveback URBs with certain errors (such as
> > > -EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
> > > like a very artificial solution, though.
> > > 
> > > > > I do see now that of all USB drivers we have two drivers that handles
> > > > > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > > > > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > > > > all errors, but the majority appear to resubmit on -EPROTO).
> > > 
> > > Any driver which immediately retries an URB after getting -EPROTO or
> > > -EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
> > > retries, is buggy.  As far as I can see, that's all there is to it.
> > 
> > I tend to agree with you on that, but adding complex back-off handling
> > of intermittent errors to every USB driver and completion handler will
> > be quite a bit of work. Unless the HCD drivers can assist, we'd at least
> > want have some part of the implementation provided by shared code.
> > 
> > Also note that simply starting to bail out on any error now would risk
> > introducing regressions for setups where intermittent errors do occur.
> 
> Agreed.
> 
> > > Why doesn't the same problem occur with other types of host controller?
> > 
> > I think we should get to the bottom of that. BBB is single core which
> > may be part of the reason why it's affected, but it's definitely related
> > to the controller as well.
> 
> Perhaps it has something to do with the timing of the completion 
> interrupts.  I don't know anything about how musb works, though.  Some 
> low-level timing information would be good to see.

The musb controller driver itself doesn't have a isr BH, so when an ep
interrupt happened, the isr directly processes the urb and called
giveback. I tried to add HCD_BH to the musb hcd .flag, the issue still
happens.

Regards,
-Bin.

MUSB interrupt storm on device removal

[Bin Liu]

On Thu, Jan 24, 2019 at 12:56:33PM +0000, Måns Rullgård wrote:
> Johan Hovold <johan@kernel.org> writes:
> 
> > On Wed, Jan 23, 2019 at 08:50:38PM +0000, Måns Rullgård wrote:
> >> Bin Liu <b-liu@ti.com> writes:
> >> 
> >> >> > > Why doesn't the same problem occur with other types of host controller?
> >> >> > 
> >> >> > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
> >> >> > giveback URBs with -EPROTO in such error case.
> >> >> 
> >> >> ehci-hcd also uses -EPROTO.
> >> >
> >> > Is it possible to test the use case on ehci?
> >> >
> >> > - connect a multi-ports usb serial device to a hub;
> >> > - open multiple ports with cat command;
> >> > - remove the usb serial device from the hub;
> >> > - console lockup happens?
> >> 
> >> It doesn't seem to happen using ehci or even musb on Allwinner A20.
> >> I have only seen the problem with musb on AM3358.
> >
> > The A20 being dual core may possible explain the difference.
> 
> Booting the A20 with nosmp it still works correctly.

Can you please debug it to see how the hub disconnect event got a chance
to be processed?

Regards,
-Bin.

MUSB interrupt storm on device removal

[Alan Stern]

On Thu, 24 Jan 2019, Johan Hovold wrote:

> > At least when -EPROTO errors are caused by device disconnect, we know 
> > that they will eventually go away when the upstream hub reports the 
> > port disconnect event.  But until then, an interrupt storm is certainly 
> > possible.
> 
> Indeed, and this isn't the first time we've had this discussion either I
> realised.
> 
> In fact, I've been hit by this myself on BBB and musb when disconnecting
> a device connected through an external hub.
> 
> At the time, the immediate causes that were making the completion
> handler take longer than usual (unaligned copy of a large buffer and a
> printk respectively) could be fixed. The problem went away, but of
> course not the underlying issue.
> 
> Note that the problem I was seeing also went away both when switching to
> a different single-core SoC using ehci-omap, or when replacing the
> external hub. IIRC it wasn't the hub workqueue that was starved as a I
> first had thought, but the hub interrupt was never even received (or
> processed at least).
> 
> Unfortunately, I never got around to investigating this further.

I guess now we have some motivation to look into this more closely.  :-)

> > > > My point was that unless you fix this at the HCD level, you will need to
> > > > add complex recovery handling to every USB driver and completion handler
> > > > (~500 of those). But perhaps that is what it needed.
> > > 
> > > okay, it probably make sense to handle the case in HCD because the
> > > number of HCD is much less.
> > 
> > One possibility is to giveback URBs with certain errors (such as
> > -EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
> > like a very artificial solution, though.
> > 
> > > > I do see now that of all USB drivers we have two drivers that handles
> > > > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > > > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > > > all errors, but the majority appear to resubmit on -EPROTO).
> > 
> > Any driver which immediately retries an URB after getting -EPROTO or
> > -EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
> > retries, is buggy.  As far as I can see, that's all there is to it.
> 
> I tend to agree with you on that, but adding complex back-off handling
> of intermittent errors to every USB driver and completion handler will
> be quite a bit of work. Unless the HCD drivers can assist, we'd at least
> want have some part of the implementation provided by shared code.
> 
> Also note that simply starting to bail out on any error now would risk
> introducing regressions for setups where intermittent errors do occur.

Agreed.

> > Why doesn't the same problem occur with other types of host controller?
> 
> I think we should get to the bottom of that. BBB is single core which
> may be part of the reason why it's affected, but it's definitely related
> to the controller as well.

Perhaps it has something to do with the timing of the completion 
interrupts.  I don't know anything about how musb works, though.  Some 
low-level timing information would be good to see.

Alan Stern

MUSB interrupt storm on device removal

[Måns Rullgård]

Johan Hovold <johan@kernel.org> writes:

> On Wed, Jan 23, 2019 at 08:50:38PM +0000, Måns Rullgård wrote:
>> Bin Liu <b-liu@ti.com> writes:
>> 
>> >> > > Why doesn't the same problem occur with other types of host controller?
>> >> > 
>> >> > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
>> >> > giveback URBs with -EPROTO in such error case.
>> >> 
>> >> ehci-hcd also uses -EPROTO.
>> >
>> > Is it possible to test the use case on ehci?
>> >
>> > - connect a multi-ports usb serial device to a hub;
>> > - open multiple ports with cat command;
>> > - remove the usb serial device from the hub;
>> > - console lockup happens?
>> 
>> It doesn't seem to happen using ehci or even musb on Allwinner A20.
>> I have only seen the problem with musb on AM3358.
>
> The A20 being dual core may possible explain the difference.

Booting the A20 with nosmp it still works correctly.

MUSB interrupt storm on device removal

[Johan Hovold]

On Wed, Jan 23, 2019 at 08:50:38PM +0000, Måns Rullgård wrote:
> Bin Liu <b-liu@ti.com> writes:
> 
> >> > > Why doesn't the same problem occur with other types of host controller?
> >> > 
> >> > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
> >> > giveback URBs with -EPROTO in such error case.
> >> 
> >> ehci-hcd also uses -EPROTO.
> >
> > Is it possible to test the use case on ehci?
> >
> > - connect a multi-ports usb serial device to a hub;
> > - open multiple ports with cat command;
> > - remove the usb serial device from the hub;
> > - console lockup happens?
> 
> It doesn't seem to happen using ehci or even musb on Allwinner A20.
> I have only seen the problem with musb on AM3358.

The A20 being dual core may possible explain the difference.

Johan

MUSB interrupt storm on device removal

[Johan Hovold]

On Wed, Jan 23, 2019 at 11:05:40AM -0500, Alan Stern wrote:
> On Wed, 23 Jan 2019, Bin Liu wrote:
> 
> > On Wed, Jan 23, 2019 at 03:55:47PM +0100, Johan Hovold wrote:
> > > On Wed, Jan 23, 2019 at 08:09:47AM -0600, Bin Liu wrote:
> > > > On Wed, Jan 23, 2019 at 09:55:49AM +0100, Johan Hovold wrote:
> > > > > On Wed, Jan 23, 2019 at 07:52:12AM +0100, Greg Kroah-Hartman wrote:
> > > 
> > > > > > That's not what any other host controller returns when a device is
> > > > > > removed, so either you are going to have to fix all USB drives for this
> > > > > > issue, or you need to fix the musb driver to not send this error for
> > > > > > when a device is removed (hint, do the latter...)
> > > > > 
> > > > > Right, this needs to be handle at the HCD level.
> > > > 
> > > > Any reason usb_serial_generic_read_bulk_callback() doesn't handle
> > > > -EPROTO in the same way as -EPIPE?
> > > 
> > > Since it is supposed to be intermittent unlike, for example, -ENOENT or
> > > -EPIPE (the latter which the device driver can recover from if it cares
> > > to implement clearing of halt).
> 
> Wait a minute.  Nothing says any of those errors is supposed to be 
> intermittent.  As long as an error has a systematic cause (as opposed 
> to random noise, for example), it will recur as often as the cause 
> does.

Right, I should have said drivers tend to treat it as always being
intermittent.

> At least when -EPROTO errors are caused by device disconnect, we know 
> that they will eventually go away when the upstream hub reports the 
> port disconnect event.  But until then, an interrupt storm is certainly 
> possible.

Indeed, and this isn't the first time we've had this discussion either I
realised.

In fact, I've been hit by this myself on BBB and musb when disconnecting
a device connected through an external hub.

At the time, the immediate causes that were making the completion
handler take longer than usual (unaligned copy of a large buffer and a
printk respectively) could be fixed. The problem went away, but of
course not the underlying issue.

Note that the problem I was seeing also went away both when switching to
a different single-core SoC using ehci-omap, or when replacing the
external hub. IIRC it wasn't the hub workqueue that was starved as a I
first had thought, but the hub interrupt was never even received (or
processed at least).

Unfortunately, I never got around to investigating this further.

> > > My point was that unless you fix this at the HCD level, you will need to
> > > add complex recovery handling to every USB driver and completion handler
> > > (~500 of those). But perhaps that is what it needed.
> > 
> > okay, it probably make sense to handle the case in HCD because the
> > number of HCD is much less.
> 
> One possibility is to giveback URBs with certain errors (such as
> -EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
> like a very artificial solution, though.
> 
> > > I do see now that of all USB drivers we have two drivers that handles
> > > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > > all errors, but the majority appear to resubmit on -EPROTO).
> 
> Any driver which immediately retries an URB after getting -EPROTO or
> -EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
> retries, is buggy.  As far as I can see, that's all there is to it.

I tend to agree with you on that, but adding complex back-off handling
of intermittent errors to every USB driver and completion handler will
be quite a bit of work. Unless the HCD drivers can assist, we'd at least
want have some part of the implementation provided by shared code.

Also note that simply starting to bail out on any error now would risk
introducing regressions for setups where intermittent errors do occur.

> > Thanks for the info.
> > I will handle this case in musb driver.
> 
> Why doesn't the same problem occur with other types of host controller?

I think we should get to the bottom of that. BBB is single core which
may be part of the reason why it's affected, but it's definitely related
to the controller as well.

Johan

MUSB interrupt storm on device removal

[Greg Kroah-Hartman]

On Wed, Jan 23, 2019 at 02:12:38PM -0600, Bin Liu wrote:
> > > > > Thanks for the info.
> > > > > I will handle this case in musb driver.
> > > > 
> > > > Why doesn't the same problem occur with other types of host controller?
> > > 
> > > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
> > > giveback URBs with -EPROTO in such error case.
> > 
> > ehci-hcd also uses -EPROTO.
> 
> Is it possible to test the use case on ehci?
> 
> - connect a multi-ports usb serial device to a hub;
> - open multiple ports with cat command;
> - remove the usb serial device from the hub;
> - console lockup happens?

No lockup happens at all for me :)

Looks like this is a musb-only issue :(

thanks,

greg k-h

MUSB interrupt storm on device removal

[Måns Rullgård]

Bin Liu <b-liu@ti.com> writes:

>> > > Why doesn't the same problem occur with other types of host controller?
>> > 
>> > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
>> > giveback URBs with -EPROTO in such error case.
>> 
>> ehci-hcd also uses -EPROTO.
>
> Is it possible to test the use case on ehci?
>
> - connect a multi-ports usb serial device to a hub;
> - open multiple ports with cat command;
> - remove the usb serial device from the hub;
> - console lockup happens?

It doesn't seem to happen using ehci or even musb on Allwinner A20.
I have only seen the problem with musb on AM3358.

MUSB interrupt storm on device removal

[Alan Stern]

On Wed, 23 Jan 2019, Bin Liu wrote:

> On Wed, Jan 23, 2019 at 12:42:36PM -0500, Alan Stern wrote:
> > On Wed, 23 Jan 2019, Bin Liu wrote:
> > 
> > > > One possibility is to giveback URBs with certain errors (such as
> > > > -EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
> > > > like a very artificial solution, though.
> > > 
> > > My plan is to add an error counter in musb driver endpoint struct, if
> > > -EPROTO has happened consequentially for a certain times, for example 3,
> > > giveback URBs with -EPIPE instead -EPROTO. This is the simplest solution
> > > I can think of, though I hate expending struct unnecessarily, this is
> > > one of the cases.
> > 
> > But -EPIPE is documented to mean that the device replied with a STALL.  
> > You would be violating the documentation.
> 
> (sigh...)

Yeah, I know.

If you really want to do this, you could update the documentation along 
with the driver.  I don't recommend it, but it's a possibility.

> > > > > > I do see now that of all USB drivers we have two drivers that handles
> > > > > > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > > > > > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > > > > > all errors, but the majority appear to resubmit on -EPROTO).
> > > > 
> > > > Any driver which immediately retries an URB after getting -EPROTO or
> > > > -EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
> > > > retries, is buggy.  As far as I can see, that's all there is to it.
> > > 
> > > Agreed, but given that majority appear to resubmit on -EPROTO as Johan
> > > said, I think better to handle it in HCD.
> > 
> > Do all those other drivers handle -EPIPE correctly?  A logical way to 
> > respond to -EPIPE is to issue a Clear-Halt request; what will happen 
> > when that also fails?
> > 
> > I think it makes more sense to continue using -EPROTO but slow down the 
> > exchange of packets so that there is no interrupt storm.
> 
> the musb driver doesn't use SOF interrupts, so SOF interrupt is
> disabled. Another way to add delay for -EPROTO would be moving such URBs
> into a new linked list and using a timer to reschedule the list. The
> musb driver already has enough mess, I really don't want to add this
> logic if there is other simpler option...

I agree.  However, it might make sense.

In theory you could enable SOF interrupts whenever one of these errors 
occurs, just temporarily.  But I suppose that's not much better than 
using a timer...

> > > > > Thanks for the info.
> > > > > I will handle this case in musb driver.
> > > > 
> > > > Why doesn't the same problem occur with other types of host controller?
> > > 
> > > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
> > > giveback URBs with -EPROTO in such error case.
> > 
> > ehci-hcd also uses -EPROTO.
> 
> Is it possible to test the use case on ehci?
> 
> - connect a multi-ports usb serial device to a hub;
> - open multiple ports with cat command;
> - remove the usb serial device from the hub;
> - console lockup happens?

I don't have a multi-port serial device to test with.  Can the test be 
carried out with a simple single-port device?

It shouldn't be too hard to find a PC with an EHCI controller.  xHCI 
might use -EPROTO too, I don't know.

Alan Stern

> > > musb controller has a register bit telling the controller has tried the
> > > transaction 3 times but didn't receive any response, then the musb
> > > driver just giveback this URB with -EPROTO.
> > 
> > Same with EHCI.
> 
> Regards,
> -Bin.

MUSB interrupt storm on device removal

[Bin Liu]

On Wed, Jan 23, 2019 at 12:42:36PM -0500, Alan Stern wrote:
> On Wed, 23 Jan 2019, Bin Liu wrote:
> 
> > > One possibility is to giveback URBs with certain errors (such as
> > > -EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
> > > like a very artificial solution, though.
> > 
> > My plan is to add an error counter in musb driver endpoint struct, if
> > -EPROTO has happened consequentially for a certain times, for example 3,
> > giveback URBs with -EPIPE instead -EPROTO. This is the simplest solution
> > I can think of, though I hate expending struct unnecessarily, this is
> > one of the cases.
> 
> But -EPIPE is documented to mean that the device replied with a STALL.  
> You would be violating the documentation.

(sigh...)

> > > > > I do see now that of all USB drivers we have two drivers that handles
> > > > > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > > > > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > > > > all errors, but the majority appear to resubmit on -EPROTO).
> > > 
> > > Any driver which immediately retries an URB after getting -EPROTO or
> > > -EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
> > > retries, is buggy.  As far as I can see, that's all there is to it.
> > 
> > Agreed, but given that majority appear to resubmit on -EPROTO as Johan
> > said, I think better to handle it in HCD.
> 
> Do all those other drivers handle -EPIPE correctly?  A logical way to 
> respond to -EPIPE is to issue a Clear-Halt request; what will happen 
> when that also fails?
> 
> I think it makes more sense to continue using -EPROTO but slow down the 
> exchange of packets so that there is no interrupt storm.

the musb driver doesn't use SOF interrupts, so SOF interrupt is
disabled. Another way to add delay for -EPROTO would be moving such URBs
into a new linked list and using a timer to reschedule the list. The
musb driver already has enough mess, I really don't want to add this
logic if there is other simpler option...

> 
> > > > Thanks for the info.
> > > > I will handle this case in musb driver.
> > > 
> > > Why doesn't the same problem occur with other types of host controller?
> > 
> > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
> > giveback URBs with -EPROTO in such error case.
> 
> ehci-hcd also uses -EPROTO.

Is it possible to test the use case on ehci?

- connect a multi-ports usb serial device to a hub;
- open multiple ports with cat command;
- remove the usb serial device from the hub;
- console lockup happens?

> > musb controller has a register bit telling the controller has tried the
> > transaction 3 times but didn't receive any response, then the musb
> > driver just giveback this URB with -EPROTO.
> 
> Same with EHCI.

Regards,
-Bin.

MUSB interrupt storm on device removal

[Alan Stern]

On Wed, 23 Jan 2019, Bin Liu wrote:

> > One possibility is to giveback URBs with certain errors (such as
> > -EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
> > like a very artificial solution, though.
> 
> My plan is to add an error counter in musb driver endpoint struct, if
> -EPROTO has happened consequentially for a certain times, for example 3,
> giveback URBs with -EPIPE instead -EPROTO. This is the simplest solution
> I can think of, though I hate expending struct unnecessarily, this is
> one of the cases.

But -EPIPE is documented to mean that the device replied with a STALL.  
You would be violating the documentation.

> > > > I do see now that of all USB drivers we have two drivers that handles
> > > > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > > > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > > > all errors, but the majority appear to resubmit on -EPROTO).
> > 
> > Any driver which immediately retries an URB after getting -EPROTO or
> > -EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
> > retries, is buggy.  As far as I can see, that's all there is to it.
> 
> Agreed, but given that majority appear to resubmit on -EPROTO as Johan
> said, I think better to handle it in HCD.

Do all those other drivers handle -EPIPE correctly?  A logical way to 
respond to -EPIPE is to issue a Clear-Halt request; what will happen 
when that also fails?

I think it makes more sense to continue using -EPROTO but slow down the 
exchange of packets so that there is no interrupt storm.

> > > Thanks for the info.
> > > I will handle this case in musb driver.
> > 
> > Why doesn't the same problem occur with other types of host controller?
> 
> Not sure, I am on musb for most of the times. Maybe other HCD doesn't
> giveback URBs with -EPROTO in such error case.

ehci-hcd also uses -EPROTO.

> musb controller has a register bit telling the controller has tried the
> transaction 3 times but didn't receive any response, then the musb
> driver just giveback this URB with -EPROTO.

Same with EHCI.

Alan Stern

MUSB interrupt storm on device removal

[Bin Liu]

On Wed, Jan 23, 2019 at 11:05:40AM -0500, Alan Stern wrote:
> On Wed, 23 Jan 2019, Bin Liu wrote:
> 
> > On Wed, Jan 23, 2019 at 03:55:47PM +0100, Johan Hovold wrote:
> > > On Wed, Jan 23, 2019 at 08:09:47AM -0600, Bin Liu wrote:
> > > > On Wed, Jan 23, 2019 at 09:55:49AM +0100, Johan Hovold wrote:
> > > > > On Wed, Jan 23, 2019 at 07:52:12AM +0100, Greg Kroah-Hartman wrote:
> > > 
> > > > > > That's not what any other host controller returns when a device is
> > > > > > removed, so either you are going to have to fix all USB drives for this
> > > > > > issue, or you need to fix the musb driver to not send this error for
> > > > > > when a device is removed (hint, do the latter...)
> > > > > 
> > > > > Right, this needs to be handle at the HCD level.
> > > > 
> > > > Any reason usb_serial_generic_read_bulk_callback() doesn't handle
> > > > -EPROTO in the same way as -EPIPE?
> > > 
> > > Since it is supposed to be intermittent unlike, for example, -ENOENT or
> > > -EPIPE (the latter which the device driver can recover from if it cares
> > > to implement clearing of halt).
> 
> Wait a minute.  Nothing says any of those errors is supposed to be 
> intermittent.  As long as an error has a systematic cause (as opposed 
> to random noise, for example), it will recur as often as the cause 
> does.
> 
> At least when -EPROTO errors are caused by device disconnect, we know 
> that they will eventually go away when the upstream hub reports the 
> port disconnect event.  But until then, an interrupt storm is certainly 
> possible.
> 
> > Okay, makes sense.
> > 
> > > 
> > > > > dwc2 fixed a similar lockup issue due to retried NAKed transaction by
> > > > > not retrying immediately:
> > > > > 
> > > > > 	38d2b5fb75c1 ("usb: dwc2: host: Don't retry NAKed transactions right away")
> > > > 
> > > > Both cases are all about device removal, but this musb case is slightly
> > > > different from this dwc2 case.
> > > > 
> > > > It is all about re-transmitting which causes interrupt storm, but in
> > > > this dwc2 case, it is the dwc2 driver doing the re-transmitting, so it
> > > > makes sense to delay it in the dwc2 driver as this referred patch does,
> > > >
> > > > but in this musb case, musb driver reports transaction error to the usb
> > > > serial driver, the usb serial driver issues the re-transmitting not the
> > > > musb driver, so I don't think the delay should be added in the musb
> > > > driver.
> > > 
> > > I didn't say it was exactly the same.
> > 
> > Yeah, I know. My point was the fix is in the place where re-transmitting
> > happens, but
> > 
> > > My point was that unless you fix this at the HCD level, you will need to
> > > add complex recovery handling to every USB driver and completion handler
> > > (~500 of those). But perhaps that is what it needed.
> > 
> > okay, it probably make sense to handle the case in HCD because the
> > number of HCD is much less.
> 
> One possibility is to giveback URBs with certain errors (such as
> -EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
> like a very artificial solution, though.

My plan is to add an error counter in musb driver endpoint struct, if
-EPROTO has happened consequentially for a certain times, for example 3,
giveback URBs with -EPIPE instead -EPROTO. This is the simplest solution
I can think of, though I hate expending struct unnecessarily, this is
one of the cases.

> 
> > > I do see now that of all USB drivers we have two drivers that handles
> > > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > > all errors, but the majority appear to resubmit on -EPROTO).
> 
> Any driver which immediately retries an URB after getting -EPROTO or
> -EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
> retries, is buggy.  As far as I can see, that's all there is to it.

Agreed, but given that majority appear to resubmit on -EPROTO as Johan
said, I think better to handle it in HCD.

> > Thanks for the info.
> > I will handle this case in musb driver.
> 
> Why doesn't the same problem occur with other types of host controller?

Not sure, I am on musb for most of the times. Maybe other HCD doesn't
giveback URBs with -EPROTO in such error case.

musb controller has a register bit telling the controller has tried the
transaction 3 times but didn't receive any response, then the musb
driver just giveback this URB with -EPROTO.

Regards,
-Bin.

MUSB interrupt storm on device removal

[Alan Stern]

On Wed, 23 Jan 2019, Bin Liu wrote:

> On Wed, Jan 23, 2019 at 03:55:47PM +0100, Johan Hovold wrote:
> > On Wed, Jan 23, 2019 at 08:09:47AM -0600, Bin Liu wrote:
> > > On Wed, Jan 23, 2019 at 09:55:49AM +0100, Johan Hovold wrote:
> > > > On Wed, Jan 23, 2019 at 07:52:12AM +0100, Greg Kroah-Hartman wrote:
> > 
> > > > > That's not what any other host controller returns when a device is
> > > > > removed, so either you are going to have to fix all USB drives for this
> > > > > issue, or you need to fix the musb driver to not send this error for
> > > > > when a device is removed (hint, do the latter...)
> > > > 
> > > > Right, this needs to be handle at the HCD level.
> > > 
> > > Any reason usb_serial_generic_read_bulk_callback() doesn't handle
> > > -EPROTO in the same way as -EPIPE?
> > 
> > Since it is supposed to be intermittent unlike, for example, -ENOENT or
> > -EPIPE (the latter which the device driver can recover from if it cares
> > to implement clearing of halt).

Wait a minute.  Nothing says any of those errors is supposed to be 
intermittent.  As long as an error has a systematic cause (as opposed 
to random noise, for example), it will recur as often as the cause 
does.

At least when -EPROTO errors are caused by device disconnect, we know 
that they will eventually go away when the upstream hub reports the 
port disconnect event.  But until then, an interrupt storm is certainly 
possible.

> Okay, makes sense.
> 
> > 
> > > > dwc2 fixed a similar lockup issue due to retried NAKed transaction by
> > > > not retrying immediately:
> > > > 
> > > > 	38d2b5fb75c1 ("usb: dwc2: host: Don't retry NAKed transactions right away")
> > > 
> > > Both cases are all about device removal, but this musb case is slightly
> > > different from this dwc2 case.
> > > 
> > > It is all about re-transmitting which causes interrupt storm, but in
> > > this dwc2 case, it is the dwc2 driver doing the re-transmitting, so it
> > > makes sense to delay it in the dwc2 driver as this referred patch does,
> > >
> > > but in this musb case, musb driver reports transaction error to the usb
> > > serial driver, the usb serial driver issues the re-transmitting not the
> > > musb driver, so I don't think the delay should be added in the musb
> > > driver.
> > 
> > I didn't say it was exactly the same.
> 
> Yeah, I know. My point was the fix is in the place where re-transmitting
> happens, but
> 
> > My point was that unless you fix this at the HCD level, you will need to
> > add complex recovery handling to every USB driver and completion handler
> > (~500 of those). But perhaps that is what it needed.
> 
> okay, it probably make sense to handle the case in HCD because the
> number of HCD is much less.

One possibility is to giveback URBs with certain errors (such as
-EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
like a very artificial solution, though.

> > I do see now that of all USB drivers we have two drivers that handles
> > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > all errors, but the majority appear to resubmit on -EPROTO).

Any driver which immediately retries an URB after getting -EPROTO or
-EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
retries, is buggy.  As far as I can see, that's all there is to it.

> Thanks for the info.
> I will handle this case in musb driver.

Why doesn't the same problem occur with other types of host controller?

Alan Stern

MUSB interrupt storm on device removal

[Bin Liu]

On Wed, Jan 23, 2019 at 03:55:47PM +0100, Johan Hovold wrote:
> On Wed, Jan 23, 2019 at 08:09:47AM -0600, Bin Liu wrote:
> > On Wed, Jan 23, 2019 at 09:55:49AM +0100, Johan Hovold wrote:
> > > On Wed, Jan 23, 2019 at 07:52:12AM +0100, Greg Kroah-Hartman wrote:
> 
> > > > That's not what any other host controller returns when a device is
> > > > removed, so either you are going to have to fix all USB drives for this
> > > > issue, or you need to fix the musb driver to not send this error for
> > > > when a device is removed (hint, do the latter...)
> > > 
> > > Right, this needs to be handle at the HCD level.
> > 
> > Any reason usb_serial_generic_read_bulk_callback() doesn't handle
> > -EPROTO in the same way as -EPIPE?
> 
> Since it is supposed to be intermittent unlike, for example, -ENOENT or
> -EPIPE (the latter which the device driver can recover from if it cares
> to implement clearing of halt).

Okay, makes sense.

> 
> > > dwc2 fixed a similar lockup issue due to retried NAKed transaction by
> > > not retrying immediately:
> > > 
> > > 	38d2b5fb75c1 ("usb: dwc2: host: Don't retry NAKed transactions right away")
> > 
> > Both cases are all about device removal, but this musb case is slightly
> > different from this dwc2 case.
> > 
> > It is all about re-transmitting which causes interrupt storm, but in
> > this dwc2 case, it is the dwc2 driver doing the re-transmitting, so it
> > makes sense to delay it in the dwc2 driver as this referred patch does,
> >
> > but in this musb case, musb driver reports transaction error to the usb
> > serial driver, the usb serial driver issues the re-transmitting not the
> > musb driver, so I don't think the delay should be added in the musb
> > driver.
> 
> I didn't say it was exactly the same.

Yeah, I know. My point was the fix is in the place where re-transmitting
happens, but

> My point was that unless you fix this at the HCD level, you will need to
> add complex recovery handling to every USB driver and completion handler
> (~500 of those). But perhaps that is what it needed.

okay, it probably make sense to handle the case in HCD because the
number of HCD is much less.

> I do see now that of all USB drivers we have two drivers that handles
> -EPROTO by resubmitting after a delay, while a handful explicitly deals
> with -EPROTO by simply stopping to resubmit (some probably bail out on
> all errors, but the majority appear to resubmit on -EPROTO).

Thanks for the info.
I will handle this case in musb driver.

Regards,
-Bin.

MUSB interrupt storm on device removal

[Johan Hovold]

On Wed, Jan 23, 2019 at 08:09:47AM -0600, Bin Liu wrote:
> On Wed, Jan 23, 2019 at 09:55:49AM +0100, Johan Hovold wrote:
> > On Wed, Jan 23, 2019 at 07:52:12AM +0100, Greg Kroah-Hartman wrote:

> > > That's not what any other host controller returns when a device is
> > > removed, so either you are going to have to fix all USB drives for this
> > > issue, or you need to fix the musb driver to not send this error for
> > > when a device is removed (hint, do the latter...)
> > 
> > Right, this needs to be handle at the HCD level.
> 
> Any reason usb_serial_generic_read_bulk_callback() doesn't handle
> -EPROTO in the same way as -EPIPE?

Since it is supposed to be intermittent unlike, for example, -ENOENT or
-EPIPE (the latter which the device driver can recover from if it cares
to implement clearing of halt).

> > dwc2 fixed a similar lockup issue due to retried NAKed transaction by
> > not retrying immediately:
> > 
> > 	38d2b5fb75c1 ("usb: dwc2: host: Don't retry NAKed transactions right away")
> 
> Both cases are all about device removal, but this musb case is slightly
> different from this dwc2 case.
> 
> It is all about re-transmitting which causes interrupt storm, but in
> this dwc2 case, it is the dwc2 driver doing the re-transmitting, so it
> makes sense to delay it in the dwc2 driver as this referred patch does,
>
> but in this musb case, musb driver reports transaction error to the usb
> serial driver, the usb serial driver issues the re-transmitting not the
> musb driver, so I don't think the delay should be added in the musb
> driver.

I didn't say it was exactly the same.

My point was that unless you fix this at the HCD level, you will need to
add complex recovery handling to every USB driver and completion handler
(~500 of those). But perhaps that is what it needed.

I do see now that of all USB drivers we have two drivers that handles
-EPROTO by resubmitting after a delay, while a handful explicitly deals
with -EPROTO by simply stopping to resubmit (some probably bail out on
all errors, but the majority appear to resubmit on -EPROTO).

Johan

MUSB interrupt storm on device removal

[Bin Liu]

On Wed, Jan 23, 2019 at 09:55:49AM +0100, Johan Hovold wrote:
> On Wed, Jan 23, 2019 at 07:52:12AM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Jan 22, 2019 at 02:52:44PM -0600, Bin Liu wrote:
> > > Hi Johan,
> > > 
> > > On Tue, Jan 22, 2019 at 02:16:30PM -0600, Bin Liu wrote:
> > > > On Tue, Jan 22, 2019 at 05:19:39PM +0000, Måns Rullgård wrote:
> > > > > Bin Liu <b-liu@ti.com> writes:
> > > > > 
> > > > > > On Mon, Jan 21, 2019 at 09:20:52PM +0000, Måns Rullgård wrote:
> > > > > >> Bin Liu <b-liu@ti.com> writes:
> > > > > >> 
> > > > > >> > On Fri, Jan 18, 2019 at 08:15:02PM +0000, Måns Rullgård wrote:
> > > > > >> >> Bin Liu <b-liu@ti.com> writes:
> > > > > >> >> 
> > > > > >> >> > On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
> > > > > >> >> >> Bin Liu <b-liu@ti.com> writes:
> > > > > >> >> >> 
> > > > > >> >> >> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
> > > > > >> >> >> >> Bin Liu <b-liu@ti.com> writes:
> > > > > >> >> >> >> 
> > > > > >> >> >> >> > Hi,
> > > > > >> >> >> >> >
> > > > > >> >> >> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> > > > > >> >> >> >> >> I have a strange problem with the musb driver in host mode on AM3358
> > > > > >> >> >> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
> > > > > >> >
> > > > > >> > Is this on beaglebone or beagleboneblack?
> > > > > >> 
> > > > > >> We've seen it with Beaglebone Enhanced [1] and BeagleCore [2] based
> > > > > >> devices.  Both are based on the AM3358 and mostly compatible with the
> > > > > >> Beaglebone Black.
> > > > > >> 
> > > > > >> [1] https://elinux.org/SanCloud_BeagleBoneEnhanced
> > > > > >> [2] http://beaglecore.com/products/bcm1str.html
> > > > > >
> > > > > > Okay, now I see you have a hub on the board. I just reproduced the
> > > > > > console lockup after added a hub in my setup. I will find some time to
> > > > > > debug this.
> > > > > >
> > > > > > FYI, there was a similar issue in a few years ago, which was that the
> > > > > > hub routine never got called during the storm then the device disconnect
> > > > > > event never got populated from the hub driver. I am wondering if this is
> > > > > > the same issue pops up again...
> > > > > 
> > > > > Now that you mentioned the hub, I tried connecting the serial adapter to
> > > > > the OTG port (in host mode) on the Beaglebone Enhanced (it's not
> > > > > normally used in this product).  When connected directly, the disconnect
> > > > > indeed works properly.  With a hub in between, it breaks.
> > > > 
> > > > Please use the following hack as a workaround for now. I will think more
> > > > about it.
> > > > 
> > > > Regards,
> > > > -Bin.
> > > > 
> > > > --------8<----------
> > > > diff git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> > > > index b59ce9ad14ce..3719f6d9b26c 100644
> > > > --- a/drivers/usb/musb/musb_host.c
> > > > +++ b/drivers/usb/musb/musb_host.c
> > > > @@ -1811,7 +1811,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
> > > >         } else if (rx_csr & MUSB_RXCSR_H_ERROR) {
> > > >                 musb_dbg(musb, "end %d RX proto error", epnum);
> > > >  
> > > > -               status = -EPROTO;
> > > > +               status = -EPIPE;
> > > >                 musb_writeb(epio, MUSB_RXINTERVAL, 0);
> > > >  
> > > >                 rx_csr &= ~MUSB_RXCSR_H_ERROR;
> > > 
> > > When a usb serial device is detached, the musb controller driver returns
> > > -EPROTO in urb giveback, then usb_serial_generic_read_bulk_callback()
> > > re-submit the urb, this circle causes interrupt storm, and the hub
> > > driver workqueue never got a chance to run to populate the device
> > > disconnect event, the whole console locks up. Do you think
> > > usb_serial_generic_read_bulk_callback() should handle -EPROTO as well?
> > 
> > Why is musb returning that error?  According to the documentation we
> > have about this type of thing
> > (Documentation/driver-api/usb/error-codes.rst), that should only happen
> > if one of the following just occured:
> > 	- bitstuff error
> > 	- no response packet received within the prescribed bus
> > 	  turn-around time
> > 	- unknown USB error
> 
> But we also have in that file:
> 
> 	This is also one of several codes that different kinds of host
> 	controller use to indicate a transfer has failed because of
> 	device disconnect.  In the interval before the hub driver starts
> 	disconnect processing, devices may receive such fault reports
> 	for every request.

Yes, it is such case - when a hub is in the topology the musb controller
itself doesn't report the device disconnect, it is the hub to do so.
From the musb controller's perspective the usb transaction runs into the
PROTO error: the controller sent IN token 3 times but all didn't receive
the response packet (because the device is already removed), so the
controller generates error interrupt, then musb driver reports the PROTO
error to the usb core and class drivers.

> 
> > That's not what any other host controller returns when a device is
> > removed, so either you are going to have to fix all USB drives for this
> > issue, or you need to fix the musb driver to not send this error for
> > when a device is removed (hint, do the latter...)
> 
> Right, this needs to be handle at the HCD level.

Any reason usb_serial_generic_read_bulk_callback() doesn't handle
-EPROTO in the same way as -EPIPE?
 
> dwc2 fixed a similar lockup issue due to retried NAKed transaction by
> not retrying immediately:
> 
> 	38d2b5fb75c1 ("usb: dwc2: host: Don't retry NAKed transactions right away")

Both cases are all about device removal, but this musb case is slightly
different from this dwc2 case.

It is all about re-transmitting which causes interrupt storm, but in
this dwc2 case, it is the dwc2 driver doing the re-transmitting, so it
makes sense to delay it in the dwc2 driver as this referred patch does,

but in this musb case, musb driver reports transaction error to the usb
serial driver, the usb serial driver issues the re-transmitting not the
musb driver, so I don't think the delay should be added in the musb
driver.

Regards,
-Bin.

MUSB interrupt storm on device removal

[Johan Hovold]

On Wed, Jan 23, 2019 at 07:52:12AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 22, 2019 at 02:52:44PM -0600, Bin Liu wrote:
> > Hi Johan,
> > 
> > On Tue, Jan 22, 2019 at 02:16:30PM -0600, Bin Liu wrote:
> > > On Tue, Jan 22, 2019 at 05:19:39PM +0000, Måns Rullgård wrote:
> > > > Bin Liu <b-liu@ti.com> writes:
> > > > 
> > > > > On Mon, Jan 21, 2019 at 09:20:52PM +0000, Måns Rullgård wrote:
> > > > >> Bin Liu <b-liu@ti.com> writes:
> > > > >> 
> > > > >> > On Fri, Jan 18, 2019 at 08:15:02PM +0000, Måns Rullgård wrote:
> > > > >> >> Bin Liu <b-liu@ti.com> writes:
> > > > >> >> 
> > > > >> >> > On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
> > > > >> >> >> Bin Liu <b-liu@ti.com> writes:
> > > > >> >> >> 
> > > > >> >> >> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
> > > > >> >> >> >> Bin Liu <b-liu@ti.com> writes:
> > > > >> >> >> >> 
> > > > >> >> >> >> > Hi,
> > > > >> >> >> >> >
> > > > >> >> >> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> > > > >> >> >> >> >> I have a strange problem with the musb driver in host mode on AM3358
> > > > >> >> >> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
> > > > >> >
> > > > >> > Is this on beaglebone or beagleboneblack?
> > > > >> 
> > > > >> We've seen it with Beaglebone Enhanced [1] and BeagleCore [2] based
> > > > >> devices.  Both are based on the AM3358 and mostly compatible with the
> > > > >> Beaglebone Black.
> > > > >> 
> > > > >> [1] https://elinux.org/SanCloud_BeagleBoneEnhanced
> > > > >> [2] http://beaglecore.com/products/bcm1str.html
> > > > >
> > > > > Okay, now I see you have a hub on the board. I just reproduced the
> > > > > console lockup after added a hub in my setup. I will find some time to
> > > > > debug this.
> > > > >
> > > > > FYI, there was a similar issue in a few years ago, which was that the
> > > > > hub routine never got called during the storm then the device disconnect
> > > > > event never got populated from the hub driver. I am wondering if this is
> > > > > the same issue pops up again...
> > > > 
> > > > Now that you mentioned the hub, I tried connecting the serial adapter to
> > > > the OTG port (in host mode) on the Beaglebone Enhanced (it's not
> > > > normally used in this product).  When connected directly, the disconnect
> > > > indeed works properly.  With a hub in between, it breaks.
> > > 
> > > Please use the following hack as a workaround for now. I will think more
> > > about it.
> > > 
> > > Regards,
> > > -Bin.
> > > 
> > > --------8<----------
> > > diff git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> > > index b59ce9ad14ce..3719f6d9b26c 100644
> > > --- a/drivers/usb/musb/musb_host.c
> > > +++ b/drivers/usb/musb/musb_host.c
> > > @@ -1811,7 +1811,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
> > >         } else if (rx_csr & MUSB_RXCSR_H_ERROR) {
> > >                 musb_dbg(musb, "end %d RX proto error", epnum);
> > >  
> > > -               status = -EPROTO;
> > > +               status = -EPIPE;
> > >                 musb_writeb(epio, MUSB_RXINTERVAL, 0);
> > >  
> > >                 rx_csr &= ~MUSB_RXCSR_H_ERROR;
> > 
> > When a usb serial device is detached, the musb controller driver returns
> > -EPROTO in urb giveback, then usb_serial_generic_read_bulk_callback()
> > re-submit the urb, this circle causes interrupt storm, and the hub
> > driver workqueue never got a chance to run to populate the device
> > disconnect event, the whole console locks up. Do you think
> > usb_serial_generic_read_bulk_callback() should handle -EPROTO as well?
> 
> Why is musb returning that error?  According to the documentation we
> have about this type of thing
> (Documentation/driver-api/usb/error-codes.rst), that should only happen
> if one of the following just occured:
> 	- bitstuff error
> 	- no response packet received within the prescribed bus
> 	  turn-around time
> 	- unknown USB error

But we also have in that file:

	This is also one of several codes that different kinds of host
	controller use to indicate a transfer has failed because of
	device disconnect.  In the interval before the hub driver starts
	disconnect processing, devices may receive such fault reports
	for every request.

> That's not what any other host controller returns when a device is
> removed, so either you are going to have to fix all USB drives for this
> issue, or you need to fix the musb driver to not send this error for
> when a device is removed (hint, do the latter...)

Right, this needs to be handle at the HCD level.

dwc2 fixed a similar lockup issue due to retried NAKed transaction by
not retrying immediately:

	38d2b5fb75c1 ("usb: dwc2: host: Don't retry NAKed transactions right away")

Note that there are some later follow-up patches to that one. Might give
some inspiration though.

Johan

MUSB interrupt storm on device removal

[Greg KH]

On Tue, Jan 22, 2019 at 02:52:44PM -0600, Bin Liu wrote:
> Hi Johan,
> 
> On Tue, Jan 22, 2019 at 02:16:30PM -0600, Bin Liu wrote:
> > On Tue, Jan 22, 2019 at 05:19:39PM +0000, Måns Rullgård wrote:
> > > Bin Liu <b-liu@ti.com> writes:
> > > 
> > > > On Mon, Jan 21, 2019 at 09:20:52PM +0000, Måns Rullgård wrote:
> > > >> Bin Liu <b-liu@ti.com> writes:
> > > >> 
> > > >> > On Fri, Jan 18, 2019 at 08:15:02PM +0000, Måns Rullgård wrote:
> > > >> >> Bin Liu <b-liu@ti.com> writes:
> > > >> >> 
> > > >> >> > On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
> > > >> >> >> Bin Liu <b-liu@ti.com> writes:
> > > >> >> >> 
> > > >> >> >> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
> > > >> >> >> >> Bin Liu <b-liu@ti.com> writes:
> > > >> >> >> >> 
> > > >> >> >> >> > Hi,
> > > >> >> >> >> >
> > > >> >> >> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> > > >> >> >> >> >> I have a strange problem with the musb driver in host mode on AM3358
> > > >> >> >> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
> > > >> >
> > > >> > Is this on beaglebone or beagleboneblack?
> > > >> 
> > > >> We've seen it with Beaglebone Enhanced [1] and BeagleCore [2] based
> > > >> devices.  Both are based on the AM3358 and mostly compatible with the
> > > >> Beaglebone Black.
> > > >> 
> > > >> [1] https://elinux.org/SanCloud_BeagleBoneEnhanced
> > > >> [2] http://beaglecore.com/products/bcm1str.html
> > > >
> > > > Okay, now I see you have a hub on the board. I just reproduced the
> > > > console lockup after added a hub in my setup. I will find some time to
> > > > debug this.
> > > >
> > > > FYI, there was a similar issue in a few years ago, which was that the
> > > > hub routine never got called during the storm then the device disconnect
> > > > event never got populated from the hub driver. I am wondering if this is
> > > > the same issue pops up again...
> > > 
> > > Now that you mentioned the hub, I tried connecting the serial adapter to
> > > the OTG port (in host mode) on the Beaglebone Enhanced (it's not
> > > normally used in this product).  When connected directly, the disconnect
> > > indeed works properly.  With a hub in between, it breaks.
> > 
> > Please use the following hack as a workaround for now. I will think more
> > about it.
> > 
> > Regards,
> > -Bin.
> > 
> > --------8<----------
> > diff git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> > index b59ce9ad14ce..3719f6d9b26c 100644
> > --- a/drivers/usb/musb/musb_host.c
> > +++ b/drivers/usb/musb/musb_host.c
> > @@ -1811,7 +1811,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
> >         } else if (rx_csr & MUSB_RXCSR_H_ERROR) {
> >                 musb_dbg(musb, "end %d RX proto error", epnum);
> >  
> > -               status = -EPROTO;
> > +               status = -EPIPE;
> >                 musb_writeb(epio, MUSB_RXINTERVAL, 0);
> >  
> >                 rx_csr &= ~MUSB_RXCSR_H_ERROR;
> 
> When a usb serial device is detached, the musb controller driver returns
> -EPROTO in urb giveback, then usb_serial_generic_read_bulk_callback()
> re-submit the urb, this circle causes interrupt storm, and the hub
> driver workqueue never got a chance to run to populate the device
> disconnect event, the whole console locks up. Do you think
> usb_serial_generic_read_bulk_callback() should handle -EPROTO as well?

Why is musb returning that error?  According to the documentation we
have about this type of thing
(Documentation/driver-api/usb/error-codes.rst), that should only happen
if one of the following just occured:
	- bitstuff error
	- no response packet received within the prescribed bus
	  turn-around time
	- unknown USB error

That's not what any other host controller returns when a device is
removed, so either you are going to have to fix all USB drives for this
issue, or you need to fix the musb driver to not send this error for
when a device is removed (hint, do the latter...)

-EPROTO is supposed to be sent when there is a real hardware error in
the system, not when a normal operation happens.

thanks,

greg k-h

MUSB interrupt storm on device removal

[Bin Liu]

Hi Johan,

On Tue, Jan 22, 2019 at 02:16:30PM -0600, Bin Liu wrote:
> On Tue, Jan 22, 2019 at 05:19:39PM +0000, Måns Rullgård wrote:
> > Bin Liu <b-liu@ti.com> writes:
> > 
> > > On Mon, Jan 21, 2019 at 09:20:52PM +0000, Måns Rullgård wrote:
> > >> Bin Liu <b-liu@ti.com> writes:
> > >> 
> > >> > On Fri, Jan 18, 2019 at 08:15:02PM +0000, Måns Rullgård wrote:
> > >> >> Bin Liu <b-liu@ti.com> writes:
> > >> >> 
> > >> >> > On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
> > >> >> >> Bin Liu <b-liu@ti.com> writes:
> > >> >> >> 
> > >> >> >> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
> > >> >> >> >> Bin Liu <b-liu@ti.com> writes:
> > >> >> >> >> 
> > >> >> >> >> > Hi,
> > >> >> >> >> >
> > >> >> >> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> > >> >> >> >> >> I have a strange problem with the musb driver in host mode on AM3358
> > >> >> >> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
> > >> >
> > >> > Is this on beaglebone or beagleboneblack?
> > >> 
> > >> We've seen it with Beaglebone Enhanced [1] and BeagleCore [2] based
> > >> devices.  Both are based on the AM3358 and mostly compatible with the
> > >> Beaglebone Black.
> > >> 
> > >> [1] https://elinux.org/SanCloud_BeagleBoneEnhanced
> > >> [2] http://beaglecore.com/products/bcm1str.html
> > >
> > > Okay, now I see you have a hub on the board. I just reproduced the
> > > console lockup after added a hub in my setup. I will find some time to
> > > debug this.
> > >
> > > FYI, there was a similar issue in a few years ago, which was that the
> > > hub routine never got called during the storm then the device disconnect
> > > event never got populated from the hub driver. I am wondering if this is
> > > the same issue pops up again...
> > 
> > Now that you mentioned the hub, I tried connecting the serial adapter to
> > the OTG port (in host mode) on the Beaglebone Enhanced (it's not
> > normally used in this product).  When connected directly, the disconnect
> > indeed works properly.  With a hub in between, it breaks.
> 
> Please use the following hack as a workaround for now. I will think more
> about it.
> 
> Regards,
> -Bin.
> 
> --------8<----------
> diff git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index b59ce9ad14ce..3719f6d9b26c 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -1811,7 +1811,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
>         } else if (rx_csr & MUSB_RXCSR_H_ERROR) {
>                 musb_dbg(musb, "end %d RX proto error", epnum);
>  
> -               status = -EPROTO;
> +               status = -EPIPE;
>                 musb_writeb(epio, MUSB_RXINTERVAL, 0);
>  
>                 rx_csr &= ~MUSB_RXCSR_H_ERROR;

When a usb serial device is detached, the musb controller driver returns
-EPROTO in urb giveback, then usb_serial_generic_read_bulk_callback()
re-submit the urb, this circle causes interrupt storm, and the hub
driver workqueue never got a chance to run to populate the device
disconnect event, the whole console locks up. Do you think
usb_serial_generic_read_bulk_callback() should handle -EPROTO as well?


Regards,
-Bin.

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 2274d9625f63..8c2a6e55de3a 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -397,6 +397,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
                                                        __func__, status);
                return;
        case -EPIPE:
+       case -EPROTO:
                dev_err(&port->dev, "%s - urb stopped: %d\n",
                                                        __func__, status);
                return;

MUSB interrupt storm on device removal

[Bin Liu]

On Tue, Jan 22, 2019 at 05:19:39PM +0000, Måns Rullgård wrote:
> Bin Liu <b-liu@ti.com> writes:
> 
> > On Mon, Jan 21, 2019 at 09:20:52PM +0000, Måns Rullgård wrote:
> >> Bin Liu <b-liu@ti.com> writes:
> >> 
> >> > On Fri, Jan 18, 2019 at 08:15:02PM +0000, Måns Rullgård wrote:
> >> >> Bin Liu <b-liu@ti.com> writes:
> >> >> 
> >> >> > On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
> >> >> >> Bin Liu <b-liu@ti.com> writes:
> >> >> >> 
> >> >> >> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
> >> >> >> >> Bin Liu <b-liu@ti.com> writes:
> >> >> >> >> 
> >> >> >> >> > Hi,
> >> >> >> >> >
> >> >> >> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> >> >> >> >> >> I have a strange problem with the musb driver in host mode on AM3358
> >> >> >> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
> >> >
> >> > Is this on beaglebone or beagleboneblack?
> >> 
> >> We've seen it with Beaglebone Enhanced [1] and BeagleCore [2] based
> >> devices.  Both are based on the AM3358 and mostly compatible with the
> >> Beaglebone Black.
> >> 
> >> [1] https://elinux.org/SanCloud_BeagleBoneEnhanced
> >> [2] http://beaglecore.com/products/bcm1str.html
> >
> > Okay, now I see you have a hub on the board. I just reproduced the
> > console lockup after added a hub in my setup. I will find some time to
> > debug this.
> >
> > FYI, there was a similar issue in a few years ago, which was that the
> > hub routine never got called during the storm then the device disconnect
> > event never got populated from the hub driver. I am wondering if this is
> > the same issue pops up again...
> 
> Now that you mentioned the hub, I tried connecting the serial adapter to
> the OTG port (in host mode) on the Beaglebone Enhanced (it's not
> normally used in this product).  When connected directly, the disconnect
> indeed works properly.  With a hub in between, it breaks.

Please use the following hack as a workaround for now. I will think more
about it.

Regards,
-Bin.

--------8<----------

diff git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index b59ce9ad14ce..3719f6d9b26c 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1811,7 +1811,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
        } else if (rx_csr & MUSB_RXCSR_H_ERROR) {
                musb_dbg(musb, "end %d RX proto error", epnum);
 
-               status = -EPROTO;
+               status = -EPIPE;
                musb_writeb(epio, MUSB_RXINTERVAL, 0);
 
                rx_csr &= ~MUSB_RXCSR_H_ERROR;

MUSB interrupt storm on device removal

[Måns Rullgård]

Bin Liu <b-liu@ti.com> writes:

> On Mon, Jan 21, 2019 at 09:20:52PM +0000, Måns Rullgård wrote:
>> Bin Liu <b-liu@ti.com> writes:
>> 
>> > On Fri, Jan 18, 2019 at 08:15:02PM +0000, Måns Rullgård wrote:
>> >> Bin Liu <b-liu@ti.com> writes:
>> >> 
>> >> > On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
>> >> >> Bin Liu <b-liu@ti.com> writes:
>> >> >> 
>> >> >> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
>> >> >> >> Bin Liu <b-liu@ti.com> writes:
>> >> >> >> 
>> >> >> >> > Hi,
>> >> >> >> >
>> >> >> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
>> >> >> >> >> I have a strange problem with the musb driver in host mode on AM3358
>> >> >> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
>> >
>> > Is this on beaglebone or beagleboneblack?
>> 
>> We've seen it with Beaglebone Enhanced [1] and BeagleCore [2] based
>> devices.  Both are based on the AM3358 and mostly compatible with the
>> Beaglebone Black.
>> 
>> [1] https://elinux.org/SanCloud_BeagleBoneEnhanced
>> [2] http://beaglecore.com/products/bcm1str.html
>
> Okay, now I see you have a hub on the board. I just reproduced the
> console lockup after added a hub in my setup. I will find some time to
> debug this.
>
> FYI, there was a similar issue in a few years ago, which was that the
> hub routine never got called during the storm then the device disconnect
> event never got populated from the hub driver. I am wondering if this is
> the same issue pops up again...

Now that you mentioned the hub, I tried connecting the serial adapter to
the OTG port (in host mode) on the Beaglebone Enhanced (it's not
normally used in this product).  When connected directly, the disconnect
indeed works properly.  With a hub in between, it breaks.

MUSB interrupt storm on device removal

[Bin Liu]

On Mon, Jan 21, 2019 at 09:20:52PM +0000, Måns Rullgård wrote:
> Bin Liu <b-liu@ti.com> writes:
> 
> > On Fri, Jan 18, 2019 at 08:15:02PM +0000, Måns Rullgård wrote:
> >> Bin Liu <b-liu@ti.com> writes:
> >> 
> >> > On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
> >> >> Bin Liu <b-liu@ti.com> writes:
> >> >> 
> >> >> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
> >> >> >> Bin Liu <b-liu@ti.com> writes:
> >> >> >> 
> >> >> >> > Hi,
> >> >> >> >
> >> >> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> >> >> >> >> I have a strange problem with the musb driver in host mode on AM3358
> >> >> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
> >
> > Is this on beaglebone or beagleboneblack?
> 
> We've seen it with Beaglebone Enhanced [1] and BeagleCore [2] based
> devices.  Both are based on the AM3358 and mostly compatible with the
> Beaglebone Black.
> 
> [1] https://elinux.org/SanCloud_BeagleBoneEnhanced
> [2] http://beaglecore.com/products/bcm1str.html

Okay, now I see you have a hub on the board. I just reproduced the
console lockup after added a hub in my setup. I will find some time to
debug this.

FYI, there was a similar issue in a few years ago, which was that the
hub routine never got called during the storm then the device disconnect
event never got populated from the hub driver. I am wondering if this is
the same issue pops up again...

Regards,
-Bin.

MUSB interrupt storm on device removal

[Måns Rullgård]

Bin Liu <b-liu@ti.com> writes:

> On Fri, Jan 18, 2019 at 08:15:02PM +0000, Måns Rullgård wrote:
>> Bin Liu <b-liu@ti.com> writes:
>> 
>> > On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
>> >> Bin Liu <b-liu@ti.com> writes:
>> >> 
>> >> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
>> >> >> Bin Liu <b-liu@ti.com> writes:
>> >> >> 
>> >> >> > Hi,
>> >> >> >
>> >> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
>> >> >> >> I have a strange problem with the musb driver in host mode on AM3358
>> >> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
>
> Is this on beaglebone or beagleboneblack?

We've seen it with Beaglebone Enhanced [1] and BeagleCore [2] based
devices.  Both are based on the AM3358 and mostly compatible with the
Beaglebone Black.

[1] https://elinux.org/SanCloud_BeagleBoneEnhanced
[2] http://beaglecore.com/products/bcm1str.html

>> >> >> >> open two or more of the ttys, then unplug the device, an interrupt storm
>> >> >> >> ensues that makes the system completely unresponsive until the watchdog
>> >> >> >> resets it.  Enabling some debug messages, I get this repeated endlessly:
>> >> >> >> 
>> >> >> >> musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
>> >> >> >> musb-hdrc musb-hdrc.1: end 2 RX proto error
>> >> >> >> musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
>> >> >> >> musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
>> >> >> >> musb-hdrc musb-hdrc.1: RXCSR2 := 2020
>> >> >> >> musb-hdrc musb-hdrc.1: end 3 RX proto error
>> >> >> >> musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
>> >> >> >> musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
>> >> >> >> musb-hdrc musb-hdrc.1: RXCSR3 := 2020
>> >> >> >> musb-hdrc musb-hdrc.1: end 4 RX proto error
>> >> >> >> musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
>> >> >> >> musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
>> >> >> >> musb-hdrc musb-hdrc.1: RXCSR4 := 2020
>> >> >> >> 
>> >> >> >> This happens with both a two-port FTDI serial adapter and a Simcom GSM
>> >> >> >> modem (Qualcomm based) using the "option" driver.  In both cases, the
>> >> >> >> problem occurs only if two or more of the ttys are opened when the
>> >> >> >> device is unplugged.
>> >> >> >
>> >> >> > Please help me to understand the test case so that I can try to
>> >> >> > replicate the issue - if I connect a multi-port FTDI adapter, use cat
>> >> >> > command to open more than one port then unplug the adapter, I should
>> >> >> > see the flooding debug messages?
>> >> >> 
>> >> >> Yes, that's what I'm seeing.  Obviously, you have to first enable those
>> >> >> debug messages.  Otherwise it just stops dead in its tracks.
>> >> >> 
>> >> >> I forgot to mention, I also tried making the interrupt handler threaded.
>> >> >> This allows the system to remain slightly responsive, but it never
>> >> >> recovers.
>> >> >> 
>> >> >> > On which kernel version do you see the problem?
>> >> >> 
>> >> >> This was on 4.19.9.  I don't see any later commits that look related,
>> >> >> but I can try some other tree if you suggest one.
>> >> >
>> >> > This should be fine. The driver was barely touched recently.
>> >> > I have only a few days left for 2018, let me try to reproduce it first
>> >> > after I am back in Jan.
>> >> 
>> >> No problem.  I suspect most of us will have some time off during the
>> >> holidays.
>> >> 
>> >> If it helps, we first noticed this with a 4.9 (stable) kernel, so it's
>> >> not a recent regression.
>> >
>> > When I tried to reproduce the issue today, something weird happened, the
>> > v4.19.9 kernel I compiled doesn't bring usb1 port on my BBB to host
>> > mode, otg state was stuck in a_idle, DevCtl register is 0x81. The latest
>
> I fixed the the bug which was in the am335x phy driver last week,
>
>> > v5.0-rc1 and somewhere in v4.16 do not have such problem. I tried git
>> > bisect, but it was screwed up somewhere need to start over. I will find
>> > some time for this issue.
>> >
>> > By the way, I don't see the interrupt storm issue with v5.0-rc1 on my
>> > BBB, when unplugged the FTDI 4-port device, the 'proto error' message
>> > only shows once for each port opened, then MUSB_DISCONNECT interrupt
>> > (0x20) happened and everything was torn down.
>> 
>> I tested this with v5.0-rc2.  Now the badness doesn't happen every time,
>> but two or three attempts is still enough to reproduce it.  Also, when
>> it does happen, the system no longer freezes completely.  It is now
>> possible to interrupt the process with the tty open, at which point the
>> kernel prints these messages:
>> 
>> ftdi_sio ttyUSB0: failed to get modem status: -71
>> ftdi_sio ttyUSB0: error from flowcontrol urb
>> 
>> Meanwhile, about 10k interrupts per second have been recorded.  The USB
>> device is still reported as connected, and the port remains unusable.
>
> but I still cannot see the interrupt storm problem in v4.19.9.
> Did you build the kernel with omap2plus_defconfig without any other
> changes?

No, we use a configuration suitable for our devices.  It shouldn't
differ in any way that matters though.

MUSB interrupt storm on device removal

[Bin Liu]

On Fri, Jan 18, 2019 at 08:15:02PM +0000, Måns Rullgård wrote:
> Bin Liu <b-liu@ti.com> writes:
> 
> > On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
> >> Bin Liu <b-liu@ti.com> writes:
> >> 
> >> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
> >> >> Bin Liu <b-liu@ti.com> writes:
> >> >> 
> >> >> > Hi,
> >> >> >
> >> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> >> >> >> I have a strange problem with the musb driver in host mode on AM3358
> >> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and

Is this on beaglebone or beagleboneblack?

> >> >> >> open two or more of the ttys, then unplug the device, an interrupt storm
> >> >> >> ensues that makes the system completely unresponsive until the watchdog
> >> >> >> resets it.  Enabling some debug messages, I get this repeated endlessly:
> >> >> >> 
> >> >> >> musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
> >> >> >> musb-hdrc musb-hdrc.1: end 2 RX proto error
> >> >> >> musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
> >> >> >> musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
> >> >> >> musb-hdrc musb-hdrc.1: RXCSR2 := 2020
> >> >> >> musb-hdrc musb-hdrc.1: end 3 RX proto error
> >> >> >> musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
> >> >> >> musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
> >> >> >> musb-hdrc musb-hdrc.1: RXCSR3 := 2020
> >> >> >> musb-hdrc musb-hdrc.1: end 4 RX proto error
> >> >> >> musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
> >> >> >> musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
> >> >> >> musb-hdrc musb-hdrc.1: RXCSR4 := 2020
> >> >> >> 
> >> >> >> This happens with both a two-port FTDI serial adapter and a Simcom GSM
> >> >> >> modem (Qualcomm based) using the "option" driver.  In both cases, the
> >> >> >> problem occurs only if two or more of the ttys are opened when the
> >> >> >> device is unplugged.
> >> >> >
> >> >> > Please help me to understand the test case so that I can try to
> >> >> > replicate the issue - if I connect a multi-port FTDI adapter, use cat
> >> >> > command to open more than one port then unplug the adapter, I should
> >> >> > see the flooding debug messages?
> >> >> 
> >> >> Yes, that's what I'm seeing.  Obviously, you have to first enable those
> >> >> debug messages.  Otherwise it just stops dead in its tracks.
> >> >> 
> >> >> I forgot to mention, I also tried making the interrupt handler threaded.
> >> >> This allows the system to remain slightly responsive, but it never
> >> >> recovers.
> >> >> 
> >> >> > On which kernel version do you see the problem?
> >> >> 
> >> >> This was on 4.19.9.  I don't see any later commits that look related,
> >> >> but I can try some other tree if you suggest one.
> >> >
> >> > This should be fine. The driver was barely touched recently.
> >> > I have only a few days left for 2018, let me try to reproduce it first
> >> > after I am back in Jan.
> >> 
> >> No problem.  I suspect most of us will have some time off during the
> >> holidays.
> >> 
> >> If it helps, we first noticed this with a 4.9 (stable) kernel, so it's
> >> not a recent regression.
> >
> > When I tried to reproduce the issue today, something weird happened, the
> > v4.19.9 kernel I compiled doesn't bring usb1 port on my BBB to host
> > mode, otg state was stuck in a_idle, DevCtl register is 0x81. The latest

I fixed the the bug which was in the am335x phy driver last week,

> > v5.0-rc1 and somewhere in v4.16 do not have such problem. I tried git
> > bisect, but it was screwed up somewhere need to start over. I will find
> > some time for this issue.
> >
> > By the way, I don't see the interrupt storm issue with v5.0-rc1 on my
> > BBB, when unplugged the FTDI 4-port device, the 'proto error' message
> > only shows once for each port opened, then MUSB_DISCONNECT interrupt
> > (0x20) happened and everything was torn down.
> 
> I tested this with v5.0-rc2.  Now the badness doesn't happen every time,
> but two or three attempts is still enough to reproduce it.  Also, when
> it does happen, the system no longer freezes completely.  It is now
> possible to interrupt the process with the tty open, at which point the
> kernel prints these messages:
> 
> ftdi_sio ttyUSB0: failed to get modem status: -71
> ftdi_sio ttyUSB0: error from flowcontrol urb
> 
> Meanwhile, about 10k interrupts per second have been recorded.  The USB
> device is still reported as connected, and the port remains unusable.

but I still cannot see the interrupt storm problem in v4.19.9.
Did you build the kernel with omap2plus_defconfig without any other
changes?

Regards,
-Bin.

MUSB interrupt storm on device removal

[Måns Rullgård]

Bin Liu <b-liu@ti.com> writes:

> On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
>> Bin Liu <b-liu@ti.com> writes:
>> 
>> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
>> >> Bin Liu <b-liu@ti.com> writes:
>> >> 
>> >> > Hi,
>> >> >
>> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
>> >> >> I have a strange problem with the musb driver in host mode on AM3358
>> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
>> >> >> open two or more of the ttys, then unplug the device, an interrupt storm
>> >> >> ensues that makes the system completely unresponsive until the watchdog
>> >> >> resets it.  Enabling some debug messages, I get this repeated endlessly:
>> >> >> 
>> >> >> musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
>> >> >> musb-hdrc musb-hdrc.1: end 2 RX proto error
>> >> >> musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
>> >> >> musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
>> >> >> musb-hdrc musb-hdrc.1: RXCSR2 := 2020
>> >> >> musb-hdrc musb-hdrc.1: end 3 RX proto error
>> >> >> musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
>> >> >> musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
>> >> >> musb-hdrc musb-hdrc.1: RXCSR3 := 2020
>> >> >> musb-hdrc musb-hdrc.1: end 4 RX proto error
>> >> >> musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
>> >> >> musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
>> >> >> musb-hdrc musb-hdrc.1: RXCSR4 := 2020
>> >> >> 
>> >> >> This happens with both a two-port FTDI serial adapter and a Simcom GSM
>> >> >> modem (Qualcomm based) using the "option" driver.  In both cases, the
>> >> >> problem occurs only if two or more of the ttys are opened when the
>> >> >> device is unplugged.
>> >> >
>> >> > Please help me to understand the test case so that I can try to
>> >> > replicate the issue - if I connect a multi-port FTDI adapter, use cat
>> >> > command to open more than one port then unplug the adapter, I should
>> >> > see the flooding debug messages?
>> >> 
>> >> Yes, that's what I'm seeing.  Obviously, you have to first enable those
>> >> debug messages.  Otherwise it just stops dead in its tracks.
>> >> 
>> >> I forgot to mention, I also tried making the interrupt handler threaded.
>> >> This allows the system to remain slightly responsive, but it never
>> >> recovers.
>> >> 
>> >> > On which kernel version do you see the problem?
>> >> 
>> >> This was on 4.19.9.  I don't see any later commits that look related,
>> >> but I can try some other tree if you suggest one.
>> >
>> > This should be fine. The driver was barely touched recently.
>> > I have only a few days left for 2018, let me try to reproduce it first
>> > after I am back in Jan.
>> 
>> No problem.  I suspect most of us will have some time off during the
>> holidays.
>> 
>> If it helps, we first noticed this with a 4.9 (stable) kernel, so it's
>> not a recent regression.
>
> When I tried to reproduce the issue today, something weird happened, the
> v4.19.9 kernel I compiled doesn't bring usb1 port on my BBB to host
> mode, otg state was stuck in a_idle, DevCtl register is 0x81. The latest
> v5.0-rc1 and somewhere in v4.16 do not have such problem. I tried git
> bisect, but it was screwed up somewhere need to start over. I will find
> some time for this issue.
>
> By the way, I don't see the interrupt storm issue with v5.0-rc1 on my
> BBB, when unplugged the FTDI 4-port device, the 'proto error' message
> only shows once for each port opened, then MUSB_DISCONNECT interrupt
> (0x20) happened and everything was torn down.

I tested this with v5.0-rc2.  Now the badness doesn't happen every time,
but two or three attempts is still enough to reproduce it.  Also, when
it does happen, the system no longer freezes completely.  It is now
possible to interrupt the process with the tty open, at which point the
kernel prints these messages:

ftdi_sio ttyUSB0: failed to get modem status: -71
ftdi_sio ttyUSB0: error from flowcontrol urb

Meanwhile, about 10k interrupts per second have been recorded.  The USB
device is still reported as connected, and the port remains unusable.

MUSB interrupt storm on device removal

[Bin Liu]

On Mon, Dec 17, 2018 at 09:36:17PM +0000, Måns Rullgård wrote:
> Bin Liu <b-liu@ti.com> writes:
> 
> > On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
> >> Bin Liu <b-liu@ti.com> writes:
> >> 
> >> > Hi,
> >> >
> >> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> >> >> I have a strange problem with the musb driver in host mode on AM3358
> >> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
> >> >> open two or more of the ttys, then unplug the device, an interrupt storm
> >> >> ensues that makes the system completely unresponsive until the watchdog
> >> >> resets it.  Enabling some debug messages, I get this repeated endlessly:
> >> >> 
> >> >> musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
> >> >> musb-hdrc musb-hdrc.1: end 2 RX proto error
> >> >> musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
> >> >> musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
> >> >> musb-hdrc musb-hdrc.1: RXCSR2 := 2020
> >> >> musb-hdrc musb-hdrc.1: end 3 RX proto error
> >> >> musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
> >> >> musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
> >> >> musb-hdrc musb-hdrc.1: RXCSR3 := 2020
> >> >> musb-hdrc musb-hdrc.1: end 4 RX proto error
> >> >> musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
> >> >> musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
> >> >> musb-hdrc musb-hdrc.1: RXCSR4 := 2020
> >> >> 
> >> >> This happens with both a two-port FTDI serial adapter and a Simcom GSM
> >> >> modem (Qualcomm based) using the "option" driver.  In both cases, the
> >> >> problem occurs only if two or more of the ttys are opened when the
> >> >> device is unplugged.
> >> >
> >> > Please help me to understand the test case so that I can try to
> >> > replicate the issue - if I connect a multi-port FTDI adapter, use cat
> >> > command to open more than one port then unplug the adapter, I should
> >> > see the flooding debug messages?
> >> 
> >> Yes, that's what I'm seeing.  Obviously, you have to first enable those
> >> debug messages.  Otherwise it just stops dead in its tracks.
> >> 
> >> I forgot to mention, I also tried making the interrupt handler threaded.
> >> This allows the system to remain slightly responsive, but it never
> >> recovers.
> >> 
> >> > On which kernel version do you see the problem?
> >> 
> >> This was on 4.19.9.  I don't see any later commits that look related,
> >> but I can try some other tree if you suggest one.
> >
> > This should be fine. The driver was barely touched recently.
> > I have only a few days left for 2018, let me try to reproduce it first
> > after I am back in Jan.
> 
> No problem.  I suspect most of us will have some time off during the
> holidays.
> 
> If it helps, we first noticed this with a 4.9 (stable) kernel, so it's
> not a recent regression.

When I tried to reproduce the issue today, something weird happened, the
v4.19.9 kernel I compiled doesn't bring usb1 port on my BBB to host
mode, otg state was stuck in a_idle, DevCtl register is 0x81. The latest
v5.0-rc1 and somewhere in v4.16 do not have such problem. I tried git
bisect, but it was screwed up somewhere need to start over. I will find
some time for this issue.

By the way, I don't see the interrupt storm issue with v5.0-rc1 on my
BBB, when unplugged the FTDI 4-port device, the 'proto error' message
only shows once for each port opened, then MUSB_DISCONNECT interrupt
(0x20) happened and everything was torn down.

Regards,
-Bin.

MUSB interrupt storm on device removal

[Måns Rullgård]

Bin Liu <b-liu@ti.com> writes:

> On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
>> Bin Liu <b-liu@ti.com> writes:
>> 
>> > Hi,
>> >
>> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
>> >> I have a strange problem with the musb driver in host mode on AM3358
>> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
>> >> open two or more of the ttys, then unplug the device, an interrupt storm
>> >> ensues that makes the system completely unresponsive until the watchdog
>> >> resets it.  Enabling some debug messages, I get this repeated endlessly:
>> >> 
>> >> musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
>> >> musb-hdrc musb-hdrc.1: end 2 RX proto error
>> >> musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
>> >> musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
>> >> musb-hdrc musb-hdrc.1: RXCSR2 := 2020
>> >> musb-hdrc musb-hdrc.1: end 3 RX proto error
>> >> musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
>> >> musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
>> >> musb-hdrc musb-hdrc.1: RXCSR3 := 2020
>> >> musb-hdrc musb-hdrc.1: end 4 RX proto error
>> >> musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
>> >> musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
>> >> musb-hdrc musb-hdrc.1: RXCSR4 := 2020
>> >> 
>> >> This happens with both a two-port FTDI serial adapter and a Simcom GSM
>> >> modem (Qualcomm based) using the "option" driver.  In both cases, the
>> >> problem occurs only if two or more of the ttys are opened when the
>> >> device is unplugged.
>> >
>> > Please help me to understand the test case so that I can try to
>> > replicate the issue - if I connect a multi-port FTDI adapter, use cat
>> > command to open more than one port then unplug the adapter, I should
>> > see the flooding debug messages?
>> 
>> Yes, that's what I'm seeing.  Obviously, you have to first enable those
>> debug messages.  Otherwise it just stops dead in its tracks.
>> 
>> I forgot to mention, I also tried making the interrupt handler threaded.
>> This allows the system to remain slightly responsive, but it never
>> recovers.
>> 
>> > On which kernel version do you see the problem?
>> 
>> This was on 4.19.9.  I don't see any later commits that look related,
>> but I can try some other tree if you suggest one.
>
> This should be fine. The driver was barely touched recently.
> I have only a few days left for 2018, let me try to reproduce it first
> after I am back in Jan.

In case it helps, the musb variant in the Allwinner A20 doesn't suffer
from this problem.

MUSB interrupt storm on device removal

[Måns Rullgård]

Bin Liu <b-liu@ti.com> writes:

> On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
>> Bin Liu <b-liu@ti.com> writes:
>> 
>> > Hi,
>> >
>> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
>> >> I have a strange problem with the musb driver in host mode on AM3358
>> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
>> >> open two or more of the ttys, then unplug the device, an interrupt storm
>> >> ensues that makes the system completely unresponsive until the watchdog
>> >> resets it.  Enabling some debug messages, I get this repeated endlessly:
>> >> 
>> >> musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
>> >> musb-hdrc musb-hdrc.1: end 2 RX proto error
>> >> musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
>> >> musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
>> >> musb-hdrc musb-hdrc.1: RXCSR2 := 2020
>> >> musb-hdrc musb-hdrc.1: end 3 RX proto error
>> >> musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
>> >> musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
>> >> musb-hdrc musb-hdrc.1: RXCSR3 := 2020
>> >> musb-hdrc musb-hdrc.1: end 4 RX proto error
>> >> musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
>> >> musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
>> >> musb-hdrc musb-hdrc.1: RXCSR4 := 2020
>> >> 
>> >> This happens with both a two-port FTDI serial adapter and a Simcom GSM
>> >> modem (Qualcomm based) using the "option" driver.  In both cases, the
>> >> problem occurs only if two or more of the ttys are opened when the
>> >> device is unplugged.
>> >
>> > Please help me to understand the test case so that I can try to
>> > replicate the issue - if I connect a multi-port FTDI adapter, use cat
>> > command to open more than one port then unplug the adapter, I should
>> > see the flooding debug messages?
>> 
>> Yes, that's what I'm seeing.  Obviously, you have to first enable those
>> debug messages.  Otherwise it just stops dead in its tracks.
>> 
>> I forgot to mention, I also tried making the interrupt handler threaded.
>> This allows the system to remain slightly responsive, but it never
>> recovers.
>> 
>> > On which kernel version do you see the problem?
>> 
>> This was on 4.19.9.  I don't see any later commits that look related,
>> but I can try some other tree if you suggest one.
>
> This should be fine. The driver was barely touched recently.
> I have only a few days left for 2018, let me try to reproduce it first
> after I am back in Jan.

No problem.  I suspect most of us will have some time off during the
holidays.

If it helps, we first noticed this with a 4.9 (stable) kernel, so it's
not a recent regression.

MUSB interrupt storm on device removal

[Bin Liu]

On Mon, Dec 17, 2018 at 07:16:08PM +0000, Måns Rullgård wrote:
> Bin Liu <b-liu@ti.com> writes:
> 
> > Hi,
> >
> > On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> >> I have a strange problem with the musb driver in host mode on AM3358
> >> (beaglebone) hardware.  If I connect a multi-port serial adapter and
> >> open two or more of the ttys, then unplug the device, an interrupt storm
> >> ensues that makes the system completely unresponsive until the watchdog
> >> resets it.  Enabling some debug messages, I get this repeated endlessly:
> >> 
> >> musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
> >> musb-hdrc musb-hdrc.1: end 2 RX proto error
> >> musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
> >> musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
> >> musb-hdrc musb-hdrc.1: RXCSR2 := 2020
> >> musb-hdrc musb-hdrc.1: end 3 RX proto error
> >> musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
> >> musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
> >> musb-hdrc musb-hdrc.1: RXCSR3 := 2020
> >> musb-hdrc musb-hdrc.1: end 4 RX proto error
> >> musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
> >> musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
> >> musb-hdrc musb-hdrc.1: RXCSR4 := 2020
> >> 
> >> This happens with both a two-port FTDI serial adapter and a Simcom GSM
> >> modem (Qualcomm based) using the "option" driver.  In both cases, the
> >> problem occurs only if two or more of the ttys are opened when the
> >> device is unplugged.
> >
> > Please help me to understand the test case so that I can try to
> > replicate the issue - if I connect a multi-port FTDI adapter, use cat
> > command to open more than one port then unplug the adapter, I should
> > see the flooding debug messages?
> 
> Yes, that's what I'm seeing.  Obviously, you have to first enable those
> debug messages.  Otherwise it just stops dead in its tracks.
> 
> I forgot to mention, I also tried making the interrupt handler threaded.
> This allows the system to remain slightly responsive, but it never
> recovers.
> 
> > On which kernel version do you see the problem?
> 
> This was on 4.19.9.  I don't see any later commits that look related,
> but I can try some other tree if you suggest one.

This should be fine. The driver was barely touched recently.
I have only a few days left for 2018, let me try to reproduce it first
after I am back in Jan.

Regards,
-Bin.

MUSB interrupt storm on device removal

[Måns Rullgård]

Bin Liu <b-liu@ti.com> writes:

> Hi,
>
> On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
>> I have a strange problem with the musb driver in host mode on AM3358
>> (beaglebone) hardware.  If I connect a multi-port serial adapter and
>> open two or more of the ttys, then unplug the device, an interrupt storm
>> ensues that makes the system completely unresponsive until the watchdog
>> resets it.  Enabling some debug messages, I get this repeated endlessly:
>> 
>> musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
>> musb-hdrc musb-hdrc.1: end 2 RX proto error
>> musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
>> musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
>> musb-hdrc musb-hdrc.1: RXCSR2 := 2020
>> musb-hdrc musb-hdrc.1: end 3 RX proto error
>> musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
>> musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
>> musb-hdrc musb-hdrc.1: RXCSR3 := 2020
>> musb-hdrc musb-hdrc.1: end 4 RX proto error
>> musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
>> musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
>> musb-hdrc musb-hdrc.1: RXCSR4 := 2020
>> 
>> This happens with both a two-port FTDI serial adapter and a Simcom GSM
>> modem (Qualcomm based) using the "option" driver.  In both cases, the
>> problem occurs only if two or more of the ttys are opened when the
>> device is unplugged.
>
> Please help me to understand the test case so that I can try to
> replicate the issue - if I connect a multi-port FTDI adapter, use cat
> command to open more than one port then unplug the adapter, I should
> see the flooding debug messages?

Yes, that's what I'm seeing.  Obviously, you have to first enable those
debug messages.  Otherwise it just stops dead in its tracks.

I forgot to mention, I also tried making the interrupt handler threaded.
This allows the system to remain slightly responsive, but it never
recovers.

> On which kernel version do you see the problem?

This was on 4.19.9.  I don't see any later commits that look related,
but I can try some other tree if you suggest one.

MUSB interrupt storm on device removal

[Bin Liu]

Hi,

On Mon, Dec 17, 2018 at 03:13:12PM +0000, Måns Rullgård wrote:
> I have a strange problem with the musb driver in host mode on AM3358
> (beaglebone) hardware.  If I connect a multi-port serial adapter and
> open two or more of the ttys, then unplug the device, an interrupt storm
> ensues that makes the system completely unresponsive until the watchdog
> resets it.  Enabling some debug messages, I get this repeated endlessly:
> 
> musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
> musb-hdrc musb-hdrc.1: end 2 RX proto error
> musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
> musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
> musb-hdrc musb-hdrc.1: RXCSR2 := 2020
> musb-hdrc musb-hdrc.1: end 3 RX proto error
> musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
> musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
> musb-hdrc musb-hdrc.1: RXCSR3 := 2020
> musb-hdrc musb-hdrc.1: end 4 RX proto error
> musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
> musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
> musb-hdrc musb-hdrc.1: RXCSR4 := 2020
> 
> This happens with both a two-port FTDI serial adapter and a Simcom GSM
> modem (Qualcomm based) using the "option" driver.  In both cases, the
> problem occurs only if two or more of the ttys are opened when the
> device is unplugged.

Please help me to understand the test case so that I can try to
replicate the issue - if I connect a multi-port FTDI adapter, use cat
command to open more than one port then unplug the adapter, I should
see the flooding debug messages?

On which kernel version do you see the problem?

Regards,
-Bin.

MUSB interrupt storm on device removal

[Måns Rullgård]

I have a strange problem with the musb driver in host mode on AM3358
(beaglebone) hardware.  If I connect a multi-port serial adapter and
open two or more of the ttys, then unplug the device, an interrupt storm
ensues that makes the system completely unresponsive until the watchdog
resets it.  Enabling some debug messages, I get this repeated endlessly:

musb-hdrc musb-hdrc.1: usbintr (0) epintr(1c0000)
musb-hdrc musb-hdrc.1: end 2 RX proto error
musb-hdrc musb-hdrc.1: ... next ep2 RX urb 72aabc13
musb-hdrc musb-hdrc.1: <-- hw2 urb 72aabc13 spd3 dev4 ep3in h_addr02 h_port00 bytes 4096
musb-hdrc musb-hdrc.1: RXCSR2 := 2020
musb-hdrc musb-hdrc.1: end 3 RX proto error
musb-hdrc musb-hdrc.1: ... next ep3 RX urb 94c2cc43
musb-hdrc musb-hdrc.1: <-- hw3 urb 94c2cc43 spd3 dev4 ep2in h_addr02 h_port00 bytes 4096
musb-hdrc musb-hdrc.1: RXCSR3 := 2020
musb-hdrc musb-hdrc.1: end 4 RX proto error
musb-hdrc musb-hdrc.1: ... next ep4 RX urb cbdc39c6
musb-hdrc musb-hdrc.1: <-- hw4 urb cbdc39c6 spd3 dev4 ep5in h_addr02 h_port00 bytes 4096
musb-hdrc musb-hdrc.1: RXCSR4 := 2020

This happens with both a two-port FTDI serial adapter and a Simcom GSM
modem (Qualcomm based) using the "option" driver.  In both cases, the
problem occurs only if two or more of the ttys are opened when the
device is unplugged.

Applying the patch below makes this problem go away, though it most
likely breaks something else.  I'm only posting it here in case it helps
identify the cause of the error.

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index b59ce9ad14ce..addf9197689c 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -337,6 +337,7 @@ static void musb_advance_schedule(struct musb *musb, struct urb *urb,
 
 	qh->is_ready = 0;
 	musb_giveback(musb, urb, status);
+	if (status != -EPROTO)
 		qh->is_ready = ready;
 
 	/* reclaim resources (and bandwidth) ASAP; deschedule it, and
@@ -2465,12 +2466,14 @@ musb_h_disable(struct usb_hcd *hcd, struct usb_host_endpoint *hep)
 	if (musb_ep_get_qh(qh->hw_ep, is_in) == qh) {
 		urb = next_urb(qh);
 
+		if (urb) {
 			/* make software (then hardware) stop ASAP */
 			if (!urb->unlinked)
 				urb->status = -ESHUTDOWN;
 
 			/* cleanup */
 			musb_cleanup_urb(urb, qh);
+		}
 
 		/* Then nuke all the others ... and advance the
 		 * queue on hw_ep (e.g. bulk ring) when we're done.