Attention is currently required from: Varshit B Pandya, Maulik V Vaghela, Paul Menzel, Subrata Banik, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58767 )
Change subject: driver/intel/mipi_camera: Add support for _DSC field
......................................................................
Patch Set 7:
(1 comment)
File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/58767/comment/e8b68a89_1767a6a1
PS7, Line 851: acpi_dsc
> shouldn't this be "cam_max_dstate" rather acpi_dsc which is not clear enough ?
`max_dstate_for_probe`
?
--
To view, visit https://review.coreboot.org/c/coreboot/+/58767
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5471f144918413a2982f86beaf3dbf7e4e66cc9b
Gerrit-Change-Number: 58767
Gerrit-PatchSet: 7
Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-CC: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Attention: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 09 Nov 2021 21:13:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Bora Guvendik, Caveh Jalali, Selma Bensaid, Maulik V Vaghela, Subrata Banik, Nick Vaccaro, Patrick Rudolph, Karthik Ramasubramanian.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58351 )
Change subject: soc/intel/common: add generic gpio lock mechanism
......................................................................
Patch Set 7:
(2 comments)
File src/soc/intel/common/block/smm/smihandler.c:
https://review.coreboot.org/c/coreboot/+/58351/comment/0786c846_2d10bf7d
PS7, Line 333: exit if SoC does not export a list
this doesn't look true?
https://review.coreboot.org/c/coreboot/+/58351/comment/e17c0fc9_50fb654c
PS7, Line 328: const struct gpio_lock_config *soc_gpios;
: const struct gpio_lock_config *mb_gpios;
: size_t soc_gpio_num;
: size_t mb_gpio_num;
:
: /* get list of gpios from SoC, exit if SoC does not export a list */
: soc_gpios = soc_gpio_lock_config(&soc_gpio_num);
:
: /* get list of gpios from mainboard, it's ok if mb does not export a list */
: mb_gpios = mb_gpio_lock_config(&mb_gpio_num);
:
: /* Lock all mainboard and soc requested gpios */
: gpio_lock_multiple_pads(soc_gpios, soc_gpio_num, mb_gpios, mb_gpio_num);
so you could just use malloc() here to allocate a list long enough to store both and not have the awkward API
```
const struct gpio_lock_config *soc_gpios;
const struct gpio_lock_config *mb_gpios;
size_t soc_gpio_num;
size_t mb_gpio_num;
soc_gpios = soc_gpio_lock_config(&soc_gpio_num);
mb_gpios = mb_gpio_lock_config(&mb_gpio_num);
const size_t total_num = soc_gpio_num + mb_gpio_num;
const struct gpio_lock_config *all_gpios = malloc(sizeof(*all_gpios) * total_num);
memcpy(all_gpios, soc_gpios, sizeof(*soc_gpios) * soc_gpio_num);
memcpy(all_gpios + soc_gpio_num, mb_gpios, sizeof(*mb_gpios) * mb_gpio_num);
/* Lock all mainboard and soc requested gpios */
gpio_lock_pads(all_gpios, total_num);
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/58351
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I42979fb89567d8bcd9392da4fb8c4113ef427b14
Gerrit-Change-Number: 58351
Gerrit-PatchSet: 7
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Boris Mittelberg <bmbm(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Tue, 09 Nov 2021 21:12:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Maulik V Vaghela, Subrata Banik, Nick Vaccaro, Patrick Rudolph, Karthik Ramasubramanian.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58352 )
Change subject: soc/intel/alderlake: enable gpio locking
......................................................................
Patch Set 8:
(1 comment)
File src/soc/intel/alderlake/gpio.c:
https://review.coreboot.org/c/coreboot/+/58352/comment/7d5e3ec4_d1626ccf
PS8, Line 252: non_host_controllable_gpios
suggestion:
`gpios_to_lock`
since SX_EXIT_HOLDOFF is not locked for non-host reasons, rather so a malicious actor can't force the device to stay in Sx states
--
To view, visit https://review.coreboot.org/c/coreboot/+/58352
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I457bab39f945ab31a89542c6498a73af70cbf9ee
Gerrit-Change-Number: 58352
Gerrit-PatchSet: 8
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Tue, 09 Nov 2021 21:05:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Bora Guvendik, Caveh Jalali, Selma Bensaid, Maulik V Vaghela, Subrata Banik, Nick Vaccaro, Patrick Rudolph, Karthik Ramasubramanian.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58351 )
Change subject: soc/intel/common: add generic gpio lock mechanism
......................................................................
Patch Set 7:
(2 comments)
File src/soc/intel/common/block/gpio/gpio.c:
https://review.coreboot.org/c/coreboot/+/58351/comment/7b24594c_e441e282
PS7, Line 461: int gpio_lock_multiple_pads(const struct gpio_lock_config *pads_a, const size_t count_a,
: const struct gpio_lock_config *pads_b, const size_t count_b)
this is kind of an awkward API to use...
I think it would make more sense to make the caller concatenate their lists together with malloc() for example, and keep this API simpler, e.g.:
`bool gpio_lock_pads(const struct gpio_lock_config *pads, size_t count);`
File src/soc/intel/common/block/smm/smihandler.c:
https://review.coreboot.org/c/coreboot/+/58351/comment/e2c8d189_c8608745
PS1, Line 348: gpio_lock_pad
> Good suggestion, Subrata. I'll add it back in guarded.
Ok I cherry-picked this latest one into my tree and timed it with the stopwatch API and the whole function call is taking `7132 us`, so less than 8ms, which is really not too bad of an impact, all things considered.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58351
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I42979fb89567d8bcd9392da4fb8c4113ef427b14
Gerrit-Change-Number: 58351
Gerrit-PatchSet: 7
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Boris Mittelberg <bmbm(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Tue, 09 Nov 2021 21:03:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Comment-In-Reply-To: Nick Vaccaro <nvaccaro(a)google.com>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59056 )
Change subject: soc/intel/common: irq: Fix type mismatch in print statement
......................................................................
soc/intel/common: irq: Fix type mismatch in print statement
CC ramstage/soc/intel/common/block/irq/irq.o
CC ramstage/soc/intel/common/block/i2c/i2c.o
CC ramstage/soc/intel/common/block/hda/hda.o
src/soc/intel/common/block/irq/irq.c: In function 'assign_fixed_pirqs':
src/soc/intel/common/block/irq/irq.c:186:90: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
186 | printk(BIOS_ERR, "ERROR: Slot %u, pirq %u, no pin for function %lu\n",
| ~~^
| |
| long unsigned int
| %u
187 | constraints->slot, fixed_pirq, i);
| ~
| |
| size_t {aka unsigned int}
CC ramstage/soc/intel/common/block/gspi/gspi.o
CC ramstage/soc/intel/common/block/graphics/graphics.o
CC ramstage/soc/intel/common/block/gpio/gpio.o
CC ramstage/soc/intel/common/block/gpio/gpio_dev.o
Change-Id: I09f4a8d22a2964471344f5dcf971dfa801555f4a
Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de>
---
M src/soc/intel/common/block/irq/irq.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/59056/1
diff --git a/src/soc/intel/common/block/irq/irq.c b/src/soc/intel/common/block/irq/irq.c
index 00aff62..c913c5a 100644
--- a/src/soc/intel/common/block/irq/irq.c
+++ b/src/soc/intel/common/block/irq/irq.c
@@ -183,7 +183,7 @@
fixed pin */
const enum pci_pin pin = fn_pin_map[i];
if (pin == PCI_INT_NONE) {
- printk(BIOS_ERR, "ERROR: Slot %u, pirq %u, no pin for function %lu\n",
+ printk(BIOS_ERR, "ERROR: Slot %u, pirq %u, no pin for function %zu\n",
constraints->slot, fixed_pirq, i);
return false;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/59056
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I09f4a8d22a2964471344f5dcf971dfa801555f4a
Gerrit-Change-Number: 59056
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: newchange
Attention is currently required from: Felix Singer, Sean Rhodes, Paul Menzel, Patrick Rudolph.
Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58798 )
Change subject: soc/intel/tigerlake/apci: Only use SCM for ChromeOS
......................................................................
Patch Set 11:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58798/comment/9c4a6a3e_efb7e717
PS11, Line 15: Orpy8
oryp8
--
To view, visit https://review.coreboot.org/c/coreboot/+/58798
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib947c3c9cd843e54d4664509c15336178c0bc99e
Gerrit-Change-Number: 58798
Gerrit-PatchSet: 11
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 09 Nov 2021 20:54:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Paul Menzel, Patrick Rudolph.
Hello build bot (Jenkins), Tim Crawford, Jeremy Soller, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58798
to look at the new patch set (#11).
Change subject: soc/intel/tigerlake/apci: Only use SCM for ChromeOS
......................................................................
soc/intel/tigerlake/apci: Only use SCM for ChromeOS
Software Connection Manager doesn't work with Linux 5.13 or later and
results in TBT ports timing out. Not advertising this results in
Firmware Connection Manager being used and TBT works correctly.
Tested on:
* StarBook Mk V
* System76 Orpy8
Signed-off-by: Sean Rhodes <sean(a)starlabs.systems>
Change-Id: Ib947c3c9cd843e54d4664509c15336178c0bc99e
---
M src/soc/intel/tigerlake/acpi/tcss.asl
1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/58798/11
--
To view, visit https://review.coreboot.org/c/coreboot/+/58798
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib947c3c9cd843e54d4664509c15336178c0bc99e
Gerrit-Change-Number: 58798
Gerrit-PatchSet: 11
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset