Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <68fdd2a3-b881-470b-c5b3-0f2fc881ed27@gpxsee.org>
Date:   Tue, 27 Sep 2022 19:54:37 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.1.2
Subject: Re: [PATCH V4 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic
 interrupt support
Content-Language: en-US
To:     Lizhi Hou <lizhi.hou@amd.com>, vkoul@kernel.org,
        dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
        trix@redhat.com
Cc:     max.zhen@amd.com, sonal.santan@amd.com, larry.liu@amd.com,
        brian.xu@amd.com
References: <1663871905-60498-1-git-send-email-lizhi.hou@amd.com>
 <1663871905-60498-3-git-send-email-lizhi.hou@amd.com>
 <a2b5c35d-ced2-ff83-c550-4bbf29973b80@gpxsee.org>
 <64388266-1707-ee20-c3ab-edb67ada68dc@amd.com>
 <c0430eb7-d607-acfd-201a-db56db982309@gpxsee.org>
 <5f77987e-49bc-e035-19e0-52c25f4adc7e@amd.com>
From:   =?UTF-8?Q?Martin_T=c5=afma?= <tumic@gpxsee.org>
In-Reply-To: <5f77987e-49bc-e035-19e0-52c25f4adc7e@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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.