[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Marek Vasut]

This will be beneficial for the PXA3XX NAND driver, which uses the NAND IDs to
identify the chip and configure the controller accordingly.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Scott Wood <scottwood@freescale.com>
---
 drivers/mtd/nand/Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index a63c0e4..46e79e3 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -35,10 +35,10 @@ COBJS-y	+= nand_spl_load.o
 endif
 else
 COBJS-y += nand.o
-COBJS-y += nand_bbt.o
-COBJS-y += nand_ids.o
 COBJS-y += nand_util.o
 endif
+COBJS-y += nand_bbt.o
+COBJS-y += nand_ids.o
 COBJS-y += nand_ecc.o
 COBJS-y += nand_base.o
 
-- 
1.7.7.1

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Scott Wood]

On 12/05/2011 05:17 PM, Marek Vasut wrote:
> This will be beneficial for the PXA3XX NAND driver, which uses the NAND IDs to
> identify the chip and configure the controller accordingly.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Scott Wood <scottwood@freescale.com>
> ---
>  drivers/mtd/nand/Makefile |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index a63c0e4..46e79e3 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -35,10 +35,10 @@ COBJS-y	+= nand_spl_load.o
>  endif
>  else
>  COBJS-y += nand.o
> -COBJS-y += nand_bbt.o
> -COBJS-y += nand_ids.o
>  COBJS-y += nand_util.o
>  endif
> +COBJS-y += nand_bbt.o
> +COBJS-y += nand_ids.o
>  COBJS-y += nand_ecc.o
>  COBJS-y += nand_base.o
>  

So, in theory with gc-sections this shouldn't increase the size of any
SPL that currently successfully links (at least in the absence of things
like weak symbols).  However, I observed a devkit8000 build go from this:

   text    data     bss     dec     hex filename
  40709    1792  197764  240265   3aa89 /tmp/u-boot-arm/spl/u-boot-spl

to this:

   text    data     bss     dec     hex filename
  42277    1792  197764  241833   3b0a9 /tmp/u-boot-arm/spl/u-boot-spl

I verified that --function-sections/-fdata-sections/--gc-sections are
being used on the SPL.  It looks like strings are not getting dropped.

If there's no way to fix this, this is going to be a big problem for the
existing users of nand_spl to migrate to the new SPL model, unless the
notion of "include a bunch of stuff and let gc-sections take care of it"
is dropped.

-Scott

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Tom Rini]

On Wed, Jan 4, 2012 at 4:56 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 12/05/2011 05:17 PM, Marek Vasut wrote:
>> This will be beneficial for the PXA3XX NAND driver, which uses the NAND IDs to
>> identify the chip and configure the controller accordingly.
>>
>> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>> Cc: Scott Wood <scottwood@freescale.com>
>> ---
>> ?drivers/mtd/nand/Makefile | ? ?4 ++--
>> ?1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>> index a63c0e4..46e79e3 100644
>> --- a/drivers/mtd/nand/Makefile
>> +++ b/drivers/mtd/nand/Makefile
>> @@ -35,10 +35,10 @@ COBJS-y ? += nand_spl_load.o
>> ?endif
>> ?else
>> ?COBJS-y += nand.o
>> -COBJS-y += nand_bbt.o
>> -COBJS-y += nand_ids.o
>> ?COBJS-y += nand_util.o
>> ?endif
>> +COBJS-y += nand_bbt.o
>> +COBJS-y += nand_ids.o
>> ?COBJS-y += nand_ecc.o
>> ?COBJS-y += nand_base.o
>>
>
> So, in theory with gc-sections this shouldn't increase the size of any
> SPL that currently successfully links (at least in the absence of things
> like weak symbols). ?However, I observed a devkit8000 build go from this:
>
> ? text ? ?data ? ? bss ? ? dec ? ? hex filename
> ?40709 ? ?1792 ?197764 ?240265 ? 3aa89 /tmp/u-boot-arm/spl/u-boot-spl
>
> to this:
>
> ? text ? ?data ? ? bss ? ? dec ? ? hex filename
> ?42277 ? ?1792 ?197764 ?241833 ? 3b0a9 /tmp/u-boot-arm/spl/u-boot-spl
>
> I verified that --function-sections/-fdata-sections/--gc-sections are
> being used on the SPL. ?It looks like strings are not getting dropped.
>
> If there's no way to fix this, this is going to be a big problem for the
> existing users of nand_spl to migrate to the new SPL model, unless the
> notion of "include a bunch of stuff and let gc-sections take care of it"
> is dropped.

I'll confirm gc-sections/etc are not as awesome as we think.  You can
drop the size of current SPL builds (for say devkit8000) by taking
things that should be dropped for us and forcing them out with #ifndef
CONFIG_SPL_BUILD.

-- 
Tom

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Marek Vasut]

> On Wed, Jan 4, 2012 at 4:56 PM, Scott Wood <scottwood@freescale.com> wrote:
> > On 12/05/2011 05:17 PM, Marek Vasut wrote:
> >> This will be beneficial for the PXA3XX NAND driver, which uses the NAND
> >> IDs to identify the chip and configure the controller accordingly.
> >> 
> >> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> >> Cc: Scott Wood <scottwood@freescale.com>
> >> ---
> >>  drivers/mtd/nand/Makefile |    4 ++--
> >>  1 files changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> >> index a63c0e4..46e79e3 100644
> >> --- a/drivers/mtd/nand/Makefile
> >> +++ b/drivers/mtd/nand/Makefile
> >> @@ -35,10 +35,10 @@ COBJS-y   += nand_spl_load.o
> >>  endif
> >>  else
> >>  COBJS-y += nand.o
> >> -COBJS-y += nand_bbt.o
> >> -COBJS-y += nand_ids.o
> >>  COBJS-y += nand_util.o
> >>  endif
> >> +COBJS-y += nand_bbt.o
> >> +COBJS-y += nand_ids.o
> >>  COBJS-y += nand_ecc.o
> >>  COBJS-y += nand_base.o
> > 
> > So, in theory with gc-sections this shouldn't increase the size of any
> > SPL that currently successfully links (at least in the absence of things
> > like weak symbols).  However, I observed a devkit8000 build go from this:
> > 
> >   text    data     bss     dec     hex filename
> >  40709    1792  197764  240265   3aa89 /tmp/u-boot-arm/spl/u-boot-spl
> > 
> > to this:
> > 
> >   text    data     bss     dec     hex filename
> >  42277    1792  197764  241833   3b0a9 /tmp/u-boot-arm/spl/u-boot-spl
> > 
> > I verified that --function-sections/-fdata-sections/--gc-sections are
> > being used on the SPL.  It looks like strings are not getting dropped.
> > 
> > If there's no way to fix this, this is going to be a big problem for the
> > existing users of nand_spl to migrate to the new SPL model, unless the
> > notion of "include a bunch of stuff and let gc-sections take care of it"
> > is dropped.
> 
> I'll confirm gc-sections/etc are not as awesome as we think.  You can
> drop the size of current SPL builds (for say devkit8000) by taking
> things that should be dropped for us and forcing them out with #ifndef
> CONFIG_SPL_BUILD.

Maybe if we had those LTO tables at the gc-sections time, it'd drop. But we 
can't rely on those.

So instead of compiling in the nand-ids, it'd be better to allow SPL to pull in 
the whole NAND/MTD stack.

M

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Tom Rini]

On Thu, Jan 5, 2012 at 2:09 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On Wed, Jan 4, 2012 at 4:56 PM, Scott Wood <scottwood@freescale.com> wrote:
>> > On 12/05/2011 05:17 PM, Marek Vasut wrote:
>> >> This will be beneficial for the PXA3XX NAND driver, which uses the NAND
>> >> IDs to identify the chip and configure the controller accordingly.
>> >>
>> >> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>> >> Cc: Scott Wood <scottwood@freescale.com>
>> >> ---
>> >> ?drivers/mtd/nand/Makefile | ? ?4 ++--
>> >> ?1 files changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>> >> index a63c0e4..46e79e3 100644
>> >> --- a/drivers/mtd/nand/Makefile
>> >> +++ b/drivers/mtd/nand/Makefile
>> >> @@ -35,10 +35,10 @@ COBJS-y ? += nand_spl_load.o
>> >> ?endif
>> >> ?else
>> >> ?COBJS-y += nand.o
>> >> -COBJS-y += nand_bbt.o
>> >> -COBJS-y += nand_ids.o
>> >> ?COBJS-y += nand_util.o
>> >> ?endif
>> >> +COBJS-y += nand_bbt.o
>> >> +COBJS-y += nand_ids.o
>> >> ?COBJS-y += nand_ecc.o
>> >> ?COBJS-y += nand_base.o
>> >
>> > So, in theory with gc-sections this shouldn't increase the size of any
>> > SPL that currently successfully links (at least in the absence of things
>> > like weak symbols). ?However, I observed a devkit8000 build go from this:
>> >
>> > ? text ? ?data ? ? bss ? ? dec ? ? hex filename
>> > ?40709 ? ?1792 ?197764 ?240265 ? 3aa89 /tmp/u-boot-arm/spl/u-boot-spl
>> >
>> > to this:
>> >
>> > ? text ? ?data ? ? bss ? ? dec ? ? hex filename
>> > ?42277 ? ?1792 ?197764 ?241833 ? 3b0a9 /tmp/u-boot-arm/spl/u-boot-spl
>> >
>> > I verified that --function-sections/-fdata-sections/--gc-sections are
>> > being used on the SPL. ?It looks like strings are not getting dropped.
>> >
>> > If there's no way to fix this, this is going to be a big problem for the
>> > existing users of nand_spl to migrate to the new SPL model, unless the
>> > notion of "include a bunch of stuff and let gc-sections take care of it"
>> > is dropped.
>>
>> I'll confirm gc-sections/etc are not as awesome as we think. ?You can
>> drop the size of current SPL builds (for say devkit8000) by taking
>> things that should be dropped for us and forcing them out with #ifndef
>> CONFIG_SPL_BUILD.
>
> Maybe if we had those LTO tables at the gc-sections time, it'd drop. But we
> can't rely on those.
>
> So instead of compiling in the nand-ids, it'd be better to allow SPL to pull in
> the whole NAND/MTD stack.

Possible, but without doing #Ifndef hacks, the size grows a good bit
(don't have the #s handy right now), and with #ifndef hacks it's just
a kb or so in growth.

-- 
Tom

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Scott Wood]

On 01/05/2012 08:15 AM, Tom Rini wrote:
> On Thu, Jan 5, 2012 at 2:09 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>> I'll confirm gc-sections/etc are not as awesome as we think.  You can
>>> drop the size of current SPL builds (for say devkit8000) by taking
>>> things that should be dropped for us and forcing them out with #ifndef
>>> CONFIG_SPL_BUILD.
>>
>> Maybe if we had those LTO tables at the gc-sections time, it'd drop. But we
>> can't rely on those.
>>
>> So instead of compiling in the nand-ids, it'd be better to allow SPL to pull in
>> the whole NAND/MTD stack.
> 
> Possible, but without doing #Ifndef hacks, the size grows a good bit
> (don't have the #s handy right now), and with #ifndef hacks it's just
> a kb or so in growth.

Whatever the set of things is that you want to pull in for these SPLs,
it needs to be a separate config option from the one that enables
libnand.o to be included, so that other SPLs can pull in smaller NAND
implementations.

Is there any reason to keep defines like CONFIG_SPL_NAND_SUPPORT (versus
LIBS-y += drivers/mtd/nand/libnand.o), if everything within that
directory needs a separate config symbol to enable it inside an SPL
(just like a normal build)?

-Scott

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Tom Rini]

On Thu, Jan 5, 2012 at 4:04 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 01/05/2012 08:15 AM, Tom Rini wrote:
>> On Thu, Jan 5, 2012 at 2:09 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> I'll confirm gc-sections/etc are not as awesome as we think. ?You can
>>>> drop the size of current SPL builds (for say devkit8000) by taking
>>>> things that should be dropped for us and forcing them out with #ifndef
>>>> CONFIG_SPL_BUILD.
>>>
>>> Maybe if we had those LTO tables at the gc-sections time, it'd drop. But we
>>> can't rely on those.
>>>
>>> So instead of compiling in the nand-ids, it'd be better to allow SPL to pull in
>>> the whole NAND/MTD stack.
>>
>> Possible, but without doing #Ifndef hacks, the size grows a good bit
>> (don't have the #s handy right now), and with #ifndef hacks it's just
>> a kb or so in growth.
>
> Whatever the set of things is that you want to pull in for these SPLs,
> it needs to be a separate config option from the one that enables
> libnand.o to be included, so that other SPLs can pull in smaller NAND
> implementations.
>
> Is there any reason to keep defines like CONFIG_SPL_NAND_SUPPORT (versus
> LIBS-y += drivers/mtd/nand/libnand.o), if everything within that
> directory needs a separate config symbol to enable it inside an SPL
> (just like a normal build)?

I think you've got it backwards.  What CONFIG_SPL_NAND_SUPPORT enables
today is more bloated than required as our 'magic' isn't working.

-- 
Tom

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Scott Wood]

On 01/05/2012 06:41 PM, Tom Rini wrote:
> On Thu, Jan 5, 2012 at 4:04 PM, Scott Wood <scottwood@freescale.com> wrote:
>> Whatever the set of things is that you want to pull in for these SPLs,
>> it needs to be a separate config option from the one that enables
>> libnand.o to be included, so that other SPLs can pull in smaller NAND
>> implementations.
>>
>> Is there any reason to keep defines like CONFIG_SPL_NAND_SUPPORT (versus
>> LIBS-y += drivers/mtd/nand/libnand.o), if everything within that
>> directory needs a separate config symbol to enable it inside an SPL
>> (just like a normal build)?
> 
> I think you've got it backwards.  What CONFIG_SPL_NAND_SUPPORT enables
> today is more bloated than required as our 'magic' isn't working.

I realize this isn't the case today -- but it's where we need to go,
since gc-sections doesn't do the job.  I was saying that I think we can
get rid of CONFIG_SPL_NAND_SUPPORT once we change to a model where every
bit of code within the directory needs some other config symbol to pull
it in.

-Scott

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Tom Rini]

On Fri, Jan 6, 2012 at 12:03 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 01/05/2012 06:41 PM, Tom Rini wrote:
>> On Thu, Jan 5, 2012 at 4:04 PM, Scott Wood <scottwood@freescale.com> wrote:
>>> Whatever the set of things is that you want to pull in for these SPLs,
>>> it needs to be a separate config option from the one that enables
>>> libnand.o to be included, so that other SPLs can pull in smaller NAND
>>> implementations.
>>>
>>> Is there any reason to keep defines like CONFIG_SPL_NAND_SUPPORT (versus
>>> LIBS-y += drivers/mtd/nand/libnand.o), if everything within that
>>> directory needs a separate config symbol to enable it inside an SPL
>>> (just like a normal build)?
>>
>> I think you've got it backwards. ?What CONFIG_SPL_NAND_SUPPORT enables
>> today is more bloated than required as our 'magic' isn't working.
>
> I realize this isn't the case today -- but it's where we need to go,
> since gc-sections doesn't do the job. ?I was saying that I think we can
> get rid of CONFIG_SPL_NAND_SUPPORT once we change to a model where every
> bit of code within the directory needs some other config symbol to pull
> it in.

Ah, OK.  But maybe this also means we need to rethink other parts of
SPL too?  I'd imagine this isn't a NAND subsystem specific issue we're
running into.

-- 
Tom

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Scott Wood]

On 01/06/2012 01:14 PM, Tom Rini wrote:
> On Fri, Jan 6, 2012 at 12:03 PM, Scott Wood <scottwood@freescale.com> wrote:
>> On 01/05/2012 06:41 PM, Tom Rini wrote:
>>> On Thu, Jan 5, 2012 at 4:04 PM, Scott Wood <scottwood@freescale.com> wrote:
>>>> Whatever the set of things is that you want to pull in for these SPLs,
>>>> it needs to be a separate config option from the one that enables
>>>> libnand.o to be included, so that other SPLs can pull in smaller NAND
>>>> implementations.
>>>>
>>>> Is there any reason to keep defines like CONFIG_SPL_NAND_SUPPORT (versus
>>>> LIBS-y += drivers/mtd/nand/libnand.o), if everything within that
>>>> directory needs a separate config symbol to enable it inside an SPL
>>>> (just like a normal build)?
>>>
>>> I think you've got it backwards.  What CONFIG_SPL_NAND_SUPPORT enables
>>> today is more bloated than required as our 'magic' isn't working.
>>
>> I realize this isn't the case today -- but it's where we need to go,
>> since gc-sections doesn't do the job.  I was saying that I think we can
>> get rid of CONFIG_SPL_NAND_SUPPORT once we change to a model where every
>> bit of code within the directory needs some other config symbol to pull
>> it in.
> 
> Ah, OK.  But maybe this also means we need to rethink other parts of
> SPL too?  I'd imagine this isn't a NAND subsystem specific issue we're
> running into.

Right, the toplevel config symbol on a directory only makes sense if
there's code that will be wanted by all SPLs using that directory -- and
given the nature of SPL, that's probably not going to be the case very
often.

-Scott

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Mike Frysinger]

On Wednesday 04 January 2012 18:56:23 Scott Wood wrote:
> On 12/05/2011 05:17 PM, Marek Vasut wrote:
> > This will be beneficial for the PXA3XX NAND driver, which uses the NAND
> > IDs to identify the chip and configure the controller accordingly.
> > 
> > --- a/drivers/mtd/nand/Makefile
> > +++ b/drivers/mtd/nand/Makefile
> > 
> >  endif
> >  else
> >  COBJS-y += nand.o
> > -COBJS-y += nand_bbt.o
> > -COBJS-y += nand_ids.o
> >  COBJS-y += nand_util.o
> >  endif
> > +COBJS-y += nand_bbt.o
> > +COBJS-y += nand_ids.o
> >  COBJS-y += nand_ecc.o
> >  COBJS-y += nand_base.o
> 
> So, in theory with gc-sections this shouldn't increase the size of any
> SPL that currently successfully links (at least in the absence of things
> like weak symbols).  However, I observed a devkit8000 build go from this:
> 
>    text    data     bss     dec     hex filename
>   40709    1792  197764  240265   3aa89 /tmp/u-boot-arm/spl/u-boot-spl
> 
> to this:
> 
>    text    data     bss     dec     hex filename
>   42277    1792  197764  241833   3b0a9 /tmp/u-boot-arm/spl/u-boot-spl
> 
> I verified that --function-sections/-fdata-sections/--gc-sections are
> being used on the SPL.  It looks like strings are not getting dropped.

specifically, "anonymous" strings (or whatever the term is).  it could be made 
to work, but it'd be ugly.  something like:

--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -22,6 +22,12 @@
 +	256	256 Byte page size
 *	512	512 Byte page size
 */
+
+static const char nand_16mib_18v_8bit[] = "NAND 16MiB 1,8V 8-bit";
+static const char nand_16mib_33v_8bit[] = "NAND 16MiB 3,3V 8-bit";
+static const char nand_16mib_18v_16bit[] = "NAND 16MiB 1,8V 16-bit";
+static const char nand_16mib_33v_16bit[] = "NAND 16MiB 3,3V 16-bit";
+
 const struct nand_flash_dev nand_flash_ids[] = {
 
 #ifdef CONFIG_MTD_NAND_MUSEUM_IDS
@@ -42,10 +48,10 @@
 	{"NAND 8MiB 3,3V 16-bit",	0x59, 512, 8, 0x2000, NAND_BUSWIDTH_16},
 #endif
 
-	{"NAND 16MiB 1,8V 8-bit",	0x33, 512, 16, 0x4000, 0},
-	{"NAND 16MiB 3,3V 8-bit",	0x73, 512, 16, 0x4000, 0},
-	{"NAND 16MiB 1,8V 16-bit",	0x43, 512, 16, 0x4000, NAND_BUSWIDTH_16},
-	{"NAND 16MiB 3,3V 16-bit",	0x53, 512, 16, 0x4000, NAND_BUSWIDTH_16},
+	{nand_16mib_18v_8bit,	0x33, 512, 16, 0x4000, 0},
+	{nand_16mib_33v_8bit,	0x73, 512, 16, 0x4000, 0},
+	{nand_16mib_18v_16bit,	0x43, 512, 16, 0x4000, NAND_BUSWIDTH_16},
+	{nand_16mib_33v_16bit,	0x53, 512, 16, 0x4000, NAND_BUSWIDTH_16},
 
 	{"NAND 32MiB 1,8V 8-bit",	0x35, 512, 32, 0x4000, 0},
 	{"NAND 32MiB 3,3V 8-bit",	0x75, 512, 32, 0x4000, 0},
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120108/62ee777e/attachment.pgp>

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Scott Wood]

On 01/08/2012 03:56 AM, Mike Frysinger wrote:
> On Wednesday 04 January 2012 18:56:23 Scott Wood wrote:
>> On 12/05/2011 05:17 PM, Marek Vasut wrote:
>>> This will be beneficial for the PXA3XX NAND driver, which uses the NAND
>>> IDs to identify the chip and configure the controller accordingly.
>>>
>>> --- a/drivers/mtd/nand/Makefile
>>> +++ b/drivers/mtd/nand/Makefile
>>>
>>>  endif
>>>  else
>>>  COBJS-y += nand.o
>>> -COBJS-y += nand_bbt.o
>>> -COBJS-y += nand_ids.o
>>>  COBJS-y += nand_util.o
>>>  endif
>>> +COBJS-y += nand_bbt.o
>>> +COBJS-y += nand_ids.o
>>>  COBJS-y += nand_ecc.o
>>>  COBJS-y += nand_base.o
>>
>> So, in theory with gc-sections this shouldn't increase the size of any
>> SPL that currently successfully links (at least in the absence of things
>> like weak symbols).  However, I observed a devkit8000 build go from this:
>>
>>    text    data     bss     dec     hex filename
>>   40709    1792  197764  240265   3aa89 /tmp/u-boot-arm/spl/u-boot-spl
>>
>> to this:
>>
>>    text    data     bss     dec     hex filename
>>   42277    1792  197764  241833   3b0a9 /tmp/u-boot-arm/spl/u-boot-spl
>>
>> I verified that --function-sections/-fdata-sections/--gc-sections are
>> being used on the SPL.  It looks like strings are not getting dropped.
> 
> specifically, "anonymous" strings (or whatever the term is).  it could be made 
> to work, but it'd be ugly.  something like:
> 
> --- a/drivers/mtd/nand/nand_ids.c
> +++ b/drivers/mtd/nand/nand_ids.c
> @@ -22,6 +22,12 @@
>  +	256	256 Byte page size
>  *	512	512 Byte page size
>  */
> +
> +static const char nand_16mib_18v_8bit[] = "NAND 16MiB 1,8V 8-bit";
> +static const char nand_16mib_33v_8bit[] = "NAND 16MiB 3,3V 8-bit";
> +static const char nand_16mib_18v_16bit[] = "NAND 16MiB 1,8V 16-bit";
> +static const char nand_16mib_33v_16bit[] = "NAND 16MiB 3,3V 16-bit";
> +
>  const struct nand_flash_dev nand_flash_ids[] = {
>  
>  #ifdef CONFIG_MTD_NAND_MUSEUM_IDS
> @@ -42,10 +48,10 @@
>  	{"NAND 8MiB 3,3V 16-bit",	0x59, 512, 8, 0x2000, NAND_BUSWIDTH_16},
>  #endif
>  
> -	{"NAND 16MiB 1,8V 8-bit",	0x33, 512, 16, 0x4000, 0},
> -	{"NAND 16MiB 3,3V 8-bit",	0x73, 512, 16, 0x4000, 0},
> -	{"NAND 16MiB 1,8V 16-bit",	0x43, 512, 16, 0x4000, NAND_BUSWIDTH_16},
> -	{"NAND 16MiB 3,3V 16-bit",	0x53, 512, 16, 0x4000, NAND_BUSWIDTH_16},
> +	{nand_16mib_18v_8bit,	0x33, 512, 16, 0x4000, 0},
> +	{nand_16mib_33v_8bit,	0x73, 512, 16, 0x4000, 0},
> +	{nand_16mib_18v_16bit,	0x43, 512, 16, 0x4000, NAND_BUSWIDTH_16},
> +	{nand_16mib_33v_16bit,	0x53, 512, 16, 0x4000, NAND_BUSWIDTH_16},
>  
>  	{"NAND 32MiB 1,8V 8-bit",	0x35, 512, 32, 0x4000, 0},
>  	{"NAND 32MiB 3,3V 8-bit",	0x75, 512, 32, 0x4000, 0},

Ugly, and we'd get regressions every time someone adds a new anonymous
string that pushes an SPL over the limit.

Unless/until the toolchain can properly GC anonymous strings, I think
finer-grained conditional compilation is the way to go.

-Scott

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Mike Frysinger]

On Monday 09 January 2012 14:41:07 Scott Wood wrote:
> Unless/until the toolchain can properly GC anonymous strings, I think
> finer-grained conditional compilation is the way to go.

i think we should use GC when it works (we've found 1 case so far where it 
doesn't), and add fine grained compilation when it doesn't (with a note with 
expected working versions).  i think this is a bug in the linker and have been 
trying to make a reduced test case that i can post to the GNU binutils peeps.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120109/2275fa1e/attachment.pgp>

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Tom Rini]

On Mon, Jan 9, 2012 at 2:21 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday 09 January 2012 14:41:07 Scott Wood wrote:
>> Unless/until the toolchain can properly GC anonymous strings, I think
>> finer-grained conditional compilation is the way to go.
>
> i think we should use GC when it works (we've found 1 case so far where it
> doesn't), and add fine grained compilation when it doesn't (with a note with
> expected working versions). ?i think this is a bug in the linker and have been
> trying to make a reduced test case that i can post to the GNU binutils peeps.

We've found two cases, at least.  Functions aren't being dropped out either.

-- 
Tom

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Tom Rini]

On Mon, Jan 9, 2012 at 2:23 PM, Tom Rini <tom.rini@gmail.com> wrote:
> On Mon, Jan 9, 2012 at 2:21 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> On Monday 09 January 2012 14:41:07 Scott Wood wrote:
>>> Unless/until the toolchain can properly GC anonymous strings, I think
>>> finer-grained conditional compilation is the way to go.
>>
>> i think we should use GC when it works (we've found 1 case so far where it
>> doesn't), and add fine grained compilation when it doesn't (with a note with
>> expected working versions). ?i think this is a bug in the linker and have been
>> trying to make a reduced test case that i can post to the GNU binutils peeps.
>
> We've found two cases, at least. ?Functions aren't being dropped out either.

To be clear, https://github.com/trini/u-boot/commit/3a7c313125eff000ac73e38d220e5f9615c1abbe
shows switching everything to the normal nand codebase, but with a
gain of only 170 bytes.  The reason it's such an otherwise small delta
is that I added some #ifndef CONFIG_SPL_BUILD to drop out unused code
that the linker should have been dropping for us, but wasn't.

-- 
Tom

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Mike Frysinger]

On Monday 09 January 2012 16:25:41 Tom Rini wrote:
> On Mon, Jan 9, 2012 at 2:23 PM, Tom Rini <tom.rini@gmail.com> wrote:
> > On Mon, Jan 9, 2012 at 2:21 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> >> On Monday 09 January 2012 14:41:07 Scott Wood wrote:
> >>> Unless/until the toolchain can properly GC anonymous strings, I think
> >>> finer-grained conditional compilation is the way to go.
> >> 
> >> i think we should use GC when it works (we've found 1 case so far where
> >> it doesn't), and add fine grained compilation when it doesn't (with a
> >> note with expected working versions).  i think this is a bug in the
> >> linker and have been trying to make a reduced test case that i can post
> >> to the GNU binutils peeps.
> > 
> > We've found two cases, at least.  Functions aren't being dropped out
> > either.
> 
> To be clear,
> https://github.com/trini/u-boot/commit/3a7c313125eff000ac73e38d220e5f9615c
> 1abbe shows switching everything to the normal nand codebase, but with a
> gain of only 170 bytes.  The reason it's such an otherwise small delta is
> that I added some #ifndef CONFIG_SPL_BUILD to drop out unused code that
> the linker should have been dropping for us, but wasn't.

how exactly do i reproduce the issue ?  what do i `make xxx` ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120110/2cf92a12/attachment.pgp>

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Tom Rini]

On Tue, Jan 10, 2012 at 11:25 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday 09 January 2012 16:25:41 Tom Rini wrote:
>> On Mon, Jan 9, 2012 at 2:23 PM, Tom Rini <tom.rini@gmail.com> wrote:
>> > On Mon, Jan 9, 2012 at 2:21 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> >> On Monday 09 January 2012 14:41:07 Scott Wood wrote:
>> >>> Unless/until the toolchain can properly GC anonymous strings, I think
>> >>> finer-grained conditional compilation is the way to go.
>> >>
>> >> i think we should use GC when it works (we've found 1 case so far where
>> >> it doesn't), and add fine grained compilation when it doesn't (with a
>> >> note with expected working versions). ?i think this is a bug in the
>> >> linker and have been trying to make a reduced test case that i can post
>> >> to the GNU binutils peeps.
>> >
>> > We've found two cases, at least. ?Functions aren't being dropped out
>> > either.
>>
>> To be clear,
>> https://github.com/trini/u-boot/commit/3a7c313125eff000ac73e38d220e5f9615c
>> 1abbe shows switching everything to the normal nand codebase, but with a
>> gain of only 170 bytes. ?The reason it's such an otherwise small delta is
>> that I added some #ifndef CONFIG_SPL_BUILD to drop out unused code that
>> the linker should have been dropping for us, but wasn't.
>
> how exactly do i reproduce the issue ? ?what do i `make xxx` ?

On that tree, build for devkit8000.

-- 
Tom

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Scott Wood]

On 01/09/2012 03:21 PM, Mike Frysinger wrote:
> On Monday 09 January 2012 14:41:07 Scott Wood wrote:
>> Unless/until the toolchain can properly GC anonymous strings, I think
>> finer-grained conditional compilation is the way to go.
> 
> i think we should use GC when it works (we've found 1 case so far where it 
> doesn't), and add fine grained compilation when it doesn't (with a note with 
> expected working versions).

Anonymous strings are hardly a rare occurance.  Plus, doing it on a case
by cases invites breakage from people who don't/can't test the SPLs that
are affected, when they add code that moves a certain file from being
not-problematic to problematic.

> i think this is a bug in the linker and have been 
> trying to make a reduced test case that i can post to the GNU binutils peeps.

If and when they fix it, and the updated toolchains are in sufficiently
wide use that we can stop supporting the old ones (at least for small
SPL targets), we can revisit the decision.

-Scott

[U-Boot] [PATCH] NAND: Allow nand_ids and nand_bbt to be compiled in SPL

[Mike Frysinger]

On Monday 09 January 2012 16:27:44 Scott Wood wrote:
> On 01/09/2012 03:21 PM, Mike Frysinger wrote:
> > On Monday 09 January 2012 14:41:07 Scott Wood wrote:
> >> Unless/until the toolchain can properly GC anonymous strings, I think
> >> finer-grained conditional compilation is the way to go.
> > 
> > i think we should use GC when it works (we've found 1 case so far where
> > it doesn't), and add fine grained compilation when it doesn't (with a
> > note with expected working versions).
> 
> Anonymous strings are hardly a rare occurance.

it isn't all anonymous strings.  there's something more going on here that is 
tickling the bug.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120110/a190a7aa/attachment.pgp>