On Wed, 2011-01-26 at 10:13 +0200, Mike Rapoport wrote: > On 01/26/11 00:24, Peter Tyser wrote: > >>>>>> As I've already pointed out (1) this covers only one possibility of > >>>>>> UART pin > >>>>>> muxing options. I agree that it makes sense when this is a part of the > >>>>>> board-specific code. However, forcing specific pins configuration in > >>>>>> the generic > >>>>>> driver seems problematic to me, even if most Tegra2 boards use the same > >>>>>> configuration. > >>>>>> As a side note, I'd prefer to have all the pin multiplexing defined in > >>>>>> one place > >>>>>> in board file rather than scattered among different drivers. > >>>>>> > >>>>> Alright. I'd like to get this wrapped up and accepted before the merge > >>>>> window > >>>>> closes so I can get on with the important bits (drivers, etc.). What > >>>>> would you like > >>>>> here? A generic function like 'pinmux_set_config(reg, val, bit)' that > >>>>> lives in the board > >>>>> files and gets called from the driver code with specifics of that > >>>>> board's/drivers pinmux > >>>>> config? Or do you see the pinmux code living entirely in the board > >>>>> files, and being done > >>>>> as part of board init? That's where it was before, BTW. > >>>> > >>>> I think that the pinmux code should live entirely in the board file and > >>>> performed as part of board init. The drivers than can assume that the > >>>> pins are > >>>> configured properly. > >>> OK. I'll move the clock/pinmux init code into armv7/tegra/board.c > >>> (since it's SoC-centric), > >>> and call it during board_init(). > >> Actually, I think it makes more sense to move pinmux_init_uart and > >> clock_init_uart to board/nvidia/common/board.c. > >> These routines are more board-specific than SoC-specific - they depend > >> on how the UART is routed on a board. > >> Let me do that & gen up a new patchset. > > > > You previously mentioned that "To date, all of our Tegra boards use > > these pinmux options for both UARTs. If a board vendor chooses to use > > different pinmuxes, then they can override these funcs in their board > > files, or use their own code triggered by their own defines. But > > according to our HW guys, the vast majority will use these pins" > > > > If the vast majority of boards really will use the same pinmuxing, it > > would be nice to provide that common muxing as a default which can be > > overridden. Perhaps moving pinmux_init_uart() and uart_clock_init() > > into armv7/tegra/*, and making them weak functions would make everyone > > happy. > > My point was that pin muxing belongs to the board code rather than to the > driver. Driver should just assume that pins are configured elsewhere and it > does > not need to deal with pin muxing at all.
I understand your point, but think if 9/10 boards use the same pin muxing its good to provide a default so we don't have 9 chunks of duplicate code floating around in board/*. What's the harm in providing a default? It can be overridden if needed. > Moreover, I'd prefer to see pinmux_board_init or something similar that > configures all the pins at once rather than collection of pinmux_init_uart, > pinmux_init_sdmmc, pinmux_init_gmi etc that will grow as more drivers are > added. I can see that point but its a different discussion. I don't know enough about the Tegra2 to comment on this, but it seems like a good idea based on previous experiences with boards with similar pinmuxing (eg mpc8260). In my last email I mentioned a table-driven approach (similar to the mpc8260 implementation?) which sounds somewhat like you're proposing. Best, Peter _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot