[PATCH 1/4 V2] booting-without-of for Freescale MSI

[PATCH 1/4 V2] booting-without-of for Freescale MSI

[Jason Jin]

Binding document adding for Freescale MSI support.

Signed-off-by: Jason Jin <Jason.jin@freescale.com>
---
Updated to V2 version per Segher's suggestion.

 Documentation/powerpc/booting-without-of.txt |   40 +++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index 1d2a772..887783c 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -57,7 +57,10 @@ Table of Contents
       n) 4xx/Axon EMAC ethernet nodes
       o) Xilinx IP cores
       p) Freescale Synchronous Serial Interface
-	  q) USB EHCI controllers
+      q) USB EHCI controllers
+      r) Freescale Display Interface Unit
+      s) Freescale on board FPGA
+      t) Freescael MSI interrupt controller
 
   VII - Marvell Discovery mv64[345]6x System Controller chips
     1) The /system-controller node
@@ -2870,6 +2873,41 @@ platforms are moved over to use the flattened-device-tree model.
 		reg = <0xe8000000 32>;
 	};
 
+    t) Freescale MSI interrupt controller
+
+    Reguired properities:
+    - compatible : set as "fsl,86xx-MSI" for 86xx cpu, "fsl,85xx-MSI" for 85xx
+      cpu and "fsl,83xx-MSI" for 83xx cpu.
+    - reg : should contain the address and the length of the shared message
+      interrupt register set.
+    - msi-available-ranges: use <start count> style section to define which
+      msi interrupt can be used in the 256 msi interrupts.
+    - interrupts : each one of the interrupts here is one entry per 32 MSIs,
+      and routed to the host interrupt controller. the interrupts should
+      be set as edge sensitive.
+    - interrupt-parent: the phandle for the interrupt controller
+      that services interrupts for this device. for 83xx cpu, the interrupts
+      are routed to IPIC, and for 85xx/86xx cpu the interrupts are routed
+      to MPIC.
+
+    Example (86xx CPU)
+	msi@41600 {
+		compatible = "fsl,86xx-MSI";
+		reg = <0x41600 0x80>;
+		msi-available-ranges = <0 0x100>;
+		interrupts = <
+			0xb0 0
+			0xb1 0
+			0xb2 0
+			0xb3 0
+			0xb4 0
+			0xb5 0
+			0xb6 0
+			0xb7 0>;
+		interrupt-parent = <&mpic>;
+	};
+
+
 VII - Marvell Discovery mv64[345]6x System Controller chips
 ===========================================================
 
-- 
1.5.4

[PATCH 2/4 V4] MSI support on 83xx/85xx/86xx board

[Jason Jin]

This MSI driver can be used on 83xx/85xx/86xx board.
In this driver, virtual interrupt host and chip were
setup. There are 256 MSI interrupts in this host, Every 32
MSI interrupts cascaded to one IPIC/MPIC interrupt.
The chip was treated as edge sensitive and some necessary
functions were setup for this chip.

Before using the MSI interrupt, PCI/PCIE device need to
ask for a MSI interrupt in the 256 MSI interrupts. A 256bit
bitmap show which MSI interrupt was used, reserve bit in
the bitmap can be used to force the device use some designate
MSI interrupt in the 256 MSI interrupts. Sometimes this is useful
for testing the all the MSI interrupts. The msi-available-ranges
property in the dts file was used for this purpose.

Signed-off-by: Jason Jin <Jason.jin@freescale.com>
---
Update to V4 version per the suggestion's from Michael Ellerman

 arch/powerpc/sysdev/Makefile  |    3 +-
 arch/powerpc/sysdev/fsl_msi.c |  446 +++++++++++++++++++++++++++++++++++++++++
 arch/powerpc/sysdev/fsl_msi.h |   42 ++++
 arch/powerpc/sysdev/fsl_pci.c |   14 ++
 4 files changed, 504 insertions(+), 1 deletions(-)
 create mode 100644 arch/powerpc/sysdev/fsl_msi.c
 create mode 100644 arch/powerpc/sysdev/fsl_msi.h

diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 6d386d0..98b6b8e 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -4,6 +4,7 @@ endif
 
 mpic-msi-obj-$(CONFIG_PCI_MSI)	+= mpic_msi.o mpic_u3msi.o mpic_pasemi_msi.o
 obj-$(CONFIG_MPIC)		+= mpic.o $(mpic-msi-obj-y)
+fsl-msi-obj-$(CONFIG_PCI_MSI)	+= fsl_msi.o
 
 obj-$(CONFIG_PPC_MPC106)	+= grackle.o
 obj-$(CONFIG_PPC_DCR_NATIVE)	+= dcr-low.o
@@ -11,7 +12,7 @@ obj-$(CONFIG_PPC_PMI)		+= pmi.o
 obj-$(CONFIG_U3_DART)		+= dart_iommu.o
 obj-$(CONFIG_MMIO_NVRAM)	+= mmio_nvram.o
 obj-$(CONFIG_FSL_SOC)		+= fsl_soc.o
-obj-$(CONFIG_FSL_PCI)		+= fsl_pci.o
+obj-$(CONFIG_FSL_PCI)		+= fsl_pci.o $(fsl-msi-obj-y)
 obj-$(CONFIG_FSL_LBC)		+= fsl_lbc.o
 obj-$(CONFIG_RAPIDIO)		+= fsl_rio.o
 obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pci.o tsi108_dev.o
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
new file mode 100644
index 0000000..be3304d
--- /dev/null
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -0,0 +1,446 @@
+/*
+ * Copyright (C) 2007-2008 Freescale Semiconductor, Inc. All rights reserved.
+ *
+ * Author: Tony Li <tony.li@freescale.com>
+ *	   Jason Jin <Jason.jin@freescale.com>
+ *
+ * The hwirq alloc and free code reuse from sysdev/mpic_msi.c
+ *
+ * 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/bitmap.h>
+#include <linux/msi.h>
+#include <linux/pci.h>
+#include <linux/of_platform.h>
+#include <sysdev/fsl_soc.h>
+#include <asm/prom.h>
+#include <asm/hw_irq.h>
+#include <asm/ppc-pci.h>
+#include "fsl_msi.h"
+
+struct fsl_msi_feature {
+	u32 fsl_pic_ip;
+	u32 msiir_offset;
+};
+
+static struct fsl_msi *fsl_msi;
+
+static inline u32 fsl_msi_read(u32 __iomem *base, unsigned int reg)
+{
+	return in_be32(base + (reg >> 2));
+}
+
+static inline void fsl_msi_write(u32 __iomem *base,
+				unsigned int reg, u32 value)
+{
+	out_be32(base + (reg >> 2), value);
+}
+
+/*
+ * 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 fsl_msi_end_irq(unsigned int virq)
+{
+}
+
+static struct irq_chip fsl_msi_chip = {
+	.mask		= mask_msi_irq,
+	.unmask		= unmask_msi_irq,
+	.ack		= fsl_msi_end_irq,
+	.typename	= " FSL-MSI  ",
+};
+
+static int fsl_msi_host_match(struct irq_host *h, struct device_node *node)
+{
+	/* Exact match*/
+	return h->of_node == node;
+}
+
+
+static int fsl_msi_host_map(struct irq_host *h, unsigned int virq,
+				irq_hw_number_t hw)
+{
+	struct irq_chip *chip = &fsl_msi_chip;
+
+	get_irq_desc(virq)->status |= IRQ_TYPE_EDGE_FALLING;
+
+	set_irq_chip_and_handler(virq, chip,  handle_edge_irq);
+
+	return 0;
+}
+
+static struct irq_host_ops fsl_msi_host_ops = {
+	.match = fsl_msi_host_match,
+	.map = fsl_msi_host_map,
+};
+
+irq_hw_number_t fsl_msi_alloc_hwirqs(struct fsl_msi *msi, int num)
+{
+	unsigned long flags;
+	int offset, order = get_count_order(num);
+
+	spin_lock_irqsave(&msi->bitmap_lock, flags);
+
+	offset = bitmap_find_free_region(msi->fsl_msi_bitmap,
+					NR_MSI_IRQS, order);
+
+	spin_unlock_irqrestore(&msi->bitmap_lock, flags);
+
+	pr_debug("%s: allocated 0x%x (2^%d) at offset 0x%x\n",
+		__func__, num, order, offset);
+
+	return offset;
+}
+
+void fsl_msi_free_hwirqs(struct fsl_msi *msi, int offset, int num)
+{
+	unsigned long flags;
+	int order = get_count_order(num);
+
+	pr_debug("%s: freeing 0x%x (2^%d) at offset 0x%x\n",
+		__func__, num, order, offset);
+
+	spin_lock_irqsave(&msi->bitmap_lock, flags);
+	bitmap_release_region(msi->fsl_msi_bitmap, offset, order);
+	spin_unlock_irqrestore(&msi->bitmap_lock, flags);
+}
+
+static int fsl_msi_free_dt_hwirqs(struct fsl_msi *msi)
+{
+	int i, len;
+	const u32 *p;
+
+	p = of_get_property(msi->of_node, "msi-available-ranges", &len);
+	if (!p) {
+		pr_debug("fsl_msi: no msi-available-ranges property found \
+				on %s\n", msi->of_node->full_name);
+		return -ENODEV;
+	}
+
+	if ((len % 0x8) != 0) {
+		printk(KERN_WARNING "fsl_msi: Malformed msi-available-ranges "
+		       "property on %s\n", msi->of_node->full_name);
+		return -EINVAL;
+	}
+
+	bitmap_allocate_region(msi->fsl_msi_bitmap, 0,
+			       get_count_order(NR_MSI_IRQS));
+
+	/* Format is: (<u32 start> <u32 count>)+ */
+	len /= sizeof(u32);
+	len /= 2;
+	for (i = 0; i < len; i++, p += 2)
+		fsl_msi_free_hwirqs(msi, *p, *(p + 1));
+
+	return 0;
+}
+
+static int fsl_msi_init_allocator(struct fsl_msi *msi_data)
+{
+	int rc, size;
+
+	size = BITS_TO_LONGS(NR_MSI_IRQS) * sizeof(u32);
+
+	msi_data->fsl_msi_bitmap = kzalloc(size, GFP_KERNEL);
+
+	if (msi_data->fsl_msi_bitmap == NULL) {
+		pr_debug("%s: ENOMEM allocating allocator bitmap!\n",
+				__func__);
+		return -ENOMEM;
+	}
+
+	rc = fsl_msi_free_dt_hwirqs(msi_data);
+	if (rc)
+		goto out_free;
+
+	return 0;
+out_free:
+	kfree(msi_data->fsl_msi_bitmap);
+
+	msi_data->fsl_msi_bitmap = NULL;
+	return rc;
+
+}
+
+static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int type)
+{
+	if (type == PCI_CAP_ID_MSIX)
+		pr_debug("fslmsi: MSI-X untested, trying anyway.\n");
+
+	return 0;
+}
+
+static void fsl_teardown_msi_irqs(struct pci_dev *pdev)
+{
+	struct msi_desc *entry;
+	struct fsl_msi *msi_data = fsl_msi;
+
+	list_for_each_entry(entry, &pdev->msi_list, list) {
+		if (entry->irq == NO_IRQ)
+			continue;
+		set_irq_msi(entry->irq, NULL);
+		fsl_msi_free_hwirqs(msi_data, virq_to_hw(entry->irq), 1);
+		irq_dispose_mapping(entry->irq);
+	}
+
+	return;
+}
+
+static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq,
+				  struct msi_msg *msg)
+{
+	struct fsl_msi *msi_data = fsl_msi;
+
+	msg->address_lo = msi_data->msi_addr_lo;
+	msg->address_hi = msi_data->msi_addr_hi;
+	msg->data = hwirq;
+
+	pr_debug("%s: allocated srs: %d, ibs: %d\n",
+		__func__, hwirq / IRQS_PER_MSI_REG, hwirq % IRQS_PER_MSI_REG);
+}
+
+static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
+{
+	irq_hw_number_t hwirq;
+	int rc;
+	unsigned int virq;
+	struct msi_desc *entry;
+	struct msi_msg msg;
+	struct fsl_msi *msi_data = fsl_msi;
+
+	list_for_each_entry(entry, &pdev->msi_list, list) {
+		hwirq = fsl_msi_alloc_hwirqs(msi_data, 1);
+		if (hwirq < 0) {
+			rc = hwirq;
+			pr_debug("%s: fail allocating msi interrupt\n",
+					__func__);
+			goto out_free;
+		}
+
+		virq = irq_create_mapping(msi_data->irqhost, hwirq);
+
+		if (virq == NO_IRQ) {
+			pr_debug("%s: fail mapping hwirq 0x%lx\n",
+					__func__, hwirq);
+			fsl_msi_free_hwirqs(msi_data, hwirq, 1);
+			rc = -ENOSPC;
+			goto out_free;
+		}
+		set_irq_msi(virq, entry);
+
+		fsl_compose_msi_msg(pdev, hwirq, &msg);
+		write_msi_msg(virq, &msg);
+	}
+	return 0;
+
+out_free:
+	return rc;
+}
+
+void fsl_msi_cascade(unsigned int irq, struct irq_desc *desc)
+{
+	unsigned int cascade_irq;
+	struct fsl_msi *msi_data = fsl_msi;
+	int msir_index = -1;
+	u32 msir_value = 0;
+	u32 intr_index;
+	u32 have_shift = 0;
+
+	spin_lock(&desc->lock);
+	if ((msi_data->feature &  FSL_PIC_IP_MASK) == FSL_PIC_IP_IPIC) {
+		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_REG)
+		cascade_irq = NO_IRQ;
+
+	desc->status |= IRQ_INPROGRESS;
+	switch (fsl_msi->feature & FSL_PIC_IP_MASK) {
+	case FSL_PIC_IP_MPIC:
+		msir_value = fsl_msi_read(msi_data->msi_regs,
+			msir_index * 0x10);
+		break;
+	case FSL_PIC_IP_IPIC:
+		msir_value = fsl_msi_read(msi_data->msi_regs, msir_index * 0x4);
+		break;
+	}
+
+	while (msir_value) {
+		intr_index = ffs(msir_value) - 1;
+
+		cascade_irq = irq_linear_revmap(msi_data->irqhost,
+				(msir_index * IRQS_PER_MSI_REG +
+					intr_index + have_shift));
+		if (cascade_irq != NO_IRQ)
+			generic_handle_irq(cascade_irq);
+		have_shift += (intr_index + 1);
+		msir_value = (msir_value >> (intr_index + 1));
+	}
+	desc->status &= ~IRQ_INPROGRESS;
+
+	switch (msi_data->feature & FSL_PIC_IP_MASK) {
+	case FSL_PIC_IP_MPIC:
+		desc->chip->eoi(irq);
+		break;
+	case FSL_PIC_IP_IPIC:
+		if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
+			desc->chip->unmask(irq);
+		break;
+	}
+unlock:
+	spin_unlock(&desc->lock);
+}
+
+static int __devinit fsl_of_msi_probe(struct of_device *dev,
+				const struct of_device_id *match)
+{
+	struct fsl_msi *msi;
+	struct resource res;
+	int err, i, count;
+	int rc;
+	int virt_msir;
+	const u32 *p;
+	struct fsl_msi_feature *tmp_data;
+
+	printk(KERN_DEBUG "Setting up fsl msi support\n");
+
+	msi = kzalloc(sizeof(struct fsl_msi), GFP_KERNEL);
+	if (!msi) {
+		dev_err(&dev->dev, "No memory for MSI structure\n");
+		err = -ENOMEM;
+		goto error_out;
+	}
+
+	msi->of_node = dev->node;
+
+	msi->irqhost = irq_alloc_host(of_node_get(dev->node),
+				IRQ_HOST_MAP_LINEAR,
+				NR_MSI_IRQS, &fsl_msi_host_ops, 0);
+	if (msi->irqhost == NULL) {
+		dev_err(&dev->dev, "No memory for MSI irqhost\n");
+		of_node_put(dev->node);
+		err = -ENOMEM;
+		goto error_out;
+	}
+
+	/* Get the MSI reg base */
+	err = of_address_to_resource(dev->node, 0, &res);
+	if (err) {
+		dev_err(&dev->dev, "%s resource error!\n",
+				dev->node->full_name);
+		goto error_out;
+	}
+
+	msi->msi_regs = ioremap(res.start, res.end - res.start + 1);
+	if (!msi->msi_regs) {
+		dev_err(&dev->dev, "ioremap problem failed\n");
+		goto error_out;
+	}
+
+	tmp_data = (struct fsl_msi_feature *)match->data;
+
+	msi->feature = tmp_data->fsl_pic_ip;
+
+	msi->irqhost->host_data = msi;
+
+	msi->msi_addr_hi = 0x0;
+	msi->msi_addr_lo = res.start + tmp_data->msiir_offset;
+
+	rc = fsl_msi_init_allocator(msi);
+	if (rc) {
+		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);
+		err = -ENODEV;
+		goto error_out;
+	}
+	if (count % 8 != 0) {
+		dev_err(&dev->dev, "Malformed interrupts property on %s\n",
+				dev->node->full_name);
+		err = -EINVAL;
+		goto error_out;
+	}
+
+	count /= sizeof(u32);
+	for (i = 0; i < count / 2; i++) {
+		if (i > NR_MSI_REG)
+			break;
+		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, fsl_msi_cascade);
+		}
+	}
+
+	fsl_msi = msi;
+
+	WARN_ON(ppc_md.setup_msi_irqs);
+	ppc_md.setup_msi_irqs = fsl_setup_msi_irqs;
+	ppc_md.teardown_msi_irqs = fsl_teardown_msi_irqs;
+	ppc_md.msi_check_device = fsl_msi_check_device;
+	return 0;
+error_out:
+	kfree(msi);
+	return err;
+}
+
+static const struct fsl_msi_feature mpic_msi_feature = {
+	.fsl_pic_ip = FSL_PIC_IP_MPIC,
+	.msiir_offset = 0x140,
+};
+
+static const struct fsl_msi_feature ipic_msi_feature = {
+	.fsl_pic_ip = FSL_PIC_IP_IPIC,
+	.msiir_offset = 0x38,
+};
+
+static const struct of_device_id fsl_of_msi_ids[] = {
+	{
+		.compatible = "fsl,86xx-MSI",
+		.data = (void *)&mpic_msi_feature,
+	},
+	{
+		.compatible = "fsl,85xx-MSI",
+		.data = (void *)&mpic_msi_feature,
+	},
+	{
+		.compatible = "fsl,83xx-MSI",
+		.data = (void *)&ipic_msi_feature,
+	},
+	{}
+};
+
+static struct of_platform_driver fsl_of_msi_driver = {
+	.name = "fsl-of-msi",
+	.match_table = fsl_of_msi_ids,
+	.probe = fsl_of_msi_probe,
+};
+
+static __init int fsl_of_msi_init(void)
+{
+	return of_register_platform_driver(&fsl_of_msi_driver);
+}
+
+subsys_initcall(fsl_of_msi_init);
diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
new file mode 100644
index 0000000..a653468
--- /dev/null
+++ b/arch/powerpc/sysdev/fsl_msi.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2007-2008 Freescale Semiconductor, Inc. All rights reserved.
+ *
+ * Author: Tony Li <tony.li@freescale.com>
+ *	   Jason Jin <Jason.jin@freescale.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 _POWERPC_SYSDEV_FSL_MSI_H
+#define _POWERPC_SYSDEV_FSL_MSI_H
+
+#define NR_MSI_REG		8
+#define IRQS_PER_MSI_REG	32
+#define NR_MSI_IRQS	(NR_MSI_REG * IRQS_PER_MSI_REG)
+
+#define FSL_PIC_IP_MASK	0x0000000F
+#define FSL_PIC_IP_MPIC	0x00000001
+#define FSL_PIC_IP_IPIC	0x00000002
+
+struct fsl_msi {
+	/* Device node of the MSI interrupt*/
+	struct device_node *of_node;
+
+	struct irq_host *irqhost;
+
+	unsigned long cascade_irq;
+
+	u32 msi_addr_lo;
+	u32 msi_addr_hi;
+	void __iomem *msi_regs;
+	u32 feature;
+
+	unsigned long *fsl_msi_bitmap;
+	spinlock_t bitmap_lock;
+};
+
+#endif /* _POWERPC_SYSDEV_FSL_MSI_H */
+
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index bf13c21..fede767 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -106,6 +106,16 @@ void __init setup_pci_cmd(struct pci_controller *hose)
 	}
 }
 
+#ifdef CONFIG_PCI_MSI
+void __init setup_pci_pcsrbar(struct pci_controller *hose)
+{
+	phys_addr_t immr_base;
+
+	immr_base = get_immrbase();
+	early_write_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, immr_base);
+}
+#endif
+
 static int fsl_pcie_bus_fixup;
 
 static void __init quirk_fsl_pcie_header(struct pci_dev *dev)
@@ -211,6 +221,10 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
 	/* Setup PEX window registers */
 	setup_pci_atmu(hose, &rsrc);
 
+	/*Setup PEXCSRBAR */
+#ifdef CONFIG_PCI_MSI
+	setup_pci_pcsrbar(hose);
+#endif
 	return 0;
 }
 
-- 
1.5.4

Re: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Segher Boessenkool]

Hi Jason,

Some more comments...

> +    - compatible : set as "fsl,86xx-MSI" for 86xx cpu, "fsl,85xx-MSI" 
> for 85xx
> +      cpu and "fsl,83xx-MSI" for 83xx cpu.

Don't use "xx", use real device names instead.  If for example the 
oldest
86xx device that implements this is 8600, and you write a tree for the 
8699,
you'd write  compatible = "fsl,8699-msi", "fsl,8600-msi";  and the 
driver
will look for "fsl,8600-msi" only usually, but it can take some special
actions for 8699 if it needs to.

If these MSI devices work really identical for 83xx, 85xx, 86xx, there 
is
no need to distinguish between them for probing either -- you want to 
use
the same driver for all, anyway.

Oh, and the norm is to write compatible values in lower case.

> +    - msi-available-ranges: use <start count> style section to define 
> which
> +      msi interrupt can be used in the 256 msi interrupts.

Please make this property optional, default to "all available".


Segher

RE: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Jin Zhengxiong]

Hi, Segher.

Thanks=20
> > +    - compatible : set as "fsl,86xx-MSI" for 86xx cpu,=20
> "fsl,85xx-MSI"=20
> > for 85xx
> > +      cpu and "fsl,83xx-MSI" for 83xx cpu.
>=20
> Don't use "xx", use real device names instead.  If for=20
> example the oldest 86xx device that implements this is 8600,=20
> and you write a tree for the 8699, you'd write  compatible =3D=20
> "fsl,8699-msi", "fsl,8600-msi";  and the driver will look for=20
> "fsl,8600-msi" only usually, but it can take some special=20
> actions for 8699 if it needs to.
>=20
> If these MSI devices work really identical for 83xx, 85xx,=20
> 86xx, there is no need to distinguish between them for=20
> probing either -- you want to use the same driver for all, anyway
.
The driver can work for 83xx, 85xx and 86xx,  but the msiir offset=20
is different for 83xx and 85xx/86xx cpu. I need this to get different=20
msiir offset in the driver.

> Oh, and the norm is to write compatible values in lower case.

Ok.
> > +    - msi-available-ranges: use <start count> style=20
> section to define
> > which
> > +      msi interrupt can be used in the 256 msi interrupts.
>=20
> Please make this property optional, default to "all available".
>=20
The value setting for this property is "all available", and this ranges
maybe
useful for some other MSI controller.=20

Jason

Re: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Segher Boessenkool]

>> If these MSI devices work really identical for 83xx, 85xx,
>> 86xx, there is no need to distinguish between them for
>> probing either -- you want to use the same driver for all, anyway
> .
> The driver can work for 83xx, 85xx and 86xx,  but the msiir offset
> is different for 83xx and 85xx/86xx cpu. I need this to get different
> msiir offset in the driver.

It's a different offset _within_ the region specified in "reg"?
Yeah, using different "compatible" is cleanest then, no doubt.

>>> +    - msi-available-ranges: use <start count> style
>> section to define
>>> which
>>> +      msi interrupt can be used in the 256 msi interrupts.
>>
>> Please make this property optional, default to "all available".
>>
> The value setting for this property is "all available", and this ranges
> maybe
> useful for some other MSI controller.

Sure, I didn't say to drop the property, just make it optional,
since most trees will want the same value anyway.


Segher

RE: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Jin Zhengxiong]

> -----Original Message-----
> From: Segher Boessenkool [mailto:segher@kernel.crashing.org]=20
> Sent: Friday, May 09, 2008 6:14 PM
> To: Jin Zhengxiong
> Cc: michael@ellerman.id.au; linuxppc-dev@ozlabs.org;=20
> galak@kernel.crashing.org
> Subject: Re: [PATCH 1/4 V2] booting-without-of for Freescale MSI
>=20
> >> If these MSI devices work really identical for 83xx, 85xx, 86xx,=20
> >> there is no need to distinguish between them for probing either --=20
> >> you want to use the same driver for all, anyway
> > .
> > The driver can work for 83xx, 85xx and 86xx,  but the msiir=20
> offset is=20
> > different for 83xx and 85xx/86xx cpu. I need this to get different=20
> > msiir offset in the driver.
>=20
> It's a different offset _within_ the region specified in "reg"?
> Yeah, using different "compatible" is cleanest then, no doubt.
>=20
Yes, It's different offset _within_ the "reg" region.

> >>> +    - msi-available-ranges: use <start count> style
> >> section to define
> >>> which
> >>> +      msi interrupt can be used in the 256 msi interrupts.
> >>
> >> Please make this property optional, default to "all available".
> >>
> > The value setting for this property is "all available", and this=20
> > ranges maybe useful for some other MSI controller.
>=20
> Sure, I didn't say to drop the property, just make it=20
> optional, since most trees will want the same value anyway.

So, The code should change to work with this change, If there's the=20
property, use it. If no, make it all available, Am I right?

Jason

Re: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Segher Boessenkool]

>> Sure, I didn't say to drop the property, just make it
>> optional, since most trees will want the same value anyway.
>
> So, The code should change to work with this change, If there's the
> property, use it. If no, make it all available, Am I right?

Yeah, exactly.


Segher

Re: [PATCH 2/4 V4] MSI support on 83xx/85xx/86xx board

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

On Fri, 2008-05-09 at 17:03 +0800, Jason Jin wrote:
> This MSI driver can be used on 83xx/85xx/86xx board.
> In this driver, virtual interrupt host and chip were
> setup. There are 256 MSI interrupts in this host, Every 32
> MSI interrupts cascaded to one IPIC/MPIC interrupt.
> The chip was treated as edge sensitive and some necessary
> functions were setup for this chip.

> diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
> new file mode 100644
> index 0000000..be3304d
> --- /dev/null
> +++ b/arch/powerpc/sysdev/fsl_msi.c
> @@ -0,0 +1,446 @@

...

> +static int fsl_msi_host_match(struct irq_host *h, struct device_node *node)
> +{
> +	/* Exact match*/
> +	return h->of_node == node;
> +}

This is the default behaviour, so technically you don't need this
routine - but I'm not fussed.

Acked-by: Michael Ellerman <michael@ellerman.id.au>

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Grant Likely]

On Fri, May 9, 2008 at 3:03 AM, Jason Jin <Jason.jin@freescale.com> wrote:
> Binding document adding for Freescale MSI support.
>
> Signed-off-by: Jason Jin <Jason.jin@freescale.com>
> ---
> Updated to V2 version per Segher's suggestion.
>
>  Documentation/powerpc/booting-without-of.txt |   40 +++++++++++++++++++++++++-
>  1 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> index 1d2a772..887783c 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -57,7 +57,10 @@ Table of Contents
>       n) 4xx/Axon EMAC ethernet nodes
>       o) Xilinx IP cores
>       p) Freescale Synchronous Serial Interface
> -         q) USB EHCI controllers
> +      q) USB EHCI controllers
> +      r) Freescale Display Interface Unit
> +      s) Freescale on board FPGA
> +      t) Freescael MSI interrupt controller
>
>   VII - Marvell Discovery mv64[345]6x System Controller chips
>     1) The /system-controller node
> @@ -2870,6 +2873,41 @@ platforms are moved over to use the flattened-device-tree model.
>                reg = <0xe8000000 32>;
>        };
>
> +    t) Freescale MSI interrupt controller
> +
> +    Reguired properities:
> +    - compatible : set as "fsl,86xx-MSI" for 86xx cpu, "fsl,85xx-MSI" for 85xx
> +      cpu and "fsl,83xx-MSI" for 83xx cpu.

As Segher mentioned; don't use 'generic' compatible values.  Use names
of *actual* devices and have newer parts claim compatibility with
older ones.

Otherwise looks good to me.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 2/4 V4] MSI support on 83xx/85xx/86xx board

[Grant Likely]

On Fri, May 9, 2008 at 3:03 AM, Jason Jin <Jason.jin@freescale.com> wrote:
> This MSI driver can be used on 83xx/85xx/86xx board.
> In this driver, virtual interrupt host and chip were
> setup. There are 256 MSI interrupts in this host, Every 32
> MSI interrupts cascaded to one IPIC/MPIC interrupt.
> The chip was treated as edge sensitive and some necessary
> functions were setup for this chip.
>
> Before using the MSI interrupt, PCI/PCIE device need to
> ask for a MSI interrupt in the 256 MSI interrupts. A 256bit
> bitmap show which MSI interrupt was used, reserve bit in
> the bitmap can be used to force the device use some designate
> MSI interrupt in the 256 MSI interrupts. Sometimes this is useful
> for testing the all the MSI interrupts. The msi-available-ranges
> property in the dts file was used for this purpose.
>
> Signed-off-by: Jason Jin <Jason.jin@freescale.com>
> ---
> Update to V4 version per the suggestion's from Michael Ellerman
> +static struct of_platform_driver fsl_of_msi_driver = {
> +       .name = "fsl-of-msi",
> +       .match_table = fsl_of_msi_ids,
> +       .probe = fsl_of_msi_probe,
> +};

nitpick: 'of' in 'fsl-of-msi' is redundant.  We *already* know that
this is an of_platform driver.  :-)

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Grant Likely]

On Fri, May 9, 2008 at 3:03 AM, Jason Jin <Jason.jin@freescale.com> wrote:
> Binding document adding for Freescale MSI support.
>
> Signed-off-by: Jason Jin <Jason.jin@freescale.com>
> ---
> Updated to V2 version per Segher's suggestion.
>
>  Documentation/powerpc/booting-without-of.txt |   40 +++++++++++++++++++++++++-
>  1 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> index 1d2a772..887783c 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -57,7 +57,10 @@ Table of Contents
>       n) 4xx/Axon EMAC ethernet nodes
>       o) Xilinx IP cores
>       p) Freescale Synchronous Serial Interface
> -         q) USB EHCI controllers
> +      q) USB EHCI controllers
> +      r) Freescale Display Interface Unit
> +      s) Freescale on board FPGA
> +      t) Freescael MSI interrupt controller

One more comment: Here we see sections r), s) & t) added.

>
>   VII - Marvell Discovery mv64[345]6x System Controller chips
>     1) The /system-controller node
> @@ -2870,6 +2873,41 @@ platforms are moved over to use the flattened-device-tree model.
>                reg = <0xe8000000 32>;
>        };
>
> +    t) Freescale MSI interrupt controller
> +

But here, you are only adding section t)

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Timur Tabi]

Grant Likely wrote:

> But here, you are only adding section t)

That's because r) and s) already exist.  The person who added those sections
didn't update the TOC.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Grant Likely]

On Fri, May 9, 2008 at 10:18 AM, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
>> But here, you are only adding section t)
>
> That's because r) and s) already exist.  The person who added those sections
> didn't update the TOC.

heh.  I'm obviously being far too lazy in my patch review.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

RE: [PATCH 2/4 V4] MSI support on 83xx/85xx/86xx board

[Jin Zhengxiong]

>=20
> ...
>=20
> > +static int fsl_msi_host_match(struct irq_host *h, struct=20
> device_node=20
> > +*node) {
> > +	/* Exact match*/
> > +	return h->of_node =3D=3D node;
> > +}
>=20
> This is the default behaviour, so technically you don't need=20
> this routine - but I'm not fussed.

You are right, I noticed there is the default match function in=20
current kernel. I can try to remove this in my driver. Thanks.

> Acked-by: Michael Ellerman <michael@ellerman.id.au>

Thanks

Jason

RE: [PATCH 1/4 V2] booting-without-of for Freescale MSI

[Jin Zhengxiong]

=20

> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On=20
> Behalf Of Grant Likely
> Sent: Friday, May 09, 2008 9:25 PM
> To: Jin Zhengxiong
> Cc: galak@kernel.crashing.org; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 1/4 V2] booting-without-of for Freescale MSI
>=20
> On Fri, May 9, 2008 at 3:03 AM, Jason Jin=20
> <Jason.jin@freescale.com> wrote:
> > Binding document adding for Freescale MSI support.
> >
> > Signed-off-by: Jason Jin <Jason.jin@freescale.com>
> > ---
> > Updated to V2 version per Segher's suggestion.
> >
> >  Documentation/powerpc/booting-without-of.txt |   40=20
> +++++++++++++++++++++++++-
> >  1 files changed, 39 insertions(+), 1 deletions(-)
> >
> > diff --git a/Documentation/powerpc/booting-without-of.txt=20
> > b/Documentation/powerpc/booting-without-of.txt
> > index 1d2a772..887783c 100644
> > --- a/Documentation/powerpc/booting-without-of.txt
> > +++ b/Documentation/powerpc/booting-without-of.txt
> > @@ -57,7 +57,10 @@ Table of Contents
> >       n) 4xx/Axon EMAC ethernet nodes
> >       o) Xilinx IP cores
> >       p) Freescale Synchronous Serial Interface
> > -         q) USB EHCI controllers
> > +      q) USB EHCI controllers
> > +      r) Freescale Display Interface Unit
> > +      s) Freescale on board FPGA
> > +      t) Freescael MSI interrupt controller
> >
> >   VII - Marvell Discovery mv64[345]6x System Controller chips
> >     1) The /system-controller node
> > @@ -2870,6 +2873,41 @@ platforms are moved over to use the=20
> flattened-device-tree model.
> >                reg =3D <0xe8000000 32>;
> >        };
> >
> > +    t) Freescale MSI interrupt controller
> > +
> > +    Reguired properities:
> > +    - compatible : set as "fsl,86xx-MSI" for 86xx cpu,=20
> "fsl,85xx-MSI" for 85xx
> > +      cpu and "fsl,83xx-MSI" for 83xx cpu.
>=20
> As Segher mentioned; don't use 'generic' compatible values. =20
> Use names of *actual* devices and have newer parts claim=20
> compatibility with older ones.

The *actual* devices that can use this MSI driver, is *all* the=20
boards that use "86xx/85xx/83xx" cpu. This compatible note actually
was used to distinguish different msiir offset for the boards use=20
MPIC and IPIC.=20
If "MPIC-MSI" or "85xx-MSI" is 'generic'. Do we need to describe it=20
as "fsl,mpc8572ds-msi" and make all the boards who use MPIC compatible
to "fsl,mpc8572ds-msi", Or each of the boards have their own compatible
name=20
and each time we add a board to support MSI, add an item to the
of_device_id table?
Please suggest, Thanks=20

Jason