Hi Khasim,
I have some remarks about the patch:
On 10/10/21 19:29, Khasim Mohammed via groups.io wrote:
> Add an initial platform DXE driver and support for ramdisk devices.
>
> Signed-off-by: Deepak Pandey <deepak.pan...@arm.com>
> Signed-off-by: Khasim Syed Mohammed <khasim.moham...@arm.com>
> ---
> .../N1Sdp/Drivers/PlatformDxe/PlatformDxe.c | 51 +++++++++++++++++++
> .../N1Sdp/Drivers/PlatformDxe/PlatformDxe.inf | 44 ++++++++++++++++
> 2 files changed, 95 insertions(+)
> create mode 100644 Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.c
> create mode 100644 Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.inf
>
> diff --git a/Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.c
> b/Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.c
> new file mode 100644
> index 0000000000..3abe2228ad
> --- /dev/null
> +++ b/Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.c
> @@ -0,0 +1,51 @@
> +/** @file
> +
> + Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/RamDisk.h>
> +
> +/**
> + Entrypoint of Platform Dxe Driver
> +
> + @param ImageHandle[in] The firmware allocated handle for the EFI
> image.
> + @param SystemTable[in] A pointer to the EFI System Table.
> +
> + @retval EFI_SUCCESS The entry point is executed successfully.
It seems the function can also return other error codes.
> +**/
> +EFI_STATUS
> +EFIAPI
> +ArmN1SdpEntryPoint (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> + EFI_RAM_DISK_PROTOCOL *RamDisk;
> + EFI_DEVICE_PATH_PROTOCOL *DevicePath;
> +
> + Status = EFI_UNSUPPORTED;
> + if (FeaturePcdGet (PcdRamDiskSupported)) {
> + Status = gBS->LocateProtocol (&gEfiRamDiskProtocolGuid, NULL, (VOID**)
> &RamDisk);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: Couldn't find the RAM Disk protocol - %r\n",
> __FUNCTION__, Status));
These 2 lines seem quite long, is it possible to split them on multiple
lines ?
(Same for the debug message below)
> + return Status;
> + }
> +
> + Status = RamDisk->Register (
> + (UINTN)PcdGet32 (PcdRamDiskBase),
> + (UINTN)PcdGet32 (PcdRamDiskSize),
> + &gEfiVirtualCdGuid,
> + NULL,
> + &DevicePath);
I think the gEfiVirtualCdGuid should be listed in a [Guid] section of
the .inf file with the following comment:
## SOMETIMES_CONSUMES ## GUID
The same should be done for
Platform/ARM/Morello/Drivers/PlatformDxe/PlatformDxeFvp.c
NIT: is it possible to have the closing parenthesis on a new line as:
&DevicePath
);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: Failed to register RAM Disk - %r\n",
> __FUNCTION__, Status));
> + }
> + }
> + return Status;
> +}
> diff --git a/Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.inf
> b/Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.inf
> new file mode 100644
> index 0000000000..925bde4063
> --- /dev/null
> +++ b/Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -0,0 +1,44 @@
> +## @file
> +# Platform DXE driver for N1Sdp
> +#
> +# Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x0001001B
> + BASE_NAME = PlatformDxe
> + FILE_GUID = 11fc8b5a-377d-47a8-aee9-0093d3d3407f
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + ENTRY_POINT = ArmN1SdpEntryPoint
> +
> +[Sources.common]
> + PlatformDxe.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + ArmPlatformPkg/ArmPlatformPkg.dec
> + EmbeddedPkg/EmbeddedPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> + Platform/ARM/N1Sdp/N1SdpPlatform.dec
> +
> +[LibraryClasses]
> + HobLib
> + UefiDriverEntryPoint
> +
> +[Protocols]
> + gEfiRamDiskProtocolGuid
> +
> +[FeaturePcd]
> + gArmN1SdpTokenSpaceGuid.PcdRamDiskSupported
> +
> +[FixedPcd]
> + gArmN1SdpTokenSpaceGuid.PcdRamDiskBase
> + gArmN1SdpTokenSpaceGuid.PcdRamDiskSize
> +
> +[Depex]
> + gEfiRamDiskProtocolGuid
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81878): https://edk2.groups.io/g/devel/message/81878
Mute This Topic: https://groups.io/mt/86219908/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
