[U-Boot] [PATCH V2 08/21] mx6: add plugin file for use with imximage.cfg

[Stefano Babic <sbabic@denx.de>]

On 24/09/2012 22:46, Troy Kisky wrote:
> On 9/23/2012 3:17 AM, Stefano Babic wrote:
>> On 22/09/2012 04:39, Troy Kisky wrote:
>>> The "plugin" command of mkimage can take this
>>> file as an argument.
>>>
>>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>> ---

Hi Troy,

>>
>> I agree with Vikram that a better explanation of what a plugin is can
>> help to understand without reading deeply into the i.MX6 manual.
>>
>> So a "plugin" is a chunk of code that can be called directly by the
>> BootROM of i.MX processors supporting V2 version of the i.MX header.
>> In my understanding, this is supported by i.MX53, too. After the plugin
>> run, the control is returned to the BootROM.
>>
>> Now that we have some basis, why do we need this mechanism to boot this
>> board ? Is it not possible to make the same initialization directly in
>> u-boot ?
>>
>> In principle, this adds stil some code that is not so easy to maintain.
> 
> I can add to README.imximage. But I'm beginning to doubt if plugins are
> going
> to be accepted at all.

I have several doubts about using the plugin. First at all, this make
the development of new iMX completely different as the rest of U-boot.
Plugin ist a feature so strictly bounded to the Freescale's iMX. The
risk here is that U-Boot imx diverges from the other SOCs, while we are
currently in a phase where we want to uniform as much as possible the
behavior with different architectures.

I am also not convinced why introducing the plugin is strictly required.
Reading Eric's answer, I get that costs for having and maintaing a
single image U-boot for different boards overcomes the benefits, and I
fully agree with him. But reading your patch and from your explanation
of plugin code, I understand that the plugin is used to detect which CPU
is running, and then to set differently registers, because offsets are
different. So the goal is again to have a single image.

>>> +
>>> +1:    movs    r0, r1, LSR #30
>>> +    beq    2f
>>> +    mov    r1, r1, LSL rCPU
>>> +    movs    r1, r1, LSR #32-10
>>> +    addne    r1, rIomux, r1, LSL #2
>>> +    cmp    r0, #3
>>> +    subne    r0, r0, #1
>>> +    orr    r1, r1, r0
>>> +
>> The reason is to write GPR12 ? But why do we need a plugin for that ? I
>> do not understand why we cannot do it in the initialization code of the
>> SOC, as we usually do.
> 
> This is not GPR12.  The address value from the cfg file is actually 3
> addresses. One for
> mx6q, one for mx6 duallite/solo, one for mx6 sololite. Each is specified
> as a 10 bit
> field which we use as a 12 bit offset within IOMUXC_BASE_ADDR (A0/A1
> forced to 0).

Ok, thanks for explanation - I had not understood before.


>>> @@ -48,6 +48,7 @@
>>>   #define GLOBAL_TIMER_BASE_ADDR          0x00A00200
>>>   #define PRIVATE_TIMERS_WD_BASE_ADDR     0x00A00600
>>>   #define IC_DISTRIBUTOR_BASE_ADDR        0x00A01000
>>> +#define L2_BASE_ADDR                    0x00A02000
>>>   #define GPV0_BASE_ADDR                  0x00B00000
>>>   #define GPV1_BASE_ADDR                  0x00C00000
>>>   #define PCIE_ARB_BASE_ADDR              0x01000000
>>>
>> This is useful in any case. I suggest you put this define in a separate
>> patch, that can flow independently into mainline.

> Hmm, do you suggest moving the L2 disable code to another spot as well ?

Easier - I suggest you put this define in a separate patch, because it
can be accessed by other U-Boot code as well.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================