Change in coreboot[master]: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47029 )


Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................

soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

WildcatPoint-LP BIOS spec lists them, and are the same for Lynxpoint.
Keep the BWG comments for now, since both platforms will be merged soon.

Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Signed-off-by: Angel Pons <[e-mail]>
---
M src/soc/intel/broadwell/pch/sata.c
1 file changed, 9 insertions(+), 0 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/47029/1

diff --git a/src/soc/intel/broadwell/pch/sata.c b/src/soc/intel/broadwell/pch/sata.c
index 91c2ff5..e9d70f1 100644
--- a/src/soc/intel/broadwell/pch/sata.c
+++ b/src/soc/intel/broadwell/pch/sata.c
@@ -95,6 +95,15 @@
 
 	/* Setup register 98h */
 	reg32 = pci_read_config32(dev, 0x98);
+	reg32 |= 1 << 19;    /* BWG step 6 */
+	reg32 |= 1 << 22;    /* BWG step 5 */
+	reg32 &= ~(0x3f << 7);
+	reg32 |= 0x04 << 7;  /* BWG step 7 */
+	reg32 |= 1 << 20;    /* BWG step 8 */
+	reg32 &= ~(0x03 << 5);
+	reg32 |= 1 << 5;     /* BWG step 9 */
+	reg32 |= 1 << 18;    /* BWG step 10 */
+	reg32 |= 1 << 29;    /* BWG step 11 */
 	reg32 &= ~((1 << 31) | (1 << 30));
 	reg32 |= 1 << 23;
 	reg32 |= 1 << 24; /* Enable MPHY Dynamic Power Gating */

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-MessageType: newchange

[Angel Pons (Code Review)]

Hello Patrick Rudolph, 

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

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

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

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................

soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

WildcatPoint-LP BIOS spec lists them, and are the same for Lynxpoint.
Keep the BWG comments for now, since both platforms will be merged soon.

Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Signed-off-by: Angel Pons <[e-mail]>
---
M src/soc/intel/broadwell/pch/sata.c
1 file changed, 9 insertions(+), 0 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/47029/2
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 2
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-CC: build bot (Jenkins) <[e-mail]>
Gerrit-MessageType: newpatchset

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47029 )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Patch Set 2:

tested?


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 2
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Fri, 13 Nov 2020 12:14:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47029 )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Patch Set 2:
> Patch Set 2:
>
> tested?

Nope, but will test soon


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 2
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Fri, 13 Nov 2020 12:18:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Nico Huber (Code Review)]

Attention is currently required from: Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47029 )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

File src/soc/intel/broadwell/pch/sata.c:

https://review.coreboot.org/c/coreboot/+/47029/comment/d74736ca_ef3ce32b 
PS3, Line 66: 	reg32 |= 1 << 29;    /* BWG step 11 */
Already set near the end of this function?



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 3
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Attention: Angel Pons <[e-mail]>
Gerrit-Comment-Date: Mon, 08 Feb 2021 21:26:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Nico Huber (Code Review)]

Attention is currently required from: Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47029 )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Patch Set 3:

(1 comment)

Commit Message:

https://review.coreboot.org/c/coreboot/+/47029/comment/2ec088be_576f2075 
PS3, Line 10: BWG
BWG is not the same as BIOS spec (even though most insiders don't
know the difference). I've also noticed that the numbers you commented
match the LP BIOS spec, not the mentioned WP-LP one.



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 3
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Attention: Angel Pons <[e-mail]>
Gerrit-Comment-Date: Mon, 08 Feb 2021 21:28:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Angel Pons (Code Review)]

Attention is currently required from: Nico Huber.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47029 )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Patch Set 3:

(1 comment)

Commit Message:

https://review.coreboot.org/c/coreboot/+/47029/comment/f9355ae6_faa33b12 
PS3, Line 10: BWG
> BWG is not the same as BIOS spec (even though most insiders don't […]
> I meant the "BWG step" comments on the code.

[Angel Pons (Code Review)]

Attention is currently required from: Nico Huber.
Hello build bot (Jenkins), Nico Huber, Patrick Rudolph, 

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

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

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

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................

soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

WildcatPoint-LP BIOS spec lists them, and are the same for Lynxpoint.

Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Signed-off-by: Angel Pons <[e-mail]>
---
M src/soc/intel/broadwell/pch/sata.c
1 file changed, 9 insertions(+), 4 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/47029/5
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 5
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Attention: Nico Huber <[e-mail]>
Gerrit-MessageType: newpatchset

[Angel Pons (Code Review)]

Attention is currently required from: Nico Huber.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47029 )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Patch Set 5:

(2 comments)

Patchset:

PS5: 
I should re-test


File src/soc/intel/broadwell/pch/sata.c:

https://review.coreboot.org/c/coreboot/+/47029/comment/c435c9fa_bcb6158d 
PS3, Line 66: 	reg32 |= 1 << 29;    /* BWG step 11 */
> Already set near the end of this function?
> Done

[Nico Huber (Code Review)]

Attention is currently required from: Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47029 )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 5
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Attention: Angel Pons <[e-mail]>
Gerrit-Comment-Date: Mon, 08 Feb 2021 21:51:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[S] Change in coreboot[master]: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

[Stefan Reinauer (Code Review)]

Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47029?usp=email )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Abandoned
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 5
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: abandon

[S] Change in coreboot[master]: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

[Angel Pons (Code Review)]

Angel Pons has restored this change. ( https://review.coreboot.org/c/coreboot/+/47029?usp=email )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Restored
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 5
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: restore

[S] Change in coreboot[main]: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47029?usp=email )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Patch Set 6:

(1 comment)

Patchset:

PS5:
> I should re-test
> Acknowledged

[S] Change in coreboot[main]: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

[Nico Huber (Code Review)]

Attention is currently required from: Angel Pons.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47029?usp=email )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................


Patch Set 6: Code-Review+2

(1 comment)

Patchset:

PS6: 
Going to trust my old +2...



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 6
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-CC: Stefan Reinauer <[e-mail]>
Gerrit-Attention: Angel Pons <[e-mail]>
Gerrit-Comment-Date: Sat, 13 Apr 2024 11:13:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[S] Change in coreboot[main]: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47029?usp=email )

Change subject: soc/intel/broadwell/pch/sata.c: Add missing SATA init steps
......................................................................

soc/intel/broadwell/pch/sata.c: Add missing SATA init steps

WildcatPoint-LP BIOS spec lists them, and are the same for Lynxpoint.

Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Signed-off-by: Angel Pons <[e-mail]>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47029
Tested-by: build bot (Jenkins) <[e-mail]>
Reviewed-by: Nico Huber <[e-mail]>
---
M src/soc/intel/broadwell/pch/sata.c
1 file changed, 9 insertions(+), 4 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved




diff --git a/src/soc/intel/broadwell/pch/sata.c b/src/soc/intel/broadwell/pch/sata.c
index edb9830..92d34c3 100644
--- a/src/soc/intel/broadwell/pch/sata.c
+++ b/src/soc/intel/broadwell/pch/sata.c
@@ -54,6 +54,15 @@
 
 	/* Setup register 98h */
 	reg32 = pci_read_config32(dev, 0x98);
+	reg32 |= 1 << 19;
+	reg32 |= 1 << 22;
+	reg32 &= ~(0x3f << 7);
+	reg32 |= 0x04 << 7;
+	reg32 |= 1 << 20;
+	reg32 &= ~(0x03 << 5);
+	reg32 |= 1 << 5;
+	reg32 |= 1 << 18;
+	reg32 |= 1 << 29; /* Enable clock gating */
 	reg32 &= ~((1 << 31) | (1 << 30));
 	reg32 |= 1 << 23;
 	reg32 |= 1 << 24; /* Enable MPHY Dynamic Power Gating */
@@ -237,10 +246,6 @@
 	reg32 |= (1 << 31) | (1 << 30) | (1 << 29);
 	pci_write_config32(dev, 0x300, reg32);
 
-	reg32 = pci_read_config32(dev, 0x98);
-	reg32 |= 1 << 29;
-	pci_write_config32(dev, 0x98, reg32);
-
 	/* Register Lock */
 	reg32 = pci_read_config32(dev, 0x9c);
 	reg32 |= (1 << 31);

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/47029?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iba28c1591affafeb37097084c2fa58128974bd00
Gerrit-Change-Number: 47029
Gerrit-PatchSet: 7
Gerrit-Owner: Angel Pons <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-CC: Stefan Reinauer <[e-mail]>
Gerrit-MessageType: merged