Page MenuHomeFreeBSD

dpaa2: add console support for FDT based systems
ClosedPublic

Authored by bz on Feb 14 2023, 3:58 PM.
Tags
None
Referenced Files
F102790879: D38592.id.diff
Sun, Nov 17, 6:10 AM
Unknown Object (File)
Sun, Nov 10, 9:35 PM
Unknown Object (File)
Oct 17 2024, 3:31 AM
Unknown Object (File)
Oct 2 2024, 9:07 PM
Unknown Object (File)
Sep 29 2024, 7:24 PM
Unknown Object (File)
Sep 29 2024, 5:40 PM
Unknown Object (File)
Sep 21 2024, 11:52 PM
Unknown Object (File)
Sep 21 2024, 5:26 PM

Details

Summary

Add DPAA2 console support for MC and AIOP (latter untested) for FDT
systems. ACPI systems are prepared but need some proper bus function
in order to get the address from MC. This will come at a later stage
once other ACPI/FDT bus parts are cleared up.
The work was originally done in July 2022 and finally switched to
bus_space[1].

Suggested by: andrew [1]

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bz requested review of this revision.Feb 14 2023, 3:58 PM

I believe there is an issue due to last minute changes which no longer read the full message buffer (uiomove switched to uiomove_buffer, ... ) I'll have t track that down tonight. cat /dev/fsl_,,, does not report an error though but I only get the first round of 128 byte buffer back now.

Switch back from uiomove_frombuf() to uiomove() as the former has
a silly check which will silently return 0 on repeated calls
(buflen <= uio->uio_offset) which after moving the first 128
bytes out will then fail on the 2nd round with new buffer contents.

sys/dev/dpaa2/dpaa2_console.c
67

Shouldn't MC_REG_MCFBA be taken into account, i.e. MC_REG_MCFBA + 0x00?

EDIT: This above was my first though when I saw the code for the first time. I didn't want to believe that memory region described in FDT consists an offset already, but here it is (from fsl-ls1088a.dtsi):

		console@8340020 {
			compatible = "fsl,dpaa2-console";
			reg = <0x00000000 0x08340020 0 0x2>;
		};
...
		fsl_mc: fsl-mc@80c000000 {
			compatible = "fsl,qoriq-mc";
			reg = <0x00000008 0x0c000000 0 0x40>,	 /* MC portal base */
			      <0x00000000 0x08340000 0 0x40000>; /* MC control reg */
...
69

Same about MC_REG_MCFBA here.

EDIT: Please, ignore it due to the reason above.

sys/dev/dpaa2/dpaa2_console.c
73

@bz MC_BUFFER_OFFSET name is misleading because it can be confused with the offset to the log buffer, i.e. hdr.start which is 0x01400000 according to DPAA2UM. Where did you get this offset from? Same for the MC_BUFFER_SIZE.

Overall changes look good to me.

The only feature I'd add is to print log buffer's content during boot to debug issues as early as possible. I'm having troubles booting HoneyComb with FDT at the moment as it hangs at mounting NFS root and resets the board.

bz marked 2 inline comments as done.Apr 14 2023, 10:00 AM
bz added inline comments.
sys/dev/dpaa2/dpaa2_console.c
73

I wonder if they came from drivers/soc/fsl/dpaa2-console.c ; it's almost a year; I should probably add the NXP Copyrights to this file as well and add a note to the references above.

  • Add NXP copyright and license given some parts likely originated from the dual-licensed linux implementation
  • Cleanup some no longer needed includes and code given we switched to bus_space functions
bz marked an inline comment as done.Apr 15 2023, 12:25 PM

@imp is it okay to add the upstream SPDX license/copyright like this?

sys/dev/dpaa2/dpaa2_console.c
73

I'd keep that name in order to keep it aligned with the upstream sources.

In D38592#899665, @dsl wrote:

May we have it committed?

@dsl Can you do a final review and if all good from your DPAA2 perspective approve?

@dsl any further comments or should I push this?

This revision is now accepted and ready to land.Apr 20 2023, 3:37 PM

No further comments. I didn't realize I can accept it :)

This revision was automatically updated to reflect the committed changes.