Le 05/08/2010 15:02, Wolfgang Denk a écrit :
Dear Rogan Dawes,
In message4C5AB1A4.3000409@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,