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:c0c::244; helo=mail-wr0-x244.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x244.google.com (mail-wr0-x244.google.com [IPv6:2a00:1450:400c:c0c::244]) (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 6A3592034A86B for ; Tue, 31 Oct 2017 20:21:37 -0700 (PDT) Received: by mail-wr0-x244.google.com with SMTP id u40so805761wrf.10 for ; Tue, 31 Oct 2017 20:25:29 -0700 (PDT) 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=HPA0PI59i52Dfy69TxBq8P6YyBJ5gb8BMP0s00gU4Do=; b=fVdpDIJRc5m01cEul2D0sLmJMYzaLlb/w3cq99MhrcsM87y2LNTwb91AHOO1pVyV1B D8g+MolhQBIo+r9F4o/NQ1atuD6X4uC7x/eWnW/Ko2m/+y/qrXK8IpE9P0PH/BQ+dBIs 8Am80MhhF51g+GizuP1rQfHMgcql6430ZvGAM= 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=HPA0PI59i52Dfy69TxBq8P6YyBJ5gb8BMP0s00gU4Do=; b=syHupXXdaaklhy63RD1K9JkvFZW8oeTatwzdpn6dUGezw5P/OA4hDaro9ysb7EF6c1 f+GW0OCFoWoRYUdbwWomlr4iKdv9KCEfiMpWCGy2PgHhEzIS0dsRauITD790mS28fO7c j6x0FLQ3CEYjAU7ZqC4GVati6LZl2j2Y5lT+csydzkKkjk+aOd8ouRvslRkEFBXcJsAf adE84VaKeQjH4Kl79j+pAQMIK287HGEqovnc5q/Otw4+6hnVFDS/zaJJJ7gCV3fv2i7q dhFRcuYGLHTouU/DDJ2zf5O7mSEb2595+zFRwygpFUHAkElmCSN5hKefIi2R4Jto7Dcf wxIA== X-Gm-Message-State: AMCzsaUCLbFylKaHKA0FrYJYZlLPun0baa/k2Yu2QchriMhcX5iKUtbU 4ztapereuDot7IdPAWfJPuvDvYf6UzQ= X-Google-Smtp-Source: ABhQp+SSFl1+/Odg0dxGg29CbAzfmJig6RLx6nG+Pw9YSjPolmfyMw96uX1SMQ0hqA5h0jUw4Rntxw== X-Received: by 10.223.156.133 with SMTP id d5mr3649147wre.29.1509506728149; Tue, 31 Oct 2017 20:25:28 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id 195sm7966764wmj.3.2017.10.31.20.25.26 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 31 Oct 2017 20:25:26 -0700 (PDT) Date: Wed, 1 Nov 2017 03:25:25 +0000 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel@lists.01.org, ard.biesheuvel@linaro.org, nadavh@marvell.com, neta@marvell.com, kostap@marvell.com, jinghua@marvell.com, jsd@semihalf.com Message-ID: <20171101032524.t56cnqir5sdfiqtl@bivouac.eciton.net> References: <1509422375-20198-1-git-send-email-mw@semihalf.com> <1509422375-20198-3-git-send-email-mw@semihalf.com> MIME-Version: 1.0 In-Reply-To: <1509422375-20198-3-git-send-email-mw@semihalf.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [platforms: PATCH 2/6] Marvell/Drivers: MvSpiFlash: Enable dynamic SPI Flash detection 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: Wed, 01 Nov 2017 03:21:37 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Oct 31, 2017 at 04:59:31AM +0100, Marcin Wojtas wrote: > Hitherto mechanism of fixing SPI flash model in the PCDs, > occured to be very inefficient and problematic. Enable > dynamic detection by reworking MvSpiFlashReadId() command, > which now uses newly added NorFlashInfoLib, that helps to > obtain description of the JEDEC compliant devices. > > This patch updates the MvSpiFlashProtocol ReadId() protocol > callback on both producer's (MvFlashDxe) and consumers' sides > (FirmwareUpdate and SpiTool applications). Because all > information about detected SPI NOR flash is now stored in > the obtained NorFlashInfo structure fields, use them instead > of the PCDs. > > Enable compilation of the NorFlashInfoLib and update > PortingGuide documentation accordingly. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas > --- > Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 +- > Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf | 4 +- > Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c | 5 +- > Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf | 2 +- > Platform/Marvell/Armada/Armada.dsc.inc | 1 + > Platform/Marvell/Armada/Armada70x0.dsc | 5 -- > Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c | 68 +++++++++++--------- > Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf | 9 +-- > Platform/Marvell/Drivers/Spi/MvSpiDxe.inf | 2 + > Platform/Marvell/Include/Protocol/Spi.h | 3 + > Platform/Marvell/Include/Protocol/SpiFlash.h | 6 +- > Platform/Marvell/Marvell.dec | 6 -- > Silicon/Marvell/Documentation/PortingGuide.txt | 18 ------ > 13 files changed, 51 insertions(+), 83 deletions(-) > > diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c > index d70645d..750e52a 100644 > --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c > +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c > @@ -94,12 +94,9 @@ SpiFlashProbe ( > ) > { > EFI_STATUS Status; > - UINT8 *FlashId; > - > - FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId); > > // Read SPI flash ID > - Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN, FlashId); > + Status = SpiFlashProtocol->ReadId (Slave, FALSE); > if (EFI_ERROR (Status)) { > return SHELL_ABORTED; > } > diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf > index 92c3333..53ea491 100644 > --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf > +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf > @@ -44,6 +44,7 @@ > FUpdate.uni > > [Packages] > + EmbeddedPkg/EmbeddedPkg.dec > MdeModulePkg/MdeModulePkg.dec > MdePkg/MdePkg.dec > Platform/Marvell/Marvell.dec > @@ -64,9 +65,6 @@ > UefiLib > UefiRuntimeServicesTableLib > > -[Pcd] > - gMarvellTokenSpaceGuid.PcdSpiFlashId > - > [Protocols] > gMarvellSpiFlashProtocolGuid > gMarvellSpiMasterProtocolGuid > diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c > index a12f2ec..68a6cf7 100644 > --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c > +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c > @@ -166,11 +166,8 @@ FlashProbe ( > ) > { > EFI_STATUS Status; > - UINT8 *FlashId; > > - FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId); > - > - Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN, FlashId); > + Status = SpiFlashProtocol->ReadId (Slave, FALSE); > if (EFI_ERROR (Status)) { > return SHELL_ABORTED; > } > diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf > index 887b9a5..a52906b 100644 > --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf > +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf > @@ -44,6 +44,7 @@ > SpiFlashCmd.uni > > [Packages] > + EmbeddedPkg/EmbeddedPkg.dec > MdePkg/MdePkg.dec > ShellPkg/ShellPkg.dec > MdeModulePkg/MdeModulePkg.dec > @@ -66,7 +67,6 @@ > > [Pcd] > gMarvellTokenSpaceGuid.PcdSpiFlashCs > - gMarvellTokenSpaceGuid.PcdSpiFlashId > gMarvellTokenSpaceGuid.PcdSpiFlashMode > > [Protocols] > diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc > index b9fc384..2cd96e6 100644 > --- a/Platform/Marvell/Armada/Armada.dsc.inc > +++ b/Platform/Marvell/Armada/Armada.dsc.inc > @@ -33,6 +33,7 @@ > ArmPlatformLib|Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf > ComPhyLib|Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf > MppLib|Platform/Marvell/Library/MppLib/MppLib.inf > + NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf > UtmiPhyLib|Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf > > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc > index 4d5f55f..8e4cdb2 100644 > --- a/Platform/Marvell/Armada/Armada70x0.dsc > +++ b/Platform/Marvell/Armada/Armada70x0.dsc > @@ -90,11 +90,6 @@ > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|10000000 > gMarvellTokenSpaceGuid.PcdSpiClockFrequency|200000000 > > - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0x70 > - gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3 > - gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536 > - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256 > - gMarvellTokenSpaceGuid.PcdSpiFlashId|{ 0x20, 0xBA, 0x18 } > gMarvellTokenSpaceGuid.PcdSpiFlashMode|3 > gMarvellTokenSpaceGuid.PcdSpiFlashCs|0 > > diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c > index ab3ed6a..703994c 100755 > --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c > +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c > @@ -107,10 +107,10 @@ MvSpiFlashWriteCommon ( > UINT8 PollBit = STATUS_REG_POLL_WIP; > UINT8 CheckStatus = 0x0; > > - CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd); > - if (CmdStatus == CMD_FLAG_STATUS) { > + if (Slave->Info->Flags & NF_WRITE_FSR) { > + CmdStatus = CMD_FLAG_STATUS; > PollBit = STATUS_REG_POLL_PEC; > - CheckStatus = PollBit; > + CheckStatus = STATUS_REG_POLL_PEC; > } > > // Send command > @@ -181,8 +181,19 @@ MvSpiFlashErase ( > UINTN EraseSize; > UINT8 Cmd[5]; > > - AddrSize = PcdGet32 (PcdSpiFlashAddressCycles); > - EraseSize = PcdGet64 (PcdSpiFlashEraseSize); > + if (Slave->Info->Flags & NF_4B_ADDR) { > + AddrSize = 4; > + } else { > + AddrSize = 3; > + } > + > + if (Slave->Info->Flags & NF_ERASE_4K) { > + Cmd[0] = CMD_ERASE_4K; > + EraseSize = SIZE_4KB; > + } else { Are 4 and 64K the only legal sizes? This patch deletes a detection of 32K size and the error message (and return) if an unknown size is encountered. > + Cmd[0] = CMD_ERASE_64K; > + EraseSize = Slave->Info->SectorSize; > + } > > // Check input parameters > if (Offset % EraseSize || Length % EraseSize) { > @@ -191,21 +202,6 @@ MvSpiFlashErase ( > return EFI_DEVICE_ERROR; > } > > - switch (EraseSize) { > - case SIZE_4KB: > - Cmd[0] = CMD_ERASE_4K; > - break; > - case SIZE_32KB: > - Cmd[0] = CMD_ERASE_32K; > - break; > - case SIZE_64KB: > - Cmd[0] = CMD_ERASE_64K; > - break; > - default: > - DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n")); > - return EFI_INVALID_PARAMETER; > - } > - > while (Length) { > EraseAddr = Offset; > > @@ -239,7 +235,11 @@ MvSpiFlashRead ( > UINT32 AddrSize, ReadAddr, ReadLength, RemainLength; > UINTN BankSel = 0; > > - AddrSize = PcdGet32 (PcdSpiFlashAddressCycles); > + if (Slave->Info->Flags & NF_4B_ADDR) { > + AddrSize = 4; > + } else { > + AddrSize = 3; > + } I see this sequence repeated many times. Could it be replaced by a macro or static helper function? Or cached in a device structure? / Leif