From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: linux-kernel@vger.kernel.org, Milton Miller <miltonm@bga.com>,
Paul Mundt <lethal@linux-sh.org>,
Thomas Gleixner <tglx@linutronix.de>,
Rob Herring <rob.herring@calxeda.com>
Subject: Re: [PATCH 11/12] irqdomain: reorganize revmap data.
Date: Sat, 16 Jun 2012 16:06:33 +1000 [thread overview]
Message-ID: <1339826793.9220.229.camel@pasglop> (raw)
In-Reply-To: <1339822897-15840-12-git-send-email-grant.likely@secretlab.ca>
On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> In struct irq_domain, reorganize the revmap data in preparation for
> unifying the linear and radix mappings.
I fail to understand the patch itself... ie:
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
> drivers/pinctrl/pinctrl-nomadik.c | 4 +--
> include/linux/irqdomain.h | 17 ++++-------
> kernel/irq/irqdomain.c | 58 ++++++++++++++-----------------------
> 3 files changed, 29 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> index b26395d..22fee45 100644
> --- a/drivers/pinctrl/pinctrl-nomadik.c
> +++ b/drivers/pinctrl/pinctrl-nomadik.c
> @@ -826,16 +826,14 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
> {
> struct nmk_gpio_chip *nmk_chip;
> struct irq_chip *host_chip = irq_get_chip(irq);
> - unsigned int first_irq;
>
> chained_irq_enter(host_chip, desc);
>
> nmk_chip = irq_get_handler_data(irq);
> - first_irq = nmk_chip->domain->revmap_data.legacy.first_irq;
> while (status) {
> int bit = __ffs(status);
>
> - generic_handle_irq(first_irq + bit);
> + generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
> status &= ~BIT(bit);
> }
That doesn't seem related to either the subject or the description
(though it looks like something valid to do per-se). Maybe belongs to a
separate patch ?
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index b46a551..ee90765 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -71,7 +71,6 @@ struct irq_domain_ops {
> * @link: Element in global irq_domain list.
> * @revmap_type: Method used for reverse mapping hwirq numbers to linux irq. This
> * will be one of the IRQ_DOMAIN_MAP_* values.
> - * @revmap_data: Revmap method specific data.
> * @ops: pointer to irq_domain methods
> * @host_data: private data pointer for use by owner. Not touched by irq_domain
> * core code.
> @@ -88,22 +87,18 @@ struct irq_domain {
>
> /* type of reverse mapping_technique */
> unsigned int revmap_type;
> - union {
> - struct {
> - unsigned int size;
> - unsigned int *revmap;
> - } linear;
> - struct {
> - unsigned int max_irq;
> - } nomap;
> - struct radix_tree_root tree;
> - } revmap_data;
> const struct irq_domain_ops *ops;
> void *host_data;
> irq_hw_number_t inval_irq;
>
> /* Optional device node pointer */
> struct device_node *of_node;
> +
> + /* Reverse mapping data */
> + unsigned int nomap_max_irq;
> + struct radix_tree_root radix_tree;
> + unsigned int linear_size;
> + unsigned int linear_revmap[];
> };
Hrm, I'm thinking maybe you should just merge those changes with patch
12/12 ... the moving around of the fields in a first patch doesn't seem
to clarify this from my perspective at least.
Cheers,
Ben.
> #ifdef CONFIG_IRQ_DOMAIN
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 0e02a1d..433971a 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -38,14 +38,14 @@ static struct irq_domain *irq_default_domain;
> * to IRQ domain, or NULL on failure.
> */
> static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
> - unsigned int revmap_type,
> + unsigned int revmap_type, int size,
> const struct irq_domain_ops *ops,
> void *host_data)
> {
> struct irq_domain *domain;
>
> - domain = kzalloc_node(sizeof(*domain), GFP_KERNEL,
> - of_node_to_nid(of_node));
> + domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
> + GFP_KERNEL, of_node_to_nid(of_node));
> if (WARN_ON(!domain))
> return NULL;
>
> @@ -54,6 +54,7 @@ static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
> domain->ops = ops;
> domain->host_data = host_data;
> domain->of_node = of_node_get(of_node);
> + domain->linear_size = size;
>
> return domain;
> }
> @@ -92,13 +93,7 @@ void irq_domain_remove(struct irq_domain *domain)
> * node when all entries are removed. Shout if there are
> * any mappings left.
> */
> - WARN_ON(domain->revmap_data.tree.height);
> - break;
> - case IRQ_DOMAIN_MAP_LINEAR:
> - kfree(domain->revmap_data.linear.revmap);
> - domain->revmap_data.linear.size = 0;
> - break;
> - case IRQ_DOMAIN_MAP_NOMAP:
> + WARN_ON(domain->radix_tree.height);
> break;
> }
> @@ -177,20 +172,11 @@ struct irq_domain *irq_domain_add_linear(struct device_node *of_node,
> void *host_data)
> {
> struct irq_domain *domain;
> - unsigned int *revmap;
>
> - revmap = kzalloc_node(sizeof(*revmap) * size, GFP_KERNEL,
> - of_node_to_nid(of_node));
> - if (WARN_ON(!revmap))
> + domain = irq_domain_alloc(of_node, IRQ_DOMAIN_MAP_LINEAR, size, ops, host_data);
> + if (!domain)
> return NULL;
>
> - domain = irq_domain_alloc(of_node, IRQ_DOMAIN_MAP_LINEAR, ops, host_data);
> - if (!domain) {
> - kfree(revmap);
> - return NULL;
> - }
> - domain->revmap_data.linear.size = size;
> - domain->revmap_data.linear.revmap = revmap;
> irq_domain_add(domain);
> return domain;
> }
> @@ -202,9 +188,9 @@ struct irq_domain *irq_domain_add_nomap(struct device_node *of_node,
> void *host_data)
> {
> struct irq_domain *domain = irq_domain_alloc(of_node,
> - IRQ_DOMAIN_MAP_NOMAP, ops, host_data);
> + IRQ_DOMAIN_MAP_NOMAP, 0, ops, host_data);
> if (domain) {
> - domain->revmap_data.nomap.max_irq = max_irq ? max_irq : ~0;
> + domain->nomap_max_irq = max_irq ? max_irq : ~0;
> irq_domain_add(domain);
> }
> return domain;
> @@ -224,9 +210,9 @@ struct irq_domain *irq_domain_add_tree(struct device_node *of_node,
> void *host_data)
> {
> struct irq_domain *domain = irq_domain_alloc(of_node,
> - IRQ_DOMAIN_MAP_TREE, ops, host_data);
> + IRQ_DOMAIN_MAP_TREE, 0, ops, host_data);
> if (domain) {
> - INIT_RADIX_TREE(&domain->revmap_data.tree, GFP_KERNEL);
> + INIT_RADIX_TREE(&domain->radix_tree, GFP_KERNEL);
> irq_domain_add(domain);
> }
> return domain;
> @@ -315,12 +301,12 @@ static void irq_domain_disassociate_many(struct irq_domain *domain,
> /* Clear reverse map */
> switch(domain->revmap_type) {
> case IRQ_DOMAIN_MAP_LINEAR:
> - if (hwirq < domain->revmap_data.linear.size)
> - domain->revmap_data.linear.revmap[hwirq] = 0;
> + if (hwirq < domain->linear_size)
> + domain->linear_revmap[hwirq] = 0;
> break;
> case IRQ_DOMAIN_MAP_TREE:
> mutex_lock(&revmap_trees_mutex);
> - radix_tree_delete(&domain->revmap_data.tree, hwirq);
> + radix_tree_delete(&domain->radix_tree, hwirq);
> mutex_unlock(&revmap_trees_mutex);
> break;
> }
> @@ -362,12 +348,12 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
>
> switch (domain->revmap_type) {
> case IRQ_DOMAIN_MAP_LINEAR:
> - if (hwirq < domain->revmap_data.linear.size)
> - domain->revmap_data.linear.revmap[hwirq] = virq;
> + if (hwirq < domain->linear_size)
> + domain->linear_revmap[hwirq] = virq;
> break;
> case IRQ_DOMAIN_MAP_TREE:
> mutex_lock(&revmap_trees_mutex);
> - radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
> + radix_tree_insert(&domain->radix_tree, hwirq, irq_data);
> mutex_unlock(&revmap_trees_mutex);
> break;
> }
> @@ -406,9 +392,9 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
> pr_debug("create_direct virq allocation failed\n");
> return 0;
> }
> - if (virq >= domain->revmap_data.nomap.max_irq) {
> + if (virq >= domain->nomap_max_irq) {
> pr_err("ERROR: no free irqs available below %i maximum\n",
> - domain->revmap_data.nomap.max_irq);
> + domain->nomap_max_irq);
> irq_free_desc(virq);
> return 0;
> }
> @@ -612,7 +598,7 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
> return irq_linear_revmap(domain, hwirq);
> case IRQ_DOMAIN_MAP_TREE:
> rcu_read_lock();
> - data = radix_tree_lookup(&domain->revmap_data.tree, hwirq);
> + data = radix_tree_lookup(&domain->radix_tree, hwirq);
> rcu_read_unlock();
> if (data)
> return data->irq;
> @@ -644,10 +630,10 @@ unsigned int irq_linear_revmap(struct irq_domain *domain,
> BUG_ON(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR);
>
> /* Check revmap bounds; complain if exceeded */
> - if (WARN_ON(hwirq >= domain->revmap_data.linear.size))
> + if (WARN_ON(hwirq >= domain->linear_size))
> return 0;
>
> - return domain->revmap_data.linear.revmap[hwirq];
> + return domain->linear_revmap[hwirq];
> }
> EXPORT_SYMBOL_GPL(irq_linear_revmap);
>
next prev parent reply other threads:[~2012-06-16 6:07 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-16 5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
2012-06-16 5:01 ` [PATCH 01/12] irqdomain: Split disassociating code into separate function Grant Likely
2012-06-16 5:57 ` Benjamin Herrenschmidt
2012-06-16 5:01 ` [PATCH 02/12] irqdomain: Always update revmap when setting up a virq Grant Likely
2012-06-16 5:57 ` Benjamin Herrenschmidt
2012-06-16 5:01 ` [PATCH 03/12] irqdomain: Support for static IRQ mapping and association Grant Likely
2012-06-16 5:58 ` Benjamin Herrenschmidt
2012-06-17 22:16 ` Grant Likely
2012-06-16 5:01 ` [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions Grant Likely
2012-06-16 5:56 ` Benjamin Herrenschmidt
2012-06-16 6:12 ` Grant Likely
2012-06-17 21:58 ` Grant Likely
2012-06-16 5:01 ` [PATCH 05/12] irqdomain: Fix irq_create_direct_mapping() to test irq_domain type Grant Likely
2012-06-16 5:01 ` [PATCH 06/12] irqdomain: eliminate slow-path revmap lookups Grant Likely
2012-06-16 5:01 ` [PATCH 07/12] irqdomain: Make ops->map hook optional Grant Likely
2012-06-16 5:59 ` Benjamin Herrenschmidt
2012-06-16 5:01 ` [PATCH 08/12] irqdomain: Replace LEGACY mapping with LINEAR Grant Likely
2012-06-16 6:01 ` Benjamin Herrenschmidt
2012-06-16 6:16 ` Grant Likely
2012-06-18 12:23 ` Mark Brown
2012-06-16 5:01 ` [PATCH 09/12] irqdomain: Reserve IRQs for legacy domain Grant Likely
2012-06-16 5:01 ` [PATCH 10/12] irqdomain: Add debugging message Grant Likely
2012-06-16 6:02 ` Benjamin Herrenschmidt
2012-06-16 5:01 ` [PATCH 11/12] irqdomain: reorganize revmap data Grant Likely
2012-06-16 6:06 ` Benjamin Herrenschmidt [this message]
2012-06-16 6:19 ` Grant Likely
2012-06-16 6:20 ` Grant Likely
2012-06-16 5:01 ` [PATCH 12/12] irqdomain: merge linear and tree reverse mappings Grant Likely
2012-06-18 12:28 ` [PATCH 00/12] irqdomain cleanup and refactoring Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1339826793.9220.229.camel@pasglop \
--to=benh@kernel.crashing.org \
--cc=grant.likely@secretlab.ca \
--cc=lethal@linux-sh.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miltonm@bga.com \
--cc=rob.herring@calxeda.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox