Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.
From: Stefan Roese <sr@denx.de>
Date: 2009-12-23 08:18:45
On Wednesday 23 December 2009 08:52:23 tmarri@amcc.com wrote:
From: Tirumala Marri <redacted>
Please find some mostly nitpicking comments below. BTW: Did you already test this on other 4xx platforms, like 440SPe or 405EX? What are your plans here?
quoted hunk ↗ jump to hunk
Signed-off-by: Tirumala Marri <redacted> --- Kernel version: 2.6.33-rc1 Testing: When 460SX configured as root as a end point E1000(Intell Ethernet card) was plugged into the one of the PCI-E ports. I was able to run the traffic with MSI interrupts. --- arch/powerpc/boot/dts/redwood.dts | 15 ++ arch/powerpc/configs/44x/redwood_defconfig | 5 +- arch/powerpc/platforms/44x/Kconfig | 1 + arch/powerpc/sysdev/Kconfig | 7 + arch/powerpc/sysdev/Makefile | 1 + arch/powerpc/sysdev/ppc4xx_msi.c | 342 ++++++++++++++++++++++++++++ arch/powerpc/sysdev/ppc4xx_msi.h | 39 ++++ 7 files changed, 408 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.c create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.hdiff --git a/arch/powerpc/boot/dts/redwood.dts b/arch/powerpc/boot/dts/redwood.dts index 81636c0..412d5f9 100644 --- a/arch/powerpc/boot/dts/redwood.dts +++ b/arch/powerpc/boot/dts/redwood.dts@@ -357,6 +357,21 @@ 0x0 0x0 0x0 0x3 &UIC3 0xa 0x4 /* swizzled int C */ 0x0 0x0 0x0 0x4 &UIC3 0xb 0x4 /* swizzled int D */>; }; + MSI: ppc4xx-msi@400300000 { + compatible = "amcc,ppc4xx-msi", "ppc4xx-msi";
Better use something like this: compatible = "amcc,ppc4xx-msi-ppc460sx", "amcc,ppc4xx-msi"; This way you could check for 460SX specials in the driver if needed.
quoted hunk ↗ jump to hunk
+ reg = < 0x4 0x00300000 0x100 + 0x4 0x00300000 0x100>; + sdr-base = <0x3B0>; + interrupts =<0 1 2 3>; + interrupt-parent = <&MSI>; + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &UIC0 0xC 1 + 1 &UIC0 0x0D 1 + 2 &UIC0 0x0E 1 + 3 &UIC0 0x0F 1>; + }; };diff --git a/arch/powerpc/configs/44x/redwood_defconfig b/arch/powerpc/configs/44x/redwood_defconfig index ed31d4f..5d16c88 100644 --- a/arch/powerpc/configs/44x/redwood_defconfig +++ b/arch/powerpc/configs/44x/redwood_defconfig@@ -158,6 +158,7 @@ CONFIG_DEFAULT_AS=y CONFIG_DEFAULT_IOSCHED="anticipatory" # CONFIG_FREEZER is not set CONFIG_PPC4xx_PCI_EXPRESS=y +CONFIG_PPC_MSI_BITMAP=y # # Platform support@@ -264,7 +265,7 @@ CONFIG_PCIEPORTBUS=y CONFIG_PCIEAER=y # CONFIG_PCIEASPM is not set CONFIG_ARCH_SUPPORTS_MSI=y -# CONFIG_PCI_MSI is not set +CONFIG_PCI_MSI=y # CONFIG_PCI_LEGACY is not set # CONFIG_PCI_DEBUG is not set # CONFIG_PCI_STUB is not set@@ -1062,7 +1063,7 @@ CONFIG_PRINT_STACK_DEPTH=64 # CONFIG_DEBUG_PAGEALLOC is not set # CONFIG_CODE_PATCHING_SELFTEST is not set # CONFIG_FTR_FIXUP_SELFTEST is not set -# CONFIG_MSI_BITMAP_SELFTEST is not set +CONFIG_MSI_BITMAP_SELFTEST=y # CONFIG_XMON is not set # CONFIG_IRQSTACKS is not set # CONFIG_VIRQ_DEBUG is not setdiff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig index 7486bff..9c3b8ca 100644 --- a/arch/powerpc/platforms/44x/Kconfig +++ b/arch/powerpc/platforms/44x/Kconfig@@ -126,6 +126,7 @@ config REDWOOD select 460SX select PCI select PPC4xx_PCI_EXPRESS + select PPC4xx_MSI help This option enables support for the AMCC PPC460SX Redwood board.diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig index 3965828..c8f1486 100644 --- a/arch/powerpc/sysdev/Kconfig +++ b/arch/powerpc/sysdev/Kconfig@@ -7,8 +7,15 @@ config PPC4xx_PCI_EXPRESS depends on PCI && 4xx default n +config PPC4xx_MSI + bool + depends on PCI_MSI + depends on PCI && 4xx + default n + config PPC_MSI_BITMAP bool depends on PCI_MSI default y if MPIC default y if FSL_PCI + default y if PPC4xx_MSIdiff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile index 5642924..4c67d2d 100644 --- a/arch/powerpc/sysdev/Makefile +++ b/arch/powerpc/sysdev/Makefile@@ -36,6 +36,7 @@ obj-$(CONFIG_PPC_I8259) += i8259.o obj-$(CONFIG_IPIC) += ipic.o obj-$(CONFIG_4xx) += uic.o obj-$(CONFIG_4xx_SOC) += ppc4xx_soc.o +obj-$(CONFIG_PPC4xx_MSI) += ppc4xx_msi.o obj-$(CONFIG_XILINX_VIRTEX) += xilinx_intc.o obj-$(CONFIG_XILINX_PCI) += xilinx_pci.o obj-$(CONFIG_OF_RTC) += of_rtc.odiff --git a/arch/powerpc/sysdev/ppc4xx_msi.c b/arch/powerpc/sysdev/ppc4xx_msi.c new file mode 100644 index 0000000..3c2ef32 --- /dev/null +++ b/arch/powerpc/sysdev/ppc4xx_msi.c@@ -0,0 +1,342 @@ +/* + * Copyright (C) 2009 Applied Micro Circuits corporation. + * + * Author: Feng Kan <fkan@amcc.com> + * Tirumala Marri <tmarri@amcc.com> + * 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; version 2 of the + * License. + */ +#include <linux/irq.h> +#include <linux/bootmem.h> +#include <linux/pci.h> +#include <linux/msi.h> +#include <linux/of_platform.h> +#include <linux/interrupt.h> +#include <linux/device.h> +#include <asm/prom.h> +#include <asm/hw_irq.h> +#include <asm/ppc-pci.h> +#include <asm/dcr.h> +#include <asm/dcr-regs.h> +#include "ppc4xx_msi.h" + +
Nitpicking: Remove one empty line here.
+static struct ppc4xx_msi *ppc4xx_msi;
+
+struct ppc4xx_msi_feature {
+ u32 ppc4xx_pic_ip;
+ u32 msiir_offset;
+};
+
+static int ppc4xx_msi_init_allocator(struct ppc4xx_msi *msi_data)
+{
+ int rc;
+
+ rc = msi_bitmap_alloc(&msi_data->bitmap, NR_MSI_IRQS,
+ msi_data->irqhost->of_node);
+ if (rc)
+ return rc;
+ rc = msi_bitmap_reserve_dt_hwirqs(&msi_data->bitmap);
+ if (rc < 0) {
+ msi_bitmap_free(&msi_data->bitmap);
+ return rc;
+ }
+ return 0;
+}
+
+Nitpicking: Remove one empty line here.
+static void ppc4xx_msi_cascade(unsigned int irq, struct irq_desc *desc)
+{
+ unsigned int cascade_irq;
+ struct ppc4xx_msi *msi_data = ppc4xx_msi;
+ int msir_index = -1;
+
+ raw_spin_lock(&desc->lock);
+ if (desc->chip->mask_ack) {
+ desc->chip->mask_ack(irq);
+ } else {
+ desc->chip->mask(irq);
+ desc->chip->ack(irq);
+ }
+
+ if (unlikely(desc->status & IRQ_INPROGRESS))
+ goto unlock;
+
+ msir_index = (int)desc->handler_data;
+
+ if (msir_index >= NR_MSI_IRQS)
+ cascade_irq = NO_IRQ;
+
+ desc->status |= IRQ_INPROGRESS;
+
+ cascade_irq = irq_linear_revmap(msi_data->irqhost, msir_index);
+ if (cascade_irq != NO_IRQ)
+ generic_handle_irq(cascade_irq);
+ desc->status &= ~IRQ_INPROGRESS;
+
+ if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
+ desc->chip->unmask(irq);
+unlock:
+ raw_spin_unlock(&desc->lock);
+}Nitpicking: Add one empty line here.
+static void ppc4xx_compose_msi_msg(struct pci_dev *pdev, int hwirq,
+ struct msi_msg *msg)
+{
+ struct ppc4xx_msi *msi_data = ppc4xx_msi;
+
+ msg->address_lo = msi_data->msi_addr_lo;
+ msg->address_hi = msi_data->msi_addr_hi;
+ msg->data = hwirq;
+}
+
+Nitpicking: Remove one empty line here.
+int ppc4xx_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+ struct msi_desc *entry;
+ int rc, hwirq;
+ unsigned int virq;
+ struct msi_msg msg;
+ struct ppc4xx_msi *msi_data = ppc4xx_msi;
+
+
+ list_for_each_entry(entry, &dev->msi_list, list) {
+ hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
+ if (hwirq < 0) {
+ rc = hwirq;
+ dev_err(&dev->dev, "%s: fail allocating msi\
+ interrupt\n", __func__);
+ goto out_free;
+ }
+
+ pr_debug(KERN_INFO"mis is %p\n", msi_data->irqhost);
+ virq = irq_create_mapping(msi_data->irqhost, hwirq);
+ if (virq == NO_IRQ) {
+ dev_err(&dev->dev, "%s: fail mapping irq\n", __func__);
+ rc = -ENOSPC;
+ goto out_free;
+ }
+
+ set_irq_msi(virq, entry);
+ ppc4xx_compose_msi_msg(dev, hwirq, &msg);
+ write_msi_msg(virq, &msg);
+ }
+
+ return 0;
+out_free:
+ return rc;
+}
+
+void ppc4xx_teardown_msi_irqs(struct pci_dev *dev)
+{
+ struct msi_desc *entry;
+ struct ppc4xx_msi *msi_data = ppc4xx_msi;
+ dev_dbg(&dev->dev, "PCIE-MSI: tearing down msi irqs\n");
+
+ list_for_each_entry(entry, &dev->msi_list, list) {
+ if (entry->irq == NO_IRQ)
+ continue;
+ set_irq_msi(entry->irq, NULL);
+ msi_bitmap_free_hwirqs(&msi_data->bitmap,
+ virq_to_hw(entry->irq), 1);
+ irq_dispose_mapping(entry->irq);
+Nitpicking: Remove one empty line here.
+ }
+
+ return;
+}
+
+static int ppc4xx_msi_check_device(struct pci_dev *pdev, int nvec, int
type) +{
+ pr_debug(KERN_INFO"PCIE-MSI:%s called. vec %x type %d\n",
+ __func__, nvec, type);
+ return 0;
+}
+
+/*
+ * We do not need this actually. The MSIR register has been read once
+ * in the cascade interrupt. So, this MSI interrupt has been acked
+*/
+static void ppc4xx_msi_end_irq(unsigned int virq)
+{
+}
+
+Nitpicking: Remove one empty line here. I'll stop mentioning this here. Please check the whole file for this.
+static struct irq_chip ppc4xx_msi_chip = {
+ .mask = mask_msi_irq,
+ .unmask = unmask_msi_irq,
+ .ack = ppc4xx_msi_end_irq,
+ .name = " UIC",
+};
+
+static int ppc4xx_msi_host_map(struct irq_host *h, unsigned int virq,
+ irq_hw_number_t hw)
+{
+ struct irq_chip *chip = &ppc4xx_msi_chip;
+
+ irq_to_desc(virq)->status |= IRQ_TYPE_EDGE_RISING;
+
+ set_irq_chip_and_handler(virq, chip, handle_edge_irq);
+
+ return 0;
+}
+
+static struct irq_host_ops ppc4xx_msi_host_ops = {
+ .map = ppc4xx_msi_host_map,
+};
+
+
+static int __devinit ppc4xx_msi_probe(struct of_device *dev,
+ const struct of_device_id *match)
+{
+ struct ppc4xx_msi *msi;
+ struct resource res, rmsi;
+ int i, count;
+ int rc;
+ int virt_msir;
+ const u32 *p;
+ const u32 *sdr_base;
+ u32 *msi_virt = NULL;
+ dma_addr_t msi_phys;
+
+
+ msi = kzalloc(sizeof(struct ppc4xx_msi), GFP_KERNEL);
+ if (!msi) {
+ dev_err(&dev->dev, "No memory for MSI structure\n");
+ rc = -ENOMEM;
+ goto error_out;
+ }
+
+ msi->irqhost = irq_alloc_host(dev->node, IRQ_HOST_MAP_LINEAR,
+ NR_MSI_IRQS, &ppc4xx_msi_host_ops, 0);
+ if (msi->irqhost == NULL) {
+ dev_err(&dev->dev, "No memory for MSI irqhost\n");
+ rc = -ENOMEM;
+ goto error_out;
+ }
+
+
+ /* Get MSI ranges */
+ rc = of_address_to_resource(dev->node, 0, &rmsi);
+ if (rc) {
+ dev_err(&dev->dev, "%s resource error!\n",
+ dev->node->full_name);
+ goto error_out;
+ }
+
+
+ /* Get the MSI reg base */
+ rc = of_address_to_resource(dev->node, 1, &res);
+ if (rc) {
+ dev_err(&dev->dev, "%s resource error!\n",
+ dev->node->full_name);
+ goto error_out;
+ }
+ /* Get the sdr-base */
+ sdr_base = (u32 *)of_get_property(dev->node, "sdr-base", NULL);
+ if (sdr_base == NULL) {
+ dev_err(&dev->dev, "%s resource error!\n",
+ dev->node->full_name);
+ goto error_out;
+ }
+ msi->sdr_base = *sdr_base;
+#if defined(CONFIG_460SX)
+ mtdcri(SDR0, msi->sdr_base, res.start >> 32);
+ mtdcri(SDR0, msi->sdr_base + 1, res.start & 0xFFFFFFFF);
+ msi->msi_regs = ioremap(res.start, res.end - res.start + 1);msi->msi_regs = ioremap(res.start, resource_size(&res));
+#else + dev_err(&dev->dev, " Invalid Device \n"); + goto error_out; +#endif
Please don't introduce such a compile-time selection here. You don't even need it, since the register bases are provided via the device-tree. So just remove the #if and it's #else part here.
quoted hunk ↗ jump to hunk
+ if (!msi->msi_regs) { + dev_err(&dev->dev, "ioremap problem failed\n"); + goto error_out; + } + /* MSI region always mapped in 4GB region*/ + msi->msi_addr_hi = 0x0; + msi_virt = dma_alloc_coherent(&dev->dev, 64, &msi_phys, + GFP_KERNEL); + if (msi_virt == NULL) { + dev_err(&dev->dev, "No memory for MSI mem space\n"); + rc = -ENOMEM; + goto error_out; + } + msi->msi_addr_lo = (u32)msi_phys; + + /* Progam the Interrupt handler Termination addr registers */ + out_be32(msi->msi_regs + PEIH_TERMADH, msi->msi_addr_hi); + out_be32(msi->msi_regs + PEIH_TERMADL, msi->msi_addr_lo); + + /* Program MSI Expected data and Mask bits */ + out_be32(msi->msi_regs + PEIH_MSIED, MSI_DATA_PATTERN); + out_be32(msi->msi_regs + PEIH_MSIMK, MSI_DATA_PATTERN); + + msi->irqhost->host_data = msi; + + if (ppc4xx_msi_init_allocator(msi)) { + dev_err(&dev->dev, "Error allocating MSI bitmap\n"); + goto error_out; + } + + p = of_get_property(dev->node, "interrupts", &count); + if (!p) { + dev_err(&dev->dev, "no interrupts property found on %s\n", + dev->node->full_name); + rc = -ENODEV; + goto error_out; + } + if (count == 0) { + dev_err(&dev->dev, "Malformed interrupts property on %s\n", + dev->node->full_name); + rc = -EINVAL; + goto error_out; + } + + for (i = 0; i < NR_MSI_IRQS; i++) { + virt_msir = irq_of_parse_and_map(dev->node, i); + if (virt_msir != NO_IRQ) { + set_irq_data(virt_msir, (void *)i); + set_irq_chained_handler(virt_msir, ppc4xx_msi_cascade); + } + } + + ppc4xx_msi = msi; + + WARN_ON(ppc_md.setup_msi_irqs); + ppc_md.setup_msi_irqs = ppc4xx_setup_msi_irqs; + ppc_md.teardown_msi_irqs = ppc4xx_teardown_msi_irqs; + ppc_md.msi_check_device = ppc4xx_msi_check_device; + return 0; +error_out: + if (msi_virt) + dma_free_coherent(&dev->dev, 64, msi_virt, msi_phys); + kfree(msi); + return rc; +} + +static const struct ppc4xx_msi_feature ppc4xx_msi_feature = { + .ppc4xx_pic_ip = 0, + .msiir_offset = 0x140, +}; + +static const struct of_device_id ppc4xx_msi_ids[] = { + { + .compatible = "amcc,ppc4xx-msi", + .data = (void *)&ppc4xx_msi_feature, + }, + {} +}; + +static struct of_platform_driver ppc4xx_msi_driver = { + .name = "ppc4xx-msi", + .match_table = ppc4xx_msi_ids, + .probe = ppc4xx_msi_probe, +}; + +static __init int ppc4xx_msi_init(void) +{ + return of_register_platform_driver(&ppc4xx_msi_driver); +} + +subsys_initcall(ppc4xx_msi_init);diff --git a/arch/powerpc/sysdev/ppc4xx_msi.h b/arch/powerpc/sysdev/ppc4xx_msi.h new file mode 100644 index 0000000..e4ae058 --- /dev/null +++ b/arch/powerpc/sysdev/ppc4xx_msi.h@@ -0,0 +1,39 @@ +/* + * Copyright (C) 2009 Applied Micro Circuits Corporation. + * + * Author: Tirumala Marri <tmarri@amcc.com> + * Feng Kan <fkan@amcc.com> + * 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; version 2 of the + * License. + */ +#ifndef __PPC4XX_MSI_H__ +#define __PPC4XX_MSI_H__ + +#include <asm/msi_bitmap.h> + +#define PEIH_TERMADH 0x00 +#define PEIH_TERMADL 0x08 +#define PEIH_MSIED 0x10 +#define PEIH_MSIMK 0x18 +#define PEIH_MSIASS 0x20 +#define PEIH_FLUSH0 0x30 +#define PEIH_FLUSH1 0x38 +#define PEIH_CNTRST 0x48 + +#define MSI_DATA_PATTERN 0x44440000 + +struct ppc4xx_msi { + struct irq_host *irqhost; + unsigned long cascade_irq; + u32 msi_addr_lo; + u32 msi_addr_hi; + void __iomem *msi_regs; + u32 feature; + struct msi_bitmap bitmap; + u32 sdr_base; +}; + +#define NR_MSI_IRQS 4 +#endif /* __PPC4XX_MSI_H__ */
Cheers, Stefan -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de