| Message ID | 65114e62-7458-b6f7-327c-f07a5096a452@sigmadesigns.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 29/03/17 12:34, Marc Gonzalez wrote: > This driver is used to work around HW bugs in the controller. > > Note: the controller does NOT support the following features. > > Legacy PCI interrupts > IO space > > Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> > --- > Documentation/devicetree/bindings/pci/tango-pcie.txt | 33 +++++++++ > drivers/pci/host/Kconfig | 7 ++ > drivers/pci/host/Makefile | 1 + > drivers/pci/host/pcie-tango.c | 150 +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 191 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/tango-pcie.txt b/Documentation/devicetree/bindings/pci/tango-pcie.txt > new file mode 100644 > index 000000000000..f8e150ec41d8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/tango-pcie.txt > @@ -0,0 +1,33 @@ > +Sigma Designs Tango PCIe controller > + > +Required properties: > + > +- compatible: "sigma,smp8759-pcie" > +- reg: address/size of PCI configuration space, and pcie_reg > +- bus-range: defined by size of PCI configuration space > +- device_type: "pci" > +- #size-cells: <2> > +- #address-cells: <3> > +- #interrupt-cells: <1> > +- ranges: translation from system to bus addresses > +- interrupts: spec for misc interrupts and MSI > +- msi-controller > + > +Example: > + > + pcie@2e000 { > + compatible = "sigma,smp8759-pcie"; > + reg = <0x50000000 SZ_64M>, <0x2e000 0x100>; > + bus-range = <0 63>; > + device_type = "pci"; > + #size-cells = <2>; > + #address-cells = <3>; > + #interrupt-cells = <1>; > + /* http://elinux.org/Device_Tree_Usage#PCI_Address_Translation */ > + /* BUS_ADDRESS(3) CPU_PHYSICAL(1) SIZE(2) */ > + ranges = <0x02000000 0x0 0x04000000 0x54000000 0x0 SZ_192M>; > + interrupts = > + <54 IRQ_TYPE_LEVEL_HIGH>, /* misc interrupts */ > + <55 IRQ_TYPE_LEVEL_HIGH>; /* MSI */ > + msi-controller; > + }; > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index d7e7c0a827c3..8a622578a760 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -285,6 +285,13 @@ config PCIE_ROCKCHIP > There is 1 internal PCIe port available to support GEN2 with > 4 slots. > > +config PCIE_TANGO > + tristate "Tango PCIe controller" > + depends on ARCH_TANGO && PCI_MSI && OF > + select PCI_HOST_COMMON > + help > + Say Y here to enable PCIe controller support on Tango SoC. Nit: in line with the other help texts, that could probably benefit from a wee bit more context (i.e. "Sigma Designs Tango SoCs") > + > config VMD > depends on PCI_MSI && X86_64 > tristate "Intel Volume Management Device Driver" > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile > index 084cb4983645..fc7ea90196f3 100644 > --- a/drivers/pci/host/Makefile > +++ b/drivers/pci/host/Makefile > @@ -32,4 +32,5 @@ obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o > obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o > obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o > +obj-$(CONFIG_PCIE_TANGO) += pcie-tango.o > obj-$(CONFIG_VMD) += vmd.o > diff --git a/drivers/pci/host/pcie-tango.c b/drivers/pci/host/pcie-tango.c > index e88850983a1d..dbc2663486b5 100644 > --- a/drivers/pci/host/pcie-tango.c > +++ b/drivers/pci/host/pcie-tango.c > @@ -192,3 +192,153 @@ static int tango_msi_probe(struct platform_device *pdev, struct tango_pcie *pcie > > return 0; > } > + > +/*** HOST BRIDGE SUPPORT ***/ > + > +static int smp8759_config_read(struct pci_bus *bus, > + unsigned int devfn, int where, int size, u32 *val) > +{ > + int ret; > + struct pci_config_window *cfg = bus->sysdata; > + struct tango_pcie *pcie = dev_get_drvdata(cfg->parent); > + > + /* > + * QUIRK #1 > + * Reads in configuration space outside devfn 0 return garbage. > + */ > + if (devfn != 0) { > + *val = ~0; /* Is this required even if we return an error? */ > + return PCIBIOS_FUNC_NOT_SUPPORTED; /* Error seems appropriate */ > + } > + > + /* > + * QUIRK #2 > + * The root complex advertizes a fake BAR, which is used to filter > + * bus-to-system requests. Hide it from Linux. > + */ > + if (where == PCI_BASE_ADDRESS_0 && bus->number == 0) { > + *val = 0; /* 0 or ~0 to hide the BAR from Linux? */ > + return PCIBIOS_SUCCESSFUL; /* Should we return error or success? */ > + } > + > + /* > + * QUIRK #3 > + * Unfortunately, config and mem spaces are muxed. > + * Linux does not support such a setting, since drivers are free > + * to access mem space directly, at any time. > + * Therefore, we can only PRAY that config and mem space accesses > + * NEVER occur concurrently. > + */ What about David's suggestion of using an IPI for safe mutual exclusion? > + writel_relaxed(1, pcie->mux); > + ret = pci_generic_config_read(bus, devfn, where, size, val); > + writel_relaxed(0, pcie->mux); > + > + return ret; > +} > + > +static int smp8759_config_write(struct pci_bus *bus, > + unsigned int devfn, int where, int size, u32 val) > +{ > + int ret; > + struct pci_config_window *cfg = bus->sysdata; > + struct tango_pcie *pcie = dev_get_drvdata(cfg->parent); > + > + writel_relaxed(1, pcie->mux); > + ret = pci_generic_config_write(bus, devfn, where, size, val); > + writel_relaxed(0, pcie->mux); > + > + return ret; > +} > + > +static struct pci_ecam_ops smp8759_ecam_ops = { > + .bus_shift = 20, > + .pci_ops = { > + .map_bus = pci_ecam_map_bus, > + .read = smp8759_config_read, > + .write = smp8759_config_write, > + } > +}; > + > +static const struct of_device_id tango_pcie_ids[] = { > + { .compatible = "sigma,smp8759-pcie" }, General tip: use your .data member here... > + { /* sentinel */ }, > +}; > + > +static void smp8759_init(struct tango_pcie *pcie, void __iomem *base) > +{ > + pcie->mux = base + 0x48; > + pcie->msi_status = base + 0x80; > + pcie->msi_mask = base + 0xa0; > + pcie->msi_doorbell = 0xa0000000 + 0x2e07c; > +} > + > +static int tango_pcie_probe(struct platform_device *pdev) > +{ > + int ret; > + void __iomem *base; > + struct resource *res; > + struct tango_pcie *pcie; > + struct device *dev = &pdev->dev; > + > + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); > + if (!pcie) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, pcie); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + if (of_device_is_compatible(dev->of_node, "sigma,smp8759-pcie")) > + smp8759_init(pcie, base); ...then retrieve it with of_device_get_match_data() here. No need to reinvent the wheel (or have to worry about the ordering of multiple compatibles once rev. n+1 comes around). > + > + ret = tango_msi_probe(pdev, pcie); > + if (ret) > + return ret; > + > + return pci_host_common_probe(pdev, &smp8759_ecam_ops); > +} > + > +static int tango_pcie_remove(struct platform_device *pdev) > +{ > + return tango_msi_remove(pdev); > +} > + > +static struct platform_driver tango_pcie_driver = { > + .probe = tango_pcie_probe, > + .remove = tango_pcie_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = tango_pcie_ids, > + }, > +}; > + > +module_platform_driver(tango_pcie_driver); > + > +#define VENDOR_SIGMA 0x1105 Should this not be in include/linux/pci_ids.h? Robin. > + > +/* > + * QUIRK #4 > + * The root complex advertizes the wrong device class. > + * Header Type 1 is for PCI-to-PCI bridges. > + */ > +static void tango_fixup_class(struct pci_dev *dev) > +{ > + dev->class = PCI_CLASS_BRIDGE_PCI << 8; > +} > +DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class); > + > +/* > + * QUIRK #5 > + * Only transfers within the root complex BAR are forwarded to the host. > + * By default, the DMA framework expects that > + * PCI address 0x8000_0000 maps to system address 0x8000_0000 > + * which is where DRAM0 is mapped. > + */ > +static void tango_fixup_bar(struct pci_dev *dev) > +{ > + pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, 0x80000000); > +} > +DECLARE_PCI_FIXUP_FINAL(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_bar); >
On 29/03/2017 14:19, Robin Murphy wrote: > On 29/03/17 12:34, Marc Gonzalez wrote: > >> + /* >> + * QUIRK #3 >> + * Unfortunately, config and mem spaces are muxed. >> + * Linux does not support such a setting, since drivers are free >> + * to access mem space directly, at any time. >> + * Therefore, we can only PRAY that config and mem space accesses >> + * NEVER occur concurrently. >> + */ > > What about David's suggestion of using an IPI for safe mutual exclusion? I was left with the impression that this wouldn't solve the problem. If a mem space access is "in flight" on core0 when core1 starts a config space access, an IPI will not prevent breakage. Did I misunderstand? For my education, what is the API to send an IPI? And the API to handle an IPI? >> + if (of_device_is_compatible(dev->of_node, "sigma,smp8759-pcie")) >> + smp8759_init(pcie, base); > > ...then retrieve it with of_device_get_match_data() here. No need to > reinvent the wheel (or have to worry about the ordering of multiple > compatibles once rev. n+1 comes around). I actually asked about this on IRC. The consensus was "use what best fits your use case". I need to do some processing based on the revision, so I thought if (chip_x) do_chip_x_init() was a good way to express my intent. Did I misunderstand? For example, the init function for rev2 currently looks like this: static void rev2_init(struct tango_pcie *pcie, void __iomem *base) { void __iomem *misc_irq = base + 0x40; void __iomem *doorbell = base + 0x8c; pcie->mux = base + 0x2c; pcie->msi_status = base + 0x4c; pcie->msi_mask = base + 0x6c; pcie->msi_doorbell = 0x80000000; writel(lower_32_bits(pcie->msi_doorbell), doorbell + 0); writel(upper_32_bits(pcie->msi_doorbell), doorbell + 4); /* Enable legacy PCI interrupts */ writel(BIT(15), misc_irq); writel(0xf << 4, misc_irq + 4); } >> +#define VENDOR_SIGMA 0x1105 > > Should this not be in include/linux/pci_ids.h? Doh! Very likely. Thanks. Regards.
On 29/03/17 13:53, Mason wrote: > On 29/03/2017 14:19, Robin Murphy wrote: > >> On 29/03/17 12:34, Marc Gonzalez wrote: >> >>> + /* >>> + * QUIRK #3 >>> + * Unfortunately, config and mem spaces are muxed. >>> + * Linux does not support such a setting, since drivers are free >>> + * to access mem space directly, at any time. >>> + * Therefore, we can only PRAY that config and mem space accesses >>> + * NEVER occur concurrently. >>> + */ >> >> What about David's suggestion of using an IPI for safe mutual exclusion? > > I was left with the impression that this wouldn't solve the problem. > If a mem space access is "in flight" on core0 when core1 starts a > config space access, an IPI will not prevent breakage. > > Did I misunderstand? > > For my education, what is the API to send an IPI? > And the API to handle an IPI? There are a few ways you could implement some custom cross-call, although in this case I think stop_machine() would probably be the most appropriate candidate. However, you're right that in general it may not actually help enough to be worthwhile - a DSB SY would ensure that in-flight transactions have at least been observed by the CPUs and any other coherent masters, but for any writes with a memory type allowing early acknowledgement (i.e. a Normal or Device mapping of a BAR) that doesn't necessarily correlate with them having reached their ultimate destination. For a PCI destination in particular, I think the normal way to ensure all posted writes have completed would be to read from config space; ah... >>> + if (of_device_is_compatible(dev->of_node, "sigma,smp8759-pcie")) >>> + smp8759_init(pcie, base); >> >> ...then retrieve it with of_device_get_match_data() here. No need to >> reinvent the wheel (or have to worry about the ordering of multiple >> compatibles once rev. n+1 comes around). > > I actually asked about this on IRC. The consensus was "use what > best fits your use case". I need to do some processing based on > the revision, so I thought > > if (chip_x) > do_chip_x_init() > > was a good way to express my intent. Did I misunderstand? No, I'm in no way disputing that; what I'm pointing out is that you already have an explicitly provided way to associate a value of "chip_x" with a given compatible string - see other callers of of_device_get_match_data() for inspiration. I don't have much of an opinion as to whether it's an enum, a static structure of offsets and callbacks, or you embrace the nasal demons and just wedge the init function pointer in there directly (this'll never run on IA-64/M68k/etc., right? :P). The point is that not only is it cleaner and scales better as the driver grows, it stops you having to worry at all about setting this trap for yourself: compatible = "rev3-with-extra-fun", "rev3"; ... if (of_device_is_compatible(dev, "rev3")) boring_init_without_extra_fun(); /* :( */ because once you've made your code robust against that, you'll realise that what you've done is wasted your time open-coding a creaky approximation of of_match_device(). Robin. > For example, the init function for rev2 currently looks like this: > > static void rev2_init(struct tango_pcie *pcie, void __iomem *base) > { > void __iomem *misc_irq = base + 0x40; > void __iomem *doorbell = base + 0x8c; > > pcie->mux = base + 0x2c; > pcie->msi_status = base + 0x4c; > pcie->msi_mask = base + 0x6c; > pcie->msi_doorbell = 0x80000000; > > writel(lower_32_bits(pcie->msi_doorbell), doorbell + 0); > writel(upper_32_bits(pcie->msi_doorbell), doorbell + 4); > > /* Enable legacy PCI interrupts */ > writel(BIT(15), misc_irq); > writel(0xf << 4, misc_irq + 4); > } > >>> +#define VENDOR_SIGMA 0x1105 >> >> Should this not be in include/linux/pci_ids.h? > > Doh! Very likely. Thanks. > > Regards. >
> > For my education, what is the API to send an IPI? > > And the API to handle an IPI? > > There are a few ways you could implement some custom cross-call, > although in this case I think stop_machine() would probably be the most > appropriate candidate. However, you're right that in general it may not > actually help enough to be worthwhile - a DSB SY would ensure that > in-flight transactions have at least been observed by the CPUs and any > other coherent masters, but for any writes with a memory type allowing > early acknowledgement (i.e. a Normal or Device mapping of a BAR) that > doesn't necessarily correlate with them having reached their ultimate > destination. For a PCI destination in particular, I think the normal way > to ensure all posted writes have completed would be to read from config > space; ah... He almost certainly doesn't need to wait for the cycle to complete, just long enough for the cycle to have been sent. David
Hi Marc,
[auto build test ERROR on v4.9-rc8]
[also build test ERROR on next-20170330]
[cannot apply to pci/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Marc-Gonzalez/Tango-PCIe-controller-support/20170330-154028
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All error/warnings (new ones prefixed by >>):
In file included from drivers/pci/host/pcie-tango.c:4:0:
>> include/linux/pci-ecam.h:29:19: error: field 'pci_ops' has incomplete type
struct pci_ops pci_ops;
^~~~~~~
>> include/linux/pci-ecam.h:57:39: warning: 'struct pci_bus' declared inside parameter list will not be visible outside of this definition or declaration
void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
^~~~~~~
>> drivers/pci/host/pcie-tango.c:198:39: warning: 'struct pci_bus' declared inside parameter list will not be visible outside of this definition or declaration
static int smp8759_config_read(struct pci_bus *bus,
^~~~~~~
drivers/pci/host/pcie-tango.c: In function 'smp8759_config_read':
>> drivers/pci/host/pcie-tango.c:202:37: error: dereferencing pointer to incomplete type 'struct pci_bus'
struct pci_config_window *cfg = bus->sysdata;
^~
>> drivers/pci/host/pcie-tango.c:211:10: error: 'PCIBIOS_FUNC_NOT_SUPPORTED' undeclared (first use in this function)
return PCIBIOS_FUNC_NOT_SUPPORTED; /* Error seems appropriate */
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-tango.c:211:10: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/pci/host/pcie-tango.c:219:15: error: 'PCI_BASE_ADDRESS_0' undeclared (first use in this function)
if (where == PCI_BASE_ADDRESS_0 && bus->number == 0) {
^~~~~~~~~~~~~~~~~~
>> drivers/pci/host/pcie-tango.c:221:10: error: 'PCIBIOS_SUCCESSFUL' undeclared (first use in this function)
return PCIBIOS_SUCCESSFUL; /* Should we return error or success? */
^~~~~~~~~~~~~~~~~~
>> drivers/pci/host/pcie-tango.c:233:8: error: implicit declaration of function 'pci_generic_config_read' [-Werror=implicit-function-declaration]
ret = pci_generic_config_read(bus, devfn, where, size, val);
^~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-tango.c: At top level:
drivers/pci/host/pcie-tango.c:239:40: warning: 'struct pci_bus' declared inside parameter list will not be visible outside of this definition or declaration
static int smp8759_config_write(struct pci_bus *bus,
^~~~~~~
drivers/pci/host/pcie-tango.c: In function 'smp8759_config_write':
drivers/pci/host/pcie-tango.c:243:37: error: dereferencing pointer to incomplete type 'struct pci_bus'
struct pci_config_window *cfg = bus->sysdata;
^~
>> drivers/pci/host/pcie-tango.c:247:8: error: implicit declaration of function 'pci_generic_config_write' [-Werror=implicit-function-declaration]
ret = pci_generic_config_write(bus, devfn, where, size, val);
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-tango.c: At top level:
>> drivers/pci/host/pcie-tango.c:256:3: error: field name not in record or union initializer
.map_bus = pci_ecam_map_bus,
^
drivers/pci/host/pcie-tango.c:256:3: note: (near initialization for 'smp8759_ecam_ops.pci_ops')
drivers/pci/host/pcie-tango.c:257:3: error: field name not in record or union initializer
.read = smp8759_config_read,
^
drivers/pci/host/pcie-tango.c:257:3: note: (near initialization for 'smp8759_ecam_ops.pci_ops')
drivers/pci/host/pcie-tango.c:258:3: error: field name not in record or union initializer
.write = smp8759_config_write,
^
drivers/pci/host/pcie-tango.c:258:3: note: (near initialization for 'smp8759_ecam_ops.pci_ops')
drivers/pci/host/pcie-tango.c: In function 'tango_fixup_class':
>> drivers/pci/host/pcie-tango.c:329:5: error: dereferencing pointer to incomplete type 'struct pci_dev'
dev->class = PCI_CLASS_BRIDGE_PCI << 8;
^~
>> drivers/pci/host/pcie-tango.c:329:15: error: 'PCI_CLASS_BRIDGE_PCI' undeclared (first use in this function)
dev->class = PCI_CLASS_BRIDGE_PCI << 8;
^~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-tango.c: At top level:
>> drivers/pci/host/pcie-tango.c:320:22: error: expected declaration specifiers or '...' before numeric constant
#define VENDOR_SIGMA 0x1105
^
>> drivers/pci/host/pcie-tango.c:331:25: note: in expansion of macro 'VENDOR_SIGMA'
DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class);
^~~~~~~~~~~~
In file included from include/linux/of.h:22:0,
from include/linux/irqdomain.h:34,
from drivers/pci/host/pcie-tango.c:3:
>> include/linux/mod_devicetable.h:16:20: error: expected declaration specifiers or '...' before '(' token
#define PCI_ANY_ID (~0)
^
>> drivers/pci/host/pcie-tango.c:331:39: note: in expansion of macro 'PCI_ANY_ID'
DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class);
^~~~~~~~~~
>> drivers/pci/host/pcie-tango.c:331:51: error: expected declaration specifiers or '...' before 'tango_fixup_class'
DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class);
^~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-tango.c: In function 'tango_fixup_bar':
>> drivers/pci/host/pcie-tango.c:342:9: error: implicit declaration of function 'pci_write_config_dword' [-Werror=implicit-function-declaration]
pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, 0x80000000);
^~~~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-tango.c:342:37: error: 'PCI_BASE_ADDRESS_0' undeclared (first use in this function)
pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, 0x80000000);
^~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-tango.c: At top level:
>> drivers/pci/host/pcie-tango.c:320:22: error: expected declaration specifiers or '...' before numeric constant
#define VENDOR_SIGMA 0x1105
^
drivers/pci/host/pcie-tango.c:344:25: note: in expansion of macro 'VENDOR_SIGMA'
DECLARE_PCI_FIXUP_FINAL(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_bar);
^~~~~~~~~~~~
In file included from include/linux/of.h:22:0,
from include/linux/irqdomain.h:34,
from drivers/pci/host/pcie-tango.c:3:
>> include/linux/mod_devicetable.h:16:20: error: expected declaration specifiers or '...' before '(' token
#define PCI_ANY_ID (~0)
^
drivers/pci/host/pcie-tango.c:344:39: note: in expansion of macro 'PCI_ANY_ID'
DECLARE_PCI_FIXUP_FINAL(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_bar);
^~~~~~~~~~
--
In file included from drivers/pci//host/pcie-tango.c:4:0:
>> include/linux/pci-ecam.h:29:19: error: field 'pci_ops' has incomplete type
struct pci_ops pci_ops;
^~~~~~~
>> include/linux/pci-ecam.h:57:39: warning: 'struct pci_bus' declared inside parameter list will not be visible outside of this definition or declaration
void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
^~~~~~~
drivers/pci//host/pcie-tango.c:198:39: warning: 'struct pci_bus' declared inside parameter list will not be visible outside of this definition or declaration
static int smp8759_config_read(struct pci_bus *bus,
^~~~~~~
drivers/pci//host/pcie-tango.c: In function 'smp8759_config_read':
drivers/pci//host/pcie-tango.c:202:37: error: dereferencing pointer to incomplete type 'struct pci_bus'
struct pci_config_window *cfg = bus->sysdata;
^~
drivers/pci//host/pcie-tango.c:211:10: error: 'PCIBIOS_FUNC_NOT_SUPPORTED' undeclared (first use in this function)
return PCIBIOS_FUNC_NOT_SUPPORTED; /* Error seems appropriate */
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c:211:10: note: each undeclared identifier is reported only once for each function it appears in
drivers/pci//host/pcie-tango.c:219:15: error: 'PCI_BASE_ADDRESS_0' undeclared (first use in this function)
if (where == PCI_BASE_ADDRESS_0 && bus->number == 0) {
^~~~~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c:221:10: error: 'PCIBIOS_SUCCESSFUL' undeclared (first use in this function)
return PCIBIOS_SUCCESSFUL; /* Should we return error or success? */
^~~~~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c:233:8: error: implicit declaration of function 'pci_generic_config_read' [-Werror=implicit-function-declaration]
ret = pci_generic_config_read(bus, devfn, where, size, val);
^~~~~~~~~~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c: At top level:
drivers/pci//host/pcie-tango.c:239:40: warning: 'struct pci_bus' declared inside parameter list will not be visible outside of this definition or declaration
static int smp8759_config_write(struct pci_bus *bus,
^~~~~~~
drivers/pci//host/pcie-tango.c: In function 'smp8759_config_write':
drivers/pci//host/pcie-tango.c:243:37: error: dereferencing pointer to incomplete type 'struct pci_bus'
struct pci_config_window *cfg = bus->sysdata;
^~
drivers/pci//host/pcie-tango.c:247:8: error: implicit declaration of function 'pci_generic_config_write' [-Werror=implicit-function-declaration]
ret = pci_generic_config_write(bus, devfn, where, size, val);
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c: At top level:
drivers/pci//host/pcie-tango.c:256:3: error: field name not in record or union initializer
.map_bus = pci_ecam_map_bus,
^
drivers/pci//host/pcie-tango.c:256:3: note: (near initialization for 'smp8759_ecam_ops.pci_ops')
drivers/pci//host/pcie-tango.c:257:3: error: field name not in record or union initializer
.read = smp8759_config_read,
^
drivers/pci//host/pcie-tango.c:257:3: note: (near initialization for 'smp8759_ecam_ops.pci_ops')
drivers/pci//host/pcie-tango.c:258:3: error: field name not in record or union initializer
.write = smp8759_config_write,
^
drivers/pci//host/pcie-tango.c:258:3: note: (near initialization for 'smp8759_ecam_ops.pci_ops')
drivers/pci//host/pcie-tango.c: In function 'tango_fixup_class':
drivers/pci//host/pcie-tango.c:329:5: error: dereferencing pointer to incomplete type 'struct pci_dev'
dev->class = PCI_CLASS_BRIDGE_PCI << 8;
^~
drivers/pci//host/pcie-tango.c:329:15: error: 'PCI_CLASS_BRIDGE_PCI' undeclared (first use in this function)
dev->class = PCI_CLASS_BRIDGE_PCI << 8;
^~~~~~~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c: At top level:
drivers/pci//host/pcie-tango.c:320:22: error: expected declaration specifiers or '...' before numeric constant
#define VENDOR_SIGMA 0x1105
^
drivers/pci//host/pcie-tango.c:331:25: note: in expansion of macro 'VENDOR_SIGMA'
DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class);
^~~~~~~~~~~~
In file included from include/linux/of.h:22:0,
from include/linux/irqdomain.h:34,
from drivers/pci//host/pcie-tango.c:3:
>> include/linux/mod_devicetable.h:16:20: error: expected declaration specifiers or '...' before '(' token
#define PCI_ANY_ID (~0)
^
drivers/pci//host/pcie-tango.c:331:39: note: in expansion of macro 'PCI_ANY_ID'
DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class);
^~~~~~~~~~
drivers/pci//host/pcie-tango.c:331:51: error: expected declaration specifiers or '...' before 'tango_fixup_class'
DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class);
^~~~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c: In function 'tango_fixup_bar':
drivers/pci//host/pcie-tango.c:342:9: error: implicit declaration of function 'pci_write_config_dword' [-Werror=implicit-function-declaration]
pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, 0x80000000);
^~~~~~~~~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c:342:37: error: 'PCI_BASE_ADDRESS_0' undeclared (first use in this function)
pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, 0x80000000);
^~~~~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c: At top level:
drivers/pci//host/pcie-tango.c:320:22: error: expected declaration specifiers or '...' before numeric constant
#define VENDOR_SIGMA 0x1105
^
drivers/pci//host/pcie-tango.c:344:25: note: in expansion of macro 'VENDOR_SIGMA'
DECLARE_PCI_FIXUP_FINAL(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_bar);
^~~~~~~~~~~~
In file included from include/linux/of.h:22:0,
from include/linux/irqdomain.h:34,
from drivers/pci//host/pcie-tango.c:3:
>> include/linux/mod_devicetable.h:16:20: error: expected declaration specifiers or '...' before '(' token
#define PCI_ANY_ID (~0)
^
drivers/pci//host/pcie-tango.c:344:39: note: in expansion of macro 'PCI_ANY_ID'
DECLARE_PCI_FIXUP_FINAL(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_bar);
^~~~~~~~~~
drivers/pci//host/pcie-tango.c:344:51: error: expected declaration specifiers or '...' before 'tango_fixup_bar'
DECLARE_PCI_FIXUP_FINAL(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_bar);
^~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c:340:13: warning: 'tango_fixup_bar' defined but not used [-Wunused-function]
static void tango_fixup_bar(struct pci_dev *dev)
^~~~~~~~~~~~~~~
drivers/pci//host/pcie-tango.c:327:13: warning: 'tango_fixup_class' defined but not used [-Wunused-function]
static void tango_fixup_class(struct pci_dev *dev)
^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/pci_ops +29 include/linux/pci-ecam.h
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 23 * struct to hold pci ops and bus shift of the config window
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 24 * for a PCI controller.
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 25 */
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 26 struct pci_config_window;
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 27 struct pci_ecam_ops {
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 28 unsigned int bus_shift;
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 @29 struct pci_ops pci_ops;
5c3d14f7 include/linux/pci-ecam.h Jayachandran C 2016-06-10 30 int (*init)(struct pci_config_window *);
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 31 };
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 32
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 33 /*
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 34 * struct to hold the mappings of a config space window. This
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 35 * is expected to be used as sysdata for PCI controllers that
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 36 * use ECAM.
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 37 */
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 38 struct pci_config_window {
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 39 struct resource res;
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 40 struct resource busr;
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 41 void *priv;
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 42 struct pci_ecam_ops *ops;
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 43 union {
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 44 void __iomem *win; /* 64-bit single mapping */
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 45 void __iomem **winp; /* 32-bit per-bus mapping */
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 46 };
5c3d14f7 include/linux/pci-ecam.h Jayachandran C 2016-06-10 47 struct device *parent;/* ECAM res was from this dev */
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 48 };
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 49
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 50 /* create and free pci_config_window */
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 51 struct pci_config_window *pci_ecam_create(struct device *dev,
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 52 struct resource *cfgres, struct resource *busr,
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 53 struct pci_ecam_ops *ops);
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 54 void pci_ecam_free(struct pci_config_window *cfg);
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 55
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 56 /* map_bus when ->sysdata is an instance of pci_config_window */
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 @57 void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 58 int where);
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 59 /* default ECAM ops */
35ff9477 drivers/pci/ecam.h Jayachandran C 2016-05-10 60 extern struct pci_ecam_ops pci_generic_ecam_ops;
:::::: The code at line 29 was first introduced by commit
:::::: 35ff9477d880986441981010585399c1d7201fee PCI: Provide common functions for ECAM mapping
:::::: TO: Jayachandran C <jchandra@broadcom.com>
:::::: CC: Bjorn Helgaas <bhelgaas@google.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Wed, Mar 29, 2017 at 01:34:45PM +0200, Marc Gonzalez wrote: > This driver is used to work around HW bugs in the controller. > > Note: the controller does NOT support the following features. > > Legacy PCI interrupts > IO space > > Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> > --- > Documentation/devicetree/bindings/pci/tango-pcie.txt | 33 +++++++++ Acked-by: Rob Herring <robh@kernel.org> > drivers/pci/host/Kconfig | 7 ++ > drivers/pci/host/Makefile | 1 + > drivers/pci/host/pcie-tango.c | 150 +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 191 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/tango-pcie.txt b/Documentation/devicetree/bindings/pci/tango-pcie.txt new file mode 100644 index 000000000000..f8e150ec41d8 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/tango-pcie.txt @@ -0,0 +1,33 @@ +Sigma Designs Tango PCIe controller + +Required properties: + +- compatible: "sigma,smp8759-pcie" +- reg: address/size of PCI configuration space, and pcie_reg +- bus-range: defined by size of PCI configuration space +- device_type: "pci" +- #size-cells: <2> +- #address-cells: <3> +- #interrupt-cells: <1> +- ranges: translation from system to bus addresses +- interrupts: spec for misc interrupts and MSI +- msi-controller + +Example: + + pcie@2e000 { + compatible = "sigma,smp8759-pcie"; + reg = <0x50000000 SZ_64M>, <0x2e000 0x100>; + bus-range = <0 63>; + device_type = "pci"; + #size-cells = <2>; + #address-cells = <3>; + #interrupt-cells = <1>; + /* http://elinux.org/Device_Tree_Usage#PCI_Address_Translation */ + /* BUS_ADDRESS(3) CPU_PHYSICAL(1) SIZE(2) */ + ranges = <0x02000000 0x0 0x04000000 0x54000000 0x0 SZ_192M>; + interrupts = + <54 IRQ_TYPE_LEVEL_HIGH>, /* misc interrupts */ + <55 IRQ_TYPE_LEVEL_HIGH>; /* MSI */ + msi-controller; + }; diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index d7e7c0a827c3..8a622578a760 100644 --- a/drivers/pci/host/Kconfig +++ b/drivers/pci/host/Kconfig @@ -285,6 +285,13 @@ config PCIE_ROCKCHIP There is 1 internal PCIe port available to support GEN2 with 4 slots. +config PCIE_TANGO + tristate "Tango PCIe controller" + depends on ARCH_TANGO && PCI_MSI && OF + select PCI_HOST_COMMON + help + Say Y here to enable PCIe controller support on Tango SoC. + config VMD depends on PCI_MSI && X86_64 tristate "Intel Volume Management Device Driver" diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile index 084cb4983645..fc7ea90196f3 100644 --- a/drivers/pci/host/Makefile +++ b/drivers/pci/host/Makefile @@ -32,4 +32,5 @@ obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o +obj-$(CONFIG_PCIE_TANGO) += pcie-tango.o obj-$(CONFIG_VMD) += vmd.o diff --git a/drivers/pci/host/pcie-tango.c b/drivers/pci/host/pcie-tango.c index e88850983a1d..dbc2663486b5 100644 --- a/drivers/pci/host/pcie-tango.c +++ b/drivers/pci/host/pcie-tango.c @@ -192,3 +192,153 @@ static int tango_msi_probe(struct platform_device *pdev, struct tango_pcie *pcie return 0; } + +/*** HOST BRIDGE SUPPORT ***/ + +static int smp8759_config_read(struct pci_bus *bus, + unsigned int devfn, int where, int size, u32 *val) +{ + int ret; + struct pci_config_window *cfg = bus->sysdata; + struct tango_pcie *pcie = dev_get_drvdata(cfg->parent); + + /* + * QUIRK #1 + * Reads in configuration space outside devfn 0 return garbage. + */ + if (devfn != 0) { + *val = ~0; /* Is this required even if we return an error? */ + return PCIBIOS_FUNC_NOT_SUPPORTED; /* Error seems appropriate */ + } + + /* + * QUIRK #2 + * The root complex advertizes a fake BAR, which is used to filter + * bus-to-system requests. Hide it from Linux. + */ + if (where == PCI_BASE_ADDRESS_0 && bus->number == 0) { + *val = 0; /* 0 or ~0 to hide the BAR from Linux? */ + return PCIBIOS_SUCCESSFUL; /* Should we return error or success? */ + } + + /* + * QUIRK #3 + * Unfortunately, config and mem spaces are muxed. + * Linux does not support such a setting, since drivers are free + * to access mem space directly, at any time. + * Therefore, we can only PRAY that config and mem space accesses + * NEVER occur concurrently. + */ + writel_relaxed(1, pcie->mux); + ret = pci_generic_config_read(bus, devfn, where, size, val); + writel_relaxed(0, pcie->mux); + + return ret; +} + +static int smp8759_config_write(struct pci_bus *bus, + unsigned int devfn, int where, int size, u32 val) +{ + int ret; + struct pci_config_window *cfg = bus->sysdata; + struct tango_pcie *pcie = dev_get_drvdata(cfg->parent); + + writel_relaxed(1, pcie->mux); + ret = pci_generic_config_write(bus, devfn, where, size, val); + writel_relaxed(0, pcie->mux); + + return ret; +} + +static struct pci_ecam_ops smp8759_ecam_ops = { + .bus_shift = 20, + .pci_ops = { + .map_bus = pci_ecam_map_bus, + .read = smp8759_config_read, + .write = smp8759_config_write, + } +}; + +static const struct of_device_id tango_pcie_ids[] = { + { .compatible = "sigma,smp8759-pcie" }, + { /* sentinel */ }, +}; + +static void smp8759_init(struct tango_pcie *pcie, void __iomem *base) +{ + pcie->mux = base + 0x48; + pcie->msi_status = base + 0x80; + pcie->msi_mask = base + 0xa0; + pcie->msi_doorbell = 0xa0000000 + 0x2e07c; +} + +static int tango_pcie_probe(struct platform_device *pdev) +{ + int ret; + void __iomem *base; + struct resource *res; + struct tango_pcie *pcie; + struct device *dev = &pdev->dev; + + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); + if (!pcie) + return -ENOMEM; + + platform_set_drvdata(pdev, pcie); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(base)) + return PTR_ERR(base); + + if (of_device_is_compatible(dev->of_node, "sigma,smp8759-pcie")) + smp8759_init(pcie, base); + + ret = tango_msi_probe(pdev, pcie); + if (ret) + return ret; + + return pci_host_common_probe(pdev, &smp8759_ecam_ops); +} + +static int tango_pcie_remove(struct platform_device *pdev) +{ + return tango_msi_remove(pdev); +} + +static struct platform_driver tango_pcie_driver = { + .probe = tango_pcie_probe, + .remove = tango_pcie_remove, + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = tango_pcie_ids, + }, +}; + +module_platform_driver(tango_pcie_driver); + +#define VENDOR_SIGMA 0x1105 + +/* + * QUIRK #4 + * The root complex advertizes the wrong device class. + * Header Type 1 is for PCI-to-PCI bridges. + */ +static void tango_fixup_class(struct pci_dev *dev) +{ + dev->class = PCI_CLASS_BRIDGE_PCI << 8; +} +DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class); + +/* + * QUIRK #5 + * Only transfers within the root complex BAR are forwarded to the host. + * By default, the DMA framework expects that + * PCI address 0x8000_0000 maps to system address 0x8000_0000 + * which is where DRAM0 is mapped. + */ +static void tango_fixup_bar(struct pci_dev *dev) +{ + pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, 0x80000000); +} +DECLARE_PCI_FIXUP_FINAL(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_bar);
This driver is used to work around HW bugs in the controller. Note: the controller does NOT support the following features. Legacy PCI interrupts IO space Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> --- Documentation/devicetree/bindings/pci/tango-pcie.txt | 33 +++++++++ drivers/pci/host/Kconfig | 7 ++ drivers/pci/host/Makefile | 1 + drivers/pci/host/pcie-tango.c | 150 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 191 insertions(+)