On 06.11.2008 13:13, Mart Raudsepp wrote: > Hello, > > Currently southbridge/amd/cs5536/cs5536.c:chipset_flash_setup() sets up > NAND flash if enable_ide_nand_flash is set to "1" in the device tree > based on a static const structure in the same file - FlashInitTable. > > The code itself supports having different NAND interfaces, specify extra > NOR for the setup and so on, based on the struct values. However in > practice this can't be used, because the struct is defined in the > southbridge code, and can't be overriden by board specific dts files > through any means - effectively telling things like "This board has NAND > on CS1" is not possible. > > Due to that the ArtecGroup ThinCan DBE61 and DBE62 do not set up NAND > flash properly in coreboot-v3. They could have (and DBE62 does) > enable_ide_nand_flash enabled in dts, but that enables it on CS0, and > not on CS1. > > I don't know how it's best to solve this. > I think the most generic way would be to make the whole 4 member array > struct definable or overridable in dts, but I don't think our dtc code > supports anything like that - at best we could have a variable length > array (cells) that is used for unwanted_vpci as well, telling that it > has to come in three member chunks, where first tells the type, second > the interface and third the (page size?) mask. But that is very > suboptimal too - a zero would terminate the array immediately, the type > is really an enum, not a 4 byte cell, etc. > > Another option is to just assume there is only one NAND to enable, and > give the chip select location in dts - this doesn't regress what is in > practice currently supported, while being the minimum necessary to setup > up NAND for ThinCan boards properly. An initial patch for that is > attached. > > Or maybe the call to chipset_flash_setup() should be pushed down to be > the responsibility of board specific C code, so it could pass in its own > FlashInitTable, after chipset_flash_setup is modified to take such a > struct instead of using the file global const struct defined in the > southbridge file? > > Thoughts? > > > Mart Raudsepp > Artec Design LLC > > > ------------------------------------------------------------------------ > > Subject: > [PATCH] cs5536: Support NAND flash on other locations than CS0 > From: > Mart Raudsepp <[EMAIL PROTECTED]> > Date: > Thu, 6 Nov 2008 13:36:20 +0200 > > > Modify chipset_flash_setup to support enabling NAND flash on other locations > than CS0, by making enable_ide_nand_flash have a non-boolean meaning where > zero > means no NAND (IDE), and 1 through 4 gives the one-based chip select array > location (so 1 means CS0, 2 means CS1, 3 means CS2 and 4 means CS3, as chip > select notation is zero-based). > > This loses the code for supporting more than one NAND chip select or different > ones than FLASH_MEM_4K, but these couldn't be supported before anyway, because > that is board specific, but the supporting structure was a static const struct > in generic southbridge specific code. > This support should be instead implemented via the device tree dts files. > > Enables NAND on ArtecGroup DBE61 and DBE62 on CS1, as that's where it is. > The end result is that these mainboards can now boot off of NAND with FILO > without local modifications to the previously existing southbridge specific > static const struct that had no chance of being upstreamed as it would break > all other CS5536 NAND boards that have it on CS0. > > Signed-off-by: Mart Raudsepp <[EMAIL PROTECTED]> > > --- a/mainboard/artecgroup/dbe61/dts > +++ b/mainboard/artecgroup/dbe61/dts > @@ -22,7 +22,7 @@ > > /* > chip southbridge/amd/cs5536_lx > - register "enable_ide_nand_flash" = "0" > + register "enable_ide_nand_flash" = "2" > > register "isa_irq" = "0" > #register "flash_irq" = "14" > >
You patched the config.lb leftovers in that dts. Please fix the real dts definitions instead. Acked-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]> Regards, Carl-Daniel -- http://www.hailfinger.org/ -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
