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 
---
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 */

[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 
---
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

[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?

[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

[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?

[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.

[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 
---
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

[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

[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

[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

[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...

[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 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47029
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
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);