| Message ID | 20120113015941.GB18732@schlenkerla.am.freescale.net |
|---|---|
| State | Accepted |
| Commit | c17834749279b02f31c1d9ce5ca8427b795bb90d |
| Delegated to: | Scott Wood |
| Headers | show |
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
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>
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(-)