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 <h...@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
>>> @@ -9,6 +9,7 @@
>>> #include <common.h>
>>> #include <nand.h>
>>> #include <errno.h>
>>> +#include <linux/mtd/concat.h>
>>>
>>> #ifndef CONFIG_SYS_NAND_BASE_LIST
>>> #define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE }
>>> @@ -30,6 +31,12 @@ static char dev_name[CONFIG_SYS_MAX_NAND_DEVICE][8];
>>>
>>> static unsigned long total_nand_size; /* in kiB */
>>>
>>> +#ifdef CONFIG_MTD_CONCAT
>>> +static int nand_devices_found;
>>> +static struct mtd_info *mtd_nand_list[CONFIG_SYS_MAX_NAND_DEVICE];
>>> +static char c_mtd_name[16];
>>> +#endif
>>
>> Why is c_mtd_name file-scope rather than local?
>
> fixed.
>
>>> +
>>> /* 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
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).
>
>>> #endif
>>>
>>> total_nand_size += mtd->size / 1024;
>>> @@ -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.
-Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
