On Wednesday 20 April 2016 08:09 PM, Simon Glass wrote:
> Hi Mugunthan,
>
> On 1 April 2016 at 05:29, Mugunthan V N <mugunthan...@ti.com> wrote:
>> Implement a NAND uclass so that the NAND devices can be
>> accessed via the DM framework.
>>
>> Signed-off-by: Mugunthan V N <mugunthan...@ti.com>
>> ---
>> drivers/mtd/nand/Kconfig | 10 +++++++
>> drivers/mtd/nand/Makefile | 2 ++
>> drivers/mtd/nand/nand-uclass.c | 62
>> ++++++++++++++++++++++++++++++++++++++++++
>> drivers/mtd/nand/nand.c | 6 +++-
>> include/dm/uclass-id.h | 1 +
>> include/nand.h | 5 ++++
>> 6 files changed, 85 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/mtd/nand/nand-uclass.c
>>
>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>> index 7787505..53b57b8 100644
>> --- a/drivers/mtd/nand/Kconfig
>> +++ b/drivers/mtd/nand/Kconfig
>> @@ -8,6 +8,16 @@ menuconfig NAND
>>
>> if NAND
>>
>> +config DM_NAND
>> + bool "Enable driver model for NAND"
>> + depends on NAND && DM
>> + help
>> + Enable driver model for NAND. The NAND interface is then
>> + implemented by the NAND uclass. Multiple NAND devices can
>> + be attached and used. The 'nand' command works as normal.
>> +
>> + If the NAND drivers doesn't support DM, say N.
>> +
>> config SYS_NAND_SELF_INIT
>> bool
>> help
>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>> index 6fb3718..7745705 100644
>> --- a/drivers/mtd/nand/Makefile
>> +++ b/drivers/mtd/nand/Makefile
>> @@ -39,6 +39,8 @@ endif # not spl
>>
>> ifdef NORMAL_DRIVERS
>>
>> +obj-$(CONFIG_DM_NAND) += nand-uclass.o
>> +
>> obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
>>
>> obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
>> diff --git a/drivers/mtd/nand/nand-uclass.c b/drivers/mtd/nand/nand-uclass.c
>> new file mode 100644
>> index 0000000..d68d357
>> --- /dev/null
>> +++ b/drivers/mtd/nand/nand-uclass.c
>> @@ -0,0 +1,62 @@
>> +/*
>> + * NAND uclass driver for NAND bus.
>> + *
>> + * (C) Copyright 2012-2016
>> + * Texas Instruments Incorporated, <www.ti.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <nand.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#ifdef CONFIG_DM_NAND
>> +
>> +nand_info_t *get_nand_dev_by_index(int idx)
>> +{
>> + nand_info_t *nand;
>> + struct udevice *dev;
>> + int ret;
>> +
>> + ret = uclass_get_device(UCLASS_NAND, idx, &dev);
>> + if (ret) {
>> + error("NAND device (%d) not found\n", idx);
>
> This should return -ENODEV. Also please avoid printf() in drivers. The
> caller can report the error.
>
Return type is pointer so returned NULL and NULL denotes no dev.
Will change this error to debug in v2.
>> + return NULL;
>> + }
>> +
>> + nand = (nand_info_t *)dev_get_uclass_priv(dev);
>> + if (!nand) {
>> + error("Nand device not ready\n");
>> + return NULL;
>> + }
>
> But if the device has been probed ((with uclass_get_device()) then
> this cannot be NULL.
This check is just a failsafe. ideally it should not fail here.
>
>> +
>> + return nand;
>> +}
>> +
>> +static int nand_child_pre_probe(struct udevice *dev)
>> +{
>> + nand_info_t *nand = dev_get_uclass_priv(dev);
>> + void *priv = dev_get_priv(dev);
>
> Please use the actual struct here, not void.
nand->priv is a void *, so used void *
>
>> +
>> + /*
>> + * Store nand device priv pointer in nand_info so that
>> + * it can be used by nand command
>> + */
>> + nand->priv = priv;
> ()> +
>> + return 0;
>> +}
>> +
>> +UCLASS_DRIVER(nand) = {
>> + .id = UCLASS_NAND,
>> + .name = "nand",
>> + .flags = DM_UC_FLAG_SEQ_ALIAS,
>> + .post_probe = nand_child_pre_probe,
>> + .per_device_auto_alloc_size = sizeof(nand_info_t),
>> +};
>> +
>> +#endif
>> diff --git a/drivers/mtd/nand/nand.c b/drivers/mtd/nand/nand.c
>> index 524ead3..c03a7e5 100644
>> --- a/drivers/mtd/nand/nand.c
>> +++ b/drivers/mtd/nand/nand.c
>> @@ -21,7 +21,7 @@ int nand_curr_device = -1;
>>
>> nand_info_t nand_info[CONFIG_SYS_MAX_NAND_DEVICE];
>>
>> -#ifndef CONFIG_SYS_NAND_SELF_INIT
>> +#if !defined(CONFIG_SYS_NAND_SELF_INIT) && !defined(CONFIG_DM_NAND)
>> static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
>> static ulong base_address[CONFIG_SYS_MAX_NAND_DEVICE] =
>> CONFIG_SYS_NAND_BASE_LIST;
>> #endif
>> @@ -63,8 +63,10 @@ int nand_register(int devnum)
>> static void nand_init_chip(int i)
>> {
>> struct mtd_info *mtd;
>> +#ifndef CONFIG_DM_NAND
>> struct nand_chip *nand = &nand_chip[i];
>> ulong base_addr = base_address[i];
>> +#endif
>> int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
>>
>> if (maxchips < 1)
>> @@ -74,11 +76,13 @@ static void nand_init_chip(int i)
>> if (!mtd)
>> return;
>>
>> +#ifndef CONFIG_DM_NAND
>> mtd->priv = nand;
>> nand->IO_ADDR_R = nand->IO_ADDR_W = (void __iomem *)base_addr;
>>
>> if (board_nand_init(nand))
>> return;
>> +#endif
>>
>> if (nand_scan(mtd, maxchips))
>> return;
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 37c4176..6a88d39 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -49,6 +49,7 @@ enum uclass_id {
>> UCLASS_MMC, /* SD / MMC card or chip */
>> UCLASS_MOD_EXP, /* RSA Mod Exp device */
>> UCLASS_MTD, /* Memory Technology Device (MTD) device */
>> + UCLASS_NAND, /* NAND bus */
>
> Is 'bus' the right word? Perhaps 'device'?
Yeah, will fix this in v2
>
>> UCLASS_NORTHBRIDGE, /* Intel Northbridge / SDRAM controller */
>> UCLASS_PANEL, /* Display panel, such as an LCD */
>> UCLASS_PANEL_BACKLIGHT, /* Backlight controller for panel */
>> diff --git a/include/nand.h b/include/nand.h
>> index 23b2414..1580970 100644
>> --- a/include/nand.h
>> +++ b/include/nand.h
>> @@ -10,6 +10,7 @@
>> #define _NAND_H_
>>
>> #include <config.h>
>> +#include <dm.h>
>>
>> /*
>> * All boards using a given driver must convert to self-init
>> @@ -144,6 +145,7 @@ int spl_nand_erase_one(int block, int page);
>> *
>> * returns pointer to the nand device info structure or NULL on failure.
>> */
>> +#ifndef CONFIG_DM_NAND
>> static inline nand_info_t *get_nand_dev_by_index(int dev)
>> {
>> if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
>> @@ -154,5 +156,8 @@ static inline nand_info_t *get_nand_dev_by_index(int dev)
>>
>> return &nand_info[dev];
>> }
>> +#else
>> +nand_info_t *get_nand_dev_by_index(int idx);
>> +#endif
>
> Can you please document in this header file what dev_get_priv() holds
> for a NAND device, and dev_get_uclass_priv().
Will document this in v2
Regards
Mugunthan V N
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
