From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 50E6A740047 for ; Thu, 2 May 2024 13:17:14 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=A00KMz6x4kmqNRQMHEw1xAwR1o8mWQujo/tBxkQR4HU=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20240206; t=1714655832; v=1; b=A921MFJViGRLkU5ZPSMs9cBsC2GErQzmLX5tEYWZ6JiKVjLXL/A+uuruggCZz3EgjmiChLlp lsNtYyWpuLdWmROGLIJw89kJ+QKzDwjktMGSmK4JAP7mmbMHDRIJruqchfUACbVAWLIqHczLXy7 uagOOhN/GswQmlTD3/1bVSr7lj4+x8Z6NeimDIw95hs4ghI0c6K/qYQXSLrzzWY7MR9l2CKOAna 1bNC7PzsHs8B9xzgAGZWhnKUCWAOaGvNwBuObILxBW/q+xfI16EGMG3o6VjxOI/s2zvKevJycXn MOtLqD/72jFcnGlDeSVZwMq7+2jMC3OdUZkgf5I+1r6dg== X-Received: by 127.0.0.2 with SMTP id mnoQYY7687511xbEAmrsUueV; Thu, 02 May 2024 06:17:12 -0700 X-Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.5014.1714655831995790212 for ; Thu, 02 May 2024 06:17:12 -0700 X-Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 467D82F4; Thu, 2 May 2024 06:17:37 -0700 (PDT) X-Received: from [10.34.111.156] (e126645.nice.Arm.com [10.34.111.156]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AA5543F71E; Thu, 2 May 2024 06:17:09 -0700 (PDT) Message-ID: <8f292b6a-4c18-48c5-8e20-5eb9c86538e0@arm.com> Date: Thu, 2 May 2024 15:17:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH RESEND edk2-platforms][PATCH V2 12/14] Platform/ARM: Add CadenceQspiNorFlashDeviceLib for NorFlashDxe To: devel@edk2.groups.io, sahil.kaushal@arm.com Cc: Ard Biesheuvel , Leif Lindholm , Sami Mujawar , sahil , "levi.yun" References: <20240423055638.1271531-1-Sahil.Kaushal@arm.com> <20240423055638.1271531-13-Sahil.Kaushal@arm.com> From: "PierreGondois" In-Reply-To: <20240423055638.1271531-13-Sahil.Kaushal@arm.com> Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Thu, 02 May 2024 06:17:12 -0700 Resent-From: pierre.gondois@arm.com Reply-To: devel@edk2.groups.io,pierre.gondois@arm.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: vZ3lk78e8IuwlJve1RxLXKvix7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=A921MFJV; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=arm.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io Hello Sahil, On 4/23/24 07:56, Sahil Kaushal via groups.io wrote: > From: sahil >=20 > In N1Sdp platform, the SoC is connected to IOFPGA which has a > Cadence Quad SPI (QSPI) controller. This QSPI controller manages > the flash chip device via QSPI bus. >=20 > This patch adds CadenceQspiNorFlashDeviceLib which is used to > manage and access the above configuration. >=20 > Signed-off-by: sahil > --- > Platform/ARM/Library/CadenceQspiNorFlashDeviceLib/CadenceQspiNorFlashDe= viceLib.inf | 32 + > Platform/ARM/Library/CadenceQspiNorFlashDeviceLib/CadenceQspiNorFlashDe= viceLib.h | 44 + > Platform/ARM/Library/CadenceQspiNorFlashDeviceLib/CadenceQspiNorFlashDe= viceLib.c | 1011 ++++++++++++++++++++ > 3 files changed, 1087 insertions(+) >=20 [snip] > + > +/** > + Converts milliseconds into number of ticks of the performance counter. > + > + @param[in] Milliseconds Milliseconds to convert into ticks. > + > + @retval Milliseconds expressed as number of ticks. > + > +**/ > +STATIC > +UINT64 > +MilliSecondsToTicks ( > + IN UINTN Milliseconds > + ) > +{ > + CONST UINT64 NanoSecondsPerTick =3D GetTimeInNanoSecond (1); > + > + return (Milliseconds * 1000000) / NanoSecondsPerTick; Should use DivU64x64Remainder() here: { UINT64 NanoSecondsPerTick; UINT64 NanoSeconds; NanoSecondsPerTick =3D GetTimeInNanoSecond (1); NanoSeconds =3D MultU64x32 (Milliseconds, 1000000); return DivU64x64Remainder (NanoSeconds, NanoSecondsPerTick, NULL); } > +} > + > +/** > + Poll Status register for NOR flash erase/write completion. > + > + @param[in] Instance NOR flash Instance. > + > + @retval EFI_SUCCESS Request is executed successfully. > + @retval EFI_TIMEOUT Operation timed out. > + @retval EFI_DEVICE_ERROR Controller operartion failed. operartion -> typo (same at another place I think) [snip] > + > +/** > + Read from nor flash. > + > + @param[in] Instance NOR flash Instance of variable s= tore region. > + @param[in] Lba The starting logical block index= to read from. > + @param[in] Offset Offset into the block at which t= o begin reading. > + @param[in] BufferSizeInBytes The number of bytes to read. > + @param[out] Buffer The pointer to a caller-allocate= d buffer that > + should copied with read data. > + > + @retval EFI_SUCCESS The read is completed. > + @retval EFI_INVALID_PARAMETER Invalid parameters passed. > +**/ > +EFI_STATUS > +NorFlashRead ( > + IN NOR_FLASH_INSTANCE *Instance, > + IN EFI_LBA Lba, > + IN UINTN Offset, > + IN UINTN BufferSizeInBytes, > + OUT VOID *Buffer > + ) > +{ > + UINTN StartAddress; > + > + // The buffer must be valid > + if (Buffer =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + // Return if we do not have any byte to read > + if (BufferSizeInBytes =3D=3D 0) { > + return EFI_SUCCESS; > + } > + > + if (((Lba * Instance->Media.BlockSize) + Offset + BufferSizeInBytes) > > + Instance->Size) > + { > + DEBUG (( > + DEBUG_ERROR, > + "NorFlashRead: ERROR - Read will exceed device size.\n" > + )); > + return EFI_INVALID_PARAMETER; > + } > + > + // Get the address to start reading from > + StartAddress =3D GET_NOR_BLOCK_ADDRESS ( > + Instance->RegionBaseAddress, > + Lba, > + Instance->Media.BlockSize > + ); > + > + // Readout the data > + CopyMem (Buffer, (UINTN *)(StartAddress + Offset), BufferSizeInBytes); The original code at: Platform/ARM/Library/P30NorFlashDeviceLib/P30NorFlashDeviceLib.c implements and uses AlignedCopyMem()/NorFlashWriteBuffer() which seems to be more efficient. Just to be sure I understand correctly, is the maximal read/write size of 4 bytes ? Meaning that these functions are not needed ? --- NorFlashWriteBuffer() is not implemented here IIUC won't be implemtned as n= ot needed. Maybe in an additional patch, the function could be removed from th= e library interface at: Platform/ARM/Include/Library/NorFlashDeviceLib.h and made static in: Platform/ARM/Library/P30NorFlashDeviceLib/P30NorFlashDeviceLib.c > + > + return EFI_SUCCESS; > +} > + > +/** > + Write a full or portion of a block. > + > + @param[in] Instance NOR flash Instance of variable = store region. > + @param[in] Lba The starting logical block inde= x to write to. > + @param[in] Offset Offset into the block at which = to begin writing. > + @param[in, out] NumBytes The total size of the buffer. > + @param[in] Buffer The pointer to a caller-allocat= ed buffer that > + contains the source for the wri= te. > + > + @retval EFI_SUCCESS The write is completed. > + @retval EFI_OUT_OF_RESOURCES Invalid Buffer passed. > + @retval EFI_BAD_BUFFER_SIZE Buffer size not enough. > + @retval EFI_DEVICE_ERROR The device reported an error. > +**/ > +EFI_STATUS > +NorFlashWriteSingleBlock ( > + IN NOR_FLASH_INSTANCE *Instance, > + IN EFI_LBA Lba, > + IN UINTN Offset, > + IN OUT UINTN *NumBytes, > + IN UINT8 *Buffer > + ) > +{ > + EFI_STATUS Status; > + UINT32 Tmp; > + UINT32 TmpBuf; > + UINT32 WordToWrite; > + UINT32 Mask; > + BOOLEAN DoErase; > + UINTN BytesToWrite; > + UINTN CurOffset; > + UINTN WordAddr; > + UINTN BlockSize; > + UINTN BlockAddress; > + UINTN PrevBlockAddress; > + > + if (Buffer =3D=3D NULL) { > + DEBUG (( > + DEBUG_ERROR, > + "NorFlashWriteSingleBlock: ERROR - Buffer is invalid\n" > + )); > + return EFI_OUT_OF_RESOURCES; EFI_INVALID_PARAMETER instead I think > + } > + > + PrevBlockAddress =3D 0; > + > + DEBUG (( > + DEBUG_INFO, > + "NorFlashWriteSingleBlock(Parameters: Lba=3D%ld, Offset=3D0x%x, " > + "*NumBytes=3D0x%x, Buffer @ 0x%08x)\n", > + Lba, > + Offset, > + *NumBytes, > + Buffer > + )); > + > + // Localise the block size to avoid de-referencing pointers all the ti= me Localise -> Locate > + BlockSize =3D Instance->Media.BlockSize; > + [snip] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118509): https://edk2.groups.io/g/devel/message/118509 Mute This Topic: https://groups.io/mt/105690947/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-