Hi again, Dan Malek writes: > There is lots of stuff that comes straight to me for merging into > sources.......
I know it's beyond your control; My plea was for other developers reading this to cc the mailing list when they send you patches, so we all know what's going on. > > .......... My ideal solution to the problem of supporting multiple board > > types is to move the magic numbers like the PHY interrupt pin and the > > PxPAR/PxDIR/etc values into each board-specific header file, > > I understand your point, but the other side of the argument is when you > don't keep things like this together, people don't realize how many > examples of how to do it exist. They also don't realize how their > change may affect someone else. Not sure I'm with you here, but I think we need to have a simple single solution to multiple board support, and to use it universally. If we replace all the board-specific #ifdefs with feature-specific ones defined in the board specific header files, everything related to a specific board is all kept together. I don't think I'm proposing anything new here, just a more complete use of the current scheme where each board type has its own header defining the features present on that board. This is also the approach 8xxROM/PPCBOOT uses, and it would be nice to keep them all consistent. Another example under the scheme would be to replace most of the usages of "#ifdef CONFIG_RPXCLASSIC" in fec.c with "#ifdef PHY_QS6612", and put "#define PHY_QS6612" in rpxclassic.h. At present, anyone adding a board which uses the QS6612 has to edit fec.c, even though it already supports their PHY. > > We took this approach with our new custom board, and haven't had to add a > > single #ifdef CONFIG_ourboardname outside the one which chooses our > > board-specific header in mpc8xx.h. > > I do that with lots of custom boards. Most are simple derivative of > what is already there. In fact, some have even choose the same I/O > pins for Ethernet (there aren't many options, and most have been used :-), > so we don't even need to add anything but another conditional on an > #ifdef. I'd much prefer the conditionals to test for board features rather than specific boards. That way you _don't_ have to add another conditional on an #ifdef every time you add a new board with existing features. In cases like register names and bit definitions, we can even avoid the conditionals altogether by using definitions like: /* Ethernet : PHY controls - connected to port X * * Field Value Explanation * ----- ----- ----------- * PHYPAUSE[x] 0 We don't support pause * PHYPWRDWN[x] 0 We don't support power down * PHYSLEEP[x] 0 We don't support sleep */ #define PHY_PAUSE ((ushort) 0x0800) #define PHY_PWRDWN ((ushort) 0x0200) #define PHY_SLEEP ((ushort) 0x0040) #define PHY_PAUSE_PORT im_ioport.iop_pXdat #define PHY_PWRDWN_PORT im_ioport.iop_pXdat #define PHY_SLEEP_PORT im_ioport.iop_pXdat > It's just not as easy as getting a patch and checking it in. I appreciate your efforts co-ordinating all this. One of the reasons for posting a patch is to stimulate discussion, and I'm interested in comments from other people too. Thanks, Graham -- Graham Stoney Principal Hardware/Software Engineer Canon Information Systems Research Australia Ph: +61 2 9805 2909 Fax: +61 2 9805 2929 ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/
