Messages in this thread | | | Date | Sun, 05 Jul 2020 21:45:36 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCHv3 2/6] irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts |
| |
On 2020-07-05 14:26, Grzegorz Jaszczyk wrote: > On Sat, 4 Jul 2020 at 11:39, Marc Zyngier <maz@kernel.org> 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...
| |