-----Original Message-----
From: Ni, Ray <ray...@intel.com>
Sent: Monday, March 1, 2021 5:52 PM
To: Michael Kubacki <mikub...@linux.microsoft.com>;
devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>; Chiu, Chasel
<chasel.c...@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desim...@intel.com>; Luo, Heng <heng....@intel.com>;
Agyeman, Prince <prince.agye...@intel.com>; gaolim...@byosoft.com.cn;
Dong, Eric <eric.d...@intel.com>; Dong, Guo <guo.d...@intel.com>
Subject: RE: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib
Refactor
Michael,
I am good with that. I am also curious how you merge all the different SPI
flash implementation into one.
Thanks,
Ray
-----Original Message-----
From: Michael Kubacki <mikub...@linux.microsoft.com>
Sent: Tuesday, March 2, 2021 3:16 AM
To: devel@edk2.groups.io; Ni, Ray <ray...@intel.com>
Cc: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>; Chiu, Chasel
<chasel.c...@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desim...@intel.com>; Luo, Heng <heng....@intel.com>;
Agyeman, Prince <prince.agye...@intel.com>;
gaolim...@byosoft.com.cn; Dong, Eric <eric.d...@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib
Refactor
Hi Ray,
That sounds reasonable to me.
I was attempting to preserve the design that isolates the
silicon-specific logic to a library via an interface to a silicon
package. However, the real abstraction here is the firmware volume block
protocol which could simply be produced by a silicon driver with the
separation of such logic to a library being an implementation detail of
the driver.
In summary, here is the updated proposal:
1. Consolidate the library interface into a single header in
IntelSiliconPkg.
2. Consolidate the library implementation into a single instance in
IntelSiliconPkg.
3. Move SpiFvbServiceSmm out of MinPlatformPkg into IntelSiliconPkg.
4. Add SpiFvbServiceStandaloneMm to IntelSiliconPkg sharing
implementation with SpiFvbServiceSmm where appropriate.
Intel board packages would then use the SpiFlashCommonLib from
IntelSiliconPkg (a generation specific instance could be created if
needed) and use the SpiFvbServiceXyz driver from IntelSiliconPkg.
Please let me know if this is acceptable and I'd be happy to send the
patches.
Thanks,
Michael
On 3/1/2021 1:07 AM, Ni, Ray wrote:
Michael,
I agree with your thoughts to consolidate the lib header and
implementation to IntelSiliconPkg.
I didn't read the different implementations. But the implementation
consolidation means you see all the existing
implementations are the same. Right?
But why don't you put the driver in IntelSiliconPkg as well? Creating an
advanced feature for this fundamental service seems
over-kill.
Thanks,
Ray
-----Original Message-----
From: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>
Sent: Monday, March 1, 2021 4:46 PM
To: Ni, Ray <ray...@intel.com>
Subject: RE: [edk2-platforms][RFC] SpiFlashCommonLib Refactor
Did you get a chance to look into this ?
-----Original Message-----
From: Michael Kubacki <mikub...@linux.microsoft.com>
Sent: Tuesday, February 16, 2021 4:58 PM
To: devel@edk2.groups.io; Ni, Ray <ray...@intel.com>; Chaganty,
Rangasai V <rangasai.v.chaga...@intel.com>; Chiu,
Chasel
<chasel.c...@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desim...@intel.com>; Luo, Heng <heng....@intel.com>;
Agyeman, Prince <prince.agye...@intel.com>;
gaolim...@byosoft.com.cn; Dong, Eric <eric.d...@intel.com>
Subject: [edk2-platforms][RFC] SpiFlashCommonLib Refactor
Hello,
I'm planning to submit support for Standalone MM in
SpiFlashCommonLib soon. Currently, there's quite a bit of duplication
with
SpiFlashCommonLib.
I would like to have this Standalone MM support be available in as
consistent of a location as possible so I'd like to see if
there is
anything I can do to help clean this up in the early part of the patch
series.
The library interface is currently defined in the following header files:
1.
Platform\Intel\MinPlatformPkg\Include\Library\SpiFlashCommonLib.h
2. Silicon\Intel\SimicsIch10Pkg\Include\Library\SpiFlashCommonLib.h
3.
Silicon\Intel\KabylakeSiliconPkg\Pch\Include\Library\SpiFlashCommonLib.h
4.
Silicon\Intel\CoffeelakeSiliconPkg\Pch\Include\Library\SpiFlashCommonLib.
h
Instances of SmmSpiFlashCommonLib implementation exist in a mix of
platform and silicon packages:
1.
Silicon\Intel\SimicsIch10Pkg\Library\SmmSpiFlashCommonLib\SmmSpiFlashC
ommonLib.inf
2.
Platform\Intel\TigerlakeOpenBoardPkg\Library\SmmSpiFlashCommonLib\S
mmSpiFlashCommonLib.inf
3.
Silicon\Intel\KabylakeSiliconPkg\Pch\Library\SmmSpiFlashCommonLib\Smm
SpiFlashCommonLib.inf
4.
Silicon\Intel\CoffeelakeSiliconPkg\Pch\Library\SmmSpiFlashCommonLib\Sm
mSpiFlashCommonLib.inf
5.
Platform\Intel\MinPlatformPkg\Flash\Library\SpiFlashCommonLibNull\SpiFla
shCommonLibNull.inf
The library class is currently consumed in the following INFs:
1.
Platform\Intel\MinPlatformPkg\Flash\SpiFvbService\SpiFvbServiceSmm.inf
2.
Platform\Intel\MinPlatformPkg\Flash\SpiFvbService\SpiFvbServiceStandalon
eMm.inf
My understanding is:
1. The header file is defined in each silicon package because silicon
cannot depend upon platform (i.e. the MinPlatformPkg
header).
2. The header is present in each silicon package because the
implementation is silicon-specific and these packages cannot
depend on one another.
3. The header is defined in MinPlatformPkg because MinPlatformPkg
should be silicon vendor agnostic (cannot depend on the
silicon packages).
4. The header is needed in MinPlatformPkg because the SpiFvbService
there depends on SPI flash operations implemented in
SpiFlashCommonLib.
Here's an initial proposal:
1. Consolidate the library interface into a single header. In
IntelSiliconPkg?
2. Consolidate library implementation into a single instance. In
IntelSiliconPkg?
3. Move SpiFvbServiceXyz out of MinPlatformPkg.
3.a. Make a "SPI flash" feature?
3.b. Allow the Intel implementation of this feature to depend on
SpiFlashCommonLib defined in IntelSiliconPkg.
Intel board packages could then use the SpiFlashCommonLib from
IntelSiliconPkg (a generation specific instance could be created if
needed) and use the SpiFvbServiceXyz driver from the "SpiFlash"
feature.
Look forward to your thoughts and feedback.
Thanks,
Michael
