[PATCH 0/3] PCI: Add support for Cavium ThunderX RC and on-SoC devices.

[PATCH 0/3] PCI: Add support for Cavium ThunderX RC and on-SoC devices.

[David Daney]

From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

The Cavium ThunderX arm64 based SoC needs a little bit of special
handling for both its PCIe Root Complexes as well as on-SoC devices
(which all appear as PCIe devices).

1/3 - Small change to allow SRIOV BARs to be given fixed addresses in
      the header fixup.

2/3 - Add quirks to support fixed BAR addresses for all on-SoC devices,
      including SRIOV BARs in the NIC.

3/3 - Add config spaces accessors to pci-host-generic driver for ThunderX RC.

This patch set depends on:

https://lkml.org/lkml/2015/9/17/799

David Daney (3):
  PCI: Allow quirks to override SRIOV BARs.
  PCI: Add quirks for devices found on Cavium ThunderX SoCs.
  PCI: generic: Add support for Cavium ThunderX PCIe root complexes.

 .../devicetree/bindings/pci/host-generic-pci.txt   |  8 +-
 drivers/pci/host/Kconfig                           |  6 ++
 drivers/pci/host/Makefile                          |  1 +
 drivers/pci/host/pci-host-generic.c                | 29 +++++++
 drivers/pci/host/quirks-thunder.c                  | 95 ++++++++++++++++++++++
 drivers/pci/iov.c                                  |  9 +-
 6 files changed, 143 insertions(+), 5 deletions(-)
 create mode 100644 drivers/pci/host/quirks-thunder.c

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] PCI: Allow quirks to override SRIOV BARs.

[David Daney]

From: David Daney <david.daney@cavium.com>

Devices with fixed BARs can install BAR resources with
IORESOURCE_PCI_FIXED from the header fixup.  Allow this to work with
the SRIOV BARs as well by testing if the BAR resource has already been
set before attempting to read it from the config space.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/iov.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index ee0ebff..f8a6e1e 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -436,8 +436,13 @@ found:
 	nres = 0;
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
 		res = &dev->resource[i + PCI_IOV_RESOURCES];
-		bar64 = __pci_read_base(dev, pci_bar_unknown, res,
-					pos + PCI_SRIOV_BAR + i * 4);
+		if (res->flags)
+			/* Already populated by quirks, just set bar64. */
+			bar64 = (res->flags & IORESOURCE_MEM_64) ? 1 : 0;
+		else
+			bar64 = __pci_read_base(dev, pci_bar_unknown, res,
+						pos + PCI_SRIOV_BAR + i * 4);
+
 		if (!res->flags)
 			continue;
 		if (resource_size(res) & (PAGE_SIZE - 1)) {
-- 
1.9.1

[PATCH 2/3] PCI: Add quirks for devices found on Cavium ThunderX SoCs.

[David Daney]

From: David Daney <david.daney@cavium.com>

The on-chip devices all have fixed bars.  So, fix them up.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/host/Kconfig          |  6 +++
 drivers/pci/host/Makefile         |  1 +
 drivers/pci/host/quirks-thunder.c | 95 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+)
 create mode 100644 drivers/pci/host/quirks-thunder.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index d5e58ba..20d700d 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -145,4 +145,10 @@ config PCIE_IPROC_BCMA
 	  Say Y here if you want to use the Broadcom iProc PCIe controller
 	  through the BCMA bus interface
 
+config PCI_QUIRKS_THUNDER
+	bool "PCI quirks for Cavium ThunderX SoCs"
+	depends on ARM64
+	help
+	  Say Y here to support ThunderX SoCs
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 140d66f..3ce7456 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
 obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
 obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
 obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
+obj-$(CONFIG_PCI_QUIRKS_THUNDER) += quirks-thunder.o
diff --git a/drivers/pci/host/quirks-thunder.c b/drivers/pci/host/quirks-thunder.c
new file mode 100644
index 0000000..d615fd8
--- /dev/null
+++ b/drivers/pci/host/quirks-thunder.c
@@ -0,0 +1,95 @@
+/*
+ * PCIe quirks for Cavium Thunder SOC
+ *
+ * Copyright (C) 2014, 2015 Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+
+#define PCI_DEVICE_ID_THUNDER_BRIDGE		0xa002
+
+/*
+ * All PCIe devices in Thunder have fixed resources, shouldn't be
+ * reassigned.
+ */
+static void thunder_pci_fixup_header(struct pci_dev *dev)
+{
+	int resno;
+
+	/* Only fixup Thunder on-chip devices. */
+	if ((dev->device & 0xff00) != 0xa000)
+		return;
+
+	for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
+		if (dev->resource[resno].flags)
+			dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;
+#ifdef CONFIG_PCI_IOV
+	if (dev->device == 0xa01e) {
+		dev->resource[PCI_IOV_RESOURCES + 0] = dev->resource[0];
+		dev->resource[PCI_IOV_RESOURCES + 0].start += 0xa0000000;
+		dev->resource[PCI_IOV_RESOURCES + 0].end =
+			dev->resource[PCI_IOV_RESOURCES + 0].start + 0x200000 - 1;
+		dev->resource[PCI_IOV_RESOURCES + 4] = dev->resource[0];
+		dev->resource[PCI_IOV_RESOURCES + 4].start += 0xe0000000;
+		dev->resource[PCI_IOV_RESOURCES + 4].end =
+			dev->resource[PCI_IOV_RESOURCES + 4].start + 0x200000 - 1;
+	}
+#endif
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, thunder_pci_fixup_header);
+
+static void thunder_pci_fixup_final(struct pci_dev *dev)
+{
+	struct resource *res;
+	int resno;
+
+	/* Only fixup Thunder on-chip devices. */
+	if ((dev->device & 0xff00) != 0xa000)
+		return;
+
+	if (dev->device == PCI_DEVICE_ID_THUNDER_BRIDGE) {
+		/*
+		 * This bridge is just for the sake of supporting ARI
+		 * for downstream devices. No resources are attached
+		 * to it.  Copy upstream root bus resources to bridge
+		 * which aide in resource claiming for downstream
+		 * devices
+		 */
+
+		struct pci_bus *bus;
+		int resno;
+
+		bus = dev->subordinate;
+		for (resno = 0; resno < PCI_BRIDGE_RESOURCE_NUM; resno++) {
+			bus->resource[resno] = pci_bus_resource_n(bus->parent,
+								  PCI_BRIDGE_RESOURCE_NUM + resno);
+		}
+
+		for (resno = PCI_BRIDGE_RESOURCES;
+		     resno <= PCI_BRIDGE_RESOURCE_END; resno++) {
+			dev->resource[resno].start = dev->resource[resno].end = 0;
+			dev->resource[resno].flags = 0;
+		}
+	} else {
+		/*
+		 * Claim the device's valid resources to set
+		 * 'res->parent' hierarchy.
+		 */
+		for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) {
+			res = &dev->resource[resno];
+			if (res->parent || !(res->flags & IORESOURCE_MEM))
+				continue;
+			pci_claim_resource(dev, resno);
+		}
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, thunder_pci_fixup_final);
+
+MODULE_DESCRIPTION("PCI quirks for Cavium Thunder ECAM based devices");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH 2/3] PCI: Add quirks for devices found on Cavium ThunderX SoCs.

[Arnd Bergmann]

On Thursday 17 September 2015 15:41:33 David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> The on-chip devices all have fixed bars.  So, fix them up.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> 

You should be able to just mark the BARs as fixed in DT and not need
this hack. If that doesn't work with current kernels, it would be
better to fix the kernel to respect the flags.

	Arnd

Re: [PATCH 2/3] PCI: Add quirks for devices found on Cavium ThunderX SoCs.

[David Daney]

On 09/18/2015 12:19 AM, Arnd Bergmann wrote:
> On Thursday 17 September 2015 15:41:33 David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> The on-chip devices all have fixed bars.  So, fix them up.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>>
>
> You should be able to just mark the BARs as fixed in DT

In the case of ACPI, there is no DT.  So we would need a different 
solution for ACPI.  What would you recommend for ACPI?

Also, can you point me to the OF device tree specification where it 
tells how to specify PCI BAR addresses, I would especially be interested 
in knowing how to specify fixed SRIOV BAR addresses in the device tree.

Thanks,
David Daney

> and not need
> this hack.

Yes, it is a bit of a hack.  That is why I put it in its own file, and 
only try to hack up PCI devices that exactly match the vendor and device 
ids that need fixing.

IMHO, putting infrastructure into drivers/pci/probe.c, et al. to handle 
this would be much more intrusive.

For the record:  The PCI Enhanced Allocation (EA) capability (approved 
by PCI SIG on 23 October 2014) is the proper way to handle this going 
forward.  However, this is not yet implemented in the SoCs that this 
patch addresses.  Our plan is to implement the EA capability in the core 
PCI code, so that we do not need to keep adding devices to this fixup code.

David Daney

Re: [PATCH 2/3] PCI: Add quirks for devices found on Cavium ThunderX SoCs.

[Arnd Bergmann]

On Friday 18 September 2015 10:00:32 David Daney wrote:
> On 09/18/2015 12:19 AM, Arnd Bergmann wrote:
> > On Thursday 17 September 2015 15:41:33 David Daney wrote:
> >> From: David Daney <david.daney@cavium.com>
> >>
> >> The on-chip devices all have fixed bars.  So, fix them up.
> >>
> >> Signed-off-by: David Daney <david.daney@cavium.com>
> >>
> >
> > You should be able to just mark the BARs as fixed in DT
> 
> In the case of ACPI, there is no DT.  So we would need a different 
> solution for ACPI.  What would you recommend for ACPI?

I would expect that this does not matter at all on ACPI, because
the devices that need it are not hot-plugged, and all boot-time
devices are probed by the firmware: the ACPI PCI implementation
does not reassign any BARs, except for the hotplug case.

> Also, can you point me to the OF device tree specification where it 
> tells how to specify PCI BAR addresses, I would especially be interested 
> in knowing how to specify fixed SRIOV BAR addresses in the device tree.

This is the 'n' bit mentioned sections 2.1.3 and 2.2.1.1 of the
PCI binding. When it is set, the OS is not supposed to try to
reassign the BAR even on machines that otherwise do a complete
rescan.

The PCI binding traditionally requires you to list all PCI devices
in DT, Linux as an extension (for the flattened DT format) allows
leaving out the devices, but in this case you probably need to
list every device that has a fixed BAR.

> Yes, it is a bit of a hack.  That is why I put it in its own file, and 
> only try to hack up PCI devices that exactly match the vendor and device 
> ids that need fixing.
> 
> IMHO, putting infrastructure into drivers/pci/probe.c, et al. to handle 
> this would be much more intrusive.

My guess is that it's already there, but even if it's not, this is a
generic well-defined case that has a standardized binding, and we should
implement that.

> For the record:  The PCI Enhanced Allocation (EA) capability (approved 
> by PCI SIG on 23 October 2014) is the proper way to handle this going 
> forward.  However, this is not yet implemented in the SoCs that this 
> patch addresses.  Our plan is to implement the EA capability in the core 
> PCI code, so that we do not need to keep adding devices to this fixup code.

Good, but still this should only be required for the embedded case where
you don't have a firmware to probe the bus.

	Arnd

Re: [PATCH 2/3] PCI: Add quirks for devices found on Cavium ThunderX SoCs.

[David Daney]

On 09/18/2015 12:45 PM, Arnd Bergmann wrote:
> On Friday 18 September 2015 10:00:32 David Daney wrote:
>> On 09/18/2015 12:19 AM, Arnd Bergmann wrote:
>>> On Thursday 17 September 2015 15:41:33 David Daney wrote:
>>>> From: David Daney <david.daney@cavium.com>
>>>>
>>>> The on-chip devices all have fixed bars.  So, fix them up.
>>>>
>>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>>>
>>>
>>> You should be able to just mark the BARs as fixed in DT

I think we can switch to PCI_PROBE_ONLY, and have all non-fixed BAR 
devices configured by firmware.  This may significantly simplify any 
quirks required in the kernel.


>>
>> In the case of ACPI, there is no DT.  So we would need a different
>> solution for ACPI.  What would you recommend for ACPI?
>
> I would expect that this does not matter at all on ACPI, because
> the devices that need it are not hot-plugged, and all boot-time
> devices are probed by the firmware: the ACPI PCI implementation
> does not reassign any BARs, except for the hotplug case.
>
>> Also, can you point me to the OF device tree specification where it
>> tells how to specify PCI BAR addresses, I would especially be interested
>> in knowing how to specify fixed SRIOV BAR addresses in the device tree.
>
> This is the 'n' bit mentioned sections 2.1.3 and 2.2.1.1 of the
> PCI binding. When it is set, the OS is not supposed to try to
> reassign the BAR even on machines that otherwise do a complete
> rescan.
>
> The PCI binding traditionally requires you to list all PCI devices
> in DT, Linux as an extension (for the flattened DT format) allows
> leaving out the devices, but in this case you probably need to
> list every device that has a fixed BAR.
>
>> Yes, it is a bit of a hack.  That is why I put it in its own file, and
>> only try to hack up PCI devices that exactly match the vendor and device
>> ids that need fixing.
>>
>> IMHO, putting infrastructure into drivers/pci/probe.c, et al. to handle
>> this would be much more intrusive.
>
> My guess is that it's already there, but even if it's not, this is a
> generic well-defined case that has a standardized binding, and we should
> implement that.
>
>> For the record:  The PCI Enhanced Allocation (EA) capability (approved
>> by PCI SIG on 23 October 2014) is the proper way to handle this going
>> forward.  However, this is not yet implemented in the SoCs that this
>> patch addresses.  Our plan is to implement the EA capability in the core
>> PCI code, so that we do not need to keep adding devices to this fixup code.
>
> Good, but still this should only be required for the embedded case where
> you don't have a firmware to probe the bus.
>
> 	Arnd
>

Re: [PATCH 2/3] PCI: Add quirks for devices found on Cavium ThunderX SoCs.

[Bjorn Helgaas]

Hi David,

On Fri, Sep 18, 2015 at 06:00:28PM -0700, David Daney wrote:
> On 09/18/2015 12:45 PM, Arnd Bergmann wrote:
> >On Friday 18 September 2015 10:00:32 David Daney wrote:
> >>On 09/18/2015 12:19 AM, Arnd Bergmann wrote:
> >>>On Thursday 17 September 2015 15:41:33 David Daney wrote:
> >>>>From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> >>>>
> >>>>The on-chip devices all have fixed bars.  So, fix them up.
> >>>>
> >>>>Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> >>>>
> >>>
> >>>You should be able to just mark the BARs as fixed in DT
> 
> I think we can switch to PCI_PROBE_ONLY, and have all non-fixed BAR
> devices configured by firmware.  This may significantly simplify any
> quirks required in the kernel.

I don't like PCI_PROBE_ONLY, and I'd like to avoid it when we can.

Your original patch description said the on-chip devices have "fixed
BARs."  In what sense are they "fixed"?  I assume they are writable
enough so we can learn their sizes?  If we can't learn their sizes, we
have bigger problems because we can't tell what space is used.

Are there other parts of the system, e.g., run-time firmware, that
depend on the devices not being moved?

> >>For the record:  The PCI Enhanced Allocation (EA) capability (approved
> >>by PCI SIG on 23 October 2014) is the proper way to handle this going
> >>forward.  However, this is not yet implemented in the SoCs that this
> >>patch addresses.  Our plan is to implement the EA capability in the core
> >>PCI code, so that we do not need to keep adding devices to this fixup code.

Sean Stalley has posted some patches to add EA support to Linux, but I
haven't merged them yet.  If we had that, another option would be to
hook into your config accessors and fabricate an EA capability.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] PCI: Add quirks for devices found on Cavium ThunderX SoCs.

[David Daney]

On 09/22/2015 06:19 AM, Bjorn Helgaas wrote:
> Hi David,
>
> On Fri, Sep 18, 2015 at 06:00:28PM -0700, David Daney wrote:
>> On 09/18/2015 12:45 PM, Arnd Bergmann wrote:
>>> On Friday 18 September 2015 10:00:32 David Daney wrote:
>>>> On 09/18/2015 12:19 AM, Arnd Bergmann wrote:
>>>>> On Thursday 17 September 2015 15:41:33 David Daney wrote:
>>>>>> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>>>>>>
>>>>>> The on-chip devices all have fixed bars.  So, fix them up.
>>>>>>
>>>>>> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>>>>>>
>>>>>
>>>>> You should be able to just mark the BARs as fixed in DT
>>
>> I think we can switch to PCI_PROBE_ONLY, and have all non-fixed BAR
>> devices configured by firmware.  This may significantly simplify any
>> quirks required in the kernel.
>
> I don't like PCI_PROBE_ONLY, and I'd like to avoid it when we can.

I don't like it either, but if it were the only way the PCI maintainers 
would allow us to support the hardware, I would rewrite the firmware to 
make it possible.  However, as you say below ...

>
> Your original patch description said the on-chip devices have "fixed
> BARs."  In what sense are they "fixed"?  I assume they are writable
> enough so we can learn their sizes?

Yes.  The BAR registers are writable, but ignored.  So, the size is 
correctly probed.  The BAR registers are initialized by 
hardware/firmware to the proper value.

>  If we can't learn their sizes, we
> have bigger problems because we can't tell what space is used.
>
> Are there other parts of the system, e.g., run-time firmware, that
> depend on the devices not being moved?

The devices cannot move, the address decoding is not programmable.  The 
BAR registers are provided as an aid in integrating the devices with OS 
PCI infrastructure.  Although, one might argue that they don't do a very 
good job of adhering to the PCI specifications...

>
>>>> For the record:  The PCI Enhanced Allocation (EA) capability (approved
>>>> by PCI SIG on 23 October 2014) is the proper way to handle this going
>>>> forward.  However, this is not yet implemented in the SoCs that this
>>>> patch addresses.  Our plan is to implement the EA capability in the core
>>>> PCI code, so that we do not need to keep adding devices to this fixup code.
>
> Sean Stalley has posted some patches to add EA support to Linux, but I
> haven't merged them yet.  If we had that, another option would be to
> hook into your config accessors and fabricate an EA capability.

To me, this is the most interesting part of your message.  If you really 
would accept a config read accessor that presented a synthetic EA 
capability, that would be ideal.  We know exactly which roots contain 
the fixed devices, so it would be a trivial exercise to provide a custom 
config accessor.

I am going to work on this in hope of eventual acceptance of this strategy.

Thanks,
David Daney


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] PCI: Add quirks for devices found on Cavium ThunderX SoCs.

[Lorenzo Pieralisi]

On Fri, Sep 18, 2015 at 08:45:50PM +0100, Arnd Bergmann wrote:
> On Friday 18 September 2015 10:00:32 David Daney wrote:
> > On 09/18/2015 12:19 AM, Arnd Bergmann wrote:
> > > On Thursday 17 September 2015 15:41:33 David Daney wrote:
> > >> From: David Daney <david.daney@cavium.com>
> > >>
> > >> The on-chip devices all have fixed bars.  So, fix them up.
> > >>
> > >> Signed-off-by: David Daney <david.daney@cavium.com>
> > >>
> > >
> > > You should be able to just mark the BARs as fixed in DT
> > 
> > In the case of ACPI, there is no DT.  So we would need a different 
> > solution for ACPI.  What would you recommend for ACPI?
> 
> I would expect that this does not matter at all on ACPI, because
> the devices that need it are not hot-plugged, and all boot-time
> devices are probed by the firmware: the ACPI PCI implementation
> does not reassign any BARs, except for the hotplug case.

What do you mean by "the ACPI PCI implementation does not reassign
any BARs" ? Do you mean on x86 ? The resource assignment is part
of the resource survey on x86, where all resources that can be claimed
are claimed, but still, some of them may be still reassigned IIUC.

On arm64 we do not carry out any resource survey at present, but
if we do (and we should), it will have to work for both DT and ACPI
systems.

> > Also, can you point me to the OF device tree specification where it 
> > tells how to specify PCI BAR addresses, I would especially be interested 
> > in knowing how to specify fixed SRIOV BAR addresses in the device tree.
> 
> This is the 'n' bit mentioned sections 2.1.3 and 2.2.1.1 of the
> PCI binding. When it is set, the OS is not supposed to try to
> reassign the BAR even on machines that otherwise do a complete
> rescan.

I do not see any code in the kernel taking care of that bit and
if I am not mistaken the code that allows creating pci devices
exists in PowerPC arch code (arch/powerpc/kernel/pci_of_scan.c) and
should be moved out of there for the other arches to use it.

Or maybe we can use DT just to fix-up the resource flags (ie
every PCI device will have its own of_node set if it is present
in the DT, we can use it to fixup its resources and related flags,
pcibios_add_device() ?).

> The PCI binding traditionally requires you to list all PCI devices
> in DT, Linux as an extension (for the flattened DT format) allows
> leaving out the devices, but in this case you probably need to
> list every device that has a fixed BAR.
> 
> > Yes, it is a bit of a hack.  That is why I put it in its own file, and 
> > only try to hack up PCI devices that exactly match the vendor and device 
> > ids that need fixing.
> > 
> > IMHO, putting infrastructure into drivers/pci/probe.c, et al. to handle 
> > this would be much more intrusive.
> 
> My guess is that it's already there, but even if it's not, this is a
> generic well-defined case that has a standardized binding, and we should
> implement that.

See above.

Thanks,
Lorenzo

> > Forthe record:  The PCI Enhanced Allocation (EA) capability (approved 
> > by PCI SIG on 23 October 2014) is the proper way to handle this going 
> > forward.  However, this is not yet implemented in the SoCs that this 
> > patch addresses.  Our plan is to implement the EA capability in the core 
> > PCI code, so that we do not need to keep adding devices to this fixup code.
> 
> Good, but still this should only be required for the embedded case where
> you don't have a firmware to probe the bus.
> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/3] PCI: Add quirks for devices found on Cavium ThunderX SoCs.

[Arnd Bergmann]

On Tuesday 22 September 2015 16:39:31 Lorenzo Pieralisi wrote:
> On Fri, Sep 18, 2015 at 08:45:50PM +0100, Arnd Bergmann wrote:
> > On Friday 18 September 2015 10:00:32 David Daney wrote:
> > > On 09/18/2015 12:19 AM, Arnd Bergmann wrote:
> > > > On Thursday 17 September 2015 15:41:33 David Daney wrote:
> > > >> From: David Daney <david.daney@cavium.com>
> > > >>
> > > >> The on-chip devices all have fixed bars.  So, fix them up.
> > > >>
> > > >> Signed-off-by: David Daney <david.daney@cavium.com>
> > > >>
> > > >
> > > > You should be able to just mark the BARs as fixed in DT
> > > 
> > > In the case of ACPI, there is no DT.  So we would need a different 
> > > solution for ACPI.  What would you recommend for ACPI?
> > 
> > I would expect that this does not matter at all on ACPI, because
> > the devices that need it are not hot-plugged, and all boot-time
> > devices are probed by the firmware: the ACPI PCI implementation
> > does not reassign any BARs, except for the hotplug case.
> 
> What do you mean by "the ACPI PCI implementation does not reassign
> any BARs" ? Do you mean on x86 ? The resource assignment is part
> of the resource survey on x86, where all resources that can be claimed
> are claimed, but still, some of them may be still reassigned IIUC.

The ACPI PCI implementation should be completely architecture
independent and do the same thing everywhere. The code is spread
over drivers/acpi/pci*, with small parts currently residing in
arch/{x86,ia64} that need to be moved to drivers/acpi/

> On arm64 we do not carry out any resource survey at present, but
> if we do (and we should), it will have to work for both DT and ACPI
> systems.

This is not a difference between DT and ACPI, but a difference between
embedded and server. Server platforms that have a proper firmware
to scan the PCI bus should not set PCI_REASSIGN_ALL_RSRC and
PCI_REASSIGN_ALL_BUS, while embedded systems that don't set up
the PCI bus before boot have to set those.

On ACPI, reassigning the resources would actually be dangerous
because the firmware may rely on devices to be at a certain
location (both bus number and mmio address), or may play tricks
here to hide stuff from the OS.

> > > Also, can you point me to the OF device tree specification where it 
> > > tells how to specify PCI BAR addresses, I would especially be interested 
> > > in knowing how to specify fixed SRIOV BAR addresses in the device tree.
> > 
> > This is the 'n' bit mentioned sections 2.1.3 and 2.2.1.1 of the
> > PCI binding. When it is set, the OS is not supposed to try to
> > reassign the BAR even on machines that otherwise do a complete
> > rescan.
> 
> I do not see any code in the kernel taking care of that bit and
> if I am not mistaken the code that allows creating pci devices
> exists in PowerPC arch code (arch/powerpc/kernel/pci_of_scan.c) and
> should be moved out of there for the other arches to use it.
> 
> Or maybe we can use DT just to fix-up the resource flags (ie
> every PCI device will have its own of_node set if it is present
> in the DT, we can use it to fixup its resources and related flags,
> pcibios_add_device() ?).

I haven't looked at the code in detail now, but I'm pretty sure that
a device node that refers to a PCI device is connected to the
dev->of_node pointer already on all architectures.

It's quite possible that we never implemented the fixed BAR logic
though, but it can be done based on those pointers.

	Arnd

[PATCH 3/3] PCI: generic: Add support for Cavium ThunderX PCIe root complexes.

[David Daney]

From: David Daney <david.daney@cavium.com>

The config space for external PCIe root complexes on some Cavium
ThunderX SoCs is very similar to CAM and ECAM, but differs in the
shift values that have to be applied to the bus and devfn numbers to
compose that address window offset.  These root complexes also have
the interesting property that there is no root bridge, so the standard
manner of limiting scanning to only the first device doesn't work.  We
can use the standard pci-host-generic driver if we make a minor
addition to handle these differences, so we...

Add a mapping function for ThunderX PCIe root complexes with a bus
shift of 24 and devfn shift of 16.  Ignore accesses for devices other
than the first device on the primary bus.

Document the whole thing in devicetree/bindings/pci/host-generic-pci.txt

Signed-off-by: David Daney <david.daney@cavium.com>
---
 .../devicetree/bindings/pci/host-generic-pci.txt   |  8 +++---
 drivers/pci/host/pci-host-generic.c                | 29 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
index 105a968..a5aed0f 100644
--- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt
+++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
@@ -14,9 +14,11 @@ tree bindings communicated in pci.txt:
 
 Properties of the host controller node:
 
-- compatible     : Must be "pci-host-cam-generic" or "pci-host-ecam-generic"
-                   depending on the layout of configuration space (CAM vs
-                   ECAM respectively).
+- compatible     : One of the following with bus:devfn:reg mapped to the
+                   PCI config space address window in the bit positions shown:
+                   "pci-host-cam-generic" -- 'CAM'  bits 16:8:0
+                   "pci-host-ecam-generic" -- 'ECAM'  bits 20:12:0
+                   "cavium,pci-host-thunder-pem" --  bits 24:16:0
 
 - device_type    : Must be "pci".
 
diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index e364232..e1d8d5b 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = {
 	}
 };
 
+static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus,
+						     unsigned int devfn,
+						     int where)
+{
+	struct gen_pci *pci = bus->sysdata;
+	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
+
+	/*
+	 * Thunder PEM is a PCIe RC, but without a root bridge.  On
+	 * the primary bus, ignore accesses for devices other than
+	 * the first device.
+	 */
+	if (idx == 0 && (devfn & ~7u))
+		return NULL;
+	return pci->cfg.win[idx] + ((devfn << 16) | where);
+}
+
+static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = {
+	.bus_shift	= 24,
+	.ops		= {
+		.map_bus	= gen_pci_map_cfg_bus_thunder_pem,
+		.read		= pci_generic_config_read,
+		.write		= pci_generic_config_write,
+	}
+};
+
 static const struct of_device_id gen_pci_of_match[] = {
 	{ .compatible = "pci-host-cam-generic",
 	  .data = &gen_pci_cfg_cam_bus_ops },
@@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = {
 	{ .compatible = "pci-host-ecam-generic",
 	  .data = &gen_pci_cfg_ecam_bus_ops },
 
+	{ .compatible = "cavium,pci-host-thunder-pem",
+	  .data = &gen_pci_cfg_thunder_pem_bus_ops },
+
 	{ },
 };
 MODULE_DEVICE_TABLE(of, gen_pci_of_match);
-- 
1.9.1

Re: [PATCH 3/3] PCI: generic: Add support for Cavium ThunderX PCIe root complexes.

[Lorenzo Pieralisi]

On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> The config space for external PCIe root complexes on some Cavium
> ThunderX SoCs is very similar to CAM and ECAM, but differs in the
> shift values that have to be applied to the bus and devfn numbers to
> compose that address window offset.  These root complexes also have
> the interesting property that there is no root bridge, so the standard
> manner of limiting scanning to only the first device doesn't work.  We
> can use the standard pci-host-generic driver if we make a minor
> addition to handle these differences, so we...
> 
> Add a mapping function for ThunderX PCIe root complexes with a bus
> shift of 24 and devfn shift of 16.  Ignore accesses for devices other
> than the first device on the primary bus.
> 
> Document the whole thing in devicetree/bindings/pci/host-generic-pci.txt
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/pci/host-generic-pci.txt   |  8 +++---
>  drivers/pci/host/pci-host-generic.c                | 29 ++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
> index 105a968..a5aed0f 100644
> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
> @@ -14,9 +14,11 @@ tree bindings communicated in pci.txt:
>  
>  Properties of the host controller node:
>  
> -- compatible     : Must be "pci-host-cam-generic" or "pci-host-ecam-generic"
> -                   depending on the layout of configuration space (CAM vs
> -                   ECAM respectively).
> +- compatible     : One of the following with bus:devfn:reg mapped to the
> +                   PCI config space address window in the bit positions shown:
> +                   "pci-host-cam-generic" -- 'CAM'  bits 16:8:0
> +                   "pci-host-ecam-generic" -- 'ECAM'  bits 20:12:0
> +                   "cavium,pci-host-thunder-pem" --  bits 24:16:0

To me that's ECAM left shifted by 4. Is not it just a matter of defining
config space base address (ie reg property) differently ?

Lorenzo

>  
>  - device_type    : Must be "pci".
>  
> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index e364232..e1d8d5b 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = {
>  	}
>  };
>  
> +static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus,
> +						     unsigned int devfn,
> +						     int where)
> +{
> +	struct gen_pci *pci = bus->sysdata;
> +	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
> +
> +	/*
> +	 * Thunder PEM is a PCIe RC, but without a root bridge.  On
> +	 * the primary bus, ignore accesses for devices other than
> +	 * the first device.
> +	 */
> +	if (idx == 0 && (devfn & ~7u))
> +		return NULL;
> +	return pci->cfg.win[idx] + ((devfn << 16) | where);
> +}
> +
> +static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = {
> +	.bus_shift	= 24,
> +	.ops		= {
> +		.map_bus	= gen_pci_map_cfg_bus_thunder_pem,
> +		.read		= pci_generic_config_read,
> +		.write		= pci_generic_config_write,
> +	}
> +};
> +
>  static const struct of_device_id gen_pci_of_match[] = {
>  	{ .compatible = "pci-host-cam-generic",
>  	  .data = &gen_pci_cfg_cam_bus_ops },
> @@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = {
>  	{ .compatible = "pci-host-ecam-generic",
>  	  .data = &gen_pci_cfg_ecam_bus_ops },
>  
> +	{ .compatible = "cavium,pci-host-thunder-pem",
> +	  .data = &gen_pci_cfg_thunder_pem_bus_ops },
> +
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, gen_pci_of_match);
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 3/3] PCI: generic: Add support for Cavium ThunderX PCIe root complexes.

[David Daney]

On 09/22/2015 09:05 AM, Lorenzo Pieralisi wrote:
> On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> The config space for external PCIe root complexes on some Cavium
>> ThunderX SoCs is very similar to CAM and ECAM, but differs in the
>> shift values that have to be applied to the bus and devfn numbers to
>> compose that address window offset.  These root complexes also have
>> the interesting property that there is no root bridge, so the standard
>> manner of limiting scanning to only the first device doesn't work.  We
>> can use the standard pci-host-generic driver if we make a minor
>> addition to handle these differences, so we...
>>
>> Add a mapping function for ThunderX PCIe root complexes with a bus
>> shift of 24 and devfn shift of 16.  Ignore accesses for devices other
>> than the first device on the primary bus.
>>
>> Document the whole thing in devicetree/bindings/pci/host-generic-pci.txt
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   .../devicetree/bindings/pci/host-generic-pci.txt   |  8 +++---
>>   drivers/pci/host/pci-host-generic.c                | 29 ++++++++++++++++++++++
>>   2 files changed, 34 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
>> index 105a968..a5aed0f 100644
>> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt
>> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
>> @@ -14,9 +14,11 @@ tree bindings communicated in pci.txt:
>>
>>   Properties of the host controller node:
>>
>> -- compatible     : Must be "pci-host-cam-generic" or "pci-host-ecam-generic"
>> -                   depending on the layout of configuration space (CAM vs
>> -                   ECAM respectively).
>> +- compatible     : One of the following with bus:devfn:reg mapped to the
>> +                   PCI config space address window in the bit positions shown:
>> +                   "pci-host-cam-generic" -- 'CAM'  bits 16:8:0
>> +                   "pci-host-ecam-generic" -- 'ECAM'  bits 20:12:0
>> +                   "cavium,pci-host-thunder-pem" --  bits 24:16:0
>
> To me that's ECAM left shifted by 4. Is not it just a matter of defining
> config space base address (ie reg property) differently ?

No.

That's like saying ECAM is just CAM shifted by 4, can't we just specify 
the "reg" differently.

The "reg" property describes the base of the config space access window. 
  CAM/ECAM/cavium,pci-host-thunder-pem describe how to generate offsets 
from that base, The offset calculation requires information that is not 
present in the "reg" property, it is enumerated in the various 
"compatible" values listed above.

David Daney

>
> Lorenzo
>
>>
>>   - device_type    : Must be "pci".
>>
>> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
>> index e364232..e1d8d5b 100644
>> --- a/drivers/pci/host/pci-host-generic.c
>> +++ b/drivers/pci/host/pci-host-generic.c
>> @@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = {
>>   	}
>>   };
>>
>> +static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus,
>> +						     unsigned int devfn,
>> +						     int where)
>> +{
>> +	struct gen_pci *pci = bus->sysdata;
>> +	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
>> +
>> +	/*
>> +	 * Thunder PEM is a PCIe RC, but without a root bridge.  On
>> +	 * the primary bus, ignore accesses for devices other than
>> +	 * the first device.
>> +	 */
>> +	if (idx == 0 && (devfn & ~7u))
>> +		return NULL;
>> +	return pci->cfg.win[idx] + ((devfn << 16) | where);
>> +}
>> +
>> +static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = {
>> +	.bus_shift	= 24,
>> +	.ops		= {
>> +		.map_bus	= gen_pci_map_cfg_bus_thunder_pem,
>> +		.read		= pci_generic_config_read,
>> +		.write		= pci_generic_config_write,
>> +	}
>> +};
>> +
>>   static const struct of_device_id gen_pci_of_match[] = {
>>   	{ .compatible = "pci-host-cam-generic",
>>   	  .data = &gen_pci_cfg_cam_bus_ops },
>> @@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = {
>>   	{ .compatible = "pci-host-ecam-generic",
>>   	  .data = &gen_pci_cfg_ecam_bus_ops },
>>
>> +	{ .compatible = "cavium,pci-host-thunder-pem",
>> +	  .data = &gen_pci_cfg_thunder_pem_bus_ops },
>> +
>>   	{ },
>>   };
>>   MODULE_DEVICE_TABLE(of, gen_pci_of_match);
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

Re: [PATCH 3/3] PCI: generic: Add support for Cavium ThunderX PCIe root complexes.

[Lorenzo Pieralisi]

On Tue, Sep 22, 2015 at 05:13:45PM +0100, David Daney wrote:
> On 09/22/2015 09:05 AM, Lorenzo Pieralisi wrote:
> > On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote:

[...]

> >>   Properties of the host controller node:
> >>
> >> -- compatible     : Must be "pci-host-cam-generic" or "pci-host-ecam-generic"
> >> -                   depending on the layout of configuration space (CAM vs
> >> -                   ECAM respectively).
> >> +- compatible     : One of the following with bus:devfn:reg mapped to the
> >> +                   PCI config space address window in the bit positions shown:
> >> +                   "pci-host-cam-generic" -- 'CAM'  bits 16:8:0
> >> +                   "pci-host-ecam-generic" -- 'ECAM'  bits 20:12:0
> >> +                   "cavium,pci-host-thunder-pem" --  bits 24:16:0
> >
> > To me that's ECAM left shifted by 4. Is not it just a matter of defining
> > config space base address (ie reg property) differently ?
> 
> No.
> 
> That's like saying ECAM is just CAM shifted by 4, can't we just specify 
> the "reg" differently.

Ok, sorry I was not clear, apologies. I meant to ask if it is something
that can be configured in your platform and apparently it is not,
I worded it wrongly.

> The "reg" property describes the base of the config space access window. 
>   CAM/ECAM/cavium,pci-host-thunder-pem describe how to generate offsets 
> from that base, The offset calculation requires information that is not 
> present in the "reg" property, it is enumerated in the various 
> "compatible" values listed above.

Agreed, this also forces you to map in more virtual address space for
a given bus, right ? I am also a bit worried about how we can make this
work with ACPI, where MCFG accessors take the bus and devfn shifts for
granted (ie ECAM specs), so I asked.

Thanks,
Lorenzo

> 
> David Daney
> 
> >
> > Lorenzo
> >
> >>
> >>   - device_type    : Must be "pci".
> >>
> >> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> >> index e364232..e1d8d5b 100644
> >> --- a/drivers/pci/host/pci-host-generic.c
> >> +++ b/drivers/pci/host/pci-host-generic.c
> >> @@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = {
> >>   	}
> >>   };
> >>
> >> +static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus,
> >> +						     unsigned int devfn,
> >> +						     int where)
> >> +{
> >> +	struct gen_pci *pci = bus->sysdata;
> >> +	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
> >> +
> >> +	/*
> >> +	 * Thunder PEM is a PCIe RC, but without a root bridge.  On
> >> +	 * the primary bus, ignore accesses for devices other than
> >> +	 * the first device.
> >> +	 */
> >> +	if (idx == 0 && (devfn & ~7u))
> >> +		return NULL;
> >> +	return pci->cfg.win[idx] + ((devfn << 16) | where);
> >> +}
> >> +
> >> +static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = {
> >> +	.bus_shift	= 24,
> >> +	.ops		= {
> >> +		.map_bus	= gen_pci_map_cfg_bus_thunder_pem,
> >> +		.read		= pci_generic_config_read,
> >> +		.write		= pci_generic_config_write,
> >> +	}
> >> +};
> >> +
> >>   static const struct of_device_id gen_pci_of_match[] = {
> >>   	{ .compatible = "pci-host-cam-generic",
> >>   	  .data = &gen_pci_cfg_cam_bus_ops },
> >> @@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = {
> >>   	{ .compatible = "pci-host-ecam-generic",
> >>   	  .data = &gen_pci_cfg_ecam_bus_ops },
> >>
> >> +	{ .compatible = "cavium,pci-host-thunder-pem",
> >> +	  .data = &gen_pci_cfg_thunder_pem_bus_ops },
> >> +
> >>   	{ },
> >>   };
> >>   MODULE_DEVICE_TABLE(of, gen_pci_of_match);
> >> --
> >> 1.9.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> 

Re: [PATCH 3/3] PCI: generic: Add support for Cavium ThunderX PCIe root complexes.

[David Daney]

On 09/22/2015 09:40 AM, Lorenzo Pieralisi wrote:
> On Tue, Sep 22, 2015 at 05:13:45PM +0100, David Daney wrote:
>> On 09/22/2015 09:05 AM, Lorenzo Pieralisi wrote:
>>> On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote:
>
> [...]
>
>>>>    Properties of the host controller node:
>>>>
>>>> -- compatible     : Must be "pci-host-cam-generic" or "pci-host-ecam-generic"
>>>> -                   depending on the layout of configuration space (CAM vs
>>>> -                   ECAM respectively).
>>>> +- compatible     : One of the following with bus:devfn:reg mapped to the
>>>> +                   PCI config space address window in the bit positions shown:
>>>> +                   "pci-host-cam-generic" -- 'CAM'  bits 16:8:0
>>>> +                   "pci-host-ecam-generic" -- 'ECAM'  bits 20:12:0
>>>> +                   "cavium,pci-host-thunder-pem" --  bits 24:16:0
>>>
>>> To me that's ECAM left shifted by 4. Is not it just a matter of defining
>>> config space base address (ie reg property) differently ?
>>
>> No.
>>
>> That's like saying ECAM is just CAM shifted by 4, can't we just specify
>> the "reg" differently.
>
> Ok, sorry I was not clear, apologies. I meant to ask if it is something
> that can be configured in your platform and apparently it is not,

No, the hardware cannot be adjusted.

> I worded it wrongly.

OK, I guess I also misunderstood what you were suggesting.

>
>> The "reg" property describes the base of the config space access window.
>>    CAM/ECAM/cavium,pci-host-thunder-pem describe how to generate offsets
>> from that base, The offset calculation requires information that is not
>> present in the "reg" property, it is enumerated in the various
>> "compatible" values listed above.
>
> Agreed, this also forces you to map in more virtual address space for
> a given bus, right ?

Yes.  It takes a full 32-bits of address space to have access to all PCI 
buses on the root complex.

> I am also a bit worried about how we can make this
> work with ACPI, where MCFG accessors take the bus and devfn shifts for
> granted (ie ECAM specs), so I asked.

If ACPI is not flexible enough to describe actual hardware, it will 
either have to be enhanced, or not used.


>
> Thanks,
> Lorenzo
>
>>
>> David Daney
>>
>>>
>>> Lorenzo
>>>
>>>>
>>>>    - device_type    : Must be "pci".
>>>>
>>>> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
>>>> index e364232..e1d8d5b 100644
>>>> --- a/drivers/pci/host/pci-host-generic.c
>>>> +++ b/drivers/pci/host/pci-host-generic.c
>>>> @@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = {
>>>>    	}
>>>>    };
>>>>
>>>> +static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus,
>>>> +						     unsigned int devfn,
>>>> +						     int where)
>>>> +{
>>>> +	struct gen_pci *pci = bus->sysdata;
>>>> +	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
>>>> +
>>>> +	/*
>>>> +	 * Thunder PEM is a PCIe RC, but without a root bridge.  On
>>>> +	 * the primary bus, ignore accesses for devices other than
>>>> +	 * the first device.
>>>> +	 */
>>>> +	if (idx == 0 && (devfn & ~7u))
>>>> +		return NULL;
>>>> +	return pci->cfg.win[idx] + ((devfn << 16) | where);
>>>> +}
>>>> +
>>>> +static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = {
>>>> +	.bus_shift	= 24,
>>>> +	.ops		= {
>>>> +		.map_bus	= gen_pci_map_cfg_bus_thunder_pem,
>>>> +		.read		= pci_generic_config_read,
>>>> +		.write		= pci_generic_config_write,
>>>> +	}
>>>> +};
>>>> +
>>>>    static const struct of_device_id gen_pci_of_match[] = {
>>>>    	{ .compatible = "pci-host-cam-generic",
>>>>    	  .data = &gen_pci_cfg_cam_bus_ops },
>>>> @@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = {
>>>>    	{ .compatible = "pci-host-ecam-generic",
>>>>    	  .data = &gen_pci_cfg_ecam_bus_ops },
>>>>
>>>> +	{ .compatible = "cavium,pci-host-thunder-pem",
>>>> +	  .data = &gen_pci_cfg_thunder_pem_bus_ops },
>>>> +
>>>>    	{ },
>>>>    };
>>>>    MODULE_DEVICE_TABLE(of, gen_pci_of_match);
>>>> --
>>>> 1.9.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>

Re: [PATCH 3/3] PCI: generic: Add support for Cavium ThunderX PCIe root complexes.

[Will Deacon]

On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> The config space for external PCIe root complexes on some Cavium
> ThunderX SoCs is very similar to CAM and ECAM, but differs in the
> shift values that have to be applied to the bus and devfn numbers to
> compose that address window offset.  These root complexes also have
> the interesting property that there is no root bridge, so the standard
> manner of limiting scanning to only the first device doesn't work.  We
> can use the standard pci-host-generic driver if we make a minor
> addition to handle these differences, so we...
> 
> Add a mapping function for ThunderX PCIe root complexes with a bus
> shift of 24 and devfn shift of 16.  Ignore accesses for devices other
> than the first device on the primary bus.
> 
> Document the whole thing in devicetree/bindings/pci/host-generic-pci.txt
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/pci/host-generic-pci.txt   |  8 +++---
>  drivers/pci/host/pci-host-generic.c                | 29 ++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 3 deletions(-)

Thanks, this looks better now:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH 3/3] PCI: generic: Add support for Cavium ThunderX PCIe root complexes.

[David Daney]

On 09/22/2015 11:52 AM, Will Deacon wrote:
> On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> The config space for external PCIe root complexes on some Cavium
>> ThunderX SoCs is very similar to CAM and ECAM, but differs in the
>> shift values that have to be applied to the bus and devfn numbers to
>> compose that address window offset.  These root complexes also have
>> the interesting property that there is no root bridge, so the standard
>> manner of limiting scanning to only the first device doesn't work.  We
>> can use the standard pci-host-generic driver if we make a minor
>> addition to handle these differences, so we...
>>
>> Add a mapping function for ThunderX PCIe root complexes with a bus
>> shift of 24 and devfn shift of 16.  Ignore accesses for devices other
>> than the first device on the primary bus.
>>
>> Document the whole thing in devicetree/bindings/pci/host-generic-pci.txt
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   .../devicetree/bindings/pci/host-generic-pci.txt   |  8 +++---
>>   drivers/pci/host/pci-host-generic.c                | 29 ++++++++++++++++++++++
>>   2 files changed, 34 insertions(+), 3 deletions(-)
>
> Thanks, this looks better now:
>
>    Acked-by: Will Deacon <will.deacon@arm.com>
>

Thanks Will.

Because patches 1/3 and 2/3 will be reworked, I will re-send this as a 
stand-alone patch.

David Daney


> Will
>