coreboot-gerrit July 2020

coreboot-gerrit@coreboot.org

1 participants
3166 discussions

Change in coreboot[master]: soc/amd/picasso: Split I2c implementation for psp_verstage
by Martin Roth (Code Review) 08 Jul '20

08 Jul '20

Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42064 ) Change subject: soc/amd/picasso: Split I2c implementation for psp_verstage ...................................................................... soc/amd/picasso: Split I2c implementation for psp_verstage BUG=b:158124527 TEST=Build & boot psp_verstage on trembyle Signed-off-by: Martin Roth <martin(a)coreboot.org> Change-Id: I45380e0c61e1bb7a94a96630e5867b7ffca0909c --- M src/soc/amd/picasso/Makefile.inc A src/soc/amd/picasso/i2c-psp.c A src/soc/amd/picasso/i2c-x86.c M src/soc/amd/picasso/i2c.c M src/soc/amd/picasso/include/soc/i2c.h 5 files changed, 86 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/42064/1 diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index ce0a7a8..250b9d1 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -13,6 +13,7 @@ bootblock-y += bootblock/pre_c.S bootblock-y += bootblock/bootblock.c bootblock-y += southbridge.c +bootblock-y += i2c-x86.c bootblock-y += i2c.c bootblock-$(CONFIG_PICASSO_UART) += uart.c bootblock-y += tsc_freq.c @@ -21,6 +22,7 @@ bootblock-y += config.c romstage-y += i2c.c +romstage-y += i2c-x86.c romstage-y += romstage.c romstage-y += gpio.c romstage-y += pmutil.c @@ -37,10 +39,16 @@ verstage-y += i2c.c verstage-y += pmutil.c verstage-y += config.c +ifeq ($(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),y) +verstage-y += i2c-psp.c +else +verstage-y += i2c-x86.c +endif verstage-$(CONFIG_PICASSO_UART) += uart.c verstage-y += tsc_freq.c ramstage-y += i2c.c +ramstage-y += i2c-x86.c ramstage-y += chip.c ramstage-y += cpu.c ramstage-y += data_fabric_util.c diff --git a/src/soc/amd/picasso/i2c-psp.c b/src/soc/amd/picasso/i2c-psp.c new file mode 100644 index 0000000..c4840cd --- /dev/null +++ b/src/soc/amd/picasso/i2c-psp.c @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +#include <commonlib/helpers.h> +#include <console/console.h> +#include <drivers/i2c/designware/dw_i2c.h> +#include <soc/i2c.h> +#include <soc/iomap.h> +#include <stdint.h> + +static uintptr_t i2c_bus_address[I2C_MASTER_DEV_COUNT]; + +uintptr_t dw_i2c_base_address(unsigned int bus) +{ + if (bus >= ARRAY_SIZE(i2c_bus_address)) + return 0; + + return i2c_bus_address[bus]; +} +void i2c_set_bar(unsigned int bus, uintptr_t bar) +{ + if (bus >= ARRAY_SIZE(i2c_bus_address)) { + printk(BIOS_ERR, "Error: i2c index out of bounds: %u.", bus); + return; + } + + i2c_bus_address[bus] = bar; +} diff --git a/src/soc/amd/picasso/i2c-x86.c b/src/soc/amd/picasso/i2c-x86.c new file mode 100644 index 0000000..444a430 --- /dev/null +++ b/src/soc/amd/picasso/i2c-x86.c @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +#include <stdint.h> +#include <soc/iomap.h> +#include <commonlib/helpers.h> +#include <drivers/i2c/designware/dw_i2c.h> + +/* + * We don't have addresses for I2C0-1. + */ +static const uintptr_t i2c_bus_address[] = { + 0, + 0, + APU_I2C2_BASE, + APU_I2C3_BASE, + APU_I2C4_BASE, /* Can only be used in slave mode */ +}; + +_Static_assert( + ARRAY_SIZE(i2c_bus_address) == I2C_MASTER_DEV_COUNT + I2C_SLAVE_DEV_COUNT, + "ARRAY_SIZE(i2c_bus_address) must equal I2C_MASTER_DEV_COUNT + I2C_SLAVE_DEV_COUNT"); + +uintptr_t dw_i2c_base_address(unsigned int bus) +{ + if (bus >= ARRAY_SIZE(i2c_bus_address)) + return 0; + + return i2c_bus_address[bus]; +} diff --git a/src/soc/amd/picasso/i2c.c b/src/soc/amd/picasso/i2c.c index 4c8c669..4818455 100644 --- a/src/soc/amd/picasso/i2c.c +++ b/src/soc/amd/picasso/i2c.c @@ -1,40 +1,18 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <device/mmio.h> -#include <acpi/acpi.h> #include <console/console.h> #include <delay.h> -#include <device/device.h> #include <drivers/i2c/designware/dw_i2c.h> #include <amdblocks/acpimmio.h> #include <soc/i2c.h> -#include <soc/iomap.h> -#include <soc/pci_devs.h> +#include <soc/soc_util.h> #include <soc/southbridge.h> #include "chip.h" /* Global to provide access to chip.c */ const char *i2c_acpi_name(const struct device *dev); -/* - * We don't have addresses for I2C0-1. - */ -static const uintptr_t i2c_bus_address[I2C_MASTER_DEV_COUNT + I2C_SLAVE_DEV_COUNT] = { - 0, - 0, - APU_I2C2_BASE, - APU_I2C3_BASE, - APU_I2C4_BASE, /* Can only be used in slave mode */ -}; - -uintptr_t dw_i2c_base_address(unsigned int bus) -{ - if (bus >= ARRAY_SIZE(i2c_bus_address)) - return 0; - - return i2c_bus_address[bus]; -} - const struct dw_i2c_bus_config *dw_i2c_get_soc_cfg(unsigned int bus) { const struct soc_amd_picasso_config *config; @@ -159,7 +137,7 @@ { uint32_t *gpio_ptr; - gpio_ptr = (uint32_t *)gpio_get_address(gpio); + gpio_ptr = gpio_get_address(gpio); save_table->mux_value = iomux_read8(gpio); save_table->control_value = read32(gpio_ptr); } @@ -169,7 +147,7 @@ { uint32_t *gpio_ptr; - gpio_ptr = (uint32_t *)gpio_get_address(gpio); + gpio_ptr = gpio_get_address(gpio); iomux_write8(gpio, save_table->mux_value); iomux_read8(gpio); write32(gpio_ptr, save_table->control_value); @@ -180,13 +158,17 @@ void sb_reset_i2c_slaves(void) { const struct soc_amd_picasso_config *cfg; - const struct device *dev = pcidev_path_on_root(GNB_DEVFN); struct soc_amd_i2c_save save_table[saved_pins_count]; uint8_t i, j, control; + /* I2C0-1 is not accessible from the x86. */ + uint32_t *i2c_gpio_ptr[] = { + NULL, + NULL, + gpio_get_address(I2C2_SCL_PIN), + gpio_get_address(I2C3_SCL_PIN), + }; - if (!dev || !dev->chip_info) - return; - cfg = dev->chip_info; + cfg = config_of_soc(); control = cfg->i2c_scl_reset & GPIO_I2C_MASK; if (control == 0) return; @@ -202,19 +184,19 @@ */ for (j = 0; j < 9; j++) { if (control & GPIO_I2C2_SCL) - write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_LOW); + write32(i2c_gpio_ptr[2], GPIO_SCL_LOW); if (control & GPIO_I2C3_SCL) - write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_LOW); + write32(i2c_gpio_ptr[3], GPIO_SCL_LOW); - read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */ + read32(i2c_gpio_ptr[3]); /* Flush posted write */ udelay(4); /* 4usec gets 85KHz for 1 pin, 70KHz for 4 pins */ if (control & GPIO_I2C2_SCL) - write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_HIGH); + write32(i2c_gpio_ptr[2], GPIO_SCL_HIGH); if (control & GPIO_I2C3_SCL) - write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_HIGH); + write32(i2c_gpio_ptr[3], GPIO_SCL_HIGH); - read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */ + read32(i2c_gpio_ptr[3]); /* Flush posted write */ udelay(4); } diff --git a/src/soc/amd/picasso/include/soc/i2c.h b/src/soc/amd/picasso/include/soc/i2c.h index 58c27a4..f468d74 100644 --- a/src/soc/amd/picasso/include/soc/i2c.h +++ b/src/soc/amd/picasso/include/soc/i2c.h @@ -27,4 +27,7 @@ void sb_reset_i2c_slaves(void); +/* Sets the base address for the specific I2C bus. */ +void i2c_set_bar(unsigned int bus, uintptr_t bar); + #endif /* __PICASSO_I2C_H__ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/42064 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I45380e0c61e1bb7a94a96630e5867b7ffca0909c Gerrit-Change-Number: 42064 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

6 33

Change in coreboot[master]: drivers/intel/mipi_camera: Add guard to protect camera resources
by Tim Wawrzynczak (Code Review) 08 Jul '20

08 Jul '20

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43003 ) Change subject: drivers/intel/mipi_camera: Add guard to protect camera resources ...................................................................... Patch Set 6: (17 comments) https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… File src/drivers/intel/mipi_camera/camera.c: https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 28: static struct resource_config *alloc_resource_config(void) : { : return (struct resource_config *) malloc(sizeof(struct resource_config)); : } : this is unnecessary, see comment below on line #625 https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 42: if (res_config) : return res_config->action; : else : return UNKNOWN_ACTION; : } nit: ternary operator would be shorter, e.g. return res_config ? res_config->action : UNKNOWN_ACTION https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 56: if (res_config) : return res_config->type; : else : return UNKNOWN_CTRL; ternary would be shorter https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 65: if (res_config) Here you either need to check that `type` is set to CLOCK, and/or set it as well, or else it could be an undefined access of res_config->clk_conf. if (res_config) { res_config->type = CLOCK; res_config->clk_conf = clk_conf; } https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 533: switch (type) { : case IMGCLK: : clk_config = resource_get_clk_config(res_config); : res_id = clk_config->clknum; : break; : case GPIO: : gpio_config = resource_get_gpio_config(res_config); : res_id = gpio_config->gpio_num; : break; : default: : printk(BIOS_ERR, "Unsupported power operation: %x\n" : "OS camera driver will likely not work", type); : return; : } : : res_index = get_resource_index(res_id, type); you can push the logic for getting res_id into get_resource_index(), i.e., this chunk can just be: res_index = get_resource_index(res_config, type), where get_resource_index can handle the `type` from res_config https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 550: if (res_index != -1) { : switch (action) { : case ENABLE: : snprintf(method_name, sizeof(method_name), ENABLE_METHOD_FORMAT, : res_index); : break; : case DISABLE: : snprintf(method_name, sizeof(method_name), DISABLE_METHOD_FORMAT, : res_index); : break; : default: : snprintf(method_name, sizeof(method_name), UNKNOWN_METHOD_FORMAT, : res_index); : acpigen_write_debug_string("Unsupported action"); : printk(BIOS_ERR, "Unsupported resource action: %x\n", action); : return; : } suggestion: a helper function e.g., `get_op_method_namestring()` would clean this up here https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 587: acpigen_write_byte nit: it's safer (and easier) to use acpigen_write_integer() https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 588: acpigen_write_byte nit: it's safer (and easier) to use acpigen_write_integer() https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 591: acpigen_write_byte nit: it's safer (and easier) to use acpigen_write_integer() https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 584: clk_config = resource_get_clk_config(res_config); : if (action == ENABLE) { : acpigen_emit_namestring("MCON"); : acpigen_write_byte(clk_config->clknum); : acpigen_write_byte(clk_config->freq); : } else if (action == DISABLE) { : acpigen_emit_namestring("MCOF"); : acpigen_write_byte(clk_config->clknum); : } else { : acpigen_write_debug_string("Unsupported clock action"); : printk(BIOS_ERR, "Unsupported clock action: %x\n" : "OS camera driver will likely not work", action); : } suggestion: a helper function, e.g. `add_clk_op(const struct clk_config*)` would make this function shorter https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 600: gpio_config = resource_get_gpio_config(res_config); : if (action == ENABLE) { : acpigen_soc_set_tx_gpio(gpio_config->gpio_num); : } else if (action == DISABLE) { : acpigen_soc_clear_tx_gpio(gpio_config->gpio_num); : } else { : acpigen_write_debug_string("Unsupported GPIO action"); : printk(BIOS_ERR, "Unsupported GPIO action: %x\n" : "OS camera driver will likely not work\n", action); : } : break; suggestion: a helper function, e.g. `add_gpio_op(const struct gpio_config*)` would make this function shorter https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 625: struct resource_config *res_config = alloc_resource_config(); You can just create a e.g. `struct resource_config res_config` on the stack, and then just pass it below like so: `resource_set_*_config(&res_config, &config->...);` . `res_config` does not escape this function's lifetime, so it's safe to keep on the stack. https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 628: i < MAX_PWR_OPS good catch 😊 https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 673: _ON method_name ? https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 662: snprintf(method_name, sizeof(method_name), ENABLE_METHOD_FORMAT, : res_mgr.cnt); : acpigen_write_method_serialized(method_name, 0); : acpigen_write_if_lequal_namestr_int(varname, 0); : resource_set_ctrl_type(res_config, seq->ops[i].type); : resource_set_action_type(res_config, ENABLE); : add_power_operation(res_config); : acpigen_pop_len(); /* if */ : : acpigen_emit_byte(INCREMENT_OP); : acpigen_emit_namestring(varname); : acpigen_pop_len(); /* _ON */ suggestion: a helper function, e.g. `write_enable_method()` would be nice https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 687: _ON method_name? https://review.coreboot.org/c/coreboot/+/43003/6/src/drivers/intel/mipi_cam… PS6, Line 675: snprintf(method_name, sizeof(method_name), DISABLE_METHOD_FORMAT, : res_mgr.cnt); : acpigen_write_method_serialized(method_name, 0); : acpigen_emit_byte(DECREMENT_OP); : acpigen_emit_namestring(varname); : : acpigen_write_if_lequal_namestr_int(varname, 0); : resource_set_ctrl_type(res_config, seq->ops[i].type); : resource_set_action_type(res_config, DISABLE); : add_power_operation(res_config); : acpigen_pop_len(); /* if */ : : acpigen_pop_len(); /* _ON */ suggestion: a helper function, e.g. `write_disable_method()` would be nice -- To view, visit https://review.coreboot.org/c/coreboot/+/43003 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1468459d5bbb2fb07bef4e0590c96dd4dbab0d9c Gerrit-Change-Number: 43003 Gerrit-PatchSet: 6 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 08 Jul 2020 19:05:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/picasso: add psp_verstage
by Raul Rangel (Code Review) 08 Jul '20

08 Jul '20

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41816 ) Change subject: soc/amd/picasso: add psp_verstage ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/41816 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia58839caa5bfbae0408702ee8d02ef482f2861c4 Gerrit-Change-Number: 41816 Gerrit-PatchSet: 12 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Eric Peers <epeers(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 08 Jul 2020 18:51:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/waddledee: Enable Audio
by Karthik Ramasubramanian (Code Review) 08 Jul '20

08 Jul '20

Karthik Ramasubramanian has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41765 ) Change subject: mb/google/waddledee: Enable Audio ...................................................................... mb/google/waddledee: Enable Audio 1. Enable HDA Pci device in devicetree 2. Enable I2C4 in devicetree and fill ACPI information 3. Pass correct IRQ GPIO for headset jack BUG=None BRANCH=None TEST=Audio playback and recording works on Waddledee. Change-Id: I77aaa27bb29460ef834c3dd090ced868f2e99616 Signed-off-by: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Signed-off-by: Yong Zhi <yong.zhi(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41765 Reviewed-by: Karthik Ramasubramanian <kramasub(a)google.com> Reviewed-by: Justin TerAvest <teravest(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/google/dedede/variants/waddledee/overridetree.cb 1 file changed, 26 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Justin TerAvest: Looks good to me, approved Karthik Ramasubramanian: Looks good to me, approved diff --git a/src/mainboard/google/dedede/variants/waddledee/overridetree.cb b/src/mainboard/google/dedede/variants/waddledee/overridetree.cb index 33c578c..10e66bb 100644 --- a/src/mainboard/google/dedede/variants/waddledee/overridetree.cb +++ b/src/mainboard/google/dedede/variants/waddledee/overridetree.cb @@ -66,5 +66,31 @@ device i2c 0x2c on end end end + device pci 19.0 on + chip drivers/i2c/generic + register "hid" = ""10EC5682"" + register "name" = ""RT58"" + register "desc" = ""Realtek RT5682"" + register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_BOTH(GPP_D16)" + register "property_count" = "1" + register "property_list[0].type" = "ACPI_DP_TYPE_INTEGER" + register "property_list[0].name" = ""realtek,jd-src"" + register "property_list[0].integer" = "1" + device i2c 1a on end + end + chip drivers/i2c/generic + register "hid" = ""10EC1015"" + register "desc" = ""Realtek SPK AMP L"" + register "uid" = "0" + device i2c 28 on end + end + chip drivers/i2c/generic + register "hid" = ""10EC1015"" + register "desc" = ""Realtek SPK AMP R"" + register "uid" = "1" + device i2c 29 on end + end + end + device pci 1f.3 on end # Intel HDA end end -- To view, visit https://review.coreboot.org/c/coreboot/+/41765 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I77aaa27bb29460ef834c3dd090ced868f2e99616 Gerrit-Change-Number: 41765 Gerrit-PatchSet: 5 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin TerAvest <teravest(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kirtika Ruchandani <kirtika(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Uday M Bhat Gerrit-Reviewer: Yong Zhi <yong.zhi(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marco Chen <marcochen(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/google/waddledee: Enable Audio
by Justin TerAvest (Code Review) 08 Jul '20

08 Jul '20

Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41765 ) Change subject: mb/google/waddledee: Enable Audio ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/41765 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I77aaa27bb29460ef834c3dd090ced868f2e99616 Gerrit-Change-Number: 41765 Gerrit-PatchSet: 4 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin TerAvest <teravest(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kirtika Ruchandani <kirtika(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Uday M Bhat Gerrit-Reviewer: Yong Zhi <yong.zhi(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marco Chen <marcochen(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 08 Jul 2020 18:45:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/waddledee: Enable Audio
by Karthik Ramasubramanian (Code Review) 08 Jul '20

08 Jul '20

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41765 ) Change subject: mb/google/waddledee: Enable Audio ...................................................................... Patch Set 4: Code-Review+2 (2 comments) https://review.coreboot.org/c/coreboot/+/41765/2/src/mainboard/google/deded… File src/mainboard/google/dedede/variants/waddledee/overridetree.cb: https://review.coreboot.org/c/coreboot/+/41765/2/src/mainboard/google/deded… PS2, Line 61: 1 > This 1 mapped to RT5682_JD1 which is correct: […] Done https://review.coreboot.org/c/coreboot/+/41765/2/src/mainboard/google/deded… PS2, Line 67: register "uid" = "0" > According to kenny_chen@realtek. […] Turn-off the EN_SPK GPIO in the follow-up CL using override GPIO configuration. -- To view, visit https://review.coreboot.org/c/coreboot/+/41765 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I77aaa27bb29460ef834c3dd090ced868f2e99616 Gerrit-Change-Number: 41765 Gerrit-PatchSet: 4 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kirtika Ruchandani <kirtika(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Uday M Bhat Gerrit-Reviewer: Yong Zhi <yong.zhi(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marco Chen <marcochen(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 08 Jul 2020 18:44:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Comment-In-Reply-To: Yong Zhi <yong.zhi(a)intel.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/picasso: add psp_verstage
by build bot (Jenkins) (Code Review) 08 Jul '20

08 Jul '20

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41816 ) Change subject: soc/amd/picasso: add psp_verstage ...................................................................... Patch Set 12: (10 comments) https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… File src/soc/amd/picasso/psp_verstage/svc.h: https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 6: #define SVC_CALL4(SVC_ID, R0, R1, R2, R3, Ret) \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 6: #define SVC_CALL4(SVC_ID, R0, R1, R2, R3, Ret) \ macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 19: #define SVC_CALL3(SVC_ID, R0, R1, R2, Ret) \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 19: #define SVC_CALL3(SVC_ID, R0, R1, R2, Ret) \ macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 30: #define SVC_CALL2(SVC_ID, R0, R1, Ret) \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 30: #define SVC_CALL2(SVC_ID, R0, R1, Ret) \ macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 40: #define SVC_CALL1(SVC_ID, R0, Ret) \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 40: #define SVC_CALL1(SVC_ID, R0, Ret) \ macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 49: #define SVC_CALL0(SVC_ID, Ret) \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41816/12/src/soc/amd/picasso/psp_v… PS12, Line 49: #define SVC_CALL0(SVC_ID, Ret) \ macros should not use a trailing semicolon -- To view, visit https://review.coreboot.org/c/coreboot/+/41816 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia58839caa5bfbae0408702ee8d02ef482f2861c4 Gerrit-Change-Number: 41816 Gerrit-PatchSet: 12 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Eric Peers <epeers(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 08 Jul 2020 18:30:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede: Enable SIS touchscreen for Waddledee
by Karthik Ramasubramanian (Code Review) 08 Jul '20

08 Jul '20

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43304 ) Change subject: mb/google/dedede: Enable SIS touchscreen for Waddledee ...................................................................... Patch Set 1: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/43304 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I42fdc5e8243d2c70c953b2f516c10f84a041c035 Gerrit-Change-Number: 43304 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Evan Green <evgreen(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin TerAvest <teravest(a)chromium.org> Gerrit-Reviewer: Marco Chen <marcochen(a)google.com> Gerrit-Comment-Date: Wed, 08 Jul 2020 18:30:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common: Init interrupts without gevent
by Martin Roth (Code Review) 08 Jul '20

08 Jul '20

Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42809 ) Change subject: soc/amd/common: Init interrupts without gevent ...................................................................... soc/amd/common: Init interrupts without gevent Previously, the code wouldn't initialize any of the special flags, including the interrupt flag, if the pin wasn't associated with a GEVENT. BUG=b:158124527 TEST=Build & boot Trembyle with psp_verstage Signed-off-by: Martin Roth <martin(a)coreboot.org> Change-Id: I65efde0a9d347db77c6518c2b988eb29718e778b --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 19 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42809/1 diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c index 2ebef8b..a5f7904 100644 --- a/src/soc/amd/common/block/gpio_banks/gpio.c +++ b/src/soc/amd/common/block/gpio_banks/gpio.c @@ -173,6 +173,17 @@ return gpio; } +static int gevent_missing(uint8_t gpio, int gevent_num, const char *string) +{ + if (gevent_num < 0) { + printk(BIOS_WARNING, "Warning: GPIO pin %d has" + " no associated gevent!\n", gpio); + printk (BIOS_WARNING, "Not programming %s\n", string); + return 1; + } + return 0; +} + __weak void soc_gpio_hook(uint8_t gpio, uint8_t mux) {} void program_gpios(const struct soc_amd_gpio *gpio_list_ptr, size_t size) @@ -211,14 +222,9 @@ iomux_read8(gpio); /* Flush posted write */ soc_gpio_hook(gpio, mux); - + printk(BIOS_DEBUG, "Setting GPIO: %d\n",gpio); if (control_flags & GPIO_SPECIAL_FLAG) { gevent_num = get_gpio_gevent(gpio, gev_tbl, gev_items); - if (gevent_num < 0) { - printk(BIOS_WARNING, "Warning: GPIO pin %d has" - " no associated gevent!\n", gpio); - continue; - } switch (control_flags & GPIO_SPECIAL_MASK) { case GPIO_DEBOUNCE_FLAG: __gpio_setbits32(gpio, GPIO_DEBOUNCE_MASK, control); @@ -233,6 +239,9 @@ /* Can't set SMI flags from PSP */ if (!can_set_smi_flags) break; + if (gevent_missing(gpio, gevent_num, "SMI Flag")) + break; + __gpio_setbits32(gpio, INT_SCI_SMI_MASK, control); program_smi(control_flags & FLAGS_TRIGGER_MASK, gevent_num); @@ -241,6 +250,8 @@ /* Can't set SCI flags from PSP */ if (!can_set_smi_flags) break; + if (gevent_missing(gpio, gevent_num, "SCI Flag")) + break; __gpio_setbits32(gpio, INT_SCI_SMI_MASK, control); fill_sci_trigger(control_flags & FLAGS_TRIGGER_MASK, gevent_num, @@ -267,7 +278,8 @@ master_switch_set(GPIO_INTERRUPT_EN); /* Set all SCI trigger polarity (high/low) and level (edge/level). */ - set_sci_trigger(&sci_trigger_cfg); + if (can_set_smi_flags) + set_sci_trigger(&sci_trigger_cfg); } int gpio_interrupt_status(gpio_t gpio) -- To view, visit https://review.coreboot.org/c/coreboot/+/42809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65efde0a9d347db77c6518c2b988eb29718e778b Gerrit-Change-Number: 42809 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: newchange

4 10

Change in coreboot[master]: soc/amd/picasso: add psp_verstage
by Martin Roth (Code Review) 08 Jul '20

08 Jul '20

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41816 ) Change subject: soc/amd/picasso: add psp_verstage ...................................................................... Patch Set 12: (1 comment) https://review.coreboot.org/c/coreboot/+/41816/10/src/soc/amd/picasso/memla… File src/soc/amd/picasso/memlayout_psp_verstage.ld: https://review.coreboot.org/c/coreboot/+/41816/10/src/soc/amd/picasso/memla… PS10, Line 55: VERSTAGE_START + VERSTAGE_SIZE > Sorry, I missed this comment until just now. […] Fixed. -- To view, visit https://review.coreboot.org/c/coreboot/+/41816 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia58839caa5bfbae0408702ee8d02ef482f2861c4 Gerrit-Change-Number: 41816 Gerrit-PatchSet: 12 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Eric Peers <epeers(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 08 Jul 2020 17:57:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2020