Re: [PATCH 1/2] ArmPlatformPkg/NorFlashDxe: use one GUID plus index to identify flash banks

[Leif Lindholm <leif.lindholm@linaro.org>]

On Mon, Nov 19, 2018 at 11:09:32AM -0800, Ard Biesheuvel wrote:
> > > @@ -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)
> >
> 
> Yes.
> 
> Before:
> 
> Mapping table
>      BLK1: Alias(s):
>           VenHw(F9B94AE2-8BA6-409B-9D56-B9B417F53CB3)
>      BLK0: Alias(s):
>           VenHw(8047DB4B-7E9C-4C0C-8EBC-DFBBAACACE8F)
> 
> After:
> 
> Mapping table
>      BLK0: Alias(s):
>           VenHw(93E34C7E-B50E-11DF-9223-2443DFD72085,00)
>      BLK1: Alias(s):
>           VenHw(93E34C7E-B50E-11DF-9223-2443DFD72085,01)

OK, I'm happy with that.
If Thomas/Nariman don't object:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

/
    Leif

> > 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
> > >