zephyr: ESP32-S3/C3 Fatal Exception output not sent when using USB Serial (ACM) Mode

Thanks for being so helpful!

To avoid all changes, you will need to create a new branch off of main and then cherry-pick your fix on top and push that. At the command line, it will look like this:

# here are the remotes.  "mygithub" is your personal github branch of the Zephyr repo.  "upstream" is the actual Zephyr repo.
git remote -vv show
mygithub	https://github.com/YOUR_GITHUB_ACCOUNT/zephyr.git (fetch)
mygithub	https://github.com/YOUR_GITHUB_ACCOUNT/zephyr.git (push)
upstream	https://github.com/zephyrproject-rtos/zephyr (fetch)
upstream	https://github.com/zephyrproject-rtos/zephyr (push)

# create a new branch based upon the latest zephyr main
git fetch upstream
git checkout main
git rebase upstream/main
git checkout -b fixes-60825-usb-fatal-exception-output

# cherry-pick your development change onto this branch
git cherry-pick HASH_OF_YOU_CHANGE_FROM_YOUR_DEV_BRANCH

# push the change to your github fork
git push mygithub

# if you make edits, then do a forced push
git push mygithub -f

There are different ways you can do this that are a little less verbose, but this one is easy to follow.

You will eventually also want to run checkpatch on more complex changes (see https://docs.zephyrproject.org/latest/contribute/guidelines.html#coding-style). You can probably skip that for this one until you get the hang of the rest of the process. I have mine setup to run it before pushing FWIW.

Great! @havoc-dlt, can you open a PR?

Well, I’ve never done one before. Consider me an absolute newbie in github collaboration… 😕 I’ll read up on it and give it a try.

I tried both versions. @havoc-dlt’s change works. For some reason @sylvioalves’s version is printing garbage – looks like some characters are getting dropped:

uart:~$ debug stack_panic 
[3> [r>V 0L[Id ia3i[4> xe9[>aa
24 )[>0[>C>500m07>0>00c

@havoc-dlt, Can you check whether the below also fix it? I haven’t tested this yet.

while (!usb_serial_jtag_ll_txfifo_writable() &&
		s_usbserial_timeout < (USBSERIAL_TIMEOUT_MAX_US / 100) &&
		!k_is_in_isr()) {

For convenience, here’s my patch for this change:

From 2e894e7d3f9ee4ec923ed6f23e13468f16b0fa6f Mon Sep 17 00:00:00 2001
From: "havoc-dlt" <havoc.dlt@gmail.com>
Date: Wed, 16 Aug 2023 09:49:12 +0200
Subject: [PATCH] esp32: fix fatal exception output in usb serial mode

---
 drivers/serial/serial_esp32_usb.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/serial/serial_esp32_usb.c b/drivers/serial/serial_esp32_usb.c
index 4a2fe24324..714034f1d6 100644
--- a/drivers/serial/serial_esp32_usb.c
+++ b/drivers/serial/serial_esp32_usb.c
@@ -68,10 +68,14 @@ static void serial_esp32_usb_poll_out(const struct device *dev, unsigned char c)
 	ARG_UNUSED(dev);
 
 	/* Wait for space in FIFO */
+	const int64_t timeout =
+	k_uptime_ticks() +
+	(int64_t)k_us_to_ticks_ceil64(USBSERIAL_TIMEOUT_MAX_US);
 	while (!usb_serial_jtag_ll_txfifo_writable() &&
-		s_usbserial_timeout < (USBSERIAL_TIMEOUT_MAX_US / 100)) {
-		k_usleep(100);
-		s_usbserial_timeout++;
+		k_uptime_ticks() < timeout) {
+		if (!k_is_in_isr()) {
+			k_usleep(100);
+		}
 	}
 
 	if (usb_serial_jtag_ll_txfifo_writable()) {
-- 
2.41.0

I might have some insight on this if nobody else already figured it out. In drivers/serial/serial_esp32_usb.c, function “serial_esp32_usb_poll_out”, there is a k_usleep call that gets blocked by an __ASSERT when this function gets called from an interrupt.

/* Wait for space in FIFO */
while (!usb_serial_jtag_ll_txfifo_writable() &&
		s_usbserial_timeout < (USBSERIAL_TIMEOUT_MAX_US / 100)) {
        k_usleep(100);
	s_usbserial_timeout++;
}

My workaround is this for now, which is ugly, because more power hungry obviously, but we’re trying to push out everything after a crash:

/* Wait for space in FIFO */
const int64_t timeout =
        k_uptime_ticks() +
        (int64_t)k_us_to_ticks_ceil64(USBSERIAL_TIMEOUT_MAX_US);
while (!usb_serial_jtag_ll_txfifo_writable() &&
                k_uptime_ticks() < timeout) {
        if (!k_is_in_isr()) {
            k_usleep(100);
        }
}

Seems to work for me at least.

@EricNRS, can you provide your shell sample code so I can reproduce the exactly same behavior?