Re: [PATCH 01/10] irq: move some interrupt arch_* functions into struct irq_chip.

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

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

On Sun, 2010-03-21 at 18:36 -0700, Yinghai Lu wrote:
> From: Ian Campbell <ian.campbell@citrix.com>
...
> To replace the x86 arch_init_chip_data functionality
> irq_to_desc_alloc_node now takes a pointer to a function to allocate
> the chip data. This is necessary to ensure the allocation happens
> under the correct locking at the core level. On PowerPC and SH
> architectures (the other users of irq_to_desc_alloc_node) pass in NULL
> which retains existing chip_data behaviour.
...
> 
> -v4: yinghai add irq_to_desc_alloc_node_x...
>      so could leave default path not changed...

Apologies for not noticing this sooner, but ..

> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -1088,7 +1088,7 @@ int arch_early_irq_init(void)
>  	return 0;
>  }
>  
> -int arch_init_chip_data(struct irq_desc *desc, int node)
> +int arch_init_irq_desc(struct irq_desc *desc, int node, init_chip_data_fn fn)
>  {
>  	desc->status |= IRQ_NOREQUEST;
>  	return 0;

This is a bit feral, that is the init_chip_data_fn.

It seems like it only exists to support the following on x86:

> +int arch_init_irq_desc(struct irq_desc *desc, int node,
> +			 init_chip_data_fn init_chip_data)
> +{
> +	if (!init_chip_data)
> +		return x86_init_chip_data(desc, node);
> +
> +	return init_chip_data(desc, node);
> +}

Which is really just a hack to avoid an if (xen) check isn't it?

It looks to me like this should just be done via a current machine
vector or platform routine, in the same way as powerpc and (I think)
ia64, ie:

> +int arch_init_irq_desc(struct irq_desc *desc, int node)
> +{
> +	return current_machine->init_chip_data(desc, node);
> +}

cheers


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