[coreboot] [PATCH] cs5536/nand: Allow setting of NAND timing values in the dts.

Mart Raudsepp mart.raudsepp at artecdesign.ee
Mon Feb 9 18:00:38 CET 2009


> On Mon, Feb 9, 2009 at 8:21 AM, Mart Raudsepp
> <mart.raudsepp at artecdesign.ee> wrote:
>> Ühel kenal päeval, E, 2009-02-09 kell 08:04, kirjutas ron minnich:
>>> nice to see people using the dts now!
>>
>> It's a bit hard to use it for the LBAR setup as well (the code in
>> chipset_flash_setup() in cs5536.c), as it needs to happen before VSA
>> init, and VSA init is done in northbridge domain phase2_fixup - so we
>> don't have a phase for NAND device to run before VSA is inited.
>> Otherwise I'd use the NAND initialization from chipsetinit() to some
>> nand_phase1 as well.
>>
>>> Acked-by: Ronald G. Minnich <rminnich at gmail.com>
>>
>> I'm going to have to NAK this myself, as it doesn't compile with my gcc
>> if there is no NAND device in the mainboard dts, as there is no
>> southbridge_amd_cs5536_nand_config in statictree.h then, and gcc
>> complains about
>> /home/leio/dev/coreboot-v3/southbridge/amd/cs5536/cs5536.c:119: error:
>> dereferencing pointer to incomplete type
>
> The problem is that you have to have a separate compilation unit if
> you have seperate dts objects.
>
> If you really want to separate out the nand dts, as you have done:
>                      /config/("southbridge/amd/cs5536/nand");
>
> then you need to have a southbridge/amd/cs5536/nand.c. See the other
> parts for example.

That means also I'm going to have two places where to enable or disable
devices - dts and Makefile. And I need to keep them in sync. And I need to
list southbridge C files in mainboard specific Makefile instead of in the
same directory.
It isn't very nice and scalable and neat.
Could we perhaps have dtc give me a DTS_HAVE_CS5536_NAND or similar
preprocessor definitions if the device is present in the static tree?

> If you want to keep the code in cs5536.c, then you'll have to put the
> settings in the cs5536 dts.

That seems like an abuse of the dts system

>> How should I approach this?
>
> See what I did in the sb600.

For the sake of argument, lets say every kilobyte matters as I might want
to also fit in a LAB in a 1MB limit. We could be talking about a whole lot
more complex code that is unconditionally compiled in unless doing ugly C
file listing in mainboard dir Makefiles than we are dealing with in this
instance.

>> On another note, can we cleanly avoid compiling all this NAND init code
>> in for the boards that surely do not have a NAND device as it can't be a
>> dynamic device?
>
> Yes. put it in its own file. That's what makefiles are for. Then those
> boards that have nand:
>
> add nand.c to the Makefile
> use the nand dts you set up.

The problem is that that listing is in a completely different Makefile
than where the file is at.

> (w.r.t. IDE)
>>Less
>> important though, as the code footprint is probably negligible.
>
> This is the really key point. We decided in a heated discussion a few
> months back that on, e.g., sb600, we'd compile all code in, whether
> used or not. But in your case, with this nand stuff, you might as
> seperate out the code in nand.c

As a completely off-topic slightly related to the above side note, I'm
still slightly annoyed at all loglevel strings getting compiled in now, no
questions asked. A maximum supported loglevel option would be nice. We
need no complete spew loglevel messages in a production ROM we
hypothetically burn into units sold to customers. Just haven't gotten
around to implement that after those changes I didn't manage to raise a
NAK on time :)


Regards,
Mart Raudsepp





More information about the coreboot mailing list