linux-kernel - Re: [PATCH] of/irq: of_irq_init: Call initialization function for all controllers

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] [day] [month] [year] [list]

Message-ID: <CAJuYYwSTA_oibeF0maCeaMnw4z2hU33wBjW_ussfLY9S-=UEmA@mail.gmail.com>
Date:	Wed, 28 Mar 2012 11:32:20 +0530
From:	Thomas Abraham <thomas.abraham@...aro.org>
To:	Rob Herring <robherring2@...il.com>
Cc:	devicetree-discuss@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
	linux-samsung-soc@...r.kernel.org, rob.herring@...xeda.com,
	patches@...aro.org
Subject: Re: [PATCH] of/irq: of_irq_init: Call initialization function for all controllers

On 26 March 2012 21:06, Thomas Abraham <thomas.abraham@...aro.org> wrote:
> On 26 March 2012 18:34, Rob Herring <robherring2@...il.com> wrote:

>> Your fix doesn't really guarantee the proper order either. It's still a
>> side effect of the implementation. Perhaps a retry mechanism would work.
>> Then the init for wakeup_eint can retry if the gic is not yet setup.
>
> Ok. A retry mechanism would be most robust solution for this case.
>

Hi Rob,

I have attempted an implementation of the the retry mechanism in the
of_irq_init() function. It allows initialization functions of
interrupt controller nodes to report -EAGAIN indicating that the
initialization was not complete and should be retried.

I have tested this and it is working fine. But the complexity of the
interrupt controller's intialization function increases because it has
to undo some of the intialization steps it completed before finding
that the its parent controller was not initialized.

The following is the diff of the changes for of_irq_init() function.
Please let know if this is acceptable.

Thanks,
Thomas.

@@ -452,7 +454,6 @@ void __init of_irq_init(const struct of_device_id *matches)
 			if (desc->interrupt_parent != parent)
 				continue;

-			list_del(&desc->list);
 			match = of_match_node(matches, desc->dev);
 			if (WARN(!match->data,
 			    "of_irq_init: no init function for %s\n",
@@ -466,6 +467,14 @@ void __init of_irq_init(const struct of_device_id *matches)
 				 desc->dev, desc->interrupt_parent);
 			irq_init_cb = match->data;
 			ret = irq_init_cb(desc->dev, desc->interrupt_parent);
+			if (ret == -EAGAIN)
+				/*
+				 * Interrupt controller's initialization did not
+				 * complete and should be retried. So let its
+				 * intc_desc be on intc_desc_list.
+				 */
+				continue;
+			list_del(&desc->list);
 			if (ret) {
 				kfree(desc);
 				continue;
@@ -482,7 +491,18 @@ void __init of_irq_init(const struct of_device_id *matches)
 		desc = list_first_entry(&intc_parent_list, typeof(*desc), list);
 		if (list_empty(&intc_parent_list) || !desc) {
 			pr_err("of_irq_init: children remain, but no parents\n");
-			break;
+			/*
+			 * If a search with NULL as parent did not result in any
+			 * new parent being found, then the scan for matching
+			 * interrupt controller nodes is considered as complete.
+			 * Otherwise, if there are pending elements on the
+			 * intc_desc_list, then retry this process again with
+			 * NULL as parent.
+			 */
+			if (!parent)
+				break;
+			parent = NULL;
+			continue;
 		}
 		list_del(&desc->list);
 		parent = desc->dev;
--
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/