Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Laszlo Ersek <lersek@redhat.com>]

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