[U-Boot] [PATCH V5 1/3] arm: Add support for MB86R0x SoCs

[Wolfgang Denk <wd@denx.de>]

Dear Matthias Weisser,

In message <1280734550-18403-2-git-send-email-weisserm@arcor.de> you wrote:
> This patch adds support for MB86R0x SoCs from Fujitsu
> 
> Signed-off-by: Matthias Weisser <weisserm@arcor.de>
...
> --- /dev/null
> +++ b/arch/arm/cpu/arm926ejs/mb86r0x/clock.c
...
> +	if (pllmode == MB86R0x_CRG_CRPR_PLLMODE_X46)
> +		return 41164767;
> +	else if (pllmode == MB86R0x_CRG_CRPR_PLLMODE_X20)
> +		return 40000000;
> +
> +	return 41164767;

Or shorter:

	if (pllmode == MB86R0x_CRG_CRPR_PLLMODE_X20)
		return 40000000;
	
	return 41164767;


> +unsigned long long get_ticks(void)
> +{
> +	struct mb86r0x_timer * timer = (struct mb86r0x_timer *)
> +					MB86R0x_TIMER_BASE;
> +	ulong now = readl(&timer->value);
> +
> +	if (now <= lastdec)	/* normal mode (non roll) */
> +		/* move stamp forward with absolut diff ticks */
> +		timestamp += lastdec - now;
> +	else			/* we have rollover of incrementer */

We need curly braces for multiline statements like this.


> +void __udelay(unsigned long usec)
> +{
> +	unsigned long long tmp;
> +	ulong tmo;
> +
> +	tmo = usec_to_tick(usec);
> +	tmp = get_ticks() + tmo;	/* get current timestamp */
> +
> +	while (get_ticks() < tmp)	/* loop till event */
> +		 /*NOP*/;

This is broken when the timer wraps around. Compare the limit against
the difference, to make use of unsigned arithmetics.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
It seems intuitively obvious to me, which  means  that  it  might  be
wrong.                                                 -- Chris Torek