From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: "Gustavo Pimentel" <gustavo.pimentel@synopsys.com>,
"Vinod Koul" <vkoul@kernel.org>,
"Jingoo Han" <jingoohan1@gmail.com>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Frank Li" <Frank.Li@nxp.com>,
"Serge Semin" <fancer.lancer@gmail.com>,
"Alexey Malahov" <Alexey.Malahov@baikalelectronics.ru>,
"Pavel Parkhomenko" <Pavel.Parkhomenko@baikalelectronics.ru>,
"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Wilczyński" <kw@linux.com>,
linux-pci@vger.kernel.org, dmaengine@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 18/25] dmaengine: dw-edma: Join Write/Read channels into a single device
Date: Fri, 25 Mar 2022 13:04:27 +0530 [thread overview]
Message-ID: <20220325073427.GF4675@thinkpad> (raw)
In-Reply-To: <20220324014836.19149-19-Sergey.Semin@baikalelectronics.ru>
On Thu, Mar 24, 2022 at 04:48:29AM +0300, Serge Semin wrote:
> Indeed there is no point in such split up because due to multiple reasons.
> First of all eDMA read and write channels belong to one physical
> controller. Splitting them up illogical. Secondly the channels
> differentiating can be done by means of the filtering and the
> dma_get_slave_caps() method. Finally having these channels handled
> separately not only needlessly complicates the code, but also causes the
> DebugFS error printed to console:
>
> >> Debugfs: Directory '1f052000.pcie' with parent 'dmaengine' already present!
>
> So to speak let's join the read/write channels into a single DMA device.
> The client drivers will be able to choose the channel with required
> capability by getting the DMA slave direction setting. It's default value
> is overridden by the dw_edma_device_caps() callback in accordance with the
> channel nature.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
I'm all in for this!
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Thanks,
Mani
> ---
> drivers/dma/dw-edma/dw-edma-core.c | 117 +++++++++++++++--------------
> drivers/dma/dw-edma/dw-edma-core.h | 5 +-
> 2 files changed, 62 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index cefa73412bf7..a391e44da039 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -209,6 +209,24 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> desc->chunks_alloc--;
> }
>
> +static void dw_edma_device_caps(struct dma_chan *dchan,
> + struct dma_slave_caps *caps)
> +{
> + struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +
> + if (chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
> + if (chan->dir == EDMA_DIR_READ)
> + caps->directions = BIT(DMA_DEV_TO_MEM);
> + else
> + caps->directions = BIT(DMA_MEM_TO_DEV);
> + } else {
> + if (chan->dir == EDMA_DIR_WRITE)
> + caps->directions = BIT(DMA_DEV_TO_MEM);
> + else
> + caps->directions = BIT(DMA_MEM_TO_DEV);
> + }
> +}
> +
> static int dw_edma_device_config(struct dma_chan *dchan,
> struct dma_slave_config *config)
> {
> @@ -723,8 +741,8 @@ static void dw_edma_free_chan_resources(struct dma_chan *dchan)
> pm_runtime_put(chan->dw->chip->dev);
> }
>
> -static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
> - u32 wr_alloc, u32 rd_alloc)
> +static int dw_edma_channel_setup(struct dw_edma_chip *chip, u32 wr_alloc,
> + u32 rd_alloc)
> {
> struct dw_edma_region *dt_region;
> struct device *dev = chip->dev;
> @@ -732,27 +750,15 @@ static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
> struct dw_edma_chan *chan;
> struct dw_edma_irq *irq;
> struct dma_device *dma;
> - u32 alloc, off_alloc;
> - u32 i, j, cnt;
> - int err = 0;
> + u32 i, ch_cnt;
> u32 pos;
>
> - if (write) {
> - i = 0;
> - cnt = dw->wr_ch_cnt;
> - dma = &dw->wr_edma;
> - alloc = wr_alloc;
> - off_alloc = 0;
> - } else {
> - i = dw->wr_ch_cnt;
> - cnt = dw->rd_ch_cnt;
> - dma = &dw->rd_edma;
> - alloc = rd_alloc;
> - off_alloc = wr_alloc;
> - }
> + ch_cnt = dw->wr_ch_cnt + dw->rd_ch_cnt;
> + dma = &dw->dma;
>
> INIT_LIST_HEAD(&dma->channels);
> - for (j = 0; (alloc || chip->nr_irqs == 1) && j < cnt; j++, i++) {
> +
> + for (i = 0; i < ch_cnt; i++) {
> chan = &dw->chan[i];
>
> dt_region = devm_kzalloc(dev, sizeof(*dt_region), GFP_KERNEL);
> @@ -762,52 +768,62 @@ static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
> chan->vc.chan.private = dt_region;
>
> chan->dw = dw;
> - chan->id = j;
> - chan->dir = write ? EDMA_DIR_WRITE : EDMA_DIR_READ;
> +
> + if (i < dw->wr_ch_cnt) {
> + chan->id = i;
> + chan->dir = EDMA_DIR_WRITE;
> + } else {
> + chan->id = i - dw->wr_ch_cnt;
> + chan->dir = EDMA_DIR_READ;
> + }
> +
> chan->configured = false;
> chan->request = EDMA_REQ_NONE;
> chan->status = EDMA_ST_IDLE;
>
> - if (write)
> - chan->ll_max = (chip->ll_region_wr[j].sz / EDMA_LL_SZ);
> + if (chan->dir == EDMA_DIR_WRITE)
> + chan->ll_max = (chip->ll_region_wr[chan->id].sz / EDMA_LL_SZ);
> else
> - chan->ll_max = (chip->ll_region_rd[j].sz / EDMA_LL_SZ);
> + chan->ll_max = (chip->ll_region_rd[chan->id].sz / EDMA_LL_SZ);
> chan->ll_max -= 1;
>
> dev_vdbg(dev, "L. List:\tChannel %s[%u] max_cnt=%u\n",
> - write ? "write" : "read", j, chan->ll_max);
> + chan->dir == EDMA_DIR_WRITE ? "write" : "read",
> + chan->id, chan->ll_max);
>
> if (chip->nr_irqs == 1)
> pos = 0;
> + else if (chan->dir == EDMA_DIR_WRITE)
> + pos = chan->id % wr_alloc;
> else
> - pos = off_alloc + (j % alloc);
> + pos = wr_alloc + chan->id % rd_alloc;
>
> irq = &dw->irq[pos];
>
> - if (write)
> - irq->wr_mask |= BIT(j);
> + if (chan->dir == EDMA_DIR_WRITE)
> + irq->wr_mask |= BIT(chan->id);
> else
> - irq->rd_mask |= BIT(j);
> + irq->rd_mask |= BIT(chan->id);
>
> irq->dw = dw;
> memcpy(&chan->msi, &irq->msi, sizeof(chan->msi));
>
> dev_vdbg(dev, "MSI:\t\tChannel %s[%u] addr=0x%.8x%.8x, data=0x%.8x\n",
> - write ? "write" : "read", j,
> + chan->dir == EDMA_DIR_WRITE ? "write" : "read", chan->id,
> chan->msi.address_hi, chan->msi.address_lo,
> chan->msi.data);
>
> chan->vc.desc_free = vchan_free_desc;
> vchan_init(&chan->vc, dma);
>
> - if (write) {
> - dt_region->paddr = chip->dt_region_wr[j].paddr;
> - dt_region->vaddr = chip->dt_region_wr[j].vaddr;
> - dt_region->sz = chip->dt_region_wr[j].sz;
> + if (chan->dir == EDMA_DIR_WRITE) {
> + dt_region->paddr = chip->dt_region_wr[chan->id].paddr;
> + dt_region->vaddr = chip->dt_region_wr[chan->id].vaddr;
> + dt_region->sz = chip->dt_region_wr[chan->id].sz;
> } else {
> - dt_region->paddr = chip->dt_region_rd[j].paddr;
> - dt_region->vaddr = chip->dt_region_rd[j].vaddr;
> - dt_region->sz = chip->dt_region_rd[j].sz;
> + dt_region->paddr = chip->dt_region_rd[chan->id].paddr;
> + dt_region->vaddr = chip->dt_region_rd[chan->id].vaddr;
> + dt_region->sz = chip->dt_region_rd[chan->id].sz;
> }
>
> dw_edma_v0_core_device_config(chan);
> @@ -819,7 +835,7 @@ static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
> dma_cap_set(DMA_CYCLIC, dma->cap_mask);
> dma_cap_set(DMA_PRIVATE, dma->cap_mask);
> dma_cap_set(DMA_INTERLEAVE, dma->cap_mask);
> - dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV);
> + dma->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> dma->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> dma->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> dma->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
> @@ -828,6 +844,7 @@ static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
> dma->dev = chip->dev;
> dma->device_alloc_chan_resources = dw_edma_alloc_chan_resources;
> dma->device_free_chan_resources = dw_edma_free_chan_resources;
> + dma->device_caps = dw_edma_device_caps;
> dma->device_config = dw_edma_device_config;
> dma->device_pause = dw_edma_device_pause;
> dma->device_resume = dw_edma_device_resume;
> @@ -841,9 +858,7 @@ static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
> dma_set_max_seg_size(dma->dev, U32_MAX);
>
> /* Register DMA device */
> - err = dma_async_device_register(dma);
> -
> - return err;
> + return dma_async_device_register(dma);
> }
>
> static inline void dw_edma_dec_irq_alloc(int *nr_irqs, u32 *alloc, u16 cnt)
> @@ -982,13 +997,8 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> if (err)
> return err;
>
> - /* Setup write channels */
> - err = dw_edma_channel_setup(chip, true, wr_alloc, rd_alloc);
> - if (err)
> - goto err_irq_free;
> -
> - /* Setup read channels */
> - err = dw_edma_channel_setup(chip, false, wr_alloc, rd_alloc);
> + /* Setup write/read channels */
> + err = dw_edma_channel_setup(chip, wr_alloc, rd_alloc);
> if (err)
> goto err_irq_free;
>
> @@ -1028,15 +1038,8 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> pm_runtime_disable(dev);
>
> /* Deregister eDMA device */
> - dma_async_device_unregister(&dw->wr_edma);
> - list_for_each_entry_safe(chan, _chan, &dw->wr_edma.channels,
> - vc.chan.device_node) {
> - tasklet_kill(&chan->vc.task);
> - list_del(&chan->vc.chan.device_node);
> - }
> -
> - dma_async_device_unregister(&dw->rd_edma);
> - list_for_each_entry_safe(chan, _chan, &dw->rd_edma.channels,
> + dma_async_device_unregister(&dw->dma);
> + list_for_each_entry_safe(chan, _chan, &dw->dma.channels,
> vc.chan.device_node) {
> tasklet_kill(&chan->vc.task);
> list_del(&chan->vc.chan.device_node);
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index e254c2fc3d9c..ec9f84a857d1 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -98,10 +98,9 @@ struct dw_edma_irq {
> struct dw_edma {
> char name[20];
>
> - struct dma_device wr_edma;
> - u16 wr_ch_cnt;
> + struct dma_device dma;
>
> - struct dma_device rd_edma;
> + u16 wr_ch_cnt;
> u16 rd_ch_cnt;
>
> struct dw_edma_irq *irq;
> --
> 2.35.1
>
next prev parent reply other threads:[~2022-03-25 7:34 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-24 1:48 [PATCH 00/25] dmaengine: dw-edma: Add RP/EP local DMA controllers support Serge Semin
2022-03-24 1:48 ` [PATCH 01/25] dmaengine: dw-edma: Drop dma_slave_config.direction field usage Serge Semin
2022-03-24 13:30 ` Manivannan Sadhasivam
2022-04-05 11:15 ` Serge Semin
2022-03-24 1:48 ` [PATCH 02/25] dmaengine: dw-edma: Fix eDMA Rd/Wr-channels and DMA-direction semantics Serge Semin
2022-03-24 1:48 ` [PATCH 03/25] dma-direct: take dma-ranges/offsets into account in resource mapping Serge Semin
2022-03-24 11:30 ` Robin Murphy
2022-04-17 22:44 ` Serge Semin
2022-04-20 7:12 ` Christoph Hellwig
2022-04-20 8:32 ` Serge Semin
2022-04-20 8:47 ` Christoph Hellwig
2022-04-20 8:55 ` Serge Semin
2022-04-21 14:45 ` Christoph Hellwig
2022-04-21 17:35 ` Serge Semin
2022-04-21 20:51 ` Robin Murphy
2022-04-24 21:46 ` Serge Semin
2022-03-24 1:48 ` [PATCH 04/25] dmaengine: Fix dma_slave_config.dst_addr description Serge Semin
2022-03-24 14:08 ` Manivannan Sadhasivam
2022-03-31 5:38 ` Vinod Koul
2022-03-31 7:13 ` Serge Semin
2022-03-31 10:50 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 05/25] dmaengine: dw-edma: Convert ll/dt phys-address to PCIe bus/DMA address Serge Semin
2022-03-24 16:23 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 06/25] dmaengine: dw-edma: Fix missing src/dst address of the interleaved xfers Serge Semin
2022-03-24 16:26 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 07/25] dmaengine: dw-edma: Don't permit non-inc " Serge Semin
2022-03-24 17:15 ` Manivannan Sadhasivam
2022-04-17 22:59 ` Serge Semin
2022-03-24 1:48 ` [PATCH 08/25] dmaengine: dw-edma: Fix invalid interleaved xfers semantics Serge Semin
2022-03-24 1:48 ` [PATCH 09/25] dmaengine: dw-edma: Add CPU to PCIe bus address translation Serge Semin
2022-03-24 17:25 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 10/25] dmaengine: dw-edma: Add PCIe bus address getter to the remote EP glue-driver Serge Semin
2022-03-24 17:41 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 11/25] dmaengine: dw-edma: Drop chancnt initialization Serge Semin
2022-03-24 17:42 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 12/25] dmaengine: dw-edma: Fix DebugFS reg entry type Serge Semin
2022-03-24 17:48 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 13/25] dmaengine: dw-edma: Stop checking debugfs_create_*() return value Serge Semin
2022-03-24 18:12 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 14/25] dmaengine: dw-edma: Add dw_edma prefix to the DebugFS nodes descriptor Serge Semin
2022-03-24 18:14 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 15/25] dmaengine: dw-edma: Convert DebugFS descs to being kz-allocated Serge Semin
2022-03-25 6:03 ` Manivannan Sadhasivam
2022-03-25 6:42 ` Manivannan Sadhasivam
2022-04-18 7:17 ` Serge Semin
2022-03-24 1:48 ` [PATCH 16/25] dmaengine: dw-edma: Simplify the DebugFS context CSRs init procedure Serge Semin
2022-03-25 6:27 ` Manivannan Sadhasivam
2022-03-25 6:31 ` Manivannan Sadhasivam
2022-04-18 8:23 ` Serge Semin
2022-03-24 1:48 ` [PATCH 17/25] dmaengine: dw-edma: Move eDMA data pointer to DebugFS node descriptor Serge Semin
2022-03-25 6:35 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 18/25] dmaengine: dw-edma: Join Write/Read channels into a single device Serge Semin
2022-03-25 7:34 ` Manivannan Sadhasivam [this message]
2022-03-24 1:48 ` [PATCH 19/25] dmaengine: dw-edma: Use DMA-engine device DebugFS subdirectory Serge Semin
2022-03-25 7:41 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 20/25] dmaengine: dw-edma: Use non-atomic io-64 methods Serge Semin
2022-03-25 8:28 ` Manivannan Sadhasivam
2022-04-18 11:37 ` Serge Semin
2022-03-24 1:48 ` [PATCH 21/25] dmaengine: dw-edma: Drop DT-region allocation Serge Semin
2022-03-25 8:33 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 22/25] dmaengine: dw-edma: Replace chip ID number with device name Serge Semin
2022-03-25 10:02 ` Manivannan Sadhasivam
2022-04-18 12:17 ` Serge Semin
2022-03-24 1:48 ` [PATCH 23/25] dmaengine: dw-edma: Bypass dma-ranges mapping for the local setup Serge Semin
2022-03-25 18:10 ` Manivannan Sadhasivam
2022-04-18 13:36 ` Serge Semin
2022-03-24 1:48 ` [PATCH 24/25] dmaengine: dw-edma: Skip cleanup procedure if no private data found Serge Semin
2022-03-25 18:15 ` Manivannan Sadhasivam
2022-04-18 13:48 ` Serge Semin
2022-04-23 14:45 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 25/25] PCI: dwc: Add DW eDMA engine support Serge Semin
2022-03-28 14:15 ` Manivannan Sadhasivam
2022-04-19 20:54 ` Serge Semin
2022-04-23 14:40 ` Manivannan Sadhasivam
2022-04-25 5:22 ` Manivannan Sadhasivam
2022-04-28 14:05 ` Serge Semin
2022-04-28 17:09 ` Manivannan Sadhasivam
2022-04-29 16:13 ` Serge Semin
2022-04-29 17:20 ` Manivannan Sadhasivam
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=20220325073427.GF4675@thinkpad \
--to=manivannan.sadhasivam@linaro.org \
--cc=Alexey.Malahov@baikalelectronics.ru \
--cc=Frank.Li@nxp.com \
--cc=Pavel.Parkhomenko@baikalelectronics.ru \
--cc=Sergey.Semin@baikalelectronics.ru \
--cc=bhelgaas@google.com \
--cc=dmaengine@vger.kernel.org \
--cc=fancer.lancer@gmail.com \
--cc=gustavo.pimentel@synopsys.com \
--cc=jingoohan1@gmail.com \
--cc=kw@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=robh@kernel.org \
--cc=vkoul@kernel.org \
/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).