[U-Boot] [PATCH 3/3][v3] [RESEND] arm: ls1046ardb: Add SD secure boot target

[york sun]

On 04/04/2017 12:06 AM, Ruchika Gupta wrote:
>> -----Original Message-----
>> From: york sun
>> Sent: Monday, April 03, 2017 9:01 PM
>> To: Ruchika Gupta <ruchika.gupta at nxp.com>; u-boot at lists.denx.de
>> Cc: Vini Pillai <vinitha.pillai at nxp.com>; Sumit Garg <sumit.garg at nxp.com>
>> Subject: Re: [PATCH 3/3][v3] [RESEND] arm: ls1046ardb: Add SD secure boot
>> target
>>
>> On 04/03/2017 12:00 AM, Ruchika Gupta wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: york sun
>>>> Sent: Saturday, April 01, 2017 1:44 AM
>>>> To: Ruchika Gupta <ruchika.gupta at nxp.com>; u-boot at lists.denx.de
>>>> Cc: Vini Pillai <vinitha.pillai at nxp.com>; Sumit Garg
>>>> <sumit.garg at nxp.com>
>>>> Subject: Re: [PATCH 3/3][v3] [RESEND] arm: ls1046ardb: Add SD secure
>>>> boot target
>>>>
>>>> On 03/29/2017 07:21 AM, Ruchika Gupta wrote:
>>>>> From: Vinitha Pillai-B57223 <vinitha.pillai at nxp.com>
>>>>>
>>>>> - Add SD secure boot target for ls1046ardb.
>>>>> - Implement board specific spl_board_init() to setup CAAM stream ID
>> and
>>>>>   corresponding stream ID in SMMU.
>>>>> - Change the u-boot size defined by a macro for copying the main
>>>>> U-Boot
>>>> by SPL
>>>>>   to also include the u-boot Secure Boot header size as header is
>>>>> appended
>>>> to
>>>>>   u-boot image. So header will also be copied from SD to DDR.
>>>>> - CONFIG_MAX_SPL_SIZE is limited to 90K.SPL is copied to OCRAM
>>>>> (128K)
>>>> where 32K
>>>>>   are reserved for use by boot ROM and 6K for the header
>>>>> - Reduce the size of CAAM driver for SPL. Since the size of spl image
>>>>>   was about 94K, Blobification functions and descriptors, that are
>>>>> not
>>>> required
>>>>>   at the time of SPL are disabled. Further error code conversion to strings
>>>>>   is disabled for SPL build. This reduces the spl image size to 92K.
>>>>>
>>>>> Signed-off-by: Vinitha Pillai <vinitha.pillai at nxp.com>
>>>>> Signed-off-by: Sumit Garg <sumit.garg at nxp.com>
>>>>> Signed-off-by: Ruchika Gupta <ruchika.gupta at nxp.com>
>>>>> ---
>>>>> Changes from v1:
>>>>> - Rebased patches to latest dependent patch set
>>>>> - With the dependent path set , spl imag size increased to 94K. So
>>>>> - additionally  reduce the spl image size by removing the functions
>>>>> from
>>>>> - CAAM driver that are not required in SPL flow
>>>>>
>>>>
>>>> <snip>
>>>>
>>>>> +#if defined(CONFIG_SPL_BUILD)
>>>>> +void spl_board_init(void)
>>>>> +{
>>>>> +#ifdef CONFIG_SECURE_BOOT
>>>>> +	/*
>>>>> +	 * In case of Secure Boot, the IBR configures the SMMU
>>>>> +	 * to allow only Secure transactions.
>>>>> +	 * SMMU must be reset in bypass mode.
>>>>> +	 * Set the ClientPD bit and Clear the USFCFG Bit
>>>>> +	*/
>>>>> +	u32 val;
>>>>> +	val = (in_le32(SMMU_SCR0) | SCR0_CLIENTPD_MASK) &
>>>> ~(SCR0_USFCFG_MASK);
>>>>> +	out_le32(SMMU_SCR0, val);
>>>>> +	val = (in_le32(SMMU_NSCR0) | SCR0_CLIENTPD_MASK) &
>>>> ~(SCR0_USFCFG_MASK);
>>>>> +	out_le32(SMMU_NSCR0, val);
>>>>> +#endif
>>>>> +}
>>>>
>>>> Is this the same as LS1043A? Can we move this function to
>>>> arch/arm/cpu/armv8/fsl-layerscape/spl.c?
>>> This is true for LS1043, LS1046, however wouldn't hold good for Chassis
>> gen3 SoC's like LS2088 , LS1088 etc. Is this file arch/arm/cpu/armv8/fsl-
>> layerscape/spl.c common for the Chassis Gen 3 SoC's also ?
>>>
>>
>> Yes, it is common for lsch3.
>>
> Since it is common for lsh3, please suggest if we should move this configuration under
> if defined(CONGIF_LS1043) || defined(CONFIG_LS1046) in arch/arm/cpu/armv8/fsl-layerscape/spl.c  or leave it in this file.
>

I prefer to have it in a common file. It is easier to maintain. Can we 
use CONFIG_FSL_LSCH2 to gate these code? If you have to use SoC name, 
please use CONFIG_ARCH_LS1043A and CONFIG_ARCH_LS1046A.

York