Warnings

[Myles Watson]

As of 5512, these are the last four warnings that are not "defined but
not used" warnings.

src/northbridge/amd/gx2/chipsetinit.c:271: warning: suggest
parentheses around '-' inside '<<'
src/northbridge/amd/lx/raminit.c:334: warning: array subscript is
above array bounds
src/northbridge/via/vx800/pci_rawops.h:43:2: warning: #warning "FIXME:
get rid of this extra copy of pci access functions."
src/southbridge/intel/pxhd/pxhd_bridge.c:70:2: warning: #warning
"Please review lots of dead code here."

Thanks,
Myles

[Stefan Reinauer]

On 4/30/10 7:13 PM, Myles Watson wrote:
> As of 5512, these are the last four warnings that are not "defined but
> not used" warnings.

What's your impression... should we activate -Wno-unused ? or should we
attempt to fix them?

I'd like to go drop all / most of the #ifdef UNUSED_CODE segments from
coreboot over time, making it easier to follow the code that is actually
there.

Most of the existing warnings are amd k8 / fam10 warnings... and it's
kind of tricky to understand why one board does init different than
another one. Why don't all boards use " wait_all_aps_started" and / or "
wait_all_core0_started" and / or "allow_all_aps_stop"? I think the
unused messages show that we have too much unnecessary diversity here
(aka rank growth)
> src/northbridge/amd/gx2/chipsetinit.c:271: warning: suggest
> parentheses around '-' inside '<<'

This would need help from someone with a GX2 (or willing to check out
the data sheets ;-)
> src/northbridge/amd/lx/raminit.c:334: warning: array subscript is
> above array bounds

Marc indicated, this could be a compiler "problem". We could maybe add
an explicit check for the array index.
> src/northbridge/via/vx800/pci_rawops.h:43:2: warning: #warning "FIXME:
> get rid of this extra copy of pci access functions."

Gone with 5513
> src/southbridge/intel/pxhd/pxhd_bridge.c:70:2: warning: #warning
> "Please review lots of dead code here."

Gone with 5514

[Myles Watson]

On Fri, Apr 30, 2010 at 11:50 AM, Stefan Reinauer  wrote:
> On 4/30/10 7:13 PM, Myles Watson wrote:
>
> As of 5512, these are the last four warnings that are not "defined but
> not used" warnings.
>
> What's your impression... should we activate -Wno-unused ? or should we
> attempt to fix them?

I don't think it's worth activating -Wno-unused-functions untill we
activate -Werror, but I think we should do that soon so that we don't
reintroduce a lot of warnings.
> I'd like to go drop all / most of the #ifdef UNUSED_CODE segments from
> coreboot over time, making it easier to follow the code that is actually
> there.

I agree.
> Most of the existing warnings are amd k8 / fam10 warnings... and it's kind
> of tricky to understand why one board does init different than another one.
> Why don't all boards use " wait_all_aps_started" and / or "
> wait_all_core0_started" and / or "allow_all_aps_stop"? I think the unused
> messages show that we have too much unnecessary diversity here (aka rank
> growth)

Yes.  There are definitely some things that need to be standardized,
and the warnings are good at pointing that out, as you've said.
> src/northbridge/amd/gx2/chipsetinit.c:271: warning: suggest
> parentheses around '-' inside '<<'
>
> This would need help from someone with a GX2 (or willing to check out the
> data sheets ;-)
>
> src/northbridge/amd/lx/raminit.c:334: warning: array subscript is
> above array bounds
>
> Marc indicated, this could be a compiler "problem". We could maybe add an
> explicit check for the array index.

It looks like it's being indexed by a function that's built into the
compiler that returns bit positions.  Since the return value could be
up to 31, I think it's a correct warning (the array's only got 8
elements.)
> src/northbridge/via/vx800/pci_rawops.h:43:2: warning: #warning "FIXME:
> get rid of this extra copy of pci access functions."
>
> Gone with 5513
>
> src/southbridge/intel/pxhd/pxhd_bridge.c:70:2: warning: #warning
> "Please review lots of dead code here."
>
> Gone with 5514

It's much nicer to have so many fewer warnings!

Thanks,
Myles

[Myles Watson]

> > src/northbridge/amd/lx/raminit.c:334: warning: array subscript is
> > above array bounds
> >
> > Marc indicated, this could be a compiler "problem". We could maybe add an
> > explicit check for the array index.
>
> It looks like it's being indexed by a function that's built into the
> compiler that returns bit positions.  Since the return value could be
> up to 31, I think it's a correct warning (the array's only got 8
> elements.)

This patch fixes the warnings.  All I did was factor out the identical
code into a function, and remove some unnecessary casts.  So I guess I
was wrong.

Signed-off-by: Myles Watson

Thanks,
Myles

[Stefan Reinauer]

On 4/30/10 10:17 PM, Myles Watson wrote:
> > > src/northbridge/amd/lx/raminit.c:334: warning: array subscript is
> > > above array bounds
> > >
> > > Marc indicated, this could be a compiler "problem". We could maybe add an
> > > explicit check for the array index.
> >
> > It looks like it's being indexed by a function that's built into the
> > compiler that returns bit positions.  Since the return value could be
> > up to 31, I think it's a correct warning (the array's only got 8
> > elements.)
>
> This patch fixes the warnings.  All I did was factor out the identical
> code into a function, and remove some unnecessary casts.  So I guess I
> was wrong.
>
> Signed-off-by: Myles Watson

Acked-by: Stefan Reinauer