[edk2-devel] [[PATCH v2] 5/7] EmbeddedPkg: Fix DwEmmc driver bugs

[Leif Lindholm]

On Fri, May 03, 2019 at 11:27:01AM +0800, tien.hock.loh at intel.com wrote:
> From: "Tien Hock, Loh" <tien.hock.loh at 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.loh at intel.com>
> Cc: Leif Lindholm <leif.lindholm at linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-