[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <mj+md-20180810.091549.51451.nikam@ucw.cz>
Date: Fri, 10 Aug 2018 11:35:01 +0200
From: Martin Mares <mj@....cz>
To: Matthew Wilcox <willy@...radead.org>
Cc: Logan Gunthorpe <logang@...tatee.com>,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linux-doc@...r.kernel.org, Stephen Bates <sbates@...thlin.com>,
Christoph Hellwig <hch@....de>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Jonathan Corbet <corbet@....net>,
Ingo Molnar <mingo@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Marc Zyngier <marc.zyngier@....com>,
Kai-Heng Feng <kai.heng.feng@...onical.com>,
Frederic Weisbecker <frederic@...nel.org>,
Dan Williams <dan.j.williams@...el.com>,
Jérôme Glisse <jglisse@...hat.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Alex Williamson <alex.williamson@...hat.com>,
Christian König <christian.koenig@....com>
Subject: Re: lspci: Display path to device
Hello!
> I don't like telling the user to grovel around lspci -t by hand. It's
> not many lines of code to add a new -P option to lspci to show the path
> to each device instead of bus:dev.fn
I like the feature, but I have a couple of minor objections to the
implementation:
> -static char options[] = "nvbxs:d:ti:mgp:qkMDQ" GENERIC_OPTIONS ;
> +static char options[] = "nvbxs:d:tPi:mgp:qkMDQ" GENERIC_OPTIONS ;
Please update the help_msg, too.
> +static void
> +show_slot_path(struct pci_dev *p)
> +{
> + struct pci_dev *d = NULL;
> +
> + if (opt_path && p->bus)
> + {
> + for (d = p->access->devices; d; d = d->next) {
> + if (d->hdrtype == -1)
> + d->hdrtype = pci_read_byte(d, PCI_HEADER_TYPE) & 0x7f;
Please do not modify the hdrtype field, it is private to libpci.
Instead, always read the PCI_HEADER_TYPE register. It will be fast since
lspci caches configuration space accesses.
> + if (d->hdrtype != PCI_HEADER_TYPE_BRIDGE &&
> + d->hdrtype != PCI_HEADER_TYPE_CARDBUS)
> + continue;
> + if (pci_read_byte(d, PCI_SECONDARY_BUS) > p->bus)
> + continue;
> + if (pci_read_byte(d, PCI_SUBORDINATE_BUS) < p->bus)
> + continue;
Beware, cardbus bridges use different registers for secondary and subordinate bus.
... the more I think about it, the more I am convinced that we want to
re-use the bus topology builder from ls-tree.c. I will give it a try,
stay tuned.
Martin
Powered by blists - more mailing lists