Submitter | Myles Watson |
---|---|
Date | 2010-07-26 16:57:18 |
Message ID | <AANLkTinHzLq_yharqa8yh8Euaa50nUgEyv1kLg7R_4BU@mail.gmail.com> |
Download | mbox | patch |
Permalink | /patch/1681/ |
State | Superseded |
Headers | show |
Comments
Myles Watson wrote: > There are many ways to fix it, but I'm not sure which one is the > correct (most future-proof) way. .. > 3. Just use the path I think this is *by far* the cleanest approach! //Peter
On Mon, Jul 26, 2010 at 11:09 AM, Peter Stuge <peter@stuge.se> wrote: > Myles Watson wrote: >> There are many ways to fix it, but I'm not sure which one is the >> correct (most future-proof) way. > .. >> 3. Just use the path > > I think this is *by far* the cleanest approach! I agree that it looks the best. I'm worried that it introduces ambiguity. #include <path/file.h> Could look in src/path/file.h or src/include/path/file.h and others Is that what we want? Should we remove -I$(src) from the command line in the long term? from src/arch/i386/Makefile.bootblock.inc: $(CC) -MMD -x assembler-with-cpp -DASSEMBLY -E -I$(src)/include -I$(src)/arch/i386/include -I$(obj) -I$(obj)/bootblock -include $(obj)/config.h -I. -I$(src) $< -o $@ It seems like it could be simpler. I also don't understand the order. Thanks, Myles
Myles Watson wrote: > >> 3. Just use the path > > > > I think this is *by far* the cleanest approach! > > I agree that it looks the best. I'm worried that it introduces > ambiguity. > > #include <path/file.h> > > Could look in src/path/file.h or src/include/path/file.h and others > > Is that what we want? Should we remove -I$(src) from the command line > in the long term? I'm not sure that I feel good about .h files outside include/ being referenced from other parts of the code. They should probably be moved to include/ if they are needed in more than one place.. > from src/arch/i386/Makefile.bootblock.inc: > > $(CC) -MMD -x assembler-with-cpp -DASSEMBLY -E -I$(src)/include > -I$(src)/arch/i386/include -I$(obj) -I$(obj)/bootblock -include > $(obj)/config.h -I. -I$(src) $< -o $@ > > It seems like it could be simpler. I think simplifying the codebase will be a continuous effort. > I also don't understand the order. Me neither. And again, why are there include files in src/arch/i386/include instead of include/arch-i386 or something? //Peter
Am 26.07.2010 20:01, schrieb Peter Stuge: > I'm not sure that I feel good about .h files outside include/ being > referenced from other parts of the code. They should probably be > moved to include/ if they are needed in more than one place.. Mainboards need to include chipset specific information, and I'd like to avoid moving all that into include/ if possible. >> I also don't understand the order. > Me neither. And again, why are there include files in > src/arch/i386/include instead of include/arch-i386 or something? It was partially inherited from newconfig, partially "what works". And I guess that was how it was handled throughout newconfig's lifecycle, too. Patrick
On Mon, Jul 26, 2010 at 12:57 PM, Patrick Georgi <patrick@georgi-clan.de> wrote: > Am 26.07.2010 20:01, schrieb Peter Stuge: >> I'm not sure that I feel good about .h files outside include/ being >> referenced from other parts of the code. They should probably be >> moved to include/ if they are needed in more than one place.. > Mainboards need to include chipset specific information, and I'd like to > avoid moving all that into include/ if possible. I think that's a reasonable exception to the rule. Some of that code could move to the chipsets, but not all of it. Thanks, Myles
Patrick Georgi wrote: > > .h files outside include/ > > They should probably be moved to include/ > > Mainboards need to include chipset specific information, and I'd > like to avoid moving all that into include/ if possible. Why not do it? Myles Watson wrote: > I think that's a reasonable exception to the rule. Why have an exception at all? > Some of that code could move to the chipsets, but not all of it. Hmm, can you clarify? //Peter
On Mon, Jul 26, 2010 at 1:22 PM, Peter Stuge <peter@stuge.se> wrote: > Patrick Georgi wrote: >> > .h files outside include/ >> > They should probably be moved to include/ >> >> Mainboards need to include chipset specific information, and I'd >> like to avoid moving all that into include/ if possible. > > Why not do it? It's used in multiple places, but only two or three usually. > Myles Watson wrote: >> I think that's a reasonable exception to the rule. > > Why have an exception at all? So src/include/ doesn't get cluttered. >> Some of that code could move to the chipsets, but not all of it. > > Hmm, can you clarify? I was thinking of some of the ACPI code, that is not mainboard-dependent but chipset-dependent. That's been slowly moving to the chipset directories. Some of the mainboard initialization code calls functions depending on what's on the board. That won't be moved. Thanks, Myles
On 7/26/10 9:28 PM, Myles Watson wrote: > I was thinking of some of the ACPI code, that is not > mainboard-dependent but chipset-dependent. That's been slowly moving > to the chipset directories. > If we can get rid of exceptions by cleaning more code up in this way we should certainly do it. Stefan
Patch
Index: svn/src/mainboard/asus/a8v-e_se/mptable.c =================================================================== --- svn/src/mainboard/asus/a8v-e_se/mptable.c (revision 5667) +++ svn/src/mainboard/asus/a8v-e_se/mptable.c (working copy) @@ -20,8 +20,8 @@ #include <string.h> #include <stdint.h> #include <arch/smp/mpspec.h> -#include <../../../southbridge/via/vt8237r/vt8237r.h> -#include <../../../southbridge/via/k8t890/k8t890.h> +#include "../../../southbridge/via/vt8237r/vt8237r.h" +#include "../../../southbridge/via/k8t890/k8t890.h" static void *smp_write_config_table(void *v) { 2. Make the path valid from src/include Index: svn/src/mainboard/asus/a8v-e_se/mptable.c =================================================================== --- svn/src/mainboard/asus/a8v-e_se/mptable.c (revision 5667) +++ svn/src/mainboard/asus/a8v-e_se/mptable.c (working copy) @@ -20,8 +20,8 @@ #include <string.h> #include <stdint.h> #include <arch/smp/mpspec.h> -#include <../../../southbridge/via/vt8237r/vt8237r.h> -#include <../../../southbridge/via/k8t890/k8t890.h> +#include <../southbridge/via/vt8237r/vt8237r.h> +#include <../southbridge/via/k8t890/k8t890.h> static void *smp_write_config_table(void *v) { 3. Just use the path Index: svn/src/mainboard/asus/a8v-e_se/mptable.c =================================================================== --- svn.orig/src/mainboard/asus/a8v-e_se/mptable.c +++ svn/src/mainboard/asus/a8v-e_se/mptable.c @@ -20,8 +20,8 @@ #include <string.h> #include <stdint.h> #include <arch/smp/mpspec.h> -#include <../../../southbridge/via/vt8237r/vt8237r.h> -#include <../../../southbridge/via/k8t890/k8t890.h> +#include <southbridge/via/vt8237r/vt8237r.h> +#include <southbridge/via/k8t890/k8t890.h> static void *smp_write_config_table(void *v) {