Config option to disable target reset on serial break

[yandld]

Hi DAPLINK developers:

I am Alex from NXP.

In file: https://github.com/ARMmbed/DAPLink/blob/master/source/daplink/usb2uart/usbd_user_cdc_acm.c

I noticed that USB_CDC_ACM_SendBreak Implementation may not correct, it should not be reset target MCU:

static U32 start_break_time = 0;
int32_t USBD_CDC_ACM_SendBreak(uint16_t dur)
{
    uint32_t end_break_time;

    // reset and send the unique id over CDC
    if (dur != 0) {
        start_break_time = os_time_get();
        target_set_state(RESET_HOLD);
    } else {
        end_break_time = os_time_get();

        // long reset -> send uID over serial (300 -> break > 3s)
        if ((end_break_time - start_break_time) >= (300)) {
            main_reset_target(1);
        } else {
            main_reset_target(0);
        }
    }

    return (1);
}

see: https://man7.org/linux/man-pages/man3/tcsendbreak.3p.html

[0xc0170]

As I recall, daplink uses serial break to reset a target. We use it also with Mbed OS testing infrastructure (how to reset a target remotely).

[0xc0170]

@yandld Is this a bug or a feature? I belive the latter. Can you confirm ?

[yandld]

I believe it's a bug, see https://man7.org/linux/man-pages/man3/tcsendbreak.3p.html there shouldn't be any "reset target" behavior when received a SEND_BREAK signal.

[flit]

@yandld Unfortunately this feature is relied upon by the Mbed OS test infrastructure, so removing it isn't really possible without breaking things. (The specific issue is that the Mbed test infrastructure accesses test boards remotely over the network, but it only serves the serial port for each board.)

However, I agree that it causes issues. In particular, macOS likes to send a break when a callout tty is closed. Having this result in a target reset caused a ton of frustration for me when I was building the Freescale Kinetis boot ROM test suite.

So, I would like to address this as best as possible by adding a configuration option to disable the reset-on-break "feature". We would still have to turn it on by default due to Mbed testing requirements, but at least you could turn it off if desired.

Another option would be to try to find some other way to request a target reset across serial. But this is unlikely. Let me know if you have ideas, though!

[flit]

Btw, the tcsendbreak() manpage doesn't say you can't handle a serial break by resetting the target. It simply states when happens on the host when you call that function (e.g., sending a sequence of 0-valued bits).

---

[Later] Rethinking this a bit… If you think of DAPLink as a transparent interface for the serial port, then yes, performing a target reset instead of transmitting the serial break to the target is indeed a problem. The point is that the host is not talking to DAPLink, it's talking to the target. The fact that DAPLink is present is simply an artifact of the implementation.

Anyways, as noted, we can't change the current behaviour without breaking behaviour that is depended upon by Mbed, as well as several partners. But I'm very much in favour of adding a config option (in fact, I have had it on my todo list for DAPLink for quite a while 😄).

[cederom]

Hello world :-)

@yandld this is a FEATURE of DAPLink that allows your Target board to be reset remotely over CDC endpoint using this particular break command. This is not a part of CDC standard but a feature implemented in DAPLink. This way you can perform automation tests and development on a remote board with no need to touch it. Very useful feature and it should stay untouched unless there are really important reasons for changing that. Please note that RESET can be both Soft (DAP) and Hard (reset line) and both are probably implemented so cutting the reset line may not be enough to block reset with break.

Is there any reason you want to remove that feature from DAPLink? Do you use break command in your Target's firmware for any other task? Does this interfere with anything you are working on? Does it have a negative impact of any sort?

[cederom]

(..) However, I agree that it causes issues. In particular, macOS likes to send a break when a callout tty is closed. Having this result in a target reset caused a ton of frustration for me when I was building the Freescale Kinetis boot ROM test suite. (..)

Have you tried minicom @flit ? It has "exit and reset" as well as "quit with no reset". I am sure this break on tty close can be configured somehow? Folks are even using screen to connect to serial console port on macOS :-)

[flit]

@cederom This case was a CI test script calling out to NXP's blhost ROM bootloader tool, not interactive. blhost is written in C/C++ using the tty ioctl interface. It seemed to be the OS itself that was sending the breaks; I certainly didn't do anything to send a break when I called close()!

(Fwiw, I use tio for interactive serial usage.)

[pfalcon]

Unfortunately this feature is relied upon by the Mbed OS test infrastructure, so removing it isn't really possible without breaking things.

+1. And not only the Mbed OS test infrastructure, we for example rely on it in https://www.lavasoftware.org/ . Actually, how I arrived here is that there's no UART BREAK support in a recent NXP LPC55S69-EVK board, with a firmware which otherwise claims to implement CMSIS-DAP (but e.g. apparently doesn't implemented the MSD feature). So, we're trying to decide whether that should be considered a bug in the LPC55S69-EVK firmware, which we should report to NXP, and reasonably expect it to be fixed, or we're on our own and will need to work around it for this particular board (which will be quite cumbersome, comparing to the situation that everything just works using UART BREAK with many CMSIS-DAP boards out there).

In that regard, @flit, maybe you know if this BREAK handling is the part of "spec" of CMSIS-DAP? If not, is it explicitly documented for the DAPLink implementation? Because I don't remember reading about it, and searching here on github, this ticket seems to be the most explicit discussion of the matter. If it's not documented, that maybe it's worth to do that?

Thanks.

[pfalcon]

(Oh, and I fully support the idea to make it configurable, but default should rather stay "BREAK -> target reset" ;-) )

[mathias-arm]

I am marking this as an idea for enhancement to make this behavior configurable.