ArmPkg/ArmPkg.dec | 2 - ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 8 +- ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +- ArmPlatformPkg/ArmPlatformPkg.dec | 4 - ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 13 +--- ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h | 6 -- ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 7 +- ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 22 +++--- ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf | 5 +- ArmVirtPkg/ArmVirtQemu.dsc | 1 + ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 + ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +- EmbeddedPkg/EmbeddedPkg.dec | 3 + EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h | 39 ++++++++++ EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c | 41 ++++++++++ EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 52 +++++++++++++ Omap35xxPkg/InterruptDxe/HardwareInterrupt.c | 81 +++++++++++++++----- Omap35xxPkg/InterruptDxe/InterruptDxe.inf | 6 +- 18 files changed, 230 insertions(+), 65 deletions(-) create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf create mode 100644 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c
Repo: https://github.com/lersek/edk2.git
Branch: depex_fixes
ArmVirtQemu boots again, it just took a few more patches than I expected
:)
Some of these patches will have to be ported to edk2-platforms, I think.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Steve Capper <steve.capper@linaro.org>
Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>
Thanks,
Laszlo
Laszlo Ersek (10):
Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
EmbeddedPkg: introduce NvVarStoreFormattedLib
ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
ArmPlatformPkg/NorFlashDxe: cue the variable driver with
NvVarStoreFormatted
ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
ArmPlatformPkg/PL031RealTimeClockLib: depend on
gEfiCpuArchProtocolGuid
ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
gEfiVariableArchProtocolGuid
ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
VariableRuntimeDxe
ArmPkg/ArmPkg.dec | 2 -
ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 8 +-
ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +-
ArmPlatformPkg/ArmPlatformPkg.dec | 4 -
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 13 +---
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h | 6 --
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 7 +-
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 22 +++---
ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf | 5 +-
ArmVirtPkg/ArmVirtQemu.dsc | 1 +
ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +-
EmbeddedPkg/EmbeddedPkg.dec | 3 +
EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h | 39 ++++++++++
EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c | 41 ++++++++++
EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 52 +++++++++++++
Omap35xxPkg/InterruptDxe/HardwareInterrupt.c | 81 +++++++++++++++-----
Omap35xxPkg/InterruptDxe/InterruptDxe.inf | 6 +-
18 files changed, 230 insertions(+), 65 deletions(-)
create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
create mode 100644 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h
create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c
--
2.14.1.3.gb7cf6e02401b
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote: > Repo: https://github.com/lersek/edk2.git > Branch: depex_fixes > > ArmVirtQemu boots again, it just took a few more patches than I expected > :) > > Some of these patches will have to be ported to edk2-platforms, I think. > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Steve Capper <steve.capper@linaro.org> > Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com> > > Thanks, > Laszlo > > Laszlo Ersek (10): > Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify > ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf" > ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex > EmbeddedPkg: introduce NvVarStoreFormattedLib > ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly > ArmPlatformPkg/NorFlashDxe: cue the variable driver with > NvVarStoreFormatted > ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid > ArmPlatformPkg/PL031RealTimeClockLib: depend on > gEfiCpuArchProtocolGuid > ArmVirtPkg/PlatformHasAcpiDtDxe: depend on > gEfiVariableArchProtocolGuid > ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into > VariableRuntimeDxe > Laszlo, Thanks a lot for taking care of this. I am glad we finally got rid of the BEFORE depex in the NOR flash driver, which has been problematic for years. For the series, Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> but please allow some time for Leif to chime in as well. Thanks, Ard. > ArmPkg/ArmPkg.dec | 2 - > ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 8 +- > ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +- > ArmPlatformPkg/ArmPlatformPkg.dec | 4 - > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 13 +--- > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h | 6 -- > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 7 +- > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 22 +++--- > ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf | 5 +- > ArmVirtPkg/ArmVirtQemu.dsc | 1 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 + > ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +- > EmbeddedPkg/EmbeddedPkg.dec | 3 + > EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h | 39 ++++++++++ > EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c | 41 ++++++++++ > EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 52 +++++++++++++ > Omap35xxPkg/InterruptDxe/HardwareInterrupt.c | 81 +++++++++++++++----- > Omap35xxPkg/InterruptDxe/InterruptDxe.inf | 6 +- > 18 files changed, 230 insertions(+), 65 deletions(-) > create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf > create mode 100644 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h > create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c > > -- > 2.14.1.3.gb7cf6e02401b > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 12 April 2018 at 11:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote: >> Repo: https://github.com/lersek/edk2.git >> Branch: depex_fixes >> >> ArmVirtQemu boots again, it just took a few more patches than I expected >> :) >> >> Some of these patches will have to be ported to edk2-platforms, I think. >> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Cc: Leif Lindholm <leif.lindholm@linaro.org> >> Cc: Steve Capper <steve.capper@linaro.org> >> Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com> >> >> Thanks, >> Laszlo >> >> Laszlo Ersek (10): >> Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify >> ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf" >> ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex >> EmbeddedPkg: introduce NvVarStoreFormattedLib >> ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly >> ArmPlatformPkg/NorFlashDxe: cue the variable driver with >> NvVarStoreFormatted >> ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid >> ArmPlatformPkg/PL031RealTimeClockLib: depend on >> gEfiCpuArchProtocolGuid >> ArmVirtPkg/PlatformHasAcpiDtDxe: depend on >> gEfiVariableArchProtocolGuid >> ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into >> VariableRuntimeDxe >> > > Laszlo, > > Thanks a lot for taking care of this. I am glad we finally got rid of > the BEFORE depex in the NOR flash driver, which has been problematic > for years. > > For the series, > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > but please allow some time for Leif to chime in as well. > > Thanks, > Ard. > > > Hi Laszlo, Thanks for this! I've booted with both DEBUG and RELEASE configurations on an FVP model using this series. I built this on Debian Stretch (using the AArch64 cross compiler). In order to test on FVP (which is defined in edk2-platforms), I made a simple modification to: edk2-platforms/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc (Basically mirroring what you did in patch #10) FWIW, feel free to add: Tested-by: Steve Capper <steve.capper@linaro.org> Cheers, -- Steve _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 04/12/18 15:39, Steve Capper wrote: > On 12 April 2018 at 11:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote: >>> Repo: https://github.com/lersek/edk2.git >>> Branch: depex_fixes >>> >>> ArmVirtQemu boots again, it just took a few more patches than I expected >>> :) >>> >>> Some of these patches will have to be ported to edk2-platforms, I think. >>> >>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> Cc: Leif Lindholm <leif.lindholm@linaro.org> >>> Cc: Steve Capper <steve.capper@linaro.org> >>> Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com> >>> >>> Thanks, >>> Laszlo >>> >>> Laszlo Ersek (10): >>> Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify >>> ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf" >>> ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex >>> EmbeddedPkg: introduce NvVarStoreFormattedLib >>> ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly >>> ArmPlatformPkg/NorFlashDxe: cue the variable driver with >>> NvVarStoreFormatted >>> ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid >>> ArmPlatformPkg/PL031RealTimeClockLib: depend on >>> gEfiCpuArchProtocolGuid >>> ArmVirtPkg/PlatformHasAcpiDtDxe: depend on >>> gEfiVariableArchProtocolGuid >>> ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into >>> VariableRuntimeDxe >>> >> >> Laszlo, >> >> Thanks a lot for taking care of this. I am glad we finally got rid of >> the BEFORE depex in the NOR flash driver, which has been problematic >> for years. >> >> For the series, >> >> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> but please allow some time for Leif to chime in as well. >> >> Thanks, >> Ard. >> >> >> > > Hi Laszlo, > Thanks for this! > > I've booted with both DEBUG and RELEASE configurations on an FVP model > using this series. > I built this on Debian Stretch (using the AArch64 cross compiler). > > In order to test on FVP (which is defined in edk2-platforms), I made a > simple modification to: > edk2-platforms/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc > (Basically mirroring what you did in patch #10) That's great; I'm relieved it wasn't more complex than that. :) > FWIW, feel free to add: > Tested-by: Steve Capper <steve.capper@linaro.org> Thank you -- I'll add your T-b to the following patches: #2, #3, #4, #5, #6, #7, #8. As those patches affect modules that seem to be used by "edk2-platforms/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc". Cheers! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 04/12/18 12:09, Ard Biesheuvel wrote: > On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote: >> Repo: https://github.com/lersek/edk2.git >> Branch: depex_fixes >> >> ArmVirtQemu boots again, it just took a few more patches than I expected >> :) >> >> Some of these patches will have to be ported to edk2-platforms, I think. >> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Cc: Leif Lindholm <leif.lindholm@linaro.org> >> Cc: Steve Capper <steve.capper@linaro.org> >> Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com> >> >> Thanks, >> Laszlo >> >> Laszlo Ersek (10): >> Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify >> ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf" >> ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex >> EmbeddedPkg: introduce NvVarStoreFormattedLib >> ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly >> ArmPlatformPkg/NorFlashDxe: cue the variable driver with >> NvVarStoreFormatted >> ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid >> ArmPlatformPkg/PL031RealTimeClockLib: depend on >> gEfiCpuArchProtocolGuid >> ArmVirtPkg/PlatformHasAcpiDtDxe: depend on >> gEfiVariableArchProtocolGuid >> ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into >> VariableRuntimeDxe >> > > Laszlo, > > Thanks a lot for taking care of this. I am glad we finally got rid of > the BEFORE depex in the NOR flash driver, which has been problematic > for years. > > For the series, > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Thank you! > > but please allow some time for Leif to chime in as well. Will certainly do! Cheers! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, Apr 12, 2018 at 12:09:46PM +0200, Ard Biesheuvel wrote: > On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote: > > Repo: https://github.com/lersek/edk2.git > > Branch: depex_fixes > > > > ArmVirtQemu boots again, it just took a few more patches than I expected > > :) > > > > Some of these patches will have to be ported to edk2-platforms, I think. > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > Cc: Steve Capper <steve.capper@linaro.org> > > Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com> > > > > Thanks, > > Laszlo > > > > Laszlo Ersek (10): > > Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify > > ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf" > > ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex > > EmbeddedPkg: introduce NvVarStoreFormattedLib > > ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly > > ArmPlatformPkg/NorFlashDxe: cue the variable driver with > > NvVarStoreFormatted > > ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid > > ArmPlatformPkg/PL031RealTimeClockLib: depend on > > gEfiCpuArchProtocolGuid > > ArmVirtPkg/PlatformHasAcpiDtDxe: depend on > > gEfiVariableArchProtocolGuid > > ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into > > VariableRuntimeDxe > > > > Laszlo, > > Thanks a lot for taking care of this. I am glad we finally got rid of > the BEFORE depex in the NOR flash driver, which has been problematic > for years. > > For the series, > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > but please allow some time for Leif to chime in as well. Well, there are a couple of places where I could nitpick on alphabetical sorting of includes, but none in files that weren't already disorderly. And like Ard, I am very grateful for this quite invasive (yet clean) bugfix. So, for 1-9/10: Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > > Thanks, > Ard. > > > > > ArmPkg/ArmPkg.dec | 2 - > > ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 8 +- > > ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +- > > ArmPlatformPkg/ArmPlatformPkg.dec | 4 - > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 13 +--- > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h | 6 -- > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 7 +- > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 22 +++--- > > ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf | 5 +- > > ArmVirtPkg/ArmVirtQemu.dsc | 1 + > > ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 + > > ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +- > > EmbeddedPkg/EmbeddedPkg.dec | 3 + > > EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h | 39 ++++++++++ > > EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c | 41 ++++++++++ > > EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 52 +++++++++++++ > > Omap35xxPkg/InterruptDxe/HardwareInterrupt.c | 81 +++++++++++++++----- > > Omap35xxPkg/InterruptDxe/InterruptDxe.inf | 6 +- > > 18 files changed, 230 insertions(+), 65 deletions(-) > > create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf > > create mode 100644 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h > > create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c > > > > -- > > 2.14.1.3.gb7cf6e02401b > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 04/12/18 19:23, Leif Lindholm wrote: > On Thu, Apr 12, 2018 at 12:09:46PM +0200, Ard Biesheuvel wrote: >> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote: >>> Repo: https://github.com/lersek/edk2.git >>> Branch: depex_fixes >>> >>> ArmVirtQemu boots again, it just took a few more patches than I expected >>> :) >>> >>> Some of these patches will have to be ported to edk2-platforms, I think. >>> >>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> Cc: Leif Lindholm <leif.lindholm@linaro.org> >>> Cc: Steve Capper <steve.capper@linaro.org> >>> Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com> >>> >>> Thanks, >>> Laszlo >>> >>> Laszlo Ersek (10): >>> Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify >>> ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf" >>> ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex >>> EmbeddedPkg: introduce NvVarStoreFormattedLib >>> ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly >>> ArmPlatformPkg/NorFlashDxe: cue the variable driver with >>> NvVarStoreFormatted >>> ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid >>> ArmPlatformPkg/PL031RealTimeClockLib: depend on >>> gEfiCpuArchProtocolGuid >>> ArmVirtPkg/PlatformHasAcpiDtDxe: depend on >>> gEfiVariableArchProtocolGuid >>> ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into >>> VariableRuntimeDxe >>> >> >> Laszlo, >> >> Thanks a lot for taking care of this. I am glad we finally got rid of >> the BEFORE depex in the NOR flash driver, which has been problematic >> for years. >> >> For the series, >> >> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> but please allow some time for Leif to chime in as well. > > Well, there are a couple of places where I could nitpick on > alphabetical sorting of includes, And, believe me, you would have my total agreement :), but in such cases, there's always a fork in the road: (a) add a separate patch that first sorts the includes and [LibraryClasses], without functional changes, or (b) just stick with the existing disorder, and get in and out as surgically as possible. My personal preference is (a), but it has drawn disagreement -- even accusations of pedantry :) :) --, and/or suggestions to squash such patches with functional changes, in the past, so I trod more lightly now. Rest assured, I didn't *miss* those places, I just elected to close my eyes! ;) > but none in files that weren't > already disorderly. And like Ard, I am very grateful for this quite > invasive (yet clean) bugfix. > > So, for 1-9/10: > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> Thank you! I shall get my TODO list (or, should I say, TODO "hash table", considering its cleanliness) in order, and then I'll get to pushing stuff. I *very* much appreciate the quick feedback on this! Cheers, Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Since you already have my r-b on the set, I'll pick up the style
topic, partly because I'm not sure if I've ever explained my
thinking publicly in words that anyone other than Ard understands.
On Thu, Apr 12, 2018 at 07:45:19PM +0200, Laszlo Ersek wrote:
> > Well, there are a couple of places where I could nitpick on
> > alphabetical sorting of includes,
>
> And, believe me, you would have my total agreement :), but in such
> cases, there's always a fork in the road: (a) add a separate patch that
> first sorts the includes and [LibraryClasses], without functional
> changes, or (b) just stick with the existing disorder, and get in and
> out as surgically as possible. My personal preference is (a), but it has
> drawn disagreement -- even accusations of pedantry :) :) --, and/or
> suggestions to squash such patches with functional changes, in the past,
> so I trod more lightly now. Rest assured, I didn't *miss* those places,
> I just elected to close my eyes! ;)
Right :)
So, yes - I do strongly agree with the idea of keeping functionality
and style changes separate (ask Evan), so that wasn't exactly what I
was referring to.
In general my internal "optimal" situation is one where the purely
functional diff leaves the code in a (quite subjectively, since it's
still not conformant) better state on average than it was.
My subjective mark of optimal is that which would minimise any
subsequent patch that was _only_ a style cleanup.
To pick an example from this set (1/10):
diff --git a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
index 6deb8c3f675c..61ad89af2758 100644
--- a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
+++ b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
@@ -41,14 +41,14 @@ [LibraryClasses]
PrintLib
UefiDriverEntryPoint
IoLib
ArmLib
[Protocols]
- gHardwareInterruptProtocolGuid
- gEfiCpuArchProtocolGuid
+ gHardwareInterruptProtocolGuid ## PRODUCES
+ gEfiCpuArchProtocolGuid ## CONSUMES ## NOTIFY
---
This one is pretty straightforward - without touching any non-modified
lines, these _could_ be reordered alphabetically.
(Yes, that change may have functional side-effects, but only on
undefined behaviour, so no different from rebasing to a version with
newer BaseTools can give you.)
Where the minimum diff logic comes in is in situations like (6/10):
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index 812dafd065b2..c40ac27a6599 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -28,12 +28,13 @@ [Sources.common]
NorFlashBlockIoDxe.c
[Packages]
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
ArmPlatformPkg/ArmPlatformPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
---
Applying my "optimal" rule here would have meant inserting the new
line before MdePkg/MdeModulePkg .decs instead. That way a cleanup
patch would end up doing
[Packages]
+ ArmPlatformPkg/ArmPlatformPkg.dec
EmbeddedPkg/EmbeddedPkg.dec
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
- ArmPlatformPkg/ArmPlatformPkg.dec
instead of
[Packages]
- MdePkg/MdePkg.dec
- MdeModulePkg/MdeModulePkg.dec
ArmPlatformPkg/ArmPlatformPkg.dec
EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
Anyway, this is all an aside - I just thought I would give you an
insight into the mind of Leif.
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 04/12/18 20:13, Leif Lindholm wrote: > Since you already have my r-b on the set, I'll pick up the style > topic, partly because I'm not sure if I've ever explained my > thinking publicly in words that anyone other than Ard understands. > > On Thu, Apr 12, 2018 at 07:45:19PM +0200, Laszlo Ersek wrote: >>> Well, there are a couple of places where I could nitpick on >>> alphabetical sorting of includes, >> >> And, believe me, you would have my total agreement :), but in such >> cases, there's always a fork in the road: (a) add a separate patch that >> first sorts the includes and [LibraryClasses], without functional >> changes, or (b) just stick with the existing disorder, and get in and >> out as surgically as possible. My personal preference is (a), but it has >> drawn disagreement -- even accusations of pedantry :) :) --, and/or >> suggestions to squash such patches with functional changes, in the past, >> so I trod more lightly now. Rest assured, I didn't *miss* those places, >> I just elected to close my eyes! ;) > > Right :) > > So, yes - I do strongly agree with the idea of keeping functionality > and style changes separate (ask Evan), so that wasn't exactly what I > was referring to. > > In general my internal "optimal" situation is one where the purely > functional diff leaves the code in a (quite subjectively, since it's > still not conformant) better state on average than it was. > > My subjective mark of optimal is that which would minimise any > subsequent patch that was _only_ a style cleanup. > > To pick an example from this set (1/10): > diff --git a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf > b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf > index 6deb8c3f675c..61ad89af2758 100644 > --- a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf > +++ b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf > @@ -41,14 +41,14 @@ [LibraryClasses] > PrintLib > UefiDriverEntryPoint > IoLib > ArmLib > > [Protocols] > - gHardwareInterruptProtocolGuid > - gEfiCpuArchProtocolGuid > + gHardwareInterruptProtocolGuid ## PRODUCES > + gEfiCpuArchProtocolGuid ## CONSUMES ## NOTIFY > > --- > This one is pretty straightforward - without touching any non-modified > lines, these _could_ be reordered alphabetically. > > (Yes, that change may have functional side-effects, but only on > undefined behaviour, so no different from rebasing to a version with > newer BaseTools can give you.) > > Where the minimum diff logic comes in is in situations like (6/10): > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > index 812dafd065b2..c40ac27a6599 100644 > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > @@ -28,12 +28,13 @@ [Sources.common] > NorFlashBlockIoDxe.c > > [Packages] > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > ArmPlatformPkg/ArmPlatformPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > > --- > Applying my "optimal" rule here would have meant inserting the new > line before MdePkg/MdeModulePkg .decs instead. That way a cleanup > patch would end up doing > > [Packages] > + ArmPlatformPkg/ArmPlatformPkg.dec > EmbeddedPkg/EmbeddedPkg.dec > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > - ArmPlatformPkg/ArmPlatformPkg.dec > > instead of > > [Packages] > - MdePkg/MdePkg.dec > - MdeModulePkg/MdeModulePkg.dec > ArmPlatformPkg/ArmPlatformPkg.dec > EmbeddedPkg/EmbeddedPkg.dec > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > > > Anyway, this is all an aside - I just thought I would give you an > insight into the mind of Leif. I do see the point -- basically, even if we don't imlement separate cleanups, and just add the functional changes on top of whatever we already have, if there are multiple ways to keep the functional changes purely functional and focused, we had better select the one that at least doesn't make the style worse. Using your [Packages] example, it's possible *not* to increase the disorder, without actually sorting those lines. I fully admit this eluded me -- I considered "messy" all-or-nothing. I'll try to remember this next time. (It's honestly a bit difficult for me, because if I take the time & effort to be considerate like this, I'd mostly (though perhaps not always) just write the patch that cleans up first.) Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 04/12/18 02:55, Laszlo Ersek wrote: > Repo: https://github.com/lersek/edk2.git > Branch: depex_fixes > > ArmVirtQemu boots again, it just took a few more patches than I expected > :) > > Some of these patches will have to be ported to edk2-platforms, I think. > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Steve Capper <steve.capper@linaro.org> > Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com> Pushed as commit range 153f5c7a93be..bf453d581ecf. Thank you all! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Thanks Laszlo.
It works for me too.
Supreeth
-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Wednesday, April 11, 2018 7:56 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Leif Lindholm <leif.lindholm@linaro.org>; Steve Capper <steve.capper@linaro.org>; Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>
Subject: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes
Repo: https://github.com/lersek/edk2.git
Branch: depex_fixes
ArmVirtQemu boots again, it just took a few more patches than I expected
:)
Some of these patches will have to be ported to edk2-platforms, I think.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Steve Capper <steve.capper@linaro.org>
Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>
Thanks,
Laszlo
Laszlo Ersek (10):
Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
EmbeddedPkg: introduce NvVarStoreFormattedLib
ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
ArmPlatformPkg/NorFlashDxe: cue the variable driver with
NvVarStoreFormatted
ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
ArmPlatformPkg/PL031RealTimeClockLib: depend on
gEfiCpuArchProtocolGuid
ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
gEfiVariableArchProtocolGuid
ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
VariableRuntimeDxe
ArmPkg/ArmPkg.dec | 2 -
ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 8 +-
ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +-
ArmPlatformPkg/ArmPlatformPkg.dec | 4 -
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 13 +---
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h | 6 --
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 7 +-
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 22 +++---
ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf | 5 +-
ArmVirtPkg/ArmVirtQemu.dsc | 1 +
ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +-
EmbeddedPkg/EmbeddedPkg.dec | 3 +
EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h | 39 ++++++++++
EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c | 41 ++++++++++
EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 52 +++++++++++++
Omap35xxPkg/InterruptDxe/HardwareInterrupt.c | 81 +++++++++++++++-----
Omap35xxPkg/InterruptDxe/InterruptDxe.inf | 6 +-
18 files changed, 230 insertions(+), 65 deletions(-) create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
create mode 100644 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h
create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c
--
2.14.1.3.gb7cf6e02401b
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 04/12/18 18:51, Supreeth Venkatesh wrote: > Thanks Laszlo. > It works for me too. Appreciate the testing! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.