Change in coreboot[master]: sb/intel/i82801jx/sata.c: Handle ABAR as a resource

[Angel Pons (Code Review)]

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

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

sb/intel/i82801jx/sata.c: Handle ABAR as a resource

Instead of directly reading ABAR without any checking, do like i82801ix
and treat it as a resource. This prevents problems if ABAR is not set.

Change-Id: I4f888b748204860b0a7e1bf5611f5f3e487e8081
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/i82801jx/sata.c
1 file changed, 6 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/42643/1

diff --git a/src/southbridge/intel/i82801jx/sata.c b/src/southbridge/intel/i82801jx/sata.c
index 73a7d82..947842a 100644
--- a/src/southbridge/intel/i82801jx/sata.c
+++ b/src/southbridge/intel/i82801jx/sata.c
@@ -20,9 +20,14 @@
 {
    int i;
    u32 reg32;
+	struct resource *res;

/* Initialize AHCI memory-mapped space */
-	u8 *abar = (u8 *)pci_read_config32(dev, PCI_BASE_ADDRESS_5);
+	res = find_resource(dev, PCI_BASE_ADDRESS_5);
+	if (!res)
+		return;
+
+	u8 *abar = res2mmio(res, 0, 0);
    printk(BIOS_DEBUG, "ABAR: %p\n", abar);

/* Set AHCI access mode.

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42643 )

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/42643/2/src/southbridge/intel/i8280... 
File src/southbridge/intel/i82801jx/sata.c:

https://review.coreboot.org/c/coreboot/+/42643/2/src/southbridge/intel/i8280... 
PS2, Line 26: find_resource
probe_resource

[Angel Pons (Code Review)]

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

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42643/2/src/southbridge/intel/i8280... 
File src/southbridge/intel/i82801jx/sata.c:

https://review.coreboot.org/c/coreboot/+/42643/2/src/southbridge/intel/i8280... 
PS2, Line 26: find_resource
> probe_resource

Hrm, I used find_resource because it's what i82801ix does. However, I see that it dies so it never returns false...

[Angel Pons (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

sb/intel/i82801jx/sata.c: Handle ABAR as a resource

Instead of directly reading ABAR without any checking, do like i82801ix
and treat it as a resource. This prevents problems if ABAR is not set.

Change-Id: I4f888b748204860b0a7e1bf5611f5f3e487e8081
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/i82801jx/sata.c
1 file changed, 6 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/42643/3

[Angel Pons (Code Review)]

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

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42643/2/src/southbridge/intel/i8280... 
File src/southbridge/intel/i82801jx/sata.c:

https://review.coreboot.org/c/coreboot/+/42643/2/src/southbridge/intel/i8280... 
PS2, Line 26: find_resource
> Hrm, I used find_resource because it's what i82801ix does. […]

Done

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42643 )

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

Patch Set 3: Code-Review+2

[Paul Menzel (Code Review)]

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

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

Patch Set 3:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/42643/3//COMMIT_MSG@10 
PS3, Line 10: and treat it as a resource. This prevents problems if ABAR is not set.
What kind of problems? ;-)

[Angel Pons (Code Review)]

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

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

Patch Set 3:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/42643/3//COMMIT_MSG@10 
PS3, Line 10: and treat it as a resource. This prevents problems if ABAR is not set.
> What kind of problems? ;-)

No idea. Most likely undefined behavior, but I haven't tried.

[Angel Pons (Code Review)]

Angel Pons has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/42643 )

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

Removed Verified+1 by build bot (Jenkins)

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42643 )

Change subject: sb/intel/i82801jx/sata.c: Handle ABAR as a resource
......................................................................

sb/intel/i82801jx/sata.c: Handle ABAR as a resource

Instead of directly reading ABAR without any checking, do like i82801ix
and treat it as a resource. This prevents problems if ABAR is not set.

Change-Id: I4f888b748204860b0a7e1bf5611f5f3e487e8081
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42643
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Rudolph 
---
M src/southbridge/intel/i82801jx/sata.c
1 file changed, 6 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Rudolph: Looks good to me, approved

diff --git a/src/southbridge/intel/i82801jx/sata.c b/src/southbridge/intel/i82801jx/sata.c
index 73a7d82..0372460 100644
--- a/src/southbridge/intel/i82801jx/sata.c
+++ b/src/southbridge/intel/i82801jx/sata.c
@@ -20,9 +20,14 @@
 {
    int i;
    u32 reg32;
+	struct resource *res;

/* Initialize AHCI memory-mapped space */
-	u8 *abar = (u8 *)pci_read_config32(dev, PCI_BASE_ADDRESS_5);
+	res = probe_resource(dev, PCI_BASE_ADDRESS_5);
+	if (!res)
+		return;
+
+	u8 *abar = res2mmio(res, 0, 0);
    printk(BIOS_DEBUG, "ABAR: %p\n", abar);

/* Set AHCI access mode.