[U-Boot] [U-Boot, 1/4] rockchip: rk322x: set the DDR region as non-secure in SPL

[Kever Yang]

Hi Philipp,


On 07/27/2017 01:40 AM, Philipp Tomsich wrote:
> Kever,
>
> On Wed, 26 Jul 2017, Kever Yang wrote:
>
>> Lets set the all the DDR region as non secure in SPL, the
>> trust like OPTEE should have the correct setting for it if
>> there is one.
>>
>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>> Acked-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
>> ---
>>
>> arch/arm/mach-rockchip/rk322x-board-spl.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm/mach-rockchip/rk322x-board-spl.c 
>> b/arch/arm/mach-rockchip/rk322x-board-spl.c
>> index 15216c7..aa8a742 100644
>> --- a/arch/arm/mach-rockchip/rk322x-board-spl.c
>> +++ b/arch/arm/mach-rockchip/rk322x-board-spl.c
>> @@ -41,6 +41,8 @@ static struct rk322x_grf * const grf = (void 
>> *)GRF_BASE;
>>              CON_IOMUX_UART2SEL_MASK,
>>              CON_IOMUX_UART2SEL_21 << CON_IOMUX_UART2SEL_SHIFT);
>> }
>> +
>> +#define SGRF_DDR_CON0 0x10150000
>
> Please use a typed const-variable (e.g. 'const u32 *') in the function 
> where this is needed.

I don't understand why I can't use the macro directly and it's 
definitely const.
>
>> void board_init_f(ulong dummy)
>> {
>>     struct udevice *dev;
>> @@ -71,6 +73,7 @@ void board_init_f(ulong dummy)
>>         return;
>>     }
>>
>> +    rk_clrreg(SGRF_DDR_CON0, 0x4000);
>
> Could you add a comment here that explains what is set and what the 
> meanings of these bits are?  I think I've seen a comment somewhere (ATF?)
> explaining that the '4' comes from enabling some sort of bypass, but it
> would be great to be able to just look here and immediately understand 
> what this line does...

Well, I will add comments here and also update the commit message.

Thanks,
- Kever
>
> Also (without any comment explaining what goes on), it feels a bit odd 
> that 'set all to non-secure' does not simply zero all bits.
>
>> #if defined(CONFIG_ROCKCHIP_SPL_BACK_TO_BROM) && 
>> !defined(CONFIG_SPL_BOARD_INIT)
>>     back_to_bootrom();
>> #endif
>>
>