On 2020-07-05 14:26, Grzegorz Jaszczyk wrote:
On Sat, 4 Jul 2020 at 11:39, Marc Zyngier <maz@xxxxxxxxxx> wrote:
On 2020-07-03 15:28, Grzegorz Jaszczyk wrote:
[...]
It still begs the question: if the HW can support both edge and level
triggered interrupts, why isn't the driver supporting this diversity?
I appreciate that your HW may only have level interrupts so far, but
what guarantees that this will forever be true? It would imply a
change
in the DT binding, which isn't desirable.
Ok, I've got your point. I will try to come up with something later
on. Probably extending interrupt-cells by one and passing interrupt
type will be enough for now. Extending this driver to actually support
it can be handled later if needed. Hope it works for you.
Writing a set_type callback to deal with this should be pretty easy.
Don't delay doing the right thing.
[...]
>> > + hwirq = hipir & GENMASK(9, 0);
>> > + virq = irq_linear_revmap(intc->domain, hwirq);
>>
>> And this is where I worry. You seems to have a single irqdomain
>> for all the muxes. Are you guaranteed that you will have no
>> overlap between muxes? And please use irq_find_mapping(), as
>> I have top-secret plans to kill irq_linear_revmap().
>
> Regarding irq_find_mapping - sure.
>
> Regarding irqdomains:
> It is a single irqdomain since the hwirq (system event) can be mapped
> to different irq_host (muxes). Patch #6
> https://lkml.org/lkml/2020/7/2/616 implements and describes how 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. Mentioned implementation ensures that specific system
> event (hwirq) can be mapped through PRUSS specific channel into a
> single host interrupt.
Patch #6 is a nightmare of its own, and I haven't fully groked it yet.
Also, this driver seems to totally ignore the 2-level routing. Where
is it set up? map/unmap in this driver do exactly *nothing*, so
something somewhere must set it up.
The map/unmap is updated in patch #6 and it deals with those 2-level
routing setup. Map is responsible for programming the Channel Map
Registers (CMRx) and Host-Interrupt Map Registers (HMRx) basing on
provided configuration from the one parsed in the xlate function.
Unmap undo whatever was done on the map. More details can be found in
patch #6.
Maybe it would be better to squash patch #6 with this one so it would
be less confusing. What is your advice?
So am I right in understanding that without patch #6, this driver does
exactly nothing? If so, it has been a waste of review time.
Please split patch #6 so that this driver does something useful
for Linux, without any of the PRU interrupt routing stuff. I want
to see a Linux-only driver that works and doesn't rely on any other
exotic feature.
M.
--
Jazz is not dead. It just smells funny...
