diff mbox series

[04/12] dt-bindings: irqchip: ti,sci-intr: Update bindings to drop the usage of gic as parent

Message ID 20200520124454.10532-5-lokeshvutla@ti.com
State Changes Requested
Headers show
Series irqchip: ti,sci-intr/inta: Update the dt bindings to accept different interrupt parents | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Lokesh Vutla May 20, 2020, 12:44 p.m. UTC
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 <lokeshvutla@ti.com>
---
 .../interrupt-controller/ti,sci-intr.txt      | 31 ++++++++++---------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Rob Herring May 28, 2020, 10:14 p.m. UTC | #1
On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla 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.

I didn't like this binding to begin with, but now you're breaking 
compatibility.

> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  .../interrupt-controller/ti,sci-intr.txt      | 31 ++++++++++---------
>  1 file changed, 16 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..8b56b2de1c73 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> @@ -44,15 +44,17 @@ 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.
>  - 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.
> +- ti,sci-dev-id:	TISCI device id of interrupt controller.
> +- ti,interrupt-ranges:	Set of triplets containing ranges that convert
> +			the INTR output interrupt numbers to parent's
> +			interrupt number. Each triplet has following entries:
> +			- First entry specifies the base for intr output irq
> +			- Second entry specifies the base for parent irqs
> +			- Third entry specifies the limit

Humm, sounds like what interrupt-map does.

>  
>  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 +64,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>;
> +	#interrupt-cells = <1>;
>  	ti,sci = <&dmsc>;
> -	ti,sci-dst-id = <56>;
> -	ti,sci-rm-range-girq = <0x1>;
> +	ti,sci-dev-id = <131>;
> +	ti,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>;
>  	...
>  };
> -- 
> 2.17.1
>
Lokesh Vutla May 29, 2020, 10:14 a.m. UTC | #2
Hi Rob,

On 29/05/20 3:44 am, Rob Herring wrote:
> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla 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.
> 
> I didn't like this binding to begin with, but now you're breaking 
> compatibility.

Yes, I do agree that this change is breaking backward compatibility. But IMHO,
this does cleanup of firmware specific properties from DT. Since this is not
deployed out yet in the wild market, I took the leverage of breaking backward
compatibility. Before accepting these changes from firmware team, I did
discuss[0] with Marc on this topic.

> 
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  .../interrupt-controller/ti,sci-intr.txt      | 31 ++++++++++---------
>>  1 file changed, 16 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..8b56b2de1c73 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> @@ -44,15 +44,17 @@ 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.
>>  - 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.
>> +- ti,sci-dev-id:	TISCI device id of interrupt controller.
>> +- ti,interrupt-ranges:	Set of triplets containing ranges that convert
>> +			the INTR output interrupt numbers to parent's
>> +			interrupt number. Each triplet has following entries:
>> +			- First entry specifies the base for intr output irq
>> +			- Second entry specifies the base for parent irqs
>> +			- Third entry specifies the limit
> 
> Humm, sounds like what interrupt-map does.

 Okay, Ill look at it.

[0]
https://lore.kernel.org/linux-arm-kernel/2331f04eacee3b06cc152fc609225233@www.loen.fr/

Thanks and regards,
Lokesh
Marc Zyngier May 29, 2020, 10:18 a.m. UTC | #3
On 2020-05-29 11:14, Lokesh Vutla wrote:
> Hi Rob,
> 
> On 29/05/20 3:44 am, Rob Herring wrote:
>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla 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.
>> 
>> I didn't like this binding to begin with, but now you're breaking
>> compatibility.
> 
> Yes, I do agree that this change is breaking backward compatibility. 
> But IMHO,
> this does cleanup of firmware specific properties from DT. Since this 
> is not
> deployed out yet in the wild market, I took the leverage of breaking 
> backward
> compatibility. Before accepting these changes from firmware team, I did
> discuss[0] with Marc on this topic.

And I assume that should anyone complain about the kernel being broken
because they have an old firmware, you'll be OK with the patches being
reverted, right?

Thanks,

         M.
Lokesh Vutla May 29, 2020, 11:13 a.m. UTC | #4
Hi Rob,

[..snip..]
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> index 1a8718f8855d..8b56b2de1c73 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> @@ -44,15 +44,17 @@ 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.
>>  - 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.
>> +- ti,sci-dev-id:	TISCI device id of interrupt controller.
>> +- ti,interrupt-ranges:	Set of triplets containing ranges that convert
>> +			the INTR output interrupt numbers to parent's
>> +			interrupt number. Each triplet has following entries:
>> +			- First entry specifies the base for intr output irq
>> +			- Second entry specifies the base for parent irqs
>> +			- Third entry specifies the limit
> 
> Humm, sounds like what interrupt-map does.

IIUC, for every irq translation there should be an entry in interrupt-map
property. These Controllers has interrupts ranging from 32, 256 and more. It
might be odd to have 256  entries in the interrupt map no? Also there are
multiple interrupt controllers which need such translations.

Thanks and regards,
Lokesh
Lokesh Vutla June 1, 2020, 11:36 a.m. UTC | #5
Hi Marc,

On 29/05/20 3:48 pm, Marc Zyngier wrote:
> On 2020-05-29 11:14, Lokesh Vutla wrote:
>> Hi Rob,
>>
>> On 29/05/20 3:44 am, Rob Herring wrote:
>>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla 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.
>>>
>>> I didn't like this binding to begin with, but now you're breaking
>>> compatibility.
>>
>> Yes, I do agree that this change is breaking backward compatibility. But IMHO,
>> this does cleanup of firmware specific properties from DT. Since this is not
>> deployed out yet in the wild market, I took the leverage of breaking backward
>> compatibility. Before accepting these changes from firmware team, I did
>> discuss[0] with Marc on this topic.
> 
> And I assume that should anyone complain about the kernel being broken
> because they have an old firmware, you'll be OK with the patches being
> reverted, right?

I am assuming there is no one to complain as there is no product available yet
with upstream kernel. Internally everyone is aware of the changes. So, yes I
would agree with you to revert the changes if someone really needs it. :)

Thanks and regards,
Lokesh
Lokesh Vutla June 15, 2020, 8:03 a.m. UTC | #6
Hi Marc,

On 01/06/20 5:06 pm, Lokesh Vutla wrote:
> Hi Marc,
> 
> On 29/05/20 3:48 pm, Marc Zyngier wrote:
>> On 2020-05-29 11:14, Lokesh Vutla wrote:
>>> Hi Rob,
>>>
>>> On 29/05/20 3:44 am, Rob Herring wrote:
>>>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla 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.
>>>>
>>>> I didn't like this binding to begin with, but now you're breaking
>>>> compatibility.
>>>
>>> Yes, I do agree that this change is breaking backward compatibility. But IMHO,
>>> this does cleanup of firmware specific properties from DT. Since this is not
>>> deployed out yet in the wild market, I took the leverage of breaking backward
>>> compatibility. Before accepting these changes from firmware team, I did
>>> discuss[0] with Marc on this topic.
>>
>> And I assume that should anyone complain about the kernel being broken
>> because they have an old firmware, you'll be OK with the patches being
>> reverted, right?
> 
> I am assuming there is no one to complain as there is no product available yet
> with upstream kernel. Internally everyone is aware of the changes. So, yes I
> would agree with you to revert the changes if someone really needs it. :)

Any chance you can shower your blessings on this series :)

Thanks and regards,
Lokesh

> 
> Thanks and regards,
> Lokesh
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Marc Zyngier June 15, 2020, 8:34 a.m. UTC | #7
On 2020-06-15 09:03, Lokesh Vutla wrote:
> Hi Marc,
> 
> On 01/06/20 5:06 pm, Lokesh Vutla wrote:
>> Hi Marc,
>> 
>> On 29/05/20 3:48 pm, Marc Zyngier wrote:
>>> On 2020-05-29 11:14, Lokesh Vutla wrote:
>>>> Hi Rob,
>>>> 
>>>> On 29/05/20 3:44 am, Rob Herring wrote:
>>>>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla 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.
>>>>> 
>>>>> I didn't like this binding to begin with, but now you're breaking
>>>>> compatibility.
>>>> 
>>>> Yes, I do agree that this change is breaking backward compatibility. 
>>>> But IMHO,
>>>> this does cleanup of firmware specific properties from DT. Since 
>>>> this is not
>>>> deployed out yet in the wild market, I took the leverage of breaking 
>>>> backward
>>>> compatibility. Before accepting these changes from firmware team, I 
>>>> did
>>>> discuss[0] with Marc on this topic.
>>> 
>>> And I assume that should anyone complain about the kernel being 
>>> broken
>>> because they have an old firmware, you'll be OK with the patches 
>>> being
>>> reverted, right?
>> 
>> I am assuming there is no one to complain as there is no product 
>> available yet
>> with upstream kernel. Internally everyone is aware of the changes. So, 
>> yes I
>> would agree with you to revert the changes if someone really needs it. 
>> :)
> 
> Any chance you can shower your blessings on this series :)

I have purposely ignored it just before and during the merge window. It 
is now firmly in my review queue.

         M.
Lokesh Vutla July 9, 2020, 9:40 a.m. UTC | #8
Hi Marc,

On 15/06/20 2:04 pm, Marc Zyngier wrote:
> On 2020-06-15 09:03, Lokesh Vutla wrote:
>> Hi Marc,
>>
>> On 01/06/20 5:06 pm, Lokesh Vutla wrote:
>>> Hi Marc,
>>>
>>> On 29/05/20 3:48 pm, Marc Zyngier wrote:
>>>> On 2020-05-29 11:14, Lokesh Vutla wrote:
>>>>> Hi Rob,
>>>>>
>>>>> On 29/05/20 3:44 am, Rob Herring wrote:
>>>>>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla 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.
>>>>>>
>>>>>> I didn't like this binding to begin with, but now you're breaking
>>>>>> compatibility.
>>>>>
>>>>> Yes, I do agree that this change is breaking backward compatibility. But IMHO,
>>>>> this does cleanup of firmware specific properties from DT. Since this is not
>>>>> deployed out yet in the wild market, I took the leverage of breaking backward
>>>>> compatibility. Before accepting these changes from firmware team, I did
>>>>> discuss[0] with Marc on this topic.
>>>>
>>>> And I assume that should anyone complain about the kernel being broken
>>>> because they have an old firmware, you'll be OK with the patches being
>>>> reverted, right?
>>>
>>> I am assuming there is no one to complain as there is no product available yet
>>> with upstream kernel. Internally everyone is aware of the changes. So, yes I
>>> would agree with you to revert the changes if someone really needs it. :)
>>
>> Any chance you can shower your blessings on this series :)
> 
> I have purposely ignored it just before and during the merge window. It is now
> firmly in my review queue.

rc4 is tagged now. Do you want me to rebase, split the series and repost it?

Thanks and reagrds,
Lokesh

> 
>         M.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
index 1a8718f8855d..8b56b2de1c73 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
@@ -44,15 +44,17 @@  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.
 - 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.
+- ti,sci-dev-id:	TISCI device id of interrupt controller.
+- ti,interrupt-ranges:	Set of triplets containing ranges that convert
+			the INTR output interrupt numbers to parent's
+			interrupt number. Each triplet has following entries:
+			- First entry specifies the base for intr output irq
+			- Second entry specifies the base for parent irqs
+			- Third entry specifies the limit
 
 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 +64,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>;
+	#interrupt-cells = <1>;
 	ti,sci = <&dmsc>;
-	ti,sci-dst-id = <56>;
-	ti,sci-rm-range-girq = <0x1>;
+	ti,sci-dev-id = <131>;
+	ti,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>;
 	...
 };