Hello Iru Cai,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/41485
to review the following change.
Change subject: sb/intel/lynxpoint: call pch_gpi_routing() in -H instead of -LP PCH ......................................................................
sb/intel/lynxpoint: call pch_gpi_routing() in -H instead of -LP PCH
GPI_ROUT is in LynxPoint-H PCH [1], and not in LynxPoint-LP [2]. So call pch_gpi_routing() when PCH is not -LP. Also add GPI_ROUT2 register according to the datasheet.
Need testing on real hardware.
[1] Intel 8 Series/C220 Series Chipset Family Platform Controller Hub (PCH) Datasheet, Document Number 328904-003 [2] Mobile 4th Generation Intel Core Processor Family I/O Datasheet, Document Number: 329003-003
Change-Id: Id9999ea9cb44565832d44a57423e89ede1c6b238 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M src/southbridge/intel/lynxpoint/chip.h M src/southbridge/intel/lynxpoint/lpc.c M src/southbridge/intel/lynxpoint/pch.h 3 files changed, 22 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41485/1
diff --git a/src/southbridge/intel/lynxpoint/chip.h b/src/southbridge/intel/lynxpoint/chip.h index ed362a2..ce38faa 100644 --- a/src/southbridge/intel/lynxpoint/chip.h +++ b/src/southbridge/intel/lynxpoint/chip.h @@ -44,6 +44,14 @@ uint8_t gpi13_routing; uint8_t gpi14_routing; uint8_t gpi15_routing; + uint8_t gpi17_routing; + uint8_t gpi19_routing; + uint8_t gpi21_routing; + uint8_t gpi22_routing; + uint8_t gpi43_routing; + uint8_t gpi56_routing; + uint8_t gpi57_routing; + uint8_t gpi60_routing;
uint32_t gpe0_en_1; uint32_t gpe0_en_2; diff --git a/src/southbridge/intel/lynxpoint/lpc.c b/src/southbridge/intel/lynxpoint/lpc.c index 3535312..e15452e 100644 --- a/src/southbridge/intel/lynxpoint/lpc.c +++ b/src/southbridge/intel/lynxpoint/lpc.c @@ -161,6 +161,18 @@ reg32 |= (config->gpi15_routing & 0x03) << 30;
pci_write_config32(dev, GPIO_ROUT, reg32); + + reg32 = 0; + reg32 |= (config->gpi17_routing & 0x03) << 0; + reg32 |= (config->gpi19_routing & 0x03) << 2; + reg32 |= (config->gpi21_routing & 0x03) << 4; + reg32 |= (config->gpi22_routing & 0x03) << 6; + reg32 |= (config->gpi43_routing & 0x03) << 8; + reg32 |= (config->gpi56_routing & 0x03) << 10; + reg32 |= (config->gpi57_routing & 0x03) << 12; + reg32 |= (config->gpi60_routing & 0x03) << 14; + + pci_write_config32(dev, GPIO_ROUT2, reg32); }
static void pch_power_options(struct device *dev) @@ -243,7 +255,7 @@ * Set the board's GPI routing on LynxPoint-H. * This is done as part of GPIO configuration on LynxPoint-LP. */ - if (pch_is_lp()) + if (!pch_is_lp()) pch_gpi_routing(dev);
/* GPE setup based on device tree configuration */ diff --git a/src/southbridge/intel/lynxpoint/pch.h b/src/southbridge/intel/lynxpoint/pch.h index 19f8637..8e1f9f1 100644 --- a/src/southbridge/intel/lynxpoint/pch.h +++ b/src/southbridge/intel/lynxpoint/pch.h @@ -214,6 +214,7 @@ #define GPIO_BASE 0x48 /* LPC GPIO Base Address Register */ #define GPIO_CNTL 0x4C /* LPC GPIO Control Register */ #define GPIO_ROUT 0xb8 +#define GPIO_ROUT2 0xbc
#define PIRQA_ROUT 0x60 #define PIRQB_ROUT 0x61
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41485 )
Change subject: sb/intel/lynxpoint: call pch_gpi_routing() in -H instead of -LP PCH ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41485/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41485/1//COMMIT_MSG@9 PS1, Line 9: GPI_ROUT GPIO_ROUT
Hello build bot (Jenkins), Patrick Rudolph, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41485
to look at the new patch set (#2).
Change subject: sb/intel/lynxpoint: call pch_gpi_routing() in -H instead of -LP PCH ......................................................................
sb/intel/lynxpoint: call pch_gpi_routing() in -H instead of -LP PCH
GPI_ROUT is in LynxPoint-H PCH [1], and not in LynxPoint-LP [2]. So call pch_gpi_routing() when PCH is not -LP. Also add GPI_ROUT2 register according to the datasheet.
Tested on HP ProBook 640 G1. The EC ACPI functions of this laptop become working after this patch.
[1] Intel 8 Series/C220 Series Chipset Family Platform Controller Hub (PCH) Datasheet, Document Number 328904-003 [2] Mobile 4th Generation Intel Core Processor Family I/O Datasheet, Document Number: 329003-003
Change-Id: Id9999ea9cb44565832d44a57423e89ede1c6b238 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M src/southbridge/intel/lynxpoint/chip.h M src/southbridge/intel/lynxpoint/lpc.c M src/southbridge/intel/lynxpoint/pch.h 3 files changed, 22 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41485/2
Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41485 )
Change subject: sb/intel/lynxpoint: call pch_gpi_routing() in -H instead of -LP PCH ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41485/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41485/1//COMMIT_MSG@9 PS1, Line 9: GPI_ROUT
GPIO_ROUT
This is the name used in the datasheet, while the code uses GPIO_ROUT.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41485 )
Change subject: sb/intel/lynxpoint: call pch_gpi_routing() in -H instead of -LP PCH ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41485/2/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/lpc.c:
https://review.coreboot.org/c/coreboot/+/41485/2/src/southbridge/intel/lynxp... PS2, Line 147: An array would be much nicer here, or some : * other method of doing this. If we're going to touch this, let's do what this comment says.
https://review.coreboot.org/c/coreboot/+/41485/2/src/southbridge/intel/lynxp... PS2, Line 262: if (!pch_is_lp()) I already did this on CB:47044
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/41485?usp=email )
Change subject: sb/intel/lynxpoint: call pch_gpi_routing() in -H instead of -LP PCH ......................................................................
Abandoned