coreboot-gerrit February 2020

coreboot-gerrit@coreboot.org

1 participants
1796 discussions

Change in coreboot[master]: soc/intel/tigerlake: Add code for early tcss
by Brandon Breitenstein (Code Review) 05 Feb '20

05 Feb '20

Brandon Breitenstein has uploaded a new patch set (#17) to the change originally created by Shaunak Saha. ( https://review.coreboot.org/c/coreboot/+/37870 ) Change subject: soc/intel/tigerlake: Add code for early tcss ...................................................................... soc/intel/tigerlake: Add code for early tcss In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port. BUG=b:141608957 BRANCH=NONE TEST: built and booted TGL U RVP Signed-off-by: Shaunak Saha <shaunak.saha(a)intel.com> Change-Id: I45c3fe9d4a2ec2f2f51b78cca2bd7e623540c00e Signed-off-by: Brandon Breitenstein <brandon.breitenstein(a)intel.com> --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c 3 files changed, 181 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/37870/17 -- To view, visit https://review.coreboot.org/c/coreboot/+/37870 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I45c3fe9d4a2ec2f2f51b78cca2bd7e623540c00e Gerrit-Change-Number: 37870 Gerrit-PatchSet: 17 Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: caveh jalali <caveh(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-CC: Vijay P Hiremath <vijay.p.hiremath(a)intel.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: libpayload: Make pci and endian handling -Wconversion safe
by Patrick Georgi (Code Review) 05 Feb '20

05 Feb '20

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37202 ) Change subject: libpayload: Make pci and endian handling -Wconversion safe ...................................................................... libpayload: Make pci and endian handling -Wconversion safe Change-Id: Ibd1b179d647f105579bd74b071344668ca0a41ef Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- M payloads/libpayload/include/endian.h M payloads/libpayload/include/pci.h M payloads/libpayload/libpci/libpci.c 3 files changed, 32 insertions(+), 30 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/37202/1 diff --git a/payloads/libpayload/include/endian.h b/payloads/libpayload/include/endian.h index b387e66..b6b5322 100644 --- a/payloads/libpayload/include/endian.h +++ b/payloads/libpayload/include/endian.h @@ -36,12 +36,12 @@ static inline uint16_t swap_bytes16(uint16_t in) { - return ((in & 0xFF) << 8) | ((in & 0xFF00) >> 8); + return (uint16_t)((in & 0xFF) << 8) | ((in & 0xFF00) >> 8); } static inline uint32_t swap_bytes32(uint32_t in) { - return ((in & 0xFF) << 24) | ((in & 0xFF00) << 8) | + return (uint16_t)((in & 0xFF) << 24) | ((in & 0xFF00) << 8) | ((in & 0xFF0000) >> 8) | ((in & 0xFF000000) >> 24); } @@ -103,28 +103,30 @@ { uint8_t const *p = (uint8_t const *)pp; - return ((p[0] << 8) | p[1]); + return (uint16_t)((p[0] << 8) | p[1]); } static inline uint32_t be32dec(const void *pp) { uint8_t const *p = (uint8_t const *)pp; - return (((unsigned)p[0] << 24) | (p[1] << 16) | (p[2] << 8) | p[3]); + return (((uint32_t)p[0] << 24) | (uint32_t)(p[1] << 16) | + (uint32_t)(p[2] << 8) | p[3]); } static inline uint16_t le16dec(const void *pp) { uint8_t const *p = (uint8_t const *)pp; - return ((p[1] << 8) | p[0]); + return (uint16_t)((p[1] << 8) | p[0]); } static inline uint32_t le32dec(const void *pp) { uint8_t const *p = (uint8_t const *)pp; - return ((p[3] << 24) | (p[2] << 16) | (p[1] << 8) | p[0]); + return ((uint32_t)(p[3] << 24) | (uint32_t)(p[2] << 16) | + (uint32_t)(p[1] << 8) | p[0]); } static inline void bebitenc(void *pp, uint32_t u, uint8_t b) diff --git a/payloads/libpayload/include/pci.h b/payloads/libpayload/include/pci.h index ff07d5c..a1bac4a 100644 --- a/payloads/libpayload/include/pci.h +++ b/payloads/libpayload/include/pci.h @@ -91,11 +91,11 @@ #define HEADER_TYPE_CARDBUS 2 #define HEADER_TYPE_MULTIFUNCTION 0x80 -#define PCI_ADDR(_bus, _dev, _fn, _reg) \ -(0x80000000 | (_bus << 16) | (_dev << 11) | (_fn << 8) | (_reg & ~3)) +#define PCI_DEV(_bus, _dev, _fn) (0x80000000 | \ +(uint32_t)(_bus << 16) | (uint32_t)(_dev << 11) | (uint32_t)(_fn << 8)) -#define PCI_DEV(_bus, _dev, _fn) \ -(0x80000000 | (_bus << 16) | (_dev << 11) | (_fn << 8)) +#define PCI_ADDR(_bus, _dev, _fn, _reg) \ +(PCI_DEV(_bus, _dev, _fn) | (uint8_t)(_reg & ~3)) #define PCI_BUS(_d) ((_d >> 16) & 0xff) #define PCI_SLOT(_d) ((_d >> 11) & 0x1f) diff --git a/payloads/libpayload/libpci/libpci.c b/payloads/libpayload/libpci/libpci.c index 82203a1..fd0332e 100644 --- a/payloads/libpayload/libpci/libpci.c +++ b/payloads/libpayload/libpci/libpci.c @@ -40,34 +40,34 @@ /* libpci interface */ u8 pci_read_byte(struct pci_dev *dev, int pos) { - return pci_read_config8(libpci_to_lb(dev), pos); + return pci_read_config8(libpci_to_lb(dev), (uint16_t)pos); } u16 pci_read_word(struct pci_dev *dev, int pos) { - return pci_read_config16(libpci_to_lb(dev), pos); + return pci_read_config16(libpci_to_lb(dev), (uint16_t)pos); } u32 pci_read_long(struct pci_dev *dev, int pos) { - return pci_read_config32(libpci_to_lb(dev), pos); + return pci_read_config32(libpci_to_lb(dev), (uint16_t)pos); } int pci_write_byte(struct pci_dev *dev, int pos, u8 data) { - pci_write_config8(libpci_to_lb(dev), pos, data); + pci_write_config8(libpci_to_lb(dev), (uint16_t)pos, data); return 1; /* success */ } int pci_write_word(struct pci_dev *dev, int pos, u16 data) { - pci_write_config16(libpci_to_lb(dev), pos, data); + pci_write_config16(libpci_to_lb(dev), (uint16_t)pos, data); return 1; /* success */ } int pci_write_long(struct pci_dev *dev, int pos, u32 data) { - pci_write_config32(libpci_to_lb(dev), pos, data); + pci_write_config32(libpci_to_lb(dev), (uint16_t)pos, data); return 1; /* success */ } @@ -110,29 +110,29 @@ char *funcp = strrchr(id, '.'); if (funcp) { - filter->func = strtoul(funcp+1, &endptr, 0); + filter->func = strtol(funcp+1, &endptr, 0); if (endptr[0] != '\0') return invalid_pci_device_string; } char *devp = strrchr(id, ':'); if (!devp) { - filter->dev = strtoul(id, &endptr, 0); + filter->dev = strtol(id, &endptr, 0); } else { - filter->dev = strtoul(devp+1, &endptr, 0); + filter->dev = strtol(devp+1, &endptr, 0); } if (endptr != funcp) return invalid_pci_device_string; if (!devp) return NULL; char *busp = strchr(id, ':'); if (busp == devp) { - filter->bus = strtoul(id, &endptr, 0); + filter->bus = strtol(id, &endptr, 0); } else { - filter->bus = strtoul(busp+1, &endptr, 0); + filter->bus = strtol(busp+1, &endptr, 0); } if (endptr != funcp) return invalid_pci_device_string; if (busp == devp) return NULL; - filter->domain = strtoul(id, &endptr, 0); + filter->domain = strtol(id, &endptr, 0); if (endptr != busp) return invalid_pci_device_string; return NULL; @@ -155,15 +155,15 @@ return 1; } -static struct pci_dev *pci_scan_single_bus(struct pci_dev *dev, int bus) +static struct pci_dev *pci_scan_single_bus(struct pci_dev *dev, uint8_t bus) { int devfn; u32 val; unsigned char hdr; for (devfn = 0; devfn < 0x100; devfn++) { - int func = devfn & 0x7; - int slot = (devfn >> 3) & 0x1f; + uint8_t func = devfn & 0x7; + uint8_t slot = (devfn >> 3) & 0x1f; val = pci_read_config32(PCI_DEV(bus, slot, func), REG_VENDOR_ID); @@ -179,7 +179,7 @@ dev->dev = slot; dev->func = func; dev->vendor_id = val & 0xffff; - dev->device_id = val >> 16; + dev->device_id = (uint16_t)(val >> 16); dev->next = 0; hdr = pci_read_config8(PCI_DEV(bus, slot, func), @@ -187,10 +187,10 @@ hdr &= 0x7F; if (hdr == HEADER_TYPE_BRIDGE || hdr == HEADER_TYPE_CARDBUS) { - unsigned int busses; - busses = pci_read_config32(PCI_DEV(bus, slot, func), - REG_PRIMARY_BUS); - busses = (busses >> 8) & 0xFF; + uint8_t busses; + busses = (uint8_t)(pci_read_config32( + PCI_DEV(bus, slot, func), + REG_PRIMARY_BUS) >> 8); /* Avoid recursion if the new bus is the same as * the old bus (insert lame The Who joke here) */ -- To view, visit https://review.coreboot.org/c/coreboot/+/37202 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibd1b179d647f105579bd74b071344668ca0a41ef Gerrit-Change-Number: 37202 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

5 8

Change in coreboot[master]: HACK trogdor: optimize coreboot.rom for T32 flash script HACK
by mturney mturney (Code Review) 05 Feb '20

05 Feb '20

Hello Aaron Durbin, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/36278 to look at the new patch set (#29). Change subject: HACK trogdor: optimize coreboot.rom for T32 flash script HACK ...................................................................... HACK trogdor: optimize coreboot.rom for T32 flash script HACK Change-Id: I5293ac9365bf4ac74bc475e70a02062f5371f9b8 Signed-off-by: T Michael Turney <mturney(a)codeaurora.org> --- M src/security/vboot/Makefile.inc A util/qualcomm/optimize_coreboot 2 files changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/36278/29 -- To view, visit https://review.coreboot.org/c/coreboot/+/36278 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5293ac9365bf4ac74bc475e70a02062f5371f9b8 Gerrit-Change-Number: 36278 Gerrit-PatchSet: 29 Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: HACK trogdor: SoC makefile BLOB support HACK
by mturney mturney (Code Review) 05 Feb '20

05 Feb '20

Hello ashk(a)codeaurora.org, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/35508 to look at the new patch set (#40). Change subject: HACK trogdor: SoC makefile BLOB support HACK ...................................................................... HACK trogdor: SoC makefile BLOB support HACK Change-Id: I85a20ef31ec91c6f22221d16fd4c3097c5cb97d1 Signed-off-by: Ashwin Kumar <ashk(a)codeaurora.org> --- M src/soc/qualcomm/sc7180/Makefile.inc 1 file changed, 119 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/35508/40 -- To view, visit https://review.coreboot.org/c/coreboot/+/35508 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I85a20ef31ec91c6f22221d16fd4c3097c5cb97d1 Gerrit-Change-Number: 35508 Gerrit-PatchSet: 40 Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: ashk(a)codeaurora.org Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by build bot (Jenkins) (Code Review) 05 Feb '20

05 Feb '20

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21774 ) Change subject: mb/dell: Add Dell Optiplex 790 ...................................................................... Patch Set 63: (1 comment) https://review.coreboot.org/c/coreboot/+/21774/63/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/gpio.c: https://review.coreboot.org/c/coreboot/+/21774/63/src/mainboard/dell/optipl… PS63, Line 110: .gpio39 = GPIO_MODE_GPIO, /* A_FP_PRES# */ 'PRES' may be misspelled - perhaps 'PRESS'? -- To view, visit https://review.coreboot.org/c/coreboot/+/21774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Gerrit-Change-Number: 21774 Gerrit-PatchSet: 63 Gerrit-Owner: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 05 Feb 2020 19:43:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by Angel Pons (Code Review) 05 Feb '20

05 Feb '20

Angel Pons has uploaded a new patch set (#63) to the change originally created by Christoph Pomaska. ( https://review.coreboot.org/c/coreboot/+/21774 ) Change subject: mb/dell: Add Dell Optiplex 790 ...................................................................... mb/dell: Add Dell Optiplex 790 This port was generated using autoport. The Optiplex 790 comes in four different sizes: MT, DT, SFF and USFF. The mainboard PCBs for MT and DT are identical, the only difference is that MT has an extra SATA port. The other PCBs are slightly different, but they are designed with intercompatibility in mind. Therefore, the four mainboard configurations are: - MT: 4 RAM slots, 4x SATA, PEG, PCIe x1, PCI, PCIe x16 - DT: 4 RAM slots, 3x SATA, PEG, PCIe x1, PCI, PCIe x16 - SFF: 4 RAM slots, 3x SATA, PEG, PCIe x16 - USFF: 2 RAM slots, 2x SATA, mPCIe This port has been tested with: SFF (needs retest) , USFF What works: - Booting Arch Linux with kernel 5.2.3-arch1-1-ARCH - PCIe (Hotplug on PCIe X1 only!) - Onboard graphics with libgfxinit (via VGA) - Hard drive activity LED - Onboard sound - Onboard Ethernet - SATA (hotplug) - IOMMU - Turning on and off (S5) - Suspend/resume (S3) - EHCI debug (rear side, bottom port on the block with ethernet) What does not work: - SuperI/O (Chip is a SCH5544-NS) - Serial port - PS/2 - Fan control (fans run at full speed) - Front LEDs (signals code 234, corrupted or defective BIOS) [1] - VBT is missing Untested: - DisplayPort? - VGA BIOS? Further notes: - Default IFD settings block reads/writes to some regions. This can be bypassed by plugging the SERVICE_MODE jumper. BIOS version A05 does not set any protected ranges, so internal flashing is possible (use a layout if the SERVICE_MODE jumper is not plugged). - The mainboard has two flash chips: SOIC16 8 MiB and SOIC8 2 MiB. They are recognized as one "Opaque flash chip" of 10 MiB in size. [1] http://www.dell.com/support/article/us/en/04/sln284978/a-reference-guide-to… Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Signed-off-by: Christoph Pomaska <github(a)aufmachen.jetzt> Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- A Documentation/mainboard/dell/optiplex_790.md A Documentation/mainboard/dell/servicemode.md M Documentation/mainboard/index.md A src/mainboard/dell/Kconfig A src/mainboard/dell/Kconfig.name A src/mainboard/dell/optiplex_790/Kconfig A src/mainboard/dell/optiplex_790/Kconfig.name A src/mainboard/dell/optiplex_790/Makefile.inc A src/mainboard/dell/optiplex_790/acpi/ec.asl A src/mainboard/dell/optiplex_790/acpi/platform.asl A src/mainboard/dell/optiplex_790/acpi/superio.asl A src/mainboard/dell/optiplex_790/acpi_tables.c A src/mainboard/dell/optiplex_790/board_info.txt A src/mainboard/dell/optiplex_790/devicetree.cb A src/mainboard/dell/optiplex_790/dsdt.asl A src/mainboard/dell/optiplex_790/early_init.c A src/mainboard/dell/optiplex_790/gma-mainboard.ads A src/mainboard/dell/optiplex_790/gpio.c A src/mainboard/dell/optiplex_790/hda_verb.c A src/mainboard/dell/optiplex_790/mainboard.c A src/mainboard/dell/optiplex_790/variants/mt_dt/overridetree.cb A src/mainboard/dell/optiplex_790/variants/sff/overridetree.cb A src/mainboard/dell/optiplex_790/variants/usff/overridetree.cb 23 files changed, 823 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/21774/63 -- To view, visit https://review.coreboot.org/c/coreboot/+/21774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Gerrit-Change-Number: 21774 Gerrit-PatchSet: 63 Gerrit-Owner: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/cse: Add boot partition related APIs
by Sridhar Siricilla (Code Review) 05 Feb '20

05 Feb '20

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35402 ) Change subject: soc/intel/common/block/cse: Add boot partition related APIs ...................................................................... Patch Set 60: (36 comments) https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… File src/soc/intel/common/block/cse/cse_bp.c: https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 136: Info > It's correct as command name is SET BOOT PARTITION INFO, Done https://review.coreboot.org/c/coreboot/+/35402/59/src/soc/intel/common/bloc… File src/soc/intel/common/block/cse/cse_bp.c: https://review.coreboot.org/c/coreboot/+/35402/59/src/soc/intel/common/bloc… PS59, Line 65: ; > Thanks..I missed it .. Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… File src/soc/intel/common/block/include/intelblocks/cse.h: https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 171: layout > Can you also add the graphic you put in commit message here? I think that is very helpful in underst […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 172: > nit: Just use spaces instead of tab? Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 173: > nit: same here Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 179: typedef > Instead of typedef, just use enum boot_partition_id { […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 180: R > nit: lowercase 'r'. Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 184: CSME > I know it's referring to the same entity, but for consistency, let's use CSE everywhere. Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 184: M > nit: lowercase 'm'. Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 193: */ > It would be good to add a sentence indicating that this is returned in response to MKHI command for […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 194: typedef > same comment as above about typedef. Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 195: > nit: period (.) at the end. Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 199: shall > nit: should? Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 210: This value isn't applicable to chrome platforms > This is a common driver code file. […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 214: BP_STATUS_MANIFEST_AND_INFO_OK > What is the difference between BP_STATUS_SUCCESS and BP_STATUS_MANIFEST_AND_INFO_OK? Does BP_STATUS_ […] BP_STATUS_MANIFEST_AND_INFO_OK is no longer valid status code. I removed it. https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 232: RO > nit: RO(BP1) Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 234: 2 > nit: (1 << 1). I think it makes it easier to read. Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 239: relavant > nit: relevant Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 244: ME RW Status > What does this indicate? Is this returned in response to some command? This status is not returned in response to any command. Basically CSE FW update library determines the status based on boot partition status and boot partition's signature. https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 253: ME_RW_SIGN_INVALID > What is the difference between ME_RW_INVALID and ME_RW_SIGN_INVALID? How is the difference determine […] ME_RW_INVALID/VALID are determined based on the boot partition status (BP_STATUS_GENERAL_FAILURE) Whereas ME_RW_SIGN_INVALID status will be indicated when there is partial update due to power failure/any reason. https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 257: partition_fw_version > Shouldn't this be cse_fw_version? I don't think it is really tied to boot partition. E.g. […] BP2 and BP3 always have same versions.But RO(BP1) and RW(BP2/BP3) can have different versions. By the way, no BP3 in the latest CSE FW. Although, I'm not convinced to name it as cse_fw_version.Thanks https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 267: CSE Reset > I am not sure how CSE reset works okay in this case. […] Yes, GLOBAL RESET(GLOBAL_RESET_TYPE) is must to take effect of CSE's boot partition change. DEVIATION: During CSE FW update, coreboot can notify CSE to boot from RO(BP1), and shall issue GLOBAL_RESET(CSE Reset Only) command.This case should be used during CSE FW Upgrade, and coreboot should issue GLOBAL_RESET(GLOBAL_RESET_TYPE) command after CSE FW update. https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 270: Returns false on failure and true on success. > I think this is fine, but I would really like the CSE functions to have a consistent scheme for retu […] I agree with you on having common scheme for return values. Thanks https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 272: uint8_t > Use "enum boot_partition_id" instead? Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 278: uint8_t > enum boot_partition_id Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 284: uint8_t > enum boot_partition_id Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 290: uint8_t > enum bp_info_flags API returns member field "flags" of cse_boot_partition_info struct. https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 296: uint8_t > enum boot_partition_id Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 296: uint8_t > enum boot_partition_status Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 302: uint8_t > enum boot_partition_id Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 308: uint8_t > enum boot_partition_id Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 308: start_end_offset > range? Just to keep the name shorter? Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 313: enabled to support CSE FW Update > Why? Shouldn't it be redudancy and not min recovery? In the latest CSE FW, this feature is not a selectable setting(or option). By default, it is enabled. Please note min recovery feature is relevant only if redundancy is enabled. RO also must support minimum recovery mode to support FW update. https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 313: must > nit: must not required here. Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 316: bool > Can return value be used to indicate true = enabled, false = disabled/error? Error and disabled cases are handled differently in CSE FW Update library so modifying return values of the API is not required. https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 328: bool > Why is a return value required for debug/print function? Done -- To view, visit https://review.coreboot.org/c/coreboot/+/35402 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f Gerrit-Change-Number: 35402 Gerrit-PatchSet: 60 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrey Petrov <anpetrov(a)fb.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 05 Feb 2020 19:31:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: 3rdparty/blobs: Update to include STM binary
by Patrick Georgi (Code Review) 05 Feb '20

05 Feb '20

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38717 ) Change subject: 3rdparty/blobs: Update to include STM binary ...................................................................... 3rdparty/blobs: Update to include STM binary Change-Id: I5f053c1270bab71aeab3bb785c60417419736b44 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- M 3rdparty/blobs 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/38717/1 diff --git a/3rdparty/blobs b/3rdparty/blobs index 034b278..7ad2d22 160000 --- a/3rdparty/blobs +++ b/3rdparty/blobs @@ -1 +1 @@ -Subproject commit 034b27818450428f70aa9316c8bd0d65bacd8ee8 +Subproject commit 7ad2d22452225a14c19b17570cb77920d8fc81a5 -- To view, visit https://review.coreboot.org/c/coreboot/+/38717 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f053c1270bab71aeab3bb785c60417419736b44 Gerrit-Change-Number: 38717 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

5 5

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by Angel Pons (Code Review) 05 Feb '20

05 Feb '20

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21774 ) Change subject: mb/dell: Add Dell Optiplex 790 ...................................................................... Patch Set 62: (26 comments) https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG@11 PS60, Line 11: There are (at least) three different mainboards: > The Optiplex 790 comes in four different sizes: MT, DT, SFF and USFF. […] Done https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG@13 PS60, Line 13: - DT: 4 RAM slots, PEG, PCIe x1, PCI, PCIe x16 > Add the SATA port info here: […] Ack (and USFF has 2x SATA) https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG@18 PS60, Line 18: The variants have different PCI/PCIe configurations and are not > The three mainboard types have different PCI/PCIe configurations, so they need to be different varia […] Ack https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG@21 PS60, Line 21: This port has been tested with: SFF, USFF > need to test DT/MT, I think? I should retest SFF as well Ack https://review.coreboot.org/c/coreboot/+/21774/62//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/21774/62//COMMIT_MSG@23 PS62, Line 23: SFF (needs retest) , USFF FIXME https://review.coreboot.org/c/coreboot/+/21774/62//COMMIT_MSG@48 PS62, Line 48: DisplayPort? FIXME https://review.coreboot.org/c/coreboot/+/21774/62//COMMIT_MSG@49 PS62, Line 49: VGA BIOS? FIXME https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/Kconfig.name: PS59: > nit: split the three form factors out, to have prettier mainboard part numbers Had to do it anyway to correctly handle PCI/PCIe https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/dsdt.asl: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 36: #include <southbridge/intel/bd82x6x/acpi/sleepstates.asl> > See https://github.com/coreboot/coreboot/blob/master/src/mainboard/gigabyte/ga-…. […] Corrected the includes. https://review.coreboot.org/c/coreboot/+/21774/62/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/gpio.c: https://review.coreboot.org/c/coreboot/+/21774/62/src/mainboard/dell/optipl… PS62, Line 110: .gpio39 = GPIO_MODE_GPIO, /* A_FP_PRES# */ > 'PRES' may be misspelled - perhaps 'PRESS'? nope, it means PRESence https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/mainboard.c: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 13: > This file is only used with the VBIOS anyway. […] Removed the empty space, VBIOS still needs testing https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/romstage.c: https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… PS60, Line 17: : #include <stdint.h> : #include <string.h> : #include <timestamp.h> : #include <arch/byteorder.h> : #include <arch/io.h> : #include <device/mmio.h> : #include <device/pci_ops.h> : #include <device/pnp_ops.h> : #include <cpu/x86/lapic.h> : #include <console/console.h> : #include <northbridge/intel/sandybridge/sandybridge.h> : #include <northbridge/intel/sandybridge/raminit_native.h> : #include <southbridge/intel/bd82x6x/pch.h> : #include <southbridge/intel/common/gpio.h> > include only what you use. […] Done, I think? https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/variants/optiplex_790_mt-sff/devicetree.cb: PS59: > use overridetrees Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 50: # FIXME: What can be hotplugged? : register "pcie_hotplug_map" = "{ 0, 0, 0, 0, 0, 0, 0, 0 }" : register "pcie_port_coalesce" = "0" > The hotplugability worked fine on vendor firmware. In any case, the hotpluggability ended up landing on USFF, which does not have PCIe x1. Moved it to MT/DT. https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 54: 0x4 > This is what makes SATA not work on USFF Enabled all 4 SATA ports that all variants used. https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/variants/optiplex_790_mt-sff/hda_verb.c: PS59: > Fix this mess, as done with other boards. Done https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/variants/optiplex_790_mt-sff/hda_verb.c: https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… PS60, Line 24: 0x0000000b > should be decimal Done https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… PS60, Line 25: /* NID 0x01: Subsystem ID. */ > All the "NID" comments are of no use they went poof https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… PS60, Line 26: 0x0 > decimal too Done https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… PS60, Line 57: 0x10ec0887, /* Codec Vendor / Device ID: Realtek */ > Pack the AZALIA_PIN_CFG macros together, and leave some space between codecs Done https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… PS60, Line 60: 0x0000000f > decimal it went poof https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… PS60, Line 62: 0x2 > decimal it went poof https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… PS60, Line 108: 0x00000004 > decimal Done https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… PS60, Line 110: 0x3 > decimal Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/variants/optiplex_790_usff/devicetree.cb: PS59: > See the other devicetree it went poof https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/variants/optiplex_790_usff/hda_verb.c: PS59: > See the other file. Also, compare for differences. it went poof -- To view, visit https://review.coreboot.org/c/coreboot/+/21774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Gerrit-Change-Number: 21774 Gerrit-PatchSet: 62 Gerrit-Owner: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 05 Feb 2020 19:12:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Christoph Pomaska <github(a)slrie.de> Comment-In-Reply-To: HAOUAS Elyes <ehaouas(a)noos.fr> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by build bot (Jenkins) (Code Review) 05 Feb '20

05 Feb '20

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21774 ) Change subject: mb/dell: Add Dell Optiplex 790 ...................................................................... Patch Set 62: (1 comment) https://review.coreboot.org/c/coreboot/+/21774/62/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/gpio.c: https://review.coreboot.org/c/coreboot/+/21774/62/src/mainboard/dell/optipl… PS62, Line 110: .gpio39 = GPIO_MODE_GPIO, /* A_FP_PRES# */ 'PRES' may be misspelled - perhaps 'PRESS'? -- To view, visit https://review.coreboot.org/c/coreboot/+/21774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Gerrit-Change-Number: 21774 Gerrit-PatchSet: 62 Gerrit-Owner: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 05 Feb 2020 19:03:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2020