Re: [PATCH v6] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller
On Fri, 30 Oct 2015 17:47:08 +0530 Bharat Kumar Gogada wrote: > Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP. > > Signed-off-by: Bharat Kumar Gogada > Signed-off-by: Ravi Kiran Gummaluri > --- > Changes for v6: > Removed repetitive code for msi handlers. > Corrected typo mistakes in device tree documentation. > --- > .../devicetree/bindings/pci/xilinx-nwl-pcie.txt| 68 ++ > drivers/pci/host/Kconfig |9 + > drivers/pci/host/Makefile |1 + > drivers/pci/host/pcie-xilinx-nwl.c | 1025 > > 4 files changed, 1103 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt > create mode 100644 drivers/pci/host/pcie-xilinx-nwl.c > [...] > diff --git a/drivers/pci/host/pcie-xilinx-nwl.c > b/drivers/pci/host/pcie-xilinx-nwl.c > new file mode 100644 > index 000..bee1bee > --- /dev/null > +++ b/drivers/pci/host/pcie-xilinx-nwl.c > @@ -0,0 +1,1025 @@ > +/* > + * PCIe host controller driver for NWL PCIe Bridge > + * Based on pcie-xilinx.c, pci-tegra.c > + * > + * (C) Copyright 2014 - 2015, Xilinx, 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include [...] > + > +struct nwl_msi { /* MSI information */ > + struct msi_controller msi_chip; /* MSI domain */ NAK. I've mentioned it clearly when I first reviewed this patch: There should be no new msi_controller structure added, and definitely none for arm64. So please get rid of this cruft. Select CONFIG_PCI_MSI_IRQ_DOMAIN, and everything will be taken care of for you. No populating of bus->msi, just have your own domain field for the MSI domain. > + DECLARE_BITMAP(used, INT_PCI_MSI_NR); /* used: Declare Bitmap > + for MSI */ > + struct irq_domain *dev_domain; > + unsigned long pages;/* pages: MSI pages */ > + struct mutex lock; /* protect used variable */ > + int irq_msi0; > + int irq_msi1; > +}; [...] > +#ifdef CONFIG_PCI_MSI > +static struct irq_chip nwl_msi_irq_chip = { > + .name = "nwl_pcie:msi", > + .irq_enable = unmask_msi_irq, > + .irq_disable = mask_msi_irq, > + .irq_mask = mask_msi_irq, > + .irq_unmask = unmask_msi_irq, > + > +}; > + > +static struct msi_domain_info nwl_msi_domain_info = { > + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS), So you cannot support MSI-X? Why? The spec seems to say otherwise (it also says you could support multi-MSI, but I'm not sure it is worth the hassle). > + .chip = &nwl_msi_irq_chip, > +}; > +#endif > + > +static void nwl_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) > +{ > + struct nwl_pcie *pcie = irq_data_get_irq_chip_data(data); > + struct nwl_msi *msi = &pcie->msi; > + > + msg->address_lo = virt_to_phys((void *)msi->pages); > + msg->address_hi = 0; Do you know for sure that this page will never be above 4GB? Given that this IP can be synthesized, I seriously doubt this is the case. Please properly populate both fields. > + msg->data = data->hwirq; > +} > + > +static int nwl_msi_set_affinity(struct irq_data *irq_data, > + const struct cpumask *mask, bool force) > +{ > + return -EINVAL; > +} > + > +static struct irq_chip nwl_irq_chip = { > + .name = "Xilinx MSI", > + .irq_compose_msi_msg = nwl_compose_msi_msg, > + .irq_set_affinity = nwl_msi_set_affinity, > +}; > + > +static int nwl_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, > + unsigned int nr_irqs, void *args) > +{ > + struct nwl_pcie *pcie = domain->host_data; > + struct nwl_msi *msi = &pcie->msi; > + unsigned long bit; > + > + mutex_lock(&msi->lock); > + bit = find_first_zero_bit(msi->used, INT_PCI_MSI_NR); > + if (bit < INT_PCI_MSI_NR) > + set_bit(bit, msi->used); > + else > + bit = -ENOSPC; > + > + mutex_unlock(&msi->lock); > + irq_domain_set_info(domain, virq, bit, &nwl_irq_chip, > + domain->host_data, handle_simple_irq, > + NULL, NULL); I'm sure irq_domain_set_info will enjoy being passed -ENOSPC as a hwirq. > + return 0; And let's ignore the error case, they are usually overrated. > +} I've stopped here (after all, I'm still on holiday). Thanks, M. -- Without deviation from the norm, progress is not possible. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in th
[PATCH v6] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller
Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP. Signed-off-by: Bharat Kumar Gogada Signed-off-by: Ravi Kiran Gummaluri --- Changes for v6: Removed repetitive code for msi handlers. Corrected typo mistakes in device tree documentation. --- .../devicetree/bindings/pci/xilinx-nwl-pcie.txt| 68 ++ drivers/pci/host/Kconfig |9 + drivers/pci/host/Makefile |1 + drivers/pci/host/pcie-xilinx-nwl.c | 1025 4 files changed, 1103 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt create mode 100644 drivers/pci/host/pcie-xilinx-nwl.c diff --git a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt new file mode 100644 index 000..3b2a9ad --- /dev/null +++ b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt @@ -0,0 +1,68 @@ +* Xilinx NWL PCIe Root Port Bridge DT description + +Required properties: +- compatible: Should contain "xlnx,nwl-pcie-2.11" +- #address-cells: Address representation for root ports, set to <3> +- #size-cells: Size representation for root ports, set to <2> +- #interrupt-cells: specifies the number of cells needed to encode an + interrupt source. The value must be 1. +- reg: Should contain Bridge, PCIe Controller registers location, + configuration sapce, and length +- reg-names: Must include the following entries: + "breg": bridge registers + "pcireg": PCIe controller registers + "cfg": configuration space region +- device_type: must be "pci" +- interrupts: Should contain NWL PCIe interrupt +- interrupt-names: Must include the following entries: + "msi1, msi0": interrupt asserted when msi is received + "intx": interrupt that is asserted when an legacy interrupt is received + "misc": interrupt asserted when miscellaneous is received +- interrupt-map-mask and interrupt-map: standard PCI properties to define the + mapping of the PCI interface to interrupt numbers. +- ranges: ranges for the PCI memory regions (I/O space region is not + supported by hardware) + Please refer to the standard PCI bus binding document for a more + detailed explanation +- msi-controller: indicates that this is MSI controller node +- msi-parent: MSI parent of the root complex itself +- legacy-interrupt-controller: Interrupt controller device node for Legacy interrupts + - interrupt-controller: identifies the node as an interrupt controller + - #interrupt-cells: should be set to 1 + - #address-cells: specifies the number of cells needed to encode an + address. The value must be 0. + + +Example: + + +nwl_pcie: pcie@fd0e { + #address-cells = <3>; + #size-cells = <2>; + compatible = "xlnx,nwl-pcie-2.11"; + #interrupt-cells = <1>; + msi-controller; + device_type = "pci"; + interrupt-parent = <&gic>; + interrupts = <0 114 4>, <0 115 4>, <0 116 4>, <0 117 4>, <0 118 4>; + interrupt-names = "msi0", "msi1", "intx", "dummy", "misc"; + interrupt-map-mask = <0x0 0x0 0x0 0x7>; + interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc 0x1>, + <0x0 0x0 0x0 0x2 &pcie_intc 0x2>, + <0x0 0x0 0x0 0x3 &pcie_intc 0x3>, + <0x0 0x0 0x0 0x4 &pcie_intc 0x4>; + + msi-parent = <&nwl_pcie>; + reg = <0x0 0xfd0e 0x1000>, + <0x0 0xfd48 0x1000>, + <0x0 0xe000 0x100>; + reg-names = "breg", "pcireg", "cfg"; + ranges = <0x0200 0x 0xe100 0x 0xe100 0 0x0f00>; + + pcie_intc: legacy-interrupt-controller { + interrupt-controller; + #address-cells = <0>; + #interrupt-cells = <1>; + }; + +}; diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index c132bdd..33bbddb 100644 --- a/drivers/pci/host/Kconfig +++ b/drivers/pci/host/Kconfig @@ -15,6 +15,15 @@ config PCI_MVEBU depends on ARCH_MVEBU || ARCH_DOVE depends on OF +config PCIE_XILINX_NWL + bool "NWL PCIe Core" + depends on ARCH_ZYNQMP + help +Say 'Y' here if you want kernel to support for Xilinx +NWL PCIe controller. The controller can act as Root Port +or End Point. The current option selection will only +support root port enabling. + config PCIE_DW bool diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile index 140d66f..6a56df7 100644 --- a/drivers/pci/host/Makefile +++ b/drivers/pci/host/Makefile @@ -10,6 +10,7 @@ obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o +obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o obj-$(CONFIG_PCI_XGENE