On Tue, 16 Apr 2019 at 02:23, Marcin Wojtas <m...@semihalf.com> wrote:
>
> Generic handling of the PMU interrupts in UEFI and Linux with
> ACPI require enabling a dedicated handler in the EL3.
> Extend the PlatInitDxe with enabler code.
>
> Because for DT world the EL3 service must remain disabled,
> switch it off in the ExitBootServicesEvent. Its execution
> depends on the gEdkiiPlatformHasAcpiGuid status check in the new
> gEfiEventReadyToBootGuid routine.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <m...@semihalf.com>
> ---
> Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf | 6 ++
> Silicon/Marvell/Include/IndustryStandard/MvSmc.h | 2 +
> Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c | 82
> ++++++++++++++++++++
> 3 files changed, 90 insertions(+)
>
> diff --git a/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
> b/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
> index e707fe9..df10526 100644
> --- a/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
> +++ b/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
> @@ -25,6 +25,7 @@
> PlatInitDxe.c
>
> [Packages]
> + ArmPkg/ArmPkg.dec
> EmbeddedPkg/EmbeddedPkg.dec
> MdeModulePkg/MdeModulePkg.dec
> MdePkg/MdePkg.dec
> @@ -32,6 +33,7 @@
>
> [LibraryClasses]
> ArmadaIcuLib
> + ArmSmcLib
> ComPhyLib
> DebugLib
> MppLib
> @@ -40,6 +42,10 @@
> UefiDriverEntryPoint
> UtmiPhyLib
>
> +[Guids]
> + gEdkiiPlatformHasAcpiGuid
> + gEfiEventReadyToBootGuid
> +
> [Protocols]
> gMarvellPlatformInitCompleteProtocolGuid ## PRODUCES
>
> diff --git a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> index 0c90f11..e5c89d9 100644
> --- a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> +++ b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> @@ -20,5 +20,7 @@
> #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002
> #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003
> #define MV_SMC_ID_DRAM_SIZE 0x82000010
> +#define MV_SMC_ID_PMU_IRQ_ENABLE 0x82000012
> +#define MV_SMC_ID_PMU_IRQ_DISABLE 0x82000013
>
> #endif //__MV_SMC_H__
> diff --git a/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c
> b/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c
> index 18b6783..4012fd7 100644
> --- a/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c
> +++ b/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c
> @@ -12,7 +12,12 @@
>
> **/
>
> +#include <Guid/EventGroup.h>
> +
> +#include <IndustryStandard/MvSmc.h>
> +
> #include <Library/ArmadaIcuLib.h>
> +#include <Library/ArmSmcLib.h>
> #include <Library/DebugLib.h>
> #include <Library/MppLib.h>
> #include <Library/MvComPhyLib.h>
> @@ -21,6 +26,62 @@
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UtmiPhyLib.h>
>
> +STATIC EFI_EVENT mArmada7k8kExitBootServicesEvent;
> +
> +/**
> + Disable extra EL3 handling of the PMU interrupts for DT world.
> +
> + @param[in] Event Event structure
> + @param[in] Context Notification context
> +
> +**/
> +STATIC
> +VOID
Please add EFIAPI here. I know it evaluates to nothing at the moment,
but since this is called via a function pointer by the DXE core, it
should have the correct prototype.
> +Armada7k8kExitBootServicesHandler (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + ARM_SMC_ARGS SmcRegs = {0};
> +
> + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_DISABLE;
> + ArmCallSmc (&SmcRegs);
> +
> + return;
> +}
> +
> +/**
> + Check if we boot with DT and trigger EBS event in such case.
> +
> + @param[in] Event Event structure
> + @param[in] Context Notification context
> +
> +**/
> +STATIC
> +VOID
Same here
> +Armada7k8kOnReadyToBootHandler (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + EFI_STATUS Status;
> + VOID *Interface;
> +
> + Status = gBS->LocateProtocol (&gEdkiiPlatformHasAcpiGuid,
> + NULL,
> + (VOID **)&Interface);
> + if (EFI_ERROR (Status)) {
Can we invert the logic (i.e. return early on success) with a comment
explaining why?
> + Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES,
> + TPL_NOTIFY,
> + Armada7k8kExitBootServicesHandler,
> + NULL,
> + &mArmada7k8kExitBootServicesEvent);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: Fail to register EBS event\n",
> __FUNCTION__));
> + }
Just use ASSERT_EFI_ERROR() here - this never fails in practice.
> + }
Please close the event - ReadyToBoot could fire multiple times and you
only want the notification once.
> +}
> +
> EFI_STATUS
> EFIAPI
> ArmadaPlatInitDxeEntryPoint (
> @@ -28,7 +89,9 @@ ArmadaPlatInitDxeEntryPoint (
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> + ARM_SMC_ARGS SmcRegs = {0};
> EFI_STATUS Status;
> + EFI_EVENT Event;
>
> DEBUG ((DEBUG_ERROR, "\nArmada Platform Init\n\n"));
>
> @@ -43,5 +106,24 @@ ArmadaPlatInitDxeEntryPoint (
> MppInitialize ();
> ArmadaIcuInitialize ();
>
> + /*
> + * Enable EL3 PMU interrupt handler and
> + * register the ReadyToBoot event.
> + */
> + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_ENABLE;
> + ArmCallSmc (&SmcRegs);
> +
> + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL,
> + TPL_CALLBACK,
> + Armada7k8kOnReadyToBootHandler,
> + NULL,
> + &gEfiEventReadyToBootGuid,
> + &Event);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR,
> + "%a: Fail to register OnReadyToBoot event\n",
> + __FUNCTION__));
> + }
> +
Just use ASSERT_EFI_ERROR here (as above)
> return EFI_SUCCESS;
> }
> --
> 2.7.4
>
I'll pick up the other patches, so no need to resend the whole series.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#39187): https://edk2.groups.io/g/devel/message/39187
Mute This Topic: https://groups.io/mt/31198635/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
