Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpipico: configurable ttys #1063

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

veremenko-y
Copy link
Contributor

@veremenko-y veremenko-y commented Apr 15, 2024

Fixes: #1062

Because not everybody is able to connect UART as boot console for Pi Pico, I added the ability to reconfigure the order of tty's.

Amount of ifdefs has been growing a lot, so I added small mktty tool that parses config and generates devttyconf.c and devttyconf.h that maps devices appropriately. Format is very simple:

# Format: tty_dev tty_dev_number
usb 1
uart 1
usb 2

For now there's still a problem of USB ACM not connecting fast enough to get all boot messages printed. I could make it so it waits for any key before boot, but then unattended boot is broken. Should I add it as another option as well?

Version 2:

Stage 1:
On startup devtty_init will wait for USB host connection, and if present before timeout expires, it will configure USB consoles first, UART second. So boot messages will be printed using detected device.

Stage 2:
At root FS mount plt_param will get executed either from CMDLINE or from MBR.
Boot arg in the format tty=[usb|uart]N,... will be parsed and tty's will get reconfigured once again.

For example, given tty=uart1,usb1,usb2 the following mapping will be configured

  • /dev/tty1 - uart1
  • /dev/tty2 - usb1
  • /dev/tty3 - usb2

@EtchedPixels EtchedPixels marked this pull request as ready for review April 15, 2024 21:39
@EtchedPixels
Copy link
Owner

That does create yet more things for the user to get wrong, and makes it ever harder to have a build that someone can just try.

It's been easier on the other ports that do this as they can probe hardware or you end up typing stuff at the boot: prompt (so you could print to several and declare whoever you type at console).

Is it reasonable to wait a couple of seconds at boot and see if there is a USB connect and if not boot with serial as the console ?

@veremenko-y
Copy link
Contributor Author

Is it reasonable to wait a couple of seconds at boot and see if there is a USB connect and if not boot with serial as the console ?

It is reasonable.
Unless you tell me to kill it, I'd like to keep the configuration of ttys. But I could instead set either default config that does detect usb1 fallback uart1 or use plt_param with default tty=detect or something.
So in this case default build will pick up whatever active device is, but one can still configure whatever tty's (I'm also planning to add second hardware UART, that's why I hope you let mkconf stay)

@EtchedPixels
Copy link
Owner

plt_param is probably the best option. You can also then set the parameters you want to indicate if there is a usb console and how many tty ports are present. See "setboot".

I really don't like the make config stuff, it's just guaranteeing a load more bug/queries from everyone trying to figure out yet another layer of indirection on configuring the device.

So my preference is strongly

  • give USB a couple of seconds to connect, if it does it's the console
  • if not assume serial

Let the user specify the number of ports on the command line (or with setboot).

So you'd then boot with USB, log in "setboot whatever" and it's done and consistent with the other ports.

@veremenko-y
Copy link
Contributor Author

I really don't like the make config stuff

You're the boss :) So current suggestion is:

  • By default and if #define CMDLINE NULL, wait DEV_USB_DETECT_TIMEOUT seconds, check if USB host is connected. If it is, put all USB devices first then UART devices into ttymap, otherwise the do the opposite and continue booting.
  • Alternatively parse boot parameter like this tty=uart1,usb1,usb2 which will fill in ttymap, and will not check for USB host
  • kputc will use UART before the devtty is initialized, in case something panics.

I just took a look at the code once more, and plt_param() is executed after device_init() and right before init is executed.

So I would have to initialize devtty with defaults as I described above, then do it again inside plt_param() which I hoped to avoid. And I don't want to change start.c without discussing it first.

I will prototype it the way I described it, but it'll likely need to change based on your response.

@veremenko-y veremenko-y changed the title rpipico: configurable ttys WIP: rpipico: configurable ttys Apr 17, 2024
@veremenko-y
Copy link
Contributor Author

I pushed the prototype for you to take a look at.

It's not fully functional yet, I still have few issues with USB console.

@veremenko-y
Copy link
Contributor Author

Sorry for the delay, it does seem to work fine with default boot, but there are some issues with locking when changing the parameters in cmdline, and I've seen few Hard Faults running the system. I have a very busy week, but I hope to finish it the following week.

@veremenko-y
Copy link
Contributor Author

Who knew that using 5v SD card reader can lead to issues...

I think it's working now.

@veremenko-y
Copy link
Contributor Author

@EtchedPixels Sorry for a ping, doing it just in case if you missed the update. If not, I'm sorry, I wont' do it again :)

FYI something broke on pico platform when you updated p_tab struct, I get strange memory corruptions. I'm still investigating what it could be... I bisected it to 7fd6add

@veremenko-y veremenko-y changed the title WIP: rpipico: configurable ttys rpipico: configurable ttys May 28, 2024
@EtchedPixels
Copy link
Owner

Weird.. I don't see anything else assuming the size and I imagine you did the obvious things like build from clean ?

@veremenko-y
Copy link
Contributor Author

veremenko-y commented May 28, 2024

Weird.. I don't see anything else assuming the size

Yeah, I'm also not sure what's going on. But it looks like some memory corruption.

and I imagine you did the obvious things like build from clean ?

I did. And commit prior to that one works, but not after.
I'm still struggling a bit with SWD debugger to properly track down where it breaks exactly. I'm going to try to use another debugger from an old STM Discovery board, and see if that works better.

@veremenko-y
Copy link
Contributor Author

Now that I'm thinking about it. I did run make clean. But I should try to purge the entire git repo from any possible changes and try again.

@veremenko-y
Copy link
Contributor Author

But outside of the segfault issue, do you have any feedback for this PR specifically?

@EtchedPixels
Copy link
Owner

Looks good to me other than pinning down the fault problem (which is probably unrelated anyway)

@EtchedPixels EtchedPixels merged commit 238d9b5 into EtchedPixels:master Jun 7, 2024
13 checks passed
@EtchedPixels
Copy link
Owner

Thanks

@veremenko-y veremenko-y deleted the pico_ttyconfig branch October 16, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants