Hi Gaurav,
Sorry for the late reply. I wanted to test the patch last week but were fixing
a bug in our platform initialization bug that caused MMC to fail, thus I were
unable to test.
I'll test it in a few days.
Thanks!
> -----Original Message-----
> From: Gaurav Jain <gaurav.j...@nxp.com>
> Sent: Tuesday, April 21, 2020 2:40 PM
> To: Loh, Tien Hock <tien.hock....@intel.com>; Ard Biesheuvel
> <ard.biesheu...@arm.com>; Leif Lindholm <l...@nuviainc.com>
> Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.ban...@nxp.com>; Haojian
> Zhuang <haojian.zhu...@linaro.org>
> Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> Transfer Limit 65535 in R/W.
>
> Hi Tien Hock
>
> Can you help to review the patch?
>
> Regards
> Gaurav Jain
>
> > -----Original Message-----
> > From: Loh, Tien Hock <tien.hock....@intel.com>
> > Sent: Tuesday, April 7, 2020 1:23 PM
> > To: Gaurav Jain <gaurav.j...@nxp.com>; Ard Biesheuvel
> > <ard.biesheu...@arm.com>; Leif Lindholm <l...@nuviainc.com>
> > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.ban...@nxp.com>;
> > Haojian Zhuang <haojian.zhu...@linaro.org>
> > Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> > Transfer Limit 65535 in R/W.
> >
> > Caution: EXT Email
> >
> > Hi Leif, Gaurav,
> >
> > The changes look good to me, but I haven't tested it on Intel's
> > SoCFPGA platform.
> > I will need some time to test it as I'm working on some other tasks,
> > maybe in a week or so.
> >
> > Thanks
> >
> > > -----Original Message-----
> > > From: Gaurav Jain <gaurav.j...@nxp.com>
> > > Sent: Tuesday, April 7, 2020 3:02 PM
> > > To: Ard Biesheuvel <ard.biesheu...@arm.com>; Leif Lindholm
> > > <l...@nuviainc.com>
> > > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.ban...@nxp.com>;
> > > Haojian Zhuang <haojian.zhu...@linaro.org>; Loh, Tien Hock
> > > <tien.hock....@intel.com>
> > > Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added
> > MaxBlock
> > > Transfer Limit 65535 in R/W.
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <ard.biesheu...@arm.com>
> > > > Sent: Monday, April 6, 2020 7:42 PM
> > > > To: Leif Lindholm <l...@nuviainc.com>; Gaurav Jain
> > > > <gaurav.j...@nxp.com>
> > > > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.ban...@nxp.com>;
> > > > Haojian Zhuang <haojian.zhu...@linaro.org>; Loh, Tien Hock
> > > > <tien.hock....@intel.com>
> > > > Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> > > > Transfer Limit 65535 in R/W.
> > > >
> > > > Caution: EXT Email
> > > >
> > > > On 4/6/20 4:08 PM, Leif Lindholm wrote:
> > > > > Hi Gaurav,
> > > > >
> > > > > Haojian, Tien Hock - can you help review/test this change?
> > > > >
> > > > > Best Regards,
> > > > >
> > > > > Leif
> > > > >
> > > > > On Fri, Apr 03, 2020 at 14:54:07 +0530, Gaurav Jain wrote:
> > > > >> Moved BlockCount calculation below BufferSize Validation checks.
> > > > >> First Ensure Buffersize is Not Zero and multiple of Media BlockSize.
> > > > >> then calculate BlockCount and perform Block checks.
> > > > >>
> > > > >> Corrected BlockCount calculation, as BufferSize is multiple of
> > > > >> BlockSize, So adding (BlockSize-1) bytes to BufferSize and then
> > > > >> divide by BlockSize will have no impact on BlockCount.
> > > > >>
> > > > >> Reading Large Images from MMC causes errors.
> > > > >> As per SD Host Controller Spec version 4.20, Restriction of
> > > > >> 16-bit Block Count transfer is 65535.
> > > > >> Max block transfer limit in single cmd is 65535 blocks.
> > > > >> Added Max Block check that can be processed is 0xFFFF.
> > > > >> then Update BlockCount on the basis of MaxBlock.
> > > > >>
> > > > >> Signed-off-by: Gaurav Jain <gaurav.j...@nxp.com>
> > > >
> > > >
> > > > Hello Gaurav,
> > > >
> > > > Could you please elaborate on the underlying need for this change?
> > > > If you are considering using this driver for future NXP platforms,
> > > > I should point out that this legacy driver is only kept around for
> > > > existing users, and new users should use the driver stack in
> > > > MdeModulePkg, which is based on the UEFI spec.
> > > >
> > > > --
> > > > Ard.
> > >
> > > Hello Ard
> > >
> > > This change is for existing Platforms as well, that are using
> > > EmbeddedPkg driver.
> > > I can see Max Block Transfer Limit in MdeModulePkg also.
> > > This Limit is not defined in EmbeddedPkg, which is causing errors on
> > > NXP existing platform While reading Large images from MMC.
> > > Block transfer limit is defined in SD spec.
> > >
> > > Regards
> > > Gaurav Jain
> > > >
> > > >
> > > >
> > > > >> ---
> > > > >> EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c | 38
> > > > ++++++++++++++++++++-----------
> > > > >> 1 file changed, 25 insertions(+), 13 deletions(-)
> > > > >>
> > > > >> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > > >> b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > > >> index 17c20c0159ba..b508c466d9c5 100644
> > > > >> --- a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > > >> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > > >> @@ -242,6 +242,8 @@ MmcIoBlocks (
> > > > >> UINTN BytesRemainingToBeTransfered;
> > > > >> UINTN BlockCount;
> > > > >> UINTN ConsumeSize;
> > > > >> + UINT32 MaxBlock;
> > > > >> + UINTN RemainingBlock;
> > > > >>
> > > > >> BlockCount = 1;
> > > > >> MmcHostInstance =
> MMC_HOST_INSTANCE_FROM_BLOCK_IO_THIS
> > > > (This); @@
> > > > >> -262,19 +264,6 @@ MmcIoBlocks (
> > > > >> return EFI_NO_MEDIA;
> > > > >> }
> > > > >>
> > > > >> - if (MMC_HOST_HAS_ISMULTIBLOCK(MmcHost) && MmcHost-
> > > > >IsMultiBlock(MmcHost)) {
> > > > >> - BlockCount = (BufferSize + This->Media->BlockSize - 1) / This-
> > >Media-
> > > > >BlockSize;
> > > > >> - }
> > > > >> -
> > > > >> - // All blocks must be within the device
> > > > >> - if ((Lba + (BufferSize / This->Media->BlockSize)) >
> > > > >> (This->Media-
> > > > >LastBlock + 1)) {
> > > > >> - return EFI_INVALID_PARAMETER;
> > > > >> - }
> > > > >> -
> > > > >> - if ((Transfer == MMC_IOBLOCKS_WRITE) && (This->Media-
> > >ReadOnly
> > > > == TRUE)) {
> > > > >> - return EFI_WRITE_PROTECTED;
> > > > >> - }
> > > > >> -
> > > > >> // Reading 0 Byte is valid
> > > > >> if (BufferSize == 0) {
> > > > >> return EFI_SUCCESS;
> > > > >> @@ -285,14 +274,36 @@ MmcIoBlocks (
> > > > >> return EFI_BAD_BUFFER_SIZE;
> > > > >> }
> > > > >>
> > > > >> + if (MMC_HOST_HAS_ISMULTIBLOCK(MmcHost) && MmcHost-
> > > > >IsMultiBlock(MmcHost)) {
> > > > >> + BlockCount = BufferSize / This->Media->BlockSize; }
> > > > >> +
> > > > >> + // All blocks must be within the device if ((Lba +
> > > > >> + (BufferSize /
> > > > >> + This->Media->BlockSize)) > (This->Media->LastBlock + 1)) {
> > > > >> + return EFI_INVALID_PARAMETER; }
> > > > >> +
> > > > >> + if ((Transfer == MMC_IOBLOCKS_WRITE) && (This->Media-
> > >ReadOnly
> > > > == TRUE)) {
> > > > >> + return EFI_WRITE_PROTECTED; }
> > > > >> +
> > > > >> // Check the alignment
> > > > >> if ((This->Media->IoAlign > 2) && (((UINTN)Buffer &
> > > > >> (This->Media-
> > > > >IoAlign - 1)) != 0)) {
> > > > >> return EFI_INVALID_PARAMETER;
> > > > >> }
> > > > >>
> > > > >> + // Max block number in single cmd is 65535 blocks.
> > > > >> + MaxBlock = 0xFFFF;
> > > > >> + RemainingBlock = BlockCount;
> > > > >> BytesRemainingToBeTransfered = BufferSize;
> > > > >> while (BytesRemainingToBeTransfered > 0) {
> > > > >>
> > > > >> + if (RemainingBlock <= MaxBlock) {
> > > > >> + BlockCount = RemainingBlock;
> > > > >> + } else {
> > > > >> + BlockCount = MaxBlock;
> > > > >> + }
> > > > >> +
> > > > >> // Check if the Card is in Ready status
> > > > >> CmdArg = MmcHostInstance->CardInfo.RCA << 16;
> > > > >> Response[0] = 0;
> > > > >> @@ -338,6 +349,7 @@ MmcIoBlocks (
> > > > >> DEBUG ((EFI_D_ERROR, "%a(): Failed to transfer block
> > > > >> and
> > > > Status:%r\n", __func__, Status));
> > > > >> }
> > > > >>
> > > > >> + RemainingBlock -= BlockCount;
> > > > >> BytesRemainingToBeTransfered -= ConsumeSize;
> > > > >> if (BytesRemainingToBeTransfered > 0) {
> > > > >> Lba += BlockCount;
> > > > >> --
> > > > >> 2.7.4
> > > > >>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#58158): https://edk2.groups.io/g/devel/message/58158
Mute This Topic: https://groups.io/mt/72845502/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
