Re: [PATCH 4/6] irqchip/irq-pruss-intc: Add helper functions to configure internal mapping

From: David Lechner <david@lechnology.com>
To: Suman Anna <s-anna@ti.com>, Marc Zyngier <marc.zyngier@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>
Cc: devicetree@vger.kernel.org,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Tony Lindgren <tony@atomide.com>, Sekhar Nori <nsekhar@ti.com>,
	linux-kernel@vger.kernel.org, "Andrew F. Davis" <afd@ti.com>,
	Lokesh Vutla <lokeshvutla@ti.com>,
	Murali Karicheri <m-karicheri2@ti.com>,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Roger Quadros <rogerq@ti.com>
Subject: Re: [PATCH 4/6] irqchip/irq-pruss-intc: Add helper functions to configure internal mapping
Date: Wed, 17 Jul 2019 12:57:29 -0500	[thread overview]
Message-ID: <22825f06-d968-03a7-585b-8cbf4123915c@lechnology.com> (raw)
In-Reply-To: <23ae1767-3531-ea57-2c82-f2657baa123f@ti.com>

On 7/16/19 6:29 PM, Suman Anna wrote:
> Hi David,
> 
> On 7/10/19 10:10 PM, David Lechner wrote:
>> On 7/7/19 10:52 PM, Suman Anna wrote:
>>> The PRUSS INTC receives a number of system input interrupt source events
>>> and supports individual control configuration and hardware
>>> prioritization.
>>> These input events can be mapped to some output host interrupts through 2
>>> levels of many-to-one mapping i.e. events to channel mapping and channels
>>> to host interrupts.
>>>
>>> This mapping information is provided through the PRU firmware that is
>>> loaded onto a PRU core/s or through the device tree node of the PRU
>>
> 
> Thanks for the thorough review and alternate solutions/suggestions.
> 
>> What will the device tree bindings for this look like?
> 
> They would be as in the below patch you already figured.

Ah, makes sense now: the mapping is defined in the remoteproc node
rather than in the interrupt controller node.

> 
>>
>> Looking back at Rob's comment on the initial series [1], I still think
>> that increasing the #interrupt-cells sounds like a reasonable solution.
>>
>> [1]: https://patchwork.kernel.org/patch/10697705/#22375155
> 
> So, there are couple of reasons why I did not use an extended
> #interrupt-cells:
> 
> 1. There is only one irq descriptor associated with each event, and the
> usage of events is typically per application. And the descriptor mapping
> is done once. We can have two different applications use the same event
> with different mappings. So we want this programming done at
> application's usage of PRU (so done when a consumer driver acquires a
> PRU processor(s) which are treated as an exclusive resource). All the
> different application properties that you saw in [1] are configured at
> the time of acquiring a PRU and reset when they release a PRU.
> 
> 2. The configuration is performed by Linux for all host interrupts and
> channels, and this was primarily done to save the very limited IRAM
> space for those needed by the PRUs. From firmware's point of view, this
> was offloaded to the ARM OS driver/infrastructure, but in general it is
> a design by contract between a PRU client driver and its firmware. Also,
> the DT binding semantics using interrupts property and request_irq()
> typically limits these to interrupts only being requested by MPU, and so
> will leave out those needed by PRUs.
> 

Hmm... case 1. is a tricky one indeed. If there are going to be times where
an event requires multiple mappings, I agree that this doesn't seem to fit
into any existing device tree bindings.

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