[U-Boot] [PATCH RFC] imx: add multi-architecture README

[Tapani]

Thank you Eric,

this is a comment to your comments. See inline.

< long snip >

> >> +The header files mx6q_pins.h and mx6dls_pins consolidate
> >> +the settings through a macro providing a common name
> >> +of SD3_DAT2__USDHC3_DAT2:
> >> +
> >> +	MX6_PAD_DECL(SD3_DAT2__USDHC3_DAT2,...)
> >> +
> >> +By using the MX6_PAD_DECL macro, this can be expanded
> >> +in one of three ways, depending on the declarations of
> >> +CONFIG_MX6x by a board file. Valid options are:
> >> +
> >> +	MX6Q	- single architecture for i.MX6DQ
> >> +	MX6DL	- single architecture for i.MX6DL/S
> >> +	MX6QDL	- multi-architecture
> >> +
> >> +In the first two cases, the MX6_PAD_DECL macro will
> >> +be expanded into a declararation with the MX6_PAD_
> >> +prefix:
> >> +	MX6_PAD_name = IOMUX_PAD(...)
> >> +
> >> +In the last case, the MX6_PAD_DECL macro will be
> >> +expanded into two sets of declarations, with the
> >> +prefix MX6Q_PAD_ for the i.MX6DQ pads and the
> >> +prefix MX6DL_PAD_ for the i.MX6DLS pads.
> >> +
> >> +This is accomplished by the header file mx6-pins.h:
> >> +
> >> +	#ifdef CONFIG_MX6QDL
> >> +	enum {
> >> +		#define MX6_PAD_DECL ...
> >> +		#include "mx6q_pins.h"
> >> +		
> >> +		#define MX6_PAD_DECL ...
> >> +		#include "mx6dl_pins.h"
> >> +	};
> >> +	#elif defined(CONFIG_MX6Q)
> >> +	enum {
> >> +		#define MX6_PAD_DECL ...
> >> +		#include "mx6q_pins.h"
> >> +	};
> >> +	#elif defined(CONFIG_MX6DL)
> >> +	enum {
> >> +		#define MX6_PAD_DECL ...
> >> +		#include "mx6dl_pins.h"
> >> +	};
> >> +	#endif
> >> +

Actually, it occurred to me yesterday that this kludge maybe solves the drawback 
of our method. Having this we can use our way to setup the pinmuxing without 
having duplicate MX6Q/MX6DL defines at all!

More about that below.

> >
> > Opinion: This is a terrible macro kludge to begin with. However, I'm afraid there
> > is no solutions completely free macro hacks, but maybe we can have less of them?
> >
> Agreed, but I don't see a way around it.
> 
> If we want to declare each pad once, we either need to always name them
> differently (i.e. MX6Q_PAD_padname and MX6DL_PAD_padname) or use a macro
> to translate the names.
> 
> I think we're all in agreement that each use of a pad reference should
> be in a generic form (i.e. shouldn't specify MX6Q_ or MX6DL_) when the
> pad is independent of the model.
> 
> We have this (single-name for a pad setting regardless of CPU variant)
> now for single-model builds (with no macro-fu) precisely because we
> named the pad declarations the same (without the Q or DL suffix).
> 

And we have the same, precisely because we named them with suffices :-)

> If we try to switch to separate names (MX6Q_PAD/MX6DL_PAD), we have an
> immediate need to refactor all board files to replace the simple
> macro names to something else.
> 
> The ~20 lines of code above provide a means of backward-compatibility.
> 
> As I side benefit, I like being able to use word-selection to grab the
> entire generic pad reference such as SD2_DAT1__USHDC2_DAT1
> from this declaration:
> 
> 	MX6_PAD_DECL(SD2_DAT1__USDHC2_DAT1,...)
> 
> and paste it into the spot where it's used:
> 	MX6_PAD_DEF(SD2_DAT1__USDHC2_DAT1)
> 
> (As opposed to having to hand-edit to remove the MX6Q_PAD_ prefix
> from one of the declarations)
> 
> > Technical point: Could you clarify how this approach scales? There are still
> > new imx6 models to be released (imx6-next).
> >
> 
> It's always tough to tell until we hash through the reference manuals.
>

Maybe you misunderstood me. We both know there are many more iMX6 CPUs 
coming next year (and they are under NDA; but the public Freescale roadmap 
confirms a imx6-next line, so we can safely say that stuff is coming).

My point was that the approach we take to make multi-variant code now
should be scalable to add additional variants in the future. Assuming the
new variants are compatible enough, but maybe requiring their own padconfs.

So we have families of cpus like (just an example): { iMX6Q, iMX6D }, 
{ iMX6S, iMX6DL }, { iMX6SL }, { iMX6x1, iMX6x2 }, { iMX6x3, iMX6x4 }, ...
We should be able to support multi-variant over as many families of cpus
as possible, without having another macro-fu nightmare when we need to
support another new CPU group.

My worry was that your approach could grow in complexity when a hypothetical
iMX6X cpu is to be supported.

> > Short sighted thinking caused the need for this mess to begin with, just trying
> > to not do the same mistake again.
> >
< snip >

> >
> >> +4. IOMUX usage in board files
> >> +-----------------------------
> >> +

< snip >

> >
> > We have suggested an alternative solution, but somehow nobody seem
> > to notice. We avoid almost all the preprocessor messing, and have the
> > definitions only once. And it would scale for even more cpu types.
> > Admittedly, the drawback is duplicate padconf macro definitions
> > (or having to convert the existing boards padconfigs).
> >
> Somehow I missed it. What I recall involved duplicating code and data.
> 
> Can you explain?
> 

Ok, for the third time :-) But this time combined with some of your suggestions.

In mx6_pins.h we can do the 20 lines of macro-fu you suggested, to create
the enums for MX6Q_PAD_x + MX6DL_PAD_x or MX6_PAD_x, depending on CPUs to 
support. Or use separate includes with duplicate padconf definitions.

Anyway assume MX6Q_PAD_x and MX6DL_PAD_x definitions are in scope.


Now somewhere (board file? mx6_pins.h?) we need one helper macro:

#define IMX6_SET_PAD(p) \
	if ( is_mx6q ) \
		mxc_iomux_v3_setup_pad(MX6Q_##p); \
	else \
		mxc_iomux_v3_setup_pad(MX6DL_##p)


Using this macro a pad can be set in code, and no need for tables!
(The compiler will do a good job on that, don't worry about the resulting code)

For instance, a board file can now initialize UART1 with:

static __init void edm_cf_imx6_init_uart(void) {
        IMX6_SET_PAD( PAD_CSI0_DAT10__UART1_TXD );
        IMX6_SET_PAD( PAD_CSI0_DAT11__UART1_RXD );
        IMX6_SET_PAD( PAD_EIM_D19__UART1_CTS );
        IMX6_SET_PAD( PAD_EIM_D20__UART1_RTS );

	imx6_add_uart(0, NULL);
}

An example of this architecture is in our kernel board file:
https://github.com/TechNexion/linux/blob/imx-3.0.35-4.1.0/arch/arm/mach-mx6/board-edm_cf_imx6.c

Using this method the boardfile part contains less macro hacks, is clearer, 
no tables, and results in shorter C code than your suggestion. (Ok, the 
clearer part could be an opinion.)

Actually, I am not sure this is mutually exclusive with your suggestion.
(Since we suspect you like your board file as much as we like ours, maybe 
it is better to not mandate how board files should do their pinmuxing?).


Anyway, let's work together on this, so we can avoid the maintenance mess the 
iMX6 products otherwise could become.


regards,

//Tapani