From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::241; helo=mail-wm0-x241.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) (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 3A55121A1096C for ; Sat, 25 Nov 2017 04:13:20 -0800 (PST) Received: by mail-wm0-x241.google.com with SMTP id b189so26361485wmd.0 for ; Sat, 25 Nov 2017 04:17:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ckTvyKIs4oRsOXF7UHjLpBud8J2SYc8MXGG58K+mLoQ=; b=ZQnhHxFZHtOILuZayh528VJocJIG8Ky6gOIFnfjim65cKNMUjxJsy8z2poUYi9WuMd H530EmfqZRx29wBwU3bDS7ENxGFkiw7mU/CsJr7Z7q7BPWl0klXJ3p/5+VgqIYrZWMlN DQkwdbDHsnZfonWRO3wD/nXrC3weqONo5J68c= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ckTvyKIs4oRsOXF7UHjLpBud8J2SYc8MXGG58K+mLoQ=; b=cxGpuGMOSCCXoxrWJEnl/wEGxVuaf4QJwav8hzAlJBQEhEQc+aP5CsbUUaNGNzkJZI 8i04U/S9g5QXkvBSmISwzubjUQChI/F21UTjQaOIfjamGAu3t/Q19f2wE8p29cB4pwiK ljtss6NHBvwptAFlk61GDVBSHHqpnYFTepydVcoAMIa6qor/q2NRDxZtNPXCM8QNRMgo nqJD3WErR3r79632HkrajjO4CZYQyCXNV7QJHzdiTiFtFA3wbCdT4aD+zZPcRpdMy/Vp 0N5IxW/ZsswRrwaCZGnBL7d9LcBJumSuNekQMwm6nBnOSfnczM9BVig0uYpkmSGmYu29 86xw== X-Gm-Message-State: AJaThX7ovcxC1mowiHd0Mwoqq7gFRMbJqA1LEmfp104I0KLVfrUBVFhI TTwHTmVE4+FUyz/EXatdSR00yw== X-Google-Smtp-Source: AGs4zMacSdJ+kySdoH2RyHSUjOyWtQf1YSaxvzDbqqna/o+r3P+cuPDHDicA1uuSE0MBOwEsQKDFDg== X-Received: by 10.28.196.70 with SMTP id u67mr13382097wmf.100.1511612257917; Sat, 25 Nov 2017 04:17:37 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id e131sm25144856wmg.1.2017.11.25.04.17.36 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 25 Nov 2017 04:17:36 -0800 (PST) Date: Sat, 25 Nov 2017 12:17:34 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, daniel.thompson@linaro.org, methavanitpong.pipat@socionext.com, masahisa.kojima@linaro.org, masami.hiramatsu@linaro.org Message-ID: <20171125121734.h3lizu3j55ph5du3@bivouac.eciton.net> References: <20171117190423.19511-1-ard.biesheuvel@linaro.org> <20171117190423.19511-2-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20171117190423.19511-2-ard.biesheuvel@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH edk2-platforms v5 1/6] 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: Sat, 25 Nov 2017 12:13:20 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Nov 17, 2017 at 07:04:18PM +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 > --- > v5: use MAX_UINT8 as 'not implemented' GPIO index So, you say that ... > Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 4 +++ > Platform/Socionext/DeveloperBox/DeveloperBox.fdf | 1 + > Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 4 +++ > Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf | 1 + > Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c | 30 +++++++++++++++++++- > Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf | 6 ++++ > Silicon/Socionext/SynQuacer/SynQuacer.dec | 3 ++ > 7 files changed, 48 insertions(+), 1 deletion(-) > > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc > index b73e88c5f29b..6c084efa9fb6 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 (GPIO pin PD[0] on the SoC) to clear the varstore > + gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0 > + > [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/DeveloperBox/DeveloperBox.fdf b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf > index 34100bb63da4..6cc523fac4f3 100644 > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf > @@ -258,6 +258,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/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf > INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > > diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc > index dd1469decc5d..c8a9f39cd1ae 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 (GPIO pin PD[0] on the SoC) 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..401cf3c81273 100644 > --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c > +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c > @@ -21,8 +21,11 @@ > #include > #include > #include > +#include > #include > > +#define CLEAR_SETTINGS_GPIO_NOT_IMPLEMENTED 0xff > + ...but then actually use 0xff. If you fold in MAX_UINT8 ... > STATIC > CONST DRAM_INFO *mDramInfo = (VOID *)(UINTN)FixedPcdGet64 (PcdDramInfoBase); > > @@ -103,10 +106,35 @@ PlatformPeim ( > VOID > ) > { > - EFI_STATUS Status; > + EMBEDDED_GPIO_PPI *Gpio; > + EFI_STATUS Status; > + UINTN Value; > + UINT8 Pin; > > ASSERT (mDramInfo->NumRegions > 0); > > + Pin = FixedPcdGet8 (PcdClearSettingsGpioPin); > + if (Pin != CLEAR_SETTINGS_GPIO_NOT_IMPLEMENTED) { > + Status = PeiServicesLocatePpi (&gEdkiiEmbeddedGpioPpiGuid, 0, NULL, > + (VOID **)&Gpio); > + ASSERT_EFI_ERROR (Status); > + > + Status = Gpio->Set (Gpio, Pin, 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, Pin, &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..cb3f836f5922 100644 > --- a/Silicon/Socionext/SynQuacer/SynQuacer.dec > +++ b/Silicon/Socionext/SynQuacer/SynQuacer.dec > @@ -30,3 +30,6 @@ [PcdsFixedAtBuild] > > gSynQuacerTokenSpaceGuid.PcdNetsecEepromBase|0|UINT32|0x00000002 > gSynQuacerTokenSpaceGuid.PcdNetsecPhyAddress|0|UINT8|0x00000003 > + > + # GPIO pin index [0 .. 31] or 0xFF for not implemented > + gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0xFF|UINT8|0x00000004 ...including here (the comment): Reviewed-by: Leif Lindholm > -- > 2.11.0 >