coreboot-gerrit December 2020

coreboot-gerrit@coreboot.org

1 participants
3165 discussions

Change in coreboot[master]: drivers/ipmi: Handle the condition when (dev->chip_info == NULL)
by Bill XIE (Code Review) 12 Dec '20

12 Dec '20

Bill XIE has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47387 ) Change subject: drivers/ipmi: Handle the condition when (dev->chip_info == NULL) ...................................................................... drivers/ipmi: Handle the condition when (dev->chip_info == NULL) Some former commits (e.g. Ieb41771c75aae902191bba5d220796e6c343f8e0) blindly assume that dev->chip_info is capable to be dereferenced, making at least compilers complain about potential null pointer dereference. They might cause crash if truly (dev->chip_info == NULL). Their code should be adjusted to be runnable even if dev->chip_info is NULL. Signed-off-by: Bill XIE <persmule(a)hardenedlinux.org> Change-Id: I1d694b12f6c42961c104fe839d4ee46c0f111197 --- M src/drivers/ipmi/ipmi_kcs_ops.c 1 file changed, 14 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/47387/1 diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c index 362f17a..3af44ef 100644 --- a/src/drivers/ipmi/ipmi_kcs_ops.c +++ b/src/drivers/ipmi/ipmi_kcs_ops.c @@ -77,8 +77,9 @@ struct ipmi_devid_rsp rsp; uint32_t man_id = 0, prod_id = 0; struct drivers_ipmi_config *conf = NULL; - struct ipmi_selftest_rsp selftestrsp; + struct ipmi_selftest_rsp selftestrsp = {{ 0, 0, 0 }, 0, 0}; uint8_t retry_count; + uint16_t bmc_boot_timeout = 0; if (!dev->enabled) return; @@ -90,8 +91,9 @@ /* Get IPMI version for ACPI and SMBIOS */ if (conf && conf->wait_for_bmc && conf->bmc_boot_timeout) { + bmc_boot_timeout = conf->bmc_boot_timeout; struct stopwatch sw; - stopwatch_init_msecs_expire(&sw, conf->bmc_boot_timeout * 1000); + stopwatch_init_msecs_expire(&sw, bmc_boot_timeout * 1000); printk(BIOS_INFO, "IPMI: Waiting for BMC...\n"); while (!stopwatch_expired(&sw)) { @@ -108,7 +110,7 @@ } printk(BIOS_INFO, "Get BMC self test result..."); - for (retry_count = 0; retry_count < conf->bmc_boot_timeout; retry_count++) { + for (retry_count = 0; retry_count < bmc_boot_timeout; retry_count++) { if (!ipmi_get_bmc_self_test_result(dev, &selftestrsp)) break; @@ -175,6 +177,7 @@ struct acpi_rsdp *rsdp) { struct drivers_ipmi_config *conf = NULL; + uint32_t uid = 0; struct acpi_spmi *spmi; s8 gpe_interrupt = -1; u32 apic_interrupt = 0; @@ -214,11 +217,12 @@ apic_interrupt = conf->apic_interrupt; } - /* Use command to get UID from ipmi_ssdt */ + if (uid_cnt > 0) + uid = uid_cnt - 1; acpi_create_ipmi(dev, spmi, (ipmi_revision_major << 8) | (ipmi_revision_minor << 4), &addr, IPMI_INTERFACE_KCS, gpe_interrupt, apic_interrupt, - conf->uid); + uid); acpi_add_table(rsdp, spmi); @@ -241,15 +245,12 @@ if (dev->chip_info) conf = dev->chip_info; - /* Use command to pass UID to ipmi_write_acpi_tables */ - conf->uid = uid_cnt++; - /* write SPMI device */ acpigen_write_scope(scope); acpigen_write_device("SPMI"); acpigen_write_name_string("_HID", "IPI0001"); acpigen_write_name_unicode("_STR", "IPMI_KCS"); - acpigen_write_name_byte("_UID", conf->uid); + acpigen_write_name_byte("_UID", uid_cnt); acpigen_write_STA(0xf); acpigen_write_name("_CRS"); acpigen_write_resourcetemplate_header(); @@ -261,8 +262,12 @@ // FIXME: is that correct? if (conf->have_apic) acpigen_write_irq(1 << conf->apic_interrupt); + + conf->uid = uid_cnt; } + uid_cnt++; + acpigen_write_resourcetemplate_footer(); acpigen_write_method("_IFT", 0); -- To view, visit https://review.coreboot.org/c/coreboot/+/47387 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1d694b12f6c42961c104fe839d4ee46c0f111197 Gerrit-Change-Number: 47387 Gerrit-PatchSet: 1 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-MessageType: newchange

4 12

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Michael Niewöhner (Code Review) 12 Dec '20

12 Dec '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47984 ) Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 12 Dec 2020 00:45:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P)
by Michael Niewöhner (Code Review) 12 Dec '20

12 Dec '20

1 0

Change in coreboot[master]: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P)
by Angel Pons (Code Review) 12 Dec '20

12 Dec '20

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38978 ) Change subject: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P) ...................................................................... Patch Set 10: (1 comment) https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… File src/mainboard/acer/es1-572/gpio_early.c: https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… PS10, Line 6: /* Name format: <pad name> / <net/pin name in schematics> */ : static const struct pad_config early_gpio_table[] = { : PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2_RXD / UART_2_CRXD_DTXD */ : PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2_TXD / UART_2_CTXD_DRXD */ : }; > the pads default to GPI Yeah, but there's code to set the pads to NF1 in soc. That's why early logs worked -- To view, visit https://review.coreboot.org/c/coreboot/+/38978 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id98788a2c5e54f70fd1cacbd70d636f5e63b2619 Gerrit-Change-Number: 38978 Gerrit-PatchSet: 10 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Comment-Date: Sat, 12 Dec 2020 00:39:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P)
by Michael Niewöhner (Code Review) 12 Dec '20

12 Dec '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38978 ) Change subject: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P) ...................................................................... Patch Set 10: (4 comments) https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… File src/mainboard/acer/es1-572/acpi/mainboard.asl: https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… PS10, Line 3: PWRB > No, PWRB isn't needed. I wasn't sure. Some boards add it. the todo was more like "find out if it's needed" https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… File src/mainboard/acer/es1-572/gma-mainboard.ads: https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… PS10, Line 1: only > License changed from GPL-2.0-or-later to GPL-2. […] ack, will change that in the new change https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… File src/mainboard/acer/es1-572/gpio.c: https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… PS10, Line 191: void mainboard_configure_gpios(void) > This means GPIOs are programmed after FSP now? yes https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… File src/mainboard/acer/es1-572/gpio_early.c: https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… PS10, Line 6: /* Name format: <pad name> / <net/pin name in schematics> */ : static const struct pad_config early_gpio_table[] = { : PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2_RXD / UART_2_CRXD_DTXD */ : PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2_TXD / UART_2_CTXD_DRXD */ : }; > Are you sure this is required? the pads default to GPI -- To view, visit https://review.coreboot.org/c/coreboot/+/38978 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id98788a2c5e54f70fd1cacbd70d636f5e63b2619 Gerrit-Change-Number: 38978 Gerrit-PatchSet: 10 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Comment-Date: Fri, 11 Dec 2020 23:38:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P)
by Angel Pons (Code Review) 12 Dec '20

12 Dec '20

1 0

Change in coreboot[master]: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P)
by Angel Pons (Code Review) 12 Dec '20

12 Dec '20

Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/38978 ) Change subject: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P) ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/38978 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id98788a2c5e54f70fd1cacbd70d636f5e63b2619 Gerrit-Change-Number: 38978 Gerrit-PatchSet: 10 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-MessageType: abandon

1 0

Change in coreboot[master]: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P)
by Angel Pons (Code Review) 11 Dec '20

11 Dec '20

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38978 ) Change subject: mb/acer: add Aspire ES1-572 and Extensa 2540 (Compal LA-E061P) ...................................................................... Patch Set 10: (4 comments) https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… File src/mainboard/acer/es1-572/acpi/mainboard.asl: https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… PS10, Line 3: PWRB No, PWRB isn't needed. https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… File src/mainboard/acer/es1-572/gma-mainboard.ads: https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… PS10, Line 1: only Um? https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… File src/mainboard/acer/es1-572/gpio.c: https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… PS10, Line 191: void mainboard_configure_gpios(void) This means GPIOs are programmed after FSP now? https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… File src/mainboard/acer/es1-572/gpio_early.c: https://review.coreboot.org/c/coreboot/+/38978/10/src/mainboard/acer/es1-57… PS10, Line 6: /* Name format: <pad name> / <net/pin name in schematics> */ : static const struct pad_config early_gpio_table[] = { : PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2_RXD / UART_2_CRXD_DTXD */ : PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2_TXD / UART_2_CTXD_DRXD */ : }; Are you sure this is required? -- To view, visit https://review.coreboot.org/c/coreboot/+/38978 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id98788a2c5e54f70fd1cacbd70d636f5e63b2619 Gerrit-Change-Number: 38978 Gerrit-PatchSet: 10 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Comment-Date: Fri, 11 Dec 2020 22:31:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Enable CSE Lite driver for TGL platform in romstage
by Tim Wawrzynczak (Code Review) 11 Dec '20

11 Dec '20

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48281 ) Change subject: soc/intel/tigerlake: Enable CSE Lite driver for TGL platform in romstage ...................................................................... Patch Set 5: (2 comments) https://review.coreboot.org/c/coreboot/+/48281/5//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/48281/5//COMMIT_MSG@9 PS5, Line 9: cse_fw_syncO cse_fw_sync https://review.coreboot.org/c/coreboot/+/48281/4/src/soc/intel/tigerlake/ro… File src/soc/intel/tigerlake/romstage/romstage.c: https://review.coreboot.org/c/coreboot/+/48281/4/src/soc/intel/tigerlake/ro… PS4, Line 136: fsp_memory_init(s3wake); : pmc_set_disb(); : if (!s3wake) { : if (CONFIG(SOC_INTEL_CSE_LITE_SKU)) : cse_fw_sync(); > >Yes, from past discussions Intel had mentioned that you cannot run HMRPFO when dram is not initiali […] Got it, the HMRPFO actually makes sense if I think about how it probably works.... the comment is helpful, thanks Sridhar! -- To view, visit https://review.coreboot.org/c/coreboot/+/48281 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib485a4d1d15989b162105deb32bb317d7a0f2856 Gerrit-Change-Number: 48281 Gerrit-PatchSet: 5 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 11 Dec 2020 21:48:10 +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> Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC|WIP] current state of making ipmi gpio generic
by Michael Niewöhner (Code Review) 11 Dec '20

11 Dec '20

Michael Niewöhner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48107 ) Change subject: [RFC|WIP] current state of making ipmi gpio generic ...................................................................... Abandoned see CB:48582 and following -- To view, visit https://review.coreboot.org/c/coreboot/+/48107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I63f604c5a5d82c94e56399e2c37bfeb54927d34f Gerrit-Change-Number: 48107 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: abandon

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2020