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

[Thomas Abraham <thomas.abraham@linaro.org>]

On 26 March 2012 21:06, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> On 26 March 2012 18:34, Rob Herring <robherring2@gmail.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;