Peichao Li has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43661 )
Change subject: mb/google/zork/vilboz: Tune I2C bus 3 clock ......................................................................
mb/google/zork/vilboz: Tune I2C bus 3 clock
Tune I2C bus 3 clock and insure it meets I2C spec.
BUG=b:161650117 TEST==flash coreboot to the DUT and measure I2C bus 3 clock frequency less than 400KHz
Signed-off-by: Peichao.Wang peichao.wang@bitland.corp-partner.google.com Change-Id: Ifa9f0bce723f55a12fd2313788c995f8326e3e7d --- M src/mainboard/google/zork/variants/vilboz/overridetree.cb 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/43661/1
diff --git a/src/mainboard/google/zork/variants/vilboz/overridetree.cb b/src/mainboard/google/zork/variants/vilboz/overridetree.cb index 49d21d5..9129e2a 100644 --- a/src/mainboard/google/zork/variants/vilboz/overridetree.cb +++ b/src/mainboard/google/zork/variants/vilboz/overridetree.cb @@ -28,8 +28,8 @@ # I2C3 for H1 register "i2c[3]" = "{ .speed = I2C_SPEED_FAST, - .rise_time_ns = 184, /* 0 to 1.26v (1.8 * .7) */ - .fall_time_ns = 42, /* 1.26v to 0 */ + .rise_time_ns = 110, + .fall_time_ns = 5, .early_init = true, }"
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43661 )
Change subject: mb/google/zork/vilboz: Tune I2C bus 3 clock ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43661 )
Change subject: mb/google/zork/vilboz: Tune I2C bus 3 clock ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG@12 PS1, Line 12: measure I2C bus 3 clock : frequency less than 400KHz It would be good to capture the actual measured frequency in the commit message. Helps for future reference.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43661 )
Change subject: mb/google/zork/vilboz: Tune I2C bus 3 clock ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG@2 PS1, Line 2: peichao.wang Please use *Peicho Wang*.
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG@7 PS1, Line 7: mb/google/zork/vilboz: Tune I2C bus 3 clock Please look at other commits, how this is formulated. (What frequency is the target?)
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG@15 PS1, Line 15: Peichao.Wang Please remove the dot between the names.
https://review.coreboot.org/c/coreboot/+/43661/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43661/1/src/mainboard/google/zork/v... PS1, Line 31: .rise_time_ns = 184, /* 0 to 1.26v (1.8 * .7) */ Are the comment incorrect?
Hello build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43661
to look at the new patch set (#2).
Change subject: mb/google/vilboz: Tune I2C bus 3 clock ......................................................................
mb/google/vilboz: Tune I2C bus 3 clock
Tune I2C bus3 frequency and insure it meets I2C spec.
BUG=b:161650117 TEST=flash coreboot to the DUT and actual measured I2C bus3 make sure it meet Spec.
Signed-off-by: Peichao Wang peichao.wang@bitland.corp-partner.google.com Change-Id: Ifa9f0bce723f55a12fd2313788c995f8326e3e7d --- M src/mainboard/google/zork/variants/vilboz/overridetree.cb 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/43661/2
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43661 )
Change subject: mb/google/vilboz: Tune I2C bus 3 clock ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43661/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43661/1/src/mainboard/google/zork/v... PS1, Line 31: .rise_time_ns = 184, /* 0 to 1.26v (1.8 * .7) */
Are the comment incorrect?
Dear Paul, we need delete these comment, these values are correct for Vilboz. Thanks a lot!
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43661 )
Change subject: mb/google/vilboz: Tune I2C bus 3 clock ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG@2 PS1, Line 2: peichao.wang
Please use *Peicho Wang*.
Done
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG@7 PS1, Line 7: mb/google/zork/vilboz: Tune I2C bus 3 clock
Please look at other commits, how this is formulated. […]
Done
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG@12 PS1, Line 12: measure I2C bus 3 clock : frequency less than 400KHz
It would be good to capture the actual measured frequency in the commit message. […]
Done
https://review.coreboot.org/c/coreboot/+/43661/1//COMMIT_MSG@15 PS1, Line 15: Peichao.Wang
Please remove the dot between the names.
Done
https://review.coreboot.org/c/coreboot/+/43661/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43661/1/src/mainboard/google/zork/v... PS1, Line 31: .rise_time_ns = 184, /* 0 to 1.26v (1.8 * .7) */
Dear Paul, we need delete these comment, these values are correct for Vilboz. […]
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43661 )
Change subject: mb/google/vilboz: Tune I2C bus 3 clock ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43661 )
Change subject: mb/google/vilboz: Tune I2C bus 3 clock ......................................................................
mb/google/vilboz: Tune I2C bus 3 clock
Tune I2C bus3 frequency and insure it meets I2C spec.
BUG=b:161650117 TEST=flash coreboot to the DUT and actual measured I2C bus3 make sure it meet Spec.
Signed-off-by: Peichao Wang peichao.wang@bitland.corp-partner.google.com Change-Id: Ifa9f0bce723f55a12fd2313788c995f8326e3e7d Reviewed-on: https://review.coreboot.org/c/coreboot/+/43661 Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/zork/variants/vilboz/overridetree.cb 1 file changed, 2 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/zork/variants/vilboz/overridetree.cb b/src/mainboard/google/zork/variants/vilboz/overridetree.cb index 0f374cb..eb5c2dd 100644 --- a/src/mainboard/google/zork/variants/vilboz/overridetree.cb +++ b/src/mainboard/google/zork/variants/vilboz/overridetree.cb @@ -28,8 +28,8 @@ # I2C3 for H1 register "i2c[3]" = "{ .speed = I2C_SPEED_FAST, - .rise_time_ns = 184, /* 0 to 1.26v (1.8 * .7) */ - .fall_time_ns = 42, /* 1.26v to 0 */ + .rise_time_ns = 110, + .fall_time_ns = 5, .early_init = true, }"
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Peichao Li (Code Review)