Maulik V Vaghela has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30545
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
mb/google/hatch: Enable SD card support for hatch
Enable support for SD card support for hatch 1. Enable PCI device for SD and also configure SD detect GPIO 2. Configure SD card related GPIOs in gpio.c
BUG=b:120914069 BRANCH=none TEST=check if code compiles correctly and verify GPIO configuration with schematics
Change-Id: I8ccaa28323b1e1fcc192e245347a96309227660b Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/mainboard/google/hatch/variants/baseboard/gpio.c 2 files changed, 24 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/30545/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb index dc17559..7978b44 100644 --- a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb @@ -80,6 +80,9 @@ register "PcieClkSrcUsage[3]" = "13" register "PcieClkSrcClkReq[3]" = "3"
+ # GPIO for SD card detect + register "sdcard_cd_gpio" = "GPP_G5" + device domain 0 on device pci 00.0 on end # Host Bridge device pci 02.0 on end # Integrated Graphics Device @@ -168,7 +171,7 @@ register "wake" = "GPE0_PME_B0" device pci 14.3 on end # CNVi wifi end - device pci 14.5 off end # SDCard + device pci 14.5 on end # SDCard device pci 15.0 on chip drivers/i2c/generic register "hid" = ""ELAN0000"" diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index 3d63997..5eeddb0 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -19,6 +19,10 @@ #include <commonlib/helpers.h>
static const struct pad_config gpio_table[] = { + /* SD_1P8_SEL => NC */ + PAD_NC(GPP_A16, DN_20K), + /* EN_PP3300_SD_DX */ + PAD_CFG_NF(GPP_A17, NONE, DEEP, NF1), /* TRACKPAD_INT_ODL */ PAD_CFG_GPI_APIC(GPP_A21, NONE, PLTRST, LEVEL, INVERT), /* SRCCLKREQ1 */ @@ -103,6 +107,22 @@ PAD_CFG_GPI(GPP_F11, NONE, PLTRST), /* PCH_MEM_STRAP3 */ PAD_CFG_GPI(GPP_F22, NONE, PLTRST), + /* SD_CMD */ + PAD_CFG_NF(GPP_G0, NONE, DEEP, NF1), + /* SD_DATA0 */ + PAD_CFG_NF(GPP_G1, NONE, DEEP, NF1), + /* SD_DATA1 */ + PAD_CFG_NF(GPP_G2, NONE, DEEP, NF1), + /* SD_DATA2 */ + PAD_CFG_NF(GPP_G3, NONE, DEEP, NF1), + /* SD_DATA3 */ + PAD_CFG_NF(GPP_G4, NONE, DEEP, NF1), + /* SD_CD# */ + PAD_CFG_NF(GPP_G5, NONE, DEEP, NF1), + /* SD_CLK */ + PAD_CFG_NF(GPP_G6, NONE, DEEP, NF1), + /* SD_WP => NC */ + PAD_NC(GPP_G7, DN_20K), };
const struct pad_config *__weak variant_gpio_table(size_t *num)
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 18: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... PS3, Line 106: };
I think we clarified elsewhere that we do not need to configure all the GPIOs and FSP will handle it […]
My understanding is that it has always been the norm to configure all GPIO pads in coreboot.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... PS18, Line 103: UP_20K Why is external pull-up required?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... PS18, Line 103: UP_20K
Why is external pull-up required?
s/external/internal
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... PS18, Line 103: UP_20K
s/external/internal
This was added since there was issue in kernel driver in Dragonegg. This patch was tested in PO and SD card was working. I can file a bug and check later if SD works after removing pull up. (This may take some time due to board unavailability)
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... PS3, Line 106: };
My understanding is that it has always been the norm to configure all GPIO pads in coreboot.
Yes that was the case earlier, if you take a look at Dragonegg we configure only required GPIOs and let FSP handle others. We can push a new patch to configure all the GPIOs.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 18:
(2 comments)
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... PS3, Line 106: };
Yes that was the case earlier, if you take a look at Dragonegg we configure only required GPIOs and let FSP handle others.
Sorry, I had missed out on the reviews for dragonegg, but we should be configuring all GPIOs in coreboot for dragonegg as well.
We can push a new patch to configure all the GPIOs.
Why do we need a new patch? Let's just push it as part of this CL.
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... PS18, Line 103: UP_20K
This was added since there was issue in kernel driver in Dragonegg. […]
Let's test the change properly and then push it in.
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 18:
(1 comment)
Patch Set 18:
(2 comments)
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... PS3, Line 106: };
Yes that was the case earlier, if you take a look at Dragonegg we configure only required GPIOs and let FSP handle others.
Sorry, I had missed out on the reviews for dragonegg, but we should be configuring all GPIOs in coreboot for dragonegg as well.
We can push a new patch to configure all the GPIOs.
Why do we need a new patch? Let's just push it as part of this CL.
Yes,for SD card related GPIOs we can do it in this patch. A separate patch, I meant it for any others that we have left out for hatch.
Hello Naresh Solanki, Aaron Durbin, Subrata Banik, Aamir Bohra, Ronak Kanabar, 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/+/30545
to look at the new patch set (#19).
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
mb/google/hatch: Enable SD card support for hatch
Enable support for SD card support for hatch 1. Enable PCI device for SD and also configure SD detect GPIO 2. Configure SD card related GPIOs in gpio.c
BUG=b:120914069 BRANCH=none TEST=Verify GPIO configuration with schematics
Change-Id: I8ccaa28323b1e1fcc192e245347a96309227660b Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/mainboard/google/hatch/variants/baseboard/gpio.c 2 files changed, 24 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/30545/19
Hello Naresh Solanki, Aaron Durbin, Subrata Banik, Aamir Bohra, Ronak Kanabar, 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/+/30545
to look at the new patch set (#20).
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
mb/google/hatch: Enable SD card support for hatch
Enable support for SD card support for hatch 1. Enable PCI device for SD and also configure SD detect GPIO 2. Configure SD card related GPIOs in gpio.c
BUG=b:120914069 BRANCH=none TEST=Verify GPIO configuration with schematics
Change-Id: I8ccaa28323b1e1fcc192e245347a96309227660b Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/mainboard/google/hatch/variants/baseboard/gpio.c 2 files changed, 24 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/30545/20
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 20:
(2 comments)
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/3/src/mainboard/google/hatch/variants/... PS3, Line 106: };
Yes that was the case earlier, if you take a look at Dragonegg we configure only required GPIOs […]
I have included all GPIOs related to SD card in this patch itself.
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/30545/18/src/mainboard/google/hatch/variants... PS18, Line 103: UP_20K
Let's test the change properly and then push it in.
I have tested changes locally on RVP board and SD card detection works without pull up. So I have removed pull up
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 20: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 20: Code-Review+1
Hello Naresh Solanki, Aaron Durbin, Subrata Banik, Aamir Bohra, Ronak Kanabar, Paul Menzel, 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/+/30545
to look at the new patch set (#21).
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
mb/google/hatch: Enable SD card support for hatch
Enable support for SD card support for hatch 1. Enable PCI device for SD and also configure SD detect GPIO 2. Configure SD card related GPIOs in gpio.c
BUG=b:120914069 BRANCH=none TEST=Verify GPIO configuration with schematics
Change-Id: I8ccaa28323b1e1fcc192e245347a96309227660b Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/mainboard/google/hatch/variants/baseboard/gpio.c 2 files changed, 24 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/30545/21
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 21: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 21: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
Patch Set 21: Code-Review+2
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30545 )
Change subject: mb/google/hatch: Enable SD card support for hatch ......................................................................
mb/google/hatch: Enable SD card support for hatch
Enable support for SD card support for hatch 1. Enable PCI device for SD and also configure SD detect GPIO 2. Configure SD card related GPIOs in gpio.c
BUG=b:120914069 BRANCH=none TEST=Verify GPIO configuration with schematics
Change-Id: I8ccaa28323b1e1fcc192e245347a96309227660b Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com Reviewed-on: https://review.coreboot.org/c/30545 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aamir Bohra aamir.bohra@intel.com Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/mainboard/google/hatch/variants/baseboard/gpio.c 2 files changed, 24 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Subrata Banik: Looks good to me, approved Aamir Bohra: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb index 5b777f8..97579d4 100644 --- a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb @@ -80,6 +80,9 @@ # ClkReq-to-ClkSrc mapping for CLK SRC 1 register "PcieClkSrcClkReq[1]" = "1"
+ # GPIO for SD card detect + register "sdcard_cd_gpio" = "GPP_G5" + device cpu_cluster 0 on device lapic 0 on end end @@ -173,7 +176,7 @@ register "wake" = "GPE0_PME_B0" device pci 14.3 on end # CNVi wifi end - device pci 14.5 off end # SDCard + device pci 14.5 on end # SDCard device pci 15.0 on chip drivers/i2c/generic register "hid" = ""ELAN0000"" diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index e855fed..b40ebd4 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -19,6 +19,10 @@ #include <commonlib/helpers.h>
static const struct pad_config gpio_table[] = { + /* SD_1P8_SEL => NC */ + PAD_NC(GPP_A16, DN_20K), + /* EN_PP3300_SD_DX */ + PAD_CFG_NF(GPP_A17, NONE, DEEP, NF1), /* TRACKPAD_INT_ODL */ PAD_CFG_GPI_APIC(GPP_A21, NONE, PLTRST, LEVEL, INVERT), /* SRCCLKREQ1 */ @@ -95,6 +99,22 @@ PAD_CFG_GPI(GPP_F11, NONE, PLTRST), /* PCH_MEM_STRAP3 */ PAD_CFG_GPI(GPP_F22, NONE, PLTRST), + /* SD_CMD */ + PAD_CFG_NF(GPP_G0, NONE, DEEP, NF1), + /* SD_DATA0 */ + PAD_CFG_NF(GPP_G1, NONE, DEEP, NF1), + /* SD_DATA1 */ + PAD_CFG_NF(GPP_G2, NONE, DEEP, NF1), + /* SD_DATA2 */ + PAD_CFG_NF(GPP_G3, NONE, DEEP, NF1), + /* SD_DATA3 */ + PAD_CFG_NF(GPP_G4, NONE, DEEP, NF1), + /* SD_CD# */ + PAD_CFG_NF(GPP_G5, NONE, DEEP, NF1), + /* SD_CLK */ + PAD_CFG_NF(GPP_G6, NONE, DEEP, NF1), + /* SD_WP => NC */ + PAD_NC(GPP_G7, DN_20K), };
const struct pad_config *__weak variant_gpio_table(size_t *num)
-
Aamir Bohra (Code Review) -
Furquan Shaikh (Code Review)
-
Maulik V Vaghela (Code Review)
-
Paul Menzel (Code Review)
-
Rizwan Qureshi (Code Review)
-
Subrata Banik (Code Review)