[coreboot] Supporting boards with NAND flash on other chip selects than CS0

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu Nov 6 13:49:42 CET 2008


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 <mart.raudsepp at artecdesign.ee>
> 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 <mart.raudsepp at artecdesign.ee>
>
> --- 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 <c-d.hailfinger.devel.2006 at gmx.net>

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list