Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Lokesh Vutla <lokeshvutla@ti.com>
To: Tony Lindgren <tony@atomide.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 21:49:21 +0530
Message-ID: <6aa37276-0833-fdf5-575b-c3ca14f776a6@ti.com> (raw)
In-Reply-To: <20190219153537.GJ15711@atomide.com>



On 2/19/2019 9:05 PM, Tony Lindgren wrote:
> * 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.

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
> 

_______________________________________________
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
2019-02-19 16:19                             ` Lokesh Vutla [this message]
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=6aa37276-0833-fdf5-575b-c3ca14f776a6@ti.com \
    --to=lokeshvutla@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jason@lakedaemon.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --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 \
    --cc=tony@atomide.com \
    /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