Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39104 )
Change subject: mb/asrock/h110m: add libgfxinit support
......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39104/10//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/39104/10//COMMIT_MSG@7
PS10, Line 7: mb/asrock/h110m: add libgfxinit support
> The board contains a VGA connector, but it doesn't work with libgfx. […]
It probably doesn't work because it is wired through a DP-to-VGA converter. This means that some DPx port must be enabled. If you use `xrandr` on Linux (with the X server) the video outputs have a name.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39104
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4d87413f87d00986111ecb7b046423ff5eac1bf1
Gerrit-Change-Number: 39104
Gerrit-PatchSet: 11
Gerrit-Owner: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 02 Mar 2020 23:26:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38978 )
Change subject: [WIP] mainboard: Add Acer ES1-572
......................................................................
Patch Set 4:
(4 comments)
> Patch Set 4: Code-Review+1
>
> (4 comments)
I'll take care of the comments someday. I would prefer to fix leaf blower syndrome first, though.
https://review.coreboot.org/c/coreboot/+/38978/4//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/38978/4//COMMIT_MSG@7
PS4, Line 7: mainboard
> mb/acer:
The "acer" folder is not there just yet (or at least it wasn't last time I checked), so the change has to add it. It's not a big deal.
https://review.coreboot.org/c/coreboot/+/38978/4//COMMIT_MSG@19
PS4, Line 19: tianocore
> TianoCore
But its folder is named "tianocore" ?
(I just can't stand the capitalized spelling)
https://review.coreboot.org/c/coreboot/+/38978/4//COMMIT_MSG@19
PS4, Line 19: Linux
> Which version?
Honestly, I have no idea.
Since my OCD loathes that question, I'll just put "Arch Linux", which has no version numbers >:)
I mean, I don't want to have to keep updating that number while I wait for my executive function to recharge.
https://review.coreboot.org/c/coreboot/+/38978/4//COMMIT_MSG@24
PS4, Line 24: need
> needs?
It's actually plural: (things that) are not working or need testing.
--
To view, visit https://review.coreboot.org/c/coreboot/+/38978
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id98788a2c5e54f70fd1cacbd70d636f5e63b2619
Gerrit-Change-Number: 38978
Gerrit-PatchSet: 4
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Comment-Date: Mon, 02 Mar 2020 23:20:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: comment
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36830 )
Change subject: sc7180: Add I2C driver
......................................................................
Patch Set 27: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/36830
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I61221ffff8afe5c7ede5abb9e194e242ab0274d8
Gerrit-Change-Number: 36830
Gerrit-PatchSet: 27
Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: Satya Priya Kakitapalli <c_skakit(a)qualcomm.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org>
Gerrit-CC: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Roja Rani Yarubandi <c_rojay(a)qualcomm.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 02 Mar 2020 23:06:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35500 )
Change subject: sc7180: Add UART support
......................................................................
Patch Set 40: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/35500/39/src/soc/qualcomm/sc7180/q…
File src/soc/qualcomm/sc7180/qupv3_uart.c:
https://review.coreboot.org/c/coreboot/+/35500/39/src/soc/qualcomm/sc7180/q…
PS39, Line 157: #ifndef __PRE_RAM__
> This macro has been removed, so this #ifndef does nothing anymore. Just remove it.
Done
https://review.coreboot.org/c/coreboot/+/35500/39/src/soc/qualcomm/sc7180/q…
PS39, Line 165: serial.regwidth = 4;
> Should add a […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/35500
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6494daa108197c030577ac86dab71f9ca6c21bdb
Gerrit-Change-Number: 35500
Gerrit-PatchSet: 40
Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org>
Gerrit-CC: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Roja Rani Yarubandi <c_rojay(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Satya Priya Kakitapalli <c_skakit(a)qualcomm.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 02 Mar 2020 23:05:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35499 )
Change subject: sc7180: Add QUPv3 FW load & config
......................................................................
Patch Set 40:
(9 comments)
Please make sure you always address all comments -- we cannot submit patches unless they pass CI and all comments are resolved in Gerrit. (You can click on "Comment Threads" to easily see all still unresolved comments across all patch sets.)
https://review.coreboot.org/c/coreboot/+/35499/39/src/soc/qualcomm/sc7180/b…
File src/soc/qualcomm/sc7180/bootblock.c:
https://review.coreboot.org/c/coreboot/+/35499/39/src/soc/qualcomm/sc7180/b…
PS39, Line 20: #include <soc/qi2s.h>
> > src/soc/qualcomm/sc7180/bootblock.c:20:10: fatal error: soc/qi2s.h: No such file or directory […]
*ping*
Still outstanding.
https://review.coreboot.org/c/coreboot/+/35499/39/src/soc/qualcomm/sc7180/i…
File src/soc/qualcomm/sc7180/include/soc/qcom_qup_se.h:
https://review.coreboot.org/c/coreboot/+/35499/39/src/soc/qualcomm/sc7180/i…
PS39, Line 471: int size);
> Should fit on one 96 characters line.
I'd say before the rest of the codebase is transitioned to 96 characters we shouldn't force new submissions to use it.
https://review.coreboot.org/c/coreboot/+/35499/40/src/soc/qualcomm/sc7180/i…
File src/soc/qualcomm/sc7180/include/soc/qupv3_config.h:
https://review.coreboot.org/c/coreboot/+/35499/40/src/soc/qualcomm/sc7180/i…
PS40, Line 54: #define QUPV3_UART_SRC_HZ 7372800
Again, this should also be used for where 7372800 appears in clock.c. You probably want to define it in <soc/clock.h>.
https://review.coreboot.org/c/coreboot/+/35499/39/src/soc/qualcomm/sc7180/q…
File src/soc/qualcomm/sc7180/qupv3_config.c:
https://review.coreboot.org/c/coreboot/+/35499/39/src/soc/qualcomm/sc7180/q…
PS39, Line 89: if (mode == GSI) {
> In earlier versions the UARTs used 'mode = FIFO', but with this patch they'll also be MIXED. […]
Looks like you did this? (Please make sure to always mark comments you addressed as "Done", Gerrit won't let us submit a patch until all comments are resolved.)
https://review.coreboot.org/c/coreboot/+/35499/39/src/soc/qualcomm/sc7180/q…
PS39, Line 174: qup_load_se_firmware
> Merged two functions.
Done
https://review.coreboot.org/c/coreboot/+/35499/40/src/soc/qualcomm/sc7180/q…
File src/soc/qualcomm/sc7180/qupv3_config.c:
https://review.coreboot.org/c/coreboot/+/35499/40/src/soc/qualcomm/sc7180/q…
PS40, Line 22: size_t fw_size;
You don't need this, so you can just pass NULL to cbfs_boot_map_with_leak().
https://review.coreboot.org/c/coreboot/+/35499/40/src/soc/qualcomm/sc7180/q…
PS40, Line 31: switch (protocol) {
How about just
const char * filename[] = {
[SE_PROTOCOL_SPI] = "fallback/spi_fw",
[SE_PROTOCOL_UART] = "fallback/uart_fw",
[SE_PROTOCOL_I2C] = "fallback/i2c_fw",
}
if (fw_list[protocol] >= SE_PROTOCOL_MAX || !filename[protocol])
die(...);
if (!fw_list[protocol]) {
fw_list[protocol] = cbfs_boot_map_with_leak(filename[protocol], CBFS_TYPE_RAW, NULL);
if (!fw_list[protocol])
die(...);
}
https://review.coreboot.org/c/coreboot/+/35499/40/src/soc/qualcomm/sc7180/q…
PS40, Line 51: assert(protocol < SE_PROTOCOL_MAX);
There are actually other protocols you defined that are not in this list (i.e. I3C). I think it would be best to just die() here.
https://review.coreboot.org/c/coreboot/+/35499/40/src/soc/qualcomm/sc7180/q…
PS40, Line 75: /* To maintain Div=4 for QcLib, configure clock to 7372800Hz for sc7180 */
nit: QcLib really shouldn't reconfigure any UART registers to begin with when being called from coreboot. It would be good if we could fix that from the QcLib side.
--
To view, visit https://review.coreboot.org/c/coreboot/+/35499
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4d91dd10488931247f81a87b0bdcc598f4bceb31
Gerrit-Change-Number: 35499
Gerrit-PatchSet: 40
Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org>
Gerrit-Reviewer: Doug Anderson <dianders(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org>
Gerrit-CC: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Roja Rani Yarubandi <c_rojay(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Satya Priya Kakitapalli <c_skakit(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Stephen Boyd <swboyd(a)chromium.org>
Gerrit-Comment-Date: Mon, 02 Mar 2020 23:04:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Roja Rani Yarubandi <c_rojay(a)qualcomm.corp-partner.google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39168 )
Change subject: vendorcode/intel/fsp/fsp2_0/tgl: Update FSP header for Tiger Lake
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/39168
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I063f921832a4e4a45eb6978b6dbb37b1ac7dde7f
Gerrit-Change-Number: 39168
Gerrit-PatchSet: 1
Gerrit-Owner: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Comment-Date: Mon, 02 Mar 2020 21:18:02 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37121 )
Change subject: soc/intel/denverton/uart.c: Clean up code
......................................................................
soc/intel/denverton/uart.c: Clean up code
Since there is only one device ID used for UART,
an array is not needed. Therefore, just save the
device ID to the device variable.
Change-Id: Icd325e1102a85cc175f6025519a47a1b64ee5b46
Signed-off-by: Felix Singer <felix.singer(a)9elements.com>
---
M src/soc/intel/denverton_ns/uart.c
1 file changed, 1 insertion(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/37121/1
diff --git a/src/soc/intel/denverton_ns/uart.c b/src/soc/intel/denverton_ns/uart.c
index 28e0e2e..3b851ee 100644
--- a/src/soc/intel/denverton_ns/uart.c
+++ b/src/soc/intel/denverton_ns/uart.c
@@ -57,15 +57,10 @@
.enable = DEVICE_NOOP
};
-static const unsigned short uart_ids[] = {
- PCI_DEVICE_ID_INTEL_DENVERTON_HSUART,
- 0
-};
-
static const struct pci_driver uart_driver __pci_driver = {
.ops = &uart_ops,
.vendor = PCI_VENDOR_ID_INTEL,
- .devices = uart_ids
+ .device = PCI_DEVICE_ID_INTEL_DENVERTON_HSUART
};
static void hide_hsuarts(void)
--
To view, visit https://review.coreboot.org/c/coreboot/+/37121
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icd325e1102a85cc175f6025519a47a1b64ee5b46
Gerrit-Change-Number: 37121
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: newchange