LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: I2C bus clock on MPC85XX systems
From: Wolfgang Grandegger @ 2009-12-22 16:18 UTC (permalink / raw)
  To: Felix Radensky; +Cc: linuxppc-dev
In-Reply-To: <4B30E7F5.8080201@embedded-sol.com>

Felix Radensky wrote:
> Hi,
> 
> Almost all MPC85XX based systems have the compatible=:"fsl-i2c" in
> respective
> i2c device tree nodes. This causes FSL i2c driver to use the following
> "backward
> compatible" values: FSR=0x31 DFSR=0x10. This is regardless of CCB clock
> frequency and i2c clock prescaler.
> 
> On my custom MPC8536 based board with 432MHz CCB clock this results in
> 65KHz i2c clock frequency (checked with scope). U-Boot correctly configures
> the clock to 400KHz.
> 
> I've fixed the problem by modifying device tree to use different
> compatible value,
> similar to what socrates board does. Is this the right approach ?

Are you aware of the properties described in
"Documentation/powerpc/dts-bindings/fsl/i2c.txt":

http://lxr.linux.no/#linux+v2.6.32/Documentation/powerpc/dts-bindings/fsl/i2c.txt

Wolfgang.

^ permalink raw reply

* RE: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.
From: Tirumala Reddy Marri @ 2009-12-22 17:55 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, writetomarri
In-Reply-To: <20091222120748.GS16448@hansolo.jdub.homelinux.org>

Josh,
 Thanks for the comments. I will fix them re-submit it.
Regards,
Marri

-----Original Message-----
From: Josh Boyer [mailto:jwboyer@gmail.com] On Behalf Of Josh Boyer
Sent: Tuesday, December 22, 2009 4:08 AM
To: Tirumala Reddy Marri
Cc: linuxppc-dev@lists.ozlabs.org; writetomarri@yahoo.com
Subject: Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

On Tue, Dec 22, 2009 at 12:49:51AM -0800, tmarri@amcc.com wrote:
>From: Tirumala Marri <tmarri@amcc.com>
>
>
>Signed-off-by: Tirumala Marri <tmarri@amcc.com>
>---
>Kernel version: 2.6.33-rc1
>Testing:
>	When 460SX configured as root as a end point E1000(Intell
Ethernet card)=20
>	was plugged into the one of the PCI-E ports. I was able to run
the traffic
>	with MSI interrupts.=20
>---
> 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           |  335
++++++++++++++++++++++++++++
> arch/powerpc/sysdev/ppc4xx_msi.h           |   49 ++++
> 7 files changed, 411 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.c
> create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.h
>
>diff --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 =3D "amcc,ppc4xx-msi", "ppc4xx-msi";
>+  			reg =3D < 0x4 0x00300000 0x100
>+  				0x4 0x00300000 0x100>;
>+  			sdr-base =3D <0x3B0>;
>+  			interrupts =3D<0 1 2 3>;
>+  			interrupt-parent =3D <&MSI>;
>+  			#interrupt-cells =3D <1>;
>+  			#address-cells =3D <0>;
>+  			#size-cells =3D <0>;
>+  			interrupt-map =3D <0 &UIC0 0xC 1
>+  				1 &UIC0 0x0D 1
>+  				2 &UIC0 0x0E 1
>+  				3 &UIC0 0x0F 1>;
>+  		};
>=20
> 	};
>=20
>diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
>index 3965828..32f5a40 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
>=20
>+config 4xx_MSI

This should probably be named PPC4xx_MSI, similar to how
PPC4xx_PCI_EXPRESS is named.

>+	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 4xx_MSI

>diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c
b/arch/powerpc/sysdev/ppc4xx_msi.c
>new file mode 100644
>index 0000000..752da4b
>--- /dev/null
>+++ b/arch/powerpc/sysdev/ppc4xx_msi.c
>@@ -0,0 +1,335 @@
>+/*
>+ * Copyright (C) 2009 Applied Micro Circuits corporation,
>+ * All rights reserved.

Please don't add the 'All rights reserved.' to new files.  It is
inaccurate and confusing given that it's a GPLv2 file.

>+ *
>+ * 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"
>+
>+
>+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 =3D msi_bitmap_alloc(&msi_data->bitmap, NR_MSI_IRQS,
>+				msi_data->irqhost->of_node);
>+	if (rc)
>+		return rc;
>+	rc =3D msi_bitmap_reserve_dt_hwirqs(&msi_data->bitmap);
>+	if (rc < 0) {
>+		msi_bitmap_free(&msi_data->bitmap);
>+		return rc;
>+	}
>+	return 0;
>+}
>+
>+
>+static void ppc4xx_msi_cascade(unsigned int irq, struct irq_desc
*desc)
>+{
>+	unsigned int cascade_irq;
>+	struct ppc4xx_msi *msi_data =3D ppc4xx_msi;
>+	int msir_index =3D -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 =3D (int)desc->handler_data;
>+
>+	if (msir_index >=3D NR_MSI_IRQS)
>+		cascade_irq =3D NO_IRQ;
>+
>+	desc->status |=3D IRQ_INPROGRESS;
>+
>+	cascade_irq =3D irq_linear_revmap(msi_data->irqhost, msir_index);
>+	if (cascade_irq !=3D NO_IRQ)
>+		generic_handle_irq(cascade_irq);
>+	desc->status &=3D ~IRQ_INPROGRESS;
>+
>+	if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
>+		desc->chip->unmask(irq);
>+unlock:
>+	raw_spin_unlock(&desc->lock);
>+}
>+static void ppc4xx_compose_msi_msg(struct pci_dev *pdev, int hwirq,
>+					struct msi_msg *msg)
>+{
>+	struct ppc4xx_msi *msi_data =3D ppc4xx_msi;
>+
>+	msg->address_lo =3D msi_data->msi_addr_lo;
>+	msg->address_hi =3D msi_data->msi_addr_hi;
>+	msg->data =3D hwirq;
>+}
>+
>+
>+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 =3D ppc4xx_msi;
>+
>+
>+	list_for_each_entry(entry, &dev->msi_list, list) {
>+		hwirq =3D msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
>+		if (hwirq < 0) {
>+			rc =3D 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 =3D irq_create_mapping(msi_data->irqhost, hwirq);
>+		if (virq =3D=3D NO_IRQ) {
>+			dev_err(&dev->dev, "%s: fail mapping irq\n",
__func__);
>+			rc =3D -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 =3D 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 =3D=3D 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);
>+
>+	}
>+
>+	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)
>+{
>+}
>+
>+
>+static struct irq_chip ppc4xx_msi_chip =3D {
>+	.mask           =3D mask_msi_irq,
>+	.unmask         =3D unmask_msi_irq,
>+	.ack            =3D ppc4xx_msi_end_irq,
>+	.name       =3D " UIC",
>+};
>+
>+static int ppc4xx_msi_host_map(struct irq_host *h, unsigned int virq,
>+				irq_hw_number_t hw)
>+{
>+	struct irq_chip *chip =3D &ppc4xx_msi_chip;
>+
>+	irq_to_desc(virq)->status |=3D 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 =3D {
>+	.map =3D 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;
>+	u32 *msi_virt =3D NULL;
>+	dma_addr_t msi_phys;
>+
>+
>+	msi =3D kzalloc(sizeof(struct ppc4xx_msi), GFP_KERNEL);
>+	if (!msi) {
>+		dev_err(&dev->dev, "No memory for MSI structure\n");
>+		rc =3D -ENOMEM;
>+		goto error_out;
>+	}
>+
>+	msi->irqhost =3D irq_alloc_host(dev->node, IRQ_HOST_MAP_LINEAR,
>+				      NR_MSI_IRQS, &ppc4xx_msi_host_ops,
0);
>+	if (msi->irqhost =3D=3D NULL) {
>+		dev_err(&dev->dev, "No memory for MSI irqhost\n");
>+		rc =3D -ENOMEM;
>+		goto error_out;
>+	}
>+
>+
>+	/* Get MSI ranges */
>+	rc =3D 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 =3D 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;
>+	}
>+#if  defined(CONFIG_460SX)
>+	mtdcri(SDR0, SDR0_PCIEH_H, PCIE_MSI_REG_BASE_H);
>+	mtdcri(SDR0, SDR0_PCIEH_L, PCIE_MSI_REG_BASE_L);
>+	msi->msi_regs =3D ioremap(((u64)PCIE_MSI_REG_BASE_H << 32) |
res.start,
>+			res.end - res.start + 1);

You defined sdr-base in the device tree.  Please use it instead of the
hard
coding.

>+#else
>+	dev_err(&dev->dev, " Invalid Device \n");
>+	goto error_out;
>+#endif
>+	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 =3D 0x0;
>+	msi_virt =3D dma_alloc_coherent(&dev->dev, 64, &msi_phys,
>+			GFP_KERNEL);
>+	if (msi_virt =3D=3D NULL) {
>+		dev_err(&dev->dev, "No memory for MSI mem space\n");
>+		rc =3D -ENOMEM;
>+		goto error_out;
>+	}
>+	msi->msi_addr_lo =3D (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 =3D msi;
>+
>+	if (ppc4xx_msi_init_allocator(msi)) {
>+		dev_err(&dev->dev, "Error allocating MSI bitmap\n");
>+		goto error_out;
>+	}
>+
>+	p =3D 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 =3D -ENODEV;
>+		goto error_out;
>+	}
>+	if (count =3D=3D 0) {
>+		dev_err(&dev->dev, "Malformed interrupts property on
%s\n",
>+				dev->node->full_name);
>+		rc =3D -EINVAL;
>+		goto error_out;
>+	}
>+
>+	for (i =3D 0; i < NR_MSI_IRQS; i++) {
>+		virt_msir =3D irq_of_parse_and_map(dev->node, i);
>+		if (virt_msir !=3D NO_IRQ) {
>+			set_irq_data(virt_msir, (void *)i);
>+			set_irq_chained_handler(virt_msir,
ppc4xx_msi_cascade);
>+		}
>+	}
>+
>+	ppc4xx_msi =3D msi;
>+
>+	WARN_ON(ppc_md.setup_msi_irqs);
>+	ppc_md.setup_msi_irqs =3D ppc4xx_setup_msi_irqs;
>+	ppc_md.teardown_msi_irqs =3D ppc4xx_teardown_msi_irqs;
>+	ppc_md.msi_check_device =3D 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 =3D {
>+	.ppc4xx_pic_ip =3D 0,
>+	.msiir_offset =3D 0x140,
>+};
>+
>+static const struct of_device_id ppc4xx_msi_ids[] =3D {
>+	{
>+		.compatible =3D "amcc,ppc4xx-msi",
>+		.data =3D (void *)&ppc4xx_msi_feature,
>+	},
>+	{}
>+};
>+
>+static struct of_platform_driver ppc4xx_msi_driver =3D {
>+	.name =3D "ppc4xx-msi",
>+	.match_table =3D ppc4xx_msi_ids,
>+	.probe =3D 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..7b8ac5c
>--- /dev/null
>+++ b/arch/powerpc/sysdev/ppc4xx_msi.h
>@@ -0,0 +1,49 @@
>+/*
>+ * Copyright (C) 2009 Applied Micro Circuits Corporation,
>+ * All rights reserved.

Same comment as above.

>+ *
>+ * Author: T	irumala 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
>+
>+#if defined(CONFIG_405Ex)
>+#define SDR0_PCIEH	0x4B1
>+#define PCIE_MSI_REG_BASE	0xef620000
>+#elif defined(CONFIG_440SPe) || defined(CONFIG_460SX)
>+#define SDR0_PCIEH_H	0x3B0
>+#define SDR0_PCIEH_L	0x3B1
>+#define PCIE_MSI_REG_BASE_L 0x00300000
>+#define PCIE_MSI_REG_BASE_H 0x00000004
>+#endif

sdr-base covers quite a bit of this.

>+
>+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;
>+};

Perhaps add an sdr_base member to this struct, similar to
how the pci port structure looks.

josh

^ permalink raw reply

* Re: I2C bus clock on MPC85XX systems
From: Felix Radensky @ 2009-12-22 18:16 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: linuxppc-dev
In-Reply-To: <4B30F165.2060300@grandegger.com>

Hi, Wolfgang

Wolfgang Grandegger wrote:
> Felix Radensky wrote:
>   
>> Hi,
>>
>> Almost all MPC85XX based systems have the compatible=:"fsl-i2c" in
>> respective
>> i2c device tree nodes. This causes FSL i2c driver to use the following
>> "backward
>> compatible" values: FSR=0x31 DFSR=0x10. This is regardless of CCB clock
>> frequency and i2c clock prescaler.
>>
>> On my custom MPC8536 based board with 432MHz CCB clock this results in
>> 65KHz i2c clock frequency (checked with scope). U-Boot correctly configures
>> the clock to 400KHz.
>>
>> I've fixed the problem by modifying device tree to use different
>> compatible value,
>> similar to what socrates board does. Is this the right approach ?
>>     
>
> Are you aware of the properties described in
> "Documentation/powerpc/dts-bindings/fsl/i2c.txt":
>
> http://lxr.linux.no/#linux+v2.6.32/Documentation/powerpc/dts-bindings/fsl/i2c.txt
>
> Wolfgang.
>
>   
Sure, I'm aware of these properties. I've used

compatible = "fsl,mpc8543-i2c", "fsl-i2c";
clock-frequency = <400000>;

for my custom board.

I think, however, that device trees for FSL reference designs should use 
them as well, to avoid setting i2c
clock to some strange values. I may be wrong, but I think most custom 
board developers borrow from
reference device trees, so having a sane starting point would help.

Thanks.

Felix.

^ permalink raw reply

* Re: I2C bus clock on MPC85XX systems
From: Wolfgang Grandegger @ 2009-12-22 18:40 UTC (permalink / raw)
  To: Felix Radensky; +Cc: linuxppc-dev
In-Reply-To: <4B310D10.4090307@embedded-sol.com>

Felix Radensky wrote:
> Hi, Wolfgang
> 
> Wolfgang Grandegger wrote:
>> Felix Radensky wrote:
>>  
>>> Hi,
>>>
>>> Almost all MPC85XX based systems have the compatible=:"fsl-i2c" in
>>> respective
>>> i2c device tree nodes. This causes FSL i2c driver to use the following
>>> "backward
>>> compatible" values: FSR=0x31 DFSR=0x10. This is regardless of CCB clock
>>> frequency and i2c clock prescaler.
>>>
>>> On my custom MPC8536 based board with 432MHz CCB clock this results in
>>> 65KHz i2c clock frequency (checked with scope). U-Boot correctly
>>> configures
>>> the clock to 400KHz.
>>>
>>> I've fixed the problem by modifying device tree to use different
>>> compatible value,
>>> similar to what socrates board does. Is this the right approach ?
>>>     
>>
>> Are you aware of the properties described in
>> "Documentation/powerpc/dts-bindings/fsl/i2c.txt":
>>
>> http://lxr.linux.no/#linux+v2.6.32/Documentation/powerpc/dts-bindings/fsl/i2c.txt
>>
>>
>> Wolfgang.
>>
>>   
> Sure, I'm aware of these properties. I've used
> 
> compatible = "fsl,mpc8543-i2c", "fsl-i2c";
> clock-frequency = <400000>;
> 
> for my custom board.
> 
> I think, however, that device trees for FSL reference designs should use
> them as well, to avoid setting i2c
> clock to some strange values. I may be wrong, but I think most custom

The FSR=0x31 DFSR=0x10 are not strange values. They have been used in
mainline kernels (< 2.6.30) for a long time.

> board developers borrow from
> reference device trees, so having a sane starting point would help.

The trees provided by Freescale are usually based on older kernel
version. Borrow from such trees is OK, but the project developers should
use a recent kernel version for development.

Wolfgang.

^ permalink raw reply

* Re: I2C bus clock on MPC85XX systems
From: Felix Radensky @ 2009-12-22 19:06 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: linuxppc-dev
In-Reply-To: <4B311281.6020801@grandegger.com>

Hi, Wolfgang

Wolfgang Grandegger wrote:
> Felix Radensky wrote:
>   
>> Hi, Wolfgang
>>
>> Wolfgang Grandegger wrote:
>>     
>>> Felix Radensky wrote:
>>>  
>>>       
>>>> Hi,
>>>>
>>>> Almost all MPC85XX based systems have the compatible=:"fsl-i2c" in
>>>> respective
>>>> i2c device tree nodes. This causes FSL i2c driver to use the following
>>>> "backward
>>>> compatible" values: FSR=0x31 DFSR=0x10. This is regardless of CCB clock
>>>> frequency and i2c clock prescaler.
>>>>
>>>> On my custom MPC8536 based board with 432MHz CCB clock this results in
>>>> 65KHz i2c clock frequency (checked with scope). U-Boot correctly
>>>> configures
>>>> the clock to 400KHz.
>>>>
>>>> I've fixed the problem by modifying device tree to use different
>>>> compatible value,
>>>> similar to what socrates board does. Is this the right approach ?
>>>>     
>>>>         
>>> Are you aware of the properties described in
>>> "Documentation/powerpc/dts-bindings/fsl/i2c.txt":
>>>
>>> http://lxr.linux.no/#linux+v2.6.32/Documentation/powerpc/dts-bindings/fsl/i2c.txt
>>>
>>>
>>> Wolfgang.
>>>
>>>   
>>>       
>> Sure, I'm aware of these properties. I've used
>>
>> compatible = "fsl,mpc8543-i2c", "fsl-i2c";
>> clock-frequency = <400000>;
>>
>> for my custom board.
>>
>> I think, however, that device trees for FSL reference designs should use
>> them as well, to avoid setting i2c
>> clock to some strange values. I may be wrong, but I think most custom
>>     
>
> The FSR=0x31 DFSR=0x10 are not strange values. They have been used in
> mainline kernels (< 2.6.30) for a long time.
>
>   
>> board developers borrow from
>> reference device trees, so having a sane starting point would help.
>>     
>
> The trees provided by Freescale are usually based on older kernel
> version. Borrow from such trees is OK, but the project developers should
> use a recent kernel version for development.
>   

I was talking about modern vanilla kernels for Freescale reference 
boards. See, for example
http://lxr.linux.no/#linux+v2.6.32/arch/powerpc/boot/dts/mpc8536ds.dts

Felix.

^ permalink raw reply

* Re: I2C bus clock on MPC85XX systems
From: Wolfgang Grandegger @ 2009-12-22 19:20 UTC (permalink / raw)
  To: Felix Radensky; +Cc: linuxppc-dev
In-Reply-To: <4B3118A9.6000704@embedded-sol.com>

Hi Felix,

Felix Radensky wrote:
> Hi, Wolfgang
> 
> Wolfgang Grandegger wrote:
[snip]
>> The trees provided by Freescale are usually based on older kernel
>> version. Borrow from such trees is OK, but the project developers should
>> use a recent kernel version for development.
>>   
> 
> I was talking about modern vanilla kernels for Freescale reference
> boards. See, for example
> http://lxr.linux.no/#linux+v2.6.32/arch/powerpc/boot/dts/mpc8536ds.dts

Only a few boards use these new properties to set a reasonable I2C
clock. But I agree, adding "clock-frequency = <100000>;" would make it
more obvious to the users how to define a proper frequency.

Wolfgang.

^ permalink raw reply

* Does gpio_to_irq() work for MPC52xx gpios?
From: Bill Gatliff @ 2009-12-22 20:40 UTC (permalink / raw)
  To: Linux/PPC Development

Guys:


Is it possible to specify an individual GPIO pin as an interrupt source
with the current MPC52xx code?

I don't see anything in mpc52xx_gpio.c that registers an interrupt
handler, which I think would be a necessary step towards demultiplexing
the GPIO interrupt event.  So I'm thinking that code to allow me to
register an interrupt handler for a specific GPIO pin just isn't there...


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

^ permalink raw reply

* Re: Does gpio_to_irq() work for MPC52xx gpios?
From: Peter Korsgaard @ 2009-12-22 21:00 UTC (permalink / raw)
  To: Bill Gatliff; +Cc: Linux/PPC Development
In-Reply-To: <4B312EDA.7030606@billgatliff.com>

>>>>> "Bill" == Bill Gatliff <bgat@billgatliff.com> writes:

 Bill> Guys:
 Bill> Is it possible to specify an individual GPIO pin as an interrupt source
 Bill> with the current MPC52xx code?

No (not yet). In Ben's latest pull request there's a patch from me to
add basic infrastructure for gpio_to_irq(). I've recently added irq
support to the mpc8xxx driver, but so far nothing has been written for
52xx.

http://patchwork.ozlabs.org/patch/41550/

-- 
Bye, Peter Korsgaard

^ permalink raw reply

* [PATCH 1/3] phylib: Fix deadlock on resume
From: Anton Vorontsov @ 2009-12-22 22:14 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-pm, Andy Fleming, linuxppc-dev

Sometimes kernel hangs on resume with the following trace:

 ucc_geth e0102000.ucc: resume
 INFO: task bash:1764 blocked for more than 120 seconds.
 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
 bash          D 0fecf43c     0  1764   1763 0x00000000
 Call Trace:
 [cf9a7c10] [c0012868] ret_from_except+0x0/0x14 (unreliable)
 --- Exception: cf9a7ce0 at __switch_to+0x4c/0x6c
     LR = 0xcf9a7cc0
 [cf9a7cd0] [c0008c14] __switch_to+0x4c/0x6c (unreliable)
 [cf9a7ce0] [c028bcfc] schedule+0x158/0x260
 [cf9a7d10] [c028c720] __mutex_lock_slowpath+0x80/0xd8
 [cf9a7d40] [c01cf388] phy_stop+0x20/0x70
 [cf9a7d50] [c01d514c] ugeth_resume+0x6c/0x13c
 [...]

Here is why.

On suspend:

- PM core starts suspending devices, ucc_geth_suspend gets called;

- ucc_geth calls phy_stop() on suspend. Note that phy_stop() is
  mostly asynchronous so it doesn't block ucc_geth's suspend routine,
  it just sets PHY_HALTED state and disables PHY's interrupts;

- Suddenly the state machine gets scheduled, it grabs the phydev->lock
  mutex and tries to process the PHY_HALTED state, so it calls
  phydev->adjust_link(phydev->attached_dev). In ucc_geth case
  adjust_link() calls msleep(), which reschedules the code flow back to
  PM core, which now finishes suspend and so we end up sleeping with
  phydev->lock mutex held.

On resume:

- PM core starts resuming devices (notice that nobody rescheduled
  the state machine yet, so the mutex is still held), the core calls
  ucc_geth's resume routine;

- ucc_geth_resume restarts the PHY with phy_stop()/phy_start()
  sequence, and the phy_*() calls are trying to grab the phydev->lock
  mutex. Here comes the deadlock.

This patch fixes the issue by stopping the state machine on suspend
and starting it again on resume.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/phy/mdio_bus.c |   24 ++++++++++++++++++++++--
 1 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index bd4e8d7..49252d3 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -303,8 +303,18 @@ static int mdio_bus_suspend(struct device * dev, pm_message_t state)
 	struct phy_driver *phydrv = to_phy_driver(dev->driver);
 	struct phy_device *phydev = to_phy_device(dev);
 
+	/*
+	 * We must stop the state machine manually, otherwise it stops out of
+	 * control, possibly with the phydev->lock held. Upon resume, netdev
+	 * may call phy routines that try to grab the same lock, and that may
+	 * lead to a deadlock.
+	 */
+	if (phydev->attached_dev)
+		phy_stop_machine(phydev);
+
 	if (!mdio_bus_phy_may_suspend(phydev))
 		return 0;
+
 	return phydrv->suspend(phydev);
 }
 
@@ -312,10 +322,20 @@ static int mdio_bus_resume(struct device * dev)
 {
 	struct phy_driver *phydrv = to_phy_driver(dev->driver);
 	struct phy_device *phydev = to_phy_device(dev);
+	int ret;
 
 	if (!mdio_bus_phy_may_suspend(phydev))
-		return 0;
-	return phydrv->resume(phydev);
+		goto no_resume;
+
+	ret = phydrv->resume(phydev);
+	if (ret < 0)
+		return ret;
+
+no_resume:
+	if (phydev->attached_dev)
+		phy_start_machine(phydev, NULL);
+
+	return 0;
 }
 
 struct bus_type mdio_bus_type = {
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 2/3] phylib: Properly reinitialize PHYs after hibernation
From: Anton Vorontsov @ 2009-12-22 22:14 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-pm, Andy Fleming, linuxppc-dev

Since hibernation assumes power loss, we should fully reinitialize
PHYs (including platform fixups), as if PHYs were just attached.

This patch factors phy_init_hw() out of phy_attach_direct(), then
converts mdio_bus to dev_pm_ops and adds an appropriate restore()
callback.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/phy/mdio_bus.c   |   50 ++++++++++++++++++++++++++++++++++++------
 drivers/net/phy/phy_device.c |   30 ++++++++++++------------
 include/linux/phy.h          |    1 +
 3 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 49252d3..667fc09 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -295,10 +295,9 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 	return true;
 }
 
-/* Suspend and resume.  Copied from platform_suspend and
- * platform_resume
- */
-static int mdio_bus_suspend(struct device * dev, pm_message_t state)
+#ifdef CONFIG_PM
+
+static int mdio_bus_suspend(struct device *dev)
 {
 	struct phy_driver *phydrv = to_phy_driver(dev->driver);
 	struct phy_device *phydev = to_phy_device(dev);
@@ -318,7 +317,7 @@ static int mdio_bus_suspend(struct device * dev, pm_message_t state)
 	return phydrv->suspend(phydev);
 }
 
-static int mdio_bus_resume(struct device * dev)
+static int mdio_bus_resume(struct device *dev)
 {
 	struct phy_driver *phydrv = to_phy_driver(dev->driver);
 	struct phy_device *phydev = to_phy_device(dev);
@@ -338,11 +337,48 @@ no_resume:
 	return 0;
 }
 
+static int mdio_bus_restore(struct device *dev)
+{
+	struct phy_device *phydev = to_phy_device(dev);
+	struct net_device *netdev = phydev->attached_dev;
+	int ret;
+
+	if (!netdev)
+		return 0;
+
+	ret = phy_init_hw(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* The PHY needs to renegotiate. */
+	phydev->link = 0;
+	phydev->state = PHY_UP;
+
+	phy_start_machine(phydev, NULL);
+
+	return 0;
+}
+
+static struct dev_pm_ops mdio_bus_pm_ops = {
+	.suspend = mdio_bus_suspend,
+	.resume = mdio_bus_resume,
+	.freeze = mdio_bus_suspend,
+	.thaw = mdio_bus_resume,
+	.restore = mdio_bus_restore,
+};
+
+#define MDIO_BUS_PM_OPS (&mdio_bus_pm_ops)
+
+#else
+
+#define MDIO_BUS_PM_OPS NULL
+
+#endif /* CONFIG_PM */
+
 struct bus_type mdio_bus_type = {
 	.name		= "mdio_bus",
 	.match		= mdio_bus_match,
-	.suspend	= mdio_bus_suspend,
-	.resume		= mdio_bus_resume,
+	.pm		= MDIO_BUS_PM_OPS,
 };
 EXPORT_SYMBOL(mdio_bus_type);
 
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b10fedd..8212b2b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -378,6 +378,20 @@ void phy_disconnect(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_disconnect);
 
+int phy_init_hw(struct phy_device *phydev)
+{
+	int ret;
+
+	if (!phydev->drv || !phydev->drv->config_init)
+		return 0;
+
+	ret = phy_scan_fixups(phydev);
+	if (ret < 0)
+		return ret;
+
+	return phydev->drv->config_init(phydev);
+}
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -425,21 +439,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	/* Do initial configuration here, now that
 	 * we have certain key parameters
 	 * (dev_flags and interface) */
-	if (phydev->drv->config_init) {
-		int err;
-
-		err = phy_scan_fixups(phydev);
-
-		if (err < 0)
-			return err;
-
-		err = phydev->drv->config_init(phydev);
-
-		if (err < 0)
-			return err;
-	}
-
-	return 0;
+	return phy_init_hw(phydev);
 }
 EXPORT_SYMBOL(phy_attach_direct);
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index b1368b8..7968def 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -447,6 +447,7 @@ struct phy_device* get_phy_device(struct mii_bus *bus, int addr);
 int phy_device_register(struct phy_device *phy);
 int phy_clear_interrupt(struct phy_device *phydev);
 int phy_config_interrupt(struct phy_device *phydev, u32 interrupts);
+int phy_init_hw(struct phy_device *phydev);
 int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 		u32 flags, phy_interface_t interface);
 struct phy_device * phy_attach(struct net_device *dev,
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 3/3] ucc_geth: Fix netdev watchdog triggering on suspend
From: Anton Vorontsov @ 2009-12-22 22:15 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-pm, Andy Fleming, linuxppc-dev

Sometimes ucc_geth fails to suspend with the following trace:

 ucc_geth e0103000.ucc: suspend
 ucc_geth e0102000.ucc: suspend
 NETDEV WATCHDOG: eth0 (ucc_geth): transmit queue 0 timed out
 ------------[ cut here ]------------
 Badness at net/sched/sch_generic.c:255
 NIP: c021cb5c LR: c021cb5c CTR: c01ab4b4
 [...]
 NIP [c021cb5c] dev_watchdog+0x298/0x2a8
 LR [c021cb5c] dev_watchdog+0x298/0x2a8
 Call Trace:
 [c0389da0] [c021cb5c] dev_watchdog+0x298/0x2a8 (unreliable)
 [c0389e00] [c0031ed8] run_timer_softirq+0x16c/0x1dc
 [c0389e50] [c002c638] __do_softirq+0xa4/0x11c
 [...]

This patch fixes the issue by properly detaching the device on
suspend, and attaching it back on resume.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/ucc_geth.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index afaf088..7225d5d 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3601,6 +3601,7 @@ static int ucc_geth_suspend(struct of_device *ofdev, pm_message_t state)
 	if (!netif_running(ndev))
 		return 0;
 
+	netif_device_detach(ndev);
 	napi_disable(&ugeth->napi);
 
 	/*
@@ -3659,7 +3660,7 @@ static int ucc_geth_resume(struct of_device *ofdev)
 	phy_start(ugeth->phydev);
 
 	napi_enable(&ugeth->napi);
-	netif_start_queue(ndev);
+	netif_device_attach(ndev);
 
 	return 0;
 }
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH] Make cpu hotplug driver lock part of ppc_md
From: Michael Ellerman @ 2009-12-22 22:29 UTC (permalink / raw)
  To: Nathan Fontenot; +Cc: linuxppc-dev, Andreas Schwab
In-Reply-To: <4B30DB8B.3030305@austin.ibm.com>

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

On Tue, 2009-12-22 at 08:45 -0600, Nathan Fontenot wrote:
> The recently introduced cpu_hotplug_driver_lock used to serialize
> cpu hotplug operations, namely for the pseries platform, causes a build
> issue for other platforms.  The base cpu hotplug code attempts
> to take this lock, but it may not be needed for all platforms.  This patch
> moves the lock/unlock routines to be part of the ppc_md structure
> so that platforms needing the lock can take it.  This also makes the
> previous cpu_hotplug_driver_lock, defined in pseries code, pseries specific.
> 
> The past failure without this patch was seen when building pmac and may
> be present in other platform builds.  The error is included below for reference.
> 
> drivers/built-in.o: In function `.store_online':
> cpu.c:(.ref.text+0xf5c): undefined reference to `.cpu_hotplug_driver_lock'
> cpu.c:(.ref.text+0xfc8): undefined reference to `.cpu_hotplug_driver_unlock'
> make: *** [.tmp_vmlinux1] Error 1

Why does the pmac code /not/ need a lock? And would it be harmless if it
was locked too?

If so, you could just make the mutex available to all powerpc code, and
rename it, and then we wouldn't need all this jiggery pokery just to
take & release a lock.

cheers

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

^ permalink raw reply

* Re: [PATCH -tip tracing/kprobes] PPC: Powerpc port of the kprobe-based event tracer
From: Masami Hiramatsu @ 2009-12-23  1:32 UTC (permalink / raw)
  To: Michael Neuling; +Cc: Mahesh Jagannath Salgaonkar, linuxppc-dev
In-Reply-To: <9823.1261342444@neuling.org>

Michael Neuling wrote:
> In message <4B2B934C.1060807@redhat.com> you wrote:
>>
>>
>> Mahesh Jagannath Salgaonkar wrote:
>>> Michael Neuling wrote:
>>>> In message <4B29EE5F.9020801@linux.vnet.ibm.com> you wrote:
>>>>> Hi Michael,
>>>>>
>>>>> Michael Neuling wrote:
>>>>>>> + * regs_get_argument_nth() - get Nth argument at function call
>>>>>>> + * @regs:    pt_regs which contains registers at function entry.
>>>>>>> + * @n:        argument number.
>>>>>>> + *
>>>>>>> + * regs_get_argument_nth() returns @n th argument of a function call.
>>>>>>> + * Since usually the kernel stack will be changed right after
>>>>>>> function en
>>>> try
>>>>>> ,
>>>>>>> + * you must use this at function entry. If the @n th entry is NOT
>>>>>>> in the
>>>>>>> + * kernel stack or pt_regs, this returns 0.
>>>>>>> + */
>>>>>>> +unsigned long regs_get_argument_nth(struct pt_regs *regs, unsigned
>>>>>>> int n)
>>>>>>> +{
>>>>>>> +    if (n < ARRAY_SIZE(arg_offs_table))
>>>>>>> +        return *(unsigned long *)((char *)regs + arg_offs_table[n]);
>>>>>>> +    else {
>>>>>>> +        /*
>>>>>>> +         * If more arguments are passed that can be stored in
>>>>>>> +         * registers, the remaining arguments are stored in the
>>>>>>> +         * parameter save area located at fixed offset from stack
>>>>>>> +         * pointer.
>>>>>>> +         * Following the PowerPC ABI, the first few arguments are
>>>>>>> +         * actually passed in registers (r3-r10), with equivalent
>>>>>>> space
>>>>>>> +         * left unused in the parameter save area.
>>>>>>> +         */
>>>>>>> +        n += (PARAMETER_SAVE_AREA_OFFSET / sizeof(unsigned long));
>>>>>>> +        return regs_get_kernel_stack_nth(regs, n);
>>>>>> How do we handle FP args?
>>>>> Currently this patch does not support FP args.
>>>>
>>>> This might be OK.  I don't think we use floating point parameters in any
>>>> function definitions in the kernel. 
>>>> We do use altivec in the raid6 driver (drivers/md/raid6altivec.uc) but
>>>> they are static inline, so they probably don't even end up as
>>>> functions. 
>>>> I guess we need to make sure that we're not limiting the interface in
>>>> such a way that we can't support it later if the above changes. 
>>>> regs_get_argument_nth returns an unsigned long which makes returning a
>>>> 128 bit VMX register impossible.  This might be a show stopper for me.
>>>> How are the x86 guys dealing with this?
>>>>
>>> Nope, x86 does not deal with bigger registers (Masami, correct me if I
>>> am wrong). The return data type is opaque to user. Hence this enables us
>>> to handle any such situations in future without effecting user space API.
>>
>> Right, we don't use those bigger registers in the kernel context.
>> (some special functions use it inside, but most of codes
>>  are just using general regs)
> 
> Sure, but kernel interfaces are for now.  What happens if this changes in
> the future?
> 
>> And regs_get_argument_nth is just an accessor of pt_regs field.
>> This means that it just provides field in pt_regs.
> 
> Sure, that's how it's implemented, but that's not what the name of the
> function suggests it does.

Hmm, OK, maybe I see what you think.
Actually, this function doesn't cover all functions in the kernel
even on x86, because each ABI depends on how function is defined,
e.g. func(fmt,...) and asmlinkage(i386) function will put all
arguments on the stack.

Originally, I had introduced this function only for helping user
of kprobe-tracer who wants to trace function arguments without
any arch specific knowledge. However, now we have perf-probe which
can analyze debuginfo to find function arguments and local variables.
So, this function is a bit out-of-dated :-)

How about removing this function from this patch? I can also
update kprobe-tracer to drop the function argument support.
Instead of that, I think it is enough to add a description of
how to get function arguments in Documentation/trace/kprobetrace.txt

Thank you,

> 
> Mikey
> 
>>
>>>>>>> +    }
>>>>>>> +}
>>>>>>> +/*
>>>>>>>   * does not yet catch signals sent when the child dies.
>>>>>>>   * in exit.c or in signal.c.
>>>>>>>   */
>>>>>>> Index: linux-2.6-tip/kernel/trace/Kconfig
>>>>>>> ===================================================================
>>>>>>> --- linux-2.6-tip.orig/kernel/trace/Kconfig
>>>>>>> +++ linux-2.6-tip/kernel/trace/Kconfig
>>>>>>> @@ -464,7 +464,7 @@ config BLK_DEV_IO_TRACE
>>>>>>>  
>>>>>>>  config KPROBE_EVENT
>>>>>>>      depends on KPROBES
>>>>>>> -    depends on X86
>>>>>>> +    depends on X86 || PPC
>>>>>>>      bool "Enable kprobes-based dynamic events"
>>>>>>>      select TRACING
>>>>>>>      default y
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Linuxppc-dev mailing list
>>>>>>> Linuxppc-dev@lists.ozlabs.org
>>>>>>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>>>>>>>
>>>>> Thanks for reviewing.
>>>>
>>>> We are creating a new user space API here, so I'm keen for others to take
>>>> a good look at the interface before we commit to something we are going
>>>> to have to keep forever. 
>>>> Who is the main consumer of this (/me is pretty ignorant of kprobes)?
>>>> What do they think of the interface?
>>>>
>>> The user space API are already present in the upstream kernel and
>>> currently only supported architecture is x86. This patch provides ppc
>>> architecture specific interfaces that enables powerpc also in par with x86.
>>
>> Yes, there is a kprobe tracer in ftrace (see Documentation/trace/kprobetrace.
> txt).
>> and this tracer is only for probing kernel (not userspace).
>>
>>>
>>> The main consumer would be kernel developers who would like to see
>>> register values, arguments and stack when the probe hits at given text
>>> address.
>>
>> Right.
>>
>> BTW, there is a user-space tools we have (tools/perf/builtin-probe.c).
>> Currently, it's only for x86. Mahesh, You just need to add a register
>> translation table in tools/perf/util/probe-finder.c for ppc support.
>>
>> Thank you!
>>
>> -- 
>> Masami Hiramatsu
>>
>> Software Engineer
>> Hitachi Computer Products (America), Inc.
>> Software Solutions Division
>>
>> e-mail: mhiramat@redhat.com
>>

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

^ permalink raw reply

* [PATCH 0/3] Build fixes for test-devicetree
From: Jeremy Kerr @ 2009-12-23  1:45 UTC (permalink / raw)
  To: Grant Likely, devicetree-discuss; +Cc: linuxppc-dev, linux-kernel

Hi all,

A small series of patches to fix minor build breakages when compiling
gcl's test-devicetree tree on ARM.

I've CC-ed lkml & linuxppc-dev for the fs/proc/ changes, but these
patches are only applicable to gcl's tree at present.

Cheers,


Jeremy

---
Jeremy Kerr (3):
      of: include linux/proc_fs.h
      of: make set_node_proc_entry private to proc_devtree.c
      proc_devtree: include linux/of.h

^ permalink raw reply

* [PATCH 1/3] of: include linux/proc_fs.h
From: Jeremy Kerr @ 2009-12-23  1:45 UTC (permalink / raw)
  To: Grant Likely, devicetree-discuss; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1261532718.575048.365032094903.0.gpush@pororo>

We use a few procfs-specific functions (eg, proc_device_tree_*) which
aren't covered by the current includes. This causes the following build
error on arm:

drivers/of/base.c: In function 'prom_add_property':
drivers/of/base.c:861: error: implicit declaration of function 'proc_device_tree_add_prop'
drivers/of/base.c: In function 'prom_remove_property':
drivers/of/base.c:902: error: implicit declaration of function 'proc_device_tree_remove_prop'
drivers/of/base.c: In function 'prom_update_property':
drivers/of/base.c:946: error: implicit declaration of function 'proc_device_tree_update_prop'

Add proc_fs.h for these prototypes.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 drivers/of/base.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 974f64e..2dcb7a3 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/spinlock.h>
+#include <linux/proc_fs.h>
 
 struct device_node *allnodes;
 

^ permalink raw reply related

* [PATCH 2/3] of: make set_node_proc_entry private to proc_devtree.c
From: Jeremy Kerr @ 2009-12-23  1:45 UTC (permalink / raw)
  To: Grant Likely, devicetree-discuss; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1261532718.575048.365032094903.0.gpush@pororo>

We only need set_node_proc_entry in proc_devtree.c, so move it there.

This fixes the !HAVE_ARCH_DEVTREE_FIXUPS build, as we can't make make
the definition in linux/of.h conditional on this #define (definitions in
asm/prom.h can't be exposed to linux/of.h, due to the enforced #include
ordering).

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 fs/proc/proc_devtree.c |    5 +++--
 include/linux/of.h     |    6 ------
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/fs/proc/proc_devtree.c b/fs/proc/proc_devtree.c
index 7ba79a5..31fc7e0 100644
--- a/fs/proc/proc_devtree.c
+++ b/fs/proc/proc_devtree.c
@@ -13,12 +13,13 @@
 #include <asm/uaccess.h>
 #include "internal.h"
 
-#ifndef HAVE_ARCH_DEVTREE_FIXUPS
 static inline void set_node_proc_entry(struct device_node *np,
 				       struct proc_dir_entry *de)
 {
-}
+#ifdef HAVE_ARCH_DEVTREE_FIXUPS
+	np->pde = de;
 #endif
+}
 
 static struct proc_dir_entry *proc_device_tree;
 
diff --git a/include/linux/of.h b/include/linux/of.h
index 67c9206..9078747 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -75,12 +75,6 @@ static inline void of_node_set_flag(struct device_node *n, unsigned long flag)
 	set_bit(flag, &n->_flags);
 }
 
-static inline void
-set_node_proc_entry(struct device_node *dn, struct proc_dir_entry *de)
-{
-	dn->pde = de;
-}
-
 extern struct device_node *of_find_all_nodes(struct device_node *prev);
 
 #if defined(CONFIG_SPARC)

^ permalink raw reply related

* [PATCH 3/3] proc_devtree: include linux/of.h
From: Jeremy Kerr @ 2009-12-23  1:45 UTC (permalink / raw)
  To: Grant Likely, devicetree-discuss; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1261532718.575048.365032094903.0.gpush@pororo>

Currenly, proc_devtree.c depends on asm/prom.h to include linux/of.h, to
provide some device-tree definitions (eg, struct property).

Instead, include linux/of.h directly. We still need asm/prom.h for
HAVE_ARCH_DEVTREE_FIXUPS.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 fs/proc/proc_devtree.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/proc/proc_devtree.c b/fs/proc/proc_devtree.c
index 31fc7e0..2ea10dd 100644
--- a/fs/proc/proc_devtree.c
+++ b/fs/proc/proc_devtree.c
@@ -9,6 +9,7 @@
 #include <linux/proc_fs.h>
 #include <linux/stat.h>
 #include <linux/string.h>
+#include <linux/of.h>
 #include <asm/prom.h>
 #include <asm/uaccess.h>
 #include "internal.h"

^ permalink raw reply related

* Re: [PATCH -tip tracing/kprobes] PPC: Powerpc port of the kprobe-based event tracer
From: Michael Neuling @ 2009-12-23  2:37 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Mahesh Jagannath Salgaonkar, linuxppc-dev
In-Reply-To: <4B317324.3000708@redhat.com>



In message <4B317324.3000708@redhat.com> you wrote:
> Michael Neuling wrote:
> > In message <4B2B934C.1060807@redhat.com> you wrote:
> >>
> >>
> >> Mahesh Jagannath Salgaonkar wrote:
> >>> Michael Neuling wrote:
> >>>> In message <4B29EE5F.9020801@linux.vnet.ibm.com> you wrote:
> >>>>> Hi Michael,
> >>>>>
> >>>>> Michael Neuling wrote:
> >>>>>>> + * regs_get_argument_nth() - get Nth argument at function call
> >>>>>>> + * @regs:    pt_regs which contains registers at function entry.
> >>>>>>> + * @n:        argument number.
> >>>>>>> + *
> >>>>>>> + * regs_get_argument_nth() returns @n th argument of a function call
.
> >>>>>>> + * Since usually the kernel stack will be changed right after
> >>>>>>> function en
> >>>> try
> >>>>>> ,
> >>>>>>> + * you must use this at function entry. If the @n th entry is NOT
> >>>>>>> in the
> >>>>>>> + * kernel stack or pt_regs, this returns 0.
> >>>>>>> + */
> >>>>>>> +unsigned long regs_get_argument_nth(struct pt_regs *regs, unsigned
> >>>>>>> int n)
> >>>>>>> +{
> >>>>>>> +    if (n < ARRAY_SIZE(arg_offs_table))
> >>>>>>> +        return *(unsigned long *)((char *)regs + arg_offs_table[n]);
> >>>>>>> +    else {
> >>>>>>> +        /*
> >>>>>>> +         * If more arguments are passed that can be stored in
> >>>>>>> +         * registers, the remaining arguments are stored in the
> >>>>>>> +         * parameter save area located at fixed offset from stack
> >>>>>>> +         * pointer.
> >>>>>>> +         * Following the PowerPC ABI, the first few arguments are
> >>>>>>> +         * actually passed in registers (r3-r10), with equivalent
> >>>>>>> space
> >>>>>>> +         * left unused in the parameter save area.
> >>>>>>> +         */
> >>>>>>> +        n += (PARAMETER_SAVE_AREA_OFFSET / sizeof(unsigned long));
> >>>>>>> +        return regs_get_kernel_stack_nth(regs, n);
> >>>>>> How do we handle FP args?
> >>>>> Currently this patch does not support FP args.
> >>>>
> >>>> This might be OK.  I don't think we use floating point parameters in any
> >>>> function definitions in the kernel. 
> >>>> We do use altivec in the raid6 driver (drivers/md/raid6altivec.uc) but
> >>>> they are static inline, so they probably don't even end up as
> >>>> functions. 
> >>>> I guess we need to make sure that we're not limiting the interface in
> >>>> such a way that we can't support it later if the above changes. 
> >>>> regs_get_argument_nth returns an unsigned long which makes returning a
> >>>> 128 bit VMX register impossible.  This might be a show stopper for me.
> >>>> How are the x86 guys dealing with this?
> >>>>
> >>> Nope, x86 does not deal with bigger registers (Masami, correct me if I
> >>> am wrong). The return data type is opaque to user. Hence this enables us
> >>> to handle any such situations in future without effecting user space API.
> >>
> >> Right, we don't use those bigger registers in the kernel context.
> >> (some special functions use it inside, but most of codes
> >>  are just using general regs)
> > 
> > Sure, but kernel interfaces are for now.  What happens if this changes in
> > the future?
> > 
> >> And regs_get_argument_nth is just an accessor of pt_regs field.
> >> This means that it just provides field in pt_regs.
> > 
> > Sure, that's how it's implemented, but that's not what the name of the
> > function suggests it does.
> 
> Hmm, OK, maybe I see what you think.
> Actually, this function doesn't cover all functions in the kernel
> even on x86, because each ABI depends on how function is defined,
> e.g. func(fmt,...) and asmlinkage(i386) function will put all
> arguments on the stack.
> 
> Originally, I had introduced this function only for helping user
> of kprobe-tracer who wants to trace function arguments without
> any arch specific knowledge. However, now we have perf-probe which
> can analyze debuginfo to find function arguments and local variables.
> So, this function is a bit out-of-dated :-)
> 
> How about removing this function from this patch? I can also
> update kprobe-tracer to drop the function argument support.
> Instead of that, I think it is enough to add a description of
> how to get function arguments in Documentation/trace/kprobetrace.txt

Sounds like a better solution to me.  

Cheers,
Mikey

> Thank you,
> 
> > 
> > Mikey
> > 
> >>
> >>>>>>> +    }
> >>>>>>> +}
> >>>>>>> +/*
> >>>>>>>   * does not yet catch signals sent when the child dies.
> >>>>>>>   * in exit.c or in signal.c.
> >>>>>>>   */
> >>>>>>> Index: linux-2.6-tip/kernel/trace/Kconfig
> >>>>>>> ===================================================================
> >>>>>>> --- linux-2.6-tip.orig/kernel/trace/Kconfig
> >>>>>>> +++ linux-2.6-tip/kernel/trace/Kconfig
> >>>>>>> @@ -464,7 +464,7 @@ config BLK_DEV_IO_TRACE
> >>>>>>>  
> >>>>>>>  config KPROBE_EVENT
> >>>>>>>      depends on KPROBES
> >>>>>>> -    depends on X86
> >>>>>>> +    depends on X86 || PPC
> >>>>>>>      bool "Enable kprobes-based dynamic events"
> >>>>>>>      select TRACING
> >>>>>>>      default y
> >>>>>>>
> >>>>>>> _______________________________________________
> >>>>>>> Linuxppc-dev mailing list
> >>>>>>> Linuxppc-dev@lists.ozlabs.org
> >>>>>>> https://lists.ozlabs.org/listinfo/linuxppc-dev
> >>>>>>>
> >>>>> Thanks for reviewing.
> >>>>
> >>>> We are creating a new user space API here, so I'm keen for others to tak
e
> >>>> a good look at the interface before we commit to something we are going
> >>>> to have to keep forever. 
> >>>> Who is the main consumer of this (/me is pretty ignorant of kprobes)?
> >>>> What do they think of the interface?
> >>>>
> >>> The user space API are already present in the upstream kernel and
> >>> currently only supported architecture is x86. This patch provides ppc
> >>> architecture specific interfaces that enables powerpc also in par with x8
6.
> >>
> >> Yes, there is a kprobe tracer in ftrace (see Documentation/trace/kprobetra
ce.
> > txt).
> >> and this tracer is only for probing kernel (not userspace).
> >>
> >>>
> >>> The main consumer would be kernel developers who would like to see
> >>> register values, arguments and stack when the probe hits at given text
> >>> address.
> >>
> >> Right.
> >>
> >> BTW, there is a user-space tools we have (tools/perf/builtin-probe.c).
> >> Currently, it's only for x86. Mahesh, You just need to add a register
> >> translation table in tools/perf/util/probe-finder.c for ppc support.
> >>
> >> Thank you!
> >>
> >> -- 
> >> Masami Hiramatsu
> >>
> >> Software Engineer
> >> Hitachi Computer Products (America), Inc.
> >> Software Solutions Division
> >>
> >> e-mail: mhiramat@redhat.com
> >>
> 
> -- 
> Masami Hiramatsu
> 
> Software Engineer
> Hitachi Computer Products (America), Inc.
> Software Solutions Division
> 
> e-mail: mhiramat@redhat.com
> 

^ permalink raw reply

* [PATCH] viodasd.c: Remove VIOD_KERN_<level> macros for printks
From: Joe Perches @ 2009-12-23  7:22 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Stephen Rothwell

Use #define pr_fmt(fmt) "viod: " fmt
Remove #define VIOD_KERN_WARNING and VIOD_KERN_INFO
Convert printk(VIOD_KERN_<level> to pr_<level>
Coalesce long format strings

Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Stephen Rothwell <sfr@canb.auug.org.au>

 drivers/block/viodasd.c |   86 +++++++++++++++++++---------------------------
 1 files changed, 36 insertions(+), 50 deletions(-)

diff --git a/drivers/block/viodasd.c b/drivers/block/viodasd.c
index a8c8b56..1b3def1 100644
--- a/drivers/block/viodasd.c
+++ b/drivers/block/viodasd.c
@@ -28,6 +28,9 @@
  * All disk operations are performed by sending messages back and forth to
  * the OS/400 partition.
  */
+
+#define pr_fmt(fmt) "viod: " fmt
+
 #include <linux/major.h>
 #include <linux/fs.h>
 #include <linux/module.h>
@@ -63,9 +66,6 @@ MODULE_LICENSE("GPL");
 
 #define VIOD_VERS		"1.64"
 
-#define VIOD_KERN_WARNING	KERN_WARNING "viod: "
-#define VIOD_KERN_INFO		KERN_INFO "viod: "
-
 enum {
 	PARTITION_SHIFT = 3,
 	MAX_DISKNO = HVMAXARCHITECTEDVIRTUALDISKS,
@@ -156,7 +156,7 @@ static int viodasd_open(struct block_device *bdev, fmode_t mode)
 			((u64)DEVICE_NO(d) << 48) | ((u64)flags << 32),
 			0, 0, 0);
 	if (hvrc != 0) {
-		printk(VIOD_KERN_WARNING "HV open failed %d\n", (int)hvrc);
+		pr_warning("HV open failed %d\n", (int)hvrc);
 		return -EIO;
 	}
 
@@ -167,9 +167,8 @@ static int viodasd_open(struct block_device *bdev, fmode_t mode)
 		const struct vio_error_entry *err =
 			vio_lookup_rc(viodasd_err_table, we.sub_result);
 
-		printk(VIOD_KERN_WARNING
-				"bad rc opening disk: %d:0x%04x (%s)\n",
-				(int)we.rc, we.sub_result, err->msg);
+		pr_warning("bad rc opening disk: %d:0x%04x (%s)\n",
+			   (int)we.rc, we.sub_result, err->msg);
 		return -EIO;
 	}
 
@@ -195,8 +194,7 @@ static int viodasd_release(struct gendisk *disk, fmode_t mode)
 			((u64)DEVICE_NO(d) << 48) /* | ((u64)flags << 32) */,
 			0, 0, 0);
 	if (hvrc != 0)
-		printk(VIOD_KERN_WARNING "HV close call failed %d\n",
-				(int)hvrc);
+		pr_warning("HV close call failed %d\n", (int)hvrc);
 	return 0;
 }
 
@@ -288,8 +286,7 @@ static int send_request(struct request *req)
 		bevent = (struct vioblocklpevent *)
 			vio_get_event_buffer(viomajorsubtype_blockio);
 		if (bevent == NULL) {
-			printk(VIOD_KERN_WARNING
-			       "error allocating disk event buffer\n");
+			pr_warning("error allocating disk event buffer\n");
 			goto error_ret;
 		}
 
@@ -333,9 +330,8 @@ static int send_request(struct request *req)
 	}
 
 	if (hvrc != HvLpEvent_Rc_Good) {
-		printk(VIOD_KERN_WARNING
-		       "error sending disk event to OS/400 (rc %d)\n",
-		       (int)hvrc);
+		pr_warning("error sending disk event to OS/400 (rc %d)\n",
+			   (int)hvrc);
 		goto error_ret;
 	}
 	spin_unlock_irqrestore(&viodasd_spinlock, flags);
@@ -402,7 +398,7 @@ retry:
 			((u64)dev_no << 48) | ((u64)flags<< 32),
 			0, 0, 0);
 	if (hvrc != 0) {
-		printk(VIOD_KERN_WARNING "bad rc on HV open %d\n", (int)hvrc);
+		pr_warning("bad rc on HV open %d\n", (int)hvrc);
 		return 0;
 	}
 
@@ -416,9 +412,8 @@ retry:
 		goto retry;
 	}
 	if (we.max_disk > (MAX_DISKNO - 1)) {
-		printk_once(VIOD_KERN_INFO
-			"Only examining the first %d of %d disks connected\n",
-			MAX_DISKNO, we.max_disk + 1);
+		printk_once(KERN_INFO pr_fmt("Only examining the first %d of %d disks connected\n"),
+			    MAX_DISKNO, we.max_disk + 1);
 	}
 
 	/* Send the close event to OS/400.  We DON'T expect a response */
@@ -432,17 +427,15 @@ retry:
 			((u64)dev_no << 48) | ((u64)flags << 32),
 			0, 0, 0);
 	if (hvrc != 0) {
-		printk(VIOD_KERN_WARNING
-		       "bad rc sending event to OS/400 %d\n", (int)hvrc);
+		pr_warning("bad rc sending event to OS/400 %d\n", (int)hvrc);
 		return 0;
 	}
 
 	if (d->dev == NULL) {
 		/* this is when we reprobe for new disks */
 		if (vio_create_viodasd(dev_no) == NULL) {
-			printk(VIOD_KERN_WARNING
-				"cannot allocate virtual device for disk %d\n",
-				dev_no);
+			pr_warning("cannot allocate virtual device for disk %d\n",
+				   dev_no);
 			return 0;
 		}
 		/*
@@ -457,15 +450,13 @@ retry:
 	spin_lock_init(&d->q_lock);
 	q = blk_init_queue(do_viodasd_request, &d->q_lock);
 	if (q == NULL) {
-		printk(VIOD_KERN_WARNING "cannot allocate queue for disk %d\n",
-				dev_no);
+		pr_warning("cannot allocate queue for disk %d\n", dev_no);
 		return 0;
 	}
 	g = alloc_disk(1 << PARTITION_SHIFT);
 	if (g == NULL) {
-		printk(VIOD_KERN_WARNING
-				"cannot allocate disk structure for disk %d\n",
-				dev_no);
+		pr_warning("cannot allocate disk structure for disk %d\n",
+			   dev_no);
 		blk_cleanup_queue(q);
 		return 0;
 	}
@@ -489,13 +480,12 @@ retry:
 	g->driverfs_dev = d->dev;
 	set_capacity(g, d->size >> 9);
 
-	printk(VIOD_KERN_INFO "disk %d: %lu sectors (%lu MB) "
-			"CHS=%d/%d/%d sector size %d%s\n",
-			dev_no, (unsigned long)(d->size >> 9),
-			(unsigned long)(d->size >> 20),
-			(int)d->cylinders, (int)d->tracks,
-			(int)d->sectors, (int)d->bytes_per_sector,
-			d->read_only ? " (RO)" : "");
+	pr_info("disk %d: %lu sectors (%lu MB) CHS=%d/%d/%d sector size %d%s\n",
+		dev_no, (unsigned long)(d->size >> 9),
+		(unsigned long)(d->size >> 20),
+		(int)d->cylinders, (int)d->tracks,
+		(int)d->sectors, (int)d->bytes_per_sector,
+		d->read_only ? " (RO)" : "");
 
 	/* register us in the global list */
 	add_disk(g);
@@ -580,8 +570,8 @@ static int viodasd_handle_read_write(struct vioblocklpevent *bevent)
 	if (error) {
 		const struct vio_error_entry *err;
 		err = vio_lookup_rc(viodasd_err_table, bevent->sub_result);
-		printk(VIOD_KERN_WARNING "read/write error %d:0x%04x (%s)\n",
-				event->xRc, bevent->sub_result, err->msg);
+		pr_warning("read/write error %d:0x%04x (%s)\n",
+			   event->xRc, bevent->sub_result, err->msg);
 		num_sect = blk_rq_sectors(req);
 	}
 	qlock = req->q->queue_lock;
@@ -606,8 +596,7 @@ static void handle_block_event(struct HvLpEvent *event)
 		return;
 	/* First, we should NEVER get an int here...only acks */
 	if (hvlpevent_is_int(event)) {
-		printk(VIOD_KERN_WARNING
-		       "Yikes! got an int in viodasd event handler!\n");
+		pr_warning("Yikes! got an int in viodasd event handler!\n");
 		if (hvlpevent_need_ack(event)) {
 			event->xRc = HvLpEvent_Rc_InvalidSubtype;
 			HvCallEvent_ackLpEvent(event);
@@ -650,7 +639,7 @@ static void handle_block_event(struct HvLpEvent *event)
 		break;
 
 	default:
-		printk(VIOD_KERN_WARNING "invalid subtype!");
+		pr_warning("invalid subtype!");
 		if (hvlpevent_need_ack(event)) {
 			event->xRc = HvLpEvent_Rc_InvalidSubtype;
 			HvCallEvent_ackLpEvent(event);
@@ -739,29 +728,26 @@ static int __init viodasd_init(void)
 		vio_set_hostlp();
 
 	if (viopath_hostLp == HvLpIndexInvalid) {
-		printk(VIOD_KERN_WARNING "invalid hosting partition\n");
+		pr_warning("invalid hosting partition\n");
 		rc = -EIO;
 		goto early_fail;
 	}
 
-	printk(VIOD_KERN_INFO "vers " VIOD_VERS ", hosting partition %d\n",
-			viopath_hostLp);
+	pr_info("vers " VIOD_VERS ", hosting partition %d\n", viopath_hostLp);
 
         /* register the block device */
 	rc =  register_blkdev(VIODASD_MAJOR, VIOD_GENHD_NAME);
 	if (rc) {
-		printk(VIOD_KERN_WARNING
-				"Unable to get major number %d for %s\n",
-				VIODASD_MAJOR, VIOD_GENHD_NAME);
+		pr_warning("Unable to get major number %d for %s\n",
+			   VIODASD_MAJOR, VIOD_GENHD_NAME);
 		goto early_fail;
 	}
 	/* Actually open the path to the hosting partition */
 	rc = viopath_open(viopath_hostLp, viomajorsubtype_blockio,
 				VIOMAXREQ + 2);
 	if (rc) {
-		printk(VIOD_KERN_WARNING
-		       "error opening path to host partition %d\n",
-		       viopath_hostLp);
+		pr_warning("error opening path to host partition %d\n",
+			   viopath_hostLp);
 		goto unregister_blk;
 	}
 
@@ -770,7 +756,7 @@ static int __init viodasd_init(void)
 
 	rc = vio_register_driver(&viodasd_driver);
 	if (rc) {
-		printk(VIOD_KERN_WARNING "vio_register_driver failed\n");
+		pr_warning("vio_register_driver failed\n");
 		goto unset_handler;
 	}
 

^ permalink raw reply related

* [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.
From: tmarri @ 2009-12-23  7:52 UTC (permalink / raw)
  To: jwboyer, linuxppc-dev; +Cc: linuxppc-dev, writetomarri, tmarri

From: Tirumala Marri <tmarri@amcc.com>

Signed-off-by: Tirumala Marri <tmarri@amcc.com>
---
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.h

diff --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";
+  			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 set
diff --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_MSI
diff --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.o
diff --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"
+
+
+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;
+}
+
+
+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);
+}
+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;
+}
+
+
+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);
+
+	}
+
+	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)
+{
+}
+
+
+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);
+#else
+	dev_err(&dev->dev, " Invalid Device \n");
+	goto error_out;
+#endif
+	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__ */
-- 
1.6.1.rc3

^ permalink raw reply related

* Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.
From: Stefan Roese @ 2009-12-23  8:18 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linuxppc-dev, writetomarri, tmarri
In-Reply-To: <1261554743-30498-1-git-send-email-tmarri@amcc.com>

On Wednesday 23 December 2009 08:52:23 tmarri@amcc.com wrote:
> From: Tirumala Marri <tmarri@amcc.com>

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?
 
> Signed-off-by: Tirumala Marri <tmarri@amcc.com>
> ---
> 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.h
> 
> diff --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.

> +  			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 set
> diff --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_MSI
> diff --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.o
> diff --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.

> +	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

^ permalink raw reply

* msi / msix on PPC405EX?
From: Weirich, Bernhard @ 2009-12-23  8:50 UTC (permalink / raw)
  To: linuxppc-dev@lists.ozlabs.org

Hello,

I am wondering whether msi / msix should work on PPC405EX. A call to pci_en=
able_msi returns: -38 / ENOSYS, function not implemented.
Can someone tell me if I am missing out something or if msi is indeed not i=
mplemented on this platform?

Thanks,=20
Bernhard=

^ permalink raw reply

* Re: msi / msix on PPC405EX?
From: Josh Boyer @ 2009-12-23 13:22 UTC (permalink / raw)
  To: Weirich, Bernhard; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6680AE1BD102B9419251E785D19F5D030178135A@ExchN1.riedel.net>

On Wed, Dec 23, 2009 at 09:50:39AM +0100, Weirich, Bernhard wrote:
>Hello,
>
>I am wondering whether msi / msix should work on PPC405EX. A call to pci_enable_msi returns: -38 / ENOSYS, function not implemented.
>Can someone tell me if I am missing out something or if msi is indeed not implemented on this platform?

It doesn't work there yet, as it has not been implemented.  There are patches
for MSI on 460SX now, and those could probably be leveraged once they're merged.

josh

^ permalink raw reply

* [HELP] BAD interrupts
From: Kumar Gopalpet-B05799 @ 2009-12-23 14:11 UTC (permalink / raw)
  To: netdev, linuxppc-dev

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

Hi all,
 
I am trying linux 2.6.32-rc3 in SMP mode. I am seeing a lot of BAD
interrupts when I do a "cat  /proc/interrupts".
 
I am running a forwarding application b/w two ethernet ports (ethernet
uses gianfar driver).
 
Has any one observed these BAD interrupts ? If so, can someone help me
in understanding why these interrupts come and how to resolve it ?
 
 
--
 
Thanks
Sandeep 

[-- Attachment #2: Type: text/html, Size: 1737 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3] of: make set_node_proc_entry private to proc_devtree.c
From: Grant Likely @ 2009-12-23 14:48 UTC (permalink / raw)
  To: Jeremy Kerr; +Cc: devicetree-discuss, linuxppc-dev, linux-kernel
In-Reply-To: <1261532718.575722.541804401419.2.gpush@pororo>

On Tue, Dec 22, 2009 at 6:45 PM, Jeremy Kerr <jeremy.kerr@canonical.com> wrote:
> We only need set_node_proc_entry in proc_devtree.c, so move it there.
>
> This fixes the !HAVE_ARCH_DEVTREE_FIXUPS build, as we can't make make
> the definition in linux/of.h conditional on this #define (definitions in
> asm/prom.h can't be exposed to linux/of.h, due to the enforced #include
> ordering).
>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

A little ugly, but it can probably be refactored and cleaned up later
(like a lot of other things in the merged code).  Otherwise these
three patches look good to me.  I've picked them up into my tree and
I'll push them out soon.

g.

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

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox