On Thu, Jan 19, 2017 at 01:49:04PM +0000, Ryan Harkin wrote:
> On 18 January 2017 at 23:27, Daniil Egranov <daniil.egra...@arm.com> wrote:
> > The Marvell Yukon MAC address load supported only on Juno R1 and R2.
> > It disabled for Juno R0 due to PCI issues on this board.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Daniil Egranov <daniil.egra...@arm.com>
>
> Tested-by: Ryan Harkin <ryan.har...@linaro.org>
>
> > ---
> > ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> > b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> > index 47ff587..e9e6990 100644
> > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> > @@ -378,6 +378,7 @@ OnEndOfDxe (
> > EFI_DEVICE_PATH_PROTOCOL* PciRootComplexDevicePath;
> > EFI_HANDLE Handle;
> > EFI_STATUS Status;
> > + UINT32 JunoRevision;
> >
> > //
> > // PCI Root Complex initialization
> > @@ -393,8 +394,12 @@ OnEndOfDxe (
> > Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath,
> > FALSE);
> > ASSERT_EFI_ERROR (Status);
> >
> > - Status = ArmJunoSetNicMacAddress ();
> > - ASSERT_EFI_ERROR (Status);
> > + GetJunoRevision (JunoRevision);
> > +
> > + if (JunoRevision != JUNO_REVISION_R0) {
> > + Status = ArmJunoSetNicMacAddress ();
> > + ASSERT_EFI_ERROR (Status);
>
> This is just an FYI, but I stacked your patch on top of mainline, like this:
>
> 5f81f61 2017-01-18 ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe:
> Fixed crash on Juno R0 [Daniil Egranov]
> 19ca06b 2017-01-19 OvmfPkg: Remove superfluous return statements.
> [Thomas Huth]
>
> The first time I ran this, Juno R0 worked fine, but on R1 and R2, the
> assert triggered:
>
> UEFI firmware (version 5f81f61 built at 11:56:52 on Jan 19 2017)
> [snip]
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT [ArmJunoDxe]
> /linaro/platforms/uefi/edk2/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c(401):
> !EFI_ERROR (Status)
>
> I worked out what is happening. And it's not to do with this patch.
> It's another fall-out from the re-work you did to the previous patch.
> It's also ultimately due to a bug the firmware.
>
> With the initial version of your "Set Marvell Yukon MAC address"
> patch, this hang didn't happen. I suspect that was because your error
> checking was weaker and certain PCIe failures didn't trigger the
> assert.
>
> To reproduce the error with this commit:
> 1) power on and boot R1 or R2 into Shell
> I do this by interrupting the boot by pressing ESCAPE and using the boot
> menu
> 2) At the Shell prompt, run "reset -s" to shutdown
> 3) At the ARM Boot Loader "Cmd>" prompt, run "reboot"
> 4) the board will hang while booting UEFI, assuming the board firmware
> doesn't die with constant messages like this:
>
> ERROR: PCIe CSR read failed to respond
> ERROR: SMBus transaction not claimed
>
> Assuming the problem is firmware, not EDK2, what should we do about it?
OK, so instinctively, my reaction was that "the reset -s bug is a
system controller firmware bug and we shouldn't work around
it". However, since it is actually disrupting Ryan's workflow, which
frequently doesn't touch PCI at all, I think downgrading the ASSERT to
an error message is a good idea short-term.
Daniil - could you make that change please?
/
Leif
> Prior to your "Set Marvell Yukon MAC address" patch, or with the
> earlier version, the board would boot anyway, but the Yukon device
> would be missing.
>
> Now it dies.
>
> I don't know which is worse, but I think hanging is worse than an
> ethernet port dropping out. Although hanging is a bit more obvious
> that there's a problem...
>
>
> > + }
> > }
> >
> > STATIC
> > --
> > 2.7.4
> >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
