[U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM

[Tapani <tapani@technexion.com>]

Stefano,

On Fri, 30 Aug 2013 16:43:28 +0200 Stefano Babic <sbabic@denx.de> wrote:

> Hi Tapani,
> > 
> > On some things we probably need some clarification, see inlined responses
> > to some of your questions.
> > 
> 
> I hope I can help you.
> 

Well, we start to get it. Correct us if we are wrong.

> 
> Ma main concern iy your patchset is that the tables must be maintained
> and duplicated in the source code. However, on the board if a pin is
> dedicated to a specific function, it will be dedicated for the same
> function even with the other variants of the processor.
> 

We agree fully. Again, no need to beat a dead horse. :-)

The reason our patch used duplicate tables was because of the way our original
idea was received on the u-boot mailing list. It had the definitions just once.

> Using C structure in u-boot is a strict rule - if you see, all code is
> done in this way. No new code is accepted with base + offset syntax.
> 

Are the strict rules written down somewhere, so people less involved in
u-boot development can read them before submitting?

We both know it is not valid C to use structs the way you want us to. 
It is a quirk of the compiler that it works at all. The coding standard 
for u-boot sounds to be very picky to adhere to the C standard.

Afair, Microsoft shot themselves in the foot badly assuming structs could be 
used this way in early versions of MS Office. (Wrote structs to files, but 
new versions could not load since the memory layout was different).

Sure, we might do it for the mmdc. But long term maintainable, it is not.

> > * It is a lot of effort to do struct accessors for huge tables. Both
> > of IOMUX and MMDC are large (offsets of 0x800+).
> 
> You forget that for iomuxc the job was already done - there is structure
> and functions to setup the pinmux.
> 

You would not accept code using the current iomux structure...

> > Having struct
> > accessors would take quite long to enter manually (two tables of 500+ entries 
> > each, and different between cpu types). This would be hours, if not a day of
> > braindead work without any tangible benefit.
> > 
> 
> Sorry, I see benefits in terms of readability and maintenability of the
> source code and it makes easier to add new boards. This is the reason
> why there are accessors for iomuxc(), as well as for most SOC's internal
> controller.
> 

If making the addition of new boards easier is a goal, there are other parts 
of the process that are currently a greater hurdle than writing the code. :-)

> > I could make those tables of { offset, value } and do the writels using
> > a for-loop, but that would just mariginally improve on the ugliness.
> 
> It is not what I meant - if we see the code, we can recognize the
> sequence how the DDR3 must be programmed. We can have a function
> realizing the logic (that is, wehich registers in which sequence) must
> be written, and taking as arguments the parameters (calibration, and so
> on) that for each chip are different. In other words, I would like that
> some kind of function will be moved into common code, and not here in
> board code.
>

To summarize, we are expected to:
(i) Create a more general DDR3 API for IMX6, to setup memory chips on any board?
(ii) Use the above API to redo our already working DDR setup for our board.
(iii) Rewrite the iomux struct(s) to more accurately reflect the iomux memory space. 
There is more than plain pinmuxing there. 
(iv) Rewrite any code that gets broken from changing the iomux struct(s)
(v) Use the new struct(s) in setting up memory for our board

Some of the above might need to be done differently for different cpu variants.

We are worried that we might not familiar enough with u-boot development to get 
such changes accepted in reasonable time.

Did we understand this correctly? 

regards,

Tapani