On 16/01/2013 23:33, Eric Nelson wrote:
Hi all,
Hi Eric,
We're preparing to launch i.MX6 Solo/Dual-Lite and Dual versions of our Nitrogen6X product line.
We made a couple of stabs at patches to support these processor variants in September and October last year.
The first attempt added run-time detection using i.MX plugins: http://lists.denx.de/pipermail/u-boot/2012-September/#134442
It was soundly rejected.
The second attempt added preprocessor support to imximage in order to allow a single board definition file to be compiled for each processor variant. http://lists.denx.de/pipermail/u-boot/2012-September/#134442
We think the approach was generally agreed to based on this comment from Tom: http://lists.denx.de/pipermail/u-boot/2012-October/137611.html
And a follow-up patch from Troy: http://lists.denx.de/pipermail/u-boot/2012-October/138162.html
But things seemed to stall here: http://lists.denx.de/pipermail/u-boot/2012-November/139918.html
Right, also if the issue (where adding rule for a single SOC or SOC family) is marginal for this patchset.
Troy submitted a number of other patches, including some fixes to the DDR initialization, but they were all made based on a 'preprocessorized' version of mx6q_4x_mt41j128.cfg.
Troy and I also had some discussion surrounding **how** the preprocessor was used to define the differences between processors: http://lists.denx.de/pipermail/u-boot/2012-October/136751.html
In a nutshell, the current patch set uses a set of preprocessor macros to configure things for each. For example, this macro call in the file mx6q_4x_mt41j128.cfg will write either 0x48254A36 (Quad or Dual) or 0x3F3F3035 (Dual-Lite/Solo) to register MMDC_P1_MPWRDLCTL:
WRITE_ENTRY2(MMDC_P1_MPWRDLCTL, 0x48254A36, 0x3F3F3035)
I read the patch and your comment on ML and agree with your comment. It seems to me that it is very tricky to add this macro to make possible to have a single imximage.cfg file. And, as you remark, this will work now with these set on boards, but things can be different with new SOC version.
The concept here is that the processors use a different internal base address, and depending on this address, the macro does its work, writing on the selected address. But again, IMHO it is tricky, and it is strictly bound to i.MX6 when imximage is thought for all i.MX processors.
Let's say, instead of it, I prefer the solution using the preprocessor or as you suggest, using different .cfg files for the memory configurations.
Nobody else seemed to comment on this, and it seems pretty critical.
Can we get some feedback before we prepare V2/V3 patches?
Our expectation is that we'll submit patches for each of the following configurations of Nitrogen6X:
nitrogen6q - Dual/Quad 1GB nitrogen6q2g - Dual/Quad 2GB nitrogen6solo - Solo 512MB nitrogen6duallite - Dual Lite 512MB nitrogen6solo1G - Solo 1GB nitrogen6duallite1G - Dual Lite 1GBOur hope is that we can do this with essentially one code base but a separate .cfg file or #ifdefs for each.
I agree on this, but then do we still need WRITE_ENTRY2() ? I miss why we need if you use separate .cfg files.
A single include/config/nitrogen6x.h with #ifdefs should allow us to exclude those features (e.g. SATA) on the Solo and Dual-Lite.
Right, I think it is a good solution.
Best regards, Stefano Babic