On Wed, Jan 31, 2024, at 10:52, Yann Sionneau wrote: > On 22/01/2023 12:54, Krzysztof Kozlowski wrote: >> On 20/01/2023 15:10, Yann Sionneau wrote: >>> + >>> +int __init kvx_ipi_ctrl_probe(irqreturn_t (*ipi_irq_handler)(int, void *)) >>> +{ >>> + struct device_node *np; >>> + int ret; >>> + unsigned int ipi_irq; >>> + void __iomem *ipi_base; >>> + >>> + np = of_find_compatible_node(NULL, NULL, "kalray,kvx-ipi-ctrl"); >> Nope, big no. >> >> Drivers go to drivers, not to arch code. Use proper driver infrastructure. > Thank you for your review. > > It raises questions on our side about how to handle this change. > > First let me describe the hardware: > > The coolidge ipi controller device handles IPI communication between cpus > inside a cluster. > > Each cpu has 8 of its dedicated irq lines (24 to 31) hard-wired to the ipi. > The ipi controller has 8 sets of 2 registers: > - a 17-bit "interrupt" register > - a 17-bit "mask" register > > Each couple of register is dedicated to 1 of the 8 irqlines. > Each of the 17 bits of interrupt/mask register > identifies a cpu (cores 0 to 15 + secure_core). > Writing bit i in interrupt register sends an irq to cpu i, according to the mask > in mask register. > Writing in interrupt/mask register couple N targets irq line N of the core. > > - Ipi generates an interrupt to the cpu when message is ready. > - Messages are delivered via Axi. > - Ipi does not have any interrupt input lines. > > > +---------------+ irq axi_w > | | i |<--/--- ipi <------ > | CPU | n | x8 > | core0 | t | > | | c | irq irq msi > | | t |<--/--- apic <----- mbox <------- > | | l | x4 > +---------------+ > with intctl = core-irq controller > > > We analyzed how other Linux ports are handling this situation (IPI) and > here are several possible solutions: > > 1/ put everything in smp.c like what longarch is doing. > * Except that IPI in longarch seems to involve writing to a special > purpose CPU register and not doing a memory mapped write like kvx. > > 2/ write a device driver in drivers/xxx/ with the content from ipi.c > * the probe would just ioremap the reg from DT and register the irq > using request_percpu_irq() > * it would export a function "kvx_ipi_send()" that would directly be > called by smp.c > * Question : where would this driver be placed in drivers/ ? > drivers/irqchip/ ? Even if this is not per-se an interrupt-controller > driver? This looks like it's close enough to the irqchip driver that you can just have it in the same file as the 'intctl' portion. Top-level irqchip implementations tend to be rather architecture specific, as does the IPI mechanism. Depending on the register layout, I think you can have a single devicetree node for the combination of the core-irq (for managing your own interrupts) and the ipi (for receiving interrupts from others), and then have a driver in drivers/irqchip to deal with both. For the ipi mechanism, trying to abstract it too much generally makes it too slow, so I would not go through a nested irqchip or a mailbox driver etc. I don't know what the 'apic' in your diagram is, so that would be either a nested irqchip or could be part of the same driver as well. Arnd