Subject: Re: [PATCH 1/1] irq: add IRQF_TRIGGER_MASK on PPI by default
To: Thomas Gleixner <tglx@linutronix.de>,
        Aniruddha Banerjee <aniruddhab@nvidia.com>
References: <fe0774b2000848a6bd065efc649c6eab@bgmail102.nvidia.com>
 <alpine.DEB.2.20.1703310953520.1780@nanos>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Thierry Reding" <treding@nvidia.com>,
        Stephen Warren <swarren@nvidia.com>,
        "Marc Zyngier" <marc.zyngier@arm.com>
From: Jon Hunter <jonathanh@nvidia.com>
Message-ID: <651c0971-9e8f-b8cb-fb06-16c8c814649c@nvidia.com>
Date: Fri, 31 Mar 2017 09:13:04 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.20.1703310953520.1780@nanos>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1390
Lines: 42


On 31/03/17 09:01, Thomas Gleixner wrote:
> On Thu, 30 Mar 2017, Aniruddha Banerjee wrote:
> 
>> add IRQF_TRIGGER_MASK on PPI by default so that the PPIs are
>> not configured as edge-triggered, which may be wrong for certain GIC
>> implementations such as the GIC-400
> 
> The above is just useless blurb.
> 
> I can't figure out at all WHY a generic interface has anything to do with
> edge trigger configuration.

I have to agree, it does not make sense in the context of the patch. The
only thing I can think of that this is trying to circumvent the lookup
of the trigger type in __setup_irq() ...

 /*
  * If the trigger type is not specified by the caller,
  * then use the default for this interrupt.
  */
 if (!(new->flags & IRQF_TRIGGER_MASK))
 	new->flags |= irqd_get_trigger_type(&desc->irq_data);

If that is the case, then this does not look correct to me and will most
likely breaking percpu interrupts that do need to lookup the type.

> I assume this is (Nvidia) GIC specific nonsense, so why are you inflicting
> this on every caller of this interface unconditionally w/o explaining what
> the impact of this change might be and why it does not cause havoc for any
> existing caller?

Yes, however, some new nonsense I am not aware of :-(

Aniruddha, why can we not just set the type correctly for the PPI in the
device-tree file and avoid this?

Cheers
Jon

-- 
nvpublic