Hello Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/40876
to review the following change.
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
soc/amd/picasso: add sd/emmc0 configuration to chip.h
In order to isolate mainboard code from direct FSPS manipulation allow sd/emmc0 configuration to be supplied by devicetree.cb.
BUG=b:153502861
Change-Id: I2569ccccd638faaf2c9ac68fe582ecb9fa967d9f Signed-off-by: Aaron Durbin adurbin@chromium.org Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Commit-Queue: Aaron Durbin adurbin@google.com Tested-by: Aaron Durbin adurbin@google.com Reviewed-by: Furquan Shaikh furquan@chromium.org --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c M src/soc/amd/picasso/include/soc/platform_descriptors.h 3 files changed, 82 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/40876/1
diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index 4cc10ef..1ede5aa 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -70,6 +70,21 @@ uint8_t core_dldo_bypass; uint8_t min_soc_vid_offset; uint8_t aclk_dpm0_freq_400MHz; + + enum { + SD_EMMC_DISABLE, + SD_EMMC_SD_LOW_SPEED, + SD_EMMC_SD_HIGH_SPEED, + SD_EMMC_SD_UHS_I_SDR_50, + SD_EMMC_SD_UHS_I_DDR_50, + SD_EMMC_SD_UHS_I_SDR_104, + SD_EMMC_EMMC_SDR_26, + SD_EMMC_EMMC_SDR_52, + SD_EMMC_EMMC_DDR_52, + SD_EMMC_EMMC_HS200, + SD_EMMC_EMMC_HS400, + SD_EMMC_EMMC_HS300, + } sd_emmc_config; };
typedef struct soc_amd_picasso_config config_t; diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c index 0dbda09..d11dae2 100644 --- a/src/soc/amd/picasso/fsp_params.c +++ b/src/soc/amd/picasso/fsp_params.c @@ -7,6 +7,55 @@ #include <fsp/api.h> #include "chip.h"
+static void fsps_update_emmc_config(FSP_S_CONFIG *scfg, + const struct soc_amd_picasso_config *cfg) +{ + int val = SD_DISABLE; + + switch (cfg->sd_emmc_config) { + case SD_EMMC_DISABLE: + val = SD_DISABLE; + break; + case SD_EMMC_SD_LOW_SPEED: + val = SD_LOW_SPEED; + break; + case SD_EMMC_SD_HIGH_SPEED: + val = SD_HIGH_SPEED; + break; + case SD_EMMC_SD_UHS_I_SDR_50: + val = SD_UHS_I_SDR_50; + break; + case SD_EMMC_SD_UHS_I_DDR_50: + val = SD_UHS_I_DDR_50; + break; + case SD_EMMC_SD_UHS_I_SDR_104: + val = SD_UHS_I_SDR_104; + break; + case SD_EMMC_EMMC_SDR_26: + val = EMMC_SDR_26; + break; + case SD_EMMC_EMMC_SDR_52: + val = EMMC_SDR_52; + break; + case SD_EMMC_EMMC_DDR_52: + val = EMMC_DDR_52; + break; + case SD_EMMC_EMMC_HS200: + val = EMMC_HS200; + break; + case SD_EMMC_EMMC_HS400: + val = EMMC_HS400; + break; + case SD_EMMC_EMMC_HS300: + val = EMMC_HS300; + break; + default: + break; + } + + scfg->emmc0_mode = val; +} + static void fill_pcie_descriptors(FSP_S_CONFIG *scfg, const picasso_fsp_pcie_descriptor *descs, size_t num) { @@ -49,7 +98,10 @@
void platform_fsp_silicon_init_params_cb(FSPS_UPD *supd) { + const struct soc_amd_picasso_config *cfg; FSP_S_CONFIG *scfg = &supd->FspsConfig;
+ cfg = config_of_soc(); + fsps_update_emmc_config(scfg, cfg); fsp_fill_pcie_ddi_descriptors(scfg); } diff --git a/src/soc/amd/picasso/include/soc/platform_descriptors.h b/src/soc/amd/picasso/include/soc/platform_descriptors.h index bc67550..7a8444b 100644 --- a/src/soc/amd/picasso/include/soc/platform_descriptors.h +++ b/src/soc/amd/picasso/include/soc/platform_descriptors.h @@ -8,6 +8,21 @@ #include <platform_descriptors.h> #include <FspsUpd.h>
+/* These tempory macros apply to emmc0_mode field in FSP_S_CONFIG. + * TODO: Remove when official definitions arrive. */ +#define SD_DISABLE 0 +#define SD_LOW_SPEED 1 +#define SD_HIGH_SPEED 2 +#define SD_UHS_I_SDR_50 3 +#define SD_UHS_I_DDR_50 4 +#define SD_UHS_I_SDR_104 5 +#define EMMC_SDR_26 6 +#define EMMC_SDR_52 7 +#define EMMC_DDR_52 8 +#define EMMC_HS200 9 +#define EMMC_HS400 10 +#define EMMC_HS300 11 + /* Mainboard callback to obtain PCIe and DDI descriptors. */ void mainboard_get_pcie_ddi_descriptors( const picasso_fsp_pcie_descriptor **pcie_descs, size_t *pcie_num,
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
Patch Set 1: Code-Review+2
Hello build bot (Jenkins), Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40876
to look at the new patch set (#2).
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
soc/amd/picasso: add sd/emmc0 configuration to chip.h
In order to isolate mainboard code from direct FSPS manipulation allow sd/emmc0 configuration to be supplied by devicetree.cb.
BUG=b:153502861
Change-Id: I2569ccccd638faaf2c9ac68fe582ecb9fa967d9f Signed-off-by: Aaron Durbin adurbin@chromium.org Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Commit-Queue: Aaron Durbin adurbin@google.com Tested-by: Aaron Durbin adurbin@google.com Reviewed-by: Furquan Shaikh furquan@chromium.org --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c M src/soc/amd/picasso/include/soc/platform_descriptors.h 3 files changed, 82 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/40876/2
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
Patch Set 2:
Rebased to fix merge conflict
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
Patch Set 2: Code-Review+2
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
Uploaded patch set 3.
Hello build bot (Jenkins), Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40876
to look at the new patch set (#3).
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
soc/amd/picasso: add sd/emmc0 configuration to chip.h
In order to isolate mainboard code from direct FSPS manipulation allow sd/emmc0 configuration to be supplied by devicetree.cb.
BUG=b:153502861
Change-Id: I2569ccccd638faaf2c9ac68fe582ecb9fa967d9f Signed-off-by: Aaron Durbin adurbin@chromium.org Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Commit-Queue: Aaron Durbin adurbin@google.com Tested-by: Aaron Durbin adurbin@google.com Reviewed-by: Furquan Shaikh furquan@chromium.org --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c M src/soc/amd/picasso/include/soc/platform_descriptors.h 3 files changed, 82 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/40876/3
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
Patch Set 3:
Should not have removed the header. All good now :)
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
Patch Set 3: Code-Review+2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
Patch Set 3: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
soc/amd/picasso: add sd/emmc0 configuration to chip.h
In order to isolate mainboard code from direct FSPS manipulation allow sd/emmc0 configuration to be supplied by devicetree.cb.
BUG=b:153502861
Change-Id: I2569ccccd638faaf2c9ac68fe582ecb9fa967d9f Signed-off-by: Aaron Durbin adurbin@chromium.org Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Commit-Queue: Aaron Durbin adurbin@google.com Tested-by: Aaron Durbin adurbin@google.com Reviewed-by: Furquan Shaikh furquan@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/40876 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c M src/soc/amd/picasso/include/soc/platform_descriptors.h 3 files changed, 82 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index edb1b69..7c68232 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -86,6 +86,21 @@ enum spi100_speed spi_fast_speed; enum spi100_speed spi_altio_speed; enum spi100_speed spi_tpm_speed; + + enum { + SD_EMMC_DISABLE, + SD_EMMC_SD_LOW_SPEED, + SD_EMMC_SD_HIGH_SPEED, + SD_EMMC_SD_UHS_I_SDR_50, + SD_EMMC_SD_UHS_I_DDR_50, + SD_EMMC_SD_UHS_I_SDR_104, + SD_EMMC_EMMC_SDR_26, + SD_EMMC_EMMC_SDR_52, + SD_EMMC_EMMC_DDR_52, + SD_EMMC_EMMC_HS200, + SD_EMMC_EMMC_HS400, + SD_EMMC_EMMC_HS300, + } sd_emmc_config; };
typedef struct soc_amd_picasso_config config_t; diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c index 0dbda09..d11dae2 100644 --- a/src/soc/amd/picasso/fsp_params.c +++ b/src/soc/amd/picasso/fsp_params.c @@ -7,6 +7,55 @@ #include <fsp/api.h> #include "chip.h"
+static void fsps_update_emmc_config(FSP_S_CONFIG *scfg, + const struct soc_amd_picasso_config *cfg) +{ + int val = SD_DISABLE; + + switch (cfg->sd_emmc_config) { + case SD_EMMC_DISABLE: + val = SD_DISABLE; + break; + case SD_EMMC_SD_LOW_SPEED: + val = SD_LOW_SPEED; + break; + case SD_EMMC_SD_HIGH_SPEED: + val = SD_HIGH_SPEED; + break; + case SD_EMMC_SD_UHS_I_SDR_50: + val = SD_UHS_I_SDR_50; + break; + case SD_EMMC_SD_UHS_I_DDR_50: + val = SD_UHS_I_DDR_50; + break; + case SD_EMMC_SD_UHS_I_SDR_104: + val = SD_UHS_I_SDR_104; + break; + case SD_EMMC_EMMC_SDR_26: + val = EMMC_SDR_26; + break; + case SD_EMMC_EMMC_SDR_52: + val = EMMC_SDR_52; + break; + case SD_EMMC_EMMC_DDR_52: + val = EMMC_DDR_52; + break; + case SD_EMMC_EMMC_HS200: + val = EMMC_HS200; + break; + case SD_EMMC_EMMC_HS400: + val = EMMC_HS400; + break; + case SD_EMMC_EMMC_HS300: + val = EMMC_HS300; + break; + default: + break; + } + + scfg->emmc0_mode = val; +} + static void fill_pcie_descriptors(FSP_S_CONFIG *scfg, const picasso_fsp_pcie_descriptor *descs, size_t num) { @@ -49,7 +98,10 @@
void platform_fsp_silicon_init_params_cb(FSPS_UPD *supd) { + const struct soc_amd_picasso_config *cfg; FSP_S_CONFIG *scfg = &supd->FspsConfig;
+ cfg = config_of_soc(); + fsps_update_emmc_config(scfg, cfg); fsp_fill_pcie_ddi_descriptors(scfg); } diff --git a/src/soc/amd/picasso/include/soc/platform_descriptors.h b/src/soc/amd/picasso/include/soc/platform_descriptors.h index bc67550..7a8444b 100644 --- a/src/soc/amd/picasso/include/soc/platform_descriptors.h +++ b/src/soc/amd/picasso/include/soc/platform_descriptors.h @@ -8,6 +8,21 @@ #include <platform_descriptors.h> #include <FspsUpd.h>
+/* These tempory macros apply to emmc0_mode field in FSP_S_CONFIG. + * TODO: Remove when official definitions arrive. */ +#define SD_DISABLE 0 +#define SD_LOW_SPEED 1 +#define SD_HIGH_SPEED 2 +#define SD_UHS_I_SDR_50 3 +#define SD_UHS_I_DDR_50 4 +#define SD_UHS_I_SDR_104 5 +#define EMMC_SDR_26 6 +#define EMMC_SDR_52 7 +#define EMMC_DDR_52 8 +#define EMMC_HS200 9 +#define EMMC_HS400 10 +#define EMMC_HS300 11 + /* Mainboard callback to obtain PCIe and DDI descriptors. */ void mainboard_get_pcie_ddi_descriptors( const picasso_fsp_pcie_descriptor **pcie_descs, size_t *pcie_num,
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40876 )
Change subject: soc/amd/picasso: add sd/emmc0 configuration to chip.h ......................................................................
Patch Set 4:
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/3094 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3093 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3092 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3091
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
Furquan Shaikh (Code Review)
-
Raul Rangel (Code Review)