Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Joe Perches <joe@perches.com>]

On Tue, 2015-01-27 at 13:06 +0100, Richard Weinberger wrote:
> Am 27.01.2015 um 13:02 schrieb Alexander Holler:
> > Am 27.01.2015 um 12:55 schrieb Richard Weinberger:
> >> On Tue, Jan 27, 2015 at 12:48 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> >>> It's an interesting detail, so inform the user about it.
[]
> >>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
[]
> >>> @@ -347,6 +347,10 @@ int mmc_add_card(struct mmc_card *card)
> >>>                          uhs_bus_speed_mode, type, card->rca);
> >>>          }
> >>>
> >>> +       if (mmc_can_secure_erase_trim(card))
> >>> +               pr_info("%s: card supports secure erase/trim\n",
> >>> +                       mmc_hostname(card->host));
> >>> +
> >>
> >> Please make this a sysfs attribute.
> >> Exposing such properties as log messages is horrible.
> > 
> > Look at the source at the message which is printed just before and decide which one you find more informational / useful.
> 
> -ENOPARSE

Because you didn't look at the code?

Alexander, while I kind of agree with Richard about various
properties being rightfully exposed in sysfs, it might have
made more short-term sense to extend the original line instead
of printing another new one.

Something like:
---
 drivers/mmc/core/bus.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index c5ef100..e282780 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -259,6 +259,7 @@ int mmc_add_card(struct mmc_card *card)
 	int ret;
 	const char *type;
 	const char *uhs_bus_speed_mode = "";
+	const char *secure_trim_erase = "";
 	static const char *const uhs_speeds[] = {
 		[UHS_SDR12_BUS_SPEED] = "SDR12 ",
 		[UHS_SDR25_BUS_SPEED] = "SDR25 ",
@@ -300,21 +301,26 @@ int mmc_add_card(struct mmc_card *card)
 		(card->sd_bus_speed < ARRAY_SIZE(uhs_speeds)))
 		uhs_bus_speed_mode = uhs_speeds[card->sd_bus_speed];
 
+	if (mmc_can_secure_erase_trim(card))
+		secure_trim_erase = " (secure erase/trim)";
+
 	if (mmc_host_is_spi(card->host)) {
-		pr_info("%s: new %s%s%s card on SPI\n",
+		pr_info("%s: new %s%s%s%s card on SPI\n",
 			mmc_hostname(card->host),
 			mmc_card_hs(card) ? "high speed " : "",
 			mmc_card_ddr52(card) ? "DDR " : "",
-			type);
+			type,
+			secure_trim_erase);
 	} else {
-		pr_info("%s: new %s%s%s%s%s card at address %04x\n",
+		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_ddr52(card) ? "DDR " : "",
-			uhs_bus_speed_mode, type, card->rca);
+			uhs_bus_speed_mode, type, secure_trim_erase,
+			card->rca);
 	}
 
 #ifdef CONFIG_DEBUG_FS