On 12.08.2024 14:18:31, Martin Jocić wrote: > On Mon, 2024-08-05 at 17:37 +0200, Marc Kleine-Budde wrote: > > On 11.07.2024 15:54:51, Martin Jocic wrote: > > > We should return from the ISR as fast as possible in order > > > not to mask a new interrupt. > > > > Do you mean really mean "mask" new interrupts or "ACK" new interrupts? > > > > I'm not familiar with the hardware, but in several other IP cores the > > driver first ACKs then IRQs then does the work. Is this an option here, > > too? > > > > regards, > > Marc > > > Sorry for the late answer, but I have been on vacation. > > Maybe I was unclear, what I meant by masking was that as I understand it, the > kernel won't issue a new call to an ISR for a specific interrupt until the ISR > has returned, thus effectively "masking" new identical interrupts. > > Kvaser's PCIe hardware uses the KCAN FPGA IP block which has dual 4K ping-pong > buffers for incoming messages shared by all (currently up to eight) channels. > I.e. while the driver processes the messages in one buffer, new incoming > messages are stored in the other and so on. > > The design of KCAN is such that a buffer must be fully read and then released. > Releasing a buffer will make the FPGA switch buffers. If the other buffer > contains at least one new incoming message the FPGA will also instantly issue a > new interrupt, if not the interrupt will be issued after receiving the first new > message. > > With IRQx interrupts, it takes a little time for the interrupt to happen, enough > for the ISR to do it's business and return, but MSI interrupts are way faster so > this time is reduced to almost nothing. So with MSI, releasing the buffer HAS to > be the very last action of the ISR before returning, otherwise the new interrupt > may be "masked" by the kernel as described above. And the interrupts are edge- > triggered so we cannot loose one, or the ping-pong reading process will stop. Edge triggered IRQs are a pain. :( > This is why the patch modifies the driver to use a single write to the SRB_CMD > register to release the buffer(s) instead of possibly two writes as before. Thanks for the explanation. Can you add some of this to the commit message, so that it doesn't get lost? > The interrupts can be (and are) reset by writing logical one bits to the IRQ > registers if that is what you mean by ACKing. Regarding your reference to other > IP cores, I'm not quite sure what you mean. Are you suggesting some kind of top- > half/bottom-half solution? I was not aware that the IRQs are edge triggered. Other IP cores with level triggered IRQs, the driver reads the IRQ register, ACKs the IRQs, then handles them and exits the IRQ handler. If another IRQ comes after ACK the IRQ handler is started again, due to the level nature of the IRQ trigger. > If so, I've tried various such variants in the hope of avoiding the "masking" > problem to no avail (IIRC, it was before my vacation). > > However, as a side note, further tests with very high incoming message loads has > shown that this patch, although improving the situation won't be enough. > Therefore I'm preparing another patch which does the processing in a threaded > interrupt using a (limited) poll loop. But more on that later. I'm looking forward for that patch. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
