[1/2] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

Message ID 20181006072812.15814-2-lokeshvutla@ti.com
State Changes Requested
Headers show
Series
  • irqchip: ti-sci-intr: Add support for K3 Interrupt Router
Related show

Commit Message

Lokesh Vutla Oct. 6, 2018, 7:28 a.m.
Add the DT binding documentation for Interrupt router driver.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 .../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

Marc Zyngier Oct. 6, 2018, 10:02 a.m. | #1
On Sat, 06 Oct 2018 08:28:11 +0100,
Lokesh Vutla <lokeshvutla@ti.com> wrote:
> 
> Add the DT binding documentation for Interrupt router driver.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  .../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
> 
> 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..681ca53cc5fb
> --- /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. There is one register per output (MUXCNTL_N) that
> +controls the selection.
> +
> +
> +                                 Interrupt Router
> +                             +----------------------+
> +                             |  Inputs     Outputs  |
> +        +-------+            | +------+             |
> +        | GPIO  |----------->| | irq0 |             |       Host IRQ
> +        +-------+            | +------+             |      controller
> +                             |    .        +-----+  |      +-------+
> +        +-------+            |    .        |  0  |  |----->|  GIC  |
> +        | INTA  |----------->|    .        +-----+  |      +-------+
> +        +-------+            |    .          .      |
> +                             | +------+      .      |
> +                             | | irqM |    +-----+  |
> +                             | +------+    |  N  |  |
> +                             |             +-----+  |
> +                             +----------------------+
> +
> +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 GIC IRQs.

I would drop the GIC here, and replace it by "parent interrupt
controller", as nothing here is GIC specific.

> +
> +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:
> +----------------------------
> +- compatible:		Must be "ti,sci-intr".
> +- 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 specifies the trigger type as defined
> +			in interrupts.txt in this directory.

Are all trigger types supported?

> +- interrupt-parent:	phandle of irq parent for TISCI intr. The parent must
> +			use the same interrupt-cells format as GIC.

Why that constraint? From what I can see, the two are fairly
independent, and the constraint looks more of a Linux driver issue
than a DT constraint.

> +- 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:	Tuple corresponding to unique TISCI resource type that
> +			defines the dst host irq ranges assigned to this
> +			interrupt router from this host context.
> +			Tuple should be of format <type subtype>.
> +
> +Example:
> +--------
> +The following example demonstrates both interrupt router node and the consumer
> +node(main gpio) on the AM654 SoC:
> +
> +main_intr: interrupt-controller@1 {
> +	compatible = "ti,sci-intr";
> +	interrupt-controller;
> +	interrupt-parent = <&gic>;
> +	#interrupt-cells = <3>;
> +	ti,sci = <&dmsc>;
> +	ti,sci-dst-id = <56>;
> +	ti,sci-rm-range-girq = <0xb 0x1>;
> +};
> +
> +main_gpio0:  main_gpio0@600000 {
> +	...
> +	interrupt-parent = <&main_intr>;
> +	interrupts = <57 256 IRQ_TYPE_EDGE_RISING>,
> +			<57 257 IRQ_TYPE_EDGE_RISING>,
> +			<57 258 IRQ_TYPE_EDGE_RISING>,
> +			<57 259 IRQ_TYPE_EDGE_RISING>,
> +			<57 260 IRQ_TYPE_EDGE_RISING>,
> +			<57 261 IRQ_TYPE_EDGE_RISING>;
> +	...
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 29c08106bd22..a23778b68d74 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14625,6 +14625,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
>  
>  THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
>  M:	Hans Verkuil <hverkuil@xs4all.nl>
> -- 
> 2.19.0
> 

Thanks,

	M.
Nishanth Menon Oct. 6, 2018, 4:29 p.m. | #2
On 12:58-20181006, Lokesh Vutla wrote:
> Add the DT binding documentation for Interrupt router driver.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  .../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
> 
> 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..681ca53cc5fb
> --- /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. There is one register per output (MUXCNTL_N) that
> +controls the selection.
> +
> +
> +                                 Interrupt Router
> +                             +----------------------+
> +                             |  Inputs     Outputs  |
> +        +-------+            | +------+             |
> +        | GPIO  |----------->| | irq0 |             |       Host IRQ
> +        +-------+            | +------+             |      controller
> +                             |    .        +-----+  |      +-------+
> +        +-------+            |    .        |  0  |  |----->|  GIC  |
> +        | INTA  |----------->|    .        +-----+  |      +-------+
> +        +-------+            |    .          .      |
> +                             | +------+      .      |
> +                             | | irqM |    +-----+  |
> +                             | +------+    |  N  |  |
> +                             |             +-----+  |
> +                             +----------------------+
> +
> +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 GIC 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:
> +----------------------------
> +- compatible:		Must be "ti,sci-intr".
> +- 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 specifies the trigger type as defined
> +			in interrupts.txt in this directory.
> +- interrupt-parent:	phandle of irq parent for TISCI intr. The parent must
> +			use the same interrupt-cells format as GIC.
> +- 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:	Tuple corresponding to unique TISCI resource type that
> +			defines the dst host irq ranges assigned to this
> +			interrupt router from this host context.
> +			Tuple should be of format <type subtype>.
> +

Rob, DT maintainers,

I'd like a feedback from DT maintainers on this 'range' topic.

TISCI Firmware [1] currently seems to define a type corresponding to a
device ID[2]. in AM6 device, for example, this is different, however
have a 1 to 1 correspondence. However, there is expectation that type will
end up as device ID in a future SoC.

While this is subject to much debate internally, I'd like some feedback if this
is OK from Device tree representation - it is true that Firmware does
look at it as type, however in some future SoC, it could be that the
values themselves may correspond one to one with a device id -> The
original wish was that types might be something reusable across SoCs,
but that is turning out to be more of a theoretical wish than any thing
practical.

[1]http://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am6x/resasg_types.html
[2]http://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am6x/devices.html
Lokesh Vutla Oct. 8, 2018, 9:46 a.m. | #3
Hi Marc,

On Saturday 06 October 2018 03:32 PM, Marc Zyngier wrote:
> On Sat, 06 Oct 2018 08:28:11 +0100,
> Lokesh Vutla <lokeshvutla@ti.com> wrote:
>>
>> Add the DT binding documentation for Interrupt router driver.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>   .../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
>>
>> 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..681ca53cc5fb
>> --- /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. There is one register per output (MUXCNTL_N) that
>> +controls the selection.
>> +
>> +
>> +                                 Interrupt Router
>> +                             +----------------------+
>> +                             |  Inputs     Outputs  |
>> +        +-------+            | +------+             |
>> +        | GPIO  |----------->| | irq0 |             |       Host IRQ
>> +        +-------+            | +------+             |      controller
>> +                             |    .        +-----+  |      +-------+
>> +        +-------+            |    .        |  0  |  |----->|  GIC  |
>> +        | INTA  |----------->|    .        +-----+  |      +-------+
>> +        +-------+            |    .          .      |
>> +                             | +------+      .      |
>> +                             | | irqM |    +-----+  |
>> +                             | +------+    |  N  |  |
>> +                             |             +-----+  |
>> +                             +----------------------+
>> +
>> +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 GIC IRQs.
> 
> I would drop the GIC here, and replace it by "parent interrupt
> controller", as nothing here is GIC specific
> 
>> +
>> +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:
>> +----------------------------
>> +- compatible:		Must be "ti,sci-intr".
>> +- 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 specifies the trigger type as defined
>> +			in interrupts.txt in this directory.
> 
> Are all trigger types supported?

Nope, only level interrupts are supported. Will fix it in v2.

> 
>> +- interrupt-parent:	phandle of irq parent for TISCI intr. The parent must
>> +			use the same interrupt-cells format as GIC.
> 
> Why that constraint? From what I can see, the two are fairly
> independent, and the constraint looks more of a Linux driver issue
> than a DT constraint.

Driver when calling irq_domain_alloc_irqs_parent(), the fwspec node that gets 
passed assumes that parent is gic. parameters are filled in with such 
assumption. Do you suggest anything to make it more generic?

> 
>> +- 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:	Tuple corresponding to unique TISCI resource type that
>> +			defines the dst host irq ranges assigned to this
>> +			interrupt router from this host context.
>> +			Tuple should be of format <type subtype>.
>> +

Thanks a lot for the review. Also, I need a suggestion regarding one more 
interrupt controller(Interrupt Aggregator) on the same SoC controlled by 
TISCI_PROTOCOL.

The Interrupt Aggregator (INTA) provides a centralized machine
which handles the termination of system events to that they can
be coherently processed by the host(s) in the system. Integration looks 
something similar https://pastebin.ubuntu.com/p/T32vbrwsch/ .

Configuration of the Intmap registers that maps global events to vint is done
by a system controller (like the Device Memory and Security Controller on K3
AM654 SoC). Driver should request the system controller to get the range
of global events and vints assigned to the requesting host. Management
of these requested resources should be handled by driver and requests
system controller to map specific global event to vint, bit pair.

There can be cases such that IRQ routes can involve both INTR and INTA like below:
	IP ---> INTA ---> INTR ----> GIC.

In these cases TISCI involves only one message with parametes(source id, source 
offset, inta_id, dst id) for configuring IRQ route till the destination. Co 
processor will detect there is INTR in the IRQ path and configure that as well.

Right now I kind of differentiated this scenario in INTA driver by passing a 
flag(TI_SCI_EVENT) to INTR driver. If such flag comes, INTR driver should avoid 
calling ti_sci api for configuring. Do you think this is the right direction or 
do you suggest a better solution.

If I am not clear in the above description, I can post an RFC for INTA driver 
for continuing this discussion.

Thanks and regards,
Lokesh
Marc Zyngier Oct. 8, 2018, 1:55 p.m. | #4
On 08/10/18 10:46, Lokesh Vutla wrote:
> Hi Marc,
> 
> On Saturday 06 October 2018 03:32 PM, Marc Zyngier wrote:
>> On Sat, 06 Oct 2018 08:28:11 +0100,
>> Lokesh Vutla <lokeshvutla@ti.com> wrote:
>>>
>>> Add the DT binding documentation for Interrupt router driver.
>>>
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> ---
>>>    .../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
>>>
>>> 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..681ca53cc5fb
>>> --- /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. There is one register per output (MUXCNTL_N) that
>>> +controls the selection.
>>> +
>>> +
>>> +                                 Interrupt Router
>>> +                             +----------------------+
>>> +                             |  Inputs     Outputs  |
>>> +        +-------+            | +------+             |
>>> +        | GPIO  |----------->| | irq0 |             |       Host IRQ
>>> +        +-------+            | +------+             |      controller
>>> +                             |    .        +-----+  |      +-------+
>>> +        +-------+            |    .        |  0  |  |----->|  GIC  |
>>> +        | INTA  |----------->|    .        +-----+  |      +-------+
>>> +        +-------+            |    .          .      |
>>> +                             | +------+      .      |
>>> +                             | | irqM |    +-----+  |
>>> +                             | +------+    |  N  |  |
>>> +                             |             +-----+  |
>>> +                             +----------------------+
>>> +
>>> +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 GIC IRQs.
>>
>> I would drop the GIC here, and replace it by "parent interrupt
>> controller", as nothing here is GIC specific
>>
>>> +
>>> +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:
>>> +----------------------------
>>> +- compatible:		Must be "ti,sci-intr".
>>> +- 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 specifies the trigger type as defined
>>> +			in interrupts.txt in this directory.
>>
>> Are all trigger types supported?
> 
> Nope, only level interrupts are supported. Will fix it in v2.
> 
>>
>>> +- interrupt-parent:	phandle of irq parent for TISCI intr. The parent must
>>> +			use the same interrupt-cells format as GIC.
>>
>> Why that constraint? From what I can see, the two are fairly
>> independent, and the constraint looks more of a Linux driver issue
>> than a DT constraint.
> 
> Driver when calling irq_domain_alloc_irqs_parent(), the fwspec node that gets
> passed assumes that parent is gic. parameters are filled in with such
> assumption. Do you suggest anything to make it more generic?

As I said, that's a Linux driver issue, not a DT specification at all. 
It is not worth it trying to generalize it in the driver implementation, 
but the DT spec it self should be generic enough.

> 
>>
>>> +- 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:	Tuple corresponding to unique TISCI resource type that
>>> +			defines the dst host irq ranges assigned to this
>>> +			interrupt router from this host context.
>>> +			Tuple should be of format <type subtype>.
>>> +
> 
> Thanks a lot for the review. Also, I need a suggestion regarding one more
> interrupt controller(Interrupt Aggregator) on the same SoC controlled by
> TISCI_PROTOCOL.
> 
> The Interrupt Aggregator (INTA) provides a centralized machine
> which handles the termination of system events to that they can
> be coherently processed by the host(s) in the system. Integration looks
> something similar https://pastebin.ubuntu.com/p/T32vbrwsch/ .
> 
> Configuration of the Intmap registers that maps global events to vint is done
> by a system controller (like the Device Memory and Security Controller on K3
> AM654 SoC). Driver should request the system controller to get the range
> of global events and vints assigned to the requesting host. Management
> of these requested resources should be handled by driver and requests
> system controller to map specific global event to vint, bit pair.

I'm sorry, but I really have no idea what the global events and the 
vints are. Maybe you should describe what this is all about, and maybe 
provide a pointer to some documentation...

> 
> There can be cases such that IRQ routes can involve both INTR and INTA like below:
> 	IP ---> INTA ---> INTR ----> GIC.
> 
> In these cases TISCI involves only one message with parametes(source id, source
> offset, inta_id, dst id) for configuring IRQ route till the destination. Co
> processor will detect there is INTR in the IRQ path and configure that as well.
> 
> Right now I kind of differentiated this scenario in INTA driver by passing a
> flag(TI_SCI_EVENT) to INTR driver. If such flag comes, INTR driver should avoid
> calling ti_sci api for configuring. Do you think this is the right direction or
> do you suggest a better solution.

Frankly, it mostly indicates that the firmware does too much, and should 
be more flexible.

> If I am not clear in the above description, I can post an RFC for INTA driver
> for continuing this discussion.

That'd be preferable, IMO. Please provide definitions for all the above 
jargon, as well as pointers to publicly available documentation, if any.

Thanks,

	M.
Lokesh Vutla Oct. 8, 2018, 3:28 p.m. | #5
On Monday 08 October 2018 07:25 PM, Marc Zyngier wrote:
> On 08/10/18 10:46, Lokesh Vutla wrote:
>> Hi Marc,
>>
>> On Saturday 06 October 2018 03:32 PM, Marc Zyngier wrote:
>>> On Sat, 06 Oct 2018 08:28:11 +0100,
>>> Lokesh Vutla <lokeshvutla@ti.com> wrote:
>>>>
>>>> Add the DT binding documentation for Interrupt router driver.
>>>>
>>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>>> ---
>>>>     .../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
>>>>
>>>> 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..681ca53cc5fb
>>>> --- /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. There is one register per output (MUXCNTL_N) that
>>>> +controls the selection.
>>>> +
>>>> +
>>>> +                                 Interrupt Router
>>>> +                             +----------------------+
>>>> +                             |  Inputs     Outputs  |
>>>> +        +-------+            | +------+             |
>>>> +        | GPIO  |----------->| | irq0 |             |       Host IRQ
>>>> +        +-------+            | +------+             |      controller
>>>> +                             |    .        +-----+  |      +-------+
>>>> +        +-------+            |    .        |  0  |  |----->|  GIC  |
>>>> +        | INTA  |----------->|    .        +-----+  |      +-------+
>>>> +        +-------+            |    .          .      |
>>>> +                             | +------+      .      |
>>>> +                             | | irqM |    +-----+  |
>>>> +                             | +------+    |  N  |  |
>>>> +                             |             +-----+  |
>>>> +                             +----------------------+
>>>> +
>>>> +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 GIC IRQs.
>>>
>>> I would drop the GIC here, and replace it by "parent interrupt
>>> controller", as nothing here is GIC specific
>>>
>>>> +
>>>> +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:
>>>> +----------------------------
>>>> +- compatible:		Must be "ti,sci-intr".
>>>> +- 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 specifies the trigger type as defined
>>>> +			in interrupts.txt in this directory.
>>>
>>> Are all trigger types supported?
>>
>> Nope, only level interrupts are supported. Will fix it in v2.
>>
>>>
>>>> +- interrupt-parent:	phandle of irq parent for TISCI intr. The parent must
>>>> +			use the same interrupt-cells format as GIC.
>>>
>>> Why that constraint? From what I can see, the two are fairly
>>> independent, and the constraint looks more of a Linux driver issue
>>> than a DT constraint.
>>
>> Driver when calling irq_domain_alloc_irqs_parent(), the fwspec node that gets
>> passed assumes that parent is gic. parameters are filled in with such
>> assumption. Do you suggest anything to make it more generic?
> 
> As I said, that's a Linux driver issue, not a DT specification at all.
> It is not worth it trying to generalize it in the driver implementation,
> but the DT spec it self should be generic enough.

okay, will fix it in next version.

> 
>>
>>>
>>>> +- 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:	Tuple corresponding to unique TISCI resource type that
>>>> +			defines the dst host irq ranges assigned to this
>>>> +			interrupt router from this host context.
>>>> +			Tuple should be of format <type subtype>.
>>>> +
>>
>> Thanks a lot for the review. Also, I need a suggestion regarding one more
>> interrupt controller(Interrupt Aggregator) on the same SoC controlled by
>> TISCI_PROTOCOL.
>>
>> The Interrupt Aggregator (INTA) provides a centralized machine
>> which handles the termination of system events to that they can
>> be coherently processed by the host(s) in the system. Integration looks
>> something similar https://pastebin.ubuntu.com/p/T32vbrwsch/ .
>>
>> Configuration of the Intmap registers that maps global events to vint is done
>> by a system controller (like the Device Memory and Security Controller on K3
>> AM654 SoC). Driver should request the system controller to get the range
>> of global events and vints assigned to the requesting host. Management
>> of these requested resources should be handled by driver and requests
>> system controller to map specific global event to vint, bit pair.
> 
> I'm sorry, but I really have no idea what the global events and the
> vints are. Maybe you should describe what this is all about, and maybe
> provide a pointer to some documentation...

Sorry I should have done that earlier. TRM is available here[1], Section 9.3 
talks about Interrupt Router, Section 10.2.7 talks about Interrupt aggregator. 
Documentation for TISCI IRQ management is available here[2].


[1] http://www.ti.com/lit/ug/spruid7a/spruid7a.pdf
[2] http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html

> 
>>
>> There can be cases such that IRQ routes can involve both INTR and INTA like below:
>> 	IP ---> INTA ---> INTR ----> GIC.
>>
>> In these cases TISCI involves only one message with parametes(source id, source
>> offset, inta_id, dst id) for configuring IRQ route till the destination. Co
>> processor will detect there is INTR in the IRQ path and configure that as well.
>>
>> Right now I kind of differentiated this scenario in INTA driver by passing a
>> flag(TI_SCI_EVENT) to INTR driver. If such flag comes, INTR driver should avoid
>> calling ti_sci api for configuring. Do you think this is the right direction or
>> do you suggest a better solution.
> 
> Frankly, it mostly indicates that the firmware does too much, and should
> be more flexible.
> 
>> If I am not clear in the above description, I can post an RFC for INTA driver
>> for continuing this discussion.
> 
> That'd be preferable, IMO. Please provide definitions for all the above

Sure, will try to post the consolidated series asap. Thanks a lot for the help.

Regards,
Lokesh
Lokesh Vutla Oct. 15, 2018, 11:05 a.m. | #6
Hi Rob, DT maintainers,

On Saturday 06 October 2018 09:59 PM, Nishanth Menon wrote:
> On 12:58-20181006, Lokesh Vutla wrote:
>> Add the DT binding documentation for Interrupt router driver.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>   .../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
>>
>> 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..681ca53cc5fb
>> --- /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. There is one register per output (MUXCNTL_N) that
>> +controls the selection.
>> +
>> +
>> +                                 Interrupt Router
>> +                             +----------------------+
>> +                             |  Inputs     Outputs  |
>> +        +-------+            | +------+             |
>> +        | GPIO  |----------->| | irq0 |             |       Host IRQ
>> +        +-------+            | +------+             |      controller
>> +                             |    .        +-----+  |      +-------+
>> +        +-------+            |    .        |  0  |  |----->|  GIC  |
>> +        | INTA  |----------->|    .        +-----+  |      +-------+
>> +        +-------+            |    .          .      |
>> +                             | +------+      .      |
>> +                             | | irqM |    +-----+  |
>> +                             | +------+    |  N  |  |
>> +                             |             +-----+  |
>> +                             +----------------------+
>> +
>> +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 GIC 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:
>> +----------------------------
>> +- compatible:		Must be "ti,sci-intr".
>> +- 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 specifies the trigger type as defined
>> +			in interrupts.txt in this directory.
>> +- interrupt-parent:	phandle of irq parent for TISCI intr. The parent must
>> +			use the same interrupt-cells format as GIC.
>> +- 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:	Tuple corresponding to unique TISCI resource type that
>> +			defines the dst host irq ranges assigned to this
>> +			interrupt router from this host context.
>> +			Tuple should be of format <type subtype>.
>> +
> 
> Rob, DT maintainers,
> 
> I'd like a feedback from DT maintainers on this 'range' topic.
> 
> TISCI Firmware [1] currently seems to define a type corresponding to a
> device ID[2]. in AM6 device, for example, this is different, however
> have a 1 to 1 correspondence. However, there is expectation that type will
> end up as device ID in a future SoC.
> 
> While this is subject to much debate internally, I'd like some feedback if this
> is OK from Device tree representation - it is true that Firmware does
> look at it as type, however in some future SoC, it could be that the
> values themselves may correspond one to one with a device id -> The
> original wish was that types might be something reusable across SoCs,
> but that is turning out to be more of a theoretical wish than any thing
> practical.
> 
> [1]http://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am6x/resasg_types.html
> [2]http://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am6x/devices.html

Any help on this topic?

Thanks and regards,
Lokesh

>
Rob Herring Oct. 17, 2018, 1:49 p.m. | #7
On Sat, Oct 06, 2018 at 11:29:29AM -0500, Nishanth Menon wrote:
> On 12:58-20181006, Lokesh Vutla wrote:
> > Add the DT binding documentation for Interrupt router driver.
> > 
> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> > ---
> >  .../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
> > 
> > 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..681ca53cc5fb
> > --- /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. There is one register per output (MUXCNTL_N) that
> > +controls the selection.
> > +
> > +
> > +                                 Interrupt Router
> > +                             +----------------------+
> > +                             |  Inputs     Outputs  |
> > +        +-------+            | +------+             |
> > +        | GPIO  |----------->| | irq0 |             |       Host IRQ
> > +        +-------+            | +------+             |      controller
> > +                             |    .        +-----+  |      +-------+
> > +        +-------+            |    .        |  0  |  |----->|  GIC  |
> > +        | INTA  |----------->|    .        +-----+  |      +-------+
> > +        +-------+            |    .          .      |
> > +                             | +------+      .      |
> > +                             | | irqM |    +-----+  |
> > +                             | +------+    |  N  |  |
> > +                             |             +-----+  |
> > +                             +----------------------+
> > +
> > +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 GIC 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:
> > +----------------------------
> > +- compatible:		Must be "ti,sci-intr".
> > +- 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 specifies the trigger type as defined
> > +			in interrupts.txt in this directory.
> > +- interrupt-parent:	phandle of irq parent for TISCI intr. The parent must
> > +			use the same interrupt-cells format as GIC.
> > +- 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:	Tuple corresponding to unique TISCI resource type that
> > +			defines the dst host irq ranges assigned to this
> > +			interrupt router from this host context.
> > +			Tuple should be of format <type subtype>.
> > +
> 
> Rob, DT maintainers,
> 
> I'd like a feedback from DT maintainers on this 'range' topic.
> 
> TISCI Firmware [1] currently seems to define a type corresponding to a
> device ID[2]. in AM6 device, for example, this is different, however
> have a 1 to 1 correspondence. However, there is expectation that type will
> end up as device ID in a future SoC.
> 
> While this is subject to much debate internally, I'd like some feedback if this
> is OK from Device tree representation - it is true that Firmware does
> look at it as type, however in some future SoC, it could be that the
> values themselves may correspond one to one with a device id -> The
> original wish was that types might be something reusable across SoCs,
> but that is turning out to be more of a theoretical wish than any thing
> practical.

I'm not sure I follow all the terminology here of type, subtype, "dst 
host irq", etc. It looks to me like you should be using interrupt-map property.

Rob

> 
> [1]http://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am6x/resasg_types.html
> [2]http://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am6x/devices.html
> -- 
> Regards,
> Nishanth Menon

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..681ca53cc5fb
--- /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. There is one register per output (MUXCNTL_N) that
+controls the selection.
+
+
+                                 Interrupt Router
+                             +----------------------+
+                             |  Inputs     Outputs  |
+        +-------+            | +------+             |
+        | GPIO  |----------->| | irq0 |             |       Host IRQ
+        +-------+            | +------+             |      controller
+                             |    .        +-----+  |      +-------+
+        +-------+            |    .        |  0  |  |----->|  GIC  |
+        | INTA  |----------->|    .        +-----+  |      +-------+
+        +-------+            |    .          .      |
+                             | +------+      .      |
+                             | | irqM |    +-----+  |
+                             | +------+    |  N  |  |
+                             |             +-----+  |
+                             +----------------------+
+
+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 GIC 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:
+----------------------------
+- compatible:		Must be "ti,sci-intr".
+- 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 specifies the trigger type as defined
+			in interrupts.txt in this directory.
+- interrupt-parent:	phandle of irq parent for TISCI intr. The parent must
+			use the same interrupt-cells format as GIC.
+- 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:	Tuple corresponding to unique TISCI resource type that
+			defines the dst host irq ranges assigned to this
+			interrupt router from this host context.
+			Tuple should be of format <type subtype>.
+
+Example:
+--------
+The following example demonstrates both interrupt router node and the consumer
+node(main gpio) on the AM654 SoC:
+
+main_intr: interrupt-controller@1 {
+	compatible = "ti,sci-intr";
+	interrupt-controller;
+	interrupt-parent = <&gic>;
+	#interrupt-cells = <3>;
+	ti,sci = <&dmsc>;
+	ti,sci-dst-id = <56>;
+	ti,sci-rm-range-girq = <0xb 0x1>;
+};
+
+main_gpio0:  main_gpio0@600000 {
+	...
+	interrupt-parent = <&main_intr>;
+	interrupts = <57 256 IRQ_TYPE_EDGE_RISING>,
+			<57 257 IRQ_TYPE_EDGE_RISING>,
+			<57 258 IRQ_TYPE_EDGE_RISING>,
+			<57 259 IRQ_TYPE_EDGE_RISING>,
+			<57 260 IRQ_TYPE_EDGE_RISING>,
+			<57 261 IRQ_TYPE_EDGE_RISING>;
+	...
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 29c08106bd22..a23778b68d74 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14625,6 +14625,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
 
 THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
 M:	Hans Verkuil <hverkuil@xs4all.nl>