[U-Boot,v2,07/26] mmc: Add a function to dump the mmc capabilities

This adds a simple helper function to display information (bus width and
mode) based on a capability mask. Useful for debug.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 drivers/mmc/mmc.c | 24 ++++++++++++++++++++++++
 include/mmc.h     |  1 +
 2 files changed, 25 insertions(+)

Hi,

On 09/21/2017 11:29 PM, Jean-Jacques Hiblot wrote:
> This adds a simple helper function to display information (bus width and
> mode) based on a capability mask. Useful for debug.

I agreed this is useful.. but there is no usage in your patch.
How did you use this? and Where does call this function..

I think it can be used the one of mmc command. how about?

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>  drivers/mmc/mmc.c | 24 ++++++++++++++++++++++++
>  include/mmc.h     |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 94b3a02..0b74e78 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -1136,6 +1136,30 @@ static void mmc_set_bus_width(struct mmc *mmc, uint width)
>  	mmc_set_ios(mmc);
>  }
>  
> +#if CONFIG_IS_ENABLED(MMC_VERBOSE) || defined(DEBUG)
> +/*
> + * helper function to display the capabilities in a human
> + * friendly manner. The capabilities include bus width and
> + * supported modes.
> + */
> +void mmc_dump_capabilities(const char *text, uint caps)
> +{
> +	enum bus_mode mode;
> +
> +	printf("%s: widths [", text);
> +	if (caps & MMC_MODE_8BIT)
> +		printf("8, ");
> +	if (caps & MMC_MODE_4BIT)
> +		printf("4, ");
> +	printf("1] modes [");
> +
> +	for (mode = MMC_LEGACY; mode < MMC_MODES_END; mode++)
> +		if (MMC_CAP(mode) & caps)
> +			printf("%s, ", mmc_mode_name(mode));
> +	printf("\b\b]\n");
> +}
> +#endif
> +
>  static int sd_select_bus_freq_width(struct mmc *mmc)
>  {
>  	int err;
> diff --git a/include/mmc.h b/include/mmc.h
> index 76bd57a..dd83f14 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -426,6 +426,7 @@ enum bus_mode {
>  };
>  
>  const char *mmc_mode_name(enum bus_mode mode);
> +void mmc_dump_capabilities(const char *text, uint caps);
>  
>  /*
>   * With CONFIG_DM_MMC enabled, struct mmc can be accessed from the MMC device
>

Hi Jaehoon,


On 22/09/2017 15:54, Jaehoon Chung wrote:
> Hi,
>
> On 09/21/2017 11:29 PM, Jean-Jacques Hiblot wrote:
>> This adds a simple helper function to display information (bus width and
>> mode) based on a capability mask. Useful for debug.
> I agreed this is useful.. but there is no usage in your patch.
> How did you use this? and Where does call this function..
>
> I think it can be used the one of mmc command. how about?
At first I added it to "mmc info" but it's more for the developer than 
the user, so I removed it from there.
At the moment it is not referenced anywhere the code, but I left it in 
place because it's indeed useful when debugging the initialization.
Thinking of it I could add something right after the card capabilities 
are discovered if debug is enabled. What do you think?

Jean-Jacques

>
> Best Regards,
> Jaehoon Chung
>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> ---
>>   drivers/mmc/mmc.c | 24 ++++++++++++++++++++++++
>>   include/mmc.h     |  1 +
>>   2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index 94b3a02..0b74e78 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -1136,6 +1136,30 @@ static void mmc_set_bus_width(struct mmc *mmc, uint width)
>>   	mmc_set_ios(mmc);
>>   }
>>   
>> +#if CONFIG_IS_ENABLED(MMC_VERBOSE) || defined(DEBUG)
>> +/*
>> + * helper function to display the capabilities in a human
>> + * friendly manner. The capabilities include bus width and
>> + * supported modes.
>> + */
>> +void mmc_dump_capabilities(const char *text, uint caps)
>> +{
>> +	enum bus_mode mode;
>> +
>> +	printf("%s: widths [", text);
>> +	if (caps & MMC_MODE_8BIT)
>> +		printf("8, ");
>> +	if (caps & MMC_MODE_4BIT)
>> +		printf("4, ");
>> +	printf("1] modes [");
>> +
>> +	for (mode = MMC_LEGACY; mode < MMC_MODES_END; mode++)
>> +		if (MMC_CAP(mode) & caps)
>> +			printf("%s, ", mmc_mode_name(mode));
>> +	printf("\b\b]\n");
>> +}
>> +#endif
>> +
>>   static int sd_select_bus_freq_width(struct mmc *mmc)
>>   {
>>   	int err;
>> diff --git a/include/mmc.h b/include/mmc.h
>> index 76bd57a..dd83f14 100644
>> --- a/include/mmc.h
>> +++ b/include/mmc.h
>> @@ -426,6 +426,7 @@ enum bus_mode {
>>   };
>>   
>>   const char *mmc_mode_name(enum bus_mode mode);
>> +void mmc_dump_capabilities(const char *text, uint caps);
>>   
>>   /*
>>    * With CONFIG_DM_MMC enabled, struct mmc can be accessed from the MMC device
>>
>

On 2 October 2017 at 02:57, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> Hi Jaehoon,
>
>
> On 22/09/2017 15:54, Jaehoon Chung wrote:
>>
>> Hi,
>>
>> On 09/21/2017 11:29 PM, Jean-Jacques Hiblot wrote:
>>>
>>> This adds a simple helper function to display information (bus width and
>>> mode) based on a capability mask. Useful for debug.
>>
>> I agreed this is useful.. but there is no usage in your patch.
>> How did you use this? and Where does call this function..
>>
>> I think it can be used the one of mmc command. how about?
>
> At first I added it to "mmc info" but it's more for the developer than the
> user, so I removed it from there.
> At the moment it is not referenced anywhere the code, but I left it in place
> because it's indeed useful when debugging the initialization.
> Thinking of it I could add something right after the card capabilities are
> discovered if debug is enabled. What do you think?
>
> Jean-Jacques

That seems reasonable.

In any case, we cannot add dead code.

- Simon