linux-kernel - Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120201000232.GK22611@ponder.secretlab.ca>
Date:	Tue, 31 Jan 2012 17:02:32 -0700
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Rob Herring <robherring2@...il.com>
Cc:	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	shawn.guo@...aro.org, b-cousson@...com,
	Rob Herring <rob.herring@...xeda.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip

On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote:
> From: Rob Herring <rob.herring@...xeda.com>
> 
> Add irq domain support to irq generic-chip. This enables users of
> generic-chip to support dynamic irq assignment needed for DT interrupt
> binding.
> 
> Signed-off-by: Rob Herring <rob.herring@...xeda.com>
> Cc: Grant Likely <grant.likely@...retlab.ca>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> ---
> @@ -39,7 +40,7 @@ void irq_gc_noop(struct irq_data *d)
>  void irq_gc_mask_disable_reg(struct irq_data *d)
>  {
>  	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> -	u32 mask = 1 << (d->irq - gc->irq_base);
> +	u32 mask = 1 << d->hwirq;

As discussed on IRC, there needs to be a 1:N relationship between
an irq_domain and generic chips, but doing that means that hwirq no
longer directly maps to the bit in the register.

This could be solved however if a mod is applied to the hwirq number
and if we're careful to line up hwirqs to those mod boundaries:

u32 mask = 1 << (d->hwirq % gc->bank_size);

>  	}
>  	gc->irq_cnt = i - gc->irq_base;
>  }
>  EXPORT_SYMBOL_GPL(irq_setup_generic_chip);
>  
> +#ifdef CONFIG_IRQ_DOMAIN
> +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np)
> +{
> +	struct irq_chip_generic *gc;
> +
> +	if (d->of_node != NULL && d->of_node == np) {
> +		list_for_each_entry(gc, &gc_list, list) {
> +			if ((gc == d->host_data) && (d == gc->domain))
> +				return 1;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int irq_gc_irq_domain_map(struct irq_domain *d, unsigned int irq,
> +				 irq_hw_number_t hw)
> +{
> +	struct irq_chip_generic *gc = d->host_data;
> +	struct irq_chip_type *ct = gc->chip_types;
> +
> +	if (gc->flags & IRQ_GC_INIT_NESTED_LOCK)
> +		irq_set_lockdep_class(irq, &irq_nested_lock_class);
> +
> +	irq_set_chip_and_handler(irq, &ct->chip, ct->handler);
> +	irq_set_chip_data(irq, gc);
> +	irq_modify_status(irq, gc->irq_clr, gc->irq_set);
> +
> +	return 0;
> +}
> +
> +static struct irq_domain_ops irq_gc_irq_domain_ops = {
> +	.match = irq_gc_irq_domain_match,
> +	.map = irq_gc_irq_domain_map,
> +	.xlate = irq_domain_xlate_onetwocell,
> +};
> +
> +/*
> + * irq_setup_generic_chip_domain - Setup a range of interrupts with a generic chip and domain
> + * @gc:		Generic irq chip holding all data
> + * @node:	Device tree node pointer for domain
> + * @msk:	Bitmask holding the irqs to initialize relative to gc->irq_base
> + * @flags:	Flags for initialization
> + * @clr:	IRQ_* bits to clear
> + * @set:	IRQ_* bits to set
> + *
> + * Set up max. 32 interrupts starting from gc->irq_base using an irq domain.
> + * Note, this initializes all interrupts to the primary irq_chip_type and its
> + * associated handler.
> + */
> +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc,
> +			    struct device_node *node, u32 msk,
> +			    enum irq_gc_flags flags, unsigned int clr,
> +			    unsigned int set)
> +{
> +	struct irq_chip_type *ct = gc->chip_types;
> +
> +	if (!node) {
> +		irq_setup_generic_chip(gc, msk, flags, clr, set);
> +		return;
> +	}

Calling irq_setup_generic_chip() should always be performed,
regardless of whether or not a node is passed in.  However, the msk
field should be passed as 0 so that none of the irqs get configured
before they are allocated.  I'd like to see all the domain code paths
look the same, regardless of whether a node pointer is provided.

The only quirk here is that if a node isn't provided, then that
probably means the range of irqs needs to be fixed; which means using
the legacy map in the current implementation.  We should sit down and
talk about this next week at Connect.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/