coreboot-gerrit February 2019

coreboot-gerrit@coreboot.org

1 participants
1249 discussions

Change in ...coreboot[master]: device/pci_ops: Apply some symmetry in headers
by Aaron Durbin (Code Review) 11 Feb '19

11 Feb '19

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31306 ) Change subject: device/pci_ops: Apply some symmetry in headers ...................................................................... Patch Set 2: > Patch Set 2: > > > Patch Set 2: Code-Review+2 > > > > would be great to fix some of that other arch deps. I assume there's a big ripple? > > I assume you refer to the FIXME comment there? I can do some sed-magic if we agree <device/pci_ops.h> is the one to add and <arch/io.h> one to (sometimes) remove from includes. Ya. I think arch/io should really not be in anything unless it is truly arch specific (io port access). All the memory mapped forced reads using volatile should really be arch agnostic. Either way, I like the direction the patches are taking. -- To view, visit https://review.coreboot.org/c/coreboot/+/31306 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8076a4867fd7472beaae0a021dcf0d9c7c905871 Gerrit-Change-Number: 31306 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 16:11:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: drivers/i2c/nct7802y: Add new hardware-monitoring IC
by Paul Menzel (Code Review) 11 Feb '19

11 Feb '19

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29475 ) Change subject: drivers/i2c/nct7802y: Add new hardware-monitoring IC ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/29475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I35ea79e12941804e398c6304a08170a776f4ca76 Gerrit-Change-Number: 29475 Gerrit-PatchSet: 4 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 16:10:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: device/pci_ops: Apply some symmetry in headers
by Kyösti Mälkki (Code Review) 11 Feb '19

11 Feb '19

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31306 ) Change subject: device/pci_ops: Apply some symmetry in headers ...................................................................... Patch Set 2: > Patch Set 2: Code-Review+2 > > would be great to fix some of that other arch deps. I assume there's a big ripple? I assume you refer to the FIXME comment there? I can do some sed-magic if we agree <device/pci_ops.h> is the one to add and <arch/io.h> one to (sometimes) remove from includes. -- To view, visit https://review.coreboot.org/c/coreboot/+/31306 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8076a4867fd7472beaae0a021dcf0d9c7c905871 Gerrit-Change-Number: 31306 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 16:09:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: drivers/i2c/lm96000: Add new hardware-monitoring IC
by Paul Menzel (Code Review) 11 Feb '19

11 Feb '19

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21194 ) Change subject: drivers/i2c/lm96000: Add new hardware-monitoring IC ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/#/c/21194/7/src/drivers/i2c/lm96000/lm96000.c File src/drivers/i2c/lm96000/lm96000.c: https://review.coreboot.org/#/c/21194/7/src/drivers/i2c/lm96000/lm96000.c@1… PS7, Line 184: timeout = 1000; : lm_config = lm96000_read(dev, LM96000_CONFIG); : while ((lm_config < 0 || !(lm_config & LM96000_READY)) && timeout--) { : mdelay(1); : lm_config = lm96000_read(dev, LM96000_CONFIG); : } : if (lm_config < 0 || !(lm_config & LM96000_READY)) { : printk(BIOS_INFO, "lm96000: Not ready after 1s.\n"); : return; : } Use the stopwatch infrastructure? -- To view, visit https://review.coreboot.org/c/coreboot/+/21194 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie7df3107bffb7f8e45e71c4c1fbe4eb0a9e3cd03 Gerrit-Change-Number: 21194 Gerrit-PatchSet: 7 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 11 Feb 2019 16:08:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: device/pci_ops: Apply some symmetry in headers
by Paul Menzel (Code Review) 11 Feb '19

11 Feb '19

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31306 ) Change subject: device/pci_ops: Apply some symmetry in headers ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/31306 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8076a4867fd7472beaae0a021dcf0d9c7c905871 Gerrit-Change-Number: 31306 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 16:06:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: device/pci_ops: Apply some symmetry in headers
by Aaron Durbin (Code Review) 11 Feb '19

11 Feb '19

1 0

Change in ...coreboot[master]: soc/amd/common: Remove redundant ACPI S3 test
by Richard Spiegel (Code Review) 11 Feb '19

11 Feb '19

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27276 ) Change subject: soc/amd/common: Remove redundant ACPI S3 test ...................................................................... Patch Set 4: -Code-Review (3 comments) https://review.coreboot.org/#/c/27276/4/src/soc/amd/common/block/pi/agesawr… File src/soc/amd/common/block/pi/agesawrapper.c: https://review.coreboot.org/#/c/27276/4/src/soc/amd/common/block/pi/agesawr… PS4, Line 425: : : > You do not even reach do_agesawrapper() lines calling these functions, with HAVE_ACPI_RESUME=n. […] You do not even reach do_agesawrapper() lines calling these functions, with HAVE_ACPI_RESUME=n. You are correct. https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig File src/soc/amd/stoneyridge/Kconfig: https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig@a53 PS4, Line 53: Why remove? True, if no S3 enabled than the linker will not add these files. But it's more clear this way, plus compile time will be a little less (the files won't be compiled at all against only not being linked). https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/chip.c File src/soc/amd/stoneyridge/chip.c: https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/chip.c@158 PS4, Line 158: acpi_s3_resume_allowed() Not sure why this more complicated test. If it's on a resume path, then certainly S3 is allowed. -- To view, visit https://review.coreboot.org/c/coreboot/+/27276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7c7a9ecbfcb82790e477d906a00f9749103b4045 Gerrit-Change-Number: 27276 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshall.dawson(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 15:29:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Comment-In-Reply-To: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: drivers/i2c/lm96000: Add new hardware-monitoring IC
by build bot (Jenkins) (Code Review) 11 Feb '19

11 Feb '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21194 ) Change subject: drivers/i2c/lm96000: Add new hardware-monitoring IC ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/#/c/21194/7/src/drivers/i2c/lm96000/lm96000.c File src/drivers/i2c/lm96000/lm96000.c: https://review.coreboot.org/#/c/21194/7/src/drivers/i2c/lm96000/lm96000.c@1… PS7, Line 155: { 2, 3, 3, 4, 5, 7, 8, 10, 13, 16, 20, 27, 32, 40, 53, 80 }; that open brace { should be on the previous line -- To view, visit https://review.coreboot.org/c/coreboot/+/21194 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie7df3107bffb7f8e45e71c4c1fbe4eb0a9e3cd03 Gerrit-Change-Number: 21194 Gerrit-PatchSet: 7 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 15:17:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/amd/common: Remove redundant ACPI S3 test
by Kyösti Mälkki (Code Review) 11 Feb '19

11 Feb '19

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27276 ) Change subject: soc/amd/common: Remove redundant ACPI S3 test ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/#/c/27276/4/src/soc/amd/common/block/pi/agesawr… File src/soc/amd/common/block/pi/agesawrapper.c: https://review.coreboot.org/#/c/27276/4/src/soc/amd/common/block/pi/agesawr… PS4, Line 425: : : > Essentially the return parameter is only used for debug message, it does not block continuation of c […] You do not even reach do_agesawrapper() lines calling these functions, with HAVE_ACPI_RESUME=n. As the commit message says, redundant tests. Furthermore, sleepstates.asl also does not advertise ACPI S3 support when HAVE_ACPI_RESUME=n. Whether OS acpid and GUIs honour that is another question and not within our reach to fix. Let me know how this commit fails on hardware testing. -- To view, visit https://review.coreboot.org/c/coreboot/+/27276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7c7a9ecbfcb82790e477d906a00f9749103b4045 Gerrit-Change-Number: 27276 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshall.dawson(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 15:05:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/amd/common: Remove redundant ACPI S3 test
by Richard Spiegel (Code Review) 11 Feb '19

11 Feb '19

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27276 ) Change subject: soc/amd/common: Remove redundant ACPI S3 test ...................................................................... Patch Set 4: Code-Review-1 (1 comment) https://review.coreboot.org/#/c/27276/4/src/soc/amd/common/block/pi/agesawr… File src/soc/amd/common/block/pi/agesawrapper.c: https://review.coreboot.org/#/c/27276/4/src/soc/amd/common/block/pi/agesawr… PS4, Line 425: : : Essentially the return parameter is only used for debug message, it does not block continuation of code. So if you disable ACPI_S3, and leave the return only on the first and/or maybe the second, S3 would be possible to enter, but when resuming system would hang (all steps must be executed or not). If you block entering S3, then it would be possible to remove these checks, otherwise I would rather you abandon this change. -- To view, visit https://review.coreboot.org/c/coreboot/+/27276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7c7a9ecbfcb82790e477d906a00f9749103b4045 Gerrit-Change-Number: 27276 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshall.dawson(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 14:42:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2019