Hi Martin,
* Martin Mares <mj@xxxxxx>:
> I like the feature, but the current implementation does not make me very
> happy.
Ok.
> First of all: scanning the slot directory again and again on each
> device lookup is a waste of resources, especially when it is not clear
> that the user of the library is interested in slots. Also, it introduces
> an ABI incompatibility: a binary does not know whether the library
> it is currently linked with has phy_slot in struct pci_dev or not.
Ok.
> I think that we should use pci_fill_info() for that. When somebody
> calls pci_fill_info() with a PCI_FILL_PHYS_SLOT flag for the first time,
> the sysfs backend scans the slot directory and fills in the slot name
> for all known devices.
I must admit, I'm not familiar enough with pciutils for this
advice to make sense to me.
It sounds like you're saying something like:
lspci.c:scan_device() calls pci_fill_info with PCI_FILL_PHYS_SLOT
This actually calls pci_fill_info_v31 which does:
if (flags & ~d->known_fields)
d->known_fields |= d->methods->fill_info(d, flags & ~d->known_fields);
That will call pci_generic_fill_info(), which should then get
some logic like:
+ if (flags & PCI_FILL_PHYS_SLOT)
+ flags |= pci_scan_slots(d, flags);
Then you want me to implement pci_scan_slots() in lib/sysfs.c?
Opening and scanning the sysfs slots directory is pretty easy,
but is there some way to iterate across all the devices? Do you
really want me to re-implement scan_devices() from lspci.c?
Sorry for the confusion, I feel pretty stupid for not
understanding exactly what you're asking for.
> Also, please base the patch on the current development tree in the
> GIT repository (or the 3.0-alpha2 release from the FTP).
I was using the git repo, but noticed that you pushed Ben's VPD
changes after I cloned. ;)
> > + d->phy_slot = pci_malloc(a, strlen(entry->d_name) + 1);
>
> This memory is leaked.
Will fix (once I understand the final implementation strategy).
> > + if (p->phy_slot)
> > + printf("\tPhysical Slot: %s\n", p->phy_slot);
>
> It would be nice to have the slot available in the machine-readable
> output format, too. (Please document that in the man page.)
Ok.
Thanks.
/ac
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
