From: Sami Mujawar sami.mujawar@arm.com
The generic PL011 driver has been updated to allow for UARTs on a board having different clock sources (as is the case on Juno). This changes the Juno code to make use of the new driver options.
The solution involves using the Baud rate and new PL011UartClkInHz PCDs to configure the UART ports.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Alexei Fedorov alexei.fedorov@arm.com Signed-off-by: Girish Pathak girish.pathak@arm.com Signed-off-by: Sami Mujawar sami.mujawar@arm.com Signed-off-by: Evan Lloyd evan.lloyd@arm.com --- Platforms/ARM/Juno/ArmJuno.dsc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/Platforms/ARM/Juno/ArmJuno.dsc b/Platforms/ARM/Juno/ArmJuno.dsc index b718794..3e17831 100644 --- a/Platforms/ARM/Juno/ArmJuno.dsc +++ b/Platforms/ARM/Juno/ArmJuno.dsc @@ -126,8 +126,7 @@ ## PL011 - Serial Terminal gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF80000 gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200 - gArmPlatformTokenSpaceGuid.PL011UartInteger|4 - gArmPlatformTokenSpaceGuid.PL011UartFractional|0 + gArmPlatformTokenSpaceGuid.PL011UartClkInHz|7372800 gArmPlatformTokenSpaceGuid.PL011UartInterrupt|115
## PL031 RealTimeClock
Sorry for reviving an old patch, but I'm just starting to go through my queue of patches I was meant to review...
On 9 March 2016 at 18:12, evan.lloyd@arm.com wrote:
From: Sami Mujawar sami.mujawar@arm.com
The generic PL011 driver has been updated to allow for UARTs on a board having different clock sources (as is the case on Juno). This changes the Juno code to make use of the new driver options.
The solution involves using the Baud rate and new PL011UartClkInHz PCDs to configure the UART ports.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Alexei Fedorov alexei.fedorov@arm.com Signed-off-by: Girish Pathak girish.pathak@arm.com Signed-off-by: Sami Mujawar sami.mujawar@arm.com Signed-off-by: Evan Lloyd evan.lloyd@arm.com
Tested-by: Ryan Harkin ryan.harkin@linaro.org
But I have a comment below:
Platforms/ARM/Juno/ArmJuno.dsc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/Platforms/ARM/Juno/ArmJuno.dsc b/Platforms/ARM/Juno/ArmJuno.dsc index b718794..3e17831 100644 --- a/Platforms/ARM/Juno/ArmJuno.dsc +++ b/Platforms/ARM/Juno/ArmJuno.dsc @@ -126,8 +126,7 @@ ## PL011 - Serial Terminal gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF80000 gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
- gArmPlatformTokenSpaceGuid.PL011UartInteger|4
- gArmPlatformTokenSpaceGuid.PL011UartFractional|0
- gArmPlatformTokenSpaceGuid.PL011UartClkInHz|7372800
Where did this number come from? I presume it's the result of a calculation. I wonder if there's a way of calling out the calculation, although I'm not even sure if PCDs can take anything other than a static value. Perhaps a comment is the best we can hope for?
gArmPlatformTokenSpaceGuid.PL011UartInterrupt|115
## PL031 RealTimeClock
2.7.0
Hi Ryan. It is apparent from you confusion that we need to clarify our comment. However, I think that is the only problem - response inline below. The original problem was that not all UARTs on Juno have the same clock source, so the PL011 driver could never work on all the UARTS. This particular change only matches the driver update, and I will add something to highlight that dependency to the comment.
-----Original Message----- From: Ryan Harkin [mailto:ryan.harkin@linaro.org] Sent: 03 May 2016 11:49 To: Evan Lloyd Cc: Linaro UEFI Mailman List; Leif Lindholm Subject: Re: [PATCH] Platforms/ARM: Juno: PL011 PCD changes
...
The generic PL011 driver has been updated to allow for UARTs on a board having different clock sources (as is the case on Juno). This changes the Juno code to make use of the new driver options.
The solution involves using the Baud rate and new PL011UartClkInHz PCDs to configure the UART ports.
...
@@ -126,8 +126,7 @@ ## PL011 - Serial Terminal gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF80000 gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
- gArmPlatformTokenSpaceGuid.PL011UartInteger|4
- gArmPlatformTokenSpaceGuid.PL011UartFractional|0
- gArmPlatformTokenSpaceGuid.PL011UartClkInHz|7372800
Where did this number come from? I presume it's the result of a calculation. I wonder if there's a way of calling out the calculation, although I'm not even sure if PCDs can take anything other than a static value. Perhaps a comment is the best we can hope for?
The clock rate is fixed in the hardware, so the value of PL011UartClkInHz is derived from the Juno TRM. The actual change is that the PL011 interface has been modified to accept a clock frequency value, rather than the Integer/fractional PCDs (they WERE the result of a calculation, which was wrong for some of the uarts).
gArmPlatformTokenSpaceGuid.PL011UartInterrupt|115
## PL031 RealTimeClock
2.7.0
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Hi Evan,
On 4 May 2016 at 14:02, Evan Lloyd Evan.Lloyd@arm.com wrote:
Hi Ryan. It is apparent from you confusion that we need to clarify our comment. However, I think that is the only problem - response inline below. The original problem was that not all UARTs on Juno have the same clock source, so the PL011 driver could never work on all the UARTS. This particular change only matches the driver update, and I will add something to highlight that dependency to the comment.
-----Original Message----- From: Ryan Harkin [mailto:ryan.harkin@linaro.org] Sent: 03 May 2016 11:49 To: Evan Lloyd Cc: Linaro UEFI Mailman List; Leif Lindholm Subject: Re: [PATCH] Platforms/ARM: Juno: PL011 PCD changes
...
The generic PL011 driver has been updated to allow for UARTs on a board having different clock sources (as is the case on Juno). This changes the Juno code to make use of the new driver options.
The solution involves using the Baud rate and new PL011UartClkInHz PCDs to configure the UART ports.
...
@@ -126,8 +126,7 @@ ## PL011 - Serial Terminal gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF80000 gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
- gArmPlatformTokenSpaceGuid.PL011UartInteger|4
- gArmPlatformTokenSpaceGuid.PL011UartFractional|0
- gArmPlatformTokenSpaceGuid.PL011UartClkInHz|7372800
Where did this number come from? I presume it's the result of a calculation. I wonder if there's a way of calling out the calculation, although I'm not even sure if PCDs can take anything other than a static value. Perhaps a comment is the best we can hope for?
The clock rate is fixed in the hardware, so the value of PL011UartClkInHz is derived from the Juno TRM. The actual change is that the PL011 interface has been modified to accept a clock frequency value, rather than the Integer/fractional PCDs (they WERE the result of a calculation, which was wrong for some of the uarts).
Ah, thanks for the clarification. I'm happy with the change, but I think it would indeed be nice to have that info in the commit message.
Thanks, Ryan.
gArmPlatformTokenSpaceGuid.PL011UartInterrupt|115
## PL031 RealTimeClock
2.7.0
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.