Re: [PATCH 1/2] genirq: Generic chip: add irq_unmap_generic_chip

[Sebastian Frias <sf84@laposte.net>]

Hi,

I don't know if somebody has had the time to look at these patches, but if you
have comments (even if related to removing the "fixes" tag if you don't
consider this a fix) please let me know.

Best regards,

Sebastian

On 08/01/2016 04:27 PM, Sebastian Frias wrote:
> Without this patch irq_domain_disassociate() cannot properly release the
> interrupt.
> Indeed, irq_map_generic_chip() checks a bit on 'gc->installed' but said bit
> is never cleared, only set.
> 
> Commit 088f40b7b027 ("genirq: Generic chip: Add linear irq domain support")
> added irq_map_generic_chip() function and also stated "This lacks a removal
> function for now".
> 
> This commit provides with an implementation of an unmap function that can
> be called by irq_domain_disassociate().
> 
> Fixes: 088f40b7b027 ("genirq: Generic chip: Add linear irq domain support")
> 
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
> ---
> 
> This is required by loadable modules requesting IRQs.
> In our case rmmod will perform free_irq() + irq_dispose_mapping().
> Without the unmap call the module cannot request the IRQ after "rmmod"
> because it is marked as "installed" by the first successful "insmod".
> 
> NOTE: While the proposed unmap() function attempts to undo as much things
> as done by the map() function, I did not find a way to undo the following:
> 
> a) irq_gc_init_mask_cache(gc, dgc->gc_flags)
> b) irq_set_lockdep_class(virq, &irq_nested_lock_class)
> c) irq_modify_status(virq, dgc->irq_flags_to_clear, dgc->irq_flags_to_set)
> 
> Feel free to comment on that matter.
> 
> ---
>  kernel/irq/generic-chip.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index abd286a..7b464cd 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -411,8 +411,34 @@ int irq_map_generic_chip(struct irq_domain *d, unsigned int virq,
>  }
>  EXPORT_SYMBOL_GPL(irq_map_generic_chip);
>  
> +void irq_unmap_generic_chip(struct irq_domain *d, unsigned int virq)
> +{
> +	struct irq_data *data = irq_domain_get_irq_data(d, virq);
> +	struct irq_domain_chip_generic *dgc = d->gc;
> +	struct irq_chip_generic *gc;
> +	unsigned int hw_irq = data->hwirq;
> +	int chip_idx, irq_idx;
> +
> +	if (!d->gc)
> +		return;
> +
> +	chip_idx = hw_irq / dgc->irqs_per_chip;
> +	if (chip_idx >= dgc->num_chips)
> +		return;
> +	gc = dgc->gc[chip_idx];
> +
> +	irq_idx = hw_irq % dgc->irqs_per_chip;
> +
> +	clear_bit(irq_idx, &gc->installed);
> +	irq_domain_set_info(d, virq, hw_irq,
> +			    &no_irq_chip, NULL, NULL, NULL, NULL);
> +
> +}
> +EXPORT_SYMBOL_GPL(irq_unmap_generic_chip);
> +
>  struct irq_domain_ops irq_generic_chip_ops = {
>  	.map	= irq_map_generic_chip,
> +	.unmap  = irq_unmap_generic_chip,
>  	.xlate	= irq_domain_xlate_onetwocell,
>  };
>  EXPORT_SYMBOL_GPL(irq_generic_chip_ops);
>