Page MenuHomeFreeBSD

Add support of UART_DEV_DBGPORT to uart_cpu_fdt.c
AbandonedPublic

Authored by stevek on Apr 18 2016, 4:38 PM.

Details

Reviewers
None
Group Reviewers
ARM
Summary

Suppport UART_DEV_DBGPORT in addition to UART_DEV_CONSOLE in uart_cpu_fdt.c

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

Build and boot on ARMv7 board (in this case this was done with the BCM56340-based board that I am working with) with debug port specified in the FDT.
Attach gdb to the remote debug port.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

stevek retitled this revision from to Add support of UART_DEV_DBGPORT to uart_cpu_fdt.c.
stevek updated this object.
stevek edited the test plan for this revision. (Show Details)
stevek set the repository for this revision to rS FreeBSD src repository - subversion.
jhibbits added a subscriber: jhibbits.

An active ARM developer should review this. Looks good to me, though.

I've discussed this with one of the Linux ARM developers. He suggested the name should be freebsd,debug-path to ensure there is no conflict with Linux.

I've discussed this with one of the Linux ARM developers. He suggested the name should be freebsd,debug-path to ensure there is no conflict with Linux.

In general, anything we invent should start with freebsd, while anything standardized should not.

In D5986#129972, @imp wrote:

I've discussed this with one of the Linux ARM developers. He suggested the name should be freebsd,debug-path to ensure there is no conflict with Linux.

In general, anything we invent should start with freebsd, while anything standardized should not.

If it makes sense to standardise it it would pay to raise the issue on devicetree@vger.kernel.org.

What is the advantage of adding this to the dts over just the kenv variable?

What is the advantage of adding this to the dts over just the kenv variable?

One case that it's been useful for us at Juniper is when we boot up what we call a "bringup kernel", where the FDT is static and a rootfs is embedded in the kernel. The loader (whatever it is, it could be something other that ubldr, loader, or u-boot) doesn't need to be able to pass environment on to the kernel that it can understand and the remote debugger port can "just work" (assuming the UART is set up correctly and the system can get booted far enough.)

Looks good to me.

sys/dev/uart/uart_cpu_fdt.c
132

most properly, this should be freebsd,debug-path since debug-path isn't a standard thing.

Updated per review comments.

It looks like recent changes to uart would require a re-work of this change, abandoning this review for now.

I've re-worked this patch and opened a PR at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=251053.

Would you be willing to create a new phabricator review for the change?

Would you be willing to create a new phabricator review for the change?

Of course :) But I'll have to understand how to do it properly first.

Would you be willing to create a new phabricator review for the change?

Of course :) But I'll have to understand how to do it properly first.

There's some information here if you haven't found it already: https://wiki.freebsd.org/Phabricator

There's some information here if you haven't found it already: https://wiki.freebsd.org/Phabricator

No, I haven't seen it yet - thanks for sharing! What's a preferred way to create a revision: via command line or web interface?

There's some information here if you haven't found it already: https://wiki.freebsd.org/Phabricator

No, I haven't seen it yet - thanks for sharing! What's a preferred way to create a revision: via command line or web interface?

It's up to you. If you upload a lot of patches I find it easier to use the arc command-line tool so you can upload git commits. If you upload a patch via the web interface, please ensure to generate a patch with lots of context (-U99999).