From: Alan Mikhak <alan.mikhak@sifive.com>
To: Kishon Vijay Abraham I <kishon@ti.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 11:16:46 -0700 [thread overview]
Message-ID: <CABEDWGxBxmiKjoPUSUaUBXUhKkUTXVX0U9ooRou8tcWJojb52g@mail.gmail.com> (raw)
In-Reply-To: <75d578c2-a98c-d1ef-1633-6dc5dc3b0913@ti.com>
On Thu, May 30, 2019 at 10:08 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi Alan,
> >
> > 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.
I agree. I wouldn't remove memcpy_toio/fromio. That is the reason this
patch introduces the '-d' flag for pcitest to communicate that user
intent across the PCIe bus to pci-epf-test so the endpoint can
initiate the transfer using either memcpy_toio/fromio or DMA.
> > 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.
Please consider the following when taking a closer look at this patch.
In my specific use case, I need to verify that any valid block size,
including a one byte transfer, can be transferred across the PCIe bus
by memcpy_toio/fromio() or by DMA either as a single block or as
linked-list. That is why, instead of deciding based on transfer size,
this patch introduces the '-L' flag for pcitest to communicate the
user intent across the PCIe bus to pci-epf-test so the endpoint can
initiate the DMA transfer using a single block or in linked-list mode.
When user issues 'pcitest -r' to perform a read buffer test,
pci-epf-test calls pci_epf_test_write() which uses memcpy_toio(). As
before, a read from the user point of view is a write from the
endpoint point of view.
When user issues 'pcitest -r -d', pci-epf-test calls a new function
pci_epf_test_write_dma() to initiate a single block DMA transfer.
When user issues 'pcitest -r -d -L', pci-epf-test calls a new function
pci_epf_test_write_dma_list() to initiate a linked-list DMA transfer.
The '-d' and '-L' flags also apply to the '-w' flag when the user
performs a write buffer test. The user can specify any valid transfer
size for any of the above examples using the '-s' flag as before.
> > 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.
Thanks for all your comments and providing the link to your
implementation of pci_epf_tx() in [1] above. It clarifies a lot and
provides a very useful reference.
Regards,
Alan Mikhak
next prev parent reply other threads:[~2019-05-31 18:17 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
2019-05-31 18:16 ` Alan Mikhak [this message]
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=CABEDWGxBxmiKjoPUSUaUBXUhKkUTXVX0U9ooRou8tcWJojb52g@mail.gmail.com \
--to=alan.mikhak@sifive.com \
--cc=Gustavo.Pimentel@synopsys.com \
--cc=arnd@arndb.de \
--cc=bhelgaas@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=jingoohan1@gmail.com \
--cc=kishon@ti.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).