From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcel Ziswiler Date: Fri, 08 Jan 2010 23:34:44 +0100 Subject: [U-Boot] [PATCH 1/3] Monahan/PXA3xx: integrate Linux NAND controller driver In-Reply-To: <20100107194405.GD28859@loki.buserror.net> References: <1261625393.3706.53.camel@com-21> <1261625629.3706.57.camel@com-21> <20100107194405.GD28859@loki.buserror.net> Message-ID: <1262990084.3673.23.camel@com-21> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Thu, 2010-01-07 at 13:44 -0600, Scott Wood wrote: > I don't think we need this. > > In fact, it could make it harder to spot conflicts when merging new versions > of the file from linux, since patch could find the context it's looking for > in the middle of an #ifndef __U_BOOT__ section when the change really needs > to be applied to the other side of the #else. > > Plus, it makes it harder to read the code. Let's share what we reasonably > can, and get rid of the rest. OK, but it es exactly how it is done with the rest of the linux MTD stuff pulled into U-Boot. I don't quite see any other practical way of achieving such shared code where we have rather diverse requirements otherwise. > Don't undef it, just don't define it. Maybe someone wants to turn debugging > on system-wide. > > > +#ifdef DEBUG > > +#define DEBUGF(fmt, args...) printf(fmt, ##args) > > +#else /* DEBUG */ > > +#define DEBUGF(x...) > > +#endif /* DEBUG */ > > We've already got MTDDEBUG. OK, agreed. > This is marked deprecated in Linux; perhaps we should take the preferred > approach of platform-specified data here as well? Hm, I thought platform data would be an over kill for U-Boot especially if it comes down to the NAND SPL case. Will look into how I could integrate that. > I don't see this upstream. That's because linux generally uses interrupts, but I wanted pure polling for U-Boot. I actually thought about first getting some of my customization upstream but the last time I submitted a mere 2 line bug fix to this driver it got completely ignored. Don't quite know how to get any MTD stuff mainline in a timely manner! > Can we arrange to pull in the relevant functions from the rest of u-boot > rather than duplicate them? Generally yes, but that would completely bloat the NAND SPL case. OK as later targets do no more have a 4K limit but rather 16K it might actually work out. I am just fearing further dependencies. > Or try to avoid the need in the NAND loader -- for example, msleep is only > used in handling NAND_CMD_RESET, which you shouldn't need. > > In fact, I'm not sure how this driver can be used at all with NAND_SPL. The > normal nand_spl/nand_boot.c uses cmd_ctrl. More complicated controllers > that can't use cmd_ctrl generally need their own nand_spl driver (such as > nand_spl/nand_boot_fsl_elbc.c). If you have a particularly large NAND boot > region, you might be able to create a nand_spl driver that uses this driver, > but typically space is too tight. It actually works just fine and boots various PXA3xx targets with my slightly modified nand_spl/nand_boot.c which I plan to submit as soon as we agree on this driver. > You can (and should) use __raw_writel/__raw_readl in U-Boot as well. > > There is also a prototype for__raw_writesl/__raw_readsl, but the > implementation seems to be missing -- perhaps you could add it generically, > rather than just in one driver. I actually had a version doing it that way but it had some nasty further dependencies so I finally decided against doing it that way. Will look into it again. Thanks Scott! -Marcel