Hello Nico Huber, Arthur Heymans, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48538
to review the following change.
Change subject: sb/intel/common/smbus_ops.c: Clean up read resources ......................................................................
sb/intel/common/smbus_ops.c: Clean up read resources
UNTESTED.
Change-Id: I1cb05131a82ebb7c45827eff8e09e445d9c695b3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/common/smbus_ops.c 1 file changed, 2 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/48538/1
diff --git a/src/southbridge/intel/common/smbus_ops.c b/src/southbridge/intel/common/smbus_ops.c index b0ecc1a..46fc58a 100644 --- a/src/southbridge/intel/common/smbus_ops.c +++ b/src/southbridge/intel/common/smbus_ops.c @@ -67,17 +67,12 @@
void smbus_read_resources(struct device *dev) { + pci_dev_read_resources(dev); + struct resource *res = new_resource(dev, PCI_BASE_ADDRESS_4); res->base = CONFIG_FIXED_SMBUS_IO_BASE; res->size = 32; res->limit = res->base + res->size - 1; res->flags = IORESOURCE_IO | IORESOURCE_FIXED | IORESOURCE_RESERVE | IORESOURCE_STORED | IORESOURCE_ASSIGNED; - - /* The memory BAR does not exist for ICH7 and earlier */ - if (CONFIG(SOUTHBRIDGE_INTEL_I82801GX)) - return; - - /* Also add MMIO resource */ - res = pci_get_resource(dev, PCI_BASE_ADDRESS_0); }
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48538 )
Change subject: sb/intel/common/smbus_ops.c: Clean up read resources ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48538/1/src/southbridge/intel/commo... File src/southbridge/intel/common/smbus_ops.c:
https://review.coreboot.org/c/coreboot/+/48538/1/src/southbridge/intel/commo... PS1, Line 70: pci_dev_read_resources(dev); This will now make BAR1 be considered
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48538 )
Change subject: sb/intel/common/smbus_ops.c: Clean up read resources ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48538/1/src/southbridge/intel/commo... File src/southbridge/intel/common/smbus_ops.c:
https://review.coreboot.org/c/coreboot/+/48538/1/src/southbridge/intel/commo... PS1, Line 70: pci_dev_read_resources(dev);
This will now make BAR1 be considered
BAR0 is 64-bit, BAR1 does not exist.
Hello build bot (Jenkins), Nico Huber, Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48538
to look at the new patch set (#2).
Change subject: sb/intel/common/smbus_ops.c: Clean up read resources ......................................................................
sb/intel/common/smbus_ops.c: Clean up read resources
Using `pci_dev_read_resources` works just as well on bd82x6x (the allocator does the same) and allows dropping the i82801gx check.
Change-Id: I1cb05131a82ebb7c45827eff8e09e445d9c695b3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/common/smbus_ops.c 1 file changed, 2 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/48538/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48538 )
Change subject: sb/intel/common/smbus_ops.c: Clean up read resources ......................................................................
Patch Set 2: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48538 )
Change subject: sb/intel/common/smbus_ops.c: Clean up read resources ......................................................................
sb/intel/common/smbus_ops.c: Clean up read resources
Using `pci_dev_read_resources` works just as well on bd82x6x (the allocator does the same) and allows dropping the i82801gx check.
Change-Id: I1cb05131a82ebb7c45827eff8e09e445d9c695b3 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48538 Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/southbridge/intel/common/smbus_ops.c 1 file changed, 2 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/southbridge/intel/common/smbus_ops.c b/src/southbridge/intel/common/smbus_ops.c index b0ecc1a..46fc58a 100644 --- a/src/southbridge/intel/common/smbus_ops.c +++ b/src/southbridge/intel/common/smbus_ops.c @@ -67,17 +67,12 @@
void smbus_read_resources(struct device *dev) { + pci_dev_read_resources(dev); + struct resource *res = new_resource(dev, PCI_BASE_ADDRESS_4); res->base = CONFIG_FIXED_SMBUS_IO_BASE; res->size = 32; res->limit = res->base + res->size - 1; res->flags = IORESOURCE_IO | IORESOURCE_FIXED | IORESOURCE_RESERVE | IORESOURCE_STORED | IORESOURCE_ASSIGNED; - - /* The memory BAR does not exist for ICH7 and earlier */ - if (CONFIG(SOUTHBRIDGE_INTEL_I82801GX)) - return; - - /* Also add MMIO resource */ - res = pci_get_resource(dev, PCI_BASE_ADDRESS_0); }
-
Angel Pons (Code Review) -
Nico Huber (Code Review)