[U-Boot] [PATCH 3/7 V3] mmc: Enhance erase handling procedure

[Haijun.Zhang at freescale.com]

Regards & Thanks.

-- Haijun


> -----Original Message-----
> From: Michael Trimarchi [mailto:michael at amarulasolutions.com]
> Sent: Wednesday, December 11, 2013 11:10 AM
> To: Zhang Haijun-B42677
> Cc: panto at antoniou-consulting.com; u-boot at lists.denx.de; Xie Xiaobo-
> R63061; Sun York-R58495; Tom Rini; Stefano Babic;
> rjbarnet at rockwellcollins.com; jh80.chung at 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 at 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 at 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
> >
> >
>