[U-Boot] [PATCH 2/2] mx5: Use default pin initializers

[Benoît Thébaudeau]

Dear Matt Sealey,

> On Thu, Aug 16, 2012 at 5:51 PM, Benoît Thébaudeau
> <benoit.thebaudeau at advansee.com> wrote:
> > Dear Matt Sealey,
> >
> >> On Thu, Aug 16, 2012 at 3:09 PM, Benoît Thébaudeau
> >> <benoit.thebaudeau at advansee.com> wrote:
> >
> > To improve code quality... And because I have local boards that
> > also needed the
> > same functions, so it avoided more duplications.
> >
> >> For i.MX we have two iomux models and function sets available for
> >> doing identical stuff. Isn't that kind of wasteful?
> >
> > There's probably room for improvement on iomux, but we have to
> > start cleanup
> > with something. If you want, you can submit patches and rebase
> > after mine.
> 
> Done, to a degree :)

Seen it.

> > would avoid you to duplicate your patch code for all boards. These
> > are two
> > different topics, and my change comes logically first.
> 
> See patches. On Marex's recommendation I based against u-boot-imx
> master rather than trying to fit it on top of your stuff. In the end,
> I would prefer that boards specifically implement pad settings in
> their own board files rather than assuming that some common function
> in the driver will suit them; we have an MX6 board here that uses the
> SD4 pads for UART2 for debug console. This is absolutely not common
> at
> all to any other MX6 board, so it would have to do UART pad setup and
> not use the generic function.

The purpose of the functions that I added is only to handle the standard pin
initializations, i.e. with the default IOMUX functions of the pins and proper
pad settings for these functions. For non-standard settings, the boards of
course need their own code.

> With pads defined as reasonable defaults, and NEW_PAD_CTRL available
> to modify pads per-board this is a more flexible setup and exactly
> why
> it was coded this way in Linux and why Freescale seemed to see fit to
> backport it to U-Boot (and why it was accepted to mainline for MX6).
> There's still some cleanup, all the other boards, and obviously MX53
> needs to be added to make it more useful.

Sure. It's much better for code clarity, size, etc. than having tons of function
calls.

> If we wish to continue with functions like mx51_uart1_setup() or so,
> then that function can reference these values, but I think you will
> find it more likely that someone in the future modifies it to take
> reference to an iomux_v3_cfg_t array so they can configure their
> board
> properly to something that doesn't meet your assumed defaults, or
> defines the pad setup internally to their board and skips calling
> your
> setup function. Then there would be no difference between calling
> mx51_uart1_setup vs. imx_iomux_v3_setup_multiple_pads anyway..
> reducing the need for it.

Yes. I don't know if it's really worth keeping my functions with the new pin
settings definitions. That would still avoid duplicating the standard init array
but on the other hand it would also mean more function calls than with a single
board pin array. Another solution could be to #define for each i.MX the part of
the array initializer corresponding to each standard peripheral pin setup, like:

iomux-mx51.h:
#define MX51_UART1_PAD_SETUP \
        MX51_PAD_UART1_RXD__UART1_RXD, \
        MX51_PAD_UART1_TXD__UART1_TXD, \
        MX51_PAD_UART1_RTS__UART1_RTS, \
        MX51_PAD_UART1_CTS__UART1_CTS,

some board.c:
static iomux_v3_cfg_t board_pads[] = {
        MX51_UART1_PAD_SETUP,
        MX51_PAD_...,
};

imx_iomux_v3_setup_multiple_pads(board_pads, ARRAY_SIZE(board_pads));

> I kept the redefinition of the pads for UART etc, even though POR
> defaults work fine, just to keep within your assumption that we
> shouldn't trust POR defaults.. although I don't think that loading
> code via JTAG and executing it (i.e. skipping the i.MX boot ROM) is a
> very common process at all as it just flies in the face of what
> Freescale intend to be the boot process for the chip, and in any
> case,
> the defaults are part of the SoC logic (and are correct in the
> documentation as far as I have checked), not some pre-U-Boot software
> configuration. Given the dangers inherent in doing this anyway, I
> don't think it would be fitting to make sure things are configured
> how
> you want (and in any case, why would someone configure UART pads
> differently to your expectations, and then let U-Boot reconfigure
> them
> on boot?)

It was only an example. You can also have interfering test code, ROM bootloader
code, datasheet errors, etc.

Best regards,
Benoît