Re: [edk2-devel] [edk2] [PATCH v2 edk2-platforms] Platform/ARM/Drivers: Add Nor Flash Driver

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Mon, 29 Apr 2019 at 07:59, Jagadeesh Ujja <jagadeesh.ujja@arm.com> wrote:
>
> hi Ard
>
> On Wed, Apr 24, 2019 at 6:37 PM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > On Wed, 24 Apr 2019 at 11:05, Jagadeesh Ujja <jagadeesh.ujja@arm.com> wrote:
> > >
> > > Fwd to new EDKII development mailing list.
> > >
> > > hi Ard,
> > >
> > > Sorry about missing your previous comment and adding here.
> > >
> > >  "Hello Jagadeesh,
> > >
> > >  Why are you moving this driver into edk2-platforms? I'd prefer to have
> > >
> > > it alongside the non-MM version instead.
> > >
> > > That would allow us to share a lot more code between the two versions."
> > > -----------------
> > >
> > > my Point of view:
> > >
> > > In ArmPlatformPkg/Drivers/NorFlashDxe driver supports block I/O and
> > > disk  I/O protocols.
> > > In case of StandaloneMM norflash driver the block I/O and disk I/O
> > > protocols are not required
> > >
> > > We don’t want to remove support of block I/O and disk I/O protocols in
> > > “ArmPlatformPkg/Drivers/NorFlashDxe”
> > > In this case “NOR_FLASH_INSTANCE” is different in both
> > > drivers(DXE/STANDALONEMM). So we end up with  having 2 different
> > > drivers.
> > >
> > > Even if we want to share common code then we end up with multiple
> > > files as  “NOR_FLASH_INSTANCE” is used in almost all files
> > > Ex: “NorFlash.c”  “NorFlash.h” NorFlashFvb.c
> > > In this case “NorFlash.c”  will have 2 copies one used by DXE and
> > > other by STANDALONE_MM driver
> > >
> > > So instead of having 2 different drivers in one place
> > > “ArmPlatformPkg/Drivers/NorFlashDxe”
> > > added a new refactored nor flash driver in "Platform/ARM/Drivers"
> > >
> > > I am not sure if any other platform is using
> > > “ArmPlatformPkg/Drivers/NorFlashDxe” and may require block I/O and
> > > disk  I/O protocols
> > > if we remove “support of block I/O and disk  I/O protocols” then we
> > > can share the common code easily.
> > >
> >
> > I see your point.
> >
> > The only reason you need block i/o and disk i/o is to use ordinary
> > partition and filesystem drivers. I guess Juno uses this for its magic
> > filesystem? If not, I'd be glad to get rid of it.
> >
> >
> Yes  "block i/o and disk i/o  of “ArmPlatformPkg/Drivers/NorFlashDxe”
> driver is used even
> in RD-platforms(edk2-platforms/Platform/ARM/SgiPkg/)(and JUNO) to list
> partition. we don't want to break
> legacy and want to have separate
> "Platform/ARM/Drivers/StandaloneMmNorFlash/" driver.
>

In that case, could we move the existing code into
edk2-platforms/Platform/ARM first? And then rip out the block i/o and
disk i/o parts from the common driver?

Leif, any thoughts?