On 17/12/15 13:16, Linus Walleij wrote:
> On Thu, Dec 17, 2015 at 11:48 AM, Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>
>> When mapping an IRQ, if a mapping already exists, then we simply return
>> the virual IRQ number. However, we do not check that the type settings for
>
> ^virtual
>
> Just that it isn't virtual, it's a Linux IRQ number, we actually use
> hwirq for the non-virtual IRQ number/offse in this function.
>
> But I know I may be fighting weathermills here.
Ok, will re-word this.
>> unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
>> {
>> + struct device_node *of_node;
>> struct irq_domain *domain;
>> irq_hw_number_t hwirq;
>> + unsigned int cur_type = IRQ_TYPE_NONE;
>> unsigned int type = IRQ_TYPE_NONE;
>> int virq;
>>
>> @@ -587,23 +589,49 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
>> if (irq_domain_translate(domain, fwspec, &hwirq, &type))
>> return 0;
>>
>> - if (irq_domain_is_hierarchy(domain)) {
>> - /*
>> - * If we've already configured this interrupt,
>> - * don't do it again, or hell will break loose.
>> - */
>> - virq = irq_find_mapping(domain, hwirq);
>> - if (virq)
>> - return virq;
>> + of_node = irq_domain_get_of_node(domain);
>
> Marc's patches went to great lengths to do this fwspec-neutral,
> i.e. it doesn't matter if it's done by DT or ACPI (or whatever).
>
> This just drives a truck through all of that by making
> the whole function OF-specific again.
Yes, was not sure if this would be popular. I was on the fence, but I
saw the following ...
if (!domain) {
pr_warn("no irq domain found for %s !\n",
of_node_full_name(to_of_node(fwspec->fwnode)));
return 0;
}
... and thought we are not completely agnostic. However, if you prefer I
park my mini else where, I can definitely drop this, no big deal ;-)
>>
>> - virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, fwspec);
>> - if (virq <= 0)
>> - return 0;
>> + /*
>> + * If we've already configured this interrupt,
>> + * don't do it again, or hell will break loose.
>> + */
>> + virq = irq_find_mapping(domain, hwirq);
>> + if (!virq) {
>> + if (irq_domain_is_hierarchy(domain)) {
>> + virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE,
>> + fwspec);
>> + if (virq <= 0)
>> + return 0;
>> + } else {
>> + virq = irq_domain_alloc_descs(-1, 1, hwirq,
>> + of_node_to_nid(of_node));
>
> What is this all of a sudden? Not even mentioned in the
> commit. Plus I bet ACPI need something else than OF nid
> passed here.
Do you mean the else part of all of the above?
So in the current code, the else part calls irq_create_mapping() and
this function internally, calls irq_find_mapping(). Given that I am now
calling irq_find_mapping() before the if, I don't really need to call
irq_create_mapping() again, I just need to call the functions in
irq_create_mapping() that allocate and setup the IRQ number. Sorry, I
did not really explain this. However, if it is simpler, I can call
irq_create_mapping() instead and may be this makes the change easier to
read.
Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
