diff mbox series

[v2] mmc-utils: Fix ffu in case of unsupported MODE_OPERATION_CODES

Message ID 20220920124749.24073-1-bruno.matic@nokia.com (mailing list archive)
State New, archived
Headers show
Series [v2] mmc-utils: Fix ffu in case of unsupported MODE_OPERATION_CODES | expand

Commit Message

Bruno Matic Sept. 20, 2022, 12:47 p.m. UTC 
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.

Changes in v2:
  - better explanation in the comment

Signed-off-by: Bruno Matic <bruno.matic@nokia.com>
---
 mmc_cmds.c | 85 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 40 deletions(-)

Comments

Avri Altman Sept. 20, 2022, 12:54 p.m. UTC | #1 
> 
> 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.
> 
> Changes in v2:
>   - better explanation in the comment
Fixes: 89cd01ed865a (mmc_utils: add ffu support)

Thanks,
Avri

> 
> Signed-off-by: Bruno Matic <bruno.matic@nokia.com>
> ---
>  mmc_cmds.c | 85 +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 45 insertions(+), 40 deletions(-)
> 
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index ef1d8c6..a3740ab 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -2897,6 +2897,16 @@ do_retry:
>                 goto out;
>         }
> 
> +       /*
> +        * By spec - check if mode operation codes are supported in ffu
> features,
> +        * if not then skip checking number of sectors programmed after 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 +2937,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
Ulf Hansson Sept. 21, 2022, 12:57 p.m. UTC | #2 
On Tue, 20 Sept 2022 at 14:54, Avri Altman <Avri.Altman@wdc.com> wrote:
>
> >
> > 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.
> >
> > Changes in v2:
> >   - better explanation in the comment
> Fixes: 89cd01ed865a (mmc_utils: add ffu support)
>
> Thanks,
> Avri
>
> >
> > Signed-off-by: Bruno Matic <bruno.matic@nokia.com>

Applied to git.kernel.org/pub/scm//utils/mmc/mmc-utils.git master, thanks!

Avri, I assume you are happy with the partch, but just forgot to add
your reviewed-by-tag? I added it, but please tell me if I should drop
it.

Kind regards
Uffe

> > ---
> >  mmc_cmds.c | 85 +++++++++++++++++++++++++++++-------------------------
> >  1 file changed, 45 insertions(+), 40 deletions(-)
> >
> > diff --git a/mmc_cmds.c b/mmc_cmds.c
> > index ef1d8c6..a3740ab 100644
> > --- a/mmc_cmds.c
> > +++ b/mmc_cmds.c
> > @@ -2897,6 +2897,16 @@ do_retry:
> >                 goto out;
> >         }
> >
> > +       /*
> > +        * By spec - check if mode operation codes are supported in ffu
> > features,
> > +        * if not then skip checking number of sectors programmed after 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 +2937,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 mbox series

Patch

diff --git a/mmc_cmds.c b/mmc_cmds.c
index ef1d8c6..a3740ab 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -2897,6 +2897,16 @@  do_retry:
 		goto out;
 	}
 
+	/*
+	 * By spec - check if mode operation codes are supported in ffu features,
+	 * if not then skip checking number of sectors programmed after 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 +2937,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: