diff mbox series

cmd: mmc: Print life time info

Message ID 20221102141710.63840-1-robert.krikke@nedap.com
State Rejected
Delegated to: Jaehoon Chung
Headers show
Series cmd: mmc: Print life time info | expand

Commit Message

Robert Krikke Nov. 2, 2022, 2:17 p.m. UTC
Added life time info:
-EXT_CSD_PRE_EOL_INFO
-EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A
-EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B

Signed-off-by: Robert Krikke <robertkrikke@gmail.com>
Reviewed-by: Harm Berntsen <harm.berntsen@nedap.com>
---
 cmd/mmc.c     | 6 ++++++
 include/mmc.h | 3 +++
 2 files changed, 9 insertions(+)

Comments

Jaehoon Chung Nov. 2, 2022, 10:49 p.m. UTC | #1
Hi,

On 11/2/22 23:17, Robert Krikke wrote:
> Added life time info:
> -EXT_CSD_PRE_EOL_INFO
> -EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A
> -EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B

I'm not sure that it really needs to display this information as mmc info.
I hope that mmc info command should be displayed essential information.

> 
> Signed-off-by: Robert Krikke <robertkrikke@gmail.com>
> Reviewed-by: Harm Berntsen <harm.berntsen@nedap.com>
> ---
>  cmd/mmc.c     | 6 ++++++
>  include/mmc.h | 3 +++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index 7bd4cd9e016..b940e320295 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -127,6 +127,12 @@ static void print_mmcinfo(struct mmc *mmc)
>  			}
>  			wp >>= 2;
>  		}
> +
> +		if (mmc->version >= MMC_VERSION_5_0) {

Move to outside of MMC_VERSION_4_41 if statement.

> +                	printf("Pre EOL Information: 0x%02X\n", ext_csd[EXT_CSD_PRE_EOL_INFO]);
> +			printf("Life Time Estimation A: 0x%02X\n", ext_csd[EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A]);
> +			printf("Life Time Estimation B: 0x%02X\n", ext_csd[EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B]);

It's not a readable value. If didn't check the eMMC5.0 spec, it's difficult to know what means 0x01, 0x02..etc..

Best Regards,
Jaehoon Chung

> +		}
>  	}
>  }
>  
> diff --git a/include/mmc.h b/include/mmc.h
> index f519d869725..d0c3e684595 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -242,6 +242,9 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>  #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
>  #define EXT_CSD_BOOT_MULT		226	/* RO */
>  #define EXT_CSD_GENERIC_CMD6_TIME       248     /* RO */
> +#define EXT_CSD_PRE_EOL_INFO		267	/* RO */
> +#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A	268	/* RO */
> +#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B	269	/* RO */
>  #define EXT_CSD_BKOPS_SUPPORT		502	/* RO */
>  
>  /*
ZHIZHIKIN Andrey Nov. 3, 2022, 9:10 a.m. UTC | #2
Hello all,

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Jaehoon Chung
> Sent: Wednesday, November 2, 2022 11:49 PM
> To: Robert Krikke <robertkrikke@gmail.com>; u-boot@lists.denx.de
> Cc: Robert Krikke <robert.krikke@nedap.com>; Harm Berntsen
> <harm.berntsen@nedap.com>
> Subject: Re: [PATCH] cmd: mmc: Print life time info
> 
> Hi,
> 
> On 11/2/22 23:17, Robert Krikke wrote:
> > Added life time info:
> > -EXT_CSD_PRE_EOL_INFO
> > -EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A
> > -EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B
> 
> I'm not sure that it really needs to display this information as mmc info.
> I hope that mmc info command should be displayed essential information.
> 
> >
> > Signed-off-by: Robert Krikke <robertkrikke@gmail.com>
> > Reviewed-by: Harm Berntsen <harm.berntsen@nedap.com>
> > ---
> >  cmd/mmc.c     | 6 ++++++
> >  include/mmc.h | 3 +++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/cmd/mmc.c b/cmd/mmc.c
> > index 7bd4cd9e016..b940e320295 100644
> > --- a/cmd/mmc.c
> > +++ b/cmd/mmc.c
> > @@ -127,6 +127,12 @@ static void print_mmcinfo(struct mmc *mmc)
> >  			}
> >  			wp >>= 2;
> >  		}
> > +
> > +		if (mmc->version >= MMC_VERSION_5_0) {
> 
> Move to outside of MMC_VERSION_4_41 if statement.
> 
> > +                	printf("Pre EOL Information: 0x%02X\n",
> ext_csd[EXT_CSD_PRE_EOL_INFO]);
> > +			printf("Life Time Estimation A: 0x%02X\n",
> ext_csd[EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A]);
> > +			printf("Life Time Estimation B: 0x%02X\n",
> ext_csd[EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B]);
> 
> It's not a readable value. If didn't check the eMMC5.0 spec, it's difficult to
> know what means 0x01, 0x02..etc..

According to JEDEC spec, this value shows the "used lifetime of the
device in <value> x 10%".

PRE_EOL_INFO [267] provides indication about device life time
reflected by average reserved blocks, and it can take only values:
- 0x00 [Not Defined]
- 0x01 [Normal Normal]
- 0x02 [Warning: Consumed 80% of reserved block]
- 0x03 [Urgent]
Everything else is Reserved.

I would also have the same comment here as Jaehoon, as from the
commit message it is not clear what is the Use Case you're trying
to solve?

If it is designed for automation - then the output is a bit cluttered
and hard to parse. If it is designed for user readability, then I
would rather suggest to convert it to what the field in ExtCSD really
shows: percentage and status accordingly.

A bit more context would be appreciated here, preferably in commit
message.

All-in-all, I really doubt that this info is useful in U-Boot. Linux
has a full readout of the ExtCSD, where the same value can be obtained.
If it is designed to stop the update and report that eMMC is at the end
of its life - then it is rather "automation" Use Case, and as I said
above - the output is way too cluttered to parse.

> 
> Best Regards,
> Jaehoon Chung
> 
> > +		}
> >  	}
> >  }
> >
> > diff --git a/include/mmc.h b/include/mmc.h
> > index f519d869725..d0c3e684595 100644
> > --- a/include/mmc.h
> > +++ b/include/mmc.h
> > @@ -242,6 +242,9 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
> >  #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
> >  #define EXT_CSD_BOOT_MULT		226	/* RO */
> >  #define EXT_CSD_GENERIC_CMD6_TIME       248     /* RO */
> > +#define EXT_CSD_PRE_EOL_INFO		267	/* RO */
> > +#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A	268	/* RO */
> > +#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B	269	/* RO */
> >  #define EXT_CSD_BKOPS_SUPPORT		502	/* RO */
> >
> >  /*

Regards,
Andrey
diff mbox series

Patch

diff --git a/cmd/mmc.c b/cmd/mmc.c
index 7bd4cd9e016..b940e320295 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -127,6 +127,12 @@  static void print_mmcinfo(struct mmc *mmc)
 			}
 			wp >>= 2;
 		}
+
+		if (mmc->version >= MMC_VERSION_5_0) {
+                	printf("Pre EOL Information: 0x%02X\n", ext_csd[EXT_CSD_PRE_EOL_INFO]);
+			printf("Life Time Estimation A: 0x%02X\n", ext_csd[EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A]);
+			printf("Life Time Estimation B: 0x%02X\n", ext_csd[EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B]);
+		}
 	}
 }
 
diff --git a/include/mmc.h b/include/mmc.h
index f519d869725..d0c3e684595 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -242,6 +242,9 @@  static inline bool mmc_is_tuning_cmd(uint cmdidx)
 #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
 #define EXT_CSD_BOOT_MULT		226	/* RO */
 #define EXT_CSD_GENERIC_CMD6_TIME       248     /* RO */
+#define EXT_CSD_PRE_EOL_INFO		267	/* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A	268	/* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B	269	/* RO */
 #define EXT_CSD_BKOPS_SUPPORT		502	/* RO */
 
 /*