From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: tien.hock.loh@intel.com) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by groups.io with SMTP; Thu, 11 Jul 2019 22:40:14 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jul 2019 22:40:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,481,1557212400"; d="scan'208";a="174364063" Received: from pgsmsx112-dag.png.intel.com (HELO PGSMSX112.gar.corp.intel.com) ([10.108.55.234]) by FMSMGA003.fm.intel.com with ESMTP; 11 Jul 2019 22:40:12 -0700 Received: from pgsmsx110.gar.corp.intel.com ([169.254.13.19]) by PGSMSX112.gar.corp.intel.com ([169.254.3.46]) with mapi id 14.03.0439.000; Fri, 12 Jul 2019 13:40:12 +0800 From: "Loh, Tien Hock" To: 'Leif Lindholm' CC: 'Haojian Zhuang' , "'devel@edk2.groups.io'" , "'thloh85@gmail.com'" , "'Ard Biesheuvel'" Subject: Re: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand polling Thread-Topic: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand polling Thread-Index: AQHVFG808YivDNA62keax2f1O0CcUKaAUIOAgAJswYCAAJPjoIASgedA///ohICAAIbQIIAwfPng Date: Fri, 12 Jul 2019 05:40:11 +0000 Message-ID: References: <1558949428-190715-1-git-send-email-tien.hock.loh@intel.com> <1558949428-190715-7-git-send-email-tien.hock.loh@intel.com> <20190528180409.xqqigzdyk4e2oamt@bivouac.eciton.net> <20190530070553.GB18524@gmail.com> <20190611090847.lryjgzjq3y5xi5io@bivouac.eciton.net> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDMyNTUzNzMtYThlYS00MzhmLTgxZGItMjQ3MmM4OGUyZDQ4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoicUlra3ZodWJYUXlBXC84Rk1IZ2FNaEpxM3RoeUFPQXE0d1VYbWpcL296ZkhKV01xMHhia3A4VGluT1wveU1qK2txTSJ9 x-originating-ip: [172.30.20.206] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Leif, Some comments inlined. > -----Original Message----- > From: Loh, Tien Hock > Sent: Tuesday, June 11, 2019 5:12 PM > To: Leif Lindholm > Cc: 'Haojian Zhuang' ; 'devel@edk2.groups.io' > ; 'thloh85@gmail.com' ; 'Ard > Biesheuvel' > Subject: RE: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand > polling >=20 > > -----Original Message----- > > From: Leif Lindholm > > Sent: Tuesday, June 11, 2019 5:09 PM > > To: Loh, Tien Hock > > Cc: 'Haojian Zhuang' ; 'devel@edk2.groups.io= ' > > ; 'thloh85@gmail.com' ; > 'Ard > > Biesheuvel' > > Subject: Re: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand > > polling > > > > On Tue, Jun 11, 2019 at 02:40:51AM +0000, Loh, Tien Hock wrote: > > > Leif, > > > > > > Since Haojian have a newer driver model that uses the > > > NonDiscoverableDeviceDxe, I believe we should be moving to use the > > > new driver. > > > > > > I'll try to test out the patches submitted by Haojian in the mean tim= e. > > > Can you help review the patch? Thanks! > > > > I can have another look at the patch, but I would really appreciate if > > you could also review it please? > > > > My problem is that I really don't have much understanding of SD/MMC > > protocols. >=20 > Sure. I'll test it out on the SoCFPGA platform first. It is quite a long = patch, so I > might take a bit of time to review. > Thanks Leif! I've reviewed and tested the driver. For SD portion the patch works correct= ly on the SoCFPGA platform. How should I ACK the patch? >=20 > > > > / > > Leif > > > > > > -----Original Message----- > > > > From: Loh, Tien Hock > > > > Sent: Thursday, May 30, 2019 3:56 PM > > > > To: Haojian Zhuang ; Leif Lindholm > > > > > > > > Cc: devel@edk2.groups.io; thloh85@gmail.com; Ard Biesheuvel > > > > > > > > Subject: RE: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand > > > > polling > > > > > > > > > -----Original Message----- > > > > > From: Haojian Zhuang > > > > > Sent: Thursday, May 30, 2019 3:06 PM > > > > > To: Leif Lindholm > > > > > Cc: Loh, Tien Hock ; > > > > > devel@edk2.groups.io; thloh85@gmail.com; Ard Biesheuvel > > > > > > > > > > Subject: Re: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc > SendCommand > > > > > polling > > > > > > > > > > On Tue, May 28, 2019 at 07:04:09PM +0100, Leif Lindholm wrote: > > > > > > +Haojian, > > > > > > > > > > > > Haojian - since you are the original author, can you comment > > > > > > on the delays? Are these silicon bug workarounds (so we need > > > > > > to add a Pcd), or does these changes work on your platforms too= ? > > > > > > > > > > I'm not in the loop, so I missed the patch series. > > > > > > > > > > The patch series can't work on my platform for the eMMC. > > > > > Although a variable is created to identify whether it's a SD or > > > > > eMMC device, it doesn't identify the eMMC device by the right > > > > > way. So the eMMC device isn't initialized successfully on my plat= form. > > > > > > > > > > 1. Since MMC framework could identify whether it's eMMC device > > > > > or SD device, we need to make device driver gets this kind of > > > > > information from the MMC framework. And we need to support > > > > > multiple eMMC/SD instances in MMC framework. > > > > > > > > Yeah my bad I didn't read through the SD/MMC specs on that. Now I > > > > check the specs and you're right, the information should be read > > > > from MMC framework. > > > > > > > > > > > > > > 2. I sent a patch series to support both eMMC device and SD > > > > > device > > before. > > > > > https://edk2.groups.io/g/devel/message/28572 > > > > > && > > > > > https://edk2.groups.io/g/devel/message/28615 > > > > > Maybe it's missed. Could you help to review that patch series? > > > > > > Leif, can you help review the patch series? Since Haojian have moved > > > the > > driver to NonDiscoverableDeviceDxe, I think that would be a more > > appropriate driver to be used going forward. Thanks! > > > > > > > > > > > > > > > > > Best Regards > > > > > Haojian > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > Leif > > > > > > > > > > > > On Mon, May 27, 2019 at 05:30:27PM +0800, > > > > > > tien.hock.loh@intel.com > > > > > wrote: > > > > > > > From: "Tien Hock, Loh" > > > > > > > > > > > > > > Change SendCommand polling mode to remove unnecessary > delay, > > > > > > > and check for transfer done only when block data is to be > > read/write. > > > > > > > This would also increase performance slightly. > > > > > > > > > > > > > > Signed-off-by: "Tien Hock, Loh" > > > > > > > Cc: Leif Lindholm > > > > > > > Cc: Ard Biesheuvel > > > > > > > --- > > > > > > > EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 43 > > > > > +++++++++++++++----- > > > > > > > 1 file changed, 33 insertions(+), 10 deletions(-) > > > > > > > > > > > > > > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > > > > > > > b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > > > > > > > index c6c8e04917..b57833458f 100644 > > > > > > > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > > > > > > > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > > > > > > > @@ -286,16 +286,13 @@ SendCommand ( > > > > > > > DWEMMC_INT_RCRC | DWEMMC_INT_RE; > > > > > > > ErrMask |=3D DWEMMC_INT_DCRC | DWEMMC_INT_DRT | > > > > > DWEMMC_INT_SBE; > > > > > > > do { > > > > > > > - MicroSecondDelay(500); > > > > > > > Data =3D MmioRead32 (DWEMMC_RINTSTS); > > > > > > > - > > > > > > > - if (Data & ErrMask) { > > > > > > > - return EFI_DEVICE_ERROR; > > > > > > > - } > > > > > > > - if (Data & DWEMMC_INT_DTO) { // Transfer Done > > > > > > > - break; > > > > > > > - } > > > > > > > } while (!(Data & DWEMMC_INT_CMD_DONE)); > > > > > > > + > > > > > > > + if (Data & ErrMask) { > > > > > > > + return EFI_DEVICE_ERROR; } > > > > > > > + > > > > > > > return EFI_SUCCESS; > > > > > > > } > > > > > > > > > > > > > > @@ -550,8 +547,9 @@ DwEmmcReadBlockData ( > > > > > > > ) > > > > > > > { > > > > > > > EFI_STATUS Status; > > > > > > > - UINT32 DescPages, CountPerPage, Count; > > > > > > > + UINT32 DescPages, CountPerPage, Count, ErrMask; > > > > > > > EFI_TPL Tpl; > > > > > > > + UINTN Rintsts =3D 0; > > > > > > > > > > > > > > Tpl =3D gBS->RaiseTPL (TPL_NOTIFY); > > > > > > > > > > > > > > @@ -574,6 +572,18 @@ DwEmmcReadBlockData ( > > > > > > > DEBUG ((DEBUG_ERROR, "Failed to read data, > > > > > mDwEmmcCommand:%x, mDwEmmcArgument:%x, Status:%r\n", > > > > mDwEmmcCommand, > > > > > mDwEmmcArgument, Status)); > > > > > > > goto out; > > > > > > > } > > > > > > > + > > > > > > > + while(!((MmioRead32(DWEMMC_RINTSTS) & > > > > (DWEMMC_INT_DTO)))) > > > > > { > > > > > > > + Rintsts =3D MmioRead32 (DWEMMC_RINTSTS); } ErrMask =3D > > > > > > > + DWEMMC_INT_EBE | DWEMMC_INT_HLE | DWEMMC_INT_RTO > | > > > > > > > + DWEMMC_INT_RCRC | DWEMMC_INT_RE | > > > > > DWEMMC_INT_DCRC | > > > > > > > + DWEMMC_INT_DRT | DWEMMC_INT_SBE; > > > > > > > + > > > > > > > + if (Rintsts & ErrMask) { > > > > > > > + Status =3D EFI_DEVICE_ERROR; > > > > > > > + goto out; > > > > > > > + } > > > > > > > out: > > > > > > > // Restore Tpl > > > > > > > gBS->RestoreTPL (Tpl); > > > > > > > @@ -589,8 +599,9 @@ DwEmmcWriteBlockData ( > > > > > > > ) > > > > > > > { > > > > > > > EFI_STATUS Status; > > > > > > > - UINT32 DescPages, CountPerPage, Count; > > > > > > > + UINT32 DescPages, CountPerPage, Count, ErrMask; > > > > > > > EFI_TPL Tpl; > > > > > > > + UINTN Rintsts =3D 0; > > > > > > > > > > > > > > Tpl =3D gBS->RaiseTPL (TPL_NOTIFY); > > > > > > > > > > > > > > @@ -613,6 +624,18 @@ DwEmmcWriteBlockData ( > > > > > > > DEBUG ((DEBUG_ERROR, "Failed to write data, > > > > > mDwEmmcCommand:%x, mDwEmmcArgument:%x, Status:%r\n", > > > > mDwEmmcCommand, > > > > > mDwEmmcArgument, Status)); > > > > > > > goto out; > > > > > > > } > > > > > > > + > > > > > > > + while(!((MmioRead32(DWEMMC_RINTSTS) & > > > > (DWEMMC_INT_DTO)))) > > > > > { > > > > > > > + Rintsts =3D MmioRead32 (DWEMMC_RINTSTS); } ErrMask =3D > > > > > > > + DWEMMC_INT_EBE | DWEMMC_INT_HLE | DWEMMC_INT_RTO > | > > > > > > > + DWEMMC_INT_RCRC | DWEMMC_INT_RE | > > > > > DWEMMC_INT_DCRC | > > > > > > > + DWEMMC_INT_DRT | DWEMMC_INT_SBE; > > > > > > > + > > > > > > > + if (Rintsts & ErrMask) { > > > > > > > + Status =3D EFI_DEVICE_ERROR; > > > > > > > + goto out; > > > > > > > + } > > > > > > > out: > > > > > > > // Restore Tpl > > > > > > > gBS->RestoreTPL (Tpl); > > > > > > > -- > > > > > > > 2.19.0 > > > > > > >