[edk2-devel] [[PATCH v2] 5/7] EmbeddedPkg: Fix DwEmmc driver bugs
Loh, Tien Hock
tien.hock.loh at intel.com
Thu May 9 03:40:50 UTC 2019
> -----Original Message-----
> From: Leif Lindholm <leif.lindholm at linaro.org>
> Sent: Friday, May 3, 2019 8:11 PM
> To: Loh, Tien Hock <tien.hock.loh at intel.com>
> Cc: devel at edk2.groups.io; thloh85 at gmail.com; Ard Biesheuvel
> <ard.biesheuvel at linaro.org>
> Subject: Re: [[PATCH v2] 5/7] EmbeddedPkg: Fix DwEmmc driver bugs
>
> 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.
OK noted.
>
> > 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.
Yes I'll move it to after the buffer null check.
>
> /
> 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 (#40241): https://edk2.groups.io/g/devel/message/40241
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list