Re: [PATCH 0/3] MdeModulePkg/ResetSystem: Implement ResetNotification protocol

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

On Sat, Jul 01, 2017 at 11:04:08PM +0200, Laszlo Ersek wrote:
> Ard, Leif,
> 
> On 06/29/17 10:32, Ruiyu Ni wrote:
> > Ruiyu Ni (3):
> >   MdePkg: Add ResetNotification protocol definition
> >   MdeModulePkg/ResetSystem: Remove unnecessary global variable
> >   MdeModulePkg/ResetSystem: Implement ResetNotification protocol
> > 
> >  .../Universal/ResetSystemRuntimeDxe/ResetSystem.c  | 146 +++++++++++++++++++--
> >  .../Universal/ResetSystemRuntimeDxe/ResetSystem.h  |  21 ++-
> >  .../ResetSystemRuntimeDxe.inf                      |   5 +-
> >  MdePkg/Include/Protocol/ResetNotification.h        |  86 ++++++++++++
> >  MdePkg/MdePkg.dec                                  |   3 +
> >  5 files changed, 247 insertions(+), 14 deletions(-)
> >  create mode 100644 MdePkg/Include/Protocol/ResetNotification.h
> > 
> 
> I think we should
> - either port this feature (patch 3/3) to EmbeddedPkg/ResetRuntimeDxe,
> - or else rebase all platforms that consume EmbeddedPkg/ResetRuntimeDxe
> to MdeModulePkg/Universal/ResetSystemRuntimeDxe, and delete
> EmbeddedPkg/ResetRuntimeDxe from the tree.
> 
> What do you guys think?

I think deleting the EmbeddedPkg one, and making the current consumers
implement ResetSystemLib instead of EfiResetSystemLib would be an
improvement.

At a quick skim, the only functionality I can see added in
EmbeddedPkg/ResetRuntimeDxe is the LibInitializeResetSystem
function. The only (ARM) platform I can see doing anything useful
there is the Armada ... and that code could move.

/
    Leif

> Other producers of gEfiResetArchProtocolGuid could be affected as well
> (just from a quick grep):
> - DuetPkg/AcpiResetDxe
> - EmulatorPkg/ResetRuntimeDxe
> - Nt32Pkg/ResetRuntimeDxe
> 
> Thanks
> Laszlo