[PATCH v2 1/2] PCI: add helper function to find root port for device

[PATCH v2 1/2] PCI: add helper function to find root port for device

[Lucas Stach]

This adds a simple way to get the root port a given device
is connected to.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
v2: new patch in v2
---
 drivers/pci/search.c | 20 ++++++++++++++++++++
 include/linux/pci.h  |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index a81f413083e4..c3ae1c52c7cf 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -385,3 +385,23 @@ int pci_dev_present(const struct pci_device_id *ids)
 	return 0;
 }
 EXPORT_SYMBOL(pci_dev_present);
+
+/**
+ * pci_get_rootport - Returns the root port the given device is connected to.
+ * @dev: PCI device for which the root port should be found.
+ */
+struct pci_dev *pci_get_rootport(struct pci_dev *dev)
+{
+	struct pci_bus *bus = dev->bus;
+
+	/* If there is no bridge on the bus the passed device is a root port. */
+	if (!bus->self)
+		return dev;
+
+	/* Walk up the PCI hierarchy to the first level below the root. */
+	while (bus->parent && bus->parent->self)
+		bus = bus->parent;
+
+	return bus->self;
+}
+EXPORT_SYMBOL(pci_get_rootport);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4c8ac5fcc224..05442db50cad 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -843,6 +843,7 @@ static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
 }
 struct pci_dev *pci_get_class(unsigned int class, struct pci_dev *from);
 int pci_dev_present(const struct pci_device_id *ids);
+struct pci_dev *pci_get_rootport(struct pci_dev *dev);
 
 int pci_bus_read_config_byte(struct pci_bus *bus, unsigned int devfn,
 			     int where, u8 *val);
-- 
2.1.3

[PATCH v2 1/2] PCI: add helper function to find root port for device

[Lucas Stach]

This adds a simple way to get the root port a given device
is connected to.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
v2: new patch in v2
---
 drivers/pci/search.c | 20 ++++++++++++++++++++
 include/linux/pci.h  |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index a81f413083e4..c3ae1c52c7cf 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -385,3 +385,23 @@ int pci_dev_present(const struct pci_device_id *ids)
 	return 0;
 }
 EXPORT_SYMBOL(pci_dev_present);
+
+/**
+ * pci_get_rootport - Returns the root port the given device is connected to.
+ * @dev: PCI device for which the root port should be found.
+ */
+struct pci_dev *pci_get_rootport(struct pci_dev *dev)
+{
+	struct pci_bus *bus = dev->bus;
+
+	/* If there is no bridge on the bus the passed device is a root port. */
+	if (!bus->self)
+		return dev;
+
+	/* Walk up the PCI hierarchy to the first level below the root. */
+	while (bus->parent && bus->parent->self)
+		bus = bus->parent;
+
+	return bus->self;
+}
+EXPORT_SYMBOL(pci_get_rootport);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4c8ac5fcc224..05442db50cad 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -843,6 +843,7 @@ static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
 }
 struct pci_dev *pci_get_class(unsigned int class, struct pci_dev *from);
 int pci_dev_present(const struct pci_device_id *ids);
+struct pci_dev *pci_get_rootport(struct pci_dev *dev);
 
 int pci_bus_read_config_byte(struct pci_bus *bus, unsigned int devfn,
 			     int where, u8 *val);
-- 
2.1.3

[PATCH v2 2/2] PCI: tegra: apply relaxed ordering fixup only on Tegra

[Lucas Stach]

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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
v2:
- split out PCI hierarchy walk
- separate code from data by moving PCI IDs into own structure
---
 drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 333a57afacc4..b77f417e1a3c 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -635,10 +635,42 @@ 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 const struct pci_device_id tegra_rootport_ids[] = {
+	{
+		/* Tegra20 4 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf0,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra20 2 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf1,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra30 4 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1c,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra30 2 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1d,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra124 4 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e12,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra124 1 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e13,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* sentinel */
+	}
+};
+
 /* Tegra PCIE requires relaxed ordering */
 static void tegra_pcie_relax_enable(struct pci_dev *dev)
 {
-	pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN);
+	if (pci_match_id(tegra_rootport_ids, pci_get_rootport(dev)))
+		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
+					 PCI_EXP_DEVCTL_RELAX_EN);
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable);
 
-- 
2.1.3

[PATCH v2 2/2] PCI: tegra: apply relaxed ordering fixup only on Tegra

[Lucas Stach]

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@pengutronix.de>
---
v2:
- split out PCI hierarchy walk
- separate code from data by moving PCI IDs into own structure
---
 drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 333a57afacc4..b77f417e1a3c 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -635,10 +635,42 @@ 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 const struct pci_device_id tegra_rootport_ids[] = {
+	{
+		/* Tegra20 4 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf0,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra20 2 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf1,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra30 4 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1c,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra30 2 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1d,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra124 4 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e12,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* Tegra124 1 lane root port */
+		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e13,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
+	}, {
+		/* sentinel */
+	}
+};
+
 /* Tegra PCIE requires relaxed ordering */
 static void tegra_pcie_relax_enable(struct pci_dev *dev)
 {
-	pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN);
+	if (pci_match_id(tegra_rootport_ids, pci_get_rootport(dev)))
+		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
+					 PCI_EXP_DEVCTL_RELAX_EN);
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable);
 
-- 
2.1.3

Re: [PATCH v2 1/2] PCI: add helper function to find root port for device

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

On Thu, Dec 18, 2014 at 08:11:42PM +0100, Lucas Stach wrote:
> This adds a simple way to get the root port a given device
> is connected to.
> 
> Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> v2: new patch in v2
> ---
>  drivers/pci/search.c | 20 ++++++++++++++++++++
>  include/linux/pci.h  |  1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index a81f413083e4..c3ae1c52c7cf 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -385,3 +385,23 @@ int pci_dev_present(const struct pci_device_id *ids)
>  	return 0;
>  }
>  EXPORT_SYMBOL(pci_dev_present);
> +
> +/**
> + * pci_get_rootport - Returns the root port the given device is connected to.
> + * @dev: PCI device for which the root port should be found.
> + */
> +struct pci_dev *pci_get_rootport(struct pci_dev *dev)

I think pci_find_root_port() would be slightly more consistent with the
existing API.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 1/2] PCI: add helper function to find root port for device

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

On Thu, Dec 18, 2014 at 08:11:42PM +0100, Lucas Stach wrote:
> This adds a simple way to get the root port a given device
> is connected to.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> v2: new patch in v2
> ---
>  drivers/pci/search.c | 20 ++++++++++++++++++++
>  include/linux/pci.h  |  1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index a81f413083e4..c3ae1c52c7cf 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -385,3 +385,23 @@ int pci_dev_present(const struct pci_device_id *ids)
>  	return 0;
>  }
>  EXPORT_SYMBOL(pci_dev_present);
> +
> +/**
> + * pci_get_rootport - Returns the root port the given device is connected to.
> + * @dev: PCI device for which the root port should be found.
> + */
> +struct pci_dev *pci_get_rootport(struct pci_dev *dev)

I think pci_find_root_port() would be slightly more consistent with the
existing API.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/2] PCI: tegra: apply relaxed ordering fixup only on Tegra

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2781 bytes --]

On Thu, Dec 18, 2014 at 08:11:43PM +0100, Lucas Stach 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@pengutronix.de>
> ---
> v2:
> - split out PCI hierarchy walk
> - separate code from data by moving PCI IDs into own structure
> ---
>  drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index 333a57afacc4..b77f417e1a3c 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -635,10 +635,42 @@ 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 const struct pci_device_id tegra_rootport_ids[] = {
> +	{
> +		/* Tegra20 4 lane root port */
> +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf0,
> +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> +	}, {
> +		/* Tegra20 2 lane root port */
> +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf1,
> +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID

The number of lanes is configurable, so I'm not sure exactly what this
comment is supposed to indicate. Are you saying that port 0 has 0x0bf0
and port 1 has 0x0bf1 as device IDs.

> +	}, {
> +		/* Tegra30 4 lane root port */
> +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1c,
> +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> +	}, {
> +		/* Tegra30 2 lane root port */
> +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1d,
> +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID

Tegra30 has three ports, so does this second entry (0x0e1d) apply to
ports 1 and 2, whereas the previous entry (0x0e1c) applies only to port
0?

> +	}, {
> +		/* Tegra124 4 lane root port */
> +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e12,
> +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> +	}, {
> +		/* Tegra124 1 lane root port */
> +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e13,
> +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID

Or perhaps what this signifies is that the first port is actually a
different device because it supports up to 4 lanes, whereas the others
support up to 2 lanes (or only 1 on Tegra124)? In that case:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/2] PCI: tegra: apply relaxed ordering fixup only on Tegra

[Lucas Stach]

Am Freitag, den 09.01.2015, 12:32 +0100 schrieb Thierry Reding:
> On Thu, Dec 18, 2014 at 08:11:43PM +0100, Lucas Stach 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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> > v2:
> > - split out PCI hierarchy walk
> > - separate code from data by moving PCI IDs into own structure
> > ---
> >  drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > index 333a57afacc4..b77f417e1a3c 100644
> > --- a/drivers/pci/host/pci-tegra.c
> > +++ b/drivers/pci/host/pci-tegra.c
> > @@ -635,10 +635,42 @@ 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 const struct pci_device_id tegra_rootport_ids[] = {
> > +	{
> > +		/* Tegra20 4 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf0,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> > +	}, {
> > +		/* Tegra20 2 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf1,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> 
> The number of lanes is configurable, so I'm not sure exactly what this
> comment is supposed to indicate. Are you saying that port 0 has 0x0bf0
> and port 1 has 0x0bf1 as device IDs.
> 

No, the device ID of the root port is dependent on the number of lanes
configured for the specific port. So if you have a 4x1 configuration you
will get to see a single device with ID 0x0bf0, in a 2x2 configuration
you will see 2 devices with ID 0x0bf1.

> > +	}, {
> > +		/* Tegra30 4 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1c,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> > +	}, {
> > +		/* Tegra30 2 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1d,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> 
> Tegra30 has three ports, so does this second entry (0x0e1d) apply to
> ports 1 and 2, whereas the previous entry (0x0e1c) applies only to port
> 0?
> 
> > +	}, {
> > +		/* Tegra124 4 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e12,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> > +	}, {
> > +		/* Tegra124 1 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e13,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> 
> Or perhaps what this signifies is that the first port is actually a
> different device because it supports up to 4 lanes, whereas the others
> support up to 2 lanes (or only 1 on Tegra124)? In that case:
> 
> Acked-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Re: [PATCH v2 2/2] PCI: tegra: apply relaxed ordering fixup only on Tegra

[Lucas Stach]

Am Freitag, den 09.01.2015, 12:32 +0100 schrieb Thierry Reding:
> On Thu, Dec 18, 2014 at 08:11:43PM +0100, Lucas Stach 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@pengutronix.de>
> > ---
> > v2:
> > - split out PCI hierarchy walk
> > - separate code from data by moving PCI IDs into own structure
> > ---
> >  drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > index 333a57afacc4..b77f417e1a3c 100644
> > --- a/drivers/pci/host/pci-tegra.c
> > +++ b/drivers/pci/host/pci-tegra.c
> > @@ -635,10 +635,42 @@ 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 const struct pci_device_id tegra_rootport_ids[] = {
> > +	{
> > +		/* Tegra20 4 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf0,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> > +	}, {
> > +		/* Tegra20 2 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf1,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> 
> The number of lanes is configurable, so I'm not sure exactly what this
> comment is supposed to indicate. Are you saying that port 0 has 0x0bf0
> and port 1 has 0x0bf1 as device IDs.
> 

No, the device ID of the root port is dependent on the number of lanes
configured for the specific port. So if you have a 4x1 configuration you
will get to see a single device with ID 0x0bf0, in a 2x2 configuration
you will see 2 devices with ID 0x0bf1.

> > +	}, {
> > +		/* Tegra30 4 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1c,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> > +	}, {
> > +		/* Tegra30 2 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e1d,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> 
> Tegra30 has three ports, so does this second entry (0x0e1d) apply to
> ports 1 and 2, whereas the previous entry (0x0e1c) applies only to port
> 0?
> 
> > +	}, {
> > +		/* Tegra124 4 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e12,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> > +	}, {
> > +		/* Tegra124 1 lane root port */
> > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0e13,
> > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> 
> Or perhaps what this signifies is that the first port is actually a
> different device because it supports up to 4 lanes, whereas the others
> support up to 2 lanes (or only 1 on Tegra124)? In that case:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>

Re: [PATCH v2 2/2] PCI: tegra: apply relaxed ordering fixup only on Tegra

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2426 bytes --]

On Fri, Jan 09, 2015 at 12:45:18PM +0100, Lucas Stach wrote:
> Am Freitag, den 09.01.2015, 12:32 +0100 schrieb Thierry Reding:
> > On Thu, Dec 18, 2014 at 08:11:43PM +0100, Lucas Stach 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@pengutronix.de>
> > > ---
> > > v2:
> > > - split out PCI hierarchy walk
> > > - separate code from data by moving PCI IDs into own structure
> > > ---
> > >  drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++++-
> > >  1 file changed, 33 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > > index 333a57afacc4..b77f417e1a3c 100644
> > > --- a/drivers/pci/host/pci-tegra.c
> > > +++ b/drivers/pci/host/pci-tegra.c
> > > @@ -635,10 +635,42 @@ 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 const struct pci_device_id tegra_rootport_ids[] = {
> > > +	{
> > > +		/* Tegra20 4 lane root port */
> > > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf0,
> > > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> > > +	}, {
> > > +		/* Tegra20 2 lane root port */
> > > +		.vendor = PCI_VENDOR_ID_NVIDIA, .device = 0x0bf1,
> > > +		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> > 
> > The number of lanes is configurable, so I'm not sure exactly what this
> > comment is supposed to indicate. Are you saying that port 0 has 0x0bf0
> > and port 1 has 0x0bf1 as device IDs.
> > 
> 
> No, the device ID of the root port is dependent on the number of lanes
> configured for the specific port. So if you have a 4x1 configuration you
> will get to see a single device with ID 0x0bf0, in a 2x2 configuration
> you will see 2 devices with ID 0x0bf1.

Okay, that's interesting to know. My ack remains valid in that case:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]