* [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
@ 2020-04-03 9:24 Gaurav Jain
2020-04-06 14:08 ` Leif Lindholm
0 siblings, 1 reply; 14+ messages in thread
From: Gaurav Jain @ 2020-04-03 9:24 UTC (permalink / raw)
To: devel; +Cc: Leif Lindholm, Ard Biesheuvel, Pankaj Bansal, Gaurav Jain
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.jain@nxp.com>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-03 9:24 [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W Gaurav Jain
@ 2020-04-06 14:08 ` Leif Lindholm
2020-04-06 14:12 ` Ard Biesheuvel
0 siblings, 1 reply; 14+ messages in thread
From: Leif Lindholm @ 2020-04-06 14:08 UTC (permalink / raw)
To: Gaurav Jain
Cc: devel, Ard Biesheuvel, Pankaj Bansal, Haojian Zhuang,
Loh, Tien Hock
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.jain@nxp.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-06 14:08 ` Leif Lindholm
@ 2020-04-06 14:12 ` Ard Biesheuvel
2020-04-07 7:01 ` [EXT] " Gaurav Jain
2020-04-27 6:19 ` Pankaj Bansal
0 siblings, 2 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-04-06 14:12 UTC (permalink / raw)
To: Leif Lindholm, Gaurav Jain
Cc: devel, Pankaj Bansal, Haojian Zhuang, Loh, Tien Hock
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.jain@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.
>> ---
>> 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
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-06 14:12 ` Ard Biesheuvel
@ 2020-04-07 7:01 ` Gaurav Jain
2020-04-07 7:52 ` Loh, Tien Hock
2020-04-27 6:19 ` Pankaj Bansal
1 sibling, 1 reply; 14+ messages in thread
From: Gaurav Jain @ 2020-04-07 7:01 UTC (permalink / raw)
To: Ard Biesheuvel, Leif Lindholm
Cc: devel@edk2.groups.io, Pankaj Bansal, Haojian Zhuang,
Loh, Tien Hock
> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Sent: Monday, April 6, 2020 7:42 PM
> To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain <gaurav.jain@nxp.com>
> Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> <tien.hock.loh@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.jain@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
> >>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-07 7:01 ` [EXT] " Gaurav Jain
@ 2020-04-07 7:52 ` Loh, Tien Hock
2020-04-21 6:39 ` Gaurav Jain
0 siblings, 1 reply; 14+ messages in thread
From: Loh, Tien Hock @ 2020-04-07 7:52 UTC (permalink / raw)
To: Gaurav Jain, Ard Biesheuvel, Leif Lindholm
Cc: devel@edk2.groups.io, Pankaj Bansal, Haojian Zhuang
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.jain@nxp.com>
> Sent: Tuesday, April 7, 2020 3:02 PM
> To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Leif Lindholm
> <leif@nuviainc.com>
> Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>; Haojian
> Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> <tien.hock.loh@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.biesheuvel@arm.com>
> > Sent: Monday, April 6, 2020 7:42 PM
> > To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain
> > <gaurav.jain@nxp.com>
> > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > <tien.hock.loh@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.jain@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
> > >>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-07 7:52 ` Loh, Tien Hock
@ 2020-04-21 6:39 ` Gaurav Jain
2020-04-27 2:15 ` Loh, Tien Hock
0 siblings, 1 reply; 14+ messages in thread
From: Gaurav Jain @ 2020-04-21 6:39 UTC (permalink / raw)
To: Loh, Tien Hock, Ard Biesheuvel, Leif Lindholm
Cc: devel@edk2.groups.io, Pankaj Bansal, Haojian Zhuang
Hi Tien Hock
Can you help to review the patch?
Regards
Gaurav Jain
> -----Original Message-----
> From: Loh, Tien Hock <tien.hock.loh@intel.com>
> Sent: Tuesday, April 7, 2020 1:23 PM
> To: Gaurav Jain <gaurav.jain@nxp.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>; Leif Lindholm <leif@nuviainc.com>
> Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> Haojian Zhuang <haojian.zhuang@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.jain@nxp.com>
> > Sent: Tuesday, April 7, 2020 3:02 PM
> > To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Leif Lindholm
> > <leif@nuviainc.com>
> > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > <tien.hock.loh@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.biesheuvel@arm.com>
> > > Sent: Monday, April 6, 2020 7:42 PM
> > > To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain
> > > <gaurav.jain@nxp.com>
> > > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > > Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > > <tien.hock.loh@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.jain@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
> > > >>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-21 6:39 ` Gaurav Jain
@ 2020-04-27 2:15 ` Loh, Tien Hock
0 siblings, 0 replies; 14+ messages in thread
From: Loh, Tien Hock @ 2020-04-27 2:15 UTC (permalink / raw)
To: Gaurav Jain, Ard Biesheuvel, Leif Lindholm
Cc: devel@edk2.groups.io, Pankaj Bansal, Haojian Zhuang
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.jain@nxp.com>
> Sent: Tuesday, April 21, 2020 2:40 PM
> To: Loh, Tien Hock <tien.hock.loh@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>; Leif Lindholm <leif@nuviainc.com>
> Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>; Haojian
> Zhuang <haojian.zhuang@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.loh@intel.com>
> > Sent: Tuesday, April 7, 2020 1:23 PM
> > To: Gaurav Jain <gaurav.jain@nxp.com>; Ard Biesheuvel
> > <ard.biesheuvel@arm.com>; Leif Lindholm <leif@nuviainc.com>
> > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > Haojian Zhuang <haojian.zhuang@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.jain@nxp.com>
> > > Sent: Tuesday, April 7, 2020 3:02 PM
> > > To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Leif Lindholm
> > > <leif@nuviainc.com>
> > > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > > Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > > <tien.hock.loh@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.biesheuvel@arm.com>
> > > > Sent: Monday, April 6, 2020 7:42 PM
> > > > To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain
> > > > <gaurav.jain@nxp.com>
> > > > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > > > Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > > > <tien.hock.loh@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.jain@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
> > > > >>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-06 14:12 ` Ard Biesheuvel
2020-04-07 7:01 ` [EXT] " Gaurav Jain
@ 2020-04-27 6:19 ` Pankaj Bansal
2020-04-29 5:17 ` Loh, Tien Hock
1 sibling, 1 reply; 14+ messages in thread
From: Pankaj Bansal @ 2020-04-27 6:19 UTC (permalink / raw)
To: Ard Biesheuvel, Leif Lindholm, Gaurav Jain, Meenakshi Aggarwal
Cc: devel@edk2.groups.io, Haojian Zhuang, Loh, Tien Hock, Varun Sethi
+ Meenakshi
> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Sent: Monday, April 6, 2020 7:42 PM
> To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain <gaurav.jain@nxp.com>
> Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>; Haojian
> Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock <tien.hock.loh@intel.com>
> Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer
> Limit 65535 in R/W.
>
> 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.jain@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.
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-27 6:19 ` Pankaj Bansal
@ 2020-04-29 5:17 ` Loh, Tien Hock
2020-04-29 11:16 ` Leif Lindholm
0 siblings, 1 reply; 14+ messages in thread
From: Loh, Tien Hock @ 2020-04-29 5:17 UTC (permalink / raw)
To: Pankaj Bansal, Ard Biesheuvel, Leif Lindholm, Gaurav Jain,
Meenakshi Aggarwal
Cc: devel@edk2.groups.io, Haojian Zhuang, Varun Sethi
Hi Ard,
I have checked the patch and it looks good.
However, I can no longer test the patch as the new DwMmc driver no longer uses the protocol.
Sorry for the delay, I initially thought I can test it until I investigated further today.
Thanks
> -----Original Message-----
> From: Pankaj Bansal <pankaj.bansal@nxp.com>
> Sent: Monday, April 27, 2020 2:19 PM
> To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Leif Lindholm
> <leif@nuviainc.com>; Gaurav Jain <gaurav.jain@nxp.com>; Meenakshi
> Aggarwal <meenakshi.aggarwal@nxp.com>
> Cc: devel@edk2.groups.io; Haojian Zhuang <haojian.zhuang@linaro.org>; Loh,
> Tien Hock <tien.hock.loh@intel.com>; Varun Sethi <V.Sethi@nxp.com>
> Subject: RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer
> Limit 65535 in R/W.
>
> + Meenakshi
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > Sent: Monday, April 6, 2020 7:42 PM
> > To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain
> > <gaurav.jain@nxp.com>
> > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > <tien.hock.loh@intel.com>
> > Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer
> > Limit 65535 in R/W.
> >
> > 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.jain@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.
> >
> >
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-29 5:17 ` Loh, Tien Hock
@ 2020-04-29 11:16 ` Leif Lindholm
2020-04-30 1:16 ` Loh, Tien Hock
0 siblings, 1 reply; 14+ messages in thread
From: Leif Lindholm @ 2020-04-29 11:16 UTC (permalink / raw)
To: Loh, Tien Hock
Cc: Pankaj Bansal, Ard Biesheuvel, Gaurav Jain, Meenakshi Aggarwal,
devel@edk2.groups.io, Haojian Zhuang, Varun Sethi
Hi Tien Hock,
Can I take that as a Reviewed-by:?
Regards,
Leif
On Wed, Apr 29, 2020 at 05:17:18 +0000, Loh, Tien Hock wrote:
> Hi Ard,
>
> I have checked the patch and it looks good.
>
> However, I can no longer test the patch as the new DwMmc driver no longer uses the protocol.
> Sorry for the delay, I initially thought I can test it until I investigated further today.
>
> Thanks
>
>
> > -----Original Message-----
> > From: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Sent: Monday, April 27, 2020 2:19 PM
> > To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Leif Lindholm
> > <leif@nuviainc.com>; Gaurav Jain <gaurav.jain@nxp.com>; Meenakshi
> > Aggarwal <meenakshi.aggarwal@nxp.com>
> > Cc: devel@edk2.groups.io; Haojian Zhuang <haojian.zhuang@linaro.org>; Loh,
> > Tien Hock <tien.hock.loh@intel.com>; Varun Sethi <V.Sethi@nxp.com>
> > Subject: RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer
> > Limit 65535 in R/W.
> >
> > + Meenakshi
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > > Sent: Monday, April 6, 2020 7:42 PM
> > > To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain
> > > <gaurav.jain@nxp.com>
> > > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > > Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > > <tien.hock.loh@intel.com>
> > > Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer
> > > Limit 65535 in R/W.
> > >
> > > 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.jain@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.
> > >
> > >
> > >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-29 11:16 ` Leif Lindholm
@ 2020-04-30 1:16 ` Loh, Tien Hock
2020-05-27 11:21 ` [EXT] " Gaurav Jain
0 siblings, 1 reply; 14+ messages in thread
From: Loh, Tien Hock @ 2020-04-30 1:16 UTC (permalink / raw)
To: Leif Lindholm
Cc: Pankaj Bansal, Ard Biesheuvel, Gaurav Jain, Meenakshi Aggarwal,
devel@edk2.groups.io, Haojian Zhuang, Varun Sethi
Hi Leif,
Yes, that's a Reviewed-by.
Thanks.
> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Wednesday, April 29, 2020 7:16 PM
> To: Loh, Tien Hock <tien.hock.loh@intel.com>
> Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>; Gaurav Jain <gaurav.jain@nxp.com>; Meenakshi
> Aggarwal <meenakshi.aggarwal@nxp.com>; devel@edk2.groups.io; Haojian
> Zhuang <haojian.zhuang@linaro.org>; Varun Sethi <V.Sethi@nxp.com>
> Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer
> Limit 65535 in R/W.
>
> Hi Tien Hock,
>
> Can I take that as a Reviewed-by:?
>
> Regards,
>
> Leif
>
> On Wed, Apr 29, 2020 at 05:17:18 +0000, Loh, Tien Hock wrote:
> > Hi Ard,
> >
> > I have checked the patch and it looks good.
> >
> > However, I can no longer test the patch as the new DwMmc driver no longer
> uses the protocol.
> > Sorry for the delay, I initially thought I can test it until I investigated further
> today.
> >
> > Thanks
> >
> >
> > > -----Original Message-----
> > > From: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > Sent: Monday, April 27, 2020 2:19 PM
> > > To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Leif Lindholm
> > > <leif@nuviainc.com>; Gaurav Jain <gaurav.jain@nxp.com>; Meenakshi
> > > Aggarwal <meenakshi.aggarwal@nxp.com>
> > > Cc: devel@edk2.groups.io; Haojian Zhuang
> > > <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > > <tien.hock.loh@intel.com>; Varun Sethi <V.Sethi@nxp.com>
> > > Subject: RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> Transfer
> > > Limit 65535 in R/W.
> > >
> > > + Meenakshi
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > > > Sent: Monday, April 6, 2020 7:42 PM
> > > > To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain
> > > > <gaurav.jain@nxp.com>
> > > > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > > > Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > > > <tien.hock.loh@intel.com>
> > > > Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> > > > Transfer Limit 65535 in R/W.
> > > >
> > > > 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.jain@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.
> > > >
> > > >
> > > >
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [EXT] RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-04-30 1:16 ` Loh, Tien Hock
@ 2020-05-27 11:21 ` Gaurav Jain
2020-06-12 8:12 ` Ard Biesheuvel
2020-11-26 7:16 ` [edk2-devel] " Gaurav Jain
0 siblings, 2 replies; 14+ messages in thread
From: Gaurav Jain @ 2020-05-27 11:21 UTC (permalink / raw)
To: Loh, Tien Hock, Leif Lindholm
Cc: Pankaj Bansal, Ard Biesheuvel, Meenakshi Aggarwal,
devel@edk2.groups.io, Haojian Zhuang, Varun Sethi
Hi Leif
Tein Hock has reviewed this patch.
Please help to merge in edk2.
Regards
Gaurav Jain
> -----Original Message-----
> From: Loh, Tien Hock <tien.hock.loh@intel.com>
> Sent: Thursday, April 30, 2020 6:47 AM
> To: Leif Lindholm <leif@nuviainc.com>
> Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>; Gaurav Jain <gaurav.jain@nxp.com>;
> Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> devel@edk2.groups.io; Haojian Zhuang <haojian.zhuang@linaro.org>; Varun
> Sethi <V.Sethi@nxp.com>
> Subject: [EXT] RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> Transfer Limit 65535 in R/W.
>
> Caution: EXT Email
>
> Hi Leif,
>
> Yes, that's a Reviewed-by.
>
> Thanks.
>
> > -----Original Message-----
> > From: Leif Lindholm <leif@nuviainc.com>
> > Sent: Wednesday, April 29, 2020 7:16 PM
> > To: Loh, Tien Hock <tien.hock.loh@intel.com>
> > Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Ard Biesheuvel
> > <ard.biesheuvel@arm.com>; Gaurav Jain <gaurav.jain@nxp.com>;
> Meenakshi
> > Aggarwal <meenakshi.aggarwal@nxp.com>; devel@edk2.groups.io;
> Haojian
> > Zhuang <haojian.zhuang@linaro.org>; Varun Sethi <V.Sethi@nxp.com>
> > Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> Transfer
> > Limit 65535 in R/W.
> >
> > Hi Tien Hock,
> >
> > Can I take that as a Reviewed-by:?
> >
> > Regards,
> >
> > Leif
> >
> > On Wed, Apr 29, 2020 at 05:17:18 +0000, Loh, Tien Hock wrote:
> > > Hi Ard,
> > >
> > > I have checked the patch and it looks good.
> > >
> > > However, I can no longer test the patch as the new DwMmc driver no
> > > longer
> > uses the protocol.
> > > Sorry for the delay, I initially thought I can test it until I
> > > investigated further
> > today.
> > >
> > > Thanks
> > >
> > >
> > > > -----Original Message-----
> > > > From: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Sent: Monday, April 27, 2020 2:19 PM
> > > > To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Leif Lindholm
> > > > <leif@nuviainc.com>; Gaurav Jain <gaurav.jain@nxp.com>; Meenakshi
> > > > Aggarwal <meenakshi.aggarwal@nxp.com>
> > > > Cc: devel@edk2.groups.io; Haojian Zhuang
> > > > <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > > > <tien.hock.loh@intel.com>; Varun Sethi <V.Sethi@nxp.com>
> > > > Subject: RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> > Transfer
> > > > Limit 65535 in R/W.
> > > >
> > > > + Meenakshi
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > > > > Sent: Monday, April 6, 2020 7:42 PM
> > > > > To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain
> > > > > <gaurav.jain@nxp.com>
> > > > > Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
> > > > > Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
> > > > > <tien.hock.loh@intel.com>
> > > > > Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> > > > > Transfer Limit 65535 in R/W.
> > > > >
> > > > > 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.jain@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.
> > > > >
> > > > >
> > > > >
> > >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [EXT] RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-05-27 11:21 ` [EXT] " Gaurav Jain
@ 2020-06-12 8:12 ` Ard Biesheuvel
2020-11-26 7:16 ` [edk2-devel] " Gaurav Jain
1 sibling, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-06-12 8:12 UTC (permalink / raw)
To: Gaurav Jain, Loh, Tien Hock, Leif Lindholm
Cc: Pankaj Bansal, Meenakshi Aggarwal, devel@edk2.groups.io,
Haojian Zhuang, Varun Sethi
On 5/27/20 1:21 PM, Gaurav Jain wrote:
> Hi Leif
>
> Tein Hock has reviewed this patch.
> Please help to merge in edk2.
>
Merged as #687
Thanks all
>> -----Original Message-----
>> From: Loh, Tien Hock <tien.hock.loh@intel.com>
>> Sent: Thursday, April 30, 2020 6:47 AM
>> To: Leif Lindholm <leif@nuviainc.com>
>> Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Ard Biesheuvel
>> <ard.biesheuvel@arm.com>; Gaurav Jain <gaurav.jain@nxp.com>;
>> Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
>> devel@edk2.groups.io; Haojian Zhuang <haojian.zhuang@linaro.org>; Varun
>> Sethi <V.Sethi@nxp.com>
>> Subject: [EXT] RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
>> Transfer Limit 65535 in R/W.
>>
>> Caution: EXT Email
>>
>> Hi Leif,
>>
>> Yes, that's a Reviewed-by.
>>
>> Thanks.
>>
>>> -----Original Message-----
>>> From: Leif Lindholm <leif@nuviainc.com>
>>> Sent: Wednesday, April 29, 2020 7:16 PM
>>> To: Loh, Tien Hock <tien.hock.loh@intel.com>
>>> Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Ard Biesheuvel
>>> <ard.biesheuvel@arm.com>; Gaurav Jain <gaurav.jain@nxp.com>;
>> Meenakshi
>>> Aggarwal <meenakshi.aggarwal@nxp.com>; devel@edk2.groups.io;
>> Haojian
>>> Zhuang <haojian.zhuang@linaro.org>; Varun Sethi <V.Sethi@nxp.com>
>>> Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
>> Transfer
>>> Limit 65535 in R/W.
>>>
>>> Hi Tien Hock,
>>>
>>> Can I take that as a Reviewed-by:?
>>>
>>> Regards,
>>>
>>> Leif
>>>
>>> On Wed, Apr 29, 2020 at 05:17:18 +0000, Loh, Tien Hock wrote:
>>>> Hi Ard,
>>>>
>>>> I have checked the patch and it looks good.
>>>>
>>>> However, I can no longer test the patch as the new DwMmc driver no
>>>> longer
>>> uses the protocol.
>>>> Sorry for the delay, I initially thought I can test it until I
>>>> investigated further
>>> today.
>>>>
>>>> Thanks
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Pankaj Bansal <pankaj.bansal@nxp.com>
>>>>> Sent: Monday, April 27, 2020 2:19 PM
>>>>> To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Leif Lindholm
>>>>> <leif@nuviainc.com>; Gaurav Jain <gaurav.jain@nxp.com>; Meenakshi
>>>>> Aggarwal <meenakshi.aggarwal@nxp.com>
>>>>> Cc: devel@edk2.groups.io; Haojian Zhuang
>>>>> <haojian.zhuang@linaro.org>; Loh, Tien Hock
>>>>> <tien.hock.loh@intel.com>; Varun Sethi <V.Sethi@nxp.com>
>>>>> Subject: RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
>>> Transfer
>>>>> Limit 65535 in R/W.
>>>>>
>>>>> + Meenakshi
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>>>>> Sent: Monday, April 6, 2020 7:42 PM
>>>>>> To: Leif Lindholm <leif@nuviainc.com>; Gaurav Jain
>>>>>> <gaurav.jain@nxp.com>
>>>>>> Cc: devel@edk2.groups.io; Pankaj Bansal <pankaj.bansal@nxp.com>;
>>>>>> Haojian Zhuang <haojian.zhuang@linaro.org>; Loh, Tien Hock
>>>>>> <tien.hock.loh@intel.com>
>>>>>> Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
>>>>>> Transfer Limit 65535 in R/W.
>>>>>>
>>>>>> 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.jain@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.
>>>>>>
>>>>>>
>>>>>>
>>>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [EXT] RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.
2020-05-27 11:21 ` [EXT] " Gaurav Jain
2020-06-12 8:12 ` Ard Biesheuvel
@ 2020-11-26 7:16 ` Gaurav Jain
1 sibling, 0 replies; 14+ messages in thread
From: Gaurav Jain @ 2020-11-26 7:16 UTC (permalink / raw)
To: Gaurav Jain, devel
[-- Attachment #1: Type: text/plain, Size: 76 bytes --]
Hi Leif
This Patch is still not merged in edk2.
Regards
Gaurav Jain
[-- Attachment #2: Type: text/html, Size: 96 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-11-26 7:16 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-03 9:24 [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W Gaurav Jain
2020-04-06 14:08 ` Leif Lindholm
2020-04-06 14:12 ` Ard Biesheuvel
2020-04-07 7:01 ` [EXT] " Gaurav Jain
2020-04-07 7:52 ` Loh, Tien Hock
2020-04-21 6:39 ` Gaurav Jain
2020-04-27 2:15 ` Loh, Tien Hock
2020-04-27 6:19 ` Pankaj Bansal
2020-04-29 5:17 ` Loh, Tien Hock
2020-04-29 11:16 ` Leif Lindholm
2020-04-30 1:16 ` Loh, Tien Hock
2020-05-27 11:21 ` [EXT] " Gaurav Jain
2020-06-12 8:12 ` Ard Biesheuvel
2020-11-26 7:16 ` [edk2-devel] " Gaurav Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox