Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34335 )
Change subject: sb/intel/bd82x6x/usb_xhci: Lock OC config and refactoring ......................................................................
sb/intel/bd82x6x/usb_xhci: Lock OC config and refactoring
* Add register defines * Set the lock bit in final function * Lock XHCC2 * Remove debug output * Prevent possible NULL pointer dereference
The ACCTRL bit is described in "Intel Pentium Processor N3500-series, J2850, J2900, ..." Document Number: 329670-004.
Untested.
Change-Id: I53fef63de518ae089a6b7ef4483893bbb7c8c44e Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/southbridge/intel/bd82x6x/pch.h M src/southbridge/intel/bd82x6x/usb_xhci.c 2 files changed, 35 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/34335/1
diff --git a/src/southbridge/intel/bd82x6x/pch.h b/src/southbridge/intel/bd82x6x/pch.h index e36cbe2..2c3c947 100644 --- a/src/southbridge/intel/bd82x6x/pch.h +++ b/src/southbridge/intel/bd82x6x/pch.h @@ -465,6 +465,10 @@ #define RMHWKCTL 0x35b0 /* 32bit */
/* XHCI USB 3.0 */ +#define XHCC 0x40 +#define ACCTRL (1u << 31) +#define XHCC2 0x44 +#define OCCFDONE (1u << 31) #define XOCM 0xc0 /* 32bit */ #define XUSB2PRM 0xd4 /* 32bit */ #define USB3PRM 0xdc /* 32bit */ diff --git a/src/southbridge/intel/bd82x6x/usb_xhci.c b/src/southbridge/intel/bd82x6x/usb_xhci.c index 85e450d..561202d 100644 --- a/src/southbridge/intel/bd82x6x/usb_xhci.c +++ b/src/southbridge/intel/bd82x6x/usb_xhci.c @@ -25,30 +25,49 @@ static void usb_xhci_init(struct device *dev) { u32 reg32; + u16 reg16; + u8 reg8; struct southbridge_intel_bd82x6x_config *config = dev->chip_info;
- printk(BIOS_DEBUG, "XHCI: Setting up controller.. "); + if (!config) + return;
if (config->xhci_overcurrent_mapping) pci_write_config32(dev, XOCM, config->xhci_overcurrent_mapping);
- /* lock overcurrent map */ - reg32 = pci_read_config32(dev, 0x44); + /* Load overcurrent map. BIT31 is WO, so don't touch it here. */ + reg32 = pci_read_config8(dev, XHCC2); reg32 |= 1; - pci_write_config32(dev, 0x44, reg32); + pci_write_config8(dev, XHCC2, reg32);
pci_write_config32(dev, XUSB2PRM, config->xhci_switchable_ports); pci_write_config32(dev, USB3PRM, config->superspeed_capable_ports);
/* Enable clock gating */ - reg32 = pci_read_config32(dev, 0x40); - reg32 &= ~((1 << 20) | (1 << 21)); - reg32 |= (1 << 19) | (1 << 18) | (1 << 17); - reg32 |= (1 << 10) | (1 << 9) | (1 << 8); - reg32 |= (1 << 31); /* lock */ - pci_write_config32(dev, 0x40, reg32); + reg16 = pci_read_config16(dev, XHCC); + reg16 |= (1 << 10) | (1 << 9) | (1 << 8); + pci_write_config16(dev, XHCC, reg16);
- printk(BIOS_DEBUG, "done.\n"); + /* BIT31 is WO, so don't touch it here. */ + reg8 = pci_read_config8(dev, XHCC+2); + reg8 &= ~((1 << 4) | (1 << 5)); + reg8 |= (1 << 3) | (1 << 2) | (1 << 1); + pci_write_config8(dev, XHCC+2, reg8); +} + +static void xhci_final(struct device *dev) +{ + u32 reg32; + + /* Lock OC config. BIT31 is WO. */ + reg32 = pci_read_config32(dev, XHCC2); + reg32 |= OCCFDONE; + pci_write_config32(dev, XHCC2, reg32); + + /* Lock xHCI registers. BIT31 is WO. */ + reg32 = pci_read_config32(dev, XHCC); + reg32 |= ACCTRL; + pci_write_config32(dev, XHCC, reg32); }
static const char *xhci_acpi_name(const struct device *dev) @@ -68,6 +87,7 @@ .scan_bus = 0, .ops_pci = &xhci_pci_ops, .acpi_name = xhci_acpi_name, + .final = xhci_final, };
static const unsigned short pci_device_ids[] = { 0x1e31, 0 };
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34335 )
Change subject: sb/intel/bd82x6x/usb_xhci: Lock OC config and refactoring ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34335/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34335/3//COMMIT_MSG@7 PS3, Line 7: refactoring *do refactoring* or *refactor code*
https://review.coreboot.org/c/coreboot/+/34335/3//COMMIT_MSG@19 PS3, Line 19: Untested Can the successful locking be verified from the OS?
https://review.coreboot.org/c/coreboot/+/34335/3/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/usb_xhci.c:
https://review.coreboot.org/c/coreboot/+/34335/3/src/southbridge/intel/bd82x... PS3, Line 52: XHCC+2 Spaces around the operator?
XHCC + 2
https://review.coreboot.org/c/coreboot/+/34335/3/src/southbridge/intel/bd82x... PS3, Line 55: XHCC+2 Ditto.
Hello Alexander Couzens, Evgeny Zinoviev, Patrick Rudolph, Alexey Derlaft, Felix Held, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), Bill XIE, Marcello Sylvester Bauer,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34335
to look at the new patch set (#5).
Change subject: sb/intel/bd82x6x/usb_xhci: Lock OC config and do refactoring ......................................................................
sb/intel/bd82x6x/usb_xhci: Lock OC config and do refactoring
* Add register defines * Set the lock bit in final function * Lock XHCC2 * Remove debug output * Prevent possible NULL pointer dereference
The ACCTRL bit is described in "Intel Pentium Processor N3500-series, J2850, J2900, ..." Document Number: 329670-004.
Untested.
Change-Id: I53fef63de518ae089a6b7ef4483893bbb7c8c44e Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/southbridge/intel/bd82x6x/pch.h M src/southbridge/intel/bd82x6x/usb_xhci.c 2 files changed, 35 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/34335/5
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34335 )
Change subject: sb/intel/bd82x6x/usb_xhci: Lock OC config and do refactoring ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34335/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34335/3//COMMIT_MSG@7 PS3, Line 7: refactoring
*do refactoring* or *refactor code*
Done
https://review.coreboot.org/c/coreboot/+/34335/3//COMMIT_MSG@19 PS3, Line 19: Untested
Can the successful locking be verified from the OS?
yes, you could try to write the register and change it contents.
https://review.coreboot.org/c/coreboot/+/34335/3/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/usb_xhci.c:
https://review.coreboot.org/c/coreboot/+/34335/3/src/southbridge/intel/bd82x... PS3, Line 52: XHCC+2
Spaces around the operator? […]
Done
https://review.coreboot.org/c/coreboot/+/34335/3/src/southbridge/intel/bd82x... PS3, Line 55: XHCC+2
Ditto.
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34335 )
Change subject: sb/intel/bd82x6x/usb_xhci: Lock OC config and do refactoring ......................................................................
Patch Set 5: Code-Review+1
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34335 )
Change subject: sb/intel/bd82x6x/usb_xhci: Lock OC config and do refactoring ......................................................................
Patch Set 5: Code-Review+1
should be tested on hardware before being merged
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34335?usp=email )
Change subject: sb/intel/bd82x6x/usb_xhci: Lock OC config and do refactoring ......................................................................
Abandoned
-
Felix Held (Code Review) -
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Stefan Reinauer (Code Review)