[v4,2/6] PCI: aardvark: Fix logic in advk_pcie_valid_device()

Commit Message

Thomas Petazzoni March 29, 2018, 8:39 a.m. UTC

From: Victor Gu <[e-mail]>

The PCI configuration space read/write functions were special casing
the situation where PCI_SLOT(devfn) != 0, and returned
PCIBIOS_DEVICE_NOT_FOUND in this case.

However, while this is what is intended for the root bus, it is not
intended for the child busses, as it prevents discovering devices with
PCI_SLOT(x) != 0. Therefore, we return PCIBIOS_DEVICE_NOT_FOUND only
if we're on the root bus.

Fixes: 8c39d710363c1 ("PCI: aardvark: Add Aardvark PCI host controller driver")
Cc: <[e-mail]>
Signed-off-by: Victor Gu <[e-mail]>
Reviewed-by: Wilson Ding <[e-mail]>
Reviewed-by: Nadav Haklai <[e-mail]>
[Thomas: tweak commit log.]
Signed-off-by: Thomas Petazzoni <[e-mail]>
---
Changes since v2:
 - The logic has been factorized into a advk_pcie_valid_device()
   helper in a previous patch, so this patch was adjusted accordingly.
---
 drivers/pci/host/pci-aardvark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Gregory CLEMENT March 29, 2018, 2:12 p.m. UTC | #1

Hi Thomas,
 
 On jeu., mars 29 2018, Thomas Petazzoni <[e-mail]> wrote:

> From: Victor Gu <[e-mail]>
>
> The PCI configuration space read/write functions were special casing
                                                                causing?
else I don't get it

> the situation where PCI_SLOT(devfn) != 0, and returned
> PCIBIOS_DEVICE_NOT_FOUND in this case.
>
> However, while this is what is intended for the root bus, it is not
> intended for the child busses, as it prevents discovering devices with
                         buses

> PCI_SLOT(x) != 0. Therefore, we return PCIBIOS_DEVICE_NOT_FOUND only
> if we're on the root bus.
>
Gregory

Thomas Petazzoni March 29, 2018, 2:21 p.m. UTC | #2

Hello,

On Thu, 29 Mar 2018 16:12:28 +0200, Gregory CLEMENT wrote:
> Hi Thomas,
>  
>  On jeu., mars 29 2018, Thomas Petazzoni <[e-mail]> wrote:
> 
> > From: Victor Gu <[e-mail]>
> >
> > The PCI configuration space read/write functions were special casing  
>                                                                 causing?
> else I don't get it

No, no it's really "special casing", i.e "making a special case". I
must admit I don't know if it's proper English, but clearly, "special
causing" doesn't make any sense, and "special casing" is what I wanted
to write.

> 
> > the situation where PCI_SLOT(devfn) != 0, and returned
> > PCIBIOS_DEVICE_NOT_FOUND in this case.
> >
> > However, while this is what is intended for the root bus, it is not
> > intended for the child busses, as it prevents discovering devices with  
>                          buses

Merriam-Webster says that both exists [1], though buses is apparently
more common. The kernel has 633 occurrences of busses, and 785
occurrences of buses.

[1] https://www.merriam-webster.com/words-at-play/plural-of-bus

So I'd say my commit log is good as-is :)

Best regards,

Thomas

Patch

diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
index 82bff709a4b3..9d7f9f1c6837 100644
--- a/drivers/pci/host/pci-aardvark.c
+++ b/drivers/pci/host/pci-aardvark.c
@@ -433,7 +433,7 @@  static int advk_pcie_wait_pio(struct advk_pcie *pcie)
 static bool advk_pcie_valid_device(struct advk_pcie *pcie, struct pci_bus *bus,
 				  int devfn)
 {
-	if (PCI_SLOT(devfn) != 0)
+	if ((bus->number == pcie->root_bus_nr) && PCI_SLOT(devfn) != 0)
 		return false;
 
 	return true;