[coreboot-gerrit] Change in coreboot[master]: amd/stoneyridge: Create new name/IRQ association
Richard Spiegel (Code Review)
gerrit at coreboot.org
Fri Dec 1 16:44:15 CET 2017
Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/22667
Change subject: amd/stoneyridge: Create new name/IRQ association
......................................................................
amd/stoneyridge: Create new name/IRQ association
Table intr_types[] is hard to maintain, and has unused spaces filled with
NULL. A new table format is needed that creates strong association between
the APIC register index and the associated IRQ name, is easy to maintain
and has no unused space (index) to indicate that a particular register is
unused while still indicating which registers are valid.
Also, the string that defines the name of associated IRQ should be declared
with "#define" in a header, but must be physically initiated in a source
file. The "#define" must make a strong association between the used register
index and the associated IRQ name. Example:
BUG=b:69868534
Change-Id: I2dde4d016cc3228e50dcfadd2d3586a3609e608d
Signed-off-by: Richard Spiegel <richard.spiegel at silverbackltd.com>
---
M src/soc/amd/common/amd_pci_util.c
M src/soc/amd/common/amd_pci_util.h
M src/soc/amd/stoneyridge/include/soc/amd_pci_int_defs.h
M src/soc/amd/stoneyridge/southbridge.c
4 files changed, 254 insertions(+), 70 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/22667/1
diff --git a/src/soc/amd/common/amd_pci_util.c b/src/soc/amd/common/amd_pci_util.c
index 577b5cb..371becf 100644
--- a/src/soc/amd/common/amd_pci_util.c
+++ b/src/soc/amd/common/amd_pci_util.c
@@ -2,6 +2,7 @@
* This file is part of the coreboot project.
*
* Copyright (C) 2014 Sage Electronic Engineering, LLC.
+ * Copyright (C) 2017 Advanced Micro Devices, 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
@@ -20,7 +21,6 @@
#include <amd_pci_util.h>
#include <pc80/i8259.h>
#include <soc/amd_pci_int_defs.h>
-#include <amd_pci_int_types.h>
const struct pirq_struct *pirq_data_ptr;
u32 pirq_data_size;
@@ -54,13 +54,16 @@
*/
void write_pci_int_table(void)
{
- u8 byte;
+ uint8_t byte;
+ size_t i, limit;
+ irq_idx_name *idx_name;
- if (picr_data_ptr == NULL || intr_data_ptr == NULL) {
+ idx_name = sb_get_apic_reg_association(&limit);
+ if (picr_data_ptr == NULL || idx_name == NULL) {
printk(BIOS_ERR, "Warning: Can't write PCI_INTR 0xC00/0xC01"
" registers because\n"
- "'mainboard_picr_data' or 'mainboard_intr_data'"
- " tables are NULL\n");
+ "'mainboard_picr_data' or"
+ " irq_association' tables are NULL\n");
return;
}
@@ -68,26 +71,37 @@
printk(BIOS_DEBUG, "PCI_INTR tables: Writing registers C00/C01 for PIC"
" mode PCI IRQ routing:\n"
"\tPCI_INTR_INDEX\t\tPCI_INTR_DATA\n");
- for (byte = 0 ; byte < FCH_INT_TABLE_SIZE ; byte++) {
- if (intr_types[byte]) {
- write_pci_int_idx(byte, 0, (u8) picr_data_ptr[byte]);
- printk(BIOS_DEBUG, "\t0x%02X %s\t: 0x%02X\n",
- byte, intr_types[byte],
- read_pci_int_idx(byte, 0));
- }
+ /*
+ * Iterate table idx_name, indexes outside the table are ignored
+ * (assumed not connected within the chip). For each iteration,
+ * get the register index "byte" and the name of the associated
+ * IRQ source for printing.
+ */
+ for (i = 0 ; i < limit; i++) {
+ byte = idx_name[i].index;
+ write_pci_int_idx(byte, 0, (u8) picr_data_ptr[byte]);
+ printk(BIOS_DEBUG, "\t0x%02X %s\t: 0x%02X\n",
+ byte, idx_name[i].name,
+ read_pci_int_idx(byte, 0));
+
}
/* APIC IRQ routine */
printk(BIOS_DEBUG, "PCI_INTR tables: Writing registers C00/C01 for APIC"
" mode PCI IRQ routing:\n"
"\tPCI_INTR_INDEX\t\tPCI_INTR_DATA\n");
- for (byte = 0 ; byte < FCH_INT_TABLE_SIZE ; byte++) {
- if (intr_types[byte]) {
- write_pci_int_idx(byte, 1, (u8) intr_data_ptr[byte]);
- printk(BIOS_DEBUG, "\t0x%02X %s\t: 0x%02X\n",
- byte, intr_types[byte],
- read_pci_int_idx(byte, 1));
- }
+ for (i = 0 ; i < limit; i++) {
+ /*
+ * Get the register index byte from the table. Indexes outside
+ * the table are ignored. The table also provides the name of
+ * the associated IRQ source for printing.
+ */
+ byte = idx_name[i].index;
+ write_pci_int_idx(byte, 1, (u8) intr_data_ptr[byte]);
+ printk(BIOS_DEBUG, "\t0x%02X %s\t: 0x%02X\n",
+ byte, idx_name[i].name,
+ read_pci_int_idx(byte, 1));
+
}
}
@@ -108,6 +122,15 @@
u16 devfn = 0; /* A PCI Device and Function number */
u8 bridged_device = 0; /* This device is on a PCI bridge */
u32 i = 0;
+ u16 idx_limit = 0;
+ size_t j, limit;
+ irq_idx_name *idx_name;
+
+ idx_name = sb_get_apic_reg_association(&limit);
+ for (j = 0; j < limit; j++) {
+ if (idx_name[j].index > idx_limit)
+ idx_limit = idx_name[j].index;
+ }
if (pirq_data_ptr == NULL) {
printk(BIOS_WARNING, "Warning: Can't write PCI IRQ assignments"
@@ -172,11 +195,11 @@
" perhaps this device was"
" defined wrong?\n");
continue;
- } else if (pci_intr_idx >= FCH_INT_TABLE_SIZE) {
+ } else if (pci_intr_idx > idx_limit) {
/* Index out of bounds */
printk(BIOS_ERR, "%s: got 0xC00/0xC01 table index"
- " 0x%x, max is 0x%x\n", __func__,
- pci_intr_idx, FCH_INT_TABLE_SIZE);
+ " 0x%x, max is 0x%02x\n", __func__,
+ pci_intr_idx, idx_limit);
continue;
}
@@ -208,10 +231,29 @@
if (bridged_device)
printk(BIOS_SPEW, "\tSwizzled to\t: %d (%s)\n",
target_pin, pin_to_str(target_pin));
- printk(BIOS_SPEW, "\tPCI_INTR idx\t: 0x%02x (%s)\n"
- "\tINT_LINE\t: 0x%X (IRQ %d)\n",
- pci_intr_idx, intr_types[pci_intr_idx],
- int_line, int_line);
+
+ /*
+ * Find the name associated with register [pci_intr_idx}
+ * and print information.
+ */
+ i = 0; /* no name yet */
+ for (j = 0; j < limit; j++) {
+ if (idx_name[j].index == pci_intr_idx) {
+ i++; /* name found */
+ break;
+ }
+ }
+ if (i) {
+ printk(BIOS_SPEW, "\tPCI_INTR idx\t: 0x%02x (%s)\n"
+ "\tINT_LINE\t: 0x%X (IRQ %d)\n",
+ pci_intr_idx, idx_name[j].name,
+ int_line, int_line);
+ } else {
+ printk(BIOS_SPEW, "Got register index 0x%02x"
+ " undefined in table irq_idx_name,\n"
+ " perhaps this device was"
+ " defined wrong?\n", pci_intr_idx);
+ }
} /* for (dev = all_devices) */
printk(BIOS_DEBUG, "PCI_CFG IRQ: Finished writing PCI config space"
" IRQ assignments\n");
diff --git a/src/soc/amd/common/amd_pci_util.h b/src/soc/amd/common/amd_pci_util.h
index 39ffced..5e285f8 100644
--- a/src/soc/amd/common/amd_pci_util.h
+++ b/src/soc/amd/common/amd_pci_util.h
@@ -2,6 +2,7 @@
* This file is part of the coreboot project.
*
* Copyright (C) 2014 Sage Electronic Engineering, LLC.
+ * Copyright (C) 2017 Advanced Micro Devices, 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
@@ -17,6 +18,7 @@
#define __AMD_PCI_UTIL_H__
#include <stdint.h>
+#include <soc/amd_pci_int_defs.h>
/* FCH index/data registers */
#define PCI_INTR_INDEX 0xc00
@@ -27,6 +29,11 @@
u8 PIN[4]; /* PINA/B/C/D are index 0/1/2/3 */
};
+typedef struct {
+ uint8_t index;
+ const char * const name;
+} irq_idx_name;
+
extern const struct pirq_struct *pirq_data_ptr;
extern u32 pirq_data_size;
extern const u8 *intr_data_ptr;
@@ -36,5 +43,6 @@
void write_pci_int_idx(u8 index, int mode, u8 data);
void write_pci_cfg_irqs(void);
void write_pci_int_table(void);
+irq_idx_name *sb_get_apic_reg_association(size_t *size);
#endif /* __AMD_PCI_UTIL_H__ */
diff --git a/src/soc/amd/stoneyridge/include/soc/amd_pci_int_defs.h b/src/soc/amd/stoneyridge/include/soc/amd_pci_int_defs.h
index a8e75f63..e16cf7e 100644
--- a/src/soc/amd/stoneyridge/include/soc/amd_pci_int_defs.h
+++ b/src/soc/amd/stoneyridge/include/soc/amd_pci_int_defs.h
@@ -2,6 +2,7 @@
* This file is part of the coreboot project.
*
* Copyright (C) 2014 Sage Electronic Engineering, LLC.
+ * Copyright (C) 2017 Advanced Micro Devices, 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
@@ -17,59 +18,136 @@
#define __AMD_PCI_INT_DEFS_H__
/*
- * PIRQ and device routing - these define the index
- * into the FCH PCI_INTR 0xC00/0xC01 interrupt
- * routing table
+ * PIRQ and device routing - these define the index into the
+ * FCH PCI_INTR 0xC00/0xC01 interrupt routing table.
+ * Note: Names of associated IRQ should be declared bellow the
+ * associated register index definition. This makes it easy to
+ * maintain index and name association, and makes it visible if
+ * a name is defined without and associated declared index or
+ * vice-versa.
*/
#define PIRQ_NC 0x1f /* Not Used */
-#define PIRQ_A 0x00 /* INT A */
-#define PIRQ_B 0x01 /* INT B */
-#define PIRQ_C 0x02 /* INT C */
-#define PIRQ_D 0x03 /* INT D */
-#define PIRQ_E 0x04 /* INT E */
-#define PIRQ_F 0x05 /* INT F */
-#define PIRQ_G 0x06 /* INT G */
-#define PIRQ_H 0x07 /* INT H */
-#define PIRQ_MISC 0x08 /* Miscellaneous IRQ Settings - See FCH Spec */
-#define PIRQ_MISC0 0x09 /* Miscellaneous0 IRQ Settings */
-#define PIRQ_MISC1 0x0a /* Miscellaneous1 IRQ Settings */
-#define PIRQ_MISC2 0x0b /* Miscellaneous2 IRQ Settings */
-#define PIRQ_SIRQA 0x0c /* Serial IRQ INTA */
-#define PIRQ_SIRQB 0x0d /* Serial IRQ INTB */
-#define PIRQ_SIRQC 0x0e /* Serial IRQ INTC */
-#define PIRQ_SIRQD 0x0f /* Serial IRQ INTD */
-#define PIRQ_SCI 0x10 /* SCI IRQ */
-#define PIRQ_SMBUS 0x11 /* SMBUS 14h.0 */
-#define PIRQ_ASF 0x12 /* ASF */
-#define PIRQ_HDA 0x13 /* HDA 14h.2 */
-#define PIRQ_FC 0x14 /* FC */
-#define PIRQ_GEC 0x15 /* GEC */
-#define PIRQ_PMON 0x16 /* Performance Monitor */
-#define PIRQ_SD 0x17 /* SD */
-#define PIRQ_IMC0 0x20 /* IMC INT0 */
-#define PIRQ_IMC1 0x21 /* IMC INT1 */
-#define PIRQ_IMC2 0x22 /* IMC INT2 */
-#define PIRQ_IMC3 0x23 /* IMC INT3 */
-#define PIRQ_IMC4 0x24 /* IMC INT4 */
-#define PIRQ_IMC5 0x25 /* IMC INT5 */
-#define PIRQ_OHCI1 0x30 /* USB OHCI 12h.0 */
-#define PIRQ_EHCI1 0x31 /* USB EHCI 12h.2 */
-#define PIRQ_OHCI2 0x32 /* USB OHCI 13h.0 */
-#define PIRQ_EHCI2 0x33 /* USB EHCI 13h.2 */
-#define PIRQ_OHCI3 0x34 /* USB OHCI 16h.0 */
-#define PIRQ_EHCI3 0x35 /* USB EHCI 16h.2 */
-#define PIRQ_OHCI4 0x36 /* USB OHCI 14h.5 */
-#define PIRQ_IDE 0x40 /* IDE 14h.1 */
-#define PIRQ_SATA 0x41 /* SATA 11h.0 */
+/* Intentionally left without a name */
-#define FCH_INT_TABLE_SIZE 0x76
+#define PIRQ_A 0x00 /* INT A */
+#define INDEX_0X00_NAME "INTA#\t"
+
+#define PIRQ_B 0x01 /* INT B */
+#define INDEX_0X01_NAME "INTB#\t"
+
+#define PIRQ_C 0x02 /* INT C */
+#define INDEX_0X02_NAME "INTC#\t"
+
+#define PIRQ_D 0x03 /* INT D */
+#define INDEX_0X03_NAME "INTD#\t"
+
+#define PIRQ_E 0x04 /* INT E */
+#define INDEX_0X04_NAME "INTE#\t"
+
+#define PIRQ_F 0x05 /* INT F */
+#define INDEX_0X05_NAME "INTF#\t"
+
+#define PIRQ_G 0x06 /* INT G */
+#define INDEX_0X06_NAME "INTG#\t"
+
+#define PIRQ_H 0x07 /* INT H */
+#define INDEX_0X07_NAME "INTH#\t"
+
+#define PIRQ_MISC 0x08 /* Miscellaneous IRQ Settings - See FCH Spec */
+#define INDEX_0X08_NAME "Misc\t"
+
+#define PIRQ_MISC0 0x09 /* Miscellaneous0 IRQ Settings */
+#define INDEX_0X09_NAME "Misc0\t"
+
+#define PIRQ_MISC1 0x0a /* Miscellaneous1 IRQ Settings */
+#define INDEX_0X0A_NAME "Misc1\t"
+
+#define PIRQ_MISC2 0x0b /* Miscellaneous2 IRQ Settings */
+#define INDEX_0X0B_NAME "Misc2\t"
+
+#define PIRQ_SIRQA 0x0c /* Serial IRQ INTA */
+#define INDEX_0X0C_NAME "Ser IRQ INTA"
+
+#define PIRQ_SIRQB 0x0d /* Serial IRQ INTB */
+#define INDEX_0X0D_NAME "Ser IRQ INTB"
+
+#define PIRQ_SIRQC 0x0e /* Serial IRQ INTC */
+#define INDEX_0X0E_NAME "Ser IRQ INTC"
+
+#define PIRQ_SIRQD 0x0f /* Serial IRQ INTD */
+#define INDEX_0X0F_NAME "Ser IRQ INTD"
+
+#define PIRQ_SCI 0x10 /* SCI IRQ */
+#define INDEX_0X10_NAME "SCI\t"
+
+#define PIRQ_SMBUS 0x11 /* SMBUS 14h.0 */
+#define INDEX_0X11_NAME "SMBUS0\t"
+
+#define PIRQ_ASF 0x12 /* ASF */
+#define INDEX_0X12_NAME "ASF\t"
+
+#define PIRQ_HDA 0x13 /* HDA 14h.2 */
+#define INDEX_0X13_NAME "HDA\t"
+
+#define PIRQ_FC 0x14 /* FC */
+#define INDEX_0X14_NAME "FC\t\t"
+
+#define PIRQ_PMON 0x16 /* Performance Monitor */
+#define INDEX_0X16_NAME "PerMon\t"
+
+#define PIRQ_SD 0x17 /* SD */
+#define INDEX_0X17_NAME "SD\t\t"
+
+#define PIRQ_SDIO 0x1a /* SDIO */
+#define INDEX_0X1A_NAME "SDIO\t"
+
+#define PIRQ_IMC0 0x20 /* IMC INT0 */
+#define INDEX_0X20_NAME "IMC INT0\t"
+
+#define PIRQ_IMC1 0x21 /* IMC INT1 */
+#define INDEX_0X21_NAME "IMC INT1\t"
+
+#define PIRQ_IMC2 0x22 /* IMC INT2 */
+#define INDEX_0X22_NAME "IMC INT2\t"
+
+#define PIRQ_IMC3 0x23 /* IMC INT3 */
+#define INDEX_0X23_NAME "IMC INT3\t"
+
+#define PIRQ_IMC4 0x24 /* IMC INT4 */
+#define INDEX_0X24_NAME "IMC INT4\t"
+
+#define PIRQ_IMC5 0x25 /* IMC INT5 */
+#define INDEX_0X25_NAME "IMC INT5\t"
+
+#define PIRQ_EHCI 0x30 /* USB EHCI 12h.0 */
+#define INDEX_0X30_NAME "EHCI\t"
+
+#define PIRQ_XHCI 0x34 /* USB XHCI 10h.0 */
+#define INDEX_0X34_NAME "XHCI\t"
+
+#define PIRQ_SATA 0x41 /* SATA 11h.0 */
+#define INDEX_0X41_NAME "SATA\t"
+
#define PIRQ_GPIO 0x62 /* GPIO Controller Interrupt */
+#define INDEX_0X62_NAME "GPIO\t"
+
#define PIRQ_I2C0 0x70
+#define INDEX_0X70_NAME "I2C0\t"
+
#define PIRQ_I2C1 0x71
+#define INDEX_0X71_NAME "I2C1\t"
+
#define PIRQ_I2C2 0x72
+#define INDEX_0X72_NAME "I2C2\t"
+
#define PIRQ_I2C3 0x73
+#define INDEX_0X73_NAME "I2C3\t"
+
#define PIRQ_UART0 0x74
+#define INDEX_0X74_NAME "UART0\t"
+
#define PIRQ_UART1 0x75
+#define INDEX_0X75_NAME "UART1\t"
#endif /* __AMD_PCI_INT_DEFS_H__ */
diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c
index 600b064..e7bc6b5 100644
--- a/src/soc/amd/stoneyridge/southbridge.c
+++ b/src/soc/amd/stoneyridge/southbridge.c
@@ -1,7 +1,7 @@
/*
* This file is part of the coreboot project.
*
- * Copyright (C) 2010 Advanced Micro Devices, Inc.
+ * Copyright (C) 2010-2017 Advanced Micro Devices, 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
@@ -27,8 +27,64 @@
#include <amd_pci_util.h>
#include <soc/southbridge.h>
#include <soc/smi.h>
+#include <soc/amd_pci_int_defs.h>
#include <fchec.h>
+/*
+ * Table of APIC register index and associated IRQ name. Using IDX_XXX_NAME
+ * provides a visible association with the index, therefor helping
+ * maintainability of table. If a new index/name is defined in
+ * amd_pci_int_defs.h, just add the pair at the end of this table.
+ * Order is not important.
+ */
+static irq_idx_name irq_association[] = {
+ {0x00, INDEX_0X00_NAME},
+ {0x01, INDEX_0X01_NAME},
+ {0x02, INDEX_0X02_NAME},
+ {0x03, INDEX_0X03_NAME},
+ {0x04, INDEX_0X04_NAME},
+ {0x05, INDEX_0X05_NAME},
+ {0x06, INDEX_0X06_NAME},
+ {0x07, INDEX_0X07_NAME},
+ {0x08, INDEX_0X08_NAME},
+ {0x09, INDEX_0X09_NAME},
+ {0x0a, INDEX_0X0A_NAME},
+ {0x0b, INDEX_0X0B_NAME},
+ {0x0c, INDEX_0X0C_NAME},
+ {0x0d, INDEX_0X0D_NAME},
+ {0x0e, INDEX_0X0E_NAME},
+ {0x0f, INDEX_0X0F_NAME},
+ {0x10, INDEX_0X10_NAME},
+ {0x11, INDEX_0X11_NAME},
+ {0x12, INDEX_0X12_NAME},
+ {0x13, INDEX_0X13_NAME},
+ {0x14, INDEX_0X14_NAME},
+ {0x16, INDEX_0X16_NAME},
+ {0x17, INDEX_0X17_NAME},
+ {0x1a, INDEX_0X1A_NAME},
+ {0x20, INDEX_0X20_NAME},
+ {0x21, INDEX_0X21_NAME},
+ {0x22, INDEX_0X22_NAME},
+ {0x23, INDEX_0X23_NAME},
+ {0x24, INDEX_0X24_NAME},
+ {0x25, INDEX_0X25_NAME},
+ {0x30, INDEX_0X30_NAME},
+ {0x34, INDEX_0X34_NAME},
+ {0x41, INDEX_0X41_NAME},
+ {0x62, INDEX_0X62_NAME},
+ {0x70, INDEX_0X70_NAME},
+ {0x71, INDEX_0X71_NAME},
+ {0x72, INDEX_0X72_NAME},
+ {0x73, INDEX_0X73_NAME},
+ {0x74, INDEX_0X74_NAME},
+ {0x75, INDEX_0X75_NAME}
+};
+
+irq_idx_name *sb_get_apic_reg_association(size_t *size)
+{
+ *size = ARRAY_SIZE(irq_association);
+ return irq_association;
+}
int acpi_get_sleep_type(void)
{
--
To view, visit https://review.coreboot.org/22667
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2dde4d016cc3228e50dcfadd2d3586a3609e608d
Gerrit-Change-Number: 22667
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Spiegel <richard.spiegel at silverbackltd.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20171201/da30c13f/attachment-0001.html>
More information about the coreboot-gerrit
mailing list