Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Lokesh Vutla <lokeshvutla@ti.com>
Cc: Nishanth Menon <nm@ti.com>,
	Device Tree Mailing List <devicetree@vger.kernel.org>,
	jason@lakedaemon.net, Peter Ujfalusi <peter.ujfalusi@ti.com>,
	marc.zyngier@arm.com, Sekhar Nori <nsekhar@ti.com>,
	linux-kernel@vger.kernel.org, Tero Kristo <t-kristo@ti.com>,
	Rob Herring <robh+dt@kernel.org>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	tglx@linutronix.de,
	Linux ARM Mailing List <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
Date: Tue, 19 Feb 2019 07:35:37 -0800
Message-ID: <20190219153537.GJ15711@atomide.com> (raw)
In-Reply-To: <84b3ec21-9ce9-b9a8-80a9-75001db43a90@ti.com>

* Lokesh Vutla <lokeshvutla@ti.com> [190219 08:51]:
> Hi Tony,
> 
> On 18/02/19 8:02 PM, Tony Lindgren wrote:
> > * Lokesh Vutla <lokeshvutla@ti.com> [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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-12  7:42 [PATCH v5 00/10] Add support for TISCI irqchip drivers Lokesh Vutla
2019-02-12  7:42 ` [PATCH v5 01/10] firmware: ti_sci: Add support to get TISCI handle using of_phandle Lokesh Vutla
2019-02-12  7:42 ` [PATCH v5 02/10] firmware: ti_sci: Add support for RM core ops Lokesh Vutla
2019-02-12  7:42 ` [PATCH v5 03/10] firmware: ti_sci: Add support for IRQ management Lokesh Vutla
2019-02-12  7:42 ` [PATCH v5 04/10] firmware: ti_sci: Add RM mapping table for am654 Lokesh Vutla
2019-02-12  7:42 ` [PATCH v5 05/10] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings Lokesh Vutla
2019-02-12 16:22   ` Tony Lindgren
2019-02-13  4:25     ` Lokesh Vutla
2019-02-13 15:26       ` Tony Lindgren
2019-02-13 20:40         ` Rob Herring
2019-02-14  8:38         ` Lokesh Vutla
2019-02-14 15:41           ` Tony Lindgren
2019-02-14 17:32             ` Lokesh Vutla
2019-02-14 17:46               ` Tony Lindgren
2019-02-14 18:02                 ` Lokesh Vutla
2019-02-15 16:16                   ` Tony Lindgren
2019-02-16  3:29                     ` Lokesh Vutla
2019-02-18 14:32                       ` Tony Lindgren
2019-02-19  8:51                         ` Lokesh Vutla
2019-02-19 15:35                           ` Tony Lindgren [this message]
2019-02-19 16:19                             ` Lokesh Vutla
2019-02-19 17:11                               ` Tony Lindgren
2019-02-19 17:56                                 ` Tony Lindgren
2019-02-20  5:53                                   ` Lokesh Vutla
2019-02-20 16:36                             ` Tony Lindgren
2019-02-20 17:17                               ` Lokesh Vutla
2019-02-12 16:30   ` Tony Lindgren
2019-02-13  4:22     ` Lokesh Vutla
2019-02-13 15:32       ` Tony Lindgren
2019-02-14  8:40         ` Lokesh Vutla
2019-02-14 15:42           ` Tony Lindgren
2019-02-16  3:30   ` Lokesh Vutla
2019-02-18 14:35     ` Tony Lindgren
2019-02-22 10:10       ` Peter Ujfalusi
2019-02-18 15:12   ` Marc Zyngier
2019-02-18 15:27     ` Lokesh Vutla
2019-02-12  7:42 ` [PATCH v5 06/10] irqchip: ti-sci-intr: Add support for Interrupt Router driver Lokesh Vutla
2019-02-18 15:52   ` Marc Zyngier
2019-02-20 13:17     ` Lokesh Vutla
2019-03-19 12:44       ` Lokesh Vutla
2019-02-12  7:42 ` [PATCH v5 07/10] dt-bindings: irqchip: Introduce TISCI Interrupt Aggregator bindings Lokesh Vutla
2019-02-12  7:42 ` [PATCH v5 08/10] irqchip: ti-sci-inta: Add support for Interrupt Aggregator driver Lokesh Vutla
2019-02-12  7:42 ` [PATCH v5 09/10] soc: ti: Add MSI domain support for K3 Interrupt Aggregator Lokesh Vutla
2019-02-13 16:41   ` Tony Lindgren
2019-02-12  7:42 ` [PATCH v5 10/10] soc: ti: am6: Enable interrupt controller drivers Lokesh Vutla
2019-02-16  3:37 ` [PATCH v5 00/10] Add support for TISCI irqchip drivers Lokesh Vutla

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190219153537.GJ15711@atomide.com \
    --to=tony@atomide.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jason@lakedaemon.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lokeshvutla@ti.com \
    --cc=marc.zyngier@arm.com \
    --cc=nm@ti.com \
    --cc=nsekhar@ti.com \
    --cc=peter.ujfalusi@ti.com \
    --cc=robh+dt@kernel.org \
    --cc=ssantosh@kernel.org \
    --cc=t-kristo@ti.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox