[U-Boot] [PATCH 1/2] nand: Introduce CONFIG_SYS_NAND_SELF_INIT

This allows a driver to run code between nand_scan_ident() and nand_scan_tail(), among other things. See the additions to doc/README.nand for details.
To allow a gradual transition, Boards that don't set CONFIG_SYS_NAND_SELF_INIT will still be initialized the old way, but new drivers should not require this, and existing drivers should be converted when convenient.
Signed-off-by: Scott Wood scottwood@freescale.com --- doc/README.nand | 62 ++++++++++++++++++++++++++++++ drivers/mtd/nand/nand.c | 96 +++++++++++++++++++++++++++++------------------ include/nand.h | 5 ++ 3 files changed, 126 insertions(+), 37 deletions(-)
diff --git a/doc/README.nand b/doc/README.nand index 023740e..04a87c9 100644 --- a/doc/README.nand +++ b/doc/README.nand @@ -120,6 +120,68 @@ Configuration Options: CONFIG_SYS_NAND_MAX_CHIPS The maximum number of NAND chips per device to be supported.
+ CONFIG_SYS_NAND_SELF_INIT + Traditionally, glue code in drivers/mtd/nand/nand.c has driven + the initialization process -- it provides the mtd and nand + structs, calls a board init function for a specific device, + calls nand_scan(), and registers with mtd. + + This arrangement does not provide drivers with the flexibility to + run code between nand_scan_ident() and nand_scan_tail(), or other + deviations from the "normal" flow. + + If a board defines CONFIG_SYS_NAND_SELF_INIT, drivers/mtd/nand/nand.c + will make one call to board_nand_init(), with no arguments. That + function is responsible for calling a driver init function for + each NAND device on the board, that performs all initialization + tasks except setting mtd->name, and registering with the rest of + U-Boot. Those last tasks are accomplished by calling nand_register() + on the new mtd device. + + Example of new init to be added to the end of an existing driver + init: + + /* + * devnum is the device number to be used in nand commands + * and in mtd->name. Must be less than + * CONFIG_SYS_NAND_MAX_DEVICE. + */ + mtd = &nand_info[devnum]; + + /* chip is struct nand_chip, and is now provided by the driver. */ + mtd->priv = &chip; + + /* + * Fill in appropriate values if this driver uses these fields, + * or uses the standard read_byte/write_buf/etc. functions from + * nand_base.c that use these fields. + */ + chip.IO_ADDR_R = ...; + chip.IO_ADDR_W = ...; + + if (nand_scan_ident(mtd, CONFIG_SYS_MAX_NAND_CHIPS, NULL)) + error out + + /* + * Insert here any code you wish to run after the chip has been + * identified, but before any other I/O is done. + */ + + if (nand_scan_tail(mtd)) + error out + + if (nand_register(devnum)) + error out + + In addition to providing more flexibility to the driver, it reduces + the difference between a U-Boot driver and its Linux counterpart. + nand_init() is now reduced to calling board_nand_init() once, and + printing a size summary. This should also make it easier to + transition to delayed NAND initialization. + + Please convert your driver even if you don't need the extra + flexibility, so that one day we can eliminate the old mechanism. + NOTE: =====
diff --git a/drivers/mtd/nand/nand.c b/drivers/mtd/nand/nand.c index d987f4c..4cf4c1c 100644 --- a/drivers/mtd/nand/nand.c +++ b/drivers/mtd/nand/nand.c @@ -23,6 +23,7 @@
#include <common.h> #include <nand.h> +#include <errno.h>
#ifndef CONFIG_SYS_NAND_BASE_LIST #define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE } @@ -31,63 +32,84 @@ DECLARE_GLOBAL_DATA_PTR;
int nand_curr_device = -1; + + nand_info_t nand_info[CONFIG_SYS_MAX_NAND_DEVICE];
+#ifndef CONFIG_SYS_NAND_SELF_INIT 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 + +static char dev_name[CONFIG_SYS_MAX_NAND_DEVICE][8]; + +static unsigned long total_nand_size; /* in kiB */ + +/* Register an initialized NAND mtd device with the U-Boot NAND command. */ +int nand_register(int devnum) +{ + struct mtd_info *mtd; + + if (devnum >= CONFIG_SYS_MAX_NAND_DEVICE) + return -EINVAL; + + mtd = &nand_info[devnum]; + + sprintf(dev_name[devnum], "nand%d", devnum); + mtd->name = dev_name[devnum]; + +#ifdef CONFIG_MTD_DEVICE + /* + * Add MTD device so that we can reference it later + * via the mtdcore infrastructure (e.g. ubi). + */ + add_mtd_device(mtd); +#endif + + total_nand_size += mtd->size / 1024;
-static const char default_nand_name[] = "nand"; -static __attribute__((unused)) char dev_name[CONFIG_SYS_MAX_NAND_DEVICE][8]; + if (nand_curr_device == -1) + nand_curr_device = devnum; + + return 0; +}
-static void nand_init_chip(struct mtd_info *mtd, struct nand_chip *nand, - ulong base_addr) +#ifndef CONFIG_SYS_NAND_SELF_INIT +static void nand_init_chip(int i) { + struct mtd_info *mtd = &nand_info[i]; + struct nand_chip *nand = &nand_chip[i]; + ulong base_addr = base_address[i]; int maxchips = CONFIG_SYS_NAND_MAX_CHIPS; - static int __attribute__((unused)) i = 0;
if (maxchips < 1) maxchips = 1; - mtd->priv = nand;
+ mtd->priv = nand; nand->IO_ADDR_R = nand->IO_ADDR_W = (void __iomem *)base_addr; - if (board_nand_init(nand) == 0) { - if (nand_scan(mtd, maxchips) == 0) { - if (!mtd->name) - mtd->name = (char *)default_nand_name; -#ifdef CONFIG_NEEDS_MANUAL_RELOC - else - mtd->name += gd->reloc_off; -#endif
-#ifdef CONFIG_MTD_DEVICE - /* - * Add MTD device so that we can reference it later - * via the mtdcore infrastructure (e.g. ubi). - */ - sprintf(dev_name[i], "nand%d", i); - mtd->name = dev_name[i++]; - add_mtd_device(mtd); -#endif - } else - mtd->name = NULL; - } else { - mtd->name = NULL; - mtd->size = 0; - } + if (board_nand_init(nand)) + return;
+ if (nand_scan(mtd, maxchips)) + return; + + nand_register(i); } +#endif
void nand_init(void) { +#ifdef CONFIG_SYS_NAND_SELF_INIT + board_nand_init(); +#else int i; - unsigned int size = 0; - for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) { - nand_init_chip(&nand_info[i], &nand_chip[i], base_address[i]); - size += nand_info[i].size / 1024; - if (nand_curr_device == -1) - nand_curr_device = i; - } - printf("%u MiB\n", size / 1024); + + for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) + nand_init_chip(i); +#endif + + printf("%lu MiB\n", total_nand_size / 1024);
#ifdef CONFIG_SYS_NAND_SELECT_DEVICE /* diff --git a/include/nand.h b/include/nand.h index d444ddc..5dd1710 100644 --- a/include/nand.h +++ b/include/nand.h @@ -30,7 +30,12 @@ extern void nand_init(void); #include <linux/mtd/mtd.h> #include <linux/mtd/nand.h>
+#ifdef CONFIG_SYS_NAND_SELF_INIT +void board_nand_init(void); +int nand_register(int devnum); +#else extern int board_nand_init(struct nand_chip *nand); +#endif
typedef struct mtd_info nand_info_t;

This driver doesn't yet make use of the added flexibility (not that that should stop anyone from converting...), but it will with the in-progress hack to support 4k-page NAND.
Signed-off-by: Scott Wood scottwood@freescale.com --- drivers/mtd/nand/fsl_elbc_nand.c | 43 +++++++++++++++++++++++++++++++++---- include/nand.h | 11 +++++++++ 2 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c index 99d1061..9076ad4 100644 --- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c @@ -22,6 +22,7 @@
#include <common.h> #include <malloc.h> +#include <nand.h>
#include <linux/mtd/mtd.h> #include <linux/mtd/nand.h> @@ -57,7 +58,6 @@ struct fsl_elbc_ctrl; /* mtd information per set */
struct fsl_elbc_mtd { - struct mtd_info mtd; struct nand_chip chip; struct fsl_elbc_ctrl *ctrl;
@@ -686,10 +686,13 @@ static void fsl_elbc_ctrl_init(void) elbc_ctrl->addr = NULL; }
-int board_nand_init(struct nand_chip *nand) +static int fsl_elbc_chip_init(int devnum, u8 *addr) { + struct mtd_info *mtd = &nand_info[devnum]; + struct nand_chip *nand; struct fsl_elbc_mtd *priv; uint32_t br = 0, or = 0; + int ret;
if (!elbc_ctrl) { fsl_elbc_ctrl_init(); @@ -702,19 +705,19 @@ int board_nand_init(struct nand_chip *nand) return -ENOMEM;
priv->ctrl = elbc_ctrl; - priv->vbase = nand->IO_ADDR_R; + priv->vbase = addr;
/* Find which chip select it is connected to. It'd be nice * if we could pass more than one datum to the NAND driver... */ for (priv->bank = 0; priv->bank < MAX_BANKS; priv->bank++) { - phys_addr_t base_addr = virt_to_phys(nand->IO_ADDR_R); + phys_addr_t phys_addr = virt_to_phys(addr);
br = in_be32(&elbc_ctrl->regs->bank[priv->bank].br); or = in_be32(&elbc_ctrl->regs->bank[priv->bank].or);
if ((br & BR_V) && (br & BR_MSEL) == BR_MS_FCM && - (br & or & BR_BA) == BR_PHYS_ADDR(base_addr)) + (br & or & BR_BA) == BR_PHYS_ADDR(phys_addr)) break; }
@@ -724,6 +727,9 @@ int board_nand_init(struct nand_chip *nand) return -ENODEV; }
+ nand = &priv->chip; + mtd->priv = nand; + elbc_ctrl->chips[priv->bank] = priv;
/* fill in nand_chip structure */ @@ -794,5 +800,32 @@ int board_nand_init(struct nand_chip *nand) } }
+ ret = nand_scan_ident(mtd, 1, NULL); + if (ret) + return ret; + + ret = nand_scan_tail(mtd); + if (ret) + return ret; + + ret = nand_register(devnum); + if (ret) + return ret; + return 0; } + +#ifndef CONFIG_SYS_NAND_BASE_LIST +#define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE } +#endif + +static unsigned long base_address[CONFIG_SYS_MAX_NAND_DEVICE] = + CONFIG_SYS_NAND_BASE_LIST; + +void board_nand_init(void) +{ + int i; + + for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) + fsl_elbc_chip_init(i, (u8 *)base_address[i]); +} diff --git a/include/nand.h b/include/nand.h index 5dd1710..8b3a1a7 100644 --- a/include/nand.h +++ b/include/nand.h @@ -24,6 +24,17 @@ #ifndef _NAND_H_ #define _NAND_H_
+#include <config.h> + +/* + * All boards using a given driver must convert to self-init + * at the same time, so do it here. When all drivers are + * converted, this will go away. + */ +#if defined(CONFIG_NAND_FSL_ELBC) +#define CONFIG_SYS_NAND_SELF_INIT +#endif + extern void nand_init(void);
#include <linux/mtd/compat.h>

On Thursday 12 January 2012 20:59:41 Scott Wood wrote:
--- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c
+#ifndef CONFIG_SYS_NAND_BASE_LIST +#define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE } +#endif
would this be better off in nand.h ? -mike

On 01/15/2012 01:29 PM, Mike Frysinger wrote:
On Thursday 12 January 2012 20:59:41 Scott Wood wrote:
--- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c
+#ifndef CONFIG_SYS_NAND_BASE_LIST +#define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE } +#endif
would this be better off in nand.h ?
I'm trying to get away from the model where the NAND subsystem pretends to know anything about how a driver talks to its hardware (except when the driver chooses to use a common NAND function that uses things like IO_ADDR_R/W). For eLBC it probably makes more sense to specify the chipselect rather than the address (we have to search for the former based on the latter), though that's a separate change that can happen on its own now that the connection to subsystem code has been severed.
-Scott

On 01/16/2012 10:51 AM, Scott Wood wrote:
On 01/15/2012 01:29 PM, Mike Frysinger wrote:
On Thursday 12 January 2012 20:59:41 Scott Wood wrote:
--- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c
+#ifndef CONFIG_SYS_NAND_BASE_LIST +#define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE } +#endif
would this be better off in nand.h ?
I'm trying to get away from the model where the NAND subsystem pretends to know anything about how a driver talks to its hardware (except when the driver chooses to use a common NAND function that uses things like IO_ADDR_R/W). For eLBC it probably makes more sense to specify the chipselect rather than the address (we have to search for the former based on the latter), though that's a separate change that can happen on its own now that the connection to subsystem code has been severed.
Also, even when there isn't a mismatch with the hardware interface, this frees up the driver to initialize in other ways, separate from a fixed list iterated over during U-Boot startup -- the addresses could come from a device tree, for example.
-Scott

On Monday 16 January 2012 11:51:14 Scott Wood wrote:
On 01/15/2012 01:29 PM, Mike Frysinger wrote:
On Thursday 12 January 2012 20:59:41 Scott Wood wrote:
--- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c
+#ifndef CONFIG_SYS_NAND_BASE_LIST +#define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE } +#endif
would this be better off in nand.h ?
I'm trying to get away from the model where the NAND subsystem pretends to know anything about how a driver talks to its hardware (except when the driver chooses to use a common NAND function that uses things like IO_ADDR_R/W). For eLBC it probably makes more sense to specify the chipselect rather than the address (we have to search for the former based on the latter), though that's a separate change that can happen on its own now that the connection to subsystem code has been severed.
so the idea would be to let CONFIG_SYS_NAND_BASE_LIST and CONFIG_SYS_NAND_BASE die for devices that could care less ? and eventually obsolete CONFIG_SYS_MAX_NAND_DEVICE ? -mike

On 01/16/2012 01:58 PM, Mike Frysinger wrote:
On Monday 16 January 2012 11:51:14 Scott Wood wrote:
On 01/15/2012 01:29 PM, Mike Frysinger wrote:
On Thursday 12 January 2012 20:59:41 Scott Wood wrote:
--- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c
+#ifndef CONFIG_SYS_NAND_BASE_LIST +#define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE } +#endif
would this be better off in nand.h ?
I'm trying to get away from the model where the NAND subsystem pretends to know anything about how a driver talks to its hardware (except when the driver chooses to use a common NAND function that uses things like IO_ADDR_R/W). For eLBC it probably makes more sense to specify the chipselect rather than the address (we have to search for the former based on the latter), though that's a separate change that can happen on its own now that the connection to subsystem code has been severed.
so the idea would be to let CONFIG_SYS_NAND_BASE_LIST and CONFIG_SYS_NAND_BASE die for devices that could care less ?
Yes.
and eventually obsolete CONFIG_SYS_MAX_NAND_DEVICE ?
This is harder, as we still have a notion of an array of enumerated NAND devices in the command line code.
-Scott

On Monday 16 January 2012 15:03:37 Scott Wood wrote:
On 01/16/2012 01:58 PM, Mike Frysinger wrote:
On Monday 16 January 2012 11:51:14 Scott Wood wrote:
On 01/15/2012 01:29 PM, Mike Frysinger wrote:
On Thursday 12 January 2012 20:59:41 Scott Wood wrote:
--- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c
+#ifndef CONFIG_SYS_NAND_BASE_LIST +#define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE } +#endif
would this be better off in nand.h ?
I'm trying to get away from the model where the NAND subsystem pretends to know anything about how a driver talks to its hardware (except when the driver chooses to use a common NAND function that uses things like IO_ADDR_R/W). For eLBC it probably makes more sense to specify the chipselect rather than the address (we have to search for the former based on the latter), though that's a separate change that can happen on its own now that the connection to subsystem code has been severed.
so the idea would be to let CONFIG_SYS_NAND_BASE_LIST and CONFIG_SYS_NAND_BASE die for devices that could care less ?
Yes.
ok ... no complaints from me then ;)
and eventually obsolete CONFIG_SYS_MAX_NAND_DEVICE ?
This is harder, as we still have a notion of an array of enumerated NAND devices in the command line code.
sure, it'll require some more plumbing to make happen -mike
participants (2)
-
Mike Frysinger
-
Scott Wood