From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22f.google.com (mail-wm0-x22f.google.com [IPv6:2a00:1450:400c:c09::22f]) (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 0CCF721EB88E3 for ; Fri, 1 Sep 2017 08:49:04 -0700 (PDT) Received: by mail-wm0-x22f.google.com with SMTP id u26so3931889wma.0 for ; Fri, 01 Sep 2017 08:51:48 -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=zF0xZUtvPuj51+n3MNA8BXRj1/IDIOXOzM9PjfmLsUU=; b=XiRpyEYuwG0eE/a2yf1Z1pptDig3Qws3cUUpWdXLEijTqAv8ELkCCdjNhC1JjtI/v1 1cQaFysEaXqQ4qq+ZTXnd6Etp37z8GxAD+iP5gGSH70DSrdBLUKiZ/wObmvMAKtVU43j NArkWSL45H0k0m1SzJBQxr1g3ZGlnQ2BgPID0= 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=zF0xZUtvPuj51+n3MNA8BXRj1/IDIOXOzM9PjfmLsUU=; b=mbBM6P9AlMY/rmD4ECuMo3Lk8BsxUj/XVoB0gyO9fPWAV3JptkubF92X3CrEf2y+S/ TKV/O1raH8n0VrtIt6E2zziTfYSP20tY8qD2rtTAuCk6sn7wdj0ZUu8MAoQCxkrH86y5 7Dz+n3aENDD74OKLwodlqcnZhuoKa+HivE+lDbeQjVzW4JATuLCZ1B3NTnNtfi9iC88X VY7yhQ/9A1yevEicgpUVbUHTjovGt6v4x3iv+B/LPln+IOAA9QKt4uczmciDOuvzZ0fi xOQnrmmRRqcm1DcxzW4sy5yGU2lpNdEMlGKMThOlp1stiYhceo/38mltYyiK9JW0hyBX hivg== X-Gm-Message-State: AHPjjUiGyzo7v9UYJbpkXs+u0MQHi7QObxtkBoNKOHugaTOZrNjgg3KF jXiTViYeNvykMCq2 X-Google-Smtp-Source: ADKCNb5i+2HjVa8Ycv3HoNznjZSIb3tNhHtTvcn4pyJG7Xk/T69hpTqnTRCPkhoFLOQITg8ymsrxwg== X-Received: by 10.28.109.142 with SMTP id b14mr656772wmi.65.1504281107160; Fri, 01 Sep 2017 08:51:47 -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 p12sm299532wrd.54.2017.09.01.08.51.45 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 01 Sep 2017 08:51:46 -0700 (PDT) Date: Fri, 1 Sep 2017 16:51:44 +0100 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, Neta Zur Hershkovits , Kostya Porotchkin , Hua Jing , Alexander Graf , semihalf-dabros-jan Message-ID: <20170901155144.zgkf23dwxfo5e5zj@bivouac.eciton.net> References: <1504271303-1782-1-git-send-email-mw@semihalf.com> <1504271303-1782-10-git-send-email-mw@semihalf.com> <20170901152144.mo5q4nabvqgbqic6@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter 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, 01 Sep 2017 15:49:04 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Sep 01, 2017 at 05:25:00PM +0200, Marcin Wojtas wrote: > 2017-09-01 17:21 GMT+02:00 Leif Lindholm : > > On Fri, Sep 01, 2017 at 03:08:21PM +0200, Marcin Wojtas wrote: > >> Although, hitherto support allowed for using configurable EraseSize, > >> the erase command was fixed to CMD_ERASE_64K. Also it was > >> assumed that EraseSize equals SectorSize, which is not true > >> for some flash devices. > > > > Another thing I immediately wish I had never learned. > > Thanks :/ > > Do you mean the code or flash quirks? :) The flash quirks... / Leif > >> Fix both issues by adding new PCD > >> (gMarvellTokenSpaceGuid.PcdSpiFlashPageSize) and using > >> this parameter properly in MvSpiFlashUpdate routine instead > >> of the EraseSize. Also erase command is adjusted to the settings. > >> Update PortingGuide accordingly. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Marcin Wojtas > >> --- > >> Platform/Marvell/Documentation/PortingGuide.txt | 3 +++ > >> Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c | 26 +++++++++++++++++----- > >> Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h | 6 +++++ > >> .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf | 1 + > >> Platform/Marvell/Marvell.dec | 1 + > >> 5 files changed, 31 insertions(+), 6 deletions(-) > >> > >> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt > >> index 3b79bd2..f637fee 100644 > >> --- a/Platform/Marvell/Documentation/PortingGuide.txt > >> +++ b/Platform/Marvell/Documentation/PortingGuide.txt > >> @@ -297,6 +297,9 @@ Folowing PCDs for spi flash driver configuration must be set properly: > >> - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize > >> (Size of SPI flash page) > >> > >> + - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize > >> + (Size of SPI flash sector, 65536 bytes by default) > >> + > >> - gMarvellTokenSpaceGuid.PcdSpiFlashId > >> (Id of SPI flash) > >> > >> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c > >> index 9a04493..f3fdba4 100755 > >> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c > >> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c > >> @@ -191,7 +191,21 @@ MvSpiFlashErase ( > >> return EFI_DEVICE_ERROR; > >> } > >> > >> - Cmd[0] = CMD_ERASE_64K; > >> + switch (EraseSize) { > >> + case SPI_ERASE_SIZE_4K: > >> + Cmd[0] = CMD_ERASE_4K; > >> + break; > >> + case SPI_ERASE_SIZE_32K: > >> + Cmd[0] = CMD_ERASE_32K; > >> + break; > >> + case SPI_ERASE_SIZE_64K: > >> + Cmd[0] = CMD_ERASE_64K; > >> + break; > >> + default: > >> + DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n")); > >> + return EFI_INVALID_PARAMETER; > >> + } > >> + > >> while (Length) { > >> EraseAddr = Offset; > >> > >> @@ -353,14 +367,14 @@ MvSpiFlashUpdate ( > >> ) > >> { > >> EFI_STATUS Status; > >> - UINT64 EraseSize, ToUpdate, Scale = 1; > >> + UINT64 SectorSize, ToUpdate, Scale = 1; > >> UINT8 *TmpBuf, *End; > >> > >> - EraseSize = PcdGet64 (PcdSpiFlashEraseSize); > >> + SectorSize = PcdGet64 (PcdSpiFlashSectorSize); > >> > >> End = Buf + ByteCount; > >> > >> - TmpBuf = (UINT8 *)AllocateZeroPool (EraseSize); > >> + TmpBuf = (UINT8 *)AllocateZeroPool (SectorSize); > >> if (TmpBuf == NULL) { > >> DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n")); > >> return EFI_OUT_OF_RESOURCES; > >> @@ -370,9 +384,9 @@ MvSpiFlashUpdate ( > >> Scale = (End - Buf) / 100; > >> > >> for (; Buf < End; Buf += ToUpdate, Offset += ToUpdate) { > >> - ToUpdate = MIN((UINT64)(End - Buf), EraseSize); > >> + ToUpdate = MIN((UINT64)(End - Buf), SectorSize); > >> Print (L" \rUpdating, %d%%", 100 - (End - Buf) / Scale); > >> - Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, EraseSize); > >> + Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, SectorSize); > >> > >> if (EFI_ERROR (Status)) { > >> DEBUG((DEBUG_ERROR, "SpiFlash: Error while updating\n")); > >> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h > >> index 3889643..646598a 100755 > >> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h > >> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h > >> @@ -57,6 +57,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > >> #define CMD_READ_ARRAY_FAST 0x0b > >> #define CMD_PAGE_PROGRAM 0x02 > >> #define CMD_BANK_WRITE 0xc5 > >> +#define CMD_ERASE_4K 0x20 > >> +#define CMD_ERASE_32K 0x52 > >> #define CMD_ERASE_64K 0xd8 > >> #define CMD_4B_ADDR_ENABLE 0xb7 > >> > >> @@ -66,6 +68,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > >> #define SPI_TRANSFER_BEGIN 0x01 // Assert CS before transfer > >> #define SPI_TRANSFER_END 0x02 // Deassert CS after transfers > >> > >> +#define SPI_ERASE_SIZE_4K 4096 > >> +#define SPI_ERASE_SIZE_32K 32768 > >> +#define SPI_ERASE_SIZE_64K 65536 > >> + > > > > Maybe just replace these with SIZE_4KB, SIZE_32KB and SIZE_64KB? > > > > Sure, will do. > > Marcin