Hi Paul, On Sep 9, 2013, at 11:14 AM, Paul Burton wrote:
> On Sun 08 Sep 2013 09:48:20 BST, Andreas Bießmann wrote: >> >> Dear Paul Burton, >> >> On 06.09.13 15:43, Paul Burton wrote: >>> >>> For SPL builds this is just dead code since we'll only need to read. >>> Eliminating it results in a significant size reduction for the SPL >>> binary, which may be critical for certain platforms where the binary >>> size is highly constrained. >>> >>> Signed-off-by: Paul Burton <paul.bur...@imgtec.com> >>> --- >>> Changes in v2: >>> - Move the mmc_bwrite & mmc_berase functions to a new mmc_write.c >>> file which is only compiled for non-SPL builds, as per a request >>> from Pantelis Antoniou. This requires that a few formerly static >>> functions in mmc.c be accessible to the new file, so they are >>> declared in a new mmc_private.h header along with the write & >>> erase functions. For what it's worth I prefered v1, but hey ho. >>> --- >>> drivers/mmc/Makefile | 2 + >>> drivers/mmc/mmc.c | 186 +-------------------------------------------- >>> drivers/mmc/mmc_private.h | 45 +++++++++++ >>> drivers/mmc/mmc_write.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 240 insertions(+), 182 deletions(-) >>> create mode 100644 drivers/mmc/mmc_private.h >>> create mode 100644 drivers/mmc/mmc_write.c >> >> >> <snip> >> >>> >>> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c >>> new file mode 100644 >>> index 0000000..dde5cf2 >>> --- /dev/null >>> +++ b/drivers/mmc/mmc_write.c >>> @@ -0,0 +1,189 @@ >>> +/* >>> + * Copyright 2008, Freescale Semiconductor, Inc >>> + * Andy Fleming >>> + * >>> + * Based vaguely on the Linux code >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#include <config.h> >>> +#include <common.h> >>> +#include <part.h> >>> +#include "mmc_private.h" >>> + >>> +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; >>> + >>> + if (mmc->high_capacity) { >>> + end = start + blkcnt - 1; >>> + } else { >>> + end = (start + blkcnt - 1) * mmc->write_bl_len; >>> + start *= mmc->write_bl_len; >>> + } >>> + >>> + if (IS_SD(mmc)) { >>> + start_cmd = SD_CMD_ERASE_WR_BLK_START; >>> + end_cmd = SD_CMD_ERASE_WR_BLK_END; >>> + } else { >>> + start_cmd = MMC_CMD_ERASE_GROUP_START; >>> + end_cmd = MMC_CMD_ERASE_GROUP_END; >>> + } >>> + >>> + cmd.cmdidx = start_cmd; >>> + cmd.cmdarg = start; >>> + cmd.resp_type = MMC_RSP_R1; >>> + >>> + err = mmc_send_cmd(mmc, &cmd, NULL); >>> + if (err) >>> + goto err_out; >>> + >>> + cmd.cmdidx = end_cmd; >>> + cmd.cmdarg = end; >>> + >>> + err = mmc_send_cmd(mmc, &cmd, NULL); >>> + if (err) >>> + goto err_out; >>> + >>> + cmd.cmdidx = MMC_CMD_ERASE; >>> + cmd.cmdarg = SECURE_ERASE; >>> + cmd.resp_type = MMC_RSP_R1b; >>> + >>> + err = mmc_send_cmd(mmc, &cmd, NULL); >>> + if (err) >>> + goto err_out; >>> + >>> + return 0; >>> + >>> +err_out: >>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) >>> + puts("mmc erase failed\n"); >>> +#endif >> >> >> this conditional compile in of puts/printf for SPL are no longer >> required, I'd prefere to remove them globally in mmc_write.c. > Ah, yes good point, I'll remove that. >> >>> >>> + return err; >>> +} >> >> >> Rest of this patch looks good to me. >> >> Best regards >> >> Andreas Bießmann > Thanks for looking at it! > > Paul > Seem good to me too. I'll give it a spin later this week and make sure nothing breaks. Regards -- Pantelis _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot