LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: ucc_geth broken in 2.6.32 by 864fdf884e82bacbe8ca5e93bd43393a61d2e2b4
From: Anton Vorontsov @ 2009-12-23 18:04 UTC (permalink / raw)
  To: Lennart Sorensen; +Cc: linuxppc-dev, linux-kernel, netdev
In-Reply-To: <20091223174019.GB762@caffeine.csclub.uwaterloo.ca>

On Wed, Dec 23, 2009 at 12:40:19PM -0500, Lennart Sorensen wrote:
> We use the ucc_geth for 6 ports (4 100Mbit and 2 Gbit ports) on an
> mpc8360e.  Up to 2.6.31 this worked fine.  2.6.32 on the other hand
> crashes very quickly after boot.

Hm. Just curious, what CPU revision you use? So that I could try
to reproduce the issue... I have MPC8360E-MDS boards, rev 2.0 and
rev rev 2.1 CPUs, Marvell PHYs. I also have MPC8360E-RDK (rev 2.1).
And I didn't see any issues with this patch.

> I managed to see the same crash when I was selectively trying to add newer
> ucc_geth patches to the 2.6.31 kernel a couple of months ago, and the same
> patch that caused a crash then seems suspect.  If I revert the patch the
> system runs completely stable.  Amusingly, the excact error message the
> patch claims to fix is in fact the error it causes to happen in my case.
> 
> So preferably 864fdf884e82bacbe8ca5e93bd43393a61d2e2b4 could be reverted

I don't see any point in reverting, because it will surely break my boards.
So, we need to fix this, not just hide.

> unless someone can fix it.

Well, I'm ready to help you with debugging.

> Now I must add that I run with the xenomai/adeos-ipipe patches as well,
> which do change interrupt handling a little,

It could be that it takes too long to stop the UCC, and xenomai
makes the timings worse, so the watchdog triggers.

Can you try the following patch?

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index afaf088..2f73e3f 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -1563,6 +1563,8 @@ static int ugeth_disable(struct ucc_geth_private *ugeth, enum comm_dir mode)
 
 static void ugeth_quiesce(struct ucc_geth_private *ugeth)
 {
+	netif_device_detach(ugeth->ndev);
+
 	/* Wait for and prevent any further xmits. */
 	netif_tx_disable(ugeth->ndev);
 
@@ -1577,7 +1579,7 @@ static void ugeth_activate(struct ucc_geth_private *ugeth)
 {
 	napi_enable(&ugeth->napi);
 	enable_irq(ugeth->ug_info->uf_info.irq);
-	netif_tx_wake_all_queues(ugeth->ndev);
+	netif_device_attach(ugeth->ndev);
 }
 
 /* Called every time the controller might need to be made

^ permalink raw reply related

* ucc_geth broken in 2.6.32 by 864fdf884e82bacbe8ca5e93bd43393a61d2e2b4
From: Lennart Sorensen @ 2009-12-23 17:40 UTC (permalink / raw)
  To: linux-kernel; +Cc: linuxppc-dev, Anton Vorontsov, Len Sorensen, netdev

We use the ucc_geth for 6 ports (4 100Mbit and 2 Gbit ports) on an
mpc8360e.  Up to 2.6.31 this worked fine.  2.6.32 on the other hand
crashes very quickly after boot.

I managed to see the same crash when I was selectively trying to add newer
ucc_geth patches to the 2.6.31 kernel a couple of months ago, and the same
patch that caused a crash then seems suspect.  If I revert the patch the
system runs completely stable.  Amusingly, the excact error message the
patch claims to fix is in fact the error it causes to happen in my case.

So preferably 864fdf884e82bacbe8ca5e93bd43393a61d2e2b4 could be reverted
unless someone can fix it.  I can't even make sense of why it is supposed
to improve anything, it certainly seems like a very unsafe change
to make.  Removing locking and disabling of interrupts before poking at
phy settings and such doesn't seem like a minor change and doesn't seem
that safe either.

Now I must add that I run with the xenomai/adeos-ipipe patches as well,
which do change interrupt handling a little, but so far this has worked
fine with the previous code and only the current code is broken for us.
I could try to build without the patch, although I would loose some major
functionality on the box doing so, and I would not be surprised if it
still fails since I believe I tried that already with 2.6.31+selected
git patches before without xenomai patched in and it still failed,
but I am only about 99% sure of that.

With the patch I get:
NETDEV WATCHDOG: eth2 (ucc_geth): transmit queue 0 timed out
------------[ cut here ]------------
Badness at c02729a8 [verbose debug info unavailable]
NIP: c02729a8 LR: c02729a8 CTR: c01b6088
REGS: c0451c40 TRAP: 0700   Not tainted  (2.6.32-trunk-8360e)
MSR: 00029032 <EE,ME,CE,IR,DR>  CR: 42042024  XER: 20000000
TASK = c041f3e8[0] 'swapper' THREAD: c0450000
GPR00: c02729a8 c0451cf0 c041f3e8 00000045 00002ae9 ffffffff c01b6afc c0422c48
GPR08: c042fde8 00000002 00000003 00010000 22042024 1001af90 1fffd000 00000000
GPR16: 00000000 c038c6d8 00000001 00200200 00000000 c0465eec c0465cec c0465aec
GPR24: c0450000 c04658ec c0423c2c df0e01c0 c0480000 df0e0000 c0423c2c 00000000
NIP [c02729a8] dev_watchdog+0x280/0x290
LR [c02729a8] dev_watchdog+0x280/0x290
Call Trace:
[c0451cf0] [c02729a8] dev_watchdog+0x280/0x290 (unreliable)
[c0451d50] [c00377c4] run_timer_softirq+0x164/0x224
[c0451da0] [c0032a38] __do_softirq+0xb8/0x13c
[c0451df0] [c00065cc] do_softirq+0xa0/0xac
[c0451e00] [c003280c] irq_exit+0x7c/0x9c
[c0451e10] [c00640c4] __ipipe_sync_stage+0x248/0x24c
[c0451e50] [c0064374] ipipe_suspend_domain+0xa0/0xf4
[c0451e70] [c00644a4] __ipipe_walk_pipeline+0xdc/0x120
[c0451e90] [c000af28] __ipipe_handle_irq+0x164/0x168
[c0451ec0] [c000b03c] __ipipe_grab_irq+0x3c/0xa4
[c0451ed0] [c0014814] __ipipe_ret_from_except+0x0/0xc
--- Exception: 501 at cpu_idle+0xe0/0xf0
    LR = cpu_idle+0xe0/0xf0
[c0451f90] [c000970c] cpu_idle+0x68/0xf0 (unreliable)
[c0451fb0] [c0003f30] rest_init+0x5c/0x6c
[c0451fc0] [c03f07ac] start_kernel+0x27c/0x2e0
[c0451ff0] [00003438] 0x3438
Instruction dump:
7d2903a6 4bfffea8 38810008 7fa3eb78 38a00040 4bfe9c81 7fe6fb78 7fa4eb78
7c651b78 3c60c03c 38631774 480b7d2d <0fe00000> 38000001 901c2fb0 4bffff94
warning: `zebra' uses 32-bit capabilities (legacy support in use)
PHY: 0:03 - Link is Up - 1000/Full
PHY: 0:09 - Link is Up - 100/Full
PHY: 0:02 - Link is Up - 100/Full
PHY: 0:0f - Link is Up - 100/Full
PHY: 0:17 - Link is Up - 100/Full

When reverted I get a stable running system with no errors.

Which port happens to fail first varies, but one always does and then
the system almost always crashes soon after.

-- 
Len Sorensen

^ permalink raw reply

* RE: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.
From: Tirumala Reddy Marri @ 2009-12-23 17:30 UTC (permalink / raw)
  To: Tirumala Reddy Marri, Stefan Roese, linuxppc-dev
  Cc: linuxppc-dev, writetomarri
In-Reply-To: <AC5E1C3367E37D44970B81A6ADD1DA2C08882A65@SDCEXCHANGE01.ad.amcc.com>

BTW once this patch gets in I will add the 405Ex,460Ex and 440Spe
support to the same.

-----Original Message-----
From: linuxppc-dev-bounces+tmarri=3Damcc.com@lists.ozlabs.org
[mailto:linuxppc-dev-bounces+tmarri=3Damcc.com@lists.ozlabs.org] On =
Behalf
Of Tirumala Reddy Marri
Sent: Wednesday, December 23, 2009 9:23 AM
To: Stefan Roese; linuxppc-dev@lists.ozlabs.org
Cc: linuxppc-dev@ozlabs.org; writetomarri@yahoo.com
Subject: RE: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

Thanks for the suggestions. I will try remove the extra lines . Add
changes you suggested.
-Marri

-----Original Message-----
From: linuxppc-dev-bounces+tmarri=3Damcc.com@lists.ozlabs.org
[mailto:linuxppc-dev-bounces+tmarri=3Damcc.com@lists.ozlabs.org] On =
Behalf
Of Stefan Roese
Sent: Wednesday, December 23, 2009 12:19 AM
To: linuxppc-dev@lists.ozlabs.org
Cc: linuxppc-dev@ozlabs.org; writetomarri@yahoo.com; Tirumala Reddy
Marri
Subject: Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

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?=20
What are your plans here?
=20
> 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
|=20
>   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
>=20
> 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";

Better use something like this:

  			compatible =3D "amcc,ppc4xx-msi-ppc460sx",
"amcc,ppc4xx-msi";

This way you could check for 460SX specials in the driver if needed.

> +  			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/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=3Dy
>  CONFIG_DEFAULT_IOSCHED=3D"anticipatory"
>  # CONFIG_FREEZER is not set
>  CONFIG_PPC4xx_PCI_EXPRESS=3Dy
> +CONFIG_PPC_MSI_BITMAP=3Dy
>=20
>  #
>  # Platform support
> @@ -264,7 +265,7 @@ CONFIG_PCIEPORTBUS=3Dy
>  CONFIG_PCIEAER=3Dy
>  # CONFIG_PCIEASPM is not set
>  CONFIG_ARCH_SUPPORTS_MSI=3Dy
> -# CONFIG_PCI_MSI is not set
> +CONFIG_PCI_MSI=3Dy
>  # 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=3D64
>  # 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=3Dy
>  # 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.
>=20
> 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
>=20
> +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)		+=3D i8259.o
>  obj-$(CONFIG_IPIC)		+=3D ipic.o
>  obj-$(CONFIG_4xx)		+=3D uic.o
>  obj-$(CONFIG_4xx_SOC)		+=3D ppc4xx_soc.o
> +obj-$(CONFIG_PPC4xx_MSI)		+=3D ppc4xx_msi.o
>  obj-$(CONFIG_XILINX_VIRTEX)	+=3D xilinx_intc.o
>  obj-$(CONFIG_XILINX_PCI)	+=3D xilinx_pci.o
>  obj-$(CONFIG_OF_RTC)		+=3D 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 =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;
> +}
> +
> +

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

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

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 =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);
> +

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=20
check the whole file for this.

> +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;
> +	const u32 *sdr_base;
> +	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;
> +	}
> +	/* Get the sdr-base */
> +	sdr_base =3D (u32 *)of_get_property(dev->node, "sdr-base", NULL);
> +	if (sdr_base =3D=3D NULL) {
> +		dev_err(&dev->dev, "%s resource error!\n",
> +				dev->node->full_name);
> +		goto error_out;
> +	}
> +	msi->sdr_base =3D *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 =3D ioremap(res.start, res.end - res.start + 1);

	msi->msi_regs =3D 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=20
it, since the register bases are provided via the device-tree. So just
remove=20
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 =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..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__ */
>=20

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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* RE: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.
From: Tirumala Reddy Marri @ 2009-12-23 17:23 UTC (permalink / raw)
  To: Stefan Roese, linuxppc-dev; +Cc: linuxppc-dev, writetomarri
In-Reply-To: <200912230918.34124.sr@denx.de>

Thanks for the suggestions. I will try remove the extra lines . Add
changes you suggested.
-Marri

-----Original Message-----
From: linuxppc-dev-bounces+tmarri=3Damcc.com@lists.ozlabs.org
[mailto:linuxppc-dev-bounces+tmarri=3Damcc.com@lists.ozlabs.org] On =
Behalf
Of Stefan Roese
Sent: Wednesday, December 23, 2009 12:19 AM
To: linuxppc-dev@lists.ozlabs.org
Cc: linuxppc-dev@ozlabs.org; writetomarri@yahoo.com; Tirumala Reddy
Marri
Subject: Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

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?=20
What are your plans here?
=20
> 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
|=20
>   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
>=20
> 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";

Better use something like this:

  			compatible =3D "amcc,ppc4xx-msi-ppc460sx",
"amcc,ppc4xx-msi";

This way you could check for 460SX specials in the driver if needed.

> +  			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/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=3Dy
>  CONFIG_DEFAULT_IOSCHED=3D"anticipatory"
>  # CONFIG_FREEZER is not set
>  CONFIG_PPC4xx_PCI_EXPRESS=3Dy
> +CONFIG_PPC_MSI_BITMAP=3Dy
>=20
>  #
>  # Platform support
> @@ -264,7 +265,7 @@ CONFIG_PCIEPORTBUS=3Dy
>  CONFIG_PCIEAER=3Dy
>  # CONFIG_PCIEASPM is not set
>  CONFIG_ARCH_SUPPORTS_MSI=3Dy
> -# CONFIG_PCI_MSI is not set
> +CONFIG_PCI_MSI=3Dy
>  # 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=3D64
>  # 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=3Dy
>  # 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.
>=20
> 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
>=20
> +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)		+=3D i8259.o
>  obj-$(CONFIG_IPIC)		+=3D ipic.o
>  obj-$(CONFIG_4xx)		+=3D uic.o
>  obj-$(CONFIG_4xx_SOC)		+=3D ppc4xx_soc.o
> +obj-$(CONFIG_PPC4xx_MSI)		+=3D ppc4xx_msi.o
>  obj-$(CONFIG_XILINX_VIRTEX)	+=3D xilinx_intc.o
>  obj-$(CONFIG_XILINX_PCI)	+=3D xilinx_pci.o
>  obj-$(CONFIG_OF_RTC)		+=3D 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 =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;
> +}
> +
> +

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

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

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 =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);
> +

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=20
check the whole file for this.

> +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;
> +	const u32 *sdr_base;
> +	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;
> +	}
> +	/* Get the sdr-base */
> +	sdr_base =3D (u32 *)of_get_property(dev->node, "sdr-base", NULL);
> +	if (sdr_base =3D=3D NULL) {
> +		dev_err(&dev->dev, "%s resource error!\n",
> +				dev->node->full_name);
> +		goto error_out;
> +	}
> +	msi->sdr_base =3D *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 =3D ioremap(res.start, res.end - res.start + 1);

	msi->msi_regs =3D 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=20
it, since the register bases are provided via the device-tree. So just
remove=20
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 =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..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__ */
>=20

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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: Does gpio_to_irq() work for MPC52xx gpios?
From: Bill Gatliff @ 2009-12-23 16:47 UTC (permalink / raw)
  To: Peter Korsgaard; +Cc: Linux/PPC Development
In-Reply-To: <87fx72wwpd.fsf@macbook.be.48ers.dk>

Peter Korsgaard wrote:
>
> 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/
>   

Ok.  I'm taking a look at that code now, planning to adapt it for the
MPC52xx unless someone raises any objections.


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

^ permalink raw reply

* Re: [PATCH] Make cpu hotplug driver lock part of ppc_md
From: Nathan Fontenot @ 2009-12-23 14:48 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev, Andreas Schwab
In-Reply-To: <1261520982.17348.8.camel@concordia>

Michael Ellerman wrote:
> 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?

The intention of the cpu_hotplug_driver_locks to add additional serialization
during cpu hotplug operations.  For pseries this is used during DLPAR of cpu
operations so that cpu hotplug actions cannot be initiated whiloe a DLPAR
operation is in flight.  For example, during DLPAR add we take the lock while
acquiring the cpu from firmware and updating the device tree with the new
cpu information, after which we hotplug add the cpu to the system.  

There is nothing harmless about taking the lock on all platforms, I was just
trying to avoid taking the lock if the additional serialization is not needed.

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

I can make the lock available to all powerpc code and not go through the
ppc_md struct, it makes no difference to me personally.  Of course this would
make all that fun pokery jiggery go away :)

-Nathan

^ 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

* [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: 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

* 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: [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

* [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

* [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

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

* [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 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 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

* 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

* 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

* [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

* [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 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

* 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

* 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


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