| Message ID | 20220909110322.25141-1-bruno.matic@nokia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] mmc-utils: correct and clean up the file handling | expand |
On Fri, 9 Sept 2022 at 13:03, Bruno Matic <bruno.matic@nokia.com> wrote: > > Add the check if the whole firmware was loaded. > Cleaned up the leftovers of handling the file in chunks. > > Changes in v3: > - checked and formated the patch > Changes in v2: > - use perror to propagate the error on file read > > Signed-off-by: Bruno Matic <bruno.matic@nokia.com> > Reviewed-by: Avri Altman <avri.altman@wdc.com> Applied to git.kernel.org/pub/scm//utils/mmc/mmc-utils.git master, thanks! Kind regards Uffe > --- > mmc_cmds.c | 66 ++++++++++++++++++++++++++---------------------------- > 1 file changed, 32 insertions(+), 34 deletions(-) > > diff --git a/mmc_cmds.c b/mmc_cmds.c > index 12b7802..ef1d8c6 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -2765,7 +2765,6 @@ int do_ffu(int nargs, char **argv) > __u8 *buf = NULL; > __u32 arg; > off_t fw_size; > - ssize_t chunk_size; > char *device; > struct mmc_ioc_multi_cmd *multi_cmd = NULL; > > @@ -2879,45 +2878,44 @@ int do_ffu(int nargs, char **argv) > multi_cmd->cmds[3].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; > multi_cmd->cmds[3].write_flag = 1; > > -do_retry: > - /* read firmware chunk */ > + /* read firmware */ > lseek(img_fd, 0, SEEK_SET); > - chunk_size = read(img_fd, buf, fw_size); > + if (read(img_fd, buf, fw_size) != fw_size) { > + perror("Could not read the firmware file: "); > + ret = -ENOSPC; > + goto out; > + } > > - if (chunk_size > 0) { > - /* send ioctl with multi-cmd */ > - ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > +do_retry: > + /* send ioctl with multi-cmd */ > + ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > > - if (ret) { > - perror("Multi-cmd ioctl"); > - /* In case multi-cmd ioctl failed before exiting from ffu mode */ > - ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); > - goto out; > - } > + if (ret) { > + perror("Multi-cmd ioctl"); > + /* In case multi-cmd ioctl failed before exiting from ffu mode */ > + ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); > + goto out; > + } > > - ret = read_extcsd(dev_fd, ext_csd); > - if (ret) { > - fprintf(stderr, "Could not read EXT_CSD from %s\n", device); > - goto out; > - } > + ret = read_extcsd(dev_fd, ext_csd); > + if (ret) { > + fprintf(stderr, "Could not read EXT_CSD from %s\n", device); > + goto out; > + } > > - /* Test if we need to restart the download */ > - sect_done = ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0] | > - ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_1] << 8 | > - ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_2] << 16 | > - ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_3] << 24; > - /* By spec, host should re-start download from the first sector if sect_done is 0 */ > - if (sect_done == 0) { > - if (retry > 0) { > - retry--; > - fprintf(stderr, "Programming failed. Retrying... (%d)\n", retry); > - goto do_retry; > - } > - fprintf(stderr, "Programming failed! Aborting...\n"); > - goto out; > - } else { > - fprintf(stderr, "Programmed %d/%jd bytes\r", sect_done * sect_size, (intmax_t)fw_size); > + /* Test if we need to restart the download */ > + sect_done = ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0] | > + ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_1] << 8 | > + ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_2] << 16 | > + ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_3] << 24; > + /* By spec, host should re-start download from the first sector if sect_done is 0 */ > + if (sect_done == 0) { > + if (retry--) { > + fprintf(stderr, "Programming failed. Retrying... (%d)\n", retry); > + goto do_retry; > } > + fprintf(stderr, "Programming failed! Aborting...\n"); > + goto out; > } > > if ((sect_done * sect_size) == fw_size) { > -- > 2.29.0 >
Tested-by: Rossler, Jakob (Nokia - DE/Ulm) <jakob.rossler@nokia.com> Best regards Jakob -----Original Message----- From: Ulf Hansson <ulf.hansson@linaro.org> Sent: Monday, September 19, 2022 4:01 PM To: Matic, Bruno (Nokia - DE/Ulm) <bruno.matic@nokia.com> Cc: avri.altman@wdc.com; linux-mmc@vger.kernel.org; CLoehle@hyperstone.com; Rossler, Jakob (Nokia - DE/Ulm) <jakob.rossler@nokia.com>; Heinonen, Aarne (Nokia - FI/Espoo) <aarne.heinonen@nokia.com> Subject: Re: [PATCH v3] mmc-utils: correct and clean up the file handling On Fri, 9 Sept 2022 at 13:03, Bruno Matic <bruno.matic@nokia.com> wrote: > > Add the check if the whole firmware was loaded. > Cleaned up the leftovers of handling the file in chunks. > > Changes in v3: > - checked and formated the patch > Changes in v2: > - use perror to propagate the error on file read > > Signed-off-by: Bruno Matic <bruno.matic@nokia.com> > Reviewed-by: Avri Altman <avri.altman@wdc.com> Applied to git.kernel.org/pub/scm//utils/mmc/mmc-utils.git master, thanks! Kind regards Uffe > --- > mmc_cmds.c | 66 ++++++++++++++++++++++++++---------------------------- > 1 file changed, 32 insertions(+), 34 deletions(-) > > diff --git a/mmc_cmds.c b/mmc_cmds.c > index 12b7802..ef1d8c6 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -2765,7 +2765,6 @@ int do_ffu(int nargs, char **argv) > __u8 *buf = NULL; > __u32 arg; > off_t fw_size; > - ssize_t chunk_size; > char *device; > struct mmc_ioc_multi_cmd *multi_cmd = NULL; > > @@ -2879,45 +2878,44 @@ int do_ffu(int nargs, char **argv) > multi_cmd->cmds[3].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; > multi_cmd->cmds[3].write_flag = 1; > > -do_retry: > - /* read firmware chunk */ > + /* read firmware */ > lseek(img_fd, 0, SEEK_SET); > - chunk_size = read(img_fd, buf, fw_size); > + if (read(img_fd, buf, fw_size) != fw_size) { > + perror("Could not read the firmware file: "); > + ret = -ENOSPC; > + goto out; > + } > > - if (chunk_size > 0) { > - /* send ioctl with multi-cmd */ > - ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > +do_retry: > + /* send ioctl with multi-cmd */ > + ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > > - if (ret) { > - perror("Multi-cmd ioctl"); > - /* In case multi-cmd ioctl failed before exiting from ffu mode */ > - ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); > - goto out; > - } > + if (ret) { > + perror("Multi-cmd ioctl"); > + /* In case multi-cmd ioctl failed before exiting from ffu mode */ > + ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); > + goto out; > + } > > - ret = read_extcsd(dev_fd, ext_csd); > - if (ret) { > - fprintf(stderr, "Could not read EXT_CSD from %s\n", device); > - goto out; > - } > + ret = read_extcsd(dev_fd, ext_csd); > + if (ret) { > + fprintf(stderr, "Could not read EXT_CSD from %s\n", device); > + goto out; > + } > > - /* Test if we need to restart the download */ > - sect_done = ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0] | > - ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_1] << 8 | > - ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_2] << 16 | > - ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_3] << 24; > - /* By spec, host should re-start download from the first sector if sect_done is 0 */ > - if (sect_done == 0) { > - if (retry > 0) { > - retry--; > - fprintf(stderr, "Programming failed. Retrying... (%d)\n", retry); > - goto do_retry; > - } > - fprintf(stderr, "Programming failed! Aborting...\n"); > - goto out; > - } else { > - fprintf(stderr, "Programmed %d/%jd bytes\r", sect_done * sect_size, (intmax_t)fw_size); > + /* Test if we need to restart the download */ > + sect_done = ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0] | > + ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_1] << 8 | > + ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_2] << 16 | > + ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_3] << 24; > + /* By spec, host should re-start download from the first sector if sect_done is 0 */ > + if (sect_done == 0) { > + if (retry--) { > + fprintf(stderr, "Programming failed. Retrying... (%d)\n", retry); > + goto do_retry; > } > + fprintf(stderr, "Programming failed! Aborting...\n"); > + goto out; > } > > if ((sect_done * sect_size) == fw_size) { > -- > 2.29.0 >
diff --git a/mmc_cmds.c b/mmc_cmds.c index 12b7802..ef1d8c6 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -2765,7 +2765,6 @@ int do_ffu(int nargs, char **argv) __u8 *buf = NULL; __u32 arg; off_t fw_size; - ssize_t chunk_size; char *device; struct mmc_ioc_multi_cmd *multi_cmd = NULL; @@ -2879,45 +2878,44 @@ int do_ffu(int nargs, char **argv) multi_cmd->cmds[3].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; multi_cmd->cmds[3].write_flag = 1; -do_retry: - /* read firmware chunk */ + /* read firmware */ lseek(img_fd, 0, SEEK_SET); - chunk_size = read(img_fd, buf, fw_size); + if (read(img_fd, buf, fw_size) != fw_size) { + perror("Could not read the firmware file: "); + ret = -ENOSPC; + goto out; + } - if (chunk_size > 0) { - /* send ioctl with multi-cmd */ - ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); +do_retry: + /* send ioctl with multi-cmd */ + ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); - if (ret) { - perror("Multi-cmd ioctl"); - /* In case multi-cmd ioctl failed before exiting from ffu mode */ - ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); - goto out; - } + if (ret) { + perror("Multi-cmd ioctl"); + /* In case multi-cmd ioctl failed before exiting from ffu mode */ + ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); + goto out; + } - ret = read_extcsd(dev_fd, ext_csd); - if (ret) { - fprintf(stderr, "Could not read EXT_CSD from %s\n", device); - goto out; - } + ret = read_extcsd(dev_fd, ext_csd); + if (ret) { + fprintf(stderr, "Could not read EXT_CSD from %s\n", device); + goto out; + } - /* Test if we need to restart the download */ - sect_done = ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0] | - ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_1] << 8 | - ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_2] << 16 | - ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_3] << 24; - /* By spec, host should re-start download from the first sector if sect_done is 0 */ - if (sect_done == 0) { - if (retry > 0) { - retry--; - fprintf(stderr, "Programming failed. Retrying... (%d)\n", retry); - goto do_retry; - } - fprintf(stderr, "Programming failed! Aborting...\n"); - goto out; - } else { - fprintf(stderr, "Programmed %d/%jd bytes\r", sect_done * sect_size, (intmax_t)fw_size); + /* Test if we need to restart the download */ + sect_done = ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0] | + ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_1] << 8 | + ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_2] << 16 | + ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_3] << 24; + /* By spec, host should re-start download from the first sector if sect_done is 0 */ + if (sect_done == 0) { + if (retry--) { + fprintf(stderr, "Programming failed. Retrying... (%d)\n", retry); + goto do_retry; } + fprintf(stderr, "Programming failed! Aborting...\n"); + goto out; } if ((sect_done * sect_size) == fw_size) {