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 th3fanbus@gmail.com --- 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 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 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...
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 th3fanbus@gmail.com --- 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 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 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 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 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 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) no-reply@coreboot.org
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 th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42643 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org --- 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.
-
Angel Pons (Code Review) -
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)