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=hKc81AIR; spf=pass (domain: linaro.org, ip: 209.85.221.65, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by groups.io with SMTP; Fri, 03 May 2019 05:07:00 -0700 Received: by mail-wr1-f65.google.com with SMTP id c12so7567826wrt.8 for ; Fri, 03 May 2019 05:07:00 -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=Q0sZMD7NVnWYjwIjfmo4Fjkydq0ffjrTOZVgd78ANac=; b=hKc81AIRp/WBIJCYJNEvdUK9H8HdDumYDZTBzbYNBwShFmZ14fHLXIxU6e+ojlXPaT DgoNpHPOpsVFXCmIC3CvfZW4dEANffABPHqNenTp3lpZ+U34t3t6UvMhIv26TOOVPz/b cXPdlS5SunUPV5uHUWSnOQfim5R/xiIKS3+ibVEn6eUKRrCT+MQD2sKAV4uc6V/mTGvI XcFDeUU0N/RSVo1+h3Ou4YDNKrWD0xZ/Z5fQD4KUiodSo70lLDSG/MyvHtyh6udsjQNn OnoXVqNliBMKmrfiWyge8JKWf7dxM9v2XrAUvFn/THV0zsjSH/NBm+aQiN6vdLxqchrF +dzA== 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=Q0sZMD7NVnWYjwIjfmo4Fjkydq0ffjrTOZVgd78ANac=; b=AuwsZJEtPGLuGJJ6tKicrOmcyJk0nAS/vciiXDuO8jhHoc+2cwi/dh7bAkPsa923jr 0ZEMPi1GSwUieC9Ni9PLAMa5yxSz4ajoqQJP9bZ/3tq70N+d3iiD+73ge35BUgx4xmcp U9xyLDFk2x+/3NehzUdA4mJp/znNYPYtUlH0qzqk0ZcnCbT+vDACD6rMMqh+w5PA5BUn hJJCwE6fDxWVjYfc4+CiIrolaiW1OzZk0xjIJCz+prozBTl0cwBycJIgT9FdpT7+TFA8 CK42XOhmvpdIf0cFA0LeFoZvPYfPgxypktgrA6+w3J46ud7Jskm+r4CvqgvxxFUEWkkj KtvA== X-Gm-Message-State: APjAAAUCLUIJs4A/GtA3tubX9SDNYMbWuWaxyU2PvU4eBZ6QdSoC1P01 VroKwG4PshO5dewwSYb5CZMcig== X-Google-Smtp-Source: APXvYqzVFc/iJLgi3bWn8yXcpx6SxwIKJAvZuSY9W+1TQiB2EbEalExFXzn6PLyXMfaMxjRtR9iy7g== X-Received: by 2002:a05:6000:10:: with SMTP id h16mr6521918wrx.151.1556885219041; Fri, 03 May 2019 05:06:59 -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 f8sm1845538wmb.32.2019.05.03.05.06.58 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 03 May 2019 05:06:58 -0700 (PDT) Date: Fri, 3 May 2019 13:06:56 +0100 From: "Leif Lindholm" To: tien.hock.loh@intel.com Cc: devel@edk2.groups.io, thloh85@gmail.com, Ard Biesheuvel Subject: Re: [[PATCH v2] 3/7] EmbeddedPkg: Fix DwEmmc driver bugs Message-ID: <20190503120656.zvcrq3n3cwm53g2b@bivouac.eciton.net> References: <1556854023-5486-1-git-send-email-tien.hock.loh@intel.com> <1556854023-5486-4-git-send-email-tien.hock.loh@intel.com> MIME-Version: 1.0 In-Reply-To: <1556854023-5486-4-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:26:59AM +0800, tien.hock.loh@intel.com wrote: > From: "Tien Hock, Loh" > > On CMD8, for SD, the controller should not expect data as this is a > SEND_IF_COND command to verify SD operating condition, and does not have > data > > Signed-off-by: "Tien Hock, Loh" > Cc: Leif Lindholm > Cc: Ard Biesheuvel > --- > EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 9 ++++++--- > EmbeddedPkg/Include/Protocol/MmcHost.h | 1 + > EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 2 ++ If you follow the guidelines at https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers when generating patches, that means interface changes (like for the .h file here) get reviewed before their uses, which makes for a much more natural review flow. > 3 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > index 058665b..04fdcbf 100644 > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > @@ -339,9 +339,12 @@ DwEmmcSendCommand ( > Cmd = 0; > break; > case MMC_INDX(8): > - Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC | > - BIT_CMD_DATA_EXPECTED | BIT_CMD_READ | > - BIT_CMD_WAIT_PRVDATA_COMPLETE; > + if (!This->IsEmmc) > + Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC | > + BIT_CMD_WAIT_PRVDATA_COMPLETE ; > + else > + Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC | > + BIT_CMD_WAIT_PRVDATA_COMPLETE | BIT_CMD_READ | BIT_CMD_DATA_EXPECTED; I think this would be more clear as Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC | BIT_CMD_WAIT_PRVDATA_COMPLETE; if (...) { Cmd |= BIT_CMD_READ | BIT_CMD_DATA_EXPECTED; } > break; > case MMC_INDX(9): > Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC | > diff --git a/EmbeddedPkg/Include/Protocol/MmcHost.h b/EmbeddedPkg/Include/Protocol/MmcHost.h > index 9e07082..ae8ea5d 100644 > --- a/EmbeddedPkg/Include/Protocol/MmcHost.h > +++ b/EmbeddedPkg/Include/Protocol/MmcHost.h > @@ -169,6 +169,7 @@ struct _EFI_MMC_HOST_PROTOCOL { > MMC_SETIOS SetIos; > MMC_ISMULTIBLOCK IsMultiBlock; > > + BOOLEAN IsEmmc; Case in point w.r.t. natural order. But I don't think a BOOLEAN is the right solution here. (We would just need to add another for each device type special case handling.) Could you instead add an EFI_MMC_HOST_CARD_TYPE enum like in EmbeddedPkg/Universal/MmcDxe/Mmc.h? / Leif > }; > > #define MMC_HOST_PROTOCOL_REVISION 0x00010002 // 1.2 > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > index 4dc0be1..fa1eda2 100755 > --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > @@ -770,8 +770,10 @@ InitializeMmcDevice ( > } > > if (MmcHostInstance->CardInfo.CardType != EMMC_CARD) { > + MmcHostInstance->MmcHost->IsEmmc = FALSE; > Status = InitializeSdMmcDevice (MmcHostInstance); > } else { > + MmcHostInstance->MmcHost->IsEmmc = TRUE; > Status = InitializeEmmcDevice (MmcHostInstance); > } > if (EFI_ERROR (Status)) { > -- > 2.2.2 >