Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH RFC v2 1/3] drivers: irqchip: pdc: Add PDC interrupt
 controller for QCOM SoCs
To:     Marc Zyngier <marc.zyngier@arm.com>,
        Lina Iyer <ilina@codeaurora.org>, <tglx@linutronix.de>,
        <jason@lakedaemon.net>
CC:     <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <sboyd@codeaurora.org>, <rnayak@codeaurora.org>,
        <asathyak@codeaurora.org>
References: <20180202142200.6229-1-ilina@codeaurora.org>
 <20180202142200.6229-2-ilina@codeaurora.org>
 <e1322b9a-4765-3c71-84b8-cb725fe457dd@arm.com>
From:   Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Message-ID: <02e2a5f9-8ecc-76b1-a3cc-c95b215e8fe1@prevas.dk>
Date:   Fri, 23 Feb 2018 13:16:08 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.6.0
MIME-Version: 1.0
In-Reply-To: <e1322b9a-4765-3c71-84b8-cb725fe457dd@arm.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2018-02-02 15:58, Marc Zyngier wrote:
> Hi Lina,
> 
> On 02/02/18 14:21, Lina Iyer wrote:
>> From : Archana Sathyakumar <asathyak@codeaurora.org>
>>
>> +
>> +static int qcom_pdc_translate(struct irq_domain *d,
>> +	struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type)
>> +{
>> +	if (is_of_node(fwspec->fwnode)) {
>> +		if (fwspec->param_count < 3)
>> +			return -EINVAL;
> 
> Why 3? Reading the DT binding, this is indeed set to 3 without any
> reason. I'd suggest this becomes 2, encoding the pin number and the
> trigger information, as the leading 0 is quite useless. Yes, I know
> other examples in the kernel are using this 0, and that was a
> consequence of retrofitting the omitted interrupt controllers (back in
> the days of the stupid gic_arch_extn...). Don't do that.
> 

Hi Marc

I'm about to send out a new revision of the ls-extirq patchset, and
thanks to you pointing me to these patches, I've read the comments on
the various revisions of this series and tried to take those into
account. But the above confused me, because in response to my first RFC
(https://patchwork.kernel.org/patch/10102643/) we have

On 2017-12-08 17:09, Marc Zyngier wrote:
> On 08/12/17 15:11, Alexander Stein wrote:
>> Hi Rasmus,
>>
>>> +
>>> +Required properties:
>>> +- compatible: should be "fsl,ls1021a-extirq"
>>> +- interrupt-controller: Identifies the node as an interrupt controller
>>> +- #interrupt-cells: Use the same format as specified by GIC in
arm,gic.txt.
>>
>> Do you really need 3 interrupt-cells here? As you've written below
you don't
>> support PPI anyway the 1st flag might be dropped then. So support
just 2 cells:
>> * IRQ number (IRQ0 - IRQ5)
>> * IRQ flags
>
> The convention for irqchip stacked on top of a GIC is to keep the
> interrupt specifier the same. It makes the maintenance if the DT much
> easier, and doesn't hurt at all.

Personally, I'd actually prefer the simpler interrupt specifiers without
a redundant 0. Maybe I'm just missing some difference between this case
and the ls-extirq one?

Thanks,
Rasmus