Re: [PATCH v2] ARM: imx: add support for TQMa6UL SoM on MBa6ulxl

[Rouven Czerwinski <r.czerwinski@pengutronix.de>]

Hi Trent,

On Thu, 2021-06-17 at 17:03 -0700, Trent Piepho wrote:
> On Thu, Jun 17, 2021 at 2:25 AM Rouven Czerwinski
> <r.czerwinski@pengutronix.de> wrote:
> > +
> > +static void setup_uart(void)
> > +{
> > +       void __iomem *iomuxbase = (void *)MX6_IOMUXC_BASE_ADDR;
> > +
> > +       imx6_ungate_all_peripherals();
> > +
> > +       imx_setup_pad(iomuxbase, MX6_PAD_UART1_TX_DATA__UART1_DCE_TX);
> > +       imx_setup_pad(iomuxbase, MX6_PAD_UART1_RX_DATA__UART1_DCE_RX);
> 
> From what I can tell, imx_setup_pad() will program the conf register
> with 0.  Not a more typical value like 0x1b0b1 that is in most dts
> files.  Does this actually work?  It seems like one needs to OR in the
> pad conf value to the pinmux config constant to actually program this
> correctly.

No, while imx_setup_pad can include the pad control, the mux defines
above do not include one and in this case the pad configuration is not
written (iomux-v3.h:137). So the imx_setup_pad above only touches the
mux configuration, but keeps the pad configuration as is.

> 
> > +       imx6_uart_setup((void *)MX6_UART1_BASE_ADDR);
> > +       pbl_set_putc(imx_uart_putc, (void *)MX6_UART1_BASE_ADDR);
> > +
> > +       pr_debug("\n");
> 
> Do you actually get the output from this?  I'm trying to add a new
> imx6ul board and it seems like one needs a delay between the uart
> setup and the uart actually working to output data.  Or perhaps this
> is just my board.

Yes, on my board this adds a newline within the PBL console.

> 
> > +static void noinline start_mba6ulx(void)
> > +{
> > +       setup_uart();
> 
> Why setup uart when LL_DEBUG is not enabled?  This dts should have
> this working without any extra setup.

Because this boards supports the PBL console and setup_uart is required
for the PBL console to work, since the PBL does not parse this
information from the DT.

> 
> > +
> > +       imx6ul_barebox_entry(__dtb_z_imx6ul_mba6ulx_start);
> > +}
> > +
> > +ENTRY_FUNCTION(start_imx6ul_mba6ulx, r0, r1, r2)
> > +{
> > +
> > +       imx6ul_cpu_lowlevel_init();
> > +
> > +       arm_setup_stack(0x00910000);
> > +
> > +       if (IS_ENABLED(CONFIG_DEBUG_LL)) {
> > +               imx6_uart_setup_ll();
> 
> Don't you need the muxing and other code in setup_uart() for this to work?

No, conveniently the board uses the default configuration for UART one.
The muxing setup for the PBL console could be removed. Sascha, what do
you think?


-- 
Pengutronix e.K.           | Rouven Czerwinski          |
Steuerwalder Str. 21       | http://www.pengutronix.de/ |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0   |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox