On Thu, 2011-01-20 at 09:41 -0700, Tom Warren wrote: > On Wed, Jan 19, 2011 at 5:04 PM, Peter Tyser <pty...@xes-inc.com> wrote: > > Hi Tom, > > Some last minutes nits: > > > > It looks like some of the new functions can be declared statically. > > It'd be nice to do so where possible. > Which functions, Peter? Please point them out specifically, thanks.
Any function that won't be called from outside the scope of the file. Eg it looks like init_uart() and setup_uart() are local functions and should be static. Those are the 2 that jumped out initially, but you should review to see if there are others. <snip> > >> +/* > >> + * Routine: uart_clock_init > >> + * Description: init the PLL and clock for the UART in uart_num > >> + */ > >> +void uart_clock_init(int uart_num) > >> +{ > > > > Are all these uart functions board-specific? They look more > > CPU-specific. If that's the case they should be moved somewhere in > > arch/arm/*. Other boards that use the Tegra2 don't want to duplicate > > this code or link into Nvidia's board/nvidia directory. > It's Tegra2 SoC-specific - that's not the CPU, per se. I guess I could move > it to arch/arm/cpu/armv7/tegra2, if you think it's important enough. I think they should be moved. If they aren't, the next board vendor (eg my company) that uses the Tegra2 will copy your board.[ch] into their board/<vendor> directory and use them as a starting point, which is a large duplication of code. Moving it somewhere in arch/arm is the "right" thing to do and will make every future tegra2 board port cleaner and easier. Best, Peter _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot