Change in ...coreboot[master]: sb/i82801gx/watchdog: Use {read,write}_pmbase function

List overview All Threads
Download

newer

older

Change in ...coreboot[master]:...

HAOUAS Elyes (Code Review)

1 May 2019 1 May '19

11:59 p.m.

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

Change subject: sb/i82801gx/watchdog: Use {read,write}_pmbase function ......................................................................

sb/i82801gx/watchdog: Use {read,write}_pmbase function

Use already defined common/pmbase functions. Also magic numbers replaced by macro and includes sorted.

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/i82801gx/watchdog.c 1 file changed, 14 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/1

diff --git a/src/southbridge/intel/i82801gx/watchdog.c b/src/southbridge/intel/i82801gx/watchdog.c index 12284b4..6b2a204 100644 --- a/src/southbridge/intel/i82801gx/watchdog.c +++ b/src/southbridge/intel/i82801gx/watchdog.c @@ -14,37 +14,37 @@ * GNU General Public License for more details. */

-#include <console/console.h> #include <arch/io.h> -#include <device/pci_ops.h> +#include <console/console.h> #include <device/device.h> #include <device/pci.h> +#include <device/pci_def.h> +#include <device/pci_ops.h> +#include <southbridge/intel/common/pmbase.h> #include <watchdog.h>

+#include "i82801gx.h" + void watchdog_off(void) { struct device *dev; - unsigned long value, base; + unsigned long value;

- /* Turn off the ICH7 watchdog. */ dev = pcidev_on_root(0x1f, 0);

/* Enable I/O space. */ - value = pci_read_config16(dev, 0x04); - value |= (1 << 10); - pci_write_config16(dev, 0x04, value); - - /* Get TCO base. */ - base = (pci_read_config32(dev, 0x40) & 0x0fffe) + 0x60; + value = pci_read_config16(dev, PCI_COMMAND); + value |= PCI_COMMAND_INT_DISABLE; + pci_write_config16(dev, PCI_COMMAND, value);

/* Disable the watchdog timer. */ - value = inw(base + 0x08); + value = read_pmbase16(TCO1_CNT); value |= 1 << 11; - outw(value, base + 0x08); + write_pmbase16(TCO1_CNT, value);

/* Clear TCO timeout status. */ - outw(0x0008, base + 0x04); - outw(0x0002, base + 0x06); + write_pmbase16(TCO1_CNT + 0x04, (1 << 3)); + write_pmbase16(TCO1_CNT + 0x06, (1 << 2));

printk(BIOS_DEBUG, "ICH7 watchdog disabled\n"); }

Attachments:

attachment.htm (text/html — 5.2 KB)

Show replies by date

Nico Huber (Code Review)

2 May 2 May

1:06 a.m.

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

Change subject: sb/i82801gx/watchdog: Use {read,write}_pmbase function ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/32533/1/src/southbridge/intel/i82801gx/watch... File src/southbridge/intel/i82801gx/watchdog.c:

https://review.coreboot.org/#/c/32533/1/src/southbridge/intel/i82801gx/watch... PS1, Line 46: write_pmbase16(TCO1_CNT + 0x04, (1 << 3)); : write_pmbase16(TCO1_CNT + 0x06, (1 << 2)); : TCO1_CNT is 0x68, not 0x60. so these are off by 8

HAOUAS Elyes (Code Review)

1:18 a.m.

New subject: Change in ...coreboot[master]: [wip]watchdog: Use common watchdo_off function

Hello Patrick Rudolph, build bot (Jenkins),

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

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

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

Change subject: [wip]watchdog: Use common watchdo_off function ......................................................................

[wip]watchdog: Use common watchdo_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Makefile.inc R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Makefile.inc M src/southbridge/intel/i82801gx/i82801gx.h D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 12 files changed, 22 insertions(+), 178 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/2

HAOUAS Elyes (Code Review)

1:21 a.m.

New subject: Change in ...coreboot[master]: [wip]watchdog: Use common watchdog_off function

Hello Patrick Rudolph, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: [wip]watchdog: Use common watchdog_off function ......................................................................

[wip]watchdog: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Makefile.inc R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 11 files changed, 21 insertions(+), 178 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/4

David Guckian (Code Review)

2 a.m.

New subject: Change in ...coreboot[master]: [wip]watchdog: Use common watchdog_off function

David Guckian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32533 )

Change subject: [wip]watchdog: Use common watchdog_off function ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/32533/4/src/southbridge/intel/common/watchdo... File src/southbridge/intel/common/watchdog.c:

https://review.coreboot.org/#/c/32533/4/src/southbridge/intel/common/watchdo... PS4, Line 55: write_pmbase16(TCOBASE + TCO2_STATUS, (1 << 2)); Some of the files you remove use: outw(0x0002, abase + 0x66); This writes 0x2, does your code write 0x4?

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 4 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Wed, 01 May 2019 17:00:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

2:33 a.m.

New subject: Change in ...coreboot[master]: [wip]watchdog: Use common watchdog_off function

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

Change subject: [wip]watchdog: Use common watchdog_off function ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/32533/4/src/southbridge/intel/common/watchdo... File src/southbridge/intel/common/watchdog.c:

https://review.coreboot.org/#/c/32533/4/src/southbridge/intel/common/watchdo... PS4, Line 55: write_pmbase16(TCOBASE + TCO2_STATUS, (1 << 2));

...

Some of the files you remove use: […]

oops, thank you

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 4 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Wed, 01 May 2019 17:33:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: David Guckian Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

2:34 a.m.

New subject: Change in ...coreboot[master]: [wip]watchdog: Use common watchdog_off function

Hello Patrick Rudolph, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: [wip]watchdog: Use common watchdog_off function ......................................................................

[wip]watchdog: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Makefile.inc R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 11 files changed, 21 insertions(+), 178 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/5

HAOUAS Elyes (Code Review)

2:44 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Hello Patrick Rudolph, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Makefile.inc R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Makefile.inc M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 10 files changed, 21 insertions(+), 125 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/6

HAOUAS Elyes (Code Review)

2:47 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/common/watchdo... File src/southbridge/intel/common/watchdog.c:

https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/common/watchdo... PS6, Line 40: value = pci_read_config16(dev, PCI_COMMAND); : value |= PCI_COMMAND_INT_DISABLE; : pci_write_config16(dev, PCI_COMMAND, value); this seems to be wrong on the chip I have. this value is = 0x0007 before and after the pci_write (on I82801GX)

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 6 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Wed, 01 May 2019 17:47:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nico Huber (Code Review)

3:31 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 6:

(3 comments)

https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/bd82x6x/Makefi... File src/southbridge/intel/bd82x6x/Makefile.inc:

https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/bd82x6x/Makefi... PS6, Line 34: ramstage-y += ../common/watchdog.c It's generally preferred to have a Kconfig + this line in the common code and select the Kconfig in the chipset code.

https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/common/watchdo... File src/southbridge/intel/common/watchdog.c:

https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/common/watchdo... PS6, Line 28: #define TCO1_STATUS 0x04 : #define TCO2_STATUS 0x06 : #define TCO1_CONTROL 0x08 Please use the names from the datasheets. It would also be nice to have these definitions in a common header file.

https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/common/watchdo... PS6, Line 50: (1 << 11) Please use the defined names like on the lhs.

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 6 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Wed, 01 May 2019 18:31:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

4 May 4 May

12:06 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Kconfig M src/southbridge/intel/bd82x6x/Makefile.inc M src/southbridge/intel/common/Kconfig M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/tco.h R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Kconfig M src/southbridge/intel/fsp_rangeley/Makefile.inc M src/southbridge/intel/i82801gx/Kconfig M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Kconfig M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Kconfig M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Kconfig M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 20 files changed, 47 insertions(+), 128 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/7

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 7 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

12:49 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Kconfig M src/southbridge/intel/bd82x6x/Makefile.inc M src/southbridge/intel/common/Kconfig M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/tco.h R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Kconfig M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Kconfig M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Kconfig M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Kconfig M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Kconfig M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 21 files changed, 55 insertions(+), 183 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/8

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 8 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

12:55 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32533 )

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 8: Code-Review+1

(3 comments)

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h File src/southbridge/intel/common/tco.h:

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h@3 PS8, Line 3: * Missing Copyright

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/watchdo... File src/southbridge/intel/common/watchdog.c:

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/watchdo... PS8, Line 18: <arch/io.h> Why do you need arch/io.h?

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/watchdo... PS8, Line 52: << Use define instead of magic value

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 8 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 03 May 2019 15:55:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

1:20 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 8:

(3 comments)

Thank you.

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h File src/southbridge/intel/common/tco.h:

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h@3 PS8, Line 3: *

...

Missing Copyright

Done

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/watchdo... File src/southbridge/intel/common/watchdog.c:

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/watchdo... PS8, Line 18: <arch/io.h>

...

Why do you need arch/io. […]

Done

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/watchdo... PS8, Line 52: <<

...

Use define instead of magic value

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 8 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 03 May 2019 16:20:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph siro@das-labor.org Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

1:20 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Kconfig M src/southbridge/intel/bd82x6x/Makefile.inc M src/southbridge/intel/common/Kconfig M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/tco.h R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Kconfig M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Kconfig M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Kconfig M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Kconfig M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Kconfig M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 21 files changed, 55 insertions(+), 182 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/9

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 9 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Nico Huber (Code Review)

2 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 9:

(4 comments)

https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig File src/southbridge/intel/common/Kconfig:

https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig... PS9, Line 69: def_bool n Just `bool` should suffice.

https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig... PS9, Line 70: depends on SOUTHBRIDGE_INTEL_COMMON Why?

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h File src/southbridge/intel/common/tco.h:

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h@1... PS8, Line 17: #define TCOBASE 0x60 this is ambiguous. maybe PMBASE_TCO_OFFSET?

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h@2... PS8, Line 21: define TCO1_TIMEOUT (1 << 3) : #define SECOND_TO_STS (1 << 1) : #define TCO_TMR_HLT (1 << 11) Please indent the names to make them visibly distinctive from the register names. And put them below the registers where they are found, e.g.

#define TCO1_STS 0x04 #define TCO1_TIMEOUT (1 << 3) #define TCO2_STS 0x06 #define SECOND_TO_STS (1 << 1)

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 9 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 03 May 2019 17:00:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

7:22 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Kconfig M src/southbridge/intel/bd82x6x/Makefile.inc M src/southbridge/intel/common/Kconfig M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/tco.h R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Kconfig M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Kconfig M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Kconfig M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Kconfig M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Kconfig M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 21 files changed, 55 insertions(+), 182 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/10

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 10 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

7:23 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Kconfig M src/southbridge/intel/bd82x6x/Makefile.inc M src/southbridge/intel/common/Kconfig M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/tco.h R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Kconfig M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Kconfig M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Kconfig M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Kconfig M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Kconfig M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 21 files changed, 54 insertions(+), 182 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/11

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 11 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

2:29 p.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 8:

(2 comments)

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h File src/southbridge/intel/common/tco.h:

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h@1... PS8, Line 17: #define TCOBASE 0x60

...

this is ambiguous. […]

Done

https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h@2... PS8, Line 21: define TCO1_TIMEOUT (1 << 3) : #define SECOND_TO_STS (1 << 1) : #define TCO_TMR_HLT (1 << 11)

...

Please indent the names to make them visibly distinctive from the […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 8 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 04 May 2019 05:29:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

2:30 p.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Kconfig M src/southbridge/intel/bd82x6x/Makefile.inc M src/southbridge/intel/common/Kconfig M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/tco.h R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Kconfig M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Kconfig M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Kconfig M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Kconfig M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Kconfig M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 21 files changed, 54 insertions(+), 182 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/12

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 12 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

3:57 p.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32533 )

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 12: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 12 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 04 May 2019 06:57:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nico Huber (Code Review)

5 May 5 May

8:11 p.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 12:

(3 comments)

https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig File src/southbridge/intel/common/Kconfig:

https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig... PS9, Line 69: def_bool n

...

Just `bool` should suffice.

Done

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/Makefi... File src/southbridge/intel/common/Makefile.inc:

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/Makefi... PS12, Line 55: ramstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_WATCHDOG) += watchdog.c It's placed inside `ifeq ($(CONFIG_SOUTHBRIDGE_INTEL_COMMON),y)` (and so is the used `pmbase.c`) so it does depend on it...

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/tco.h File src/southbridge/intel/common/tco.h:

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/tco.h@... PS12, Line 25: #define TCO_TMR_HLT (1 << 11) indent?

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 12 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 05 May 2019 11:11:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

8:25 p.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 12:

(2 comments)

Thank you

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/Makefi... File src/southbridge/intel/common/Makefile.inc:

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/Makefi... PS12, Line 55: ramstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_WATCHDOG) += watchdog.c

...

It's placed inside `ifeq ($(CONFIG_SOUTHBRIDGE_INTEL_COMMON),y)` […]

Done

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/tco.h File src/southbridge/intel/common/tco.h:

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/tco.h@... PS12, Line 25: #define TCO_TMR_HLT (1 << 11)

...

indent?

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 12 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 05 May 2019 11:25:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

8:25 p.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Kconfig M src/southbridge/intel/bd82x6x/Makefile.inc M src/southbridge/intel/common/Kconfig M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/tco.h R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Kconfig M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Kconfig M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Kconfig M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Kconfig M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Kconfig M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 21 files changed, 55 insertions(+), 182 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/13

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 13 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Nico Huber (Code Review)

9:29 p.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 13:

(2 comments)

https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig File src/southbridge/intel/common/Kconfig:

https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig... PS9, Line 70: depends on SOUTHBRIDGE_INTEL_COMMON

...

Why?

I only asked `why`, didn't mean to imply that it is wrong.

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/Makefi... File src/southbridge/intel/common/Makefile.inc:

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/Makefi... PS12, Line 55: ramstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_WATCHDOG) += watchdog.c

...

Done

We are talking a little past each other (or rather, I'm talking past you). What I meant was that your original `depends on SOUTHBRIDGE_INTEL_COMMON` was correct. And the explanation is that `pmbase.c` is only added to the build when it is selected.

NB. where you place `watchdog.c` in this file doesn't matter much. Dependencies should be modeled in Kconfig.

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 13 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 05 May 2019 12:29:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

11:02 p.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), David Guckian, Patrick Georgi, Martin Roth,

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

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

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/bd82x6x/Kconfig M src/southbridge/intel/bd82x6x/Makefile.inc M src/southbridge/intel/common/Kconfig M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/tco.h R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Kconfig M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Kconfig M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Kconfig M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Kconfig M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Kconfig M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 21 files changed, 56 insertions(+), 182 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32533/14

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 14 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Nico Huber (Code Review)

6 May 6 May

2:06 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 14: Code-Review+2

(3 comments)

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/Makefi... File src/southbridge/intel/common/Makefile.inc:

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/Makefi... PS12, Line 55: ramstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_WATCHDOG) += watchdog.c

...

We are talking a little past each other (or rather, I'm […]

Done

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/tco.h File src/southbridge/intel/common/tco.h:

https://review.coreboot.org/#/c/32533/12/src/southbridge/intel/common/tco.h@... PS12, Line 25: #define TCO_TMR_HLT (1 << 11)

...

Done

Ack

https://review.coreboot.org/#/c/32533/14/src/southbridge/intel/fsp_rangeley/... File src/southbridge/intel/fsp_rangeley/watchdog.c:

https://review.coreboot.org/#/c/32533/14/src/southbridge/intel/fsp_rangeley/... PS14, Line 37: David, Fei, can you confirm if this bit is writable on Rangeley?

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 14 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Fei Wang Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: David Guckian Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 05 May 2019 17:06:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

David Guckian (Code Review)

7 May 7 May

6:39 p.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

David Guckian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32533 )

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

Patch Set 14: Code-Review+1

(1 comment)

https://review.coreboot.org/#/c/32533/14/src/southbridge/intel/fsp_rangeley/... File src/southbridge/intel/fsp_rangeley/watchdog.c:

https://review.coreboot.org/#/c/32533/14/src/southbridge/intel/fsp_rangeley/... PS14, Line 37:

...

David, Fei, can you confirm if this bit is writable […]

The I/O space cannot be disabled on LPC, so I guess writing a 1 here is redundant.

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 14 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Fei Wang Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 07 May 2019 09:39:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Patrick Georgi (Code Review)

8 May 8 May

1:01 a.m.

New subject: Change in ...coreboot[master]: sb/{ICH7,NM10,PCH}: Use common watchdog_off function

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

Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................

sb/{ICH7,NM10,PCH}: Use common watchdog_off function

Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/coreboot/+/32533 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: David Guckian --- M src/southbridge/intel/bd82x6x/Kconfig M src/southbridge/intel/bd82x6x/Makefile.inc M src/southbridge/intel/common/Kconfig M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/tco.h R src/southbridge/intel/common/watchdog.c M src/southbridge/intel/fsp_rangeley/Kconfig M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/watchdog.c M src/southbridge/intel/i82801gx/Kconfig M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/watchdog.c M src/southbridge/intel/i82801ix/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Kconfig M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Kconfig M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Kconfig M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/watchdog.c 21 files changed, 56 insertions(+), 182 deletions(-)

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved David Guckian: Looks good to me, but someone else must approve

diff --git a/src/southbridge/intel/bd82x6x/Kconfig b/src/southbridge/intel/bd82x6x/Kconfig index dae3c32..fc3e9fc 100644 --- a/src/southbridge/intel/bd82x6x/Kconfig +++ b/src/southbridge/intel/bd82x6x/Kconfig @@ -44,6 +44,7 @@ select HAVE_INTEL_CHIPSET_LOCKDOWN select SOUTHBRIDGE_INTEL_COMMON_SMM select SOUTHBRIDGE_INTEL_COMMON_ACPI_MADT + select SOUTHBRIDGE_INTEL_COMMON_WATCHDOG

config EHCI_BAR hex diff --git a/src/southbridge/intel/bd82x6x/Makefile.inc b/src/southbridge/intel/bd82x6x/Makefile.inc index a950e5c..023f5d3 100644 --- a/src/southbridge/intel/bd82x6x/Makefile.inc +++ b/src/southbridge/intel/bd82x6x/Makefile.inc @@ -31,7 +31,6 @@ ramstage-srcs += src/mainboard/$(MAINBOARDDIR)/hda_verb.c

ramstage-y += me_status.c -ramstage-y += watchdog.c

ramstage-$(CONFIG_ELOG) += elog.c

diff --git a/src/southbridge/intel/common/Kconfig b/src/southbridge/intel/common/Kconfig index 0bda06e..c3bd90d 100644 --- a/src/southbridge/intel/common/Kconfig +++ b/src/southbridge/intel/common/Kconfig @@ -65,6 +65,10 @@ and S3 resume (always done by coreboot). Select this to let coreboot to do this on normal boot path.

+config SOUTHBRIDGE_INTEL_COMMON_WATCHDOG + bool + depends on SOUTHBRIDGE_INTEL_COMMON + if SOUTHBRIDGE_INTEL_COMMON_FINALIZE

choice diff --git a/src/southbridge/intel/common/Makefile.inc b/src/southbridge/intel/common/Makefile.inc index 1085f6c..4cf6e6f 100644 --- a/src/southbridge/intel/common/Makefile.inc +++ b/src/southbridge/intel/common/Makefile.inc @@ -27,6 +27,8 @@

romstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_PMCLIB) += pmclib.c

+ramstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_WATCHDOG) += watchdog.c + ifeq ($(CONFIG_SOUTHBRIDGE_INTEL_COMMON),y)

verstage-y += pmbase.c diff --git a/src/southbridge/intel/common/tco.h b/src/southbridge/intel/common/tco.h new file mode 100644 index 0000000..9d6f153 --- /dev/null +++ b/src/southbridge/intel/common/tco.h @@ -0,0 +1,27 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (c) 2019 Elyes Haouas ehaouas@noos.fr + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef SOUTHBRIDGE_INTEL_COMMON_TCO_H +#define SOUTHBRIDGE_INTEL_COMMON_TCO_H + +#define PMBASE_TCO_OFFSET 0x60 +#define TCO1_STS 0x04 +#define TCO1_TIMEOUT (1 << 3) +#define TCO2_STS 0x06 +#define SECOND_TO_STS (1 << 1) +#define TCO1_CNT 0x08 +#define TCO_TMR_HLT (1 << 11) + +#endif /* SOUTHBRIDGE_INTEL_COMMON_TCO_H */ diff --git a/src/southbridge/intel/bd82x6x/watchdog.c b/src/southbridge/intel/common/watchdog.c similarity index 67% rename from src/southbridge/intel/bd82x6x/watchdog.c rename to src/southbridge/intel/common/watchdog.c index 6373a39..778a7a9 100644 --- a/src/southbridge/intel/bd82x6x/watchdog.c +++ b/src/southbridge/intel/common/watchdog.c @@ -16,17 +16,16 @@ */

#include <console/console.h> -#include <device/pci_ops.h> #include <device/device.h> #include <device/pci.h> #include <device/pci_def.h> +#include <device/pci_ops.h> #include <southbridge/intel/common/pmbase.h> -#include <southbridge/intel/bd82x6x/pch.h> - +#include <southbridge/intel/common/tco.h> #include <watchdog.h>

/* - * Disable PCH watchdog timer + * Disable ICH-NM10-PCH watchdog timer */ void watchdog_off(void) { @@ -36,21 +35,25 @@ /* Get LPC device. */ dev = pcidev_on_root(0x1f, 0);

- /* Disable interrupt. */ value = pci_read_config16(dev, PCI_COMMAND); - value |= PCI_COMMAND_INT_DISABLE; + + if (CONFIG(SOUTHBRIDGE_INTEL_FSP_RANGELEY)) { + /* Enable I/O space. */ + value |= PCI_COMMAND_IO; + } else { + /* Disable interrupt. */ + value |= PCI_COMMAND_INT_DISABLE; + } pci_write_config16(dev, PCI_COMMAND, value);

/* Disable the watchdog timer. */ - value = read_pmbase16(TCO1_CNT); + value = read_pmbase16(PMBASE_TCO_OFFSET + TCO1_CNT); value |= TCO_TMR_HLT; - write_pmbase16(TCO1_CNT, value); + write_pmbase16(PMBASE_TCO_OFFSET + TCO1_CNT, value);

/* Clear TCO timeout status. */ - write_pmbase16(TCO1_STS, TCO1_TIMEOUT); - write_pmbase16(TCO2_STS, SECOND_TO_STS); + write_pmbase16(PMBASE_TCO_OFFSET + TCO1_STS, TCO1_TIMEOUT); + write_pmbase16(PMBASE_TCO_OFFSET + TCO2_STS, SECOND_TO_STS);

- /* FIXME: Set RCBA GCS Bit5 "No Reboot" ? */ - - printk(BIOS_DEBUG, "PCH: watchdog disabled\n"); + printk(BIOS_DEBUG, "ICH-NM10-PCH: watchdog disabled\n"); } diff --git a/src/southbridge/intel/fsp_rangeley/Kconfig b/src/southbridge/intel/fsp_rangeley/Kconfig index c15c48d..4526cb3 100644 --- a/src/southbridge/intel/fsp_rangeley/Kconfig +++ b/src/southbridge/intel/fsp_rangeley/Kconfig @@ -31,6 +31,7 @@ select INTEL_DESCRIPTOR_MODE_CAPABLE select SOUTHBRIDGE_INTEL_COMMON select SOUTHBRIDGE_INTEL_COMMON_SMBUS + select SOUTHBRIDGE_INTEL_COMMON_WATCHDOG

config EHCI_BAR hex diff --git a/src/southbridge/intel/fsp_rangeley/Makefile.inc b/src/southbridge/intel/fsp_rangeley/Makefile.inc index ac5888c..7fc8601 100644 --- a/src/southbridge/intel/fsp_rangeley/Makefile.inc +++ b/src/southbridge/intel/fsp_rangeley/Makefile.inc @@ -19,7 +19,6 @@ ramstage-y += soc.c ramstage-y += lpc.c ramstage-y += sata.c -ramstage-y += watchdog.c ramstage-y += spi.c ramstage-y += smbus.c ramstage-y += acpi.c diff --git a/src/southbridge/intel/fsp_rangeley/watchdog.c b/src/southbridge/intel/fsp_rangeley/watchdog.c deleted file mode 100644 index f18af89..0000000 --- a/src/southbridge/intel/fsp_rangeley/watchdog.c +++ /dev/null @@ -1,53 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2008-2009 coresystems GmbH - * Copyright (C) 2011 Google Inc. - * Copyright (C) 2013 Sage Electronic Engineering, LLC. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; version 2 of - * the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#include <console/console.h> -#include <arch/io.h> -#include <device/pci_ops.h> -#include <device/device.h> -#include <device/pci.h> -#include <watchdog.h> -#include "soc.h" - -void watchdog_off(void) -{ - struct device *dev; - u32 value, abase; - - /* Turn off the watchdog. */ - dev = pcidev_on_root(0x1f, 0); - - /* Enable I/O space. */ - value = pci_read_config16(dev, 0x04); - value |= 1; - pci_write_config16(dev, 0x04, value); - - /* Get TCO base. */ - abase = (pci_read_config32(dev, ABASE) & ~0xf); - - /* Disable the watchdog timer. */ - value = inw(abase + 0x68); - value |= 1 << 11; - outw(value, abase + 0x68); - - /* Clear TCO timeout status. */ - outw(0x0008, abase + 0x64); - outw(0x0002, abase + 0x66); - - printk(BIOS_DEBUG, "TCO Watchdog disabled\n"); -} diff --git a/src/southbridge/intel/i82801gx/Kconfig b/src/southbridge/intel/i82801gx/Kconfig index a7d65c5..2d6e938 100644 --- a/src/southbridge/intel/i82801gx/Kconfig +++ b/src/southbridge/intel/i82801gx/Kconfig @@ -30,6 +30,7 @@ select SOUTHBRIDGE_INTEL_COMMON_RCBA_PIRQ select INTEL_HAS_TOP_SWAP select SOUTHBRIDGE_INTEL_COMMON_SMM + select SOUTHBRIDGE_INTEL_COMMON_WATCHDOG

if SOUTHBRIDGE_INTEL_I82801GX

diff --git a/src/southbridge/intel/i82801gx/Makefile.inc b/src/southbridge/intel/i82801gx/Makefile.inc index b72ca23..32a4bf5 100644 --- a/src/southbridge/intel/i82801gx/Makefile.inc +++ b/src/southbridge/intel/i82801gx/Makefile.inc @@ -30,8 +30,6 @@

ramstage-srcs += src/mainboard/$(MAINBOARDDIR)/hda_verb.c

-ramstage-y += watchdog.c - smm-$(CONFIG_HAVE_SMI_HANDLER) += smihandler.c

romstage-y += early_smbus.c diff --git a/src/southbridge/intel/i82801gx/watchdog.c b/src/southbridge/intel/i82801gx/watchdog.c deleted file mode 100644 index 12284b4..0000000 --- a/src/southbridge/intel/i82801gx/watchdog.c +++ /dev/null @@ -1,50 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2008-2009 coresystems GmbH - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; version 2 of - * the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#include <console/console.h> -#include <arch/io.h> -#include <device/pci_ops.h> -#include <device/device.h> -#include <device/pci.h> -#include <watchdog.h> - -void watchdog_off(void) -{ - struct device *dev; - unsigned long value, base; - - /* Turn off the ICH7 watchdog. */ - dev = pcidev_on_root(0x1f, 0); - - /* Enable I/O space. */ - value = pci_read_config16(dev, 0x04); - value |= (1 << 10); - pci_write_config16(dev, 0x04, value); - - /* Get TCO base. */ - base = (pci_read_config32(dev, 0x40) & 0x0fffe) + 0x60; - - /* Disable the watchdog timer. */ - value = inw(base + 0x08); - value |= 1 << 11; - outw(value, base + 0x08); - - /* Clear TCO timeout status. */ - outw(0x0008, base + 0x04); - outw(0x0002, base + 0x06); - - printk(BIOS_DEBUG, "ICH7 watchdog disabled\n"); -} diff --git a/src/southbridge/intel/i82801ix/Kconfig b/src/southbridge/intel/i82801ix/Kconfig index 44b2cbc..a269773 100644 --- a/src/southbridge/intel/i82801ix/Kconfig +++ b/src/southbridge/intel/i82801ix/Kconfig @@ -29,6 +29,7 @@ select SOUTHBRIDGE_INTEL_COMMON_PMCLIB select INTEL_DESCRIPTOR_MODE_CAPABLE select ACPI_INTEL_HARDWARE_SLEEP_VALUES + select SOUTHBRIDGE_INTEL_COMMON_WATCHDOG

if SOUTHBRIDGE_INTEL_I82801IX

diff --git a/src/southbridge/intel/i82801ix/Makefile.inc b/src/southbridge/intel/i82801ix/Makefile.inc index 3cc7da5..caa4932 100644 --- a/src/southbridge/intel/i82801ix/Makefile.inc +++ b/src/southbridge/intel/i82801ix/Makefile.inc @@ -29,8 +29,6 @@

ramstage-srcs += src/mainboard/$(MAINBOARDDIR)/hda_verb.c

-ramstage-y += ../i82801gx/watchdog.c - ifneq ($(CONFIG_SMM_TSEG),y) ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi.c ramstage-$(CONFIG_HAVE_SMI_HANDLER) += ../../../cpu/x86/smm/smmrelocate.S diff --git a/src/southbridge/intel/i82801jx/Kconfig b/src/southbridge/intel/i82801jx/Kconfig index be2d289..b423eca 100644 --- a/src/southbridge/intel/i82801jx/Kconfig +++ b/src/southbridge/intel/i82801jx/Kconfig @@ -33,6 +33,7 @@ select ACPI_INTEL_HARDWARE_SLEEP_VALUES select HAVE_POWER_STATE_AFTER_FAILURE select HAVE_POWER_STATE_PREVIOUS_AFTER_FAILURE + select SOUTHBRIDGE_INTEL_COMMON_WATCHDOG

if SOUTHBRIDGE_INTEL_I82801JX

diff --git a/src/southbridge/intel/i82801jx/Makefile.inc b/src/southbridge/intel/i82801jx/Makefile.inc index c333566..6626bb5 100644 --- a/src/southbridge/intel/i82801jx/Makefile.inc +++ b/src/southbridge/intel/i82801jx/Makefile.inc @@ -29,8 +29,6 @@

ramstage-srcs += src/mainboard/$(MAINBOARDDIR)/hda_verb.c

-ramstage-y += ../i82801gx/watchdog.c - smm-$(CONFIG_HAVE_SMI_HANDLER) += smihandler.c

romstage-y += early_smbus.c diff --git a/src/southbridge/intel/ibexpeak/Kconfig b/src/southbridge/intel/ibexpeak/Kconfig index bb6e22c..4763133 100644 --- a/src/southbridge/intel/ibexpeak/Kconfig +++ b/src/southbridge/intel/ibexpeak/Kconfig @@ -40,6 +40,7 @@ select HAVE_INTEL_CHIPSET_LOCKDOWN select HAVE_POWER_STATE_AFTER_FAILURE select HAVE_POWER_STATE_PREVIOUS_AFTER_FAILURE + select SOUTHBRIDGE_INTEL_COMMON_WATCHDOG

config EHCI_BAR hex diff --git a/src/southbridge/intel/ibexpeak/Makefile.inc b/src/southbridge/intel/ibexpeak/Makefile.inc index 5c89030..2fb3718 100644 --- a/src/southbridge/intel/ibexpeak/Makefile.inc +++ b/src/southbridge/intel/ibexpeak/Makefile.inc @@ -31,7 +31,6 @@ ramstage-srcs += src/mainboard/$(MAINBOARDDIR)/hda_verb.c

ramstage-y += ../bd82x6x/me_status.c -ramstage-y += ../bd82x6x/watchdog.c

ramstage-$(CONFIG_ELOG) += ../bd82x6x/elog.c ramstage-y += madt.c diff --git a/src/southbridge/intel/lynxpoint/Kconfig b/src/southbridge/intel/lynxpoint/Kconfig index 5573ec9..67e20be 100644 --- a/src/southbridge/intel/lynxpoint/Kconfig +++ b/src/southbridge/intel/lynxpoint/Kconfig @@ -42,6 +42,7 @@ select COMMON_FADT select HAVE_POWER_STATE_AFTER_FAILURE select HAVE_POWER_STATE_PREVIOUS_AFTER_FAILURE + select SOUTHBRIDGE_INTEL_COMMON_WATCHDOG

config INTEL_LYNXPOINT_LP bool diff --git a/src/southbridge/intel/lynxpoint/Makefile.inc b/src/southbridge/intel/lynxpoint/Makefile.inc index 04e0bc9..f0bfa5b 100644 --- a/src/southbridge/intel/lynxpoint/Makefile.inc +++ b/src/southbridge/intel/lynxpoint/Makefile.inc @@ -37,7 +37,6 @@

ramstage-y += rcba.c ramstage-y += me_status.c -ramstage-y += watchdog.c ramstage-y += acpi.c

ramstage-$(CONFIG_ELOG) += elog.c diff --git a/src/southbridge/intel/lynxpoint/watchdog.c b/src/southbridge/intel/lynxpoint/watchdog.c deleted file mode 100644 index 545d3d5..0000000 --- a/src/southbridge/intel/lynxpoint/watchdog.c +++ /dev/null @@ -1,56 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2008-2009 coresystems GmbH - * Copyright (C) 2011 Google Inc. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; version 2 of - * the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#include <console/console.h> -#include <arch/io.h> -#include <device/pci_ops.h> -#include <device/device.h> -#include <device/pci.h> -#include <watchdog.h> - - // - // Disable PCH Watchdog timer at SB_RCBA+0x3410 - // - // Mmio32((MmPci32(0, 0, 0x1F, 0, 0xF0) & ~BIT0), 0x3410) |= 0x20; - // -void watchdog_off(void) -{ - struct device *dev; - unsigned long value, base; - - /* Turn off the ICH7 watchdog. */ - dev = pcidev_on_root(0x1f, 0); - - /* Enable I/O space. */ - value = pci_read_config16(dev, 0x04); - value |= (1 << 10); - pci_write_config16(dev, 0x04, value); - - /* Get TCO base. */ - base = (pci_read_config32(dev, 0x40) & 0x0fffe) + 0x60; - - /* Disable the watchdog timer. */ - value = inw(base + 0x08); - value |= 1 << 11; - outw(value, base + 0x08); - - /* Clear TCO timeout status. */ - outw(0x0008, base + 0x04); - outw(0x0002, base + 0x06); - - printk(BIOS_DEBUG, "PCH watchdog disabled\n"); -}

-- To view, visit https://review.coreboot.org/c/coreboot/+/32533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I704780b6ae7238560dcb72fc027addc1089e0674 Gerrit-Change-Number: 32533 Gerrit-PatchSet: 15 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Fei Wang Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1979

days inactive

1985

days old

coreboot-gerrit@coreboot.org

28 comments

5 participants

tags (0)

participants (5)

David Guckian (Code Review)
HAOUAS Elyes (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)
Patrick Rudolph (Code Review)