On Mon, Feb 9, 2009 at 8:21 AM, Mart Raudsepp mart.raudsepp@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@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