On Tue, 12 Apr 2011 11:44:26 +0200
Stefano Babic <sba...@denx.de> wrote:
> On 04/11/2011 09:16 PM, Scott Wood wrote:
> > This only controls the davinci driver, so it should be
> > CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE.
> >
> > Is this really board-specific?
>
> No, really not.
>
> > Does the davinci driver ever support
> > subpage?
>
> No, it does not. This problem affects all boards using the davinci NAND
> controller. At least with 850 or OMAP-l1 processors, that I have tested.
>
> >> @@ -193,7 +193,8 @@ typedef enum {
> >> && (chip->page_shift > 9))
> >>
> >> /* Mask to zero out the chip options, which come from the id table */
> >> -#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR)
> >> +#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR & \
> >> + ~NAND_NO_SUBPAGE_WRITE)
> >
> > I wonder what we really need CHIPOPTIONS_MSK for? Silently ignoring what
> > the driver asked for doesn't seem right. Can't we just OR the two
> > sources? And it would be a driver error to specify a flag that doesn't
> > make sense to be set this way (i.e. only do it with the "doesn't support
> > X" flags).
>
> Ben reports quite the same issue, as this patch mixes chip options with
> NAND controller capabilities.
>
> Probably it is better as I answered to Ben to use
> CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE in nand_base.c to switch off subpage
> access instead of setting the chip as not able to handle subpages,
> changing the NAND_CHIPOPTIONS_MSK.
Davinci-specific #defines do not belong in nand_base.c[1]. The controller
driver should be able to set "this isn't supported" options just as well as
the chip data -- I just don't think it should be limited to this specific
one.
For example, fsl_elbc_nand.c sets NAND_NO_READRDY and NAND_NO_AUTOINCR.
Before this thread, I didn't realize it they were getting ignored. Things
work anyway because the former is an optimization, and the latter is getting
forced on after the masking, for some reason -- does autoincr simply not
work? Can we remove the code? :-)
-Scott
[1] Nor should it be turned back into a non-davinci define -- what if there
are multiple NAND controllers supported, and only one requires this? It's
not so bad in U-Boot (I'd still rather avoid it though), but this approach
is not going to go over well in Linux.
How is Linux handling this?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
