Hi Tapani,
On 11/12/2013 04:05 AM, Tapani wrote:
Thank you Eric,
< snip >
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.
I think this scales linearly with the number of CPU variants.
I'm also not sure that any of the new processors will be pin-compatible, which is really the only time this effort helps.
< 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.t 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); }
I didn't catch that you were using a global (local) variable for is_mx6q so the compiler can optimize this, but I would still prefer an array and a single call to imx_iomux_v3_setup_multiple_pads().
I would also suggest re-visiting the pad setting in each block (uart, SD, i2c, etc) and consider a single block of pad setup, regardless of whether it's done with a table or individual calls.
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/...
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?).
I actually don't like our board file so much, but don't want to thrash it without a clear(er) direction.
Anyway, let's work together on this, so we can avoid the maintenance mess the iMX6 products otherwise could become.
Sounds good.
I'm hoping to get some feedback from at least Fabio and Stefano on this.
They've been notably silent, but I'm sure it's because they're busy...
Regards,
Eric