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

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