| Message ID | 1386653960-4511-3-git-send-email-haijun.zhang@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | Pantelis Antoniou |
| Headers | show |
Hi Haijun, On Dec 10, 2013, at 7:39 AM, Haijun Zhang wrote: > This patch enhances the currently implemented erase procedure in u-boot, > which has the following problems/missing features..." > ^ this is exactly what I responded earlier. What are the problems? What are the missing features? > Erass sequence: > 1. check if erase command is support by card. If not return. > 2. Check the erase range to see if it was aligned. The min erase size > should be one erase group. SD card it was one block(512), mmc card > it should be one erase group. > 3. If not, aligned the erase rang according to the erase group size. > 4. Send erase command to erase card once one erase group. > 5. If it was SD card, erase with arg 0x00000000 should be send. > else if support secure feature, erase with arg 0x80000000(Spec eMMC 4.41). > else erase with arg 0x00000000.(Trim and discard is ingnored here) > 6. Check card status after erase. > > Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com> > --- > changes for V3: > - update the commit message and secure feature supporting judgment. > > drivers/mmc/mmc_write.c | 68 +++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 58 insertions(+), 10 deletions(-) > > diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c > index aa2fdef..c2dafa3 100644 > --- a/drivers/mmc/mmc_write.c > +++ b/drivers/mmc/mmc_write.c > @@ -17,6 +17,10 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt) > struct mmc_cmd cmd; > ulong end; > int err, start_cmd, end_cmd; > + uint arg = 0; > + > + if (!(mmc->cmdclass & CCC_ERASE)) > + return NOT_SUPPORT; > > if (mmc->high_capacity) { > end = start + blkcnt - 1; > @@ -48,8 +52,15 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt) > if (err) > goto err_out; > > + /* SD card only support %MMC_ERASE_ARG */ > + if (!IS_SD(mmc) && > + (mmc->sec_feature_support & EXT_CSD_SEC_ER_EN)) > + arg = MMC_SECURE_ERASE_ARG; > + else > + arg = MMC_ERASE_ARG; > + > cmd.cmdidx = MMC_CMD_ERASE; > - cmd.cmdarg = SECURE_ERASE; > + cmd.cmdarg = arg; > cmd.resp_type = MMC_RSP_R1b; > > err = mmc_send_cmd(mmc, &cmd, NULL); > @@ -69,24 +80,61 @@ unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt) > struct mmc *mmc = find_mmc_device(dev_num); > lbaint_t blk = 0, blk_r = 0; > int timeout = 1000; > + int res; > + bool align = true; > > if (!mmc) > return -1; > > + if (!(mmc->cmdclass & CCC_ERASE)) { > + printf("\nErase operation is not support by card\n"); > + return NOT_SUPPORT; > + } > + > if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size)) > + align = false; > + > + res = start % mmc->erase_grp_size; > + if (res) { > + res = mmc->erase_grp_size - res; > + start += res; > + if (blkcnt > res) > + blkcnt -= res; > + else { > + printf("\nErase size smaller than Erase group " > + "size [0x%x] is not support by the device.\n", > + mmc->erase_grp_size); > + return NOT_SUPPORT; > + } > + } > + > + res = blkcnt % mmc->erase_grp_size; > + if (res) > + blkcnt -= res; > + > + if (!blkcnt) { > + printf("\nErase size smaller than Erase group size [0x%x] " > + "is not support by the device.\n", > + mmc->erase_grp_size); > + return NOT_SUPPORT; > + } > + > + if (!align) > printf("\n\nCaution! Your devices Erase group is 0x%x\n" > - "The erase range would be change to " > - "0x" LBAF "~0x" LBAF "\n\n", > - mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1), > - ((start + blkcnt + mmc->erase_grp_size) > - & ~(mmc->erase_grp_size - 1)) - 1); > + "The erase range would be change to " > + "0x" LBAF "~0x" LBAF "\n\n", > + mmc->erase_grp_size, start, start + blkcnt); > + > > while (blk < blkcnt) { > - blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ? > - mmc->erase_grp_size : (blkcnt - blk); > + if ((blkcnt - blk) >= mmc->erase_grp_size) > + blk_r = mmc->erase_grp_size; > err = mmc_erase_t(mmc, start + blk, blk_r); > - if (err) > - break; > + if (err) { > + printf("\nErase range from 0x" LBAF "~0x" LBAF > + " Failed\n", start + blk, blkcnt); > + return COMM_ERR; > + } > > blk += blk_r; > > -- > 1.8.4.1 > >
Hi On Tue, Dec 10, 2013 at 6:39 AM, Haijun Zhang <haijun.zhang@freescale.com> wrote: > This patch enhances the currently implemented erase procedure in u-boot, > which has the following problems/missing features..." > > Erass sequence: > 1. check if erase command is support by card. If not return. > 2. Check the erase range to see if it was aligned. The min erase size > should be one erase group. SD card it was one block(512), mmc card > it should be one erase group. > 3. If not, aligned the erase rang according to the erase group size. > 4. Send erase command to erase card once one erase group. > 5. If it was SD card, erase with arg 0x00000000 should be send. > else if support secure feature, erase with arg 0x80000000(Spec eMMC 4.41). > else erase with arg 0x00000000.(Trim and discard is ingnored here) > 6. Check card status after erase. > > Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com> > --- > changes for V3: > - update the commit message and secure feature supporting judgment. > > drivers/mmc/mmc_write.c | 68 +++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 58 insertions(+), 10 deletions(-) > > diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c > index aa2fdef..c2dafa3 100644 > --- a/drivers/mmc/mmc_write.c > +++ b/drivers/mmc/mmc_write.c > @@ -17,6 +17,10 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt) > struct mmc_cmd cmd; > ulong end; > int err, start_cmd, end_cmd; > + uint arg = 0; > + > + if (!(mmc->cmdclass & CCC_ERASE)) > + return NOT_SUPPORT; > > if (mmc->high_capacity) { > end = start + blkcnt - 1; > @@ -48,8 +52,15 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt) > if (err) > goto err_out; > > + /* SD card only support %MMC_ERASE_ARG */ > + if (!IS_SD(mmc) && > + (mmc->sec_feature_support & EXT_CSD_SEC_ER_EN)) > + arg = MMC_SECURE_ERASE_ARG; > + else > + arg = MMC_ERASE_ARG; > + > cmd.cmdidx = MMC_CMD_ERASE; > - cmd.cmdarg = SECURE_ERASE; > + cmd.cmdarg = arg; > cmd.resp_type = MMC_RSP_R1b; > > err = mmc_send_cmd(mmc, &cmd, NULL); > @@ -69,24 +80,61 @@ unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt) > struct mmc *mmc = find_mmc_device(dev_num); > lbaint_t blk = 0, blk_r = 0; > int timeout = 1000; > + int res; > + bool align = true; > > if (!mmc) > return -1; > > + if (!(mmc->cmdclass & CCC_ERASE)) { > + printf("\nErase operation is not support by card\n"); > + return NOT_SUPPORT; > + } > + > if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size)) > + align = false; > + > + res = start % mmc->erase_grp_size; > + if (res) { ... Well I don't know if it works end = start + bkcnt; start += group_size - (start % group_size); end -= (end % group_size); if (start >= end) printf("invalid size\n"); But why not somenthing like this? Michael > + start += mmc->erase_grp_size - res; if (res < blkcnt) { } > + if (blkcnt > res) > + blkcnt -= res; > + else { > + printf("\nErase size smaller than Erase group " > + "size [0x%x] is not support by the device.\n", > + mmc->erase_grp_size); > + return NOT_SUPPORT; > + } > + } > + > + res = (blkcnt - res) % mmc->erase_grp_size; > + if (res) > + blkcnt -= res; > + > + if (!blkcnt) { > + printf("\nErase size smaller than Erase group size [0x%x] " > + "is not support by the device.\n", > + mmc->erase_grp_size); > + return NOT_SUPPORT; > + } > + > + if (!align) > printf("\n\nCaution! Your devices Erase group is 0x%x\n" > - "The erase range would be change to " > - "0x" LBAF "~0x" LBAF "\n\n", > - mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1), > - ((start + blkcnt + mmc->erase_grp_size) > - & ~(mmc->erase_grp_size - 1)) - 1); > + "The erase range would be change to " > + "0x" LBAF "~0x" LBAF "\n\n", > + mmc->erase_grp_size, start, start + blkcnt); > + > > while (blk < blkcnt) { > - blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ? > - mmc->erase_grp_size : (blkcnt - blk); > + if ((blkcnt - blk) >= mmc->erase_grp_size) > + blk_r = mmc->erase_grp_size; > err = mmc_erase_t(mmc, start + blk, blk_r); > - if (err) > - break; > + if (err) { > + printf("\nErase range from 0x" LBAF "~0x" LBAF > + " Failed\n", start + blk, blkcnt); > + return COMM_ERR; > + } > > blk += blk_r; > > -- > 1.8.4.1 > >
Regards & Thanks. -- Haijun > -----Original Message----- > From: Michael Trimarchi [mailto:michael@amarulasolutions.com] > Sent: Wednesday, December 11, 2013 11:10 AM > To: Zhang Haijun-B42677 > Cc: panto@antoniou-consulting.com; u-boot@lists.denx.de; Xie Xiaobo- > R63061; Sun York-R58495; Tom Rini; Stefano Babic; > rjbarnet@rockwellcollins.com; jh80.chung@samsung.com > Subject: Re: [PATCH 3/7 V3] mmc: Enhance erase handling procedure > > Hi > > On Tue, Dec 10, 2013 at 6:39 AM, Haijun Zhang <haijun.zhang@freescale.com> > wrote: > > This patch enhances the currently implemented erase procedure in > > u-boot, which has the following problems/missing features..." > > > > Erass sequence: > > 1. check if erase command is support by card. If not return. > > 2. Check the erase range to see if it was aligned. The min erase size > > should be one erase group. SD card it was one block(512), mmc card it > > should be one erase group. > > 3. If not, aligned the erase rang according to the erase group size. > > 4. Send erase command to erase card once one erase group. > > 5. If it was SD card, erase with arg 0x00000000 should be send. > > else if support secure feature, erase with arg 0x80000000(Spec eMMC > 4.41). > > else erase with arg 0x00000000.(Trim and discard is ingnored here) > > 6. Check card status after erase. > > > > Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com> > > --- > > changes for V3: > > - update the commit message and secure feature supporting > judgment. > > > > drivers/mmc/mmc_write.c | 68 > > +++++++++++++++++++++++++++++++++++++++++-------- > > 1 file changed, 58 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c index > > aa2fdef..c2dafa3 100644 > > --- a/drivers/mmc/mmc_write.c > > +++ b/drivers/mmc/mmc_write.c > > @@ -17,6 +17,10 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong > start, lbaint_t blkcnt) > > struct mmc_cmd cmd; > > ulong end; > > int err, start_cmd, end_cmd; > > + uint arg = 0; > > + > > + if (!(mmc->cmdclass & CCC_ERASE)) > > + return NOT_SUPPORT; > > > > if (mmc->high_capacity) { > > end = start + blkcnt - 1; @@ -48,8 +52,15 @@ static > > ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt) > > if (err) > > goto err_out; > > > > + /* SD card only support %MMC_ERASE_ARG */ > > + if (!IS_SD(mmc) && > > + (mmc->sec_feature_support & EXT_CSD_SEC_ER_EN)) > > + arg = MMC_SECURE_ERASE_ARG; > > + else > > + arg = MMC_ERASE_ARG; > > + > > cmd.cmdidx = MMC_CMD_ERASE; > > - cmd.cmdarg = SECURE_ERASE; > > + cmd.cmdarg = arg; > > cmd.resp_type = MMC_RSP_R1b; > > > > err = mmc_send_cmd(mmc, &cmd, NULL); @@ -69,24 +80,61 @@ > > unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt) > > struct mmc *mmc = find_mmc_device(dev_num); > > lbaint_t blk = 0, blk_r = 0; > > int timeout = 1000; > > + int res; > > + bool align = true; > > > > if (!mmc) > > return -1; > > > > + if (!(mmc->cmdclass & CCC_ERASE)) { > > + printf("\nErase operation is not support by card\n"); > > + return NOT_SUPPORT; > > + } > > + > > if ((start % mmc->erase_grp_size) || (blkcnt % > > mmc->erase_grp_size)) > > + align = false; > > + > > + res = start % mmc->erase_grp_size; > > + if (res) { > ... > > Well I don't know if it works > end = start + bkcnt; > start += group_size - (start % group_size); > end -= (end % group_size); > if (start >= end) > printf("invalid size\n"); > > But why not somenthing like this? > Michael Yes, it's simpler. I'll update it in my next patch version V5. And how about other patches? Haijun > > > + start += mmc->erase_grp_size - res; > if (res < blkcnt) { > > } > > > + if (blkcnt > res) > > + blkcnt -= res; > > + else { > > + printf("\nErase size smaller than Erase group " > > + "size [0x%x] is not support by the > device.\n", > > + mmc->erase_grp_size); > > + return NOT_SUPPORT; > > + } > > + } > > + > > + res = (blkcnt - res) % mmc->erase_grp_size; > > + if (res) > > + blkcnt -= res; > > + > > + if (!blkcnt) { > > + printf("\nErase size smaller than Erase group size > [0x%x] " > > + "is not support by the device.\n", > > + mmc->erase_grp_size); > > + return NOT_SUPPORT; > > + } > > + > > + if (!align) > > printf("\n\nCaution! Your devices Erase group is > 0x%x\n" > > - "The erase range would be change to " > > - "0x" LBAF "~0x" LBAF "\n\n", > > - mmc->erase_grp_size, start & ~(mmc- > >erase_grp_size - 1), > > - ((start + blkcnt + mmc->erase_grp_size) > > - & ~(mmc->erase_grp_size - 1)) - 1); > > + "The erase range would be change to " > > + "0x" LBAF "~0x" LBAF "\n\n", > > + mmc->erase_grp_size, start, start + blkcnt); > > + > > > > while (blk < blkcnt) { > > - blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ? > > - mmc->erase_grp_size : (blkcnt - blk); > > + if ((blkcnt - blk) >= mmc->erase_grp_size) > > + blk_r = mmc->erase_grp_size; > > err = mmc_erase_t(mmc, start + blk, blk_r); > > - if (err) > > - break; > > + if (err) { > > + printf("\nErase range from 0x" LBAF "~0x" LBAF > > + " Failed\n", start + blk, blkcnt); > > + return COMM_ERR; > > + } > > > > blk += blk_r; > > > > -- > > 1.8.4.1 > > > > >
diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c index aa2fdef..c2dafa3 100644 --- a/drivers/mmc/mmc_write.c +++ b/drivers/mmc/mmc_write.c @@ -17,6 +17,10 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt) struct mmc_cmd cmd; ulong end; int err, start_cmd, end_cmd; + uint arg = 0; + + if (!(mmc->cmdclass & CCC_ERASE)) + return NOT_SUPPORT; if (mmc->high_capacity) { end = start + blkcnt - 1; @@ -48,8 +52,15 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt) if (err) goto err_out; + /* SD card only support %MMC_ERASE_ARG */ + if (!IS_SD(mmc) && + (mmc->sec_feature_support & EXT_CSD_SEC_ER_EN)) + arg = MMC_SECURE_ERASE_ARG; + else + arg = MMC_ERASE_ARG; + cmd.cmdidx = MMC_CMD_ERASE; - cmd.cmdarg = SECURE_ERASE; + cmd.cmdarg = arg; cmd.resp_type = MMC_RSP_R1b; err = mmc_send_cmd(mmc, &cmd, NULL); @@ -69,24 +80,61 @@ unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt) struct mmc *mmc = find_mmc_device(dev_num); lbaint_t blk = 0, blk_r = 0; int timeout = 1000; + int res; + bool align = true; if (!mmc) return -1; + if (!(mmc->cmdclass & CCC_ERASE)) { + printf("\nErase operation is not support by card\n"); + return NOT_SUPPORT; + } + if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size)) + align = false; + + res = start % mmc->erase_grp_size; + if (res) { + res = mmc->erase_grp_size - res; + start += res; + if (blkcnt > res) + blkcnt -= res; + else { + printf("\nErase size smaller than Erase group " + "size [0x%x] is not support by the device.\n", + mmc->erase_grp_size); + return NOT_SUPPORT; + } + } + + res = blkcnt % mmc->erase_grp_size; + if (res) + blkcnt -= res; + + if (!blkcnt) { + printf("\nErase size smaller than Erase group size [0x%x] " + "is not support by the device.\n", + mmc->erase_grp_size); + return NOT_SUPPORT; + } + + if (!align) printf("\n\nCaution! Your devices Erase group is 0x%x\n" - "The erase range would be change to " - "0x" LBAF "~0x" LBAF "\n\n", - mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1), - ((start + blkcnt + mmc->erase_grp_size) - & ~(mmc->erase_grp_size - 1)) - 1); + "The erase range would be change to " + "0x" LBAF "~0x" LBAF "\n\n", + mmc->erase_grp_size, start, start + blkcnt); + while (blk < blkcnt) { - blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ? - mmc->erase_grp_size : (blkcnt - blk); + if ((blkcnt - blk) >= mmc->erase_grp_size) + blk_r = mmc->erase_grp_size; err = mmc_erase_t(mmc, start + blk, blk_r); - if (err) - break; + if (err) { + printf("\nErase range from 0x" LBAF "~0x" LBAF + " Failed\n", start + blk, blkcnt); + return COMM_ERR; + } blk += blk_r;
This patch enhances the currently implemented erase procedure in u-boot, which has the following problems/missing features..." Erass sequence: 1. check if erase command is support by card. If not return. 2. Check the erase range to see if it was aligned. The min erase size should be one erase group. SD card it was one block(512), mmc card it should be one erase group. 3. If not, aligned the erase rang according to the erase group size. 4. Send erase command to erase card once one erase group. 5. If it was SD card, erase with arg 0x00000000 should be send. else if support secure feature, erase with arg 0x80000000(Spec eMMC 4.41). else erase with arg 0x00000000.(Trim and discard is ingnored here) 6. Check card status after erase. Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com> --- changes for V3: - update the commit message and secure feature supporting judgment. drivers/mmc/mmc_write.c | 68 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 10 deletions(-)