Re: [PATCH 08/10] mtd: add MTD_MLCNANDFLASH case for mtd_type_show()

[Brian Norris <computersforpeace@gmail.com>]

On Mon, Aug 12, 2013 at 01:42:51PM +0800, Huang Shijie wrote:
> The current mtd_type_show() misses the MTD_MLCNANDFLASH case.
> This patch adds the case for it.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/mtdcore.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 6aa952b..c7cee29 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -157,6 +157,9 @@ static ssize_t mtd_type_show(struct device *dev,
>  	case MTD_UBIVOLUME:
>  		type = "ubi";
>  		break;
> +	case MTD_MLCNANDFLASH:
> +		type = "MLC nand";

The current convention uses lower-case, single-word names. And your
selection isn't very consistent with the SLC NAND case (but I see that
you're trying to change the SLC NAND case). I'd go with "mlc-nand" or
just "mlc".

But more importantly, this is probably a break in ABI. At a minimum,
this needs documentation here in Documentation/ABI. I know tools which
currently check only for "nand", and if the name suddely becomes
different for MLC, that is a breakage.

But really, does user-space need to know SLC vs. MLC? If so, this needs
a clear argument for why in the patch description. Perhaps to
differentiate whether or not JFFS2 support is even possible? I'm not
opposed to adding a new name, especially since the MTD_MLCNANDFLASH
macro has existed in mtd/mtd-abi.h for a long time (but it was rotten).
Just do it sensibly (i.e., better name string, proper ABI documentation
inlcuded in this patch, explain the reason for the ABI addition in this
patch, etc.).

Brian