From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Fri, 08 Jan 2010 17:13:51 -0600 Subject: [U-Boot] [PATCH 1/3] Monahan/PXA3xx: integrate Linux NAND controller driver In-Reply-To: <1262990084.3673.23.camel@com-21> References: <1261625393.3706.53.camel@com-21> <1261625629.3706.57.camel@com-21> <20100107194405.GD28859@loki.buserror.net> <1262990084.3673.23.camel@com-21> Message-ID: <4B47BC2F.8050305@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Marcel Ziswiler wrote: > 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. There is some of this in the common NAND code, which was there when I took over the subsystem. I'd rather move in the direction of having less of it than more. I haven't seen much of this in the drivers. > I don't quite see any other practical way of > achieving such shared code where we have rather diverse requirements > otherwise. But you're not actually sharing the code. You're reusing parts of it with lots of changes (which I'm guessing aren't going to make it back into Linux) and dead code. I don't see how keeping the dead bits around helps keep things in sync. It's usually easy enough to look at conflicts and figure out whether a change needs to be brought over, and how. The information about what is different can be obtained at any point using diff. One thing that would help is to keep track in the comments of which Linux version/SHA1 it corresponds to. Note that stuff like mtd/compat.h is different, in that it's allowing some very commonly used Linux facilities to be used by the shared code as-is without needing any ifdefs in the main code. >> 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 suppose it depends what it ends up looking like -- but simply having platform code provide a struct with some timings and such seems pretty reasonable. The method of getting that data to the driver would likely be different than in Linux. >> I don't see this upstream. > > That's because linux generally uses interrupts, but I wanted pure > polling for U-Boot. Right, the implication was that unless there's a plan to support polling in Linux, this amounts to another #ifdef __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! It might have just been overlooked -- try pinging the maintainer and the MTD list. >> 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. I was thinking we could identify a subset that NAND SPL and similar things are generally going to need, and #ifndef the rest (similar to CONFIG_NS16550_MIN_FUNCTIONS). Or, at least have a NAND SPL mini-library so that there's only one duplication instead of one per board/driver/etc. At least memcpy can replace some open-coding done in existing NAND SPL code. > 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. OK, that modified nand_boot.c is what I was wondering about (other than the size). -Scott