Re: [PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[Marcin Wojtas <mw@semihalf.com>]

2017-07-04 13:49 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> On 4 July 2017 at 12:47, Marcin Wojtas <mw@semihalf.com> wrote:
>> 2017-07-04 13:41 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
>>> On Tue, Jul 04, 2017 at 01:19:10PM +0200, Marcin Wojtas wrote:
>>>> 2017-07-04 13:10 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>>> > On 4 July 2017 at 12:04, Marcin Wojtas <mw@semihalf.com> wrote:
>>>> >> Hi,
>>>> >>
>>>> >> 2017-07-04 13:00 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>>> >>> Switch to the generic implementation of the ResetSystem() runtime
>>>> >>> service call, which is preferred over the one in EmbeddedPkg. This
>>>> >>> involves selecting another DXE runtime driver, and morphing the
>>>> >>> existing EfiResetSystemLib implementation into a ResetSystemLib one
>>>> >>> (which is what the generic driver depends on for platform glue)
>>>> >>>
>>>> >>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> >>> ---
>>>> >>>  Platforms/Marvell/Armada/Armada.dsc.inc                       |   4 +-
>>>> >>>  Platforms/Marvell/Armada/Armada70x0.fdf                       |   2 +-
>>>> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c   | 110 ++++++++++++++------
>>>> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf |   7 +-
>>>> >>>  4 files changed, 87 insertions(+), 36 deletions(-)
>>>> >>>
>>>> >>> diff --git a/Platforms/Marvell/Armada/Armada.dsc.inc b/Platforms/Marvell/Armada/Armada.dsc.inc
>>>> >>> index 475fc6d05379..32eaa2942564 100644
>>>> >>> --- a/Platforms/Marvell/Armada/Armada.dsc.inc
>>>> >>> +++ b/Platforms/Marvell/Armada/Armada.dsc.inc
>>>> >>> @@ -94,7 +94,7 @@
>>>> >>>    # Reset and Time libraries
>>>> >>>    EfiTimeBaseLib|OpenPlatformPkg/Library/EfiTimeBaseLib/EfiTimeBaseLib.inf
>>>> >>>    RealTimeClockLib|OpenPlatformPkg/Platforms/Marvell/Armada/Library/RealTimeClockLib/RealTimeClockLib.inf
>>>> >>> -  EfiResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>>>> >>> +  ResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>>>> >>
>>>> >> Isn't it better to switch to generic PSCI:
>>>> >> EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
>>>> >> and remove MvResetSystemLib whatsoever?
>>>> >>
>>>> >
>>>> > Yes. But Leif tells me that 7040 does not support PSCI?
>>>>
>>>> Leif, where do you have this information from?
>>>
>>> From the source code in OpenPlatformPkg, which does not use it and the
>>> documentation that does not mention it.
>>
>> Ok, long time ago in early SoC revision, we PSCI was indeed broken and
>> later, we were not aware of the ArmPsciResetSystemLib existence:)
>> On the WIP branch, we have a replacement commit:
>> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commit/b3fe877fe7207475ba293a1fc9b0a5c8f873d2c9
>> which will be submitted of course.
>>
>
> OK. But EfiSystemResetLib and EmbeddedPkg/ResetRuntimeDxe are going
> away, so this commit cannot be merged as-is.

I understand, I wasn't aware of those plans.

Thanks,
Marcin

>
> But since you are saying 7040 should use PSCI, I can update the patch
> and drop MvResetSystemLib entirely.