Patchwork SMM included twice

login
register
about
Submitter Myles Watson
Date 2009-10-26 16:14:39
Message ID <2831fecf0910260914h4431b427n27c7dfa7e3d7a822@mail.gmail.com>
Download mbox | patch
Permalink /patch/490/
State Accepted
Headers show

Comments

Myles Watson - 2009-10-26 16:14:39
src/cpu/x86/smm/Makefile.inc:33: warning: overriding commands for target
`/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm.o'
src/cpu/x86/smm/Makefile.inc:33: warning: ignoring old commands for target
`/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm.o'
src/cpu/x86/smm/Makefile.inc:36: warning: overriding commands for target
`/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm'
src/cpu/x86/smm/Makefile.inc:36: warning: ignoring old commands for target
`/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm'
src/cpu/x86/smm/Makefile.inc:41: warning: overriding commands for target
`/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm_bin.c'
src/cpu/x86/smm/Makefile.inc:41: warning: ignoring old commands for target
`/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm_bin.c'

myles@orangutan:svn$ grep -Ir subdirs.*smm src/ | grep -v svn
src/cpu/x86/Makefile.inc:subdirs-y += smm
src/cpu/via/model_c7/Makefile.inc:subdirs-y += ../../x86/smm
src/cpu/via/model_c3/Makefile.inc:subdirs-y += ../../x86/smm
src/cpu/intel/ep80579/Makefile.inc:subdirs-y += ../../x86/smm

First it gets included with src/cpu/x86, then with every cpu model.  Can we
just remove those?

subdirs-y += amd
subdirs-y += intel
subdirs-y += via
subdirs-y += emulation
subdirs-y += x86

This is a little strange, though.  All the rest are brands.

To be consistent, I think we should remove subdirs-y += x86 from
src/cpu/Makefile.inc, and let every CPU include it.  Patch attached.

Signed-off-by: Myles Watson <mylesgw@gmail.com>

I'd be equally happy with every CPU (since we only have x86), just doing
subdir-y += ../../x86 so they didn't have to be repeated.

Thanks,
Myles
Stefan Reinauer - 2009-10-26 16:36:20
Myles Watson wrote:
> src/cpu/x86/smm/Makefile.inc:33: warning: overriding commands for
> target
> `/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm.o'
> src/cpu/x86/smm/Makefile.inc:33: warning: ignoring old commands for
> target
> `/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm.o'
> src/cpu/x86/smm/Makefile.inc:36: warning: overriding commands for
> target
> `/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm'
> src/cpu/x86/smm/Makefile.inc:36: warning: ignoring old commands for
> target
> `/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm'
> src/cpu/x86/smm/Makefile.inc:41: warning: overriding commands for
> target
> `/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm_bin.c'
> src/cpu/x86/smm/Makefile.inc:41: warning: ignoring old commands for
> target
> `/home/myles/try/buildrom-devel/work/coreboot/svn/build/cpu/x86/smm/smm_bin.c'
>
> myles@orangutan:svn$ grep -Ir subdirs.*smm src/ | grep -v svn
> src/cpu/x86/Makefile.inc:subdirs-y += smm
> src/cpu/via/model_c7/Makefile.inc:subdirs-y += ../../x86/smm
> src/cpu/via/model_c3/Makefile.inc:subdirs-y += ../../x86/smm
> src/cpu/intel/ep80579/Makefile.inc:subdirs-y += ../../x86/smm
>
> First it gets included with src/cpu/x86, then with every cpu model. 
> Can we just remove those?
>
> subdirs-y += amd
> subdirs-y += intel
> subdirs-y += via
> subdirs-y += emulation
> subdirs-y += x86
>
> This is a little strange, though.  All the rest are brands.
>
> To be consistent, I think we should remove subdirs-y += x86 from
> src/cpu/Makefile.inc, and let every CPU include it.  Patch attached.
>
> Signed-off-by: Myles Watson <mylesgw@gmail.com <mailto:mylesgw@gmail.com>>
>
> I'd be equally happy with every CPU (since we only have x86), just
> doing subdir-y += ../../x86 so they didn't have to be repeated.

Ooops my fault.. Whatever appeals people most ...

There's an additional Kconfig flag which guards the actual addition of
code, so we could include it in x86/Makefile.inc so we don't have to add
it everywhere.

If a socket includes intel/model_6ex and intel/model_6fx and both
include smm, we would get duplicates again,.. so just including x86 from
cpu and including smm from x86 seems the cleanest way to me, since smm
really is an x86 thing, across cpu types.

Stefan
Myles Watson - 2009-10-26 16:51:52
Ooops my fault.. Whatever appeals people most ...
>
Whatever seems most consistent.


> If a socket includes intel/model_6ex and intel/model_6fx and both
> include smm, we would get duplicates again,.. so just including x86 from
> cpu and including smm from x86 seems the cleanest way to me, since smm
> really is an x86 thing, across cpu types.
>
That's true for a lot of the x86/* files.  Should we remove them all from
cpus?

Thanks,
Myles
Stefan Reinauer - 2009-10-26 17:07:33
Myles Watson wrote:
>
>
>
>     Ooops my fault.. Whatever appeals people most ...
>
> Whatever seems most consistent.
>  
>
>     If a socket includes intel/model_6ex and intel/model_6fx and both
>     include smm, we would get duplicates again,.. so just including
>     x86 from
>     cpu and including smm from x86 seems the cleanest way to me, since smm
>     really is an x86 thing, across cpu types.
>
> That's true for a lot of the x86/* files.  Should we remove them all
> from cpus?
No idea..
Myles Watson - 2009-10-26 17:22:32
On Mon, Oct 26, 2009 at 11:07 AM, Stefan Reinauer <stepan@coresystems.de>wrote:

> Myles Watson wrote:
> >
> >
> >
> >     Ooops my fault.. Whatever appeals people most ...
> >
> > Whatever seems most consistent.
> >
> >
> >     If a socket includes intel/model_6ex and intel/model_6fx and both
> >     include smm, we would get duplicates again,
>
I don't see that with Kontron.  Is there another board that does that for
you with my patch applied?


> .. so just including
> >     x86 from
> >     cpu and including smm from x86 seems the cleanest way to me, since
> smm
> >     really is an x86 thing, across cpu types.
> >
> > That's true for a lot of the x86/* files.  Should we remove them all
> > from cpus?
> No idea..
>
Until we figure that out, it seems like my patch is quick and relatively
painless.

Thanks,
Myles

Patch

Index: svn/src/cpu/Makefile.inc
===================================================================
--- svn.orig/src/cpu/Makefile.inc
+++ svn/src/cpu/Makefile.inc
@@ -2,4 +2,3 @@  subdirs-y += amd
 subdirs-y += intel
 subdirs-y += via
 subdirs-y += emulation
-subdirs-y += x86
Index: svn/src/cpu/x86/Makefile.inc
===================================================================
--- svn.orig/src/cpu/x86/Makefile.inc
+++ /dev/null
@@ -1 +0,0 @@ 
-subdirs-y += smm