Message ID | 1378475004-6923-1-git-send-email-paul.burton@imgtec.com |
---|---|
State | Accepted |
Delegated to: | Pantelis Antoniou |
Headers | show |
Hi Paul, On Sep 6, 2013, at 4:43 PM, 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.burton@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 > > diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile > index bedf833..06280d1 100644 > --- a/drivers/mmc/Makefile > +++ b/drivers/mmc/Makefile > @@ -34,6 +34,8 @@ COBJS-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o > COBJS-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o > ifdef CONFIG_SPL_BUILD > COBJS-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o > +else > +COBJS-$(CONFIG_GENERIC_MMC) += mmc_write.o > endif > > COBJS := $(COBJS-y) > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index 30a985b..666f77b 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -15,6 +15,7 @@ > #include <malloc.h> > #include <linux/list.h> > #include <div64.h> > +#include "mmc_private.h" > > /* Set block count limit because of 16 bit register limit on some hardware*/ > #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT > @@ -52,8 +53,7 @@ int __board_mmc_getcd(struct mmc *mmc) { > int board_mmc_getcd(struct mmc *mmc)__attribute__((weak, > alias("__board_mmc_getcd"))); > > -static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, > - struct mmc_data *data) > +int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) > { > struct mmc_data backup; > int ret; > @@ -114,7 +114,7 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, > return ret; > } > > -static int mmc_send_status(struct mmc *mmc, int timeout) > +int mmc_send_status(struct mmc *mmc, int timeout) > { > struct mmc_cmd cmd; > int err, retries = 5; > @@ -162,7 +162,7 @@ static int mmc_send_status(struct mmc *mmc, int timeout) > return 0; > } > > -static int mmc_set_blocklen(struct mmc *mmc, int len) > +int mmc_set_blocklen(struct mmc *mmc, int len) > { > struct mmc_cmd cmd; > > @@ -192,184 +192,6 @@ struct mmc *find_mmc_device(int dev_num) > return NULL; > } > > -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 > - return err; > -} > - > -static unsigned long > -mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt) > -{ > - int err = 0; > - struct mmc *mmc = find_mmc_device(dev_num); > - lbaint_t blk = 0, blk_r = 0; > - int timeout = 1000; > - > - if (!mmc) > - return -1; > - > -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > - if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size)) > - 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); > -#endif > - > - while (blk < blkcnt) { > - blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ? > - mmc->erase_grp_size : (blkcnt - blk); > - err = mmc_erase_t(mmc, start + blk, blk_r); > - if (err) > - break; > - > - blk += blk_r; > - > - /* Waiting for the ready status */ > - if (mmc_send_status(mmc, timeout)) > - return 0; > - } > - > - return blk; > -} > - > -static ulong > -mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*src) > -{ > - struct mmc_cmd cmd; > - struct mmc_data data; > - int timeout = 1000; > - > - if ((start + blkcnt) > mmc->block_dev.lba) { > -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > - printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n", > - start + blkcnt, mmc->block_dev.lba); > -#endif > - return 0; > - } > - > - if (blkcnt == 0) > - return 0; > - else if (blkcnt == 1) > - cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK; > - else > - cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK; > - > - if (mmc->high_capacity) > - cmd.cmdarg = start; > - else > - cmd.cmdarg = start * mmc->write_bl_len; > - > - cmd.resp_type = MMC_RSP_R1; > - > - data.src = src; > - data.blocks = blkcnt; > - data.blocksize = mmc->write_bl_len; > - data.flags = MMC_DATA_WRITE; > - > - if (mmc_send_cmd(mmc, &cmd, &data)) { > -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > - printf("mmc write failed\n"); > -#endif > - return 0; > - } > - > - /* SPI multiblock writes terminate using a special > - * token, not a STOP_TRANSMISSION request. > - */ > - if (!mmc_host_is_spi(mmc) && blkcnt > 1) { > - cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION; > - cmd.cmdarg = 0; > - cmd.resp_type = MMC_RSP_R1b; > - if (mmc_send_cmd(mmc, &cmd, NULL)) { > -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > - printf("mmc fail to send stop cmd\n"); > -#endif > - return 0; > - } > - } > - > - /* Waiting for the ready status */ > - if (mmc_send_status(mmc, timeout)) > - return 0; > - > - return blkcnt; > -} > - > -static ulong > -mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void*src) > -{ > - lbaint_t cur, blocks_todo = blkcnt; > - > - struct mmc *mmc = find_mmc_device(dev_num); > - if (!mmc) > - return 0; > - > - if (mmc_set_blocklen(mmc, mmc->write_bl_len)) > - return 0; > - > - do { > - cur = (blocks_todo > mmc->b_max) ? mmc->b_max : blocks_todo; > - if(mmc_write_blocks(mmc, start, cur, src) != cur) > - return 0; > - blocks_todo -= cur; > - start += cur; > - src += cur * mmc->write_bl_len; > - } while (blocks_todo > 0); > - > - return blkcnt; > -} > - > static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start, > lbaint_t blkcnt) > { > diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h > new file mode 100644 > index 0000000..16dcf9f > --- /dev/null > +++ b/drivers/mmc/mmc_private.h > @@ -0,0 +1,45 @@ > +/* > + * Copyright 2008,2010 Freescale Semiconductor, Inc > + * Andy Fleming > + * > + * Based (loosely) on the Linux code > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef _MMC_PRIVATE_H_ > +#define _MMC_PRIVATE_H_ > + > +#include <mmc.h> > + > +extern int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, > + struct mmc_data *data); > +extern int mmc_send_status(struct mmc *mmc, int timeout); > +extern int mmc_set_blocklen(struct mmc *mmc, int len); > + > +#ifndef CONFIG_SPL_BUILD > + > +extern unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt); > + > +extern ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, > + const void *src); > + > +#else /* CONFIG_SPL_BUILD */ > + > +/* SPL will never write or erase, declare dummies to reduce code size. */ > + > +static inline unsigned long mmc_berase(int dev_num, lbaint_t start, > + lbaint_t blkcnt) > +{ > + return 0; > +} > + > +static inline ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, > + const void *src) > +{ > + return 0; > +} > + > +#endif /* CONFIG_SPL_BUILD */ > + > +#endif /* _MMC_PRIVATE_H_ */ > 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 > + return err; > +} > + > +unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt) > +{ > + int err = 0; > + struct mmc *mmc = find_mmc_device(dev_num); > + lbaint_t blk = 0, blk_r = 0; > + int timeout = 1000; > + > + if (!mmc) > + return -1; > + > +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > + if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size)) > + 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); > +#endif > + > + while (blk < blkcnt) { > + blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ? > + mmc->erase_grp_size : (blkcnt - blk); > + err = mmc_erase_t(mmc, start + blk, blk_r); > + if (err) > + break; > + > + blk += blk_r; > + > + /* Waiting for the ready status */ > + if (mmc_send_status(mmc, timeout)) > + return 0; > + } > + > + return blk; > +} > + > +static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start, > + lbaint_t blkcnt, const void *src) > +{ > + struct mmc_cmd cmd; > + struct mmc_data data; > + int timeout = 1000; > + > + if ((start + blkcnt) > mmc->block_dev.lba) { > +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > + printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n", > + start + blkcnt, mmc->block_dev.lba); > +#endif > + return 0; > + } > + > + if (blkcnt == 0) > + return 0; > + else if (blkcnt == 1) > + cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK; > + else > + cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK; > + > + if (mmc->high_capacity) > + cmd.cmdarg = start; > + else > + cmd.cmdarg = start * mmc->write_bl_len; > + > + cmd.resp_type = MMC_RSP_R1; > + > + data.src = src; > + data.blocks = blkcnt; > + data.blocksize = mmc->write_bl_len; > + data.flags = MMC_DATA_WRITE; > + > + if (mmc_send_cmd(mmc, &cmd, &data)) { > +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > + printf("mmc write failed\n"); > +#endif > + return 0; > + } > + > + /* SPI multiblock writes terminate using a special > + * token, not a STOP_TRANSMISSION request. > + */ > + if (!mmc_host_is_spi(mmc) && blkcnt > 1) { > + cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION; > + cmd.cmdarg = 0; > + cmd.resp_type = MMC_RSP_R1b; > + if (mmc_send_cmd(mmc, &cmd, NULL)) { > +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > + printf("mmc fail to send stop cmd\n"); > +#endif > + return 0; > + } > + } > + > + /* Waiting for the ready status */ > + if (mmc_send_status(mmc, timeout)) > + return 0; > + > + return blkcnt; > +} > + > +ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void *src) > +{ > + lbaint_t cur, blocks_todo = blkcnt; > + > + struct mmc *mmc = find_mmc_device(dev_num); > + if (!mmc) > + return 0; > + > + if (mmc_set_blocklen(mmc, mmc->write_bl_len)) > + return 0; > + > + do { > + cur = (blocks_todo > mmc->b_max) ? mmc->b_max : blocks_todo; > + if (mmc_write_blocks(mmc, start, cur, src) != cur) > + return 0; > + blocks_todo -= cur; > + start += cur; > + src += cur * mmc->write_bl_len; > + } while (blocks_todo > 0); > + > + return blkcnt; > +} > -- > 1.8.4 > > Acked-by: Pantelis Antoniou <panto@antoniou-consulting.com> Looks good to me; let's hear what other people say too. Tom? Regards -- Pantelis
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.burton@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. > + return err; > +} Rest of this patch looks good to me. Best regards Andreas Bießmann
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.burton@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
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.burton@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
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile index bedf833..06280d1 100644 --- a/drivers/mmc/Makefile +++ b/drivers/mmc/Makefile @@ -34,6 +34,8 @@ COBJS-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o COBJS-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o ifdef CONFIG_SPL_BUILD COBJS-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o +else +COBJS-$(CONFIG_GENERIC_MMC) += mmc_write.o endif COBJS := $(COBJS-y) diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 30a985b..666f77b 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -15,6 +15,7 @@ #include <malloc.h> #include <linux/list.h> #include <div64.h> +#include "mmc_private.h" /* Set block count limit because of 16 bit register limit on some hardware*/ #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT @@ -52,8 +53,7 @@ int __board_mmc_getcd(struct mmc *mmc) { int board_mmc_getcd(struct mmc *mmc)__attribute__((weak, alias("__board_mmc_getcd"))); -static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, - struct mmc_data *data) +int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) { struct mmc_data backup; int ret; @@ -114,7 +114,7 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, return ret; } -static int mmc_send_status(struct mmc *mmc, int timeout) +int mmc_send_status(struct mmc *mmc, int timeout) { struct mmc_cmd cmd; int err, retries = 5; @@ -162,7 +162,7 @@ static int mmc_send_status(struct mmc *mmc, int timeout) return 0; } -static int mmc_set_blocklen(struct mmc *mmc, int len) +int mmc_set_blocklen(struct mmc *mmc, int len) { struct mmc_cmd cmd; @@ -192,184 +192,6 @@ struct mmc *find_mmc_device(int dev_num) return NULL; } -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 - return err; -} - -static unsigned long -mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt) -{ - int err = 0; - struct mmc *mmc = find_mmc_device(dev_num); - lbaint_t blk = 0, blk_r = 0; - int timeout = 1000; - - if (!mmc) - return -1; - -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) - if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size)) - 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); -#endif - - while (blk < blkcnt) { - blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ? - mmc->erase_grp_size : (blkcnt - blk); - err = mmc_erase_t(mmc, start + blk, blk_r); - if (err) - break; - - blk += blk_r; - - /* Waiting for the ready status */ - if (mmc_send_status(mmc, timeout)) - return 0; - } - - return blk; -} - -static ulong -mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*src) -{ - struct mmc_cmd cmd; - struct mmc_data data; - int timeout = 1000; - - if ((start + blkcnt) > mmc->block_dev.lba) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) - printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n", - start + blkcnt, mmc->block_dev.lba); -#endif - return 0; - } - - if (blkcnt == 0) - return 0; - else if (blkcnt == 1) - cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK; - else - cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK; - - if (mmc->high_capacity) - cmd.cmdarg = start; - else - cmd.cmdarg = start * mmc->write_bl_len; - - cmd.resp_type = MMC_RSP_R1; - - data.src = src; - data.blocks = blkcnt; - data.blocksize = mmc->write_bl_len; - data.flags = MMC_DATA_WRITE; - - if (mmc_send_cmd(mmc, &cmd, &data)) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) - printf("mmc write failed\n"); -#endif - return 0; - } - - /* SPI multiblock writes terminate using a special - * token, not a STOP_TRANSMISSION request. - */ - if (!mmc_host_is_spi(mmc) && blkcnt > 1) { - cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION; - cmd.cmdarg = 0; - cmd.resp_type = MMC_RSP_R1b; - if (mmc_send_cmd(mmc, &cmd, NULL)) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) - printf("mmc fail to send stop cmd\n"); -#endif - return 0; - } - } - - /* Waiting for the ready status */ - if (mmc_send_status(mmc, timeout)) - return 0; - - return blkcnt; -} - -static ulong -mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void*src) -{ - lbaint_t cur, blocks_todo = blkcnt; - - struct mmc *mmc = find_mmc_device(dev_num); - if (!mmc) - return 0; - - if (mmc_set_blocklen(mmc, mmc->write_bl_len)) - return 0; - - do { - cur = (blocks_todo > mmc->b_max) ? mmc->b_max : blocks_todo; - if(mmc_write_blocks(mmc, start, cur, src) != cur) - return 0; - blocks_todo -= cur; - start += cur; - src += cur * mmc->write_bl_len; - } while (blocks_todo > 0); - - return blkcnt; -} - static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start, lbaint_t blkcnt) { diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h new file mode 100644 index 0000000..16dcf9f --- /dev/null +++ b/drivers/mmc/mmc_private.h @@ -0,0 +1,45 @@ +/* + * Copyright 2008,2010 Freescale Semiconductor, Inc + * Andy Fleming + * + * Based (loosely) on the Linux code + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _MMC_PRIVATE_H_ +#define _MMC_PRIVATE_H_ + +#include <mmc.h> + +extern int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, + struct mmc_data *data); +extern int mmc_send_status(struct mmc *mmc, int timeout); +extern int mmc_set_blocklen(struct mmc *mmc, int len); + +#ifndef CONFIG_SPL_BUILD + +extern unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt); + +extern ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, + const void *src); + +#else /* CONFIG_SPL_BUILD */ + +/* SPL will never write or erase, declare dummies to reduce code size. */ + +static inline unsigned long mmc_berase(int dev_num, lbaint_t start, + lbaint_t blkcnt) +{ + return 0; +} + +static inline ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, + const void *src) +{ + return 0; +} + +#endif /* CONFIG_SPL_BUILD */ + +#endif /* _MMC_PRIVATE_H_ */ 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 + return err; +} + +unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt) +{ + int err = 0; + struct mmc *mmc = find_mmc_device(dev_num); + lbaint_t blk = 0, blk_r = 0; + int timeout = 1000; + + if (!mmc) + return -1; + +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) + if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size)) + 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); +#endif + + while (blk < blkcnt) { + blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ? + mmc->erase_grp_size : (blkcnt - blk); + err = mmc_erase_t(mmc, start + blk, blk_r); + if (err) + break; + + blk += blk_r; + + /* Waiting for the ready status */ + if (mmc_send_status(mmc, timeout)) + return 0; + } + + return blk; +} + +static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start, + lbaint_t blkcnt, const void *src) +{ + struct mmc_cmd cmd; + struct mmc_data data; + int timeout = 1000; + + if ((start + blkcnt) > mmc->block_dev.lba) { +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) + printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n", + start + blkcnt, mmc->block_dev.lba); +#endif + return 0; + } + + if (blkcnt == 0) + return 0; + else if (blkcnt == 1) + cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK; + else + cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK; + + if (mmc->high_capacity) + cmd.cmdarg = start; + else + cmd.cmdarg = start * mmc->write_bl_len; + + cmd.resp_type = MMC_RSP_R1; + + data.src = src; + data.blocks = blkcnt; + data.blocksize = mmc->write_bl_len; + data.flags = MMC_DATA_WRITE; + + if (mmc_send_cmd(mmc, &cmd, &data)) { +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) + printf("mmc write failed\n"); +#endif + return 0; + } + + /* SPI multiblock writes terminate using a special + * token, not a STOP_TRANSMISSION request. + */ + if (!mmc_host_is_spi(mmc) && blkcnt > 1) { + cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION; + cmd.cmdarg = 0; + cmd.resp_type = MMC_RSP_R1b; + if (mmc_send_cmd(mmc, &cmd, NULL)) { +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) + printf("mmc fail to send stop cmd\n"); +#endif + return 0; + } + } + + /* Waiting for the ready status */ + if (mmc_send_status(mmc, timeout)) + return 0; + + return blkcnt; +} + +ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void *src) +{ + lbaint_t cur, blocks_todo = blkcnt; + + struct mmc *mmc = find_mmc_device(dev_num); + if (!mmc) + return 0; + + if (mmc_set_blocklen(mmc, mmc->write_bl_len)) + return 0; + + do { + cur = (blocks_todo > mmc->b_max) ? mmc->b_max : blocks_todo; + if (mmc_write_blocks(mmc, start, cur, src) != cur) + return 0; + blocks_todo -= cur; + start += cur; + src += cur * mmc->write_bl_len; + } while (blocks_todo > 0); + + return blkcnt; +}
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.burton@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