coreboot-gerrit December 2021

coreboot-gerrit@coreboot.org

1 participants
5525 discussions

Change in coreboot[master]: mb/facebook/fbg1701: Remove ONBOARD_SAMSUNG_MEM
by Frans Hendriks (Code Review) 02 Dec '21

02 Dec '21

Attention is currently required from: Michał Żygowski, Frans Hendriks, Arthur Heymans, Wim Vervoorn. Hello build bot (Jenkins), Michał Żygowski, Angel Pons, Arthur Heymans, Wim Vervoorn, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59754 to look at the new patch set (#3). Change subject: mb/facebook/fbg1701: Remove ONBOARD_SAMSUNG_MEM ...................................................................... mb/facebook/fbg1701: Remove ONBOARD_SAMSUNG_MEM CONFIG_ONBOARD_SAMSUMG_MEM was used to force Samsung memory. CPLD returns different values for every board revision. Use this value to determine the memory type. BUG = N/A TEST = Boot Facebook FBG1701 Rev 1.0 - 1.4 Change-Id: I21b5ddc430410a1e8b3e9012d0c07d278880ff47 Signed-off-by: Frans Hendriks <fhendriks(a)eltan.com> --- M src/mainboard/facebook/fbg1701/romstage.c 1 file changed, 17 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/59754/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/59754 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21b5ddc430410a1e8b3e9012d0c07d278880ff47 Gerrit-Change-Number: 59754 Gerrit-PatchSet: 3 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Wim Vervoorn <wvervoorn(a)eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Wim Vervoorn <wvervoorn(a)eltan.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/mediatek: Add PCIe support
by Yu-Ping Wu (Code Review) 02 Dec '21

02 Dec '21

Attention is currently required from: Hung-Te Lin, Furquan Shaikh, Jianjun Wang. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56791 ) Change subject: soc/mediatek: Add PCIe support ...................................................................... Patch Set 6: (1 comment) File src/soc/mediatek/common/pcie.c: https://review.coreboot.org/c/coreboot/+/56791/comment/1554e65c_52fefde3 PS6, Line 285: ret = mtk_pcie_startup_port(ctrl); : if (ret) : return; Rewrite as if (mtk_pcie_startup_port(ctrl) < 0) return; -- To view, visit https://review.coreboot.org/c/coreboot/+/56791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9b6adaafa20aeee136372ec9564273f86776da0 Gerrit-Change-Number: 56791 Gerrit-PatchSet: 6 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com> Gerrit-Comment-Date: Thu, 02 Dec 2021 08:58:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Add Kconfigs for all PCH types
by Usha P (Code Review) 02 Dec '21

02 Dec '21

Attention is currently required from: Usha P, Patrick Rudolph. Usha P has uploaded a new patch set (#3) to the change originally created by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/59804 ) Change subject: soc/intel/alderlake: Add Kconfigs for all PCH types ...................................................................... soc/intel/alderlake: Add Kconfigs for all PCH types The Alder Lake code currently supports the PCH-M and PCH-P types, which have some differences (so far, only the amount of PCIe I/O). Mainboards can use the `SOC_INTEL_ALDERLAKE_PCH_M` Kconfig option to specify which PCH type they use: select the option to choose PCH-M, do not select the option to choose PCH-P. While this works, it can be confusing once more PCH types are added. Introduce the `SOC_INTEL_ALDERLAKE_PCH_P` Kconfig option so that boards have to explicitly choose a PCH type. Also, use this option to restrict the PCH-P defaults for PCH-dependent settings to avoid unintended reuse of the PCH-P defaults when adding a new PCH type. To make sure only one PCH type is selected, add some preprocessor in `bootblock.h` to provoke a build-time error if this requirement is not met. Kconfig doesn't seem to have a mechanism to describe sets of mutually-exclusive bool options that allows said options to be selected (a `choice` block doesn't allow its elements to be selected). Finally, adapt the ADL boards accordingly. Change-Id: I7deca820e08ce2b5a220f3c97a511a4f3464a976 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/mainboard/google/brya/Kconfig M src/mainboard/intel/adlrvp/Kconfig M src/mainboard/intel/shadowmountain/Kconfig M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/include/soc/bootblock.h 5 files changed, 26 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/59804/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/59804 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7deca820e08ce2b5a220f3c97a511a4f3464a976 Gerrit-Change-Number: 59804 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.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: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Usha P <usha.p(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/alderlake: Add support for ADL-N PCH
by Usha P (Code Review) 02 Dec '21

02 Dec '21

Attention is currently required from: Reka Norman, Paul Menzel, Rizwan Qureshi, Subrata Banik, Angel Pons, Krishna P Bhat D, Patrick Rudolph, Kangheui Won. Hello build bot (Jenkins), Reka Norman, Maulik V Vaghela, Rizwan Qureshi, Tim Wawrzynczak, Angel Pons, Subrata Banik, Sridhar Siricilla, Krishna P Bhat D, Patrick Rudolph, Kangheui Won, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59752 to look at the new patch set (#5). Change subject: soc/intel/alderlake: Add support for ADL-N PCH ...................................................................... soc/intel/alderlake: Add support for ADL-N PCH Introduce the `SOC_INTEL_ALDERLAKE_PCH_N` Kconfig option and use it to specify the correct amount of PCIe I/O. Document number 645550 indicates that Alder Lake-N has 12 PCH root ports and no CPU root ports. Document number 645548 indicates ADL-N has 5 clock sources and 5 clock request signals. Signed-off-by: Usha P <usha.p(a)intel.com> Change-Id: I7ebbcdcdb1ccc34b80ec71ac3e591fe4ad6b1904 --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/include/soc/bootblock.h 2 files changed, 12 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/59752/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/59752 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ebbcdcdb1ccc34b80ec71ac3e591fe4ad6b1904 Gerrit-Change-Number: 59752 Gerrit-PatchSet: 5 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Kangheui Won <khwon(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(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: Reka Norman <rekanorman(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Reka Norman <rekanorman(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Kangheui Won <khwon(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/alderlake: Add Kconfigs for all PCH types
by Usha P (Code Review) 02 Dec '21

02 Dec '21

Attention is currently required from: Angel Pons, Usha P, Patrick Rudolph. Usha P has uploaded a new patch set (#2) to the change originally created by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/59804 ) Change subject: soc/intel/alderlake: Add Kconfigs for all PCH types ...................................................................... soc/intel/alderlake: Add Kconfigs for all PCH types The Alder Lake code currently supports the PCH-M and PCH-P types, which have some differences (so far, only the amount of PCIe I/O). Mainboards can use the `SOC_INTEL_ALDERLAKE_PCH_M` Kconfig option to specify which PCH type they use: select the option to choose PCH-M, do not select the option to choose PCH-P. While this works, it can be confusing once more PCH types are added. Introduce the `SOC_INTEL_ALDERLAKE_PCH_P` Kconfig option so that boards have to explicitly choose a PCH type. Also, use this option to restrict the PCH-P defaults for PCH-dependent settings to avoid unintended reuse of the PCH-P defaults when adding a new PCH type. To make sure only one PCH type is selected, add some preprocessor in `bootblock.h` to provoke a build-time error if this requirement is not met. Kconfig doesn't seem to have a mechanism to describe sets of mutually-exclusive bool options that allows said options to be selected (a `choice` block doesn't allow its elements to be selected). Finally, adapt the ADL boards accordingly. Change-Id: I7deca820e08ce2b5a220f3c97a511a4f3464a976 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/mainboard/google/brya/Kconfig M src/mainboard/intel/adlrvp/Kconfig M src/mainboard/intel/shadowmountain/Kconfig M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/include/soc/bootblock.h 5 files changed, 26 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/59804/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59804 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7deca820e08ce2b5a220f3c97a511a4f3464a976 Gerrit-Change-Number: 59804 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.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: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Usha P <usha.p(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: lib: Add fls() (Find Last Set)
by Yu-Ping Wu (Code Review) 02 Dec '21

02 Dec '21

Attention is currently required from: Hung-Te Lin, Shelley Chen, Paul Menzel, Julius Werner, Jianjun Wang. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59738 ) Change subject: lib: Add fls() (Find Last Set) ...................................................................... Patch Set 4: (1 comment) File src/include/lib.h: https://review.coreboot.org/c/coreboot/+/59738/comment/943a74e0_3204b46e PS4, Line 56: /* Find Last Set: fls(1) == 1, fls(0) == 0, fls(1 << 31) == 32 */ > Hmmm... this has the same problem that we originally had with ffs(). […] Probably worth mentioning the background in the comment here. I was just wondering why it's called __ffs() instead of ffs(). -- To view, visit https://review.coreboot.org/c/coreboot/+/59738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib458abfec7e03b2979569a8440a6e69b0285ac32 Gerrit-Change-Number: 59738 Gerrit-PatchSet: 4 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com> Gerrit-Comment-Date: Thu, 02 Dec 2021 08:32:32 +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]: lib: Add fls() (Find Last Set)
by Yu-Ping Wu (Code Review) 02 Dec '21

02 Dec '21

Attention is currently required from: Hung-Te Lin, Shelley Chen, Paul Menzel, Jianjun Wang. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59738 ) Change subject: lib: Add fls() (Find Last Set) ...................................................................... Patch Set 4: (1 comment) File src/commonlib/bsd/include/commonlib/bsd/helpers.h: https://review.coreboot.org/c/coreboot/+/59738/comment/36eeea12_ffebabeb PS1, Line 137: fls > See CB:56543 as an example. Tests can be run by […] I've created the unit tests for lib.h in CB:59834. Feel free to rebase this on top of that. -- To view, visit https://review.coreboot.org/c/coreboot/+/59738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib458abfec7e03b2979569a8440a6e69b0285ac32 Gerrit-Change-Number: 59738 Gerrit-PatchSet: 4 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com> Gerrit-Comment-Date: Thu, 02 Dec 2021 08:24:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/analogix/anx7625: Utilize retry() macro
by Hung-Te Lin (Code Review) 02 Dec '21

02 Dec '21

Hung-Te Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59659 ) Change subject: drivers/analogix/anx7625: Utilize retry() macro ...................................................................... drivers/analogix/anx7625: Utilize retry() macro Utilize retry() macro in wait_aux_op_finish() and anx7625_init() to simplify the code. BUG=none TEST=emerge-asurada coreboot BRANCH=none Change-Id: I207e7075e8ac905efd5f201dd54658dedf531568 Signed-off-by: Yu-Ping Wu <yupingso(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59659 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Hung-Te Lin <hungte(a)chromium.org> --- M src/drivers/analogix/anx7625/anx7625.c 1 file changed, 5 insertions(+), 18 deletions(-) Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved diff --git a/src/drivers/analogix/anx7625/anx7625.c b/src/drivers/analogix/anx7625/anx7625.c index 153266c..81f27d0 100644 --- a/src/drivers/analogix/anx7625/anx7625.c +++ b/src/drivers/analogix/anx7625/anx7625.c @@ -6,6 +6,7 @@ #include <edid.h> #include <gpio.h> #include <string.h> +#include <types.h> #include "anx7625.h" @@ -133,20 +134,11 @@ static int wait_aux_op_finish(uint8_t bus) { uint8_t val; - int loop; - int success = 0; int ret; - for (loop = 0; loop < 150; loop++) { - mdelay(2); - anx7625_reg_read(bus, RX_P0_ADDR, AP_AUX_CTRL_STATUS, &val); - if (!(val & AP_AUX_CTRL_OP_EN)) { - success = 1; - break; - } - } - - if (!success) { + if (!retry(150, + (anx7625_reg_read(bus, RX_P0_ADDR, AP_AUX_CTRL_STATUS, &val), + !(val & AP_AUX_CTRL_OP_EN)), mdelay(2))) { ANXERROR("Timed out waiting aux operation.\n"); return -1; } @@ -874,13 +866,8 @@ int anx7625_init(uint8_t bus) { int retry_hpd_change = 50; - int retry_power_on = 3; - while (--retry_power_on) { - if (anx7625_power_on_init(bus) >= 0) - break; - } - if (!retry_power_on) { + if (!retry(3, anx7625_power_on_init(bus) >= 0)) { ANXERROR("Failed to power on.\n"); return -1; } -- To view, visit https://review.coreboot.org/c/coreboot/+/59659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I207e7075e8ac905efd5f201dd54658dedf531568 Gerrit-Change-Number: 59659 Gerrit-PatchSet: 2 Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: drivers/analogix/anx7625: Fix edid_read()
by Hung-Te Lin (Code Review) 02 Dec '21

02 Dec '21

Hung-Te Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59540 ) Change subject: drivers/analogix/anx7625: Fix edid_read() ...................................................................... drivers/analogix/anx7625: Fix edid_read() The current implementations of edid_read() and segments_edid_read() have a few problems: 1. The type of variable `c` is incorrect, not matching the return type of sp_tx_aux_rd(). In addition, the meaning of `c` is unknown. 2. It is pointless to do `cnt++` when sp_tx_aux_rd() fails. 3. These two functions ignore the return value of anx7625_reg_block_read(). 4. In segments_edid_read(), anx7625_reg_write() might return a positive value on failure. Fix all of the 4 issues, and modify the code to be closer to kernel 5.10's implementation (drivers/gpu/drm/bridge/analogix/anx7625.c). Note that, however, unlike in kernel, anx7625_reg_block_read() here doesn't return the number of bytes. On success, 0 is returned instead. In addition, following coreboot's convention, always return negative error codes. In particular, change the return value to -1 for edid_read() and segments_edid_read() on failure. BUG=b:207055969 TEST=emerge-asurada coreboot BRANCH=none Change-Id: Ife9d7d97df2926b4581ba519a152c9efed8cd969 Signed-off-by: Yu-Ping Wu <yupingso(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59540 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Hung-Te Lin <hungte(a)chromium.org> --- M src/drivers/analogix/anx7625/anx7625.c 1 file changed, 76 insertions(+), 68 deletions(-) Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved diff --git a/src/drivers/analogix/anx7625/anx7625.c b/src/drivers/analogix/anx7625/anx7625.c index 11e9ed1..153266c 100644 --- a/src/drivers/analogix/anx7625/anx7625.c +++ b/src/drivers/analogix/anx7625/anx7625.c @@ -54,9 +54,11 @@ } ret = i2c_writeb(bus, saddr, offset, 0x00); - if (ret < 0) + if (ret < 0) { ANXERROR("Failed to access %#x:%#x\n", saddr, offset); - return ret; + return ret; + } + return 0; } static int anx7625_reg_read(uint8_t bus, uint8_t saddr, uint8_t offset, @@ -70,7 +72,7 @@ ANXERROR("Failed to read i2c reg=%#x:%#x\n", saddr, offset); return ret; } - return *val; + return 0; } static int anx7625_reg_block_read(uint8_t bus, uint8_t saddr, uint8_t reg_addr, @@ -80,10 +82,12 @@ i2c_access_workaround(bus, saddr); ret = i2c_read_bytes(bus, saddr, reg_addr, buf, len); - if (ret < 0) + if (ret < 0) { ANXERROR("Failed to read i2c block=%#x:%#x[len=%#x]\n", saddr, reg_addr, len); - return ret; + return ret; + } + return 0; } static int anx7625_reg_write(uint8_t bus, uint8_t saddr, uint8_t reg_addr, @@ -93,10 +97,11 @@ i2c_access_workaround(bus, saddr); ret = i2c_writeb(bus, saddr, reg_addr, reg_val); - if (ret < 0) + if (ret < 0) { ANXERROR("Failed to write i2c id=%#x:%#x\n", saddr, reg_addr); - - return ret; + return ret; + } + return 0; } static int anx7625_write_or(uint8_t bus, uint8_t saddr, uint8_t offset, @@ -128,30 +133,31 @@ static int wait_aux_op_finish(uint8_t bus) { uint8_t val; - int ret = -1; int loop; + int success = 0; + int ret; for (loop = 0; loop < 150; loop++) { mdelay(2); anx7625_reg_read(bus, RX_P0_ADDR, AP_AUX_CTRL_STATUS, &val); if (!(val & AP_AUX_CTRL_OP_EN)) { - ret = 0; + success = 1; break; } } - if (ret != 0) { + if (!success) { ANXERROR("Timed out waiting aux operation.\n"); - return ret; + return -1; } ret = anx7625_reg_read(bus, RX_P0_ADDR, AP_AUX_CTRL_STATUS, &val); if (ret < 0 || val & 0x0F) { ANXDEBUG("aux status %02x\n", val); - ret = -1; + return -1; } - return ret; + return 0; } static unsigned long gcd(unsigned long a, unsigned long b) @@ -210,7 +216,7 @@ ANXERROR("pixelclock %u higher than %lu, " "output may be unstable\n", pixelclock, PLL_OUT_FREQ_ABS_MAX / POST_DIVIDER_MIN); - return 1; + return -1; } if (pixelclock < PLL_OUT_FREQ_ABS_MIN / POST_DIVIDER_MAX) { @@ -218,7 +224,7 @@ ANXERROR("pixelclock %u lower than %lu, " "output may be unstable\n", pixelclock, PLL_OUT_FREQ_ABS_MIN / POST_DIVIDER_MAX); - return 1; + return -1; } post_divider = 1; @@ -237,7 +243,7 @@ if (post_divider > POST_DIVIDER_MAX) { ANXERROR("cannot find property post_divider(%d)\n", post_divider); - return 1; + return -1; } } @@ -256,7 +262,7 @@ if (pixelclock * post_divider > PLL_OUT_FREQ_ABS_MAX) { ANXINFO("act clock(%u) large than maximum(%lu)\n", pixelclock * post_divider, PLL_OUT_FREQ_ABS_MAX); - return 1; + return -1; } *m = pixelclock; @@ -292,10 +298,12 @@ ret |= anx7625_write_or(bus, RX_P1_ADDR, MIPI_DIGITAL_PLL_7, MIPI_PLL_RESET_N); - if (ret < 0) + if (ret < 0) { ANXERROR("IO error.\n"); + return ret; + } - return ret; + return 0; } static int anx7625_dsi_video_config(uint8_t bus, struct display_timing *dt) @@ -305,10 +313,8 @@ int ret; uint8_t post_divider = 0; - ret = anx7625_calculate_m_n(dt->pixelclock * 1000, &m, &n, - &post_divider); - - if (ret != 0) { + if (anx7625_calculate_m_n(dt->pixelclock * 1000, &m, &n, + &post_divider) < 0) { ANXERROR("cannot get property m n value.\n"); return -1; } @@ -385,10 +391,12 @@ ret |= anx7625_odfc_config(bus, post_divider - 1); - if (ret < 0) + if (ret < 0) { ANXERROR("mipi dsi setup IO error.\n"); + return ret; + } - return ret; + return 0; } static int anx7625_swap_dsi_lane3(uint8_t bus) @@ -400,7 +408,7 @@ ret = anx7625_reg_read(bus, RX_P1_ADDR, MIPI_SWAP, &val); if (ret < 0) { ANXERROR("IO error: access MIPI_SWAP.\n"); - return -1; + return ret; } val |= (1 << MIPI_SWAP_CH3); @@ -461,10 +469,12 @@ ret |= anx7625_reg_write(bus, RX_P1_ADDR, MIPI_LANE_CTRL_10, 0x00); ret |= anx7625_reg_write(bus, RX_P1_ADDR, MIPI_LANE_CTRL_10, 0x80); - if (ret < 0) + if (ret < 0) { ANXERROR("IO error: mipi dsi enable init failed.\n"); + return ret; + } - return ret; + return 0; } static int anx7625_dsi_config(uint8_t bus, struct display_timing *dt) @@ -487,12 +497,13 @@ /* clear mute flag */ ret |= anx7625_write_and(bus, RX_P0_ADDR, AP_AV_STATUS, ~AP_MIPI_MUTE); - if (ret < 0) + if (ret < 0) { ANXERROR("IO error: enable mipi rx failed.\n"); - else - ANXINFO("success to config DSI\n"); + return ret; + } - return ret; + ANXINFO("success to config DSI\n"); + return 0; } static int sp_tx_rst_aux(uint8_t bus) @@ -549,34 +560,32 @@ static int edid_read(uint8_t bus, uint8_t offset, uint8_t *pblock_buf) { - uint8_t c, cnt = 0; + int ret, cnt; - c = 0; for (cnt = 0; cnt < 3; cnt++) { sp_tx_aux_wr(bus, offset); /* set I2C read com 0x01 mot = 0 and read 16 bytes */ - c = sp_tx_aux_rd(bus, 0xf1); + ret = sp_tx_aux_rd(bus, 0xf1); - if (c == 1) { + if (ret < 0) { sp_tx_rst_aux(bus); ANXERROR("edid read failed, reset!\n"); - cnt++; } else { - anx7625_reg_block_read(bus, RX_P0_ADDR, - AP_AUX_BUFF_START, - MAX_DPCD_BUFFER_SIZE, pblock_buf); - return 0; + if (anx7625_reg_block_read(bus, RX_P0_ADDR, + AP_AUX_BUFF_START, + MAX_DPCD_BUFFER_SIZE, + pblock_buf) >= 0) + return 0; } } - return 1; + return -1; } static int segments_edid_read(uint8_t bus, uint8_t segment, uint8_t *buf, uint8_t offset) { - uint8_t c, cnt = 0; - int ret; + int ret, cnt; /* write address only */ ret = anx7625_reg_write(bus, RX_P0_ADDR, AP_AUX_ADDR_7_0, 0x30); @@ -598,21 +607,21 @@ for (cnt = 0; cnt < 3; cnt++) { sp_tx_aux_wr(bus, offset); /* set I2C read com 0x01 mot = 0 and read 16 bytes */ - c = sp_tx_aux_rd(bus, 0xf1); + ret = sp_tx_aux_rd(bus, 0xf1); - if (c == 1) { - ret = sp_tx_rst_aux(bus); + if (ret < 0) { + sp_tx_rst_aux(bus); ANXERROR("segment read failed, reset!\n"); - cnt++; } else { - ret = anx7625_reg_block_read(bus, RX_P0_ADDR, - AP_AUX_BUFF_START, - MAX_DPCD_BUFFER_SIZE, buf); - return ret; + if (anx7625_reg_block_read(bus, RX_P0_ADDR, + AP_AUX_BUFF_START, + MAX_DPCD_BUFFER_SIZE, + buf) >= 0) + return 0; } } - return ret; + return -1; } static int sp_tx_edid_read(uint8_t bus, uint8_t *pedid_blocks_buf, @@ -621,9 +630,7 @@ uint8_t offset, edid_pos; int count, blocks_num; uint8_t pblock_buf[MAX_DPCD_BUFFER_SIZE]; - uint8_t i; - uint8_t g_edid_break = 0; - int ret; + int i, ret, g_edid_break = 0; /* address initial */ ret = anx7625_reg_write(bus, RX_P0_ADDR, AP_AUX_ADDR_7_0, 0x50); @@ -637,7 +644,7 @@ blocks_num = sp_tx_get_edid_block(bus); if (blocks_num < 0) - return blocks_num; + return -1; count = 0; @@ -647,10 +654,10 @@ case 1: for (i = 0; i < 8; i++) { offset = (i + count * 8) * MAX_DPCD_BUFFER_SIZE; - g_edid_break = edid_read(bus, offset, - pblock_buf); + g_edid_break = !!edid_read(bus, offset, + pblock_buf); - if (g_edid_break == 1) + if (g_edid_break) break; if (offset <= size - MAX_DPCD_BUFFER_SIZE) @@ -668,11 +675,11 @@ edid_pos = (i + count * 8) * MAX_DPCD_BUFFER_SIZE; - if (g_edid_break == 1) + if (g_edid_break) break; segments_edid_read(bus, count / 2, - pblock_buf, offset); + pblock_buf, offset); if (edid_pos <= size - MAX_DPCD_BUFFER_SIZE) memcpy(&pedid_blocks_buf[edid_pos], pblock_buf, @@ -834,12 +841,13 @@ anx7625_parse_edid(edid, &dt); ret = anx7625_dsi_config(bus, &dt); - if (ret < 0) + if (ret < 0) { ANXERROR("MIPI phy setup error.\n"); - else - ANXINFO("MIPI phy setup OK.\n"); + return ret; + } - return ret; + ANXINFO("MIPI phy setup OK.\n"); + return 0; } int anx7625_dp_get_edid(uint8_t bus, struct edid *out) @@ -869,7 +877,7 @@ int retry_power_on = 3; while (--retry_power_on) { - if (anx7625_power_on_init(bus) == 0) + if (anx7625_power_on_init(bus) >= 0) break; } if (!retry_power_on) { -- To view, visit https://review.coreboot.org/c/coreboot/+/59540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ife9d7d97df2926b4581ba519a152c9efed8cd969 Gerrit-Change-Number: 59540 Gerrit-PatchSet: 5 Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Wei-Shun Chang <weishunc(a)google.com> Gerrit-Reviewer: Xin Ji <xji(a)analogix.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: lib: Fix edge case of log2_ceil()
by Jakub Czapiga (Code Review) 02 Dec '21

02 Dec '21

Attention is currently required from: Julius Werner, Yu-Ping Wu. Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59834 ) Change subject: lib: Fix edge case of log2_ceil() ...................................................................... Patch Set 1: (1 comment) File tests/lib/lib-test.c: https://review.coreboot.org/c/coreboot/+/59834/comment/3a47fe9c_c3423019 PS1, Line 46: log2_ceil(0xffffffff) > In the original implementation this will be 31: […] Nice catch. It really should have been 32, but until now it behaved like ordinary log2() without the ceil() -- To view, visit https://review.coreboot.org/c/coreboot/+/59834 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If868f793b909a6ad7fc48a7affac15e2c714fa2e Gerrit-Change-Number: 59834 Gerrit-PatchSet: 1 Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 02 Dec 2021 08:23:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2021