From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Tyser Date: Thu, 20 Jan 2011 11:15:50 -0600 Subject: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support In-Reply-To: References: <1295471986-2395-1-git-send-email-twarren@nvidia.com> <1295471986-2395-2-git-send-email-twarren@nvidia.com> <1295481869.13088.71.camel@petert> Message-ID: <1295543750.13088.85.camel@petert> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Thu, 2011-01-20 at 09:41 -0700, Tom Warren wrote: > On Wed, Jan 19, 2011 at 5:04 PM, Peter Tyser 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. > >> +/* > >> + * 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/ 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