[U-Boot] [PATCH v1 3/7] nand: add nand mtd concat support

[Heiko Schocher]

Hello Scott,

Am 25.05.2016 um 16:51 schrieb Scott Wood:
> On 05/25/2016 01:10 AM, Heiko Schocher wrote:
>> Hello Scott,
>>
>> Am 25.05.2016 um 07:30 schrieb Scott Wood:
>>> On 05/25/2016 12:07 AM, Heiko Schocher wrote:
>>>> add for nand devices mtd concat support. Generic MTD concat
>>>> support is already ported to mainline, and used in the cfi_mtd
>>>> driver. This patch adds it similiar for nand devices.
>>>>
>>>> Signed-off-by: Heiko Schocher <hs at denx.de>
>>>> ---
>>>>
>>>>    drivers/mtd/nand/nand.c | 29 +++++++++++++++++++++++++++++
>>>>    1 file changed, 29 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/nand/nand.c b/drivers/mtd/nand/nand.c
>>>> index 8f0a921..5413123 100644
>>>> --- a/drivers/mtd/nand/nand.c
>>>> +++ b/drivers/mtd/nand/nand.c
[...]
>>>> +
>>>>    /* Register an initialized NAND mtd device with the U-Boot NAND command. */
>>>>    int nand_register(int devnum)
>>>>    {
>>>> @@ -49,6 +56,9 @@ int nand_register(int devnum)
>>>>    	 * via the mtdcore infrastructure (e.g. ubi).
>>>>    	 */
>>>>    	add_mtd_device(mtd);
>>>> +#ifdef CONFIG_MTD_CONCAT
>>>> +	mtd_nand_list[nand_devices_found++] = mtd;
>>>> +#endif
>>>
>>> Why do we need another list of NAND devices?  Use nand_info[].
>>
>> Ah, correct ... struct nand_info_t == struct mtd_info ... Hmm...
>> why we need a "struct nand_info_t" ?
>
> It's a typedef, not a struct...  and we don't need it.  I already have a

Oh, yes, sorry.

> patch in the set I'm working on for the Linux NAND sync that removes the
> typedef, and another that converts nand_info[] into an array of pointers
> (which should make calling mtd_concat_create easier).

Thanks!

>>>> @@ -102,4 +112,23 @@ void nand_init(void)
>>>>    	 */
>>>>    	board_nand_select_device(nand_info[nand_curr_device].priv, nand_curr_device);
>>>>    #endif
>>>> +
>>>> +#ifdef CONFIG_MTD_CONCAT
>>>> +	if (nand_devices_found > 1) {
>>>> +		struct mtd_info *mtd;
>>>> +
>>>> +		/*
>>>> +		 * We detected multiple devices. Concatenate them together.
>>>> +		 */
>>>> +		sprintf(c_mtd_name, "nand%d", nand_devices_found);
>>>> +		mtd = mtd_concat_create(mtd_nand_list, nand_devices_found,
>>>> +					c_mtd_name);
>>>> +
>>>> +		if (mtd == NULL)
>>>> +			return;
>>>> +
>>>> +		if (add_mtd_device(mtd))
>>>> +			return;
>>>> +	}
>>>> +#endif /* CONFIG_MTD_CONCAT */
>>>>    }
>>>
>>> Please don't add new features that depend on the old-style NAND init.
>>
>> Hmm... my add works for the "#ifdef CONFIG_SYS_NAND_SELF_INIT" case
>> and the "else" case ... where would be the appropriate place?
>
> You're right...  For some reason I was thinking that none of this was
> called with self-init.

Ok, so I let it here.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany