From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=MZ9/BPFt; spf=pass (domain: linaro.org, ip: 209.85.221.67, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by groups.io with SMTP; Fri, 03 May 2019 05:11:21 -0700 Received: by mail-wr1-f67.google.com with SMTP id e28so7629875wra.0 for ; Fri, 03 May 2019 05:11:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=eEtea6Go9j/HN1qZ8UCPBwARbWqPHHCbFLNQPBCxVnI=; b=MZ9/BPFtT1BWytm2AcR0Otk849si9z9ZaOtCC9gWCH/e+Y8lOs+xetvHQYQ0LPOAOP Ud0ZYT+UvzdnJ99CcPW2mMzQzElnTl8aIyYK06cObgPVUTXu/oLmraFB69Xqc2AfuKK+ W/pscjGjZWxoO4scYc9lZNcHZtoE/WWikhe8tzmP76k/jbhTOWU/xfPUFesRItJMhFRP j6KP/zOyYKoJ70wkQ88R1Zh4llf/eaHsyZgifP5vsQr5VVHwJnE6c29Gw7sM+5WpjN0i WVHxqWsNES2XXvxkLX5AbGcs72Nw7/FCVKYMo5FMra1xUI6OOerOgvjkBYdJXGyV8ese JW+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=eEtea6Go9j/HN1qZ8UCPBwARbWqPHHCbFLNQPBCxVnI=; b=L559kSBs1nxIwPrP2yIHJsCRt9qUp7zdHUCOo62WAIwCgKySWag5DCB2lfd85fCLWX txqG8wVIc/01BbEL15jmr5+NRVBKtDCYKpxT7b/WGFmtK33cCGfQnpERX317jl4E8Q8b lYELAtBY6G5CXJZ9TO4Jxjv+91T73TTFdVlfYug3qbVeCPcDL88QiUuSeJ9qdhpWHX1U bgrPuYOezVv8/pTaOLopV1GjoKVIPTh4ybMZ1XCED55iIiqGiFPOB/QBMDQ6J4ZiN5a+ 5vXh2Kud/wYOK2gAKlZzvl4y71kiipoSD5/BxSmJATysQdp8pyvBXVIxf692YoV9l22M YuOg== X-Gm-Message-State: APjAAAWe+uuQK4kccrQVfiPL71dRHA+kA0oXdhdh84zyAe/0SQU8s2fb 1T6NeqliGo9GFoUjwW7e0gznLA== X-Google-Smtp-Source: APXvYqxVwkTBmtojo5DftAN5u/dGHhd64CKcf6Q62UTBMlrZuIcJiJ8uMLi17RxN9fvGxw5jPzDSPA== X-Received: by 2002:a05:6000:118a:: with SMTP id g10mr7040671wrx.233.1556885479463; Fri, 03 May 2019 05:11:19 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id 130sm2558386wmd.15.2019.05.03.05.11.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 03 May 2019 05:11:18 -0700 (PDT) Date: Fri, 3 May 2019 13:11:17 +0100 From: "Leif Lindholm" To: tien.hock.loh@intel.com Cc: devel@edk2.groups.io, thloh85@gmail.com, Ard Biesheuvel Subject: Re: [[PATCH v2] 5/7] EmbeddedPkg: Fix DwEmmc driver bugs Message-ID: <20190503121117.eursvy5tozgxz2sc@bivouac.eciton.net> References: <1556854023-5486-1-git-send-email-tien.hock.loh@intel.com> <1556854023-5486-6-git-send-email-tien.hock.loh@intel.com> MIME-Version: 1.0 In-Reply-To: <1556854023-5486-6-git-send-email-tien.hock.loh@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, May 03, 2019 at 11:27:01AM +0800, tien.hock.loh@intel.com wrote: > From: "Tien Hock, Loh" > > 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" > Cc: Leif Lindholm > Cc: Ard Biesheuvel > --- > 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 >