Re: [PATCH V4 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic interrupt support

From: Lizhi Hou
Date: Tue Sep 27 2022 - 14:15:02 EST



On 9/27/22 10:54, Martin Tůma wrote:
On 27. 09. 22 19:18, Lizhi Hou wrote:

On 9/27/22 09:46, Martin Tůma wrote:
On 27. 09. 22 18:28, Lizhi Hou wrote:

Okay, I got the point. How about changing request/remove APIs to enable/disable APIs as below

      xdma_enable_user_irq(struct platform_device *pdev, u32 user_irq_index, u32 *irq)

             user_irq_index: user logic interrupt wire index. (XDMA driver determines how system IRQs are mapped to DMA channels and user logic wires)

             irq: IRQ number returned for registering interrupt handler (request_irq()) or passing to existing platform driver.

     xdma_disable_user_irq(struct platform_device *pdev, u32 user_irq_index)

Does this make sense to you?


I think even the "irq" parameter in the enable function is surplus as the parent driver (the driver of the actual PCIe card) knows* what PCI irq he has to allocate without XDMA providing the number.

xdma_enable_user_irq(struct platform_device *pdev, u32 user_irq_index);
xdma_disable_user_irq(struct platform_device *pdev, u32 user_irq_index);

should be all that is needed.

M.

* something like:
pci_irq_vector((pdev), PCI_BAR_ID) + NUM_C2H_CHANNELS + NUM_H2C_CHANNELS
can be used from the PCIe driver

How does parent driver know the first few vectors will be assigned to DMA channel?  Parent diver should not assume the first (NUM_C2H_CHANNELS+NUM_H2C_CHANNELS) are for DMA channel.

Parent driver passes the system IRQ range  to XDMA driver, and only XDMA driver knows what IRQs are used by DMA channel and what IRQs are mapped to user logic wires. I would keep the "u32 *irq" argument.


The parent driver knows how much DMA channels it wants/allocates. If it is possible to allocate different IRQs than the first NUM_C2H_CHANNELS + NUM_H2C_CHANNELS to the XDMA IP core, than that parameter may be needed, but I haven't seen such HW. Moreover, every parent driver author should IMHO know how the channel and user IRQs are mapped in their specific HW configuration so this info can be "hard-wired" in the parent driver, but I'm fine with it when the irq parameter is kept anyway. All I really need is that the enable/disable logic is split from the irq allocate/register logic so I can use the other platform drivers.

M.

Thanks Mark, all your comments are very helpful. And glad to know you are fine with irq parameter. The IRQ binding is set by xdma driver ( xdma_set_vector_reg() ). That means xdma driver is able to map any system IRQ to dma channel or user logic. That is why I prefer to keep irq parameter. Just a FYI.

Lizhi