[U-Boot] [PATCH] ti: clocks: Fix do_enable_clocks() to accept NULL pointers as input parameters

Lokesh Vutla lokeshvutla at ti.com
Fri Mar 24 09:17:46 UTC 2017



On Friday 24 March 2017 02:41 PM, Lukasz Majewski wrote:
> Hi Lokesh,
> 
>>
>>
>> On 3/24/2017 3:54 AM, Lukasz Majewski wrote:
>>> Up till this commit passing NULL as input parameter was allowed,
>>> but not handled properly.
>>>
>>> When one passed NULL to one of this function parameters, the code
>>> was executed causing data abort.
>>>
>>> However, what is more interesting, the abort was not caught because
>>> of code execution in HYP mode with masked CPSR A bit ("Imprecise
>>> Data Abort mask bit). The TI's AM57xx SoC switch to HYP mode with A
>>> bit masked in lowlevel_init.S due to SMC call. Such operation (by
>>> default) is performed in SoC ROM code.
>>>
>>> The problem would pop up when one:
>>> - Switch back to SVC mode after disabling LPAE support
>>> - Somebody enables A bit (by executing cpsie a asm instruction)
>>>
>>> and then the previously described exception would be caught.
>>>
>>> Signed-off-by: Lukasz Majewski <lukma at denx.de>
>>> ---
>>>  arch/arm/cpu/armv7/omap-common/clocks-common.c | 10 ++++++----
>>
>> This has been moved to arch/arm/mach-omap2/clocks-common.c
>> Please use the latest U-Boot.
> 
> Ok. I'm not working on a cutting-edge u-boot.
> 
>>
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv7/omap-common/clocks-common.c
>>> b/arch/arm/cpu/armv7/omap-common/clocks-common.c index
>>> 097b8e3..157155a 100644 ---
>>> a/arch/arm/cpu/armv7/omap-common/clocks-common.c +++
>>> b/arch/arm/cpu/armv7/omap-common/clocks-common.c @@ -822,27 +822,29
>>> @@ void do_enable_clocks(u32 const *clk_domains, u32 i, max = 100;
>>>  
>>>  	/* Put the clock domains in SW_WKUP mode */
>>> -	for (i = 0; (i < max) && clk_domains[i]; i++) {
>>> +	for (i = 0; (i < max) && clk_domains && clk_domains[i];
>>> i++) {
>>
>> Instead of checking for clk_domains every time, can we use max as
>> ARRAY_SIZE(clk_domains)?
> 
> do_enable_clocks() accepts pointer to u32 as an argument
> (clk_domains). IMHO the ARRAY_SIZE(clk_domains) would be 1 - always.
> 

you are right. Discard my comment.

Thanks and regards,
Lokesh


More information about the U-Boot mailing list