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

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

[Scott Wood]

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;
 
-- 
1.7.7.rc3.4.g8d714

[U-Boot] [PATCH 2/2] nand/fsl_elbc: Convert to self-init

[Scott Wood]

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>
-- 
1.7.7.rc3.4.g8d714

[U-Boot] [PATCH 2/2] nand/fsl_elbc: Convert to self-init

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120115/af2305f2/attachment.pgp>

[U-Boot] [PATCH 2/2] nand/fsl_elbc: Convert to self-init

[Scott Wood]

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

[U-Boot] [PATCH 2/2] nand/fsl_elbc: Convert to self-init

[Scott Wood]

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

[U-Boot] [PATCH 2/2] nand/fsl_elbc: Convert to self-init

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120116/785374c1/attachment.pgp>

[U-Boot] [PATCH 2/2] nand/fsl_elbc: Convert to self-init

[Scott Wood]

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

[U-Boot] [PATCH 2/2] nand/fsl_elbc: Convert to self-init

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120116/707193e5/attachment.pgp>