coreboot-gerrit February 2020

coreboot-gerrit@coreboot.org

1 participants
1796 discussions

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

17 Feb '20

Aamir Bohra 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 would this be inherently taken care by: #if !defined(__SIMPLE_DEVICE__) #include <device/device.h> #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #endif in include/soc/pci_devs.h? -- 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-Comment-Date: Mon, 17 Feb 2020 08:17:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vboot: push clear recovery mode switch until later
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/+/38779 ) Change subject: vboot: push clear recovery mode switch until later ...................................................................... vboot: push clear recovery mode switch until later On some platforms, FSP initialization may cause a reboot. Push clearing the recovery mode switch until after FSP code runs, so that a manual recovery request (three-finger salute) will function correctly under this condition. BUG=b:124141368, b:35576380 TEST=make clean && make test-abuild BRANCH=none Change-Id: I30c02787c620b937e5a50a5ed94ac906e3112dad Signed-off-by: Joel Kitching <kitching(a)google.com> --- M src/security/vboot/bootmode.c M src/security/vboot/vboot_logic.c 2 files changed, 19 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/38779/1 diff --git a/src/security/vboot/bootmode.c b/src/security/vboot/bootmode.c index 83baa81..61b3d76 100644 --- a/src/security/vboot/bootmode.c +++ b/src/security/vboot/bootmode.c @@ -24,6 +24,25 @@ #include <security/vboot/vbnv.h> #include <security/vboot/vboot_common.h> +static void vboot_log_and_clear_recovery_mode_switch(void *unused) +{ + /* Log the recovery mode switches if required, before clearing them. */ + log_recovery_mode_switch(); + + /* + * The recovery mode switch is cleared (typically backed by EC) here + * to allow multiple queries to get_recovery_mode_switch() and have + * them return consistent results during the verified boot path as well + * as dram initialization. x86 systems ignore the saved dram settings + * in the recovery path in order to start from a clean slate. Therefore + * clear the state here since this function is called when memory + * is known to be up. + */ + clear_recovery_mode_switch(); +} +BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, + vboot_log_and_clear_recovery_mode_switch, NULL); + static int vboot_get_recovery_reason_shared_data(void) { struct vb2_shared_data *sd = vb2_get_sd(vboot_get_context()); diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 1d17a17..eabf8f7 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -269,26 +269,6 @@ vboot_extend_pcr(ctx, 1, HWID_DIGEST_PCR); } -static void vboot_log_and_clear_recovery_mode_switch(int unused) -{ - /* Log the recovery mode switches if required, before clearing them. */ - log_recovery_mode_switch(); - - /* - * The recovery mode switch is cleared (typically backed by EC) here - * to allow multiple queries to get_recovery_mode_switch() and have - * them return consistent results during the verified boot path as well - * as dram initialization. x86 systems ignore the saved dram settings - * in the recovery path in order to start from a clean slate. Therefore - * clear the state here since this function is called when memory - * is known to be up. - */ - clear_recovery_mode_switch(); -} -#if !CONFIG(VBOOT_STARTS_IN_ROMSTAGE) -ROMSTAGE_CBMEM_INIT_HOOK(vboot_log_and_clear_recovery_mode_switch) -#endif - /** * Verify and select the firmware in the RW image * @@ -449,11 +429,6 @@ vboot_is_firmware_slot_a(ctx) ? 'A' : 'B'); verstage_main_exit: - /* If CBMEM is not up yet, let the ROMSTAGE_CBMEM_INIT_HOOK take care - of running this function. */ - if (ENV_ROMSTAGE && CONFIG(VBOOT_STARTS_IN_ROMSTAGE)) - vboot_log_and_clear_recovery_mode_switch(0); - /* Save recovery reason in case of unexpected reboots on x86. */ vboot_save_recovery_reason_vbnv(); -- To view, visit https://review.coreboot.org/c/coreboot/+/38779 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I30c02787c620b937e5a50a5ed94ac906e3112dad Gerrit-Change-Number: 38779 Gerrit-PatchSet: 1 Gerrit-Owner: Joel Kitching <kitching(a)google.com> Gerrit-MessageType: newchange

3 29

Change in coreboot[master]: soc/intel/apollolake: Display apollolake platform information
by Usha P (Code Review) 17 Feb '20

17 Feb '20

Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38824 ) Change subject: soc/intel/apollolake: Display apollolake platform information ...................................................................... Patch Set 3: > Patch Set 3: > > > Patch Set 3: > > > > > Patch Set 3: > > > > > > > Patch Set 3: > > > > > > > > Is there a reason why this needs to be part of bootblock? > > > > We have size constrains in bootblock anyway and on the other hand have a plenty of space in the ramstage? Wouldn't it be as useful in ramstage? > > > > > > good point, right now its more over parity with other soc design we have where bootblock does report platform early. > > > > > > But fare point to move those in romstage may be as these are sorts of information to have, nothing as such mandatory to have inside BB itself. Agree ? > > > > Sure, romstage is OK as well OFC. > > I just stumbled across this issue as the bootblock size was increased and I do not see a good reason for it in this context. > > yeah make sense. > > @Usha, possible for you to test this CL moving report platform into romstage ? > > Also how about 1 dedicated CL for rest of IA-SoC to move report platform into romstage. > > Once sorted we can create common report_platform.c for all SOC and use it from romstage? Yes Subrata, this can be done. I will test this CL by moving it into romstage and update. -- To view, visit https://review.coreboot.org/c/coreboot/+/38824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4edfeae7faee9f5f80698cf34b31fdcb066a813 Gerrit-Change-Number: 38824 Gerrit-PatchSet: 3 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> 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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 07:53:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/apollolake: Display apollolake platform information
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/+/38824 ) Change subject: soc/intel/apollolake: Display apollolake platform information ...................................................................... Patch Set 3: > Patch Set 3: > > > Patch Set 3: > > > > > Patch Set 3: > > > > > > Is there a reason why this needs to be part of bootblock? > > > We have size constrains in bootblock anyway and on the other hand have a plenty of space in the ramstage? Wouldn't it be as useful in ramstage? > > > > good point, right now its more over parity with other soc design we have where bootblock does report platform early. > > > > But fare point to move those in romstage may be as these are sorts of information to have, nothing as such mandatory to have inside BB itself. Agree ? > > Sure, romstage is OK as well OFC. > I just stumbled across this issue as the bootblock size was increased and I do not see a good reason for it in this context. yeah make sense. @Usha, possible for you to test this CL moving report platform into romstage ? Also how about 1 dedicated CL for rest of IA-SoC to move report platform into romstage. Once sorted we can create common report_platform.c for all SOC and use it from romstage? -- To view, visit https://review.coreboot.org/c/coreboot/+/38824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4edfeae7faee9f5f80698cf34b31fdcb066a813 Gerrit-Change-Number: 38824 Gerrit-PatchSet: 3 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> 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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 07:48:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/apollolake: Display apollolake platform information
by Werner Zeh (Code Review) 17 Feb '20

17 Feb '20

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38824 ) Change subject: soc/intel/apollolake: Display apollolake platform information ...................................................................... Patch Set 3: > Patch Set 3: > > > Patch Set 3: > > > > Is there a reason why this needs to be part of bootblock? > > We have size constrains in bootblock anyway and on the other hand have a plenty of space in the ramstage? Wouldn't it be as useful in ramstage? > > good point, right now its more over parity with other soc design we have where bootblock does report platform early. > > But fare point to move those in romstage may be as these are sorts of information to have, nothing as such mandatory to have inside BB itself. Agree ? Sure, romstage is OK as well OFC. I just stumbled across this issue as the bootblock size was increased and I do not see a good reason for it in this context. -- To view, visit https://review.coreboot.org/c/coreboot/+/38824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4edfeae7faee9f5f80698cf34b31fdcb066a813 Gerrit-Change-Number: 38824 Gerrit-PatchSet: 3 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> 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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 07:45:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/apollolake: Display apollolake platform information
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/+/38824 ) Change subject: soc/intel/apollolake: Display apollolake platform information ...................................................................... Patch Set 3: > Patch Set 3: > > Is there a reason why this needs to be part of bootblock? > We have size constrains in bootblock anyway and on the other hand have a plenty of space in the ramstage? Wouldn't it be as useful in ramstage? good point, right now its more over parity with other soc design we have where bootblock does report platform early. But fare point to move those in romstage may be as these are sorts of information to have, nothing as such mandatory to have inside BB itself. Agree ? -- To view, visit https://review.coreboot.org/c/coreboot/+/38824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4edfeae7faee9f5f80698cf34b31fdcb066a813 Gerrit-Change-Number: 38824 Gerrit-PatchSet: 3 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> 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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 07:40:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu: Add microcode at pre-defined address
by Arthur Heymans (Code Review) 17 Feb '20

17 Feb '20

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38640 ) Change subject: cpu: Add microcode at pre-defined address ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/c/coreboot/+/38640/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/38640/6//COMMIT_MSG@7 PS6, Line 7: c > the assembly code around FSP-T invocation actually finds microcode location and size. […] I'm aware that it's not possible to give the microcode location to FSP-T during runtime. Coreboot has assembly code using only registers (for pre-CAR use) to find the location of the microcode cbfs file and then find the proper update and do the update itself. So using that and completely skipping FSP-T doing the update would be preferable. -- To view, visit https://review.coreboot.org/c/coreboot/+/38640 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6c02601a7ac64078e556e2032baeccaf27f77da2 Gerrit-Change-Number: 38640 Gerrit-PatchSet: 7 Gerrit-Owner: Andrey Petrov <anpetrov(a)fb.com> Gerrit-Reviewer: Andrey Petrov <anpetrov(a)fb.com> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 17 Feb 2020 07:25:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Andrey Petrov <anpetrov(a)fb.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/apollolake: Display apollolake platform information
by Werner Zeh (Code Review) 17 Feb '20

17 Feb '20

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38824 ) Change subject: soc/intel/apollolake: Display apollolake platform information ...................................................................... Patch Set 3: Is there a reason why this needs to be part of bootblock? We have size constrains in bootblock anyway and on the other hand have a plenty of space in the ramstage? Wouldn't it be as useful in ramstage? -- To view, visit https://review.coreboot.org/c/coreboot/+/38824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4edfeae7faee9f5f80698cf34b31fdcb066a813 Gerrit-Change-Number: 38824 Gerrit-PatchSet: 3 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> 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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 06:46:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

Hello Aaron Durbin, Patrick Rudolph, Arthur Heymans, Aamir Bohra, Duncan Laurie, build bot (Jenkins), Nico Huber, Furquan Shaikh, Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/30209 to look at the new patch set (#2). Change subject: soc/intel/common/block/lpc: create LPC_GET_DEV macro ...................................................................... soc/intel/common/block/lpc: create LPC_GET_DEV macro This patch removes redundent declaration of pci device structure in every function (for __SIMPLE_DEVICE__ and !__SIMPLE_DEVICE__) by adding LPC_GET_DEV macro. Change-Id: I84a6102bf3849e9d4fe28e4c6a11bc7badcf5114 Signed-off-by: Subrata Banik <subrata.banik(a)intel.com> --- M src/soc/intel/common/block/lpc/lpc_lib.c 1 file changed, 29 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/30209/2 -- 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/lpc: create lpc_get_device() helper function
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_device() helper function ...................................................................... Patch Set 1: (3 comments) https://review.coreboot.org/c/coreboot/+/30209/1/src/soc/intel/common/block… File src/soc/intel/common/block/lpc/lpc_lib.c: https://review.coreboot.org/c/coreboot/+/30209/1/src/soc/intel/common/block… PS1, Line 30: PCI_DEV(0, PCI_SLOT(devfn), PCI_FUNC(devfn)) > pci_devfn_t is a u32. I don't see why you need a function for this. […] Ack https://review.coreboot.org/c/coreboot/+/30209/1/src/soc/intel/common/block… PS1, Line 32: PCI_DEV_INVALID > no. […] Ack https://review.coreboot.org/c/coreboot/+/30209/1/src/soc/intel/common/block… PS1, Line 48: #define LPC_GET_DEV lpc_get_device() > I understand why you want to remove the __SIMPLE_DEVICE__ definition here. […] Ack -- 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: 1 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 06:42:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2020