On 2014/9/23 17:43, Joe.C wrote:
> On Mon, 2014-09-22 at 16:17 +0800, Jiang Liu wrote:
>> @@ -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.
>
>> 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);
>> +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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
