[PATCH V6 01/24] mmc: core: Cleanup printing of speed mode at card insertion

Victor Shih posted 24 patches 2 years, 9 months ago
There is a newer version of this series
[PATCH V6 01/24] mmc: core: Cleanup printing of speed mode at card insertion
Posted by Victor Shih 2 years, 9 months ago
The current print of the bus speed mode in mmc_add_card() has grown over
the years and is now difficult to parse. Let's clean up the code and also
take the opportunity to properly announce "DDR" for eMMCs as
"high speed DDR", which is according to the eMMC spec.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
---
 drivers/mmc/core/bus.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 36679f4e9acc..bbbbdbca5366 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -299,6 +299,7 @@ int mmc_add_card(struct mmc_card *card)
 {
 	int ret;
 	const char *type;
+	const char *speed_mode = "";
 	const char *uhs_bus_speed_mode = "";
 	static const char *const uhs_speeds[] = {
 		[UHS_SDR12_BUS_SPEED] = "SDR12 ",
@@ -337,25 +338,29 @@ int mmc_add_card(struct mmc_card *card)
 		break;
 	}
 
+	if (mmc_card_hs(card))
+		speed_mode = "high speed ";
+	else if (mmc_card_uhs(card))
+		speed_mode = "ultra high speed ";
+	else if	(mmc_card_ddr52(card))
+		speed_mode = "high speed DDR ";
+	else if (mmc_card_hs200(card))
+		speed_mode = "HS200 ";
+	else if (mmc_card_hs400es(card))
+		speed_mode = "HS400 Enhanced strobe ";
+	else if (mmc_card_hs400(card))
+		speed_mode = "HS400 ";
+
 	if (mmc_card_uhs(card) &&
 		(card->sd_bus_speed < ARRAY_SIZE(uhs_speeds)))
 		uhs_bus_speed_mode = uhs_speeds[card->sd_bus_speed];
 
-	if (mmc_host_is_spi(card->host)) {
-		pr_info("%s: new %s%s%s card on SPI\n",
-			mmc_hostname(card->host),
-			mmc_card_hs(card) ? "high speed " : "",
-			mmc_card_ddr52(card) ? "DDR " : "",
-			type);
-	} else {
-		pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
-			mmc_hostname(card->host),
-			mmc_card_uhs(card) ? "ultra high speed " :
-			(mmc_card_hs(card) ? "high speed " : ""),
-			mmc_card_hs400(card) ? "HS400 " :
-			(mmc_card_hs200(card) ? "HS200 " : ""),
-			mmc_card_hs400es(card) ? "Enhanced strobe " : "",
-			mmc_card_ddr52(card) ? "DDR " : "",
+	if (mmc_host_is_spi(card->host))
+		pr_info("%s: new %s%s card on SPI\n",
+			mmc_hostname(card->host), speed_mode, type);
+	else {
+		pr_info("%s: new %s%s%s card at address %04x\n",
+			mmc_hostname(card->host), speed_mode,
 			uhs_bus_speed_mode, type, card->rca);
 	}
 
-- 
2.25.1
Re: [PATCH V6 01/24] mmc: core: Cleanup printing of speed mode at card insertion
Posted by Adrian Hunter 2 years, 8 months ago
On 13/12/22 11:00, Victor Shih wrote:
> The current print of the bus speed mode in mmc_add_card() has grown over
> the years and is now difficult to parse. Let's clean up the code and also
> take the opportunity to properly announce "DDR" for eMMCs as
> "high speed DDR", which is according to the eMMC spec.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
> ---
>  drivers/mmc/core/bus.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 36679f4e9acc..bbbbdbca5366 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -299,6 +299,7 @@ int mmc_add_card(struct mmc_card *card)
>  {
>  	int ret;
>  	const char *type;
> +	const char *speed_mode = "";
>  	const char *uhs_bus_speed_mode = "";
>  	static const char *const uhs_speeds[] = {
>  		[UHS_SDR12_BUS_SPEED] = "SDR12 ",
> @@ -337,25 +338,29 @@ int mmc_add_card(struct mmc_card *card)
>  		break;
>  	}
>  
> +	if (mmc_card_hs(card))
> +		speed_mode = "high speed ";
> +	else if (mmc_card_uhs(card))
> +		speed_mode = "ultra high speed ";
> +	else if	(mmc_card_ddr52(card))
> +		speed_mode = "high speed DDR ";
> +	else if (mmc_card_hs200(card))
> +		speed_mode = "HS200 ";
> +	else if (mmc_card_hs400es(card))
> +		speed_mode = "HS400 Enhanced strobe ";
> +	else if (mmc_card_hs400(card))
> +		speed_mode = "HS400 ";
> +
>  	if (mmc_card_uhs(card) &&
>  		(card->sd_bus_speed < ARRAY_SIZE(uhs_speeds)))
>  		uhs_bus_speed_mode = uhs_speeds[card->sd_bus_speed];
>  
> -	if (mmc_host_is_spi(card->host)) {
> -		pr_info("%s: new %s%s%s card on SPI\n",
> -			mmc_hostname(card->host),
> -			mmc_card_hs(card) ? "high speed " : "",
> -			mmc_card_ddr52(card) ? "DDR " : "",
> -			type);
> -	} else {
> -		pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
> -			mmc_hostname(card->host),
> -			mmc_card_uhs(card) ? "ultra high speed " :
> -			(mmc_card_hs(card) ? "high speed " : ""),
> -			mmc_card_hs400(card) ? "HS400 " :
> -			(mmc_card_hs200(card) ? "HS200 " : ""),
> -			mmc_card_hs400es(card) ? "Enhanced strobe " : "",
> -			mmc_card_ddr52(card) ? "DDR " : "",
> +	if (mmc_host_is_spi(card->host))
> +		pr_info("%s: new %s%s card on SPI\n",
> +			mmc_hostname(card->host), speed_mode, type);
> +	else {

This bracket wasn't in V5.  Brackets should be on both
branches of the if-clause or neither.

> +		pr_info("%s: new %s%s%s card at address %04x\n",
> +			mmc_hostname(card->host), speed_mode,
>  			uhs_bus_speed_mode, type, card->rca);
>  	}
>
Re: [PATCH V6 01/24] mmc: core: Cleanup printing of speed mode at card insertion
Posted by Victor Shih 2 years, 6 months ago
Hi, Adrian

On Fri, Jan 6, 2023 at 5:22 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 13/12/22 11:00, Victor Shih wrote:
> > The current print of the bus speed mode in mmc_add_card() has grown over
> > the years and is now difficult to parse. Let's clean up the code and also
> > take the opportunity to properly announce "DDR" for eMMCs as
> > "high speed DDR", which is according to the eMMC spec.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
> > ---
> >  drivers/mmc/core/bus.c | 35 ++++++++++++++++++++---------------
> >  1 file changed, 20 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> > index 36679f4e9acc..bbbbdbca5366 100644
> > --- a/drivers/mmc/core/bus.c
> > +++ b/drivers/mmc/core/bus.c
> > @@ -299,6 +299,7 @@ int mmc_add_card(struct mmc_card *card)
> >  {
> >       int ret;
> >       const char *type;
> > +     const char *speed_mode = "";
> >       const char *uhs_bus_speed_mode = "";
> >       static const char *const uhs_speeds[] = {
> >               [UHS_SDR12_BUS_SPEED] = "SDR12 ",
> > @@ -337,25 +338,29 @@ int mmc_add_card(struct mmc_card *card)
> >               break;
> >       }
> >
> > +     if (mmc_card_hs(card))
> > +             speed_mode = "high speed ";
> > +     else if (mmc_card_uhs(card))
> > +             speed_mode = "ultra high speed ";
> > +     else if (mmc_card_ddr52(card))
> > +             speed_mode = "high speed DDR ";
> > +     else if (mmc_card_hs200(card))
> > +             speed_mode = "HS200 ";
> > +     else if (mmc_card_hs400es(card))
> > +             speed_mode = "HS400 Enhanced strobe ";
> > +     else if (mmc_card_hs400(card))
> > +             speed_mode = "HS400 ";
> > +
> >       if (mmc_card_uhs(card) &&
> >               (card->sd_bus_speed < ARRAY_SIZE(uhs_speeds)))
> >               uhs_bus_speed_mode = uhs_speeds[card->sd_bus_speed];
> >
> > -     if (mmc_host_is_spi(card->host)) {
> > -             pr_info("%s: new %s%s%s card on SPI\n",
> > -                     mmc_hostname(card->host),
> > -                     mmc_card_hs(card) ? "high speed " : "",
> > -                     mmc_card_ddr52(card) ? "DDR " : "",
> > -                     type);
> > -     } else {
> > -             pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
> > -                     mmc_hostname(card->host),
> > -                     mmc_card_uhs(card) ? "ultra high speed " :
> > -                     (mmc_card_hs(card) ? "high speed " : ""),
> > -                     mmc_card_hs400(card) ? "HS400 " :
> > -                     (mmc_card_hs200(card) ? "HS200 " : ""),
> > -                     mmc_card_hs400es(card) ? "Enhanced strobe " : "",
> > -                     mmc_card_ddr52(card) ? "DDR " : "",
> > +     if (mmc_host_is_spi(card->host))
> > +             pr_info("%s: new %s%s card on SPI\n",
> > +                     mmc_hostname(card->host), speed_mode, type);
> > +     else {
>
> This bracket wasn't in V5.  Brackets should be on both
> branches of the if-clause or neither.
>

I will update it in V7 version.

> > +             pr_info("%s: new %s%s%s card at address %04x\n",
> > +                     mmc_hostname(card->host), speed_mode,
> >                       uhs_bus_speed_mode, type, card->rca);
> >       }
> >
>

Thanks, Victor Shih