Martijn Berger has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39965 )
Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ......................................................................
pcidev: On arm64 we are not getting a valid BAR0 address
It seems that there is some code to work around a possible bug in old versions of libpci. This trusts libpci if it is new enough
Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Signed-off-by: Martijn Berger martijn.berger@gmail.com --- M pcidev.c M programmer.h 2 files changed, 30 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/39965/1
diff --git a/pcidev.c b/pcidev.c index 54c1fd3..973acde 100644 --- a/pcidev.c +++ b/pcidev.c @@ -30,20 +30,47 @@ TYPE_UNKNOWN };
+int pci_bar_nuber_from_offset(int offset) +{ + switch (offset) { + case PCI_BASE_ADDRESS_0: + return 0; + case PCI_BASE_ADDRESS_1: + return 1; + case PCI_BASE_ADDRESS_2: + return 2; + case PCI_BASE_ADDRESS_3: + return 3; + case PCI_BASE_ADDRESS_4: + return 4; + case PCI_BASE_ADDRESS_5: + return 5; + default: + return -1; + } + return -1; +} + uintptr_t pcidev_readbar(struct pci_dev *dev, int bar) { uint64_t addr; uint32_t upperaddr; uint8_t headertype; uint16_t supported_cycles; + int bar_number; enum pci_bartype bartype = TYPE_UNKNOWN;
headertype = pci_read_byte(dev, PCI_HEADER_TYPE) & 0x7f; msg_pspew("PCI header type 0x%02x\n", headertype); + bar_number = pci_bar_nuber_from_offset(bar);
- /* Don't use dev->base_addr[x] (as value for 'bar'), won't work on older libpci. */ - addr = pci_read_long(dev, bar); + if (PCI_LIB_VERSION >= 0x030500 && bar_number > -1) { + addr = dev->base_addr[bar_number]; + } else { + /* Don't use dev->base_addr[x] (as value for 'bar'), won't work on older libpci. */ + addr = pci_read_long(dev, bar); + }
/* Sanity checks. */ switch (headertype) { diff --git a/programmer.h b/programmer.h index 9a41be1..5d4c1d7 100644 --- a/programmer.h +++ b/programmer.h @@ -190,6 +190,7 @@ // FIXME: This needs to be local, not global(?) extern struct pci_access *pacc; int pci_init_common(void); +int pci_bar_nuber_from_offset(int offset); uintptr_t pcidev_readbar(struct pci_dev *dev, int bar); struct pci_dev *pcidev_init(const struct dev_entry *devs, int bar); /* rpci_write_* are reversible writes. The original PCI config space register
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39965 )
Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG@7 PS1, Line 7: pcidev: On arm64 we are not getting a valid BAR0 address The commit summary should be a sentence in imperative tense, which summarizes what this change does. The currrent sentence briefly describes the problem you have encountered.
https://review.coreboot.org/c/flashrom/+/39965/1/pcidev.c File pcidev.c:
https://review.coreboot.org/c/flashrom/+/39965/1/pcidev.c@33 PS1, Line 33: nuber number
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39965 )
Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ......................................................................
Patch Set 1:
(2 comments)
Thank you for your fixes!
https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG@9 PS1, Line 9: It seems that there is some code to work around a possible bug in : old versions of libpci. Please add the problem description again. Maybe (no idea if correct):
… On arm64, this results in an invalid BAR0 address.
https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG@10 PS1, Line 10: This trusts libpci if it is new enough Please add a dot/period at the end of the sentence. Please mention, the version in the commit message too.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/39965
to look at the new patch set (#2).
Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ......................................................................
pcidev: On arm64 we are not getting a valid BAR0 address
It seems that there is some code to work around a possible bug in old versions of libpci. This trusts libpci if it is new enough
Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Signed-off-by: Martijn Berger martijn.berger@gmail.com --- M pcidev.c M programmer.h 2 files changed, 30 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/39965/2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39965 )
Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/flashrom/+/39965/3/pcidev.c File pcidev.c:
https://review.coreboot.org/c/flashrom/+/39965/3/pcidev.c@54 PS3, Line 54: int bar unsigned bar
https://review.coreboot.org/c/flashrom/+/39965/3/pcidev.c@66 PS3, Line 66: bar_number = pci_bar_number_from_offset(bar); why not just `int bar_number = bar < 6 ? bar : -1;` here and make bar unsigned?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39965 )
Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ......................................................................
Patch Set 4: Code-Review+1
(3 comments)
https://review.coreboot.org/c/flashrom/+/39965/4/pcidev.c File pcidev.c:
https://review.coreboot.org/c/flashrom/+/39965/4/pcidev.c@33 PS4, Line 33: int pci_bar_number_from_offset(int offset) I'd make this static
https://review.coreboot.org/c/flashrom/+/39965/4/pcidev.c@48 PS4, Line 48: default: : return -1; : } : return -1; One of these return statements is redundant
https://review.coreboot.org/c/flashrom/+/39965/4/pcidev.c@60 PS4, Line 60: bar_number bar_index?
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Martijn Berger (Code Review)
-
Paul Menzel (Code Review)