From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Rapoport Date: Tue, 25 Jan 2011 23:12:00 +0200 Subject: [U-Boot] [PATCH V5 2/4] serial: Add Tegra2 serial port support In-Reply-To: References: <1295651217-32421-1-git-send-email-twarren@nvidia.com> <1295651217-32421-3-git-send-email-twarren@nvidia.com> <4D3E85BD.4060201@compulab.co.il> Message-ID: <4D3F3CA0.3080909@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 01/25/11 18:50, Tom Warren wrote: > Mike, > > On Tue, Jan 25, 2011 at 1:11 AM, Mike Rapoport wrote: >> On 01/22/11 01:06, Tom Warren wrote: >>> Signed-off-by: Tom Warren >>> --- >> >> [ snip ] >> >>> +/* >>> + * Routine: pin_mux_uart >>> + * Description: setup the pin muxes/tristate values for a UART >>> + */ >>> +static void pin_mux_uart(void) >>> +{ >>> + pinmux_tri_ctlr *const pmt = (pinmux_tri_ctlr *)NV_PA_APB_MISC_BASE; >>> + u32 reg; >>> + >>> +#if CONFIG_TEGRA2_ENABLE_UARTA >>> + reg = readl(&pmt->pmt_ctl_c); >>> + reg &= 0xFFF0FFFF; /* IRRX_/IRTX_SEL [19:16] = 00 UARTA */ >>> + writel(reg, &pmt->pmt_ctl_c); >>> + >>> + reg = readl(&pmt->pmt_tri_a); >>> + reg &= ~Z_IRRX; /* Z_IRRX = normal (0) */ >>> + reg &= ~Z_IRTX; /* Z_IRTX = normal (0) */ >>> + writel(reg, &pmt->pmt_tri_a); >>> +#endif /* CONFIG_TEGRA2_ENABLE_UARTA */ >>> +#if CONFIG_TEGRA2_ENABLE_UARTD >>> + reg = readl(&pmt->pmt_ctl_b); >>> + reg &= 0xFFFFFFF3; /* GMC_SEL [3:2] = 00, UARTD */ >>> + writel(reg, &pmt->pmt_ctl_b); >>> + >>> + reg = readl(&pmt->pmt_tri_a); >>> + reg &= ~Z_GMC; /* Z_GMC = normal (0) */ >>> + writel(reg, &pmt->pmt_tri_a); >>> +#endif /* CONFIG_TEGRA2_ENABLE_UARTD */ >> >> 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. > If there are examples that you approve of in any extant U-Boot code (omap, > ti91, davinci, etc.), please point them out. I'd really like to > finalize this patch series. I think OMAP is a good example. There's set_muxconf_regs function in each board file that is responsible for configuration of all the pins. >> (1) http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/92887/focus=93233 >> >> >> -- >> Sincerely yours, >> Mike. > Thanks, > > Tom >> -- Sincerely yours, Mike.