On Sat, Dec 10, 2011 at 2:42 AM, Graeme Russ graeme.russ@gmail.com wrote:
Hi Vadim,
Oops, dropped the ML...
On Dec 10, 2011 9:20 PM, "Graeme Russ" graeme.russ@gmail.com wrote:
Hi Vadim,
On Dec 10, 2011 12:14 PM, "Graeme Russ" graeme.russ@gmail.com wrote:
Hi Vadim,
On Dec 10, 2011 12:08 PM, "Vadim Bendebury" vbendeb@chromium.org wrote:
On Fri, Dec 9, 2011 at 3:57 PM, Graeme Russ graeme.russ@gmail.com wrote:
Hi Vadim,
On Dec 10, 2011 10:31 AM, "Vadim Bendebury" vbendeb@chromium.org wrote:
There have been a couple of unused variable cases, causing compilation warnings when building the eNET target.
While the board/eNET/eNET.c:last_stage_init() case seems a leftover from a quick edit, the arch/x86/cpu/sc520/sc520_timer.c:sc520_udelay() seems to actually require accessing the registers discarding the values.
Thanks for picking this up, I've been a bit preoccupie lately.
I'll need to look a bit more closely, but there should be no need for such trickery...
Regards,
Graeme
Hi Graeme, thank you for a quick response.
Do you mean that there is no need to read the registers before the actual udelay functionality or do you have another way of pacifying the compiler in mind?
On closer inspection, I can't think of a better way
Acked-by: Graeme Russ graeme.russ@gmail.com
Sorry, bit I just had a closer look at this and after reading the datasheet I've come to the realization that the udelay function is broken - not badly, but it can timeout up to 1ms early.
The read of swtmrmilli reads the current value of the millisecond timer, latches the value of the internal microsecond timer and resets the millisecond timer to zero
The read of swtmrmicro reads the latched microsecond value
The code assumes that reading swtmrmicro resets the microsecond counter, which it does not. So if the internal microsecond timer is, say, 900 then udelay(500) for example will return without any delay at all
I need to fix this, but cannot do so any time soon...
I have no objection to the compiler warning fix, but is there any point in applying a cosmetic fix to broken code?
I'm not near my dev PC so the formatting is all wrong, but this should fix the bug and the compiler warning:
void sc520_udelay(unsigned long usec) { int m; long u; long start_us;
/* Reset millisecond counter */ m = readw(&sc520_mmcr->swtmrmilli); m = 0;
Looks reasonable, not much difference from the original submission IMO.
The important thing is that there should be no compilation warnings allowed, as if there are a few 'legitimate' ones, it becomes easy to let the illegitimate ones to slip through :P
cheers, /vb
/* Read initial microsecond count */ start_us = readw(&sc520_mmcr->swtmrmicro);
do {
m += readw(&sc520_mmcr->swtmrmilli); u = readw(&sc520_mmcr->swtmrmicro) + (m * 1000); } while ((u - start_us) < usec); }
Regards,
Graeme