[U-Boot] [PATCH 2/9] MX51: Add initial support for the Freescale MX51

[Wolfgang Denk]

Dear Stefano Babic,

In message <4B540829.3080305 at denx.de> you wrote:
>
> >> +	unsigned long now, last = readl(&cur_gpt->counter);
> >> +	long tmo = usec * (CONFIG_MX51_CLK32 / 1000) / 1000;
> >> +
> >> +	if (!tmo)
> >> +		tmo = 1;
> >> +
> >> +	while (tmo > 0) {
> >> +		now = readl(&cur_gpt->counter);
> >> +		if (last > now)
> >> +			tmo -= 0xFFFFFFFF - last + now;
> >> +		else
> >> +			tmo -= now - last;
> > 
> > Is this correct wrap-around handling?
> 
> Well, the code is quite the same as in omap3. It seems that omap suffers
> from the same issue. Best thing here should be to read the timer before

Maybe, I didn;t check. Eventually OMAP can be fixed, too.

> the loop to get a timestamp and to check in the loop for the
> wrap-around, resetting the counter if needed.

Do we really have to make it such complicated? All this are "usinged
int" operations, right? Then let's just make use of the properties of
"usinged int" artihmentics...

IMO the code should be rewritten like this:

	unsigned long start, now, timeout;

	start = now = readl(&cur_gpt->counter);

	while ((now - start) < timeout)
		now = readl(&cur_gpt->counter);

This should handle wraparounds nicely.

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
Lack of skill dictates economy of style.                - Joey Ramone