Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
On 18/02/2019 16.35, Tony Lindgren wrote: > * Lokesh Vutla [190216 03:30]: >> On 2/12/2019 1:12 PM, Lokesh Vutla wrote: >>> +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 4. >>> + 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. >>> + Fourth cell should be 1 if the irq is coming from >>> + 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. >> >> Please help me here. As said this is the TISCI device id for the host >> interrupt controller. While sending message to the system co-processor >> this ID needs to be specified so that the irq route gets discovered and >> configured. Atleast with the current design device Ids are not >> discoverable. Can you mention what can be improved here? Is there any >> such example where a firmware supports querying the deivce ids? >> >> Also do you have any further comments on this patch? > > No reg property above. So if the interrupt router is not accessible > by Linux like you're saying, you should not set up a dts node for > it at all. It is accessible via tisci but no direct register access. > > Regards, > > Tony > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Tony, On 2/20/2019 10:06 PM, Tony Lindgren wrote: > Hi, > > Some more info on chained irq vs mux below that might > help. > > * Tony Lindgren [190219 15:36]: >> * Lokesh Vutla [190219 08:51]: >>> With this can you tell me how can we not have a device-tree and still >>> support >>> irq allocation? >> >> Using standard dts reg property to differentiate the interrupt >> router instances. And if the interrupt router is a mux, you should >> treat it as a mux rather than a chained interrupt controller. >> >> We do have drivers/mux nowadays, not sure if it helps in this case >> as at least timer interrupts need to be configured very early. > > Adding Linus Walleij to Cc since he posted a good test to > consider if something should use chained (or nested) irq: > > "individual masking and ACKing bits and can all be used at the > same time" [0] Interrupt Router just routes M inputs to N outputs. One input can only be mapped to one output. This is a clear case of a hierarchical domain and the driver is implementing it. Thanks and regards, Lokesh > > Not sure if we have that documented somewhere? > > But seems like the interrupt router should be set up as > a separate mux driver talking with firmware that the > interrupt controller driver calls on request_irq(> > Cheers, > > Tony > > > [0] https://marc.info/?l=linux-omap&m=155065629529311&w=2 >
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi, Some more info on chained irq vs mux below that might help. * Tony Lindgren [190219 15:36]: > * Lokesh Vutla [190219 08:51]: > > With this can you tell me how can we not have a device-tree and still > > support > > irq allocation? > > Using standard dts reg property to differentiate the interrupt > router instances. And if the interrupt router is a mux, you should > treat it as a mux rather than a chained interrupt controller. > > We do have drivers/mux nowadays, not sure if it helps in this case > as at least timer interrupts need to be configured very early. Adding Linus Walleij to Cc since he posted a good test to consider if something should use chained (or nested) irq: "individual masking and ACKing bits and can all be used at the same time" [0] Not sure if we have that documented somewhere? But seems like the interrupt router should be set up as a separate mux driver talking with firmware that the interrupt controller driver calls on request_irq()? Cheers, Tony [0] https://marc.info/?l=linux-omap&m=155065629529311&w=2
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Tony, On 19/02/19 11:26 PM, Tony Lindgren wrote: > * Tony Lindgren [190219 17:11]: >> * Lokesh Vutla [190219 16:19]: >>> yes. How different is this from any of the above mentioned drivers using >>> firmware specific ids. Like sci pm domain[1] driver utilizes the same >>> device id for enabling any device in the system. Similarly clock >>> driver[2] uses the same device ids and clock ids specified by firmware. >>> There are more which similarly represents firmware ids from DT. >>> >>> [1] Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>> [2] Documentation/devicetree/bindings/clock/ti,sci-clk.txt >> >> That's horrible. We really must not use any firmware invented >> numbers in the device as they do not describe hardware. > > No firmware invented numbers in the device tree I mean naturally. > Drivers do whatever they need to do to deal with the firmware. Let's look at these similar other examples available inside Linux: 1: ./Documentation/devicetree/bindings/arm/arm,scmi.txt mentions the following: - #clock-cells : Should be 1. Contains the Clock ID value used by SCMI commands. - #power-domain-cells : Should be 1. Contains the device or the power domain ID value used by SCMI commands. 2: Documentation/devicetree/bindings/arm/arm,scpi.txt mentions the following: - #power-domain-cells : Should be 1. Contains the device or the power domain ID value used by SCPI commands. 3: Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt the firmware specified identifier are defined in the following header files: include/dt-bindings/clock/tegra186-clock.h include/dt-bindings/power/tegra186-powergate.h include/dt-bindings/reset/tegra186-reset.h 4. Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt mentions the following: "Output clocks are registered based on clock information received from firmware. Output clocks indexes are mentioned in include/dt-bindings/clock/xlnx,zynqmp-clk.h." 5. Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt mentions the following: - #power-domain-cells: Must be 1. Contains the Resource ID used by SCU commands. See detailed Resource ID list from: include/dt-bindings/firmware/imx/rsrc.h - The clock consumer should specify the desired clock by having the clock ID in its "clocks" phandle cell. See the full list of clock IDs from: include/dt-bindings/clock/imx8qxp-clock. 6. Documentation/devicetree/bindings/arm/psci.txt have the following properties: - cpu_suspend : Function ID for CPU_SUSPEND operation - cpu_off : Function ID for CPU_OFF operation - cpu_on: Function ID for CPU_ON operation - migrate : Function ID for MIGRATE operation All the above examples uses the firmware identifiers for devices/clocks or for other functionalities and use them directly in DT. These are all somewhat similar to TI sysfw which runs on a micro-controller and tries to abstract certain functionalities from HLOS. There are many more such examples but I listed only a few users. The feedback you are providing is not going to work for any of the above listed firmware interfaces. Thanks and regards, Lokesh
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Tony Lindgren [190219 17:11]: > * Lokesh Vutla [190219 16:19]: > > yes. How different is this from any of the above mentioned drivers using > > firmware specific ids. Like sci pm domain[1] driver utilizes the same > > device id for enabling any device in the system. Similarly clock > > driver[2] uses the same device ids and clock ids specified by firmware. > > There are more which similarly represents firmware ids from DT. > > > > [1] Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt > > [2] Documentation/devicetree/bindings/clock/ti,sci-clk.txt > > That's horrible. We really must not use any firmware invented > numbers in the device as they do not describe hardware. No firmware invented numbers in the device tree I mean naturally. Drivers do whatever they need to do to deal with the firmware. Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Lokesh Vutla [190219 16:19]: > yes. How different is this from any of the above mentioned drivers using > firmware specific ids. Like sci pm domain[1] driver utilizes the same > device id for enabling any device in the system. Similarly clock > driver[2] uses the same device ids and clock ids specified by firmware. > There are more which similarly represents firmware ids from DT. > > [1] Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt > [2] Documentation/devicetree/bindings/clock/ti,sci-clk.txt That's horrible. We really must not use any firmware invented numbers in the device as they do not describe hardware. Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
On 2/19/2019 9:05 PM, Tony Lindgren wrote:
> * Lokesh Vutla [190219 08:51]:
>> Hi Tony,
>>
>> On 18/02/19 8:02 PM, Tony Lindgren wrote:
>>> * Lokesh Vutla [190216 03:30]:
On 2/15/2019 9:46 PM, Tony Lindgren wrote:
> The dts node for the interrupt controller should describe a
> proper Linux device, that is with reg entries and so on.
You are asking to just keep the compatible property :)
>>>
>>> Right, and then I realized this node is missing the standard
>>> reg entry too. And you're saying the registers are not even
>>> accissible from Linux.
>>>
>>> So based on that IMO you should not even have a device tree
>>> node for it at all. You should just have the interrupt
>>
>> Practically lets look at what all I am adding in the DT node. Below is one
>> such
>> example:
>>
>> main_intr: interrupt-controller0 {
>> compatible = "ti,sci-intr";
>> interrupt-controller;
>> interrupt-parent = <&gic500>;
>> #interrupt-cells = <4>;
>> ti,sci = <&dmsc>;
>> ti,sci-dst-id = <56>;
>> ti,sci-rm-range-girq = <0x1>;
>> };
>>
>> The following 4 properties are required at least for probing, to represent
>> the
>> hierarchy and for interrupt definition:
>> compatible = "ti,sci-intr";
>> interrupt-controller;
>> interrupt-parent = <&gic500>;
>> #interrupt-cells = <4>;
>>
>> The remaining 3 properties represents the TISCI interface. Let's go step by
>> step:
>> * ti,sci = <&dmsc> :This is the phandle to the firmware protocol driver using
>> which the messages are sent
>> * ti,sci-dst-id = <56> : This is the TISCI device ID for the parent
>> controller
>> for which your irqs needs to be connected. As I said this cannot be queried
>> from
>> sysfw and this is the input to the messages that are send to sysfw.
>
> Let's not add anything that does not describe hardware to the device
> tree. This is ID is an invented number used by the firmware.
>
>> * ti,sci-rm-range-girq = <0x1>: This define the ids using which the
>> parent-irq
>> ranges that are allocated to this interrupt router instance can be queried
>> from
>> sysfw.
>> If the above 2 properties are to be added as driver phandle then for every
>> instance of interrupt router in the SoC, a new compatible needs to be
>> created. I
>> don't think this is a desirable solution.
>
> To me it seems that the interrupt router _must_ have proper IO
> configuration registers available to the Linux running SoC.
>
> Are you sure the interrupt route does not have proper IO
> configuration registers available for the Linux running SoC?
>
> If the there are not, I'd be surprised how the SoC is designed :)
>
> So assuming it does, you should just use the standard device tree
> reg property to differentiate between the various interrupt router
> instances. And then you can have the driver talk to the firmware
> in a way where the driver instances are separate even if no IO
> access to these shared registers is done by the Linux running SoC.
>
> But see also the mux comment below.
>
>> With this can you tell me how can we not have a device-tree and still support
>> irq allocation?
>
> Using standard dts reg property to differentiate the interrupt
> router instances. And if the interrupt router is a mux, you should
> treat it as a mux rather than a chained interrupt controller.
>
> We do have drivers/mux nowadays, not sure if it helps in this case
> as at least timer interrupts need to be configured very early.
>
>> Also, this is not the first time a driver based on a firmware is being added.
>> K2g clock, power and reset drivers are based on this where device ids are
>> being
>> passed from consumers. Similarly arm scpi based drivers are also available.
>
> Having drivers communicate with firmware is quite standard.
yes. How different is this from any of the above mentioned drivers using
firmware specific ids. Like sci pm domain[1] driver utilizes the same
device id for enabling any device in the system. Similarly clock
driver[2] uses the same device ids and clock ids specified by firmware.
There are more which similarly represents firmware ids from DT.
[1] Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
[2] Documentation/devicetree/bindings/clock/ti,sci-clk.txt
Thanks and regards,
Lokesh
>
> However, stuffing firmware specific data to the device tree
> does not describe the hardware and must not be done.
>
> Regards,
>
> Tony
>
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Lokesh Vutla [190219 08:51]:
> Hi Tony,
>
> On 18/02/19 8:02 PM, Tony Lindgren wrote:
> > * Lokesh Vutla [190216 03:30]:
> >> On 2/15/2019 9:46 PM, Tony Lindgren wrote:
> >>> The dts node for the interrupt controller should describe a
> >>> proper Linux device, that is with reg entries and so on.
> >>
> >> You are asking to just keep the compatible property :)
> >
> > Right, and then I realized this node is missing the standard
> > reg entry too. And you're saying the registers are not even
> > accissible from Linux.
> >
> > So based on that IMO you should not even have a device tree
> > node for it at all. You should just have the interrupt
>
> Practically lets look at what all I am adding in the DT node. Below is one
> such
> example:
>
> main_intr: interrupt-controller0 {
> compatible = "ti,sci-intr";
> interrupt-controller;
> interrupt-parent = <&gic500>;
> #interrupt-cells = <4>;
> ti,sci = <&dmsc>;
> ti,sci-dst-id = <56>;
> ti,sci-rm-range-girq = <0x1>;
> };
>
> The following 4 properties are required at least for probing, to represent the
> hierarchy and for interrupt definition:
> compatible = "ti,sci-intr";
> interrupt-controller;
> interrupt-parent = <&gic500>;
> #interrupt-cells = <4>;
>
> The remaining 3 properties represents the TISCI interface. Let's go step by
> step:
> * ti,sci = <&dmsc> :This is the phandle to the firmware protocol driver using
> which the messages are sent
> * ti,sci-dst-id = <56> : This is the TISCI device ID for the parent controller
> for which your irqs needs to be connected. As I said this cannot be queried
> from
> sysfw and this is the input to the messages that are send to sysfw.
Let's not add anything that does not describe hardware to the device
tree. This is ID is an invented number used by the firmware.
> * ti,sci-rm-range-girq = <0x1>: This define the ids using which the
> parent-irq
> ranges that are allocated to this interrupt router instance can be queried
> from
> sysfw.
> If the above 2 properties are to be added as driver phandle then for every
> instance of interrupt router in the SoC, a new compatible needs to be
> created. I
> don't think this is a desirable solution.
To me it seems that the interrupt router _must_ have proper IO
configuration registers available to the Linux running SoC.
Are you sure the interrupt route does not have proper IO
configuration registers available for the Linux running SoC?
If the there are not, I'd be surprised how the SoC is designed :)
So assuming it does, you should just use the standard device tree
reg property to differentiate between the various interrupt router
instances. And then you can have the driver talk to the firmware
in a way where the driver instances are separate even if no IO
access to these shared registers is done by the Linux running SoC.
But see also the mux comment below.
> With this can you tell me how can we not have a device-tree and still support
> irq allocation?
Using standard dts reg property to differentiate the interrupt
router instances. And if the interrupt router is a mux, you should
treat it as a mux rather than a chained interrupt controller.
We do have drivers/mux nowadays, not sure if it helps in this case
as at least timer interrupts need to be configured very early.
> Also, this is not the first time a driver based on a firmware is being added.
> K2g clock, power and reset drivers are based on this where device ids are
> being
> passed from consumers. Similarly arm scpi based drivers are also available.
Having drivers communicate with firmware is quite standard.
However, stuffing firmware specific data to the device tree
does not describe the hardware and must not be done.
Regards,
Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Tony,
On 18/02/19 8:02 PM, Tony Lindgren wrote:
> * Lokesh Vutla [190216 03:30]:
>> On 2/15/2019 9:46 PM, Tony Lindgren wrote:
>>> The dts node for the interrupt controller should describe a
>>> proper Linux device, that is with reg entries and so on.
>>
>> You are asking to just keep the compatible property :)
>
> Right, and then I realized this node is missing the standard
> reg entry too. And you're saying the registers are not even
> accissible from Linux.
>
> So based on that IMO you should not even have a device tree
> node for it at all. You should just have the interrupt
Practically lets look at what all I am adding in the DT node. Below is one such
example:
main_intr: interrupt-controller0 {
compatible = "ti,sci-intr";
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <4>;
ti,sci = <&dmsc>;
ti,sci-dst-id = <56>;
ti,sci-rm-range-girq = <0x1>;
};
The following 4 properties are required at least for probing, to represent the
hierarchy and for interrupt definition:
compatible = "ti,sci-intr";
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <4>;
The remaining 3 properties represents the TISCI interface. Let's go step by
step:
* ti,sci = <&dmsc> :This is the phandle to the firmware protocol driver using
which the messages are sent
* ti,sci-dst-id = <56> : This is the TISCI device ID for the parent controller
for which your irqs needs to be connected. As I said this cannot be queried from
sysfw and this is the input to the messages that are send to sysfw.
* ti,sci-rm-range-girq = <0x1>: This define the ids using which the parent-irq
ranges that are allocated to this interrupt router instance can be queried from
sysfw.
If the above 2 properties are to be added as driver phandle then for every
instance of interrupt router in the SoC, a new compatible needs to be created. I
don't think this is a desirable solution.
With this can you tell me how can we not have a device-tree and still support
irq allocation?
Also, this is not the first time a driver based on a firmware is being added.
K2g clock, power and reset drivers are based on this where device ids are being
passed from consumers. Similarly arm scpi based drivers are also available.
Thanks and regards,
Lokesh
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Marc, On 18/02/19 8:42 PM, Marc Zyngier wrote: > On Tue, 12 Feb 2019 13:12:32 +0530 > Lokesh Vutla wrote: > >> Add the DT binding documentation for Interrupt router driver. >> >> Signed-off-by: Lokesh Vutla >> --- >> Changes since v4: >> - None >> >> .../interrupt-controller/ti,sci-intr.txt | 85 +++ >> MAINTAINERS | 1 + >> 2 files changed, 86 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 ..4b0ca797fda1 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt >> @@ -0,0 +1,85 @@ >> +Texas Instruments K3 Interrupt Router >> += >> + >> +The Interrupt Router (INTR) module provides a mechanism to route 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 | |->| IRQ | >> +| 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 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: >> + >> +- 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 4. >> +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. >> +Fourth cell should be 1 if the irq is coming from >> +interrupt aggregator else 0. > > This is odd. Doesn't the aggregator have a device ID too, which could > be used to discriminate between the two? For that we have to store the list of INTA device IDs connected to the INTR in the router driver. Again we have to get this list from DT. I felt this is easier to differentiate the INTA interrupts. If you prefer to get the list of ids and store it in INTR driver, I can change that by adding an extra DT property. I guess you assumed that there is a single INTA attached to an INTR. There are cases where there are more than one INTA connected to INTR. We will have to handle that as well. Thanks and regards, Lokesh
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
On Tue, 12 Feb 2019 13:12:32 +0530 Lokesh Vutla wrote: > Add the DT binding documentation for Interrupt router driver. > > Signed-off-by: Lokesh Vutla > --- > Changes since v4: > - None > > .../interrupt-controller/ti,sci-intr.txt | 85 +++ > MAINTAINERS | 1 + > 2 files changed, 86 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 ..4b0ca797fda1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt > @@ -0,0 +1,85 @@ > +Texas Instruments K3 Interrupt Router > += > + > +The Interrupt Router (INTR) module provides a mechanism to route 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 | |->| IRQ | > +| 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 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: > + > +- 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 4. > + 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. > + Fourth cell should be 1 if the irq is coming from > + interrupt aggregator else 0. This is odd. Doesn't the aggregator have a device ID too, which could be used to discriminate between the two? Thanks, M. -- Without deviation from the norm, progress is not possible.
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Lokesh Vutla [190216 03:30]: > On 2/12/2019 1:12 PM, Lokesh Vutla wrote: > > +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 4. > > + 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. > > + Fourth cell should be 1 if the irq is coming from > > + 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. > > Please help me here. As said this is the TISCI device id for the host > interrupt controller. While sending message to the system co-processor > this ID needs to be specified so that the irq route gets discovered and > configured. Atleast with the current design device Ids are not > discoverable. Can you mention what can be improved here? Is there any > such example where a firmware supports querying the deivce ids? > > Also do you have any further comments on this patch? No reg property above. So if the interrupt router is not accessible by Linux like you're saying, you should not set up a dts node for it at all. Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Lokesh Vutla [190216 03:30]: > On 2/15/2019 9:46 PM, Tony Lindgren wrote: > > The dts node for the interrupt controller should describe a > > proper Linux device, that is with reg entries and so on. > > You are asking to just keep the compatible property :) Right, and then I realized this node is missing the standard reg entry too. And you're saying the registers are not even accissible from Linux. So based on that IMO you should not even have a device tree node for it at all. You should just have the interrupt controller driver do the muxing on request_irq() using tisci calls. If that's not true, and these mux registers are accessible from Linux, then set up proper dts node with reg entries. And have the driver deal with the firmware based on the compatible node. Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Rob,
On 2/12/2019 1:12 PM, Lokesh Vutla wrote:
> Add the DT binding documentation for Interrupt router driver.
>
> Signed-off-by: Lokesh Vutla
> ---
> Changes since v4:
> - None
>
> .../interrupt-controller/ti,sci-intr.txt | 85 +++
> MAINTAINERS | 1 +
> 2 files changed, 86 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 ..4b0ca797fda1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> @@ -0,0 +1,85 @@
> +Texas Instruments K3 Interrupt Router
> +=
> +
> +The Interrupt Router (INTR) module provides a mechanism to route 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 | |->| IRQ |
> +| 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 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:
> +
> +- 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 4.
> + 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.
> + Fourth cell should be 1 if the irq is coming from
> + 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.
Please help me here. As said this is the TISCI device id for the host
interrupt controller. While sending message to the system co-processor
this ID needs to be specified so that the irq route gets discovered and
configured. Atleast with the current design device Ids are not
discoverable. Can you mention what can be improved here? Is there any
such example where a firmware supports querying the deivce ids?
Also do you have any further comments on this patch?
Thanks and regards,
Lokesh
> +- 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";
> + interrupt-controller;
> + interrupt-parent = <&gic500>;
> + #interrupt-cells = <4>;
> + ti,sci = <&dmsc>;
> + ti,sci-dst-id = <56>;
> + ti,sci-rm-range-girq = <0x1>;
> +};
> +
> +main_gpio0
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Tony, On 2/15/2019 9:46 PM, Tony Lindgren wrote: > Hi, > > * Lokesh Vutla [190214 18:03]: >> On 2/14/2019 11:16 PM, Tony Lindgren wrote: >>> But I'd rather have a proper hardware based phandle + index >>> type mapping in the dts if possible though. >> >> The idea about sysfw here is that Linux is not aware of anything about >> this device(Interrupt Router). It cannot even access any of its >> registers. As a user Linux should know who is the parent to which the >> Interrut router output should be configured. Then query sysfw about the >> range of gic irqs allocated to it. Now for configuration, Linux should >> pass the the input to interrupt router, gic irq no, and gic id(by which >> sysfw uniquely identifies GIC interrupt controller with the SoC). Based >> on these parameters Interrupt Router registers gets configured. > > If the interrupt router hardawre is hidden away from Linux, > just leave it out of the device tree completely and have the > interrupt controller driver request the routing. Yes while requesting you should at-least specify which is your destination interrupt-controller Else how does the sysfw even know to whom the requester wants the routing to happen to. You do know that we are dealing with a heterogeneous system where there are more the one destination interrupt controllers(GIC, R5 VIM etc etc..). This is what the DT property is specifying and we cannot query a device based on a name. > > The dts node for the interrupt controller should describe a > proper Linux device, that is with reg entries and so on. You are asking to just keep the compatible property :) I am no where denying that. But the cases where the firmware does the configuration DT spec[1] clearly mentions about the interface. Please take a look at arm-psci devicetree binding documentation where the function ids are represented using which each psci function is invoked. [1] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.2/devicetree-specification-v0.2.pdf Thanks and regards, Lokesh
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi, * Lokesh Vutla [190214 18:03]: > On 2/14/2019 11:16 PM, Tony Lindgren wrote: > > But I'd rather have a proper hardware based phandle + index > > type mapping in the dts if possible though. > > The idea about sysfw here is that Linux is not aware of anything about > this device(Interrupt Router). It cannot even access any of its > registers. As a user Linux should know who is the parent to which the > Interrut router output should be configured. Then query sysfw about the > range of gic irqs allocated to it. Now for configuration, Linux should > pass the the input to interrupt router, gic irq no, and gic id(by which > sysfw uniquely identifies GIC interrupt controller with the SoC). Based > on these parameters Interrupt Router registers gets configured. If the interrupt router hardawre is hidden away from Linux, just leave it out of the device tree completely and have the interrupt controller driver request the routing. The dts node for the interrupt controller should describe a proper Linux device, that is with reg entries and so on. Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Tony, On 2/14/2019 11:16 PM, Tony Lindgren wrote: > * Lokesh Vutla [190214 17:32]: >> Hi Tony, >> Please do not snip the on going discussion. >> >> On 2/14/2019 9:11 PM, Tony Lindgren wrote: >>> * Lokesh Vutla [190214 08:39]: IMHO, device ids are something which can be used in DT. There are many other things like the interrupt ranges etc.. which are discoverable from sysfw and we are implementing it. >>> >>> We need to describe hardware in the device tree, not firmware. >>> >>> If you have something discoverable from the firmware, you should >>> have the device driver query it from sysfw based on a hardware >>> property, not based on some invented enumeration in the firmware. >> >> Yes we are already querying sysfw for all the irq ranges that can be >> discoverable. The topic of discussion here is about the parent interrupt >> controller id. I am not sure how you are expecting an id be discoverable >> from system firmware especially with a name. > > Well names are quite standard in dts (but should be used with > the phandle + offset). Think for example interrupt-names and > reg-names :) > >>> If there is some device to firmware translation needed, hide that >>> into the device driver and keep it out of the device tree. >> >> If preferred this can be moved to of_match_data attached to each >> compatible property. Then for each SoC a new compatible needs to be created. > > Hiding the ID into the device driver and compatible property > makes sense to me if the id is based on SoC + firmware. > > But I'd rather have a proper hardware based phandle + index > type mapping in the dts if possible though. The idea about sysfw here is that Linux is not aware of anything about this device(Interrupt Router). It cannot even access any of its registers. As a user Linux should know who is the parent to which the Interrut router output should be configured. Then query sysfw about the range of gic irqs allocated to it. Now for configuration, Linux should pass the the input to interrupt router, gic irq no, and gic id(by which sysfw uniquely identifies GIC interrupt controller with the SoC). Based on these parameters Interrupt Router registers gets configured. So for the above configuration we need the gic_id for which the dt property "ti,sci-dst-id" is used. Thanks and regards, Lokesh > > What does this id really consist of? > > Regards, > > Tony >
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Lokesh Vutla [190214 17:32]: > Hi Tony, > Please do not snip the on going discussion. > > On 2/14/2019 9:11 PM, Tony Lindgren wrote: > > * Lokesh Vutla [190214 08:39]: > >> IMHO, device ids are something which can be used in DT. There are many > >> other > >> things like the interrupt ranges etc.. which are discoverable from sysfw > >> and we > >> are implementing it. > > > > We need to describe hardware in the device tree, not firmware. > > > > If you have something discoverable from the firmware, you should > > have the device driver query it from sysfw based on a hardware > > property, not based on some invented enumeration in the firmware. > > Yes we are already querying sysfw for all the irq ranges that can be > discoverable. The topic of discussion here is about the parent interrupt > controller id. I am not sure how you are expecting an id be discoverable > from system firmware especially with a name. Well names are quite standard in dts (but should be used with the phandle + offset). Think for example interrupt-names and reg-names :) > > If there is some device to firmware translation needed, hide that > > into the device driver and keep it out of the device tree. > > If preferred this can be moved to of_match_data attached to each > compatible property. Then for each SoC a new compatible needs to be created. Hiding the ID into the device driver and compatible property makes sense to me if the id is based on SoC + firmware. But I'd rather have a proper hardware based phandle + index type mapping in the dts if possible though. What does this id really consist of? Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Tony, Please do not snip the on going discussion. On 2/14/2019 9:11 PM, Tony Lindgren wrote: > * Lokesh Vutla [190214 08:39]: >> IMHO, device ids are something which can be used in DT. There are many other >> things like the interrupt ranges etc.. which are discoverable from sysfw and >> we >> are implementing it. > > We need to describe hardware in the device tree, not firmware. > > If you have something discoverable from the firmware, you should > have the device driver query it from sysfw based on a hardware > property, not based on some invented enumeration in the firmware. Yes we are already querying sysfw for all the irq ranges that can be discoverable. The topic of discussion here is about the parent interrupt controller id. I am not sure how you are expecting an id be discoverable from system firmware especially with a name. > If there is some device to firmware translation needed, hide that > into the device driver and keep it out of the device tree. If preferred this can be moved to of_match_data attached to each compatible property. Then for each SoC a new compatible needs to be created. > > For example, look at the interrupt binding where the interrupt > is phandle to the controller and the bit offset from the interrupt > controller instance. > > You need to use device IO address + bit offset (or register > offset) type indexing for device tree here. Something out of > the TRM that makes sense to developers. > [1] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.2/devicetree-specification-v0.2.pdf Thanks and regards, Lokesh
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Lokesh Vutla [190214 08:40]: > On 13/02/19 9:02 PM, Tony Lindgren wrote: > > OK so maybe update the description along those lines saying it's > > a shared piece of hardware between various independent SoC > > clusters which may or may not be running Linux. > > IMHO, SoC integration is out of scope of this document. If you insist I can > add > the details. Certainly you need to justify how come such a critical piece of information as interrupt line routing is out of control of Linux here. Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Lokesh Vutla [190214 08:39]: > IMHO, device ids are something which can be used in DT. There are many other > things like the interrupt ranges etc.. which are discoverable from sysfw and > we > are implementing it. We need to describe hardware in the device tree, not firmware. If you have something discoverable from the firmware, you should have the device driver query it from sysfw based on a hardware property, not based on some invented enumeration in the firmware. If there is some device to firmware translation needed, hide that into the device driver and keep it out of the device tree. For example, look at the interrupt binding where the interrupt is phandle to the controller and the bit offset from the interrupt controller instance. You need to use device IO address + bit offset (or register offset) type indexing for device tree here. Something out of the TRM that makes sense to developers. Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
On 13/02/19 9:02 PM, Tony Lindgren wrote: > * Lokesh Vutla [190213 04:23]: >> Hi Tony, >> >> On 12/02/19 10:00 PM, Tony Lindgren wrote: >>> Hi, >>> >>> * Lokesh Vutla [190212 07:43]: +The Interrupt Router (INTR) module provides a mechanism to route 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 | |->| IRQ | +| INTA |--->|.+-+ | +---+ ++---+|. . | + | +--+ . | + | | irqM |+-+ | + | +--+| N | | + | +-+ | + +--+ >>> >>> Is this always one-to-one mapping or can the same interrupt be routed to >>> multiple targets like to the SoC and some coprocessor? >> >> Yes, it is always one-to-one. Output of INTR can only be attached to one of >> the >> processor. > > OK > +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 >>> >>> Care to describe a bit why the interrupts need to be routed by a system >>> controller? >> >> K3 architecture defines a heterogeneous system where multiple heterogeneous >> cores are serving its own usecases. Given that there are multiple ways in >> which >> a device IRQ can be routed using INTR, like either it can be routed to HLOS >> core(A53 int this case) or it can be routed to any other coprocessor >> available >> in the system(like R5). If every sw running in each co-processor is allowed >> to >> program this INTR then there is a high probability that one sw executing on >> one >> core can damage other heterogeneous core. Mainly to avoid this damage the >> configuration of all the INTRs and INTAs are done in a centralized >> place(sysfw). >> Any user for programming its IRQ route should send a message to sysfw with >> the >> parameters. These parameters are policed by sysfw and does the configuration. > > OK so maybe update the description along those lines saying it's > a shared piece of hardware between various independent SoC > clusters which may or may not be running Linux. IMHO, SoC integration is out of scope of this document. If you insist I can add the details. Thanks and regards, Lokesh
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Tony,
On 13/02/19 8:56 PM, Tony Lindgren wrote:
> * Lokesh Vutla [190213 04:26]:
>> Hi Tony,
>>
>> On 12/02/19 9:52 PM, Tony Lindgren wrote:
>>> Hi,
>>>
>>> * Lokesh Vutla [190212 07:43]:
+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"; +
interrupt-controller; +interrupt-parent = <&gic500>; +
#interrupt-cells = <4>; + ti,sci = <&dmsc>; + ti,sci-dst-id = <56>; +
ti,sci-rm-range-girq = <0x1>; +};
>>>
>>> Can you describe a bit what the "ti,sci-dst-id" is above?
>>>
>>> These IDs seem to be listed at at [0] below, but is it really a property
>>> of the hardware? Or is it some enumeration of SoC devices in the
>>> firmware?
>>
>> This is the way that sysfw describes the hardware. In this case it is GIC
>> and it is identified by this ID.
>
> If this ID is an enumeration in the sysfw rather than an actual hardware
> property it should not be in the device tree. If so,
Devicetree-specification-v0.2[1] "Section 1.1 Purpose and Scope" mentions that
devicetree specification provides a complete boot program to client program
interface definition. Where boot program here is the sysfw and client program is
Linux. In this case we are describing the id which is the destination interrupt
controller to which the irqs are supposed to be attached.
> the device driver should request the id from the sysfw based on a name. That
> is, if no struct device or device phandle can
>From a scalability perspective using a name to get a device id might worsen
things. There are hundreds of devices within the SoC and standardizing a name
for each device and making sure using the same name across all future SoCs might
be a bit pain. If there are more than one instance of the same device then name
that should be requested is different with in the same driver.
IMHO, device ids are something which can be used in DT. There are many other
things like the interrupt ranges etc.. which are discoverable from sysfw and we
are implementing it.
> be used.
> > The problem with using enumeration in the dts is that it requires
> maintaining the dts, driver(s) and possibly firmware in sync. And that might
The idea here is device ids are not going to change for a SoC. For new SoCs
within the same architecture ids will change and we will have new dts or this
new SoC.
> change between SoCs variants when new devices get added and removed.
>
[1]
https://github.com/devicetree-org/devicetree-specification/releases/download/v0.2/devicetree-specification-v0.2.pdf
Thanks and regards,
Lokesh
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
On Wed, Feb 13, 2019 at 07:26:20AM -0800, Tony Lindgren wrote:
> * Lokesh Vutla [190213 04:26]:
> > Hi Tony,
> >
> > On 12/02/19 9:52 PM, Tony Lindgren wrote:
> > > Hi,
> > >
> > > * Lokesh Vutla [190212 07:43]:
> > >> +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";
> > >> +interrupt-controller;
> > >> +interrupt-parent = <&gic500>;
> > >> +#interrupt-cells = <4>;
> > >> +ti,sci = <&dmsc>;
> > >> +ti,sci-dst-id = <56>;
> > >> +ti,sci-rm-range-girq = <0x1>;
> > >> +};
> > >
> > > Can you describe a bit what the "ti,sci-dst-id" is above?
> > >
> > > These IDs seem to be listed at at [0] below, but is it really a property
> > > of the hardware? Or is it some enumeration of SoC devices in the firmware?
> >
> > This is the way that sysfw describes the hardware. In this case it is GIC
> > and it
> > is identified by this ID.
>
> If this ID is an enumeration in the sysfw rather than an actual
> hardware property it should not be in the device tree. If so,
> the device driver should request the id from the sysfw based
> on a name. That is, if no struct device or device phandle can
> be used.
>
> The problem with using enumeration in the dts is that it
> requires maintaining the dts, driver(s) and possibly firmware
> in sync. And that might change between SoCs variants when new
> devices get added and removed.
IOW, don't repeat h/w designers mistakes and make your firmware
discoverable. We have DT only for what is not discoverable.
Rob
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Lokesh Vutla [190213 04:23]: > Hi Tony, > > On 12/02/19 10:00 PM, Tony Lindgren wrote: > > Hi, > > > > * Lokesh Vutla [190212 07:43]: > >> +The Interrupt Router (INTR) module provides a mechanism to route 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 | |->| IRQ | > >> +| INTA |--->|.+-+ | +---+ > >> ++---+|. . | > >> + | +--+ . | > >> + | | irqM |+-+ | > >> + | +--+| N | | > >> + | +-+ | > >> + +--+ > > > > Is this always one-to-one mapping or can the same interrupt be routed to > > multiple targets like to the SoC and some coprocessor? > > Yes, it is always one-to-one. Output of INTR can only be attached to one of > the > processor. OK > >> +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 > > > > Care to describe a bit why the interrupts need to be routed by a system > > controller? > > K3 architecture defines a heterogeneous system where multiple heterogeneous > cores are serving its own usecases. Given that there are multiple ways in > which > a device IRQ can be routed using INTR, like either it can be routed to HLOS > core(A53 int this case) or it can be routed to any other coprocessor available > in the system(like R5). If every sw running in each co-processor is allowed to > program this INTR then there is a high probability that one sw executing on > one > core can damage other heterogeneous core. Mainly to avoid this damage the > configuration of all the INTRs and INTAs are done in a centralized > place(sysfw). > Any user for programming its IRQ route should send a message to sysfw with the > parameters. These parameters are policed by sysfw and does the configuration. OK so maybe update the description along those lines saying it's a shared piece of hardware between various independent SoC clusters which may or may not be running Linux. Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
* Lokesh Vutla [190213 04:26]:
> Hi Tony,
>
> On 12/02/19 9:52 PM, Tony Lindgren wrote:
> > Hi,
> >
> > * Lokesh Vutla [190212 07:43]:
> >> +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";
> >> + interrupt-controller;
> >> + interrupt-parent = <&gic500>;
> >> + #interrupt-cells = <4>;
> >> + ti,sci = <&dmsc>;
> >> + ti,sci-dst-id = <56>;
> >> + ti,sci-rm-range-girq = <0x1>;
> >> +};
> >
> > Can you describe a bit what the "ti,sci-dst-id" is above?
> >
> > These IDs seem to be listed at at [0] below, but is it really a property
> > of the hardware? Or is it some enumeration of SoC devices in the firmware?
>
> This is the way that sysfw describes the hardware. In this case it is GIC and
> it
> is identified by this ID.
If this ID is an enumeration in the sysfw rather than an actual
hardware property it should not be in the device tree. If so,
the device driver should request the id from the sysfw based
on a name. That is, if no struct device or device phandle can
be used.
The problem with using enumeration in the dts is that it
requires maintaining the dts, driver(s) and possibly firmware
in sync. And that might change between SoCs variants when new
devices get added and removed.
Regards,
Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Tony,
On 12/02/19 9:52 PM, Tony Lindgren wrote:
> Hi,
>
> * Lokesh Vutla [190212 07:43]:
>> +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";
>> +interrupt-controller;
>> +interrupt-parent = <&gic500>;
>> +#interrupt-cells = <4>;
>> +ti,sci = <&dmsc>;
>> +ti,sci-dst-id = <56>;
>> +ti,sci-rm-range-girq = <0x1>;
>> +};
>
> Can you describe a bit what the "ti,sci-dst-id" is above?
>
> These IDs seem to be listed at at [0] below, but is it really a property
> of the hardware? Or is it some enumeration of SoC devices in the firmware?
This is the way that sysfw describes the hardware. In this case it is GIC and it
is identified by this ID.
Thanks and regards,
Lokesh
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi Tony, On 12/02/19 10:00 PM, Tony Lindgren wrote: > Hi, > > * Lokesh Vutla [190212 07:43]: >> +The Interrupt Router (INTR) module provides a mechanism to route 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 | |->| IRQ | >> +| INTA |--->|.+-+ | +---+ >> ++---+|. . | >> + | +--+ . | >> + | | irqM |+-+ | >> + | +--+| N | | >> + | +-+ | >> + +--+ > > Is this always one-to-one mapping or can the same interrupt be routed to > multiple targets like to the SoC and some coprocessor? Yes, it is always one-to-one. Output of INTR can only be attached to one of the processor. > >> +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 > > Care to describe a bit why the interrupts need to be routed by a system > controller? K3 architecture defines a heterogeneous system where multiple heterogeneous cores are serving its own usecases. Given that there are multiple ways in which a device IRQ can be routed using INTR, like either it can be routed to HLOS core(A53 int this case) or it can be routed to any other coprocessor available in the system(like R5). If every sw running in each co-processor is allowed to program this INTR then there is a high probability that one sw executing on one core can damage other heterogeneous core. Mainly to avoid this damage the configuration of all the INTRs and INTAs are done in a centralized place(sysfw). Any user for programming its IRQ route should send a message to sysfw with the parameters. These parameters are policed by sysfw and does the configuration. Thanks and regards, Lokesh
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi, * Lokesh Vutla [190212 07:43]: > +The Interrupt Router (INTR) module provides a mechanism to route 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 | |->| IRQ | > +| INTA |--->|.+-+ | +---+ > ++---+|. . | > + | +--+ . | > + | | irqM |+-+ | > + | +--+| N | | > + | +-+ | > + +--+ Is this always one-to-one mapping or can the same interrupt be routed to multiple targets like to the SoC and some coprocessor? > +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 Care to describe a bit why the interrupts need to be routed by a system controller? Regards, Tony
Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Hi,
* Lokesh Vutla [190212 07:43]:
> +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";
> + interrupt-controller;
> + interrupt-parent = <&gic500>;
> + #interrupt-cells = <4>;
> + ti,sci = <&dmsc>;
> + ti,sci-dst-id = <56>;
> + ti,sci-rm-range-girq = <0x1>;
> +};
Can you describe a bit what the "ti,sci-dst-id" is above?
These IDs seem to be listed at at [0] below, but is it really a property
of the hardware? Or is it some enumeration of SoC devices in the firmware?
Regards,
Tony
[0] http://downloads.ti.com/tisci/esd/latest/5_soc_doc/am6x/irq_dsts.html
