On Thu, Oct 13, 2016 at 08:27:15PM +0800, Heyi Guo wrote:
+## 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.
OK, that makes sense.
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?
No. But it would be nice to have a short coment: "# Overriding original defined in LibraryClasses.AARCH64 in Pc660.dsc.inc"
Regards,
Leif
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