Le 05/08/2010 15:02, Wolfgang Denk a écrit : > Dear Rogan Dawes, > > In message<4c5ab1a4.3000...@dawes.za.net> you wrote: >> This may be a stupid comment, but from my perspective implementing >> Albert's orion5x changes for my DNS323, all I am doing is copying a lot >> of what Albert is doing for the edminiv2 verbatim. >> >> Would it not make sense perhaps to define defaults in a SoC config file, >> and then allow them to be overridden as required for each specific board? > > That would make sense, but quite often you don't know what will be > common code and what not when writing the first version of such code. > > I recommend that you discuss with Albert (as part of the review > process here) what should be handled as common driver code that you > can easily reuse. > > It is pretty likely that your copied code would not be accepted for > mainline, and you will be requested to factor out common parts when > you submit it (that's quite often the fate for the second or third in > such a row). > > Best regards, > > Wolfgang Denk
Rogan, As Wolfgang points out, we need to know which code can be common and which cannot. IIRC, you are creating a new board for dns323, so you most probably copied the edminiv2 board code (board/LaCie/edminiv2/) and the edminiv2 config (include/configs/edminiv2). As for configurations, I think they should be kept separate -- unless Wolfgang wants to do something similar to what is happening at the moment with ARM defconfigs in the Linux kernel, but he showed no sign of it. :) As for the edminiv2 board code, it only provides three functions, board_flash_get_legacy(), board_init() and reset_phy(). We know that flash_get_legacy() cannot be shared (we both have non-CFI compliant flash chips, but they're different). Granted, we could rewrite it as a common/ function and keep the board-specifics in CONFIG_FLASH_LEGACY_xxxx defines (or a struct), but I don't see a benefit in code size, simplicity or clarity in this. We could try to commonalize board_init(), which should only differ in machine type, but it is very small, so I don't see a great benefit in factorizing, and if any initialization is to be added later on to one board, we'd have to revert somehow. What we could share is reset_phy() if your board has an MV88E1116 Ethernet phy; then you could move reset_phy() to a driver in net/ that would be built conditionally to e.g. CONFIG_PHY_MV88E1116, and then you would add the corresponding #define to the edminiv2 and dns323 config files. The rest of my devs (gbe, ide) is only a matter of setting the right configuration. Granted, that makes our config files more similar, but as I said, just because they are similar is not enough of a reason to factorize these IMO. Rogan, what other code do you think you'd be duplicating? Amicalement, -- Albert. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
