cs5536/nand: Allow setting of NAND timing values in the dts.
The reset value for NAND timings is the slowest possible for Flash interface. Implement optionally setting it to a different value inside the NAND device. Set it to appropriate values for Artec Group DBE61 and DBE62. This results in a roughly two times quicker read time as measured by hdparm for these boards.
Signed-off-by: Mart Raudsepp mart.raudsepp@artecdesign.ee --- mainboard/artecgroup/dbe61/dts | 5 ++++- mainboard/artecgroup/dbe62/dts | 5 ++++- southbridge/amd/cs5536/cs5536.c | 18 ++++++++++++++++++ southbridge/amd/cs5536/nand | 4 ++++ 4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/mainboard/artecgroup/dbe61/dts b/mainboard/artecgroup/dbe61/dts index a6995ce..cb46292 100644 --- a/mainboard/artecgroup/dbe61/dts +++ b/mainboard/artecgroup/dbe61/dts @@ -109,8 +109,11 @@ end /* USB Port Power Handling setting. */ pph = "0xf5"; }; - pci@f,1 { + pci@f,1 { /* NAND Flash */ /config/("southbridge/amd/cs5536/nand"); + /* Timings */ + nandf_data = "0x01200120"; + nandf_ctl = "0x00000120"; }; }; }; diff --git a/mainboard/artecgroup/dbe62/dts b/mainboard/artecgroup/dbe62/dts index 8089e79..73dc5a7 100644 --- a/mainboard/artecgroup/dbe62/dts +++ b/mainboard/artecgroup/dbe62/dts @@ -63,8 +63,11 @@ /* USB Port Power Handling setting. */ pph = "0xf5"; }; - pci@f,1 { + pci@f,1 { /* NAND Flash */ /config/("southbridge/amd/cs5536/nand"); + /* Timings */ + nandf_data = "0x01200120"; + nandf_ctl = "0x00000120"; }; }; }; diff --git a/southbridge/amd/cs5536/cs5536.c b/southbridge/amd/cs5536/cs5536.c index 5437937..ea90fd4 100644 --- a/southbridge/amd/cs5536/cs5536.c +++ b/southbridge/amd/cs5536/cs5536.c @@ -109,6 +109,24 @@ static void hide_vpci(u32 vpci_devid) static void nand_phase2(struct device *dev) { if (dev->enabled) { + struct southbridge_amd_cs5536_nand_config *nand; + struct msr msr; + + /* Set up timings */ + nand = (struct southbridge_amd_cs5536_nand_config *)dev->device_configuration; + msr.hi = 0x0; + + if (nand->nandf_data) { + msr.lo = nand->nandf_data; + wrmsr(MDD_NANDF_DATA, msr); + printk(BIOS_DEBUG, "NANDF_DATA set to 0x%08x\n", msr.lo); + } + if (nand->nandf_ctl) { + msr.lo = nand->nandf_ctl; + wrmsr(MDD_NANDF_CTL, msr); + printk(BIOS_DEBUG, "NANDF_CTL set to 0x%08x\n", msr.lo); + } + /* Tell VSA to use FLASH PCI header. Not IDE header. */ hide_vpci(0x800079C4); } diff --git a/southbridge/amd/cs5536/nand b/southbridge/amd/cs5536/nand index 69f4fa4..62c28b0 100644 --- a/southbridge/amd/cs5536/nand +++ b/southbridge/amd/cs5536/nand @@ -20,4 +20,8 @@
{ device_operations = "cs5536_nand"; + + /* NAND timings per data book and NAND chip on board. 0x0 leaves to reset value. */ + nandf_data = "0x0"; + nandf_ctl = "0x0"; };
nice to see people using the dts now!
Acked-by: Ronald G. Minnich rminnich@gmail.com
Ü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
So the problem here is that while this code is only ran if the static dts has a NAND device (and it goes inside the conditional where the struct is dereferenced only if the device is enabled), it is always compiled in. Therefore we can't really dereference things in this manner if the device can ever be missing from statictree.h, which is the case if omitted from mainboard dts as it is right now.
How should I approach this? Add dummy NAND devices to all boards that say "disabled;"? Access the config structure some other way? Introduce a dirty HAVE_CS5536_NAND that duplicates dts NAND device enable? Have dtc emit a structure for it even if nothing sources the southbridge nand.dts? (that's quite surely a "no" though, just bringing out all possibilities I can imagine)
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? Same deal with IDE code on boards using NAND. Less important though, as the code footprint is probably negligible.
Regards, Mart Raudsepp
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.
If you want to keep the code in cs5536.c, then you'll have to put the settings in the cs5536 dts.
How should I approach this?
See what I did in the sb600.
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.
(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
ron
-----Original Message----- From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] On Behalf Of Mart Raudsepp Sent: Monday, February 09, 2009 9:22 AM To: coreboot@coreboot.org Subject: Re: [coreboot] [PATCH] cs5536/nand: Allow setting of NAND timing values in the dts.
Ü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
We have this problem with IDE flags on the AMD 8111 chipset too. I'm not sure what the best way to do it is. So far we've just been forced to include the (possibly disabled) device in the dts.
Thanks, Myles
On Mon, Feb 9, 2009 at 8:36 AM, Myles Watson mylesgw@gmail.com wrote:
We have this problem with IDE flags on the AMD 8111 chipset too. I'm not sure what the best way to do it is. So far we've just been forced to include the (possibly disabled) device in the dts.
That was a consequence of the decision we made on this list, just ref. the sb600 discussion, that the code to be compiled is listed in the Makefile for the part (e.g. southbridge), not the mainboard.
I don't see a huge problem with putting disabled devices in the mainboard dts. Yes, we see it all the time as we create the mainboard dts, but the person who actually builds ROMs won't be paying that much attention to the mainboard dts.
ron
Mart Raudsepp wrote:
/* Timings */nandf_data = "0x01200120";nandf_ctl = "0x00000120";
ron minnich wrote:
nice to see people using the dts now!
Is there any way those values could be expressed in a more human readable form? Do we even want to move dts values in that direction?
//Peter
On Mon, Feb 9, 2009 at 8:21 AM, Peter Stuge peter@stuge.se wrote:
Is there any way those values could be expressed in a more human readable form? Do we even want to move dts values in that direction?
yes, because there will inevitably be chipsets (most likely from intel) for which we know certain values but we can not say why they are what they are.
ron
-
Mart Raudsepp -
Myles Watson -
Peter Stuge -
ron minnich