Patchwork Set the register based on the ROMSIZE (Patch is updated)

login
register
about
Submitter Bao, Zheng
Date 2010-12-13 09:47:33
Message ID <DD1CC71B621B004FA76856E5129D6B170459DF2B@sbjgexmb1.amd.com>
Download mbox | patch
Permalink /patch/2427/
State Accepted
Commit r6178
Headers show

Comments

Bao, Zheng - 2010-12-13 09:47:33
Set the ROMSIZE as 4MB.

Signed-off-by: Zheng Bao <zheng.bao@amd.com>


  *          PN 43366_sb7xx_bdg_pub_1.00, June 2009, section 3.1, page
14.
@@ -39,7 +39,7 @@
 	device_t dev;
 
 	dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_ATI,
-				PCI_DEVICE_ID_ATI_SB700_LPC), 0);
+				       PCI_DEVICE_ID_ATI_SB700_LPC), 0);
 
 	/* Decode variable LPC ROM address ranges 1 and 2. */
 	reg8 = pci_read_config8(dev, 0x48);
@@ -57,8 +57,10 @@
 	 * Enable LPC ROM range start at:
 	 * 0xfff8(0000): 512KB
 	 * 0xfff0(0000): 1MB
+	 * 0xffe0(0000): 2MB
+	 * 0xffc0(0000): 4MB
 	 */
-	pci_write_config16(dev, 0x6c, 0xfff0); /* 1 MB */
+	pci_write_config16(dev, 0x6c, 0xffc0); /* 4 MB */
 	/* Enable LPC ROM range end at 0xffff(ffff). */
 	pci_write_config16(dev, 0x6e, 0xffff);
 }

> -----Original Message-----
> From: coreboot-bounces@coreboot.org
[mailto:coreboot-bounces@coreboot.org]
> On Behalf Of Stefan Reinauer
> Sent: Saturday, December 11, 2010 2:48 AM
> To: Scott Duplichan
> Cc: 'Peter Stuge'; coreboot@coreboot.org
> Subject: Re: [coreboot] [PATCH] Set the register based on the ROMSIZE
> 
> * Scott Duplichan <scott@notabs.org> [101210 18:13]:
> > Peter Stuge wrote:
> >
> > ]I have a greater problem with this;
> > ]
> > ]It assumes that one image will always go into one and the same size
> > ]of flash chip.
> >
> > I think I understand your concern. It is convenient to be able to
> > use a bigger chip to test a smaller image. But limiting the decode
> > size will only block access to the usused portion of the chip,
correct?
> 
> One of the original design ideas of CBFS (or LAR in v3) was that we
can
> build a single image, with, say 256KB, and resize that same image
later
> on in case someone wants to put in fallback/normal, a linux kernel
> payload or stuff like that.
> 
> Stefan
> 
> --
> coreboot mailing list: coreboot@coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
Set the ROMSIZE as 4MB.

Signed-off-by: Zheng Bao <zheng.bao@amd.com>
Peter Stuge - 2010-12-13 18:54:47
Bao, Zheng wrote:
> Set the ROMSIZE as 4MB.
> 
> Signed-off-by: Zheng Bao <zheng.bao@amd.com>

Thanks!

Acked-by: Peter Stuge <peter@stuge.se>
Uwe Hermann - 2010-12-14 02:05:07
Hi,

On Mon, Dec 13, 2010 at 05:47:33PM +0800, Bao, Zheng wrote:
> +config BOOTBLOCK_SOUTHBRIDGE_INIT
> +	string
> +	default "southbridge/amd/sb600/bootblock.c"
> +	depends on SOUTHBRIDGE_AMD_SB600
> +
[...]  
> +config BOOTBLOCK_SOUTHBRIDGE_INIT
> +	string
> +	default "southbridge/amd/sb700/bootblock.c"
> +	depends on SOUTHBRIDGE_AMD_SB700
> +

Thanks for adding these, looks like I forgot them in the TINY_BOOTBLOCK
patches.


> - * Enable 1MB (LPC) ROM access at 0xFFF00000 - 0xFFFFFFFF.
> + * Enable 4MB (LPC) ROM access at 0xFFC00000 - 0xFFFFFFFF.
>   *
>   * Hardware should enable LPC ROM by pin straps. This function does not
>   * handle the theoretically possible PCI ROM, FWH, or SPI ROM
> configurations.
>   *
> - * The SB700 power-on default is to map 256K ROM space.
> + * The SB700 power-on default is to map 512K ROM space.

Nice! I guess these new values are correct, however I could not find
this specified in the SB700 datasheets (?) Is this undocumented or did
I not look in the right place?

The BKDG at
http://support.amd.com/us/Embedded_TechDocs/43366_sb7xx_bdg_pub_1.00.pdf
says:

"Upon system power on, the SB700 enables 256K ROM by default. The BIOS needs
to enable 512K ROM or up to 1M for LPC ROM, if required."

It also only mentions 512KB and 1MB configs (no mention of up to 4MB
being supported). Is the datasheet outdated maybe?


Thanks, Uwe.
Bao, Zheng - 2010-12-14 02:16:35
I also start to be confused.

In the sb700 rrg, it says the default value of reg [0x14,3,0x6c] is
0xFFF8, which means 512K. But actually, I took a look at the real board
with a blank BIOS chip. The default value is 0xFFF0, which means 1MB.

I think we should believe the real thing other than the datasheet when
they conflict.

Zheng


> -----Original Message-----
> From: Uwe Hermann [mailto:uwe@hermann-uwe.de]
> Sent: Tuesday, December 14, 2010 10:05 AM
> To: Bao, Zheng
> Cc: Stefan Reinauer; Scott Duplichan; Peter Stuge;
coreboot@coreboot.org
> Subject: Re: [coreboot] [PATCH] Set the register based on the ROMSIZE
> (Patch is updated)
> 
> Hi,
> 
> On Mon, Dec 13, 2010 at 05:47:33PM +0800, Bao, Zheng wrote:
> > +config BOOTBLOCK_SOUTHBRIDGE_INIT
> > +	string
> > +	default "southbridge/amd/sb600/bootblock.c"
> > +	depends on SOUTHBRIDGE_AMD_SB600
> > +
> [...]
> > +config BOOTBLOCK_SOUTHBRIDGE_INIT
> > +	string
> > +	default "southbridge/amd/sb700/bootblock.c"
> > +	depends on SOUTHBRIDGE_AMD_SB700
> > +
> 
> Thanks for adding these, looks like I forgot them in the
TINY_BOOTBLOCK
> patches.
> 
> 
> > - * Enable 1MB (LPC) ROM access at 0xFFF00000 - 0xFFFFFFFF.
> > + * Enable 4MB (LPC) ROM access at 0xFFC00000 - 0xFFFFFFFF.
> >   *
> >   * Hardware should enable LPC ROM by pin straps. This function does
not
> >   * handle the theoretically possible PCI ROM, FWH, or SPI ROM
> > configurations.
> >   *
> > - * The SB700 power-on default is to map 256K ROM space.
> > + * The SB700 power-on default is to map 512K ROM space.
> 
> Nice! I guess these new values are correct, however I could not find
> this specified in the SB700 datasheets (?) Is this undocumented or did
> I not look in the right place?
> 
> The BKDG at
>
http://support.amd.com/us/Embedded_TechDocs/43366_sb7xx_bdg_pub_1.00.pdf
> says:
> 
> "Upon system power on, the SB700 enables 256K ROM by default. The BIOS
> needs
> to enable 512K ROM or up to 1M for LPC ROM, if required."
> 
> It also only mentions 512KB and 1MB configs (no mention of up to 4MB
> being supported). Is the datasheet outdated maybe?
> 
> 
> Thanks, Uwe.
> --
> http://hermann-uwe.de     | http://sigrok.org
> http://randomprojects.org | http://unmaintained-free-software.org
Scott - 2010-12-14 04:19:18
]I also start to be confused.

]In the sb700 rrg, it says the default value of reg [0x14,3,0x6c] is
]0xFFF8, which means 512K. But actually, I took a look at the real board
]with a blank BIOS chip. The default value is 0xFFF0, which means 1MB.

Yes, you are correct. The documentation is wrong. This creates a problem
when booting on simnow because simnow matches the documentation, and not
the real BIOS. When I build Mahogany or Kino for a 1MB flash chip, it
will run on real hardware but fail on simnow. I have to use a private 
patch or modify the 6C register manually to test Mahogany or Kino on simnow.

Thanks,
Scott

]I think we should believe the real thing other than the datasheet when
]they conflict.

]Zheng


> -----Original Message-----
> From: Uwe Hermann [mailto:uwe@hermann-uwe.de]
> Sent: Tuesday, December 14, 2010 10:05 AM
> To: Bao, Zheng
> Cc: Stefan Reinauer; Scott Duplichan; Peter Stuge;
coreboot@coreboot.org
> Subject: Re: [coreboot] [PATCH] Set the register based on the ROMSIZE
> (Patch is updated)
> 
> Hi,
> 
> On Mon, Dec 13, 2010 at 05:47:33PM +0800, Bao, Zheng wrote:
> > +config BOOTBLOCK_SOUTHBRIDGE_INIT
> > +	string
> > +	default "southbridge/amd/sb600/bootblock.c"
> > +	depends on SOUTHBRIDGE_AMD_SB600
> > +
> [...]
> > +config BOOTBLOCK_SOUTHBRIDGE_INIT
> > +	string
> > +	default "southbridge/amd/sb700/bootblock.c"
> > +	depends on SOUTHBRIDGE_AMD_SB700
> > +
> 
> Thanks for adding these, looks like I forgot them in the
TINY_BOOTBLOCK
> patches.
> 
> 
> > - * Enable 1MB (LPC) ROM access at 0xFFF00000 - 0xFFFFFFFF.
> > + * Enable 4MB (LPC) ROM access at 0xFFC00000 - 0xFFFFFFFF.
> >   *
> >   * Hardware should enable LPC ROM by pin straps. This function does
not
> >   * handle the theoretically possible PCI ROM, FWH, or SPI ROM
> > configurations.
> >   *
> > - * The SB700 power-on default is to map 256K ROM space.
> > + * The SB700 power-on default is to map 512K ROM space.
> 
> Nice! I guess these new values are correct, however I could not find
> this specified in the SB700 datasheets (?) Is this undocumented or did
> I not look in the right place?
> 
> The BKDG at
>
http://support.amd.com/us/Embedded_TechDocs/43366_sb7xx_bdg_pub_1.00.pdf
> says:
> 
> "Upon system power on, the SB700 enables 256K ROM by default. The BIOS
> needs
> to enable 512K ROM or up to 1M for LPC ROM, if required."
> 
> It also only mentions 512KB and 1MB configs (no mention of up to 4MB
> being supported). Is the datasheet outdated maybe?

The sb700 rrg documentation for register 6C in function 3 indirectly
states 4GB and greater is supported. Some LPC flash chips (SST49LF004B)
locate their manufacturer and device ID code at FFBC0000, so decoding
this address must be supported.
developer.amd.com/assets/43009_sb7xx_rrg_pub_1.00.pdf

Thanks,
Scott

> 
> 
> Thanks, Uwe.
> --
> http://hermann-uwe.de     | http://sigrok.org
> http://randomprojects.org | http://unmaintained-free-software.org

Patch

Index: src/southbridge/amd/sb600/Kconfig
===================================================================
--- src/southbridge/amd/sb600/Kconfig	(revision 6169)
+++ src/southbridge/amd/sb600/Kconfig	(working copy)
@@ -23,6 +23,11 @@ 
 	select HAVE_USBDEBUG
 	select TINY_BOOTBLOCK
 
+config BOOTBLOCK_SOUTHBRIDGE_INIT
+	string
+	default "southbridge/amd/sb600/bootblock.c"
+	depends on SOUTHBRIDGE_AMD_SB600
+
 config EHCI_BAR
 	hex
 	default 0xfef00000 if SOUTHBRIDGE_AMD_SB600
Index: src/southbridge/amd/sb700/Kconfig
===================================================================
--- src/southbridge/amd/sb700/Kconfig	(revision 6169)
+++ src/southbridge/amd/sb700/Kconfig	(working copy)
@@ -23,6 +23,11 @@ 
 	select HAVE_USBDEBUG
 	select TINY_BOOTBLOCK
 
+config BOOTBLOCK_SOUTHBRIDGE_INIT
+	string
+	default "southbridge/amd/sb700/bootblock.c"
+	depends on SOUTHBRIDGE_AMD_SB700
+
 config SOUTHBRIDGE_AMD_SB700_SKIP_ISA_DMA_INIT
 	bool
 	default n
Index: src/southbridge/amd/sb700/bootblock.c
===================================================================
--- src/southbridge/amd/sb700/bootblock.c	(revision 6169)
+++ src/southbridge/amd/sb700/bootblock.c	(working copy)
@@ -23,12 +23,12 @@ 
 #include <device/pci_ids.h>
 
 /*
- * Enable 1MB (LPC) ROM access at 0xFFF00000 - 0xFFFFFFFF.
+ * Enable 4MB (LPC) ROM access at 0xFFC00000 - 0xFFFFFFFF.
  *
  * Hardware should enable LPC ROM by pin straps. This function does not
  * handle the theoretically possible PCI ROM, FWH, or SPI ROM configurations.
  *
- * The SB700 power-on default is to map 256K ROM space.
+ * The SB700 power-on default is to map 512K ROM space.
  *
  * Details: AMD SB700/710/750 BIOS Developer's Guide (BDG), Rev. 1.00,
  *          PN 43366_sb7xx_bdg_pub_1.00, June 2009, section 3.1, page 14.
@@ -39,7 +39,7 @@ 
 	device_t dev;
 
 	dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_ATI,
-				PCI_DEVICE_ID_ATI_SB700_LPC), 0);
+				       PCI_DEVICE_ID_ATI_SB700_LPC), 0);
 
 	/* Decode variable LPC ROM address ranges 1 and 2. */
 	reg8 = pci_read_config8(dev, 0x48);
@@ -57,8 +57,10 @@ 
 	 * Enable LPC ROM range start at:
 	 * 0xfff8(0000): 512KB
 	 * 0xfff0(0000): 1MB
+	 * 0xffe0(0000): 2MB
+	 * 0xffc0(0000): 4MB
 	 */
-	pci_write_config16(dev, 0x6c, 0xfff0); /* 1 MB */
+	pci_write_config16(dev, 0x6c, 0xffc0); /* 4 MB */
 	/* Enable LPC ROM range end at 0xffff(ffff). */
 	pci_write_config16(dev, 0x6e, 0xffff);
 }