Hi Heinrich,

thanks for the patch

On 30/07/2017 20:59, Heinrich Schuchardt wrote:

The behavior get_clocks depends on the unitialized value of bootmode.

By setting it to zero we get a defined behavior and can get rid of superfluos coding.

The problem was indicated by cppcheck.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

I have no hardware to test this. Please, check thorougly before merging.

---

---

arch/m68k/cpu/mcf5227x/speed.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/m68k/cpu/mcf5227x/speed.c b/arch/m68k/cpu/mcf5227x/speed.c index 44de4a6701..978cc7adc6 100644 --- a/arch/m68k/cpu/mcf5227x/speed.c +++ b/arch/m68k/cpu/mcf5227x/speed.c @@ -73,14 +73,8 @@ int get_clocks(void)

ccm_t *ccm = (ccm_t *)MMAP_CCM; pll_t *pll = (pll_t *)MMAP_PLL;

• int vco, temp, pcrvalue, pfdr;
• u8 bootmode;
• pcrvalue = in_be32(&pll->pcr) & 0xFF0F0FFF;
• pfdr = pcrvalue >> 24;
• if (pfdr == 0x1E)

• bootmode = 0;	/* Normal Mode */
  

• int vco, temp, pcrvalue;

• u8 bootmode = 0; /* Normal Mode */;

  #ifdef CONFIG_CF_SBF bootmode = 3; /* Serial Mode */

sure uninitialized vars are an issue.This change works and make things simpler.

But looking the code i have a doubt now: this code seems attempting to detect "runtime" the boot mode, getting clocks from pll registers already set i.e. form SBF header. So there can be a case where jumpers (or pull up/down resistors) are set to SBF (11) and "bootmode" variable will be set to 3 only if CONFIG_CF_SBF is set, otherwise it will be 0 and not 3.

Of course, if we try to boot by SBF and CONFIG_CF_SBF is not set, u-boot will not boot in anyway, since spi init will be totally missing.

But in case a different "fully runtime" proposal could be:

diff --git a/arch/m68k/cpu/mcf5227x/speed.c b/arch/m68k/cpu/mcf5227x/speed.c index 44de4a6701..e21c1d574b 100644 --- a/arch/m68k/cpu/mcf5227x/speed.c +++ b/arch/m68k/cpu/mcf5227x/speed.c @@ -74,7 +74,7 @@ int get_clocks(void) ccm_t *ccm = (ccm_t *)MMAP_CCM; pll_t *pll = (pll_t *)MMAP_PLL; int vco, temp, pcrvalue, pfdr; - u8 bootmode; + u8 bootmode = 3;

pcrvalue = in_be32(&pll->pcr) & 0xFF0F0FFF; pfdr = pcrvalue >> 24; @@ -82,10 +82,6 @@ int get_clocks(void) if (pfdr == 0x1E) bootmode = 0; /* Normal Mode */

-#ifdef CONFIG_CF_SBF - bootmode = 3; /* Serial Mode */ -#endif - if (bootmode == 0) { /* Normal mode */ vco = ((in_be32(&pll->pcr) & 0xFF000000) >> 24) * CONFIG_SYS_INPUT_CLKSRC;

What do you think ?

Regards, Angelo Dureghello