Sergei Shtylyov <[email protected]> writes: > Hello. > > Kevin Hilman wrote: > >>>>>> Either way, the lack of a complete proposal (not necessarily >>>>>> in the form of patches) makes it hard to get anywhere with >>>>>> such OMAP-L1xx discussions... >>>>>> >>>>> I think I've expressed it clear enough: common shared code is >>>>> to be moved to plat-davinci/ and OMAP-L1x support is to be >>>>> added into new mach- directory. >>>> And yet Kevin felt that was missing details (not complete)... >>>> >>>> While that sounds plausible to me at this point, it's also >>>> clear that the missing details could make a big difference. >>>> >>>> I suspect that until patches appear, discussion can get no >>>> further. Plus, if it's going to be "mach-omap-L1" it'd >>>> be good to have enough detail that the OMAP team (and RMK) >>>> can see why it should pair with "plat-davinci" instead of >>>> the more obvious "plat-omap". >>>> >>> Regarding the name >>> ------------------ >>> >>> I really don't like the omap creep that's happening. The da830 is >>> based on davinci and not at all on an omap so I'd like to keep it >>> contained as possible. TI has expressed a strong desire to have >>> both so our current plan is the rename the defconfig >>> to "da830_omapl137_defconfig" and menuconfig text options to say >>> "DA830/OMAP-L137...". I don't know if this will be acceptable to the >>> community. >>> >> >> I don't care much about the defconfig names or the Kconfig option >> names. The bigger question is what to to use as function/#define >> prefixes (current code uses DM646X_* DM355_*, DM644X_* etc.. I think >> using both will result in long names for not very much gain. >> >> Personally, if TI is OK with it, for kernel-internal names, I would >> lean towards the da8xx prefix rather thant the omap prefix since it >> would result in less confusion with the omap names. >> > > I doubt TI would agree but let's hear from them... > >>> Regarding the code split >>> ------------------------ >>> >>> I agree, I don't think there has been a convincing argument yet. >>> I'll try to make one... >>> >>> First some background. We originally put the da8xx code (what we've >>> called it but da830 may be a better name) in mach-davinci. We fought >>> with that for quite some time but all the #ifdef's and other ugly >>> things continued to add up as we learned more nits about the hardware >>> that made it more & more different than a true davinci. So, we ended >>> up splitting the code. >>> >>> The biggest drivers for splitting the code (that I remember ATM) were: >>> >>> 1) Reduce the amount of #ifdef-ery and other ugliness; however we didn't >>> split include/asm-arm/arch-davinci (but neither did omap). >>> >> >> If done right, support for multiple devices should need very minimal >> #ifdefs, if any. > > Haha. How about the "ARM #ifdef hell" -- #ifdef's around every > platform device? > >>> 2) Remove unnecessary bloat from davinci code & data laying around >>> but entirely useless to us. >>> >> >> Again, when done right the bloat can be mostly handled with __init >> data and functions. And for production, simply compile only with the >> device/board you care about. >> > > Even platform devices can be handled with __init? Have we switched > to dynamic device allocation? > >>> Some more notes: >>> >>> - We (or at least *I*) had no desire to have the same kernel binary >>> run on both a da8xx and a davinci. So, cutting out the davinci >>> runtime code & data that was wasting memory was "A Good Thing (tm)". >>> >> >> The single kernel image is a HUGE win for the maintenance of the git >> tree where a single kernel tree will have support for multiple devices >> with potentially multiple boards per device. Yes, it leads to larger >> > > Well, that explains your desire to have everything crammed into > mach-davinci/... > >> kernels and potentially slower boot-time. However the goal of the >> single-image is for development and maintenance, not necessarily for >> products. That being said, when the multiple-device support is done >> correctly, the size and performance problems disappear when you simply >> build a kernel for a single device and board. Even in multiple device >> kernels, __init data and functions allows you to recover any wasted >> memory, and conditional cpu_is_* code can be optimized away: >> > > That's simply not quite true. Platfrom devices can't be __initdata.
You're right. platform_devices can contribute to bloat, that's why many have used (and liked) the "#ifdef hell" you mentioned above. However, recently on linux-arm-kernel, there has been a strong preference for dropping the "#ifdef hell" and accepting the slight bloat. >> The current DaVinci git can build a single kernel image for dm6446, >> dm355 and dm6467. When reviewing, testing and merging patches for >> DaVinci git is a very significant savings for me to apply the patch to >> a single tree, build a single image and boot it and test it on >> multiple boards. It would be a major slowdown to have to do a kernel >> rebuild for every patch for every device. >> > > These 3 subarchs are real DaVincis with most vital devices being > common and mapped at the same address. OMAP-L1x is not. > >>> - We put the common code in plat-davinci and took the da8xx code >>> out of mach-davinci and put into a new dir, mach-da8xx. Basically >>> following the model of the omap code. >>> >>> - Split clock.c. The main reason was different base addrs for the >>> psc & pll cntrl base. >>> >> >> This type of thing is handled in various parts of current davinci git >> (and OMAP git) by using cpu_is_* macros at init time to set the >> correct base address. Then, the base address is just passed around to >> the various functions. > > We've been thru all that -- you could really have saved your time > not telling us how it's done. >> Splitting when primarly the base address is different results in >> duplicated code which is almost identical. IMHO, this becomes very >> messy to maintain. >> > > I don't see any particular mess in our code (at least after I've > spent significant time on cleaning it up :-). > And you shouldn't really make such declaration without seeing the > actual code. My comment was based on Mark's statement that the main reason was the base addresses. If the code was available to me, or if patches had been submitted for review, maybe I would see the light. >>> - Split time.c. Again different base addresses was main reason for the >>> split. >>> >> >> Use cpu_is_* at init time. >> > > Been there, done that... though with the timers the different base > addresses was not the only difference. > >>> - Entirely different dma controller. >>> >> >> Hmm, doesn't it have EDMA3, which AFAICT is basically the same block >> that is on dm355. >> > > Same as on all DaVincis. > >>> - There are probably others that I've forgotten. >>> >>> None of these by themselves are all that convincing but they add up to >>> a mess if they aren't split. Either way, we need some sort of factoring >>> out of common code so the question is, do we keep the da8xx and common >>> code in the same directory or not? >>> >> >> I will continue to argue for the same directory until pursuaded >> otherwise. >> > > We could argue to the end of the world. The difference between our > positions is that since you're the maintainer, you can enforce your > private opinion on us. Nobody is forcing you to do anything, and nobody has made any decisions. This forum is for technical discussions about the various options for adding new device support. Yes, I will make the final decision, but I am not a dictator. In fact, I am quite easily pursuaded by technical arguments as has happened many times on this list and on other lists. Hoewever, it would be more helpful if we could keep things at the technical level without the personal attacks. >> Personally, I think splitting them results in a bigger mess as it >> results in more code that is mostly the same but will evolve >> differently. >> > > I think it will be exactly opposite. E.g. with alomost all IP cores > moved in the address space,registering the platfrom devices will turn > into mega mess. In my experience, different base addresses as a result of moving IP blocks does not necessarily lead to a mess. Between OMAP2 and OMAP3 there were many things that moved. It requires a small amount of init-time code to set the base address in the platform_device before registering and that's it. >>> IMHO, there's enough complete different stuff on the da8xx that it >>> warrants its own directory (and therefore common code should go in >>> a plat-<xx> directory). Otherwise, we're just putting two different >>> things in the same bucket for no valid reason. This is debatable, >>> of course. MHO would change if there really was a strong desire to >>> have the same kernel binary run on both types of SoC's. Is there? >>> >> >> Absolutely. >> > > Here's an example of enforcing your private opinion... Yes, this is my private opinion, but I am not enforcing anything. I am attempting to share the technical arguments behind my private opinion so folks know where I'm coming from. >> As the person who has to maintain a single code base that will run on >> all these platforms, I have a *very* strong desire to have a single >> binary run across multiple SoCs. > > May be sholld also ask tbe opinion of TI and the real users? I am very eager to hear more opinions. That's what this list is all about. >> In summary, I think some of the current splitting decisions made sense >> at the time when there was a concern for minimally changing existing, >> working code. However, for getting this code upstream, there is a >> much stronger concern for good structure, code recycling and long term >> maintenace of a single code base to support multiple devices and >> boards. Unfortunately, it does indeed require more work up front, but >> in my view of things, it is very much worth it. >> > > At least MUSB-wise, that work would be largely a wasted > effort. Therefore I'm not going to participate in it... > > > WBR, Sergei Kevin _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
