Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Surendranath R Gurivireddy (Code Review)

1 Nov 2019 1 Nov '19

12:30 a.m.

Surendranath R Gurivireddy has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

Signed-off-by: Surendranath Gurivireddy surendranath.r.gurivireddy@intel.com Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 --- M src/soc/intel/cannonlake/fsp_params.c 1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/1

diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index 0713ef4..27b109d 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -273,6 +273,9 @@ params->Usb2AfePehalfbit[i] = config->usb2_ports[i].pre_emp_bit; }

+ /* Disable USB2 PHY Power gating */ + params->PchUsb2PhySusPgEnable = 0; + for (i = 0; i < ARRAY_SIZE(config->usb3_ports); i++) { params->PortUsb30Enable[i] = config->usb3_ports[i].enable; params->Usb3OverCurrentPin[i] = config->usb3_ports[i].ocpin;

Attachments:

attachment.htm (text/html — 2.6 KB)

Show replies by date

build bot (Jenkins) (Code Review)

1 Nov 1 Nov

12:31 a.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/1/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/36519/1/src/soc/intel/cannonlake/fs... PS1, Line 277: params->PchUsb2PhySusPgEnable = 0; please, no spaces at the start of a line

Surendranath R Gurivireddy (Code Review)

8:28 p.m.

Hello Patrick Rudolph, build bot (Jenkins),

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

Workaround for an issue seen when a specific charger is connected while in S0ix state

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/2

Surendranath R Gurivireddy (Code Review)

8:29 p.m.

Hello Patrick Rudolph, build bot (Jenkins),

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

Workaround for an issue seen when a specific charger is connected while in S0ix state

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/3

EricR Lai (Code Review)

5 Nov 5 Nov

6:31 a.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... PS3, Line 277: params->PchUsb2PhySusPgEnable = 0; Any possible side effect here? power consumption?

EricR Lai (Code Review)

6:41 a.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... PS3, Line 277: params->PchUsb2PhySusPgEnable = 0;

...

Any possible side effect here? power consumption?

Sorry, I just checked the #599886.

EricR Lai (Code Review)

1:33 p.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 3:

I think this is better to expose config in devicetree. At least, hatch based project won't be effected by this CL.

Tim Wawrzynczak (Code Review)

5:54 p.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 3:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/36519/3//COMMIT_MSG@9 PS3, Line 9: Workaround for an issue seen when a specific charger is connected while : in S0ix state Which charger? What was the issue that this solves?

EricR Lai (Code Review)

6:01 p.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 3:

...

Patch Set 3:

(1 comment)

Apple adapter. You can refer the issue b:133775942

Tim Wawrzynczak (Code Review)

6:15 p.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 3:

(1 comment)

...

Patch Set 3:

...
Patch Set 3:

(1 comment)

Apple adapter. You can refer the issue b:133775942

Thanks Eric

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... PS3, Line 277: params->PchUsb2PhySusPgEnable = 0;

...

Sorry, I just checked the #599886.

Yeah, this definitely needs to go into devicetree; you don't know what effect this might have on other cannonlake mainboards.

Duncan Laurie (Code Review)

7:04 p.m.

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 3: Code-Review-1

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... PS3, Line 277: params->PchUsb2PhySusPgEnable = 0;

...

Yeah, this definitely needs to go into devicetree; you don't know what effect this might have on oth […]

IMO this is just a proof-of-concept patch and not something we could use for a production device.

Surendranath R Gurivireddy (Code Review)

6 Nov 6 Nov

1:56 a.m.

Hello Patrick Rudolph, Duncan Laurie, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

Workaround to disable USB2 PHY power gating to fix issue seen when Apple 87W USB-C charger is connected in S0ix state on WHL platforms (based on Intel's recommendation)

Added devicetree entry to set the flag to disable/enable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

Signed-off-by: Surendranath Gurivireddy surendranath.r.gurivireddy@intel.com Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 --- M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/mainboard/google/sarien/variants/arcada/devicetree.cb M src/mainboard/google/sarien/variants/sarien/devicetree.cb M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 6 files changed, 8 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/4

Surendranath R Gurivireddy (Code Review)

2:02 a.m.

Surendranath R Gurivireddy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... PS3, Line 277: params->PchUsb2PhySusPgEnable = 0;

...

IMO this is just a proof-of-concept patch and not something we could use for a production device.

Verification of the patch is still in progress to assess the power impact. This change is done based on the Intel's recommendation (Sighting is attached in the partner issue_ As suggested, added flag in the device tree and set to 0 only for WHL platforms

EricR Lai (Code Review)

3:49 a.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/36519/4/src/mainboard/google/dralli... PS4, Line 41: register "PchUsb2PhySusPgEnable" = "1" drallion has this issue as well. Need client to decide need this or not.

Surendranath R Gurivireddy (Code Review)

7 Nov 7 Nov

12:19 a.m.

Hello Patrick Rudolph, Bernardo Perez Priego, Aamir Bohra, Selma Bensaid, Duncan Laurie, Tim Wawrzynczak, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

Workaround to disable USB2 PHY power gating to fix issue seen when Apple 87W USB-C charger is connected in S0ix state on WHL platforms (based on Intel's recommendation). Issue is seen on CML platforms also.

Added devicetree entry to set the flag to disable/enable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/5

EricR Lai (Code Review)

1:47 p.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/5/src/soc/intel/cannonlake/ch... File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/36519/5/src/soc/intel/cannonlake/ch... PS5, Line 133: uint8_t PchUsb2PhySusPgEnable; Add comment /* USB2 PHY power gating */

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 5 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 07 Nov 2019 12:47:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Surendranath R Gurivireddy (Code Review)

8 Nov 8 Nov

8:11 p.m.

Hello Patrick Rudolph, Bernardo Perez Priego, Aamir Bohra, Selma Bensaid, Duncan Laurie, Tim Wawrzynczak, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

Added devicetree entry to set the flag to disable/enable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

Signed-off-by: Surendranath Gurivireddy surendranath.r.gurivireddy@intel.com Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 --- A howq M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/mainboard/google/sarien/variants/arcada/devicetree.cb M src/mainboard/google/sarien/variants/sarien/devicetree.cb M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 7 files changed, 575,155 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/6

EricR Lai (Code Review)

9 Nov 9 Nov

3:25 a.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 6: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 6 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.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: Sat, 09 Nov 2019 02:25:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Tim Wawrzynczak (Code Review)

11 Nov 11 Nov

5:03 p.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 6:

I think something is goofy with Jenkings. You should try re-uploading it to see if that will make the build bot happy.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 6 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.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: Mon, 11 Nov 2019 16:03:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Surendranath R Gurivireddy (Code Review)

11:25 p.m.

Hello Patrick Rudolph, EricR Lai, Bernardo Perez Priego, Aamir Bohra, Selma Bensaid, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

Added devicetree entry to set the flag to disable/enable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/7

Surendranath R Gurivireddy (Code Review)

12 Nov 12 Nov

8:42 p.m.

Hello Patrick Rudolph, EricR Lai, Bernardo Perez Priego, Aamir Bohra, Selma Bensaid, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

Added devicetree entry to set the flag to disable/enable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/8

Tim Wawrzynczak (Code Review)

8:57 p.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 8:

Hey Martin, something in Jenkins is unhappy with this CL:

https://qa.coreboot.org/job/coreboot-gerrit/108734/

Test Result (1 failure / +1) junit.xml.[failed-to-read]

Any ideas?

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 8 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 12 Nov 2019 19:57:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

EricR Lai (Code Review)

13 Nov 13 Nov

1:47 p.m.

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

Patch Set 8:

Something wrong by mistake adding howq... It's better to resubmit this?

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 8 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 13 Nov 2019 12:47:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Surendranath R Gurivireddy (Code Review)

15 Nov 15 Nov

7:50 p.m.

Hello Patrick Rudolph, EricR Lai, Bernardo Perez Priego, Aamir Bohra, Selma Bensaid, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP] ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating [WIP]

Added devicetree entry to set the flag to disable/enable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/9

Surendranath R Gurivireddy (Code Review)

7:58 p.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Hello Patrick Rudolph, EricR Lai, Bernardo Perez Priego, Aamir Bohra, Selma Bensaid, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating

Added devicetree entry to set the flag to disable/enable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/10

Nico Huber (Code Review)

8:30 p.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

Patch Set 10:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... PS3, Line 277: params->PchUsb2PhySusPgEnable = 0;

...

This change is done based on the Intel's recommendation (Sighting is attached in the partner issue_

Please add a reference (how to get that sighting document) to the commit message. Those `partner issues` are not public, AFAIK.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 10 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 15 Nov 2019 19:30:54 +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: Duncan Laurie dlaurie@chromium.org Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Comment-In-Reply-To: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-MessageType: comment

Surendranath R Gurivireddy (Code Review)

19 Nov 19 Nov

1:31 a.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Hello Patrick Rudolph, EricR Lai, Bernardo Perez Priego, Aamir Bohra, Selma Bensaid, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating

Workaround to disable USB2 PHY power gating to fix issue seen when Apple 87W USB-C charger is connected in S0ix state on WHL platforms (based on Intel's recommendation - https://cdrdv2.intel.com/v1/dl/getContent/599886). Issue is seen on CML platforms also. So,disabled power gating for Drallion.

Added devicetree entry to set the flag to disable/enable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/11

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 11 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Paul Menzel (Code Review)

1:29 p.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

Patch Set 11:

(2 comments)

https://review.coreboot.org/c/coreboot/+/36519/11//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/36519/11//COMMIT_MSG@13 PS11, Line 13: So,disabled So, disable …

https://review.coreboot.org/c/coreboot/+/36519/11//COMMIT_MSG@15 PS11, Line 15: Added Add

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 11 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 19 Nov 2019 12:29:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Surendranath R Gurivireddy (Code Review)

5:23 p.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Hello Patrick Rudolph, EricR Lai, Bernardo Perez Priego, Aamir Bohra, Selma Bensaid, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating

Add devicetree entry to set the flag to disable/enable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/12

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 12 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Tim Wawrzynczak (Code Review)

8:40 p.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

Patch Set 12: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 12 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 19 Nov 2019 19:40:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Duncan Laurie (Code Review)

22 Nov 22 Nov

12:25 a.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

Patch Set 12:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/12/src/soc/intel/cannonlake/c... File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/36519/12/src/soc/intel/cannonlake/c... PS12, Line 134: PchUsb2PhySusPgEnable Should this be inverted (default enabled) so boards don't have to explicitly enable?

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 12 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 21 Nov 2019 23:25:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Surendranath R Gurivireddy (Code Review)

12:51 a.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Surendranath R Gurivireddy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

Patch Set 12:

(5 comments)

https://review.coreboot.org/c/coreboot/+/36519/11//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/36519/11//COMMIT_MSG@13 PS11, Line 13: So,disabled

...

So, disable …

Done

https://review.coreboot.org/c/coreboot/+/36519/11//COMMIT_MSG@15 PS11, Line 15: Added

...

Add

Done

https://review.coreboot.org/c/coreboot/+/36519/12/src/soc/intel/cannonlake/c... File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/36519/12/src/soc/intel/cannonlake/c... PS12, Line 134: PchUsb2PhySusPgEnable

...

Should this be inverted (default enabled) so boards don't have to explicitly enable?

It's a good suggestion. I wanted to keep the same meaning/name as the corresponding FSP parameter to which this value is assigned. For Hatch, it's set to 1 by default. Please let me know if you still want it to be changed.

https://review.coreboot.org/c/coreboot/+/36519/5/src/soc/intel/cannonlake/ch... File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/36519/5/src/soc/intel/cannonlake/ch... PS5, Line 133: uint8_t PchUsb2PhySusPgEnable;

...

Add comment /* USB2 PHY power gating */

Done

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/36519/3/src/soc/intel/cannonlake/fs... PS3, Line 277: params->PchUsb2PhySusPgEnable = 0;

...

...
This change is done based on the Intel's recommendation (Sighting is attached in the partner issue […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 12 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 21 Nov 2019 23:51:59 +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: Duncan Laurie dlaurie@chromium.org Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-MessageType: comment

Duncan Laurie (Code Review)

12:56 a.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

Patch Set 12:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/12/src/soc/intel/cannonlake/c... File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/36519/12/src/soc/intel/cannonlake/c... PS12, Line 134: PchUsb2PhySusPgEnable

...

It's a good suggestion. […]

It looks like the FSP variable defaults to 1 though, which is something we can't do here. If someone looked at the FSP BSF and saw it says default 1 they might not set it in coreboot.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 12 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 21 Nov 2019 23:56:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Duncan Laurie dlaurie@chromium.org Comment-In-Reply-To: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-MessageType: comment

Surendranath R Gurivireddy (Code Review)

26 Nov 26 Nov

3:35 a.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Hello Patrick Rudolph, EricR Lai, Bernardo Perez Priego, Aamir Bohra, Selma Bensaid, Duncan Laurie, Tim Wawrzynczak, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating

Add devicetree entry to set the flag to disable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

Signed-off-by: Surendranath Gurivireddy surendranath.r.gurivireddy@intel.com Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 --- M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/sarien/variants/arcada/devicetree.cb M src/mainboard/google/sarien/variants/sarien/devicetree.cb M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 5 files changed, 11 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/36519/13

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 13 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Surendranath R Gurivireddy (Code Review)

3:38 a.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Surendranath R Gurivireddy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36519/12/src/soc/intel/cannonlake/c... File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/36519/12/src/soc/intel/cannonlake/c... PS12, Line 134: PchUsb2PhySusPgEnable

...

It looks like the FSP variable defaults to 1 though, which is something we can't do here. […]

@Duncan, Please see the updated patch set based on your suggestion.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 13 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 26 Nov 2019 02:38:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Duncan Laurie dlaurie@chromium.org Comment-In-Reply-To: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-MessageType: comment

Duncan Laurie (Code Review)

3:57 a.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

Patch Set 13: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 13 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 26 Nov 2019 02:57:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

EricR Lai (Code Review)

2:59 p.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

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

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

Patch Set 13: Code-Review+2

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/36519/3//COMMIT_MSG@9 PS3, Line 9: Workaround for an issue seen when a specific charger is connected while : in S0ix state

...

Which charger? What was the issue that this solves?

Ack

https://review.coreboot.org/c/coreboot/+/36519/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/36519/4/src/mainboard/google/dralli... PS4, Line 41: register "PchUsb2PhySusPgEnable" = "1"

...

drallion has this issue as well. Need client to decide need this or not.

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 13 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 26 Nov 2019 13:59:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: EricR Lai ericr_lai@compal.corp-partner.google.com Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-MessageType: comment

Patrick Georgi (Code Review)

27 Nov 27 Nov

2:50 p.m.

New subject: Change in coreboot[master]: soc/intel/cannonlake: Disable USB2 PHY Power gating

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36519 )

Change subject: soc/intel/cannonlake: Disable USB2 PHY Power gating ......................................................................

soc/intel/cannonlake: Disable USB2 PHY Power gating

Add devicetree entry to set the flag to disable USB2 PHY power gating for different CNL PCH based platforms

BUG=b:133775942 TEST=Connect Apple 87W USB-C charger when the system is in sleep and check if the system wakes up after that

Signed-off-by: Surendranath Gurivireddy surendranath.r.gurivireddy@intel.com Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Reviewed-on: https://review.coreboot.org/c/coreboot/+/36519 Reviewed-by: Duncan Laurie dlaurie@chromium.org Reviewed-by: EricR Lai ericr_lai@compal.corp-partner.google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/sarien/variants/arcada/devicetree.cb M src/mainboard/google/sarien/variants/sarien/devicetree.cb M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 5 files changed, 11 insertions(+), 0 deletions(-)

Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved EricR Lai: Looks good to me, approved

diff --git a/src/mainboard/google/drallion/variants/drallion/devicetree.cb b/src/mainboard/google/drallion/variants/drallion/devicetree.cb index ed44f4f..6ecb689 100644 --- a/src/mainboard/google/drallion/variants/drallion/devicetree.cb +++ b/src/mainboard/google/drallion/variants/drallion/devicetree.cb @@ -38,6 +38,8 @@ register "PchPmSlpSusMinAssert" = "4" # 4s register "PchPmSlpAMinAssert" = "4" # 2s register "PchUnlockGpioPads" = "1" + # USB2 PHY Power gating + register "PchUsb2PhySusPgDisable" = "1"

register "speed_shift_enable" = "1" register "psys_pmax" = "140" diff --git a/src/mainboard/google/sarien/variants/arcada/devicetree.cb b/src/mainboard/google/sarien/variants/arcada/devicetree.cb index 4af8ca2..6bc3df1 100644 --- a/src/mainboard/google/sarien/variants/arcada/devicetree.cb +++ b/src/mainboard/google/sarien/variants/arcada/devicetree.cb @@ -27,6 +27,8 @@ register "PchPmSlpSusMinAssert" = "4" # 4s register "PchPmSlpAMinAssert" = "4" # 2s register "PchUnlockGpioPads" = "1" + # USB2 PHY Power gating + register "PchUsb2PhySusPgDisable" = "1"

register "speed_shift_enable" = "1" register "psys_pmax" = "140" diff --git a/src/mainboard/google/sarien/variants/sarien/devicetree.cb b/src/mainboard/google/sarien/variants/sarien/devicetree.cb index cd216e5..b2aa8d5 100644 --- a/src/mainboard/google/sarien/variants/sarien/devicetree.cb +++ b/src/mainboard/google/sarien/variants/sarien/devicetree.cb @@ -30,6 +30,8 @@ register "PchPmSlpS4MinAssert" = "4" # 4s register "PchPmSlpSusMinAssert" = "4" # 4s register "PchPmSlpAMinAssert" = "4" # 2s + # USB2 PHY Power gating + register "PchUsb2PhySusPgDisable" = "1"

register "speed_shift_enable" = "1" register "s0ix_enable" = "1" diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h index 507290f..f08fd0a 100644 --- a/src/soc/intel/cannonlake/chip.h +++ b/src/soc/intel/cannonlake/chip.h @@ -130,6 +130,8 @@ uint16_t usb2_wake_enable_bitmap; /* Wake Enable Bitmap for USB3 ports */ uint16_t usb3_wake_enable_bitmap; + /* USB2 PHY power gating */ + uint8_t PchUsb2PhySusPgDisable;

/* SATA related */ enum { diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index 0713ef4..dfc7e22 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -273,6 +273,9 @@ params->Usb2AfePehalfbit[i] = config->usb2_ports[i].pre_emp_bit; }

+ if (config->PchUsb2PhySusPgDisable) + params->PchUsb2PhySusPgEnable = 0; + for (i = 0; i < ARRAY_SIZE(config->usb3_ports); i++) { params->PortUsb30Enable[i] = config->usb3_ports[i].enable; params->Usb3OverCurrentPin[i] = config->usb3_ports[i].ocpin;

-- To view, visit https://review.coreboot.org/c/coreboot/+/36519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95909c73de758fccc7f616a330c1e1f0667e8c25 Gerrit-Change-Number: 36519 Gerrit-PatchSet: 14 Gerrit-Owner: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Bernardo Perez Priego bernardo.perez.priego@intel.com Gerrit-Reviewer: Bora Guvendik bora.guvendik@intel.com 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: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Selma Bensaid selma.bensaid@intel.com Gerrit-Reviewer: Surendranath R Gurivireddy surendranath.r.gurivireddy@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin Roth martinroth@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1762

days inactive

1789

days old

coreboot-gerrit@coreboot.org

37 comments

8 participants

tags (0)

participants (8)

build bot (Jenkins) (Code Review)
Duncan Laurie (Code Review)
EricR Lai (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)
Surendranath R Gurivireddy (Code Review)
Tim Wawrzynczak (Code Review)