[PATCH v1] mmc-utils: FFU Status check for device without FW install support

[PATCH v1] mmc-utils: FFU Status check for device without FW install support

[Zhan Liu]

 [PATCH v1] mmc-utils: FFU Status check for device without FW install support

From: Zhan Liu <liuzhanjobs@gmail.com>

Problem and my changes

               ffu:also check ffu status for FW install not support
device to catch the possible error of FW download error
(FFU_STATUS[0x26] = 0x12), which is not captured by current mmc-utils.
current mmc-utils simple ask user to reboot, which give the user false
impression that the FW update is successful. they will only found it
is not after they check the FW version with extcsd read command and
have not idea what is wrong since at this time the FFU_STATUS has been
reset to 0x00. With this patch, user will know that FW download is
failed.

when check the devce don't support FW install, read extcsd and check
the FFU_STATUS value. If it is 0x00, ask user to reboot. If not, print
the error message and exit.



---

Signed-off-by:  Zhan Liu <liuzhanjobs@gmail.com>

---

diff --git a/mmc.h b/mmc.h

index 6f1bf3e..5b06410 100644

--- a/mmc.h

+++ b/mmc.h

@@ -229,6 +229,14 @@

#define EXT_CSD_SEC_GB_CL_EN                         (1<<4)

#define EXT_CSD_SEC_ER_EN                  (1<<0)



+/*

+ * FFU status definition

+ */

+#define EXT_CSD_FFU_SUCCESS
             (0x00)

+#define EXT_CSD_FFU_GENERAL_ERROR                                        (0x10)

+#define EXT_CSD_FFU_FIWMWARE_INSTALL_ERROR                   (0x11)

+#define EXT_CSD_FFU_FIWMWARE_DOWNLOAD_ERROR                         (0x12)

+



 /* From kernel linux/mmc/core.h */

#define MMC_RSP_NONE          0
 /* no response */

diff --git a/mmc_cmds.c b/mmc_cmds.c

index 936e0c5..10bdb94 100644

--- a/mmc_cmds.c

+++ b/mmc_cmds.c

@@ -2962,9 +2962,38 @@ do_retry:

              * 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); //get
the current extcsd after FW download

+                           if (ret) {

+                                         fprintf(stderr, "Could not
read EXT_CSD from %s\n", device);

+                                         goto out;

+                           }

+

+                           switch (ext_csd[EXT_CSD_FFU_STATUS]) {

+                           case EXT_CSD_FFU_SUCCESS:

+                                         fprintf(stderr, "Please
reboot to complete firmware installation on %s\n", device);

+                                         ret = 0;

+                                         goto out;

+

+                           case EXT_CSD_FFU_GENERAL_ERROR:

+                                         fprintf(stderr, "FFU General
Error on %s\n", device);

+                                         ret = 0;

+                                         goto out;

+

+                           case EXT_CSD_FFU_FIWMWARE_INSTALL_ERROR:
//may never happen since we have not install firmware

+                                         fprintf(stderr, "FFU Install
Error on %s\n", device);

+                                         ret = 0;

+                                         goto out;

+

+                           case EXT_CSD_FFU_FIWMWARE_DOWNLOAD_ERROR:
//main purpose is to check this since it will be cleared after power
cycle

+                                         fprintf(stderr, "FFU FW
Download Error on %s\n", device);

+                                         ret = 0;

+                                         goto out;

+

+                           default:

+                                         fprintf(stderr, "Unknown FFU
Status on %s\n", device);

+                                         ret = 0;

+                                         goto out;

+                           }

             }



              ret = read_extcsd(dev_fd, ext_csd);

RE: [PATCH v1] mmc-utils: FFU Status check for device without FW install support

[Avri Altman]

Hi,
Thank you for your patch.
Please use a text-only mail format.
Some more guidelines are here - https://www.kernel.org/doc/html/v6.8/process/submitting-patches.html

> Micron Confidential
Please remove.

>[PATCH v1] mmc-utils: FFU Status check for device without FW install support
Maybe just: mmc-utils: Always report FFU Status

>
>From: Zhan Liu mailto:zliua@micron.com
>
>Problem and my changes 
>               ffu:also check ffu status for FW install not support device to catch the possible error of FW download error (FFU_STATUS[0x26] = 0x12), which is not captured by current mmc-utils. current mmc-utils simple ask user to reboot, which give the user false impression that the FW >update is successful. they will only found it is not after they check the FW version with extcsd read command and have not idea what is wrong since at this time the FFU_STATUS has been reset to 0x00. With this patch, user will know that FW download is failed. 
>when check the devce don't support FW install, read extcsd and check the FFU_STATUS value. If it is 0x00, ask user to reboot. If not, print the error message and exit.
I agree.
Even if the device doesn't support MODE_OPERATION_CODES,
There is no reason to omit its ffu-status report.

<snip>
>             if (!ext_csd[EXT_CSD_FFU_FEATURES]) {
>-                           fprintf(stderr, "Please reboot to complete firmware installation on %s\n", device);
>-                           ret = 0;
>-                           goto out;
We report the status later - anyway, let's us it.
How about, instead of being so verbose about the possible statuses,
Just add a "report-status" label and goto it instead of out?

<snip>

>Micron Confidential
Please remove

Thanks,
Avri