On 2014/9/23 17:43, Joe.C wrote:

On Mon, 2014-09-22 at 16:17 +0800, Jiang Liu wrote:

quoted

@@ -388,7 +389,6 @@ EXPORT_SYMBOL_GPL(irq_create_direct_mapping);
 unsigned int irq_create_mapping(struct irq_domain *domain,
 				irq_hw_number_t hwirq)
 {
-	unsigned int hint;
 	int virq;
 
 	pr_debug("irq_create_mapping(0x%p, 0x%lx)\n", domain, hwirq);

@@ -410,12 +410,8 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
 	}
 
 	/* Allocate a virtual interrupt number */
-	hint = hwirq % nr_irqs;
-	if (hint == 0)
-		hint++;
-	virq = irq_alloc_desc_from(hint, of_node_to_nid(domain->of_node));
-	if (virq <= 0)
-		virq = irq_alloc_desc_from(1, of_node_to_nid(domain->of_node));
+	virq = irq_domain_alloc_descs(-1, 1, hwirq,
+				      of_node_to_nid(domain->of_node));

If I read this correct, the resulting virq is different after your
change.

It should have the same effect. We just factored out original code as
a function, so it could be reused.

quoted

 	if (virq <= 0) {
 		pr_debug("-> virq allocation failed\n");
 		return 0;

@@ -490,7 +486,10 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
 	}
 
 	/* Create mapping */
-	virq = irq_create_mapping(domain, hwirq);
+	if (irq_domain_is_hierarchy(domain))
+		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
+	else
+		virq = irq_create_mapping(domain, hwirq);
 	if (!virq)
 		return virq;

hwirq returned from xlat above is lost. Without hwirq or virq, how do we
know which irq are we working for?
Also, if the irq_desc/irq_data was already created, this will create
another one. Should we do irq_find_mapping just like irq_create_mapping?

When irq_create_of_mapping is called, IRQ desc/irq_data haven't been
allocated yet. The parameter irq_data is type of struct of_phandle_args
instead of struct irq_data:)

We pass irq_data to irq_domain_alloc_irqs(), the we could reconstruct
hwirq from irq_data if needed.

To make the code clearer, I plan to change code as below:
unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
{
        struct irq_domain *domain;
        irq_hw_number_t hwirq;
        unsigned int type = IRQ_TYPE_NONE;
        unsigned int virq;

        domain = irq_data->np ? irq_find_host(irq_data->np) :
irq_default_domain;
        if (!domain) {
                pr_warn("no irq domain found for %s !\n",
                        of_node_full_name(irq_data->np));
                return 0;
        }

+        if (irq_domain_is_hierarchy(domain))
+                return irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE,
irq_data);
+
        /* If domain has no translation, then we assume interrupt line */
        if (domain->ops->xlate == NULL)
                hwirq = irq_data->args[0];
        else {
                if (domain->ops->xlate(domain, irq_data->np, irq_data->args,
                                        irq_data->args_count, &hwirq,
&type))
                        return 0;
        }

        /* Create mapping */
        virq = irq_create_mapping(domain, hwirq);
        if (!virq)
                return virq;

        /* Set type if specified and different than the current one */
        if (type != IRQ_TYPE_NONE &&
            type != irq_get_trigger_type(virq))
                irq_set_irq_type(virq, type);
        return virq;
}
EXPORT_SYMBOL_GPL(irq_create_of_mapping);

quoted

+int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
+			    unsigned int nr_irqs, int node, void *arg,
+			    bool realloc)
+{
+	int i, ret, virq;
+
+	if (domain == NULL) {
+		domain = irq_default_domain;
+		if (WARN(!domain, "domain is NULL; cannot allocate IRQ\n"))
+			return -EINVAL;
+	}
+
+	if (!domain->ops->alloc) {
+		pr_debug("domain->ops->alloc() is NULL\n");
+		return -ENOSYS;
+	}
+
+	if (realloc && irq_base >= 0) {
+		virq =  irq_base;

                         ^
extra space here.

Will fix it in next version.
Thanks, Joe!
Gerry

Joe.C