| Message ID | 20241113053023.1870736-1-caleb.connolly@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: don't print 'MMC:' if there are no MMC devices | expand |
On Wed, 13 Nov 2024 at 07:30, Caleb Connolly <caleb.connolly@linaro.org> wrote: > > It may be the case that MMC support is enabled even though the board > we're booting on doesn't have any MMC devices. Move the print over to > the print_mmc_devices() function where we can only print it if we > actually have MMC devices. > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > common/board_r.c | 1 - > drivers/mmc/mmc-uclass.c | 4 +++- > drivers/mmc/mmc_legacy.c | 5 +++++ > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/common/board_r.c b/common/board_r.c > index 62228a723e12..232a8fd19f03 100644 > --- a/common/board_r.c > +++ b/common/board_r.c > @@ -384,9 +384,8 @@ static int initr_onenand(void) > > #ifdef CONFIG_MMC > static int initr_mmc(void) > { > - puts("MMC: "); > mmc_initialize(gd->bd); > return 0; > } > #endif > diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c > index c8db4f811c2f..56fe29249c36 100644 > --- a/drivers/mmc/mmc-uclass.c > +++ b/drivers/mmc/mmc-uclass.c > @@ -384,9 +384,11 @@ void print_mmc_devices(char separator) > dev; > uclass_next_device(&dev), first = false) { > struct mmc *m = mmc_get_mmc_dev(dev); > > - if (!first) { > + if (first) { > + printf("MMC: "); > + } else { > printf("%c", separator); > if (separator != '\n') > puts(" "); > } > diff --git a/drivers/mmc/mmc_legacy.c b/drivers/mmc/mmc_legacy.c > index 8f8ba34be717..f4a049a4a4d4 100644 > --- a/drivers/mmc/mmc_legacy.c > +++ b/drivers/mmc/mmc_legacy.c > @@ -98,8 +98,9 @@ void print_mmc_devices(char separator) > { > struct mmc *m; > struct list_head *entry; > char *mmc_type; > + bool first = true; > > list_for_each(entry, &mmc_devices) { > m = list_entry(entry, struct mmc, link); > > @@ -107,8 +108,12 @@ void print_mmc_devices(char separator) > mmc_type = IS_SD(m) ? "SD" : "eMMC"; > else > mmc_type = NULL; > > + if (first) { > + printf("MMC: "); > + first = false; > + } > printf("%s: %d", m->cfg->name, m->block_dev.devnum); > if (mmc_type) > printf(" (%s)", mmc_type); > > -- > 2.47.0 > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Hello Caleb, Thanks for the patch. Please, see a comment below. On 2024-11-13 06:30, Caleb Connolly wrote: > It may be the case that MMC support is enabled even though the board > we're booting on doesn't have any MMC devices. Move the print over to > the print_mmc_devices() function where we can only print it if we > actually have MMC devices. > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > common/board_r.c | 1 - > drivers/mmc/mmc-uclass.c | 4 +++- > drivers/mmc/mmc_legacy.c | 5 +++++ > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/common/board_r.c b/common/board_r.c > index 62228a723e12..232a8fd19f03 100644 > --- a/common/board_r.c > +++ b/common/board_r.c > @@ -384,9 +384,8 @@ static int initr_onenand(void) > > #ifdef CONFIG_MMC > static int initr_mmc(void) > { > - puts("MMC: "); > mmc_initialize(gd->bd); > return 0; > } > #endif > diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c > index c8db4f811c2f..56fe29249c36 100644 > --- a/drivers/mmc/mmc-uclass.c > +++ b/drivers/mmc/mmc-uclass.c > @@ -384,9 +384,11 @@ void print_mmc_devices(char separator) > dev; > uclass_next_device(&dev), first = false) { > struct mmc *m = mmc_get_mmc_dev(dev); > > - if (!first) { > + if (first) { > + printf("MMC: "); > + } else { > printf("%c", separator); > if (separator != '\n') > puts(" "); > } > diff --git a/drivers/mmc/mmc_legacy.c b/drivers/mmc/mmc_legacy.c > index 8f8ba34be717..f4a049a4a4d4 100644 > --- a/drivers/mmc/mmc_legacy.c > +++ b/drivers/mmc/mmc_legacy.c > @@ -98,8 +98,9 @@ void print_mmc_devices(char separator) > { > struct mmc *m; > struct list_head *entry; > char *mmc_type; > + bool first = true; > > list_for_each(entry, &mmc_devices) { > m = list_entry(entry, struct mmc, link); > > @@ -107,8 +108,12 @@ void print_mmc_devices(char separator) > mmc_type = IS_SD(m) ? "SD" : "eMMC"; > else > mmc_type = NULL; > > + if (first) { > + printf("MMC: "); > + first = false; > + } Can't we simply check for "entry == &mmc_devices->next" here and avoid the introduction of "first"? > printf("%s: %d", m->cfg->name, m->block_dev.devnum); > if (mmc_type) > printf(" (%s)", mmc_type);
On 13/11/2024 10:48, Dragan Simic wrote: > Hello Caleb, > > Thanks for the patch. Please, see a comment below. > > On 2024-11-13 06:30, Caleb Connolly wrote: >> It may be the case that MMC support is enabled even though the board >> we're booting on doesn't have any MMC devices. Move the print over to >> the print_mmc_devices() function where we can only print it if we >> actually have MMC devices. >> >> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> >> --- >> common/board_r.c | 1 - >> drivers/mmc/mmc-uclass.c | 4 +++- >> drivers/mmc/mmc_legacy.c | 5 +++++ >> 3 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/common/board_r.c b/common/board_r.c >> index 62228a723e12..232a8fd19f03 100644 >> --- a/common/board_r.c >> +++ b/common/board_r.c >> @@ -384,9 +384,8 @@ static int initr_onenand(void) >> >> #ifdef CONFIG_MMC >> static int initr_mmc(void) >> { >> - puts("MMC: "); >> mmc_initialize(gd->bd); >> return 0; >> } >> #endif >> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c >> index c8db4f811c2f..56fe29249c36 100644 >> --- a/drivers/mmc/mmc-uclass.c >> +++ b/drivers/mmc/mmc-uclass.c >> @@ -384,9 +384,11 @@ void print_mmc_devices(char separator) >> dev; >> uclass_next_device(&dev), first = false) { >> struct mmc *m = mmc_get_mmc_dev(dev); >> >> - if (!first) { >> + if (first) { >> + printf("MMC: "); >> + } else { >> printf("%c", separator); >> if (separator != '\n') >> puts(" "); >> } >> diff --git a/drivers/mmc/mmc_legacy.c b/drivers/mmc/mmc_legacy.c >> index 8f8ba34be717..f4a049a4a4d4 100644 >> --- a/drivers/mmc/mmc_legacy.c >> +++ b/drivers/mmc/mmc_legacy.c >> @@ -98,8 +98,9 @@ void print_mmc_devices(char separator) >> { >> struct mmc *m; >> struct list_head *entry; >> char *mmc_type; >> + bool first = true; >> >> list_for_each(entry, &mmc_devices) { >> m = list_entry(entry, struct mmc, link); >> >> @@ -107,8 +108,12 @@ void print_mmc_devices(char separator) >> mmc_type = IS_SD(m) ? "SD" : "eMMC"; >> else >> mmc_type = NULL; >> >> + if (first) { >> + printf("MMC: "); >> + first = false; >> + } > > Can't we simply check for "entry == &mmc_devices->next" here and > avoid the introduction of "first"? That would be clever, yeah I can do this in v2. I hvaen't found a way to test mmc_legacy thus far, does anyone know how I could? > >> printf("%s: %d", m->cfg->name, m->block_dev.devnum); >> if (mmc_type) >> printf(" (%s)", mmc_type);
Hello Caleb, On 2024-11-13 14:08, Caleb Connolly wrote: > On 13/11/2024 10:48, Dragan Simic wrote: >> Thanks for the patch. Please, see a comment below. >> >> On 2024-11-13 06:30, Caleb Connolly wrote: >>> It may be the case that MMC support is enabled even though the board >>> we're booting on doesn't have any MMC devices. Move the print over to >>> the print_mmc_devices() function where we can only print it if we >>> actually have MMC devices. >>> >>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> >>> --- >>> common/board_r.c | 1 - >>> drivers/mmc/mmc-uclass.c | 4 +++- >>> drivers/mmc/mmc_legacy.c | 5 +++++ >>> 3 files changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/common/board_r.c b/common/board_r.c >>> index 62228a723e12..232a8fd19f03 100644 >>> --- a/common/board_r.c >>> +++ b/common/board_r.c >>> @@ -384,9 +384,8 @@ static int initr_onenand(void) >>> >>> #ifdef CONFIG_MMC >>> static int initr_mmc(void) >>> { >>> - puts("MMC: "); >>> mmc_initialize(gd->bd); >>> return 0; >>> } >>> #endif >>> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c >>> index c8db4f811c2f..56fe29249c36 100644 >>> --- a/drivers/mmc/mmc-uclass.c >>> +++ b/drivers/mmc/mmc-uclass.c >>> @@ -384,9 +384,11 @@ void print_mmc_devices(char separator) >>> dev; >>> uclass_next_device(&dev), first = false) { >>> struct mmc *m = mmc_get_mmc_dev(dev); >>> >>> - if (!first) { >>> + if (first) { >>> + printf("MMC: "); >>> + } else { >>> printf("%c", separator); >>> if (separator != '\n') >>> puts(" "); >>> } >>> diff --git a/drivers/mmc/mmc_legacy.c b/drivers/mmc/mmc_legacy.c >>> index 8f8ba34be717..f4a049a4a4d4 100644 >>> --- a/drivers/mmc/mmc_legacy.c >>> +++ b/drivers/mmc/mmc_legacy.c >>> @@ -98,8 +98,9 @@ void print_mmc_devices(char separator) >>> { >>> struct mmc *m; >>> struct list_head *entry; >>> char *mmc_type; >>> + bool first = true; >>> >>> list_for_each(entry, &mmc_devices) { >>> m = list_entry(entry, struct mmc, link); >>> >>> @@ -107,8 +108,12 @@ void print_mmc_devices(char separator) >>> mmc_type = IS_SD(m) ? "SD" : "eMMC"; >>> else >>> mmc_type = NULL; >>> >>> + if (first) { >>> + printf("MMC: "); >>> + first = false; >>> + } >> >> Can't we simply check for "entry == &mmc_devices->next" here and >> avoid the introduction of "first"? > > That would be clever, yeah I can do this in v2. Thanks, please make sure to Cc me in the v2 submission. > I hvaen't found a way to test mmc_legacy thus far, does anyone know how > I could? > >>> printf("%s: %d", m->cfg->name, m->block_dev.devnum); >>> if (mmc_type) >>> printf(" (%s)", mmc_type);
On Wed, Nov 13, 2024 at 06:30:08AM +0100, Caleb Connolly wrote: > It may be the case that MMC support is enabled even though the board > we're booting on doesn't have any MMC devices. Move the print over to > the print_mmc_devices() function where we can only print it if we > actually have MMC devices. > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> I'm not sure I like this. What we do / don't find on startup is part of the not-exactly-API. It's true that if we don't print an MMC line at all, and we should have MMC, the user (and any scripts that parse console output) but now we're also increasing the code size a little bit too. I can be convinced this is a good idea, but I'm not there yet.
On 13/11/2024 15:24, Tom Rini wrote: > On Wed, Nov 13, 2024 at 06:30:08AM +0100, Caleb Connolly wrote: > >> It may be the case that MMC support is enabled even though the board >> we're booting on doesn't have any MMC devices. Move the print over to >> the print_mmc_devices() function where we can only print it if we >> actually have MMC devices. >> >> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > > I'm not sure I like this. What we do / don't find on startup is part of > the not-exactly-API. It's true that if we don't print an MMC line at > all, and we should have MMC, the user (and any scripts that parse > console output) but now we're also increasing the code size a little bit > too. I can be convinced this is a good idea, but I'm not there yet. Hmm, fair enough. I'll offer some more context, maybe there's a smarter approach here I'm not seeing. The main place this shows up is on Qualcomm boards. Since all Qualcomm armv8 targets are supported with qcom_defconfig (just by adjusting which DTB is used), we can't know at build time whether the board has MMC. I guess my thinking behind this patch comes from a bigger picture desire to get UFS and MMC more aligned. The number of devices with UFS is definitely going up, and I would argue that U-Boot's inconsistent treatment of these two storage classes (obviously a result of their relative age and support in the codebase) is really unintuitive and weird for users (nevermind that the "scsi" command is used for UFS devices, cute though it is). I'm really wary to open this whole can of worms, since I guess it would require some larger efforts and collaboration to fix. But maybe this patch (or one like it) would be better suited in the context of some larger effort to unify storage backends? Kind regards, >
Hi, > -----Original Message----- > From: Caleb Connolly <caleb.connolly@linaro.org> > Sent: Wednesday, November 13, 2024 2:30 PM > To: Caleb Connolly <caleb.connolly@linaro.org>; Christian Marangi <ansuelsmth@gmail.com>; Dragan Simic > <dsimic@manjaro.org>; Ilias Apalodimas <ilias.apalodimas@linaro.org>; Jaehoon Chung > <jh80.chung@samsung.com>; Jerome Forissier <jerome.forissier@linaro.org>; Jonas Karlman > <jonas@kwiboo.se>; Marek Vasut <marek.vasut+renesas@mailbox.org>; Peng Fan <peng.fan@nxp.com>; Peter > Robinson <pbrobinson@gmail.com>; Rasmus Villemoes <rasmus.villemoes@prevas.dk>; Simon Glass > <sjg@chromium.org>; Sughosh Ganu <sughosh.ganu@linaro.org>; Tom Rini <trini@konsulko.com> > Cc: u-boot@lists.denx.de > Subject: [PATCH] mmc: don't print 'MMC:' if there are no MMC devices > > It may be the case that MMC support is enabled even though the board > we're booting on doesn't have any MMC devices. Move the print over to > the print_mmc_devices() function where we can only print it if we > actually have MMC devices. > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > common/board_r.c | 1 - > drivers/mmc/mmc-uclass.c | 4 +++- > drivers/mmc/mmc_legacy.c | 5 +++++ > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/common/board_r.c b/common/board_r.c > index 62228a723e12..232a8fd19f03 100644 > --- a/common/board_r.c > +++ b/common/board_r.c > @@ -384,9 +384,8 @@ static int initr_onenand(void) > > #ifdef CONFIG_MMC > static int initr_mmc(void) > { > - puts("MMC: "); > mmc_initialize(gd->bd); > return 0; > } > #endif > diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c > index c8db4f811c2f..56fe29249c36 100644 > --- a/drivers/mmc/mmc-uclass.c > +++ b/drivers/mmc/mmc-uclass.c > @@ -384,9 +384,11 @@ void print_mmc_devices(char separator) > dev; > uclass_next_device(&dev), first = false) { > struct mmc *m = mmc_get_mmc_dev(dev); > > - if (!first) { > + if (first) { > + printf("MMC: "); > + } else { Frankly, I don't know if it needs to print "MMC; " at here. It's already knowing mmc with mmc command. In board_r, To clarify which device is showing "MMC", but not here. Best Regards, Jaehoon Chung > printf("%c", separator); > if (separator != '\n') > puts(" "); > } > diff --git a/drivers/mmc/mmc_legacy.c b/drivers/mmc/mmc_legacy.c > index 8f8ba34be717..f4a049a4a4d4 100644 > --- a/drivers/mmc/mmc_legacy.c > +++ b/drivers/mmc/mmc_legacy.c > @@ -98,8 +98,9 @@ void print_mmc_devices(char separator) > { > struct mmc *m; > struct list_head *entry; > char *mmc_type; > + bool first = true; > > list_for_each(entry, &mmc_devices) { > m = list_entry(entry, struct mmc, link); > > @@ -107,8 +108,12 @@ void print_mmc_devices(char separator) > mmc_type = IS_SD(m) ? "SD" : "eMMC"; > else > mmc_type = NULL; > > + if (first) { > + printf("MMC: "); > + first = false; > + } > printf("%s: %d", m->cfg->name, m->block_dev.devnum); > if (mmc_type) > printf(" (%s)", mmc_type); > > -- > 2.47.0
Hi, > -----Original Message----- > From: Caleb Connolly <caleb.connolly@linaro.org> > Sent: Wednesday, November 13, 2024 11:47 PM > To: Tom Rini <trini@konsulko.com> > Cc: Christian Marangi <ansuelsmth@gmail.com>; Dragan Simic <dsimic@manjaro.org>; Ilias Apalodimas > <ilias.apalodimas@linaro.org>; Jaehoon Chung <jh80.chung@samsung.com>; Jerome Forissier > <jerome.forissier@linaro.org>; Jonas Karlman <jonas@kwiboo.se>; Marek Vasut > <marek.vasut+renesas@mailbox.org>; Peng Fan <peng.fan@nxp.com>; Peter Robinson <pbrobinson@gmail.com>; > Rasmus Villemoes <rasmus.villemoes@prevas.dk>; Simon Glass <sjg@chromium.org>; Sughosh Ganu > <sughosh.ganu@linaro.org>; u-boot@lists.denx.de > Subject: Re: [PATCH] mmc: don't print 'MMC:' if there are no MMC devices > > > > On 13/11/2024 15:24, Tom Rini wrote: > > On Wed, Nov 13, 2024 at 06:30:08AM +0100, Caleb Connolly wrote: > > > >> It may be the case that MMC support is enabled even though the board > >> we're booting on doesn't have any MMC devices. Move the print over to > >> the print_mmc_devices() function where we can only print it if we > >> actually have MMC devices. > >> > >> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > > > > I'm not sure I like this. What we do / don't find on startup is part of > > the not-exactly-API. It's true that if we don't print an MMC line at > > all, and we should have MMC, the user (and any scripts that parse > > console output) but now we're also increasing the code size a little bit > > too. I can be convinced this is a good idea, but I'm not there yet. > > Hmm, fair enough. I'll offer some more context, maybe there's a smarter > approach here I'm not seeing. > > The main place this shows up is on Qualcomm boards. Since all Qualcomm > armv8 targets are supported with qcom_defconfig (just by adjusting which > DTB is used), we can't know at build time whether the board has MMC. I'm also not sure if it has to apply this at this time. Partially, I understood what you said about your case. But In my case, the printing MMC information was useful to do debug at booting time. (correct dtb or not, or wrong configuration, etc) Best Regards, Jaehoon Chung > > I guess my thinking behind this patch comes from a bigger picture desire > to get UFS and MMC more aligned. The number of devices with UFS is > definitely going up, and I would argue that U-Boot's inconsistent > treatment of these two storage classes (obviously a result of their > relative age and support in the codebase) is really unintuitive and > weird for users (nevermind that the "scsi" command is used for UFS > devices, cute though it is). > > I'm really wary to open this whole can of worms, since I guess it would > require some larger efforts and collaboration to fix. But maybe this > patch (or one like it) would be better suited in the context of some > larger effort to unify storage backends? > > Kind regards, > > > > -- > // Caleb (they/them)
On Wed, Nov 13, 2024 at 03:47:16PM +0100, Caleb Connolly wrote: > > > On 13/11/2024 15:24, Tom Rini wrote: > > On Wed, Nov 13, 2024 at 06:30:08AM +0100, Caleb Connolly wrote: > > > >> It may be the case that MMC support is enabled even though the board > >> we're booting on doesn't have any MMC devices. Move the print over to > >> the print_mmc_devices() function where we can only print it if we > >> actually have MMC devices. > >> > >> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > > > > I'm not sure I like this. What we do / don't find on startup is part of > > the not-exactly-API. It's true that if we don't print an MMC line at > > all, and we should have MMC, the user (and any scripts that parse > > console output) but now we're also increasing the code size a little bit > > too. I can be convinced this is a good idea, but I'm not there yet. > > Hmm, fair enough. I'll offer some more context, maybe there's a smarter > approach here I'm not seeing. > > The main place this shows up is on Qualcomm boards. Since all Qualcomm > armv8 targets are supported with qcom_defconfig (just by adjusting which > DTB is used), we can't know at build time whether the board has MMC. > > I guess my thinking behind this patch comes from a bigger picture desire > to get UFS and MMC more aligned. The number of devices with UFS is > definitely going up, and I would argue that U-Boot's inconsistent > treatment of these two storage classes (obviously a result of their > relative age and support in the codebase) is really unintuitive and > weird for users (nevermind that the "scsi" command is used for UFS > devices, cute though it is). > > I'm really wary to open this whole can of worms, since I guess it would > require some larger efforts and collaboration to fix. But maybe this > patch (or one like it) would be better suited in the context of some > larger effort to unify storage backends? I get it now, thanks. And yeah, this is part of our inconsistency in printing information. I'd rather leave things alone here (assuming the MMC: printout is reasonable in the case of no devices, similar to how the Net: printout is reasonable in that case) and wait for a longer term / high level solution wherein for example as we go down the device tree we give some consistent level of information for everything (and more or less info depending on verbosity configured perhaps).
Hi Tom, On 14/11/2024 19:02, Tom Rini wrote: > On Wed, Nov 13, 2024 at 03:47:16PM +0100, Caleb Connolly wrote: >> >> >> On 13/11/2024 15:24, Tom Rini wrote: >>> On Wed, Nov 13, 2024 at 06:30:08AM +0100, Caleb Connolly wrote: >>> >>>> It may be the case that MMC support is enabled even though the board >>>> we're booting on doesn't have any MMC devices. Move the print over to >>>> the print_mmc_devices() function where we can only print it if we >>>> actually have MMC devices. >>>> >>>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> >>> >>> I'm not sure I like this. What we do / don't find on startup is part of >>> the not-exactly-API. It's true that if we don't print an MMC line at >>> all, and we should have MMC, the user (and any scripts that parse >>> console output) but now we're also increasing the code size a little bit >>> too. I can be convinced this is a good idea, but I'm not there yet. >> >> Hmm, fair enough. I'll offer some more context, maybe there's a smarter >> approach here I'm not seeing. >> >> The main place this shows up is on Qualcomm boards. Since all Qualcomm >> armv8 targets are supported with qcom_defconfig (just by adjusting which >> DTB is used), we can't know at build time whether the board has MMC. >> >> I guess my thinking behind this patch comes from a bigger picture desire >> to get UFS and MMC more aligned. The number of devices with UFS is >> definitely going up, and I would argue that U-Boot's inconsistent >> treatment of these two storage classes (obviously a result of their >> relative age and support in the codebase) is really unintuitive and >> weird for users (nevermind that the "scsi" command is used for UFS >> devices, cute though it is). >> >> I'm really wary to open this whole can of worms, since I guess it would >> require some larger efforts and collaboration to fix. But maybe this >> patch (or one like it) would be better suited in the context of some >> larger effort to unify storage backends? > > I get it now, thanks. And yeah, this is part of our inconsistency in > printing information. I'd rather leave things alone here (assuming the > MMC: printout is reasonable in the case of no devices, similar to how > the Net: printout is reasonable in that case) and wait for a longer term > / high level solution wherein for example as we go down the device tree > we give some consistent level of information for everything (and more or > less info depending on verbosity configured perhaps). > Thanks for taking a look. Something like what you're suggesting sounds good to me, and I agree it's fine to leave things as-is until we have such a plan. Kind regards,
diff --git a/common/board_r.c b/common/board_r.c index 62228a723e12..232a8fd19f03 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -384,9 +384,8 @@ static int initr_onenand(void) #ifdef CONFIG_MMC static int initr_mmc(void) { - puts("MMC: "); mmc_initialize(gd->bd); return 0; } #endif diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index c8db4f811c2f..56fe29249c36 100644 --- a/drivers/mmc/mmc-uclass.c +++ b/drivers/mmc/mmc-uclass.c @@ -384,9 +384,11 @@ void print_mmc_devices(char separator) dev; uclass_next_device(&dev), first = false) { struct mmc *m = mmc_get_mmc_dev(dev); - if (!first) { + if (first) { + printf("MMC: "); + } else { printf("%c", separator); if (separator != '\n') puts(" "); } diff --git a/drivers/mmc/mmc_legacy.c b/drivers/mmc/mmc_legacy.c index 8f8ba34be717..f4a049a4a4d4 100644 --- a/drivers/mmc/mmc_legacy.c +++ b/drivers/mmc/mmc_legacy.c @@ -98,8 +98,9 @@ void print_mmc_devices(char separator) { struct mmc *m; struct list_head *entry; char *mmc_type; + bool first = true; list_for_each(entry, &mmc_devices) { m = list_entry(entry, struct mmc, link); @@ -107,8 +108,12 @@ void print_mmc_devices(char separator) mmc_type = IS_SD(m) ? "SD" : "eMMC"; else mmc_type = NULL; + if (first) { + printf("MMC: "); + first = false; + } printf("%s: %d", m->cfg->name, m->block_dev.devnum); if (mmc_type) printf(" (%s)", mmc_type);
It may be the case that MMC support is enabled even though the board we're booting on doesn't have any MMC devices. Move the print over to the print_mmc_devices() function where we can only print it if we actually have MMC devices. Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> --- common/board_r.c | 1 - drivers/mmc/mmc-uclass.c | 4 +++- drivers/mmc/mmc_legacy.c | 5 +++++ 3 files changed, 8 insertions(+), 2 deletions(-)