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

[Marc Zyngier <maz@kernel.org>]

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...