On Fri, May 03, 2019 at 11:27:01AM +0800, tien.hock....@intel.com wrote:
> From: "Tien Hock, Loh" <tien.hock....@intel.com>
> 
> Send command when MMC ask for response in DwEmmcReceiveResponse, and
> command is a pending command (eg. DMA needs to be set up first)
> 
> Signed-off-by: "Tien Hock, Loh" <tien.hock....@intel.com>
> Cc: Leif Lindholm <leif.lindh...@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
>  EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c 
> b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> index 32572a9..a69d9ab 100644
> --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> @@ -398,8 +398,11 @@ DwEmmcSendCommand (
>      mDwEmmcCommand = Cmd;
>      mDwEmmcArgument = Argument;
>    } else {
> +    mDwEmmcCommand = Cmd;
> +    mDwEmmcArgument = Argument;
>      Status = SendCommand (Cmd, Argument);
>    }
> +

I agree a space looks better here, but please don't add unrelated
whitespace as part of a functional change.

>    return Status;
>  }
>  
> @@ -410,6 +413,11 @@ DwEmmcReceiveResponse (
>    IN UINT32*                    Buffer
>    )
>  {
> +  EFI_STATUS Status = EFI_SUCCESS;
> +
> +  if(IsPendingReadCommand (mDwEmmcCommand) || 
> IsPendingWriteCommand(mDwEmmcCommand))

  {

> +    Status = SendCommand (mDwEmmcCommand, mDwEmmcArgument);

  }

> +
>    if (Buffer == NULL) {
>      return EFI_INVALID_PARAMETER;
>    }

Should this test not come first in the function?
If the code is relying on the side effect of the SendCommand () being
performed even if Buffer is invalid, that needs a very detailed
comment.

/
    Leif


> @@ -427,7 +435,7 @@ DwEmmcReceiveResponse (
>      Buffer[2] = MmioRead32 (DWEMMC_RESP2);
>      Buffer[3] = MmioRead32 (DWEMMC_RESP3);
>    }
> -  return EFI_SUCCESS;
> +  return Status;
>  }
>  
>  VOID
> -- 
> 2.2.2
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39957): https://edk2.groups.io/g/devel/message/39957
Mute This Topic: https://groups.io/mt/31480081/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to