public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	ray.ni@intel.com, rangasai.v.chaganty@intel.com,
	chasel.chiu@intel.com, nathaniel.l.desimone@intel.com,
	heng.luo@intel.com, prince.agyeman@intel.com,
	gaoliming@byosoft.com.cn, "Dong, Eric" <eric.dong@intel.com>
Subject: [edk2-platforms][RFC] SpiFlashCommonLib Refactor
Date: Tue, 16 Feb 2021 16:58:16 -0800	[thread overview]
Message-ID: <2a2a1cc0-3de9-fbd5-1bfb-eb5dbc9d1bc6@linux.microsoft.com> (raw)

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\SmmSpiFlashCommonLib.inf

2. 
Platform\Intel\TigerlakeOpenBoardPkg\Library\SmmSpiFlashCommonLib\SmmSpiFlashCommonLib.inf

3. 
Silicon\Intel\KabylakeSiliconPkg\Pch\Library\SmmSpiFlashCommonLib\SmmSpiFlashCommonLib.inf

4. 
Silicon\Intel\CoffeelakeSiliconPkg\Pch\Library\SmmSpiFlashCommonLib\SmmSpiFlashCommonLib.inf

5. 
Platform\Intel\MinPlatformPkg\Flash\Library\SpiFlashCommonLibNull\SpiFlashCommonLibNull.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\SpiFvbServiceStandaloneMm.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

             reply	other threads:[~2021-02-17  0:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-17  0:58 Michael Kubacki [this message]
     [not found] ` <DM6PR11MB44760BD0B29FD9074047A984B69A9@DM6PR11MB4476.namprd11.prod.outlook.com>
2021-03-01  9:07   ` [edk2-platforms][RFC] SpiFlashCommonLib Refactor Ni, Ray
2021-03-01 19:16     ` [edk2-devel] " Michael Kubacki
2021-03-02  0:52       ` Ni, Ray
2021-03-03 21:58         ` Guo Dong
2021-03-03 22:54           ` Michael Kubacki
2021-03-03 23:22             ` Guo Dong
2021-03-03 23:38               ` Ni, Ray
2021-03-03 23:59                 ` Bret Barkelew
2021-03-04  0:54                   ` Ni, Ray
2021-03-04  1:49                     ` Michael Kubacki

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=2a2a1cc0-3de9-fbd5-1bfb-eb5dbc9d1bc6@linux.microsoft.com \
    --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