From: Kishon Vijay Abraham I <kishon@ti.com>
To: Alan Mikhak <alan.mikhak@sifive.com>
Cc: Gustavo Pimentel <Gustavo.Pimentel@synopsys.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
"arnd@arndb.de" <arnd@arndb.de>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"jingoohan1@gmail.com" <jingoohan1@gmail.com>,
"bhelgaas@google.com" <bhelgaas@google.com>,
"wen.yang99@zte.com.cn" <wen.yang99@zte.com.cn>,
"kjlu@umn.edu" <kjlu@umn.edu>,
"linux-riscv@lists.infradead.org"
<linux-riscv@lists.infradead.org>,
"palmer@sifive.com" <palmer@sifive.com>,
"paul.walmsley@sifive.com" <paul.walmsley@sifive.com>,
Vinod Koul <vkoul@kernel.org>
Subject: Re: [PATCH] PCI: endpoint: Add DMA to Linux PCI EP Framework
Date: Fri, 31 May 2019 10:36:45 +0530 [thread overview]
Message-ID: <75d578c2-a98c-d1ef-1633-6dc5dc3b0913@ti.com> (raw)
In-Reply-To: <CABEDWGxLeD-K8PjkD5hPSTFGJKs2hxEaAVO+nE5eC9Nx2yw=ig@mail.gmail.com>
Hi Alan,
On 30/05/19 11:26 PM, Alan Mikhak wrote:
> On Wed, May 29, 2019 at 10:48 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>
>> +Vinod Koul
>>
>> Hi,
>>
>>>>> On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
>>>>> <Gustavo.Pimentel@synopsys.com> wrote:
>>>>>>
>>>>>> Hi Alan,
>>>>>>
>>>>>> This patch implementation is very HW implementation dependent and
>>>>>> requires the DMA to exposed through PCIe BARs, which aren't always the
>>>>>> case. Besides, you are defining some control bits on
>>>>>> include/linux/pci-epc.h that may not have any meaning to other types of
>>>>>> DMA.
>>>>>>
>>>>>> I don't think this was what Kishon had in mind when he developed the
>>>>>> pcitest, but let see what Kishon was to say about it.
>>>>>>
>>>>>> I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
>>>>>> and which I submitted some days ago.
>>>>>> By having a DMA driver which implemented using DMAengine API, means the
>>>>>> pcitest can use the DMAengine client API, which will be completely
>>>>>> generic to any other DMA implementation.
>>
>> right, my initial thought process was to use only dmaengine APIs in
>> pci-epf-test so that the system DMA or DMA within the PCIe controller can be
>> used transparently. But can we register DMA within the PCIe controller to the
>> DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
>> (ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.
>>
>> If DMA within the PCIe controller cannot be registered in DMA subsystem, we
>> should use something like what Alan has done in this patch with dma_read ops.
>> The dma_read ops implementation in the EP controller can either use dmaengine
>> APIs or use the DMA within the PCIe controller.
>>
>> I'll review the patch separately.
>>
>> Thanks
>> Kishon
>
> Hi Kishon,
>
> I have some improvements in mind for a v2 patch in response to
> feedback from Gustavo Pimentel that the current implementation is HW
> specific. I hesitate from submitting a v2 patch because it seems best
> to seek comment on possible directions this may be taking.
>
> One alternative is to wait for or modify test functions in
> pci-epf-test.c to call DMAengine client APIs, if possible. I imagine
> pci-epf-test.c test functions would still allocate the necessary local
> buffer on the endpoint side for the same canned tests for everyone to
> use. They would prepare the buffer in the existing manner by filling
> it with random bytes and calculate CRC in the case of a write test.
> However, they would then initiate DMA operations by using DMAengine
> client APIs in a generic way instead of calling memcpy_toio() and
> memcpy_fromio(). They would post-process the buffer in the existing
No, you can't remove memcpy_toio/memcpy_fromio APIs. There could be platforms
without system DMA or they could have system DMA but without MEMCOPY channels
or without DMA in their PCI controller.
> manner such as the checking for CRC in the case of a read test.
> Finally, they would release the resources and report results back to
> the user of pcitest across the PCIe bus through the existing methods.
>
> Another alternative I have in mind for v2 is to change the struct
> pci_epc_dma that this patch added to pci-epc.h from the following:
>
> struct pci_epc_dma {
> u32 control;
> u32 size;
> u64 sar;
> u64 dar;
> };
>
> to something similar to the following:
>
> struct pci_epc_dma {
> size_t size;
> void *buffer;
> int flags;
> };
>
> The 'flags' field can be a bit field or separate boolean values to
> specify such things as linked-list mode vs single-block, etc.
> Associated #defines would be removed from pci-epc.h to be replaced if
> needed with something generic. The 'size' field specifies the size of
> DMA transfer that can fit in the buffer.
I still have to look closer into your DMA patch but linked-list mode or single
block mode shouldn't be an user select-able option but should be determined by
the size of transfer.
>
> That way the dma test functions in pci-epf-test.c can simply kmalloc
> and prepare a local buffer on the endpoint side for the DMA transfer
> and pass its pointer down the stack using the 'buffer' field to lower
> layers. This would allow different PCIe controller drivers to
> implement DMA or not according to their needs. Each implementer can
> decide to use DMAengine client API, which would be preferable, or
> directly read or write to DMA hardware registers to suit their needs.
yes, that would be my preferred method as well. In fact I had implemented
pci_epf_tx() in [1], as a way for pci-epf-test to pass buffer address to
endpoint controller driver. I had also implemented helpers for platforms using
system DMA (i.e uses DMAengine).
Thanks
Kishon
[1] ->
http://git.ti.com/cgit/cgit.cgi/ti-linux-kernel/ti-linux-kernel.git/tree/drivers/pci/endpoint/pci-epf-core.c?h=ti-linux-4.19.y
>
> I would appreciate feedback and comment on such choices as part of this review.
>
> Regards,
> Alan Mikhak
>
next prev parent reply other threads:[~2019-05-31 5:08 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-23 22:24 [PATCH] PCI: endpoint: Add DMA to Linux PCI EP Framework Alan Mikhak
2019-05-24 8:59 ` Gustavo Pimentel
2019-05-24 19:42 ` Alan Mikhak
2019-05-27 9:09 ` Gustavo Pimentel
2019-05-29 22:37 ` Alan Mikhak
2019-05-30 5:46 ` Kishon Vijay Abraham I
2019-05-30 17:56 ` Alan Mikhak
2019-05-31 5:06 ` Kishon Vijay Abraham I [this message]
2019-05-31 18:16 ` Alan Mikhak
2019-06-03 4:41 ` Kishon Vijay Abraham I
2019-06-03 17:42 ` Alan Mikhak
2019-09-13 12:11 ` Kishon Vijay Abraham I
2019-09-13 17:39 ` Alan Mikhak
2019-05-31 5:07 ` Vinod Koul
2019-05-31 5:20 ` Kishon Vijay Abraham I
2019-05-31 6:32 ` Vinod Koul
2019-05-31 7:49 ` Arnd Bergmann
2019-06-03 4:29 ` Kishon Vijay Abraham I
2019-06-03 4:24 ` Kishon Vijay Abraham I
2019-06-03 4:42 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=75d578c2-a98c-d1ef-1633-6dc5dc3b0913@ti.com \
--to=kishon@ti.com \
--cc=Gustavo.Pimentel@synopsys.com \
--cc=alan.mikhak@sifive.com \
--cc=arnd@arndb.de \
--cc=bhelgaas@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=jingoohan1@gmail.com \
--cc=kjlu@umn.edu \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=palmer@sifive.com \
--cc=paul.walmsley@sifive.com \
--cc=vkoul@kernel.org \
--cc=wen.yang99@zte.com.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).