Re: [U-Boot] A bit about board.c, board.c
Hi Wolfgang, On Sun, Oct 23, 2011 at 1:14 AM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message CAPnjgZ3ZdkF-dnLnC8=L55ZDWznWW6wqu0eJ7puTAT=3rcm...@mail.gmail.com you wrote: My suggestion was: CONFIG_ARCH_GENERIC_BOARD - you get lib/board.c not CONFIG_ARCH_GENERIC_BOARD - you get arch/xxx/lib/board.c If we do something like this, then the other way round. There should never be any #defines needed for the standard case - i. e. for using lib/board.c, OK, makes sense. Also, ARCH_GENERIC_BOARD is not a good name as it does not really tell the reader that you are referencing to a specific source file here - most people will think you are referring to some hardware properties. OK, in my testing I have put this into config.mk in arch/xxx/config.mk: # Move to unified board system later CONFIG_SYS_LEGACY_BOARD := y We can remove it arch by arch as things move over. So far I have some tentative patches which move ARM and x86 over to a general board setup. The main issues are relocation and SPL. For relocation I feel that a C implementation makes more sense. Please scream if you disagree, but I can see no good reason so far for anything but the very last bit (where the stack pointer is adjusted) being in assembler. x86 actually does this. With ARM there are a few functions in its board.c which don't appear in other archs. Bearing in mind that I want to create a digestible patch series I am thinking of leaving the function arrays from arch/xxx/lib/board.c in these architecture-specific files for now, and just moving the common functions over to the common file. This means that init_baudrate() will be in the generic board file, for example, but board_init (which may in fact be mis-named) stays in arch/arm/lib/board.c. Both will be referenced from the function array in arch/arm/lib/board.c. The obvious step after that is to move the function arrays into the generic code, but I think it is reasonable to do that afterwards since at present it is full of #ifdefs and will only get worse when archs merge. As a later step we can examine some of the odder and more esoteric functions and see if they can be moved to board files, drivers, etc. But it is too big a step I think to do this now. This is particularly true of PowerPC which has a huge amount of stuff in there... Doing this later also reduces the potential for breakages. Also as a later step we have Graeme's initcalls. To me this offer great benefits but they are not absolutely required for this clean up. Let' s see how things look. The generic board files therefore become (unless there are objections to source files in the board/ directory): board/board_f.c board/board_r.c board/reloc.c with a small architecture-specific relocation function in: arch/xxx/lib/arch-reloc.c With this also it would be nice to standardise the link symbols a little more. Mostly they are the same but there are variations. I plan to add something like include/asm-generic/link_symbols.h to declare these for C code in one place. One annoying thing is that people use .lds files to add additional code (other than start.S) at the start of the image. It would be nice to have a Makefile variable which collects these files as the build proceeds instead of putting then in the .lds file (one less reason to even need a board-specific .lds file). My plan at present is to send several small patch series over the next month or two in this sequence: - ARM .lds clean-up (sent, although I have a few more cleans-ups for it and it is missing SPL) - Very simple general relocation framework (with link symbol stuff) - Generic board support (but with no archs using it) - Move ARM over and clean up dead code - Move x86 over - Move PPC over The benefits of this effort will be: - one copy of most of the various functions in arch/xxx/lib/board.c instead of 10 - reduced assembler and code duplicate in start.S (I am thinking of ARM here) - naturally bring less-developed archs closer to the functionality of more-developed Regards, Simon Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de My name is Linus Torvalds, you messed with my kernel, prepare to die - Linus Torvalds in pine.lnx.3.91.960426110644.24860i-100...@linux.cs.helsinki.fi ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] A bit about board.c, board.c
Hi Simon, Le 22/10/2011 18:40, Simon Glass a écrit : Hi Albert, On Sat, Oct 22, 2011 at 12:17 AM, Albert ARIBAUD albert.u.b...@aribaud.net wrote: Le 22/10/2011 07:11, Simon Glass a écrit : Hi, Each architecture has its own board.c but they are mostly quite similar. New features such as fdt, boot timing, trace, profiling, etc. must be done separately for each arch, even if there are no architecture-specific bits. What would you say to adding something like lib/board.c which is a simplified cleaned-up copy of one of the existing board.c files, and a CONFIG_ARCH_GENERIC_BOARD option to select that in preference to the architecture-specific one. Then perhaps people could try it out and we could slowly move to it over time... I'd say anything that factorizes ARM code is a good thing. :) However I'm not in favor of a CONFIG option that would force the board code to provide some functions just in odrer to override one, not if weak My suggestion was: CONFIG_ARCH_GENERIC_BOARD - you get lib/board.c not CONFIG_ARCH_GENERIC_BOARD - you get arch/xxx/lib/board.c definitions can do a finer job. I am looking into that already for cache management function specialization, and it seems like there is the same kind of possibility here, with lib functions defined weak and the board code overriding the ones it deems necessary to. ok. Just one little point. Weak symbols are the punishment our C++ overlords gave us for not having to understand what code is executed when we instantiate a C++ subclass. It is nice to know what code is being linked in and avoid having to disassemble to find out. I think using weak symbols to avoid a OBJS-$(CONFIG_ARCH_GENERIC_BOARD) in a couple of Makefiles risks going too far. The problem I see with a config option is that it is all-or-nothing: either all cache functions are defined at the lib level, or none is. I want a finer-grain, function-by-function, approach. For the moment I think weak symbols are a nice solution, although I do recognize that it will be less visible than configuration options. Regards, Simon Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] A bit about board.c, board.c
Dear Simon Glass, In message CAPnjgZ3ZdkF-dnLnC8=L55ZDWznWW6wqu0eJ7puTAT=3rcm...@mail.gmail.com you wrote: My suggestion was: CONFIG_ARCH_GENERIC_BOARD - you get lib/board.c not CONFIG_ARCH_GENERIC_BOARD - you get arch/xxx/lib/board.c If we do something like this, then the other way round. There should never be any #defines needed for the standard case - i. e. for using lib/board.c, Also, ARCH_GENERIC_BOARD is not a good name as it does not really tell the reader that you are referencing to a specific source file here - most people will think you are referring to some hardware properties. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de My name is Linus Torvalds, you messed with my kernel, prepare to die - Linus Torvalds in pine.lnx.3.91.960426110644.24860i-100...@linux.cs.helsinki.fi ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] A bit about board.c, board.c
Le 22/10/2011 07:11, Simon Glass a écrit : Hi, Each architecture has its own board.c but they are mostly quite similar. New features such as fdt, boot timing, trace, profiling, etc. must be done separately for each arch, even if there are no architecture-specific bits. What would you say to adding something like lib/board.c which is a simplified cleaned-up copy of one of the existing board.c files, and a CONFIG_ARCH_GENERIC_BOARD option to select that in preference to the architecture-specific one. Then perhaps people could try it out and we could slowly move to it over time... I'd say anything that factorizes ARM code is a good thing. :) However I'm not in favor of a CONFIG option that would force the board code to provide some functions just in odrer to override one, not if weak definitions can do a finer job. I am looking into that already for cache management function specialization, and it seems like there is the same kind of possibility here, with lib functions defined weak and the board code overriding the ones it deems necessary to. I will look into it a bit more this morning. Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] A bit about board.c, board.c
Hi Albert, On Sat, Oct 22, 2011 at 12:17 AM, Albert ARIBAUD albert.u.b...@aribaud.net wrote: Le 22/10/2011 07:11, Simon Glass a écrit : Hi, Each architecture has its own board.c but they are mostly quite similar. New features such as fdt, boot timing, trace, profiling, etc. must be done separately for each arch, even if there are no architecture-specific bits. What would you say to adding something like lib/board.c which is a simplified cleaned-up copy of one of the existing board.c files, and a CONFIG_ARCH_GENERIC_BOARD option to select that in preference to the architecture-specific one. Then perhaps people could try it out and we could slowly move to it over time... I'd say anything that factorizes ARM code is a good thing. :) However I'm not in favor of a CONFIG option that would force the board code to provide some functions just in odrer to override one, not if weak My suggestion was: CONFIG_ARCH_GENERIC_BOARD - you get lib/board.c not CONFIG_ARCH_GENERIC_BOARD - you get arch/xxx/lib/board.c definitions can do a finer job. I am looking into that already for cache management function specialization, and it seems like there is the same kind of possibility here, with lib functions defined weak and the board code overriding the ones it deems necessary to. ok. Just one little point. Weak symbols are the punishment our C++ overlords gave us for not having to understand what code is executed when we instantiate a C++ subclass. It is nice to know what code is being linked in and avoid having to disassemble to find out. I think using weak symbols to avoid a OBJS-$(CONFIG_ARCH_GENERIC_BOARD) in a couple of Makefiles risks going too far. Regards, Simon Regards, Simon Regards, Simon I will look into it a bit more this morning. Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] A bit about board.c, board.c
Hi Simon, On 23/10/11 03:36, Simon Glass wrote: Hi Graeme, Did you mean to send to list? Oops, yes I did - Done now On Fri, Oct 21, 2011 at 10:49 PM, Graeme Russ graeme.r...@gmail.com wrote: Hi Simon On Oct 22, 2011 4:11 PM, Simon Glass s...@chromium.org wrote: Hi, Each architecture has its own board.c but they are mostly quite similar. New features such as fdt, boot timing, trace, profiling, etc. must be done separately for each arch, even if there are no architecture-specific bits. Yes, it is a mess - something my unit sequence patches tried to clean up What would you say to adding something like lib/board.c which is a simplified cleaned-up copy of one of the existing board.c files, and a 100% agree CONFIG_ARCH_GENERIC_BOARD option to select that in preference to the architecture-specific one. Then perhaps people could try it out and we could slowly move to it over time... I vote to pick an arch, convert it to a unified style and move it to lib/board.c and then merge each arch over. This should be done in a single series rather than the ol' 'migrate over time' which never happens. I thing you mean merge each arch over in its own series? x86 is a good arch to start with because the number of boards is so small (1) Also, I'd personally like the init sequence patches I posted earlier to be re-examined I already examined and thought it was good. Let's be careful to keep it simple in the sense that we only need a very small number of init functions. Most of the board_init_r() code should not be there as I understand it (e.g. on ARM MMC, USB, NAND should be inited if/when used and not before). Need to be careful not to confuse this bit with driver init or any refactor of the driver model. So we have things like - init memory and make it so we can relocate code, etc. (this is called from start.S at present) - init the CPU core - arch init like turn off caches, MMU, flush TLBs, etc. - early board init (hopefully just requires an initcall in board code if needed) - the current init sequence like banner, serial, etc. - relocate - console init - board_init (initcall in board code if needed) - (hopefully all other post-relocation init can be punted) So board.c becomes a few functions and about a dozen initcalls. Albert will want to use weak symbols instead of #ifdef, and we will be done. Regards, Simon Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] A bit about board.c, board.c
Hi Simon, On 23/10/11 08:41, Graeme Russ wrote: Hi Simon, On 23/10/11 03:36, Simon Glass wrote: Hi Graeme, [snip] I vote to pick an arch, convert it to a unified style and move it to lib/board.c and then merge each arch over. This should be done in a single series rather than the ol' 'migrate over time' which never happens. I thing you mean merge each arch over in its own series? I mean merge all arches in one series x86 is a good arch to start with because the number of boards is so small (1) Also, I'd personally like the init sequence patches I posted earlier to be re-examined I already examined and thought it was good. Let's be careful to keep it simple in the sense that we only need a very small number of init functions. Most of the board_init_r() code should not be there as I understand it (e.g. on ARM MMC, USB, NAND should be inited if/when used and not before). Need to be careful not to confuse this bit with driver init or any refactor of the driver model. So we have things like My biggest gripe with the init code is the lack of consistency. There are two prime examples of this: 1) The pre-relocation sequence and post-relocation sequences can be implemented using identical code (a sequence loop) - x86 almost gets there, other arches only loop for one, and have sequential logic for the other 2) Even with the init loop, there is a lot of sequential code after the loop that could be merged into the loop - init memory and make it so we can relocate code, etc. (this is called from start.S at present) - init the CPU core - arch init like turn off caches, MMU, flush TLBs, etc. - early board init (hopefully just requires an initcall in board code if needed) - the current init sequence like banner, serial, etc. - relocate - console init - board_init (initcall in board code if needed) - (hopefully all other post-relocation init can be punted) So board.c becomes a few functions and about a dozen initcalls. Albert will want to use weak symbols instead of #ifdef, and we will be done. The INIT_CALL approach eliminated this need - If the .c file does not get compiled in, the INIT_CALL does not get included. And adding a new INIT_CALL was as simple as including the macro in the .c file without needing to mess with board.c at all - simple :) INIT_CALL is a completely separate issue to what you are looking at and would be easier to implement after board.c was merged into lib Regard, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
