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 (#57017): https://edk2.groups.io/g/devel/message/57017
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]
-=-=-=-=-=-=-=-=-=-=-=-
