Messages in this thread | | | Date | Tue, 27 Sep 2022 11:14:44 -0700 | Subject | Re: [PATCH V4 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic interrupt support | From | Lizhi Hou <> |
| |
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
| |