Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/tigerlake: Update SD card ACPI device ......................................................................
src/soc/intel/tigerlake: Update SD card ACPI device
1. Add _DSM method 2. Add support to turn on/off the power enable signal in _PS0/_PS3 methods.
Signed-off-by: Aamir Bohra aamir.bohra@intel.com Change-Id: I4f944caa535bdc946eef1e0f518fe3ee344187b9 --- M src/soc/intel/tigerlake/acpi/scs.asl M src/soc/intel/tigerlake/include/soc/gpio_soc_defs_jsl.h 2 files changed, 72 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/39581/1
diff --git a/src/soc/intel/tigerlake/acpi/scs.asl b/src/soc/intel/tigerlake/acpi/scs.asl index a9ff93c..6b391a8 100644 --- a/src/soc/intel/tigerlake/acpi/scs.asl +++ b/src/soc/intel/tigerlake/acpi/scs.asl @@ -1,7 +1,7 @@ /* * This file is part of the coreboot project. * - * Copyright (C) 2019 Intel Corporation. + * Copyright (C) 2020 Intel Corporation. * * 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 @@ -84,6 +84,7 @@ Name (_ADR, 0x00140005) Name (_DDN, "SD Controller") Name (TEMP, 0) + Name (DSUU, ToUUID("f6c13ea5-65cd-461f-ab7a-29f7e8d5bd61"))
OperationRegion (SDPC, PCI_Config, 0x00, 0x100) Field (SDPC, WordAcc, NoLock, Preserve) @@ -95,6 +96,63 @@ PGEN, 1, /* PG_ENABLE */ }
+ /* _DSM x86 Device Specific Method + * Arg0: UUID Unique function identifier + * Arg1: Integer Revision Level + * Arg2: Integer Function Index (0 = Return Supported Functions) + * Arg3: Package Parameters + */ + Method (_DSM, 4) + { + If (LEqual (Arg0, DSUU)) { + /* Check the revision */ + If (LGreaterEqual (Arg1, Zero)) { + /* + * Function Index 0 the return value is a buffer containing + * one bit for each function index, starting with zero. + * Bit 0 - Indicates whether there is support for any functions other than function 0. + * Bit 1 - Indicates support to clear power control register + * Bit 2 - Indicates support to set power control register + * Bit 3 - Indicates support to set 1.8V signalling + * Bit 4 - Indicates support to set 3.3V signalling + * Bit 5 - Indicates support for HS200 mode + * Bit 6 - Indicates support for HS400 mode + * Bit 9 - Indicates eMMC I/O Driver Strength + */ + /* + * For SD we have to support functions to + * set 1.8V signalling and 3.3V signalling [BIT4, BIT3] + */ + If (LEqual (Arg2, Zero)) { + Return (Buffer () { 0x19 }) + } + /* + * Function Index 3: Set 1.8v signalling. + * We put a sleep of 100ms in this method to + * work around a known issue with detecting + * UHS SD card on PCH. This is to compensate + * for the SD VR slowness. + */ + If (LEqual (Arg2, 3)) { + Sleep (100) + Return(Buffer () { 0x00 }) + } + /* + * Function Index 4: Set 3.3v signalling. + * We put a sleep of 100ms in this method to + * work around a known issue with detecting + * UHS SD card on PCH. This is to compensate + * for the SD VR slowness. + */ + If (LEqual (Arg2, 4)) { + Sleep (100) + Return(Buffer () { 0x00 }) + } + } + } + Return(Buffer() { 0x0 }) + } + Method(_INI) { /* Clear register 0x1C20/0x4820 */ @@ -111,6 +169,9 @@ /* Set Power State to D0 */ And (PMCR, 0xFFFC, PMCR) Store (PMCR, TEMP) + + /* Change pad mode to Native */ + GPMO(SD_PWR_EN_PIN, 0x1) }
Method (_PS3, 0, Serialized) @@ -120,6 +181,15 @@ /* Set Power State to D3 */ Or (PMCR, 0x0003, PMCR) Store (PMCR, TEMP) + + /* Change pad mode to GPIO control */ + GPMO(SD_PWR_EN_PIN, 0x0) + + /* Enable Tx Buffer */ + GTXE(SD_PWR_EN_PIN, 0x1) + + /* Drive TX to zero */ + CTXS(SD_PWR_EN_PIN) }
Device (CARD) diff --git a/src/soc/intel/tigerlake/include/soc/gpio_soc_defs_jsl.h b/src/soc/intel/tigerlake/include/soc/gpio_soc_defs_jsl.h index ce7d0d8..05e5696 100644 --- a/src/soc/intel/tigerlake/include/soc/gpio_soc_defs_jsl.h +++ b/src/soc/intel/tigerlake/include/soc/gpio_soc_defs_jsl.h @@ -37,7 +37,7 @@
#define GPIO_NUM_GROUPS 11 #define GPIO_MAX_NUM_PER_GROUP 24 - +#define SD_PWR_EN_PIN GPP_H1 /* * GPIOs are ordered monotonically increasing to match ACPI/OS driver. */
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 2:
(3 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/39581/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39581/1//COMMIT_MSG@11 PS1, Line 11: signal in _PS0/_PS3 methods.
Please re-flow for 75 characters.
Done
https://review.coreboot.org/c/coreboot/+/39581/1/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/scs.asl:
https://review.coreboot.org/c/coreboot/+/39581/1/src/soc/intel/tigerlake/acp... PS1, Line 99: /* _DSM x86 Device Specific Method
Done
https://review.coreboot.org/c/coreboot/+/39581/1/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/gpio_soc_defs_jsl.h:
https://review.coreboot.org/c/coreboot/+/39581/1/src/soc/intel/tigerlake/inc... PS1, Line 40: #define SD_PWR_EN_PIN GPP_H1
Align?
Done
Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 2: Code-Review+2
Hello build bot (Jenkins), Justin TerAvest, Evan Green, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39581
to look at the new patch set (#3).
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
src/soc/intel/jasperlake: Update SD card ACPI device
1. Add _DSM method 2. Add support to turn on/off the power enable signal in _PS0/_PS3 methods.
BUG=b:161242573 TEST=Verify SD card works in UHS mode.
Signed-off-by: Aamir Bohra aamir.bohra@intel.com Change-Id: I4f944caa535bdc946eef1e0f518fe3ee344187b9 --- M src/soc/intel/jasperlake/acpi/scs.asl M src/soc/intel/jasperlake/include/soc/gpio_soc_defs.h 2 files changed, 77 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/39581/3
Evan Green has uploaded a new patch set (#4) to the change originally created by Aamir Bohra. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
src/soc/intel/jasperlake: Update SD card ACPI device
1. Add _DSM method 2. Add support to turn on/off the power enable signal in _PS0/_PS3 methods.
Signed-off-by: Aamir Bohra aamir.bohra@intel.com Change-Id: I4f944caa535bdc946eef1e0f518fe3ee344187b9 --- M src/soc/intel/jasperlake/acpi/scs.asl M src/soc/intel/jasperlake/include/soc/gpio_soc_defs.h 2 files changed, 77 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/39581/4
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 4:
I rebased this to try and get away from the builder error. Can somebody +2 this again?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... File src/soc/intel/jasperlake/acpi/scs.asl:
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 95: If (LEqual (Arg0, DSUU)) { Please use ASL 2.0 syntax.
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... File src/soc/intel/jasperlake/acpi/scs.asl:
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 95: If (LEqual (Arg0, DSUU)) {
Please use ASL 2.0 syntax.
Just to confirm, that would be If (Arg0 == DSUU) {?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 4:
(9 comments)
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... File src/soc/intel/jasperlake/acpi/scs.asl:
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 95: If (LEqual (Arg0, DSUU)) {
Just to confirm, that would be If (Arg0 == DSUU) {?
Yes.
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 97: LGreaterEqual (Arg1, Zero) Arg1 >= 0
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 115: LEqual (Arg2, Zero) Arg2 == 0
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 125: LEqual (Arg2, 3) Arg2 == 3
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 136: LEqual (Arg2, 4) Arg2 == 4
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 159: And (PMCR, 0xFFFC, PMCR) PMCR = PMCR & 0xFFFC
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 160: Store (PMCR, TEMP) TEMP = PMCR
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 171: Or (PMCR, 0x0003, PMCR) PMCR = PMCR | 0x0003
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 172: Store (PMCR, TEMP) TEMP = PMCR
Evan Green has uploaded a new patch set (#5) to the change originally created by Aamir Bohra. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
src/soc/intel/jasperlake: Update SD card ACPI device
1. Add _DSM method 2. Add support to turn on/off the power enable signal in _PS0/_PS3 methods.
Signed-off-by: Aamir Bohra aamir.bohra@intel.com Change-Id: I4f944caa535bdc946eef1e0f518fe3ee344187b9 --- M src/soc/intel/jasperlake/acpi/scs.asl M src/soc/intel/jasperlake/include/soc/gpio_soc_defs.h 2 files changed, 73 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/39581/5
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 5:
(9 comments)
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... File src/soc/intel/jasperlake/acpi/scs.asl:
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 95: If (LEqual (Arg0, DSUU)) {
Yes.
Done
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 97: LGreaterEqual (Arg1, Zero)
Arg1 >= 0
Done
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 115: LEqual (Arg2, Zero)
Arg2 == 0
Done
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 125: LEqual (Arg2, 3)
Arg2 == 3
Done
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 136: LEqual (Arg2, 4)
Arg2 == 4
Done
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 159: And (PMCR, 0xFFFC, PMCR)
PMCR = PMCR & 0xFFFC
Done
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 160: Store (PMCR, TEMP)
TEMP = PMCR
Done
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 171: Or (PMCR, 0x0003, PMCR)
PMCR = PMCR | 0x0003
Done
https://review.coreboot.org/c/coreboot/+/39581/4/src/soc/intel/jasperlake/ac... PS4, Line 172: Store (PMCR, TEMP)
TEMP = PMCR
Done
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 5:
Anybody for a +2?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 5: Code-Review+2
Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
Patch Set 5: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39581 )
Change subject: src/soc/intel/jasperlake: Update SD card ACPI device ......................................................................
src/soc/intel/jasperlake: Update SD card ACPI device
1. Add _DSM method 2. Add support to turn on/off the power enable signal in _PS0/_PS3 methods.
Signed-off-by: Aamir Bohra aamir.bohra@intel.com Change-Id: I4f944caa535bdc946eef1e0f518fe3ee344187b9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/39581 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Justin TerAvest teravest@chromium.org --- M src/soc/intel/jasperlake/acpi/scs.asl M src/soc/intel/jasperlake/include/soc/gpio_soc_defs.h 2 files changed, 73 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Justin TerAvest: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/jasperlake/acpi/scs.asl b/src/soc/intel/jasperlake/acpi/scs.asl index 508fb9b..b58608f 100644 --- a/src/soc/intel/jasperlake/acpi/scs.asl +++ b/src/soc/intel/jasperlake/acpi/scs.asl @@ -71,6 +71,7 @@ Name (_ADR, 0x00140005) Name (_DDN, "SD Controller") Name (TEMP, 0) + Name (DSUU, ToUUID("f6c13ea5-65cd-461f-ab7a-29f7e8d5bd61"))
OperationRegion (SDPC, PCI_Config, 0x00, 0x100) Field (SDPC, WordAcc, NoLock, Preserve) @@ -82,6 +83,65 @@ PGEN, 1, /* PG_ENABLE */ }
+ /* + * _DSM x86 Device Specific Method + * Arg0: UUID Unique function identifier + * Arg1: Integer Revision Level + * Arg2: Integer Function Index (0 = Return Supported Functions) + * Arg3: Package Parameters + */ + Method (_DSM, 4) + { + If (Arg0 == DSUU) { + /* Check the revision */ + If (Arg1 >= 0) { + /* + * Function Index 0 the return value is a buffer containing + * one bit for each function index, starting with zero. + * Bit 0 - Indicates whether there is support for any + * functions other than function 0. + * Bit 1 - Indicates support to clear power control register + * Bit 2 - Indicates support to set power control register + * Bit 3 - Indicates support to set 1.8V signalling + * Bit 4 - Indicates support to set 3.3V signalling + * Bit 5 - Indicates support for HS200 mode + * Bit 6 - Indicates support for HS400 mode + * Bit 9 - Indicates eMMC I/O Driver Strength + */ + /* + * For SD we have to support functions to + * set 1.8V signalling and 3.3V signalling [BIT4, BIT3] + */ + If (Arg2 == 0) { + Return (Buffer () { 0x19 }) + } + /* + * Function Index 3: Set 1.8v signalling. + * We put a sleep of 100ms in this method to + * work around a known issue with detecting + * UHS SD card on PCH. This is to compensate + * for the SD VR slowness. + */ + If (Arg2 == 3) { + Sleep (100) + Return(Buffer () { 0x00 }) + } + /* + * Function Index 4: Set 3.3v signalling. + * We put a sleep of 100ms in this method to + * work around a known issue with detecting + * UHS SD card on PCH. This is to compensate + * for the SD VR slowness. + */ + If (Arg2 == 4) { + Sleep (100) + Return(Buffer () { 0x00 }) + } + } + } + Return(Buffer() { 0x0 }) + } + Method(_INI) { /* Clear register 0x1C20/0x4820 */ @@ -98,6 +158,9 @@ /* Set Power State to D0 */ PMCR = PMCR & 0xFFFC TEMP = PMCR + + /* Change pad mode to Native */ + GPMO(SD_PWR_EN_PIN, 0x1) }
Method (_PS3, 0, Serialized) @@ -107,6 +170,15 @@ /* Set Power State to D3 */ PMCR = PMCR | 0x0003 TEMP = PMCR + + /* Change pad mode to GPIO control */ + GPMO(SD_PWR_EN_PIN, 0x0) + + /* Enable Tx Buffer */ + GTXE(SD_PWR_EN_PIN, 0x1) + + /* Drive TX to zero */ + CTXS(SD_PWR_EN_PIN) }
Device (CARD) diff --git a/src/soc/intel/jasperlake/include/soc/gpio_soc_defs.h b/src/soc/intel/jasperlake/include/soc/gpio_soc_defs.h index 011b785..8d6064c 100644 --- a/src/soc/intel/jasperlake/include/soc/gpio_soc_defs.h +++ b/src/soc/intel/jasperlake/include/soc/gpio_soc_defs.h @@ -24,6 +24,7 @@
#define GPIO_NUM_GROUPS 12 #define GPIO_MAX_NUM_PER_GROUP 24 +#define SD_PWR_EN_PIN GPP_H1
/* * GPIOs are ordered monotonically increasing to match ACPI/OS driver.
-
Aamir Bohra (Code Review) -
Angel Pons (Code Review)
-
Evan Green (Code Review)
-
Justin TerAvest (Code Review)
-
Patrick Georgi (Code Review)