LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: Xilinx PowerPC
From: David H. Lynch Jr. @ 2008-04-20 23:48 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-embedded
In-Reply-To: <fa686aa40804201548o503e9b19p4081cec653a1acd2@mail.gmail.com>

Thanks,

    I have alot of work to do on our stuff, I might as well see if I can
move to the powerpc tree at the same time.

    BTW is there even the beginings of a non-xilinx lltemac driver out
there ? There were hints on the list, but I have not seen anything.

    I would be happy to help advance the ball on anything anyone has
started.

   

Grant Likely wrote:
> On Sun, Apr 20, 2008 at 2:31 PM, David H. Lynch Jr. <dhlii@dlasys.net> wrote:
>   
>> Thanks.
>>
>>     I am running linus's 2.6.25-rc9, but I can pull the Xilinx tree or
>>  yours - when your server is up.
>>
>>     I can not find any Xilinx powerpc configs in arch/powerpc/config
>>     Do I just need to do a
>>     make ARCH=powerpc menuconfig and create one from scratch ?
>>     
>
> That's right; I haven't merged any defconfigs.  Roll your own.
>
>   
>>     Is simpleboot in your tree or the xilinx tree, if  can not find it
>>  in Linus's ?
>>     
>
> If you want to use the "simpleboot" wrapper; then you'll need to pull
> paulus' tree (Linus hasn't yet pulled his tree; but he probably will
> any moment now).
>
> Cheers,
> g.
>
>   


-- 
Dave Lynch 					  	    DLA Systems
Software Development:  				         Embedded Linux
717.627.3770 	       dhlii@dlasys.net 	  http://www.dlasys.net
fax: 1.253.369.9244 			           Cell: 1.717.587.7774
Over 25 years' experience in platforms, languages, and technologies too numerous to list.

"Any intelligent fool can make things bigger and more complex... It takes a touch of genius - and a lot of courage to move in the opposite direction."
Albert Einstein

^ permalink raw reply

* Re: [PATCH 2.6.26?] Raise the upper limit of NR_CPUS.
From: Tony Breeds @ 2008-04-21  1:41 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: LinuxPPC-dev, Paul Mackerras
In-Reply-To: <1208514829.8424.0.camel@concordia>

On Fri, Apr 18, 2008 at 08:33:44PM +1000, Michael Ellerman wrote:
 
> What happens on non prom-init platforms?

.... Um yeah they wont work.

I'll try a slightly different approach.

Yours Tony

  linux.conf.au    http://www.marchsouth.org/
  Jan 19 - 24 2009 The Australian Linux Technical Conference!

^ permalink raw reply

* Re: [PATCH 2.6.26?] Raise the upper limit of NR_CPUS.
From: Benjamin Herrenschmidt @ 2008-04-21  2:03 UTC (permalink / raw)
  To: Tony Breeds; +Cc: LinuxPPC-dev, Paul Mackerras
In-Reply-To: <20080418053349.GE20457@bakeyournoodle.com>


On Fri, 2008-04-18 at 15:33 +1000, Tony Breeds wrote:
> As the pacas are statically initialised increasing NR_CPUS beyond 128,
> means that any additional pacas will be empty  ... which is bad.
> 
> This patch adds the required functionality to fill in any excess pacas
> at runtime.
> 
> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> ---
> I know it's late, but can this be considered for 2.6.26?

NAK.

You must NEVER manipulate kernel globals from prom_init.c. The fact that
prom_init is linked with the kernel is an "accident" which may change.

Your scheme would break among others with kexec or non-OF bootloaders

Ben.

^ permalink raw reply

* Help! mpc8247 porting kernel 2.6.25 problem.
From: 张世乐 @ 2008-04-21  3:06 UTC (permalink / raw)
  To: Linuxppc-embedded

Hi All,

I encounter a problem when porting kernel 2.6.25 to a mpc8247 platform board.

after run command line: bootm 300000 - 600000
the output of u-boot like this:

[snip]

Uncompressing Kernel Image ... OK
Booting using the fdt at 0x600000

[nothing output and hanging here]

I use the led on the board to trace the kernel fund it stop in the function

setup_common_caches  in arch/powerpc/kernel/cpu_setup_6xx.S

Can you give me some ideas?

Thanks!

^ permalink raw reply

* RE: [PATCH 1/2] [v3][POWERPC] refactor dcr code
From: Stephen Neuendorffer @ 2008-04-21  3:56 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <20080419093500.752c1930@zod.rchland.ibm.com>

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



> > +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> > +{
> > +	if (host.type == NATIVE)
> > +		dcr_unmap_native(host.host.native, dcr_c);
> > +	else
> > +		dcr_unmap_mmio(host.host.mmio, dcr_c);
>
> What happens if host.type == INVALID?  Same question for the other
> accessors in dcr_*_generic.

I guess looking back on it, I assumed that MAP_OK would return 0, meaning that behavior was undefined,
but I agree it's probably safer to have some error reporting there...  There starts to become a speed tradeoff
at some point, which would make function pointers more attractive.  If the ioremap does fail, or the
dcr-access-method can't be determined, then dcr_unmap_mmio would probably SEGV anyway, although that's
not something I'd really want to rely on.  I'll put an error case in there.

> > +enum host_type_t {MMIO, NATIVE, INVALID};
>
> Should these be DCR_HOST_MMIO, DCR_HOST_NATIVE, DCR_HOST_INVALID?
>
> I worry about the generic nature of the names.

Also seems reasonable, 

Steve

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

^ permalink raw reply

* DMA on AMCC ppc440epx
From: Leonid @ 2008-04-21  4:55 UTC (permalink / raw)
  To: Linuxppc-embedded

Hi:

Is anybody using DMA on this AMCC CPU and if anybody knows where I can
get some sample working code or HOWTO documentation or anything?

I never used DMA on AMCC CPUs but have heard there are some problems but
I am not sure.

Thanks,

Leonid.

^ permalink raw reply

* Re: [PATCH 1/3] MSI driver for Freescale 83xx/85xx/86xx cpu
From: Michael Ellerman @ 2008-04-21  6:21 UTC (permalink / raw)
  To: Jason Jin; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <1208597267-30960-1-git-send-email-Jason.jin@freescale.com>

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

> This MSI driver can be used on 83xx/85xx/86xx board.
> In this driver, virtual interrupt host and chip were
> setup. There are 256 MSI interrupts in this host, Every 32
> MSI interrupts cascaded to one IPIC/MPIC interrupt.
> The chip was treated as edge sensitive and some necessary
> functions were setup for this chip.
> 
> Before using the MSI interrupt, PCI/PCIE device need to
> ask for a MSI interrupt in the 256 MSI interrupts. A 256bit
> bitmap show which MSI interrupt was used, reserve bit in
> the bitmap can be used to force the device use some designate
> MSI interrupt in the 256 MSI interrupts. Sometimes this is useful
> for testing the all the MSI interrupts. The msi-available-ranges
> property in the dts file was used for this purpose.
 

Hi Jason, some comments inline ...

 
> diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
> index 6d386d0..bfd3fe4 100644
> --- a/arch/powerpc/sysdev/Makefile
> +++ b/arch/powerpc/sysdev/Makefile
> @@ -4,6 +4,7 @@ endif
>  
>  mpic-msi-obj-$(CONFIG_PCI_MSI)	+= mpic_msi.o mpic_u3msi.o mpic_pasemi_msi.o
>  obj-$(CONFIG_MPIC)		+= mpic.o $(mpic-msi-obj-y)
> +obj-$(CONFIG_PCI_MSI)		+= fsl_msi.o

Do we really always want to build this if we have MSI? Might it depend
on something else as well? CONFIG_FSL_PCI maybe?

> diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
> new file mode 100644
> index 0000000..e8132cf
> --- /dev/null
> +++ b/arch/powerpc/sysdev/fsl_msi.c
> @@ -0,0 +1,457 @@
> +/*
> + * Copyright (C) 2007-2008 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * Author: Tony Li <tony.li@freescale.com>
> + *	   Jason Jin <Jason.jin@freescale.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2 of the
> + * License.
> + *
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/bootmem.h>
> +#include <linux/bitmap.h>
> +#include <linux/msi.h>
> +#include <asm/prom.h>
> +#include <asm/hw_irq.h>
> +#include <linux/pci.h>
> +#include <asm/ppc-pci.h>
> +#include <linux/of_platform.h>
> +
> +#include <sysdev/fsl_soc.h>
> +#include "fsl_msi.h"

People consider it good style to have the linux includes before the
asm includes.

> +#ifdef DEBUG
> +#define pr_debug(fmt...) printk(fmt)
> +#else
> +#define pr_debug(fmt...)
> +#endif

Please don't do this, just use pr_debug(). In fact I don't see where you
do use it :)

> +/* A bit ugly, can we get this from the pci_dev somehow? */
> +static struct fsl_msi *fsl_msi;

I recognise that comment :)  The answer is "no" we can't (easily) get
this from the pci_dev.

> +static inline u32 fsl_msi_read(u32 __iomem *base,
> +				unsigned int reg)

This would fit in 80 chars wouldn't it?

> +{
> +	return in_be32(base + (reg >> 2));
> +}
> +
> +static inline void fsl_msi_write(u32 __iomem *base,
> +				unsigned int reg, u32 value)
> +{
> +	out_be32(base + (reg >> 2), value);
> +}
> +
> +#define	fsl_msi_irq_to_hw(virq)	 ((unsigned int)irq_map[virq].hwirq)

I can't see where this is used, you probably don't need it.

> +/*
> + * We do not need this actually. The MSIR register has been read once
> + * in the cascade interrupt. So, this MSI interrupt has been acked
> +*/
> +static void fsl_msi_end_irq(unsigned int virq)
> +{
> +}

I guess the generic code assumes you have an ack, bummer.

> +static struct irq_chip fsl_msi_chip = {
> +	.mask		= mask_msi_irq,
> +	.unmask		= unmask_msi_irq,
> +	.ack		= fsl_msi_end_irq,
> +	.typename	= " FSL-MSI  ",

I'd rather you didn't try and pad the name by hand, if we want /proc/interrupts
to look pretty we should do that in show_interrupts().

> +static int fsl_msi_host_match(struct irq_host *h, struct device_node *node)
> +{
> +	struct fsl_msi *msi = h->host_data;
> +
> +	/* Exact match, unless node is NULL */
> +	return msi->of_node == NULL || msi->of_node == node;
> +}

Do you really want the MSI to be the default irq host?

> +static int fsl_msi_host_map(struct irq_host *h, unsigned int virq,
> +				irq_hw_number_t hw)
> +{
> +	struct fsl_msi *msi = h->host_data;
> +	struct irq_chip *chip = msi->hc_irq;
> +
> +	set_irq_chip_data(virq, msi);

You don't seem to use chip_data anywhere?

> +	get_irq_desc(virq)->status |= IRQ_TYPE_EDGE_FALLING;
> +
> +	set_irq_chip_and_handler(virq, chip,  handle_edge_irq);
> +
> +	return 0;
> +}
> +
> +static struct irq_host_ops fsl_msi_host_ops = {
> +	.match = fsl_msi_host_match,
> +	.map = fsl_msi_host_map,
> +};
> +
> +irq_hw_number_t fsl_msi_alloc_hwirqs(struct fsl_msi *msi, int num)
> +{
> +	unsigned long flags;
> +	int offset, order = get_count_order(num);
> +
> +	spin_lock_irqsave(&msi->bitmap_lock, flags);
> +
> +	offset = bitmap_find_free_region(msi->fsl_msi_bitmap,
> +					NR_MSI_IRQS, order);
> +
> +	spin_unlock_irqrestore(&msi->bitmap_lock, flags);
> +
> +	pr_debug("%s: allocated 0x%x (2^%d) at offset 0x%x\n",
> +		__func__, num, order, offset);
> +
> +	return offset;
> +}
> +
> +void fsl_msi_free_hwirqs(struct fsl_msi *msi, int offset, int num)
> +{
> +	unsigned long flags;
> +	int order = get_count_order(num);
> +
> +	pr_debug("%s: freeing 0x%x (2^%d) at offset 0x%x\n",
> +		__func__, num, order, offset);
> +
> +	spin_lock_irqsave(&msi->bitmap_lock, flags);
> +	bitmap_release_region(msi->fsl_msi_bitmap, offset, order);
> +	spin_unlock_irqrestore(&msi->bitmap_lock, flags);
> +}
> +
> +static int fsl_msi_reserve_dt_hwirqs(struct fsl_msi *msi)
> +{
> +	int i, len;
> +	const u32 *p;
> +
> +	p = of_get_property(msi->of_node, "msi-available-ranges", &len);
> +	if (!p) {
> +		pr_debug("fsl_msi: no msi-available-ranges property found \
> +				on %s\n", msi->of_node->full_name);
> +		return -ENODEV;
> +	}
> +
> +	if (len & 0x8 != 0) {
> +		printk(KERN_WARNING "fsl_msi: Malformed msi-available-ranges "
> +		       "property on %s\n", msi->of_node->full_name);
> +		return -EINVAL;
> +	}

Do you really want a bitwise and with 0x8?

> +
> +	bitmap_allocate_region(msi->fsl_msi_bitmap, 0,
> +			       get_count_order(msi->irq_count));
> +
> +	/* Format is: (<u32 start> <u32 count>)+ */
> +	len /= sizeof(u32);
> +	len /= 2;
> +	for (i = 0; i < len; i++, p += 2)
> +		fsl_msi_free_hwirqs(msi, *p, *(p + 1));
> +
> +	return 0;
> +}

We could share a bunch of that code with mpic_msi.c, but that's not your
job - I'll look at doing a patch once this goes in.

> +static int fsl_msi_init_allocator(struct fsl_msi *msi)
> +{
> +	int rc, size;
> +
> +	size = BITS_TO_LONGS(NR_MSI_IRQS) * sizeof(long);
> +
> +	msi->fsl_msi_bitmap = kmalloc(size, GFP_KERNEL);
> +
> +	if (msi->fsl_msi_bitmap == NULL) {
> +		pr_debug("%s: ENOMEM allocating allocator bitmap!\n",
> +				__func__);
> +		return -ENOMEM;
> +	}
> +	memset(msi->fsl_msi_bitmap, 0, size);

Use kzalloc() and you can lose the memset.

> +	rc = fsl_msi_reserve_dt_hwirqs(msi);

This routine is badly named (my fault), it really frees the available
MSI ranges.

> +	if (rc)
> +		goto out_free;
> +
> +	return 0;
> +out_free:
> +	if (mem_init_done)
> +		kfree(msi->fsl_msi_bitmap);

You don't need to test mem_init_done, this is running at initcall time
which is way later than mem_init.

> +	msi->fsl_msi_bitmap = NULL;
> +	return rc;
> +
> +}
> +
> +static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int type)
> +{
> +	if (type == PCI_CAP_ID_MSIX)
> +		pr_debug("fslmsi: MSI-X untested, trying anyway.\n");
> +
> +	return 0;
> +}
> +
> +static void fsl_teardown_msi_irqs(struct pci_dev *pdev)
> +{
> +	struct msi_desc *entry;
> +	struct fsl_msi *msi = fsl_msi;
> +
> +	list_for_each_entry(entry, &pdev->msi_list, list) {
> +		if (entry->irq == NO_IRQ)
> +			continue;
> +		set_irq_msi(entry->irq, NULL);
> +		fsl_msi_free_hwirqs(msi, virq_to_hw(entry->irq), 1);
> +		irq_dispose_mapping(entry->irq);
> +	}
> +
> +	return;
> +}
> +
> +static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq,
> +				  struct msi_msg *msg)
> +{
> +	unsigned int srs;
> +	unsigned int ibs;
> +	struct fsl_msi *msi = fsl_msi;
> +
> +	srs = hwirq / INT_PER_MSIR;
> +	ibs = hwirq % INT_PER_MSIR;
> +
> +	msg->address_lo = msi->msi_addr_lo;
> +	msg->address_hi = msi->msi_addr_hi;
> +	msg->data = (srs << 5) | (ibs & 0x1F);

Is the 5 and 0x1F independent of the INT_PER_MSIR value? Given the
current values isn't this a no-op, or am I missing something?

> +	pr_debug("%s: allocated srs: %d, ibs: %d\n",
> +		__func__, srs, ibs);
> +}
> +
> +static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> +{
> +	irq_hw_number_t hwirq;
> +	int rc;
> +	unsigned int virq;
> +	struct msi_desc *entry;
> +	struct msi_msg msg;
> +	struct fsl_msi *msi = fsl_msi;

A couple of places you put this into a local called "msi" which is not the
greatest name in the world IMHO :)

> +	list_for_each_entry(entry, &pdev->msi_list, list) {
> +		hwirq = fsl_msi_alloc_hwirqs(msi, 1);
> +		if (hwirq < 0) {
> +			rc = hwirq;
> +			pr_debug("%s: fail allocating msi interrupt\n",
> +					__func__);
> +			goto out_free;
> +		}
> +
> +		virq = irq_create_mapping(msi->irqhost, hwirq);
> +
> +		if (virq == NO_IRQ) {
> +			pr_debug("%s: fail mapping hwirq 0x%lx\n",
> +					__func__, hwirq);
> +			fsl_msi_free_hwirqs(msi, hwirq, 1);
> +			rc = -ENOSPC;
> +			goto out_free;
> +		}
> +		set_irq_msi(virq, entry);
> +
> +		fsl_compose_msi_msg(pdev, hwirq, &msg);
> +		write_msi_msg(virq, &msg);
> +	}
> +	return 0;
> +
> +out_free:
> +	fsl_teardown_msi_irqs(pdev);

You don't need to call teardown, the generic code does that for you.

> +	return rc;
> +}
> +
> +void fsl_msi_cascade(unsigned int irq, struct irq_desc *desc)
> +{
> +	unsigned int cascade_irq;
> +	struct fsl_msi *msi = fsl_msi;
> +	int msir_index = -1;
> +	int i;
> +	u32 msir_value = 0;
> +	u32 intr_index;
> +	u32 have_shift = 0;
> +
> +	spin_lock(&desc->lock);
> +	if ((msi->feature &  FSL_PIC_IP_MASK) == FSL_PIC_IP_IPIC) {
> +		if (desc->chip->mask_ack)
> +			desc->chip->mask_ack(irq);
> +		else {
> +			desc->chip->mask(irq);
> +			desc->chip->ack(irq);
> +		}
> +	}
> +
> +	if (unlikely(desc->status & IRQ_INPROGRESS))
> +		goto unlock;
> +
> +	for (i = 0; i < NR_MSIR; i++)
> +		if (irq == msi->msir[i]) {
> +			msir_index = i;
> +			break;
> +		}

This is a bit ugly :)  Because you get the *msi from fsl_msi (above), you could
store the msir_index in the handler_data (with set_irq_data), and save doing
this loop.

> +	if (i >= NR_MSIR)
> +		cascade_irq = NO_IRQ;
> +
> +	desc->status |= IRQ_INPROGRESS;
> +	switch (fsl_msi->feature & FSL_PIC_IP_MASK) {
> +	case FSL_PIC_IP_MPIC:
> +		msir_value = fsl_msi_read(msi->msi_regs, msir_index * 0x10);
> +		break;
> +	case FSL_PIC_IP_IPIC:
> +		msir_value = fsl_msi_read(msi->msi_regs, msir_index * 0x4);
> +		break;
> +	}
> +
> +	while (msir_value) {
> +		intr_index = ffs(msir_value) - 1;
> +
> +		cascade_irq = irq_linear_revmap(msi->irqhost,
> +			(msir_index * INT_PER_MSIR + intr_index + have_shift));
> +
> +		if (cascade_irq != NO_IRQ)
> +			generic_handle_irq(cascade_irq);
> +		have_shift += (intr_index + 1);
> +		msir_value = (msir_value >> (intr_index + 1));
> +	}

It took me a while to grok all the shifting and so on here, I don't know if
there's a cleaner way to do it.

> +	desc->status &= ~IRQ_INPROGRESS;
> +
> +	switch (fsl_msi->feature & FSL_PIC_IP_MASK) {
> +	case FSL_PIC_IP_MPIC:
> +		desc->chip->eoi(irq);
> +		break;
> +	case FSL_PIC_IP_IPIC:
> +		if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
> +		desc->chip->unmask(irq);

Missing indent.

> +		break;
> +	}
> +unlock:
> +	spin_unlock(&desc->lock);
> +}
> +
> +static int __devinit fsl_of_msi_probe(struct of_device *dev,
> +				const struct of_device_id *match)
> +{
> +	struct fsl_msi *msi;
> +	struct resource res;
> +	int err, i, count;
> +	int rc;
> +	int virt_msir;
> +	const u32 *p;
> +	struct fsl_msi_data *tmp_data;
> +
> +	printk(KERN_INFO "Setting up fsl msi support\n");

KERN_DEBUG would do here IMHO, users don't usually need to see it.

> +	msi = kzalloc(sizeof(struct fsl_msi), GFP_KERNEL);
> +	if (!msi) {
> +		dev_err(&dev->dev, "No memory for MSI structure\n");
> +		err = -ENOMEM;
> +		goto error_out;
> +	}
> +
> +	msi->of_node = dev->node;
> +
> +	msi->irqhost = irq_alloc_host(of_node_get(dev->node),
> +				IRQ_HOST_MAP_LINEAR,
> +				NR_MSI_IRQS, &fsl_msi_host_ops, 0);
> +	if (msi->irqhost == NULL) {
> +		dev_err(&dev->dev, "No memory for MSI irqhost\n");
> +		err = -ENOMEM;

You need an of_node_put(dev->node) in the error case here.

> +		goto error_out;
> +	}
> +
> +	/*Get the MSI reg base */

Missing space after /*

> +	err = of_address_to_resource(dev->node, 0, &res);
> +	if (err) {
> +		dev_err(&dev->dev, "Can't get %s property 'reg'\n",
> +				dev->node->full_name);

That's a little misleading, aren't there's lots of reasons
of_address_to_resource() might fail?

> +		goto error_out;
> +	}
> +
> +	msi->msi_regs = ioremap(res.start, res.end - res.start + 1);

ioremap() can fail.

> +	tmp_data = (struct fsl_msi_data *)match->data;
> +
> +	msi->feature = tmp_data->fsl_pic_ip;
> +
> +	msi->irqhost->host_data = msi;
> +	msi->hc_irq = &fsl_msi_chip;

Any reason this needs to be a variable, isn't it always &fsl_msi_chip?

> +	msi->msi_addr_hi = 0x0;
> +	msi->msi_addr_lo = res.start + tmp_data->msiir_offset;
> +
> +	msi->irq_count = NR_MSI_IRQS;

Ditto.

> +	p = of_get_property(dev->node, "interrupts", &count);
> +	if (!p) {
> +		dev_err(&dev->dev, "no msi-interrupts property found on %s\n",
> +				dev->node->full_name);
> +		err = -ENODEV;
> +		goto error_out;
> +	}
> +	if (count % 8 != 0) {
> +		dev_err(&dev->dev, "Malformed msi-interrupts property on %s\n",
> +				dev->node->full_name);

Messages don't match the code, "interrupts" vs "msi-interrupts".

> +		err = -EINVAL;
> +		goto error_out;
> +	}
> +
> +	count /= sizeof(u32);
> +	for (i = 0; i < count / 2; i++) {
> +		if (i > NR_MSIR)
> +			break;
> +		virt_msir = irq_of_parse_and_map(dev->node, i);
> +		if (virt_msir != NO_IRQ) {
> +			set_irq_data(virt_msir, msi);
> +			set_irq_chained_handler(virt_msir, fsl_msi_cascade);
> +			msi->msir[i] = virt_msir;
> +		} else
> +			msi->msir[i] = NO_IRQ;
> +	}
> +
> +	rc = fsl_msi_init_allocator(msi);
> +	if (rc) {
> +		dev_err(&dev->dev, "Error allocating MSI bitmap\n");

If you hit this then the error case needs to get rid of all the irq mappings
you created just above (which it doesn't). It would be simpler if you just move
this call above the for loop, then you don't need to worry about it.

> +		goto error_out;
> +	}
> +
> +	fsl_msi = msi;
> +
> +	WARN_ON(ppc_md.setup_msi_irqs);
> +	ppc_md.setup_msi_irqs = fsl_setup_msi_irqs;
> +	ppc_md.teardown_msi_irqs = fsl_teardown_msi_irqs;
> +	ppc_md.msi_check_device = fsl_msi_check_device;
> +	return 0;
> +error_out:
> +	if (msi)
> +		kfree(msi);

kfree(NULL) is fine.

> +	return err;
> +}
> +
> +static const struct fsl_msi_data mpic_msi_feature = {FSL_PIC_IP_MPIC, 0x140};
> +static const struct fsl_msi_data ipic_msi_feature = {FSL_PIC_IP_IPIC, 0x38};
> +
> +static const struct of_device_id fsl_of_msi_ids[] = {
> +	{
> +		.compatible = "fsl,MPIC-MSI",
> +		.data = (void *)&mpic_msi_feature,
> +	},
> +	{
> +		.compatible = "fsl,IPIC-MSI",
> +		.data = (void *)&ipic_msi_feature,
> +	},
> +	{}
> +};
> +
> +static struct of_platform_driver fsl_of_msi_driver = {
> +	.name = "fsl-of-msi",
> +	.match_table = fsl_of_msi_ids,
> +	.probe = fsl_of_msi_probe,
> +};
> +
> +static __init int fsl_of_msi_init(void)
> +{
> +	return of_register_platform_driver(&fsl_of_msi_driver);
> +}
> +
> +subsys_initcall(fsl_of_msi_init);
> diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
> new file mode 100644
> index 0000000..7eef9ec
> --- /dev/null
> +++ b/arch/powerpc/sysdev/fsl_msi.h
> @@ -0,0 +1,40 @@
> +#ifndef _ASM_POWERPC_MSI_H
> +#define _ASM_POWERPC_MSI_H

Include guards don't match the filename, should be _POWERPC_SYSDEV_FSL_MSI_H

> +#define NR_MSIR	8
> +#define INT_PER_MSIR	32
> +#define NR_MSI_IRQS	(NR_MSIR * INT_PER_MSIR)
> +
> +#define FSL_PIC_IP_MASK	0x0000000F
> +#define FSL_PIC_IP_MPIC	0x00000001
> +#define FSL_PIC_IP_IPIC	0x00000002
> +
> +struct fsl_msi {
> +	/* Device node of the MSI interrupt*/
> +	struct device_node *of_node;
> +
> +	struct irq_host *irqhost;
> +	struct irq_chip *hc_irq;
> +
> +	unsigned long cascade_irq;
> +	unsigned int msir[NR_MSIR];
> +
> +	u32 msi_addr_lo;
> +	u32 msi_addr_hi;
> +	void __iomem *msi_regs;
> +	u32 irq_count;
> +	u32 feature;
> +
> +	spinlock_t fsl_msi_lock;

Unused?

> +	unsigned long *fsl_msi_bitmap;

Do we need fsl_msi in the name?

> +	spinlock_t bitmap_lock;
> +	const char *name;

Unused?

> +};
> +
> +struct fsl_msi_data {
> +	u32 fsl_pic_ip;
> +	u32 msiir_offset;
> +};
> +
> +#endif /* _ASM_POWERPC_MSI_H */
> +
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index bf13c21..fede767 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -106,6 +106,16 @@ void __init setup_pci_cmd(struct pci_controller *hose)
>  	}
>  }
>  
> +#ifdef CONFIG_PCI_MSI
> +void __init setup_pci_pcsrbar(struct pci_controller *hose)
> +{
> +	phys_addr_t immr_base;
> +
> +	immr_base = get_immrbase();
> +	early_write_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, immr_base);
> +}
> +#endif

Up to you, but I prefer an empty static inline here which saves an #ifdef
at the call site.

>  static int fsl_pcie_bus_fixup;
>  
>  static void __init quirk_fsl_pcie_header(struct pci_dev *dev)
> @@ -211,6 +221,10 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
>  	/* Setup PEX window registers */
>  	setup_pci_atmu(hose, &rsrc);
>  
> +	/*Setup PEXCSRBAR */
> +#ifdef CONFIG_PCI_MSI
> +	setup_pci_pcsrbar(hose);
> +#endif
>  	return 0;
>  }


cheers

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

^ permalink raw reply

* linux-next: kvm/powerpc merge conflict
From: Stephen Rothwell @ 2008-04-21  7:34 UTC (permalink / raw)
  To: Avi Kivity, Paul Mackerras; +Cc: linuxppc-dev, linux-next, Hollis Blanchard

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

Hi all,

Today's linux-next merge of the powerpc got a conflict in
include/asm-powerpc/mmu-44x.h between commit
dbc0b98a9feb95a65275d9937d815a3bd4300103 ("ppc: Export tlb_44x_hwater for
KVM") in the kvm tree and commit d04ceb3fc294ea2c4f538a04343f3a473953a3b0
("[POWERPC] Move phys_addr_t definition into asm/types.h") in the powerpc
tree.  The fixup was trivial.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: RFC: MPC5200 PSC AC97 driver
From: Juergen Beisert @ 2008-04-21  8:02 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <4809DF54.2080609@genesi-usa.com>

Matt,

On Saturday 19 April 2008 14:02, Matt Sealey wrote:
> Grant Likely wrote:
> > On Fri, Apr 18, 2008 at 12:11 PM, Matt Sealey <matt@genesi-usa.com> wro=
te:
> >>  Juergen Beisert wrote:
> >
> > BTW, is anyone trying to shepherd this driver into the ALSA tree?  Its
> > out of my area of expertise and responsibility, so I haven't been
> > pursuing it.
>
> We (well, I) were going to try but we got some oopses; from one thing or
> another, but with Juergen's fixes and cell-index in place, now it plays
> audio.
>
> Now it can be pushed to ALSA on that basis, but I'm still trying to get my
> head around where we need to throw patches and if it goes into ALSA Drive=
rs
> first or the kernel or both? It's kind of convoluted compared to the ease
> of just CC'ing netdev or linux-ide..

Please don't forget what Sylvian said about this driver: "I also completely=
=20
skipped the 5200 (not B) support ..." and your own "I think it should be le=
ft=20
noted here that the CCR size changed from 16 bits to 32 bits from 5200 to=20
5200B in order to reduce confusion.". Its not solved yet. So any user of an=
=20
old MPC5200 will be surprised...

Juergen

=2D-=20
Dipl.-Ing. Juergen Beisert | http://www.pengutronix.de
=C2=A0Pengutronix - Linux Solutions for Science and Industry
=C2=A0   Handelsregister: Amtsgericht Hildesheim, HRA 2686
=C2=A0 =C2=A0 =C2=A0    Vertretung Sued/Muenchen, Germany
   Phone: +49-8766-939 228 |  Fax: +49-5121-206917-9

^ permalink raw reply

* Re: removal of arch/ppc in 2.6.27?
From: David Woodhouse @ 2008-04-21  8:03 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <18443.13999.770734.826892@cargo.ozlabs.ibm.com>

On Sun, 2008-04-20 at 22:27 +1000, Paul Mackerras wrote:
> Marvin writes:
> 
> > will this be the end of life for all the PReP's ? I remember a patch posted 
> > some month ago, but didn't heard anything since then. Any news? Or just let 
> > it die quietly?
> 
> No, I'm still planning on getting PReP support over to arch/powerpc,
> but getting time to work on it has been the difficulty.
> 
> What sort of PReP do you have?

I have a 43P-140, which I bought on eBay a few weeks ago to add to the
Fedora testing pile. Foolishly I was trying to get the CHRP kernel
booting on it, just because it says 'chrp' in its device_type.

Unfortunately I seem to have killed it -- I tried to update its firmware
from TIG97251 to TIG04204 and although the update looked successful, it
never came back. Says 'F2A' on the diagnostic panel. I renamed the
firmware file on the floppy to 'precover.img' as the documentation
suggests, but it never even looks at the floppy drive when it starts up.

Anyone know if the firmware flash on these can be removed and flashed
externally, or have any better suggestions?

-- 
dwmw2

^ permalink raw reply

* [PATCH 0/5] IB/ehca: IB compliance fix, tracing verbosity and module parameters
From: Joachim Fenkes @ 2008-04-21  8:03 UTC (permalink / raw)
  To: LinuxPPC-Dev, LKML, OF-General, Roland Dreier, OF-EWG
  Cc: Stefan Roscher, Christoph Raisch

[1/5] makes the driver reject SQ WRs if the QP is not in RTS
[2/5] bumps a lot of tracing into higher debug_levels
[3/5] removes the mr_largepage parameter
[4/5] changes some bool-ish module parms into actual bools,
      also updates some descriptions
[5/5] bumps the version number to 0026

Please review these patches and queue them for inclusion into 2.6.26 if you
think they're okay.

Thanks!
  Joachim

-- 
Joachim Fenkes  --  eHCA Linux Driver Developer and Hardware Tamer
IBM Deutschland Entwicklung GmbH  --  Dept. 3627 (I/O Firmware Dev. 2)
Schoenaicher Strasse 220  --  71032 Boeblingen  --  Germany
eMail: fenkes@de.ibm.com

^ permalink raw reply

* [PATCH 1/5] IB/ehca: Prevent posting of SQ WQEs if QP not in RTS
From: Joachim Fenkes @ 2008-04-21  8:04 UTC (permalink / raw)
  To: LinuxPPC-Dev, LKML, OF-General, Roland Dreier, OF-EWG
  Cc: Stefan Roscher, Christoph Raisch
In-Reply-To: <200804211003.10695.fenkes@de.ibm.com>

...as required by IB Spec, C10-29.

Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
---
 drivers/infiniband/hw/ehca/ehca_classes.h |    1 +
 drivers/infiniband/hw/ehca/ehca_qp.c      |    3 +++
 drivers/infiniband/hw/ehca/ehca_reqs.c    |    5 +++++
 3 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_classes.h b/drivers/infiniband/hw/ehca/ehca_classes.h
index 0d13fe0..3d6d946 100644
--- a/drivers/infiniband/hw/ehca/ehca_classes.h
+++ b/drivers/infiniband/hw/ehca/ehca_classes.h
@@ -160,6 +160,7 @@ struct ehca_qp {
 	};
 	u32 qp_type;
 	enum ehca_ext_qp_type ext_type;
+	enum ib_qp_state state;
 	struct ipz_queue ipz_squeue;
 	struct ipz_queue ipz_rqueue;
 	struct h_galpas galpas;
diff --git a/drivers/infiniband/hw/ehca/ehca_qp.c b/drivers/infiniband/hw/ehca/ehca_qp.c
index 3eb14a5..5a653d7 100644
--- a/drivers/infiniband/hw/ehca/ehca_qp.c
+++ b/drivers/infiniband/hw/ehca/ehca_qp.c
@@ -550,6 +550,7 @@ static struct ehca_qp *internal_create_qp(
 	spin_lock_init(&my_qp->spinlock_r);
 	my_qp->qp_type = qp_type;
 	my_qp->ext_type = parms.ext_type;
+	my_qp->state = IB_QPS_RESET;
 
 	if (init_attr->recv_cq)
 		my_qp->recv_cq =
@@ -1508,6 +1509,8 @@ static int internal_modify_qp(struct ib_qp *ibqp,
 	if (attr_mask & IB_QP_QKEY)
 		my_qp->qkey = attr->qkey;
 
+	my_qp->state = qp_new_state;
+
 modify_qp_exit2:
 	if (squeue_locked) { /* this means: sqe -> rts */
 		spin_unlock_irqrestore(&my_qp->spinlock_s, flags);
diff --git a/drivers/infiniband/hw/ehca/ehca_reqs.c b/drivers/infiniband/hw/ehca/ehca_reqs.c
index a20bbf4..0b2359e 100644
--- a/drivers/infiniband/hw/ehca/ehca_reqs.c
+++ b/drivers/infiniband/hw/ehca/ehca_reqs.c
@@ -421,6 +421,11 @@ int ehca_post_send(struct ib_qp *qp,
 	int ret = 0;
 	unsigned long flags;
 
+	if (unlikely(my_qp->state != IB_QPS_RTS)) {
+		ehca_err(qp->device, "QP not in RTS state  qpn=%x", qp->qp_num);
+		return -EINVAL;
+	}
+
 	/* LOCK the QUEUE */
 	spin_lock_irqsave(&my_qp->spinlock_s, flags);
 
-- 
1.5.5

^ permalink raw reply related

* [PATCH 2/5] IB/ehca: Move high-volume debug output to higher debug levels
From: Joachim Fenkes @ 2008-04-21  8:05 UTC (permalink / raw)
  To: LinuxPPC-Dev, LKML, OF-General, Roland Dreier, OF-EWG
  Cc: Stefan Roscher, Christoph Raisch
In-Reply-To: <200804211003.10695.fenkes@de.ibm.com>

Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
---
 drivers/infiniband/hw/ehca/ehca_irq.c    |    2 +-
 drivers/infiniband/hw/ehca/ehca_main.c   |   14 ++++++--
 drivers/infiniband/hw/ehca/ehca_mrmw.c   |   16 ++++++----
 drivers/infiniband/hw/ehca/ehca_qp.c     |   12 ++++----
 drivers/infiniband/hw/ehca/ehca_reqs.c   |   46 ++++++++++++++---------------
 drivers/infiniband/hw/ehca/ehca_uverbs.c |    6 +--
 drivers/infiniband/hw/ehca/hcp_if.c      |   23 ++++++++-------
 7 files changed, 63 insertions(+), 56 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_irq.c b/drivers/infiniband/hw/ehca/ehca_irq.c
index b5ca94c..ca5eb0c 100644
--- a/drivers/infiniband/hw/ehca/ehca_irq.c
+++ b/drivers/infiniband/hw/ehca/ehca_irq.c
@@ -633,7 +633,7 @@ static inline int find_next_online_cpu(struct ehca_comp_pool *pool)
 	unsigned long flags;
 
 	WARN_ON_ONCE(!in_interrupt());
-	if (ehca_debug_level)
+	if (ehca_debug_level >= 3)
 		ehca_dmp(&cpu_online_map, sizeof(cpumask_t), "");
 
 	spin_lock_irqsave(&pool->last_cpu_lock, flags);
diff --git a/drivers/infiniband/hw/ehca/ehca_main.c b/drivers/infiniband/hw/ehca/ehca_main.c
index 65b3362..4379bef 100644
--- a/drivers/infiniband/hw/ehca/ehca_main.c
+++ b/drivers/infiniband/hw/ehca/ehca_main.c
@@ -85,8 +85,8 @@ module_param_named(lock_hcalls,   ehca_lock_hcalls,   bool, S_IRUGO);
 MODULE_PARM_DESC(open_aqp1,
 		 "AQP1 on startup (0: no (default), 1: yes)");
 MODULE_PARM_DESC(debug_level,
-		 "debug level"
-		 " (0: no debug traces (default), 1: with debug traces)");
+		 "Amount of debug output (0: none (default), 1: traces, "
+		 "2: some dumps, 3: lots)");
 MODULE_PARM_DESC(hw_level,
 		 "hardware level"
 		 " (0: autosensing (default), 1: v. 0.20, 2: v. 0.21)");
@@ -275,6 +275,7 @@ static int ehca_sense_attributes(struct ehca_shca *shca)
 	u64 h_ret;
 	struct hipz_query_hca *rblock;
 	struct hipz_query_port *port;
+	const char *loc_code;
 
 	static const u32 pgsize_map[] = {
 		HCA_CAP_MR_PGSIZE_4K,  0x1000,
@@ -283,6 +284,12 @@ static int ehca_sense_attributes(struct ehca_shca *shca)
 		HCA_CAP_MR_PGSIZE_16M, 0x1000000,
 	};
 
+	ehca_gen_dbg("Probing adapter %s...",
+		     shca->ofdev->node->full_name);
+	loc_code = of_get_property(shca->ofdev->node, "ibm,loc-code", NULL);
+	if (loc_code)
+		ehca_gen_dbg(" ... location lode=%s", loc_code);
+
 	rblock = ehca_alloc_fw_ctrlblock(GFP_KERNEL);
 	if (!rblock) {
 		ehca_gen_err("Cannot allocate rblock memory.");
@@ -567,8 +574,7 @@ static int ehca_destroy_aqp1(struct ehca_sport *sport)
 
 static ssize_t ehca_show_debug_level(struct device_driver *ddp, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n",
-			ehca_debug_level);
+	return snprintf(buf, PAGE_SIZE, "%d\n", ehca_debug_level);
 }
 
 static ssize_t ehca_store_debug_level(struct device_driver *ddp,
diff --git a/drivers/infiniband/hw/ehca/ehca_mrmw.c b/drivers/infiniband/hw/ehca/ehca_mrmw.c
index f26997f..46ae4eb 100644
--- a/drivers/infiniband/hw/ehca/ehca_mrmw.c
+++ b/drivers/infiniband/hw/ehca/ehca_mrmw.c
@@ -1794,8 +1794,9 @@ static int ehca_check_kpages_per_ate(struct scatterlist *page_list,
 	int t;
 	for (t = start_idx; t <= end_idx; t++) {
 		u64 pgaddr = page_to_pfn(sg_page(&page_list[t])) << PAGE_SHIFT;
-		ehca_gen_dbg("chunk_page=%lx value=%016lx", pgaddr,
-			     *(u64 *)abs_to_virt(phys_to_abs(pgaddr)));
+		if (ehca_debug_level >= 3)
+			ehca_gen_dbg("chunk_page=%lx value=%016lx", pgaddr,
+				     *(u64 *)abs_to_virt(phys_to_abs(pgaddr)));
 		if (pgaddr - PAGE_SIZE != *prev_pgaddr) {
 			ehca_gen_err("uncontiguous page found pgaddr=%lx "
 				     "prev_pgaddr=%lx page_list_i=%x",
@@ -1862,10 +1863,13 @@ static int ehca_set_pagebuf_user2(struct ehca_mr_pginfo *pginfo,
 						pgaddr &
 						~(pginfo->hwpage_size - 1));
 				}
-				ehca_gen_dbg("kpage=%lx chunk_page=%lx "
-					     "value=%016lx", *kpage, pgaddr,
-					     *(u64 *)abs_to_virt(
-						     phys_to_abs(pgaddr)));
+				if (ehca_debug_level >= 3) {
+					u64 val = *(u64 *)abs_to_virt(
+						phys_to_abs(pgaddr));
+					ehca_gen_dbg("kpage=%lx chunk_page=%lx "
+						     "value=%016lx",
+						     *kpage, pgaddr, val);
+				}
 				prev_pgaddr = pgaddr;
 				i++;
 				pginfo->kpage_cnt++;
diff --git a/drivers/infiniband/hw/ehca/ehca_qp.c b/drivers/infiniband/hw/ehca/ehca_qp.c
index 5a653d7..57bef11 100644
--- a/drivers/infiniband/hw/ehca/ehca_qp.c
+++ b/drivers/infiniband/hw/ehca/ehca_qp.c
@@ -966,7 +966,7 @@ static int prepare_sqe_rts(struct ehca_qp *my_qp, struct ehca_shca *shca,
 		 qp_num, bad_send_wqe_p);
 	/* convert wqe pointer to vadr */
 	bad_send_wqe_v = abs_to_virt((u64)bad_send_wqe_p);
-	if (ehca_debug_level)
+	if (ehca_debug_level >= 2)
 		ehca_dmp(bad_send_wqe_v, 32, "qp_num=%x bad_wqe", qp_num);
 	squeue = &my_qp->ipz_squeue;
 	if (ipz_queue_abs_to_offset(squeue, (u64)bad_send_wqe_p, &q_ofs)) {
@@ -979,7 +979,7 @@ static int prepare_sqe_rts(struct ehca_qp *my_qp, struct ehca_shca *shca,
 	wqe = (struct ehca_wqe *)ipz_qeit_calc(squeue, q_ofs);
 	*bad_wqe_cnt = 0;
 	while (wqe->optype != 0xff && wqe->wqef != 0xff) {
-		if (ehca_debug_level)
+		if (ehca_debug_level >= 2)
 			ehca_dmp(wqe, 32, "qp_num=%x wqe", qp_num);
 		wqe->nr_of_data_seg = 0; /* suppress data access */
 		wqe->wqef = WQEF_PURGE; /* WQE to be purged */
@@ -1451,7 +1451,7 @@ static int internal_modify_qp(struct ib_qp *ibqp,
 		/* no support for max_send/recv_sge yet */
 	}
 
-	if (ehca_debug_level)
+	if (ehca_debug_level >= 2)
 		ehca_dmp(mqpcb, 4*70, "qp_num=%x", ibqp->qp_num);
 
 	h_ret = hipz_h_modify_qp(shca->ipz_hca_handle,
@@ -1766,7 +1766,7 @@ int ehca_query_qp(struct ib_qp *qp,
 	if (qp_init_attr)
 		*qp_init_attr = my_qp->init_attr;
 
-	if (ehca_debug_level)
+	if (ehca_debug_level >= 2)
 		ehca_dmp(qpcb, 4*70, "qp_num=%x", qp->qp_num);
 
 query_qp_exit1:
@@ -1814,7 +1814,7 @@ int ehca_modify_srq(struct ib_srq *ibsrq, struct ib_srq_attr *attr,
 		goto modify_srq_exit0;
 	}
 
-	if (ehca_debug_level)
+	if (ehca_debug_level >= 2)
 		ehca_dmp(mqpcb, 4*70, "qp_num=%x", my_qp->real_qp_num);
 
 	h_ret = hipz_h_modify_qp(shca->ipz_hca_handle, my_qp->ipz_qp_handle,
@@ -1867,7 +1867,7 @@ int ehca_query_srq(struct ib_srq *srq, struct ib_srq_attr *srq_attr)
 	srq_attr->srq_limit = EHCA_BMASK_GET(
 		MQPCB_CURR_SRQ_LIMIT, qpcb->curr_srq_limit);
 
-	if (ehca_debug_level)
+	if (ehca_debug_level >= 2)
 		ehca_dmp(qpcb, 4*70, "qp_num=%x", my_qp->real_qp_num);
 
 query_srq_exit1:
diff --git a/drivers/infiniband/hw/ehca/ehca_reqs.c b/drivers/infiniband/hw/ehca/ehca_reqs.c
index 0b2359e..bbe0436 100644
--- a/drivers/infiniband/hw/ehca/ehca_reqs.c
+++ b/drivers/infiniband/hw/ehca/ehca_reqs.c
@@ -81,7 +81,7 @@ static inline int ehca_write_rwqe(struct ipz_queue *ipz_rqueue,
 			recv_wr->sg_list[cnt_ds].length;
 	}
 
-	if (ehca_debug_level) {
+	if (ehca_debug_level >= 3) {
 		ehca_gen_dbg("RECEIVE WQE written into ipz_rqueue=%p",
 			     ipz_rqueue);
 		ehca_dmp(wqe_p, 16*(6 + wqe_p->nr_of_data_seg), "recv wqe");
@@ -281,7 +281,7 @@ static inline int ehca_write_swqe(struct ehca_qp *qp,
 		return -EINVAL;
 	}
 
-	if (ehca_debug_level) {
+	if (ehca_debug_level >= 3) {
 		ehca_gen_dbg("SEND WQE written into queue qp=%p ", qp);
 		ehca_dmp( wqe_p, 16*(6 + wqe_p->nr_of_data_seg), "send wqe");
 	}
@@ -459,13 +459,14 @@ int ehca_post_send(struct ib_qp *qp,
 			goto post_send_exit0;
 		}
 		wqe_cnt++;
-		ehca_dbg(qp->device, "ehca_qp=%p qp_num=%x wqe_cnt=%d",
-			 my_qp, qp->qp_num, wqe_cnt);
 	} /* eof for cur_send_wr */
 
 post_send_exit0:
 	iosync(); /* serialize GAL register access */
 	hipz_update_sqa(my_qp, wqe_cnt);
+	if (unlikely(ret || ehca_debug_level >= 2))
+		ehca_dbg(qp->device, "ehca_qp=%p qp_num=%x wqe_cnt=%d ret=%i",
+			 my_qp, qp->qp_num, wqe_cnt, ret);
 	my_qp->message_count += wqe_cnt;
 	spin_unlock_irqrestore(&my_qp->spinlock_s, flags);
 	return ret;
@@ -525,13 +526,14 @@ static int internal_post_recv(struct ehca_qp *my_qp,
 			goto post_recv_exit0;
 		}
 		wqe_cnt++;
-		ehca_dbg(dev, "ehca_qp=%p qp_num=%x wqe_cnt=%d",
-			 my_qp, my_qp->real_qp_num, wqe_cnt);
 	} /* eof for cur_recv_wr */
 
 post_recv_exit0:
 	iosync(); /* serialize GAL register access */
 	hipz_update_rqa(my_qp, wqe_cnt);
+	if (unlikely(ret || ehca_debug_level >= 2))
+	    ehca_dbg(dev, "ehca_qp=%p qp_num=%x wqe_cnt=%d ret=%i",
+		     my_qp, my_qp->real_qp_num, wqe_cnt, ret);
 	spin_unlock_irqrestore(&my_qp->spinlock_r, flags);
 	return ret;
 }
@@ -575,16 +577,17 @@ static inline int ehca_poll_cq_one(struct ib_cq *cq, struct ib_wc *wc)
 	struct ehca_cq *my_cq = container_of(cq, struct ehca_cq, ib_cq);
 	struct ehca_cqe *cqe;
 	struct ehca_qp *my_qp;
-	int cqe_count = 0;
+	int cqe_count = 0, is_error;
 
 poll_cq_one_read_cqe:
 	cqe = (struct ehca_cqe *)
 		ipz_qeit_get_inc_valid(&my_cq->ipz_queue);
 	if (!cqe) {
 		ret = -EAGAIN;
-		ehca_dbg(cq->device, "Completion queue is empty ehca_cq=%p "
-			 "cq_num=%x ret=%i", my_cq, my_cq->cq_number, ret);
-		goto  poll_cq_one_exit0;
+		if (ehca_debug_level >= 3)
+			ehca_dbg(cq->device, "Completion queue is empty  "
+				 "my_cq=%p cq_num=%x", my_cq, my_cq->cq_number);
+		goto poll_cq_one_exit0;
 	}
 
 	/* prevents loads being reordered across this point */
@@ -614,7 +617,7 @@ poll_cq_one_read_cqe:
 			ehca_dbg(cq->device,
 				 "Got CQE with purged bit qp_num=%x src_qp=%x",
 				 cqe->local_qp_number, cqe->remote_qp_number);
-			if (ehca_debug_level)
+			if (ehca_debug_level >= 2)
 				ehca_dmp(cqe, 64, "qp_num=%x src_qp=%x",
 					 cqe->local_qp_number,
 					 cqe->remote_qp_number);
@@ -627,11 +630,13 @@ poll_cq_one_read_cqe:
 		}
 	}
 
-	/* tracing cqe */
-	if (unlikely(ehca_debug_level)) {
+	is_error = cqe->status & WC_STATUS_ERROR_BIT;
+
+	/* trace error CQEs if debug_level >= 1, trace all CQEs if >= 3 */
+	if (unlikely(ehca_debug_level >= 3 || (ehca_debug_level && is_error))) {
 		ehca_dbg(cq->device,
-			 "Received COMPLETION ehca_cq=%p cq_num=%x -----",
-			 my_cq, my_cq->cq_number);
+			 "Received %sCOMPLETION ehca_cq=%p cq_num=%x -----",
+			 is_error ? "ERROR " : "", my_cq, my_cq->cq_number);
 		ehca_dmp(cqe, 64, "ehca_cq=%p cq_num=%x",
 			 my_cq, my_cq->cq_number);
 		ehca_dbg(cq->device,
@@ -654,8 +659,9 @@ poll_cq_one_read_cqe:
 		/* update also queue adder to throw away this entry!!! */
 		goto poll_cq_one_exit0;
 	}
+
 	/* eval ib_wc_status */
-	if (unlikely(cqe->status & WC_STATUS_ERROR_BIT)) {
+	if (unlikely(is_error)) {
 		/* complete with errors */
 		map_ib_wc_status(cqe->status, &wc->status);
 		wc->vendor_err = wc->status;
@@ -676,14 +682,6 @@ poll_cq_one_read_cqe:
 	wc->imm_data = cpu_to_be32(cqe->immediate_data);
 	wc->sl = cqe->service_level;
 
-	if (unlikely(wc->status != IB_WC_SUCCESS))
-		ehca_dbg(cq->device,
-			 "ehca_cq=%p cq_num=%x WARNING unsuccessful cqe "
-			 "OPType=%x status=%x qp_num=%x src_qp=%x wr_id=%lx "
-			 "cqe=%p", my_cq, my_cq->cq_number, cqe->optype,
-			 cqe->status, cqe->local_qp_number,
-			 cqe->remote_qp_number, cqe->work_request_id, cqe);
-
 poll_cq_one_exit0:
 	if (cqe_count > 0)
 		hipz_update_feca(my_cq, cqe_count);
diff --git a/drivers/infiniband/hw/ehca/ehca_uverbs.c b/drivers/infiniband/hw/ehca/ehca_uverbs.c
index 1b07f2b..e43ed8f 100644
--- a/drivers/infiniband/hw/ehca/ehca_uverbs.c
+++ b/drivers/infiniband/hw/ehca/ehca_uverbs.c
@@ -211,8 +211,7 @@ static int ehca_mmap_qp(struct vm_area_struct *vma, struct ehca_qp *qp,
 		break;
 
 	case 1: /* qp rqueue_addr */
-		ehca_dbg(qp->ib_qp.device, "qp_num=%x rqueue",
-			 qp->ib_qp.qp_num);
+		ehca_dbg(qp->ib_qp.device, "qp_num=%x rq", qp->ib_qp.qp_num);
 		ret = ehca_mmap_queue(vma, &qp->ipz_rqueue,
 				      &qp->mm_count_rqueue);
 		if (unlikely(ret)) {
@@ -224,8 +223,7 @@ static int ehca_mmap_qp(struct vm_area_struct *vma, struct ehca_qp *qp,
 		break;
 
 	case 2: /* qp squeue_addr */
-		ehca_dbg(qp->ib_qp.device, "qp_num=%x squeue",
-			 qp->ib_qp.qp_num);
+		ehca_dbg(qp->ib_qp.device, "qp_num=%x sq", qp->ib_qp.qp_num);
 		ret = ehca_mmap_queue(vma, &qp->ipz_squeue,
 				      &qp->mm_count_squeue);
 		if (unlikely(ret)) {
diff --git a/drivers/infiniband/hw/ehca/hcp_if.c b/drivers/infiniband/hw/ehca/hcp_if.c
index 7029aa6..5245e13 100644
--- a/drivers/infiniband/hw/ehca/hcp_if.c
+++ b/drivers/infiniband/hw/ehca/hcp_if.c
@@ -123,8 +123,9 @@ static long ehca_plpar_hcall_norets(unsigned long opcode,
 	int i, sleep_msecs;
 	unsigned long flags = 0;
 
-	ehca_gen_dbg("opcode=%lx " HCALL7_REGS_FORMAT,
-		     opcode, arg1, arg2, arg3, arg4, arg5, arg6, arg7);
+	if (unlikely(ehca_debug_level >= 2))
+		ehca_gen_dbg("opcode=%lx " HCALL7_REGS_FORMAT,
+			     opcode, arg1, arg2, arg3, arg4, arg5, arg6, arg7);
 
 	for (i = 0; i < 5; i++) {
 		/* serialize hCalls to work around firmware issue */
@@ -148,7 +149,8 @@ static long ehca_plpar_hcall_norets(unsigned long opcode,
 				     opcode, ret, arg1, arg2, arg3,
 				     arg4, arg5, arg6, arg7);
 		else
-			ehca_gen_dbg("opcode=%lx ret=%li", opcode, ret);
+			if (unlikely(ehca_debug_level >= 2))
+				ehca_gen_dbg("opcode=%lx ret=%li", opcode, ret);
 
 		return ret;
 	}
@@ -172,8 +174,10 @@ static long ehca_plpar_hcall9(unsigned long opcode,
 	int i, sleep_msecs;
 	unsigned long flags = 0;
 
-	ehca_gen_dbg("INPUT -- opcode=%lx " HCALL9_REGS_FORMAT, opcode,
-		     arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9);
+	if (unlikely(ehca_debug_level >= 2))
+		ehca_gen_dbg("INPUT -- opcode=%lx " HCALL9_REGS_FORMAT, opcode,
+			     arg1, arg2, arg3, arg4, arg5,
+			     arg6, arg7, arg8, arg9);
 
 	for (i = 0; i < 5; i++) {
 		/* serialize hCalls to work around firmware issue */
@@ -201,7 +205,7 @@ static long ehca_plpar_hcall9(unsigned long opcode,
 				     ret, outs[0], outs[1], outs[2], outs[3],
 				     outs[4], outs[5], outs[6], outs[7],
 				     outs[8]);
-		} else
+		} else if (unlikely(ehca_debug_level >= 2))
 			ehca_gen_dbg("OUTPUT -- ret=%li " HCALL9_REGS_FORMAT,
 				     ret, outs[0], outs[1], outs[2], outs[3],
 				     outs[4], outs[5], outs[6], outs[7],
@@ -381,7 +385,7 @@ u64 hipz_h_query_port(const struct ipz_adapter_handle adapter_handle,
 				      r_cb,	             /* r6 */
 				      0, 0, 0, 0);
 
-	if (ehca_debug_level)
+	if (ehca_debug_level >= 2)
 		ehca_dmp(query_port_response_block, 64, "response_block");
 
 	return ret;
@@ -731,9 +735,6 @@ u64 hipz_h_alloc_resource_mr(const struct ipz_adapter_handle adapter_handle,
 	u64 ret;
 	u64 outs[PLPAR_HCALL9_BUFSIZE];
 
-	ehca_gen_dbg("kernel PAGE_SIZE=%x access_ctrl=%016x "
-		     "vaddr=%lx length=%lx",
-		     (u32)PAGE_SIZE, access_ctrl, vaddr, length);
 	ret = ehca_plpar_hcall9(H_ALLOC_RESOURCE, outs,
 				adapter_handle.handle,            /* r4 */
 				5,                                /* r5 */
@@ -758,7 +759,7 @@ u64 hipz_h_register_rpage_mr(const struct ipz_adapter_handle adapter_handle,
 {
 	u64 ret;
 
-	if (unlikely(ehca_debug_level >= 2)) {
+	if (unlikely(ehca_debug_level >= 3)) {
 		if (count > 1) {
 			u64 *kpage;
 			int i;
-- 
1.5.5

^ permalink raw reply related

* [PATCH 3/5] IB/ehca: Remove mr_largepage parameter
From: Joachim Fenkes @ 2008-04-21  8:06 UTC (permalink / raw)
  To: LinuxPPC-Dev, LKML, OF-General, Roland Dreier, OF-EWG
  Cc: Stefan Roscher, Christoph Raisch
In-Reply-To: <200804211003.10695.fenkes@de.ibm.com>

Always enable large page support; didn't seem to cause problems for anyone.

Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
---
 drivers/infiniband/hw/ehca/ehca_main.c |   22 +++-------------------
 1 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_main.c b/drivers/infiniband/hw/ehca/ehca_main.c
index 4379bef..ab02ac8 100644
--- a/drivers/infiniband/hw/ehca/ehca_main.c
+++ b/drivers/infiniband/hw/ehca/ehca_main.c
@@ -60,7 +60,6 @@ MODULE_VERSION(HCAD_VERSION);
 static int ehca_open_aqp1     = 0;
 static int ehca_hw_level      = 0;
 static int ehca_poll_all_eqs  = 1;
-static int ehca_mr_largepage  = 1;
 
 int ehca_debug_level   = 0;
 int ehca_nr_ports      = 2;
@@ -79,7 +78,6 @@ module_param_named(port_act_time, ehca_port_act_time, int, S_IRUGO);
 module_param_named(poll_all_eqs,  ehca_poll_all_eqs,  int, S_IRUGO);
 module_param_named(static_rate,   ehca_static_rate,   int, S_IRUGO);
 module_param_named(scaling_code,  ehca_scaling_code,  int, S_IRUGO);
-module_param_named(mr_largepage,  ehca_mr_largepage,  int, S_IRUGO);
 module_param_named(lock_hcalls,   ehca_lock_hcalls,   bool, S_IRUGO);
 
 MODULE_PARM_DESC(open_aqp1,
@@ -104,9 +102,6 @@ MODULE_PARM_DESC(static_rate,
 		 "set permanent static rate (default: disabled)");
 MODULE_PARM_DESC(scaling_code,
 		 "set scaling code (0: disabled/default, 1: enabled)");
-MODULE_PARM_DESC(mr_largepage,
-		 "use large page for MR (0: use PAGE_SIZE (default), "
-		 "1: use large page depending on MR size");
 MODULE_PARM_DESC(lock_hcalls,
 		 "serialize all hCalls made by the driver "
 		 "(default: autodetect)");
@@ -357,11 +352,9 @@ static int ehca_sense_attributes(struct ehca_shca *shca)
 
 	/* translate supported MR page sizes; always support 4K */
 	shca->hca_cap_mr_pgsize = EHCA_PAGESIZE;
-	if (ehca_mr_largepage) { /* support extra sizes only if enabled */
-		for (i = 0; i < ARRAY_SIZE(pgsize_map); i += 2)
-			if (rblock->memory_page_size_supported & pgsize_map[i])
-				shca->hca_cap_mr_pgsize |= pgsize_map[i + 1];
-	}
+	for (i = 0; i < ARRAY_SIZE(pgsize_map); i += 2)
+		if (rblock->memory_page_size_supported & pgsize_map[i])
+			shca->hca_cap_mr_pgsize |= pgsize_map[i + 1];
 
 	/* query max MTU from first port -- it's the same for all ports */
 	port = (struct hipz_query_port *)rblock;
@@ -663,14 +656,6 @@ static ssize_t ehca_show_adapter_handle(struct device *dev,
 }
 static DEVICE_ATTR(adapter_handle, S_IRUGO, ehca_show_adapter_handle, NULL);
 
-static ssize_t ehca_show_mr_largepage(struct device *dev,
-				      struct device_attribute *attr,
-				      char *buf)
-{
-	return sprintf(buf, "%d\n", ehca_mr_largepage);
-}
-static DEVICE_ATTR(mr_largepage, S_IRUGO, ehca_show_mr_largepage, NULL);
-
 static struct attribute *ehca_dev_attrs[] = {
 	&dev_attr_adapter_handle.attr,
 	&dev_attr_num_ports.attr,
@@ -687,7 +672,6 @@ static struct attribute *ehca_dev_attrs[] = {
 	&dev_attr_cur_mw.attr,
 	&dev_attr_max_pd.attr,
 	&dev_attr_max_ah.attr,
-	&dev_attr_mr_largepage.attr,
 	NULL
 };
 
-- 
1.5.5

^ permalink raw reply related

* [PATCH 4/5] IB/ehca: Make some module parameters bool, update descriptions
From: Joachim Fenkes @ 2008-04-21  8:06 UTC (permalink / raw)
  To: LinuxPPC-Dev, LKML, OF-General, Roland Dreier, OF-EWG
  Cc: Stefan Roscher, Christoph Raisch
In-Reply-To: <200804211003.10695.fenkes@de.ibm.com>

Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
---
 drivers/infiniband/hw/ehca/ehca_main.c |   37 +++++++++++++++----------------
 1 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_main.c b/drivers/infiniband/hw/ehca/ehca_main.c
index ab02ac8..45fe35a 100644
--- a/drivers/infiniband/hw/ehca/ehca_main.c
+++ b/drivers/infiniband/hw/ehca/ehca_main.c
@@ -69,41 +69,40 @@ int ehca_static_rate   = -1;
 int ehca_scaling_code  = 0;
 int ehca_lock_hcalls   = -1;
 
-module_param_named(open_aqp1,     ehca_open_aqp1,     int, S_IRUGO);
-module_param_named(debug_level,   ehca_debug_level,   int, S_IRUGO);
-module_param_named(hw_level,      ehca_hw_level,      int, S_IRUGO);
-module_param_named(nr_ports,      ehca_nr_ports,      int, S_IRUGO);
-module_param_named(use_hp_mr,     ehca_use_hp_mr,     int, S_IRUGO);
-module_param_named(port_act_time, ehca_port_act_time, int, S_IRUGO);
-module_param_named(poll_all_eqs,  ehca_poll_all_eqs,  int, S_IRUGO);
-module_param_named(static_rate,   ehca_static_rate,   int, S_IRUGO);
-module_param_named(scaling_code,  ehca_scaling_code,  int, S_IRUGO);
+module_param_named(open_aqp1,     ehca_open_aqp1,     bool, S_IRUGO);
+module_param_named(debug_level,   ehca_debug_level,   int,  S_IRUGO);
+module_param_named(hw_level,      ehca_hw_level,      int,  S_IRUGO);
+module_param_named(nr_ports,      ehca_nr_ports,      int,  S_IRUGO);
+module_param_named(use_hp_mr,     ehca_use_hp_mr,     bool, S_IRUGO);
+module_param_named(port_act_time, ehca_port_act_time, int,  S_IRUGO);
+module_param_named(poll_all_eqs,  ehca_poll_all_eqs,  bool, S_IRUGO);
+module_param_named(static_rate,   ehca_static_rate,   int,  S_IRUGO);
+module_param_named(scaling_code,  ehca_scaling_code,  bool, S_IRUGO);
 module_param_named(lock_hcalls,   ehca_lock_hcalls,   bool, S_IRUGO);
 
 MODULE_PARM_DESC(open_aqp1,
-		 "AQP1 on startup (0: no (default), 1: yes)");
+		 "Open AQP1 on startup (default: no)");
 MODULE_PARM_DESC(debug_level,
 		 "Amount of debug output (0: none (default), 1: traces, "
 		 "2: some dumps, 3: lots)");
 MODULE_PARM_DESC(hw_level,
-		 "hardware level"
-		 " (0: autosensing (default), 1: v. 0.20, 2: v. 0.21)");
+		 "Hardware level (0: autosensing (default), "
+		 "0x10..0x14: eHCA, 0x20..0x23: eHCA2)");
 MODULE_PARM_DESC(nr_ports,
 		 "number of connected ports (-1: autodetect, 1: port one only, "
 		 "2: two ports (default)");
 MODULE_PARM_DESC(use_hp_mr,
-		 "high performance MRs (0: no (default), 1: yes)");
+		 "Use high performance MRs (default: no)");
 MODULE_PARM_DESC(port_act_time,
-		 "time to wait for port activation (default: 30 sec)");
+		 "Time to wait for port activation (default: 30 sec)");
 MODULE_PARM_DESC(poll_all_eqs,
-		 "polls all event queues periodically"
-		 " (0: no, 1: yes (default))");
+		 "Poll all event queues periodically (default: yes)");
 MODULE_PARM_DESC(static_rate,
-		 "set permanent static rate (default: disabled)");
+		 "Set permanent static rate (default: no static rate)");
 MODULE_PARM_DESC(scaling_code,
-		 "set scaling code (0: disabled/default, 1: enabled)");
+		 "Enable scaling code (default: no)");
 MODULE_PARM_DESC(lock_hcalls,
-		 "serialize all hCalls made by the driver "
+		 "Serialize all hCalls made by the driver "
 		 "(default: autodetect)");
 
 DEFINE_RWLOCK(ehca_qp_idr_lock);
-- 
1.5.5

^ permalink raw reply related

* [PATCH 5/5] IB/ehca: Bump version number to 0026
From: Joachim Fenkes @ 2008-04-21  8:08 UTC (permalink / raw)
  To: LinuxPPC-Dev, LKML, OF-General, Roland Dreier, OF-EWG
  Cc: Stefan Roscher, Christoph Raisch
In-Reply-To: <200804211003.10695.fenkes@de.ibm.com>

Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
---
 drivers/infiniband/hw/ehca/ehca_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_main.c b/drivers/infiniband/hw/ehca/ehca_main.c
index 45fe35a..6504897 100644
--- a/drivers/infiniband/hw/ehca/ehca_main.c
+++ b/drivers/infiniband/hw/ehca/ehca_main.c
@@ -50,7 +50,7 @@
 #include "ehca_tools.h"
 #include "hcp_if.h"
 
-#define HCAD_VERSION "0025"
+#define HCAD_VERSION "0026"
 
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_AUTHOR("Christoph Raisch <raisch@de.ibm.com>");
-- 
1.5.5

^ permalink raw reply related

* Re: [PATCH] [POWERPC] mpc5200: Switch mpc5200 dts files to dts-v1 format
From: Sascha Hauer @ 2008-04-21  8:43 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, m8
In-Reply-To: <20080418160812.11897.5654.stgit@trillian.secretlab.ca>

On Fri, Apr 18, 2008 at 09:10:04AM -0700, Grant Likely wrote:
> Update dts files to current format
> 
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 
>  arch/powerpc/boot/dts/cm5200.dts    |   98 +++++++++++------------
>  arch/powerpc/boot/dts/lite5200.dts  |  132 ++++++++++++++++----------------
>  arch/powerpc/boot/dts/lite5200b.dts |  146 +++++++++++++++++------------------
>  arch/powerpc/boot/dts/motionpro.dts |  118 ++++++++++++++--------------
>  arch/powerpc/boot/dts/tqm5200.dts   |   80 ++++++++++---------
>  5 files changed, 286 insertions(+), 288 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/cm5200.dts b/arch/powerpc/boot/dts/cm5200.dts
> index c6ca631..2f74cc4 100644
> --- a/arch/powerpc/boot/dts/cm5200.dts
> +++ b/arch/powerpc/boot/dts/cm5200.dts
> @@ -10,11 +10,7 @@
>   * option) any later version.
>   */
>  
> -/*
> - * WARNING: Do not depend on this tree layout remaining static just yet.
> - * The MPC5200 device tree conventions are still in flux
> - * Keep an eye on the linuxppc-dev mailing list for more details
> - */
> +/dts-v1/;
>  
>  / {
>  	model = "schindler,cm5200";
> @@ -29,10 +25,10 @@
>  		PowerPC,5200@0 {
>  			device_type = "cpu";
>  			reg = <0>;
> -			d-cache-line-size = <20>;
> -			i-cache-line-size = <20>;
> -			d-cache-size = <4000>;		// L1, 16K
> -			i-cache-size = <4000>;		// L1, 16K
> +			d-cache-line-size = <32>;
> +			i-cache-line-size = <32>;

Has the standard number format changed to decimal lately? Otherwise this
looks wrong. There are more occurences of this in the other dts files.

Sascha

-- 
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-

^ permalink raw reply

* stuck on update mpc834x based board from 2.6.20 to 2.6.21
From: Fathi Boudra @ 2008-04-21  8:45 UTC (permalink / raw)
  To: linuxppc-embedded

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

Hi,

I've got a mpc8347e based board with a working 2.6.20 kernel and a dtb.
I wasn't able to boot on a more recent kernel (>2.6.20).

To update my config. I tried ppc and powerpc arch.:

a) make ARCH=ppc CROSS_COMPILE=ppc_6xx- oldconfig
then select 6xx/MPC834X SYS

b) make ARCH=powerpc CROSS_COMPILE=ppc_6xx- oldconfig
then select 83XX/MPC834X MDS

On each generated kernel, I'm stuck at:
Booting using flat device tree at 0x3c0000

For infos, I use ELDK 4.2, U-Boot 1.1.4 and always the same dtb.

did I miss something ?

cheers,

Fathi

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

^ permalink raw reply

* Re: [PATCH 1/5] IB/ehca: Prevent posting of SQ WQEs if QP not in RTS
From: Joachim Fenkes @ 2008-04-21  8:45 UTC (permalink / raw)
  To: LinuxPPC-Dev; +Cc: LKML, OF-EWG, Christoph Raisch, OF-General, Stefan Roscher
In-Reply-To: <200804211004.44666.fenkes@de.ibm.com>

On Monday 21 April 2008 10:04, Joachim Fenkes wrote:

> +	if (unlikely(my_qp->state != IB_QPS_RTS)) {
> +		ehca_err(qp->device, "QP not in RTS state  qpn=%x", qp->qp_num);
> +		return -EINVAL;
> +	}

Myself, I'm not very happy with using EINVAL, but I can't think of a more
fitting return code. Also, this is what nes, amso and cxgb3 return in such a
case; ipath posts an error CQE and mthca/mlx4 don't do this check at all
(AFAICS).

Better suggestions, anyone?

Regards,
  Joachim

^ permalink raw reply

* Re: [PATCH 2.6.26?] Raise the upper limit of NR_CPUS.
From: Michael Ellerman @ 2008-04-21  9:05 UTC (permalink / raw)
  To: benh; +Cc: LinuxPPC-dev, Paul Mackerras
In-Reply-To: <1208743427.10486.2.camel@pasglop>

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

On Mon, 2008-04-21 at 12:03 +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2008-04-18 at 15:33 +1000, Tony Breeds wrote:
> > As the pacas are statically initialised increasing NR_CPUS beyond 128,
> > means that any additional pacas will be empty  ... which is bad.
> > 
> > This patch adds the required functionality to fill in any excess pacas
> > at runtime.
> > 
> > Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> > ---
> > I know it's late, but can this be considered for 2.6.26?
> 
> NAK.

Tony, you should NAK Ben tomorrow after lunch if you know what I mean :)

> You must NEVER manipulate kernel globals from prom_init.c. The fact that
> prom_init is linked with the kernel is an "accident" which may change.

A very long running accident, we should really look at fixing it some
time.

> Your scheme would break among others with kexec or non-OF bootloaders

I talked to Tony about it this morning and we came up with a scheme that
should work for all cases.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

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

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

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

^ permalink raw reply

* linux-next: x86-latest/powerpc-next merge conflict
From: Stephen Rothwell @ 2008-04-21  9:12 UTC (permalink / raw)
  To: Ingo Molnar, Paul Mackerras; +Cc: Heukelum, linuxppc-dev, linux-next, Alexander

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

Hi all,

Today's linux-next merge of the x86-latest tree got a conflict in
include/asm-powerpc/bitops.h between commit
cd008c0f03f3d451e5fbd108b8e74079d402be64 ("generic: implement __fls on
all 64-bit archs") from the x86-latest tree and commit
9f264be6101c42cb9e471c58322fb83a5cde1461 ("[POWERPC] Optimize fls64() on
64-bit processors") from the powerpc-next tree.  The fixup was not quite
trivial and is worth a look to see if I got it right.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH 2.6.26?] Raise the upper limit of NR_CPUS.
From: Benjamin Herrenschmidt @ 2008-04-21  9:24 UTC (permalink / raw)
  To: michael; +Cc: LinuxPPC-dev, Paul Mackerras
In-Reply-To: <1208768706.9955.9.camel@concordia>


On Mon, 2008-04-21 at 19:05 +1000, Michael Ellerman wrote:
> 
> Tony, you should NAK Ben tomorrow after lunch if you know what I
> mean :)

Should I hide ? :-)

> > You must NEVER manipulate kernel globals from prom_init.c. The fact
> that
> > prom_init is linked with the kernel is an "accident" which may
> change.
> 
> A very long running accident, we should really look at fixing it some
> time.

Patch welcome :-)

Historically, it used to manipulate some kernel data structures, but we
fixed all of that when we introduced the flat DT model.

> > Your scheme would break among others with kexec or non-OF
> bootloaders
> 
> I talked to Tony about it this morning and we came up with a scheme
> that
> should work for all cases.

Excellent.

Cheers,
Ben.

^ permalink raw reply

* Re: linux-next: x86-latest/powerpc-next merge conflict
From: Ingo Molnar @ 2008-04-21  9:51 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Alexander van Heukelum, linuxppc-dev, linux-next, Paul Mackerras
In-Reply-To: <20080421191231.41a34aef.sfr@canb.auug.org.au>


* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next merge of the x86-latest tree got a conflict in 
> include/asm-powerpc/bitops.h between commit 
> cd008c0f03f3d451e5fbd108b8e74079d402be64 ("generic: implement __fls on 
> all 64-bit archs") from the x86-latest tree and commit 
> 9f264be6101c42cb9e471c58322fb83a5cde1461 ("[POWERPC] Optimize fls64() 
> on 64-bit processors") from the powerpc-next tree.  The fixup was not 
> quite trivial and is worth a look to see if I got it right.

Paul, do you agree with those generic bitops changes? Just in case it's 
not obvious from previous discussions: we'll push them upstream via a 
separate pull request, not via usual x86.git changes. They originated 
from x86.git but grew into a more generic improvement for all. They sit 
in x86.git for tester convenience but are of course not pure x86 changes 
anymore.

	Ingo

^ permalink raw reply

* Re: Add compat handler for PTRACE_GETSIGINFO
From: Christoph Hellwig @ 2008-04-21  9:57 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: linuxppc-dev
In-Reply-To: <je1w51ak2e.fsf@sykes.suse.de>

On Sat, Apr 19, 2008 at 06:25:13PM +0200, Andreas Schwab wrote:
> Current versions of gdb require a working implementation of
> PTRACE_GETSIGINFO for proper watchpoint support.  Since struct siginfo
> contains pointers it must be converted when passed to a 32-bit debugger.

Roland just posted a patch to do this in generic code.

^ permalink raw reply

* RE: [PATCH 1/3] MSI driver for Freescale 83xx/85xx/86xx cpu
From: Jin Zhengxiong @ 2008-04-21 10:01 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev, Gala Kumar
In-Reply-To: <1208758907.9955.4.camel@concordia>

Hi, Michael,

Thank you very much for you input, please see my inline answer.

B.R
Jason=20

> -----Original Message-----
> From: Michael Ellerman [mailto:michael@ellerman.id.au]=20
> Sent: Monday, April 21, 2008 2:22 PM
> To: Jin Zhengxiong
> Cc: linuxppc-dev@ozlabs.org; Gala Kumar
> Subject: Re: [PATCH 1/3] MSI driver for Freescale 83xx/85xx/86xx cpu
>=20
> > This MSI driver can be used on 83xx/85xx/86xx board.
> > In this driver, virtual interrupt host and chip were
> > setup. There are 256 MSI interrupts in this host, Every 32
> > MSI interrupts cascaded to one IPIC/MPIC interrupt.
> > The chip was treated as edge sensitive and some necessary
> > functions were setup for this chip.
> >=20
> > Before using the MSI interrupt, PCI/PCIE device need to
> > ask for a MSI interrupt in the 256 MSI interrupts. A 256bit
> > bitmap show which MSI interrupt was used, reserve bit in
> > the bitmap can be used to force the device use some designate
> > MSI interrupt in the 256 MSI interrupts. Sometimes this is useful
> > for testing the all the MSI interrupts. The msi-available-ranges
> > property in the dts file was used for this purpose.
> =20
>=20
> Hi Jason, some comments inline ...
>=20
> =20
> > diff --git a/arch/powerpc/sysdev/Makefile=20
> b/arch/powerpc/sysdev/Makefile
> > index 6d386d0..bfd3fe4 100644
> > --- a/arch/powerpc/sysdev/Makefile
> > +++ b/arch/powerpc/sysdev/Makefile
> > @@ -4,6 +4,7 @@ endif
> > =20
> >  mpic-msi-obj-$(CONFIG_PCI_MSI)	+=3D mpic_msi.o=20
> mpic_u3msi.o mpic_pasemi_msi.o
> >  obj-$(CONFIG_MPIC)		+=3D mpic.o $(mpic-msi-obj-y)
> > +obj-$(CONFIG_PCI_MSI)		+=3D fsl_msi.o
>=20
> Do we really always want to build this if we have MSI? Might it depend
> on something else as well? CONFIG_FSL_PCI maybe?
>=20
I'll try to change the depend.

> > diff --git a/arch/powerpc/sysdev/fsl_msi.c=20
> b/arch/powerpc/sysdev/fsl_msi.c
> > new file mode 100644
> > index 0000000..e8132cf
> > --- /dev/null
> > +++ b/arch/powerpc/sysdev/fsl_msi.c
> > @@ -0,0 +1,457 @@
> > +/*
> > + * Copyright (C) 2007-2008 Freescale Semiconductor, Inc.=20
> All rights reserved.
> > + *
> > + * Author: Tony Li <tony.li@freescale.com>
> > + *	   Jason Jin <Jason.jin@freescale.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; version 2 of the
> > + * License.
> > + *
> > + */
> > +
> > +#include <linux/irq.h>
> > +#include <linux/bootmem.h>
> > +#include <linux/bitmap.h>
> > +#include <linux/msi.h>
> > +#include <asm/prom.h>
> > +#include <asm/hw_irq.h>
> > +#include <linux/pci.h>
> > +#include <asm/ppc-pci.h>
> > +#include <linux/of_platform.h>
> > +
> > +#include <sysdev/fsl_soc.h>
> > +#include "fsl_msi.h"
>=20
> People consider it good style to have the linux includes before the
> asm includes.
>=20
OK

> > +#ifdef DEBUG
> > +#define pr_debug(fmt...) printk(fmt)
> > +#else
> > +#define pr_debug(fmt...)
> > +#endif
>=20
> Please don't do this, just use pr_debug(). In fact I don't=20
> see where you
> do use it :)
>=20
OK.

> > +/* A bit ugly, can we get this from the pci_dev somehow? */
> > +static struct fsl_msi *fsl_msi;
>=20
> I recognise that comment :)  The answer is "no" we can't (easily) get
> this from the pci_dev.
>=20
> > +static inline u32 fsl_msi_read(u32 __iomem *base,
> > +				unsigned int reg)
>=20
> This would fit in 80 chars wouldn't it?
>=20
OK

> > +{
> > +	return in_be32(base + (reg >> 2));
> > +}
> > +
> > +static inline void fsl_msi_write(u32 __iomem *base,
> > +				unsigned int reg, u32 value)
> > +{
> > +	out_be32(base + (reg >> 2), value);
> > +}
> > +
> > +#define	fsl_msi_irq_to_hw(virq)	 ((unsigned=20
> int)irq_map[virq].hwirq)
>=20
> I can't see where this is used, you probably don't need it.
>
I'll remove this.
=20
> > +/*
> > + * We do not need this actually. The MSIR register has=20
> been read once
> > + * in the cascade interrupt. So, this MSI interrupt has been acked
> > +*/
> > +static void fsl_msi_end_irq(unsigned int virq)
> > +{
> > +}
>=20
> I guess the generic code assumes you have an ack, bummer.
>=20
Yes, the generic code did not check it.

> > +static struct irq_chip fsl_msi_chip =3D {
> > +	.mask		=3D mask_msi_irq,
> > +	.unmask		=3D unmask_msi_irq,
> > +	.ack		=3D fsl_msi_end_irq,
> > +	.typename	=3D " FSL-MSI  ",
>=20
> I'd rather you didn't try and pad the name by hand, if we=20
> want /proc/interrupts
> to look pretty we should do that in show_interrupts().
>=20
Thanks, but I feel it's easier and cleaner to add the typename here to
make=20
the /proc/interrupts clear.=20

> > +static int fsl_msi_host_match(struct irq_host *h, struct=20
> device_node *node)
> > +{
> > +	struct fsl_msi *msi =3D h->host_data;
> > +
> > +	/* Exact match, unless node is NULL */
> > +	return msi->of_node =3D=3D NULL || msi->of_node =3D=3D node;
> > +}
>=20
> Do you really want the MSI to be the default irq host?
>=20
Thanks. I'll change the host match function.

> > +static int fsl_msi_host_map(struct irq_host *h, unsigned int virq,
> > +				irq_hw_number_t hw)
> > +{
> > +	struct fsl_msi *msi =3D h->host_data;
> > +	struct irq_chip *chip =3D msi->hc_irq;
> > +
> > +	set_irq_chip_data(virq, msi);
>=20
> You don't seem to use chip_data anywhere?
>=20
I'll check this.

> > +	get_irq_desc(virq)->status |=3D IRQ_TYPE_EDGE_FALLING;
> > +
> > +	set_irq_chip_and_handler(virq, chip,  handle_edge_irq);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct irq_host_ops fsl_msi_host_ops =3D {
> > +	.match =3D fsl_msi_host_match,
> > +	.map =3D fsl_msi_host_map,
> > +};
> > +
> > +irq_hw_number_t fsl_msi_alloc_hwirqs(struct fsl_msi *msi, int num)
> > +{
> > +	unsigned long flags;
> > +	int offset, order =3D get_count_order(num);
> > +
> > +	spin_lock_irqsave(&msi->bitmap_lock, flags);
> > +
> > +	offset =3D bitmap_find_free_region(msi->fsl_msi_bitmap,
> > +					NR_MSI_IRQS, order);
> > +
> > +	spin_unlock_irqrestore(&msi->bitmap_lock, flags);
> > +
> > +	pr_debug("%s: allocated 0x%x (2^%d) at offset 0x%x\n",
> > +		__func__, num, order, offset);
> > +
> > +	return offset;
> > +}
> > +
> > +void fsl_msi_free_hwirqs(struct fsl_msi *msi, int offset, int num)
> > +{
> > +	unsigned long flags;
> > +	int order =3D get_count_order(num);
> > +
> > +	pr_debug("%s: freeing 0x%x (2^%d) at offset 0x%x\n",
> > +		__func__, num, order, offset);
> > +
> > +	spin_lock_irqsave(&msi->bitmap_lock, flags);
> > +	bitmap_release_region(msi->fsl_msi_bitmap, offset, order);
> > +	spin_unlock_irqrestore(&msi->bitmap_lock, flags);
> > +}
> > +
> > +static int fsl_msi_reserve_dt_hwirqs(struct fsl_msi *msi)
> > +{
> > +	int i, len;
> > +	const u32 *p;
> > +
> > +	p =3D of_get_property(msi->of_node, "msi-available-ranges", &len);
> > +	if (!p) {
> > +		pr_debug("fsl_msi: no msi-available-ranges=20
> property found \
> > +				on %s\n", msi->of_node->full_name);
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (len & 0x8 !=3D 0) {
> > +		printk(KERN_WARNING "fsl_msi: Malformed=20
> msi-available-ranges "
> > +		       "property on %s\n", msi->of_node->full_name);
> > +		return -EINVAL;
> > +	}
>=20
> Do you really want a bitwise and with 0x8?
>=20
The range for the msi interrupt can be seperated to several part.=20
This can used to check the if the ranges is correct.=20

> > +
> > +	bitmap_allocate_region(msi->fsl_msi_bitmap, 0,
> > +			       get_count_order(msi->irq_count));
> > +
> > +	/* Format is: (<u32 start> <u32 count>)+ */
> > +	len /=3D sizeof(u32);
> > +	len /=3D 2;
> > +	for (i =3D 0; i < len; i++, p +=3D 2)
> > +		fsl_msi_free_hwirqs(msi, *p, *(p + 1));
> > +
> > +	return 0;
> > +}
>=20
> We could share a bunch of that code with mpic_msi.c, but=20
> that's not your
> job - I'll look at doing a patch once this goes in.
>=20
Thanks for the code you write for the MPIC msi.
 I'll use this until you share the code.

> > +static int fsl_msi_init_allocator(struct fsl_msi *msi)
> > +{
> > +	int rc, size;
> > +
> > +	size =3D BITS_TO_LONGS(NR_MSI_IRQS) * sizeof(long);
> > +
> > +	msi->fsl_msi_bitmap =3D kmalloc(size, GFP_KERNEL);
> > +
> > +	if (msi->fsl_msi_bitmap =3D=3D NULL) {
> > +		pr_debug("%s: ENOMEM allocating allocator bitmap!\n",
> > +				__func__);
> > +		return -ENOMEM;
> > +	}
> > +	memset(msi->fsl_msi_bitmap, 0, size);
>=20
> Use kzalloc() and you can lose the memset.
>=20
OK.

> > +	rc =3D fsl_msi_reserve_dt_hwirqs(msi);
>=20
> This routine is badly named (my fault), it really frees the available
> MSI ranges.
>=20
> > +	if (rc)
> > +		goto out_free;
> > +
> > +	return 0;
> > +out_free:
> > +	if (mem_init_done)
> > +		kfree(msi->fsl_msi_bitmap);
>=20
> You don't need to test mem_init_done, this is running at initcall time
> which is way later than mem_init.
>=20
Yes, I'll remove this. This code once used at just after the mpic was
intialized,and this
was left here.

> > +	msi->fsl_msi_bitmap =3D NULL;
> > +	return rc;
> > +
> > +}
> > +
> > +static int fsl_msi_check_device(struct pci_dev *pdev, int=20
> nvec, int type)
> > +{
> > +	if (type =3D=3D PCI_CAP_ID_MSIX)
> > +		pr_debug("fslmsi: MSI-X untested, trying anyway.\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static void fsl_teardown_msi_irqs(struct pci_dev *pdev)
> > +{
> > +	struct msi_desc *entry;
> > +	struct fsl_msi *msi =3D fsl_msi;
> > +
> > +	list_for_each_entry(entry, &pdev->msi_list, list) {
> > +		if (entry->irq =3D=3D NO_IRQ)
> > +			continue;
> > +		set_irq_msi(entry->irq, NULL);
> > +		fsl_msi_free_hwirqs(msi, virq_to_hw(entry->irq), 1);
> > +		irq_dispose_mapping(entry->irq);
> > +	}
> > +
> > +	return;
> > +}
> > +
> > +static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq,
> > +				  struct msi_msg *msg)
> > +{
> > +	unsigned int srs;
> > +	unsigned int ibs;
> > +	struct fsl_msi *msi =3D fsl_msi;
> > +
> > +	srs =3D hwirq / INT_PER_MSIR;
> > +	ibs =3D hwirq % INT_PER_MSIR;
> > +
> > +	msg->address_lo =3D msi->msi_addr_lo;
> > +	msg->address_hi =3D msi->msi_addr_hi;
> > +	msg->data =3D (srs << 5) | (ibs & 0x1F);
>=20
> Is the 5 and 0x1F independent of the INT_PER_MSIR value? Given the
> current values isn't this a no-op, or am I missing something?
>=20
Do you mean there're another way to get the msg->data from the hwirq? =20

> > +	pr_debug("%s: allocated srs: %d, ibs: %d\n",
> > +		__func__, srs, ibs);
> > +}
> > +
> > +static int fsl_setup_msi_irqs(struct pci_dev *pdev, int=20
> nvec, int type)
> > +{
> > +	irq_hw_number_t hwirq;
> > +	int rc;
> > +	unsigned int virq;
> > +	struct msi_desc *entry;
> > +	struct msi_msg msg;
> > +	struct fsl_msi *msi =3D fsl_msi;
>=20
> A couple of places you put this into a local called "msi"=20
> which is not the
> greatest name in the world IMHO :)
>=20
Thank you, I'll try to use another name, Do you have any suggestion?

> > +	list_for_each_entry(entry, &pdev->msi_list, list) {
> > +		hwirq =3D fsl_msi_alloc_hwirqs(msi, 1);
> > +		if (hwirq < 0) {
> > +			rc =3D hwirq;
> > +			pr_debug("%s: fail allocating msi interrupt\n",
> > +					__func__);
> > +			goto out_free;
> > +		}
> > +
> > +		virq =3D irq_create_mapping(msi->irqhost, hwirq);
> > +
> > +		if (virq =3D=3D NO_IRQ) {
> > +			pr_debug("%s: fail mapping hwirq 0x%lx\n",
> > +					__func__, hwirq);
> > +			fsl_msi_free_hwirqs(msi, hwirq, 1);
> > +			rc =3D -ENOSPC;
> > +			goto out_free;
> > +		}
> > +		set_irq_msi(virq, entry);
> > +
> > +		fsl_compose_msi_msg(pdev, hwirq, &msg);
> > +		write_msi_msg(virq, &msg);
> > +	}
> > +	return 0;
> > +
> > +out_free:
> > +	fsl_teardown_msi_irqs(pdev);
>=20
> You don't need to call teardown, the generic code does that for you.
>=20
OK.

> > +	return rc;
> > +}
> > +
> > +void fsl_msi_cascade(unsigned int irq, struct irq_desc *desc)
> > +{
> > +	unsigned int cascade_irq;
> > +	struct fsl_msi *msi =3D fsl_msi;
> > +	int msir_index =3D -1;
> > +	int i;
> > +	u32 msir_value =3D 0;
> > +	u32 intr_index;
> > +	u32 have_shift =3D 0;
> > +
> > +	spin_lock(&desc->lock);
> > +	if ((msi->feature &  FSL_PIC_IP_MASK) =3D=3D FSL_PIC_IP_IPIC) {
> > +		if (desc->chip->mask_ack)
> > +			desc->chip->mask_ack(irq);
> > +		else {
> > +			desc->chip->mask(irq);
> > +			desc->chip->ack(irq);
> > +		}
> > +	}
> > +
> > +	if (unlikely(desc->status & IRQ_INPROGRESS))
> > +		goto unlock;
> > +
> > +	for (i =3D 0; i < NR_MSIR; i++)
> > +		if (irq =3D=3D msi->msir[i]) {
> > +			msir_index =3D i;
> > +			break;
> > +		}
>=20
> This is a bit ugly :)  Because you get the *msi from fsl_msi=20
> (above), you could
> store the msir_index in the handler_data (with set_irq_data),=20
> and save doing
> this loop.
>=20
I'll find if the handler data *msi was used somewhere. if not I change
to
save the msir_index to the handler data.=20

> > +	if (i >=3D NR_MSIR)
> > +		cascade_irq =3D NO_IRQ;
> > +
> > +	desc->status |=3D IRQ_INPROGRESS;
> > +	switch (fsl_msi->feature & FSL_PIC_IP_MASK) {
> > +	case FSL_PIC_IP_MPIC:
> > +		msir_value =3D fsl_msi_read(msi->msi_regs,=20
> msir_index * 0x10);
> > +		break;
> > +	case FSL_PIC_IP_IPIC:
> > +		msir_value =3D fsl_msi_read(msi->msi_regs,=20
> msir_index * 0x4);
> > +		break;
> > +	}
> > +
> > +	while (msir_value) {
> > +		intr_index =3D ffs(msir_value) - 1;
> > +
> > +		cascade_irq =3D irq_linear_revmap(msi->irqhost,
> > +			(msir_index * INT_PER_MSIR + intr_index=20
> + have_shift));
> > +
> > +		if (cascade_irq !=3D NO_IRQ)
> > +			generic_handle_irq(cascade_irq);
> > +		have_shift +=3D (intr_index + 1);
> > +		msir_value =3D (msir_value >> (intr_index + 1));
> > +	}
>=20
> It took me a while to grok all the shifting and so on here, I=20
> don't know if
> there's a cleaner way to do it.
>=20
To improve the efficency here, I tried to use the bit operation which
can=20
carry out by the cpu with one instruction. Maybe I also need to improve=20
readability :).

> > +	desc->status &=3D ~IRQ_INPROGRESS;
> > +
> > +	switch (fsl_msi->feature & FSL_PIC_IP_MASK) {
> > +	case FSL_PIC_IP_MPIC:
> > +		desc->chip->eoi(irq);
> > +		break;
> > +	case FSL_PIC_IP_IPIC:
> > +		if (!(desc->status & IRQ_DISABLED) &&=20
> desc->chip->unmask)
> > +		desc->chip->unmask(irq);
>=20
> Missing indent.
>=20
OK.

> > +		break;
> > +	}
> > +unlock:
> > +	spin_unlock(&desc->lock);
> > +}
> > +
> > +static int __devinit fsl_of_msi_probe(struct of_device *dev,
> > +				const struct of_device_id *match)
> > +{
> > +	struct fsl_msi *msi;
> > +	struct resource res;
> > +	int err, i, count;
> > +	int rc;
> > +	int virt_msir;
> > +	const u32 *p;
> > +	struct fsl_msi_data *tmp_data;
> > +
> > +	printk(KERN_INFO "Setting up fsl msi support\n");
>=20
> KERN_DEBUG would do here IMHO, users don't usually need to see it.
>=20
> > +	msi =3D kzalloc(sizeof(struct fsl_msi), GFP_KERNEL);
> > +	if (!msi) {
> > +		dev_err(&dev->dev, "No memory for MSI structure\n");
> > +		err =3D -ENOMEM;
> > +		goto error_out;
> > +	}
> > +
> > +	msi->of_node =3D dev->node;
> > +
> > +	msi->irqhost =3D irq_alloc_host(of_node_get(dev->node),
> > +				IRQ_HOST_MAP_LINEAR,
> > +				NR_MSI_IRQS, &fsl_msi_host_ops, 0);
> > +	if (msi->irqhost =3D=3D NULL) {
> > +		dev_err(&dev->dev, "No memory for MSI irqhost\n");
> > +		err =3D -ENOMEM;
>=20
> You need an of_node_put(dev->node) in the error case here.
>=20
Thanks, I'll add it.

> > +		goto error_out;
> > +	}
> > +
> > +	/*Get the MSI reg base */
>=20
> Missing space after /*
>=20
> > +	err =3D of_address_to_resource(dev->node, 0, &res);
> > +	if (err) {
> > +		dev_err(&dev->dev, "Can't get %s property 'reg'\n",
> > +				dev->node->full_name);
>=20
> That's a little misleading, aren't there's lots of reasons
> of_address_to_resource() might fail?
>=20
> > +		goto error_out;
> > +	}
> > +
> > +	msi->msi_regs =3D ioremap(res.start, res.end - res.start + 1);
>=20
> ioremap() can fail.
>=20
OK,  I'll add the error process code here.

> > +	tmp_data =3D (struct fsl_msi_data *)match->data;
> > +
> > +	msi->feature =3D tmp_data->fsl_pic_ip;
> > +
> > +	msi->irqhost->host_data =3D msi;
> > +	msi->hc_irq =3D &fsl_msi_chip;
>=20
> Any reason this needs to be a variable, isn't it always &fsl_msi_chip?
>=20
> > +	msi->msi_addr_hi =3D 0x0;
> > +	msi->msi_addr_lo =3D res.start + tmp_data->msiir_offset;
> > +
> > +	msi->irq_count =3D NR_MSI_IRQS;
>=20
> Ditto.
>=20
> > +	p =3D of_get_property(dev->node, "interrupts", &count);
> > +	if (!p) {
> > +		dev_err(&dev->dev, "no msi-interrupts property=20
> found on %s\n",
> > +				dev->node->full_name);
> > +		err =3D -ENODEV;
> > +		goto error_out;
> > +	}
> > +	if (count % 8 !=3D 0) {
> > +		dev_err(&dev->dev, "Malformed msi-interrupts=20
> property on %s\n",
> > +				dev->node->full_name);
>=20
> Messages don't match the code, "interrupts" vs "msi-interrupts".
>=20
Thanks, I'll change the description.

> > +		err =3D -EINVAL;
> > +		goto error_out;
> > +	}
> > +
> > +	count /=3D sizeof(u32);
> > +	for (i =3D 0; i < count / 2; i++) {
> > +		if (i > NR_MSIR)
> > +			break;
> > +		virt_msir =3D irq_of_parse_and_map(dev->node, i);
> > +		if (virt_msir !=3D NO_IRQ) {
> > +			set_irq_data(virt_msir, msi);
> > +			set_irq_chained_handler(virt_msir,=20
> fsl_msi_cascade);
> > +			msi->msir[i] =3D virt_msir;
> > +		} else
> > +			msi->msir[i] =3D NO_IRQ;
> > +	}
> > +
> > +	rc =3D fsl_msi_init_allocator(msi);
> > +	if (rc) {
> > +		dev_err(&dev->dev, "Error allocating MSI bitmap\n");
>=20
> If you hit this then the error case needs to get rid of all=20
> the irq mappings
> you created just above (which it doesn't). It would be=20
> simpler if you just move
> this call above the for loop, then you don't need to worry about it.
>
Thanks, good idea.
=20
> > +		goto error_out;
> > +	}
> > +
> > +	fsl_msi =3D msi;
> > +
> > +	WARN_ON(ppc_md.setup_msi_irqs);
> > +	ppc_md.setup_msi_irqs =3D fsl_setup_msi_irqs;
> > +	ppc_md.teardown_msi_irqs =3D fsl_teardown_msi_irqs;
> > +	ppc_md.msi_check_device =3D fsl_msi_check_device;
> > +	return 0;
> > +error_out:
> > +	if (msi)
> > +		kfree(msi);
>=20
> kfree(NULL) is fine.
>=20
> > +	return err;
> > +}
> > +
> > +static const struct fsl_msi_data mpic_msi_feature =3D=20
> {FSL_PIC_IP_MPIC, 0x140};
> > +static const struct fsl_msi_data ipic_msi_feature =3D=20
> {FSL_PIC_IP_IPIC, 0x38};
> > +
> > +static const struct of_device_id fsl_of_msi_ids[] =3D {
> > +	{
> > +		.compatible =3D "fsl,MPIC-MSI",
> > +		.data =3D (void *)&mpic_msi_feature,
> > +	},
> > +	{
> > +		.compatible =3D "fsl,IPIC-MSI",
> > +		.data =3D (void *)&ipic_msi_feature,
> > +	},
> > +	{}
> > +};
> > +
> > +static struct of_platform_driver fsl_of_msi_driver =3D {
> > +	.name =3D "fsl-of-msi",
> > +	.match_table =3D fsl_of_msi_ids,
> > +	.probe =3D fsl_of_msi_probe,
> > +};
> > +
> > +static __init int fsl_of_msi_init(void)
> > +{
> > +	return of_register_platform_driver(&fsl_of_msi_driver);
> > +}
> > +
> > +subsys_initcall(fsl_of_msi_init);
> > diff --git a/arch/powerpc/sysdev/fsl_msi.h=20
> b/arch/powerpc/sysdev/fsl_msi.h
> > new file mode 100644
> > index 0000000..7eef9ec
> > --- /dev/null
> > +++ b/arch/powerpc/sysdev/fsl_msi.h
> > @@ -0,0 +1,40 @@
> > +#ifndef _ASM_POWERPC_MSI_H
> > +#define _ASM_POWERPC_MSI_H
>=20
> Include guards don't match the filename, should be=20
> _POWERPC_SYSDEV_FSL_MSI_H
>=20
OK.

> > +#define NR_MSIR	8
> > +#define INT_PER_MSIR	32
> > +#define NR_MSI_IRQS	(NR_MSIR * INT_PER_MSIR)
> > +
> > +#define FSL_PIC_IP_MASK	0x0000000F
> > +#define FSL_PIC_IP_MPIC	0x00000001
> > +#define FSL_PIC_IP_IPIC	0x00000002
> > +
> > +struct fsl_msi {
> > +	/* Device node of the MSI interrupt*/
> > +	struct device_node *of_node;
> > +
> > +	struct irq_host *irqhost;
> > +	struct irq_chip *hc_irq;
> > +
> > +	unsigned long cascade_irq;
> > +	unsigned int msir[NR_MSIR];
> > +
> > +	u32 msi_addr_lo;
> > +	u32 msi_addr_hi;
> > +	void __iomem *msi_regs;
> > +	u32 irq_count;
> > +	u32 feature;
> > +
> > +	spinlock_t fsl_msi_lock;
>=20
> Unused?
>=20
> > +	unsigned long *fsl_msi_bitmap;
>=20
> Do we need fsl_msi in the name?
>=20
> > +	spinlock_t bitmap_lock;
> > +	const char *name;
>=20
> Unused?
>=20
> > +};
> > +
> > +struct fsl_msi_data {
> > +	u32 fsl_pic_ip;
> > +	u32 msiir_offset;
> > +};
> > +
> > +#endif /* _ASM_POWERPC_MSI_H */
> > +
> > diff --git a/arch/powerpc/sysdev/fsl_pci.c=20
> b/arch/powerpc/sysdev/fsl_pci.c
> > index bf13c21..fede767 100644
> > --- a/arch/powerpc/sysdev/fsl_pci.c
> > +++ b/arch/powerpc/sysdev/fsl_pci.c
> > @@ -106,6 +106,16 @@ void __init setup_pci_cmd(struct=20
> pci_controller *hose)
> >  	}
> >  }
> > =20
> > +#ifdef CONFIG_PCI_MSI
> > +void __init setup_pci_pcsrbar(struct pci_controller *hose)
> > +{
> > +	phys_addr_t immr_base;
> > +
> > +	immr_base =3D get_immrbase();
> > +	early_write_config_dword(hose, 0, 0,=20
> PCI_BASE_ADDRESS_0, immr_base);
> > +}
> > +#endif
>=20
> Up to you, but I prefer an empty static inline here which=20
> saves an #ifdef
> at the call site.
>=20
> >  static int fsl_pcie_bus_fixup;
> > =20
> >  static void __init quirk_fsl_pcie_header(struct pci_dev *dev)
> > @@ -211,6 +221,10 @@ int __init fsl_add_bridge(struct=20
> device_node *dev, int is_primary)
> >  	/* Setup PEX window registers */
> >  	setup_pci_atmu(hose, &rsrc);
> > =20
> > +	/*Setup PEXCSRBAR */
> > +#ifdef CONFIG_PCI_MSI
> > +	setup_pci_pcsrbar(hose);
> > +#endif
> >  	return 0;
> >  }
>=20
>=20
> cheers
>=20

^ 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