[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

Simon Glass
2016-04-20T14:39:45Z

Hi Mugunthan,

On 1 April 2016 at 05:29, Mugunthan V N <mugunthanvnm@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 <mugunthanvnm@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 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.

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

> +
> + /*
> + * 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'?

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

>
> #endif /* _NAND_H_ */
> --
> 2.8.0.rc3.9.g44915db
>

Regards,
Simon