diff mbox series

[v6,06/12] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

Message ID 20190410041358.16809-7-lokeshvutla@ti.com
State Superseded, archived
Headers show
Series Add support for TISCI Interrupt controller drivers | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Lokesh Vutla April 10, 2019, 4:13 a.m. UTC 
Add the DT binding documentation for Interrupt router driver.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- Introduced a new property for specifying router trigger type.
- Dropped the trigger type from interrupt cells property.

Marc,
	Firmware change to not differentiate INTA interrupts and peripheral
	interrupts might take couple more weeks, but it is going to come.
	I am posting this series to get a review on the INTA driver.

 .../interrupt-controller/ti,sci-intr.txt      | 83 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt

Comments

Tony Lindgren April 11, 2019, 3 p.m. UTC | #1 
Hi,

* Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
> +Example:
> +--------
> +The following example demonstrates both interrupt router node and the consumer
> +node(main gpio) on the AM654 SoC:
> +
> +main_intr: interrupt-controller0 {
> +	compatible = "ti,sci-intr";
> +	ti,intr-trigger-type = <1>;
> +	interrupt-controller;
> +	interrupt-parent = <&gic500>;
> +	#interrupt-cells = <3>;
> +	ti,sci = <&dmsc>;
> +	ti,sci-dst-id = <56>;
> +	ti,sci-rm-range-girq = <0x1>;
> +};

To me it seems there should not be too many of these interrupt
controller nodes for each SoC. Maybe you're already planning on
doing it, but I suggest that you just add more specific compatibles
and then look up the dst-id from a mapping table in the driver
similar to what patch 04/12 in this series is already doing.

That way you don't need to add custom TI specific (firmware
defined) device tree properties listed above ;)

Regards,

Tony
Lokesh Vutla April 12, 2019, 4:24 a.m. UTC | #2 
On 11/04/19 8:30 PM, Tony Lindgren wrote:
> Hi,
> 
> * Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
>> +Example:
>> +--------
>> +The following example demonstrates both interrupt router node and the consumer
>> +node(main gpio) on the AM654 SoC:
>> +
>> +main_intr: interrupt-controller0 {
>> +	compatible = "ti,sci-intr";
>> +	ti,intr-trigger-type = <1>;
>> +	interrupt-controller;
>> +	interrupt-parent = <&gic500>;
>> +	#interrupt-cells = <3>;
>> +	ti,sci = <&dmsc>;
>> +	ti,sci-dst-id = <56>;
>> +	ti,sci-rm-range-girq = <0x1>;
>> +};
> 
> To me it seems there should not be too many of these interrupt
> controller nodes for each SoC. Maybe you're already planning on
> doing it, but I suggest that you just add more specific compatibles
> and then look up the dst-id from a mapping table in the driver
> similar to what patch 04/12 in this series is already doing.
> 
> That way you don't need to add custom TI specific (firmware
> defined) device tree properties listed above ;)

I am tired of arguing on this. We did close this topic in the previous version
of this series. Why do you want to keep re visiting the same. Sorry, I am not
going change unless I receive a Nack from Marc or Rob.

Thanks and regards,
Lokesh
Tero Kristo April 12, 2019, 8:42 a.m. UTC | #3 
On 12/04/2019 07:24, Lokesh Vutla wrote:
> 
> 
> On 11/04/19 8:30 PM, Tony Lindgren wrote:
>> Hi,
>>
>> * Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
>>> +Example:
>>> +--------
>>> +The following example demonstrates both interrupt router node and the consumer
>>> +node(main gpio) on the AM654 SoC:
>>> +
>>> +main_intr: interrupt-controller0 {
>>> +	compatible = "ti,sci-intr";
>>> +	ti,intr-trigger-type = <1>;
>>> +	interrupt-controller;
>>> +	interrupt-parent = <&gic500>;
>>> +	#interrupt-cells = <3>;
>>> +	ti,sci = <&dmsc>;
>>> +	ti,sci-dst-id = <56>;
>>> +	ti,sci-rm-range-girq = <0x1>;
>>> +};
>>
>> To me it seems there should not be too many of these interrupt
>> controller nodes for each SoC. Maybe you're already planning on
>> doing it, but I suggest that you just add more specific compatibles
>> and then look up the dst-id from a mapping table in the driver
>> similar to what patch 04/12 in this series is already doing.
>>
>> That way you don't need to add custom TI specific (firmware
>> defined) device tree properties listed above ;)
> 

< snip: I think Lokesh had a bad day or something :) >

Anyways, the reason why we want these as custom properties in the DT is 
that there are multiple instances of the routers within one SoC. On 
AM65x we have MAIN NAVSS, MCU NAVSS, GPIOs for both etc., if we add 
driver data for each of these, it easily explodes quite a bit. 
Especially going forward with new SoCs.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tony Lindgren April 12, 2019, 3:33 p.m. UTC | #4 
* Tero Kristo <t-kristo@ti.com> [190412 08:43]:
> On 12/04/2019 07:24, Lokesh Vutla wrote:
> > 
> > 
> > On 11/04/19 8:30 PM, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > * Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
> > > > +Example:
> > > > +--------
> > > > +The following example demonstrates both interrupt router node and the consumer
> > > > +node(main gpio) on the AM654 SoC:
> > > > +
> > > > +main_intr: interrupt-controller0 {
> > > > +	compatible = "ti,sci-intr";
> > > > +	ti,intr-trigger-type = <1>;
> > > > +	interrupt-controller;
> > > > +	interrupt-parent = <&gic500>;
> > > > +	#interrupt-cells = <3>;
> > > > +	ti,sci = <&dmsc>;
> > > > +	ti,sci-dst-id = <56>;
> > > > +	ti,sci-rm-range-girq = <0x1>;
> > > > +};
> > > 
> > > To me it seems there should not be too many of these interrupt
> > > controller nodes for each SoC. Maybe you're already planning on
> > > doing it, but I suggest that you just add more specific compatibles
> > > and then look up the dst-id from a mapping table in the driver
> > > similar to what patch 04/12 in this series is already doing.
> > > 
> > > That way you don't need to add custom TI specific (firmware
> > > defined) device tree properties listed above ;)
> > 
> 
> < snip: I think Lokesh had a bad day or something :) >

Certainly custom bindings need to be discussed properly on the lists :)

> Anyways, the reason why we want these as custom properties in the DT is that
> there are multiple instances of the routers within one SoC. On AM65x we have
> MAIN NAVSS, MCU NAVSS, GPIOs for both etc., if we add driver data for each
> of these, it easily explodes quite a bit. Especially going forward with new
> SoCs.

Yup and I keep getting worried about this every time I see it still. But like
I've mentioned when we chatted offline, as long as Arnd, Marc and Rob ack this
use, I can live with it.

I'm just trying to avoid another "ti,hwmods" type custom property here..
We are still dealing with it to replace it with just standard use of
"compatible" property, and every time Rob sees it he still comments on it :)

Reagrds,

Tony
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
new file mode 100644
index 000000000000..613911013db1
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
@@ -0,0 +1,83 @@ 
+Texas Instruments K3 Interrupt Router
+=====================================
+
+The Interrupt Router (INTR) module provides a mechanism to mux M
+interrupt inputs to N interrupt outputs, where all M inputs are selectable
+to be driven per N output. An Interrupt Router can either handle edge triggered
+or level triggered interrupts and that is fixed in hardware.
+
+                                 Interrupt Router
+                             +----------------------+
+                             |  Inputs     Outputs  |
+        +-------+            | +------+             |
+        | GPIO  |----------->| | irq0 |             |       Host IRQ
+        +-------+            | +------+             |      controller
+                             |    .        +-----+  |      +-------+
+        +-------+            |    .        |  0  |  |----->|  IRQ  |
+        | INTA  |----------->|    .        +-----+  |      +-------+
+        +-------+            |    .          .      |
+                             | +------+      .      |
+                             | | irqM |    +-----+  |
+                             | +------+    |  N  |  |
+                             |             +-----+  |
+                             +----------------------+
+
+There is one register per output (MUXCNTL_N) that controls the selection.
+Configuration of these MUXCNTL_N registers is done by a system controller
+(like the Device Memory and Security Controller on K3 AM654 SoC). System
+controller will keep track of the used and unused registers within the Router.
+Driver should request the system controller to get the range of GIC IRQs
+assigned to the requesting hosts. It is the drivers responsibility to keep
+track of Host IRQs.
+
+Communication between the host processor running an OS and the system
+controller happens through a protocol called TI System Control Interface
+(TISCI protocol). For more details refer:
+Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
+
+TISCI Interrupt Router Node:
+----------------------------
+Required Properties:
+- compatible:		Must be "ti,sci-intr".
+- ti,intr-trigger-type:	Should be 1 if the interrupt router supports edge
+			triggered interrupts, 4 for 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 3.
+			First cell should contain the TISCI device ID of source
+			Second cell should contain the interrupt source offset
+			within the device.
+			Third cell should be 1 if the irq is coming from the
+			interrupt aggregator else 0.
+- 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.
+
+For more details on TISCI IRQ resource management refer:
+http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
+
+Example:
+--------
+The following example demonstrates both interrupt router node and the consumer
+node(main gpio) on the AM654 SoC:
+
+main_intr: interrupt-controller0 {
+	compatible = "ti,sci-intr";
+	ti,intr-trigger-type = <1>;
+	interrupt-controller;
+	interrupt-parent = <&gic500>;
+	#interrupt-cells = <3>;
+	ti,sci = <&dmsc>;
+	ti,sci-dst-id = <56>;
+	ti,sci-rm-range-girq = <0x1>;
+};
+
+main_gpio0: gpio@600000 {
+	...
+	interrupt-parent = <&main_intr>;
+	interrupts = <57 256 0>, <57 257 0>, <57 258 0>,
+		     <57 259 0>, <57 260 0>, <57 261 0>;
+	...
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 2359e12e4c41..9c4e71187ca1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15349,6 +15349,7 @@  F:	Documentation/devicetree/bindings/reset/ti,sci-reset.txt
 F:	Documentation/devicetree/bindings/clock/ti,sci-clk.txt
 F:	drivers/clk/keystone/sci-clk.c
 F:	drivers/reset/reset-ti-sci.c
+F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
 
 Texas Instruments ASoC drivers
 M:	Peter Ujfalusi <peter.ujfalusi@ti.com>