Message ID | 1506004213-22620-8-git-send-email-jjhiblot@ti.com |
---|---|
State | Accepted |
Commit | 4c9d2aaa7ea720ff8c304dd8621afa3ed085486e |
Delegated to: | Jaehoon Chung |
Headers | show |
Series | mmc: Add support for HS200 and UHS modes | expand |
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
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
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(+)