在 10/13/2016 4:33 PM, Leif Lindholm 写道:
Please change WA to workaround in subject (there are too many possible things it could be an abbreviation of to make it simple to read :)
On Thu, Oct 13, 2016 at 10:00:10AM +0800, Heyi Guo wrote:
Timer interrupt cannot be reported after EDK2 commit 7989300, for GIC on D02/D03 is not fully ARM GIC compliant. The issue has been fixed on newer chips so we use a WA for D02 and D03 only.
workaround
On D02 and D03, IRQ will be latched in GIC logic except virutal timer interrupt IRQ #27, so we change to use virtual timer instead of physical in UEFI.
Change-Id: Ie8eca7e4dea45a3a318ee32783ddaa15363065e2 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Heyi Guo heyi.guo@linaro.org
Platforms/Hisilicon/D02/Pv660D02.dsc | 10 ++++++++-- Platforms/Hisilicon/D02/Pv660D02.fdf | 2 +- Platforms/Hisilicon/D03/D03.dsc | 10 +++++++++- Platforms/Hisilicon/D03/D03.fdf | 2 +- 4 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/Platforms/Hisilicon/D02/Pv660D02.dsc b/Platforms/Hisilicon/D02/Pv660D02.dsc index d025bdd..4b4a0b7 100644 --- a/Platforms/Hisilicon/D02/Pv660D02.dsc +++ b/Platforms/Hisilicon/D02/Pv660D02.dsc @@ -72,6 +72,13 @@ PlatformBdsLib|OpenPlatformPkg/Chips/Hisilicon/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf +## GIC on D02/D03 is not fully ARM GIC compatible: IRQ cannot be cancelled when +## input signal is deasserted, except for virtual timer interrupt IRQ #27. So +## we choose to use virutal timer instead of physical one as a workaround.
Typo: virutal -> virtual
+## This library instance is to override the one in Pv660.dsc.inc.
This whole block is a very useful comment to describe an otherwise non-obvious change, thanks.
+[LibraryClasses.AARCH64]
Out of interest - why is this added for AARCH64 only? Does the platform build and function properly for ARM, and is this workaround then not needed there?
This is because the original setting in Pv660.dsc.inc is under [LibraryClasses.AARCH64], so I think if I just use a common section here, it might not be able to override the one in Pv660.dsc.inc, according to Library Instance precedence rule.
The reason why it was put under [LibraryClasses.AARCH64] in Pv660.dsc.inc is unclear now; probably it was just copied from other platform.
So we may need another patch to change it in Pv660.dsc.inc first, if we want to fix the one in D02 dsc file. Do you think it is necessary to do that?
Thanks.
Heyi
- ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.inf
- [LibraryClasses.common.SEC] ArmPlatformLib|OpenPlatformPkg/Chips/Hisilicon/Library/ArmPlatformLibPv660/ArmPlatformLibSec.inf
@@ -341,8 +348,7 @@ ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
- #ArmPkg/Drivers/TimerDxe/TimerDxe
- ArmPlatformPkg/Drivers/SP804TimerDxe/SP804TimerDxe.inf
- ArmPkg/Drivers/TimerDxe/TimerDxe.inf ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf
IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe.inf diff --git a/Platforms/Hisilicon/D02/Pv660D02.fdf b/Platforms/Hisilicon/D02/Pv660D02.fdf index 69be1f1..fa0dc2d 100644 --- a/Platforms/Hisilicon/D02/Pv660D02.fdf +++ b/Platforms/Hisilicon/D02/Pv660D02.fdf @@ -195,7 +195,7 @@ READ_LOCK_STATUS = TRUE #INF EmbeddedPkg/SimpleTextInOutSerial/SimpleTextInOutSerial.inf INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
- INF ArmPlatformPkg/Drivers/SP804TimerDxe/SP804TimerDxe.inf
- INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf INF ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf
# diff --git a/Platforms/Hisilicon/D03/D03.dsc b/Platforms/Hisilicon/D03/D03.dsc index 83a18b1..ccf16a2 100644 --- a/Platforms/Hisilicon/D03/D03.dsc +++ b/Platforms/Hisilicon/D03/D03.dsc @@ -81,6 +81,14 @@ LpcLib|OpenPlatformPkg/Chips/Hisilicon/Binary/Hi1610/Library/LpcLib/LpcLib.inf SerialPortLib|OpenPlatformPkg/Chips/Hisilicon/Binary/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
+## GIC on D02/D03 is not fully ARM GIC compatible: IRQ cannot be cancelled when +## input signal is deasserted, except for virtual timer interrupt IRQ #27. So +## we choose to use virutal timer instead of physical one as a workaround.
Typo: virutal -> virtual
+## This library instance is to override the one in Pv660.dsc.inc. +[LibraryClasses.AARCH64]
Out of interest - why is this added for AARCH64 only? Does the platform build and function properly for ARM, and is this workaround then not needed there?
- ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.inf
- [LibraryClasses.common.SEC] ArmPlatformLib|OpenPlatformPkg/Chips/Hisilicon/Library/ArmPlatformLibPv660/ArmPlatformLibSec.inf
@@ -396,7 +404,7 @@ ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
- ArmPlatformPkg/Drivers/SP804TimerDxe/SP804TimerDxe.inf
- ArmPkg/Drivers/TimerDxe/TimerDxe.inf
ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe.inf diff --git a/Platforms/Hisilicon/D03/D03.fdf b/Platforms/Hisilicon/D03/D03.fdf index 8144151..8ba3bd0 100644 --- a/Platforms/Hisilicon/D03/D03.fdf +++ b/Platforms/Hisilicon/D03/D03.fdf @@ -187,7 +187,7 @@ READ_LOCK_STATUS = TRUE # Simple TextIn/TextOut for UEFI Terminal INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
- INF ArmPlatformPkg/Drivers/SP804TimerDxe/SP804TimerDxe.inf
- INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
INF ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf
For my part, with these minor fixes, I'm happy with this patch. Ard: do you agree that this is the best fix for the problem?
Regards,
Leif
1.9.1