From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Mackerras MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Message-ID: <15540.15381.392984.428785@argo.ozlabs.ibm.com> Date: Wed, 10 Apr 2002 23:20:21 +1000 (EST) To: msokolov@ivan.Harhan.ORG (Michael Sokolov) Cc: linuxppc-dev@lists.linuxppc.org Subject: Re: CONFIG_GENERIC_PPC32 In-Reply-To: <0204062023.AA17004@ivan.Harhan.ORG> References: <0204062023.AA17004@ivan.Harhan.ORG> Reply-To: paulus@samba.org Sender: owner-linuxppc-dev@lists.linuxppc.org List-Id: Michael Sokolov writes: > Some commentary is in order on this cset. I strongly recommend that Paulus, > people interested in bi_recs enhancements, and people interested in GT-64260 > support check it out and give it a good look. I have had a quick look... > Any number of platforms can be included in the > configuration. Each is selected with a bool in config.in so the user can > include only the platform(s) that s/he needs, while distribution makers can > ship one kernel for the world. A laudable goal. I like the idea of having each platform selected with a bool rather than just having a single choice of platform. I also agree about the CONFIG_ALL_PPC name but no-one has ever been able to come up with a better name for the PReP/PMac/CHRP combination. However, there are some aspects of the way you have done your CONFIG_GENERIC_PPC32 that I don't like. In particular I don't like the list of ifdefs in setup.c. Whenever that sort of thing appears it is a sign that we need to rethink how we do things. The old drivers/net/Space.c was a classic example, and it was a mistake and it basically became unmaintainable. Fortunately we have better ways to handle that sort of thing these days. In particular we can use ELF sections in creative ways to make lists of things without having to have strings of ifdefs. In fact, any place where we have lists of things, one per platform or one per config option, is a place where we are going to potentially get maintainability problems as the number of platforms we support grows. At the moment we can't really avoid that completely in the Makefiles and config.in files. We can avoid it in C source and we can mostly avoid it in header files (but not completely at this stage). As a guideline, ultimately I want it to be the case that you can add support for a new board without changing *any* existing source files in the kernel tree, i.e. just by adding files. (That means that anything that lists platforms or config options will need to be autogenerated.) The other thing I don't like is the bi_rec changes. While I like the idea of passing in device information in bi_recs, I really don't like the use of binary tags for the various specific pieces of information that we want to specify for the different devices. This is ultimately once again a maintainability concern. Using an enumeration like that basically forces us into having a central repository for assigning the numbers and that is going to get us into problems down the track. Instead I think that both the names of the pieces of information, and the values of things like the device type, should be represented as strings. Using strings just gives us orders of magnitude more flexibility and extensibility, and is much more suitable for the sort of decentralized and distributed development that we do. Now if we are worried about space then we can get creative about how the strings are stored in the bi_recs, for instance we could store each unique string exactly once in a string table and then just use a 16-bit index in each place where we want to refer to a string. We could put the string table in a bi_rec of its own, and even gzip it if necessary. About the early_serial_init changes - using early_serial_init is nice when the serial driver is built in, but the problem is that when the serial driver is a module, we don't get given the opportunity to do the early_serial_init calls between when the module is loaded and when it runs rs_init. Otherwise I would have decreed the use of early_serial_init some time ago. :) Regards, Paul. ** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/