Change in ...coreboot[master]: {gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32608

Change subject: {gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function
......................................................................

{gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function

Use already defined system_reset() and full_reset() functions.

Change-Id: Ic29fab70cf7f23d49c3eeeb97c984c523f973972
Signed-off-by: Elyes HAOUAS 
---
M src/northbridge/intel/gm45/early_reset.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/romstage.c
M src/northbridge/intel/x4x/raminit.c
4 files changed, 7 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/32608/1

diff --git a/src/northbridge/intel/gm45/early_reset.c b/src/northbridge/intel/gm45/early_reset.c
index 9f919cf..2a764aa 100644
--- a/src/northbridge/intel/gm45/early_reset.c
+++ b/src/northbridge/intel/gm45/early_reset.c
@@ -16,6 +16,7 @@

#include <types.h>
 #include <arch/io.h>
+#include <cf9_reset.h>
 #include <device/pci_ops.h>
 #include <halt.h>
 #include "gm45.h"
@@ -65,6 +66,5 @@

/* Perform system reset through CF9 interface. */
    outb(0x02, 0xcf9); /* Set system reset bit. */
-	outb(0x06, 0xcf9); /* Set CPU reset bit, too. */
-	halt();
+	system_reset(); /* Set CPU reset bit, too. */
 }
diff --git a/src/northbridge/intel/pineview/raminit.c b/src/northbridge/intel/pineview/raminit.c
index 48bca36..4e7e03f 100644
--- a/src/northbridge/intel/pineview/raminit.c
+++ b/src/northbridge/intel/pineview/raminit.c
@@ -15,6 +15,7 @@
  */

#include <arch/io.h>
+#include <cf9_reset.h>
 #include <device/mmio.h>
 #include <device/pci_ops.h>
 #include <console/console.h>
@@ -1746,7 +1747,7 @@
    pci_write_config8(PCI_DEV(0, 0x1f, 0), 0xa4, pmcon3);
    if (reset) {
    	printk(BIOS_DEBUG, "Power cycle reset...\n");
-		outb(0xe, 0xcf9);
+		full_reset();
    }
 }

diff --git a/src/northbridge/intel/sandybridge/romstage.c b/src/northbridge/intel/sandybridge/romstage.c
index 3fab3be..781b9ac 100644
--- a/src/northbridge/intel/sandybridge/romstage.c
+++ b/src/northbridge/intel/sandybridge/romstage.c
@@ -18,6 +18,7 @@
 #include <stdint.h>
 #include <console/console.h>
 #include <arch/io.h>
+#include <cf9_reset.h>
 #include <device/pci_ops.h>
 #include <cpu/x86/lapic.h>
 #include <timestamp.h>
@@ -48,8 +49,7 @@
    int s3resume = 0;

if (MCHBAR16(SSKPD) == 0xCAFE) {
-		outb(0x6, 0xcf9);
-		halt ();
+		system_reset();
    }

if (bist == 0)
diff --git a/src/northbridge/intel/x4x/raminit.c b/src/northbridge/intel/x4x/raminit.c
index 4d5bdce..6dc2e5f 100644
--- a/src/northbridge/intel/x4x/raminit.c
+++ b/src/northbridge/intel/x4x/raminit.c
@@ -625,8 +625,7 @@
    	pci_write_config8(PCI_DEV(0, 0, 0), 0xf0, reg8 |  (1 << 2));

printk(BIOS_DEBUG, "Reset...\n");
-		outb(0xe, 0xcf9);
-		asm ("hlt");
+		full_reset();
    }
    pmcon2 |= 0x80;
    pci_write_config8(PCI_DEV(0, 0x1f, 0), 0xa2, pmcon2);

[HAOUAS Elyes (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Damien Zammit,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32608

to look at the new patch set (#2).

Change subject: {gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function
......................................................................

{gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function

Use already defined system_reset() and full_reset() functions.

Change-Id: Ic29fab70cf7f23d49c3eeeb97c984c523f973972
Signed-off-by: Elyes HAOUAS 
---
M src/northbridge/intel/gm45/early_reset.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/romstage.c
M src/northbridge/intel/x4x/raminit.c
4 files changed, 7 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/32608/2

[HAOUAS Elyes (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Damien Zammit,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32608

to look at the new patch set (#3).

Change subject: {gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function
......................................................................

{gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function

Use already defined system_reset() and full_reset() functions.

Change-Id: Ic29fab70cf7f23d49c3eeeb97c984c523f973972
Signed-off-by: Elyes HAOUAS 
---
M src/northbridge/intel/gm45/early_reset.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/romstage.c
M src/northbridge/intel/x4x/raminit.c
4 files changed, 8 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/32608/3

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32608 )

Change subject: {gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function
......................................................................

Patch Set 3:

(4 comments)

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/gm45/early_res... 
File src/northbridge/intel/gm45/early_reset.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/gm45/early_res... 
PS3, Line 67: 	/* Perform system reset through CF9 interface. */
not sure, if the comment is still useful

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/pineview/ramin... 
File src/northbridge/intel/pineview/raminit.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/pineview/ramin... 
PS3, Line 1749: 		printk(BIOS_DEBUG, "Power cycle reset...\n");
I assume full_reset() already prints something like that.

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/sandybridge/ro... 
File src/northbridge/intel/sandybridge/romstage.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/sandybridge/ro... 
PS3, Line 53: 	}
Braces are not needed anymore.

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/x4x/raminit.c 
File src/northbridge/intel/x4x/raminit.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/x4x/raminit.c@... 
PS3, Line 626: 		printk(BIOS_DEBUG, "Reset...\n");
full_reset() already prints something.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32608 )

Change subject: {gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function
......................................................................

Patch Set 3:

(4 comments)

viel dank

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/gm45/early_res... 
File src/northbridge/intel/gm45/early_reset.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/gm45/early_res... 
PS3, Line 67: 	/* Perform system reset through CF9 interface. */
> not sure, if the comment is still useful

Done

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/pineview/ramin... 
File src/northbridge/intel/pineview/raminit.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/pineview/ramin... 
PS3, Line 1749: 		printk(BIOS_DEBUG, "Power cycle reset...\n");
> I assume full_reset() already prints something like that.

Done

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/sandybridge/ro... 
File src/northbridge/intel/sandybridge/romstage.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/sandybridge/ro... 
PS3, Line 53: 	}
> Braces are not needed anymore.

Done

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/x4x/raminit.c 
File src/northbridge/intel/x4x/raminit.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/x4x/raminit.c@... 
PS3, Line 626: 		printk(BIOS_DEBUG, "Reset...\n");
> full_reset() already prints something.

Done

[HAOUAS Elyes (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Damien Zammit,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32608

to look at the new patch set (#4).

Change subject: {gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function
......................................................................

{gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function

Use already defined system_reset() and full_reset() functions.

Change-Id: Ic29fab70cf7f23d49c3eeeb97c984c523f973972
Signed-off-by: Elyes HAOUAS 
---
M src/northbridge/intel/gm45/early_reset.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/romstage.c
M src/northbridge/intel/x4x/raminit.c
4 files changed, 10 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/32608/4

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32608 )

Change subject: {gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function
......................................................................

Patch Set 4: Code-Review+2

(4 comments)

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/gm45/early_res... 
File src/northbridge/intel/gm45/early_reset.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/gm45/early_res... 
PS3, Line 67: 	/* Perform system reset through CF9 interface. */
> Done

Ack

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/pineview/ramin... 
File src/northbridge/intel/pineview/raminit.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/pineview/ramin... 
PS3, Line 1749: 		printk(BIOS_DEBUG, "Power cycle reset...\n");
> Done

Ack

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/sandybridge/ro... 
File src/northbridge/intel/sandybridge/romstage.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/sandybridge/ro... 
PS3, Line 53: 	}
> Done

Ack

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/x4x/raminit.c 
File src/northbridge/intel/x4x/raminit.c:

https://review.coreboot.org/#/c/32608/3/src/northbridge/intel/x4x/raminit.c@... 
PS3, Line 626: 		printk(BIOS_DEBUG, "Reset...\n");
> Done

Ack

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32608 )

Change subject: {gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function
......................................................................

{gm45,pineview,sandybridge,x4x}: Use {full,system}_reset() function

Use already defined system_reset() and full_reset() functions.

Change-Id: Ic29fab70cf7f23d49c3eeeb97c984c523f973972
Signed-off-by: Elyes HAOUAS 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32608
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
M src/northbridge/intel/gm45/early_reset.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/romstage.c
M src/northbridge/intel/x4x/raminit.c
4 files changed, 10 insertions(+), 17 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/src/northbridge/intel/gm45/early_reset.c b/src/northbridge/intel/gm45/early_reset.c
index 9f919cf..3f095a2 100644
--- a/src/northbridge/intel/gm45/early_reset.c
+++ b/src/northbridge/intel/gm45/early_reset.c
@@ -16,8 +16,9 @@

#include <types.h>
 #include <arch/io.h>
+#include <cf9_reset.h>
 #include <device/pci_ops.h>
-#include <halt.h>
+
 #include "gm45.h"

void gm45_early_reset(void/*const timings_t *const timings*/)
@@ -63,8 +64,5 @@
    /* Normally, we would set this after successful raminit. */
    MCHBAR32(DCC_MCHBAR) |= (1 << 19);

-	/* Perform system reset through CF9 interface. */
-	outb(0x02, 0xcf9); /* Set system reset bit. */
-	outb(0x06, 0xcf9); /* Set CPU reset bit, too. */
-	halt();
+	system_reset();
 }
diff --git a/src/northbridge/intel/pineview/raminit.c b/src/northbridge/intel/pineview/raminit.c
index 48bca36..144905f 100644
--- a/src/northbridge/intel/pineview/raminit.c
+++ b/src/northbridge/intel/pineview/raminit.c
@@ -15,6 +15,7 @@
  */

#include <arch/io.h>
+#include <cf9_reset.h>
 #include <device/mmio.h>
 #include <device/pci_ops.h>
 #include <console/console.h>
@@ -1744,10 +1745,8 @@
    }
    pci_write_config8(PCI_DEV(0, 0x1f, 0), 0xa2, pmcon2);
    pci_write_config8(PCI_DEV(0, 0x1f, 0), 0xa4, pmcon3);
-	if (reset) {
-		printk(BIOS_DEBUG, "Power cycle reset...\n");
-		outb(0xe, 0xcf9);
-	}
+	if (reset)
+		full_reset();
 }

static void sdram_dradrb(struct sysinfo *s)
diff --git a/src/northbridge/intel/sandybridge/romstage.c b/src/northbridge/intel/sandybridge/romstage.c
index 3fab3be..43316a2 100644
--- a/src/northbridge/intel/sandybridge/romstage.c
+++ b/src/northbridge/intel/sandybridge/romstage.c
@@ -18,6 +18,7 @@
 #include <stdint.h>
 #include <console/console.h>
 #include <arch/io.h>
+#include <cf9_reset.h>
 #include <device/pci_ops.h>
 #include <cpu/x86/lapic.h>
 #include <timestamp.h>
@@ -47,10 +48,8 @@
 {
    int s3resume = 0;

-	if (MCHBAR16(SSKPD) == 0xCAFE) {
-		outb(0x6, 0xcf9);
-		halt ();
-	}
+	if (MCHBAR16(SSKPD) == 0xCAFE)
+		system_reset();

if (bist == 0)
    	enable_lapic();
diff --git a/src/northbridge/intel/x4x/raminit.c b/src/northbridge/intel/x4x/raminit.c
index 4d5bdce..60d3b55 100644
--- a/src/northbridge/intel/x4x/raminit.c
+++ b/src/northbridge/intel/x4x/raminit.c
@@ -22,7 +22,6 @@
 #include <cpu/x86/cache.h>
 #include <cpu/x86/mtrr.h>
 #include <arch/cpu.h>
-#include <halt.h>
 #if CONFIG(SOUTHBRIDGE_INTEL_I82801GX)
 #include <southbridge/intel/i82801gx/i82801gx.h> /* smbus_read_byte */
 #else
@@ -624,9 +623,7 @@
    	reg8 = pci_read_config8(PCI_DEV(0, 0, 0), 0xf0);
    	pci_write_config8(PCI_DEV(0, 0, 0), 0xf0, reg8 |  (1 << 2));

-		printk(BIOS_DEBUG, "Reset...\n");
-		outb(0xe, 0xcf9);
-		asm ("hlt");
+		full_reset();
    }
    pmcon2 |= 0x80;
    pci_write_config8(PCI_DEV(0, 0x1f, 0), 0xa2, pmcon2);