| Message ID | 5074D75D.8050200@boundarydevices.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
Am 10/10/2012 04:03, schrieb Troy Kisky: > On 10/8/2012 6:38 AM, Stefano Babic wrote: >> On 04/10/2012 03:47, Troy Kisky wrote: >>> The '#' used as comments in the files cause the preprocessor >>> trouble, so change to /* */. >>> >>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> >>> --- >> Hi Troy, >> >>> Makefile | 3 +- >>> board/esg/ima3-mx53/imximage.cfg | 120 ++++++----- >>> board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg | 90 ++++---- >>> board/freescale/mx25pdk/imximage.cfg | 77 +++---- >>> board/freescale/mx51evk/imximage.cfg | 114 +++++----- >>> board/freescale/mx53ard/imximage_dd3.cfg | 83 ++++---- >>> board/freescale/mx53evk/imximage.cfg | 86 ++++---- >>> board/freescale/mx53loco/imximage.cfg | 83 ++++---- >>> board/freescale/mx53smd/imximage.cfg | 83 ++++---- >>> board/freescale/mx6qarm2/imximage.cfg | 88 ++++---- >>> board/genesi/mx51_efikamx/imximage_mx.cfg | 132 ++++++------ >>> board/genesi/mx51_efikamx/imximage_sb.cfg | 126 +++++------ >>> board/ttcontrol/vision2/imximage_hynix.cfg | 295 >>> ++++++++++++++------------ >>> 13 files changed, 727 insertions(+), 653 deletions(-) >>> >> I see the C preprocessor as an optional feature, instead of a rule >> everybody must follow. >> >>> diff --git a/Makefile b/Makefile >>> index a40d4cc..64ff1b8 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -431,7 +431,8 @@ $(obj)u-boot.img: $(obj)u-boot.bin >>> -d $< $@ >>> $(obj)u-boot.imx: $(obj)u-boot.bin >>> - $(obj)tools/mkimage -n $(CONFIG_IMX_CONFIG) -T imximage \ >>> + $(CC) -E -x c $(CONFIG_IMX_CONFIG) -I./include -o >>> $(obj)imxcfg.imx >>> + $(obj)tools/mkimage -n $(obj)imxcfg.imx -T imximage \ >>> -e $(CONFIG_SYS_TEXT_BASE) -d $< $@ >> In fact, adding this rule here requires that each board configuration >> must be changed. And for all of them, running the preprocessor is >> unnnecessary. >> >> What about to add this rule only to the Makefile of the boards that >> require preprocessing ? >> >> Best regards, >> Stefano Babic >> > How about this to do the job.... > > > > > > Subject: [PATCH 17/32] boards.cfg: run mx6q_4x_mt41j128.pcfg through C > preprocessor > > The '#' used as comments in the cfg file cause the preprocessor > trouble, so change to /* */. Also, rename mx6q_4x_mt41j128.cfg > to mx6q_4x_mt41j128.pcfg. > > Files with extension of .pcfg are run through the preprocessor > before being given to mkimage. > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> > > --- > v4: Don't run every file through preprocessor, only .pcfg files > --- > .gitignore | 1 + > Makefile | 16 +++- > ...{mx6q_4x_mt41j128.cfg => mx6q_4x_mt41j128.pcfg} | 90 > ++++++++++---------- > boards.cfg | 4 +- > 4 files changed, 63 insertions(+), 48 deletions(-) > rename board/freescale/imx/ddr/{mx6q_4x_mt41j128.cfg => > mx6q_4x_mt41j128.pcfg} (65%) > > diff --git a/.gitignore b/.gitignore > index d91e91b..e5273bd 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -15,6 +15,7 @@ > *.swp > *.patch > *.bin > +*.pcfgtmp > > # Build tree > /build-* > diff --git a/Makefile b/Makefile > index a40d4cc..99666b9 100644 > --- a/Makefile > +++ b/Makefile > @@ -430,8 +430,17 @@ $(obj)u-boot.img: $(obj)u-boot.bin > sed -e 's/"[ ]*$$/ for $(BOARD) board"/') \ > -d $< $@ > > -$(obj)u-boot.imx: $(obj)u-boot.bin > - $(obj)tools/mkimage -n $(CONFIG_IMX_CONFIG) -T imximage \ > +ifeq ($(suffix $(patsubst "%",%,$(CONFIG_IMX_CONFIG))),.pcfg) > +$(obj)$(patsubst "%",%,$(CONFIG_IMX_CONFIG))tmp: %.pcfgtmp : %.pcfg > + $(CC) -E -x c $< -I./include -o $@ > + > +$(obj)u-boot.imx: %.imx : %.bin $(obj)$(patsubst > "%",%,$(CONFIG_IMX_CONFIG))tmp > +else > +$(obj)u-boot.imx: %.imx : %.bin $(patsubst "%",%,$(CONFIG_IMX_CONFIG)) > +endif > + > +$(obj)u-boot.imx: > + $(obj)tools/mkimage -n $(filter-out %.bin,$^) -T > imximage \ > -e $(CONFIG_SYS_TEXT_BASE) -d $< $@ > Does it work something more direct (I have not tested, really...) ? In board Makefile, for example board/freescale/mx6qsabrelite, a rule for the imximage file: $(CONFIG_IMX_CONFIG): <your base file, mx6q_4x_mt41j128.cfg maybe> $(CC) -E -x c $< -I./include -o $@ And let unchanged in the main Makefile. So CONFIG_IMX_CONFIG is produced and you do not need to change logic in the main Makefile. Regards, Stefano
On 10/11/2012 4:11 AM, Stefano Babic wrote: > Does it work something more direct (I have not tested, really...) ? > > In board Makefile, for example board/freescale/mx6qsabrelite, a rule for > the imximage file: > > $(CONFIG_IMX_CONFIG): <your base file, mx6q_4x_mt41j128.cfg maybe> > $(CC) -E -x c $< -I./include -o $@ > > And let unchanged in the main Makefile. So CONFIG_IMX_CONFIG is produced > and you do not need to change logic in the main Makefile. > > Regards, > Stefano > The advantages I see of changing the main Makefile are 1. Easy for other boards to use the preprocessor. You merely need to change the file extension to pcfg. 2. Easy to clean the temporary generated file. The main Makefile deletes files with .pcfgtmp extension. 3. The file referred to by boards.cfg actually exists before the build starts. 4. The temporary file can be placed in an out-of-tree directory for make -O builds Using the file extension to determine whether to use the preprocessor is also what gcc uses to preprocess ".S" files while skipping this for ".s" files. I believe that at least other mx6 boards will quickly change to using the preprocessor as well to add support for solo/duallite, so total line count should eventually be less with changes to the main makefile. Having said that, I really have no problem going your route, I just don't prefer it. Let me know. Thanks Troy
Am 11/10/2012 22:33, schrieb Troy Kisky: > On 10/11/2012 4:11 AM, Stefano Babic wrote: >> Does it work something more direct (I have not tested, really...) ? >> >> In board Makefile, for example board/freescale/mx6qsabrelite, a rule for >> the imximage file: >> >> $(CONFIG_IMX_CONFIG): <your base file, mx6q_4x_mt41j128.cfg maybe> >> $(CC) -E -x c $< -I./include -o $@ >> >> And let unchanged in the main Makefile. So CONFIG_IMX_CONFIG is produced >> and you do not need to change logic in the main Makefile. >> >> Regards, >> Stefano >> > > The advantages I see of changing the main Makefile are > 1. Easy for other boards to use the preprocessor. You merely > need to change the file extension to pcfg. I set Tom in CC, because changing the main Makefile is more related to the whole project, and not only for the i.MX. One reason to move into the board directory is that there was a decision to move rules related to only one arch or SOC where they belong to, that is in the corresponding arch/ or board/ directory. > > 2. Easy to clean the temporary generated file. The main Makefile > deletes files with .pcfgtmp extension. > > 3. The file referred to by boards.cfg actually exists before the build > starts. This is true, but I do not understand which is the advantage. A lot of files are generated, also .c or .S files. If it exists or not, it does not matter. > > 4. The temporary file can be placed in an out-of-tree directory for > make -O builds > > Using the file extension to determine whether to use the preprocessor is > also > what gcc uses to preprocess ".S" files while skipping this for ".s" files. > > I believe that at least other mx6 boards will quickly change to using > the preprocessor > as well to add support for solo/duallite, so total line count should > eventually be > less with changes to the main makefile. Ok, but if this true, the rule should be moved to the mx6 directory, and should not be valid for other i.MX that do not need it. > Having said that, I really have no problem going your route, I just > don't prefer it. > Let me know. Let's wait to know Tom's opinion. Regards, Stefano
On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: [snip] > One reason to move into the board directory is that there was a decision > to move rules related to only one arch or SOC where they belong to, that > is in the corresponding arch/ or board/ directory. I'll admit that maybe my make-fu is off, but that idea doesn't work, at least for SPL. So I'd really like someone to make that work first. > > 2. Easy to clean the temporary generated file. The main Makefile > > deletes files with .pcfgtmp extension. > > > > 3. The file referred to by boards.cfg actually exists before the build > > starts. > > This is true, but I do not understand which is the advantage. A lot of > files are generated, also .c or .S files. If it exists or not, it does > not matter. > > > > > 4. The temporary file can be placed in an out-of-tree directory for > > make -O builds > > > > Using the file extension to determine whether to use the preprocessor is > > also > > what gcc uses to preprocess ".S" files while skipping this for ".s" files. > > > > I believe that at least other mx6 boards will quickly change to using > > the preprocessor > > as well to add support for solo/duallite, so total line count should > > eventually be > > less with changes to the main makefile. > > Ok, but if this true, the rule should be moved to the mx6 directory, and > should not be valid for other i.MX that do not need it. Introducing slight differences to the image generation rules per family generation when we could just have one rule that works fine for all generations is one worry I have about the notion of moving things out of a top level Makefile and putting them elsewhere. > > Having said that, I really have no problem going your route, I just > > don't prefer it. > > Let me know. > > Let's wait to know Tom's opinion. How about this, if we convert the existing cfg files to '@' comments and use the LDSCRIPT style preprocessor rule instead of another one? I assume there's improvements that could be done to the mx5 ones if we preprocessed them. Or no? I'm looking for opinions here myself still..
Hi Tom, On Thu, 11 Oct 2012 16:15:02 -0700, Tom Rini <trini@ti.com> wrote: > On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: > > [snip] > > One reason to move into the board directory is that there was a decision > > to move rules related to only one arch or SOC where they belong to, that > > is in the corresponding arch/ or board/ directory. > > I'll admit that maybe my make-fu is off, but that idea doesn't work, at > least for SPL. So I'd really like someone to make that work first. Tom, can you be more specific than 'it doesn't work'? :) Seriously, though, I'm interested in understand what the make issue is there, because I am indeed a proponent of putting files where they belong to, so if help is needed there, I would try to. Amicalement,
On Sat, Oct 13, 2012 at 3:11 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Tom, > > On Thu, 11 Oct 2012 16:15:02 -0700, Tom Rini <trini@ti.com> wrote: > >> On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: >> >> [snip] >> > One reason to move into the board directory is that there was a decision >> > to move rules related to only one arch or SOC where they belong to, that >> > is in the corresponding arch/ or board/ directory. >> >> I'll admit that maybe my make-fu is off, but that idea doesn't work, at >> least for SPL. So I'd really like someone to make that work first. > > Tom, can you be more specific than 'it doesn't work'? :) > > Seriously, though, I'm interested in understand what the make issue is > there, because I am indeed a proponent of putting files where they > belong to, so if help is needed there, I would try to. I have had no luck moving things like the 'MLO' rule from spl/Makefile to anywhere else. Same with the 'checkthumb' rule in the top-level Makefile.
Hi Tom, (seems like gmail does not honor the rule that replies should drop the "(was: xxxxx)" part in an e-mail subject; but hey, neither does Claws apparently. Sigh.) On Sat, 13 Oct 2012 08:17:41 -0700, Tom Rini <trini@ti.com> wrote: > On Sat, Oct 13, 2012 at 3:11 AM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: > > Hi Tom, > > > > On Thu, 11 Oct 2012 16:15:02 -0700, Tom Rini <trini@ti.com> wrote: > > > >> On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: > >> > >> [snip] > >> > One reason to move into the board directory is that there was a decision > >> > to move rules related to only one arch or SOC where they belong to, that > >> > is in the corresponding arch/ or board/ directory. > >> > >> I'll admit that maybe my make-fu is off, but that idea doesn't work, at > >> least for SPL. So I'd really like someone to make that work first. > > > > Tom, can you be more specific than 'it doesn't work'? :) > > > > Seriously, though, I'm interested in understand what the make issue is > > there, because I am indeed a proponent of putting files where they > > belong to, so if help is needed there, I would try to. > > I have had no luck moving things like the 'MLO' rule from spl/Makefile > to anywhere else. Same with the 'checkthumb' rule in the top-level > Makefile. Ok, now it is more precise :) but still not enough for me to efficiently try and analyze the issue. Let's take the checkthumb rule. Where did you try to move it? Amicalement,
On Sun, Oct 14, 2012 at 1:37 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Tom, > > (seems like gmail does not honor the rule that replies should drop the > "(was: xxxxx)" part in an e-mail subject; but hey, neither does Claws > apparently. Sigh.) > > On Sat, 13 Oct 2012 08:17:41 -0700, Tom Rini <trini@ti.com> wrote: > >> On Sat, Oct 13, 2012 at 3:11 AM, Albert ARIBAUD >> <albert.u.boot@aribaud.net> wrote: >> > Hi Tom, >> > >> > On Thu, 11 Oct 2012 16:15:02 -0700, Tom Rini <trini@ti.com> wrote: >> > >> >> On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: >> >> >> >> [snip] >> >> > One reason to move into the board directory is that there was a decision >> >> > to move rules related to only one arch or SOC where they belong to, that >> >> > is in the corresponding arch/ or board/ directory. >> >> >> >> I'll admit that maybe my make-fu is off, but that idea doesn't work, at >> >> least for SPL. So I'd really like someone to make that work first. >> > >> > Tom, can you be more specific than 'it doesn't work'? :) >> > >> > Seriously, though, I'm interested in understand what the make issue is >> > there, because I am indeed a proponent of putting files where they >> > belong to, so if help is needed there, I would try to. >> >> I have had no luck moving things like the 'MLO' rule from spl/Makefile >> to anywhere else. Same with the 'checkthumb' rule in the top-level >> Makefile. > > Ok, now it is more precise :) but still not enough for me to > efficiently try and analyze the issue. > > Let's take the checkthumb rule. Where did you try to move it? I tried both arch/arm/config.mk arch/arm/Makefile and couldn't make either work.
On 10/11/2012 4:15 PM, Tom Rini wrote: > On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: > > [snip] >> One reason to move into the board directory is that there was a decision >> to move rules related to only one arch or SOC where they belong to, that >> is in the corresponding arch/ or board/ directory. > I'll admit that maybe my make-fu is off, but that idea doesn't work, at > least for SPL. So I'd really like someone to make that work first. > >>> 2. Easy to clean the temporary generated file. The main Makefile >>> deletes files with .pcfgtmp extension. >>> >>> 3. The file referred to by boards.cfg actually exists before the build >>> starts. >> This is true, but I do not understand which is the advantage. A lot of >> files are generated, also .c or .S files. If it exists or not, it does >> not matter. Consistency was my point here. Every other file in boards.cfg exists prior to build. >>> 4. The temporary file can be placed in an out-of-tree directory for >>> make -O builds >>> >>> Using the file extension to determine whether to use the preprocessor is >>> also >>> what gcc uses to preprocess ".S" files while skipping this for ".s" files. >>> >>> I believe that at least other mx6 boards will quickly change to using >>> the preprocessor >>> as well to add support for solo/duallite, so total line count should >>> eventually be >>> less with changes to the main makefile. >> Ok, but if this true, the rule should be moved to the mx6 directory, and >> should not be valid for other i.MX that do not need it. > Introducing slight differences to the image generation rules per family > generation when we could just have one rule that works fine for all > generations is one worry I have about the notion of moving things out of > a top level Makefile and putting them elsewhere. > >>> Having said that, I really have no problem going your route, I just >>> don't prefer it. >>> Let me know. >> Let's wait to know Tom's opinion. > How about this, if we convert the existing cfg files to '@' comments and > use the LDSCRIPT style preprocessor rule instead of another one? I > assume there's improvements that could be done to the mx5 ones if we > preprocessed them. Or no? I'm looking for opinions here myself still.. > I had previously converted all existing cfg files to /* */ comments. That style of comment seems common for LDSCRIPTs as well. '@''s actually give me an error. arm-eabi-ld:u-boot.lds:1: ignoring invalid character `@' in expression I do believe mx5 files can benefit from preprocessing. I can see the advantage of converting everything now. I also like flexibility of not forcing every cfg file to change now. So, I am setting on the fence. If I have to take a position, I'd fall on the side of the smaller patch set of a gradual conversion, just because I like smaller patches. Troy
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10/17/12 13:32, Troy Kisky wrote: > On 10/11/2012 4:15 PM, Tom Rini wrote: >> On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: >> >> [snip] >>> One reason to move into the board directory is that there was a >>> decision to move rules related to only one arch or SOC where >>> they belong to, that is in the corresponding arch/ or board/ >>> directory. >> I'll admit that maybe my make-fu is off, but that idea doesn't >> work, at least for SPL. So I'd really like someone to make that >> work first. >> >>>> 2. Easy to clean the temporary generated file. The main >>>> Makefile deletes files with .pcfgtmp extension. >>>> >>>> 3. The file referred to by boards.cfg actually exists before >>>> the build starts. >>> This is true, but I do not understand which is the advantage. A >>> lot of files are generated, also .c or .S files. If it exists >>> or not, it does not matter. > > Consistency was my point here. Every other file in boards.cfg > exists prior to build. > >>>> 4. The temporary file can be placed in an out-of-tree >>>> directory for make -O builds >>>> >>>> Using the file extension to determine whether to use the >>>> preprocessor is also what gcc uses to preprocess ".S" files >>>> while skipping this for ".s" files. >>>> >>>> I believe that at least other mx6 boards will quickly change >>>> to using the preprocessor as well to add support for >>>> solo/duallite, so total line count should eventually be less >>>> with changes to the main makefile. >>> Ok, but if this true, the rule should be moved to the mx6 >>> directory, and should not be valid for other i.MX that do not >>> need it. >> Introducing slight differences to the image generation rules per >> family generation when we could just have one rule that works >> fine for all generations is one worry I have about the notion of >> moving things out of a top level Makefile and putting them >> elsewhere. >> >>>> Having said that, I really have no problem going your route, >>>> I just don't prefer it. Let me know. >>> Let's wait to know Tom's opinion. >> How about this, if we convert the existing cfg files to '@' >> comments and use the LDSCRIPT style preprocessor rule instead of >> another one? I assume there's improvements that could be done to >> the mx5 ones if we preprocessed them. Or no? I'm looking for >> opinions here myself still.. >> > > I had previously converted all existing cfg files to /* */ > comments. That style of comment seems common for LDSCRIPTs as well. > '@''s actually give me an error. > > arm-eabi-ld:u-boot.lds:1: ignoring invalid character `@' in > expression Right, but in u-boot.lds.S it gets preprocessed away, at least I swear I changed and tested that. My thinking being it was a smaller diff delta. But my final point being I don't think we should start introducing artificial differences here just because older boards may not use it. That doing that leads to bit rot. > I do believe mx5 files can benefit from preprocessing. I can see > the advantage of converting everything now. I also like flexibility > of not forcing every cfg file to change now. So, I am setting on > the fence. If I have to take a position, I'd fall on the side of > the smaller patch set of a gradual conversion, just because I like > smaller patches. I'm on the other side only because "later" sometimes never happens. Doing it as a series of smaller patches, now might be fine too... - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQfx2GAAoJENk4IS6UOR1W/YYQALM7ejRg8kZ2JLgJ5j0Ae6UG 49ZhsNtaJYS4p224wOvGTHkKMNg0apD1bO6r+aZ17mD/tNmJCzg46zwpVMeyydf4 /DBlQsxAHP1cpe5+5Gf2q/UXuzbL3XiQEN/ELea8jpY0eW3LImNB7PEAX3DaIcQE NlretiUdWZmsdrtKPt16SBtC+yRqJXbRu9UEA6WKhKdLoAjhUm0NMDNzNHEsbsyV DcAWa7MuppZ9x3UC6KHWtcjNZgKHlfRoRvYRWc0H+pVX/QTejhIh+dFN2Tx3Lu/0 8hGDLN+rBZS8yooPkiOtDEcAX8N/mj2pODqGvIuGBiPTXvauGHXGJfnscZMBhJoi jKWqPzvpmUM8TR94ucadXszAb4GaGBZYy++w6kfb57InBTBy4+HwXMPEbqhv8LMm VpuzN3sxNYW+KtaIUB5kaoznGI1zK7hY+/5n6EBYebZHE3zLdyMRKnvpq2bCO21r IZsj+ki1STi6VBgWKg7uORAQzDIpCN9DTKauM4lVnxdYXLkOc2f3Zz8r17C+VrtQ go7PShkF45djJr6EjbZHJ1jMuyT2+gw4QzyNDFv1coojDV7YF4MsTwXTi3R2EoMz POwbt9crwe1O9EvbP85qYrznfG1ogNK8ZRSoSfz4sNvoYipZ6rTSiGyAq5eVT4yH E5GKf6wg4ujGTgLm2djj =rpFn -----END PGP SIGNATURE-----
On 10/17/2012 2:05 PM, Tom Rini wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 10/17/12 13:32, Troy Kisky wrote: >> On 10/11/2012 4:15 PM, Tom Rini wrote: >>> On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: >>> >>> [snip] >>>> One reason to move into the board directory is that there was a >>>> decision to move rules related to only one arch or SOC where >>>> they belong to, that is in the corresponding arch/ or board/ >>>> directory. >>> I'll admit that maybe my make-fu is off, but that idea doesn't >>> work, at least for SPL. So I'd really like someone to make that >>> work first. >>> >>>>> 2. Easy to clean the temporary generated file. The main >>>>> Makefile deletes files with .pcfgtmp extension. >>>>> >>>>> 3. The file referred to by boards.cfg actually exists before >>>>> the build starts. >>>> This is true, but I do not understand which is the advantage. A >>>> lot of files are generated, also .c or .S files. If it exists >>>> or not, it does not matter. >> Consistency was my point here. Every other file in boards.cfg >> exists prior to build. >> >>>>> 4. The temporary file can be placed in an out-of-tree >>>>> directory for make -O builds >>>>> >>>>> Using the file extension to determine whether to use the >>>>> preprocessor is also what gcc uses to preprocess ".S" files >>>>> while skipping this for ".s" files. >>>>> >>>>> I believe that at least other mx6 boards will quickly change >>>>> to using the preprocessor as well to add support for >>>>> solo/duallite, so total line count should eventually be less >>>>> with changes to the main makefile. >>>> Ok, but if this true, the rule should be moved to the mx6 >>>> directory, and should not be valid for other i.MX that do not >>>> need it. >>> Introducing slight differences to the image generation rules per >>> family generation when we could just have one rule that works >>> fine for all generations is one worry I have about the notion of >>> moving things out of a top level Makefile and putting them >>> elsewhere. >>> >>>>> Having said that, I really have no problem going your route, >>>>> I just don't prefer it. Let me know. >>>> Let's wait to know Tom's opinion. >>> How about this, if we convert the existing cfg files to '@' >>> comments and use the LDSCRIPT style preprocessor rule instead of >>> another one? I assume there's improvements that could be done to >>> the mx5 ones if we preprocessed them. Or no? I'm looking for >>> opinions here myself still.. >>> >> I had previously converted all existing cfg files to /* */ >> comments. That style of comment seems common for LDSCRIPTs as well. >> '@''s actually give me an error. >> >> arm-eabi-ld:u-boot.lds:1: ignoring invalid character `@' in >> expression > Right, but in u-boot.lds.S it gets preprocessed away, at least I swear > I changed and tested that. My thinking being it was a smaller diff > delta. Good point. Is there some magic parameter to pass to gcc to strip @. My current command line is expanded to arm-eabi-gcc -E -x c board/freescale/imx/ddr/mx6q_4x_mt41j128.pcfg -g -Os -fno-common -ffixed-r8 -msoft-float -D__KERNEL__ -DCONFIG_SYS_TEXT_BASE=0x17800000 -I/home/tkisky/u-boot-imx6/include -fno-builtin -ffreestanding -nostdinc -isystem /home/tkisky/myandroid/prebuilt/linux-x86/toolchain/arm-eabi-4.4.3/bin/../lib/gcc/arm-eabi/4.4.3/include -pipe -DCONFIG_ARM -D__ARM__ -marm -mno-thumb-interwork -mabi=aapcs-linux -march=armv7-a -o board/freescale/imx/ddr/mx6q_4x_mt41j128.pcfgtmp Alternatively, I could send a small patch to mkimage to ignore @ lines along with the currently ignored # lines. I grepped all lds files in u-boot, but could not find one that used @ as a comment indicator. I don't know what/where u-boot.lds.S is. > But my final point being I don't think we should start > introducing artificial differences here just because older boards may > not use it. That doing that leads to bit rot. > >> I do believe mx5 files can benefit from preprocessing. I can see >> the advantage of converting everything now. I also like flexibility >> of not forcing every cfg file to change now. So, I am setting on >> the fence. If I have to take a position, I'd fall on the side of >> the smaller patch set of a gradual conversion, just because I like >> smaller patches. > I'm on the other side only because "later" sometimes never happens. > Doing it as a series of smaller patches, now might be fine too... > > - -- > Tom >
On Wed, Oct 17, 2012 at 02:38:47PM -0700, Troy Kisky wrote: > On 10/17/2012 2:05 PM, Tom Rini wrote: > >-----BEGIN PGP SIGNED MESSAGE----- > >Hash: SHA1 > > > >On 10/17/12 13:32, Troy Kisky wrote: > >>On 10/11/2012 4:15 PM, Tom Rini wrote: > >>>On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: > >>> > >>>[snip] > >>>>One reason to move into the board directory is that there was a > >>>>decision to move rules related to only one arch or SOC where > >>>>they belong to, that is in the corresponding arch/ or board/ > >>>>directory. > >>>I'll admit that maybe my make-fu is off, but that idea doesn't > >>>work, at least for SPL. So I'd really like someone to make that > >>>work first. > >>> > >>>>>2. Easy to clean the temporary generated file. The main > >>>>>Makefile deletes files with .pcfgtmp extension. > >>>>> > >>>>>3. The file referred to by boards.cfg actually exists before > >>>>>the build starts. > >>>>This is true, but I do not understand which is the advantage. A > >>>>lot of files are generated, also .c or .S files. If it exists > >>>>or not, it does not matter. > >>Consistency was my point here. Every other file in boards.cfg > >>exists prior to build. > >> > >>>>>4. The temporary file can be placed in an out-of-tree > >>>>>directory for make -O builds > >>>>> > >>>>>Using the file extension to determine whether to use the > >>>>>preprocessor is also what gcc uses to preprocess ".S" files > >>>>>while skipping this for ".s" files. > >>>>> > >>>>>I believe that at least other mx6 boards will quickly change > >>>>>to using the preprocessor as well to add support for > >>>>>solo/duallite, so total line count should eventually be less > >>>>>with changes to the main makefile. > >>>>Ok, but if this true, the rule should be moved to the mx6 > >>>>directory, and should not be valid for other i.MX that do not > >>>>need it. > >>>Introducing slight differences to the image generation rules per > >>>family generation when we could just have one rule that works > >>>fine for all generations is one worry I have about the notion of > >>>moving things out of a top level Makefile and putting them > >>>elsewhere. > >>> > >>>>>Having said that, I really have no problem going your route, > >>>>>I just don't prefer it. Let me know. > >>>>Let's wait to know Tom's opinion. > >>>How about this, if we convert the existing cfg files to '@' > >>>comments and use the LDSCRIPT style preprocessor rule instead of > >>>another one? I assume there's improvements that could be done to > >>>the mx5 ones if we preprocessed them. Or no? I'm looking for > >>>opinions here myself still.. > >>> > >>I had previously converted all existing cfg files to /* */ > >>comments. That style of comment seems common for LDSCRIPTs as well. > >>'@''s actually give me an error. > >> > >>arm-eabi-ld:u-boot.lds:1: ignoring invalid character `@' in > >>expression > >Right, but in u-boot.lds.S it gets preprocessed away, at least I swear > >I changed and tested that. My thinking being it was a smaller diff > >delta. > > Good point. Is there some magic parameter to pass to gcc to strip @. Lets just go with /* */ comments, everyone understands that.
Hi Tom, On Sun, 14 Oct 2012 18:24:58 -0700, Tom Rini <trini@ti.com> wrote: > On Sun, Oct 14, 2012 at 1:37 AM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: > > Hi Tom, > > > > (seems like gmail does not honor the rule that replies should drop the > > "(was: xxxxx)" part in an e-mail subject; but hey, neither does Claws > > apparently. Sigh.) > > > > On Sat, 13 Oct 2012 08:17:41 -0700, Tom Rini <trini@ti.com> wrote: > > > >> On Sat, Oct 13, 2012 at 3:11 AM, Albert ARIBAUD > >> <albert.u.boot@aribaud.net> wrote: > >> > Hi Tom, > >> > > >> > On Thu, 11 Oct 2012 16:15:02 -0700, Tom Rini <trini@ti.com> wrote: > >> > > >> >> On Fri, Oct 12, 2012 at 12:27:09AM +0200, stefano babic wrote: > >> >> > >> >> [snip] > >> >> > One reason to move into the board directory is that there was a decision > >> >> > to move rules related to only one arch or SOC where they belong to, that > >> >> > is in the corresponding arch/ or board/ directory. > >> >> > >> >> I'll admit that maybe my make-fu is off, but that idea doesn't work, at > >> >> least for SPL. So I'd really like someone to make that work first. > >> > > >> > Tom, can you be more specific than 'it doesn't work'? :) > >> > > >> > Seriously, though, I'm interested in understand what the make issue is > >> > there, because I am indeed a proponent of putting files where they > >> > belong to, so if help is needed there, I would try to. > >> > >> I have had no luck moving things like the 'MLO' rule from spl/Makefile > >> to anywhere else. Same with the 'checkthumb' rule in the top-level > >> Makefile. > > > > Ok, now it is more precise :) but still not enough for me to > > efficiently try and analyze the issue. > > > > Let's take the checkthumb rule. Where did you try to move it? > > I tried both arch/arm/config.mk arch/arm/Makefile and couldn't make either work. Thanks. I'm adding this to my todo list, although not as an immediate priority. Amicalement,
diff --git a/.gitignore b/.gitignore index d91e91b..e5273bd 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ *.swp *.patch *.bin +*.pcfgtmp # Build tree /build-* diff --git a/Makefile b/Makefile index a40d4cc..99666b9 100644 --- a/Makefile +++ b/Makefile @@ -430,8 +430,17 @@ $(obj)u-boot.img: $(obj)u-boot.bin sed -e 's/"[ ]*$$/ for $(BOARD) board"/') \ -d $< $@ -$(obj)u-boot.imx: $(obj)u-boot.bin - $(obj)tools/mkimage -n $(CONFIG_IMX_CONFIG) -T imximage \ +ifeq ($(suffix $(patsubst "%",%,$(CONFIG_IMX_CONFIG))),.pcfg) +$(obj)$(patsubst "%",%,$(CONFIG_IMX_CONFIG))tmp: %.pcfgtmp : %.pcfg + $(CC) -E -x c $< -I./include -o $@ + +$(obj)u-boot.imx: %.imx : %.bin $(obj)$(patsubst "%",%,$(CONFIG_IMX_CONFIG))tmp +else +$(obj)u-boot.imx: %.imx : %.bin $(patsubst "%",%,$(CONFIG_IMX_CONFIG)) +endif + +$(obj)u-boot.imx: + $(obj)tools/mkimage -n $(filter-out %.bin,$^) -T imximage \ -e $(CONFIG_SYS_TEXT_BASE) -d $< $@ $(obj)u-boot.kwb: $(obj)u-boot.bin @@ -794,7 +803,8 @@ clean: @rm -f $(TIMESTAMP_FILE) $(VERSION_FILE) @find $(OBJTREE) -type f \ \( -name 'core' -o -name '*.bak' -o -name '*~' -o -name '*.su' \ - -o -name '*.o' -o -name '*.a' -o -name '*.exe' \) -print \ + -o -name '*.o' -o -name '*.a' -o -name '*.exe' \ + -o -name '*.pcfgtmp' \) -print \ | xargs rm -f # Removes everything not needed for testing u-boot diff --git a/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg b/board/freescale/imx/ddr/mx6q_4x_mt41j128.pcfg similarity index 65% rename from board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg rename to board/freescale/imx/ddr/mx6q_4x_mt41j128.pcfg diff --git a/boards.cfg b/boards.cfg index e9e073e..677beac 100644 --- a/boards.cfg +++ b/boards.cfg @@ -232,8 +232,8 @@ ima3-mx53 arm armv7 ima3-mx53 esg vision2 arm armv7 vision2 ttcontrol mx5 vision2:IMX_CONFIG=board/ttcontrol/vision2/imx image_hynix.cfg mx6qarm2 arm armv7 mx6qarm2 freescale mx6 mx6qarm2:IMX_CONFIG=board/freescale/mx6qarm2/i mximage.cfg mx6qsabreauto arm armv7 mx6qsabreauto