Change in coreboot[master]: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Tim Wawrzynczak (Code Review)

17 Jul 2019 17 Jul '19

5:32 p.m.

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table

When generating entries in SSDT for DesignWare I2C controllers, only use the speed selected in the devicetree, instead of trying all of them. This quiets a message which looks like a bug ("dw_i2c: bad counts").

Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/i2c/designware/dw_i2c.c 1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/34385/1

diff --git a/src/drivers/i2c/designware/dw_i2c.c b/src/drivers/i2c/designware/dw_i2c.c index 93b662a..af61857 100644 --- a/src/drivers/i2c/designware/dw_i2c.c +++ b/src/drivers/i2c/designware/dw_i2c.c @@ -851,6 +851,9 @@

/* Report timing values for the OS driver */ for (i = 0; i < DW_I2C_SPEED_CONFIG_COUNT; i++) { + if (speeds[i] != bcfg->speed) + continue; + /* Generate speed config. */ if (dw_i2c_gen_speed_config(dw_i2c_addr, speeds[i], bcfg, &sgen) < 0)

Attachments:

attachment.htm (text/html — 2.7 KB)

Show replies by date

Tim Wawrzynczak (Code Review)

17 Jul 17 Jul

5:33 p.m.

Tim Wawrzynczak has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table

BUG=b:137298661 BRANCH=none TEST=Boot and verify that I2C controllers still function, and the nastygram message is gone.

Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/i2c/designware/dw_i2c.c 1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/34385/2

Paul Fagerburg (Code Review)

5:44 p.m.

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 2: Code-Review+1

Furquan Shaikh (Code Review)

5:50 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... File src/drivers/i2c/designware/dw_i2c.c:

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... PS2, Line 853: for (i = 0; i < DW_I2C_SPEED_CONFIG_COUNT; i++) Do we need the loop at all?

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... PS2, Line 854: bcfg->speed What happens when bcfg->speed is not set? Do we need a default in that case?

Tim Wawrzynczak (Code Review)

5:57 p.m.

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... File src/drivers/i2c/designware/dw_i2c.c:

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... PS2, Line 853: for (i = 0; i < DW_I2C_SPEED_CONFIG_COUNT; i++)

...

Do we need the loop at all?

That's fair...

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... PS2, Line 854: bcfg->speed

...

What happens when bcfg->speed is not set? Do we need a default in that case?

Since it comes from the device-tree, and they are static structures, it will be assigned as zero by default. It seems like I2C_SPEED_STANDARD would be the safe default (every I2C device has to support that speed). I'll check for zero and use I2C_SPEED_STANDARD in that case.

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 17 Jul 2019 15:57:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

6:01 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... File src/drivers/i2c/designware/dw_i2c.c:

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... PS2, Line 854: bcfg->speed

...

Since it comes from the device-tree, and they are static structures, it will be assigned as zero by […]

sounds good.

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 17 Jul 2019 16:01:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Tim Wawrzynczak (Code Review)

9:52 p.m.

Hello Karthik Ramasubramanian, EricR Lai, Paul Fagerburg, Shelley Chen, build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/34385

to look at the new patch set (#3).

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table

BUG=b:137298661 BRANCH=none TEST=Boot and verify that I2C controllers still function, and the nastygram message is gone.

Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/i2c/designware/dw_i2c.c 1 file changed, 7 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/34385/3

Paul Fagerburg (Code Review)

10:21 p.m.

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 3: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 17 Jul 2019 20:21:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

10:37 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 3: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 17 Jul 2019 20:37:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Tim Wawrzynczak (Code Review)

10:45 p.m.

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... File src/drivers/i2c/designware/dw_i2c.c:

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... PS2, Line 853: for (i = 0; i < DW_I2C_SPEED_CONFIG_COUNT; i++)

...

That's fair...

Done

https://review.coreboot.org/c/coreboot/+/34385/2/src/drivers/i2c/designware/... PS2, Line 854: bcfg->speed

...

sounds good.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 17 Jul 2019 20:45:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

EricR Lai (Code Review)

18 Jul 18 Jul

7:06 a.m.

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 3: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/34385/3/src/drivers/i2c/designware/... File src/drivers/i2c/designware/dw_i2c.c:

https://review.coreboot.org/c/coreboot/+/34385/3/src/drivers/i2c/designware/... PS3, Line 846: speed = (bcfg->speed == 0) ? I2C_SPEED_STANDARD : bcfg->speed; Can we use default FAST as like kernel driver default?

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 18 Jul 2019 05:06:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Menzel (Code Review)

2:40 p.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34385/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/34385/3//COMMIT_MSG@11 PS3, Line 11: This quiets a message which looks like a bug ("dw_i2c: bad counts"). In the Linux kernel?

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 18 Jul 2019 12:40:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

3:50 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 3: -Code-Review

(1 comment)

https://review.coreboot.org/c/coreboot/+/34385/3/src/drivers/i2c/designware/... File src/drivers/i2c/designware/dw_i2c.c:

https://review.coreboot.org/c/coreboot/+/34385/3/src/drivers/i2c/designware/... PS3, Line 846: speed = (bcfg->speed == 0) ? I2C_SPEED_STANDARD : bcfg->speed;

...

Can we use default FAST as like kernel driver default?

That is a good point.

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 18 Jul 2019 13:50:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-MessageType: comment

Tim Wawrzynczak (Code Review)

4:42 p.m.

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/34385/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/34385/3//COMMIT_MSG@11 PS3, Line 11: This quiets a message which looks like a bug ("dw_i2c: bad counts").

...

In the Linux kernel?

In this driver; I'll update the msg.

https://review.coreboot.org/c/coreboot/+/34385/3/src/drivers/i2c/designware/... File src/drivers/i2c/designware/dw_i2c.c:

https://review.coreboot.org/c/coreboot/+/34385/3/src/drivers/i2c/designware/... PS3, Line 846: speed = (bcfg->speed == 0) ? I2C_SPEED_STANDARD : bcfg->speed;

...

That is a good point.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 18 Jul 2019 14:42:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: EricR Lai ericr_lai@compal.corp-partner.google.com Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Tim Wawrzynczak (Code Review)

4:43 p.m.

Hello EricR Lai, Karthik Ramasubramanian, Paul Fagerburg, Duncan Laurie, Shelley Chen, build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/34385

to look at the new patch set (#4).

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table

BUG=b:137298661 BRANCH=none TEST=Boot and verify that I2C controllers still function, and the nastygram message is gone.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/34385/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Furquan Shaikh (Code Review)

4:44 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 18 Jul 2019 14:44:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Fagerburg (Code Review)

4:46 p.m.

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 18 Jul 2019 14:46:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Martin Roth (Code Review)

19 Jul 19 Jul

6:44 p.m.

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34385 )

Change subject: drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table ......................................................................

drivers/i2c/dw: Don't try to generate unselected speeds in ACPI table

BUG=b:137298661 BRANCH=none TEST=Boot and verify that I2C controllers still function, and the nastygram message is gone.

Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/34385 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Paul Fagerburg pfagerburg@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/i2c/designware/dw_i2c.c 1 file changed, 7 insertions(+), 17 deletions(-)

Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Paul Fagerburg: Looks good to me, approved

diff --git a/src/drivers/i2c/designware/dw_i2c.c b/src/drivers/i2c/designware/dw_i2c.c index 93b662a..760a735 100644 --- a/src/drivers/i2c/designware/dw_i2c.c +++ b/src/drivers/i2c/designware/dw_i2c.c @@ -817,14 +817,9 @@ const struct dw_i2c_bus_config *bcfg; uintptr_t dw_i2c_addr; struct dw_i2c_speed_config sgen; - enum i2c_speed speeds[DW_I2C_SPEED_CONFIG_COUNT] = { - I2C_SPEED_STANDARD, - I2C_SPEED_FAST, - I2C_SPEED_FAST_PLUS, - I2C_SPEED_HIGH, - }; - int i, bus; + int bus; const char *path; + unsigned int speed;

if (!dev->enabled) return; @@ -847,20 +842,15 @@ if (!path) return;

- acpigen_write_scope(path); + /* Ensure a default speed is available */ + speed = (bcfg->speed == 0) ? I2C_SPEED_FAST : bcfg->speed;

/* Report timing values for the OS driver */ - for (i = 0; i < DW_I2C_SPEED_CONFIG_COUNT; i++) { - /* Generate speed config. */ - if (dw_i2c_gen_speed_config(dw_i2c_addr, speeds[i], bcfg, - &sgen) < 0) - continue; - - /* Generate ACPI based on selected speed config */ + if (dw_i2c_gen_speed_config(dw_i2c_addr, speed, bcfg, &sgen) >= 0) { + acpigen_write_scope(path); dw_i2c_acpi_write_speed_config(&sgen); + acpigen_pop_len(); } - - acpigen_pop_len(); }

static int dw_i2c_dev_transfer(struct device *dev,

-- To view, visit https://review.coreboot.org/c/coreboot/+/34385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07207ec95652e8af1a42bfe31214f61a183a134e Gerrit-Change-Number: 34385 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Paul Fagerburg pfagerburg@chromium.org Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

2112

days inactive

2114

days old

coreboot-gerrit@coreboot.org

17 comments

6 participants

tags (0)

participants (6)

EricR Lai (Code Review)
Furquan Shaikh (Code Review)
Martin Roth (Code Review)
Paul Fagerburg (Code Review)
Paul Menzel (Code Review)
Tim Wawrzynczak (Code Review)