Skip to content

UefiPayloadPlg: Configure serial based on Hob Data#10769

Closed
Sean-StarLabs wants to merge 3 commits intotianocore:masterfrom
StarLabsLtd:wip/sean/debug
Closed

UefiPayloadPlg: Configure serial based on Hob Data#10769
Sean-StarLabs wants to merge 3 commits intotianocore:masterfrom
StarLabsLtd:wip/sean/debug

Conversation

@Sean-StarLabs
Copy link
Contributor

No description provided.

Fixes serial output on platforms using coreboot and a non-default clock rate
such as AMD Picasso and newer.

Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Change-Id: I91290397852176754e9a34ec6e5829044f41d15a
…ialInfo

Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Change-Id: If764bd7c0b691cf887205471d0343fdf62372141
…AL_PAYLOAD_SERIAL_PORT_INFO

Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Change-Id: I9bcaf03ab63f6a45d2cf25a580f7a2eba388cbbd
Copy link
Contributor

@gguo11837463 gguo11837463 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check my comment, once it got gixed I will give you code review + 1.

BOOLEAN UseMmio;
UINT8 RegisterStride;
UINT32 BaudRate;
UINT32 ClockRate;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up backward compatible for structure, I want to recommend you put this code from line 22 to line 24

return Status;
}

Status = PcdSet32S (PcdSerialClockRate, SerialPortInfo->ClockRate);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add backward compatible check here.
if (SerialPortInfo->ClockRate != 0) {
Status = PcdSet32S (PcdSerialClockRate, SerialPortInfo->ClockRate);
}

UniversalSerialPort->UseMmio = (SerialPortInfo.Type == 1) ? FALSE : TRUE;
UniversalSerialPort->RegisterBase = SerialPortInfo.BaseAddr;
UniversalSerialPort->BaudRate = SerialPortInfo.Baud;
UniversalSerialPort->ClockRate = SerialPortInfo.InputHertz;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this has been handled by commit (#6215), could you check if existing implementation is good enough?

Some background:
We prefer to not touch UniversalPayload/SerialPortInfo.h as that will require USF->UPL spec update which is unnecessary effort if we could eliminate.(https://universalscalablefirmware.github.io/documentation/2_universal_payload.html#serial-information)

The PcdSerialClockRate should already have the value you need in line438 without relying on new field in UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO structure.

@github-actions
Copy link

This PR has been automatically marked as stale because it has not had activity in 60 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added the stale Due to lack of updates, this item is pending deletion. label Apr 20, 2025
@mergify
Copy link

mergify bot commented Apr 20, 2025

PR can not be merged due to conflict. Please rebase and resubmit

@github-actions github-actions bot removed the stale Due to lack of updates, this item is pending deletion. label Apr 21, 2025
@github-actions
Copy link

This PR has been automatically marked as stale because it has not had activity in 60 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added the stale Due to lack of updates, this item is pending deletion. label Jun 20, 2025
@github-actions
Copy link

This pull request has been automatically been closed because it did not have any activity in 60 days and no follow up within 7 days after being marked stale. Thank you for your contributions.

@github-actions github-actions bot closed this Jun 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale Due to lack of updates, this item is pending deletion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants