Hi everyone,
Previous patch did pass the checks and applied to the repository, but since I use yocto as my development environment
the warnings about hunk offsets were ignored.
I have now re-rolled patch on top of a clean mmc-utils repository - changes are the same.
Hopefully this will be ok now.
Best regards,
Bruno Matić
-----------------------------------------------------------------------------------------------
Add the check if the whole firmware was loaded.
Cleaned up the leftovers of handling the file in chunks.
Signed-off-by: Bruno Matic <bruno.matic@xxxxxxxxx>
---
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
-----Original Message-----
From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
Sent: Tuesday, September 6, 2022 10:29 AM
To: Matic, Bruno (Nokia - DE/Ulm) <bruno.matic@xxxxxxxxx>
Cc: Avri Altman <Avri.Altman@xxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx; Christian Löhle <CLoehle@xxxxxxxxxxxxxx>; Rossler, Jakob (Nokia - DE/Ulm) <jakob.rossler@xxxxxxxxx>; Heinonen, Aarne (Nokia - FI/Espoo) <aarne.heinonen@xxxxxxxxx>
Subject: Re: [PATCH] mmc-utils: correct and clean up the file handling
On Mon, 15 Aug 2022 at 15:11, Matic, Bruno (Nokia - DE/Ulm) <bruno.matic@xxxxxxxxx> wrote:
>
> Hi everyone,
> As said, here is the reworked patch.
>
>
> Add the check if the whole firmware was loaded.
> Cleaned up the leftovers of handling the file in chunks.
>
> Signed-off-by: Bruno Matic <bruno.matic@xxxxxxxxx>
Hi Bruno,
Unfortunately, I was not able to apply this patch.
$subject patch was not accepted by the mmc patchwork [1], which I am using to manage the patches. Please make sure to conform to the process of submitting patches [2] and run scripts/checkpatch.pl before re-submitting.
Kind regards
Uffe
[1]
https://patchwork.kernel.org/project/linux-mmc/list/
[2]
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> ---
> mmc_cmds.c | 66
> ++++++++++++++++++++++++++----------------------------
> 1 file changed, 32 insertions(+), 34 deletions(-)
>
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 70480df..7d37e93 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -2812,7 +2812,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;
> __u32 blocks = 1;
> @@ -2925,45 +2924,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
>
