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

[Lukasz Majewski <lukma@denx.de>]

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@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.

> Similarly other places.
> 
> Thanks and regards,
> Lokesh
> 
> >  		enable_clock_domain(clk_domains[i],
> >  				    CD_CLKCTRL_CLKTRCTRL_SW_WKUP);
> >  	}
> >  
> >  	/* Clock modules that need to be put in HW_AUTO */
> > -	for (i = 0; (i < max) && clk_modules_hw_auto[i]; i++) {
> > +	for (i = 0; (i < max) && clk_modules_hw_auto &&
> > +		     clk_modules_hw_auto[i]; i++) {
> >  		enable_clock_module(clk_modules_hw_auto[i],
> >  				    MODULE_CLKCTRL_MODULEMODE_HW_AUTO,
> >  				    wait_for_enable);
> >  	};
> >  
> >  	/* Clock modules that need to be put in SW_EXPLICIT_EN
> > mode */
> > -	for (i = 0; (i < max) && clk_modules_explicit_en[i]; i++) {
> > +	for (i = 0; (i < max) && clk_modules_explicit_en &&
> > +		     clk_modules_explicit_en[i]; i++) {
> >  		enable_clock_module(clk_modules_explicit_en[i],
> >  				    MODULE_CLKCTRL_MODULEMODE_SW_EXPLICIT_EN,
> >  				    wait_for_enable);
> >  	};
> >  
> >  	/* Put the clock domains in HW_AUTO mode now */
> > -	for (i = 0; (i < max) && clk_domains[i]; i++) {
> > +	for (i = 0; (i < max) && clk_domains && clk_domains[i];
> > i++) { enable_clock_domain(clk_domains[i],
> >  				    CD_CLKCTRL_CLKTRCTRL_HW_AUTO);
> >  	}
> > 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de