coreboot-gerrit November 2020

coreboot-gerrit@coreboot.org

1 participants
3243 discussions

Change in coreboot[master]: soc/intel/skylake: Support NHLT 1ch DMIC
by Patrick Georgi (Code Review) 24 Nov '20

24 Nov '20

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43072 ) Change subject: soc/intel/skylake: Support NHLT 1ch DMIC ...................................................................... soc/intel/skylake: Support NHLT 1ch DMIC Allows advertising support for a 1ch array DMIC in the NHLT table. Boards use the NHLT if a microphone is connected to the DSP. Tested on an Acer Aspire VN7-572G (Skylake-U) on Windows 10. A custom ALSA topology will be required for Linux. Change-Id: Idba3a714faab5ca1958de7dcfc0fc667c60ea7fd Signed-off-by: Benjamin Doron <benjamin.doron00(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43072 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/nhlt/Makefile.inc M src/soc/intel/skylake/nhlt/dmic.c 3 files changed, 47 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index ed81e24..ba3af84 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -161,6 +161,12 @@ help Choose this option if you have a PCH-H chipset. +config NHLT_DMIC_1CH + bool + default n + help + Include DSP firmware settings for 1 channel DMIC array. + config NHLT_DMIC_2CH bool default n diff --git a/src/soc/intel/skylake/nhlt/Makefile.inc b/src/soc/intel/skylake/nhlt/Makefile.inc index 5c8bd80..7596492 100644 --- a/src/soc/intel/skylake/nhlt/Makefile.inc +++ b/src/soc/intel/skylake/nhlt/Makefile.inc @@ -15,6 +15,7 @@ NHLT_BLOB_PATH = 3rdparty/blobs/soc/intel/skylake/nhlt-blobs endif +DMIC_1CH_48KHZ_16B = dmic-1ch-48khz-16b.bin DMIC_2CH_48KHZ_16B = dmic-2ch-48khz-16b.bin DMIC_2CH_48KHZ_32B = dmic-2ch-48khz-32b.bin DMIC_4CH_48KHZ_16B = dmic-4ch-48khz-16b.bin @@ -31,6 +32,10 @@ SSM4567_CAPTURE = ssm4567-capture-4ch-48khz-32b.bin DA7219_RENDER_CAPTURE = dialog-2ch-48khz-24b.bin +cbfs-files-$(CONFIG_NHLT_DMIC_1CH) += $(DMIC_1CH_48KHZ_16B) +$(DMIC_1CH_48KHZ_16B)-file := $(NHLT_BLOB_PATH)/$(DMIC_1CH_48KHZ_16B) +$(DMIC_1CH_48KHZ_16B)-type := raw + cbfs-files-$(CONFIG_NHLT_DMIC_2CH) += $(DMIC_2CH_48KHZ_16B) $(DMIC_2CH_48KHZ_16B)-file := $(NHLT_BLOB_PATH)/$(DMIC_2CH_48KHZ_16B) $(DMIC_2CH_48KHZ_16B)-type := raw diff --git a/src/soc/intel/skylake/nhlt/dmic.c b/src/soc/intel/skylake/nhlt/dmic.c index 76c1990..b8802a6 100644 --- a/src/soc/intel/skylake/nhlt/dmic.c +++ b/src/soc/intel/skylake/nhlt/dmic.c @@ -2,6 +2,39 @@ #include <soc/nhlt.h> +static const struct nhlt_format_config dmic_1ch_formats[] = { + /* 48 KHz 16-bits per sample. */ + { + .num_channels = 1, + .sample_freq_khz = 48, + .container_bits_per_sample = 16, + .valid_bits_per_sample = 16, + .speaker_mask = SPEAKER_FRONT_CENTER, + .settings_file = "dmic-1ch-48khz-16b.bin", + }, +}; + +static const struct nhlt_dmic_array_config dmic_1ch_mic_config = { + .tdm_config = { + .config_type = NHLT_TDM_BASIC, + }, + .array_type = NHLT_MIC_ARRAY_VENDOR_DEFINED, +}; + +static const struct nhlt_endp_descriptor dmic_1ch_descriptors[] = { + { + .link = NHLT_LINK_PDM, + .device = NHLT_PDM_DEV_CAVS15, + .direction = NHLT_DIR_CAPTURE, + .vid = NHLT_VID, + .did = NHLT_DID_DMIC, + .cfg = &dmic_1ch_mic_config, + .cfg_size = sizeof(dmic_1ch_mic_config), + .formats = dmic_1ch_formats, + .num_formats = ARRAY_SIZE(dmic_1ch_formats), + }, +}; + static const struct nhlt_format_config dmic_2ch_formats[] = { /* 48 KHz 16-bits per sample. */ { @@ -91,6 +124,9 @@ int nhlt_soc_add_dmic_array(struct nhlt *nhlt, int num_channels) { switch (num_channels) { + case 1: + return nhlt_add_endpoints(nhlt, dmic_1ch_descriptors, + ARRAY_SIZE(dmic_1ch_descriptors)); case 2: return nhlt_add_endpoints(nhlt, dmic_2ch_descriptors, ARRAY_SIZE(dmic_2ch_descriptors)); -- To view, visit https://review.coreboot.org/c/coreboot/+/43072 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idba3a714faab5ca1958de7dcfc0fc667c60ea7fd Gerrit-Change-Number: 43072 Gerrit-PatchSet: 16 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/intel/skylake: Use correct NHLT_PDM_DEV definition
by Patrick Georgi (Code Review) 24 Nov '20

24 Nov '20

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45010 ) Change subject: soc/intel/skylake: Use correct NHLT_PDM_DEV definition ...................................................................... soc/intel/skylake: Use correct NHLT_PDM_DEV definition According to the NHLT specification[1], PDM_DEV is defined as "1" on Kabylake based platforms. coreboot currently sets it to "0" on all platforms. Add an entry to the enum and use it to define NHLT_PDM_DEV for Kabylake. "Device Type" will resume from "2" on all platforms, but entries are currently reserved. Tested on an Acer Aspire VN7-572G (Skylake-U), which has a 1ch array DMIC, on Windows 10. 1. https://01.org/sites/default/files/595976_intel_sst_nhlt.pdf Change-Id: Ifbc67228c9e7af7db5154d597ca8d67860cfd2ed Signed-off-by: Benjamin Doron <benjamin.doron00(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/45010 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/include/nhlt.h M src/soc/intel/skylake/nhlt/dmic.c 2 files changed, 3 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/src/include/nhlt.h b/src/include/nhlt.h index 3355801..167be52 100644 --- a/src/include/nhlt.h +++ b/src/include/nhlt.h @@ -187,6 +187,7 @@ enum { NHLT_PDM_DEV, + NHLT_PDM_DEV_CAVS15, // NHLT_PDM_DEV on cAVS1.5 (KBL) based platforms }; /* Endpoint direction. */ diff --git a/src/soc/intel/skylake/nhlt/dmic.c b/src/soc/intel/skylake/nhlt/dmic.c index 16eb605..76c1990 100644 --- a/src/soc/intel/skylake/nhlt/dmic.c +++ b/src/soc/intel/skylake/nhlt/dmic.c @@ -33,7 +33,7 @@ static const struct nhlt_endp_descriptor dmic_2ch_descriptors[] = { { .link = NHLT_LINK_PDM, - .device = NHLT_PDM_DEV, + .device = NHLT_PDM_DEV_CAVS15, .direction = NHLT_DIR_CAPTURE, .vid = NHLT_VID, .did = NHLT_DID_DMIC, @@ -77,7 +77,7 @@ static const struct nhlt_endp_descriptor dmic_4ch_descriptors[] = { { .link = NHLT_LINK_PDM, - .device = NHLT_PDM_DEV, + .device = NHLT_PDM_DEV_CAVS15, .direction = NHLT_DIR_CAPTURE, .vid = NHLT_VID, .did = NHLT_DID_DMIC, -- To view, visit https://review.coreboot.org/c/coreboot/+/45010 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbc67228c9e7af7db5154d597ca8d67860cfd2ed Gerrit-Change-Number: 45010 Gerrit-PatchSet: 9 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/google/volteer/variants/delbin: Enhance I2C5 bus freq closer 400 kHz
by Frank Chu (Code Review) 24 Nov '20

24 Nov '20

Frank Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47736 ) Change subject: mb/google/volteer/variants/delbin: Enhance I2C5 bus freq closer 400 kHz ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/47736 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6d60abe15645dc51ed9ee30975d2521b8940c2d0 Gerrit-Change-Number: 47736 Gerrit-PatchSet: 3 Gerrit-Owner: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-CC: Hank Lin <hank2_lin(a)pegatron.corp-partner.google.com> Gerrit-CC: Ken Lu <ken_lu(a)pegatron.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 24 Nov 2020 08:46:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/hatch: add non-ChromeOS FMAPs
by Matt DeVillier (Code Review) 24 Nov '20

24 Nov '20

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47724 ) Change subject: mb/google/hatch: add non-ChromeOS FMAPs ...................................................................... mb/google/hatch: add non-ChromeOS FMAPs Add 16MiB/32MiB FMAPs for both hatch and puff baseboard-based boards Change-Id: I296ef6ed5b46655b7d283105acdc201b6293bbd1 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/mainboard/google/hatch/Kconfig A src/mainboard/google/hatch/default-hatch-16MiB.fmd A src/mainboard/google/hatch/default-hatch-32MiB.fmd A src/mainboard/google/hatch/default-puff-16MiB.fmd A src/mainboard/google/hatch/default-puff-32MiB.fmd 5 files changed, 84 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/47724/1 diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 12e5638..6df8fc4 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -96,15 +96,19 @@ if BOARD_GOOGLE_BASEBOARD_HATCH config FMDFILE string - default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/chromeos-hatch-16MiB.fmd" if BOARD_ROMSIZE_KB_16384 - default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/chromeos-hatch-32MiB.fmd" if BOARD_ROMSIZE_KB_32768 + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/chromeos-hatch-16MiB.fmd" if BOARD_ROMSIZE_KB_16384 && CHROMEOS + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/chromeos-hatch-32MiB.fmd" if BOARD_ROMSIZE_KB_32768 && CHROMEOS + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/default-hatch-16MiB.fmd" if BOARD_ROMSIZE_KB_16384 + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/default-hatch-32MiB.fmd" if BOARD_ROMSIZE_KB_32768 endif if BOARD_GOOGLE_BASEBOARD_PUFF config FMDFILE string - default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/chromeos-puff-16MiB.fmd" if BOARD_ROMSIZE_KB_16384 - default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/chromeos-puff-32MiB.fmd" if BOARD_ROMSIZE_KB_32768 + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/chromeos-puff-16MiB.fmd" if BOARD_ROMSIZE_KB_16384 && CHROMEOS + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/chromeos-puff-32MiB.fmd" if BOARD_ROMSIZE_KB_32768 && CHROMEOS + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/default-puff-16MiB.fmd" if BOARD_ROMSIZE_KB_16384 + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/default-puff-32MiB.fmd" if BOARD_ROMSIZE_KB_32768 config POWER_OFF_ON_CR50_UPDATE bool diff --git a/src/mainboard/google/hatch/default-hatch-16MiB.fmd b/src/mainboard/google/hatch/default-hatch-16MiB.fmd new file mode 100644 index 0000000..d07d922 --- /dev/null +++ b/src/mainboard/google/hatch/default-hatch-16MiB.fmd @@ -0,0 +1,17 @@ +FLASH@0xff000000 0x1000000 { + SI_ALL@0x0 0x400000 { + SI_DESC@0x0 0x1000 + SI_ME@0x1000 0x3ff000 + } + SI_BIOS@0x400000 0xc00000 { + # SMMSTORE requires 64k alignment + SMMSTORE 0x40000 + UNIFIED_MRC_CACHE 0x20000 { + RECOVERY_MRC_CACHE 0x10000 + RW_MRC_CACHE 0x10000 + } + FMAP 0x800 + RO_VPD 0x4000 + COREBOOT(CBFS) + } +} diff --git a/src/mainboard/google/hatch/default-hatch-32MiB.fmd b/src/mainboard/google/hatch/default-hatch-32MiB.fmd new file mode 100644 index 0000000..d7455cf --- /dev/null +++ b/src/mainboard/google/hatch/default-hatch-32MiB.fmd @@ -0,0 +1,20 @@ +FLASH@0xff000000 0x2000000 { + SI_ALL@0x0 0x400000 { + SI_DESC@0x0 0x1000 + SI_ME@0x1000 0x3ff000 + } + # Place the start of BIOS region such that it aligns with + # the start of the 16MiB boundary. Since this is a 32MiB + # SPI flash only the top 16MiB actually gets memory mapped. + SI_BIOS@0x1000000 0x1000000 { + # SMMSTORE requires 64k alignment + SMMSTORE 0x40000 + UNIFIED_MRC_CACHE 0x20000 { + RECOVERY_MRC_CACHE 0x10000 + RW_MRC_CACHE 0x10000 + } + FMAP 0x800 + RO_VPD 0x4000 + COREBOOT(CBFS) + } +} diff --git a/src/mainboard/google/hatch/default-puff-16MiB.fmd b/src/mainboard/google/hatch/default-puff-16MiB.fmd new file mode 100644 index 0000000..591b469 --- /dev/null +++ b/src/mainboard/google/hatch/default-puff-16MiB.fmd @@ -0,0 +1,18 @@ +FLASH@0xff000000 0x1000000 { + SI_ALL@0x0 0x300000 { + SI_DESC@0x0 0x1000 + SI_ME@0x1000 0x2ff000 + } + SI_BIOS@0x300000 0xd00000 { + # SMMSTORE requires 64k alignment + SMMSTORE 0x40000 + UNIFIED_MRC_CACHE 0x20000 { + RECOVERY_MRC_CACHE 0x10000 + RW_MRC_CACHE 0x10000 + } + FMAP 0x800 + RO_VPD 0x4000 + RW_SPD_CACHE(PRESERVE) 0x1000 + COREBOOT(CBFS) + } +} diff --git a/src/mainboard/google/hatch/default-puff-32MiB.fmd b/src/mainboard/google/hatch/default-puff-32MiB.fmd new file mode 100644 index 0000000..bdb9baf --- /dev/null +++ b/src/mainboard/google/hatch/default-puff-32MiB.fmd @@ -0,0 +1,21 @@ +FLASH@0xff000000 0x2000000 { + SI_ALL@0x0 0x300000 { + SI_DESC@0x0 0x1000 + SI_ME@0x1000 0x2ff000 + } + # Place the start of BIOS region such that it aligns with + # the start of the 16MiB boundary. Since this is a 32MiB + # SPI flash only the top 16MiB actually gets memory mapped. + SI_BIOS@0x1000000 0x1000000 { + # SMMSTORE requires 64k alignment + SMMSTORE 0x40000 + UNIFIED_MRC_CACHE 0x20000 { + RECOVERY_MRC_CACHE 0x10000 + RW_MRC_CACHE 0x10000 + } + FMAP 0x800 + RO_VPD 0x4000 + RW_SPD_CACHE(PRESERVE) 0x1000 + COREBOOT(CBFS) + } +} -- To view, visit https://review.coreboot.org/c/coreboot/+/47724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I296ef6ed5b46655b7d283105acdc201b6293bbd1 Gerrit-Change-Number: 47724 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-MessageType: newchange

5 13

Change in coreboot[master]: soc/intel/tigerlake: Refactor TCSS port mux config
by Caveh Jalali (Code Review) 24 Nov '20

24 Nov '20

Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47860 ) Change subject: soc/intel/tigerlake: Refactor TCSS port mux config ...................................................................... Patch Set 2: (4 comments) > Patch Set 1: > > > Patch Set 1: > > > > > Patch Set 1: > > > > > > > Patch Set 1: > > > > > > > > also another suggestion, could be another patch, this is already so much better than just raw constants, but would be nice to just specify GPP_A10 for example in the bias_control fields; that would mean decoding the bank and pin numbers though. > > > > > > ya, i was looking for a way to decode the pin numbers. is that done on the FSP side? > > > > I don't know of a great way, other than > > ``` > > if (gpio_num >= GPP_A0 && gpio_num <= GPP_A24) { > > bank = GPIO_BANK_A; > > } else if (gpio_num >= GPP_B0 && gpio_num <= GPP_B23) { > > bank = GPIO_BANK_B; > > } > > ``` > > > > or the equivalent as a table + loop. I haven't looked much beyond the UPD itself TBH > > We have most of the required code already present in soc/intel/common/block/gpio/gpio.c. I would recommend looking into that: > > const struct pad_community *comm = gpio_get_community(pad); // pad would be what you input from the mainboard e.g. GPP_E10. > int rel_offset = relative_pad_in_comm(comm, pad); > > That should give you the relative offset that needs to be filled in. For the group(bank) encoding, has anyone checked with Intel how GPP_E translates to 0xe? I don't think there is any hardware register that defines this. I fear that we are leaking FSP assumptions/macros into coreboot here. Anyways, this can be handled by: > > int group_idx = gpio_group_index(comm, rel_offset); > > and then have a helper function that can return the bank # expected by FSP. Either you can have that implemented only for TGL now or update pad_group to add a field for fsp_bank_number. We should bring this up with FSP team as well because we cannot just keep expecting users to understand/know the internal assumptions of FSP w.r.t. macros or ids. this is getting a bit messy. i got as far as computing the group offset, but not the group number encoding used here. i opened https://issuetracker.google.com/174116646 to get this resolved. https://review.coreboot.org/c/coreboot/+/47860/1/src/soc/intel/tigerlake/ch… File src/soc/intel/tigerlake/chip.h: https://review.coreboot.org/c/coreboot/+/47860/1/src/soc/intel/tigerlake/ch… PS1, Line 339: struct tcss_config { > suggestion: A little bit of documentation about using IOM_AUX_ORI_BIAS_CTRL for the bias_control* fi […] Done https://review.coreboot.org/c/coreboot/+/47860/1/src/soc/intel/tigerlake/fs… File src/soc/intel/tigerlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/47860/1/src/soc/intel/tigerlake/fs… PS1, Line 135: TGL_LP_GPIO_ID > Can you please add a comment why this is required to be set even if there is no bias_control informa […] Done https://review.coreboot.org/c/coreboot/+/47860/1/src/soc/intel/tigerlake/fs… PS1, Line 139: params->TcssAuxOri |= IOM_TCSS_PORT_CTRL(i, > nit: blank line after continue Done https://review.coreboot.org/c/coreboot/+/47860/1/src/soc/intel/tigerlake/in… File src/soc/intel/tigerlake/include/soc/early_tcss.h: https://review.coreboot.org/c/coreboot/+/47860/1/src/soc/intel/tigerlake/in… PS1, Line 102: TGL_LP_GPIO_ID > Can you please add a comment here that this is expected to be set by FSP and it represents the GPIO […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/47860 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ca69cba646e3dcb7dc73e5d95aedc88fcde0e81 Gerrit-Change-Number: 47860 Gerrit-PatchSet: 2 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 24 Nov 2020 06:39:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Refactor TCSS port mux config
by Caveh Jalali (Code Review) 24 Nov '20

24 Nov '20

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Nick Vaccaro, Brandon Breitenstein, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/47860 to look at the new patch set (#2). Change subject: soc/intel/tigerlake: Refactor TCSS port mux config ...................................................................... soc/intel/tigerlake: Refactor TCSS port mux config This changes how TCSS port configurations are specified in device trees. Instead of using registers of bitmasks, each port's configuration is encapsulated in a struct with members for each configuration parameter. the TcssAuxOri bitmask and IomTypeCPortPadCfg array are replaced with a struct tcss_config which encapsulates the same information on a per port basis. BRANCH=volteer BUG=b:163476857 TEST=verified external USB-C monitor shows up in both cable orientations in combination with following patches Signed-off-by: Caveh Jalali <caveh(a)chromium.org> Change-Id: I0ca69cba646e3dcb7dc73e5d95aedc88fcde0e81 --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/eldrid/overridetree.cb M src/mainboard/google/volteer/variants/elemi/overridetree.cb M src/mainboard/google/volteer/variants/malefor/overridetree.cb M src/mainboard/google/volteer/variants/volteer/overridetree.cb M src/mainboard/google/volteer/variants/volteer2/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c M src/soc/intel/tigerlake/include/soc/early_tcss.h 11 files changed, 104 insertions(+), 49 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/47860/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/47860 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ca69cba646e3dcb7dc73e5d95aedc88fcde0e81 Gerrit-Change-Number: 47860 Gerrit-PatchSet: 2 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/volteer: Create drobit variant
by Frank Chu (Code Review) 24 Nov '20

24 Nov '20

Frank Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47787 ) Change subject: mb/google/volteer: Create drobit variant ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/47787 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I63b7312bba236bd5af028359804d042f6850d8ba Gerrit-Change-Number: 47787 Gerrit-PatchSet: 1 Gerrit-Owner: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)chromium.org> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com> Gerrit-CC: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-CC: Hank Lin <hank2_lin(a)pegatron.corp-partner.google.com> Gerrit-CC: Kane Chen <kane_chen(a)pegatron.corp-partner.google.com> Gerrit-CC: Ken Lu <ken_lu(a)pegatron.corp-partner.google.com> Gerrit-Comment-Date: Tue, 24 Nov 2020 04:25:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in em100[master]: WIP: Parse command table from installer file
by Stefan Reinauer (Code Review) 24 Nov '20

24 Nov '20

Stefan Reinauer has uploaded this change for review. ( https://review.coreboot.org/c/em100/+/37538 ) Change subject: WIP: Parse command table from installer file ...................................................................... WIP: Parse command table from installer file Change-Id: I126a16e2ae5d5b9df5846d9bef45fde307751c05 Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M Makefile A ini.c A ini.h M trace.c 4 files changed, 527 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/em100 refs/changes/38/37538/1 diff --git a/Makefile b/Makefile index fd1be91..7f90cef 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,7 @@ XZ = xz/xz_crc32.c xz/xz_crc64.c xz/xz_dec_bcj.c xz/xz_dec_lzma2.c xz/xz_dec_stream.c SOURCES = em100.c firmware.c fpga.c hexdump.c sdram.c spi.c system.c trace.c usb.c -SOURCES += image.c curl.c chips.c tar.c $(XZ) +SOURCES += image.c curl.c chips.c tar.c ini.c $(XZ) OBJECTS = $(SOURCES:.c=.o) all: dep em100 diff --git a/ini.c b/ini.c new file mode 100644 index 0000000..b4d5921 --- /dev/null +++ b/ini.c @@ -0,0 +1,284 @@ +/* inih -- simple .INI file parser + +SPDX-License-Identifier: BSD-3-Clause + +Copyright (C) 2009-2019, Ben Hoyt + +inih is released under the New BSD license (see LICENSE.txt). Go to the project +home page for more info: + +https://github.com/benhoyt/inih + +*/ + +#if defined(_MSC_VER) && !defined(_CRT_SECURE_NO_WARNINGS) +#define _CRT_SECURE_NO_WARNINGS +#endif + +#include <stdio.h> +#include <ctype.h> +#include <string.h> + +#include "ini.h" + +#if !INI_USE_STACK +#include <stdlib.h> +#endif + +#define MAX_SECTION 50 +#define MAX_NAME 50 + +/* Used by ini_parse_string() to keep track of string parsing state. */ +typedef struct { + const char* ptr; + size_t num_left; +} ini_parse_string_ctx; + +/* Strip whitespace chars off end of given string, in place. Return s. */ +static char* rstrip(char* s) +{ + char* p = s + strlen(s); + while (p > s && isspace((unsigned char)(*--p))) + *p = '\0'; + return s; +} + +/* Return pointer to first non-whitespace char in given string. */ +static char* lskip(const char* s) +{ + while (*s && isspace((unsigned char)(*s))) + s++; + return (char*)s; +} + +/* Return pointer to first char (of chars) or inline comment in given string, + or pointer to null at end of string if neither found. Inline comment must + be prefixed by a whitespace character to register as a comment. */ +static char* find_chars_or_comment(const char* s, const char* chars) +{ +#if INI_ALLOW_INLINE_COMMENTS + int was_space = 0; + while (*s && (!chars || !strchr(chars, *s)) && + !(was_space && strchr(INI_INLINE_COMMENT_PREFIXES, *s))) { + was_space = isspace((unsigned char)(*s)); + s++; + } +#else + while (*s && (!chars || !strchr(chars, *s))) { + s++; + } +#endif + return (char*)s; +} + +/* Version of strncpy that ensures dest (size bytes) is null-terminated. */ +static char* strncpy0(char* dest, const char* src, size_t size) +{ + strncpy(dest, src, size - 1); + dest[size - 1] = '\0'; + return dest; +} + +/* See documentation in header file. */ +int ini_parse_stream(ini_reader reader, void* stream, ini_handler handler, + void* user) +{ + /* Uses a fair bit of stack (use heap instead if you need to) */ +#if INI_USE_STACK + char line[INI_MAX_LINE]; + int max_line = INI_MAX_LINE; +#else + char* line; + size_t max_line = INI_INITIAL_ALLOC; +#endif +#if INI_ALLOW_REALLOC && !INI_USE_STACK + char* new_line; + size_t offset; +#endif + char section[MAX_SECTION] = ""; + char prev_name[MAX_NAME] = ""; + + char* start; + char* end; + char* name; + char* value; + int lineno = 0; + int error = 0; + +#if !INI_USE_STACK + line = (char*)malloc(INI_INITIAL_ALLOC); + if (!line) { + return -2; + } +#endif + +#if INI_HANDLER_LINENO +#define HANDLER(u, s, n, v) handler(u, s, n, v, lineno) +#else +#define HANDLER(u, s, n, v) handler(u, s, n, v) +#endif + + /* Scan through stream line by line */ + while (reader(line, (int)max_line, stream) != NULL) { +#if INI_ALLOW_REALLOC && !INI_USE_STACK + offset = strlen(line); + while (offset == max_line - 1 && line[offset - 1] != '\n') { + max_line *= 2; + if (max_line > INI_MAX_LINE) + max_line = INI_MAX_LINE; + new_line = realloc(line, max_line); + if (!new_line) { + free(line); + return -2; + } + line = new_line; + if (reader(line + offset, (int)(max_line - offset), stream) == NULL) + break; + if (max_line >= INI_MAX_LINE) + break; + offset += strlen(line + offset); + } +#endif + + lineno++; + + start = line; +#if INI_ALLOW_BOM + if (lineno == 1 && (unsigned char)start[0] == 0xEF && + (unsigned char)start[1] == 0xBB && + (unsigned char)start[2] == 0xBF) { + start += 3; + } +#endif + start = lskip(rstrip(start)); + + if (strchr(INI_START_COMMENT_PREFIXES, *start)) { + /* Start-of-line comment */ + } +#if INI_ALLOW_MULTILINE + else if (*prev_name && *start && start > line) { + /* Non-blank line with leading whitespace, treat as continuation + of previous name's value (as per Python configparser). */ + if (!HANDLER(user, section, prev_name, start) && !error) + error = lineno; + } +#endif + else if (*start == '[') { + /* A "[section]" line */ + end = find_chars_or_comment(start + 1, "]"); + if (*end == ']') { + *end = '\0'; + strncpy0(section, start + 1, sizeof(section)); + *prev_name = '\0'; +#if INI_CALL_HANDLER_ON_NEW_SECTION + if (!HANDLER(user, section, NULL, NULL) && !error) + error = lineno; +#endif + } + else if (!error) { + /* No ']' found on section line */ + error = lineno; + } + } + else if (*start) { + /* Not a comment, must be a name[=:]value pair */ + end = find_chars_or_comment(start, "=:"); + if (*end == '=' || *end == ':') { + *end = '\0'; + name = rstrip(start); + value = end + 1; +#if INI_ALLOW_INLINE_COMMENTS + end = find_chars_or_comment(value, NULL); + if (*end) + *end = '\0'; +#endif + value = lskip(value); + rstrip(value); + + /* Valid name[=:]value pair found, call handler */ + strncpy0(prev_name, name, sizeof(prev_name)); + if (!HANDLER(user, section, name, value) && !error) + error = lineno; + } + else if (!error) { + /* No '=' or ':' found on name[=:]value line */ +#if INI_ALLOW_NO_VALUE + *end = '\0'; + name = rstrip(start); + if (!HANDLER(user, section, name, NULL) && !error) + error = lineno; +#else + error = lineno; +#endif + } + } + +#if INI_STOP_ON_FIRST_ERROR + if (error) + break; +#endif + } + +#if !INI_USE_STACK + free(line); +#endif + + return error; +} + +/* See documentation in header file. */ +int ini_parse_file(FILE* file, ini_handler handler, void* user) +{ + return ini_parse_stream((ini_reader)fgets, file, handler, user); +} + +/* See documentation in header file. */ +int ini_parse(const char* filename, ini_handler handler, void* user) +{ + FILE* file; + int error; + + file = fopen(filename, "r"); + if (!file) + return -1; + error = ini_parse_file(file, handler, user); + fclose(file); + return error; +} + +/* An ini_reader function to read the next line from a string buffer. This + is the fgets() equivalent used by ini_parse_string(). */ +static char* ini_reader_string(char* str, int num, void* stream) { + ini_parse_string_ctx* ctx = (ini_parse_string_ctx*)stream; + const char* ctx_ptr = ctx->ptr; + size_t ctx_num_left = ctx->num_left; + char* strp = str; + char c; + + if (ctx_num_left == 0 || num < 2) + return NULL; + + while (num > 1 && ctx_num_left != 0) { + c = *ctx_ptr++; + ctx_num_left--; + *strp++ = c; + if (c == '\n') + break; + num--; + } + + *strp = '\0'; + ctx->ptr = ctx_ptr; + ctx->num_left = ctx_num_left; + return str; +} + +/* See documentation in header file. */ +int ini_parse_string(const char* string, ini_handler handler, void* user) { + ini_parse_string_ctx ctx; + + ctx.ptr = string; + ctx.num_left = strlen(string); + return ini_parse_stream((ini_reader)ini_reader_string, &ctx, handler, + user); +} diff --git a/ini.h b/ini.h new file mode 100644 index 0000000..d2e07e6 --- /dev/null +++ b/ini.h @@ -0,0 +1,148 @@ +/* inih -- simple .INI file parser + +SPDX-License-Identifier: BSD-3-Clause + +Copyright (C) 2009-2019, Ben Hoyt + +inih is released under the New BSD license (see LICENSE.txt). Go to the project +home page for more info: + +https://github.com/benhoyt/inih + +*/ + +#ifndef __INI_H__ +#define __INI_H__ + +/* Make this header file easier to include in C++ code */ +#ifdef __cplusplus +extern "C" { +#endif + +#include <stdio.h> + +/* Nonzero if ini_handler callback should accept lineno parameter. */ +#ifndef INI_HANDLER_LINENO +#define INI_HANDLER_LINENO 0 +#endif + +/* Typedef for prototype of handler function. */ +#if INI_HANDLER_LINENO +typedef int (*ini_handler)(void* user, const char* section, + const char* name, const char* value, + int lineno); +#else +typedef int (*ini_handler)(void* user, const char* section, + const char* name, const char* value); +#endif + +/* Typedef for prototype of fgets-style reader function. */ +typedef char* (*ini_reader)(char* str, int num, void* stream); + +/* Parse given INI-style file. May have [section]s, name=value pairs + (whitespace stripped), and comments starting with ';' (semicolon). Section + is "" if name=value pair parsed before any section heading. name:value + pairs are also supported as a concession to Python's configparser. + + For each name=value pair parsed, call handler function with given user + pointer as well as section, name, and value (data only valid for duration + of handler call). Handler should return nonzero on success, zero on error. + + Returns 0 on success, line number of first error on parse error (doesn't + stop on first error), -1 on file open error, or -2 on memory allocation + error (only when INI_USE_STACK is zero). +*/ +int ini_parse(const char* filename, ini_handler handler, void* user); + +/* Same as ini_parse(), but takes a FILE* instead of filename. This doesn't + close the file when it's finished -- the caller must do that. */ +int ini_parse_file(FILE* file, ini_handler handler, void* user); + +/* Same as ini_parse(), but takes an ini_reader function pointer instead of + filename. Used for implementing custom or string-based I/O (see also + ini_parse_string). */ +int ini_parse_stream(ini_reader reader, void* stream, ini_handler handler, + void* user); + +/* Same as ini_parse(), but takes a zero-terminated string with the INI data +instead of a file. Useful for parsing INI data from a network socket or +already in memory. */ +int ini_parse_string(const char* string, ini_handler handler, void* user); + +/* Nonzero to allow multi-line value parsing, in the style of Python's + configparser. If allowed, ini_parse() will call the handler with the same + name for each subsequent line parsed. */ +#ifndef INI_ALLOW_MULTILINE +#define INI_ALLOW_MULTILINE 1 +#endif + +/* Nonzero to allow a UTF-8 BOM sequence (0xEF 0xBB 0xBF) at the start of + the file. See https://github.com/benhoyt/inih/issues/21 */ +#ifndef INI_ALLOW_BOM +#define INI_ALLOW_BOM 1 +#endif + +/* Chars that begin a start-of-line comment. Per Python configparser, allow + both ; and # comments at the start of a line by default. */ +#ifndef INI_START_COMMENT_PREFIXES +#define INI_START_COMMENT_PREFIXES ";#" +#endif + +/* Nonzero to allow inline comments (with valid inline comment characters + specified by INI_INLINE_COMMENT_PREFIXES). Set to 0 to turn off and match + Python 3.2+ configparser behaviour. */ +#ifndef INI_ALLOW_INLINE_COMMENTS +#define INI_ALLOW_INLINE_COMMENTS 1 +#endif +#ifndef INI_INLINE_COMMENT_PREFIXES +#define INI_INLINE_COMMENT_PREFIXES ";" +#endif + +/* Nonzero to use stack for line buffer, zero to use heap (malloc/free). */ +#ifndef INI_USE_STACK +#define INI_USE_STACK 1 +#endif + +/* Maximum line length for any line in INI file (stack or heap). Note that + this must be 3 more than the longest line (due to '\r', '\n', and '\0'). */ +#ifndef INI_MAX_LINE +#define INI_MAX_LINE 200 +#endif + +/* Nonzero to allow heap line buffer to grow via realloc(), zero for a + fixed-size buffer of INI_MAX_LINE bytes. Only applies if INI_USE_STACK is + zero. */ +#ifndef INI_ALLOW_REALLOC +#define INI_ALLOW_REALLOC 0 +#endif + +/* Initial size in bytes for heap line buffer. Only applies if INI_USE_STACK + is zero. */ +#ifndef INI_INITIAL_ALLOC +#define INI_INITIAL_ALLOC 200 +#endif + +/* Stop parsing on first error (default is to keep parsing). */ +#ifndef INI_STOP_ON_FIRST_ERROR +#define INI_STOP_ON_FIRST_ERROR 0 +#endif + +/* Nonzero to call the handler at the start of each new section (with + name and value NULL). Default is to only call the handler on + each name=value pair. */ +#ifndef INI_CALL_HANDLER_ON_NEW_SECTION +#define INI_CALL_HANDLER_ON_NEW_SECTION 0 +#endif + +/* Nonzero to allow a name without a value (no '=' or ':' on the line) and + call the handler with value NULL in this case. Default is to treat + no-value lines as an error. */ +#ifndef INI_ALLOW_NO_VALUE +#define INI_ALLOW_NO_VALUE 0 +#endif + +#ifdef __cplusplus +} +#endif + +#endif /* __INI_H__ */ diff --git a/trace.c b/trace.c index 31b6e06..07d93ab 100644 --- a/trace.c +++ b/trace.c @@ -12,8 +12,89 @@ */ #include <stdio.h> +#include <stdlib.h> #include <string.h> #include "em100.h" +#include "ini.h" + +typedef struct +{ + int id; + int iomode; + int address; + int data; + int dummy; + const char *description; + const char *remarks; +} spi_command_t; + +spi_command_t spi_commands[256]; + +static int handler(void* user, const char* section, const char* name, + const char* value) +{ + spi_command_t *cmd = (spi_command_t *)user; + + int id = strtol(section, NULL, 16); + if (id < 0 || id > 255) + return 0; /* unknown command */ + + cmd[id].id = id; + /* + * [00] + * IO_Mode=0 + * Address=0 + * Data=0 + * Dummy=0 + * Description=No operation(0x00) + * Remarks=Macronix + */ + + if (strcmp(name, "IO_Mode") == 0) { + cmd[id].iomode = atoi(value); + } else if (strcmp(name, "Address") == 0) { + cmd[id].address = atoi(value); + } else if (strcmp(name, "Data") == 0) { + cmd[id].data = atoi(value); + } else if (strcmp(name, "Dummy") == 0) { + cmd[id].dummy = atoi(value); + } else if (strcmp(name, "Description") == 0) { + cmd[id].description = strdup(value); + } else if (strcmp(name, "Remarks") == 0) { + cmd[id].remarks = strdup(value); + } else { + return 0; /* unknown name, error */ + } + + return 1; +} + +static int read_command_table(void) +{ + int i; + char *command_table_name = get_em100_file("EM100ProCmdIOTable.ini"); + + if (ini_parse(command_table_name, handler, &spi_commands) < 0) { + printf("Can't load 'EM100ProCmdIOTable.ini'\n"); + free(command_table_name); + return 1; + } + free(command_table_name); + + for (i=0; i<256; i++) { + if (spi_commands[i].id != i) + continue; + printf("SPI Command: %02x\n", spi_commands[i].id); + printf(" iomode: %d\n", spi_commands[i].iomode); + printf(" address: %d\n", spi_commands[i].address); + printf(" data: %d\n", spi_commands[i].data); + printf(" dummy: %d\n", spi_commands[i].dummy); + printf(" description: %s\n", spi_commands[i].description); + printf(" remarks: %s\n", spi_commands[i].remarks); + } + + return 0; +} /* SPI Trace related operations */ @@ -34,15 +115,6 @@ return 1; } -/** - * read_spi_trace: fetch SPI trace data - * @param em100: em100 device structure - * globals: curpos, counter, cmdid - * - * out(16 bytes): bc 00 00 00 08 00 00 00 00 15 00 00 00 00 00 00 - * in(8x8192 bytes): 2 bytes (BE) number of records (0..0x3ff), - * then records of 8 bytes each - */ static unsigned int counter = 0; static unsigned char curpos = 0; static unsigned char cmdid = 0xff; // timestamp, so never a valid command id @@ -145,6 +217,16 @@ } #define MAX_TRACE_BLOCKLENGTH 6 + +/** + * read_spi_trace: fetch SPI trace data + * @param em100: em100 device structure + * globals: curpos, counter, cmdid + * + * out(16 bytes): bc 00 00 00 08 00 00 00 00 15 00 00 00 00 00 00 + * in(8x8192 bytes): 2 bytes (BE) number of records (0..0x3ff), + * then records of 8 bytes each + */ int read_spi_trace(struct em100 *em100, int display_terminal, unsigned long addr_offset) { @@ -159,6 +241,9 @@ static unsigned long long start_timestamp = 0; static struct spi_cmd_values *spi_cmd_vals = &spi_command_list[3]; + if (!read_command_table()) + return 0; + if (!read_report_buffer(em100, reportdata)) return 0; -- To view, visit https://review.coreboot.org/c/em100/+/37538 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: em100 Gerrit-Branch: master Gerrit-Change-Id: I126a16e2ae5d5b9df5846d9bef45fde307751c05 Gerrit-Change-Number: 37538 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: newchange

1 7

Change in coreboot[master]: soc/intel/tigerlake: Refactor TCSS port mux config
by Furquan Shaikh (Code Review) 24 Nov '20

24 Nov '20

1 0

Change in coreboot[master]: soc/intel/tigerlake: Refactor TCSS port mux config
by Furquan Shaikh (Code Review) 24 Nov '20

24 Nov '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47860 ) Change subject: soc/intel/tigerlake: Refactor TCSS port mux config ...................................................................... Patch Set 1: > Patch Set 1: > > > Patch Set 1: > > > > > Patch Set 1: > > > > > > also another suggestion, could be another patch, this is already so much better than just raw constants, but would be nice to just specify GPP_A10 for example in the bias_control fields; that would mean decoding the bank and pin numbers though. > > > > ya, i was looking for a way to decode the pin numbers. is that done on the FSP side? > > I don't know of a great way, other than > ``` > if (gpio_num >= GPP_A0 && gpio_num <= GPP_A24) { > bank = GPIO_BANK_A; > } else if (gpio_num >= GPP_B0 && gpio_num <= GPP_B23) { > bank = GPIO_BANK_B; > } > ``` > > or the equivalent as a table + loop. I haven't looked much beyond the UPD itself TBH We have most of the required code already present in soc/intel/common/block/gpio/gpio.c. I would recommend looking into that: const struct pad_community *comm = gpio_get_community(pad); // pad would be what you input from the mainboard e.g. GPP_E10. int rel_offset = relative_pad_in_comm(comm, pad); That should give you the relative offset that needs to be filled in. For the group(bank) encoding, has anyone checked with Intel how GPP_E translates to 0xe? I don't think there is any hardware register that defines this. I fear that we are leaking FSP assumptions/macros into coreboot here. Anyways, this can be handled by: int group_idx = gpio_group_index(comm, rel_offset); and then have a helper function that can return the bank # expected by FSP. Either you can have that implemented only for TGL now or update pad_group to add a field for fsp_bank_number. We should bring this up with FSP team as well because we cannot just keep expecting users to understand/know the internal assumptions of FSP w.r.t. macros or ids. -- To view, visit https://review.coreboot.org/c/coreboot/+/47860 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ca69cba646e3dcb7dc73e5d95aedc88fcde0e81 Gerrit-Change-Number: 47860 Gerrit-PatchSet: 1 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 23 Nov 2020 23:16:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2020