Change in coreboot[master]: mb/google/kahlee/treeya: Make sure touchpad data hold time more than ...
Peichao Li has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36945 )
Change subject: mb/google/kahlee/treeya: Make sure touchpad data hold time more than 300ns ......................................................................
mb/google/kahlee/treeya: Make sure touchpad data hold time more than 300ns
According to SI team request, need tune I2C bus2 data hold time more than 300ns
BUG=b:144736027 TEST=build firmware and masure I2C bus2 data hold time
Signed-off-by: Peichao Wang peichao.wang@bitland.corp-partner.google.com Change-Id: Idc58a595c77eba8544f27682a284be6aac5dbe25 --- M src/mainboard/google/kahlee/variants/treeya/devicetree.cb 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36945/1
diff --git a/src/mainboard/google/kahlee/variants/treeya/devicetree.cb b/src/mainboard/google/kahlee/variants/treeya/devicetree.cb index e8477ee..e35f00c 100644 --- a/src/mainboard/google/kahlee/variants/treeya/devicetree.cb +++ b/src/mainboard/google/kahlee/variants/treeya/devicetree.cb @@ -44,6 +44,7 @@ .speed = I2C_SPEED_FAST, .rise_time_ns = 3, .fall_time_ns = 2, + .data_hold_time_ns = 400, }"
# Enable I2C3 for touchscreen at 400kHz
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36945 )
Change subject: mb/google/kahlee/treeya: Make sure touchpad data hold time more than 300ns ......................................................................
Patch Set 1: Code-Review+1
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36945 )
Change subject: mb/google/kahlee/treeya: Make sure touchpad data hold time more than 300ns ......................................................................
Patch Set 1:
(6 comments)
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@8 PS1, Line 8: than 300ns The commit message summary must go on one line. Maybe (whatever “touchpad data hold time” is):
Hold touchpad data 400 ns
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@10 PS1, Line 10: need tune need to tune
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@10 PS1, Line 10: SI What does that mean?
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@11 PS1, Line 11: more than 300ns Why do you increase it to 400 ns then?
What is the default value?
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@14 PS1, Line 14: TEST=build firmware and masure I2C bus2 data hold time How did you measure it?
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@14 PS1, Line 14: masure measure
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36945 )
Change subject: mb/google/kahlee/treeya: Make sure touchpad data hold time more than 300ns ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@8 PS1, Line 8: than 300ns
The commit message summary must go on one line. Maybe (whatever “touchpad data hold time” is): […]
Just for the record, multi-line git subjects are well-defined. However, we generally abide by the "not more than 50 characters in the git subject" rule which pretty much makes the point moot.
Hello Julius Werner, Tim Wawrzynczak, build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36945
to look at the new patch set (#2).
Change subject: mb/google/kahlee/treeya: Set touchpad hold time to 400ns ......................................................................
mb/google/kahlee/treeya: Set touchpad hold time to 400ns
According to SI team request, need tune I2C bus 2 data hold time more than 300ns
BUG=b:144736027 TEST=build firmware and measure I2C bus 2 data hold time
Signed-off-by: Peichao Wang peichao.wang@bitland.corp-partner.google.com Change-Id: Idc58a595c77eba8544f27682a284be6aac5dbe25 --- M src/mainboard/google/kahlee/variants/treeya/devicetree.cb 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36945/2
Hello Julius Werner, Tim Wawrzynczak, build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36945
to look at the new patch set (#3).
Change subject: mb/google/kahlee/treeya: Set touchpad hold time to 400ns ......................................................................
mb/google/kahlee/treeya: Set touchpad hold time to 400ns
According to SI team request, need tune I2C bus 2 data hold time more than 300ns
BUG=b:144736027 TEST=build firmware and measure I2C bus 2 data hold time
Signed-off-by: Peichao Wang peichao.wang@bitland.corp-partner.google.com Change-Id: Idc58a595c77eba8544f27682a284be6aac5dbe25 --- M src/mainboard/google/kahlee/variants/treeya/devicetree.cb 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36945/3
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36945 )
Change subject: mb/google/kahlee/treeya: Set touchpad hold time to 400ns ......................................................................
Patch Set 3: Code-Review+2
Martin Roth has uploaded a new patch set (#4) to the change originally created by Peichao Li. ( https://review.coreboot.org/c/coreboot/+/36945 )
Change subject: mb/google/kahlee/treeya: Set touchpad hold time to 400ns ......................................................................
mb/google/kahlee/treeya: Set touchpad hold time to 400ns
According to SI team request, need to tune I2C bus 2 data hold time more than 300ns
BUG=b:144736027 TEST=build firmware and measure I2C bus 2 data hold time
Signed-off-by: Peichao Wang peichao.wang@bitland.corp-partner.google.com Change-Id: Idc58a595c77eba8544f27682a284be6aac5dbe25 --- M src/mainboard/google/kahlee/variants/treeya/devicetree.cb 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36945/4
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36945 )
Change subject: mb/google/kahlee/treeya: Set touchpad hold time to 400ns ......................................................................
Patch Set 4:
(6 comments)
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@8 PS1, Line 8: than 300ns
Just for the record, multi-line git subjects are well-defined. […]
Done
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@10 PS1, Line 10: need tune
need to tune
Done
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@10 PS1, Line 10: SI
What does that mean?
According to the chip manufacturer (silicon team)
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@11 PS1, Line 11: more than 300ns
Why do you increase it to 400 ns then? […]
400ns > 300ns There is no default value - how long is a piece of string?
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@14 PS1, Line 14: masure
measure
Done
https://review.coreboot.org/c/coreboot/+/36945/1//COMMIT_MSG@14 PS1, Line 14: TEST=build firmware and masure I2C bus2 data hold time
How did you measure it?
The normal way is with an Oscilloscope.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36945 )
Change subject: mb/google/kahlee/treeya: Set touchpad hold time to 400ns ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36945 )
Change subject: mb/google/kahlee/treeya: Set touchpad hold time to 400ns ......................................................................
mb/google/kahlee/treeya: Set touchpad hold time to 400ns
According to SI team request, need to tune I2C bus 2 data hold time more than 300ns
BUG=b:144736027 TEST=build firmware and measure I2C bus 2 data hold time
Signed-off-by: Peichao Wang peichao.wang@bitland.corp-partner.google.com Change-Id: Idc58a595c77eba8544f27682a284be6aac5dbe25 Reviewed-on: https://review.coreboot.org/c/coreboot/+/36945 Reviewed-by: Patrick Georgi pgeorgi@google.com Reviewed-by: Martin Roth martinroth@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/kahlee/variants/treeya/devicetree.cb 1 file changed, 1 insertion(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Martin Roth: Looks good to me, approved Peichao Li: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/kahlee/variants/treeya/devicetree.cb b/src/mainboard/google/kahlee/variants/treeya/devicetree.cb index e8477ee..e35f00c 100644 --- a/src/mainboard/google/kahlee/variants/treeya/devicetree.cb +++ b/src/mainboard/google/kahlee/variants/treeya/devicetree.cb @@ -44,6 +44,7 @@ .speed = I2C_SPEED_FAST, .rise_time_ns = 3, .fall_time_ns = 2, + .data_hold_time_ns = 400, }"
# Enable I2C3 for touchscreen at 400kHz
-
Martin Roth (Code Review) -
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Peichao Li (Code Review)