On Fri, May 15, 2009 at 12:16:09AM +0200, Carl-Daniel Hailfinger wrote:
> Please remove graphics cards and CD-ROM/DVD drives unless you have
> matching docs. I know that ATI graphics cards don't have sufficient
> documentation for flashing and I just checked with the contact I had who
> extended flashrom a bit. He was only able to read from ATI graphics
> cards, and even the reads were unreliable. ID and other stuff was a no-go.
>
> We can always raise expectations later if we actually see this stuff on
> the horizon.
Yeah, dropped all but the SATA cards ruik is working on (for now).
> > +void get_io_perms(void);
> > #if defined(__FreeBSD__) || defined(__DragonFly__)
> > extern int io_fd;
> > #endif
>
> Drop this hunk. It's already part of r512.
Done.
> > - addr = pci_read_long(dev, PCI_IO_BASE_ADDRESS) & ~0x03;
> > + addr = (uint32_t)(dev->base_addr[0] & ~0x03);
> >
> > - printf("Found NIC \"3COM %s\" (%04x:%04x), addr = 0x%x\n",
> > - nics[i].device_name, PCI_VENDOR_ID_3COM,
> > - nics[i].device_id, addr);
> > + printf("Found NIC \"3COM %s\" (%04x:%04x, %02x:%02x.%x, "
> > + "0x%x)\n", nics[i].device_name, dev->vendor_id,
> > + dev->device_id, dev->bus, dev->dev, dev->func, addr);
> >
>
> Can you please extend this message to tell the user what is what? I had
> to read the code to understand what the third value (BAR) was.
As per IRC discussion the BAR/addr is no longer printed, it's useless,
BDF format is explained better now (I hope).
> > - return addr;
> > + return -1;
Changed to 0 as per IRC discussion, the return value is uint32_t.
> > + pci_filter_init(pacc, &filter);
> >
> > + /* Filter by vendor, or bb:dd.f if supplied by the user. */
> > if (nic_pcidev != NULL) {
> > - pci_filter_init(pacc, &filter);
> > -
> > if ((msg = pci_filter_parse_slot(&filter, nic_pcidev))) {
> > fprintf(stderr, "Error: %s\n", msg);
> > exit(1);
> > }
> > + } else {
> > + filter.vendor = PCI_VENDOR_ID_3COM;
> > }
> >
>
> I'm unhappy with the new logic here. We either filter by bus:dev.fn or
> by vendor ID. The vendor ID filter should always be active. Otherwise,
> an incorrectly specified bdf can match a non-3com card by accident as
> long as the device ID matches.
> Please move the content of the else branch out of the if clause and run
> it unconditionally.
Done.
Committed in r513.
Uwe.
--
http://www.hermann-uwe.de | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
