Change in coreboot[master]: soc/amd/picasso: Add emmc_config->preset_drive_strength

[Raul Rangel (Code Review)]

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45097 )

Change subject: soc/amd/picasso: Add emmc_config->preset_drive_strength
......................................................................

soc/amd/picasso: Add emmc_config->preset_drive_strength

This change allows passing in the preset drive strength to FSP.

BUG=b:159823235
TEST=Made sure presets are unchanged.

Signed-off-by: Raul E Rangel 
Change-Id: I44951cacb1e9d788016a70283cf9688bf88a09f4
---
M src/soc/amd/picasso/chip.h
M src/soc/amd/picasso/fsp_params.c
2 files changed, 20 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/45097/1

diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h
index efac418..c70de65 100644
--- a/src/soc/amd/picasso/chip.h
+++ b/src/soc/amd/picasso/chip.h
@@ -153,6 +153,24 @@
    		SD_EMMC_EMMC_HS400,
    		SD_EMMC_EMMC_HS300,
    	} timing;
+
+		/*
+		 * Sets the drive strength reflected in the UHS-I SDHCI Preset Value
+		 * Registers. The drive strength is also set in the undocumented EMMCCFG
+		 * HS400 preset register. It is not possible to read this value from the
+		 * SDHCI Preset Value Registers.
+		 *
+		 * According to the SDHCI spec:
+		 *   The host should select the weakest drive strength that meets rise /
+		 *   fall time requirement at system operating frequency.
+		 */
+		enum {
+			SD_EMMC_DRIVE_STRENGTH_DEFAULT,
+			SD_EMMC_DRIVE_STRENGTH_B,
+			SD_EMMC_DRIVE_STRENGTH_A,
+			SD_EMMC_DRIVE_STRENGTH_C,
+			SD_EMMC_DRIVE_STRENGTH_D,
+		} preset_drive_strength;
    } emmc_config;

uint8_t xhci0_force_gen1;
diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c
index 1e4f2a5..c7c3657 100644
--- a/src/soc/amd/picasso/fsp_params.c
+++ b/src/soc/amd/picasso/fsp_params.c
@@ -54,6 +54,8 @@
    }

scfg->emmc0_mode = val;
+
+	scfg->emmc0_drive_strength = cfg->emmc_config.preset_drive_strength;
 }

static void fill_dxio_descriptors(FSP_S_CONFIG *scfg,

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45097 )

Change subject: soc/amd/picasso: Add emmc_config->preset_drive_strength
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45097/1/src/soc/amd/picasso/chip.h 
File src/soc/amd/picasso/chip.h:

https://review.coreboot.org/c/coreboot/+/45097/1/src/soc/amd/picasso/chip.h@... 
PS1, Line 159: MMCCFG
             : 		 * HS400 preset
nit I wouldn't name any undocumented registers.

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45097 )

Change subject: soc/amd/picasso: Add emmc_config->preset_drive_strength
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45097/1/src/soc/amd/picasso/chip.h 
File src/soc/amd/picasso/chip.h:

https://review.coreboot.org/c/coreboot/+/45097/1/src/soc/amd/picasso/chip.h@... 
PS1, Line 159: MMCCFG
             : 		 * HS400 preset
> nit I wouldn't name any undocumented registers.

How about:
> The drive strength is also set in the HS400 preset register.

?

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45097 )

Change subject: soc/amd/picasso: Add emmc_config->preset_drive_strength
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45097/1/src/soc/amd/picasso/chip.h 
File src/soc/amd/picasso/chip.h:

https://review.coreboot.org/c/coreboot/+/45097/1/src/soc/amd/picasso/chip.h@... 
PS1, Line 159: MMCCFG
             : 		 * HS400 preset
> How about: […]

Ack

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45097 )

Change subject: soc/amd/picasso: Add emmc_config->preset_drive_strength
......................................................................

Patch Set 1: Code-Review+1

Change in coreboot[master]: soc/amd/picasso: Add eMMC driver strength and init kHz settings

[Raul Rangel (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Aaron Durbin, Felix Held,

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

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

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

Change subject: soc/amd/picasso: Add eMMC driver strength and init kHz settings
......................................................................

soc/amd/picasso: Add eMMC driver strength and init kHz settings

This allows passing in the presets to FSP.

I will set the UPD values after all the zork boards have had their
presets correctly set. This way we don't override the UPD defaults with
0s.

BUG=b:159823235
TEST=Build test

Signed-off-by: Raul E Rangel 
Change-Id: I44951cacb1e9d788016a70283cf9688bf88a09f4
---
M src/soc/amd/picasso/chip.h
1 file changed, 27 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/45097/2

Change in coreboot[master]: soc/amd/picasso: Add eMMC driver strength and init kHz settings

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45097 )

Change subject: soc/amd/picasso: Add eMMC driver strength and init kHz settings
......................................................................

Patch Set 2: Code-Review+2

Change in coreboot[master]: soc/amd/picasso: Add eMMC driver strength and init kHz settings

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45097 )

Change subject: soc/amd/picasso: Add eMMC driver strength and init kHz settings
......................................................................

soc/amd/picasso: Add eMMC driver strength and init kHz settings

This allows passing in the presets to FSP.

I will set the UPD values after all the zork boards have had their
presets correctly set. This way we don't override the UPD defaults with
0s.

BUG=b:159823235
TEST=Build test

Signed-off-by: Raul E Rangel 
Change-Id: I44951cacb1e9d788016a70283cf9688bf88a09f4
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45097
Tested-by: build bot (Jenkins) 
Reviewed-by: Marshall Dawson 
---
M src/soc/amd/picasso/chip.h
1 file changed, 27 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Marshall Dawson: Looks good to me, approved

diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h
index 9c9ae7f..a39549e 100644
--- a/src/soc/amd/picasso/chip.h
+++ b/src/soc/amd/picasso/chip.h
@@ -39,6 +39,13 @@

#define USB_PORT_COUNT	6

+enum sd_emmc_driver_strength {
+	SD_EMMC_DRIVE_STRENGTH_B,
+	SD_EMMC_DRIVE_STRENGTH_A,
+	SD_EMMC_DRIVE_STRENGTH_C,
+	SD_EMMC_DRIVE_STRENGTH_D,
+};
+
 struct soc_amd_picasso_config {
    struct soc_amd_common_config common_config;
    /*
@@ -162,6 +169,26 @@
    		SD_EMMC_EMMC_HS400,
    		SD_EMMC_EMMC_HS300,
    	} timing;
+
+		/*
+		 * Sets the driver strength reflected in the SDHCI Preset Value Registers.
+		 *
+		 * According to the SDHCI spec:
+		 *   The host should select the weakest drive strength that meets rise /
+		 *   fall time requirement at system operating frequency.
+		 */
+		enum sd_emmc_driver_strength sdr104_hs400_driver_strength;
+		enum sd_emmc_driver_strength ddr50_driver_strength;
+		enum sd_emmc_driver_strength sdr50_driver_strength;
+
+		/*
+		 * Sets the frequency in kHz reflected in the Initialization Preset Value
+		 * Register.
+		 *
+		 * This value is used while in open-drain mode, and has a maximum value of
+		 * 400 kHz.
+		 */
+		uint16_t init_khz_preset;
    } emmc_config;

uint8_t xhci0_force_gen1;