linux-kernel - Re: [RFC 1/2] irq_domain: Create common xlate functions that device drivers can use

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]

Date:	Thu, 26 Jan 2012 11:15:39 -0700
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Rob Herring <robherring2@...il.com>
Cc:	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linuxppc-dev@...ts.ozlabs.org,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [RFC 1/2] irq_domain: Create common xlate functions that device
 drivers can use

On Thu, Jan 26, 2012 at 7:50 AM, Rob Herring <robherring2@...il.com> wrote:
> On 01/25/2012 11:59 AM, Grant Likely wrote:
>> On Tue, Jan 24, 2012 at 10:35 PM, Grant Likely
>> <grant.likely@...retlab.ca> wrote:
>>> On Tue, Jan 24, 2012 at 6:50 PM, Rob Herring <robherring2@...il.com> wrote:
>>>>
>>>>
>>>> On 01/24/2012 06:18 PM, Grant Likely wrote:
>>>>> Rather than having each interrupt controller driver creating its own barely
>>>>> unique .xlate function for irq_domain, create a library of translators which
>>>>> any driver can use directly.
>>>>>
>>>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>>>>> index e7379a3..5e497a0 100644
>>>>> --- a/include/linux/irqdomain.h
>>>>> +++ b/include/linux/irqdomain.h
>>>>> @@ -110,6 +110,9 @@ struct irq_domain {
>>>>>       void *host_data;
>>>>>       irq_hw_number_t inval_irq;
>>>>>
>>>>> +     /* Data for common irq xlate functions */
>>>>> +     unsigned int xlate_type;
>>>>> +
>>>>
>>>> How does this get set? Do we want interrupt controllers messing with the
>>>> domain struct directly long term?
>>>
>>> It defaults to IRQ_TYPE_NONE in the alloc function and drivers can
>>> override it.  Alternately it could be made part of the
>>> irq_domain_add() arguments, but I'm not thrilled with adding a whole
>>> bunch of arguments to the function prototype.
>>
>> Do you like this better (built on top of this patch)?
>
> Yes, I think it's better.

I've changed my mind on this after looking at the generated code.
These functions are so simple that it actually is larger (both in
source and object size) to call out to a common function.  I've gone
back to each _xlate_*() function open coding the two assignments.

g.

>
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index ef2b1fe..7856c04 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -701,12 +701,10 @@ int irq_domain_xlate_onecell(struct irq_domain
>> *d, struct device_node *ctrlr,
>>                            const u32 *intspec, unsigned int intsize,
>>                            unsigned long *out_hwirq, unsigned int *out_type)
>>  {
>> -     if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
>> -              ctrlr->full_name, intsize))
>> +     if (WARN_ON(intsize < 1))
>>               return -EINVAL;
>> -     *out_hwirq = intspec[0];
>> -     *out_type = d->xlate_type;
>> -     return 0;
>> +     return irq_domain_xlate_onetwocell(d, ctrlr, intspec, 1,
>> +                                        out_hwirq, out_type);
>>  }
>>  EXPORT_SYMBOL_GPL(irq_domain_xlate_onecell);
>>
>> @@ -721,12 +719,10 @@ int irq_domain_xlate_twocell(struct irq_domain
>> *d, struct device_node *ctrlr,
>>                       const u32 *intspec, unsigned int intsize,
>>                       irq_hw_number_t *out_hwirq, unsigned int *out_type)
>>  {
>> -     if (WARN(intsize < 2, "Bad intspec for %s: intsize=%i < 2\n",
>> -              ctrlr->full_name, intsize))
>> +     if (WARN_ON(intsize < 2))
>>               return -EINVAL;
>> -     *out_hwirq = intspec[0];
>> -     *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
>> -     return 0;
>> +     return irq_domain_xlate_onetwocell(d, ctrlr, intspec, intsize,
>> +                                        out_hwirq, out_type);
>>  }
>>  EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
>>
>> @@ -746,8 +742,7 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d,
>>                               const u32 *intspec, unsigned int intsize,
>>                               unsigned long *out_hwirq, unsigned int *out_type)
>>  {
>> -     if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
>> -              ctrlr->full_name, intsize))
>> +     if (WARN_ON(intsize < 1))
>>               return -EINVAL;
>>       *out_hwirq = intspec[0];
>>       *out_type = d->xlate_type;
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
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/