lkml.org 
[lkml]   [2017]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC PATCH] genirq: Avoid unnecessary low level irq function calls
Hi Thomas,

thanx for your comments.

On 06/26/2017 04:20 PM, Thomas Gleixner wrote:
> Jeffy,
>
> On Mon, 26 Jun 2017, Jeffy Chen wrote:
>> void irq_enable(struct irq_desc *desc)
>> {
>> - irq_state_clr_disabled(desc);
>> - if (desc->irq_data.chip->irq_enable)
>> - desc->irq_data.chip->irq_enable(&desc->irq_data);
>> - else
>> - desc->irq_data.chip->irq_unmask(&desc->irq_data);
>> - irq_state_clr_masked(desc);
>> + if (irqd_is_started(&desc->irq_data) &&
>> + !irqd_irq_disabled(&desc->irq_data)) {
>> + unmask_irq(desc);
>
> I'm having a hard time to understand the logic here.
>
> if (started && !disabled)
> unmask()
>
> That does not make any sense. If you need that to work around a state
> inconsistency then it needs to be addressed there and not worked around
> here.
right, we already set disabled state in desc_set_defaults, so the
disabled state should be consistency here. so just
if(!disabled)
unmask()
else
enable()
?

and i saw we didn't set masked state in desc_set_defaults, i'll send a
patch for that, and remove unmask_irq's started check too.
>
>> + } else {
>> + irq_state_clr_disabled(desc);
>> + if (desc->irq_data.chip->irq_enable) {
>> + desc->irq_data.chip->irq_enable(&desc->irq_data);
>> + irq_state_clr_masked(desc);
>> + } else {
>> + unmask_irq(desc);
>> + }
>> + }
>> }
>>
>> static void __irq_disable(struct irq_desc *desc, bool mask)
>> {
>> - irq_state_set_disabled(desc);
>> - if (desc->irq_data.chip->irq_disable) {
>> - desc->irq_data.chip->irq_disable(&desc->irq_data);
>> - irq_state_set_masked(desc);
>> - } else if (mask) {
>> - mask_irq(desc);
>> + if (irqd_irq_disabled(&desc->irq_data)) {
>> + if (mask)
>> + mask_irq(desc);
>> + } else {
>> + irq_state_set_disabled(desc);
>> + if (desc->irq_data.chip->irq_disable) {
>> + desc->irq_data.chip->irq_disable(&desc->irq_data);
>> + irq_state_set_masked(desc);
>> + } else if (mask) {
>> + mask_irq(desc);
>> + }
>> }
>> }
>>
>> @@ -311,18 +322,21 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
>>
>> static inline void mask_ack_irq(struct irq_desc *desc)
>> {
>> - if (desc->irq_data.chip->irq_mask_ack)
>> + if (desc->irq_data.chip->irq_mask_ack) {
>> desc->irq_data.chip->irq_mask_ack(&desc->irq_data);
>> - else {
>> - desc->irq_data.chip->irq_mask(&desc->irq_data);
>> + irq_state_set_masked(desc);
>> + } else {
>> + mask_irq(desc);
>> if (desc->irq_data.chip->irq_ack)
>> desc->irq_data.chip->irq_ack(&desc->irq_data);
>> }
>> - irq_state_set_masked(desc);
>> }
>>
>> void mask_irq(struct irq_desc *desc)
>> {
>> + if (irqd_irq_masked(&desc->irq_data))
>> + return;
>> +
>> if (desc->irq_data.chip->irq_mask) {
>> desc->irq_data.chip->irq_mask(&desc->irq_data);
>> irq_state_set_masked(desc);
>> @@ -331,6 +345,10 @@ void mask_irq(struct irq_desc *desc)
>>
>> void unmask_irq(struct irq_desc *desc)
>> {
>> + if (irqd_is_started(&desc->irq_data) &&
>> + !irqd_irq_masked(&desc->irq_data))
>> + return;
>
> Ditto.
>
> Thanks,
>
> tglx
>
>
>


\
 
 \ /
  Last update: 2017-06-26 11:58    [W:0.483 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site