coreboot-gerrit June 2019

coreboot-gerrit@coreboot.org

1 participants
1454 discussions

Change in ...coreboot[master]: mb/upsquared: Align partitions with 4K
by Felix Singer (Code Review) 25 Jun '19

25 Jun '19

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33784 ) Change subject: mb/upsquared: Align partitions with 4K ...................................................................... Patch Set 2: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/33784 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If1e99b74ded5f743a3ad4fc829ae9747276c483a Gerrit-Change-Number: 33784 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 25 Jun 2019 12:27:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mb/upsquared: Fix partition alignment
by Felix Singer (Code Review) 25 Jun '19

25 Jun '19

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33784 ) Change subject: mb/upsquared: Fix partition alignment ...................................................................... Patch Set 1: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/33784 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If1e99b74ded5f743a3ad4fc829ae9747276c483a Gerrit-Change-Number: 33784 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 25 Jun 2019 12:18:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mainboard/facebook/fbg1701: Rename tc348860_table
by Frans Hendriks (Code Review) 25 Jun '19

25 Jun '19

Frans Hendriks has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33736 Change subject: mainboard/facebook/fbg1701: Rename tc348860_table ...................................................................... mainboard/facebook/fbg1701: Rename tc348860_table tc348860_table contains the eDP to MIPI Bridge controller type. b101uan08_table used the LCD Panel type. Use LCD Panel type for name of tables. Remove the incomplete resolution comments and specify the resolution at the startof the table to 1200x1920. BUG=N/A TEST=Config eDP and verify LCD panels are working on Facebook FBG-1701 Change-Id: Ic152ea1f95f155ab76638b57a259d37ce6f43037 Signed-off-by: Frans Hendriks <fhendriks(a)eltan.com> --- M src/mainboard/facebook/fbg1701/ramstage.c 1 file changed, 4 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/33736/1 diff --git a/src/mainboard/facebook/fbg1701/ramstage.c b/src/mainboard/facebook/fbg1701/ramstage.c index 3903a64..5c78bcc 100644 --- a/src/mainboard/facebook/fbg1701/ramstage.c +++ b/src/mainboard/facebook/fbg1701/ramstage.c @@ -28,8 +28,8 @@ u8 data[6]; } __packed; -static const struct edp_data tc348860_table[] = { - /* set eDP bridge to eDP 1920 */ +static const struct edp_data b101uan01_table[] = { + /* set eDP bridge to 1200x1920 */ /* IO */ { 6, 0x68, { 0x08, 0x00, 0x01, 0x00, 0x00, 0x00 } }, /* Boot */ @@ -173,7 +173,7 @@ }; static const struct edp_data b101uan08_table[] = { - /* set eDP bridge to eDP 1920 */ + /* set eDP bridge to 1200x1920 */ /* IO Voltage Setting */ { 6, 0x68, { 0x08, 0x00, 0x01, 0x00, 0x00, 0x00 } }, /* Boot Settings */ @@ -333,7 +333,7 @@ CPLD_PCB_VERSION_BIT; printk(BIOS_DEBUG, "CPLD version: %x\n", cpld_version); if (cpld_version < 7) - edptable = tc348860_table; + edptable = b101uan01_table; else edptable = b101uan08_table; @@ -341,7 +341,6 @@ outb(CPLD_CMD_RESET_DSI_BRIDGE_ACTIVE, CPLD_RESET_PORT); outb(CPLD_CMD_RESET_DSI_BRIDGE_INACTIVE, CPLD_RESET_PORT); - /* set eDP bridge to eDP 1920 */ while (edptable->payload_length) { loops = 5; do { @@ -360,7 +359,6 @@ void mainboard_silicon_init_params(SILICON_INIT_UPD *params) { - /* Configure the eDP bridge to eDP 1920 */ mainboard_configure_edp_bridge(); if (CONFIG(FSP1_1_DISPLAY_LOGO)) { -- To view, visit https://review.coreboot.org/c/coreboot/+/33736 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic152ea1f95f155ab76638b57a259d37ce6f43037 Gerrit-Change-Number: 33736 Gerrit-PatchSet: 1 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-MessageType: newchange

4 6

Change in ...coreboot[master]: sdm845: Port I2C driver
by Akash Asthana (Code Review) 25 Jun '19

25 Jun '19

Akash Asthana has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29104 ) Change subject: sdm845: Port I2C driver ...................................................................... Patch Set 35: (2 comments) https://review.coreboot.org/#/c/29104/35/src/mainboard/google/cheza/mainboa… File src/mainboard/google/cheza/mainboard.c: https://review.coreboot.org/#/c/29104/35/src/mainboard/google/cheza/mainboa… PS35, Line 46: i2c_init(QUPV3_1_SE4, I2C_SPEED_FAST); > nit: Please add a comment on the side explaining what this is (e.g. […] Done https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/i2c.c File src/soc/qualcomm/sdm845/i2c.c: https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/i2c.c@158 PS35, Line 158: /* If stretch bit is set to 1 master will send repeated start > Incorrect comment style, please use […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/29104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifb76564d8a11427423dd14d8ba7c8c7d500ef346 Gerrit-Change-Number: 29104 Gerrit-PatchSet: 35 Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org> Gerrit-Reviewer: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 25 Jun 2019 11:04:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mb/lenovo/z61t: Remove `fn_ctrl_swap` option
by Peter Lemenkov (Code Review) 25 Jun '19

25 Jun '19

Hello Nico Huber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/33364 to review the following change. Change subject: mb/lenovo/z61t: Remove `fn_ctrl_swap` option ...................................................................... mb/lenovo/z61t: Remove `fn_ctrl_swap` option It seems that the EC on t60/x60 and modifications doesn't support it. This wasn't even introduced in z61t so let's remove the remaining bits. This commit follows up on commit a5fcc2e4 with Change-Id Id2964002406a5fcf992f0ffc3627e3f66a2bb13f ("mb/lenovo/x60/t60: Remove `fn_ctrl_swap` option"). Untested. Change-Id: Ifd5e7823af305cc4a0194ee2097a749e43680c55 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Signed-off-by: Peter Lemenkov <lemenkov(a)gmail.com> --- M src/mainboard/lenovo/z61t/cmos.layout 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/33364/1 diff --git a/src/mainboard/lenovo/z61t/cmos.layout b/src/mainboard/lenovo/z61t/cmos.layout index b9e2b44..75ad427 100644 --- a/src/mainboard/lenovo/z61t/cmos.layout +++ b/src/mainboard/lenovo/z61t/cmos.layout @@ -78,7 +78,7 @@ 952 8 h 0 volume 960 1 e 9 first_battery 961 1 e 1 trackpoint -962 1 e 1 fn_ctrl_swap +#962 1 r 0 unused 963 1 e 1 sticky_fn 964 1 e 1 power_management_beeps 965 1 e 1 low_battery_beep -- To view, visit https://review.coreboot.org/c/coreboot/+/33364 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd5e7823af305cc4a0194ee2097a749e43680c55 Gerrit-Change-Number: 33364 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Lemenkov <lemenkov(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

3 7

Change in ...coreboot[master]: mb/up/squared: Remove unnecessary code
by Patrick Rudolph (Code Review) 25 Jun '19

25 Jun '19

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33136 ) Change subject: mb/up/squared: Remove unnecessary code ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/33136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If7dae4f24a9fcb01d2d47063dd3a0f4ce6c120d2 Gerrit-Change-Number: 33136 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(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: Tue, 25 Jun 2019 07:41:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mb/up/squared: Remove unnecessary code
by Felix Singer (Code Review) 25 Jun '19

25 Jun '19

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33136 ) Change subject: mb/up/squared: Remove unnecessary code ...................................................................... Patch Set 6: (9 comments) This change is ready for review. https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c File src/mainboard/up/squared/ramstage.c: https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c… PS5, Line 32: silconfig->IpuEn = 0x0; // 0x1 > set to zero by soc if ipu pci device is set to off in devicetree Done https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c… PS5, Line 46: silconfig->IshEnable = 0x0; // 0x1 > set to zero by soc if ish pci device is set to off in devicetree Done https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c… PS5, Line 47: silconfig->SpiEiss = 0x0; // 0x1 > set by soc tp zero Done https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c… PS5, Line 49: silconfig->SdcardEnabled = 0x0; // 0x1 > set to zero by soc if sdcard pci device is set to off in devicetree Done https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c… PS5, Line 52: silconfig->VtdEnable = 0x1; // 0x0 > set by soc if enable_vtd is set in devicetree Done https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c… PS5, Line 53: silconfig->MonitorMwaitEnable = 0x0; // 0x1 > set by soc code Done https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c… PS5, Line 57: silconfig->PcieRootPortEn[0] = 0x1; // 0x0 > set by soc if PCIe device in devicetree Done https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c… PS5, Line 62: silconfig->PcieRpHotPlug[1] = 0x0; // 0x1 > set by soc to register "pcie_rp_hotplug_enable" in devicetree I would leave this here, since most of the PCIe root port options are here and thus it's less fragmented. https://review.coreboot.org/#/c/33136/5/src/mainboard/up/squared/ramstage.c… PS5, Line 65: silconfig->PcieRpClkReqNumber[1] = 0x3; // 0x5 > set by soc if register "pcie_rp_clkreq_pin" is in devicetree Same -- To view, visit https://review.coreboot.org/c/coreboot/+/33136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If7dae4f24a9fcb01d2d47063dd3a0f4ce6c120d2 Gerrit-Change-Number: 33136 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 25 Jun 2019 06:26:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sdm845: Add QUPv3 FW load & config
by Julius Werner (Code Review) 25 Jun '19

25 Jun '19

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25372 ) Change subject: sdm845: Add QUPv3 FW load & config ...................................................................... Patch Set 75: (8 comments) https://review.coreboot.org/#/c/25372/75/src/mainboard/google/cheza/mainboa… File src/mainboard/google/cheza/mainboard.c: https://review.coreboot.org/#/c/25372/75/src/mainboard/google/cheza/mainboa… PS75, Line 52: write32((void *)0x11874c, 0x21); Note that this can't stay here like this. I forgot what it does by now, but if it actually needs to be done in firmware we need to find a way to do it cleanly. (You can mvoe it into a separate HACK patch for now if you don't want to block this one.) https://review.coreboot.org/#/c/25372/6/src/mainboard/google/cheza/qupv3_co… File src/mainboard/google/cheza/qupv3_config.c: https://review.coreboot.org/#/c/25372/6/src/mainboard/google/cheza/qupv3_co… PS6, Line 18: struct se_cfg se_mappings[QUPV3_SE_MAX] = > In SDM845, we have two QUPV3 wrappers HW, each wrapper QUPV3 wrapper HW has 8 Serial engines. […] Whoops, sorry for missing this. So you're concerned about the sequence in fw_init_qup_common()? How about we just call that from sdm845/bootblock.c but have the rest of the QUP initialization triggered by the individual drivers that need it? As far as I can tell, init_and_load_se_firmware() only touches a single QUP's register space. That's the part we should run on-demand. https://review.coreboot.org/#/c/25372/75/src/soc/qualcomm/sdm845/include/so… File src/soc/qualcomm/sdm845/include/soc/qupv3_config.h: https://review.coreboot.org/#/c/25372/75/src/soc/qualcomm/sdm845/include/so… PS75, Line 23: #endif // _SDM845_QUPV3_CONFIG_H_ Please use /* C89 */ instead of // C99 comments (please fix globally). https://review.coreboot.org/#/c/25372/75/src/soc/qualcomm/sdm845/qupv3_fw_c… File src/soc/qualcomm/sdm845/qupv3_fw_config.c: https://review.coreboot.org/#/c/25372/75/src/soc/qualcomm/sdm845/qupv3_fw_c… PS75, Line 166: static uintptr_t qupv3_se_base_addr[QUPV3_SE_MAX] = { You should be using the global 'qup' array we introduced for the SPI and I2C drivers instead of this now. https://review.coreboot.org/#/c/25372/75/src/soc/qualcomm/sdm845/qupv3_fw_c… PS75, Line 200: reg_value = read32(se_base + GENI_OUTPUT_CTRL_REG) & All these register accesses should be using the struct qup_regs that we introduced for the SPI/I2C drivers now. https://review.coreboot.org/#/c/25372/75/src/soc/qualcomm/sdm845/qupv3_fw_c… PS75, Line 396: reg_value = read32((void *)qupv3_common_base_list[i] + For all these register accesses you should make a new struct overlay for the common part of the QUPV3_WRAP. https://review.coreboot.org/#/c/25372/75/src/soc/qualcomm/sdm845/qupv3_fw_c… PS75, Line 425: write32((void *)0x0015200C + 0, 0x3FFFFFC0); All accesses to GCC registers should be made from clock.c. https://review.coreboot.org/#/c/25372/75/src/soc/qualcomm/sdm845/qupv3_fw_c… PS75, Line 427: fw_init_qup_common(); Can we change this to something like qup_common_init(qup_wrap0); qup_common_init(qup_wrap1); to avoid having to loop over a 2-entry array? -- To view, visit https://review.coreboot.org/c/coreboot/+/25372 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6e87f868ecbe2a8e51d94c045ad76b99bb1b345d Gerrit-Change-Number: 25372 Gerrit-PatchSet: 75 Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mukesh Savaliya <msavaliy(a)qualcomm.corp-partner.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-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-Comment-Date: Tue, 25 Jun 2019 01:10:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sdm845: Add UART support
by Julius Werner (Code Review) 25 Jun '19

25 Jun '19

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25373 ) Change subject: sdm845: Add UART support ...................................................................... Patch Set 75: (2 comments) https://review.coreboot.org/#/c/25373/74/src/soc/qualcomm/sdm845/Kconfig File src/soc/qualcomm/sdm845/Kconfig: https://review.coreboot.org/#/c/25373/74/src/soc/qualcomm/sdm845/Kconfig@40 PS74, Line 40: default 9 > Yes, 9 is the fixed QUP instance for console UART on SDM845 products. Okay, in that case let's leave this here and remove it from cheza/Kconfig (so Cheza will just use this default). Other boards can still override it if necessary. https://review.coreboot.org/#/c/25373/74/src/soc/qualcomm/sdm845/uart.c File src/soc/qualcomm/sdm845/uart.c: https://review.coreboot.org/#/c/25373/74/src/soc/qualcomm/sdm845/uart.c@146 PS74, Line 146: write32(&regs->geni_m_cmd0, 0x08000000); > In UART protocol, we write 0x8000000 to the main engine to start TX transfers and the same to second […] Sounds good. -- To view, visit https://review.coreboot.org/c/coreboot/+/25373 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I827906e820bc15b7f60fdd7876a54c9ed36a48a1 Gerrit-Change-Number: 25373 Gerrit-PatchSet: 75 Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mukesh Savaliya <msavaliy(a)qualcomm.corp-partner.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-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-Comment-Date: Tue, 25 Jun 2019 00:47:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sdm845: Port I2C driver
by Julius Werner (Code Review) 25 Jun '19

25 Jun '19

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29104 ) Change subject: sdm845: Port I2C driver ...................................................................... Patch Set 35: (5 comments) Still has some issues that look like merge mistakes. Clean those up and rebase to the front? https://review.coreboot.org/#/c/29104/35/src/mainboard/google/cheza/mainboa… File src/mainboard/google/cheza/mainboard.c: https://review.coreboot.org/#/c/29104/35/src/mainboard/google/cheza/mainboa… PS35, Line 46: i2c_init(QUPV3_1_SE4, I2C_SPEED_FAST); nit: Please add a comment on the side explaining what this is (e.g. /* speaker amp */) https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/i2c.c File src/soc/qualcomm/sdm845/i2c.c: https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/i2c.c@158 PS35, Line 158: /* If stretch bit is set to 1 master will send repeated start Incorrect comment style, please use /* * This * one */ or /* This one */ (or just shorten it, all this needs to say is something like /* stretch means end with repeated start, not stop */) https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/include/so… File src/soc/qualcomm/sdm845/include/soc/qupv3_fw_config.h: https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/include/so… PS35, Line 20: #include <soc/qcom_qup_se.h> This should not be in this patch, something needs to be cleaned up earlier in the series. https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/qupv3_fw_c… File src/soc/qualcomm/sdm845/qupv3_fw_config.c: https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/qupv3_fw_c… PS35, Line 60: ??? https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/spi_qup.c File src/soc/qualcomm/sdm845/spi_qup.c: https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/spi_qup.c@… PS35, Line 49: ??? -- To view, visit https://review.coreboot.org/c/coreboot/+/29104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifb76564d8a11427423dd14d8ba7c8c7d500ef346 Gerrit-Change-Number: 29104 Gerrit-PatchSet: 35 Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org> Gerrit-Reviewer: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 25 Jun 2019 00:44:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2019