Change in coreboot[master]: sb/intel/lynxpoint: Correct read width in RMW cycle

[Angel Pons (Code Review)]

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

Change subject: sb/intel/lynxpoint: Correct read width in RMW cycle
......................................................................

sb/intel/lynxpoint: Correct read width in RMW cycle

The register is 32 bits wide, so do not read 16 bits out of it.

Change-Id: I18fbba0603579417e09ae4eb4eb273f7fcd903fc
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/lynxpoint/sata.c
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/47098/1

diff --git a/src/southbridge/intel/lynxpoint/sata.c b/src/southbridge/intel/lynxpoint/sata.c
index fdbe062..8fb6a94 100644
--- a/src/southbridge/intel/lynxpoint/sata.c
+++ b/src/southbridge/intel/lynxpoint/sata.c
@@ -67,7 +67,7 @@
    udelay(2);

/* Setup register 98h */
-	reg32 = pci_read_config16(dev, 0x98);
+	reg32 = pci_read_config32(dev, 0x98);
    reg32 |= 1 << 19;    /* BWG step 6 */
    reg32 |= 1 << 22;    /* BWG step 5 */
    reg32 &= ~(0x3f << 7);

[Patrick Rudolph (Code Review)]

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

Change subject: sb/intel/lynxpoint: Correct read width in RMW cycle
......................................................................

Patch Set 4:

(1 comment)

File src/southbridge/intel/lynxpoint/sata.c:

https://review.coreboot.org/c/coreboot/+/47098/comment/f83c2810_ce0c7d5f 
PS4, Line 70: 	reg32 = pci_read_config32(dev, 0x98);
This was likely done on purpose to not clear bits [16:32]. Some bits here are RWONCE and are undefined on the first read.

[Angel Pons (Code Review)]

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

Change subject: sb/intel/lynxpoint: Correct read width in RMW cycle
......................................................................

Patch Set 4:

(1 comment)

File src/southbridge/intel/lynxpoint/sata.c:

https://review.coreboot.org/c/coreboot/+/47098/comment/554b751d_c22cfedd 
PS4, Line 70: 	reg32 = pci_read_config32(dev, 0x98);
> This was likely done on purpose to not clear bits [16:32]. […]

IIRC, I checked reference code and it does a 32-bit read. I'll check again and update the commit message accordingly (say "reference code version F.O.O does this" or similar).

Until then, I'm leaving this as unresolved

[Angel Pons (Code Review)]

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

Change subject: sb/intel/lynxpoint: Correct read width in RMW cycle
......................................................................

Patch Set 4:

(1 comment)

File src/southbridge/intel/lynxpoint/sata.c:

https://review.coreboot.org/c/coreboot/+/47098/comment/ad39be4d_f6eb40f3 
PS4, Line 70: 	reg32 = pci_read_config32(dev, 0x98);
> IIRC, I checked reference code and it does a 32-bit read. […]

I've checked, and reference code always does 32-bit operations.

[Angel Pons (Code Review)]

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

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

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

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

Change subject: sb/intel/lynxpoint: Correct read width in RMW cycle
......................................................................

sb/intel/lynxpoint: Correct read width in RMW cycle

The register is 32 bits wide, so do not read 16 bits out of it.
LynxPoint PCH reference code version 1.9.1 always uses 32-bit accesses.

Change-Id: I18fbba0603579417e09ae4eb4eb273f7fcd903fc
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/lynxpoint/sata.c
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/47098/5

[Nico Huber (Code Review)]

Attention is currently required from: Patrick Rudolph, Angel Pons, Arthur Heymans.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47098 )

Change subject: sb/intel/lynxpoint: Correct read width in RMW cycle
......................................................................

Patch Set 6: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: sb/intel/lynxpoint: Correct read width in RMW cycle
......................................................................

sb/intel/lynxpoint: Correct read width in RMW cycle

The register is 32 bits wide, so do not read 16 bits out of it.
LynxPoint PCH reference code version 1.9.1 always uses 32-bit accesses.

Change-Id: I18fbba0603579417e09ae4eb4eb273f7fcd903fc
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47098
Reviewed-by: Nico Huber 
Tested-by: build bot (Jenkins) 
---
M src/southbridge/intel/lynxpoint/sata.c
1 file changed, 1 insertion(+), 1 deletion(-)

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

diff --git a/src/southbridge/intel/lynxpoint/sata.c b/src/southbridge/intel/lynxpoint/sata.c
index 33a048d..2a35600 100644
--- a/src/southbridge/intel/lynxpoint/sata.c
+++ b/src/southbridge/intel/lynxpoint/sata.c
@@ -74,7 +74,7 @@
    udelay(2);

/* Setup register 98h */
-	reg32 = pci_read_config16(dev, 0x98);
+	reg32 = pci_read_config32(dev, 0x98);
    reg32 |= 1 << 19;    /* BWG step 6 */
    reg32 |= 1 << 22;    /* BWG step 5 */
    reg32 &= ~(0x3f << 7);