Change in coreboot[master]: sb/intel/bd82x6x/pch.c: Extract common functions

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/49167 )

Change subject: sb/intel/bd82x6x/pch.c: Extract common functions
......................................................................

sb/intel/bd82x6x/pch.c: Extract common functions

PCH identification functions and `pch_iobp_update` are used in multiple
stages. Move them out of `pch.c` to drop some ugly preprocessor usage.
Subsequent commits will use `pch_iobp_update` in romstage as well.

Change-Id: I8d33338a4f74fd03c8f99f8fcece99b63c28adab
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/bd82x6x/Makefile.inc
A src/southbridge/intel/bd82x6x/common.c
M src/southbridge/intel/bd82x6x/pch.c
M src/southbridge/intel/bd82x6x/pch.h
4 files changed, 122 insertions(+), 119 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/49167/1

diff --git a/src/southbridge/intel/bd82x6x/Makefile.inc b/src/southbridge/intel/bd82x6x/Makefile.inc
index 687fc97..469a50e 100644
--- a/src/southbridge/intel/bd82x6x/Makefile.inc
+++ b/src/southbridge/intel/bd82x6x/Makefile.inc
@@ -7,6 +7,7 @@

ramstage-y += pch.c
 ramstage-y += azalia.c
+ramstage-y += common.c
 ramstage-y += fadt.c
 ramstage-y += lpc.c
 ramstage-y += pci.c
@@ -26,8 +27,9 @@

ramstage-$(CONFIG_ELOG) += elog.c

-smm-y += smihandler.c me.c me_8.x.c pch.c me_common.c
+smm-y += common.c smihandler.c me.c me_8.x.c me_common.c

+romstage-y += common.c
 romstage-y += me_status.c
 romstage-y += early_rcba.c
 romstage-y += early_pch.c
diff --git a/src/southbridge/intel/bd82x6x/common.c b/src/southbridge/intel/bd82x6x/common.c
new file mode 100644
index 0000000..7480174
--- /dev/null
+++ b/src/southbridge/intel/bd82x6x/common.c
@@ -0,0 +1,118 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#define __SIMPLE_DEVICE__
+
+#include <console/console.h>
+#include <delay.h>
+#include <device/pci.h>
+#include <device/pci_def.h>
+#include <device/pci_ops.h>
+#include <stdint.h>
+
+#include "pch.h"
+
+int pch_silicon_revision(void)
+{
+	static int pch_revision_id = -1;
+
+	if (pch_revision_id < 0)
+		pch_revision_id = pci_read_config8(PCH_LPC_DEV,	PCI_REVISION_ID);
+
+	return pch_revision_id;
+}
+
+int pch_silicon_type(void)
+{
+	static int pch_type = -1;
+
+	if (pch_type < 0)
+		pch_type = pci_read_config8(PCH_LPC_DEV, PCI_DEVICE_ID + 1);
+
+	return pch_type;
+}
+
+int pch_silicon_supported(int type, int rev)
+{
+	int cur_type = pch_silicon_type();
+	int cur_rev = pch_silicon_revision();
+
+	switch (type) {
+	case PCH_TYPE_CPT:
+		/* CougarPoint minimum revision */
+		if (cur_type == PCH_TYPE_CPT && cur_rev >= rev)
+			return 1;
+		/* PantherPoint any revision */
+		if (cur_type == PCH_TYPE_PPT)
+			return 1;
+		break;
+
+	case PCH_TYPE_PPT:
+		/* PantherPoint minimum revision */
+		if (cur_type == PCH_TYPE_PPT && cur_rev >= rev)
+			return 1;
+		break;
+	}
+
+	return 0;
+}
+
+#define IOBP_RETRY 1000
+static inline int iobp_poll(void)
+{
+	unsigned int try = IOBP_RETRY;
+	u32 data;
+
+	while (try--) {
+		data = RCBA32(IOBPS);
+		if ((data & 1) == 0)
+			return 1;
+		udelay(10);
+	}
+
+	printk(BIOS_ERR, "IOBP timeout\n");
+	return 0;
+}
+
+void pch_iobp_update(u32 address, u32 andvalue, u32 orvalue)
+{
+	u32 data;
+
+	/* Set the address */
+	RCBA32(IOBPIRI) = address;
+
+	/* READ OPCODE */
+	if (pch_silicon_supported(PCH_TYPE_CPT, PCH_STEP_B0))
+		RCBA32(IOBPS) = IOBPS_RW_BX;
+	else
+		RCBA32(IOBPS) = IOBPS_READ_AX;
+	if (!iobp_poll())
+		return;
+
+	/* Read IOBP data */
+	data = RCBA32(IOBPD);
+	if (!iobp_poll())
+		return;
+
+	/* Check for successful transaction */
+	if ((RCBA32(IOBPS) & 0x6) != 0) {
+		printk(BIOS_ERR, "IOBP read 0x%08x failed\n", address);
+		return;
+	}
+
+	/* Update the data */
+	data &= andvalue;
+	data |= orvalue;
+
+	/* WRITE OPCODE */
+	if (pch_silicon_supported(PCH_TYPE_CPT, PCH_STEP_B0))
+		RCBA32(IOBPS) = IOBPS_RW_BX;
+	else
+		RCBA32(IOBPS) = IOBPS_WRITE_AX;
+	if (!iobp_poll())
+		return;
+
+	/* Write IOBP data */
+	RCBA32(IOBPD) = data;
+	if (!iobp_poll())
+		return;
+}
diff --git a/src/southbridge/intel/bd82x6x/pch.c b/src/southbridge/intel/bd82x6x/pch.c
index 82b95f6..1c8786d 100644
--- a/src/southbridge/intel/bd82x6x/pch.c
+++ b/src/southbridge/intel/bd82x6x/pch.c
@@ -11,123 +11,6 @@
 #include "chip.h"
 #include "pch.h"

-int pch_silicon_revision(void)
-{
-	static int pch_revision_id = -1;
-
-#ifdef __SIMPLE_DEVICE__
-	pci_devfn_t dev = PCI_DEV(0, 0x1f, 0);
-#else
-	struct device *dev = pcidev_on_root(0x1f, 0);
-#endif
-
-	if (pch_revision_id < 0)
-		pch_revision_id = pci_read_config8(dev,	PCI_REVISION_ID);
-	return pch_revision_id;
-}
-
-int pch_silicon_type(void)
-{
-	static int pch_type = -1;
-
-#ifdef __SIMPLE_DEVICE__
-	pci_devfn_t dev = PCI_DEV(0, 0x1f, 0);
-#else
-	struct device *dev = pcidev_on_root(0x1f, 0);
-#endif
-
-	if (pch_type < 0)
-		pch_type = pci_read_config8(dev, PCI_DEVICE_ID + 1);
-	return pch_type;
-}
-
-static int pch_silicon_supported(int type, int rev)
-{
-	int cur_type = pch_silicon_type();
-	int cur_rev = pch_silicon_revision();
-
-	switch (type) {
-	case PCH_TYPE_CPT:
-		/* CougarPoint minimum revision */
-		if (cur_type == PCH_TYPE_CPT && cur_rev >= rev)
-			return 1;
-		/* PantherPoint any revision */
-		if (cur_type == PCH_TYPE_PPT)
-			return 1;
-		break;
-
-	case PCH_TYPE_PPT:
-		/* PantherPoint minimum revision */
-		if (cur_type == PCH_TYPE_PPT && cur_rev >= rev)
-			return 1;
-		break;
-	}
-
-	return 0;
-}
-
-#define IOBP_RETRY 1000
-static inline int iobp_poll(void)
-{
-	unsigned int try = IOBP_RETRY;
-	u32 data;
-
-	while (try--) {
-		data = RCBA32(IOBPS);
-		if ((data & 1) == 0)
-			return 1;
-		udelay(10);
-	}
-
-	printk(BIOS_ERR, "IOBP timeout\n");
-	return 0;
-}
-
-void pch_iobp_update(u32 address, u32 andvalue, u32 orvalue)
-{
-	u32 data;
-
-	/* Set the address */
-	RCBA32(IOBPIRI) = address;
-
-	/* READ OPCODE */
-	if (pch_silicon_supported(PCH_TYPE_CPT, PCH_STEP_B0))
-		RCBA32(IOBPS) = IOBPS_RW_BX;
-	else
-		RCBA32(IOBPS) = IOBPS_READ_AX;
-	if (!iobp_poll())
-		return;
-
-	/* Read IOBP data */
-	data = RCBA32(IOBPD);
-	if (!iobp_poll())
-		return;
-
-	/* Check for successful transaction */
-	if ((RCBA32(IOBPS) & 0x6) != 0) {
-		printk(BIOS_ERR, "IOBP read 0x%08x failed\n", address);
-		return;
-	}
-
-	/* Update the data */
-	data &= andvalue;
-	data |= orvalue;
-
-	/* WRITE OPCODE */
-	if (pch_silicon_supported(PCH_TYPE_CPT, PCH_STEP_B0))
-		RCBA32(IOBPS) = IOBPS_RW_BX;
-	else
-		RCBA32(IOBPS) = IOBPS_WRITE_AX;
-	if (!iobp_poll())
-		return;
-
-	/* Write IOBP data */
-	RCBA32(IOBPD) = data;
-	if (!iobp_poll())
-		return;
-}
-
-#ifndef __SIMPLE_DEVICE__
 /* Set bit in function disable register to hide this device */
 static void pch_hide_devfn(unsigned int devfn)
 {
@@ -426,4 +309,3 @@
    CHIP_NAME("Intel Series 6/7 (Cougar Point/Panther Point) Southbridge")
    .enable_dev = pch_enable,
 };
-#endif
diff --git a/src/southbridge/intel/bd82x6x/pch.h b/src/southbridge/intel/bd82x6x/pch.h
index 1840a2b..dcab839 100644
--- a/src/southbridge/intel/bd82x6x/pch.h
+++ b/src/southbridge/intel/bd82x6x/pch.h
@@ -42,6 +42,7 @@

int pch_silicon_revision(void);
 int pch_silicon_type(void);
+int pch_silicon_supported(int type, int rev);
 void pch_iobp_update(u32 address, u32 andvalue, u32 orvalue);

void enable_usb_bar(void);

[Felix Singer (Code Review)]

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49167 )

Change subject: sb/intel/bd82x6x/pch.c: Extract common functions
......................................................................

Patch Set 1: Code-Review+1

[Angel Pons (Code Review)]

Hello  Felix Singer, build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,

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

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

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

Change subject: sb/intel/bd82x6x/pch.c: Extract common functions
......................................................................

sb/intel/bd82x6x/pch.c: Extract common functions

PCH identification functions and `pch_iobp_update` are used in multiple
stages. Move them out of `pch.c` to drop some ugly preprocessor usage.
Subsequent commits will use `pch_iobp_update` in romstage as well.

Change-Id: I8d33338a4f74fd03c8f99f8fcece99b63c28adab
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/bd82x6x/Makefile.inc
A src/southbridge/intel/bd82x6x/common.c
M src/southbridge/intel/bd82x6x/pch.c
M src/southbridge/intel/bd82x6x/pch.h
4 files changed, 122 insertions(+), 119 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/49167/2

[M] Change in coreboot[master]: sb/intel/bd82x6x/pch.c: Extract common functions

[Martin L Roth (Code Review)]

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/49167?usp=email )

Change subject: sb/intel/bd82x6x/pch.c: Extract common functions
......................................................................

Abandoned

This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.

[M] Change in coreboot[master]: sb/intel/bd82x6x/pch.c: Extract common functions

[Angel Pons (Code Review)]

Angel Pons has restored this change. ( https://review.coreboot.org/c/coreboot/+/49167?usp=email )

Change subject: sb/intel/bd82x6x/pch.c: Extract common functions
......................................................................

Restored

[M] Change in coreboot[master]: sb/intel/bd82x6x/pch.c: Extract common functions

[Felix Singer (Code Review)]

Attention is currently required from: Angel Pons.

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49167?usp=email )

Change subject: sb/intel/bd82x6x/pch.c: Extract common functions
......................................................................

Patch Set 2: Code-Review+2

[M] Change in coreboot[master]: sb/intel/bd82x6x/pch.c: Extract common functions

[Felix Singer (Code Review)]

Attention is currently required from: Angel Pons.

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49167?usp=email )

Change subject: sb/intel/bd82x6x/pch.c: Extract common functions
......................................................................

Patch Set 2:

(1 comment)

Patchset:

PS2: 
Needs a rebase

[M] Change in coreboot[master]: sb/intel/bd82x6x/pch.c: Extract common functions

[Patrick Rudolph (Code Review)]

Attention is currently required from: Angel Pons.

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49167?usp=email )

Change subject: sb/intel/bd82x6x/pch.c: Extract common functions
......................................................................

Patch Set 2:

(1 comment)

Patchset:

PS2: 
Rebased onto main here CB:79624

[M] Change in coreboot[master]: sb/intel/bd82x6x/pch.c: Extract common functions

[Felix Singer (Code Review)]

Felix Singer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/49167?usp=email )

Change subject: sb/intel/bd82x6x/pch.c: Extract common functions
......................................................................

Abandoned

Superseded by CB:79624