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