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

[Troy Kisky <troy.kisky@boundarydevices.com>]

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.

>
>>   arch/arm/cpu/armv7/mx6/Makefile          |    5 +-
>>   arch/arm/cpu/armv7/mx6/plugin.S          |  164 ++++++++++++++++++++++++++++++
>>   arch/arm/include/asm/arch-mx6/imx-regs.h |    1 +
>>   3 files changed, 169 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/arm/cpu/armv7/mx6/plugin.S
>>
>> diff --git a/arch/arm/cpu/armv7/mx6/Makefile b/arch/arm/cpu/armv7/mx6/Makefile
>> index cbce411..b1fce4e 100644
>> --- a/arch/arm/cpu/armv7/mx6/Makefile
>> +++ b/arch/arm/cpu/armv7/mx6/Makefile
>> @@ -33,11 +33,14 @@ SOBJS   = lowlevel_init.o
>>   SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
>>   OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
>>   
>> -all:	$(obj).depend $(LIB)
>> +all:	$(obj).depend $(LIB) plugin.bin
>>   
>>   $(LIB):	$(OBJS)
>>   	$(call cmd_link_o_target, $(OBJS))
>>   
>> +plugin.bin: plugin.o
>> +	$(OBJCOPY) -O binary --gap-fill 0xff $< $@
> If we add a plugin mechanism, we can have several plugins (booting
> directly from Net, maybe ?). We should then have a general mechanism. A
> directory "plugins" here can contain the code, and it is compiled only
> if a CONFIG_ is set or better if required from imximage.cfg

CONFIG_xx I understand, but can you describe an implementation from 
imximage.cfg?


>
>
>> +
>>   #########################################################################
>>   
>>   # defines $(obj).depend target
>> diff --git a/arch/arm/cpu/armv7/mx6/plugin.S b/arch/arm/cpu/armv7/mx6/plugin.S
>> new file mode 100644
>> index 0000000..99c6b20
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv7/mx6/plugin.S
>> @@ -0,0 +1,164 @@
>> +/*
>> + * Copyright (C) 2012 Boundary Devices Inc.
>> + *
>> + * Licensed under the GPL-2 or later.
>> + */
>> +#include <config.h>
>> +#include <asm/arch/imx-regs.h>
>> +
>> +#define HAB_RVT_ENTRY		0x98
>> +#define HAB_RVT_FAIL_SAFE_VECT	0xbc
>> +#define HAB_RVT_LOAD_DATA	0xc8
>> +
>> +#define HDR_SELF_PTR	0x14
>> +#define HDR_BOOT_DATA	0x20
>> +#define HDR_IMAGE_LEN	0x24
>> +
>> +#define L2X0_CTRL	0x100
>> +#define SCU_CONFIG	0x004
>> +
>> +/*
>> + * Disable L2 cache because ROM will turn it on when a plugin is used.
>> + * There are cache coherence problems if cache is on when Linux kernel
>> + * expects it to be off.
>> + */
>> +.macro disable_l2_cache
>> +	ldr	r1, =L2_BASE_ADDR
>> +	mov	r0, #0x0
>> +	str	r0, [r1, #L2X0_CTRL]
>> +.endm
>> +
>> +
>> +/*
>> + * plugin_start(void **start, size_t *bytes, UINT32 *ivt_offset)
>> + */
>> +plugin_start:
>> +/* Save the return address and the function arguments */
>> +	push	{r0-r8, lr}
>> +
>> +/* r0-r2 must be  >= 0x100 and must be 4 byte aligned */
>> +	cmp	r0, #0x100
>> +	cmphs	r1, #0x100
>> +	cmphs	r2, #0x100
>> +
>> +/* rCPU: 22 - mx6q, 12 - mx6dl, 12|0x100 - solo, 2 - sololite */
>> +#define rCPU	r2
>> +#define rIomux	r3
>> +#define rVal0	r4	/* mx6q value */
>> +#define rVal1	r5	/* mx6dl value */
>> +#define rVal2	r6	/* mx6solo value */
>> +#define rVal3	r7	/* mx6sololite value */
>> +#define rFlag	lr
>> +#define rTable	r8
>> +
>> +	orr	rFlag, r0, r1
>> +	orr	rFlag, rFlag, r2
>> +	orrlo	rFlag, rFlag, #1
>> +
>> +	mov	rCPU, #22		/* mx6q */
>> +	mov	r1, #SCU_BASE_ADDR
>> +	ldr	r0, [r1, #SCU_CONFIG]
>> +	and	r0, r0, #3
>> +	cmp	r0, #3			/* is mx6q? */
>> +	movne	rCPU, #12		/* mx6dl */
>> +	cmpne	r0, #1			/* is mx6dl? */
>> +	movne	rCPU, #2		/* mx6 sololite */
>> +
>> +	ldrne	r1, =ANATOP_BASE_ADDR
>> +	ldrne	r0, [r1, #0x280]
>> +	movne	r0, r0, LSR #16
>> +	cmpne	r0, #0x60		/* is mx6 Sololite? */
>> +	movne	rCPU, #12 | 0x100	/* Solo */
> Ok - until here you have checked which processor is running. Now the
> more obscure code:
>
>> +
>> +	mov	rVal0, #0
>> +	mov	rVal1, #0
>> +	mov	rVal2, #0
>> +	mov	rVal3, #0
>> +	ldr	rIomux, =IOMUXC_BASE_ADDR
>> +	adr	rTable, mx6_table
>> +	b	3f
>
>
>> +
>> +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).


>
>> +2:	ands	r0, r1, #3
>> +	bic	r1, r1, #3
>> +	ldrne	rVal0, [rTable], #4
>> +	movne	rVal1, rVal0
>> +	movne	rVal2, rVal0
>> +	movne	rVal3, rVal0
>> +	subnes	r0, r0, #1
>> +	ldrne	rVal1, [rTable], #4
>> +	movne	rVal2, rVal1
>> +	movne	rVal3, rVal1
>> +	subnes	r0, r0, #1
>> +	ldrne	rVal2, [rTable], #4
>> +	ldrne	rVal3, [rTable], #4
>> +
>> +	mov	r0, rVal0
>> +	cmp	rCPU, #22
>> +	movne	r0, rVal1
>> +	cmpne	rCPU, #12
>> +	movne	r0, rVal2
>> +	cmpne	rCPU, #12|0x100
>> +	movne	r0, rVal3
>> +	cmp	r1, #0
>> +	strne	r0, [r1]
>> +3:	ldr	r1, [rTable], #4
>> +	cmp	r1, #0
>> +	bne	1b
>> +
>> +	tst	rFlag, #3
>> +	bne	4f		/* Branch if not called as plugin */
>> +/* Align end of table to 64 byte boundary */
>> +	sub	rTable, rTable, #1
>> +	orr	rTable, rTable, #0x3f
>> +	add	rTable, rTable, #1
>> +	ldr	r2, [rTable, #HDR_SELF_PTR]
>> +	ldr	r0, [rTable, #HDR_BOOT_DATA]
>> +	ldr	r1, [rTable, #HDR_IMAGE_LEN]
>> +	sub	rTable, r2, r0
>> +	mov	r2, r0
>> +	mov	r3, r1
>> +	mov	r4, #0
>> +	push	{r0-r4}
>> +	mov	r0, #HAB_RVT_LOAD_DATA
>> +	ldr	r4, [r0]
>> +	mov	r0, sp
>> +	add	r1, sp, #4
>> +	add	r2, sp, #8
>> +	blx	r4
> Sorry, I need help to understand this code

Now that DDR is initialized, this is calling back into the ROM code
so that it can finish loading u-boot.

>
>> +
>> +	disable_l2_cache
>> +	pop	{r4, r5}
>> +	add	sp, sp, #12
>> +	pop	{r0-r3}
>> +/*
>> + * Before returning to ROM, we need to fill the return values arguments
>> + * to our function.
>> + * plugin_start(void **start, size_t *bytes, UINT32 *ivt_offset)
> As the i.MX and the API suggest, it should be possible to write C code
> for a plugin. Or am I wrong ?


I don't see why not. But this code is currently position independent. It 
would be nice to maintain that.

>
>
>> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h
>> index 8834c59..5c133b2 100644
>> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
>> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
>> @@ -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.
>
> Best regards,
> Stefano Babic
>

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