Hello Iru Cai,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to review the following change.
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
Currently only LynxPoint IOBP registers are added. The SATA IOBP registers are found in coreboot sb/intel/lynxpoint, and the USB IOBP registers are found in mrc.bin.
Tested with Dell Latitude E7240, which fails to read SATA_IOBP_SP*G3IR (does LynxPoint really use them?) and reads other IOBP registers successfully.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 153 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/1
diff --git a/util/inteltool/Makefile b/util/inteltool/Makefile index 1ae04d4..3b692fd 100644 --- a/util/inteltool/Makefile +++ b/util/inteltool/Makefile @@ -16,7 +16,7 @@
OBJS = inteltool.o pcr.o cpu.o gpio.o gpio_groups.o rootcmplx.o powermgt.o \ - memory.o pcie.o amb.o ivy_memory.o spi.o gfx.o ahci.o lpc.o + memory.o pcie.o amb.o ivy_memory.o spi.o gfx.o ahci.o lpc.o iobp.o
OS_ARCH = $(shell uname) ifeq ($(OS_ARCH), Darwin) diff --git a/util/inteltool/inteltool.h b/util/inteltool/inteltool.h index b314fe4..6c361b4 100644 --- a/util/inteltool/inteltool.h +++ b/util/inteltool/inteltool.h @@ -386,6 +386,7 @@ int print_pmbase(struct pci_dev *sb, struct pci_access *pacc); int print_lpc(struct pci_dev *sb, struct pci_access *pacc); int print_rcba(struct pci_dev *sb); +void print_iobp(struct pci_dev *sb, volatile uint8_t *rcba); int print_gpios(struct pci_dev *sb, int show_all, int show_diffs); const struct gpio_community *const *get_gpio_communities(struct pci_dev *const sb, size_t* community_count, diff --git a/util/inteltool/iobp.c b/util/inteltool/iobp.c new file mode 100644 index 0000000..dafcb00 --- /dev/null +++ b/util/inteltool/iobp.c @@ -0,0 +1,149 @@ +/* inteltool - dump all registers on an Intel CPU + chipset based system */ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <stdio.h> +#include <stdlib.h> +#include "inteltool.h" + +#define RCBA8(x) (*((volatile u8 *)(rcba + x))) +#define RCBA16(x) (*((volatile u16 *)(rcba + x))) +#define RCBA32(x) (*((volatile u32 *)(rcba + x))) +#define RCBA64(x) (*((volatile u64 *)(rcba + x))) +/* IO Buffer Programming */ +#define IOBPIRI 0x2330 +#define IOBPD 0x2334 +#define IOBPS 0x2338 +#define IOBPS_READY 0x0001 +#define IOBPS_TX_MASK 0x0006 +#define IOBPS_MASK 0xff00 +#define IOBPS_READ 0x0600 +#define IOBPS_WRITE 0x0700 +#define IOBPU 0x233a +#define IOBPU_MAGIC 0xf000 + +#define IOBP_RETRY 1000 +static inline int iobp_poll(volatile uint8_t *rcba) +{ + for (int try = IOBP_RETRY; try > 0; try --) { + u16 status = RCBA16(IOBPS); + if ((status & IOBPS_READY) == 0) + return 1; + // udelay(10); + } + + printf("IOBP: timeout waiting for transaction to complete\n"); + return 0; +} + +static u32 pch_iobp_read(volatile uint8_t *rcba, u32 address) +{ + u16 status; + + if (!iobp_poll(rcba)) + return 0; + + /* Set the address */ + RCBA32(IOBPIRI) = address; + + /* READ OPCODE */ + status = RCBA16(IOBPS); + status &= ~IOBPS_MASK; + status |= IOBPS_READ; + RCBA16(IOBPS) = status; + + /* Undocumented magic */ + RCBA16(IOBPU) = IOBPU_MAGIC; + + /* Set ready bit */ + status = RCBA16(IOBPS); + status |= IOBPS_READY; + RCBA16(IOBPS) = status; + + if (!iobp_poll(rcba)) + return 0; + + /* Check for successful transaction */ + status = RCBA16(IOBPS); + if (status & IOBPS_TX_MASK) { + printf("IOBP: read 0x%08x failed\n", address); + return 0; + } + + /* Read IOBP data */ + return RCBA32(IOBPD); +} + +typedef struct { + u32 addr; + char *name; +} iobp_register_t; + +static const iobp_register_t lynxpoint_iobp_registers[] = { + /* SATA IOBP */ + {0xea000151, "SATA_IOBP_SP0G3IR"}, + {0xea000051, "SATA_IOBP_SP1G3IR"}, + {0xea002550, "SATA_IOBP_SP0DTLE_DATA"}, + {0xea002554, "SATA_IOBP_SP0DTLE_EDGE"}, + {0xea002750, "SATA_IOBP_SP1DTLE_DATA"}, + {0xea002754, "SATA_IOBP_SP1DTLE_EDGE"}, + /* USB port IOBP */ + {0xe5004100, "USBP01"}, + {0xe5004200, "USBP02"}, + {0xe5004300, "USBP03"}, + {0xe5004400, "USBP04"}, + {0xe5004500, "USBP05"}, + {0xe5004600, "USBP06"}, + {0xe5004700, "USBP07"}, + {0xe5004800, "USBP08"}, + /* LynxPoint-LP doesn't have port 9-14, the IOBP values are zero */ + {0xe5004900, "USBP09"}, + {0xe5004a00, "USBP10"}, + {0xe5004b00, "USBP11"}, + {0xe5004c00, "USBP12"}, + {0xe5004d00, "USBP13"}, + {0xe5004e00, "USBP14"}, +}; + +void print_iobp(struct pci_dev *sb, volatile uint8_t *rcba) +{ + const iobp_register_t *iobp_registers = NULL; + size_t iobp_size = 0; + + switch (sb->device_id) { + case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_FULL: + case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_PREM: + case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_BASE: + case PCI_DEVICE_ID_INTEL_C8_MOBILE: + case PCI_DEVICE_ID_INTEL_C8_DESKTOP: + case PCI_DEVICE_ID_INTEL_Z87: + case PCI_DEVICE_ID_INTEL_Z85: + case PCI_DEVICE_ID_INTEL_HM86: + case PCI_DEVICE_ID_INTEL_H87: + case PCI_DEVICE_ID_INTEL_HM87: + case PCI_DEVICE_ID_INTEL_Q85: + case PCI_DEVICE_ID_INTEL_Q87: + case PCI_DEVICE_ID_INTEL_QM87: + case PCI_DEVICE_ID_INTEL_B85: + case PCI_DEVICE_ID_INTEL_C222: + case PCI_DEVICE_ID_INTEL_C224: + case PCI_DEVICE_ID_INTEL_C226: + case PCI_DEVICE_ID_INTEL_H81: + iobp_registers = lynxpoint_iobp_registers; + iobp_size = ARRAY_SIZE(lynxpoint_iobp_registers); + break; + default: + break; + } + + if (iobp_size == 0) + return; + + printf("\n============= IOBP ==============\n\n"); + + for (size_t i = 0; i < iobp_size; i++) { + u32 address = iobp_registers[i].addr; + const char *name = iobp_registers[i].name; + u32 v = pch_iobp_read(rcba, address); + printf("0x%08x: 0x%08x (%s)\n", address, v, name); + } +} diff --git a/util/inteltool/rootcmplx.c b/util/inteltool/rootcmplx.c index 490897f..9819af5 100644 --- a/util/inteltool/rootcmplx.c +++ b/util/inteltool/rootcmplx.c @@ -138,6 +138,8 @@ printf("0x%04x: 0x%08x\n", i, read32(rcba + i)); }
+ print_iobp(sb, rcba); + unmap_physical((void *)rcba, size); return 0; }
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 3:
(10 comments)
https://review.coreboot.org/c/coreboot/+/41473/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41473/3//COMMIT_MSG@13 PS3, Line 13: SATA_IOBP_SP*G3IR : (does LynxPoint really use them?) They don't seem to exist.
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@8 PS3, Line 8: x please use braces around macro parameters: `(x)`
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@8 PS3, Line 8: rcba this hidden parameter is ugly.
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@13 PS3, Line 13: #define IOBPIRI 0x2330 : #define IOBPD 0x2334 : #define IOBPS 0x2338 : #define IOBPS_READY 0x0001 : #define IOBPS_TX_MASK 0x0006 : #define IOBPS_MASK 0xff00 : #define IOBPS_READ 0x0600 : #define IOBPS_WRITE 0x0700 : #define IOBPU 0x233a : #define IOBPU_MAGIC 0xf000 Align values with tabs?
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@27 PS3, Line 27: no space on unary operators
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@84 PS3, Line 84: {0xea000051, "SATA_IOBP_SP1G3IR"}, missing per-port registers: (address is for port 0, calculate the other addresses accordingly)
SECRT88 EA002488h SECRT8C EA00248Ch SECRT90 EA002490h SECRT98 EA002498h SECRR1C EA00251Ch SECRR58 EA002558h
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@85 PS3, Line 85: {0xea002550, "SATA_IOBP_SP0DTLE_DATA"}, This is SECRR50, and there's up to six of them (one per SATA port):
Port 0: EA002550h Port 1: EA002750h Port 2: EA000950h Port 3: EA000B50h Port 4: EA002150h Port 5: EA002350h
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@86 PS3, Line 86: {0xea002554, "SATA_IOBP_SP0DTLE_EDGE"}, This is SECRR54 and is the same story as SECRR50
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@88 PS3, Line 88: {0xea002754, "SATA_IOBP_SP1DTLE_EDGE"}, Missing global registers:
SECRF00 EA008100h SECRF04 EA008104h
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@90 PS3, Line 90: USBP01 These registers are officially called U2ECR
Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@8 PS3, Line 8: rcba
this hidden parameter is ugly.
What is hidden?
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@13 PS3, Line 13: #define IOBPIRI 0x2330 : #define IOBPD 0x2334 : #define IOBPS 0x2338 : #define IOBPS_READY 0x0001 : #define IOBPS_TX_MASK 0x0006 : #define IOBPS_MASK 0xff00 : #define IOBPS_READ 0x0600 : #define IOBPS_WRITE 0x0700 : #define IOBPU 0x233a : #define IOBPU_MAGIC 0xf000
Align values with tabs?
It's a copy of lynxpoint/pch.h.
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@90 PS3, Line 90: USBP01
These registers are officially called U2ECR
Hmm, searched U2ECR and found https://medium.com/@jacksonchen_43335/bios-read-write-iobp-for-sata-or-usb-a..., ECR is "electrical control register".
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@8 PS3, Line 8: rcba
What is hidden?
`rcba`. If you use this macro, a variable named `rcba` needs to be declared
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@13 PS3, Line 13: #define IOBPIRI 0x2330 : #define IOBPD 0x2334 : #define IOBPS 0x2338 : #define IOBPS_READY 0x0001 : #define IOBPS_TX_MASK 0x0006 : #define IOBPS_MASK 0xff00 : #define IOBPS_READ 0x0600 : #define IOBPS_WRITE 0x0700 : #define IOBPU 0x233a : #define IOBPU_MAGIC 0xf000
It's a copy of lynxpoint/pch.h.
Still, it would help w.r.t. readability.
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@90 PS3, Line 90: USBP01
Hmm, searched U2ECR and found https://medium. […]
Yes, it's the same as here: https://miro.medium.com/max/670/1*2iJaywnJdqjNeCJQSm-8kw.png
Hello build bot (Jenkins), Stefan Reinauer, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to look at the new patch set (#4).
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
Currently only LynxPoint IOBP registers are added. The SATA IOBP registers are found in coreboot sb/intel/lynxpoint, and the USB IOBP registers are found in mrc.bin.
Tested with Dell Latitude E7240, which fails to read SATA_IOBP_SP*G3IR (does LynxPoint really use them?) and reads other IOBP registers successfully.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 151 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/4
Hello build bot (Jenkins), Stefan Reinauer, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to look at the new patch set (#5).
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
Currently only LynxPoint IOBP registers are added.
Tested with Dell Latitude E7240, in which inteltool outputs the IOBP register values except the SATA P2/P3 registers (LynxPoint-LP doesn't have these).
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 202 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/5
Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 5:
(8 comments)
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@8 PS3, Line 8: x
please use braces around macro parameters: `(x)`
Done
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@27 PS3, Line 27:
no space on unary operators
Done
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@84 PS3, Line 84: {0xea000051, "SATA_IOBP_SP1G3IR"},
missing per-port registers: (address is for port 0, calculate the other addresses accordingly) […]
Done
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@85 PS3, Line 85: {0xea002550, "SATA_IOBP_SP0DTLE_DATA"},
This is SECRR50, and there's up to six of them (one per SATA port): […]
Done
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@86 PS3, Line 86: {0xea002554, "SATA_IOBP_SP0DTLE_EDGE"},
This is SECRR54 and is the same story as SECRR50
Done
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@88 PS3, Line 88: {0xea002754, "SATA_IOBP_SP1DTLE_EDGE"},
Missing global registers: […]
Done
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@90 PS3, Line 90: USBP01
Yes, it's the same as here: https://miro.medium.com/max/670/1*2iJaywnJdqjNeCJQSm-8kw. […]
Done
https://review.coreboot.org/c/coreboot/+/41473/5/util/inteltool/iobp.c File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/5/util/inteltool/iobp.c@11 PS5, Line 11: #define IOBPIRI 0x2330 : #define IOBPD 0x2334 : #define IOBPS 0x2338 : #define IOBPS_READY 0x0001 : #define IOBPS_TX_MASK 0x0006 : #define IOBPS_MASK 0xff00 : #define IOBPS_READ 0x0600 : #define IOBPS_WRITE 0x0700 : #define IOBPU 0x233a : #define IOBPU_MAGIC 0xf000 Oh, I need to align the values instead of macros.
Pablo Stebler has uploaded a new patch set (#7) to the change originally created by Iru Cai (vimacs). ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
Currently only LynxPoint IOBP registers are added. The SATA IOBP registers are found in coreboot sb/intel/lynxpoint, and the USB IOBP registers are found in mrc.bin.
Tested with Dell Latitude E7240, which fails to read SATA_IOBP_SP*G3IR (does LynxPoint really use them?) and reads other IOBP registers successfully.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 153 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/7
Hello build bot (Jenkins), Pablo Stebler, Stefan Reinauer, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to look at the new patch set (#9).
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
Currently only LynxPoint IOBP registers are added.
Tested with Dell Latitude E7240, in which inteltool outputs the IOBP register values except the SATA P2/P3 registers (LynxPoint-LP doesn't have these).
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 202 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/9
Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@13 PS3, Line 13: #define IOBPIRI 0x2330 : #define IOBPD 0x2334 : #define IOBPS 0x2338 : #define IOBPS_READY 0x0001 : #define IOBPS_TX_MASK 0x0006 : #define IOBPS_MASK 0xff00 : #define IOBPS_READ 0x0600 : #define IOBPS_WRITE 0x0700 : #define IOBPU 0x233a : #define IOBPU_MAGIC 0xf000
Still, it would help w.r.t. readability.
Done
https://review.coreboot.org/c/coreboot/+/41473/5/util/inteltool/iobp.c File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/5/util/inteltool/iobp.c@11 PS5, Line 11: #define IOBPIRI 0x2330 : #define IOBPD 0x2334 : #define IOBPS 0x2338 : #define IOBPS_READY 0x0001 : #define IOBPS_TX_MASK 0x0006 : #define IOBPS_MASK 0xff00 : #define IOBPS_READ 0x0600 : #define IOBPS_WRITE 0x0700 : #define IOBPU 0x233a : #define IOBPU_MAGIC 0xf000
Oh, I need to align the values instead of macros.
Done
Hello build bot (Jenkins), Pablo Stebler, Stefan Reinauer, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to look at the new patch set (#10).
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
Currently LynxPoint IOBP registers are added, and it prints LynxPoint-LP IOBP registers for WildcatPoint-LP.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 258 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/10
Hello build bot (Jenkins), Pablo Stebler, Stefan Reinauer, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to look at the new patch set (#11).
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
Currently LynxPoint IOBP registers are added, and it prints LynxPoint-LP IOBP registers for WildcatPoint-LP.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 322 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/11
Hello build bot (Jenkins), Pablo Stebler, Stefan Reinauer, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to look at the new patch set (#13).
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
Currently LynxPoint IOBP registers are added, and it prints LynxPoint-LP IOBP registers for WildcatPoint-LP.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 324 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/13
Hello build bot (Jenkins), Pablo Stebler, Stefan Reinauer, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to look at the new patch set (#17).
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
This patch also adds LynxPoint and WildcatPoint-LP IOBP registers, which is used to get the USB and SATA configuration values for autoport.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 324 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/17
Attention is currently required from: Angel Pons. Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 17:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/41473/comment/adcb6008_0d831e5a PS3, Line 13: SATA_IOBP_SP*G3IR : (does LynxPoint really use them?)
They don't seem to exist.
removed
File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/comment/2b2d0b41_5c4c26f7 PS3, Line 8: rcba
`rcba`. […]
changed to use the rcba parameter
Attention is currently required from: Angel Pons, Iru Cai (vimacs). Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 17: Code-Review+1
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/41473/comment/d2e97f59_07f8e00a PS3, Line 13: SATA_IOBP_SP*G3IR : (does LynxPoint really use them?)
They don't seem to exist.
Ack
File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/comment/612986d3_5450e537 PS3, Line 8: rcba
`rcba`. […]
Done
File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/comment/4e490d2b_e8ec860c PS17, Line 8: , nit: space after the comma (same for RCBA32)
https://review.coreboot.org/c/coreboot/+/41473/comment/f0f79405_0b5ba3d2 PS17, Line 76: char * should be `const char *`, which means "pointer to `const char`"
https://review.coreboot.org/c/coreboot/+/41473/comment/d28db478_9b3472a1 PS17, Line 74: typedef struct { : u32 addr; : char *name; : } iobp_register_t; Is a typedef really needed?
struct iobp_register { u32 addr; const char *name; };
https://review.coreboot.org/c/coreboot/+/41473/comment/80f3df99_cea96487 PS17, Line 88: {0xea002558, "SECRR58P0"}, I'd like to avoid repeating the registers for each port. Also, this doesn't account for HSIO lane muxing. I'll look into it and make follow-up patches.
Nothing to do in this change.
Attention is currently required from: Angel Pons, Iru Cai (vimacs). Hello build bot (Jenkins), Pablo Stebler, Stefan Reinauer, Angel Pons, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to look at the new patch set (#18).
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
This patch also adds LynxPoint and WildcatPoint-LP IOBP registers, which is used to get the USB and SATA configuration values for autoport.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 324 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/18
Attention is currently required from: Angel Pons. Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 18:
(3 comments)
File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/comment/2054c2cd_b95027c1 PS17, Line 8: ,
nit: space after the comma (same for RCBA32)
Done
https://review.coreboot.org/c/coreboot/+/41473/comment/02d97764_963162da PS17, Line 76: char *
should be `const char *`, which means "pointer to `const char`"
Done
https://review.coreboot.org/c/coreboot/+/41473/comment/095849af_4c6ea8f4 PS17, Line 74: typedef struct { : u32 addr; : char *name; : } iobp_register_t;
Is a typedef really needed? […]
Not needed, but the following array definitions will have one more "struct" like "struct iobp_register" without using typedef. And other files in inteltool also use typedef like io_register_t, msr_entry_t.
Attention is currently required from: Iru Cai (vimacs). Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 18:
(1 comment)
File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/comment/f1c1629b_8fa44989 PS17, Line 74: typedef struct { : u32 addr; : char *name; : } iobp_register_t;
Not needed, but the following array definitions will have one more "struct" like "struct iobp_regist […]
https://doc.coreboot.org/contributing/coding_style.html#typedefs
Attention is currently required from: Iru Cai (vimacs). Hello build bot (Jenkins), Pablo Stebler, Stefan Reinauer, Angel Pons, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41473
to look at the new patch set (#19).
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
This patch also adds LynxPoint and WildcatPoint-LP IOBP registers, which is used to get the USB and SATA configuration values for autoport.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 324 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41473/19
Attention is currently required from: Angel Pons. Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 19:
(1 comment)
File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/comment/b711b926_1fbbf3cb PS17, Line 74: typedef struct { : u32 addr; : char *name; : } iobp_register_t;
I see. The code is updated.
Attention is currently required from: Iru Cai. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 22: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
inteltool: Support dumping IOBP register values
This patch also adds LynxPoint and WildcatPoint-LP IOBP registers, which is used to get the USB and SATA configuration values for autoport.
Change-Id: I1f11640fdff59a5317f19057476f7e48c2956ab9 Signed-off-by: Iru Cai mytbk920423@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41473 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M util/inteltool/Makefile M util/inteltool/inteltool.h A util/inteltool/iobp.c M util/inteltool/rootcmplx.c 4 files changed, 324 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/util/inteltool/Makefile b/util/inteltool/Makefile index 1ae04d4..3b692fd 100644 --- a/util/inteltool/Makefile +++ b/util/inteltool/Makefile @@ -16,7 +16,7 @@
OBJS = inteltool.o pcr.o cpu.o gpio.o gpio_groups.o rootcmplx.o powermgt.o \ - memory.o pcie.o amb.o ivy_memory.o spi.o gfx.o ahci.o lpc.o + memory.o pcie.o amb.o ivy_memory.o spi.o gfx.o ahci.o lpc.o iobp.o
OS_ARCH = $(shell uname) ifeq ($(OS_ARCH), Darwin) diff --git a/util/inteltool/inteltool.h b/util/inteltool/inteltool.h index 4a6eba4..1ad1224 100644 --- a/util/inteltool/inteltool.h +++ b/util/inteltool/inteltool.h @@ -401,6 +401,7 @@ int print_pmbase(struct pci_dev *sb, struct pci_access *pacc); int print_lpc(struct pci_dev *sb, struct pci_access *pacc); int print_rcba(struct pci_dev *sb); +void print_iobp(struct pci_dev *sb, volatile uint8_t *rcba); int print_gpios(struct pci_dev *sb, int show_all, int show_diffs); const struct gpio_community *const *get_gpio_communities(struct pci_dev *const sb, size_t* community_count, diff --git a/util/inteltool/iobp.c b/util/inteltool/iobp.c new file mode 100644 index 0000000..e6c4185 --- /dev/null +++ b/util/inteltool/iobp.c @@ -0,0 +1,320 @@ +/* inteltool - dump all registers on an Intel CPU + chipset based system */ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <stdio.h> +#include <stdlib.h> +#include "inteltool.h" + +#define RCBA16(rcba, x) (*((volatile u16 *)((rcba) + (x)))) +#define RCBA32(rcba, x) (*((volatile u32 *)((rcba) + (x)))) +/* IO Buffer Programming */ +#define IOBPIRI 0x2330 +#define IOBPD 0x2334 +#define IOBPS 0x2338 +#define IOBPS_READY 0x0001 +#define IOBPS_TX_MASK 0x0006 +#define IOBPS_MASK 0xff00 +#define IOBPS_READ 0x0600 +#define IOBPS_WRITE 0x0700 +#define IOBPU 0x233a +#define IOBPU_MAGIC 0xf000 + +#define IOBP_RETRY 1000 +static inline int iobp_poll(volatile uint8_t *rcba) +{ + for (int try = IOBP_RETRY; try > 0; try--) { + u16 status = RCBA16(rcba, IOBPS); + if ((status & IOBPS_READY) == 0) + return 1; + // udelay(10); + } + + printf("IOBP: timeout waiting for transaction to complete\n"); + return 0; +} + +static u32 pch_iobp_read(volatile uint8_t *rcba, u32 address) +{ + u16 status; + + if (!iobp_poll(rcba)) + return 0; + + /* Set the address */ + RCBA32(rcba, IOBPIRI) = address; + + /* READ OPCODE */ + status = RCBA16(rcba, IOBPS); + status &= ~IOBPS_MASK; + status |= IOBPS_READ; + RCBA16(rcba, IOBPS) = status; + + /* Undocumented magic */ + RCBA16(rcba, IOBPU) = IOBPU_MAGIC; + + /* Set ready bit */ + status = RCBA16(rcba, IOBPS); + status |= IOBPS_READY; + RCBA16(rcba, IOBPS) = status; + + if (!iobp_poll(rcba)) + return 0; + + /* Check for successful transaction */ + status = RCBA16(rcba, IOBPS); + if (status & IOBPS_TX_MASK) { + printf("IOBP: read 0x%08x failed\n", address); + return 0; + } + + /* Read IOBP data */ + return RCBA32(rcba, IOBPD); +} + +struct iobp_register { + u32 addr; + const char *name; +}; + +static const struct iobp_register lynxpoint_iobp_registers[] = { + /* SATA Electrical Control Register */ + {0xea002488, "SECRT88P0"}, + {0xea00248c, "SECRT8CP0"}, + {0xea002490, "SECRT90P0"}, + {0xea002498, "SECRT98P0"}, + {0xea00251c, "SECRR1CP0"}, + {0xea002550, "SECRR50P0"}, + {0xea002554, "SECRR54P0"}, + {0xea002558, "SECRR58P0"}, + + {0xea002688, "SECRT88P1"}, + {0xea00268c, "SECRT8CP1"}, + {0xea002690, "SECRT90P1"}, + {0xea002698, "SECRT98P1"}, + {0xea00271c, "SECRR1CP1"}, + {0xea002750, "SECRR50P1"}, + {0xea002754, "SECRR54P1"}, + {0xea002758, "SECRR58P1"}, + + {0xea000888, "SECRT88P2"}, + {0xea00088c, "SECRT8CP2"}, + {0xea000890, "SECRT90P2"}, + {0xea000898, "SECRT98P2"}, + {0xea00091c, "SECRR1CP2"}, + {0xea000950, "SECRR50P2"}, + {0xea000954, "SECRR54P2"}, + {0xea000958, "SECRR58P2"}, + + {0xea000a88, "SECRT88P3"}, + {0xea000a8c, "SECRT8CP3"}, + {0xea000a90, "SECRT90P3"}, + {0xea000a98, "SECRT98P3"}, + {0xea000b1c, "SECRR1CP3"}, + {0xea000b50, "SECRR50P3"}, + {0xea000b54, "SECRR54P3"}, + {0xea000b58, "SECRR58P3"}, + + {0xea002088, "SECRT88P4"}, + {0xea00208c, "SECRT8CP4"}, + {0xea002090, "SECRT90P4"}, + {0xea002098, "SECRT98P4"}, + {0xea00211c, "SECRR1CP4"}, + {0xea002150, "SECRR50P4"}, + {0xea002154, "SECRR54P4"}, + {0xea002158, "SECRR58P4"}, + + {0xea002288, "SECRT88P5"}, + {0xea00228c, "SECRT8CP5"}, + {0xea002290, "SECRT90P5"}, + {0xea002298, "SECRT98P5"}, + {0xea00231c, "SECRR1CP5"}, + {0xea002350, "SECRR50P5"}, + {0xea002354, "SECRR54P5"}, + {0xea002358, "SECRR58P5"}, + + {0xea008100, "SECRF00"}, + {0xea008104, "SECRF04"}, + + /* USB 2.0 Electrical Control Register */ + {0xe5004100, "U2ECRP01"}, + {0xe5004200, "U2ECRP02"}, + {0xe5004300, "U2ECRP03"}, + {0xe5004400, "U2ECRP04"}, + {0xe5004500, "U2ECRP05"}, + {0xe5004600, "U2ECRP06"}, + {0xe5004700, "U2ECRP07"}, + {0xe5004800, "U2ECRP08"}, + {0xe5004900, "U2ECRP09"}, + {0xe5004a00, "U2ECRP10"}, + {0xe5004b00, "U2ECRP11"}, + {0xe5004c00, "U2ECRP12"}, + {0xe5004d00, "U2ECRP13"}, + {0xe5004e00, "U2ECRP14"}, + + /* IOBP related to USB 3.0 ports */ + /* port 1 */ + {0xe900175c, ""}, + {0xe9001760, ""}, + {0xe9001768, ""}, + {0xe9001770, ""}, + {0xe90017cc, ""}, + /* port 2 */ + {0xe900155c, ""}, + {0xe9001560, ""}, + {0xe9001568, ""}, + {0xe9001570, ""}, + {0xe90015cc, ""}, + /* port 3 */ + {0xe9002f5c, ""}, + {0xe9002f60, ""}, + {0xe9002f68, ""}, + {0xe9002f70, ""}, + {0xe9002fcc, ""}, + /* port 4 */ + {0xe9002d5c, ""}, + {0xe9002d60, ""}, + {0xe9002d68, ""}, + {0xe9002d70, ""}, + {0xe9002dcc, ""}, + /* port 5 */ + {0xe900335c, ""}, + {0xe9003360, ""}, + {0xe9003368, ""}, + {0xe9003370, ""}, + {0xe90033cc, ""}, + /* port 6 */ + {0xe900315c, ""}, + {0xe9003160, ""}, + {0xe9003168, ""}, + {0xe9003170, ""}, + {0xe90031cc, ""}, +}; + +static const struct iobp_register lynxpoint_lp_iobp_registers[] = { + /* SATA Electrical Control Register */ + {0xea002688, "SECRT88P0"}, + {0xea00268c, "SECRT8CP0"}, + {0xea002690, "SECRT90P0"}, + {0xea002698, "SECRT98P0"}, + {0xea00271c, "SECRR1CP0"}, + {0xea002750, "SECRR50P0"}, + {0xea002754, "SECRR54P0"}, + {0xea002758, "SECRR58P0"}, + + {0xea002488, "SECRT88P1"}, + {0xea00248c, "SECRT8CP1"}, + {0xea002490, "SECRT90P1"}, + {0xea002498, "SECRT98P1"}, + {0xea00251c, "SECRR1CP1"}, + {0xea002550, "SECRR50P1"}, + {0xea002554, "SECRR54P1"}, + {0xea002558, "SECRR58P1"}, + + {0xea002288, "SECRT88P2"}, + {0xea00228c, "SECRT8CP2"}, + {0xea002290, "SECRT90P2"}, + {0xea002298, "SECRT98P2"}, + {0xea00231c, "SECRR1CP2"}, + {0xea002350, "SECRR50P2"}, + {0xea002354, "SECRR54P2"}, + {0xea002358, "SECRR58P2"}, + + {0xea002088, "SECRT88P3"}, + {0xea00208c, "SECRT8CP3"}, + {0xea002090, "SECRT90P3"}, + {0xea002098, "SECRT98P3"}, + {0xea00211c, "SECRR1CP3"}, + {0xea002150, "SECRR50P3"}, + {0xea002154, "SECRR54P3"}, + {0xea002158, "SECRR58P3"}, + + {0xea008100, "SECRF00"}, + {0xea008104, "SECRF04"}, + + /* USB 2.0 Electrical Control Register, 8 for -U SoC and 10 for Core-M */ + {0xe5004100, "U2ECRP01"}, + {0xe5004200, "U2ECRP02"}, + {0xe5004300, "U2ECRP03"}, + {0xe5004400, "U2ECRP04"}, + {0xe5004500, "U2ECRP05"}, + {0xe5004600, "U2ECRP06"}, + {0xe5004700, "U2ECRP07"}, + {0xe5004800, "U2ECRP08"}, + {0xe5004900, "U2ECRP09"}, + {0xe5004a00, "U2ECRP10"}, + + /* IOBP related to USB 3.0 ports */ + /* port 1 */ + {0xe900215c, ""}, + {0xe9002160, ""}, + {0xe9002168, ""}, + {0xe9002170, ""}, + {0xe90021cc, ""}, + /* port 2 */ + {0xe900235c, ""}, + {0xe9002360, ""}, + {0xe9002368, ""}, + {0xe9002370, ""}, + {0xe90023cc, ""}, + /* port 3 */ + {0xe900255c, ""}, + {0xe9002560, ""}, + {0xe9002568, ""}, + {0xe9002570, ""}, + {0xe90025cc, ""}, + /* port 4 */ + {0xe900275c, ""}, + {0xe9002760, ""}, + {0xe9002768, ""}, + {0xe9002770, ""}, + {0xe90027cc, ""}, +}; + +void print_iobp(struct pci_dev *sb, volatile uint8_t *rcba) +{ + const struct iobp_register *iobp_registers = NULL; + size_t iobp_size = 0; + + switch (sb->device_id) { + case PCI_DEVICE_ID_INTEL_C8_MOBILE: + case PCI_DEVICE_ID_INTEL_C8_DESKTOP: + case PCI_DEVICE_ID_INTEL_Z87: + case PCI_DEVICE_ID_INTEL_Z85: + case PCI_DEVICE_ID_INTEL_HM86: + case PCI_DEVICE_ID_INTEL_H87: + case PCI_DEVICE_ID_INTEL_HM87: + case PCI_DEVICE_ID_INTEL_Q85: + case PCI_DEVICE_ID_INTEL_Q87: + case PCI_DEVICE_ID_INTEL_QM87: + case PCI_DEVICE_ID_INTEL_B85: + case PCI_DEVICE_ID_INTEL_C222: + case PCI_DEVICE_ID_INTEL_C224: + case PCI_DEVICE_ID_INTEL_C226: + case PCI_DEVICE_ID_INTEL_H81: + iobp_registers = lynxpoint_iobp_registers; + iobp_size = ARRAY_SIZE(lynxpoint_iobp_registers); + break; + case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_FULL: + case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_PREM: + case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_BASE: + case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_PREM: + case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP: + iobp_registers = lynxpoint_lp_iobp_registers; + iobp_size = ARRAY_SIZE(lynxpoint_lp_iobp_registers); + break; + default: + break; + } + + if (iobp_size == 0) + return; + + printf("\n============= IOBP ==============\n\n"); + + for (size_t i = 0; i < iobp_size; i++) { + u32 address = iobp_registers[i].addr; + const char *name = iobp_registers[i].name; + u32 v = pch_iobp_read(rcba, address); + printf("0x%08x: 0x%08x (%s)\n", address, v, name); + } +} diff --git a/util/inteltool/rootcmplx.c b/util/inteltool/rootcmplx.c index b1e4939..da9144a 100644 --- a/util/inteltool/rootcmplx.c +++ b/util/inteltool/rootcmplx.c @@ -139,6 +139,8 @@ printf("0x%04x: 0x%08x\n", i, read32(rcba + i)); }
+ print_iobp(sb, rcba); + unmap_physical((void *)rcba, size); return 0; }
-
Angel Pons (Code Review) -
Felix Held (Code Review)
-
Iru Cai (vimacs) (Code Review)
-
Pablo Stebler (Code Review)