From: "Leif Lindholm" <leif.lindholm@linaro.org>
To: "Loh, Tien Hock" <tien.hock.loh@intel.com>
Cc: 'Haojian Zhuang' <haojian.zhuang@linaro.org>,
"'devel@edk2.groups.io'" <devel@edk2.groups.io>,
"'thloh85@gmail.com'" <thloh85@gmail.com>,
'Ard Biesheuvel' <ard.biesheuvel@linaro.org>
Subject: Re: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand polling
Date: Tue, 11 Jun 2019 10:08:47 +0100 [thread overview]
Message-ID: <20190611090847.lryjgzjq3y5xi5io@bivouac.eciton.net> (raw)
In-Reply-To: <EF88013823EA1B42AC7142BA7DA05E0634CAC3D8@PGSMSX110.gar.corp.intel.com>
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 time.
> 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.
/
Leif
> > -----Original Message-----
> > From: Loh, Tien Hock
> > Sent: Thursday, May 30, 2019 3:56 PM
> > To: Haojian Zhuang <haojian.zhuang@linaro.org>; Leif Lindholm
> > <leif.lindholm@linaro.org>
> > Cc: devel@edk2.groups.io; thloh85@gmail.com; Ard Biesheuvel
> > <ard.biesheuvel@linaro.org>
> > Subject: RE: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand
> > polling
> >
> > > -----Original Message-----
> > > From: Haojian Zhuang <haojian.zhuang@linaro.org>
> > > Sent: Thursday, May 30, 2019 3:06 PM
> > > To: Leif Lindholm <leif.lindholm@linaro.org>
> > > Cc: Loh, Tien Hock <tien.hock.loh@intel.com>; devel@edk2.groups.io;
> > > thloh85@gmail.com; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > 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 platform.
> > >
> > > 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" <tien.hock.loh@intel.com>
> > > > >
> > > > > 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" <tien.hock.loh@intel.com>
> > > > > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > ---
> > > > > 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 |= DWEMMC_INT_DCRC | DWEMMC_INT_DRT |
> > > DWEMMC_INT_SBE;
> > > > > do {
> > > > > - MicroSecondDelay(500);
> > > > > Data = 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 = 0;
> > > > >
> > > > > Tpl = 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 = MmioRead32 (DWEMMC_RINTSTS); } ErrMask =
> > > > > + 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 = 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 = 0;
> > > > >
> > > > > Tpl = 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 = MmioRead32 (DWEMMC_RINTSTS); } ErrMask =
> > > > > + 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 = EFI_DEVICE_ERROR;
> > > > > + goto out;
> > > > > + }
> > > > > out:
> > > > > // Restore Tpl
> > > > > gBS->RestoreTPL (Tpl);
> > > > > --
> > > > > 2.19.0
> > > > >
next prev parent reply other threads:[~2019-06-11 9:08 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-27 9:30 [PATCH v2 0/7] Fix some bugs with DwEmmc Loh, Tien Hock
2019-05-27 9:30 ` [PATCH v2 1/7] EmbeddedPkg: Add SD command support for DwEmmc Loh, Tien Hock
2019-05-28 17:37 ` Leif Lindholm
2019-05-27 9:30 ` [PATCH v2 2/7] EmbeddedPkg: Fix DwEmmc CMD8 support for SD Loh, Tien Hock
2019-05-28 17:45 ` Leif Lindholm
2019-05-27 9:30 ` [PATCH v2 3/7] EmbeddedPkg: Send command when MMC ask for response Loh, Tien Hock
2019-05-28 17:56 ` Leif Lindholm
2019-05-27 9:30 ` [PATCH v2 4/7] EmbeddedPkg: Fix response check flag Loh, Tien Hock
2019-05-28 17:58 ` Leif Lindholm
2019-05-27 9:30 ` [PATCH v2 5/7] EmbeddedPkg: Clear CTYPE on initialization Loh, Tien Hock
2019-05-28 17:58 ` Leif Lindholm
2019-05-27 9:30 ` [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand polling Loh, Tien Hock
2019-05-28 18:04 ` Leif Lindholm
2019-05-30 7:05 ` Haojian Zhuang
2019-05-30 7:56 ` Loh, Tien Hock
2019-06-11 2:40 ` Loh, Tien Hock
2019-06-11 9:08 ` Leif Lindholm [this message]
2019-06-11 9:12 ` Loh, Tien Hock
2019-07-12 5:40 ` Loh, Tien Hock
2019-05-27 9:30 ` [PATCH v2 7/7] EmbeddedPkg: Fix DwEmmc read/write size in preparing DMA size Loh, Tien Hock
2019-05-28 17:50 ` Leif Lindholm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190611090847.lryjgzjq3y5xi5io@bivouac.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox