| Message ID | 20220919161852.28072-1-bruno.matic@nokia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc-utils: correct ffu in case of unsupported MODE_OPERATION_CODES | expand |
Adding detail to the description, the functionality of patch from Bruno is described in JESD84-B51A, Chapter 6.6.18, especially in 6th paragraph. -- Aarne Heinonen -----Original Message----- From: Bruno Matic <bruno.matic@nokia.com> Sent: Monday, September 19, 2022 7:19 PM To: linux-mmc@vger.kernel.org Cc: ulf.hansson@linaro.org; avri.altman@wdc.com; Rossler, Jakob (Nokia - DE/Ulm) <jakob.rossler@nokia.com>; Heinonen, Aarne (Nokia - FI/Espoo) <aarne.heinonen@nokia.com>; Matic, Bruno (Nokia - DE/Ulm) <bruno.matic@nokia.com> Subject: [PATCH] mmc-utils: correct ffu in case of unsupported MODE_OPERATION_CODES Move up the return in case MODE_OPERATION_CODES is not supported. According to the specification when MODE_OPERATION_CODES is not supported no checks should be done against NUM_OF_FW_SEC_PROG. Signed-off-by: Bruno Matic <bruno.matic@nokia.com> --- mmc_cmds.c | 82 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/mmc_cmds.c b/mmc_cmds.c index ef1d8c6..81fdc7e 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -2897,6 +2897,13 @@ do_retry: goto out; } + /* check mode operation for ffu install*/ + if (!ext_csd[EXT_CSD_FFU_FEATURES]) { + fprintf(stderr, "Please reboot to complete firmware installation on %s\n", device); + ret = 0; + goto out; + } + ret = read_extcsd(dev_fd, ext_csd); if (ret) { fprintf(stderr, "Could not read EXT_CSD from %s\n", device); @@ -2927,49 +2934,44 @@ do_retry: goto out; } - /* check mode operation for ffu install*/ - if (!ext_csd[EXT_CSD_FFU_FEATURES]) { - fprintf(stderr, "Please reboot to complete firmware installation on %s\n", device); - } else { - fprintf(stderr, "Installing firmware on %s...\n", device); - /* Re-enter ffu mode and install the firmware */ - multi_cmd->num_of_cmds = 2; - - /* set ext_csd to install mode */ - multi_cmd->cmds[1].opcode = MMC_SWITCH; - multi_cmd->cmds[1].blksz = 0; - multi_cmd->cmds[1].blocks = 0; - multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) | - (EXT_CSD_MODE_OPERATION_CODES << 16) | - (EXT_CSD_FFU_INSTALL << 8) | - EXT_CSD_CMD_SET_NORMAL; - multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; - multi_cmd->cmds[1].write_flag = 1; - - /* send ioctl with multi-cmd */ - ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); + fprintf(stderr, "Installing firmware on %s...\n", device); + /* Re-enter ffu mode and install the firmware */ + multi_cmd->num_of_cmds = 2; - if (ret) { - perror("Multi-cmd ioctl failed setting install mode"); - /* In case multi-cmd ioctl failed before exiting from ffu mode */ - ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); - goto out; - } + /* set ext_csd to install mode */ + multi_cmd->cmds[1].opcode = MMC_SWITCH; + multi_cmd->cmds[1].blksz = 0; + multi_cmd->cmds[1].blocks = 0; + multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) | + (EXT_CSD_MODE_OPERATION_CODES << 16) | + (EXT_CSD_FFU_INSTALL << 8) | + EXT_CSD_CMD_SET_NORMAL; + multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; + multi_cmd->cmds[1].write_flag = 1; - ret = read_extcsd(dev_fd, ext_csd); - if (ret) { - fprintf(stderr, "Could not read EXT_CSD from %s\n", device); - goto out; - } + /* send ioctl with multi-cmd */ + ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); - /* return status */ - ret = ext_csd[EXT_CSD_FFU_STATUS]; - if (ret) { - fprintf(stderr, "%s: error %d during FFU install:\n", device, ret); - goto out; - } else { - fprintf(stderr, "FFU finished successfully\n"); - } + if (ret) { + perror("Multi-cmd ioctl failed setting install mode"); + /* 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; + } + + /* return status */ + ret = ext_csd[EXT_CSD_FFU_STATUS]; + if (ret) { + fprintf(stderr, "%s: error %d during FFU install:\n", device, ret); + goto out; + } else { + fprintf(stderr, "FFU finished successfully\n"); } out: -- 2.29.0
> Move up the return in case MODE_OPERATION_CODES is not supported. > According to the specification when MODE_OPERATION_CODES is not > supported no checks should be done against NUM_OF_FW_SEC_PROG. > > Signed-off-by: Bruno Matic <bruno.matic@nokia.com> > --- > mmc_cmds.c | 82 ++++++++++++++++++++++++++++-------------------------- > 1 file changed, 42 insertions(+), 40 deletions(-) > > diff --git a/mmc_cmds.c b/mmc_cmds.c > index ef1d8c6..81fdc7e 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -2897,6 +2897,13 @@ do_retry: > goto out; > } > > + /* check mode operation for ffu install*/ > + if (!ext_csd[EXT_CSD_FFU_FEATURES]) { You haven't read the ext_csd yet.... Thanks, Avri > + fprintf(stderr, "Please reboot to complete firmware installation on > %s\n", device); > + ret = 0; > + goto out; > + } > + > ret = read_extcsd(dev_fd, ext_csd); > if (ret) { > fprintf(stderr, "Could not read EXT_CSD from %s\n", device); @@ - > 2927,49 +2934,44 @@ do_retry: > goto out; > } > > - /* check mode operation for ffu install*/ > - if (!ext_csd[EXT_CSD_FFU_FEATURES]) { > - fprintf(stderr, "Please reboot to complete firmware installation on > %s\n", device); > - } else { > - fprintf(stderr, "Installing firmware on %s...\n", device); > - /* Re-enter ffu mode and install the firmware */ > - multi_cmd->num_of_cmds = 2; > - > - /* set ext_csd to install mode */ > - multi_cmd->cmds[1].opcode = MMC_SWITCH; > - multi_cmd->cmds[1].blksz = 0; > - multi_cmd->cmds[1].blocks = 0; > - multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) > | > - (EXT_CSD_MODE_OPERATION_CODES << 16) | > - (EXT_CSD_FFU_INSTALL << 8) | > - EXT_CSD_CMD_SET_NORMAL; > - multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | > MMC_CMD_AC; > - multi_cmd->cmds[1].write_flag = 1; > - > - /* send ioctl with multi-cmd */ > - ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > + fprintf(stderr, "Installing firmware on %s...\n", device); > + /* Re-enter ffu mode and install the firmware */ > + multi_cmd->num_of_cmds = 2; > > - if (ret) { > - perror("Multi-cmd ioctl failed setting install mode"); > - /* In case multi-cmd ioctl failed before exiting from ffu mode */ > - ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); > - goto out; > - } > + /* set ext_csd to install mode */ > + multi_cmd->cmds[1].opcode = MMC_SWITCH; > + multi_cmd->cmds[1].blksz = 0; > + multi_cmd->cmds[1].blocks = 0; > + multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) | > + (EXT_CSD_MODE_OPERATION_CODES << 16) | > + (EXT_CSD_FFU_INSTALL << 8) | > + EXT_CSD_CMD_SET_NORMAL; > + multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | > MMC_CMD_AC; > + multi_cmd->cmds[1].write_flag = 1; > > - ret = read_extcsd(dev_fd, ext_csd); > - if (ret) { > - fprintf(stderr, "Could not read EXT_CSD from %s\n", device); > - goto out; > - } > + /* send ioctl with multi-cmd */ > + ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > > - /* return status */ > - ret = ext_csd[EXT_CSD_FFU_STATUS]; > - if (ret) { > - fprintf(stderr, "%s: error %d during FFU install:\n", device, ret); > - goto out; > - } else { > - fprintf(stderr, "FFU finished successfully\n"); > - } > + if (ret) { > + perror("Multi-cmd ioctl failed setting install mode"); > + /* 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; > + } > + > + /* return status */ > + ret = ext_csd[EXT_CSD_FFU_STATUS]; > + if (ret) { > + fprintf(stderr, "%s: error %d during FFU install:\n", device, ret); > + goto out; > + } else { > + fprintf(stderr, "FFU finished successfully\n"); > } > > out: > -- > 2.29.0
> > > Move up the return in case MODE_OPERATION_CODES is not supported. > > According to the specification when MODE_OPERATION_CODES is not > > supported no checks should be done against NUM_OF_FW_SEC_PROG. And I wasn't able to find in your code, where you are skipping the check against sect_done, In case MODE_OPERATION_CODES field is not supported. Thanks, Avri > > > > Signed-off-by: Bruno Matic <bruno.matic@nokia.com> > > --- > > mmc_cmds.c | 82 > > ++++++++++++++++++++++++++++-------------------------- > > 1 file changed, 42 insertions(+), 40 deletions(-) > > > > diff --git a/mmc_cmds.c b/mmc_cmds.c > > index ef1d8c6..81fdc7e 100644 > > --- a/mmc_cmds.c > > +++ b/mmc_cmds.c > > @@ -2897,6 +2897,13 @@ do_retry: > > goto out; > > } > > > > + /* check mode operation for ffu install*/ > > + if (!ext_csd[EXT_CSD_FFU_FEATURES]) { > You haven't read the ext_csd yet.... > > Thanks, > Avri > > > + fprintf(stderr, "Please reboot to complete firmware > > + installation on > > %s\n", device); > > + ret = 0; > > + goto out; > > + } > > + > > ret = read_extcsd(dev_fd, ext_csd); > > if (ret) { > > fprintf(stderr, "Could not read EXT_CSD from %s\n", > > device); @@ - > > 2927,49 +2934,44 @@ do_retry: > > goto out; > > } > > > > - /* check mode operation for ffu install*/ > > - if (!ext_csd[EXT_CSD_FFU_FEATURES]) { > > - fprintf(stderr, "Please reboot to complete firmware installation on > > %s\n", device); > > - } else { > > - fprintf(stderr, "Installing firmware on %s...\n", device); > > - /* Re-enter ffu mode and install the firmware */ > > - multi_cmd->num_of_cmds = 2; > > - > > - /* set ext_csd to install mode */ > > - multi_cmd->cmds[1].opcode = MMC_SWITCH; > > - multi_cmd->cmds[1].blksz = 0; > > - multi_cmd->cmds[1].blocks = 0; > > - multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << > 24) > > | > > - (EXT_CSD_MODE_OPERATION_CODES << 16) | > > - (EXT_CSD_FFU_INSTALL << 8) | > > - EXT_CSD_CMD_SET_NORMAL; > > - multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | > > MMC_CMD_AC; > > - multi_cmd->cmds[1].write_flag = 1; > > - > > - /* send ioctl with multi-cmd */ > > - ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > > + fprintf(stderr, "Installing firmware on %s...\n", device); > > + /* Re-enter ffu mode and install the firmware */ > > + multi_cmd->num_of_cmds = 2; > > > > - if (ret) { > > - perror("Multi-cmd ioctl failed setting install mode"); > > - /* In case multi-cmd ioctl failed before exiting from ffu mode */ > > - ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); > > - goto out; > > - } > > + /* set ext_csd to install mode */ > > + multi_cmd->cmds[1].opcode = MMC_SWITCH; > > + multi_cmd->cmds[1].blksz = 0; > > + multi_cmd->cmds[1].blocks = 0; > > + multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) | > > + (EXT_CSD_MODE_OPERATION_CODES << 16) | > > + (EXT_CSD_FFU_INSTALL << 8) | > > + EXT_CSD_CMD_SET_NORMAL; > > + multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | > > MMC_CMD_AC; > > + multi_cmd->cmds[1].write_flag = 1; > > > > - ret = read_extcsd(dev_fd, ext_csd); > > - if (ret) { > > - fprintf(stderr, "Could not read EXT_CSD from %s\n", device); > > - goto out; > > - } > > + /* send ioctl with multi-cmd */ > > + ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > > > > - /* return status */ > > - ret = ext_csd[EXT_CSD_FFU_STATUS]; > > - if (ret) { > > - fprintf(stderr, "%s: error %d during FFU install:\n", device, ret); > > - goto out; > > - } else { > > - fprintf(stderr, "FFU finished successfully\n"); > > - } > > + if (ret) { > > + perror("Multi-cmd ioctl failed setting install mode"); > > + /* 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; > > + } > > + > > + /* return status */ > > + ret = ext_csd[EXT_CSD_FFU_STATUS]; > > + if (ret) { > > + fprintf(stderr, "%s: error %d during FFU install:\n", device, ret); > > + goto out; > > + } else { > > + fprintf(stderr, "FFU finished successfully\n"); > > } > > > > out: > > -- > > 2.29.0
Hi Avri,
ext_csd is read in line 2789, close to the beginning of do_ffu.
The mode check and skip is done by the following snippet in the patch:
/* check mode operation for ffu install*/
if (!ext_csd[EXT_CSD_FFU_FEATURES]) {
fprintf(stderr, "Please reboot to complete firmware installation on %s\n", device);
ret = 0;
goto out;
}
According to the spec when the FFU_FETURES field is 0 then MODE_OPERATION_CODES is not supported
and sect_done checks should be skipped with no error.
Further reads of ext_csd are done in case FFU_FETURES field is 1 - to check the NUM_OF_FW_SEC_PROG value.
I admit that the patch looks ugly but that is because the check was moved up in the code from an if-else block that
had to be refactored.
Best regards,
Bruno Matić
-----Original Message-----
From: Avri Altman <Avri.Altman@wdc.com>
Sent: Monday, September 19, 2022 11:49 PM
To: Avri Altman <Avri.Altman@wdc.com>; Matic, Bruno (Nokia - DE/Ulm) <bruno.matic@nokia.com>; linux-mmc@vger.kernel.org
Cc: ulf.hansson@linaro.org; Rossler, Jakob (Nokia - DE/Ulm) <jakob.rossler@nokia.com>; Heinonen, Aarne (Nokia - FI/Espoo) <aarne.heinonen@nokia.com>
Subject: RE: [PATCH] mmc-utils: correct ffu in case of unsupported MODE_OPERATION_CODES
>
> > Move up the return in case MODE_OPERATION_CODES is not supported.
> > According to the specification when MODE_OPERATION_CODES is not
> > supported no checks should be done against NUM_OF_FW_SEC_PROG.
And I wasn't able to find in your code, where you are skipping the check against sect_done, In case MODE_OPERATION_CODES field is not supported.
Thanks,
Avri
> >
> > Signed-off-by: Bruno Matic <bruno.matic@nokia.com>
> > ---
> > mmc_cmds.c | 82
> > ++++++++++++++++++++++++++++--------------------------
> > 1 file changed, 42 insertions(+), 40 deletions(-)
> >
> > diff --git a/mmc_cmds.c b/mmc_cmds.c index ef1d8c6..81fdc7e 100644
> > --- a/mmc_cmds.c
> > +++ b/mmc_cmds.c
> > @@ -2897,6 +2897,13 @@ do_retry:
> > goto out;
> > }
> >
> > + /* check mode operation for ffu install*/
> > + if (!ext_csd[EXT_CSD_FFU_FEATURES]) {
> You haven't read the ext_csd yet....
>
> Thanks,
> Avri
>
> > + fprintf(stderr, "Please reboot to complete firmware
> > + installation on
> > %s\n", device);
> > + ret = 0;
> > + goto out;
> > + }
> > +
> > ret = read_extcsd(dev_fd, ext_csd);
> > if (ret) {
> > fprintf(stderr, "Could not read EXT_CSD from %s\n",
> > device); @@ -
> > 2927,49 +2934,44 @@ do_retry:
> > goto out;
> > }
> >
> > - /* check mode operation for ffu install*/
> > - if (!ext_csd[EXT_CSD_FFU_FEATURES]) {
> > - fprintf(stderr, "Please reboot to complete firmware installation on
> > %s\n", device);
> > - } else {
> > - fprintf(stderr, "Installing firmware on %s...\n", device);
> > - /* Re-enter ffu mode and install the firmware */
> > - multi_cmd->num_of_cmds = 2;
> > -
> > - /* set ext_csd to install mode */
> > - multi_cmd->cmds[1].opcode = MMC_SWITCH;
> > - multi_cmd->cmds[1].blksz = 0;
> > - multi_cmd->cmds[1].blocks = 0;
> > - multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE <<
> 24)
> > |
> > - (EXT_CSD_MODE_OPERATION_CODES << 16) |
> > - (EXT_CSD_FFU_INSTALL << 8) |
> > - EXT_CSD_CMD_SET_NORMAL;
> > - multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B |
> > MMC_CMD_AC;
> > - multi_cmd->cmds[1].write_flag = 1;
> > -
> > - /* send ioctl with multi-cmd */
> > - ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd);
> > + fprintf(stderr, "Installing firmware on %s...\n", device);
> > + /* Re-enter ffu mode and install the firmware */
> > + multi_cmd->num_of_cmds = 2;
> >
> > - if (ret) {
> > - perror("Multi-cmd ioctl failed setting install mode");
> > - /* In case multi-cmd ioctl failed before exiting from ffu mode */
> > - ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]);
> > - goto out;
> > - }
> > + /* set ext_csd to install mode */
> > + multi_cmd->cmds[1].opcode = MMC_SWITCH;
> > + multi_cmd->cmds[1].blksz = 0;
> > + multi_cmd->cmds[1].blocks = 0;
> > + multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) |
> > + (EXT_CSD_MODE_OPERATION_CODES << 16) |
> > + (EXT_CSD_FFU_INSTALL << 8) |
> > + EXT_CSD_CMD_SET_NORMAL;
> > + multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B |
> > MMC_CMD_AC;
> > + multi_cmd->cmds[1].write_flag = 1;
> >
> > - ret = read_extcsd(dev_fd, ext_csd);
> > - if (ret) {
> > - fprintf(stderr, "Could not read EXT_CSD from %s\n", device);
> > - goto out;
> > - }
> > + /* send ioctl with multi-cmd */
> > + ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd);
> >
> > - /* return status */
> > - ret = ext_csd[EXT_CSD_FFU_STATUS];
> > - if (ret) {
> > - fprintf(stderr, "%s: error %d during FFU install:\n", device, ret);
> > - goto out;
> > - } else {
> > - fprintf(stderr, "FFU finished successfully\n");
> > - }
> > + if (ret) {
> > + perror("Multi-cmd ioctl failed setting install mode");
> > + /* 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;
> > + }
> > +
> > + /* return status */
> > + ret = ext_csd[EXT_CSD_FFU_STATUS];
> > + if (ret) {
> > + fprintf(stderr, "%s: error %d during FFU install:\n", device, ret);
> > + goto out;
> > + } else {
> > + fprintf(stderr, "FFU finished successfully\n");
> > }
> >
> > out:
> > --
> > 2.29.0
> Hi Avri, > > ext_csd is read in line 2789, close to the beginning of do_ffu. Ahha - you are in the do_retry clause. Ok. > The mode check and skip is done by the following snippet in the patch: > /* check mode operation for ffu install*/ > if (!ext_csd[EXT_CSD_FFU_FEATURES]) { > fprintf(stderr, "Please reboot to complete firmware installation on > %s\n", device); > ret = 0; > goto out; > } > According to the spec when the FFU_FETURES field is 0 then > MODE_OPERATION_CODES is not supported and sect_done checks should > be skipped with no error. > Further reads of ext_csd are done in case FFU_FETURES field is 1 - to check > the NUM_OF_FW_SEC_PROG value. Correct. > > I admit that the patch looks ugly but that is because the check was moved up > in the code from an if-else block that had to be refactored. Actually I find it quite elegant. Please elaborate the comment above if (!ext_csd[EXT_CSD_FFU_FEATURES]) with your above explanation. Also please add a fixes tag. Thanks, Avri > > Best regards, > Bruno Matić > > -----Original Message----- > From: Avri Altman <Avri.Altman@wdc.com> > Sent: Monday, September 19, 2022 11:49 PM > To: Avri Altman <Avri.Altman@wdc.com>; Matic, Bruno (Nokia - DE/Ulm) > <bruno.matic@nokia.com>; linux-mmc@vger.kernel.org > Cc: ulf.hansson@linaro.org; Rossler, Jakob (Nokia - DE/Ulm) > <jakob.rossler@nokia.com>; Heinonen, Aarne (Nokia - FI/Espoo) > <aarne.heinonen@nokia.com> > Subject: RE: [PATCH] mmc-utils: correct ffu in case of unsupported > MODE_OPERATION_CODES > > > > > > Move up the return in case MODE_OPERATION_CODES is not supported. > > > According to the specification when MODE_OPERATION_CODES is not > > > supported no checks should be done against NUM_OF_FW_SEC_PROG. > And I wasn't able to find in your code, where you are skipping the check > against sect_done, In case MODE_OPERATION_CODES field is not supported. > > > Thanks, > Avri > > > > > > > Signed-off-by: Bruno Matic <bruno.matic@nokia.com> > > > --- > > > mmc_cmds.c | 82 > > > ++++++++++++++++++++++++++++-------------------------- > > > 1 file changed, 42 insertions(+), 40 deletions(-) > > > > > > diff --git a/mmc_cmds.c b/mmc_cmds.c index ef1d8c6..81fdc7e 100644 > > > --- a/mmc_cmds.c > > > +++ b/mmc_cmds.c > > > @@ -2897,6 +2897,13 @@ do_retry: > > > goto out; > > > } > > > > > > + /* check mode operation for ffu install*/ > > > + if (!ext_csd[EXT_CSD_FFU_FEATURES]) { > > You haven't read the ext_csd yet.... > > > > Thanks, > > Avri > > > > > + fprintf(stderr, "Please reboot to complete firmware > > > + installation on > > > %s\n", device); > > > + ret = 0; > > > + goto out; > > > + } > > > + > > > ret = read_extcsd(dev_fd, ext_csd); > > > if (ret) { > > > fprintf(stderr, "Could not read EXT_CSD from %s\n", > > > device); @@ - > > > 2927,49 +2934,44 @@ do_retry: > > > goto out; > > > } > > > > > > - /* check mode operation for ffu install*/ > > > - if (!ext_csd[EXT_CSD_FFU_FEATURES]) { > > > - fprintf(stderr, "Please reboot to complete firmware installation > on > > > %s\n", device); > > > - } else { > > > - fprintf(stderr, "Installing firmware on %s...\n", device); > > > - /* Re-enter ffu mode and install the firmware */ > > > - multi_cmd->num_of_cmds = 2; > > > - > > > - /* set ext_csd to install mode */ > > > - multi_cmd->cmds[1].opcode = MMC_SWITCH; > > > - multi_cmd->cmds[1].blksz = 0; > > > - multi_cmd->cmds[1].blocks = 0; > > > - multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE > << > > 24) > > > | > > > - (EXT_CSD_MODE_OPERATION_CODES << 16) | > > > - (EXT_CSD_FFU_INSTALL << 8) | > > > - EXT_CSD_CMD_SET_NORMAL; > > > - multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | > MMC_RSP_R1B | > > > MMC_CMD_AC; > > > - multi_cmd->cmds[1].write_flag = 1; > > > - > > > - /* send ioctl with multi-cmd */ > > > - ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > > > + fprintf(stderr, "Installing firmware on %s...\n", device); > > > + /* Re-enter ffu mode and install the firmware */ > > > + multi_cmd->num_of_cmds = 2; > > > > > > - if (ret) { > > > - perror("Multi-cmd ioctl failed setting install mode"); > > > - /* In case multi-cmd ioctl failed before exiting from ffu mode > */ > > > - ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); > > > - goto out; > > > - } > > > + /* set ext_csd to install mode */ > > > + multi_cmd->cmds[1].opcode = MMC_SWITCH; > > > + multi_cmd->cmds[1].blksz = 0; > > > + multi_cmd->cmds[1].blocks = 0; > > > + multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << > 24) | > > > + (EXT_CSD_MODE_OPERATION_CODES << 16) | > > > + (EXT_CSD_FFU_INSTALL << 8) | > > > + EXT_CSD_CMD_SET_NORMAL; > > > + multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | > > > MMC_CMD_AC; > > > + multi_cmd->cmds[1].write_flag = 1; > > > > > > - ret = read_extcsd(dev_fd, ext_csd); > > > - if (ret) { > > > - fprintf(stderr, "Could not read EXT_CSD from %s\n", device); > > > - goto out; > > > - } > > > + /* send ioctl with multi-cmd */ > > > + ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); > > > > > > - /* return status */ > > > - ret = ext_csd[EXT_CSD_FFU_STATUS]; > > > - if (ret) { > > > - fprintf(stderr, "%s: error %d during FFU install:\n", device, > ret); > > > - goto out; > > > - } else { > > > - fprintf(stderr, "FFU finished successfully\n"); > > > - } > > > + if (ret) { > > > + perror("Multi-cmd ioctl failed setting install mode"); > > > + /* 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; > > > + } > > > + > > > + /* return status */ > > > + ret = ext_csd[EXT_CSD_FFU_STATUS]; > > > + if (ret) { > > > + fprintf(stderr, "%s: error %d during FFU install:\n", device, ret); > > > + goto out; > > > + } else { > > > + fprintf(stderr, "FFU finished successfully\n"); > > > } > > > > > > out: > > > -- > > > 2.29.0
diff --git a/mmc_cmds.c b/mmc_cmds.c index ef1d8c6..81fdc7e 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -2897,6 +2897,13 @@ do_retry: goto out; } + /* check mode operation for ffu install*/ + if (!ext_csd[EXT_CSD_FFU_FEATURES]) { + fprintf(stderr, "Please reboot to complete firmware installation on %s\n", device); + ret = 0; + goto out; + } + ret = read_extcsd(dev_fd, ext_csd); if (ret) { fprintf(stderr, "Could not read EXT_CSD from %s\n", device); @@ -2927,49 +2934,44 @@ do_retry: goto out; } - /* check mode operation for ffu install*/ - if (!ext_csd[EXT_CSD_FFU_FEATURES]) { - fprintf(stderr, "Please reboot to complete firmware installation on %s\n", device); - } else { - fprintf(stderr, "Installing firmware on %s...\n", device); - /* Re-enter ffu mode and install the firmware */ - multi_cmd->num_of_cmds = 2; - - /* set ext_csd to install mode */ - multi_cmd->cmds[1].opcode = MMC_SWITCH; - multi_cmd->cmds[1].blksz = 0; - multi_cmd->cmds[1].blocks = 0; - multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) | - (EXT_CSD_MODE_OPERATION_CODES << 16) | - (EXT_CSD_FFU_INSTALL << 8) | - EXT_CSD_CMD_SET_NORMAL; - multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; - multi_cmd->cmds[1].write_flag = 1; - - /* send ioctl with multi-cmd */ - ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); + fprintf(stderr, "Installing firmware on %s...\n", device); + /* Re-enter ffu mode and install the firmware */ + multi_cmd->num_of_cmds = 2; - if (ret) { - perror("Multi-cmd ioctl failed setting install mode"); - /* In case multi-cmd ioctl failed before exiting from ffu mode */ - ioctl(dev_fd, MMC_IOC_CMD, &multi_cmd->cmds[3]); - goto out; - } + /* set ext_csd to install mode */ + multi_cmd->cmds[1].opcode = MMC_SWITCH; + multi_cmd->cmds[1].blksz = 0; + multi_cmd->cmds[1].blocks = 0; + multi_cmd->cmds[1].arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) | + (EXT_CSD_MODE_OPERATION_CODES << 16) | + (EXT_CSD_FFU_INSTALL << 8) | + EXT_CSD_CMD_SET_NORMAL; + multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; + multi_cmd->cmds[1].write_flag = 1; - ret = read_extcsd(dev_fd, ext_csd); - if (ret) { - fprintf(stderr, "Could not read EXT_CSD from %s\n", device); - goto out; - } + /* send ioctl with multi-cmd */ + ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd); - /* return status */ - ret = ext_csd[EXT_CSD_FFU_STATUS]; - if (ret) { - fprintf(stderr, "%s: error %d during FFU install:\n", device, ret); - goto out; - } else { - fprintf(stderr, "FFU finished successfully\n"); - } + if (ret) { + perror("Multi-cmd ioctl failed setting install mode"); + /* 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; + } + + /* return status */ + ret = ext_csd[EXT_CSD_FFU_STATUS]; + if (ret) { + fprintf(stderr, "%s: error %d during FFU install:\n", device, ret); + goto out; + } else { + fprintf(stderr, "FFU finished successfully\n"); } out:
Move up the return in case MODE_OPERATION_CODES is not supported. According to the specification when MODE_OPERATION_CODES is not supported no checks should be done against NUM_OF_FW_SEC_PROG. Signed-off-by: Bruno Matic <bruno.matic@nokia.com> --- mmc_cmds.c | 82 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 42 insertions(+), 40 deletions(-)