Page MenuHomeFreeBSD

Add support of UART_DEV_DBGPORT to uart_bus_fdt.c
ClosedPublic

Authored by dsl_mcusim.org on Nov 30 2020, 3:18 PM.

Details

Summary
NOTE: This patch is a re-worked copy of an abandoned one attached to the review D5986.

Support of UART_DEV_DBGPORT in addition to UART_DEV_CONSOLE added to
uart_bus_fdt.c where "hw.fdt.debug" can override the FDT settings.
The following properties are looked at in /chosen for the debug node:
freebsd,debug-path.

Test Plan

I tested it on BeagleBone Black with kgdb connected, but I also have BeagleBone (white)
and RPi2 model B to check this patch if necessary.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Hi! Thanks for updating this change, I was actually looking just last week at why this isn't supported.

sys/dev/uart/uart_bus_fdt.c
182 ↗(On Diff #80151)

propnames_dbg seems preferable to me since this is a list.

193 ↗(On Diff #80151)

Perhaps this was already discussed in the old review, but hw.fdt.debug seems vague to me. Maybe hw.fdt.debug_console or hw.fdt.dbgport?

196 ↗(On Diff #80151)

Although it should not be an issue in practice, UART_DEV_CONSOLE should be an explicit case with default being a catch-all returning ENXIO.

sys/dev/uart/uart_cpu_arm64.c
104 ↗(On Diff #80151)

You will need to allow UART_DEV_DBGPORT here as well or it will fail to attach.

sys/dev/uart/uart_cpu_fdt.c
91 ↗(On Diff #80151)

I suspect you don't need the line break. FreeBSD's style(9) allows lines to be 80 characters in length.

I've tried to follow all of the comments during the review.

Please, take a look at the updated patch.

sys/dev/uart/uart_bus_fdt.c
182 ↗(On Diff #80151)

I've re-named it as propnames_dbgport in order to align with hw.fdt.dbgport.

sys/dev/uart/uart_cpu_arm64.c
104 ↗(On Diff #80151)

It should be removed with uart_cpu_fdt_probe checking for console types it supports.

sys/dev/uart/uart_cpu_fdt.c
90 ↗(On Diff #80192)

You don't need the extra parentheses

dsl_mcusim.org marked an inline comment as done.

Unnecessary tests of the devtype argument have been removed from uart_cpu_getdev() in sys/dev/uart/uart_cpu_arm64.c and sys/dev/uart/uart_cpu_fdt.c. uart_cpu_fdt_probe() checks for console types it supports.

This revision is now accepted and ready to land.Dec 2 2020, 2:31 PM

@mhorne Should I update a patch attached to the PR also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=251053 ?

Linking the review in the PR is enough. We should wait a little longer for any further comments, and if there are none I can commit the change.

Edit: I missed andrew's acceptance. I can commit it shortly.

This revision was automatically updated to reflect the committed changes.