Change in coreboot[master]: soc/intel/tigerlake: Set dynamic pad termination for SD CMD pin
Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39341 )
Change subject: soc/intel/tigerlake: Set dynamic pad termination for SD CMD pin ......................................................................
soc/intel/tigerlake: Set dynamic pad termination for SD CMD pin
Change-Id: I1bdb2d8cf2b16cb43f39927fe616d177a9e54f48 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/tigerlake/fsp_params_jsl.c 1 file changed, 9 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/39341/1
diff --git a/src/soc/intel/tigerlake/fsp_params_jsl.c b/src/soc/intel/tigerlake/fsp_params_jsl.c index 8eb3fba..0e196ac 100644 --- a/src/soc/intel/tigerlake/fsp_params_jsl.c +++ b/src/soc/intel/tigerlake/fsp_params_jsl.c @@ -26,6 +26,8 @@ #include <soc/soc_chip.h> #include <string.h>
+#define DYNAMIC_PAD_TERMINATION 0x1F + static const pci_devfn_t serial_io_dev[] = { PCH_DEVFN_I2C0, PCH_DEVFN_I2C1, @@ -143,7 +145,13 @@ }
/* SDCard related configuration */ - params->ScsSdCardEnabled = pcidev_path_on_root(PCH_DEVFN_SDCARD) ? dev->enabled : 0; + dev = pcidev_path_on_root(PCH_DEVFN_SDCARD); + if (!dev) { + params->ScsSdCardEnabled = 0; + } else { + params->ScsSdCardEnabled = dev->enabled; + params->SdCardGpioCmdPadTermination = DYNAMIC_PAD_TERMINATION; + }
params->Device4Enable = config->Device4Enable;
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39341 )
Change subject: soc/intel/tigerlake: Set dynamic pad termination for SD CMD pin ......................................................................
Patch Set 2: Code-Review-1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39341/2/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params_jsl.c:
https://review.coreboot.org/c/coreboot/+/39341/2/src/soc/intel/tigerlake/fsp... PS2, Line 153: params->SdCardGpioCmdPadTermination = DYNAMIC_PAD_TERMINATION; I don't see why this would be required. Coreboot is capable of configuring pad termination to be native.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39341 )
Change subject: soc/intel/tigerlake: Set dynamic pad termination for SD CMD pin ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39341/2/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params_jsl.c:
https://review.coreboot.org/c/coreboot/+/39341/2/src/soc/intel/tigerlake/fsp... PS2, Line 146: dev->enabled There is a bug here - dev is not setup with the PCH_DEVFN_SDCARD. So it is checking the enabled flag of whatever device that was previously resolved i.e. IGD here.
https://review.coreboot.org/c/coreboot/+/39341/2/src/soc/intel/tigerlake/fsp... PS2, Line 153: params->SdCardGpioCmdPadTermination = DYNAMIC_PAD_TERMINATION;
I don't see why this would be required. […]
Also the UPD comment/documentation does not mention what 0x1F means. It includes until 0x19. Can you please check that? So is default UPD value messing with the NATIVE configuration?
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39341 )
Change subject: soc/intel/tigerlake: Set dynamic pad termination for SD CMD pin ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39341/2/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params_jsl.c:
https://review.coreboot.org/c/coreboot/+/39341/2/src/soc/intel/tigerlake/fsp... PS2, Line 153: params->SdCardGpioCmdPadTermination = DYNAMIC_PAD_TERMINATION;
Also the UPD comment/documentation does not mention what 0x1F means. It includes until 0x19. […]
Looking at pad dump, we do not need this CL, I see the termination configured as native.
Aamir Bohra has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/39341 )
Change subject: soc/intel/tigerlake: Set dynamic pad termination for SD CMD pin ......................................................................
Abandoned
the CL is not needed, pad termination is getting set to native.
-
Aamir Bohra (Code Review) -
Furquan Shaikh (Code Review)
-
Karthik Ramasubramanian (Code Review)