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
--- Begin Message ---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 looses 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]> --- mainboard/artecgroup/dbe61/dts | 2 +- mainboard/artecgroup/dbe62/dts | 4 +- southbridge/amd/cs5536/cs5536.c | 68 ++++++++++++++------------------------- southbridge/amd/cs5536/dts | 2 +- 4 files changed, 28 insertions(+), 48 deletions(-) diff --git a/mainboard/artecgroup/dbe61/dts b/mainboard/artecgroup/dbe61/dts index 69a6217..6eaaf8a 100644 --- 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" diff --git a/mainboard/artecgroup/dbe62/dts b/mainboard/artecgroup/dbe62/dts index aaa08a0..67c4d63 100644 --- a/mainboard/artecgroup/dbe62/dts +++ b/mainboard/artecgroup/dbe62/dts @@ -44,8 +44,8 @@ /* GPIO(0-0x20) for INT D:C:B:A, 0xFF=none. * See virtual PIC spec. */ enable_gpio_int_route = "0x0D0C0700"; - /* 0:IDE 1:FLASH */ - enable_ide_nand_flash = "1"; + /* 0:IDE; 1:FLASH on CS0, 2:FLASH on CS1, 3:FLASH on CS2, 4:FLASH on CS3. */ + enable_ide_nand_flash = "2"; /* we use com2 since that is on the dongle */ com2_enable = "1"; /* Set com2 address to be COM1 */ diff --git a/southbridge/amd/cs5536/cs5536.c b/southbridge/amd/cs5536/cs5536.c index 62d59b2..a4a7327 100644 --- a/southbridge/amd/cs5536/cs5536.c +++ b/southbridge/amd/cs5536/cs5536.c @@ -68,19 +68,6 @@ static const struct acpi_init acpi_init_table[] = { {0, 0} }; -struct FLASH_DEVICE { - unsigned char fType; /* Flash type: NOR or NAND */ - unsigned char fInterface; /* Flash interface: I/O or memory */ - unsigned long fMask; /* Flash size/mask */ -}; - -static const struct FLASH_DEVICE FlashInitTable[] = { - {FLASH_TYPE_NAND, FLASH_IF_MEM, FLASH_MEM_4K}, /* CS0, or Flash Device 0 */ - {FLASH_TYPE_NONE, 0, 0}, /* CS1, or Flash Device 1 */ - {FLASH_TYPE_NONE, 0, 0}, /* CS2, or Flash Device 2 */ - {FLASH_TYPE_NONE, 0, 0}, /* CS3, or Flash Device 3 */ -}; - static const u32 FlashPort[] = { MDD_LBAR_FLSH0, MDD_LBAR_FLSH1, @@ -154,38 +141,31 @@ static void pm_chipset_init(void) * correct size info. Call this routine only if flash needs to be * configured (don't call it if you want IDE). */ -static void chipset_flash_setup(void) +static void chipset_flash_setup(struct southbridge_amd_cs5536_dts_config *sb) { int i; struct msr msr; printk(BIOS_DEBUG, "chipset_flash_setup: Start\n"); - for (i = 0; i < ARRAY_SIZE(FlashInitTable); i++) { - if (FlashInitTable[i].fType != FLASH_TYPE_NONE) { - printk(BIOS_DEBUG, "Enable CS%d\n", i); - /* We need to configure the memory/IO mask. */ - msr = rdmsr(FlashPort[i]); - msr.hi = 0; /* Start with "enabled" bit clear. */ - if (FlashInitTable[i].fType == FLASH_TYPE_NAND) - msr.hi |= 0x00000002; - else - msr.hi &= ~0x00000002; - if (FlashInitTable[i].fInterface == FLASH_IF_MEM) - msr.hi |= 0x00000004; - else - msr.hi &= ~0x00000004; - msr.hi |= FlashInitTable[i].fMask; - printk(BIOS_DEBUG, "MSR(0x%08X, %08X_%08X)\n", - FlashPort[i], msr.hi, msr.lo); - wrmsr(FlashPort[i], msr); - - /* Now write-enable the device. */ - msr = rdmsr(MDD_NORF_CNTRL); - msr.lo |= (1 << i); - printk(BIOS_DEBUG, "MSR(0x%08X, %08X_%08X)\n", - MDD_NORF_CNTRL, msr.hi, msr.lo); - wrmsr(MDD_NORF_CNTRL, msr); - } + if (sb->enable_ide_nand_flash <= 4) { + i = sb->enable_ide_nand_flash - 1; + printk(BIOS_DEBUG, "Enable CS%d\n", i); + /* We need to configure the memory/IO mask. */ + msr = rdmsr(FlashPort[i]); + msr.hi = 0; /* Start with "enabled" bit clear. */ + msr.hi |= 0x00000002; /* For FLASH_TYPE_NAND */ + msr.hi |= 0x00000004; /* For FLASH_IF_MEM */ + msr.hi |= FLASH_MEM_4K; + printk(BIOS_DEBUG, "MSR(0x%08X, %08X_%08X)\n", + FlashPort[i], msr.hi, msr.lo); + wrmsr(FlashPort[i], msr); + + /* Now write-enable the device. */ + msr = rdmsr(MDD_NORF_CNTRL); + msr.lo |= (1 << i); + printk(BIOS_DEBUG, "MSR(0x%08X, %08X_%08X)\n", + MDD_NORF_CNTRL, msr.hi, msr.lo); + wrmsr(MDD_NORF_CNTRL, msr); } printk(BIOS_DEBUG, "chipset_flash_setup: Finish\n"); } @@ -596,9 +576,9 @@ void chipsetinit(void) /* Flash BAR size setup. */ printk(BIOS_ERR, "%sDoing chipset_flash_setup()\n", - sb->enable_ide_nand_flash == 1 ? "" : "Not "); - if (sb->enable_ide_nand_flash == 1) - chipset_flash_setup(); + sb->enable_ide_nand_flash != 0 ? "" : "Not "); + if (sb->enable_ide_nand_flash != 0) + chipset_flash_setup(sb); /* Set up hardware clock gating. */ /* TODO: Why the extra block here? Can it be removed? */ @@ -680,7 +660,7 @@ static void southbridge_init(struct device *dev) printk(BIOS_ERR, "cs5536: %s: enable_ide_nand_flash is %d\n", __FUNCTION__, sb->enable_ide_nand_flash); - if (sb->enable_ide_nand_flash == 1) + if (sb->enable_ide_nand_flash != 0) enable_ide_nand_flash_header(); enable_USB_port4(sb); diff --git a/southbridge/amd/cs5536/dts b/southbridge/amd/cs5536/dts index dcfda42..c714a73 100644 --- a/southbridge/amd/cs5536/dts +++ b/southbridge/amd/cs5536/dts @@ -33,7 +33,7 @@ /* GPIO(0-0x20) for INT D:C:B:A, 0xFF=none. See virtual PIC spec. */ enable_gpio_int_route = "0"; - /* 0:IDE 1:FLASH, if you are using NAND flash instead of IDE drive. */ + /* 0:IDE; 1:FLASH on CS0, 2:FLASH on CS1, 3:FLASH on CS2, 4:FLASH on CS3. */ enable_ide_nand_flash = "0"; /* Enable USB Port 4 (0:host 1:device). -- 1.6.0.2
--- End Message ---
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
