Re: pci_domain_nr vs. /sys/devices

From: Matthew Wilcox (willy@debian.org)
Date: Tue Jun 17 2003 - 08:55:48 EST


On Tue, Jun 17, 2003 at 02:49:48PM +1000, Anton Blanchard wrote:
> I like it. I think we do need the bus number in the top level since we
> could have multiple host bridges on the same domain. I put a quick patch
> together, it lays things out as such:
>
> /sys/devices/pci0002:00/0002:00:02.2

Yep, I have a patch to do the same thing. Sorry, didn't realise you
were working on this too; I should've cc'd you.

> It also adds the domain to /proc/pci (are there userspace tools that
> parse this directly?):
>
> Domain 2, Bus 0, device 2, function 2:

Probably ;-(

> And only creates /proc/bus/pci entries for the first domain. I was going
> to extend it one level to encode the domain but I now think we should just
> move that functionality into sysfs and be done with it. Willy, you had a
> patch that exposed BARS etc in sysfs didnt you? X and lspci etc will need
> updating to match, but they are currently broken.

Yes. Some people felt my patch didn't go far enough (they wanted
_everything_ as its own little file, and damn the dentry/inode
consumption!), but it's resurrectable.

My personal feeling is that we should leave the resources file alone
(except for fixing its formatting; patch already with greg), expose the
config file and expose the contents of the resources.

> I chose to add the domain into dev->slot_name since its needed for matching
> kernel messages to drivers. Im wondering if we should make this conditional
> on pci domain support since it does add some noise for those who couldnt
> care less about domains.

I think we probably shouldn't since that requires additional testing &
fixing of stuff for those of us with multiple-domain boxes.

> Finally there was some shuffling required to make pci_bus_exists work
> (passing in a pci_bus *, ->sysdata and ->number must be initialised
> before calling it). There are some uses of pci_bus_exists in x86 that
> will need updating.

I actually eliminated pci_bus_exists() completely in my tree.
pci_find_bus() does the job just as well.

> Thoungts?

We're definitely moving in the same direction ;-)

Here's one place we differ...

> ===== drivers/pci/proc.c 1.29 vs edited =====
> --- 1.29/drivers/pci/proc.c Wed Jun 11 02:33:14 2003
> +++ edited/drivers/pci/proc.c Tue Jun 17 09:32:20 2003
> @@ -382,6 +382,10 @@
> if (!proc_initialized)
> return -EACCES;
>
> + /* Backwards compatibility for domain 0 only */
> + if (pci_domain_nr(dev->bus) != 0)
> + return 0;
> +
> if (!(de = bus->procdir)) {
> sprintf(name, "%02x", bus->number);
> de = bus->procdir = proc_mkdir(name, proc_bus_pci_dir);

I create them anyway, but put the domain on the front. I bet that'll
break Alpha too (I've read further in the thread but need to reply from
here ..)

> @@ -470,8 +474,9 @@
> pci_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
> pci_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
> pci_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
> - seq_printf(m, " Bus %2d, device %3d, function %2d:\n",
> - dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
> + seq_printf(m, " Domain %2d, Bus %2d, device %3d, function %2d:\n",
> + pci_domain_nr(dev->bus), dev->bus->number, PCI_SLOT(dev->devfn),
> + PCI_FUNC(dev->devfn));
> class = pci_class_name(class_rev >> 16);
> if (class)
> seq_printf(m, " %s", class);

I'd prefer not to touch it.

> ===== include/linux/pci.h 1.90 vs edited =====
> --- 1.90/include/linux/pci.h Wed Jun 11 16:49:42 2003
> +++ edited/include/linux/pci.h Tue Jun 17 10:27:32 2003
> @@ -414,7 +414,7 @@
> struct resource dma_resource[DEVICE_COUNT_DMA];
> struct resource irq_resource[DEVICE_COUNT_IRQ];
>
> - char slot_name[8]; /* slot name */
> + char slot_name[13]; /* slot name */
>
> /* These fields are used by common fixups */
> unsigned int transparent:1; /* Transparent PCI bridge */

Let's make slot_name a pointer to the struct device busid.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon Jun 23 2003 - 22:00:21 EST