RE: [PATCH v12] [PATCH] PCI: Xilinx-NWL-PCIe: Adding support for Xilinx NWL PCIe Host Controller
From: Bharat Kumar Gogada
Date: Mon Mar 14 2016 - 10:52:20 EST
Thanks Bjorn.
Sorry for troubling you, I have built it it did not break anything, it is working fine.
Thanks
Bharat
> On Sun, Mar 06, 2016 at 10:02:14PM +0530, Bharat Kumar Gogada wrote:
> > Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP.
> >
> > Reviewed-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> > Acked-by: Rob Herring <robh@xxxxxxxxxx>
> > Signed-off-by: Bharat Kumar Gogada <bharatku@xxxxxxxxxx>
> > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xxxxxxxxxx>
> > ---
> > Changes for v12:
> > -> Removed nwl_setup_sspl function, it will be added after more
> > testing.
> > -> Broke nwl_pcie_link_up into nwl_pcie_link_up, nwl_phy_link_up
> > functions.
> > -> Using generic functions for configuration read and write.
> > -> Removed unneccessary comments.
> > -> Removed unneccessary new lines.
> > -> Added #define for number of legacy interrupts.
> > -> Changed MSI interrupt handlers registering sequence.
> > ---
> > .../devicetree/bindings/pci/xilinx-nwl-pcie.txt | 68 ++
> > drivers/pci/host/Kconfig | 10 +
> > drivers/pci/host/Makefile | 1 +
> > drivers/pci/host/pcie-xilinx-nwl.c | 912 +++++++++++++++++++++
> > 4 files changed, 991 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> > create mode 100644 drivers/pci/host/pcie-xilinx-nwl.c
>
> I applied this to pci/host-xilinx-nwl for v4.6.
>
> I cleaned up a bunch of stuff: whitespace, spelling, unused definitions, etc.,
> and also some minor code cleanups. I can't build it myself, so please check
> and make sure I didn't break anything.
>
> You can browse or check out the branch from here:
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-
> xilinx-nwl
>
> Here's the diff showing what I changed relative to the patch you
> posted:
>
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> index 90e8f32..337fc97 100644
> --- a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> @@ -7,7 +7,7 @@ Required properties:
> - #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
> + configuration space, and length
> - reg-names: Must include the following entries:
> "breg": bridge registers
> "pcireg": PCIe controller registers
> @@ -15,8 +15,8 @@ Required properties:
> - 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
> + "msi1, msi0": interrupt asserted when MSI is received
> + "intx": interrupt asserted when a 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.
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index
> 466a601..c39989a 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -21,7 +21,7 @@ config PCIE_XILINX_NWL
> depends on ARCH_ZYNQMP
> select PCI_MSI_IRQ_DOMAIN if PCI_MSI
> help
> - Say 'Y' here if you want kernel to support for Xilinx
> + Say 'Y' here if you want kernel 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.
> diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-
> nwl.c
> index e5774dc..7a2dc2e 100644
> --- a/drivers/pci/host/pcie-xilinx-nwl.c
> +++ b/drivers/pci/host/pcie-xilinx-nwl.c
> @@ -27,26 +27,15 @@
>
> /* Bridge core config registers */
> #define BRCFG_PCIE_RX0 0x00000000
> -#define BRCFG_PCIE_RX1 0x00000004
> -#define BRCFG_AXI_MASTER 0x00000008
> -#define BRCFG_PCIE_TX 0x0000000C
> #define BRCFG_INTERRUPT 0x00000010
> -#define BRCFG_RAM_DISABLE0 0x00000014
> -#define BRCFG_RAM_DISABLE1 0x00000018
> -#define BRCFG_PCIE_RELAXED_ORDER 0x0000001C
> #define BRCFG_PCIE_RX_MSG_FILTER 0x00000020
>
> -/* Attribute registers */
> -#define NWL_ATTRIB_100 0x00000190
> -
> /* Egress - Bridge translation registers */
> #define E_BREG_CAPABILITIES 0x00000200
> -#define E_BREG_STATUS 0x00000204
> #define E_BREG_CONTROL 0x00000208
> #define E_BREG_BASE_LO 0x00000210
> #define E_BREG_BASE_HI 0x00000214
> #define E_ECAM_CAPABILITIES 0x00000220
> -#define E_ECAM_STATUS 0x00000224
> #define E_ECAM_CONTROL 0x00000228
> #define E_ECAM_BASE_LO 0x00000230
> #define E_ECAM_BASE_HI 0x00000234
> @@ -68,11 +57,6 @@
> #define MSGF_MSI_STATUS_HI 0x00000444
> #define MSGF_MSI_MASK_LO 0x00000448
> #define MSGF_MSI_MASK_HI 0x0000044C
> -#define MSGF_RX_FIFO_POP 0x00000484
> -#define MSGF_RX_FIFO_TYPE 0x00000488
> -#define MSGF_RX_FIFO_ADDRLO 0x00000490
> -#define MSGF_RX_FIFO_ADDRHI 0x00000494
> -#define MSGF_RX_FIFO_DATA 0x00000498
>
> /* Msg filter mask bits */
> #define CFG_ENABLE_PM_MSG_FWD BIT(1)
> @@ -116,10 +100,6 @@
> MSGF_MISC_SR_PCIE_CORE | \
> MSGF_MISC_SR_PCIE_CORE_ERR)
>
> -/* Message rx fifo type mask bits */
> -#define MSGF_RX_FIFO_TYPE_MSI (1)
> -#define MSGF_RX_FIFO_TYPE_TYPE GENMASK(1, 0)
> -
> /* Legacy interrupt status mask bits */
> #define MSGF_LEG_SR_INTA BIT(0)
> #define MSGF_LEG_SR_INTB BIT(1)
> @@ -144,30 +124,27 @@
>
> /* E_ECAM status mask bits */
> #define E_ECAM_PRESENT BIT(0)
> -#define E_ECAM_SR_WR_PEND BIT(16)
> -#define E_ECAM_SR_RD_PEND BIT(0)
> -#define E_ECAM_SR_MASKALL (E_ECAM_SR_WR_PEND |
> E_ECAM_SR_RD_PEND)
> #define E_ECAM_CR_ENABLE BIT(0)
> #define E_ECAM_SIZE_LOC GENMASK(20, 16)
> #define E_ECAM_SIZE_SHIFT 16
> #define ECAM_BUS_LOC_SHIFT 20
> #define ECAM_DEV_LOC_SHIFT 12
> #define NWL_ECAM_VALUE_DEFAULT 12
> -#define NWL_ECAM_SIZE_MIN 4096
>
> -#define ATTR_UPSTREAM_FACING BIT(6)
> #define CFG_DMA_REG_BAR GENMASK(2, 0)
>
> #define INT_PCI_MSI_NR (2 * 32)
> -
> #define INTX_NUM 4
> +
> /* Readin the PS_LINKUP */
> #define PS_LINKUP_OFFSET 0x00000238
> #define PCIE_PHY_LINKUP_BIT BIT(0)
> #define PHY_RDY_LINKUP_BIT BIT(1)
> -#define PCIE_USER_LINKUP 0
> -#define PHY_RDY_LINKUP 1
> -#define LINKUP_ITER_CHECK 5
> +
> +/* Parameters for the waiting for link up routine */
> +#define LINK_WAIT_MAX_RETRIES 10
> +#define LINK_WAIT_USLEEP_MIN 90000
> +#define LINK_WAIT_USLEEP_MAX 100000
>
> struct nwl_msi { /* MSI information */
> struct irq_domain *msi_domain;
> @@ -194,7 +171,6 @@ struct nwl_pcie {
> u32 ecam_value;
> u8 last_busno;
> u8 root_busno;
> - u8 link_up;
> struct nwl_msi msi;
> struct irq_domain *legacy_irq_domain;
> };
> @@ -223,11 +199,26 @@ static bool nwl_phy_link_up(struct nwl_pcie *pcie)
> return false;
> }
>
> +static int nwl_wait_for_link(struct nwl_pcie *pcie) {
> + int retries;
> +
> + /* check if the link is up or not */
> + for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
> + if (nwl_phy_link_up(pcie))
> + return 0;
> + usleep_range(LINK_WAIT_USLEEP_MIN,
> LINK_WAIT_USLEEP_MAX);
> + }
> +
> + dev_err(pcie->dev, "PHY link never came up\n");
> + return -ETIMEDOUT;
> +}
> +
> static bool nwl_pcie_valid_device(struct pci_bus *bus, unsigned int devfn) {
> struct nwl_pcie *pcie = bus->sysdata;
>
> - /* Check link,before accessing downstream ports */
> + /* Check link before accessing downstream ports */
> if (bus->number != pcie->root_busno) {
> if (!nwl_pcie_link_up(pcie))
> return false;
> @@ -250,9 +241,8 @@ static bool nwl_pcie_valid_device(struct pci_bus
> *bus, unsigned int devfn)
> * Return: Base address of the configuration space needed to be
> * accessed.
> */
> -static void __iomem *nwl_pcie_map_bus(struct pci_bus *bus,
> - unsigned int devfn,
> - int where)
> +static void __iomem *nwl_pcie_map_bus(struct pci_bus *bus, unsigned int
> devfn,
> + int where)
> {
> struct nwl_pcie *pcie = bus->sysdata;
> int relbus;
> @@ -323,8 +313,7 @@ static void nwl_pcie_leg_handler(struct irq_desc
> *desc)
>
> while ((status = nwl_bridge_readl(pcie, MSGF_LEG_STATUS) &
> MSGF_LEG_SR_MASKALL) != 0) {
> - for_each_set_bit(bit, &status, 4) {
> -
> + for_each_set_bit(bit, &status, INTX_NUM) {
> virq = irq_find_mapping(pcie->legacy_irq_domain,
> bit + 1);
> if (virq)
> @@ -357,29 +346,25 @@ static void nwl_pcie_handle_msi_irq(struct
> nwl_pcie *pcie, u32 status_reg) static void
> nwl_pcie_msi_handler_high(struct irq_desc *desc) {
> struct irq_chip *chip = irq_desc_get_chip(desc);
> - struct nwl_pcie *pcie;
> + struct nwl_pcie *pcie = irq_desc_get_handler_data(desc);
>
> chained_irq_enter(chip, desc);
> - pcie = irq_desc_get_handler_data(desc);
> nwl_pcie_handle_msi_irq(pcie, MSGF_MSI_STATUS_HI);
> -
> chained_irq_exit(chip, desc);
> }
>
> static void nwl_pcie_msi_handler_low(struct irq_desc *desc) {
> struct irq_chip *chip = irq_desc_get_chip(desc);
> - struct nwl_pcie *pcie;
> + struct nwl_pcie *pcie = irq_desc_get_handler_data(desc);
>
> chained_irq_enter(chip, desc);
> - pcie = irq_desc_get_handler_data(desc);
> nwl_pcie_handle_msi_irq(pcie, MSGF_MSI_STATUS_LO);
> -
> chained_irq_exit(chip, desc);
> }
>
> static int nwl_legacy_map(struct irq_domain *domain, unsigned int irq,
> - irq_hw_number_t hwirq)
> + irq_hw_number_t hwirq)
> {
> irq_set_chip_and_handler(irq, &dummy_irq_chip,
> handle_simple_irq);
> irq_set_chip_data(irq, domain->host_data); @@ -441,16 +426,13
> @@ static int nwl_irq_domain_alloc(struct irq_domain *domain, unsigned int
> virq,
> mutex_lock(&msi->lock);
> bit = bitmap_find_next_zero_area(msi->bitmap, INT_PCI_MSI_NR,
> 0,
> nr_irqs, 0);
> - if (bit < INT_PCI_MSI_NR)
> - bitmap_set(msi->bitmap, bit, nr_irqs);
> - else
> - bit = -ENOSPC;
> -
> - if (bit < 0) {
> + if (bit >= INT_PCI_MSI_NR) {
> mutex_unlock(&msi->lock);
> - return bit;
> + return -ENOSPC;
> }
>
> + bitmap_set(msi->bitmap, bit, nr_irqs);
> +
> for (i = 0; i < nr_irqs; i++) {
> irq_domain_set_info(domain, virq + i, bit + i, &nwl_irq_chip,
> domain->host_data, handle_simple_irq, @@
> -518,14 +500,14 @@ static int nwl_pcie_init_msi_irq_domain(struct nwl_pcie
> *pcie)
> struct nwl_msi *msi = &pcie->msi;
>
> msi->dev_domain = irq_domain_add_linear(NULL,
> INT_PCI_MSI_NR,
> - &dev_msi_domain_ops, pcie);
> + &dev_msi_domain_ops,
> pcie);
> if (!msi->dev_domain) {
> dev_err(pcie->dev, "failed to create dev IRQ domain\n");
> return -ENOMEM;
> }
> msi->msi_domain = pci_msi_create_irq_domain(fwnode,
> -
> &nwl_msi_domain_info,
> - msi->dev_domain);
> + &nwl_msi_domain_info,
> + msi->dev_domain);
> if (!msi->msi_domain) {
> dev_err(pcie->dev, "failed to create msi IRQ domain\n");
> irq_domain_remove(msi->dev_domain);
> @@ -557,7 +539,6 @@ static int nwl_pcie_init_irq_domain(struct nwl_pcie
> *pcie)
> }
>
> nwl_pcie_init_msi_irq_domain(pcie);
> -
> return 0;
> }
>
> @@ -582,9 +563,10 @@ static int nwl_pcie_enable_msi(struct nwl_pcie
> *pcie, struct pci_bus *bus)
> ret = -EINVAL;
> goto err;
> }
> - /* Register msi handler */
> +
> irq_set_chained_handler_and_data(msi->irq_msi1,
> nwl_pcie_msi_handler_high, pcie);
> +
> /* Get msi_0 IRQ number */
> msi->irq_msi0 = platform_get_irq_byname(pdev, "msi0");
> if (msi->irq_msi0 < 0) {
> @@ -593,9 +575,9 @@ static int nwl_pcie_enable_msi(struct nwl_pcie *pcie,
> struct pci_bus *bus)
> goto err;
> }
>
> - /* Register msi handler */
> irq_set_chained_handler_and_data(msi->irq_msi0,
> nwl_pcie_msi_handler_low, pcie);
> +
> /* Check for msii_present bit */
> ret = nwl_bridge_readl(pcie, I_MSII_CAPABILITIES) & MSII_PRESENT;
> if (!ret) {
> @@ -618,7 +600,7 @@ static int nwl_pcie_enable_msi(struct nwl_pcie *pcie,
> struct pci_bus *bus)
> nwl_bridge_writel(pcie, upper_32_bits(base), I_MSII_BASE_HI);
>
> /*
> - * For high range msi interrupts: disable, clear any pending,
> + * For high range MSI interrupts: disable, clear any pending,
> * and enable
> */
> nwl_bridge_writel(pcie, (u32)~MSGF_MSI_SR_HI_MASK,
> MSGF_MSI_MASK_HI); @@ -629,7 +611,7 @@ static int
> nwl_pcie_enable_msi(struct nwl_pcie *pcie, struct pci_bus *bus)
> nwl_bridge_writel(pcie, MSGF_MSI_SR_HI_MASK,
> MSGF_MSI_MASK_HI);
>
> /*
> - * For low range msi interrupts: disable, clear any pending,
> + * For low range MSI interrupts: disable, clear any pending,
> * and enable
> */
> nwl_bridge_writel(pcie, (u32)~MSGF_MSI_SR_LO_MASK,
> MSGF_MSI_MASK_LO); @@ -651,15 +633,13 @@ static int
> nwl_pcie_bridge_init(struct nwl_pcie *pcie)
> struct platform_device *pdev = to_platform_device(pcie->dev);
> u32 breg_val, ecam_val, first_busno = 0;
> int err;
> - int check_link_up = 0;
> - bool link_up;
>
> - /* Check for BREG present bit */
> breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) &
> BREG_PRESENT;
> if (!breg_val) {
> dev_err(pcie->dev, "BREG is not present\n");
> return breg_val;
> }
> +
> /* Write bridge_off to breg base */
> nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_breg_base),
> E_BREG_BASE_LO);
> @@ -680,15 +660,10 @@ static int nwl_pcie_bridge_init(struct nwl_pcie
> *pcie)
> /* Enable msg filtering details */
> nwl_bridge_writel(pcie, CFG_ENABLE_MSG_FILTER_MASK,
> BRCFG_PCIE_RX_MSG_FILTER);
> - do {
> - link_up = nwl_phy_link_up(pcie);
> - if (!link_up) {
> - check_link_up++;
> - if (check_link_up > LINKUP_ITER_CHECK)
> - return -ENODEV;
> - msleep(1000);
> - }
> - } while (!link_up);
> +
> + err = nwl_wait_for_link(pcie);
> + if (err)
> + return err;
>
> ecam_val = nwl_bridge_readl(pcie, E_ECAM_CAPABILITIES) &
> E_ECAM_PRESENT;
> if (!ecam_val) {
> @@ -718,8 +693,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
> ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
> writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
>
> - pcie->link_up = nwl_pcie_link_up(pcie);
> - if (pcie->link_up)
> + if (nwl_pcie_link_up(pcie))
> dev_info(pcie->dev, "Link is UP\n");
> else
> dev_info(pcie->dev, "Link is DOWN\n"); @@ -731,7 +705,7
> @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
> pcie->irq_misc);
> return -EINVAL;
> }
> - /* Register misc handler */
> +
> err = devm_request_irq(pcie->dev, pcie->irq_misc,
> nwl_pcie_misc_handler, IRQF_SHARED,
> "nwl_pcie:misc", pcie);
> @@ -770,7 +744,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
> }
>
> static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> - struct platform_device *pdev)
> + struct platform_device *pdev)
> {
> struct device_node *node = pcie->dev->of_node;
> struct resource *res;
> @@ -809,7 +783,6 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> return -EINVAL;
> }
>
> - /* Register intx handler */
> irq_set_chained_handler_and_data(pcie->irq_intx,
> nwl_pcie_leg_handler, pcie);
>
> @@ -835,16 +808,15 @@ static int nwl_pcie_probe(struct platform_device
> *pdev)
> if (!pcie)
> return -ENOMEM;
>
> - pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
> -
> pcie->dev = &pdev->dev;
> + pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
>
> err = nwl_pcie_parse_dt(pcie, pdev);
> if (err) {
> dev_err(pcie->dev, "Parsing DT failed\n");
> return err;
> }
> - /* Bridge initialization */
> +
> err = nwl_pcie_bridge_init(pcie);
> if (err) {
> dev_err(pcie->dev, "HW Initalization failed\n"); @@ -868,7
> +840,6 @@ static int nwl_pcie_probe(struct platform_device *pdev)
> if (!bus)
> return -ENOMEM;
>
> - /* Enable MSI */
> if (IS_ENABLED(CONFIG_PCI_MSI)) {
> err = nwl_pcie_enable_msi(pcie, bus);
> if (err < 0) {
> @@ -883,7 +854,6 @@ static int nwl_pcie_probe(struct platform_device
> *pdev)
> pcie_bus_configure_settings(child);
> pci_bus_add_devices(bus);
> platform_set_drvdata(pdev, pcie);
> -
> return 0;
> }
>
> @@ -893,7 +863,6 @@ static int nwl_pcie_remove(struct platform_device
> *pdev)
>
> nwl_pcie_free_irq_domain(pcie);
> platform_set_drvdata(pdev, NULL);
> -
> return 0;
> }
>