Change in ...coreboot[master]: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups
Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32175
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups
This implementation corrects the GPE DWx mapping for GPIO groups. The assignments is done in GPIO MISCFG register for all GPIO communities. And configures the which GPIO communities get register as Tier1.
BUG=b:121212459 TEST: Verified the GPIO MISCFG is getting set as per updated map.
Change-Id: I451997367025a6dc9e5931bd649524e935ad6aca Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h 1 file changed, 8 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/32175/1
diff --git a/src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h b/src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h index 03f4314..cca2bc4 100644 --- a/src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h +++ b/src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h @@ -26,14 +26,14 @@ #define GPP_B 1 #define GPP_G 2 #define GROUP_SPI 3 -#define GPP_D 4 -#define GPP_F 5 -#define GPP_H 6 -#define GROUP_VGPIO 7 -#define GPD 9 -#define GROUP_AZA 0xA -#define GROUP_CPU 0xB -#define GPP_C 0xC +#define GPP_D 5 +#define GPP_F 6 +#define GPP_H 7 +#define GROUP_VGPIO 8 +#define GPD 0xA +#define GROUP_AZA 0xB +#define GROUP_CPU 0xC +#define GPP_C 4 #define GPP_E 0xD #define GROUP_JTAG 0xE #define GROUP_HVMOS 0xF
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/32175/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32175/1//COMMIT_MSG@14 PS1, Line 14: Verified the GPIO MISCFG is getting set as per updated map. Does GPE_STS get set correctly in DWx register? Does wake work fine each of the DWx register configuration? Was this verified on CNL, WHL and CML?
https://review.coreboot.org/#/c/32175/1/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/#/c/32175/1/src/soc/intel/cannonlake/include/soc... PS1, Line 25: 0 Since you are changing these, can you please add 0x in front of all the values to ensure they are consistent.
https://review.coreboot.org/#/c/32175/1/src/soc/intel/cannonlake/include/soc... PS1, Line 39: #define GROUP_HVMOS 0xF No 0x9?
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 1:
That's not match with EDS #565870 right?
Hello Patrick Rudolph, Subrata Banik, Rizwan Qureshi, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32175
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups
This implementation corrects the GPE DWx mapping for GPIO groups. The assignments is done in GPIO MISCFG register for all GPIO communities. And configures the which GPIO communities get register as Tier1.
BUG=b:121212459 TEST: Verified the GPIO MISCFG is getting set as per updated map.
Change-Id: I451997367025a6dc9e5931bd649524e935ad6aca Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h 1 file changed, 13 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/32175/2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/#/c/32175/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32175/1//COMMIT_MSG@14 PS1, Line 14: Verified the GPIO MISCFG is getting set as per updated map.
Does GPE_STS get set correctly in DWx register? Does wake work fine each of the DWx register configu […]
I checked for GPE_STS for touchpad on hatch , verified STS set for A21(DW0) and D21(DW2) wake instances. TPM does not timeout waiting on IRQ in verstage and depthcharge(mapped to DW1). This is all on hatch.
https://review.coreboot.org/#/c/32175/1/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/#/c/32175/1/src/soc/intel/cannonlake/include/soc... PS1, Line 25: 0
Since you are changing these, can you please add 0x in front of all the values to ensure they are co […]
Done. Updated in PS#2
https://review.coreboot.org/#/c/32175/1/src/soc/intel/cannonlake/include/soc... PS1, Line 39: #define GROUP_HVMOS 0xF
No 0x9?
It is mapped to VGPIO , since VGPIO has 40 pins, 32 is mapped to 0x8 and rest are mapped to 0x9
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 2:
Patch Set 1:
That's not match with EDS #565870 right?
No. EDS needs an update.
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 2: Code-Review-1
Please make sure the register value match with EDS or UEFI BIOS settings.
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 2: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/32175/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32175/1//COMMIT_MSG@14 PS1, Line 14: Verified the GPIO MISCFG is getting set as per updated map.
I checked for GPE_STS for touchpad on hatch , verified STS set for A21(DW0) and D21(DW2) wake instan […]
TPM was working fine earlier as well so we were just getting lucky that DW1_21 was set all the time and so it never caused timeouts?
Are you able to clear DW1_21 in GPE_STS and does that take effect? I think you had reported in another issue that DW1_21 stays set all the time?
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/32175/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32175/1//COMMIT_MSG@14 PS1, Line 14: Verified the GPIO MISCFG is getting set as per updated map.
TPM was working fine earlier as well so we were just getting lucky that DW1_21 was set all the time […]
TPM was working fine earlier as well so we were just getting lucky that DW1_21 was set all the time and so it never caused timeouts?
Yes. For the current code(gpio configuration(level, non-invert)), I see the status is set all the time. That is expected too, since the PMC GPIOCFG values are correct and status would just mirror the gpio rx state.
Are you able to clear DW1_21 in GPE_STS and does that take effect? I think you had reported in another issue that DW1_21 stays set all the time?
On setting the RxInv(for both level and edge configuration) Bit the status does get clear and it exists from get_gpe_status, without timer expiry. Otherwise if RxInv is not set the code always exits on timer expiry since sts & mask never gets 0. Will further update.
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32175 )
Change subject: soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups ......................................................................
soc/intel/cannonlake: Correct the GPE DWx mapping for GPIO groups
This implementation corrects the GPE DWx mapping for GPIO groups. The assignments is done in GPIO MISCFG register for all GPIO communities. And configures the which GPIO communities get register as Tier1.
BUG=b:121212459 TEST: Verified the GPIO MISCFG is getting set as per updated map.
Change-Id: I451997367025a6dc9e5931bd649524e935ad6aca Signed-off-by: Aamir Bohra aamir.bohra@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32175 Reviewed-by: Rizwan Qureshi rizwan.qureshi@intel.com Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Lijian Zhao lijian.zhao@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h 1 file changed, 13 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Rizwan Qureshi: Looks good to me, approved Lijian Zhao: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h b/src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h index 03f4314..5990144 100644 --- a/src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h +++ b/src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h @@ -22,18 +22,19 @@ * The GPIO groups are accessed through register blocks called * communities. */ -#define GPP_A 0 -#define GPP_B 1 -#define GPP_G 2 -#define GROUP_SPI 3 -#define GPP_D 4 -#define GPP_F 5 -#define GPP_H 6 -#define GROUP_VGPIO 7 -#define GPD 9 -#define GROUP_AZA 0xA -#define GROUP_CPU 0xB -#define GPP_C 0xC +#define GPP_A 0x0 +#define GPP_B 0x1 +#define GPP_G 0x2 +#define GROUP_SPI 0x3 +#define GPP_D 0x5 +#define GPP_F 0x6 +#define GPP_H 0x7 +#define GROUP_VGPIO0 0x8 +#define GROUP_VGPIO1 0x9 +#define GPD 0xA +#define GROUP_AZA 0xB +#define GROUP_CPU 0xC +#define GPP_C 0x4 #define GPP_E 0xD #define GROUP_JTAG 0xE #define GROUP_HVMOS 0xF
-
Aamir Bohra (Code Review) -
Furquan Shaikh (Code Review)
-
Lijian Zhao (Code Review)
-
Patrick Georgi (Code Review)
-
Rizwan Qureshi (Code Review)