From: thomas.petazzoni@free-electrons•com (Thomas Petazzoni)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2 1/7] PCI: aardvark: fix logic in PCI configuration read/write functions
Date: Tue, 9 Jan 2018 17:49:18 +0100 [thread overview]
Message-ID: <20180109174918.5c4b9ee6@windsurf.lan> (raw)
In-Reply-To: <20171005172330.GP25517@bhelgaas-glaptop.roam.corp.google.com>
Hello Bjorn,
Again, reviving this very old thread :-)
On Thu, 5 Oct 2017 12:23:30 -0500, Bjorn Helgaas wrote:
> > - if (PCI_SLOT(devfn) != 0) {
> > + if ((bus->number == pcie->root_bus_nr) && (PCI_SLOT(devfn) != 0)) {
>
> I'm fine with this, but please take a look at these:
>
> 8e7ca8ca5fd8 PCI: xilinx: Relax device number checking to allow SR-IOV
> e18934b5e9c7 PCI: designware: Relax device number checking to allow SR-IOV
> d99e30b7936a PCI: altera: Relax device number checking to allow SR-IOV
>
> and make sure that reasoning doesn't apply here, too.
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8e7ca8ca5fd8
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e18934b5e9c7
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d99e30b7936a
The original code for xilinx/designware/altera was doing:
if (bus->number == port->root_busno && devfn > 0)
return false;
if (bus->primary == port->root_busno && devfn > 0)
return false;
I.e, it was checking both if bus->number *and* bus->primary were equal
to port->root_busno.
The commit you points removed the check on bus->primary, keeping the
check on bus->number.
Your patch for the Aadvark driver only adds a check on bus->number, i.e
exactly what the xilinx/designware/altera code is still doing today:
Altera:
/* access only one slot on each root port */
if (bus->number == pcie->root_bus_nr && dev > 0)
return false;
Designware:
/* access only one slot on each root port */
if (bus->number == pp->root_bus_nr && dev > 0)
return 0;
Xilinx:
/* Only one device down on each root port */
if (bus->number == port->root_busno && devfn > 0)
return false;
Aardvark (with our patch):
if ((bus->number == pcie->root_bus_nr) && (PCI_SLOT(devfn) != 0)) {
*val = 0xffffffff;
return PCIBIOS_DEVICE_NOT_FOUND;
}
So we're doing exactly the same thing.
Do you agree ?
Best regards,
Thomas Petazzoni
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
next prev parent reply other threads:[~2018-01-09 16:49 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-28 12:58 [PATCH v2 0/7] PCI: aardvark: improve compatibility with PCI devices Thomas Petazzoni
2017-09-28 12:58 ` [PATCH v2 1/7] PCI: aardvark: fix logic in PCI configuration read/write functions Thomas Petazzoni
2017-10-05 17:23 ` Bjorn Helgaas
2018-01-09 16:49 ` Thomas Petazzoni [this message]
2018-01-10 1:11 ` Bjorn Helgaas
2017-09-28 12:58 ` [PATCH v2 2/7] PCI: aardvark: set PIO_ADDR_LS correctly in advk_pcie_rd_conf() Thomas Petazzoni
2017-10-05 17:25 ` Bjorn Helgaas
2018-01-09 16:10 ` Thomas Petazzoni
2017-09-28 12:58 ` [PATCH v2 3/7] PCI: aardvark: set host and device to the same MAX payload size Thomas Petazzoni
2017-10-05 17:31 ` Bjorn Helgaas
2018-01-09 15:39 ` Thomas Petazzoni
2018-01-09 22:14 ` Bjorn Helgaas
2018-01-12 10:14 ` Thomas Petazzoni
2018-01-12 14:40 ` Bjorn Helgaas
2018-01-12 15:46 ` Thomas Petazzoni
2018-01-12 19:39 ` Bjorn Helgaas
2017-09-28 12:58 ` [PATCH v2 4/7] PCI: aardvark: use isr1 instead of isr0 interrupt in legacy irq mode Thomas Petazzoni
2017-09-28 12:58 ` [PATCH v2 5/7] PCI: aardvark: disable LOS state by default Thomas Petazzoni
2017-10-05 17:46 ` Bjorn Helgaas
2017-10-09 6:54 ` Thomas Petazzoni
2017-09-28 12:58 ` [PATCH v2 6/7] PCI: aardvark: fix PCIe max read request size setting Thomas Petazzoni
2017-09-28 12:58 ` [PATCH v2 7/7] PCI: aardvark: define IRQ related hooks in pci_host_bridge Thomas Petazzoni
2017-10-05 17:55 ` Bjorn Helgaas
2017-10-05 19:25 ` Thomas Petazzoni
2017-10-05 15:53 ` [PATCH v2 0/7] PCI: aardvark: improve compatibility with PCI devices Thomas Petazzoni
2017-10-05 18:16 ` Bjorn Helgaas
2017-10-05 19:35 ` Thomas Petazzoni
2017-10-06 8:47 ` Lorenzo Pieralisi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180109174918.5c4b9ee6@windsurf.lan \
--to=thomas.petazzoni@free-electrons$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox