* [edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011
2024-01-10 23:52 [edk2-devel] [PATCH 0/5] Platform/RaspberryPi: Various minor fixes Jeremy Linton
@ 2024-01-10 23:52 ` Jeremy Linton
2024-01-11 7:46 ` Ard Biesheuvel
2024-01-10 23:52 ` [edk2-devel] [PATCH 2/5] Silicon/Broadcom/BcmGenetDxe: Suppress some bogus compiler warnings Jeremy Linton
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2024-01-10 23:52 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore, quic_llindhol, Jeremy Linton
The rpi's config.txt controls which uart (pl011, or miniuart) is
selected as the console. TFA and edk2 follow its lead, but if the
miniuart is selected as the primary and the machine is booted in ACPI
mode the baud/etc is never configured for the pl011. The linux kernel
won't reconfigure it either as its listed as a "SBSA" uart, so it
simply won't work.
This re-enables BT on the pl011 in ACPI mode, and it somewhat starts
to work again.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
.../DualSerialPortLib/DualSerialPortLib.c | 37 +++++++++++--------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index d2f983bf0a..79545d93d6 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -76,6 +76,8 @@ SerialPortInitialize (
EFI_PARITY_TYPE Parity;
UINT8 DataBits;
EFI_STOP_BITS_TYPE StopBits;
+ RETURN_STATUS Ret;
+ UINTN Timeout;
//
// First thing we need to do is determine which of PL011 or miniUART is selected
@@ -85,23 +87,27 @@ SerialPortInitialize (
UsePl011UartSet = TRUE;
}
- if (UsePl011Uart) {
- BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
+ // always init the pl011 on the pi4, linux expects a SBSA uart to be at 115200
+ // this means we need to set the baud/etc even if we arn't using it as a console
+ if ((UsePl011Uart) || (RPI_MODEL == 4)) {
ReceiveFifoDepth = 0; // Use default FIFO depth
+ BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
- return PL011UartInitializePort (
- PL011_UART_REGISTER_BASE,
- PL011UartClockGetFreq(),
- &BaudRate,
- &ReceiveFifoDepth,
- &Parity,
- &DataBits,
- &StopBits
- );
- } else {
+ Ret = PL011UartInitializePort (
+ PL011_UART_REGISTER_BASE,
+ PL011UartClockGetFreq(),
+ &BaudRate,
+ &ReceiveFifoDepth,
+ &Parity,
+ &DataBits,
+ &StopBits
+ );
+ }
+
+ if (!UsePl011Uart) {
SerialRegisterBase = MINI_UART_REGISTER_BASE;
Divisor = SerialPortGetDivisor (PcdGet32 (PcdSerialBaudRate));
@@ -127,7 +133,8 @@ SerialPortInitialize (
// Wait for the serial port to be ready.
// Verify that both the transmit FIFO and the shift register are empty.
//
- while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
+ Timeout = 1000;
+ while (((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) && (Timeout--));
//
// Configure baud rate
@@ -158,9 +165,9 @@ SerialPortInitialize (
// Put Modem Control Register(MCR) into its reset state of 0x00.
//
SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
-
- return RETURN_SUCCESS;
+ Ret = RETURN_SUCCESS;
}
+ return Ret;
}
/**
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113544): https://edk2.groups.io/g/devel/message/113544
Mute This Topic: https://groups.io/mt/103652853/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011
2024-01-10 23:52 ` [edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011 Jeremy Linton
@ 2024-01-11 7:46 ` Ard Biesheuvel
2024-01-11 15:02 ` Jeremy Linton
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2024-01-11 7:46 UTC (permalink / raw)
To: Jeremy Linton; +Cc: devel, ardb+tianocore, quic_llindhol
Hi Jeremy,
Thanks for the patches.
On Thu, 11 Jan 2024 at 00:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> The rpi's config.txt controls which uart (pl011, or miniuart) is
> selected as the console. TFA and edk2 follow its lead, but if the
> miniuart is selected as the primary and the machine is booted in ACPI
> mode the baud/etc is never configured for the pl011. The linux kernel
> won't reconfigure it either as its listed as a "SBSA" uart, so it
> simply won't work.
>
> This re-enables BT on the pl011 in ACPI mode, and it somewhat starts
> to work again.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> .../DualSerialPortLib/DualSerialPortLib.c | 37 +++++++++++--------
> 1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> index d2f983bf0a..79545d93d6 100644
> --- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> @@ -76,6 +76,8 @@ SerialPortInitialize (
> EFI_PARITY_TYPE Parity;
> UINT8 DataBits;
> EFI_STOP_BITS_TYPE StopBits;
> + RETURN_STATUS Ret;
> + UINTN Timeout;
What is this for?
>
> //
> // First thing we need to do is determine which of PL011 or miniUART is selected
> @@ -85,23 +87,27 @@ SerialPortInitialize (
> UsePl011UartSet = TRUE;
> }
>
> - if (UsePl011Uart) {
> - BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
> + // always init the pl011 on the pi4, linux expects a SBSA uart to be at 115200
> + // this means we need to set the baud/etc even if we arn't using it as a console
> + if ((UsePl011Uart) || (RPI_MODEL == 4)) {
> ReceiveFifoDepth = 0; // Use default FIFO depth
> + BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
Shouldn't we hardcode 115200 here if !UsePl011Uart?
> Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
> DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
> StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
>
> - return PL011UartInitializePort (
> - PL011_UART_REGISTER_BASE,
> - PL011UartClockGetFreq(),
> - &BaudRate,
> - &ReceiveFifoDepth,
> - &Parity,
> - &DataBits,
> - &StopBits
> - );
> - } else {
> + Ret = PL011UartInitializePort (
> + PL011_UART_REGISTER_BASE,
> + PL011UartClockGetFreq(),
> + &BaudRate,
> + &ReceiveFifoDepth,
> + &Parity,
> + &DataBits,
> + &StopBits
> + );
> + }
> +
> + if (!UsePl011Uart) {
> SerialRegisterBase = MINI_UART_REGISTER_BASE;
> Divisor = SerialPortGetDivisor (PcdGet32 (PcdSerialBaudRate));
>
> @@ -127,7 +133,8 @@ SerialPortInitialize (
> // Wait for the serial port to be ready.
> // Verify that both the transmit FIFO and the shift register are empty.
> //
> - while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
> + Timeout = 1000;
> + while (((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) && (Timeout--));
>
> //
> // Configure baud rate
> @@ -158,9 +165,9 @@ SerialPortInitialize (
> // Put Modem Control Register(MCR) into its reset state of 0x00.
> //
> SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
> -
> - return RETURN_SUCCESS;
> + Ret = RETURN_SUCCESS;
> }
> + return Ret;
> }
>
> /**
> --
> 2.43.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113589): https://edk2.groups.io/g/devel/message/113589
Mute This Topic: https://groups.io/mt/103652853/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011
2024-01-11 7:46 ` Ard Biesheuvel
@ 2024-01-11 15:02 ` Jeremy Linton
0 siblings, 0 replies; 10+ messages in thread
From: Jeremy Linton @ 2024-01-11 15:02 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel, ardb+tianocore, quic_llindhol
Hi,
On 1/11/24 01:46, Ard Biesheuvel wrote:
> Hi Jeremy,
>
> Thanks for the patches.
Thanks for looking at them!
>
>
> On Thu, 11 Jan 2024 at 00:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>
>> The rpi's config.txt controls which uart (pl011, or miniuart) is
>> selected as the console. TFA and edk2 follow its lead, but if the
>> miniuart is selected as the primary and the machine is booted in ACPI
>> mode the baud/etc is never configured for the pl011. The linux kernel
>> won't reconfigure it either as its listed as a "SBSA" uart, so it
>> simply won't work.
>>
>> This re-enables BT on the pl011 in ACPI mode, and it somewhat starts
>> to work again.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> .../DualSerialPortLib/DualSerialPortLib.c | 37 +++++++++++--------
>> 1 file changed, 22 insertions(+), 15 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
>> index d2f983bf0a..79545d93d6 100644
>> --- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
>> +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
>> @@ -76,6 +76,8 @@ SerialPortInitialize (
>> EFI_PARITY_TYPE Parity;
>> UINT8 DataBits;
>> EFI_STOP_BITS_TYPE StopBits;
>> + RETURN_STATUS Ret;
>> + UINTN Timeout;
>
> What is this for?
IIRC, its as you see, being used below to avoid an infinite loop when
the miniuart doesn't become ready. That shouldn't be possible with this
set, so its a safety change.
But IIRC, the infinite loop can be triggered by trying to enable the
miniuart early, when its not the console, before the config dxe is run
to turn the clock/regulator on. Which is what I was originally trying to
do in this patch by configuring both uarts unconditionally.
>
>
>>
>> //
>> // First thing we need to do is determine which of PL011 or miniUART is selected
>> @@ -85,23 +87,27 @@ SerialPortInitialize (
>> UsePl011UartSet = TRUE;
>> }
>>
>> - if (UsePl011Uart) {
>> - BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
>> + // always init the pl011 on the pi4, linux expects a SBSA uart to be at 115200
>> + // this means we need to set the baud/etc even if we arn't using it as a console
^ I should probably fix the spelling issues there too.
>> + if ((UsePl011Uart) || (RPI_MODEL == 4)) {
>> ReceiveFifoDepth = 0; // Use default FIFO depth
>> + BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
>
> Shouldn't we hardcode 115200 here if !UsePl011Uart?
Probably? For spec compliance for sure.
Although the BT as I mentioned is flaky, it will scan and connect to
things like keyboards/mice and sorta work, although it may drop the
connections/etc. A2dp devices are a no-go. But, running the port faster
than 115200 seems to at least extend how long it works. So, presumably
part of the problem is that the BT wants to bump the baud rate and reset
the device, neither of which it can do in ACPI mode at the moment. And
for clarity it does some of this in DT mode too, so its not entirely
just baud rate and reset problems. I spent a fair bit of time a year+
back trying to get the BT to work reliably in just DT mode with mainline
on the miniuart as well as on the pl011 where it sorta works and moved
on to other things before solving the problem.
So, I guess I can hard-code it here, at least then we are spec compliant.
>
>> Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
>> DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
>> StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
>>
>> - return PL011UartInitializePort (
>> - PL011_UART_REGISTER_BASE,
>> - PL011UartClockGetFreq(),
>> - &BaudRate,
>> - &ReceiveFifoDepth,
>> - &Parity,
>> - &DataBits,
>> - &StopBits
>> - );
>> - } else {
>> + Ret = PL011UartInitializePort (
>> + PL011_UART_REGISTER_BASE,
>> + PL011UartClockGetFreq(),
>> + &BaudRate,
>> + &ReceiveFifoDepth,
>> + &Parity,
>> + &DataBits,
>> + &StopBits
>> + );
>> + }
>> +
>> + if (!UsePl011Uart) {
>> SerialRegisterBase = MINI_UART_REGISTER_BASE;
>> Divisor = SerialPortGetDivisor (PcdGet32 (PcdSerialBaudRate));
>>
>> @@ -127,7 +133,8 @@ SerialPortInitialize (
>> // Wait for the serial port to be ready.
>> // Verify that both the transmit FIFO and the shift register are empty.
>> //
>> - while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
>> + Timeout = 1000;
>> + while (((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) && (Timeout--));
>>
>> //
>> // Configure baud rate
>> @@ -158,9 +165,9 @@ SerialPortInitialize (
>> // Put Modem Control Register(MCR) into its reset state of 0x00.
>> //
>> SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
>> -
>> - return RETURN_SUCCESS;
>> + Ret = RETURN_SUCCESS;
>> }
>> + return Ret;
>> }
>>
>> /**
>> --
>> 2.43.0
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113621): https://edk2.groups.io/g/devel/message/113621
Mute This Topic: https://groups.io/mt/103652853/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread
* [edk2-devel] [PATCH 2/5] Silicon/Broadcom/BcmGenetDxe: Suppress some bogus compiler warnings
2024-01-10 23:52 [edk2-devel] [PATCH 0/5] Platform/RaspberryPi: Various minor fixes Jeremy Linton
2024-01-10 23:52 ` [edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011 Jeremy Linton
@ 2024-01-10 23:52 ` Jeremy Linton
2024-01-10 23:52 ` [edk2-devel] [PATCH 3/5] Platform/RaspberryPi: Cleanup menu visibility Jeremy Linton
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Jeremy Linton @ 2024-01-10 23:52 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore, quic_llindhol, Jeremy Linton
Some recent GCC revisions will throw warnings about values being used
before being initialized. But in the case where the lack of initialization
is the result of the called function returning error status the EFI_ERROR()
macro/error seems to confuse the compiler about the fact that the value
is then never used.
So, while the code appears to be fine, lets just zero the variables
anyway to make the compiler happy.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c | 2 ++
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 3 +++
2 files changed, 5 insertions(+)
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
index 9e5d30fafd..2d5f70170e 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
@@ -381,6 +381,8 @@ GenericPhyUpdateConfig (
BOOLEAN LinkUp;
Status = GenericPhyGetLinkStatus (Phy);
+ Speed = 0;
+ Duplex = 0;
LinkUp = EFI_ERROR (Status) ? FALSE : TRUE;
if (Phy->LinkUp != LinkUp) {
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 3b51a86d65..7a7c398b1f 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -731,6 +731,9 @@ GenetSimpleNetworkReceive (
UINT8 *Frame;
UINTN FrameLength;
+ DescIndex = 0;
+ FrameLength = 0;
+
if (This == NULL || Buffer == NULL) {
DEBUG ((DEBUG_ERROR, "%a: Invalid parameter (missing handle or buffer)\n",
__FUNCTION__));
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113545): https://edk2.groups.io/g/devel/message/113545
Mute This Topic: https://groups.io/mt/103652854/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 10+ messages in thread* [edk2-devel] [PATCH 3/5] Platform/RaspberryPi: Cleanup menu visibility
2024-01-10 23:52 [edk2-devel] [PATCH 0/5] Platform/RaspberryPi: Various minor fixes Jeremy Linton
2024-01-10 23:52 ` [edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011 Jeremy Linton
2024-01-10 23:52 ` [edk2-devel] [PATCH 2/5] Silicon/Broadcom/BcmGenetDxe: Suppress some bogus compiler warnings Jeremy Linton
@ 2024-01-10 23:52 ` Jeremy Linton
2024-01-10 23:52 ` [edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox Jeremy Linton
2024-01-10 23:52 ` [edk2-devel] [PATCH 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT Jeremy Linton
4 siblings, 0 replies; 10+ messages in thread
From: Jeremy Linton @ 2024-01-10 23:52 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore, quic_llindhol, Jeremy Linton
Lets allow some of these options to change when the
system is in ACPI+DT mode. Plus the fan temp should
be disabled when ACPI isn't enabled.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index e8bf16312d..f668b7a553 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -196,7 +196,7 @@ formset
endoneof;
#if (RPI_MODEL == 4)
- grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+ grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
oneof varid = FanOnGpio.Enabled,
prompt = STRING_TOKEN(STR_ADVANCED_FANONGPIO_PROMPT),
help = STRING_TOKEN(STR_ADVANCED_FANONGPIO_HELP),
@@ -207,7 +207,7 @@ formset
endoneof;
endif;
- grayoutif ideqval FanOnGpio.Enabled == 0;
+ grayoutif ideqval FanOnGpio.Enabled == 0 OR ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
numeric varid = FanTemp.Value,
prompt = STRING_TOKEN(STR_ADVANCED_FANTEMP_PROMPT),
help = STRING_TOKEN(STR_ADVANCED_FANTEMP_HELP),
@@ -219,7 +219,7 @@ formset
endif;
suppressif ideqval XhciPci.Value == 2;
- grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+ grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
oneof varid = XhciPci.Value,
prompt = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PROMPT),
help = STRING_TOKEN(STR_ADVANCED_XHCIPCI_HELP),
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113546): https://edk2.groups.io/g/devel/message/113546
Mute This Topic: https://groups.io/mt/103652855/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 10+ messages in thread* [edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox
2024-01-10 23:52 [edk2-devel] [PATCH 0/5] Platform/RaspberryPi: Various minor fixes Jeremy Linton
` (2 preceding siblings ...)
2024-01-10 23:52 ` [edk2-devel] [PATCH 3/5] Platform/RaspberryPi: Cleanup menu visibility Jeremy Linton
@ 2024-01-10 23:52 ` Jeremy Linton
2024-01-11 7:48 ` Ard Biesheuvel
2024-01-10 23:52 ` [edk2-devel] [PATCH 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT Jeremy Linton
4 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2024-01-10 23:52 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore, quic_llindhol, Jeremy Linton
Its a complete tossup whether removing the mailbox call after we have
set up the XHCI works for a given kernel+distro in DT mode. So lets
give users which want to try DT the option of flipping this on/off.
Users that don't want to have to deal with DT, can use ACPI.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 ++++++++++
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 1 +
.../Drivers/ConfigDxe/ConfigDxeHii.uni | 5 +++++
.../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 +++++++++++++++
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 4 ++++
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 1 +
Platform/RaspberryPi/RPi3/RPi3.dsc | 6 ++++++
Platform/RaspberryPi/RPi4/RPi4.dsc | 7 +++++++
Platform/RaspberryPi/RaspberryPi.dec | 1 +
9 files changed, 50 insertions(+)
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 3dcf2bac0d..2484787982 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -298,6 +298,16 @@ SetupVariables (
Status = PcdSet32S (PcdXhciPci, 1);
ASSERT_EFI_ERROR (Status);
}
+
+ Size = sizeof (UINT32);
+ Status = gRT->GetVariable (L"XhciReload",
+ &gConfigDxeFormSetGuid,
+ NULL, &Size, &Var32);
+ if (EFI_ERROR (Status)) {
+ Status = PcdSet32S (PcdXhciReload, PcdGet32 (PcdXhciReload));
+ ASSERT_EFI_ERROR (Status);
+ }
+
}
} else {
/*
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 6f6e8f42ac..475e645537 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -96,6 +96,7 @@
gRaspberryPiTokenSpaceGuid.PcdUartInUse
gRaspberryPiTokenSpaceGuid.PcdXhciPci
gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
+ gRaspberryPiTokenSpaceGuid.PcdXhciReload
[Depex]
gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 5ec17072c3..8130638876 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -62,6 +62,11 @@
#string STR_ADVANCED_XHCIPCI_XHCI #language en-US "XHCI"
#string STR_ADVANCED_XHCIPCI_PCIE #language en-US "PCIe"
+#string STR_ADVANCED_XHCIRELOAD_PROMPT #language en-US "DT Reload XHCI firmware"
+#string STR_ADVANCED_XHCIRELOAD_HELP #language en-US "OS should reload XHCI firmware on reset"
+#string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
+#string STR_ADVANCED_XHCIRELOAD_RELOAD #language en-US "Reload"
+
#string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
#string STR_ADVANCED_ASSET_TAG_HELP #language en-US "Set the system Asset Tag"
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index f668b7a553..f13b70711d 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -61,6 +61,11 @@ formset
name = XhciPci,
guid = CONFIGDXE_FORM_SET_GUID;
+ efivarstore ADVANCED_XHCIPCI_VARSTORE_DATA,
+ attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+ name = XhciReload,
+ guid = CONFIGDXE_FORM_SET_GUID;
+
efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
name = SystemTableMode,
@@ -228,6 +233,16 @@ formset
option text = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PCIE), value = 1, flags = 0;
endoneof;
endif;
+
+ grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+ oneof varid = XhciReload.Value,
+ prompt = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_PROMPT),
+ help = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_HELP),
+ flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+ option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_DISABLE), value = 0, flags = DEFAULT;
+ option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_RELOAD), value = 1, flags = 0;
+ endoneof;
+ endif;
endif;
#endif
string varid = AssetTag.AssetTag,
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index cbbc2ad30d..dd4fc0a05e 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -375,6 +375,10 @@ SyncPcie (
return EFI_NOT_FOUND;
}
+ if (PcdGet32 (PcdXhciReload) != 1) {
+ return EFI_SUCCESS;
+ }
+
/*
* Now that we are always running without DMA translation, and with a 3G
* limit, there shouldn't be a need to reset/reload the XHCI. The
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
index 26f84e5940..d9fb6ee480 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
@@ -47,3 +47,4 @@
[Pcd]
gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
+ gRaspberryPiTokenSpaceGuid.PcdXhciReload
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index a967cd5a20..f5361ab95e 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -529,6 +529,12 @@
#
gRaspberryPiTokenSpaceGuid.PcdXhciPci|L"XhciPci"|gConfigDxeFormSetGuid|0x0|0
+ # DT contains XHCI quirk node (not valid on rpi3)
+ #
+ # 0 - DISABLED
+ #
+ gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
+
#
# Common UEFI ones.
#
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 8c49c67ab2..4e91eb9aea 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -547,6 +547,13 @@
#
gRaspberryPiTokenSpaceGuid.PcdXhciPci|L"XhciPci"|gConfigDxeFormSetGuid|0x0|0
+ # DT contains XHCI quirk node
+ #
+ # 0 - No reload
+ # 1 - Yes, DT has Reload
+ #
+ gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
+
#
# Common UEFI ones.
#
diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
index 17b6061a05..6bd16a5ae9 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -73,3 +73,4 @@
gRaspberryPiTokenSpaceGuid.PcdUartInUse|1|UINT32|0x00000021
gRaspberryPiTokenSpaceGuid.PcdXhciPci|0|UINT32|0x00000022
gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate|0|UINT32|0x00000023
+ gRaspberryPiTokenSpaceGuid.PcdXhciReload|0|UINT32|0x00000024
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113547): https://edk2.groups.io/g/devel/message/113547
Mute This Topic: https://groups.io/mt/103652856/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox
2024-01-10 23:52 ` [edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox Jeremy Linton
@ 2024-01-11 7:48 ` Ard Biesheuvel
0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2024-01-11 7:48 UTC (permalink / raw)
To: Jeremy Linton; +Cc: devel, ardb+tianocore, quic_llindhol
On Thu, 11 Jan 2024 at 00:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Its a complete tossup whether removing the mailbox call after we have
> set up the XHCI works for a given kernel+distro in DT mode. So lets
> give users which want to try DT the option of flipping this on/off.
>
> Users that don't want to have to deal with DT, can use ACPI.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 ++++++++++
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 1 +
> .../Drivers/ConfigDxe/ConfigDxeHii.uni | 5 +++++
> .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 +++++++++++++++
> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 4 ++++
> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 1 +
> Platform/RaspberryPi/RPi3/RPi3.dsc | 6 ++++++
> Platform/RaspberryPi/RPi4/RPi4.dsc | 7 +++++++
> Platform/RaspberryPi/RaspberryPi.dec | 1 +
> 9 files changed, 50 insertions(+)
>
This looks ok to me but it doesn't appear to apply on today's edk2-platforms.
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 3dcf2bac0d..2484787982 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -298,6 +298,16 @@ SetupVariables (
> Status = PcdSet32S (PcdXhciPci, 1);
> ASSERT_EFI_ERROR (Status);
> }
> +
> + Size = sizeof (UINT32);
> + Status = gRT->GetVariable (L"XhciReload",
> + &gConfigDxeFormSetGuid,
> + NULL, &Size, &Var32);
> + if (EFI_ERROR (Status)) {
> + Status = PcdSet32S (PcdXhciReload, PcdGet32 (PcdXhciReload));
> + ASSERT_EFI_ERROR (Status);
> + }
> +
> }
> } else {
> /*
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index 6f6e8f42ac..475e645537 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -96,6 +96,7 @@
> gRaspberryPiTokenSpaceGuid.PcdUartInUse
> gRaspberryPiTokenSpaceGuid.PcdXhciPci
> gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
> + gRaspberryPiTokenSpaceGuid.PcdXhciReload
>
> [Depex]
> gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 5ec17072c3..8130638876 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> @@ -62,6 +62,11 @@
> #string STR_ADVANCED_XHCIPCI_XHCI #language en-US "XHCI"
> #string STR_ADVANCED_XHCIPCI_PCIE #language en-US "PCIe"
>
> +#string STR_ADVANCED_XHCIRELOAD_PROMPT #language en-US "DT Reload XHCI firmware"
> +#string STR_ADVANCED_XHCIRELOAD_HELP #language en-US "OS should reload XHCI firmware on reset"
> +#string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
> +#string STR_ADVANCED_XHCIRELOAD_RELOAD #language en-US "Reload"
> +
> #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
> #string STR_ADVANCED_ASSET_TAG_HELP #language en-US "Set the system Asset Tag"
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index f668b7a553..f13b70711d 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -61,6 +61,11 @@ formset
> name = XhciPci,
> guid = CONFIGDXE_FORM_SET_GUID;
>
> + efivarstore ADVANCED_XHCIPCI_VARSTORE_DATA,
> + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> + name = XhciReload,
> + guid = CONFIGDXE_FORM_SET_GUID;
> +
> efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
> attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> name = SystemTableMode,
> @@ -228,6 +233,16 @@ formset
> option text = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PCIE), value = 1, flags = 0;
> endoneof;
> endif;
> +
> + grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
> + oneof varid = XhciReload.Value,
> + prompt = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_PROMPT),
> + help = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_HELP),
> + flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> + option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_DISABLE), value = 0, flags = DEFAULT;
> + option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_RELOAD), value = 1, flags = 0;
> + endoneof;
> + endif;
> endif;
> #endif
> string varid = AssetTag.AssetTag,
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> index cbbc2ad30d..dd4fc0a05e 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -375,6 +375,10 @@ SyncPcie (
> return EFI_NOT_FOUND;
> }
>
> + if (PcdGet32 (PcdXhciReload) != 1) {
> + return EFI_SUCCESS;
> + }
> +
> /*
> * Now that we are always running without DMA translation, and with a 3G
> * limit, there shouldn't be a need to reset/reload the XHCI. The
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> index 26f84e5940..d9fb6ee480 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> @@ -47,3 +47,4 @@
>
> [Pcd]
> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
> + gRaspberryPiTokenSpaceGuid.PcdXhciReload
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index a967cd5a20..f5361ab95e 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -529,6 +529,12 @@
> #
> gRaspberryPiTokenSpaceGuid.PcdXhciPci|L"XhciPci"|gConfigDxeFormSetGuid|0x0|0
>
> + # DT contains XHCI quirk node (not valid on rpi3)
> + #
> + # 0 - DISABLED
> + #
> + gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
> +
> #
> # Common UEFI ones.
> #
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 8c49c67ab2..4e91eb9aea 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -547,6 +547,13 @@
> #
> gRaspberryPiTokenSpaceGuid.PcdXhciPci|L"XhciPci"|gConfigDxeFormSetGuid|0x0|0
>
> + # DT contains XHCI quirk node
> + #
> + # 0 - No reload
> + # 1 - Yes, DT has Reload
> + #
> + gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
> +
> #
> # Common UEFI ones.
> #
> diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
> index 17b6061a05..6bd16a5ae9 100644
> --- a/Platform/RaspberryPi/RaspberryPi.dec
> +++ b/Platform/RaspberryPi/RaspberryPi.dec
> @@ -73,3 +73,4 @@
> gRaspberryPiTokenSpaceGuid.PcdUartInUse|1|UINT32|0x00000021
> gRaspberryPiTokenSpaceGuid.PcdXhciPci|0|UINT32|0x00000022
> gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate|0|UINT32|0x00000023
> + gRaspberryPiTokenSpaceGuid.PcdXhciReload|0|UINT32|0x00000024
> --
> 2.43.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113590): https://edk2.groups.io/g/devel/message/113590
Mute This Topic: https://groups.io/mt/103652856/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread
* [edk2-devel] [PATCH 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT
2024-01-10 23:52 [edk2-devel] [PATCH 0/5] Platform/RaspberryPi: Various minor fixes Jeremy Linton
` (3 preceding siblings ...)
2024-01-10 23:52 ` [edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox Jeremy Linton
@ 2024-01-10 23:52 ` Jeremy Linton
2024-01-11 7:49 ` Ard Biesheuvel
4 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2024-01-10 23:52 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore, quic_llindhol, Jeremy Linton
Since we are updating the DT memory map and telling it how
we have configured the PCIe, there isn't a reason for moving the
MMIO window. In fact this appears to fix OpenBSD+DT as well as
it makes the linux XHCI reset sequence happier.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 24 ++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index dd4fc0a05e..e6942df6a3 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -375,6 +375,30 @@ SyncPcie (
return EFI_NOT_FOUND;
}
+ // move the MMIO window too
+ DmaRanges[0] = cpu_to_fdt32 (0x02000000); //non prefech 32-bit
+ DmaRanges[1] = cpu_to_fdt32 (0x00000000); //bus addr @ 0x0f8000000
+ DmaRanges[2] = cpu_to_fdt32 (0xf8000000);
+ DmaRanges[3] = cpu_to_fdt32 (0x00000006); //cpu addr @ 0x600000000
+ DmaRanges[4] = cpu_to_fdt32 (0x00000000);
+ DmaRanges[5] = cpu_to_fdt32 (0x00000000);
+ DmaRanges[6] = cpu_to_fdt32 (0x04000000); // len = 0x4000 0000
+
+ DEBUG ((DEBUG_INFO, "%a: Updating PCIe ranges\n", __func__));
+
+ /*
+ * Match dma-ranges with the EDK2+ACPI setup we are using. This works
+ * around a failure in Linux and OpenBSD to reset the PCIe/XHCI correctly
+ * when in DT mode.
+ */
+ Retval = fdt_setprop (mFdtImage, Node, "ranges",
+ DmaRanges, sizeof DmaRanges);
+ if (Retval != 0) {
+ DEBUG ((DEBUG_ERROR, "%a: failed to locate PCIe MMIO 'ranges' property (%d)\n",
+ __func__, Retval));
+ return EFI_NOT_FOUND;
+ }
+
if (PcdGet32 (PcdXhciReload) != 1) {
return EFI_SUCCESS;
}
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113548): https://edk2.groups.io/g/devel/message/113548
Mute This Topic: https://groups.io/mt/103652857/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [edk2-devel] [PATCH 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT
2024-01-10 23:52 ` [edk2-devel] [PATCH 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT Jeremy Linton
@ 2024-01-11 7:49 ` Ard Biesheuvel
0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2024-01-11 7:49 UTC (permalink / raw)
To: Jeremy Linton; +Cc: devel, ardb+tianocore, quic_llindhol
On Thu, 11 Jan 2024 at 00:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Since we are updating the DT memory map and telling it how
> we have configured the PCIe, there isn't a reason for moving the
> MMIO window. In fact this appears to fix OpenBSD+DT as well as
> it makes the linux XHCI reset sequence happier.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 24 ++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> index dd4fc0a05e..e6942df6a3 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -375,6 +375,30 @@ SyncPcie (
> return EFI_NOT_FOUND;
> }
>
> + // move the MMIO window too
> + DmaRanges[0] = cpu_to_fdt32 (0x02000000); //non prefech 32-bit
> + DmaRanges[1] = cpu_to_fdt32 (0x00000000); //bus addr @ 0x0f8000000
> + DmaRanges[2] = cpu_to_fdt32 (0xf8000000);
> + DmaRanges[3] = cpu_to_fdt32 (0x00000006); //cpu addr @ 0x600000000
> + DmaRanges[4] = cpu_to_fdt32 (0x00000000);
> + DmaRanges[5] = cpu_to_fdt32 (0x00000000);
> + DmaRanges[6] = cpu_to_fdt32 (0x04000000); // len = 0x4000 0000
> +
Is there any way we can use symbolic constants here?
> + DEBUG ((DEBUG_INFO, "%a: Updating PCIe ranges\n", __func__));
> +
> + /*
> + * Match dma-ranges with the EDK2+ACPI setup we are using. This works
> + * around a failure in Linux and OpenBSD to reset the PCIe/XHCI correctly
> + * when in DT mode.
> + */
Please sync the comment with the code.
> + Retval = fdt_setprop (mFdtImage, Node, "ranges",
> + DmaRanges, sizeof DmaRanges);
> + if (Retval != 0) {
> + DEBUG ((DEBUG_ERROR, "%a: failed to locate PCIe MMIO 'ranges' property (%d)\n",
> + __func__, Retval));
> + return EFI_NOT_FOUND;
> + }
> +
> if (PcdGet32 (PcdXhciReload) != 1) {
> return EFI_SUCCESS;
> }
> --
> 2.43.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113591): https://edk2.groups.io/g/devel/message/113591
Mute This Topic: https://groups.io/mt/103652857/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread