Hi Lucas,
Apologies for taking so long to come back to this. The patch looks ok
to me, just a minor comment about the Tegra PCI detection:
On Fri, Nov 14, 2014 at 4:37 AM, Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
> The fixup to enable relaxed ordering on all PCI devices was
> executed unconditionally if the Tegra PCI host driver was
> built into the kernel. This doesn't play nice with a
> multiplatform kernel executed on other platforms which
> may not need this fixup.
>
> Make sure to only apply the fixup if the root port is
> a Tegra.
>
> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> ---
> drivers/pci/host/pci-tegra.c | 26 +++++++++++++++++++++++++-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index 3d43874319be..d5a14f22ebb8 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -647,10 +647,34 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class);
>
> +static int tegra_pcie_root_is_tegra(struct pci_dev *dev)
> +{
> + struct pci_bus *bus = dev->bus;
> + struct pci_dev *root_bridge;
> +
> + /* walk up the PCIe hierarchy to the first level below the root bus */
> + while (bus->parent && bus->parent->self)
> + bus = bus->parent;
> +
> + /*
> + * If there is no bridge on the bus the passed device is the root
> + * bridge itself.
> + */
> + root_bridge = bus->self ? bus->self : dev;
> + if (root_bridge->vendor == PCI_VENDOR_ID_NVIDIA &&
> + (root_bridge->device == 0x0bf0 || root_bridge->device == 0x0bf1 ||
> + root_bridge->device == 0x0e1c || root_bridge->device == 0x0e1d ||
> + root_bridge->device == 0x0e12 || root_bridge->device == 0x0e13))
> + return 1;
I am not very familiar with PCI so sorry if these are stupid
questions, but where do these device IDs come from? Are they needed at
all, e.g. can't you just test against root_bridge->vendor ==
PCI_VENDOR_ID_NVIDIA to detect a NVIDIA root? Is the list susceptible
to increase as new chips get released? If that's the case, how can we
make sure we won't forget to update it?
If you need to test against the device ID, it might be more legible
(and easier to update) if you use a switch case, e.g:
if (root_bridge->vendor != PCI_VENDOR_ID_NVIDIA)
return 0;
switch (root_bridge->device) {
case 0x0bf0:
case 0x0bf1:
case 0x0e1c:
case 0x0e1d:
case 0x0e12:
case 0x0e12:
return 1;
default:
return 0;
}
Otherwise I have not noticed any problem using this patch.
Tested-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
--
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
