Change in coreboot[master]: mb/asus/p2b: Enable hardware monitor access via ISA bus

[Keith Hui (Code Review)]

Keith Hui has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41561 )

Change subject: mb/asus/p2b: Enable hardware monitor access via ISA bus
......................................................................

mb/asus/p2b: Enable hardware monitor access via ISA bus

Setup a 8-byte I/O range at 0x290-0x297 on PIIX4's generic device 9
which activates a chip select when this range is accessed.

On the P2B family this connects to the W83781D hardware monitor,
allowing access to it over the ISA bus, just like vendor firmware.

TEST=lm-sensors can detect chip and get readings over ISA.

Change-Id: Iaed1df7230359e94c580c305f4769c8bb4f5fce0
Signed-off-by: Keith Hui 
---
A src/mainboard/asus/p2b/mainboard.c
1 file changed, 29 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/41561/1

diff --git a/src/mainboard/asus/p2b/mainboard.c b/src/mainboard/asus/p2b/mainboard.c
new file mode 100644
index 0000000..ffdc78a
--- /dev/null
+++ b/src/mainboard/asus/p2b/mainboard.c
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <device/pci_ops.h>
+#include <southbridge/intel/i82371eb/i82371eb.h>
+
+/**
+ * Mainboard specific enables.
+ *
+ * @param chip_info Ignored
+ */
+static void mainboard_init(void *chip_info)
+{
+	const pci_devfn_t px43 = PCI_DEV(0, 4, 3);
+	u32 reg;
+	/*
+	 * Setup an 8-byte generic I/O decode block at device 9.
+	 * This will be for W83781D hardware monitor.
+	 * Port 0x290 mask 0x007
+	 *
+	 * This should enable access to W83781D over the ISA bus.
+	 */
+	reg = pci_s_read_config32(px43, 0x60); /* DEVRESB */
+	reg |= (0x290 | (0xe7 << 16));
+	pci_s_write_config32(px43, 0x60, reg);
+}
+
+struct chip_operations mainboard_ops = {
+	.init = mainboard_init
+};

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41561 )

Change subject: mb/asus/p2b: Enable hardware monitor access via ISA bus
......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
File src/mainboard/asus/p2b/mainboard.c:

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
PS1, Line 22: 0x60
Maybe #define this register?

[Keith Hui (Code Review)]

Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41561 )

Change subject: mb/asus/p2b: Enable hardware monitor access via ISA bus
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
File src/mainboard/asus/p2b/mainboard.c:

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
PS1, Line 22: 0x60
> Maybe #define this register?

Yes I should, if it's OK to expand my scope for. (I need to edit i82371eb.h for this.)

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41561 )

Change subject: mb/asus/p2b: Enable hardware monitor access via ISA bus
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
File src/mainboard/asus/p2b/mainboard.c:

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
PS1, Line 22: 0x60
> Yes I should, if it's OK to expand my scope for. (I need to edit i82371eb.h for this. […]

You can make two commits: one that adds register definitions to i82371eb.h, and then have this change use them

Don't worry, I will make sure to review both of them 😊

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41561 )

Change subject: mb/asus/p2b: Enable hardware monitor access via ISA bus
......................................................................

Patch Set 1:

(2 comments)

Very nice.

https://review.coreboot.org/c/coreboot/+/41561/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/41561/1//COMMIT_MSG@9 
PS1, Line 9: Setup
Set up

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
File src/mainboard/asus/p2b/mainboard.c:

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
PS1, Line 16: Setup
The verb is spelled with a space: Set up.

Change in coreboot[master]: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus

[Keith Hui (Code Review)]

Hello build bot (Jenkins), Angel Pons,

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

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

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

Change subject: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus
......................................................................

mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus

Set up a 8-byte I/O range at 0x290-0x297 as PIIX4's generic device 9,
which activates a chip select when this range is accessed.

On the P2B family it connects to the W83781D hardware monitor,
allowing access to it over the ISA bus, just like vendor firmware.

Apparently this does not work on p3b-f, but no ill effects observed
either.

TEST=On p2b-ls lm-sensors can detect chip and get readings over ISA.

Change-Id: Iaed1df7230359e94c580c305f4769c8bb4f5fce0
Signed-off-by: Keith Hui 
---
A src/mainboard/asus/p2b/mainboard.c
1 file changed, 29 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/41561/2

Change in coreboot[master]: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus

[Keith Hui (Code Review)]

Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41561 )

Change subject: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus
......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/41561/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/41561/1//COMMIT_MSG@9 
PS1, Line 9: Setup
> Set up

Done

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
File src/mainboard/asus/p2b/mainboard.c:

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
PS1, Line 16: Setup
> The verb is spelled with a space: Set up.

Done

https://review.coreboot.org/c/coreboot/+/41561/1/src/mainboard/asus/p2b/main... 
PS1, Line 22: 0x60
> You can make two commits: one that adds register definitions to i82371eb. […]

Done

Change in coreboot[master]: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41561 )

Change subject: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/41561/2/src/mainboard/asus/p2b/main... 
File src/mainboard/asus/p2b/mainboard.c:

https://review.coreboot.org/c/coreboot/+/41561/2/src/mainboard/asus/p2b/main... 
PS2, Line 24: 0x60
Hmmm, this one slipped through

Change in coreboot[master]: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus

[Keith Hui (Code Review)]

Hello build bot (Jenkins), Angel Pons,

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

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

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

Change subject: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus
......................................................................

mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus

Set up a 8-byte I/O range at 0x290-0x297 as PIIX4's generic device 9,
which activates a chip select when this range is accessed.

On the P2B family it connects to the W83781D hardware monitor,
allowing access to it over the ISA bus, just like vendor firmware.

Apparently this does not work on p3b-f, but no ill effects observed
either.

TEST=On p2b-ls lm-sensors can detect chip and get readings over ISA.

Change-Id: Iaed1df7230359e94c580c305f4769c8bb4f5fce0
Signed-off-by: Keith Hui 
---
A src/mainboard/asus/p2b/mainboard.c
1 file changed, 29 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/41561/3

Change in coreboot[master]: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41561 )

Change subject: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/41561/2/src/mainboard/asus/p2b/main... 
File src/mainboard/asus/p2b/mainboard.c:

https://review.coreboot.org/c/coreboot/+/41561/2/src/mainboard/asus/p2b/main... 
PS2, Line 24: 0x60
> Hmmm, this one slipped through

Done

Change in coreboot[master]: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41561 )

Change subject: mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus
......................................................................

mb/asus/p2b: Enable hardware monitor access via I/O on ISA bus

Set up a 8-byte I/O range at 0x290-0x297 as PIIX4's generic device 9,
which activates a chip select when this range is accessed.

On the P2B family it connects to the W83781D hardware monitor,
allowing access to it over the ISA bus, just like vendor firmware.

Apparently this does not work on p3b-f, but no ill effects observed
either.

TEST=On p2b-ls lm-sensors can detect chip and get readings over ISA.

Change-Id: Iaed1df7230359e94c580c305f4769c8bb4f5fce0
Signed-off-by: Keith Hui 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41561
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
---
A src/mainboard/asus/p2b/mainboard.c
1 file changed, 29 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/mainboard/asus/p2b/mainboard.c b/src/mainboard/asus/p2b/mainboard.c
new file mode 100644
index 0000000..87838b4
--- /dev/null
+++ b/src/mainboard/asus/p2b/mainboard.c
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <device/pci_ops.h>
+#include <southbridge/intel/i82371eb/i82371eb.h>
+
+/**
+ * Mainboard specific enables.
+ *
+ * @param chip_info Ignored
+ */
+static void mainboard_init(void *chip_info)
+{
+	const pci_devfn_t px43 = PCI_DEV(0, 4, 3);
+	u32 reg;
+	/*
+	 * Set up an 8-byte generic I/O decode block at device 9.
+	 * This will be for W83781D hardware monitor.
+	 * Port 0x290 mask 0x007
+	 *
+	 * This should enable access to W83781D over the ISA bus.
+	 */
+	reg = pci_s_read_config32(px43, DEVRESB);
+	reg |= (0x290 | (0xe7 << 16));
+	pci_s_write_config32(px43, DEVRESB, reg);
+}
+
+struct chip_operations mainboard_ops = {
+	.init = mainboard_init
+};