On Tue, 6 Sept 2022 at 12:14, Matic, Bruno (Nokia - DE/Ulm) <bruno.matic@xxxxxxxxx> wrote: > > 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. Sorry, but no this isn't much better it seems. From the guide [1], there are a couple of important steps that you need to follow. Please have a closer look. 1) Format the patch according to the "canonical patch format" with "git format-patch". 2. Run ./scripts/checkpatch.pl (the script is available in kernel code tree) on the formatted patch - and fix the potential warnings/errors. 3) Submit your patch as plain text with git send-email. Hope this helps! Kind regards Uffe [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > 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 > >
