Change in coreboot[master]: mb/google/dedede/var/magolor: Configure I2C high and low time
Ren Kuo has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46613 ) Change subject: mb/google/dedede/var/magolor: Configure I2C high and low time ...................................................................... mb/google/dedede/var/magolor: Configure I2C high and low time Configure the I2C bus high and low time for all enabled I2C buses. BUG=b:168783630 TEST=Measured the I2C bus frequency reduce to 387 KHz. Signed-off-by: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 --- M src/mainboard/google/dedede/variants/magolor/overridetree.cb 1 file changed, 24 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/46613/1 diff --git a/src/mainboard/google/dedede/variants/magolor/overridetree.cb b/src/mainboard/google/dedede/variants/magolor/overridetree.cb index f41e9fa..625546e 100644 --- a/src/mainboard/google/dedede/variants/magolor/overridetree.cb +++ b/src/mainboard/google/dedede/variants/magolor/overridetree.cb @@ -25,18 +25,42 @@ }, .i2c[0] = { .speed = I2C_SPEED_FAST, + .speed_config[0] = { + .speed = I2C_SPEED_FAST, + .scl_lcnt = 190, + .scl_hcnt = 100, + .sda_hold = 40, + } }, .i2c[1] = { .speed = I2C_SPEED_FAST, }, .i2c[2] = { .speed = I2C_SPEED_FAST, + .speed_config[0] = { + .speed = I2C_SPEED_FAST, + .scl_lcnt = 190, + .scl_hcnt = 100, + .sda_hold = 40, + } }, .i2c[3] = { .speed = I2C_SPEED_FAST, + .speed_config[0] = { + .speed = I2C_SPEED_FAST, + .scl_lcnt = 190, + .scl_hcnt = 100, + .sda_hold = 40, + } }, .i2c[4] = { .speed = I2C_SPEED_FAST, + .speed_config[0] = { + .speed = I2C_SPEED_FAST, + .scl_lcnt = 190, + .scl_hcnt = 100, + .sda_hold = 40, + } }, }" -- To view, visit https://review.coreboot.org/c/coreboot/+/46613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Gerrit-Change-Number: 46613 Gerrit-PatchSet: 1 Gerrit-Owner: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-MessageType: newchange
Ren Kuo has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46613 ) Change subject: mb/google/dedede/var/magolor: Configure I2C high and low time ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/46613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Gerrit-Change-Number: 46613 Gerrit-PatchSet: 1 Gerrit-Owner: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Henry Sun <henrysun@google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com> Gerrit-Reviewer: Marco Chen <marcochen@google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Comment-Date: Wed, 21 Oct 2020 03:35:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46613 ) Change subject: mb/google/dedede/var/magolor: Configure I2C high and low time ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/46613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Gerrit-Change-Number: 46613 Gerrit-PatchSet: 1 Gerrit-Owner: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Henry Sun <henrysun@google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com> Gerrit-Reviewer: Marco Chen <marcochen@google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Wed, 21 Oct 2020 03:58:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46613 ) Change subject: mb/google/dedede/var/magolor: Configure I2C high and low time ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/c/coreboot/+/46613/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/46613/1//COMMIT_MSG@12 PS1, Line 12: TEST=Measured the I2C bus frequency reduce to 387 KHz. What was the value before? https://review.coreboot.org/c/coreboot/+/46613/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/magolor/overridetree.cb: https://review.coreboot.org/c/coreboot/+/46613/1/src/mainboard/google/dedede... PS1, Line 27: .speed = I2C_SPEED_FAST, : .speed_config[0] = { : .speed = I2C_SPEED_FAST, Is `.speed` really specified twice? -- To view, visit https://review.coreboot.org/c/coreboot/+/46613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Gerrit-Change-Number: 46613 Gerrit-PatchSet: 1 Gerrit-Owner: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Henry Sun <henrysun@google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com> Gerrit-Reviewer: Marco Chen <marcochen@google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 21 Oct 2020 10:22:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Ren Kuo has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46613 ) Change subject: mb/google/dedede/var/magolor: Configure I2C high and low time ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/46613/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/46613/1//COMMIT_MSG@12 PS1, Line 12: TEST=Measured the I2C bus frequency reduce to 387 KHz.
What was the value before? the i2c bus frequency is 438.809KHz in previous settings. refer to https://partnerissuetracker.corp.google.com/issues/168783630#comment1
-- To view, visit https://review.coreboot.org/c/coreboot/+/46613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Gerrit-Change-Number: 46613 Gerrit-PatchSet: 1 Gerrit-Owner: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Henry Sun <henrysun@google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com> Gerrit-Reviewer: Marco Chen <marcochen@google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 26 Oct 2020 02:47:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: comment
Ren Kuo has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46613 ) Change subject: mb/google/dedede/var/magolor: Configure I2C high and low time ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/46613/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/magolor/overridetree.cb: https://review.coreboot.org/c/coreboot/+/46613/1/src/mainboard/google/dedede... PS1, Line 27: .speed = I2C_SPEED_FAST, : .speed_config[0] = { : .speed = I2C_SPEED_FAST,
Is `. […] from current fsp code, the "speed" will be specified twice. initial i2c parameters by different power on stages.
FSP code, struct definition: struct soc_intel_common_config{ ... struct dw_i2c_bus_config ... } struct dw_i2c_bus_config { /* Bus should be enabled prior to ramstage with temporary base */ ... enum i2c_speed speed; ... /* Specific bus speed configuration */ struct dw_i2c_speed_config } struct dw_i2c_speed_config { enum i2c_speed speed; uint16_t scl_lcnt; uint16_t scl_hcnt; uint32_t sda_hold; }; -- To view, visit https://review.coreboot.org/c/coreboot/+/46613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Gerrit-Change-Number: 46613 Gerrit-PatchSet: 1 Gerrit-Owner: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Henry Sun <henrysun@google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com> Gerrit-Reviewer: Marco Chen <marcochen@google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 26 Oct 2020 05:56:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: comment
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46613 ) Change subject: mb/google/dedede/var/magolor: Configure I2C high and low time ...................................................................... mb/google/dedede/var/magolor: Configure I2C high and low time Configure the I2C bus high and low time for all enabled I2C buses. BUG=b:168783630 TEST=Measured the I2C bus frequency reduce to 387 KHz. Signed-off-by: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Reviewed-on: https://review.coreboot.org/c/coreboot/+/46613 Reviewed-by: Karthik Ramasubramanian <kramasub@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org> --- M src/mainboard/google/dedede/variants/magolor/overridetree.cb 1 file changed, 24 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Ren Kuo: Looks good to me, but someone else must approve Karthik Ramasubramanian: Looks good to me, approved diff --git a/src/mainboard/google/dedede/variants/magolor/overridetree.cb b/src/mainboard/google/dedede/variants/magolor/overridetree.cb index f41e9fa..625546e 100644 --- a/src/mainboard/google/dedede/variants/magolor/overridetree.cb +++ b/src/mainboard/google/dedede/variants/magolor/overridetree.cb @@ -25,18 +25,42 @@ }, .i2c[0] = { .speed = I2C_SPEED_FAST, + .speed_config[0] = { + .speed = I2C_SPEED_FAST, + .scl_lcnt = 190, + .scl_hcnt = 100, + .sda_hold = 40, + } }, .i2c[1] = { .speed = I2C_SPEED_FAST, }, .i2c[2] = { .speed = I2C_SPEED_FAST, + .speed_config[0] = { + .speed = I2C_SPEED_FAST, + .scl_lcnt = 190, + .scl_hcnt = 100, + .sda_hold = 40, + } }, .i2c[3] = { .speed = I2C_SPEED_FAST, + .speed_config[0] = { + .speed = I2C_SPEED_FAST, + .scl_lcnt = 190, + .scl_hcnt = 100, + .sda_hold = 40, + } }, .i2c[4] = { .speed = I2C_SPEED_FAST, + .speed_config[0] = { + .speed = I2C_SPEED_FAST, + .scl_lcnt = 190, + .scl_hcnt = 100, + .sda_hold = 40, + } }, }" -- To view, visit https://review.coreboot.org/c/coreboot/+/46613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Gerrit-Change-Number: 46613 Gerrit-PatchSet: 2 Gerrit-Owner: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Henry Sun <henrysun@google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com> Gerrit-Reviewer: Marco Chen <marcochen@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: merged
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46613 ) Change subject: mb/google/dedede/var/magolor: Configure I2C high and low time ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/46613/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/46613/1//COMMIT_MSG@12 PS1, Line 12: TEST=Measured the I2C bus frequency reduce to 387 KHz.
the i2c bus frequency is 438.809KHz in previous settings. […] Thank you for your anwser. I am not allowed to view that bug report, so please update the commit message next time with that information. If possible, commit messages should be selfcontained.
-- To view, visit https://review.coreboot.org/c/coreboot/+/46613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Gerrit-Change-Number: 46613 Gerrit-PatchSet: 2 Gerrit-Owner: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Henry Sun <henrysun@google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com> Gerrit-Reviewer: Marco Chen <marcochen@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 27 Oct 2020 09:14:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Comment-In-Reply-To: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: comment
Ren Kuo has posted comments on this change by Ren Kuo. ( https://review.coreboot.org/c/coreboot/+/46613?usp=email ) Change subject: mb/google/dedede/var/magolor: Configure I2C high and low time ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/46613/comment/b8ee59db_029ff655?usp... : PS1, Line 12: TEST=Measured the I2C bus frequency reduce to 387 KHz.
Thank you for your anwser. […] OK.Ｈere is the full bug comment:
Kimber Cheng<kimber.cheng@quanta.corp-partner.google.com> #1Sep 17, 2020 11:36PM Created issue, assigned to magolor-odm-external@google.com. 11:36PM Measured CPU I2C signals, the clock frequency higher than 400KHz. AP-I2C0 : 438.809KHz AP-I2C2 : 442.738KHz AP-I2C3 : 440.412KHz AP-I2C4 : 440.516KHz Use below FW, the result were same. FW : 13389.0.0 FW : 13455.0.0 FW : 13474.0.0 Please refer data. https://drive.google.com/drive/folders/1XeYBzozAAqxjPYOAS7ep2XXrbB5qPDUR -- To view, visit https://review.coreboot.org/c/coreboot/+/46613?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f5b81815f86db7bdcea95a95b9c9b235b4a34b1 Gerrit-Change-Number: 46613 Gerrit-PatchSet: 2 Gerrit-Owner: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-Reviewer: Henry Sun <henrysun@google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com> Gerrit-Reviewer: Marco Chen <marcochen@google.com> Gerrit-Reviewer: Patrick Georgi <patrick@coreboot.org> Gerrit-Reviewer: Ren Kuo <ren.kuo@quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Comment-Date: Wed, 17 Sep 2025 01:58:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter@mailbox.org> Comment-In-Reply-To: Ren Kuo <ren.kuo@quanta.corp-partner.google.com>
participants (4)
-
Karthik Ramasubramanian (Code Review) -
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Ren Kuo (Code Review)