Re: [PATCH] powerpc/pseries: Make request_ras_irqs() available to other pseries code

[Michael Ellerman <michael@ellerman.id.au>]

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

On Mon, 2010-05-17 at 22:33 +1000, Mark Nelson wrote:
> At the moment only the RAS code uses event-sources interrupts (for EPOW
> events and internal errors) so request_ras_irqs() (which actually requests
> the event-sources interrupts) is found in ras.c and is static.

Hi Mark,

Just a few niggles,

...

> +
> +void request_event_sources_irqs(struct device_node *np,
> +				irq_handler_t handler,
> +				const char *name)
> +{
> +	int i, index, count = 0;
> +	struct of_irq oirq;
> +	const u32 *opicprop;
> +	unsigned int opicplen;
> +	unsigned int virqs[16];
> +
> +	/* Check for obsolete "open-pic-interrupt" property. If present, then
> +	 * map those interrupts using the default interrupt host and default
> +	 * trigger
> +	 */
> +	opicprop = of_get_property(np, "open-pic-interrupt", &opicplen);
> +	if (opicprop) {
> +		opicplen /= sizeof(u32);
> +		for (i = 0; i < opicplen; i++) {
> +			if (count > 15)
> +				break;
> +			virqs[count] = irq_create_mapping(NULL, *(opicprop++));
> +			if (virqs[count] == NO_IRQ)
> +				printk(KERN_ERR "Unable to allocate interrupt "
> +				       "number for %s\n", np->full_name);
> +			else
> +				count++;
> +
> +		}
> +	}
> +	/* Else use normal interrupt tree parsing */
> +	else {
> +		/* First try to do a proper OF tree parsing */
> +		for (index = 0; of_irq_map_one(np, index, &oirq) == 0;
> +		     index++) {
> +			if (count > 15)
> +				break;
> +			virqs[count] = irq_create_of_mapping(oirq.controller,
> +							    oirq.specifier,
> +							    oirq.size);
> +			if (virqs[count] == NO_IRQ)
> +				printk(KERN_ERR "Unable to allocate interrupt "
> +				       "number for %s\n", np->full_name);
> +			else
> +				count++;
> +		}
> +	}
> +
> +	/* Now request them */
> +	for (i = 0; i < count; i++) {
> +		if (request_irq(virqs[i], handler, 0, name, NULL)) {
> +			printk(KERN_ERR "Unable to request interrupt %d for "
> +			       "%s\n", virqs[i], np->full_name);
> +			return;
> +		}
> +	}

Existing code I know, but the error handling in here is a little lax,
what's not going to work if we miss some or all of the interrupts?

> Index: upstream/arch/powerpc/platforms/pseries/event_sources.h
> ===================================================================
> --- /dev/null
> +++ upstream/arch/powerpc/platforms/pseries/event_sources.h
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright (C) 2001 Dave Engebretsen IBM Corporation
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
> + */
> +
> +#ifndef _POWERPC_EVENT_SOURCES_H
> +#define _POWERPC_EVENT_SOURCES_H
> +
> +#include <linux/interrupt.h>
> +
> +struct device_node;
> +
> +extern void request_event_sources_irqs(struct device_node *np,
> +				       irq_handler_t handler, const char *name);

This could just go in platforms/pseries/pseries.h

> Index: upstream/arch/powerpc/platforms/pseries/ras.c
> ===================================================================
> --- upstream.orig/arch/powerpc/platforms/pseries/ras.c
> +++ upstream/arch/powerpc/platforms/pseries/ras.c
> @@ -50,6 +50,7 @@
>  #include <asm/firmware.h>
>  
>  #include "pseries.h"
> +#include "event_sources.h"

:)


cheers

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