Re: [PATCH v3 6/9] irqchip/ti-sci-intr: Add support for INTR being a parent to INTR

From: Marc Zyngier <maz@kernel.org>
To: Lokesh Vutla <lokeshvutla@ti.com>
Cc: Nishanth Menon <nm@ti.com>,
	Peter Ujfalusi <peter.ujfalusi@ti.com>,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Device Tree Mailing List <devicetree@vger.kernel.org>,
	Sekhar Nori <nsekhar@ti.com>, Tero Kristo <t-kristo@ti.com>,
	Rob Herring <robh+dt@kernel.org>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linux ARM Mailing List <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 6/9] irqchip/ti-sci-intr: Add support for INTR being a parent to INTR
Date: Sat, 25 Jul 2020 15:42:44 +0100	[thread overview]
Message-ID: <71b33062f87f66e8c865b9e80b36585c@kernel.org> (raw)
In-Reply-To: <e19a84ff-d990-d87a-1df0-fd5b10654e3a@ti.com>

On 2020-07-25 15:37, Lokesh Vutla wrote:
> On 25/07/20 7:36 pm, Marc Zyngier wrote:
>> On Fri, 24 Jul 2020 15:18:34 +0100,
>> Lokesh Vutla <lokeshvutla@ti.com> wrote:
>>> 
>>> Driver assumes that Interrupt parent to Interrupt router is always 
>>> GIC.
>>> This is not true always and an Interrupt Router can be a parent to
>>> Interrupt Router. Update the driver to detect the parent and request 
>>> the
>>> parent irqs accordingly.
>>> 
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> ---
>>>  drivers/irqchip/irq-ti-sci-intr.c | 150 
>>> ++++++++++++++++++------------
>>>  1 file changed, 91 insertions(+), 59 deletions(-)
>>> 
>>> diff --git a/drivers/irqchip/irq-ti-sci-intr.c 
>>> b/drivers/irqchip/irq-ti-sci-intr.c
>>> index 59d51a20bbd8..0b73816e77fc 100644
>>> --- a/drivers/irqchip/irq-ti-sci-intr.c
>>> +++ b/drivers/irqchip/irq-ti-sci-intr.c

[...]

>>> -	ret = of_property_read_u32(dev_of_node(dev), "ti,sci-dst-id",
>>> -				   &intr->dst_id);
>>> +	ret = of_property_read_u32(dev_of_node(dev), "ti,sci-dev-id",
>>> +				   &pdev->id);
>> 
>> This feels very dodgy. You are hijacking a random field in the
>> platform device data structure, which shouldn't be any of your
> 
> pdev->id is pretty much unused field today. I was kind of hoping that 
> in future
> it can be utilized to populated field with TISCI device IDs in some 
> generic
> manner. So thought of using it now.

It isn't unused. DT doesn't make active use of it for now, not that's 
not
something you can rely on.

>> business. What was wrong with having a separate field for something
>> that is obviously platform specific?
> 
> No reason. I could just add a new filed in the intr structure.

Please do.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel