Re: [coreboot] [Patch] Flashrom: Add nforce2 chipset enable.

[Uwe Hermann]

On Mon, May 25, 2009 at 02:41:06PM +0200, Luc Verhaegen wrote:
> Add NForce2 chipset enable.

Thanks a lot! I tested it on an ASUS A7N8X Deluxe (rev. 2.00) and it
does indeed work fine (erase/write doesn't work before the patch).

(Sidenote: the PCB says "A7N8X rev 2.00" and doesn't mention "Deluxe",
but according to the websites and dmidecode it really is a Deluxe):

http://www.asus.com/product.aspx?P_ID=Uz3L1ioFuZkCI0VO
http://www.asus.com/product.aspx?P_ID=wAsRYm41KTp78MFC

(I have two SATA ports and two NIC ports, which hints at the Deluxe)

dmidecode also says:
Version: ASUS A7N8X2.0 Deluxe ACPI BIOS Rev 1007
> This should, hopefully, fix the ASUS A7N8X-E.

I'm not so sure here. The original wiki report was done by me I think,
but that was a report for some other board I had temporary access to,
I guess. I'd rather not touch the ASUS A7N8X-E Deluxe status for now, until
somebody has actually tested it on hardware.

However, please add the "ASUS A7N8X Deluxe" (note the missing "-E" in
the name) to the known-good-without-board-enables-required list
(boards_ok[]) in board_enable.c. I have verified that it works with your
nforce2 patch and does _not_ require further board-enables.
> While the other chipset enables for nvidia could potentially also work,
> this one, by not touching other bits, seems like the safest solution.
>
> Signed-off-by Luc Verhaegen

Acked-by: Uwe Hermann

with the changed mentioned in this mail.
> Index: chipset_enable.c
>
> --- chipset_enable.c	(revision 545)
> +++ chipset_enable.c	(working copy)
> @@ -769,6 +769,19 @@
>    return 0;
>  }
>
> +static int enable_flash_nvidia_nforce2(struct pci_dev *dev, const char *name)
> +{
> uint8_t tmp;
> pci_write_byte(dev, 0x92, 0);
> tmp = pci_read_byte(dev, 0x6d);
> tmp |= 0x01;
> pci_write_byte(dev, 0x6d, tmp);
> return 0;

Please use tabs for indentation as per coding guidelines.
> +}
>
> static int enable_flash_ck804(struct pci_dev *dev, const char *name)
>  {
>    uint8_t old, new;
> @@ -981,6 +994,7 @@
>    {0x8086, 0x122e, OK, "Intel", "PIIX",		enable_flash_piix4},
>    {0x10de, 0x0050, OK, "NVIDIA", "CK804",		enable_flash_ck804}, /* LPC */
>    {0x10de, 0x0051, OK, "NVIDIA", "CK804",		enable_flash_ck804}, /* Pro */
> {0x10de, 0x0060, NT, "NVIDIA", "NForce2",       enable_flash_nvidia_nforce2},

^^
Can be "OK" now, as I tested this on hardware.

Hm, and maybe change "enable_flash_nvidia_nforce2" into
"enable_flash_nforce2" for brevity and consistency with
enable_flash_ck804 et al... (i.e. drop the "nvidia" part).
> /* Slave, should not be here, to fix known bug for A01. */
>    {0x10de, 0x00d3, OK, "NVIDIA", "CK804",		enable_flash_ck804},
>    {0x10de, 0x0260, NT, "NVIDIA", "MCP51",		enable_flash_ck804},
> Index: board_enable.c
> ===================================================================
> --- board_enable.c	(revision 546)
> +++ board_enable.c	(working copy)
> @@ -784,7 +784,6 @@
>  const struct board_info boards_bad[] = {
>    /* Verified non-working boards (for now). */
>    { "Abit",		"IS-10", },
> { "ASUS",		"A7N8X-E Deluxe", },

Please drop this hunk until we have a test report for that hardware.

Uwe.