From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Nelson Date: Tue, 12 Nov 2013 07:34:13 -0700 Subject: [U-Boot] [PATCH RFC] imx: add multi-architecture README In-Reply-To: <20131112190558.1ba54c6f@triceratops> References: <1384027962-11556-1-git-send-email-eric.nelson@boundarydevices.com> <20131111200305.72372960@triceratops> <5280F3C8.2060105@boundarydevices.com> <20131112190558.1ba54c6f@triceratops> Message-ID: <52823C65.9080300@boundarydevices.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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