Change in coreboot[master]: mb/google/dedede: Add SD card support

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Varshit B Pandya (Code Review)

3 Mar 2020 3 Mar '20

5:17 p.m.

Varshit B Pandya has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/gpio.c 2 files changed, 13 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/1

diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index d82dfc7..69899ac 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -106,6 +106,10 @@ # Enable EMMC HS400 mode register "ScsEmmcHs400Enabled" = "1"

+ # SD card configuration + register "sdcard_cd_gpio" = "VGPIO_39" + register "SdCardPowerEnableActiveHigh" = "1" + # Display related UPDs # Select eDP for port A register "DdiPortAConfig" = "1" @@ -167,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 end # I2C 0 device pci 15.1 on end # I2C 1 device pci 15.2 on end # I2C 2 diff --git a/src/mainboard/google/dedede/variants/baseboard/gpio.c b/src/mainboard/google/dedede/variants/baseboard/gpio.c index 2b93159..753f7ba 100644 --- a/src/mainboard/google/dedede/variants/baseboard/gpio.c +++ b/src/mainboard/google/dedede/variants/baseboard/gpio.c @@ -272,21 +272,19 @@ PAD_CFG_NF(GPP_F18, NONE, DEEP, NF1),

/* G0 : SD_CMD */ - PAD_NC(GPP_G0, NONE), + PAD_CFG_NF(GPP_G0, NATIVE, DEEP, NF1), /* G1 : SD_DATA0 */ - PAD_NC(GPP_G1, NONE), + PAD_CFG_NF(GPP_G1, NATIVE, DEEP, NF1), /* G2 : SD_DATA1 */ - PAD_NC(GPP_G2, NONE), + PAD_CFG_NF(GPP_G2, NATIVE, DEEP, NF1), /* G3 : SD_DATA2 */ - PAD_NC(GPP_G3, NONE), + PAD_CFG_NF(GPP_G3, NATIVE, DEEP, NF1), /* G4 : SD_DATA3 */ - PAD_NC(GPP_G4, NONE), - /* G5 : SD_CD_ODL */ - PAD_NC(GPP_G5, NONE), + PAD_CFG_NF(GPP_G4, NATIVE, DEEP, NF1), + /* G5 : SD_CD# */ + PAD_CFG_NF(GPP_G5, NONE, PLTRST, NF1), /* G6 : SD_CLK */ - PAD_NC(GPP_G6, NONE), - /* G7 : SD_SDIO_WP */ - PAD_NC(GPP_G7, NONE), + PAD_CFG_NF(GPP_G6, NONE, DEEP, NF1),

/* H0 : WWAN_PERST */ PAD_NC(GPP_H0, NONE),

Attachments:

attachment.htm (text/html — 5.6 KB)

Show replies by date

Furquan Shaikh (Code Review)

3 Mar 3 Mar

6:13 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 111: SdCardPowerEnableActiveHigh This is unused in soc/intel/tigerlake.

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 285: PLTRST Why is this set to PLTRST and others to DEEP? Is that to avoid interrupt storms when booting from S5? Is that a known issue for JSL too?

Karthik Ramasubramanian (Code Review)

6:17 p.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 284: SD_CD_ODL */ This is the pin name as in the schematics. Please keep it that way to avoid confusion in the future.

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 288: /* G7 : SD_SDIO_WP */ : PAD_NC(GPP_G7, NONE), Please leave it as NC and donot remove it.

Karthik Ramasubramanian (Code Review)

6:19 p.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 110: VGPIO_39 What is GPP_G5 then? It is being routed to the card detect line as per the schematics.

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 285: PLTRST

...

Why is this set to PLTRST and others to DEEP? Is that to avoid interrupt storms when booting from S5 […]

I have the same question as Furquan.

Varshit B Pandya (Code Review)

4 Mar 4 Mar

3:11 p.m.

Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 2:

(5 comments)

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 110: VGPIO_39

...

What is GPP_G5 then? It is being routed to the card detect line as per the schematics.

Hello Karthik, When the controller is in D3 state, card detect is recognized via virtual GPIO, the details can be referred from this cros-bug b#123350329 You can also find this info in JSL EDS Vol-1, in Chapter 10, 10.4.11 section.

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 111: SdCardPowerEnableActiveHigh

...

This is unused in soc/intel/tigerlake.

Hello Furquan, It is FSP default, so removing it.

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 284: SD_CD_ODL */

...

This is the pin name as in the schematics. Please keep it that way to avoid confusion in the future.

Done

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 288: /* G7 : SD_SDIO_WP */ : PAD_NC(GPP_G7, NONE),

...

Please leave it as NC and donot remove it.

Done

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 285: PLTRST

...

I have the same question as Furquan.

Hello Furquan and Karthik,

I have not checked this yet, will check and update

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 2 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Aamir Bohra aamir.bohra@intel.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 04 Mar 2020 15:11:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Varshit B Pandya (Code Review)

3:37 p.m.

Hello build bot (Jenkins), Karthik Ramasubramanian,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39237

to look at the new patch set (#3).

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/3

Varshit B Pandya (Code Review)

5:29 p.m.

Hello build bot (Jenkins), Karthik Ramasubramanian,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39237

to look at the new patch set (#4).

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/4

Varshit B Pandya (Code Review)

5 Mar 5 Mar

4:41 p.m.

Hello build bot (Jenkins), Karthik Ramasubramanian,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39237

to look at the new patch set (#5).

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/5

Karthik Ramasubramanian (Code Review)

6 Mar 6 Mar

3:53 a.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/5/src/mainboard/google/dedede... PS5, Line 101: NATIVE What is NATIVE pad termination config mean? With that alone, SD Card is not getting detected by the OS. I need to set the UPD SdCardGpioCmdPadTermination = 0x1f to get SD Card detected on both boot and Hot Plug.

Varshit B Pandya (Code Review)

4:38 a.m.

Hello build bot (Jenkins), Karthik Ramasubramanian,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39237

to look at the new patch set (#6).

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/6

Aamir Bohra (Code Review)

9 Mar 9 Mar

1:55 p.m.

Aamir Bohra has uploaded a new patch set (#9) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/9

Varshit B Pandya (Code Review)

11 Mar 11 Mar

11:19 a.m.

Hello build bot (Jenkins), Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39237

to look at the new patch set (#10).

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/10

Aamir Bohra (Code Review)

1:27 p.m.

Aamir Bohra has uploaded a new patch set (#11) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/11

Karthik Ramasubramanian (Code Review)

4:27 p.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 11: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 11 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 11 Mar 2020 16:27:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Aamir Bohra (Code Review)

12 Mar 12 Mar

4:52 a.m.

Aamir Bohra has uploaded a new patch set (#13) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/13

Patrick Georgi (Code Review)

7:43 a.m.

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 13:

(1 comment)

There are still some open comments in patch set 2 and 5. Please review and handle as appropriate.

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 111: SdCardPowerEnableActiveHigh

...

Hello Furquan, […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 13 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 12 Mar 2020 07:43:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Varshit B Pandya varshit.b.pandya@intel.com Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

13 Mar 13 Mar

6:04 a.m.

Aamir Bohra has uploaded a new patch set (#15) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

BUG=b:150872580 TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/15

Aamir Bohra (Code Review)

3:39 p.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 15:

I am seeing inconsistency with SD card enumeration , will debug and revise CL

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 15 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 13 Mar 2020 15:39:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

9:46 p.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 15: -Code-Review

...

Patch Set 15:

I am seeing inconsistency with SD card enumeration , will debug and revise CL

I don't think ACPI definitions for Storage Controllers(scs.asl) are not included in Tigerlake's southbridge.asl. Could that be the reason for the enumeration problem?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 15 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 13 Mar 2020 21:46:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

11:52 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 15:

...

Patch Set 15:

I am seeing inconsistency with SD card enumeration , will debug and revise CL

Can you please attach logs to the bug? Also provide details of what test you ran when you see the issues?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 15 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 13 Mar 2020 23:52:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Aamir Bohra (Code Review)

16 Mar 16 Mar

9:19 a.m.

Aamir Bohra has uploaded a new patch set (#16) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

BUG=b:150872580 TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/16

Karthik Ramasubramanian (Code Review)

27 Mar 27 Mar

11:25 p.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 16:

Add jsl_port as topic so that this CL is not forgotten after TGL/JSL split.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 16 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 27 Mar 2020 23:25:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

7 Apr 7 Apr

6:56 p.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 20: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 20 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 07 Apr 2020 18:56:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Aamir Bohra (Code Review)

19 May 19 May

5:09 p.m.

Aamir Bohra has uploaded a new patch set (#21) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

BUG=b:150872580 TEST=Verify SD card detect and enumeration

Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/gpio.c 2 files changed, 18 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/21

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 21 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Aamir Bohra (Code Review)

20 May 20 May

8:30 a.m.

Aamir Bohra has uploaded a new patch set (#22) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

BUG=b:150872580 TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/22

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 22 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Aamir Bohra (Code Review)

5:22 p.m.

Aamir Bohra has uploaded a new patch set (#23) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

BUG=b:150872580 TEST=Verify SD card detect and enumeration

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/23

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 23 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Aamir Bohra (Code Review)

22 May 22 May

5:51 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 23:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/5/src/mainboard/google/dedede... PS5, Line 101: NATIVE

...

What is NATIVE pad termination config mean? With that alone, SD Card is not getting detected by the […]

native PAD termination would allow controller to dynamically manage the pad terminations. I see it set for GPP_G0 , even if SdCardGpioCmdPadTermination is not set to 0x1F.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 23 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 22 May 2020 05:51:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

6:02 a.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 23:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7 What is the default state of this pin? On hatch, we had to configure this as NF with internal pull-down to prevent write protection from being enabled by the controller.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 23 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 22 May 2020 06:02:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Aamir Bohra (Code Review)

6:57 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 23:

(4 comments)

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 110: VGPIO_39

...

Hello Karthik, […]

Done

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 284: SD_CD_ODL */

...

Done

Done

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 288: /* G7 : SD_SDIO_WP */ : PAD_NC(GPP_G7, NONE),

...

Done

Done

https://review.coreboot.org/c/coreboot/+/39237/2/src/mainboard/google/dedede... PS2, Line 285: PLTRST

...

Hello Furquan and Karthik, […]

I did not see the difference in interrupt count in /proc/interrupt between DEEP and PLTRST configuration for GPP_G5. I think it is ok to reset SD status on pltrst and reconfigure on boot up.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 23 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 22 May 2020 06:57:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Varshit B Pandya varshit.b.pandya@intel.com Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

24 May 24 May

6:23 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 23:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

What is the default state of this pin? On hatch, we had to configure this as NF with internal pull-d […]

Hi Furquan, On hatch , as I remember the SD writes were failing due to SD WP getting asserted. I checked for write to SD card with G7 set to NC. Was able to make a write.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 23 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 24 May 2020 06:23:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

6:24 a.m.

Aamir Bohra has uploaded a new patch set (#24) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

BUG=b:150872580 TEST=Verify SD card enumeration and able read/write content.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/24

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 24 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Aamir Bohra (Code Review)

6:25 a.m.

Aamir Bohra has uploaded a new patch set (#25) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

BUG=b:150872580 TEST=Verify SD card enumeration and read/write transactions.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/25

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 25 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Aamir Bohra (Code Review)

27 May 27 May

4:21 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 25:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

Hi Furquan, On hatch , as I remember the SD writes were failing due to SD WP getting asserted. […]

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 25 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 27 May 2020 04:21:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

5:46 a.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 25: Code-Review+2

(2 comments)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

Ack

Thanks for the confirmation, Aamir!

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... PS25, Line 272: NATIVE I think the reason you are setting this to NATIVE is because the controller would want to pull up to 1.8 or 3.3V depending upon mode? If that understanding is correct, can you please add a comment here?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 25 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 27 May 2020 05:46:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

4:16 p.m.

Aamir Bohra has uploaded a new patch set (#26) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

BUG=b:150872580 TEST=Verify SD card enumeration and read/write transactions.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/26

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 26 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Aamir Bohra (Code Review)

4:24 p.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 26:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

Thanks for the confirmation, Aamir!

Furquan , I further checked setting the WP as NF1 and adding a 20K PD. I see a clean dmesg now for mmc0. earlier below errors were seen. [ 8.346777] mmcblk0: recovery failed! [ 8.351034] blk_update_request: I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... PS25, Line 272: NATIVE

...

I think the reason you are setting this to NATIVE is because the controller would want to pull up to […]

correct. I added the native termination for SD clk as well. EDS(vol 1 10.4.10) recommends setting native for CMD, DATA and CLK signals.

After setting the CLK terminati to native on the clock tuning errors are not seen.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 26 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 27 May 2020 16:24:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

4:29 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 26:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

Furquan , I further checked setting the WP as NF1 and adding a 20K PD. […]

Do you understand what the errors mean? And how GPP_G7/SD_SDIO_WP was causing those?

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... PS25, Line 272: NATIVE

...

correct. I added the native termination for SD clk as well. EDS(vol 1 10.4. […]

Thanks Aamir! Can you please add the reference to the EDS section as well in the comment here?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 26 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 27 May 2020 16:29:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

9:40 p.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 26:

(3 comments)

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... PS26, Line 258: device pci 14.5 on end # SDCard Nit: an extra space to align with the rest of the 'end' below.

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... PS25, Line 272: NATIVE

...

Thanks Aamir! Can you please add the reference to the EDS section as well in the comment here?

I remember seeing the CLK Termination to Native in the EDS Vol1. Glad to see that the clock tuning errors are addressed with the termination.

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... PS26, Line 387: DEEP Should this pad reset config align with the Card detect GPIO GPP_G5 i.e. PLTRST

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 26 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 27 May 2020 21:40:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

28 May 28 May

5:09 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 26:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

Do you understand what the errors mean? And how GPP_G7/SD_SDIO_WP was causing those?

Still to check on that. I'll check and update.

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... PS26, Line 387: DEEP

...

Should this pad reset config align with the Card detect GPIO GPP_G5 i.e. […]

It should align with the G5, I'll check test and update.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 26 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 May 2020 05:09:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

5:34 p.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 26:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

Still to check on that. I'll check and update.

I checked for the below errors: [ 8.346777] mmcblk0: recovery failed! [ 8.351034] blk_update_request: I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0 mmc_blk_mq_rw_recovery the recovery fail is due failure to reset the card in mmc_blk_mq_complete_prev_req call. The mmc_blk_mq_complete_prev_req seems to get triggered if card continuously runs busy due to incomplete transaction as part of transfer queue.

I tried setting the GPIO to NC with pull down 20k config, to check pulling it to gnd can have similar impact. But I still the dmesg errors. The current configuration works fine.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 26 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 May 2020 17:34:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

6:15 p.m.

Aamir Bohra has uploaded a new patch set (#27) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

SD card CMD. DATA and CLK GPIOs are set for native pad termination as per recommendation in EDS vol1 section 10.4.10

BUG=b:150872580 TEST=Verify SD card enumeration and read/write transactions.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/27

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 27 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Aamir Bohra (Code Review)

6:17 p.m.

Aamir Bohra has uploaded a new patch set (#28) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

SD card CMD. DATA and CLK GPIOs are set for native pad termination as per recommendation in EDS vol1 section 10.4.10

BUG=b:150872580 TEST=Verify SD card enumeration and read/write transactions.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39237/28

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 28 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Aamir Bohra (Code Review)

6:17 p.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 27:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... PS26, Line 258: device pci 14.5 on end # SDCard

...

Nit: an extra space to align with the rest of the 'end' below.

Done

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/26/src/mainboard/google/deded... PS26, Line 387: DEEP

...

It should align with the G5, I'll check test and update.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 27 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 May 2020 18:17:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

8:40 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 28: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

I checked for the below errors: […]

Thanks Aamir! I still don't understand why the WP creates problems only at init time. Probably transient state? We don't need to block this change, but it would be good to understand the dependency/recommendation on how the SD WP should be configured when unused.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 28 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 May 2020 20:40:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

29 May 29 May

5:49 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 28:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

Thanks Aamir! I still don't understand why the WP creates problems only at init time. […]

ok, yes. based on current observations the NF configuration seems to be must for resolving the dmesg errors.

Below are still open:

1. Does controller expects to be configured WP as NF, even if not connected. 2. Why are tranfer request not getting completed and card status is set busy with NC configuration.

We can update the bug based on the findings.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 28 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 29 May 2020 05:49:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

5:50 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 28:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/23/src/mainboard/google/deded... PS23, Line 284: GPP_G7

...

ok, yes. […]

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 28 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 29 May 2020 05:50:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

6:33 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 28:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39237/25/src/mainboard/google/deded... PS25, Line 272: NATIVE

...

I remember seeing the CLK Termination to Native in the EDS Vol1. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 28 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 29 May 2020 06:33:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

6:55 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 28: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 28 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 29 May 2020 06:55:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Aamir Bohra (Code Review)

6:55 a.m.

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 28: -Code-Review

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 28 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 29 May 2020 06:55:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

2 Jun 2 Jun

3:26 a.m.

Karthik Ramasubramanian has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

mb/google/dedede: Add SD card support

1. Configure SD card GPIOs. 2. Set SD card power polarity and card detect configs.

SD card CMD. DATA and CLK GPIOs are set for native pad termination as per recommendation in EDS vol1 section 10.4.10

BUG=b:150872580 TEST=Verify SD card enumeration and read/write transactions.

Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Signed-off-by: Aamir Bohra aamir.bohra@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39237 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/gpio.c 2 files changed, 19 insertions(+), 10 deletions(-)

Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved

diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index 72ce7c1..a856b22 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -114,6 +114,11 @@ # Enable EMMC HS400 mode register "ScsEmmcHs400Enabled" = "1"

+ # GPIO for SD card detect + register "sdcard_cd_gpio" = "VGPIO_39" + # SD card power enable polarity + register "SdCardPowerEnableActiveHigh" = "1" + # Display related UPDs # Select eDP for port A register "DdiPortAConfig" = "1" @@ -255,7 +260,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 end # I2C 0 device pci 15.1 on end # I2C 1 device pci 15.2 on end # I2C 2 diff --git a/src/mainboard/google/dedede/variants/baseboard/gpio.c b/src/mainboard/google/dedede/variants/baseboard/gpio.c index ca28e3d..e62140e 100644 --- a/src/mainboard/google/dedede/variants/baseboard/gpio.c +++ b/src/mainboard/google/dedede/variants/baseboard/gpio.c @@ -267,26 +267,26 @@ PAD_CFG_NF(GPP_F18, NONE, DEEP, NF1),

/* G0 : SD_CMD */ - PAD_NC(GPP_G0, NONE), + PAD_CFG_NF(GPP_G0, NATIVE, DEEP, NF1), /* G1 : SD_DATA0 */ - PAD_NC(GPP_G1, NONE), + PAD_CFG_NF(GPP_G1, NATIVE, DEEP, NF1), /* G2 : SD_DATA1 */ - PAD_NC(GPP_G2, NONE), + PAD_CFG_NF(GPP_G2, NATIVE, DEEP, NF1), /* G3 : SD_DATA2 */ - PAD_NC(GPP_G3, NONE), + PAD_CFG_NF(GPP_G3, NATIVE, DEEP, NF1), /* G4 : SD_DATA3 */ - PAD_NC(GPP_G4, NONE), + PAD_CFG_NF(GPP_G4, NATIVE, DEEP, NF1), /* G5 : SD_CD_ODL */ - PAD_NC(GPP_G5, NONE), + PAD_CFG_NF(GPP_G5, NONE, PLTRST, NF1), /* G6 : SD_CLK */ - PAD_NC(GPP_G6, NONE), + PAD_CFG_NF(GPP_G6, NATIVE, DEEP, NF1), /* G7 : SD_SDIO_WP */ - PAD_NC(GPP_G7, NONE), + PAD_CFG_NF(GPP_G7, DN_20K, DEEP, NF1),

/* H0 : WWAN_PERST */ PAD_NC(GPP_H0, NONE), /* H1 : EN_PP3300_SD_U */ - PAD_NC(GPP_H1, NONE), + PAD_CFG_NF(GPP_H1, NONE, DEEP, NF1), /* H2 : CNV_CLKREQ0 */ PAD_CFG_NF(GPP_H2, NONE, DEEP, NF3), /* H3 : GPP_H03/SX_EXIT_HOLDOFF_N */ @@ -382,6 +382,10 @@ PAD_NC(GPD9, NONE), /* GPD10 : AP_SLP_S5_L */ PAD_NC(GPD10, NONE), + + /* SD card detect virtual GPIO */ + PAD_CFG_GPI_GPIO_DRIVER(VGPIO_39, NONE, PLTRST), + };

/* Early pad configuration in bootblock */

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 29 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

9elements QA (Code Review)

4:17 a.m.

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39237 )

Change subject: mb/google/dedede: Add SD card support ......................................................................

Patch Set 29:

Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/4508 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4507 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4506 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/4505

Please note: This test is under development and might not be accurate at all!

-- To view, visit https://review.coreboot.org/c/coreboot/+/39237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90c8ceb85ada23718ff7b6fd7013317c818dd532 Gerrit-Change-Number: 39237 Gerrit-PatchSet: 29 Gerrit-Owner: Varshit B Pandya varshit.b.pandya@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Ian Feng ian_feng@compal.corp-partner.google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 02 Jun 2020 04:17:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1638

days inactive

1729

days old

coreboot-gerrit@coreboot.org

50 comments

6 participants

tags (0)

participants (6)

9elements QA (Code Review)
Aamir Bohra (Code Review)
Furquan Shaikh (Code Review)
Karthik Ramasubramanian (Code Review)
Patrick Georgi (Code Review)
Varshit B Pandya (Code Review)