On Fri, Nov 16, 2018 at 04:45:23PM -0800, Ard Biesheuvel wrote:
> Currently, each flash bank controlled by ArmPlatformPkg/NorFlashDxe
> has its own VendorHw GUID, and instances of NorFlashPlatformLib
> describe each bank to the driver, along with the GUID for each.
>
> This works ok for bare metal platforms, but it would be useful for
> virtual platforms if we could obtain this information from a
> device tree, which would require us to invent GUIDs on the fly,
> given that the 'cfi-flash' binding does not include a GUID.
>
> So instead, let's switch to a single GUID for all flash banks,
> and update the driver's device path handling to include an index
> to identify each bank uniquely.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 15 +++++++++------
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h | 3 +++
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> index 46e815beb343..60a06e4a5058 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> @@ -82,10 +82,14 @@ NOR_FLASH_INSTANCE mNorFlashInstanceTemplate = {
> {
> HARDWARE_DEVICE_PATH,
> HW_VENDOR_DP,
> - { (UINT8)sizeof(VENDOR_DEVICE_PATH),
> (UINT8)((sizeof(VENDOR_DEVICE_PATH)) >> 8) }
> + {
> + (UINT8)(OFFSET_OF(NOR_FLASH_DEVICE_PATH, End)),
> + (UINT8)(OFFSET_OF(NOR_FLASH_DEVICE_PATH, End) >> 8)
> + }
> },
> { 0x0, 0x0, 0x0, { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 } }, // GUID
> ... NEED TO BE FILLED
> },
> + 0, // Index
> {
> END_DEVICE_PATH_TYPE,
> END_ENTIRE_DEVICE_PATH_SUBTYPE,
> @@ -99,10 +103,9 @@ NorFlashCreateInstance (
> IN UINTN NorFlashDeviceBase,
> IN UINTN NorFlashRegionBase,
> IN UINTN NorFlashSize,
> - IN UINT32 MediaId,
> + IN UINT32 Index,
> IN UINT32 BlockSize,
> IN BOOLEAN SupportFvb,
> - IN CONST GUID *NorFlashGuid,
> OUT NOR_FLASH_INSTANCE** NorFlashInstance
> )
> {
> @@ -121,11 +124,12 @@ NorFlashCreateInstance (
> Instance->Size = NorFlashSize;
>
> Instance->BlockIoProtocol.Media = &Instance->Media;
> - Instance->Media.MediaId = MediaId;
> + Instance->Media.MediaId = Index;
> Instance->Media.BlockSize = BlockSize;
> Instance->Media.LastBlock = (NorFlashSize / BlockSize)-1;
>
> - CopyGuid (&Instance->DevicePath.Vendor.Guid, NorFlashGuid);
> + CopyGuid (&Instance->DevicePath.Vendor.Guid, &gEfiCallerIdGuid);
Just sanity checking: this sets the VendorGuid to the NorFlashDxe
GUID? (93E34C7E-B50E-11DF-9223-2443DFD72085)
If not, can you explain it to me slowly? :)
Thomas, Nariman: would this change cause any transient issues for
anything that has set Boot#### options in any of your configurations?
And if it would, is that a big deal?
(Ard has a separate patch that fixes up any default values.)
/
Leif
> + Instance->DevicePath.Index = Index;
>
> Instance->ShadowBuffer = AllocateRuntimePool (BlockSize);;
> if (Instance->ShadowBuffer == NULL) {
> @@ -1311,7 +1315,6 @@ NorFlashInitialise (
> Index,
> NorFlashDevices[Index].BlockSize,
> ContainVariableStorage,
> - &NorFlashDevices[Index].Guid,
> &mNorFlashInstances[Index]
> );
> if (EFI_ERROR(Status)) {
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
> b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
> index 5c07694fbfaa..8886aa43d9f3 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
> @@ -122,10 +122,13 @@
>
> typedef struct _NOR_FLASH_INSTANCE NOR_FLASH_INSTANCE;
>
> +#pragma pack(1)
> typedef struct {
> VENDOR_DEVICE_PATH Vendor;
> + UINT8 Index;
> EFI_DEVICE_PATH_PROTOCOL End;
> } NOR_FLASH_DEVICE_PATH;
> +#pragma pack()
>
> struct _NOR_FLASH_INSTANCE {
> UINT32 Signature;
> --
> 2.17.1
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
