From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::242; helo=mail-io0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x242.google.com (mail-io0-x242.google.com [IPv6:2607:f8b0:4001:c06::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9AA5920349DAC for ; Fri, 17 Nov 2017 09:38:21 -0800 (PST) Received: by mail-io0-x242.google.com with SMTP id w127so9517773iow.11 for ; Fri, 17 Nov 2017 09:42:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=02aLJdp/4yFlvqTfx2/80Ph1OnKrhPmT36iYFaY95f4=; b=M57XUvFy6gqizi0Cjk4dHgL5NQE0S+FQtsrx5ScTNrTrljBSoK1hF1RLLqgejWvK/w umz5iPEpCGoJNbk8MWKjxOq4aVqzqWyF+LraRy5Sowt/qQ0zMYxLFNbu/a7xEA+5o6Oe PMxr4iuGid3gnNEKMLpu4Yj5UAQo1ewm3e1/c= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=02aLJdp/4yFlvqTfx2/80Ph1OnKrhPmT36iYFaY95f4=; b=M2JgD/kjo/hSb6m7s25GZZU+VETV7+y7yJw528fPGVsNHHHqd3A8YMZigCfAKO13bO 7NwD/t/ij0zpQ7hk9VKY/eROEK677RpjhraKm7GOKjGjgKxloLB/DWAHVMjSmYsDBW4n oYXyUvbHsO+bBWsRjIRL8lEao1yH/d/cPYJLw/SzfGZH+PXdeIEyr7IdApzzG+g7YXQ3 xLSidu85nQKZF9aCGJOxC2N1wR2F08tHl28owVtfgY1nRcb++rqQJZppaMzoutW2XeuR SL5RcP/bfJRZuUj1knCsaY0A6GyX7kjJpx7qqDmA7pxOgXQgEeoukg8/PfHsTpiSSHEV ZIpQ== X-Gm-Message-State: AJaThX7LZPU0BZxfG93NOCdHMlT1oYpLBRC2HBsKoicpME1+OEUt5CbJ p8Vrw+5Sae9WHDFJf/4ug9mem/c0gIq/Ht5y0LYJ+Q== X-Google-Smtp-Source: AGs4zMb/fKAk8EOtUf28VjPrDWYJ3rtc1isQrwpdr73HkvZxbkciDJbt9HRvmhH+QPHfNHinF/+xNuPyX4J8cvRdmPQ= X-Received: by 10.107.69.17 with SMTP id s17mr651320ioa.253.1510940551214; Fri, 17 Nov 2017 09:42:31 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.3 with HTTP; Fri, 17 Nov 2017 09:42:30 -0800 (PST) In-Reply-To: <20171117161046.uaxf4qhjzkdupzd5@bivouac.eciton.net> References: <20171110142127.12018-1-ard.biesheuvel@linaro.org> <20171110142127.12018-30-ard.biesheuvel@linaro.org> <20171117161046.uaxf4qhjzkdupzd5@bivouac.eciton.net> From: Ard Biesheuvel Date: Fri, 17 Nov 2017 17:42:30 +0000 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Daniel Thompson , Masami Hiramatsu , =?UTF-8?B?UGlwYXQv44Oh44K/44Ov44OL44OD44OI44Od44OzIOODlOODkeODg+ODiA==?= , Masahisa Kojima Subject: Re: [PATCH edk2-platforms v4 29/34] Silicon/SynQuacer: implement 'clear NVRAM' feature using a DIP switch X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Nov 2017 17:38:21 -0000 Content-Type: text/plain; charset="UTF-8" On 17 November 2017 at 16:10, Leif Lindholm wrote: > On Fri, Nov 10, 2017 at 02:21:22PM +0000, Ard Biesheuvel wrote: >> Ordinary computers typically have a physical switch or jumper on the >> board that allows non-volatile settings to be cleared. Let's implement >> the same using DIP switch #1 on block #3, and clear the EFI variable >> store if it is set to ON at boot time. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> --- >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 4 ++++ >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 4 ++++ >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf | 1 + >> Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c | 25 +++++++++++++++++++- >> Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf | 6 +++++ >> Silicon/Socionext/SynQuacer/SynQuacer.dec | 2 ++ >> 6 files changed, 41 insertions(+), 1 deletion(-) >> >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc >> index 10d070773cdc..af978db2c034 100644 >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc >> @@ -381,6 +381,9 @@ [PcdsFixedAtBuild.common] >> gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId|0x4f524e4c # LNRO >> gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision|1 >> >> + # set DIP switch DSW3-PIN1 to clear the varstore >> + gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0 >> + > > Right, so could you change this value to 1 ... > OK >> [PcdsPatchableInModule] >> gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0 >> gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0 >> @@ -418,6 +421,7 @@ [Components.common] >> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf >> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf >> MdeModulePkg/Universal/CapsulePei/CapsulePei.inf >> + Silicon/Socionext/SynQuacer/Drivers/SynQuacerGpioPei/SynQuacerGpioPei.inf >> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf { >> >> NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf >> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc >> index 4630d78bce93..4034bcfe82c5 100644 >> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc >> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc >> @@ -369,6 +369,9 @@ [PcdsFixedAtBuild.common] >> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08420000 >> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 >> >> + # set DIP switch DSW3-PIN1 to clear the varstore >> + gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0 >> + >> [PcdsPatchableInModule] >> gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0 >> gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0 >> @@ -406,6 +409,7 @@ [Components.common] >> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf >> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf >> MdeModulePkg/Universal/CapsulePei/CapsulePei.inf >> + Silicon/Socionext/SynQuacer/Drivers/SynQuacerGpioPei/SynQuacerGpioPei.inf >> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf { >> >> NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf >> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf >> index 365085c8f243..4577bd316a1f 100644 >> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf >> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf >> @@ -248,6 +248,7 @@ [FV.FVMAIN_COMPACT] >> INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf >> INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf >> INF MdeModulePkg/Universal/CapsulePei/CapsulePei.inf >> + INF Silicon/Socionext/SynQuacer/Drivers/SynQuacerGpioPei/SynQuacerGpioPei.inf >> INF RuleOverride = FMP_IMAGE_DESC Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf >> INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf >> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c >> index 358dd5a91f08..bd8ee7a368f5 100644 >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c >> @@ -21,6 +21,7 @@ >> #include >> #include >> #include >> +#include >> #include >> >> STATIC >> @@ -103,10 +104,32 @@ PlatformPeim ( >> VOID >> ) >> { >> - EFI_STATUS Status; >> + EMBEDDED_GPIO_PPI *Gpio; >> + EFI_STATUS Status; >> + UINTN Value; > > And use a local variable Pin, initialized to > FixedPcdGet32 (PcdClearSettingsGpioPin) - 1? > (You could then also have an assert verifying PcdClearSettingsGpioPin > != 0, to make fallback to default value flash warnings.) > Right. The DEBUG build already produces the NorFlashDxe blurb that the FV is being reinitialized. In any case, given that the SoC does number its GPIOs 0 - 31, the fact that developerbox DSW3 pins are 1 based does not mean 0 is unallocated. So I think it would make more sense to use UINT32_MAX as unused. >> >> ASSERT (mDramInfo->NumRegions > 0); >> >> + Status = PeiServicesLocatePpi (&gEdkiiEmbeddedGpioPpiGuid, 0, NULL, >> + (VOID **)&Gpio); >> + ASSERT_EFI_ERROR (Status); >> + >> + Status = Gpio->Set (Gpio, FixedPcdGet32 (PcdClearSettingsGpioPin), >> + GPIO_MODE_INPUT); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_WARN, "%a: failed to set GPIO as input - %r\n", __FUNCTION__, >> + Status)); >> + } else { >> + Status = Gpio->Get (Gpio, FixedPcdGet32 (PcdClearSettingsGpioPin), &Value); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_WARN, "%a: failed to get GPIO state - %r\n", __FUNCTION__, >> + Status)); >> + } else if (Value > 0) { >> + DEBUG ((DEBUG_INFO, "%a: clearing NVRAM\n", __FUNCTION__)); >> + PeiServicesSetBootMode (BOOT_WITH_DEFAULT_SETTINGS); >> + } >> + } >> + >> // >> // Record the first region into PcdSystemMemoryBase and PcdSystemMemorySize. >> // This is the region we will use for UEFI itself. >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf >> index 70eb715d44e3..a6501fb205e1 100644 >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf >> @@ -25,6 +25,7 @@ [Sources] >> >> [Packages] >> ArmPkg/ArmPkg.dec >> + EmbeddedPkg/EmbeddedPkg.dec >> MdePkg/MdePkg.dec >> MdeModulePkg/MdeModulePkg.dec >> Silicon/Socionext/SynQuacer/SynQuacer.dec >> @@ -40,11 +41,16 @@ [LibraryClasses] >> [FixedPcd] >> gArmTokenSpaceGuid.PcdFvBaseAddress >> gArmTokenSpaceGuid.PcdFvSize >> + gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin >> gSynQuacerTokenSpaceGuid.PcdDramInfoBase >> >> [Ppis] >> + gEdkiiEmbeddedGpioPpiGuid ## CONSUMES >> gSynQuacerDramInfoPpiGuid ## PRODUCES >> >> [Pcd] >> gArmTokenSpaceGuid.PcdSystemMemoryBase >> gArmTokenSpaceGuid.PcdSystemMemorySize >> + >> +[Depex] >> + gEdkiiEmbeddedGpioPpiGuid >> diff --git a/Silicon/Socionext/SynQuacer/SynQuacer.dec b/Silicon/Socionext/SynQuacer/SynQuacer.dec >> index 1a683b81521b..c11550469cd0 100644 >> --- a/Silicon/Socionext/SynQuacer/SynQuacer.dec >> +++ b/Silicon/Socionext/SynQuacer/SynQuacer.dec >> @@ -30,3 +30,5 @@ [PcdsFixedAtBuild] >> >> gSynQuacerTokenSpaceGuid.PcdNetsecEepromBase|0|UINT32|0x00000002 >> gSynQuacerTokenSpaceGuid.PcdNetsecPhyAddress|0|UINT8|0x00000003 >> + >> + gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0|UINT32|0x00000004 > > And add a small comment for this line that this refers to block 3 and > 0 means "don't use"? > Block 3 is platform specific not SoC specific.