public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Eric Nelson <eric.nelson@boundarydevices.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH RFC] imx: add multi-architecture README
Date: Tue, 12 Nov 2013 07:34:13 -0700	[thread overview]
Message-ID: <52823C65.9080300@boundarydevices.com> (raw)
In-Reply-To: <20131112190558.1ba54c6f@triceratops>

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/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?).
>

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

      reply	other threads:[~2013-11-12 14:34 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-09 20:12 [U-Boot] [PATCH RFC] imx: add multi-architecture README Eric Nelson
2013-11-09 20:23 ` Eric Nelson
2013-11-09 21:16   ` Wolfgang Denk
2013-11-09 22:24     ` Eric Nelson
2013-11-09 21:14 ` Wolfgang Denk
2013-11-09 22:57   ` Eric Nelson
2013-11-11 12:03 ` Tapani
2013-11-11 15:12   ` Eric Nelson
2013-11-11 17:51     ` Eric Nelson
     [not found]     ` <528124FA.6030203@boundarydevices.com>
2013-11-11 19:18       ` Eric Nelson
2013-11-11 19:36         ` Eric Nelson
2013-11-12 11:05     ` Tapani
2013-11-12 14:34       ` Eric Nelson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52823C65.9080300@boundarydevices.com \
    --to=eric.nelson@boundarydevices.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox