[U-Boot] [PATCH v4 1/2] armv8/ls1043a: fixup GIC offset according to SVR and SCFG_GIC400_ALIGN[GIC_ADDR_BIT]

[york sun]

On 10/26/2016 03:39 AM, Wenbin Song wrote:
>>> +
>>> +ENTRY(smp_kick_all_cpus)
>>> +	/* Kick secondary cpus up by SGI 0 interrupt */
>>> +	mov	x29, lr			/* Save LR */
>>> +	bl	fix_gic_offset
>>> +	bl	gic_kick_secondary_cpus
>>> +	mov	lr, x29			/* Restore LR */
>>> +	ret
>>> +ENDPROC(smp_kick_all_cpus)
>>
>> Another way to do this is to implement a weak get_gic_offset function in
>> start.S to return GICD_BASE. You can implement another function here to
>> return correct offset. If you want to replace smp_kick_all_cpus, you may want
>> to keep the #if condition to check CONFIG_GICV2 or CONFIG_GICV3, in case
>> we need to debug without GIC.
>
> [wenbin] hi,  Could you explain what's the mean of "debug without GIC" ?
> 	    The HAS_FEATURE_GIC4K_ALIGN is a feature(or, to be more exact, it is a bug for ls1043a rev1.0 silicon )  only belonging to ls1043a.
>                      So if ARCH_LS1043A is selected , the CONFIG_GICV2 will be selected, too.
>
>

The weak function smp_kick_all_cpus has #if to gate the code, so it 
won't run unless either GICv2 or GICv3 is enabled. You made change to 
this function but dropped the #if condition. My concern is future SoC or 
debugging on am emulator where we disable GIC intentionally, this code 
will cause trouble. I suggest you keep the #if condition.

<snip>

	
>> If GIC_ADDR_BIT is always set for SoCs with 4K alignment, you can get
>> rid of the Kconfig option.
>
> [Wenbin]
>
> The GIC_ADDR_BIT is set or cleaned by PBI command.  It is added on rev1.1 silicon.
>

OK, so this bit is not set by hardware. Please work with design team to 
conform if you can revert the logic for backward compatibility, i.e. use 
the same bit value for rev 1.0 and rev 1.1 for 4K alignment, and a 
non-default value for 64K alignment.

York