[U-Boot] [PATCH] Fix compilation warnings when building eNET

[Vadim Bendebury]

On Sat, Dec 10, 2011 at 2:42 AM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Vadim,
>
> Oops, dropped the ML...
>
> On Dec 10, 2011 9:20 PM, "Graeme Russ" <graeme.russ at gmail.com> wrote:
>>
>> Hi Vadim,
>>
>> On Dec 10, 2011 12:14 PM, "Graeme Russ" <graeme.russ at gmail.com> wrote:
>> >
>> > Hi Vadim,
>> >
>> > On Dec 10, 2011 12:08 PM, "Vadim Bendebury" <vbendeb at chromium.org>
>> > wrote:
>> > >
>> > > On Fri, Dec 9, 2011 at 3:57 PM, Graeme Russ <graeme.russ at gmail.com>
>> > > wrote:
>> > > >
>> > > > Hi Vadim,
>> > > >
>> > > > On Dec 10, 2011 10:31 AM, "Vadim Bendebury" <vbendeb at 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 at 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