STM32F407 (plus probably others): lockup on CDC-ACM via OTG, triggered by SUSPEND

[Hawhill]

Hi,

I've been hunting down a bug that bit me for a few days now. I just couldn't get USB CDC-ACM to work - however, sometimes (especially in debugging situations), it did. I think I have it all sorted out now.

Example code:
https://github.com/hwhw/stm32f4-project ... RT-USB_BUG
(note this includes a custom board config that has *not* been generated by automated tools, but manually edited - sorry. Board config is for a China-sourced "vcc-gnd.com" board. Bug is likely to happen on all other boards, too, but the special trigger is probably the behaviour of the USB host)

Description of the bug as far as I can see:

• first, soon after connecting, some data is put into the USB output queue.
  (1)
• a transmit is triggered for the OTG device, data is handed over to the TX FIFO via otg_fifo_write_from_buffer()
  (2)
• host PC (ArchLinux, kernel version is 4.7.4) puts USB device to suspend soon thereafter. Probably rightly so, since no terminal program is connected (yet) to the ACM tty.
• this triggers the callback in usbcfg.c:290 ...
• which will then call sduDisconnectI(&SDU1) ...
• which will - among other things - call obqResetI() on its output queue ...
• which will reset the queue, as its name suggests.
  
  (3)
• at some point, a terminal program is connected to the ACM TTY on the host PC.
• this triggers a wakeup callback. Does nothing in default config in usbcfg.c.
  
  (4)
• at *this* point, the OTG device actually sends the data from its FIFO and asserts the TX DONE interrupt (as per a usbmon Wireshark dump)
• this will be handled by sduDataTransmitted() ...
• which will dutyfully try to free the buffer using obqReleaseEmptyBufferI() ...
• which fails and triggers chSysHalt().

There is some critical timing and host action involved:

• at (1), the transmit must actually have been triggered
• at (2), the host PC must put the device into USB SUSPEND state before acknowledging the data. In many cases - sometimes debugging was enough - at this point I did not get the "right" (or rather wrong) timing, so the TX DONE was signalled before the USB SUSPEND got in - all fine in those cases, no bug.

Rather dirty workaround for the bug: don't handle USB SUSPEND. See usbcfg.c for an experimental #ifndef.

Where's the bug?

• not sure if Linux should put the device into USB SUSPEND at that point
• in any case, the STM32 should not lock up even then
• OTG device might serve a TX DONE interrupt veeeeeery late. (Note: the data actually has been transmit by the OTG device indeed right before the interrupt.) So the OTG device probably isn't to blame
• ChibiOS' sduDisconnectI()
  
  • might have to make sure to prevent the OTG device from transmitting data later on?
    
  • probably just isn't the right thing to do when we get put into USB SUSPEND anyway (this is code from the demos/STM32/RT-STM32F407-OLIMEX_E407-LWIP-FATFS-USB example)

This is probably the very same bug that is talked about in these posts:
viewtopic.php?f=16&t=3476 (for STM32F746)

How would I go on with this? I'd have a go at writing a fix (aside from just not handling USB suspend at all), any suggestions for that?

[Giovanni]

Hi,

My first idea is that we do "too much" in the suspend callback OR we are missing to do something in wakeup. The description is very clear, ideas anybody?

Moving this thread in "bug reports" for further review.

Giovanni

[Korken]

This seems to indeed be my problem as well as discussed before.
I noticed that this does not seem to happen on any Debian derived distribution (Ubuntu/Mint etc), I only have this issue with Arch based distributions.

I will follow this thread with interest.

[Giovanni]

Hi,

The explanation is very clear, thanks, I will try to implement a fix for this but I will need help with validation of the fix, I cannot reproduce it here so I can't test.

I wonder if a similar problem could occur on the receive side: RX callback after the queue has been reset asynchronously.

Probably the right fix could be to no reset queues, could you try to remove lines 311 and 312 in hal_serial_usb.c?

Giovanni

[Hawhill]

I will test soon and give feedback then.

Unfortunately, I think this might clog the queue for situations when there is a "half-transmitted" package in the queue (but not yet sent) and the device disables USB for proper reasons - e.g. remote end is gone. There will be no TX ack in those cases, so it is probably correct to clear the queue.

USB suspend handling seems to be a very difficult thing overall. I think the "proper" solution is to ignore it in the examples. I happened to have the problem because I copied the example code (usbcfg.c), which did "something" on suspend which I didn't think about until it came and bit me. Maybe the example code should simply do nothing.

Proper USB suspend handling would have to do much more than just going into a logical-off state: First and foremost, the device shall go into low-power consumption, if it is bus-powered. My (hobbyist) devices don't do this - that's because I'm too lazy to think about a "low power mode", which is probably not what I want the device to have in the first place. I could write my own host drivers that hold the device active (or configure Linux' USB-CDC-ACM driver to not put the device into suspend, i.e. deactivate auto-suspend). But for reasons of convenience I prefer to ignore the problem. Fortunately, my host hardware does accept this (device continuing to consume bus power).

So, overall, I'm still unsure about how to handle this. In the end, each application must decide on how to handle USB suspend. If it's for a device undergoing certification, there won't be a way to weasle out of implementing a real low-power state with bells and whistles (or rather, without). Otherwise, I think there's nothing to gain with the call to usbDisable() upon suspend event (in usbcfg.c in the demo projects).

[Giovanni]

I think that "proper" handling would require:

On the USB driver side:

1) Abort of current transactions (RX and TX), this is missing, I am not even sure if it is even possible on OTG peripheral right now.
2) Informing the application through callbacks for suspend/wakeup.

On the Serial-USB side:

1) Reset of queues like it is now, the driver would make sure there would not be spurious callbacks after the suspend callback.
2) Restarting of the RX transaction on wakeup, this would require a new API sduReconnectI().

Giovanni

[Hawhill]

Just fyi: The suggested quick fix does indeed work - as expected. I just tested on a F407.

[Giovanni]

Hi,

Good to know, it confirms everything, I still have to decide the proper way to fix it.

Giovanni

[Giovanni]

Hi,

I committed a tentative fix, I believe this is the right way.

1) Now buffer queues have a "suspended" state that makes read and write operations non-blocking and return MSG_RESET (undecided if it should be MSG_TIMEOUT instead).
2) The Serial-USB driver no more resets buffer queues but puts them in the new suspended state, this way current transactions should be able to complete and the application would not be stuck in read or write.
3) On wakeup buffer queues are placed again in normal state.

Note that this required extensive changes to Serial-USB and buffer queues, you need to update usbcfg.c in your applications. I did it for all demos in the ChibiOS tree but not those in community repository, maintainers please take note.

I would appreciate testing and feedback on the change, thanks.

Giovanni

[Giovanni]

Ping.

Any comment?

Giovanni