Re: [PATCH 11/26] MdeModulePkg UfsBlockIoPei: Remove redundant functions

["Wu, Hao A" <hao.a.wu@intel.com>]

One minor comment:

Please help to remove the comment that mentioned 'UfsPeimUnlinkMemBlock'
within function UfsPeimFreeMemPool().

For me,
  //
  // Unlink all the memory blocks from the pool, then free them.
  //
will be fine.

With that change,
Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> shenglei
> Sent: Wednesday, August 08, 2018 4:47 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric; Zeng, Star
> Subject: [edk2] [PATCH 11/26] MdeModulePkg UfsBlockIoPei: Remove
> redundant functions
> 
> The functions that are never called have been removed.
> They are UfsPeimUnlinkMemBlock,UfsPeimRequestSense,
> UfsPeimInquiry,UfsRwAttributes,UfsClearFlag,UfsReadFlag
> and UfsFindAvailableSlotInTmrl
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1062
> 
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shenglei <shenglei.zhang@intel.com>
> ---
>  .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c     | 101 ----------
>  MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHcMem.c |  24 ---
>  MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c   | 180 ------------------
>  3 files changed, 305 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
> b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
> index 505b92cfc1..204e456413 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
> @@ -137,53 +137,7 @@ UFS_PEIM_HC_PRIVATE_DATA   gUfsHcPeimTemplate
> = {
>    }
>  };
> 
> -/**
> -  Execute Request Sense SCSI command on a specific UFS device.
> -
> -  @param[in]  Private              A pointer to UFS_PEIM_HC_PRIVATE_DATA data
> structure.
> -  @param[in]  Lun                  The lun on which the SCSI cmd executed.
> -  @param[out] DataBuffer           A pointer to output sense data.
> -  @param[out] DataBufferLength     The length of output sense data.
> -
> -  @retval EFI_SUCCESS              The command executed successfully.
> -  @retval EFI_DEVICE_ERROR         A device error occurred while attempting
> to send SCSI Request Packet.
> -  @retval EFI_TIMEOUT              A timeout occurred while waiting for the SCSI
> Request Packet to execute.
> -
> -**/
> -EFI_STATUS
> -UfsPeimRequestSense (
> -  IN     UFS_PEIM_HC_PRIVATE_DATA        *Private,
> -  IN     UINTN                           Lun,
> -     OUT VOID                            *DataBuffer,
> -     OUT UINT32                          *DataBufferLength
> -  )
> -{
> -  UFS_SCSI_REQUEST_PACKET                Packet;
> -  UINT8                                  Cdb[UFS_SCSI_OP_LENGTH_SIX];
> -  EFI_STATUS                             Status;
> 
> -  ZeroMem (&Packet, sizeof (UFS_SCSI_REQUEST_PACKET));
> -  ZeroMem (Cdb, sizeof (Cdb));
> -
> -  Cdb[0]  = EFI_SCSI_OP_REQUEST_SENSE;
> -
> -  Packet.Timeout          = UFS_TIMEOUT;
> -  Packet.Cdb              = Cdb;
> -  Packet.CdbLength        = sizeof (Cdb);
> -  Packet.DataDirection    = UfsDataIn;
> -  Packet.InDataBuffer     = DataBuffer;
> -  Packet.InTransferLength = *DataBufferLength;
> -  Packet.SenseData        = NULL;
> -  Packet.SenseDataLength  = 0;
> -
> -  Status = UfsExecScsiCmds (Private,(UINT8)Lun, &Packet);
> -
> -  if (!EFI_ERROR (Status)) {
> -    *DataBufferLength = Packet.InTransferLength;
> -  }
> -
> -  return Status;
> -}
> 
>  /**
>    Execute TEST UNITY READY SCSI command on a specific UFS device.
> @@ -231,62 +185,7 @@ UfsPeimTestUnitReady (
>    return Status;
>  }
> 
> -/**
> -  Execute INQUIRY SCSI command on a specific UFS device.
> -
> -  @param[in]  Private              A pointer to UFS_PEIM_HC_PRIVATE_DATA data
> structure.
> -  @param[in]  Lun                  The lun on which the SCSI cmd executed.
> -  @param[out] Inquiry              A pointer to Inquiry data buffer.
> -  @param[out] InquiryLengths       The length of output Inquiry data.
> -  @param[out] SenseData            A pointer to output sense data.
> -  @param[out] SenseDataLength      The length of output sense data.
> -
> -  @retval EFI_SUCCESS              The command executed successfully.
> -  @retval EFI_DEVICE_ERROR         A device error occurred while attempting
> to send SCSI Request Packet.
> -  @retval EFI_TIMEOUT              A timeout occurred while waiting for the SCSI
> Request Packet to execute.
> -
> -**/
> -EFI_STATUS
> -UfsPeimInquiry (
> -  IN     UFS_PEIM_HC_PRIVATE_DATA     *Private,
> -  IN     UINTN                        Lun,
> -     OUT VOID                         *Inquiry,
> -     OUT UINT32                       *InquiryLength,
> -     OUT VOID                         *SenseData,  OPTIONAL
> -     OUT UINT8                        *SenseDataLength
> -  )
> -{
> -  UFS_SCSI_REQUEST_PACKET             Packet;
> -  UINT8                               Cdb[UFS_SCSI_OP_LENGTH_SIX];
> -  EFI_STATUS                          Status;
> -
> -  ZeroMem (&Packet, sizeof (UFS_SCSI_REQUEST_PACKET));
> -  ZeroMem (Cdb, sizeof (Cdb));
> 
> -  Cdb[0]  = EFI_SCSI_OP_INQUIRY;
> -  Cdb[4]  = sizeof (EFI_SCSI_INQUIRY_DATA);
> -
> -  Packet.Timeout          = UFS_TIMEOUT;
> -  Packet.Cdb              = Cdb;
> -  Packet.CdbLength        = sizeof (Cdb);
> -  Packet.InDataBuffer     = Inquiry;
> -  Packet.InTransferLength = *InquiryLength;
> -  Packet.DataDirection    = UfsDataIn;
> -  Packet.SenseData        = SenseData;
> -  Packet.SenseDataLength  = *SenseDataLength;
> -
> -  Status = UfsExecScsiCmds (Private, (UINT8)Lun, &Packet);
> -
> -  if (*SenseDataLength != 0) {
> -    *SenseDataLength = Packet.SenseDataLength;
> -  }
> -
> -  if (!EFI_ERROR (Status)) {
> -    *InquiryLength = Packet.InTransferLength;
> -  }
> -
> -  return Status;
> -}
> 
>  /**
>    Execute READ CAPACITY(10) SCSI command on a specific UFS device.
> diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHcMem.c
> b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHcMem.c
> index d69e6e7227..d334faa0bb 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHcMem.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHcMem.c
> @@ -219,31 +219,7 @@ UfsPeimIsMemBlockEmpty (
>    return TRUE;
>  }
> 
> -/**
> -  Unlink the memory block from the pool's list.
> -
> -  @param  Head           The block list head of the memory's pool.
> -  @param  BlockToUnlink  The memory block to unlink.
> 
> -**/
> -VOID
> -UfsPeimUnlinkMemBlock (
> -  IN UFS_PEIM_MEM_BLOCK      *Head,
> -  IN UFS_PEIM_MEM_BLOCK      *BlockToUnlink
> -  )
> -{
> -  UFS_PEIM_MEM_BLOCK         *Block;
> -
> -  ASSERT ((Head != NULL) && (BlockToUnlink != NULL));
> -
> -  for (Block = Head; Block != NULL; Block = Block->Next) {
> -    if (Block->Next == BlockToUnlink) {
> -      Block->Next         = BlockToUnlink->Next;
> -      BlockToUnlink->Next = NULL;
> -      break;
> -    }
> -  }
> -}
> 
>  /**
>    Initialize the memory management pool for the host controller.
> diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> index e36ff8c02a..936f25da5e 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> @@ -694,31 +694,7 @@ UfsFindAvailableSlotInTrl (
>    return EFI_SUCCESS;
>  }
> 
> -/**
> -  Find out available slot in task management transfer list of a UFS device.
> -
> -  @param[in]  Private       The pointer to the UFS_PEIM_HC_PRIVATE_DATA
> data structure.
> -  @param[out] Slot          The available slot.
> -
> -  @retval EFI_SUCCESS       The available slot was found successfully.
> -
> -**/
> -EFI_STATUS
> -UfsFindAvailableSlotInTmrl (
> -  IN     UFS_PEIM_HC_PRIVATE_DATA     *Private,
> -     OUT UINT8                        *Slot
> -  )
> -{
> -  ASSERT ((Private != NULL) && (Slot != NULL));
> -
> -  //
> -  // The simplest algo to always use slot 0.
> -  // TODO: enhance it to support async transfer with multiple slot.
> -  //
> -  *Slot = 0;
> 
> -  return EFI_SUCCESS;
> -}
> 
>  /**
>    Start specified slot in transfer list of a UFS device.
> @@ -897,113 +873,7 @@ Exit:
>    return Status;
>  }
> 
> -/**
> -  Read or write specified attribute of a UFS device.
> -
> -  @param[in]      Private       The pointer to the UFS_PEIM_HC_PRIVATE_DATA
> data structure.
> -  @param[in]      Read          The boolean variable to show r/w direction.
> -  @param[in]      AttrId        The ID of Attribute.
> -  @param[in]      Index         The Index of Attribute.
> -  @param[in]      Selector      The Selector of Attribute.
> -  @param[in, out] Attributes    The value of Attribute to be read or written.
> -
> -  @retval EFI_SUCCESS           The Attribute was read/written successfully.
> -  @retval EFI_DEVICE_ERROR      A device error occurred while attempting to
> r/w the Attribute.
> -  @retval EFI_TIMEOUT           A timeout occurred while waiting for the
> completion of r/w the Attribute.
> -
> -**/
> -EFI_STATUS
> -UfsRwAttributes (
> -  IN     UFS_PEIM_HC_PRIVATE_DATA     *Private,
> -  IN     BOOLEAN                      Read,
> -  IN     UINT8                        AttrId,
> -  IN     UINT8                        Index,
> -  IN     UINT8                        Selector,
> -  IN OUT UINT32                       *Attributes
> -  )
> -{
> -  EFI_STATUS                           Status;
> -  UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
> -  UINT8                                Slot;
> -  UTP_TRD                              *Trd;
> -  UINTN                                Address;
> -  UTP_QUERY_RESP_UPIU                  *QueryResp;
> -  UINT8                                *CmdDescBase;
> -  UINT32                               CmdDescSize;
> -  UINT32                               ReturnData;
> -
> -  ZeroMem (&Packet, sizeof
> (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
> -
> -  if (Read) {
> -    Packet.DataDirection     = UfsDataIn;
> -    Packet.Opcode            = UtpQueryFuncOpcodeRdAttr;
> -  } else {
> -    Packet.DataDirection     = UfsDataOut;
> -    Packet.Opcode            = UtpQueryFuncOpcodeWrAttr;
> -  }
> -  Packet.DescId              = AttrId;
> -  Packet.Index               = Index;
> -  Packet.Selector            = Selector;
> -  Packet.Timeout             = UFS_TIMEOUT;
> -
> -  //
> -  // Find out which slot of transfer request list is available.
> -  //
> -  Status = UfsFindAvailableSlotInTrl (Private, &Slot);
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -
> -  Trd = ((UTP_TRD*)Private->UtpTrlBase) + Slot;
> -  //
> -  // Fill transfer request descriptor to this slot.
> -  //
> -  Status = UfsCreateDMCommandDesc (Private, &Packet, Trd);
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -
> -  //
> -  // Check the transfer request result.
> -  //
> -  CmdDescBase = (UINT8 *)(UINTN)(LShiftU64 ((UINT64)Trd->UcdBaU, 32) |
> LShiftU64 ((UINT64)Trd->UcdBa, 7));
> -  QueryResp   = (UTP_QUERY_RESP_UPIU*)(CmdDescBase + Trd->RuO * sizeof
> (UINT32));
> -  CmdDescSize = Trd->RuO * sizeof (UINT32) + Trd->RuL * sizeof (UINT32);
> -
> -  //
> -  // Start to execute the transfer request.
> -  //
> -  UfsStartExecCmd (Private, Slot);
> -
> -  //
> -  // Wait for the completion of the transfer request.
> -  //
> -  Address = Private->UfsHcBase + UFS_HC_UTRLDBR_OFFSET;
> -  Status = UfsWaitMemSet (Address, BIT0 << Slot, 0, Packet.Timeout);
> -  if (EFI_ERROR (Status)) {
> -    goto Exit;
> -  }
> -
> -  if (QueryResp->QueryResp != 0) {
> -    DumpQueryResponseResult (QueryResp->QueryResp);
> -    Status = EFI_DEVICE_ERROR;
> -    goto Exit;
> -  }
> -
> -  if (Trd->Ocs == 0) {
> -    ReturnData = QueryResp->Tsf.Value;
> -    SwapLittleEndianToBigEndian ((UINT8*)&ReturnData, sizeof (UINT32));
> -    *Attributes = ReturnData;
> -  } else {
> -    Status = EFI_DEVICE_ERROR;
> -  }
> -
> -Exit:
> -  UfsStopExecCmd (Private, Slot);
> -  UfsPeimFreeMem (Private->Pool, CmdDescBase, CmdDescSize);
> 
> -  return Status;
> -}
> 
>  /**
>    Read or write specified flag of a UFS device.
> @@ -1143,57 +1013,7 @@ UfsSetFlag (
>    return Status;
>  }
> 
> -/**
> -  Clear specified flag to 0 on a UFS device.
> -
> -  @param[in]  Private           The pointer to the UFS_PEIM_HC_PRIVATE_DATA
> data structure.
> -  @param[in]  FlagId            The ID of flag to be cleared.
> -
> -  @retval EFI_SUCCESS           The flag was cleared successfully.
> -  @retval EFI_DEVICE_ERROR      A device error occurred while attempting to
> clear the flag.
> -  @retval EFI_TIMEOUT           A timeout occurred while waiting for the
> completion of clearing the flag.
> -
> -**/
> -EFI_STATUS
> -UfsClearFlag (
> -  IN  UFS_PEIM_HC_PRIVATE_DATA     *Private,
> -  IN  UINT8                        FlagId
> -  )
> -{
> -  EFI_STATUS             Status;
> -  UINT8                  Value;
> -
> -  Value  = 0;
> -  Status = UfsRwFlags (Private, FALSE, FlagId, &Value);
> -
> -  return Status;
> -}
> -
> -/**
> -  Read specified flag from a UFS device.
> -
> -  @param[in]  Private           The pointer to the UFS_PEIM_HC_PRIVATE_DATA
> data structure.
> -  @param[in]  FlagId            The ID of flag to be read.
> -  @param[out] Value             The flag's value.
> -
> -  @retval EFI_SUCCESS           The flag was read successfully.
> -  @retval EFI_DEVICE_ERROR      A device error occurred while attempting to
> read the flag.
> -  @retval EFI_TIMEOUT           A timeout occurred while waiting for the
> completion of reading the flag.
> -
> -**/
> -EFI_STATUS
> -UfsReadFlag (
> -  IN     UFS_PEIM_HC_PRIVATE_DATA     *Private,
> -  IN     UINT8                        FlagId,
> -     OUT UINT8                        *Value
> -  )
> -{
> -  EFI_STATUS                           Status;
> -
> -  Status = UfsRwFlags (Private, TRUE, FlagId, Value);
> 
> -  return Status;
> -}
> 
>  /**
>    Sends NOP IN cmd to a UFS device for initialization process request.
> --
> 2.18.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel