coreboot-gerrit February 2020

coreboot-gerrit@coreboot.org

1 participants
1797 discussions

Change in coreboot[master]: soc/intel/common/block/lpc: create LPC_GET_DEV macro
by Nico Huber (Code Review) 17 Feb '20

17 Feb '20

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30209 ) Change subject: soc/intel/common/block/lpc: create LPC_GET_DEV macro ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… File src/soc/intel/common/block/lpc/lpc_lib.c: https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… PS2, Line 29: #if !defined(__SIMPLE_DEVICE__) : #include <device/device.h> : #define LPC_GET_DEV pcidev_on_root(PCH_DEV_SLOT_LPC, 0x0) : #else : #define LPC_GET_DEV PCH_DEV_LPC : #endif > > > > Hence, avoids the need for any #if in platform code. > > > > > > FYI, this is soc code not the platform code. hatch, poppy are the platform code in my understanding. this is soc library. i was adhering old review comments from Arthur who has suggested to have a macro rather function names. > > > > Sorry, I don't know how you call things. To me, a platform in this > > context is chipset + CPU. What I actually meant is #if outside of > > src/include/device/. > > > > About Arthur's comments, they were for patch set 1 which looks much > > differently. Also, the original problem patch set 1 tried to solve > > was fixed in the meantime. There was > > > > #ifdef __SIMPLE_DEVICE__ > > pci_devfn_t dev; > > #else > > struct device *dev; > > #endif > > > > earlier, but isn't anymore. Which explains the commit message, maybe. > > i'm not very sure about that, Arthur's concern is still active herehttps://github.com/coreboot/coreboot/blob/master/src/soc/intel/common/b… Ok, that looks ugly, so? My shot at it: https://review.coreboot.org/c/coreboot/+/38946 > > and i was coping the same into this file, hence he has suggested to use macros rather function Are you sure he meant adding macros or maybe using the existing ones? > > Maybe let's back up and start with the problem you are trying to > > solve? Is there anything you want to add to / change in this file > > that isn't possible because of the `#define __SIMPLE_DEVICE__`? > > problem is having different macros for "dev". > Inside pci_read_config16() function we are using PCH_DEV_LPC and https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… here we are using struct device * > hence user has to have lots of knowledge about different use case inside croeboot, which i was trying to avoid and provide more abstraction I agree, but this has to be handled at a much lower level, in src/device/. The coreboot infrastructure wasn't ready for the amount of code sharing between stages as we have it today in soc/intel/. People added it never- theless, so now we sit on a pile of technical debt. -- To view, visit https://review.coreboot.org/c/coreboot/+/30209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84a6102bf3849e9d4fe28e4c6a11bc7badcf5114 Gerrit-Change-Number: 30209 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> 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: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 17 Feb 2020 17:42:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Documentation/superio: format generic PNP documentation
by Felix Held (Code Review) 17 Feb '20

17 Feb '20

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38519 ) Change subject: Documentation/superio: format generic PNP documentation ...................................................................... Documentation/superio: format generic PNP documentation Change-Id: Id12ec4d5f11f4285a1379cf32a5d0f6cd2ce9e70 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M Documentation/superio/common/pnp.md 1 file changed, 23 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/38519/1 diff --git a/Documentation/superio/common/pnp.md b/Documentation/superio/common/pnp.md index 314cac2..948078f 100644 --- a/Documentation/superio/common/pnp.md +++ b/Documentation/superio/common/pnp.md @@ -15,13 +15,13 @@ Super I/O chips connected via LPC to the southbridge usually have their I/O-mapped configuration interface with a size of two bytes at the base -address 0x2e or 0x4e. Other PNP devices have their configuration +address `0x2e` or `0x4e`. Other PNP devices have their configuration interface at other addresses. The two byte registers allow access to an indirect 256 bytes big -register space that contains the configuration. By writing the index -to the lower byte (e.g. 0x2e), you can access the register contents at -that index by reading/writing the higher byte (e.g. 0x2f). +register space that contains the configuration. By writing the index to +the lower byte (e.g. `0x2e`), you can access the register contents at +that index by reading/writing the higher byte (e.g. `0x2f`). To prevent accidental changes of the Super I/O (SIO) configuration, the SIOs need a configuration mode unlock sequence. After changing the @@ -31,18 +31,18 @@ ## Logical device numbers (LDN) Each PNP device can contain multiple logical devices. The bytes from -0x00 to 0x2f in the indirect configuration register space are common -for all LDNs, but some SIO chips require a certain LDN to be selected -in order to write certain registers in there. An LDN gets selected by -writing the LDN number to the LDN select register 0x07. Registers 0x30 -to 0xFF are specific to each LDN number. +`0x00` to `0x2f` in the indirect configuration register space are common +for all LDNs, but some SIO chips require a certain LDN to be selected in +order to write certain registers in there. An LDN gets selected by +writing the LDN number to the LDN select register `0x07`. Registers +`0x30` to `0xFF` are specific to each LDN number. coreboot encodes the physical LDN number in the lower byte of the LDN number. ### Virtual logical device numbers -Register 0x30 is the LDN enable register and since it is an 8 bit +Register `0x30` is the LDN enable register and since it is an 8 bit register, it can contain up to 8 enable bits for different parts of the functionality of that logical device. To set a certain enable bit in one physical LDN, the concept of virtual LDNs was introduced. @@ -54,7 +54,7 @@ ## I/O resources -Starting at register address 0x60, each LDN has 2 byte wide I/O base +Starting at register address `0x60`, each LDN has 2 byte wide I/O base address registers. The size of an I/O resource is always a power of two. @@ -67,29 +67,29 @@ the resource has N address bits and a size of 2\*\*N bytes. The mask address is also the highest possible address to map the I/O region. -A typical example for an I/O resource mask is 0x07f8 which is -0b0000011111111000 in binary notation. The three LSBs are zeros here, +A typical example for an I/O resource mask is `0x07f8` which is +`0b0000011111111000` in binary notation. The three LSBs are zeros here, so it's an eight byte I/O resource with three address offset bits inside the resource. The highest base address it can be mapped to is -0x07f8, so the region will end at 0x07ff. +`0x07f8`, so the region will end at `0x07ff`. The Super I/O datasheets typically contain the information about the I/O resource masks. On most Super I/O chips the mask can also be found -out by writing 0xffff to the corresponding I/O base address register +out by writing `0xffff` to the corresponding I/O base address register and reading back the value; since the lowest and highest bits are hard-wired to zero according to the I/O resource size and maximal possible I/O address, this gives the mask. ## IRQ resources -Each physical LDN has up to two configurable interrupt request -register pairs 0x70, 0x71 and 0x72, 0x73. Each pair can be configured -to use a certain IRQ number. Writing 1 to 15 into the first register +Each physical LDN has up to two configurable interrupt request register +pairs `0x70`, `0x71` and `0x72`, `0x73`. Each pair can be configured to +use a certain IRQ number. Writing 1 to 15 into the first register selects the IRQ number generated by the corresponding IRQ source and -enables IRQ generation; writing 0 to it disables the generation of -IRQs for the source. The second register selects the IRQ type (level -or edge) and IRQ level (high or low). For LPC SIOs the IRQ type is -hard-wired to edge. +enables IRQ generation; writing 0 to it disables the generation of IRQs +for the source. The second register selects the IRQ type (level or edge) +and IRQ level (high or low). For LPC SIOs the IRQ type is hard-wired to +edge. On the LPC bus a shared SERIRQ line is used to signal IRQs to the host; the IRQ number gets encoded by the number of LPC clock cycles @@ -106,7 +106,7 @@ ## DRQ resources Each physical LDN has two legacy ISA-style DMA request channel -registers at 0x74 and 0x75. Those are only used for legacy devices +registers at `0x74` and `0x75`. Those are only used for legacy devices like parallel printer ports or floppy disk controllers. Each device using LPC legacy DMA needs its own LDMA line to the host. -- To view, visit https://review.coreboot.org/c/coreboot/+/38519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id12ec4d5f11f4285a1379cf32a5d0f6cd2ce9e70 Gerrit-Change-Number: 38519 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

4 12

Change in ...coreboot[master]: mb/lenovo/x1_carbon_gen1: Swap Fn-F2 and Fn-F3
by Nicola Corna (Code Review) 17 Feb '20

17 Feb '20

Nicola Corna has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31470 Change subject: mb/lenovo/x1_carbon_gen1: Swap Fn-F2 and Fn-F3 ...................................................................... mb/lenovo/x1_carbon_gen1: Swap Fn-F2 and Fn-F3 thinkpad_acpi expects the battery hotkey (KEY_BATTERY) on scancode 0x01 (Fn-F2) and the lock hotkey (KEY_COFFEE) on scancode 0x02 (Fn-F3). This is true for most of the Thinkpads, however on the X1 Carbon Gen1 (and possibly others), the battery hotkey is not present and the lock one is instead on Fn-F3. Swap the RHK calls in _Q11 (Fn-F2) and _Q12 (Fn-F3) to fix the issue, so that the lock hotkey is on F3 and the battery one is on F2 (even if it's not marked so). Change-Id: Ib2d96be1a7815d7d03e6e8c6d300fd671c8598ca Signed-off-by: Nicola Corna <nicola(a)corna.info> --- M src/ec/lenovo/h8/acpi/ec.asl M src/mainboard/lenovo/x1_carbon_gen1/dsdt.asl 2 files changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/31470/1 diff --git a/src/ec/lenovo/h8/acpi/ec.asl b/src/ec/lenovo/h8/acpi/ec.asl index 327a2cf..6ffba5e 100644 --- a/src/ec/lenovo/h8/acpi/ec.asl +++ b/src/ec/lenovo/h8/acpi/ec.asl @@ -197,12 +197,20 @@ Method (_Q11, 0, NotSerialized) { +#if IS_ENABLED(EC_LENOVO_H8_SWAP_FNF2_FNF3) + ^HKEY.RHK (0x03) +#else ^HKEY.RHK (0x02) +#endif } Method (_Q12, 0, NotSerialized) { +#if IS_ENABLED(EC_LENOVO_H8_SWAP_FNF2_FNF3) + ^HKEY.RHK (0x02) +#else ^HKEY.RHK (0x03) +#endif } Method (_Q64, 0, NotSerialized) diff --git a/src/mainboard/lenovo/x1_carbon_gen1/dsdt.asl b/src/mainboard/lenovo/x1_carbon_gen1/dsdt.asl index 8c9bd5a..2696607 100644 --- a/src/mainboard/lenovo/x1_carbon_gen1/dsdt.asl +++ b/src/mainboard/lenovo/x1_carbon_gen1/dsdt.asl @@ -20,6 +20,7 @@ #define BRIGHTNESS_DOWN \_SB.PCI0.GFX0.DECB #define ACPI_VIDEO_DEVICE \_SB.PCI0.GFX0 #define EC_LENOVO_H8_ME_WORKAROUND 1 +#define EC_LENOVO_H8_SWAP_FNF2_FNF3 1 #include <arch/acpi.h> DefinitionBlock( -- To view, visit https://review.coreboot.org/c/coreboot/+/31470 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2d96be1a7815d7d03e6e8c6d300fd671c8598ca Gerrit-Change-Number: 31470 Gerrit-PatchSet: 1 Gerrit-Owner: Nicola Corna <nicola(a)corna.info> Gerrit-MessageType: newchange

4 11

Change in coreboot[master]: src/northbridge/amd/pi/00730F01/northbridge.c: enable ACS and AER for...
by Michał Żygowski (Code Review) 17 Feb '20

17 Feb '20

Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35313 ) Change subject: src/northbridge/amd/pi/00730F01/northbridge.c: enable ACS and AER for PCIe ports ...................................................................... src/northbridge/amd/pi/00730F01/northbridge.c: enable ACS and AER for PCIe ports Currently it is impossible to enable ACS with AGESA by setting the correct bit for AmdInitMid phase. AGESA code path does not call the right function that enables these functionalities. Disabled ACS result in multiple PCIe devices to be assigned to the same IOMMU group. Without IOMMU group separation the devices cannot be passed through independently. Enable Access Control Services and Advanced Error Reporting for PCI Express bridges in order to have PCIe devices in separate IOMMU groups for correct passthrough. TEST=run dmesg on Debian Buster and check whether PCIe devices have separate groups Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Change-Id: I10a8eff0ba37196692f9db6519e498fe535ecd15 --- M src/northbridge/amd/pi/00730F01/northbridge.c 1 file changed, 19 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/35313/1 diff --git a/src/northbridge/amd/pi/00730F01/northbridge.c b/src/northbridge/amd/pi/00730F01/northbridge.c index ba17c61..fede1e9 100644 --- a/src/northbridge/amd/pi/00730F01/northbridge.c +++ b/src/northbridge/amd/pi/00730F01/northbridge.c @@ -783,6 +783,25 @@ pci_write_config32(dev, 0xF8, 0); pci_write_config32(dev, 0xFC, 5); /* TODO: move it to dsdt.asl */ + /* Select GPP link core IO Link Strap Control register 0xB0 */ + pci_write_config32(dev, 0xE0, 0x014000B0); + value = pci_read_config32(dev, 0xE4); + + /* Enable AER (bit 5) and ACS (bit 6 undocumented) */ + value |= (BIT(5) | BIT(6)); + pci_write_config32(dev, 0xE4, value); + + /* Select GPP link core Wrapper register 0x00 (undocumented) */ + pci_write_config32(dev, 0xE0, 0x01300000); + value = pci_read_config32(dev, 0xE4); + + /* Enable ACS capabilities straps including sub-items. From lspci it + * looks like these bits enable: Source Validation and Translation + * Blocking + */ + value |= (BIT(24) | BIT(25) | BIT(26)); + pci_write_config32(dev, 0xE4, value); + /* disable No Snoop */ dev = pcidev_on_root(1, 1); if (dev != NULL) { -- To view, visit https://review.coreboot.org/c/coreboot/+/35313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I10a8eff0ba37196692f9db6519e498fe535ecd15 Gerrit-Change-Number: 35313 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: newchange

5 16

Change in coreboot[master]: soc/intel/common/block/lpc: create LPC_GET_DEV macro
by Subrata Banik (Code Review) 17 Feb '20

17 Feb '20

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30209 ) Change subject: soc/intel/common/block/lpc: create LPC_GET_DEV macro ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… File src/soc/intel/common/block/lpc/lpc_lib.c: https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… PS2, Line 29: #if !defined(__SIMPLE_DEVICE__) : #include <device/device.h> : #define LPC_GET_DEV pcidev_on_root(PCH_DEV_SLOT_LPC, 0x0) : #else : #define LPC_GET_DEV PCH_DEV_LPC : #endif > > > > > Compiles fine as by Aamir's suggestion, btw. > > > > > > > > yes compilation will work as long as #define __SIMPLE_DEVICE__ is there. > > > > > > No, I meant this patch but with PCH_DEV_LPC from soc/pci_devs.h instead > > > of your new macro works fine. > > > > Do you mean PCH_DEV_LPC from soc/pci_devs.h will work without having #define __SIMPLE_DEVICE__ at line 18? > > Yes, if you do the same, related modifications below, i.e. remove the > pci_devfn_t declarations. i'm not sure if i understood that, i was trying to understand what benefit this #define __SIMPLE_DEVICE__ is giving here as we don't have any such macro define elsewhere in common block. Now you are telling this is to provide "stable" code across all stage unlike stage specific compilation. > > > > > > > > > > > > > > > But i don't know why we have this guard at first place and want to compile this entire file as part of simple device. is that saving anything ? general query. > > > > > > It provides a stable API even if the file is compiled for ramstage and > > > pre-ram stages. > > > > Intention is not to break the stability here for sure, just wondering if you have real reason why #define __SIMPLE_DEVICE__ been added at first place ? > > Let me rephrase: > > If we #define __SIMPLE_DEVICE__ on top of a .c file. All included > headers will behave the same, no matter if we compile for ramstage > or not. That's what I called `stable API`. The same API for ramstage > and others. > > If we don't #define __SIMPLE_DEVICE__ explicitly, it's still defined > by the build system for pre-ram stages. Then the #if in header files > apply in one way or the other depending on if we build for ramstage > or not. This was originally intended for the headers files in src/ > include/device/. > > So, the explicit define (as it was done here) makes it easier to > compile the same .c file for both ramstage and pre-ram stages > _without_ further tricks (like #if in include/soc/pci_def.h). It > makes, however, usage of `struct device` harder. > > The way include/soc/pci_def.h does it, with the #if, has led to > many errors in the past (mostly due to missing null-checks). Hence, > I call it discouraged. > > > > > > Hence, avoids the need for any #if in platform code. > > > > FYI, this is soc code not the platform code. hatch, poppy are the platform code in my understanding. this is soc library. i was adhering old review comments from Arthur who has suggested to have a macro rather function names. > > Sorry, I don't know how you call things. To me, a platform in this > context is chipset + CPU. What I actually meant is #if outside of > src/include/device/. > > About Arthur's comments, they were for patch set 1 which looks much > differently. Also, the original problem patch set 1 tried to solve > was fixed in the meantime. There was > > #ifdef __SIMPLE_DEVICE__ > pci_devfn_t dev; > #else > struct device *dev; > #endif > > earlier, but isn't anymore. Which explains the commit message, maybe. i'm not very sure about that, Arthur's concern is still active herehttps://github.com/coreboot/coreboot/blob/master/src/soc/intel/common/b… and i was coping the same into this file, hence he has suggested to use macros rather function > > Maybe let's back up and start with the problem you are trying to > solve? Is there anything you want to add to / change in this file > that isn't possible because of the `#define __SIMPLE_DEVICE__`? problem is having different macros for "dev". Inside pci_read_config16() function we are using PCH_DEV_LPC and https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… here we are using struct device * hence user has to have lots of knowledge about different use case inside croeboot, which i was trying to avoid and provide more abstraction > > > > > > because you can't use them anymore to get a `struct device *`. That's > > > why there are some explicit calls to pcidev_on_root() below. > > > > yes, that's the point. this could have been avoided but it has some other assumptions :) about "struct device *" > > Why avoid it? The code is more explicit this way. It's clear to the > reader that a pointer is involved, which makes review and maintenance > easier. i believe above answer can clarify -- To view, visit https://review.coreboot.org/c/coreboot/+/30209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84a6102bf3849e9d4fe28e4c6a11bc7badcf5114 Gerrit-Change-Number: 30209 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> 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: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 17 Feb 2020 16:08:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/autoport: Fix typo
by Angel Pons (Code Review) 17 Feb '20

17 Feb '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38925 ) Change subject: util/autoport: Fix typo ...................................................................... util/autoport: Fix typo Also reflow the paragraph in which the typo was hiding a bit. Change-Id: I2fea01fe23af21c2540fa90154ce29af3e74776b Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M util/autoport/readme.md 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/38925/1 diff --git a/util/autoport/readme.md b/util/autoport/readme.md index b73b441..6744137 100644 --- a/util/autoport/readme.md +++ b/util/autoport/readme.md @@ -355,9 +355,9 @@ ## `c*_acpower` and `c*_battery` -Which mwait states to match to which ACPI levels. Normall, there is no -need to modify anything unless your device has very special power -saving requirements. +Which mwait states to match to which ACPI levels. Normally, there is no +need to modify anything unless your device has very special power saving +requirements. ## `install_intel_vga_int15_handler` -- To view, visit https://review.coreboot.org/c/coreboot/+/38925 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2fea01fe23af21c2540fa90154ce29af3e74776b Gerrit-Change-Number: 38925 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

5 4

Change in coreboot[master]: mb/google/slippy: Fix IRQ of the ambient light sensor
by Nicolò (Code Review) 17 Feb '20

17 Feb '20

Nicolò has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37992 ) Change subject: mb/google/slippy: Fix IRQ of the ambient light sensor ...................................................................... mb/google/slippy: Fix IRQ of the ambient light sensor Change-Id: Ib2a8356d261d211d5ed5c0b035c94ec56b9c25b3 Signed-off-by: Nicolò Veronese <nicveronese(a)gmail.com> --- M src/mainboard/google/slippy/onboard.h 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/37992/1 diff --git a/src/mainboard/google/slippy/onboard.h b/src/mainboard/google/slippy/onboard.h index 9e1dc8b..7c40e23 100644 --- a/src/mainboard/google/slippy/onboard.h +++ b/src/mainboard/google/slippy/onboard.h @@ -17,7 +17,7 @@ #define ONBOARD_H #define BOARD_LIGHTSENSOR_NAME "lightsensor" -#define BOARD_LIGHTSENSOR_IRQ 51 /* PIRQT */ +#define BOARD_LIGHTSENSOR_IRQ 35 /* PIRQT */ #define BOARD_LIGHTSENSOR_I2C_BUS 2 /* I2C1 */ #define BOARD_LIGHTSENSOR_I2C_ADDR 0x44 -- To view, visit https://review.coreboot.org/c/coreboot/+/37992 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2a8356d261d211d5ed5c0b035c94ec56b9c25b3 Gerrit-Change-Number: 37992 Gerrit-PatchSet: 1 Gerrit-Owner: Nicolò Gerrit-MessageType: newchange

3 6

Change in coreboot[master]: mb/google/drallion/variants/drallion: Update thermal configuration fo...
by John Su (Code Review) 17 Feb '20

17 Feb '20

John Su has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38658 ) Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ...................................................................... mb/google/drallion/variants/drallion: Update thermal configuration for DPTF Follow thermal table for fine tuning. 1. Update PSV values for sensers. 2. Change PL1 min value from 4w to 5w. 3. Change PL1 max value from 15w to 12w. 3. Change PL2 min value from 15w to 12w. BUG=b:148627484 TEST=Built and tested on drallion Change-Id: I957d41e3c14f6dbcec8c3555382895698beabe40 Signed-off-by: John Su <john_su(a)compal.corp-partner.google.com> --- M src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl 1 file changed, 6 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/38658/1 diff --git a/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl b/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl index 4ecdf1a..be8fa39 100644 --- a/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl @@ -19,19 +19,19 @@ /* Skin Sensor for CPU VR temperature monitor */ #define DPTF_TSR0_SENSOR_ID 1 #define DPTF_TSR0_SENSOR_NAME "Skin" -#define DPTF_TSR0_PASSIVE 64 +#define DPTF_TSR0_PASSIVE 60 #define DPTF_TSR0_CRITICAL 127 /* Memory Sensor for DDR temperature monitor */ #define DPTF_TSR1_SENSOR_ID 2 #define DPTF_TSR1_SENSOR_NAME "DDR" -#define DPTF_TSR1_PASSIVE 54 +#define DPTF_TSR1_PASSIVE 52 #define DPTF_TSR1_CRITICAL 127 /* M.2 Sensor for Ambient temperature monitor */ #define DPTF_TSR2_SENSOR_ID 3 #define DPTF_TSR2_SENSOR_NAME "Ambient" -#define DPTF_TSR2_PASSIVE 40 +#define DPTF_TSR2_PASSIVE 90 #define DPTF_TSR2_CRITICAL 127 #undef DPTF_ENABLE_FAN_CONTROL @@ -56,15 +56,15 @@ 0x2, /* Revision */ Package () { /* Power Limit 1 */ 0, /* PowerLimitIndex, 0 for Power Limit 1 */ - 4000, /* PowerLimitMinimum */ - 15000, /* PowerLimitMaximum */ + 5000, /* PowerLimitMinimum */ + 12000, /* PowerLimitMaximum */ 100000, /* TimeWindowMinimum */ 100000, /* TimeWindowMaximum */ 100 /* StepSize */ }, Package () { /* Power Limit 2 */ 1, /* PowerLimitIndex, 1 for Power Limit 2 */ - 15000, /* PowerLimitMinimum */ + 12000, /* PowerLimitMinimum */ 51000, /* PowerLimitMaximum */ 280000, /* TimeWindowMinimum */ 280000, /* TimeWindowMaximum */ -- To view, visit https://review.coreboot.org/c/coreboot/+/38658 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I957d41e3c14f6dbcec8c3555382895698beabe40 Gerrit-Change-Number: 38658 Gerrit-PatchSet: 1 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-MessageType: newchange

6 17

Change in coreboot[master]: soc/intel/tigerlake: Enable Audio on TGL
by Srinidhi N Kaushik (Code Review) 17 Feb '20

17 Feb '20

Srinidhi N Kaushik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38147 ) Change subject: soc/intel/tigerlake: Enable Audio on TGL ...................................................................... soc/intel/tigerlake: Enable Audio on TGL Configure UPD's to support Audio enablement. BUG=none BRANCH=none TEST=Build and boot tigerlake rvp board Signed-off-by: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Change-Id: Idd3927a33d303ed5a663b5b838f43ed4ebc7a0db --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/romstage/fsp_params_tgl.c 2 files changed, 28 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/38147/1 diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h index 29d75f1..a1af2da 100644 --- a/src/soc/intel/tigerlake/chip.h +++ b/src/soc/intel/tigerlake/chip.h @@ -30,6 +30,10 @@ #include <soc/serialio.h> #include <soc/usb.h> +#define MAX_HD_AUDIO_DMIC_LINKS 2 +#define MAX_HD_AUDIO_SNDW_LINKS 4 +#define MAX_HD_AUDIO_SSP_LINKS 6 + struct soc_intel_tigerlake_config { /* Common struct containing soc config data required by common code */ @@ -115,6 +119,16 @@ uint8_t SataPortsEnable[8]; uint8_t SataPortsDevSlp[8]; + /* Audio related */ + uint8_t PchHdaDspEnable; + uint8_t PchHdaAudioLinkHdaEnable; + uint8_t PchHdaAudioLinkDmicEnable[MAX_HD_AUDIO_DMIC_LINKS]; + uint8_t PchHdaAudioLinkSspEnable[MAX_HD_AUDIO_SSP_LINKS]; + uint8_t PchHdaAudioLinkSndwEnable[MAX_HD_AUDIO_SNDW_LINKS]; + uint8_t PchHdaIDispLinkTmode; + uint8_t PchHdaIDispLinkFrequency; + uint8_t PchHdaIDispCodecDisconnect; + /* PCIe Root Ports */ uint8_t PcieRpEnable[CONFIG_MAX_ROOT_PORTS]; /* PCIe output clocks type to Pcie devices. diff --git a/src/soc/intel/tigerlake/romstage/fsp_params_tgl.c b/src/soc/intel/tigerlake/romstage/fsp_params_tgl.c index 4c309dd..41d0fd1 100644 --- a/src/soc/intel/tigerlake/romstage/fsp_params_tgl.c +++ b/src/soc/intel/tigerlake/romstage/fsp_params_tgl.c @@ -21,6 +21,7 @@ #include <soc/pci_devs.h> #include <soc/romstage.h> #include <soc/soc_chip.h> +#include <string.h> /* Debug interface flag */ enum debug_interface_flag { @@ -108,6 +109,19 @@ m_cfg->ECT = 0; /* Early Command Training Enabled */ m_cfg->RefClk = 0; /* Auto Select CLK freq */ + /* Audio: HDAUDIO_LINK_MODE_I2S_SSP */ + m_cfg->PchHdaDspEnable = config->PchHdaDspEnable; + m_cfg->PchHdaAudioLinkHdaEnable = config->PchHdaAudioLinkHdaEnable; + memcpy(m_cfg->PchHdaAudioLinkDmicEnable, config->PchHdaAudioLinkDmicEnable, + sizeof(m_cfg->PchHdaAudioLinkDmicEnable)); + memcpy(m_cfg->PchHdaAudioLinkSspEnable, config->PchHdaAudioLinkSspEnable, + sizeof(m_cfg->PchHdaAudioLinkSspEnable)); + memcpy(m_cfg->PchHdaAudioLinkSndwEnable, config->PchHdaAudioLinkSndwEnable, + sizeof(m_cfg->PchHdaAudioLinkSndwEnable)); + m_cfg->PchHdaIDispLinkTmode = config->PchHdaIDispLinkTmode; + m_cfg->PchHdaIDispLinkFrequency = config->PchHdaIDispLinkFrequency; + m_cfg->PchHdaIDispCodecDisconnect = config->PchHdaIDispCodecDisconnect; + m_cfg->PchHdaAudioLinkDmicClkAPinMux[0] = GPIO_VER2_LP_MUXING_DMIC0_CLKA_GPP_S6; m_cfg->PchHdaAudioLinkDmicClkBPinMux[0] = GPIO_VER2_LP_MUXING_DMIC0_CLKB_GPP_S2; m_cfg->PchHdaAudioLinkDmicDataPinMux[0] = GPIO_VER2_LP_MUXING_DMIC0_DATA_GPP_S7; -- To view, visit https://review.coreboot.org/c/coreboot/+/38147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idd3927a33d303ed5a663b5b838f43ed4ebc7a0db Gerrit-Change-Number: 38147 Gerrit-PatchSet: 1 Gerrit-Owner: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-MessageType: newchange

10 58

Change in coreboot[master]: vboot: remove GBB flag FAFT_KEY_OVERIDE
by Joel Kitching (Code Review) 17 Feb '20

17 Feb '20

Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37462 ) Change subject: vboot: remove GBB flag FAFT_KEY_OVERIDE ...................................................................... vboot: remove GBB flag FAFT_KEY_OVERIDE This flag was originally used to add some workarounds in firmware to get FAFT passing. Since we don't care about running FAFT on ToT for these boards, and since we'd like to avoid similar workarounds in the future, remove this flag. BUG=b:124141368, chromium:965914 TEST=make clean && make test-abuild BRANCH=none Change-Id: I79af304e9608a30c6839cd616378c7330c3de00a Signed-off-by: Joel Kitching <kitching(a)google.com> --- M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc 2 files changed, 0 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/37462/1 diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index a829443..dfacc00 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -301,10 +301,6 @@ bool "Allow booting to legacy in dev mode even if dev_boot_legacy=0" default n -config GBB_FLAG_FAFT_KEY_OVERIDE - bool "Allow booting using alternative keys for FAFT servo testing" - default n - config GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC bool "Disable EC software sync" default n diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc index 5292bd1..04c090d 100644 --- a/src/security/vboot/Makefile.inc +++ b/src/security/vboot/Makefile.inc @@ -218,7 +218,6 @@ $(call bool-to-mask,$(CONFIG_GBB_FLAG_DISABLE_FW_ROLLBACK_CHECK),0x20) \ $(call bool-to-mask,$(CONFIG_GBB_FLAG_ENTER_TRIGGERS_TONORM),0x40) \ $(call bool-to-mask,$(CONFIG_GBB_FLAG_FORCE_DEV_BOOT_LEGACY),0x80) \ - $(call bool-to-mask,$(CONFIG_GBB_FLAG_FAFT_KEY_OVERIDE),0x100) \ $(call bool-to-mask,$(CONFIG_GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC),0x200) \ $(call bool-to-mask,$(CONFIG_GBB_FLAG_DEFAULT_DEV_BOOT_LEGACY),0x400) \ $(call bool-to-mask,$(CONFIG_GBB_FLAG_DISABLE_PD_SOFTWARE_SYNC),0x800) \ -- To view, visit https://review.coreboot.org/c/coreboot/+/37462 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I79af304e9608a30c6839cd616378c7330c3de00a Gerrit-Change-Number: 37462 Gerrit-PatchSet: 1 Gerrit-Owner: Joel Kitching <kitching(a)google.com> Gerrit-MessageType: newchange

4 6

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2020