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 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to