Re: [edk2-devel] [PATCH RESEND edk2-platforms][PATCH V2 01/14] Platform/ARM/NorFlashDxe: Move DiskIo related functions out of NorFlash.c

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Sahil,

Thank you for this patch.

These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 23/04/2024 06:56 am, Sahil Kaushal wrote:
> From: sahil <sahil@arm.com>
>
> Moving these functions from NorFlash.c to NorFlashBlockIoDxe.c as
> they are not dependent on any particular flash implementation.
>
> Signed-off-by: sahil <sahil@arm.com>
> ---
>   Platform/ARM/Drivers/NorFlashDxe/NorFlash.c           | 129 --------------------
>   Platform/ARM/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c | 129 ++++++++++++++++++++
>   2 files changed, 129 insertions(+), 129 deletions(-)
>
> diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
> index 1b431073ee93..60854ef2a7d0 100644
> --- a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
> +++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
> @@ -807,135 +807,6 @@ NorFlashWriteSingleBlock (
>     return EFI_SUCCESS;
>
>   }
>
>   
>
> -/*
>
> -  Although DiskIoDxe will automatically install the DiskIO protocol whenever
>
> -  we install the BlockIO protocol, its implementation is sub-optimal as it reads
>
> -  and writes entire blocks using the BlockIO protocol. In fact we can access
>
> -  NOR flash with a finer granularity than that, so we can improve performance
>
> -  by directly producing the DiskIO protocol.
>
> -*/
>
> -
>
> -/**
>
> -  Read BufferSize bytes from Offset into Buffer.
>
> -
>
> -  @param  This                  Protocol instance pointer.
>
> -  @param  MediaId               Id of the media, changes every time the media is replaced.
>
> -  @param  Offset                The starting byte offset to read from
>
> -  @param  BufferSize            Size of Buffer
>
> -  @param  Buffer                Buffer containing read data
>
> -
>
> -  @retval EFI_SUCCESS           The data was read correctly from the device.
>
> -  @retval EFI_DEVICE_ERROR      The device reported an error while performing the read.
>
> -  @retval EFI_NO_MEDIA          There is no media in the device.
>
> -  @retval EFI_MEDIA_CHANGED     The MediaId does not match the current device.
>
> -  @retval EFI_INVALID_PARAMETER The read request contains device addresses that are not
>
> -                                valid for the device.
>
> -
>
> -**/
>
> -EFI_STATUS
>
> -EFIAPI
>
> -NorFlashDiskIoReadDisk (
>
> -  IN EFI_DISK_IO_PROTOCOL  *This,
>
> -  IN UINT32                MediaId,
>
> -  IN UINT64                DiskOffset,
>
> -  IN UINTN                 BufferSize,
>
> -  OUT VOID                 *Buffer
>
> -  )
>
> -{
>
> -  NOR_FLASH_INSTANCE  *Instance;
>
> -  UINT32              BlockSize;
>
> -  UINT32              BlockOffset;
>
> -  EFI_LBA             Lba;
>
> -
>
> -  Instance = INSTANCE_FROM_DISKIO_THIS (This);
>
> -
>
> -  if (MediaId != Instance->Media.MediaId) {
>
> -    return EFI_MEDIA_CHANGED;
>
> -  }
>
> -
>
> -  BlockSize = Instance->Media.BlockSize;
>
> -  Lba       = (EFI_LBA)DivU64x32Remainder (DiskOffset, BlockSize, &BlockOffset);
>
> -
>
> -  return NorFlashRead (Instance, Lba, BlockOffset, BufferSize, Buffer);
>
> -}
>
> -
>
> -/**
>
> -  Writes a specified number of bytes to a device.
>
> -
>
> -  @param  This       Indicates a pointer to the calling context.
>
> -  @param  MediaId    ID of the medium to be written.
>
> -  @param  Offset     The starting byte offset on the logical block I/O device to write.
>
> -  @param  BufferSize The size in bytes of Buffer. The number of bytes to write to the device.
>
> -  @param  Buffer     A pointer to the buffer containing the data to be written.
>
> -
>
> -  @retval EFI_SUCCESS           The data was written correctly to the device.
>
> -  @retval EFI_WRITE_PROTECTED   The device can not be written to.
>
> -  @retval EFI_DEVICE_ERROR      The device reported an error while performing the write.
>
> -  @retval EFI_NO_MEDIA          There is no media in the device.
>
> -  @retval EFI_MEDIA_CHANGED     The MediaId does not match the current device.
>
> -  @retval EFI_INVALID_PARAMETER The write request contains device addresses that are not
>
> -                                 valid for the device.
>
> -
>
> -**/
>
> -EFI_STATUS
>
> -EFIAPI
>
> -NorFlashDiskIoWriteDisk (
>
> -  IN EFI_DISK_IO_PROTOCOL  *This,
>
> -  IN UINT32                MediaId,
>
> -  IN UINT64                DiskOffset,
>
> -  IN UINTN                 BufferSize,
>
> -  IN VOID                  *Buffer
>
> -  )
>
> -{
>
> -  NOR_FLASH_INSTANCE  *Instance;
>
> -  UINT32              BlockSize;
>
> -  UINT32              BlockOffset;
>
> -  EFI_LBA             Lba;
>
> -  UINTN               RemainingBytes;
>
> -  UINTN               WriteSize;
>
> -  EFI_STATUS          Status;
>
> -
>
> -  Instance = INSTANCE_FROM_DISKIO_THIS (This);
>
> -
>
> -  if (MediaId != Instance->Media.MediaId) {
>
> -    return EFI_MEDIA_CHANGED;
>
> -  }
>
> -
>
> -  BlockSize = Instance->Media.BlockSize;
>
> -  Lba       = (EFI_LBA)DivU64x32Remainder (DiskOffset, BlockSize, &BlockOffset);
>
> -
>
> -  RemainingBytes = BufferSize;
>
> -
>
> -  // Write either all the remaining bytes, or the number of bytes that bring
>
> -  // us up to a block boundary, whichever is less.
>
> -  // (DiskOffset | (BlockSize - 1)) + 1) rounds DiskOffset up to the next
>
> -  // block boundary (even if it is already on one).
>
> -  WriteSize = MIN (RemainingBytes, ((DiskOffset | (BlockSize - 1)) + 1) - DiskOffset);
>
> -
>
> -  do {
>
> -    if (WriteSize == BlockSize) {
>
> -      // Write a full block
>
> -      Status = NorFlashWriteFullBlock (Instance, Lba, Buffer, BlockSize / sizeof (UINT32));
>
> -    } else {
>
> -      // Write a partial block
>
> -      Status = NorFlashWriteSingleBlock (Instance, Lba, BlockOffset, &WriteSize, Buffer);
>
> -    }
>
> -
>
> -    if (EFI_ERROR (Status)) {
>
> -      return Status;
>
> -    }
>
> -
>
> -    // Now continue writing either all the remaining bytes or single blocks.
>
> -    RemainingBytes -= WriteSize;
>
> -    Buffer          = (UINT8 *)Buffer + WriteSize;
>
> -    Lba++;
>
> -    BlockOffset = 0;
>
> -    WriteSize   = MIN (RemainingBytes, BlockSize);
>
> -  } while (RemainingBytes);
>
> -
>
> -  return Status;
>
> -}
>
> -
>
>   EFI_STATUS
>
>   NorFlashReset (
>
>     IN  NOR_FLASH_INSTANCE  *Instance
>
> diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c b/Platform/ARM/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> index 9d4732c6905a..9b8c76a171d4 100644
> --- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> +++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> @@ -121,3 +121,132 @@ NorFlashBlockIoFlushBlocks (
>     // Nothing to do so just return without error
>
>     return EFI_SUCCESS;
>
>   }
>
> +
>
> +/*
>
> +  Although DiskIoDxe will automatically install the DiskIO protocol whenever
>
> +  we install the BlockIO protocol, its implementation is sub-optimal as it reads
>
> +  and writes entire blocks using the BlockIO protocol. In fact we can access
>
> +  NOR flash with a finer granularity than that, so we can improve performance
>
> +  by directly producing the DiskIO protocol.
>
> +*/
>
> +
>
> +/**
>
> +  Read BufferSize bytes from Offset into Buffer.
>
> +
>
> +  @param  This                  Protocol instance pointer.
>
> +  @param  MediaId               Id of the media, changes every time the media is replaced.
>
> +  @param  Offset                The starting byte offset to read from
>
> +  @param  BufferSize            Size of Buffer
>
> +  @param  Buffer                Buffer containing read data
>
> +
>
> +  @retval EFI_SUCCESS           The data was read correctly from the device.
>
> +  @retval EFI_DEVICE_ERROR      The device reported an error while performing the read.
>
> +  @retval EFI_NO_MEDIA          There is no media in the device.
>
> +  @retval EFI_MEDIA_CHANGED     The MediaId does not match the current device.
>
> +  @retval EFI_INVALID_PARAMETER The read request contains device addresses that are not
>
> +                                valid for the device.
>
> +
>
> +**/
>
> +EFI_STATUS
>
> +EFIAPI
>
> +NorFlashDiskIoReadDisk (
>
> +  IN EFI_DISK_IO_PROTOCOL  *This,
>
> +  IN UINT32                MediaId,
>
> +  IN UINT64                DiskOffset,
>
> +  IN UINTN                 BufferSize,
>
> +  OUT VOID                 *Buffer
>
> +  )
>
> +{
>
> +  NOR_FLASH_INSTANCE  *Instance;
>
> +  UINT32              BlockSize;
>
> +  UINT32              BlockOffset;
>
> +  EFI_LBA             Lba;
>
> +
>
> +  Instance = INSTANCE_FROM_DISKIO_THIS (This);
>
> +
>
> +  if (MediaId != Instance->Media.MediaId) {
>
> +    return EFI_MEDIA_CHANGED;
>
> +  }
>
> +
>
> +  BlockSize = Instance->Media.BlockSize;
>
> +  Lba       = (EFI_LBA)DivU64x32Remainder (DiskOffset, BlockSize, &BlockOffset);
>
> +
>
> +  return NorFlashRead (Instance, Lba, BlockOffset, BufferSize, Buffer);
>
> +}
>
> +
>
> +/**
>
> +  Writes a specified number of bytes to a device.
>
> +
>
> +  @param  This       Indicates a pointer to the calling context.
>
> +  @param  MediaId    ID of the medium to be written.
>
> +  @param  Offset     The starting byte offset on the logical block I/O device to write.
>
> +  @param  BufferSize The size in bytes of Buffer. The number of bytes to write to the device.
>
> +  @param  Buffer     A pointer to the buffer containing the data to be written.
>
> +
>
> +  @retval EFI_SUCCESS           The data was written correctly to the device.
>
> +  @retval EFI_WRITE_PROTECTED   The device can not be written to.
>
> +  @retval EFI_DEVICE_ERROR      The device reported an error while performing the write.
>
> +  @retval EFI_NO_MEDIA          There is no media in the device.
>
> +  @retval EFI_MEDIA_CHANGED     The MediaId does not match the current device.
>
> +  @retval EFI_INVALID_PARAMETER The write request contains device addresses that are not
>
> +                                 valid for the device.
>
> +
>
> +**/
>
> +EFI_STATUS
>
> +EFIAPI
>
> +NorFlashDiskIoWriteDisk (
>
> +  IN EFI_DISK_IO_PROTOCOL  *This,
>
> +  IN UINT32                MediaId,
>
> +  IN UINT64                DiskOffset,
>
> +  IN UINTN                 BufferSize,
>
> +  IN VOID                  *Buffer
>
> +  )
>
> +{
>
> +  NOR_FLASH_INSTANCE  *Instance;
>
> +  UINT32              BlockSize;
>
> +  UINT32              BlockOffset;
>
> +  EFI_LBA             Lba;
>
> +  UINTN               RemainingBytes;
>
> +  UINTN               WriteSize;
>
> +  EFI_STATUS          Status;
>
> +
>
> +  Instance = INSTANCE_FROM_DISKIO_THIS (This);
>
> +
>
> +  if (MediaId != Instance->Media.MediaId) {
>
> +    return EFI_MEDIA_CHANGED;
>
> +  }
>
> +
>
> +  BlockSize = Instance->Media.BlockSize;
>
> +  Lba       = (EFI_LBA)DivU64x32Remainder (DiskOffset, BlockSize, &BlockOffset);
>
> +
>
> +  RemainingBytes = BufferSize;
>
> +
>
> +  // Write either all the remaining bytes, or the number of bytes that bring
>
> +  // us up to a block boundary, whichever is less.
>
> +  // (DiskOffset | (BlockSize - 1)) + 1) rounds DiskOffset up to the next
>
> +  // block boundary (even if it is already on one).
>
> +  WriteSize = MIN (RemainingBytes, ((DiskOffset | (BlockSize - 1)) + 1) - DiskOffset);
>
> +
>
> +  do {
>
> +    if (WriteSize == BlockSize) {
>
> +      // Write a full block
>
> +      Status = NorFlashWriteFullBlock (Instance, Lba, Buffer, BlockSize / sizeof (UINT32));
>
> +    } else {
>
> +      // Write a partial block
>
> +      Status = NorFlashWriteSingleBlock (Instance, Lba, BlockOffset, &WriteSize, Buffer);
>
> +    }
>
> +
>
> +    if (EFI_ERROR (Status)) {
>
> +      return Status;
>
> +    }
>
> +
>
> +    // Now continue writing either all the remaining bytes or single blocks.
>
> +    RemainingBytes -= WriteSize;
>
> +    Buffer          = (UINT8 *)Buffer + WriteSize;
>
> +    Lba++;
>
> +    BlockOffset = 0;
>
> +    WriteSize   = MIN (RemainingBytes, BlockSize);
>
> +  } while (RemainingBytes);
>
> +
>
> +  return Status;
>
> +}
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118957): https://edk2.groups.io/g/devel/message/118957
Mute This Topic: https://groups.io/mt/105690936/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-