public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Thomas Abraham <thomas.abraham@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	edk2-devel@lists.01.org,
	Nariman Poushin <nariman.poushin@linaro.org>
Subject: Re: [PATCH edk2-platforms] Platform/ARM: replace hardcoded VenHW() device paths referring to NOR flash
Date: Tue, 20 Nov 2018 14:10:47 +0000	[thread overview]
Message-ID: <20181120141047.w6wicd56c3iuofey@bivouac.eciton.net> (raw)
In-Reply-To: <CAJuA9aixM=KS33SfvWHjm_MCriA8yoEU7PwXt7iKGJvRq9r5Qg@mail.gmail.com>

On Tue, Nov 20, 2018 at 07:31:34PM +0530, Thomas Abraham wrote:
> On Tue, Nov 20, 2018 at 1:01 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > On Mon, 19 Nov 2018 at 11:30, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > >
> > > Hmm, a follow-up question (which does not affect my r-b):
> > >
> > > We still keep the manually generated GUIDs in
> > > ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c,
> > > Platform/ARM/JunoPkg/Library/NorFlashJunoLib/NorFlashJuno.c,
> > > Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.c,
> > > Platform/ARM/VExpressPkg/Library/NorFlashArmVExpressLib/NorFlashArmVExpress.c,
> > > Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c,
> > >
> > > and indeed in the NOR_FLASH_DESCRIPTION struct in
> > > ArmPlatformPkg/Include/Library/NorFlashPlatformLib.h
> > >
> > > Is this an opportunity for a negative diffstat?
> >
> > Yes. I was anticipating some discussion on renaming the NOR flash
> > blocks, but indeed, we can drop all of the above once the rename
> > change is in.
> >
> >
> > > On Mon, Nov 19, 2018 at 07:12:47PM +0000, Leif Lindholm wrote:
> > > > On Fri, Nov 16, 2018 at 05:23:08PM -0800, Ard Biesheuvel wrote:
> > > > > The ArmPlatformPkg NOR flash driver has been updated to use device paths
> > > > > consisting of a fixed GUID and a numeric index rather than a separate GUID
> > > > > for each flash bank on a given system. This means all explicit device path
> > > > > references to NOR flash banks have to be brought up to date as well.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > >
> > > > This looks good to me, but I'd like a comment from Thomas/Nariman.
> > > >
> > > > With that:
> > > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> > > >
> 
> Tested this patch and "[PATCH 0/2] ArmPlatformPkg, ArmVirtPkg:
> discover NOR flash banks from DTB" patch series on the SGI-575
> platform. Access to NOR flash works fine with bank index as well.

That is good to know, but the platform I'm more concerned about is
Juno with its crazy bootmonfs. There are still plenty of those used in
test farms and suchlike.

> Should GUID's listed in all the instances of NOR_FLASH_DESCRIPTION in
> edk2-platforms be removed in this patch?

That was what I alluded to above. Ard prefers to do that as a separate
patch once this is all merged, and I don't mind that approach.

Regards,

Leif

> Thanks,
> Thomas.
> 
> 
> > > > > ---
> > > > >  Platform/ARM/JunoPkg/ArmJuno.dec                  | 2 +-
> > > > >  Platform/ARM/JunoPkg/ArmJuno.dsc                  | 2 +-
> > > > >  Platform/ARM/SgiPkg/SgiPlatform.dsc               | 2 +-
> > > > >  Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc | 2 +-
> > > > >  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc      | 2 +-
> > > > >  5 files changed, 5 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/Platform/ARM/JunoPkg/ArmJuno.dec b/Platform/ARM/JunoPkg/ArmJuno.dec
> > > > > index edbbb827ad45..c511796d8913 100644
> > > > > --- a/Platform/ARM/JunoPkg/ArmJuno.dec
> > > > > +++ b/Platform/ARM/JunoPkg/ArmJuno.dec
> > > > > @@ -44,7 +44,7 @@
> > > > >    gArmJunoTokenSpaceGuid.PcdSynopsysUsbEhciBaseAddress|0x7FFC0000|UINT32|0x00000005
> > > > >
> > > > >    # Juno Device Trees are loaded from NOR Flash
> > > > > -  gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/board.dtb"|VOID*|0x00000008
> > > > > +  gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath|L"VenHw(93E34C7E-B50E-11DF-9223-2443DFD72085,00)/board.dtb"|VOID*|0x00000008
> > > > >
> > > > >    # MHU Register base used by SCMI Mailbox transport
> > > > >    gArmJunoTokenSpaceGuid.PcdArmMtlDoorBell|0x2B1F0000|UINT64|0x00000024
> > > > > diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc b/Platform/ARM/JunoPkg/ArmJuno.dsc
> > > > > index ac3d63bd4d39..55b9d64d3deb 100644
> > > > > --- a/Platform/ARM/JunoPkg/ArmJuno.dsc
> > > > > +++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
> > > > > @@ -176,7 +176,7 @@
> > > > >    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|24
> > > > >
> > > > >    # List of Device Paths that support BootMonFs
> > > > > -  gArmBootMonFsTokenSpaceGuid.PcdBootMonFsSupportedDevicePaths|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)"
> > > > > +  gArmBootMonFsTokenSpaceGuid.PcdBootMonFsSupportedDevicePaths|L"VenHw(93E34C7E-B50E-11DF-9223-2443DFD72085,00)"
> > > > >
> > > > >    #
> > > > >    # ARM Architectural Timer Frequency
> > > > > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> > > > > index 3f0ca9872e79..06d866d6dc30 100644
> > > > > --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
> > > > > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> > > > > @@ -163,7 +163,7 @@
> > > > >    gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x1C170000
> > > > >
> > > > >    # List of Device Paths that support BootMonFs
> > > > > -  gArmBootMonFsTokenSpaceGuid.PcdBootMonFsSupportedDevicePaths|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)"
> > > > > +  gArmBootMonFsTokenSpaceGuid.PcdBootMonFsSupportedDevicePaths|L"VenHw(93E34C7E-B50E-11DF-9223-2443DFD72085,00)"
> > > > >
> > > > >    # ARM OS Loader
> > > > >    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
> > > > > diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc b/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc
> > > > > index 0542808f7d0c..3a3b4e3dbbc8 100644
> > > > > --- a/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc
> > > > > +++ b/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc
> > > > > @@ -172,7 +172,7 @@
> > > > >    #
> > > > >    # Define the device path to the FDT for the platform
> > > > >    #
> > > > > -  gFdtPlatformDxeTokenSpaceGuid.PcdFdtDevicePaths|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/ca15a7"
> > > > > +  gFdtPlatformDxeTokenSpaceGuid.PcdFdtDevicePaths|L"VenHw(93E34C7E-B50E-11DF-9223-2443DFD72085,00)/ca15a7"
> > > > >
> > > > >    #
> > > > >    # ARM Architectural Timer Frequency
> > > > > diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> > > > > index 752e525c3469..713c5637b074 100644
> > > > > --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> > > > > +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> > > > > @@ -355,7 +355,7 @@
> > > > >    #
> > > > >
> > > > >    # List of Device Paths that support BootMonFs
> > > > > -  gArmBootMonFsTokenSpaceGuid.PcdBootMonFsSupportedDevicePaths|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59);VenHw(1F15DA3C-37FF-4070-B471-BB4AF12A724A)"
> > > > > +  gArmBootMonFsTokenSpaceGuid.PcdBootMonFsSupportedDevicePaths|L"VenHw(93E34C7E-B50E-11DF-9223-2443DFD72085,00);VenHw(93E34C7E-B50E-11DF-9223-2443DFD72085,02)"
> > > > >
> > > > >    # RunAxf support via Dynamic Shell Command protocol
> > > > >    # We want to use the Shell Libraries but don't want it to initialise
> > > > > --
> > > > > 2.17.1
> > > > >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel


  parent reply	other threads:[~2018-11-20 14:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-17  1:23 [PATCH edk2-platforms] Platform/ARM: replace hardcoded VenHW() device paths referring to NOR flash Ard Biesheuvel
2018-11-19 19:12 ` Leif Lindholm
2018-11-19 19:29   ` Leif Lindholm
2018-11-19 19:31     ` Ard Biesheuvel
2018-11-20 14:01       ` Thomas Abraham
2018-11-20 14:02         ` Ard Biesheuvel
2018-11-20 14:10         ` Leif Lindholm [this message]
2018-11-20 14:30           ` Thomas Abraham

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181120141047.w6wicd56c3iuofey@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox