[U-Boot] [PATCH v3 2/2] env: mmc: add erase-function

Frank Wunderlich frank-w at public-files.de
Fri Apr 26 15:18:11 UTC 2019


Hi Simon

thanks for your review..

> > +       if (!mmc)
> > +               return CMD_RET_FAILURE;
> > +
> > +       blk = CONFIG_ENV_OFFSET / mmc->read_bl_len;
> > +       cnt = CONFIG_ENV_SIZE / mmc->read_bl_len;
>
> Hmm, this doesn't work with redundant env. To fix this, you probably
> need to touch patch 1/2, add a parameter to the command specifying
> which env to erase (or both) and pass it to this callback.

i had not understand the idea behind redundant-offset...you mean i should let user choose which offset to clear? save uses this way to determine the offset:

int copy = 0;
#ifdef CONFIG_ENV_OFFSET_REDUND
    if (gd->env_valid == ENV_VALID) //use redundant offset if default location holds valid environment (and ENV_OFFSET_REDUND is defined)??
        copy = 1;
#endif

    if (mmc_get_env_addr(mmc, copy, &offset)) {

should i use the same way or something like this (additional parameter to callback, also command "env erase" needs additional optional parameter):

#ifdef CONFIG_ENV_OFFSET_REDUND
    if (use_redund)
        blk = CONFIG_ENV_OFFSET_REDUND / mmc->read_bl_len;
    else
#endif
        blk = CONFIG_ENV_OFFSET / mmc->read_bl_len;


> > +
> > +       printf("\nMMC erase env: dev # %d, block # %d (0x%x), count %d (0x%x)\n",
> > +              dev, blk, blk * mmc->read_bl_len,
> > +              cnt, cnt * mmc->read_bl_len);
> > +
> > +       if (mmc_getwp(mmc) == 1) {
> > +               printf("Error: card is write protected!\n");
> > +               return CMD_RET_FAILURE;
> > +       }
> > +       n = blk_derase(mmc_get_blk_desc(mmc), blk, cnt);
> > +       printf("%d blocks erased: %s\n", n, (n == cnt) ? "OK" : "ERROR");
> > +
> > +       return (n == cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> > +}
>
> Overall, I think this function should more resemble the save function.
> That should make it easier to follow for someone trying to understand
> this file.

i first copied the save-function and removed/changed anything to match new usecase...so the flow is same as for save. i removed the gotos because they are not needed because i think i don't need "fini_mmc_for_env(mmc);", do i?

regards Frank


More information about the U-Boot mailing list