linux-kernel - Re: [PATCH v6] 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]

Date:	Thu, 9 Feb 2012 11:48:07 -0800
From:	Shawn Guo <shawn.guo@...aro.org>
To:	Rob Herring <robherring2@...il.com>
Cc:	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Grant Likely <grant.likely@...retlab.ca>,
	Thomas Gleixner <tglx@...utronix.de>, b-cousson@...com,
	Rob Herring <rob.herring@...xeda.com>
Subject: Re: [PATCH v6] irq: add irq_domain support to generic-chip

On Wed, Feb 08, 2012 at 04:55:22PM -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.
> 
> Thanks to Shawn Guo for fixes and testing.
> 
> Signed-off-by: Rob Herring <rob.herring@...xeda.com>
> Cc: Grant Likely <grant.likely@...retlab.ca>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> ---
> Here's the latest version. This has fixes from Shawn Guo, so should be
> working. This version is also available here:
> 
Rob, yes it's working.  But I think it should be working in a better
way after I play it a little bit more.  See comment below.
...
> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> +				int num_ct, unsigned int irq_base,
> +				void __iomem *reg_base,
> +				irq_flow_handler_t handler, u32 hwirq_cnt,
> +				enum irq_gc_flags flags, unsigned int clr,
> +				unsigned int set,
> +				void (*gc_init_cb)(struct irq_chip_generic *),
> +				void *private)
> +{
> +	int ret, i;
> +	struct irq_chip_generic **gc;
> +	struct irq_domain *d;
> +	int num_gc = ALIGN(hwirq_cnt, 32) / 32;
> +
> +	gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
> +	if (!gc)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < num_gc; i++) {
> +		gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
> +			       reg_base, handler);
> +		if (!gc[i]) {
> +			ret = -ENOMEM;
> +			goto err;
> +		}
> +		gc[i]->hwirq_base = i * 32;
> +		gc[i]->private = private;
> +
> +		gc_init_cb(gc[i]);
> +
> +		irq_setup_generic_chip(gc[i], 0, flags, clr, set);
> +	}
> +
> +	if (node)
> +		d = irq_domain_add_linear(node, hwirq_cnt,
> +					  &irq_gc_irq_domain_ops, gc);
> +	else
> +		d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
> +					  &irq_gc_irq_domain_ops, gc);
> +
I do not think it's good to make this decision based on whether it's
dt or non-dt case.  That said, it's fairly valid and actually
encouraged that non-dt users use linear domain too, and also the
irqdomain core has no limit on dt users to use legacy domain.  Instead,
it's much more reasonable to make this decision based on if irq_base,
something like you did in your v3 patch.

 
-       if (node)
+       if ((int) irq_base < 0)

Otherwise,

Acked-by: Shawn Guo <shawn.guo@...aro.org>

Tested on imx51 with tzic and gpio interrupt controller.

Regards,
Shawn

> +	for (i = 0; i < num_gc; i++)
> +		gc[i]->domain = d;
> +
> +	return 0;
> +
> + err:
> +	for ( ; i >= 0; i--)
> +		kfree(gc[i]);
> +	kfree(gc);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain);
> +#endif
> +
--
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/