On 8/12/19 2:39 PM, Suman Anna wrote:
Hi David,
On 8/8/19 12:09 PM, David Lechner wrote:
On 8/2/19 4:26 PM, Suman Anna wrote:
Point is different applications might use mapping differently as per
their firmware and driver/application design and their split across one
or more PRUs (design by contract). And we need to set this up at runtime
when the application driver is getting run. We will have either the Soft
UART or the Ethernet running at a time depending on the end goal desired
I have an idea that we can use multiple struct irq_domains to make
this work in the existing IRQ framework, but it would be helpful to
know more about the bigger picture first.
Yeah, would be great if there is a way this can be solved without having
to introduce additional API.
Here is what I came up with to use existing IRQ APIs to implement event
mapping.
Basically it is the same as my previous suggestion [1], with the
addition of
multiple IRQ domains.
First of all, many thanks for looking into the problem and providing
patches for the alternate solutions. If we were to not use any exported
functions, this approach does seem to be a viable solution. I am going
to play around with both [1] and this patch with all our existing
usecases and see if I run into any issues.
So, w.r.t this patch compared to [1], is the multiple IRQ domain solving
anything specifically? Our main issue is the re-purposing of a event
(and its mapping depending on the application), and the same issue will
remain whether we have multiple domains or not. Also, now we would
expect an event to migrate between different domains based on its usage.
The only thing using multiple IRQ domains gets us is that it allows us to
have multiple IRQ descriptors (virq) for a single PRU event. In other
words, if we needed to map a single system event to both a PRU core and
the MCU interrupt controller at the same time, then we would need separate
IRQ domains to do this. I we would never need to do something like this,
then we don't the IRQ domains.
Previously, you said "We can have two different applications use the same
event with different mappings." So I took this to mean that the events
would actually be mapped in hardware at the same time, but now I
understand it to just mean that a single firmware blob could contain
multiple mappings that contain the same events, but won't actually be used
at the same time. So if this is the case, then we probably don't need to
mess with IRQ domains.
The idea is that each external interrupt controller (or DMA controller,
etc.)
that is connected to the PRUSS interrupt controller is considered an
interrupt
domain. One of the objections to my previous patch was that we could
only have
one IRQ descriptor per event. Now we can have one descriptor per event per
domain.
I am still proposing that we use the interrupt-cells and identical vendor
resource data structures in the PRU firmware be used to provide the mapping
information. (As a side note, I still think it is important to include
EVTSEL
on AM18xx in order to fully describe the event.)
W.r.t EVTSEL, it is a global value and applies to a range of events. I
have another equivalent register/functionality on most of the other SoCs
as well (a register in PRUSS_CFG space) that muxes standard events vs
MII_RT events. Again, that is limited to only a subset of all the system
events. So, should this continue to be a per event specifier, it will be
yet another mapping configuration data item (my idea was to manage this
once per application within the PRU remoteproc driver along with the
fwspec mapping).
I guess it just seems a bit fragile to me to specify EVTSEL elsewhere. My
thinking is that the first event registered that requires a specific EVTSEL
value "wins" and if any other events are registered with a different EVTSEL
value, then we will get an error. Likewise, if all users of a specific
EVTSEL value are unmapped, then it is up for grabs for any value again.
On the other hand, with a global value as you have proposed, we can just
leave comments in the device tree and the firmware about which EVTSEL value
is required for a specific event number. We won't be able to catch mistakes
at runtime, but at least there will be something to remind us what we did
wrong. So, I suppose that is good enough.
