Patchwork [U-Boot,V3,17/32] imximage.cfg: run files through C preprocessor

login
register
mail settings
Submitter Troy Kisky
Date Oct. 10, 2012, 2:03 a.m.
Message ID <5074D75D.8050200@boundarydevices.com>
Download mbox | patch
Permalink /patch/190533/
State Superseded
Delegated to: Stefano Babic
Headers show

Comments

Troy Kisky - Oct. 10, 2012, 2:03 a.m.
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%)

freescale      mx6 mx6qsabreauto:IMX_CONFIG=board/freescale/mx6qs
abreauto/imximage.cfg
-mx6qsabrelite                arm         armv7 mx6qsabrelite       
freescale      mx6 mx6qsabrelite:IMX_CONFIG=board/freescale/imx/d
dr/mx6q_4x_mt41j128.cfg
-mx6qsabresd                  arm         armv7 mx6qsabresd         
freescale      mx6 mx6qsabresd:IMX_CONFIG=board/freescale/imx/ddr
/mx6q_4x_mt41j128.cfg
+mx6qsabrelite                arm         armv7 mx6qsabrelite       
freescale      mx6 mx6qsabrelite:IMX_CONFIG=board/freescale/imx/d
dr/mx6q_4x_mt41j128.pcfg
+mx6qsabresd                  arm         armv7 mx6qsabresd         
freescale      mx6 mx6qsabresd:IMX_CONFIG=board/freescale/imx/ddr
/mx6q_4x_mt41j128.pcfg
  cm_t35                       arm         armv7 cm_t35              
-              omap3
  omap3_overo                  arm         armv7 overo               
-              omap3
  omap3_pandora                arm         armv7 pandora             
-              omap3
--
1.7.9.5
Stefano Babic - Oct. 11, 2012, 11:11 a.m.
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
Troy Kisky - Oct. 11, 2012, 8:33 p.m.
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
Stefano Babic - Oct. 11, 2012, 10:27 p.m.
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
Tom Rini - Oct. 11, 2012, 11:15 p.m.
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..
Albert ARIBAUD - Oct. 13, 2012, 10:11 a.m.
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,
Tom Rini - Oct. 13, 2012, 3:17 p.m.
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.
Albert ARIBAUD - Oct. 14, 2012, 8:37 a.m.
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,
Tom Rini - Oct. 15, 2012, 1:24 a.m.
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.
Troy Kisky - Oct. 17, 2012, 8:32 p.m.
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
Tom Rini - Oct. 17, 2012, 9:05 p.m.
-----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-----
Troy Kisky - Oct. 17, 2012, 9:38 p.m.
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
>
Tom Rini - Oct. 17, 2012, 10:29 p.m.
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.
Albert ARIBAUD - Oct. 23, 2012, 6:30 a.m.
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,

Patch

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