[flashrom] flashrom -p gfxnvidia detect my atheros ath9k wifi card
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Sat Jun 30 01:40:44 CEST 2012
Am 28.06.2012 20:28 schrieb Denis 'GNUtoo' Carikli:
> On Wed, 2012-06-27 at 09:05 +0200, Carl-Daniel Hailfinger wrote:
>> > Hi Denis,
>> >
>> > Am 26.06.2012 01:53 schrieb Denis 'GNUtoo' Carikli:
>>> > > # ./flashrom -p gfxnvidia -VVV
>>> > > flashrom v0.9.5.2-r1546 on Linux 3.0.0-19-generic-pae (i686)
>>> > > flashrom was built with libpci 3.1.7, GCC 4.6.1, little endian
>>> > > Command line (3 args): ./flashrom -p gfxnvidia -VVV
>>> > > Calibrating delay loop... OK.
>>> > > Initializing gfxnvidia programmer
>>> > > Found "NVIDIA RIVA TNT2 Ultra" (168c:0029, BDF 03:06.0).
>>> > > [...]
>> >
>> > This should not have happened. Apparently the PCI code (either in
>> > flashrom, or libpci) ignores the vendor, and only matches the device in
>> > this case.
>> >
>> > Ah yes. pcidev.c:pcidev_validate() only checks device_id match, not
>> > vendor_id match. Ouch!
>> >
>> > Thanks for reporting this bug!
>> >
I'd be very interested in the flashrom verbose output for the following
patch:
Check vendor_id for PCI based external programmers.
Clean up PCI device detection code.
Note: Slight changes in behaviour are possible, especially on dual/quad
chip NICs which appear as more than one PCI device. Found devices are no
longer printed at _pinfo level, but rather at _pdbg level.
This patch is essentially a small code move, better device counting and
lots of indentation changes (look at diff -uw to see real code changes).
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Index: flashrom-pcidev_check_vendorid_cleanup/pcidev.c
===================================================================
--- flashrom-pcidev_check_vendorid_cleanup/pcidev.c (Revision 1547)
+++ flashrom-pcidev_check_vendorid_cleanup/pcidev.c (Arbeitskopie)
@@ -35,157 +35,130 @@
TYPE_UNKNOWN
};
-uintptr_t pcidev_validate(struct pci_dev *dev, int bar,
- const struct pcidev_status *devs)
+uintptr_t pcidev_validate(struct pci_dev *dev, int bar)
{
- int i;
uint64_t addr;
uint32_t upperaddr;
uint8_t headertype;
uint16_t supported_cycles;
enum pci_bartype bartype = TYPE_UNKNOWN;
- for (i = 0; devs[i].device_name != NULL; i++) {
- if (dev->device_id != devs[i].device_id)
- continue;
- msg_pinfo("Found \"%s %s\" (%04x:%04x, BDF %02x:%02x.%x).\n",
- devs[i].vendor_name, devs[i].device_name,
- dev->vendor_id, dev->device_id, dev->bus, dev->dev,
- dev->func);
+ headertype = pci_read_byte(dev, PCI_HEADER_TYPE) & 0x7f;
+ msg_pspew("PCI header type 0x%02x\n", headertype);
- headertype = pci_read_byte(dev, PCI_HEADER_TYPE) & 0x7f;
- msg_pspew("PCI header type 0x%02x\n", headertype);
+ /* Don't use dev->base_addr[x] (as value for 'bar'), won't work on older libpci. */
+ addr = pci_read_long(dev, bar);
- /*
- * 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) {
- case PCI_HEADER_TYPE_NORMAL:
- switch (bar) {
- case PCI_BASE_ADDRESS_0:
- case PCI_BASE_ADDRESS_1:
- case PCI_BASE_ADDRESS_2:
- case PCI_BASE_ADDRESS_3:
- case PCI_BASE_ADDRESS_4:
- case PCI_BASE_ADDRESS_5:
- if ((addr & PCI_BASE_ADDRESS_SPACE) ==
- PCI_BASE_ADDRESS_SPACE_IO)
- bartype = TYPE_IOBAR;
- else
- bartype = TYPE_MEMBAR;
- break;
- case PCI_ROM_ADDRESS:
- bartype = TYPE_ROMBAR;
- break;
- }
+ /* Sanity checks. */
+ switch (headertype) {
+ case PCI_HEADER_TYPE_NORMAL:
+ switch (bar) {
+ case PCI_BASE_ADDRESS_0:
+ case PCI_BASE_ADDRESS_1:
+ case PCI_BASE_ADDRESS_2:
+ case PCI_BASE_ADDRESS_3:
+ case PCI_BASE_ADDRESS_4:
+ case PCI_BASE_ADDRESS_5:
+ if ((addr & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO)
+ bartype = TYPE_IOBAR;
+ else
+ bartype = TYPE_MEMBAR;
break;
- case PCI_HEADER_TYPE_BRIDGE:
- switch (bar) {
- case PCI_BASE_ADDRESS_0:
- case PCI_BASE_ADDRESS_1:
- if ((addr & PCI_BASE_ADDRESS_SPACE) ==
- PCI_BASE_ADDRESS_SPACE_IO)
- bartype = TYPE_IOBAR;
- else
- bartype = TYPE_MEMBAR;
- break;
- case PCI_ROM_ADDRESS1:
- bartype = TYPE_ROMBAR;
- break;
- }
+ case PCI_ROM_ADDRESS:
+ bartype = TYPE_ROMBAR;
break;
- case PCI_HEADER_TYPE_CARDBUS:
+ }
+ break;
+ case PCI_HEADER_TYPE_BRIDGE:
+ switch (bar) {
+ case PCI_BASE_ADDRESS_0:
+ case PCI_BASE_ADDRESS_1:
+ if ((addr & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO)
+ bartype = TYPE_IOBAR;
+ else
+ bartype = TYPE_MEMBAR;
break;
- default:
- msg_perr("Unknown PCI header type 0x%02x, BAR type "
- "cannot be determined reliably.\n", headertype);
+ case PCI_ROM_ADDRESS1:
+ bartype = TYPE_ROMBAR;
break;
}
+ break;
+ case PCI_HEADER_TYPE_CARDBUS:
+ break;
+ default:
+ msg_perr("Unknown PCI header type 0x%02x, BAR type cannot be determined reliably.\n",
+ headertype);
+ break;
+ }
- supported_cycles = pci_read_word(dev, PCI_COMMAND);
+ supported_cycles = pci_read_word(dev, PCI_COMMAND);
- msg_pdbg("Requested BAR is ");
- switch (bartype) {
- case TYPE_MEMBAR:
- msg_pdbg("MEM");
- if (!(supported_cycles & PCI_COMMAND_MEMORY)) {
- msg_perr("MEM BAR access requested, but device "
- "has MEM space accesses disabled.\n");
- /* TODO: Abort here? */
- }
- msg_pdbg(", %sbit, %sprefetchable\n",
- ((addr & 0x6) == 0x0) ? "32" :
- (((addr & 0x6) == 0x4) ? "64" : "reserved"),
- (addr & 0x8) ? "" : "not ");
- if ((addr & 0x6) == 0x4) {
- /* The spec says that a 64-bit register consumes
- * two subsequent dword locations.
- */
- upperaddr = pci_read_long(dev, bar + 4);
- if (upperaddr != 0x00000000) {
- /* Fun! A real 64-bit resource. */
- if (sizeof(uintptr_t) != sizeof(uint64_t)) {
- msg_perr("BAR unreachable!");
- /* TODO: Really abort here? If
- * multiple PCI devices match,
- * we might never tell the user
- * about the other devices.
- */
- return 0;
- }
- addr |= (uint64_t)upperaddr << 32;
+ msg_pdbg("Requested BAR is ");
+ switch (bartype) {
+ case TYPE_MEMBAR:
+ msg_pdbg("MEM");
+ if (!(supported_cycles & PCI_COMMAND_MEMORY)) {
+ msg_perr("MEM BAR access requested, but device "
+ "has MEM space accesses disabled.\n");
+ /* TODO: Abort here? */
+ }
+ msg_pdbg(", %sbit, %sprefetchable\n",
+ ((addr & 0x6) == 0x0) ? "32" :
+ (((addr & 0x6) == 0x4) ? "64" : "reserved"),
+ (addr & 0x8) ? "" : "not ");
+ if ((addr & 0x6) == 0x4) {
+ /* The spec says that a 64-bit register consumes
+ * two subsequent dword locations.
+ */
+ upperaddr = pci_read_long(dev, bar + 4);
+ if (upperaddr != 0x00000000) {
+ /* Fun! A real 64-bit resource. */
+ if (sizeof(uintptr_t) != sizeof(uint64_t)) {
+ msg_perr("BAR unreachable!");
+ /* TODO: Really abort here? If
+ * multiple PCI devices match,
+ * we might never tell the user
+ * about the other devices.
+ */
+ return 0;
}
+ addr |= (uint64_t)upperaddr << 32;
}
- addr &= PCI_BASE_ADDRESS_MEM_MASK;
- break;
- case TYPE_IOBAR:
- msg_pdbg("I/O\n");
+ }
+ addr &= PCI_BASE_ADDRESS_MEM_MASK;
+ break;
+ case TYPE_IOBAR:
+ msg_pdbg("I/O\n");
#if __FLASHROM_HAVE_OUTB__
- if (!(supported_cycles & PCI_COMMAND_IO)) {
- msg_perr("I/O BAR access requested, but device "
- "has I/O space accesses disabled.\n");
- /* TODO: Abort here? */
- }
+ if (!(supported_cycles & PCI_COMMAND_IO)) {
+ msg_perr("I/O BAR access requested, but device "
+ "has I/O space accesses disabled.\n");
+ /* TODO: Abort here? */
+ }
#else
- msg_perr("I/O BAR access requested, but flashrom does "
- "not support I/O BAR access on this platform "
- "(yet).\n");
+ msg_perr("I/O BAR access requested, but flashrom does "
+ "not support I/O BAR access on this platform "
+ "(yet).\n");
#endif
- addr &= PCI_BASE_ADDRESS_IO_MASK;
- break;
- case TYPE_ROMBAR:
- msg_pdbg("ROM\n");
- /* Not sure if this check is needed. */
- if (!(supported_cycles & PCI_COMMAND_MEMORY)) {
- msg_perr("MEM BAR access requested, but device "
- "has MEM space accesses disabled.\n");
- /* TODO: Abort here? */
- }
- addr &= PCI_ROM_ADDRESS_MASK;
- break;
- case TYPE_UNKNOWN:
- msg_perr("BAR type unknown, please report a bug at "
- "flashrom at flashrom.org\n");
+ addr &= PCI_BASE_ADDRESS_IO_MASK;
+ break;
+ case TYPE_ROMBAR:
+ msg_pdbg("ROM\n");
+ /* Not sure if this check is needed. */
+ if (!(supported_cycles & PCI_COMMAND_MEMORY)) {
+ msg_perr("MEM BAR access requested, but device "
+ "has MEM space accesses disabled.\n");
+ /* TODO: Abort here? */
}
-
- if (devs[i].status == NT) {
- msg_pinfo("===\nThis PCI device is UNTESTED. Please "
- "report the 'flashrom -p xxxx' output \n"
- "to flashrom at flashrom.org if it works "
- "for you. Please add the name of your\n"
- "PCI device to the subject. Thank you for "
- "your help!\n===\n");
- }
-
- return (uintptr_t)addr;
+ addr &= PCI_ROM_ADDRESS_MASK;
+ break;
+ case TYPE_UNKNOWN:
+ msg_perr("BAR type unknown, please report a bug at "
+ "flashrom at flashrom.org\n");
}
- return 0;
+ return (uintptr_t)addr;
}
uintptr_t pcidev_init(int bar, const struct pcidev_status *devs)
@@ -195,6 +168,7 @@
char *pcidev_bdf;
char *msg = NULL;
int found = 0;
+ int i;
uintptr_t addr = 0, curaddr = 0;
pacc = pci_alloc(); /* Get the pci_access structure */
@@ -214,10 +188,29 @@
for (dev = pacc->devices; dev; dev = dev->next) {
if (pci_filter_match(&filter, dev)) {
+ /* Check against list of supported devices. */
+ for (i = 0; devs[i].device_name != NULL; i++)
+ if ((dev->vendor_id == devs[i].vendor_id) &&
+ (dev->device_id == devs[i].device_id))
+ break;
+ /* Not supported, try the next one. */
+ if (devs[i].device_name == NULL)
+ continue;
+
+ msg_pdbg("Found \"%s %s\" (%04x:%04x, BDF %02x:%02x.%x).\n", devs[i].vendor_name,
+ devs[i].device_name, dev->vendor_id, dev->device_id, dev->bus, dev->dev,
+ dev->func);
+ if (devs[i].status == NT)
+ msg_pinfo("===\nThis PCI device is UNTESTED. Please report the 'flashrom -p "
+ "xxxx' output \n"
+ "to flashrom at flashrom.org if it works for you. Please add the name "
+ "of your\n"
+ "PCI device to the subject. Thank you for your help!\n===\n");
+
/* FIXME: We should count all matching devices, not
* just those with a valid BAR.
*/
- if ((addr = pcidev_validate(dev, bar, devs)) != 0) {
+ if ((addr = pcidev_validate(dev, bar)) != 0) {
curaddr = addr;
pcidev_dev = dev;
found++;
Index: flashrom-pcidev_check_vendorid_cleanup/nicintel.c
===================================================================
--- flashrom-pcidev_check_vendorid_cleanup/nicintel.c (Revision 1547)
+++ flashrom-pcidev_check_vendorid_cleanup/nicintel.c (Arbeitskopie)
@@ -87,7 +87,7 @@
goto error_out_unmap;
/* FIXME: Using pcidev_dev _will_ cause pretty explosions in the future. */
- addr = pcidev_validate(pcidev_dev, PCI_BASE_ADDRESS_0, nics_intel);
+ addr = pcidev_validate(pcidev_dev, PCI_BASE_ADDRESS_0);
/* FIXME: This is not an aligned mapping. Use 4k? */
nicintel_control_bar = physmap("Intel NIC control/status reg",
addr, NICINTEL_CONTROL_MEMMAP_SIZE);
Index: flashrom-pcidev_check_vendorid_cleanup/programmer.h
===================================================================
--- flashrom-pcidev_check_vendorid_cleanup/programmer.h (Revision 1547)
+++ flashrom-pcidev_check_vendorid_cleanup/programmer.h (Arbeitskopie)
@@ -227,7 +227,7 @@
const char *vendor_name;
const char *device_name;
};
-uintptr_t pcidev_validate(struct pci_dev *dev, int bar, const struct pcidev_status *devs);
+uintptr_t pcidev_validate(struct pci_dev *dev, int bar);
uintptr_t pcidev_init(int bar, const struct pcidev_status *devs);
/* rpci_write_* are reversible writes. The original PCI config space register
* contents will be restored on shutdown.
--
http://www.hailfinger.org/
More information about the flashrom
mailing list