Subject: Re: [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent

From: Lokesh Vutla <[email-masked]>
Date: Wed, 9 Oct 2019 10:30:12 +0530

Hi Marc,

On 03/10/19 6:45 PM, Lokesh Vutla wrote:
> Hi Marc,
> 
> On 02/10/19 6:27 PM, Marc Zyngier wrote:
>> On Mon, 23 Sep 2019 09:54:04 +0530
>> Lokesh Vutla <[email-masked]> wrote:
>>
>>> Drop the firmware related dt-bindings and use the hardware specified
>>> interrupt numbers within Interrupt Router. This ensures interrupt router
>>> DT node need not assume any interrupt parent type.
>>>
>>> Signed-off-by: Lokesh Vutla <[email-masked]>
>>> ---
>>>  .../interrupt-controller/ti,sci-intr.txt      | 28 +++++++++----------
>>>  1 file changed, 13 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>> index 1a8718f8855d..de5de2a4b467 100644
>>> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>> @@ -44,15 +44,14 @@ Required Properties:
>>>  			4: If intr supports level triggered interrupts.
>>>  - interrupt-controller:	Identifies the node as an interrupt controller
>>>  - #interrupt-cells:	Specifies the number of cells needed to encode an
>>> -			interrupt source. The value should be 2.
>>> -			First cell should contain the TISCI device ID of source
>>> -			Second cell should contain the interrupt source offset
>>> -			within the device.
>>> +			interrupt source. The value should be 1.
>>> +			First cell should contain interrupt router input number
>>> +			as specified by hardware.
>>
>> This is breaking compatibility with existing kernels. Why isn't that a
>> problem?
> 
> Yes, I am not denying that backward compatibility is broken. But I feel this is
> a good cleanup for representing INTR interrupts. With this child nodes will pass
> the INTR specific number rather the device specific offset. Actually I tried
> following as what GIC is representing.
> 
> As there are only two users upstreamed, I though it is better to clean it up
> asap. Do you prefer maintaining the existing usage with a different compatible?

Just looking for your opinion here. If I try to maintain DT backward compatible,
are you okay with the overall approach. Will clean and repost patches once
firmware is ready.

Thank and regards,
Lokesh

> 
> 
>>
>>> +- power-domains:	phandle pointing to the corresponding PM domain node
>>> +			and an ID representing the device.
>>
>> Why is this power-domain thing part of this patch? Is it related?
>>
>>>  - ti,sci:		Phandle to TI-SCI compatible System controller node.
>>> -- ti,sci-dst-id:	TISCI device ID of the destination IRQ controller.
>>> -- ti,sci-rm-range-girq:	Array of TISCI subtype ids representing the host irqs
>>> -			assigned to this interrupt router. Each subtype id
>>> -			corresponds to a range of host irqs.
>>> +- interrupt-ranges:	Ranges that convert the INTR output interrupt numbers to
>>> +			parent's interrupt number.
>>
>> This isn't a standard property, is it? If it isn't, it should be
>> documented a bit more than just that.
> 
> Nope. Ill update it. So, the reason this is added is that this ranges converts
> the output of INTR to parent interrupt number. This is to have consistent dt
> convention in the following 2 scenarios:
> - INTR -> GIC (GIC is parent of INTR)
> - INTR -> INTR (INTR is parent of INTR)
> 
> Thanks and regards,
> Lokesh
> 
>>
>>>  
>>>  For more details on TISCI IRQ resource management refer:
>>>  http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
>>> @@ -62,21 +61,20 @@ Example:
>>>  The following example demonstrates both interrupt router node and the consumer
>>>  node(main gpio) on the AM654 SoC:
>>>  
>>> -main_intr: interrupt-controller0 {
>>> +main_gpio_intr: interrupt-controller0 {
>>>  	compatible = "ti,sci-intr";
>>>  	ti,intr-trigger-type = <1>;
>>>  	interrupt-controller;
>>>  	interrupt-parent = <&gic500>;
>>> -	#interrupt-cells = <2>;
>>> +	power-domains = <&k3_pds 131 TI_SCI_PD_SHARED>;
>>> +	#interrupt-cells = <1>;
>>>  	ti,sci = <&dmsc>;
>>> -	ti,sci-dst-id = <56>;
>>> -	ti,sci-rm-range-girq = <0x1>;
>>> +	interrupt-ranges = <0 360 32>;
>>>  };
>>>  
>>>  main_gpio0: gpio@600000 {
>>>  	...
>>> -	interrupt-parent = <&main_intr>;
>>> -	interrupts = <57 256>, <57 257>, <57 258>,
>>> -		     <57 259>, <57 260>, <57 261>;
>>> +	interrupt-parent = <&main_gpio_intr>;
>>> +	interrupts = <192>, <193>, <194>, <195>, <196>, <197>;
>>>  	...
>>>  };
>>
>> Thanks,
>>
>> 	M.
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> [email-masked]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel