From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mugunthan V N Date: Thu, 21 Apr 2016 11:25:40 +0530 Subject: [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass In-Reply-To: References: <1459510190-26306-1-git-send-email-mugunthanvnm@ti.com> <1459510190-26306-5-git-send-email-mugunthanvnm@ti.com> Message-ID: <57186B5C.1050506@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wednesday 20 April 2016 08:09 PM, Simon Glass wrote: > Hi Mugunthan, > > On 1 April 2016 at 05:29, Mugunthan V N wrote: >> Implement a NAND uclass so that the NAND devices can be >> accessed via the DM framework. >> >> Signed-off-by: Mugunthan V N >> --- >> 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, >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> + >> +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 >> +#include >> >> /* >> * 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