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

[Brian Norris]

On Mon, Aug 12, 2013 at 7:20 PM, Huang Shijie <b32955 at freescale.com> wrote:
> 于 2013年08月13日 09:05, Brian Norris 写道:
>
>> 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.
>
> thanks for pointing this. If the tool may check the "nand", I prefer to do
> not change the ABI.

OK. And I guess for a tool that wants to check the type for SLC vs.
MLC from userspace, they can still use ioctl(MEMGETINFO) which has
always documented the MTD_MLCNANDFLASH type (we just didn't use it
right).

>> But really, does user-space need to know SLC vs. MLC? If so, this needs
>
> I expose it to user-space only for mtd-info.
> But if these two patches will break current tools (not the mtd-info), i
> will abandon these two patches.

I think the "nand" to "SLC nand" patch should be dropped, but this
patch is a different story. Without this patch, MLC NAND will be
exported as "unknown". So you need a new patch which will combine the
'switch (mtd->type)' cases for MTD_NANDFLASH and MTD_MLCNANDFLASH.
Otherwise, your patch set leaves MLC exported as "unknown" (a
definite, undesirable breakage).

[Side note: onenand already uses MTD_MLCNANDLFASH, so this is already
exported as "unknkown" in the current upstream. I assume no one cares
about this entry for MLC onenand, if such a thing exists.]

>> 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.).

Regards,
Brian