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=jcpZE3I9; 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; Tue, 28 May 2019 10:46:00 -0700 Received: by mail-wr1-f65.google.com with SMTP id x4so2087889wrt.6 for ; Tue, 28 May 2019 10:45:59 -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=OBmJ8zcGnEgB66Vfh7fkQ3XjHcvnAzJ87+0BM0K7x40=; b=jcpZE3I9xqBVsRJiHqcTEBwntBxmlz+89ADQ3FjVD2PuA7ChrhIvlj2n7cLXkksZjl zyAjtRc4cdz/2eO0lSYzsbwpco97TESz4zzuBG733odGVGgG6ccGntT3nDwWg6uhBTLt gNf/OQCRbR1wjNtGn9ybt4E96mjOocW87o/v+EBE4ApSAqlpvV3A0kw3QEYKLMIwVObc J7qv01bcxEXRa9mgVLaAbwVUnsbjORvyzJjowO4Nz2qU8A1/6JFBZXtHOHp5kO+E/364 VqgfDUm5hUy+HL6nv87h+ycQDE9TqXIge2VbHycYbUjNV+IQYauHWl6le7msa12HcdNS Cy0w== 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=OBmJ8zcGnEgB66Vfh7fkQ3XjHcvnAzJ87+0BM0K7x40=; b=igGyeaVWjeN+Gw+NHAdVNlTRPh7npTM1qCDHeFtSJNvmQJi2YwgPUNzGN3HrHRzJjA S6ONAzQl+iF3yCdWPCxp4PpjEjFihm+BryENJfDhqZ9W/wKLIFJfpyuKRJqm8Y1Jeh+U Ee+0b0gY39CjYWtnotF7glOQrO8s5TfZqAjquuFJM6bVUC0YH0uZEVs77wjgtdi4ZIFb O0CMgnO9bj6ozHh28hLD6fP7cVucDBU+QN0PB8zFkIl3WxvuT38aRuHRGjUFQT23wqac PLAIf9lDOs9vF8iuJpNmCVIG9kIKTkIf+8eUt00rIQKBDZVPclqBa3FgBt+Ag5zD6H9o 61CA== X-Gm-Message-State: APjAAAUtgBiyo60xhf5gYhJVchzeTHFH9ocYXbf2Ocgv7bEsicgzbhmY 5p0ZAsMSor9WRSVf+nYlKfOyWw== X-Google-Smtp-Source: APXvYqxCroKZJRditW90J3xCGBZx3EsMMrMVOmugppQ0hvomE7sj9oG+CKzslpNALB8DMfEgR2bf2A== X-Received: by 2002:adf:ea89:: with SMTP id s9mr22248555wrm.322.1559065558348; Tue, 28 May 2019 10:45:58 -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 k13sm2059055wmj.10.2019.05.28.10.45.57 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 28 May 2019 10:45:57 -0700 (PDT) Date: Tue, 28 May 2019 18:45: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 2/7] EmbeddedPkg: Fix DwEmmc CMD8 support for SD Message-ID: <20190528174556.m2kq5cyysjl6kzgs@bivouac.eciton.net> References: <1558949428-190715-1-git-send-email-tien.hock.loh@intel.com> <1558949428-190715-3-git-send-email-tien.hock.loh@intel.com> MIME-Version: 1.0 In-Reply-To: <1558949428-190715-3-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 Mon, May 27, 2019 at 05:30:23PM +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 > > -- > v2 > - Change IsEmmc to EFI_MMC_HOST_CARD_TYPE Version information goes below ---. Never type this manually - sooner or later you will leave out a like above and accidentally have it included in the commit message. It is also helpful to include this information in a cover letter [0/7]. For simple changes like this, there is no need to list it individually in each patch. > --- > EmbeddedPkg/Include/Protocol/MmcHost.h | 6 ++++++ > EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 9 ++++++--- > EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 2 ++ > 3 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/EmbeddedPkg/Include/Protocol/MmcHost.h b/EmbeddedPkg/Include/Protocol/MmcHost.h > index 9e07082680..7807744721 100644 > --- a/EmbeddedPkg/Include/Protocol/MmcHost.h > +++ b/EmbeddedPkg/Include/Protocol/MmcHost.h > @@ -151,6 +151,11 @@ typedef BOOLEAN (EFIAPI *MMC_ISMULTIBLOCK) ( > IN EFI_MMC_HOST_PROTOCOL *This > ); > > +typedef enum { > + EMMC, > + SD > +} EFI_MMC_HOST_CARD_TYPE; > + > struct _EFI_MMC_HOST_PROTOCOL { > > UINT32 Revision; > @@ -169,6 +174,7 @@ struct _EFI_MMC_HOST_PROTOCOL { > MMC_SETIOS SetIos; > MMC_ISMULTIBLOCK IsMultiBlock; > > + EFI_MMC_HOST_CARD_TYPE HostCardType; > }; > > #define MMC_HOST_PROTOCOL_REVISION 0x00010002 // 1.2 > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > index 420487757d..fd3a5bf685 100644 > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > @@ -340,9 +340,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->HostCardType == SD) > + 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 saw no comment on my feedback from previous revision that --- 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; } --- and the request was not followed. Also, always use { and } with if/else. / Leif > break; > case MMC_INDX(9): > Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC | > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > index 4dc0be125c..c816ae09ee 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->HostCardType = SD; > Status = InitializeSdMmcDevice (MmcHostInstance); > } else { > + MmcHostInstance->MmcHost->HostCardType = EMMC; > Status = InitializeEmmcDevice (MmcHostInstance); > } > if (EFI_ERROR (Status)) { > -- > 2.19.0 >