This is a final patchset in the series created in the framework of
my Baikal-T1 PCIe/eDMA-related work:
[1: Done v5] PCI: dwc: Various fixes and cleanups
Link: https://lore.kernel.org/linux-pci/[email protected]/
Merged: kernel 6.0-rc1
[2: Done v4] PCI: dwc: Add hw version and dma-ranges support
Link: https://lore.kernel.org/linux-pci/[email protected]/
Merged: kernel 6.0-rc1
[3: Done v7] PCI: dwc: Add generic resources and Baikal-T1 support
Link: https://lore.kernel.org/linux-pci/[email protected]/
Merged: kernel 6.2-rc1
[4: In-review v9] dmaengine: dw-edma: Add RP/EP local DMA controllers support
Link: ---you are looking at it---
Note it is very recommended to merge the patchsets in the same order as
they are listed in the set above in order to have them applied smoothly.
Since the patchsets 1-3 have already been merged into the mainline kernel
this series can be applied via any DMA-engine or PCI repos.
Here is a short summary regarding this patchset. The series starts with
fixes patches. We discovered that the dw-edma-pcie.c driver incorrectly
initializes the LL/DT base addresses for the platforms with not matching
CPU and PCIe memory spaces. It is fixed by using the pci_bus_address()
method to get a correct base address. After that you can find a series of
the interleaved xfers fixes. It turned out the interleaved transfers
implementation didn't work quite correctly from the very beginning for
instance missing src/dst addresses initialization, etc. In the framework
of the next two patches we suggest to add a new platform-specific
callback - pci_address() and use it to convert the CPU address to the PCIe
space address. It is at least required for the DW eDMA remote End-point
setup on the platforms with not-matching CPU/PCIe address spaces. In case
of the DW eDMA local RP/EP setup the conversion will be done automatically
by the outbound iATU (if no DMA-bypass flag is specified for the
corresponding iATU window). Then we introduce a set of the patches to make
the DebugFS part of the code supporting the multi-eDMA controllers
platforms. It starts with several cleanup patches and is closed joining
the Read/Write channels into a single DMA-device as they originally should
have been. After that you can find the patches with adding the non-atomic
io-64 methods usage, dropping DT-region descriptors allocation, replacing
chip IDs with the device name. In addition to that in order to have the
eDMA embedded into the DW PCIe RP/EP supported we need to bypass the
dma-ranges-based memory ranges mapping since in case of the root port DT
node it's applicable for the peripheral PCIe devices only. Finally at the
series closure we introduce a generic DW eDMA controller support being
available in the DW PCIe Root Port/Endpoint driver.
Link: https://lore.kernel.org/linux-pci/[email protected]/
Changelog v2:
- Drop the patches:
[PATCH 1/25] dmaengine: dw-edma: Drop dma_slave_config.direction field usage
[PATCH 2/25] dmaengine: dw-edma: Fix eDMA Rd/Wr-channels and DMA-direction semantics
since they are going to be merged in in the framework of the
Frank's patchset.
- Add a new patch: "dmaengine: dw-edma: Release requested IRQs on
failure."
- Drop __iomem qualifier from the struct dw_edma_debugfs_entry instance
definition in the dw_edma_debugfs_u32_get() method. (@Manivannan)
- Add a new patch: "dmaengine: dw-edma: Rename DebugFS dentry variables to
'dent'." (@Manivannan)
- Slightly extend the eDMA name array size. (@Manivannan)
- Change the specific DMA mapping comment a bit to being
clearer. (@Manivannan)
- Add a new patch: "PCI: dwc: Add generic iATU/eDMA CSRs space detection
method."
- Don't fail eDMA detection procedure if the DW eDMA driver couldn't probe
device. That happens if the driver is disabled. (@Manivannan)
- Add "dma" registers resource mapping procedure. (@Manivannan)
- Move the eDMA CSRs space detection into the dw_pcie_map_detect() method.
- Remove eDMA on the dw_pcie_ep_init() internal errors. (@Manivannan)
- Remove eDMA in the dw_pcie_ep_exit() method.
- Move the dw_pcie_edma_detect() method execution to the tail of the
dw_pcie_ep_init() function.
Link: https://lore.kernel.org/linux-pci/[email protected]/
Changelog v3:
- Conditionally set dchan->dev->device.dma_coherent field since it can
be missing on some platforms. (@Manivannan)
- Drop the patch: "PCI: dwc: Add generic iATU/eDMA CSRs space detection
method". A similar modification has been done in another patchset.
- Add more comprehensive and less regression prune eDMA block detection
procedure.
- Drop the patch: "dma-direct: take dma-ranges/offsets into account in
resource mapping". It will be separately reviewed.
- Remove Manivannan tb tag from the modified patches.
- Rebase onto the kernel v5.18.
Link: https://lore.kernel.org/linux-pci/[email protected]
Changelog v4:
- Rabase onto the laters Frank Li series:
Link: https://lore.kernel.org/all/[email protected]/
- Add Vinod' Ab-tag.
- Rebase onto the kernel v5.19-rcX.
Link: https://lore.kernel.org/linux-pci/[email protected]
Changelog v5:
- Just resend.
- Rebase onto the kernel v6.0-rc2.
Link: https://lore.kernel.org/linux-pci/[email protected]
Changelog v6:
- Fix some patchlog and in-line comments misspells. (@Bjorn)
- Directly call *_dma_configure() method on the DW eDMA channel child
device used for the DMA buffers mapping. (@Robin)
- Explicitly set the DMA-mask of the child device in the channel
allocation proecedure. (@Robin)
- Rebase onto the kernel v6.1-rc3.
Link: https://lore.kernel.org/linux-pci/[email protected]/
Changelog v7:
- Activate the mapping auto-detection procedure for IP-cores older than
5.40a. The viewport-based access has been removed since that
version. (@Yoshihiro)
- Drop the patch
[PATCH v6 22/24] dmaengine: dw-edma: Bypass dma-ranges mapping for the local setup
since the problem has been fixed in the commit f1ad5338a4d5 ("of: Fix
"dma-ranges" handling for bus controllers"). (@Robin)
- Add a new patch:
[PATCH v7 23/25] PCI: dwc: Restore DMA-mask after MSI-data allocation
(@Robin)
- Add a new patch:
[PATCH v7 24/25] PCI: bt1: Set 64-bit DMA-mask
(@Robin)
Link: https://lore.kernel.org/linux-pci/[email protected]/
Changelog v8:
- Add a new patch:
[PATCH v8 23/26] dmaengine: dw-edma: Relax driver config settings
(@tbot)
- Replace the patch
[PATCH v7 23/25] PCI: dwc: Restore DMA-mask after MSI-data allocation
with a new one:
[PATCH v8 24/26] PCI: dwc: Set coherent DMA-mask on MSI-address allocation
(@Robin, @Christoph)
Link: https://lore.kernel.org/linux-pci/[email protected]/
Changelog v9:
- Add a new patch:
[PATCH v9 23/27] dmaengine: dw-edma: Add mem-mapped LL-entries support
(@tbot)
- Rebase onto the kernel 6.2-rc3.
Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Pavel Parkhomenko <[email protected]>
Cc: "Krzysztof Wilczyński" <[email protected]>
Cc: caihuoqing <[email protected]>
Cc: Yoshihiro Shimoda <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Serge Semin (27):
dmaengine: Fix dma_slave_config.dst_addr description
dmaengine: dw-edma: Release requested IRQs on failure
dmaengine: dw-edma: Convert ll/dt phys-address to PCIe bus/DMA address
dmaengine: dw-edma: Fix missing src/dst address of the interleaved
xfers
dmaengine: dw-edma: Don't permit non-inc interleaved xfers
dmaengine: dw-edma: Fix invalid interleaved xfers semantics
dmaengine: dw-edma: Add CPU to PCIe bus address translation
dmaengine: dw-edma: Add PCIe bus address getter to the remote EP
glue-driver
dmaengine: dw-edma: Drop chancnt initialization
dmaengine: dw-edma: Fix DebugFS reg entry type
dmaengine: dw-edma: Stop checking debugfs_create_*() return value
dmaengine: dw-edma: Add dw_edma prefix to the DebugFS nodes descriptor
dmaengine: dw-edma: Convert DebugFS descs to being kz-allocated
dmaengine: dw-edma: Rename DebugFS dentry variables to 'dent'
dmaengine: dw-edma: Simplify the DebugFS context CSRs init procedure
dmaengine: dw-edma: Move eDMA data pointer to DebugFS node descriptor
dmaengine: dw-edma: Join Write/Read channels into a single device
dmaengine: dw-edma: Use DMA-engine device DebugFS subdirectory
dmaengine: dw-edma: Use non-atomic io-64 methods
dmaengine: dw-edma: Drop DT-region allocation
dmaengine: dw-edma: Replace chip ID number with device name
dmaengine: dw-edma: Skip cleanup procedure if no private data found
dmaengine: dw-edma: Add mem-mapped LL-entries support
dmaengine: dw-edma: Relax driver config settings
PCI: dwc: Set coherent DMA-mask on MSI-address allocation
PCI: bt1: Set 64-bit DMA-mask
PCI: dwc: Add DW eDMA engine support
drivers/dma/dw-edma/Kconfig | 5 +-
drivers/dma/dw-edma/dw-edma-core.c | 196 ++++-----
drivers/dma/dw-edma/dw-edma-core.h | 10 +-
drivers/dma/dw-edma/dw-edma-pcie.c | 56 ++-
drivers/dma/dw-edma/dw-edma-v0-core.c | 121 +++---
drivers/dma/dw-edma/dw-edma-v0-core.h | 1 -
drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 372 ++++++++----------
drivers/dma/dw-edma/dw-edma-v0-debugfs.h | 5 -
drivers/pci/controller/dwc/pcie-bt1.c | 4 +
.../pci/controller/dwc/pcie-designware-ep.c | 12 +-
.../pci/controller/dwc/pcie-designware-host.c | 24 +-
drivers/pci/controller/dwc/pcie-designware.c | 195 +++++++++
drivers/pci/controller/dwc/pcie-designware.h | 21 +
include/linux/dma/edma.h | 25 +-
include/linux/dmaengine.h | 2 +-
15 files changed, 652 insertions(+), 397 deletions(-)
--
2.39.0
The rest of the locally defined and used methods and structures have
dw_edma prefix in their names. It's right in accordance with the kernel
coding style to follow the locally defined rule of naming. Let's add that
prefix to the debugfs_entries structure too especially seeing it's name
may be confusing as if that structure belongs to the global DebugFS space.
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
index 6e7f3ef60ca7..2121ffc33cf3 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
@@ -46,7 +46,7 @@ static struct {
void __iomem *end;
} lim[2][EDMA_V0_MAX_NR_CH];
-struct debugfs_entries {
+struct dw_edma_debugfs_entry {
const char *name;
void __iomem *reg;
};
@@ -94,7 +94,7 @@ static int dw_edma_debugfs_u32_get(void *data, u64 *val)
}
DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_edma_debugfs_u32_get, NULL, "0x%08llx\n");
-static void dw_edma_debugfs_create_x32(const struct debugfs_entries entries[],
+static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry entries[],
int nr_entries, struct dentry *dir)
{
int i;
@@ -108,8 +108,7 @@ static void dw_edma_debugfs_create_x32(const struct debugfs_entries entries[],
static void dw_edma_debugfs_regs_ch(struct dw_edma_v0_ch_regs __iomem *regs,
struct dentry *dir)
{
- int nr_entries;
- const struct debugfs_entries debugfs_regs[] = {
+ const struct dw_edma_debugfs_entry debugfs_regs[] = {
REGISTER(ch_control1),
REGISTER(ch_control2),
REGISTER(transfer_size),
@@ -120,6 +119,7 @@ static void dw_edma_debugfs_regs_ch(struct dw_edma_v0_ch_regs __iomem *regs,
REGISTER(llp.lsb),
REGISTER(llp.msb),
};
+ int nr_entries;
nr_entries = ARRAY_SIZE(debugfs_regs);
dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, dir);
@@ -127,7 +127,7 @@ static void dw_edma_debugfs_regs_ch(struct dw_edma_v0_ch_regs __iomem *regs,
static void dw_edma_debugfs_regs_wr(struct dentry *dir)
{
- const struct debugfs_entries debugfs_regs[] = {
+ const struct dw_edma_debugfs_entry debugfs_regs[] = {
/* eDMA global registers */
WR_REGISTER(engine_en),
WR_REGISTER(doorbell),
@@ -148,7 +148,7 @@ static void dw_edma_debugfs_regs_wr(struct dentry *dir)
WR_REGISTER(ch67_imwr_data),
WR_REGISTER(linked_list_err_en),
};
- const struct debugfs_entries debugfs_unroll_regs[] = {
+ const struct dw_edma_debugfs_entry debugfs_unroll_regs[] = {
/* eDMA channel context grouping */
WR_REGISTER_UNROLL(engine_chgroup),
WR_REGISTER_UNROLL(engine_hshake_cnt.lsb),
@@ -191,7 +191,7 @@ static void dw_edma_debugfs_regs_wr(struct dentry *dir)
static void dw_edma_debugfs_regs_rd(struct dentry *dir)
{
- const struct debugfs_entries debugfs_regs[] = {
+ const struct dw_edma_debugfs_entry debugfs_regs[] = {
/* eDMA global registers */
RD_REGISTER(engine_en),
RD_REGISTER(doorbell),
@@ -213,7 +213,7 @@ static void dw_edma_debugfs_regs_rd(struct dentry *dir)
RD_REGISTER(ch45_imwr_data),
RD_REGISTER(ch67_imwr_data),
};
- const struct debugfs_entries debugfs_unroll_regs[] = {
+ const struct dw_edma_debugfs_entry debugfs_unroll_regs[] = {
/* eDMA channel context grouping */
RD_REGISTER_UNROLL(engine_chgroup),
RD_REGISTER_UNROLL(engine_hshake_cnt.lsb),
@@ -256,7 +256,7 @@ static void dw_edma_debugfs_regs_rd(struct dentry *dir)
static void dw_edma_debugfs_regs(void)
{
- const struct debugfs_entries debugfs_regs[] = {
+ const struct dw_edma_debugfs_entry debugfs_regs[] = {
REGISTER(ctrl_data_arb_prior),
REGISTER(ctrl),
};
--
2.39.0
DW eDMA controller always increments both source and destination
addresses. Permitting DMA interleaved transfers with no src_inc/dst_inc
flags set may lead to unexpected behaviour for the device users. Let's fix
that by terminating the interleaved transfers if at least one of the
dma_interleaved_template.{src_inc,dst_inc} flag is initialized with false
value. Note in addition to that we need to increase the source and
destination addresses accordingly after each iteration.
Fixes: 85e7518f42c8 ("dmaengine: dw-edma: Add device_prep_interleave_dma() support")
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-core.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 778d91d9fc1b..35588e14f79a 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -385,6 +385,8 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
return NULL;
if (xfer->xfer.il->numf > 0 && xfer->xfer.il->frame_size > 0)
return NULL;
+ if (!xfer->xfer.il->src_inc || !xfer->xfer.il->dst_inc)
+ return NULL;
} else {
return NULL;
}
@@ -484,15 +486,13 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
struct dma_interleaved_template *il = xfer->xfer.il;
struct data_chunk *dc = &il->sgl[i];
- if (il->src_sgl) {
- src_addr += burst->sz;
+ src_addr += burst->sz;
+ if (il->src_sgl)
src_addr += dmaengine_get_src_icg(il, dc);
- }
- if (il->dst_sgl) {
- dst_addr += burst->sz;
+ dst_addr += burst->sz;
+ if (il->dst_sgl)
dst_addr += dmaengine_get_dst_icg(il, dc);
- }
}
}
--
2.39.0
The interleaved DMA transfer support added in commit 85e7518f42c8
("dmaengine: dw-edma: Add device_prep_interleave_dma() support") seems
contradicting to what the DMA-engine defines. The next conditional
statements:
if (!xfer->xfer.il->numf)
return NULL;
if (xfer->xfer.il->numf > 0 && xfer->xfer.il->frame_size > 0)
return NULL;
basically mean that numf can't be zero and frame_size must always be zero,
otherwise the transfer won't be executed. But further the transfer
execution method takes the frames size from the
dma_interleaved_template.sgl[] array for each frame. That array in
accordance with [1] is supposed to be of
dma_interleaved_template.frame_size size, which as we discovered before
the code expects to be zero. So judging by the dw_edma_device_transfer()
implementation the method implies the dma_interleaved_template.sgl[] array
being of dma_interleaved_template.numf size, which is wrong. Since the
dw_edma_device_transfer() method doesn't permit
dma_interleaved_template.frame_size being non-zero then actual multi-chunk
interleaved transfer turns to be unsupported even though the code implies
having it supported.
Let's fix that by adding a fully functioning support of the interleaved
DMA transfers. First of all dma_interleaved_template.frame_size is
supposed to be greater or equal to one thus having at least simple linear
chunked frames. Secondly we can create a walk-through all over the chunks
and frames just by initializing the number of the eDMA burst transactios
as a multiple of dma_interleaved_template.numf and
dma_interleaved_template.frame_size and getting the frame_size-modulo of
the iteration step as an index of the dma_interleaved_template.sgl[]
array. The rest of the dw_edma_device_transfer() method code can be left
unchanged.
[1] include/linux/dmaengine.h: doc struct dma_interleaved_template
Fixes: 85e7518f42c8 ("dmaengine: dw-edma: Add device_prep_interleave_dma() support")
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-core.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 35588e14f79a..d5c4192141ef 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -332,6 +332,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
struct dw_edma_chunk *chunk;
struct dw_edma_burst *burst;
struct dw_edma_desc *desc;
+ size_t fsz = 0;
u32 cnt = 0;
int i;
@@ -381,9 +382,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
if (xfer->xfer.sg.len < 1)
return NULL;
} else if (xfer->type == EDMA_XFER_INTERLEAVED) {
- if (!xfer->xfer.il->numf)
- return NULL;
- if (xfer->xfer.il->numf > 0 && xfer->xfer.il->frame_size > 0)
+ if (!xfer->xfer.il->numf || xfer->xfer.il->frame_size < 1)
return NULL;
if (!xfer->xfer.il->src_inc || !xfer->xfer.il->dst_inc)
return NULL;
@@ -413,10 +412,8 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
cnt = xfer->xfer.sg.len;
sg = xfer->xfer.sg.sgl;
} else if (xfer->type == EDMA_XFER_INTERLEAVED) {
- if (xfer->xfer.il->numf > 0)
- cnt = xfer->xfer.il->numf;
- else
- cnt = xfer->xfer.il->frame_size;
+ cnt = xfer->xfer.il->numf * xfer->xfer.il->frame_size;
+ fsz = xfer->xfer.il->frame_size;
}
for (i = 0; i < cnt; i++) {
@@ -438,7 +435,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
else if (xfer->type == EDMA_XFER_SCATTER_GATHER)
burst->sz = sg_dma_len(sg);
else if (xfer->type == EDMA_XFER_INTERLEAVED)
- burst->sz = xfer->xfer.il->sgl[i].size;
+ burst->sz = xfer->xfer.il->sgl[i % fsz].size;
chunk->ll_region.sz += burst->sz;
desc->alloc_sz += burst->sz;
@@ -481,10 +478,9 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
if (xfer->type == EDMA_XFER_SCATTER_GATHER) {
sg = sg_next(sg);
- } else if (xfer->type == EDMA_XFER_INTERLEAVED &&
- xfer->xfer.il->frame_size > 0) {
+ } else if (xfer->type == EDMA_XFER_INTERLEAVED) {
struct dma_interleaved_template *il = xfer->xfer.il;
- struct data_chunk *dc = &il->sgl[i];
+ struct data_chunk *dc = &il->sgl[i % fsz];
src_addr += burst->sz;
if (il->src_sgl)
--
2.39.0
First of all they never return NULL. So checking their return value for
being not NULL just pointless. Secondly the DebugFS subsystem is designed
in a way to be used as simple as possible. So if one of the
debugfs_create_*() method in a hierarchy fails, the following methods will
just silently return the passed erroneous parental dentry. Finally the
code is supposed to be working no matter whether anything DebugFS-related
fails. So in order to make code simpler and DebugFS-independent let's drop
the debugfs_create_*() methods return value checking in the same way as
the most of the kernel drivers do.
Note in order to preserve some memory space we suggest to skip the DebugFS
nodes initialization if the file system in unavailable.
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
index 8e61810dea4b..6e7f3ef60ca7 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
@@ -100,9 +100,8 @@ static void dw_edma_debugfs_create_x32(const struct debugfs_entries entries[],
int i;
for (i = 0; i < nr_entries; i++) {
- if (!debugfs_create_file_unsafe(entries[i].name, 0444, dir,
- entries[i].reg, &fops_x32))
- break;
+ debugfs_create_file_unsafe(entries[i].name, 0444, dir,
+ entries[i].reg, &fops_x32);
}
}
@@ -168,8 +167,6 @@ static void dw_edma_debugfs_regs_wr(struct dentry *dir)
char name[16];
regs_dir = debugfs_create_dir(WRITE_STR, dir);
- if (!regs_dir)
- return;
nr_entries = ARRAY_SIZE(debugfs_regs);
dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir);
@@ -184,8 +181,6 @@ static void dw_edma_debugfs_regs_wr(struct dentry *dir)
snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
ch_dir = debugfs_create_dir(name, regs_dir);
- if (!ch_dir)
- return;
dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].wr, ch_dir);
@@ -237,8 +232,6 @@ static void dw_edma_debugfs_regs_rd(struct dentry *dir)
char name[16];
regs_dir = debugfs_create_dir(READ_STR, dir);
- if (!regs_dir)
- return;
nr_entries = ARRAY_SIZE(debugfs_regs);
dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir);
@@ -253,8 +246,6 @@ static void dw_edma_debugfs_regs_rd(struct dentry *dir)
snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
ch_dir = debugfs_create_dir(name, regs_dir);
- if (!ch_dir)
- return;
dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].rd, ch_dir);
@@ -273,8 +264,6 @@ static void dw_edma_debugfs_regs(void)
int nr_entries;
regs_dir = debugfs_create_dir(REGISTERS_STR, dw->debugfs);
- if (!regs_dir)
- return;
nr_entries = ARRAY_SIZE(debugfs_regs);
dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir);
@@ -285,6 +274,9 @@ static void dw_edma_debugfs_regs(void)
void dw_edma_v0_debugfs_on(struct dw_edma *_dw)
{
+ if (!debugfs_initialized())
+ return;
+
dw = _dw;
if (!dw)
return;
@@ -294,8 +286,6 @@ void dw_edma_v0_debugfs_on(struct dw_edma *_dw)
return;
dw->debugfs = debugfs_create_dir(dw->name, NULL);
- if (!dw->debugfs)
- return;
debugfs_create_u32("mf", 0444, dw->debugfs, &dw->chip->mf);
debugfs_create_u16("wr_ch_cnt", 0444, dw->debugfs, &dw->wr_ch_cnt);
--
2.39.0
The MSI target address requires to be reserved within the lowest 4GB
memory in order to support the PCIe peripherals with no 64-bit MSI TLPs
support. Since the allocation is done from the DMA-coherent memory let's
modify the allocation procedure to setting the coherent DMA-mask only and
avoiding the streaming DMA-mask modification. Thus at least the streaming
DMA operations would work with no artificial limitations. It will be
specifically useful for the eDMA-capable controllers so the corresponding
DMA-engine clients would map the DMA buffers with no need in the SWIOTLB
intervention for the buffers allocated above the 4GB memory region.
While at it let's add a brief comment about the reason of having the MSI
target address allocated from the DMA-coherent memory limited with the 4GB
upper bound.
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Robin Murphy <[email protected]>
---
Changelog v8:
- This is a new patch added on v8 stage of the series.
(@Robin, @Christoph)
---
drivers/pci/controller/dwc/pcie-designware-host.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 3ab6ae3712c4..e10608af39b4 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -366,7 +366,16 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
dw_chained_msi_isr, pp);
}
- ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+ /*
+ * Even though the iMSI-RX Module supports 64-bit addresses some
+ * peripheral PCIe devices may lack the 64-bit messages support. In
+ * order not to miss MSI TLPs from those devices the MSI target address
+ * has to be reserved within the lowest 4GB.
+ * Note until there is a better alternative found the reservation is
+ * done by allocating from the artificially limited DMA-coherent
+ * memory.
+ */
+ ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
if (ret)
dev_warn(dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
--
2.39.0
In general the Synopsys PCIe EndPoint IP prototype kit can be attached to
a PCIe bus with any PCIe Host controller including to the one with
distinctive from CPU address space. Due to that we need to make sure that
the source and destination addresses of the DMA-slave devices are properly
converted to the PCIe bus address space, otherwise the DMA transaction
will not only work as expected, but may cause the memory corruption with
subsequent system crash. Let's do that by introducing a new
dw_edma_pcie_address() method defined in the dw-edma-pcie.c, which will
perform the denoted translation by using the pcibios_resource_to_bus()
method.
Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
Note this patch depends on the patch "dmaengine: dw-edma: Add CPU to PCIe
bus address translation" from this series.
---
drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 04c95cba1244..f530bacfd716 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -95,8 +95,23 @@ static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
return pci_irq_vector(to_pci_dev(dev), nr);
}
+static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_bus_region region;
+ struct resource res = {
+ .flags = IORESOURCE_MEM,
+ .start = cpu_addr,
+ .end = cpu_addr,
+ };
+
+ pcibios_resource_to_bus(pdev->bus, ®ion, &res);
+ return region.start;
+}
+
static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
.irq_vector = dw_edma_pcie_irq_vector,
+ .pci_address = dw_edma_pcie_address,
};
static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
--
2.39.0
The interleaved DMA transfers support was added in the commit 85e7518f42c8
("dmaengine: dw-edma: Add device_prep_interleave_dma() support"). It
seems like the support was broken from the very beginning. Depending on
the selected channel either source or destination address are left
uninitialized which was obviously wrong. I don't really know how come the
original modification was working for the commit author. Anyway let's fix
it by initializing the destination address of the eDMA burst descriptors
for the DEV_TO_MEM interleaved operations and by initializing the source
address of the eDMA burst descriptors for the MEM_TO_DEV interleaved
operations.
Fixes: 85e7518f42c8 ("dmaengine: dw-edma: Add device_prep_interleave_dma() support")
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index a8c1bd9c7ae9..778d91d9fc1b 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -455,6 +455,8 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
* and destination addresses are increased
* by the same portion (data length)
*/
+ } else if (xfer->type == EDMA_XFER_INTERLEAVED) {
+ burst->dar = dst_addr;
}
} else {
burst->dar = dst_addr;
@@ -470,6 +472,8 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
* and destination addresses are increased
* by the same portion (data length)
*/
+ } else if (xfer->type == EDMA_XFER_INTERLEAVED) {
+ burst->sar = src_addr;
}
}
--
2.39.0
Currently all the DW eDMA DebugFS nodes descriptors are allocated on
stack, while the DW eDMA driver private data and CSR limits are statically
preserved. Such design won't work for the multi-eDMA platforms. As a
preparation to adding the multi-eDMA system setups support we need to have
each DebugFS node separately allocated and described. Afterwards we'll put
an addition info there like Read/Write channel flag, channel ID, DW eDMA
private data reference.
Note this conversion is mainly required due to having the legacy DW eDMA
controllers with indirect Read/Write channels context CSRs access. If we
didn't need to have a synchronized access to these registers the DebugFS
code of the driver would have been much simpler.
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
Changelog v2:
- Drop __iomem qualifier from the struct dw_edma_debugfs_entry instance
definition in the dw_edma_debugfs_u32_get() method. (@Manivannan)
---
drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
index 2121ffc33cf3..78f15e4b07ac 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
@@ -53,7 +53,8 @@ struct dw_edma_debugfs_entry {
static int dw_edma_debugfs_u32_get(void *data, u64 *val)
{
- void __iomem *reg = data;
+ struct dw_edma_debugfs_entry *entry = data;
+ void __iomem *reg = entry->reg;
if (dw->chip->mf == EDMA_MF_EDMA_LEGACY &&
reg >= (void __iomem *)®s->type.legacy.ch) {
@@ -94,14 +95,22 @@ static int dw_edma_debugfs_u32_get(void *data, u64 *val)
}
DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_edma_debugfs_u32_get, NULL, "0x%08llx\n");
-static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry entries[],
+static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry ini[],
int nr_entries, struct dentry *dir)
{
+ struct dw_edma_debugfs_entry *entries;
int i;
+ entries = devm_kcalloc(dw->chip->dev, nr_entries, sizeof(*entries),
+ GFP_KERNEL);
+ if (!entries)
+ return;
+
for (i = 0; i < nr_entries; i++) {
+ entries[i] = ini[i];
+
debugfs_create_file_unsafe(entries[i].name, 0444, dir,
- entries[i].reg, &fops_x32);
+ &entries[i], &fops_x32);
}
}
--
2.39.0
From very beginning of the DW eDMA driver live in the kernel the method
dw_edma_irq_request() hasn't been designed quite correct. In case if the
request_irq() method fails to initialize the IRQ handler at some point the
previously requested IRQs will be left initialized. It's prune to errors
up to the system crash. Let's fix that by releasing the previously
requested IRQs in the cleanup-on-error path of the dw_edma_irq_request()
function.
Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
Changelog v2:
- This is a new patch added in v2 iteration of the series.
---
drivers/dma/dw-edma/dw-edma-core.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index c54b24ff5206..a8c1bd9c7ae9 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -893,10 +893,8 @@ static int dw_edma_irq_request(struct dw_edma *dw,
dw_edma_interrupt_read,
IRQF_SHARED, dw->name,
&dw->irq[i]);
- if (err) {
- dw->nr_irqs = i;
- return err;
- }
+ if (err)
+ goto err_irq_free;
if (irq_get_msi_desc(irq))
get_cached_msi_msg(irq, &dw->irq[i].msi);
@@ -905,6 +903,14 @@ static int dw_edma_irq_request(struct dw_edma *dw,
dw->nr_irqs = i;
}
+ return 0;
+
+err_irq_free:
+ for (i--; i >= 0; i--) {
+ irq = chip->ops->irq_vector(dev, i);
+ free_irq(irq, &dw->irq[i]);
+ }
+
return err;
}
--
2.39.0
Starting from commit 9575632052ba ("dmaengine: make slave address
physical") the source and destination addresses of the DMA-slave device
have been converted to being defined in CPU address space. It's DMA-device
driver responsibility to properly convert them to the reachable DMA bus
spaces. In case of the DW eDMA device, the source or destination
peripheral (slave) devices reside PCIe bus space. Thus we need to perform
the PCIe Host/EP windows-based (i.e. ranges DT-property) addresses
translation otherwise the eDMA transactions won't work as expected (or can
be even harmful) in case if the CPU and PCIe address spaces don't match.
Note 1. Even though the DMA interleaved template has both source and
destination addresses declared of dma_addr_t type only CPU memory range is
supposed to be mapped in a way so to be seen by the DMA device since it's
a subject of the DMA getting towards the system side. The device part must
not be mapped since slave device resides in the PCIe bus space, which
isn't affected by IOMMUs or iATU translations. DW PCIe eDMA generates
corresponding MWr/MRd TLPs on its own.
Note 2. This functionality is mainly required for the remote eDMA setup
since the CPU address must be manually translated into the PCIe bus space
before being written to LLI.{SAR,DAR}. If eDMA is embedded into the
locally accessible DW PCIe RP/EP software-based translation isn't required
since it will be done by hardware by means of the Outbound iATU as long as
the DMA_BYPASS flag is cleared. If the later flag is set or there is no
Outbound iATU entry found to which the SAR or DAR falls in (for Read and
Write channel respectfully), there won't be any translation performed but
DMA will proceed with the corresponding source/destination address as is.
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-core.c | 18 +++++++++++++++++-
include/linux/dma/edma.h | 15 +++++++++++++++
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index d5c4192141ef..6c9f95a8e397 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -39,6 +39,17 @@ struct dw_edma_desc *vd2dw_edma_desc(struct virt_dma_desc *vd)
return container_of(vd, struct dw_edma_desc, vd);
}
+static inline
+u64 dw_edma_get_pci_address(struct dw_edma_chan *chan, phys_addr_t cpu_addr)
+{
+ struct dw_edma_chip *chip = chan->dw->chip;
+
+ if (chip->ops->pci_address)
+ return chip->ops->pci_address(chip->dev, cpu_addr);
+
+ return cpu_addr;
+}
+
static struct dw_edma_burst *dw_edma_alloc_burst(struct dw_edma_chunk *chunk)
{
struct dw_edma_burst *burst;
@@ -327,11 +338,11 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
{
struct dw_edma_chan *chan = dchan2dw_edma_chan(xfer->dchan);
enum dma_transfer_direction dir = xfer->direction;
- phys_addr_t src_addr, dst_addr;
struct scatterlist *sg = NULL;
struct dw_edma_chunk *chunk;
struct dw_edma_burst *burst;
struct dw_edma_desc *desc;
+ u64 src_addr, dst_addr;
size_t fsz = 0;
u32 cnt = 0;
int i;
@@ -406,6 +417,11 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
dst_addr = chan->config.dst_addr;
}
+ if (dir == DMA_DEV_TO_MEM)
+ src_addr = dw_edma_get_pci_address(chan, (phys_addr_t)src_addr);
+ else
+ dst_addr = dw_edma_get_pci_address(chan, (phys_addr_t)dst_addr);
+
if (xfer->type == EDMA_XFER_CYCLIC) {
cnt = xfer->xfer.cyclic.cnt;
} else if (xfer->type == EDMA_XFER_SCATTER_GATHER) {
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index a864978ddd27..380a0a3e251f 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -23,8 +23,23 @@ struct dw_edma_region {
size_t sz;
};
+/**
+ * struct dw_edma_core_ops - platform-specific eDMA methods
+ * @irq_vector: Get IRQ number of the passed eDMA channel. Note the
+ * method accepts the channel id in the end-to-end
+ * numbering with the eDMA write channels being placed
+ * first in the row.
+ * @pci_address: Get PCIe bus address corresponding to the passed CPU
+ * address. Note there is no need in specifying this
+ * function if the address translation is performed by
+ * the DW PCIe RP/EP controller with the DW eDMA device in
+ * subject and DMA_BYPASS isn't set for all the outbound
+ * iATU windows. That will be done by the controller
+ * automatically.
+ */
struct dw_edma_core_ops {
int (*irq_vector)(struct device *dev, unsigned int nr);
+ u64 (*pci_address)(struct device *dev, phys_addr_t cpu_addr);
};
enum dw_edma_map_format {
--
2.39.0
Since the DW eDMA driver now supports eDMA controllers embedded into the
locally accessible DW PCIe Root Ports and Endpoints, we can use the
updated interface to register DW eDMA as DMA engine device if it's
available. In order to successfully do that the DW PCIe core driver need
to perform some preparations first. First of all it needs to find out the
eDMA controller CSRs base address, whether they are accessible over the
Port Logic or iATU unrolled space. Afterwards it can try to auto-detect
the eDMA controller availability and number of read/write channels.
If none was found the procedure will just silently halt with no error
returned. Secondly the platform is supposed to provide either combined or
per-channel IRQ signals. If no valid IRQs set is found the procedure will
also halt with no error returned so to be backward compatible with the
platforms where DW PCIe controllers have eDMA embedded but lack of the
IRQs defined for them. Finally before actually probing the eDMA device we
need to allocate LLP items buffers. After that the DW eDMA can be
registered. If registration is successful the info-message regarding the
number of detected Read/Write eDMA channels will be printed to the system
as is done for the iATU settings.
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
Changelog v2:
- Don't fail eDMA detection procedure if the DW eDMA driver couldn't probe
device. That happens if the driver is disabled. (@Manivannan)
- Add "dma" registers resource mapping procedure. (@Manivannan)
- Move the eDMA CSRs space detection into the dw_pcie_map_detect() method.
- Remove eDMA on the dw_pcie_ep_init() internal errors. (@Manivannan)
- Remove eDMA in the dw_pcie_ep_exit() method.
- Move the dw_pcie_edma_detect() method execution to the tail of the
dw_pcie_ep_init() function.
Changelog v3:
- Add more comprehensive and less regression prune eDMA block detection
procedure.
- Remove Manivannan tb tag since the patch content has been changed.
Changelog v6:
- Fix some patchlog and in-line comments misspells. (@Bjorn)
Changelog v7:
- Activate the mapping auto-detection procedure for IP-cores older than
5.40a since the viewport-based access has been removed since that
version. (@Yoshihiro)
---
.../pci/controller/dwc/pcie-designware-ep.c | 12 +-
.../pci/controller/dwc/pcie-designware-host.c | 13 +-
drivers/pci/controller/dwc/pcie-designware.c | 195 ++++++++++++++++++
drivers/pci/controller/dwc/pcie-designware.h | 21 ++
4 files changed, 238 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index d06654895eba..f9182f8d552f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -612,8 +612,11 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
struct pci_epc *epc = ep->epc;
+ dw_pcie_edma_remove(pci);
+
pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
epc->mem->window.page_size);
@@ -785,6 +788,10 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
goto err_exit_epc_mem;
}
+ ret = dw_pcie_edma_detect(pci);
+ if (ret)
+ goto err_free_epc_mem;
+
if (ep->ops->get_features) {
epc_features = ep->ops->get_features(ep);
if (epc_features->core_init_notifier)
@@ -793,10 +800,13 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
ret = dw_pcie_ep_init_complete(ep);
if (ret)
- goto err_free_epc_mem;
+ goto err_remove_edma;
return 0;
+err_remove_edma:
+ dw_pcie_edma_remove(pci);
+
err_free_epc_mem:
pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
epc->mem->window.page_size);
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index e10608af39b4..a9e3dda36c21 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -476,14 +476,18 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
dw_pcie_iatu_detect(pci);
- ret = dw_pcie_setup_rc(pp);
+ ret = dw_pcie_edma_detect(pci);
if (ret)
goto err_free_msi;
+ ret = dw_pcie_setup_rc(pp);
+ if (ret)
+ goto err_remove_edma;
+
if (!dw_pcie_link_up(pci)) {
ret = dw_pcie_start_link(pci);
if (ret)
- goto err_free_msi;
+ goto err_remove_edma;
}
/* Ignore errors, the link may come up later */
@@ -500,6 +504,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
err_stop_link:
dw_pcie_stop_link(pci);
+err_remove_edma:
+ dw_pcie_edma_remove(pci);
+
err_free_msi:
if (pp->has_msi_ctrl)
dw_pcie_free_msi(pp);
@@ -521,6 +528,8 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
dw_pcie_stop_link(pci);
+ dw_pcie_edma_remove(pci);
+
if (pp->has_msi_ctrl)
dw_pcie_free_msi(pp);
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 6d5d619ab2e9..53a16b8b6ac2 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -12,6 +12,7 @@
#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/delay.h>
+#include <linux/dma/edma.h>
#include <linux/gpio/consumer.h>
#include <linux/ioport.h>
#include <linux/of.h>
@@ -142,6 +143,18 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
if (!pci->atu_size)
pci->atu_size = SZ_4K;
+ /* eDMA region can be mapped to a custom base address */
+ if (!pci->edma.reg_base) {
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dma");
+ if (res) {
+ pci->edma.reg_base = devm_ioremap_resource(pci->dev, res);
+ if (IS_ERR(pci->edma.reg_base))
+ return PTR_ERR(pci->edma.reg_base);
+ } else if (pci->atu_size >= 2 * DEFAULT_DBI_DMA_OFFSET) {
+ pci->edma.reg_base = pci->atu_base + DEFAULT_DBI_DMA_OFFSET;
+ }
+ }
+
/* LLDD is supposed to manually switch the clocks and resets state */
if (dw_pcie_cap_is(pci, REQ_RES)) {
ret = dw_pcie_get_clocks(pci);
@@ -782,6 +795,188 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci)
pci->region_align / SZ_1K, (pci->region_limit + 1) / SZ_1G);
}
+static u32 dw_pcie_readl_dma(struct dw_pcie *pci, u32 reg)
+{
+ u32 val = 0;
+ int ret;
+
+ if (pci->ops && pci->ops->read_dbi)
+ return pci->ops->read_dbi(pci, pci->edma.reg_base, reg, 4);
+
+ ret = dw_pcie_read(pci->edma.reg_base + reg, 4, &val);
+ if (ret)
+ dev_err(pci->dev, "Read DMA address failed\n");
+
+ return val;
+}
+
+static int dw_pcie_edma_irq_vector(struct device *dev, unsigned int nr)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ char name[6];
+ int ret;
+
+ if (nr >= EDMA_MAX_WR_CH + EDMA_MAX_RD_CH)
+ return -EINVAL;
+
+ ret = platform_get_irq_byname_optional(pdev, "dma");
+ if (ret > 0)
+ return ret;
+
+ snprintf(name, sizeof(name), "dma%u", nr);
+
+ return platform_get_irq_byname_optional(pdev, name);
+}
+
+static struct dw_edma_core_ops dw_pcie_edma_ops = {
+ .irq_vector = dw_pcie_edma_irq_vector,
+};
+
+static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
+{
+ u32 val;
+
+ /*
+ * Indirect eDMA CSRs access has been completely removed since v5.40a
+ * thus no space is now reserved for the eDMA channels viewport and
+ * former DMA CTRL register is no longer fixed to FFs.
+ */
+ if (dw_pcie_ver_is_ge(pci, 540A))
+ val = 0xFFFFFFFF;
+ else
+ val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
+
+ if (val == 0xFFFFFFFF && pci->edma.reg_base) {
+ pci->edma.mf = EDMA_MF_EDMA_UNROLL;
+
+ val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
+ } else if (val != 0xFFFFFFFF) {
+ pci->edma.mf = EDMA_MF_EDMA_LEGACY;
+
+ pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
+ } else {
+ return -ENODEV;
+ }
+
+ pci->edma.dev = pci->dev;
+
+ if (!pci->edma.ops)
+ pci->edma.ops = &dw_pcie_edma_ops;
+
+ pci->edma.flags |= DW_EDMA_CHIP_LOCAL;
+
+ pci->edma.ll_wr_cnt = FIELD_GET(PCIE_DMA_NUM_WR_CHAN, val);
+ pci->edma.ll_rd_cnt = FIELD_GET(PCIE_DMA_NUM_RD_CHAN, val);
+
+ /* Sanity check the channels count if the mapping was incorrect */
+ if (!pci->edma.ll_wr_cnt || pci->edma.ll_wr_cnt > EDMA_MAX_WR_CH ||
+ !pci->edma.ll_rd_cnt || pci->edma.ll_rd_cnt > EDMA_MAX_RD_CH)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int dw_pcie_edma_irq_verify(struct dw_pcie *pci)
+{
+ struct platform_device *pdev = to_platform_device(pci->dev);
+ u16 ch_cnt = pci->edma.ll_wr_cnt + pci->edma.ll_rd_cnt;
+ char name[6];
+ int ret;
+
+ if (pci->edma.nr_irqs == 1)
+ return 0;
+ else if (pci->edma.nr_irqs > 1)
+ return pci->edma.nr_irqs != ch_cnt ? -EINVAL : 0;
+
+ ret = platform_get_irq_byname_optional(pdev, "dma");
+ if (ret > 0) {
+ pci->edma.nr_irqs = 1;
+ return 0;
+ }
+
+ for (; pci->edma.nr_irqs < ch_cnt; pci->edma.nr_irqs++) {
+ snprintf(name, sizeof(name), "dma%d", pci->edma.nr_irqs);
+
+ ret = platform_get_irq_byname_optional(pdev, name);
+ if (ret <= 0)
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int dw_pcie_edma_ll_alloc(struct dw_pcie *pci)
+{
+ struct dw_edma_region *ll;
+ dma_addr_t paddr;
+ int i;
+
+ for (i = 0; i < pci->edma.ll_wr_cnt; i++) {
+ ll = &pci->edma.ll_region_wr[i];
+ ll->sz = DMA_LLP_MEM_SIZE;
+ ll->vaddr.mem = dmam_alloc_coherent(pci->dev, ll->sz,
+ &paddr, GFP_KERNEL);
+ if (!ll->vaddr.mem)
+ return -ENOMEM;
+
+ ll->paddr = paddr;
+ }
+
+ for (i = 0; i < pci->edma.ll_rd_cnt; i++) {
+ ll = &pci->edma.ll_region_rd[i];
+ ll->sz = DMA_LLP_MEM_SIZE;
+ ll->vaddr.mem = dmam_alloc_coherent(pci->dev, ll->sz,
+ &paddr, GFP_KERNEL);
+ if (!ll->vaddr.mem)
+ return -ENOMEM;
+
+ ll->paddr = paddr;
+ }
+
+ return 0;
+}
+
+int dw_pcie_edma_detect(struct dw_pcie *pci)
+{
+ int ret;
+
+ /* Don't fail if no eDMA was found (for the backward compatibility) */
+ ret = dw_pcie_edma_find_chip(pci);
+ if (ret)
+ return 0;
+
+ /* Don't fail on the IRQs verification (for the backward compatibility) */
+ ret = dw_pcie_edma_irq_verify(pci);
+ if (ret) {
+ dev_err(pci->dev, "Invalid eDMA IRQs found\n");
+ return 0;
+ }
+
+ ret = dw_pcie_edma_ll_alloc(pci);
+ if (ret) {
+ dev_err(pci->dev, "Couldn't allocate LLP memory\n");
+ return ret;
+ }
+
+ /* Don't fail if the DW eDMA driver can't find the device */
+ ret = dw_edma_probe(&pci->edma);
+ if (ret && ret != -ENODEV) {
+ dev_err(pci->dev, "Couldn't register eDMA device\n");
+ return ret;
+ }
+
+ dev_info(pci->dev, "eDMA: unroll %s, %hu wr, %hu rd\n",
+ pci->edma.mf == EDMA_MF_EDMA_UNROLL ? "T" : "F",
+ pci->edma.ll_wr_cnt, pci->edma.ll_rd_cnt);
+
+ return 0;
+}
+
+void dw_pcie_edma_remove(struct dw_pcie *pci)
+{
+ dw_edma_remove(&pci->edma);
+}
+
void dw_pcie_setup(struct dw_pcie *pci)
{
u32 val;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 393dfb931df6..79713ce075cc 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -15,6 +15,7 @@
#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/dma-mapping.h>
+#include <linux/dma/edma.h>
#include <linux/gpio/consumer.h>
#include <linux/irq.h>
#include <linux/msi.h>
@@ -31,6 +32,7 @@
#define DW_PCIE_VER_480A 0x3438302a
#define DW_PCIE_VER_490A 0x3439302a
#define DW_PCIE_VER_520A 0x3532302a
+#define DW_PCIE_VER_540A 0x3534302a
#define __dw_pcie_ver_cmp(_pci, _ver, _op) \
((_pci)->version _op DW_PCIE_VER_ ## _ver)
@@ -167,6 +169,18 @@
#define PCIE_MSIX_DOORBELL 0x948
#define PCIE_MSIX_DOORBELL_PF_SHIFT 24
+/*
+ * eDMA CSRs. DW PCIe IP-core v4.70a and older had the eDMA registers accessible
+ * over the Port Logic registers space. Afterwards the unrolled mapping was
+ * introduced so eDMA and iATU could be accessed via a dedicated registers
+ * space.
+ */
+#define PCIE_DMA_VIEWPORT_BASE 0x970
+#define PCIE_DMA_UNROLL_BASE 0x80000
+#define PCIE_DMA_CTRL 0x008
+#define PCIE_DMA_NUM_WR_CHAN GENMASK(3, 0)
+#define PCIE_DMA_NUM_RD_CHAN GENMASK(19, 16)
+
#define PCIE_PL_CHK_REG_CONTROL_STATUS 0xB20
#define PCIE_PL_CHK_REG_CHK_REG_START BIT(0)
#define PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS BIT(1)
@@ -215,6 +229,7 @@
* this offset, if atu_base not set.
*/
#define DEFAULT_DBI_ATU_OFFSET (0x3 << 20)
+#define DEFAULT_DBI_DMA_OFFSET PCIE_DMA_UNROLL_BASE
#define MAX_MSI_IRQS 256
#define MAX_MSI_IRQS_PER_CTRL 32
@@ -226,6 +241,9 @@
#define MAX_IATU_IN 256
#define MAX_IATU_OUT 256
+/* Default eDMA LLP memory size */
+#define DMA_LLP_MEM_SIZE PAGE_SIZE
+
struct dw_pcie;
struct dw_pcie_rp;
struct dw_pcie_ep;
@@ -369,6 +387,7 @@ struct dw_pcie {
int num_lanes;
int link_gen;
u8 n_fts[2];
+ struct dw_edma_chip edma;
struct clk_bulk_data app_clks[DW_PCIE_NUM_APP_CLKS];
struct clk_bulk_data core_clks[DW_PCIE_NUM_CORE_CLKS];
struct reset_control_bulk_data app_rsts[DW_PCIE_NUM_APP_RSTS];
@@ -408,6 +427,8 @@ int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index);
void dw_pcie_setup(struct dw_pcie *pci);
void dw_pcie_iatu_detect(struct dw_pcie *pci);
+int dw_pcie_edma_detect(struct dw_pcie *pci);
+void dw_pcie_edma_remove(struct dw_pcie *pci);
static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
{
--
2.39.0
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 <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-core.c | 116 +++++++++++++++--------------
drivers/dma/dw-edma/dw-edma-core.h | 5 +-
2 files changed, 61 insertions(+), 60 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index ecd3e8f7ac5d..c3ecae4287d0 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -208,6 +208,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)
{
@@ -717,8 +735,7 @@ static void dw_edma_free_chan_resources(struct dma_chan *dchan)
}
}
-static int dw_edma_channel_setup(struct dw_edma *dw, bool write,
- u32 wr_alloc, u32 rd_alloc)
+static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
{
struct dw_edma_chip *chip = dw->chip;
struct dw_edma_region *dt_region;
@@ -726,27 +743,15 @@ static int dw_edma_channel_setup(struct dw_edma *dw, 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 || dw->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);
@@ -756,52 +761,62 @@ static int dw_edma_channel_setup(struct dw_edma *dw, 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 (dw->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);
@@ -813,7 +828,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, 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;
@@ -822,6 +837,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, 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;
@@ -835,9 +851,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, 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 +996,8 @@ int dw_edma_probe(struct dw_edma_chip *chip)
if (err)
return err;
- /* Setup write channels */
- err = dw_edma_channel_setup(dw, true, wr_alloc, rd_alloc);
- if (err)
- goto err_irq_free;
-
- /* Setup read channels */
- err = dw_edma_channel_setup(dw, false, wr_alloc, rd_alloc);
+ /* Setup write/read channels */
+ err = dw_edma_channel_setup(dw, wr_alloc, rd_alloc);
if (err)
goto err_irq_free;
@@ -1022,15 +1031,8 @@ int dw_edma_remove(struct dw_edma_chip *chip)
free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
/* 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 85df2d511907..b576a8fff45a 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.39.0
Since we are about to add the eDMA channels direction support to the
debugfs module it will be confusing to have both the DebugFS directory and
the channels direction short names used in the same code. As a preparation
patch let's convert the DebugFS dentry 'dir' variables to having the
'dent' name so to prevent the confusion.
Suggested-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
Changelog v2:
- This is a new patch added in v2. (@Manivannan)
---
drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 46 ++++++++++++------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
index 78f15e4b07ac..7bb3363b40e4 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
@@ -96,7 +96,7 @@ static int dw_edma_debugfs_u32_get(void *data, u64 *val)
DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_edma_debugfs_u32_get, NULL, "0x%08llx\n");
static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry ini[],
- int nr_entries, struct dentry *dir)
+ int nr_entries, struct dentry *dent)
{
struct dw_edma_debugfs_entry *entries;
int i;
@@ -109,13 +109,13 @@ static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry ini[],
for (i = 0; i < nr_entries; i++) {
entries[i] = ini[i];
- debugfs_create_file_unsafe(entries[i].name, 0444, dir,
+ debugfs_create_file_unsafe(entries[i].name, 0444, dent,
&entries[i], &fops_x32);
}
}
static void dw_edma_debugfs_regs_ch(struct dw_edma_v0_ch_regs __iomem *regs,
- struct dentry *dir)
+ struct dentry *dent)
{
const struct dw_edma_debugfs_entry debugfs_regs[] = {
REGISTER(ch_control1),
@@ -131,10 +131,10 @@ static void dw_edma_debugfs_regs_ch(struct dw_edma_v0_ch_regs __iomem *regs,
int nr_entries;
nr_entries = ARRAY_SIZE(debugfs_regs);
- dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, dir);
+ dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, dent);
}
-static void dw_edma_debugfs_regs_wr(struct dentry *dir)
+static void dw_edma_debugfs_regs_wr(struct dentry *dent)
{
const struct dw_edma_debugfs_entry debugfs_regs[] = {
/* eDMA global registers */
@@ -171,34 +171,34 @@ static void dw_edma_debugfs_regs_wr(struct dentry *dir)
WR_REGISTER_UNROLL(ch6_pwr_en),
WR_REGISTER_UNROLL(ch7_pwr_en),
};
- struct dentry *regs_dir, *ch_dir;
+ struct dentry *regs_dent, *ch_dent;
int nr_entries, i;
char name[16];
- regs_dir = debugfs_create_dir(WRITE_STR, dir);
+ regs_dent = debugfs_create_dir(WRITE_STR, dent);
nr_entries = ARRAY_SIZE(debugfs_regs);
- dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir);
+ dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dent);
if (dw->chip->mf == EDMA_MF_HDMA_COMPAT) {
nr_entries = ARRAY_SIZE(debugfs_unroll_regs);
dw_edma_debugfs_create_x32(debugfs_unroll_regs, nr_entries,
- regs_dir);
+ regs_dent);
}
for (i = 0; i < dw->wr_ch_cnt; i++) {
snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
- ch_dir = debugfs_create_dir(name, regs_dir);
+ ch_dent = debugfs_create_dir(name, regs_dent);
- dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].wr, ch_dir);
+ dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].wr, ch_dent);
lim[0][i].start = ®s->type.unroll.ch[i].wr;
lim[0][i].end = ®s->type.unroll.ch[i].padding_1[0];
}
}
-static void dw_edma_debugfs_regs_rd(struct dentry *dir)
+static void dw_edma_debugfs_regs_rd(struct dentry *dent)
{
const struct dw_edma_debugfs_entry debugfs_regs[] = {
/* eDMA global registers */
@@ -236,27 +236,27 @@ static void dw_edma_debugfs_regs_rd(struct dentry *dir)
RD_REGISTER_UNROLL(ch6_pwr_en),
RD_REGISTER_UNROLL(ch7_pwr_en),
};
- struct dentry *regs_dir, *ch_dir;
+ struct dentry *regs_dent, *ch_dent;
int nr_entries, i;
char name[16];
- regs_dir = debugfs_create_dir(READ_STR, dir);
+ regs_dent = debugfs_create_dir(READ_STR, dent);
nr_entries = ARRAY_SIZE(debugfs_regs);
- dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir);
+ dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dent);
if (dw->chip->mf == EDMA_MF_HDMA_COMPAT) {
nr_entries = ARRAY_SIZE(debugfs_unroll_regs);
dw_edma_debugfs_create_x32(debugfs_unroll_regs, nr_entries,
- regs_dir);
+ regs_dent);
}
for (i = 0; i < dw->rd_ch_cnt; i++) {
snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
- ch_dir = debugfs_create_dir(name, regs_dir);
+ ch_dent = debugfs_create_dir(name, regs_dent);
- dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].rd, ch_dir);
+ dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].rd, ch_dent);
lim[1][i].start = ®s->type.unroll.ch[i].rd;
lim[1][i].end = ®s->type.unroll.ch[i].padding_2[0];
@@ -269,16 +269,16 @@ static void dw_edma_debugfs_regs(void)
REGISTER(ctrl_data_arb_prior),
REGISTER(ctrl),
};
- struct dentry *regs_dir;
+ struct dentry *regs_dent;
int nr_entries;
- regs_dir = debugfs_create_dir(REGISTERS_STR, dw->debugfs);
+ regs_dent = debugfs_create_dir(REGISTERS_STR, dw->debugfs);
nr_entries = ARRAY_SIZE(debugfs_regs);
- dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir);
+ dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dent);
- dw_edma_debugfs_regs_wr(regs_dir);
- dw_edma_debugfs_regs_rd(regs_dir);
+ dw_edma_debugfs_regs_wr(regs_dent);
+ dw_edma_debugfs_regs_rd(regs_dent);
}
void dw_edma_v0_debugfs_on(struct dw_edma *_dw)
--
2.39.0
Instead of splitting the 64-bits IOs up into two 32-bits ones it's
possible to use the already available non-atomic readq/writeq methods
implemented exactly for such cases. They are defined in the dedicated
header files io-64-nonatomic-lo-hi.h/io-64-nonatomic-hi-lo.h. So in case
if the 64-bits readq/writeq methods are unavailable on some platforms at
consideration, the corresponding drivers can have any of these headers
included and stop locally re-implementing the 64-bits IO accessors taking
into account the non-atomic nature of the included methods. Let's do that
in the DW eDMA driver too. Note by doing so we can discard the
CONFIG_64BIT config ifdefs from the code.
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-v0-core.c | 55 +++++++++------------------
1 file changed, 18 insertions(+), 37 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 66f296daac5a..51a34b43434c 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -8,6 +8,8 @@
#include <linux/bitfield.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
+
#include "dw-edma-core.h"
#include "dw-edma-v0-core.h"
#include "dw-edma-v0-regs.h"
@@ -53,8 +55,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
SET_32(dw, rd_##name, value); \
} while (0)
-#ifdef CONFIG_64BIT
-
#define SET_64(dw, name, value) \
writeq(value, &(__dw_regs(dw)->name))
@@ -80,8 +80,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
SET_64(dw, rd_##name, value); \
} while (0)
-#endif /* CONFIG_64BIT */
-
#define SET_COMPAT(dw, name, value) \
writel(value, &(__dw_regs(dw)->type.unroll.name))
@@ -164,14 +162,13 @@ static inline u32 readl_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
#define SET_LL_32(ll, value) \
writel(value, ll)
-#ifdef CONFIG_64BIT
-
static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
u64 value, void __iomem *addr)
{
+ unsigned long flags;
+
if (dw->chip->mf == EDMA_MF_EDMA_LEGACY) {
u32 viewport_sel;
- unsigned long flags;
raw_spin_lock_irqsave(&dw->lock, flags);
@@ -181,22 +178,22 @@ static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
writel(viewport_sel,
&(__dw_regs(dw)->type.legacy.viewport_sel));
- writeq(value, addr);
+ }
+
+ writeq(value, addr);
+ if (dw->chip->mf == EDMA_MF_EDMA_LEGACY)
raw_spin_unlock_irqrestore(&dw->lock, flags);
- } else {
- writeq(value, addr);
- }
}
static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
const void __iomem *addr)
{
- u32 value;
+ unsigned long flags;
+ u64 value;
if (dw->chip->mf == EDMA_MF_EDMA_LEGACY) {
u32 viewport_sel;
- unsigned long flags;
raw_spin_lock_irqsave(&dw->lock, flags);
@@ -206,12 +203,12 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
writel(viewport_sel,
&(__dw_regs(dw)->type.legacy.viewport_sel));
- value = readq(addr);
+ }
+
+ value = readq(addr);
+ if (dw->chip->mf == EDMA_MF_EDMA_LEGACY)
raw_spin_unlock_irqrestore(&dw->lock, flags);
- } else {
- value = readq(addr);
- }
return value;
}
@@ -225,8 +222,6 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
#define SET_LL_64(ll, value) \
writeq(value, ll)
-#endif /* CONFIG_64BIT */
-
/* eDMA management callbacks */
void dw_edma_v0_core_off(struct dw_edma *dw)
{
@@ -325,19 +320,10 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
/* Transfer size */
SET_LL_32(&lli[i].transfer_size, child->sz);
/* SAR */
- #ifdef CONFIG_64BIT
- SET_LL_64(&lli[i].sar.reg, child->sar);
- #else /* CONFIG_64BIT */
- SET_LL_32(&lli[i].sar.lsb, lower_32_bits(child->sar));
- SET_LL_32(&lli[i].sar.msb, upper_32_bits(child->sar));
- #endif /* CONFIG_64BIT */
+ SET_LL_64(&lli[i].sar.reg, child->sar);
/* DAR */
- #ifdef CONFIG_64BIT
- SET_LL_64(&lli[i].dar.reg, child->dar);
- #else /* CONFIG_64BIT */
- SET_LL_32(&lli[i].dar.lsb, lower_32_bits(child->dar));
- SET_LL_32(&lli[i].dar.msb, upper_32_bits(child->dar));
- #endif /* CONFIG_64BIT */
+ SET_LL_64(&lli[i].dar.reg, child->dar);
+
i++;
}
@@ -349,12 +335,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
/* Channel control */
SET_LL_32(&llp->control, control);
/* Linked list */
- #ifdef CONFIG_64BIT
- SET_LL_64(&llp->llp.reg, chunk->ll_region.paddr);
- #else /* CONFIG_64BIT */
- SET_LL_32(&llp->llp.lsb, lower_32_bits(chunk->ll_region.paddr));
- SET_LL_32(&llp->llp.msb, upper_32_bits(chunk->ll_region.paddr));
- #endif /* CONFIG_64BIT */
+ SET_LL_64(&llp->llp.reg, chunk->ll_region.paddr);
}
void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
--
2.39.0
In accordance with the dw_edma_region.paddr field semantics it is supposed
to be initialized with a memory base address visible by the DW eDMA
controller. If the DMA engine is embedded into the DW PCIe Host/EP
controller, then the address should belong to the Local CPU/Application
memory. If eDMA is remotely accessible across the PCIe bus via the PCIe
memory IOs, then the address needs to be a part of the PCIe bus memory
space. The later case hasn't been well covered in the corresponding
glue-driver. Since in general the PCIe memory space doesn't have to match
the CPU memory space and the pci_dev.resource[] arrays contain the
resources defined in the CPU memory space, a proper conversion needs to be
performed, otherwise either the driver won't properly work or much worse
the memory corruption will happen. The conversion can be done by means of
the pci_bus_address() method. Let's use it to retrieve the LL, DT and CSRs
PCIe memory ranges.
Note in addition to that we need to extend the dw_edma_region.paddr field
size. The field normally contains a memory range base address to be set in
the DW eDMA Linked-List pointer register or as a base address of the
Linked-List data buffer. In accordance with [1] the LL range is supposed
to be created in the Local CPU/Application memory, but depending on the DW
eDMA utilization the memory can be created as a part of the PCIe bus
address space (as in the case of the DW PCIe EP prototype kit). Thus in
the former case the dw_edma_region.paddr field should have the dma_addr_t
type, while in the later one - pci_bus_addr_t. Seeing the corresponding
CSRs are always 64-bits wide let's convert the dw_edma_region.paddr field
type to be u64 and let the client code logic to make sure it has a valid
address visible by the DW eDMA controller. For instance the DW eDMA PCIe
glue-driver initializes the field with the addresses from the PCIe bus
memory space.
[1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
v.5.40a, March 2019, p.1103
Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-pcie.c | 8 ++++----
include/linux/dma/edma.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index d6b5e2463884..04c95cba1244 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -231,7 +231,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
ll_region->vaddr += ll_block->off;
- ll_region->paddr = pdev->resource[ll_block->bar].start;
+ ll_region->paddr = pci_bus_address(pdev, ll_block->bar);
ll_region->paddr += ll_block->off;
ll_region->sz = ll_block->sz;
@@ -240,7 +240,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
dt_region->vaddr += dt_block->off;
- dt_region->paddr = pdev->resource[dt_block->bar].start;
+ dt_region->paddr = pci_bus_address(pdev, dt_block->bar);
dt_region->paddr += dt_block->off;
dt_region->sz = dt_block->sz;
}
@@ -256,7 +256,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
ll_region->vaddr += ll_block->off;
- ll_region->paddr = pdev->resource[ll_block->bar].start;
+ ll_region->paddr = pci_bus_address(pdev, ll_block->bar);
ll_region->paddr += ll_block->off;
ll_region->sz = ll_block->sz;
@@ -265,7 +265,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
dt_region->vaddr += dt_block->off;
- dt_region->paddr = pdev->resource[dt_block->bar].start;
+ dt_region->paddr = pci_bus_address(pdev, dt_block->bar);
dt_region->paddr += dt_block->off;
dt_region->sz = dt_block->sz;
}
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 7d8062e9c544..a864978ddd27 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -18,7 +18,7 @@
struct dw_edma;
struct dw_edma_region {
- phys_addr_t paddr;
+ u64 paddr;
void __iomem *vaddr;
size_t sz;
};
--
2.39.0
Hello folks,
On Fri, Jan 13, 2023 at 08:13:42PM +0300, Serge Semin wrote:
> This is a final patchset in the series created in the framework of
> my Baikal-T1 PCIe/eDMA-related work:
>
> [1: Done v5] PCI: dwc: Various fixes and cleanups
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Merged: kernel 6.0-rc1
> [2: Done v4] PCI: dwc: Add hw version and dma-ranges support
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Merged: kernel 6.0-rc1
> [3: Done v7] PCI: dwc: Add generic resources and Baikal-T1 support
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Merged: kernel 6.2-rc1
> [4: In-review v9] dmaengine: dw-edma: Add RP/EP local DMA controllers support
> Link: ---you are looking at it---
>
> Note it is very recommended to merge the patchsets in the same order as
> they are listed in the set above in order to have them applied smoothly.
> Since the patchsets 1-3 have already been merged into the mainline kernel
> this series can be applied via any DMA-engine or PCI repos.
>
> Here is a short summary regarding this patchset. The series starts with
> fixes patches. We discovered that the dw-edma-pcie.c driver incorrectly
> initializes the LL/DT base addresses for the platforms with not matching
> CPU and PCIe memory spaces. It is fixed by using the pci_bus_address()
> method to get a correct base address. After that you can find a series of
> the interleaved xfers fixes. It turned out the interleaved transfers
> implementation didn't work quite correctly from the very beginning for
> instance missing src/dst addresses initialization, etc. In the framework
> of the next two patches we suggest to add a new platform-specific
> callback - pci_address() and use it to convert the CPU address to the PCIe
> space address. It is at least required for the DW eDMA remote End-point
> setup on the platforms with not-matching CPU/PCIe address spaces. In case
> of the DW eDMA local RP/EP setup the conversion will be done automatically
> by the outbound iATU (if no DMA-bypass flag is specified for the
> corresponding iATU window). Then we introduce a set of the patches to make
> the DebugFS part of the code supporting the multi-eDMA controllers
> platforms. It starts with several cleanup patches and is closed joining
> the Read/Write channels into a single DMA-device as they originally should
> have been. After that you can find the patches with adding the non-atomic
> io-64 methods usage, dropping DT-region descriptors allocation, replacing
> chip IDs with the device name. In addition to that in order to have the
> eDMA embedded into the DW PCIe RP/EP supported we need to bypass the
> dma-ranges-based memory ranges mapping since in case of the root port DT
> node it's applicable for the peripheral PCIe devices only. Finally at the
> series closure we introduce a generic DW eDMA controller support being
> available in the DW PCIe Root Port/Endpoint driver.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v2:
> - Drop the patches:
> [PATCH 1/25] dmaengine: dw-edma: Drop dma_slave_config.direction field usage
> [PATCH 2/25] dmaengine: dw-edma: Fix eDMA Rd/Wr-channels and DMA-direction semantics
> since they are going to be merged in in the framework of the
> Frank's patchset.
> - Add a new patch: "dmaengine: dw-edma: Release requested IRQs on
> failure."
> - Drop __iomem qualifier from the struct dw_edma_debugfs_entry instance
> definition in the dw_edma_debugfs_u32_get() method. (@Manivannan)
> - Add a new patch: "dmaengine: dw-edma: Rename DebugFS dentry variables to
> 'dent'." (@Manivannan)
> - Slightly extend the eDMA name array size. (@Manivannan)
> - Change the specific DMA mapping comment a bit to being
> clearer. (@Manivannan)
> - Add a new patch: "PCI: dwc: Add generic iATU/eDMA CSRs space detection
> method."
> - Don't fail eDMA detection procedure if the DW eDMA driver couldn't probe
> device. That happens if the driver is disabled. (@Manivannan)
> - Add "dma" registers resource mapping procedure. (@Manivannan)
> - Move the eDMA CSRs space detection into the dw_pcie_map_detect() method.
> - Remove eDMA on the dw_pcie_ep_init() internal errors. (@Manivannan)
> - Remove eDMA in the dw_pcie_ep_exit() method.
> - Move the dw_pcie_edma_detect() method execution to the tail of the
> dw_pcie_ep_init() function.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v3:
> - Conditionally set dchan->dev->device.dma_coherent field since it can
> be missing on some platforms. (@Manivannan)
> - Drop the patch: "PCI: dwc: Add generic iATU/eDMA CSRs space detection
> method". A similar modification has been done in another patchset.
> - Add more comprehensive and less regression prune eDMA block detection
> procedure.
> - Drop the patch: "dma-direct: take dma-ranges/offsets into account in
> resource mapping". It will be separately reviewed.
> - Remove Manivannan tb tag from the modified patches.
> - Rebase onto the kernel v5.18.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]
> Changelog v4:
> - Rabase onto the laters Frank Li series:
> Link: https://lore.kernel.org/all/[email protected]/
> - Add Vinod' Ab-tag.
> - Rebase onto the kernel v5.19-rcX.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]
> Changelog v5:
> - Just resend.
> - Rebase onto the kernel v6.0-rc2.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]
> Changelog v6:
> - Fix some patchlog and in-line comments misspells. (@Bjorn)
> - Directly call *_dma_configure() method on the DW eDMA channel child
> device used for the DMA buffers mapping. (@Robin)
> - Explicitly set the DMA-mask of the child device in the channel
> allocation proecedure. (@Robin)
> - Rebase onto the kernel v6.1-rc3.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v7:
> - Activate the mapping auto-detection procedure for IP-cores older than
> 5.40a. The viewport-based access has been removed since that
> version. (@Yoshihiro)
> - Drop the patch
> [PATCH v6 22/24] dmaengine: dw-edma: Bypass dma-ranges mapping for the local setup
> since the problem has been fixed in the commit f1ad5338a4d5 ("of: Fix
> "dma-ranges" handling for bus controllers"). (@Robin)
> - Add a new patch:
> [PATCH v7 23/25] PCI: dwc: Restore DMA-mask after MSI-data allocation
> (@Robin)
> - Add a new patch:
> [PATCH v7 24/25] PCI: bt1: Set 64-bit DMA-mask
> (@Robin)
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v8:
> - Add a new patch:
> [PATCH v8 23/26] dmaengine: dw-edma: Relax driver config settings
> (@tbot)
> - Replace the patch
> [PATCH v7 23/25] PCI: dwc: Restore DMA-mask after MSI-data allocation
> with a new one:
> [PATCH v8 24/26] PCI: dwc: Set coherent DMA-mask on MSI-address allocation
> (@Robin, @Christoph)
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v9:
> - Add a new patch:
> [PATCH v9 23/27] dmaengine: dw-edma: Add mem-mapped LL-entries support
> (@tbot)
> - Rebase onto the kernel 6.2-rc3.
This is currently the latest version of the patchset. There are
several new patches (suggested by @Yoshihiro, @Robin and @tbot) and a
few updates have been introduced since the last @Vinod' and
@Manivannan' reviews. Thus I had to drop their Rb/Tb-tags from the
cover-letter and from some of the patches. Other than that the series
seems to be ready to be merged in (@Robin already blessed the
DMA-mask-related patches). Since all the preceding patchsets have
already been mainlined it can be done via any repo: DMA-engine, PCI,
etc.
-Serge(y)
>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Pavel Parkhomenko <[email protected]>
> Cc: "Krzysztof Wilczyński" <[email protected]>
> Cc: caihuoqing <[email protected]>
> Cc: Yoshihiro Shimoda <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Serge Semin (27):
> dmaengine: Fix dma_slave_config.dst_addr description
> dmaengine: dw-edma: Release requested IRQs on failure
> dmaengine: dw-edma: Convert ll/dt phys-address to PCIe bus/DMA address
> dmaengine: dw-edma: Fix missing src/dst address of the interleaved
> xfers
> dmaengine: dw-edma: Don't permit non-inc interleaved xfers
> dmaengine: dw-edma: Fix invalid interleaved xfers semantics
> dmaengine: dw-edma: Add CPU to PCIe bus address translation
> dmaengine: dw-edma: Add PCIe bus address getter to the remote EP
> glue-driver
> dmaengine: dw-edma: Drop chancnt initialization
> dmaengine: dw-edma: Fix DebugFS reg entry type
> dmaengine: dw-edma: Stop checking debugfs_create_*() return value
> dmaengine: dw-edma: Add dw_edma prefix to the DebugFS nodes descriptor
> dmaengine: dw-edma: Convert DebugFS descs to being kz-allocated
> dmaengine: dw-edma: Rename DebugFS dentry variables to 'dent'
> dmaengine: dw-edma: Simplify the DebugFS context CSRs init procedure
> dmaengine: dw-edma: Move eDMA data pointer to DebugFS node descriptor
> dmaengine: dw-edma: Join Write/Read channels into a single device
> dmaengine: dw-edma: Use DMA-engine device DebugFS subdirectory
> dmaengine: dw-edma: Use non-atomic io-64 methods
> dmaengine: dw-edma: Drop DT-region allocation
> dmaengine: dw-edma: Replace chip ID number with device name
> dmaengine: dw-edma: Skip cleanup procedure if no private data found
> dmaengine: dw-edma: Add mem-mapped LL-entries support
> dmaengine: dw-edma: Relax driver config settings
> PCI: dwc: Set coherent DMA-mask on MSI-address allocation
> PCI: bt1: Set 64-bit DMA-mask
> PCI: dwc: Add DW eDMA engine support
>
> drivers/dma/dw-edma/Kconfig | 5 +-
> drivers/dma/dw-edma/dw-edma-core.c | 196 ++++-----
> drivers/dma/dw-edma/dw-edma-core.h | 10 +-
> drivers/dma/dw-edma/dw-edma-pcie.c | 56 ++-
> drivers/dma/dw-edma/dw-edma-v0-core.c | 121 +++---
> drivers/dma/dw-edma/dw-edma-v0-core.h | 1 -
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 372 ++++++++----------
> drivers/dma/dw-edma/dw-edma-v0-debugfs.h | 5 -
> drivers/pci/controller/dwc/pcie-bt1.c | 4 +
> .../pci/controller/dwc/pcie-designware-ep.c | 12 +-
> .../pci/controller/dwc/pcie-designware-host.c | 24 +-
> drivers/pci/controller/dwc/pcie-designware.c | 195 +++++++++
> drivers/pci/controller/dwc/pcie-designware.h | 21 +
> include/linux/dma/edma.h | 25 +-
> include/linux/dmaengine.h | 2 +-
> 15 files changed, 652 insertions(+), 397 deletions(-)
>
> --
> 2.39.0
>
>
On Fri, Jan 13, 2023 at 08:13:42PM +0300, Serge Semin wrote:
> This is a final patchset in the series created in the framework of
> my Baikal-T1 PCIe/eDMA-related work:
>
> [1: Done v5] PCI: dwc: Various fixes and cleanups
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Merged: kernel 6.0-rc1
> [2: Done v4] PCI: dwc: Add hw version and dma-ranges support
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Merged: kernel 6.0-rc1
> [3: Done v7] PCI: dwc: Add generic resources and Baikal-T1 support
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Merged: kernel 6.2-rc1
> [4: In-review v9] dmaengine: dw-edma: Add RP/EP local DMA controllers support
> Link: ---you are looking at it---
>
> Note it is very recommended to merge the patchsets in the same order as
> they are listed in the set above in order to have them applied smoothly.
> Since the patchsets 1-3 have already been merged into the mainline kernel
> this series can be applied via any DMA-engine or PCI repos.
>
> Here is a short summary regarding this patchset. The series starts with
> fixes patches. We discovered that the dw-edma-pcie.c driver incorrectly
> initializes the LL/DT base addresses for the platforms with not matching
> CPU and PCIe memory spaces. It is fixed by using the pci_bus_address()
> method to get a correct base address. After that you can find a series of
> the interleaved xfers fixes. It turned out the interleaved transfers
> implementation didn't work quite correctly from the very beginning for
> instance missing src/dst addresses initialization, etc. In the framework
> of the next two patches we suggest to add a new platform-specific
> callback - pci_address() and use it to convert the CPU address to the PCIe
> space address. It is at least required for the DW eDMA remote End-point
> setup on the platforms with not-matching CPU/PCIe address spaces. In case
> of the DW eDMA local RP/EP setup the conversion will be done automatically
> by the outbound iATU (if no DMA-bypass flag is specified for the
> corresponding iATU window). Then we introduce a set of the patches to make
> the DebugFS part of the code supporting the multi-eDMA controllers
> platforms. It starts with several cleanup patches and is closed joining
> the Read/Write channels into a single DMA-device as they originally should
> have been. After that you can find the patches with adding the non-atomic
> io-64 methods usage, dropping DT-region descriptors allocation, replacing
> chip IDs with the device name. In addition to that in order to have the
> eDMA embedded into the DW PCIe RP/EP supported we need to bypass the
> dma-ranges-based memory ranges mapping since in case of the root port DT
> node it's applicable for the peripheral PCIe devices only. Finally at the
> series closure we introduce a generic DW eDMA controller support being
> available in the DW PCIe Root Port/Endpoint driver.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v2:
> - Drop the patches:
> [PATCH 1/25] dmaengine: dw-edma: Drop dma_slave_config.direction field usage
> [PATCH 2/25] dmaengine: dw-edma: Fix eDMA Rd/Wr-channels and DMA-direction semantics
> since they are going to be merged in in the framework of the
> Frank's patchset.
> - Add a new patch: "dmaengine: dw-edma: Release requested IRQs on
> failure."
> - Drop __iomem qualifier from the struct dw_edma_debugfs_entry instance
> definition in the dw_edma_debugfs_u32_get() method. (@Manivannan)
> - Add a new patch: "dmaengine: dw-edma: Rename DebugFS dentry variables to
> 'dent'." (@Manivannan)
> - Slightly extend the eDMA name array size. (@Manivannan)
> - Change the specific DMA mapping comment a bit to being
> clearer. (@Manivannan)
> - Add a new patch: "PCI: dwc: Add generic iATU/eDMA CSRs space detection
> method."
> - Don't fail eDMA detection procedure if the DW eDMA driver couldn't probe
> device. That happens if the driver is disabled. (@Manivannan)
> - Add "dma" registers resource mapping procedure. (@Manivannan)
> - Move the eDMA CSRs space detection into the dw_pcie_map_detect() method.
> - Remove eDMA on the dw_pcie_ep_init() internal errors. (@Manivannan)
> - Remove eDMA in the dw_pcie_ep_exit() method.
> - Move the dw_pcie_edma_detect() method execution to the tail of the
> dw_pcie_ep_init() function.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v3:
> - Conditionally set dchan->dev->device.dma_coherent field since it can
> be missing on some platforms. (@Manivannan)
> - Drop the patch: "PCI: dwc: Add generic iATU/eDMA CSRs space detection
> method". A similar modification has been done in another patchset.
> - Add more comprehensive and less regression prune eDMA block detection
> procedure.
> - Drop the patch: "dma-direct: take dma-ranges/offsets into account in
> resource mapping". It will be separately reviewed.
> - Remove Manivannan tb tag from the modified patches.
> - Rebase onto the kernel v5.18.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]
> Changelog v4:
> - Rabase onto the laters Frank Li series:
> Link: https://lore.kernel.org/all/[email protected]/
> - Add Vinod' Ab-tag.
> - Rebase onto the kernel v5.19-rcX.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]
> Changelog v5:
> - Just resend.
> - Rebase onto the kernel v6.0-rc2.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]
> Changelog v6:
> - Fix some patchlog and in-line comments misspells. (@Bjorn)
> - Directly call *_dma_configure() method on the DW eDMA channel child
> device used for the DMA buffers mapping. (@Robin)
> - Explicitly set the DMA-mask of the child device in the channel
> allocation proecedure. (@Robin)
> - Rebase onto the kernel v6.1-rc3.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v7:
> - Activate the mapping auto-detection procedure for IP-cores older than
> 5.40a. The viewport-based access has been removed since that
> version. (@Yoshihiro)
> - Drop the patch
> [PATCH v6 22/24] dmaengine: dw-edma: Bypass dma-ranges mapping for the local setup
> since the problem has been fixed in the commit f1ad5338a4d5 ("of: Fix
> "dma-ranges" handling for bus controllers"). (@Robin)
> - Add a new patch:
> [PATCH v7 23/25] PCI: dwc: Restore DMA-mask after MSI-data allocation
> (@Robin)
> - Add a new patch:
> [PATCH v7 24/25] PCI: bt1: Set 64-bit DMA-mask
> (@Robin)
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v8:
> - Add a new patch:
> [PATCH v8 23/26] dmaengine: dw-edma: Relax driver config settings
> (@tbot)
> - Replace the patch
> [PATCH v7 23/25] PCI: dwc: Restore DMA-mask after MSI-data allocation
> with a new one:
> [PATCH v8 24/26] PCI: dwc: Set coherent DMA-mask on MSI-address allocation
> (@Robin, @Christoph)
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v9:
> - Add a new patch:
> [PATCH v9 23/27] dmaengine: dw-edma: Add mem-mapped LL-entries support
> (@tbot)
> - Rebase onto the kernel 6.2-rc3.
>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Pavel Parkhomenko <[email protected]>
> Cc: "Krzysztof Wilczyński" <[email protected]>
> Cc: caihuoqing <[email protected]>
> Cc: Yoshihiro Shimoda <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Serge Semin (27):
> dmaengine: Fix dma_slave_config.dst_addr description
> dmaengine: dw-edma: Release requested IRQs on failure
> dmaengine: dw-edma: Convert ll/dt phys-address to PCIe bus/DMA address
> dmaengine: dw-edma: Fix missing src/dst address of the interleaved
> xfers
> dmaengine: dw-edma: Don't permit non-inc interleaved xfers
> dmaengine: dw-edma: Fix invalid interleaved xfers semantics
> dmaengine: dw-edma: Add CPU to PCIe bus address translation
> dmaengine: dw-edma: Add PCIe bus address getter to the remote EP
> glue-driver
> dmaengine: dw-edma: Drop chancnt initialization
> dmaengine: dw-edma: Fix DebugFS reg entry type
> dmaengine: dw-edma: Stop checking debugfs_create_*() return value
> dmaengine: dw-edma: Add dw_edma prefix to the DebugFS nodes descriptor
> dmaengine: dw-edma: Convert DebugFS descs to being kz-allocated
> dmaengine: dw-edma: Rename DebugFS dentry variables to 'dent'
> dmaengine: dw-edma: Simplify the DebugFS context CSRs init procedure
> dmaengine: dw-edma: Move eDMA data pointer to DebugFS node descriptor
> dmaengine: dw-edma: Join Write/Read channels into a single device
> dmaengine: dw-edma: Use DMA-engine device DebugFS subdirectory
> dmaengine: dw-edma: Use non-atomic io-64 methods
> dmaengine: dw-edma: Drop DT-region allocation
> dmaengine: dw-edma: Replace chip ID number with device name
> dmaengine: dw-edma: Skip cleanup procedure if no private data found
> dmaengine: dw-edma: Add mem-mapped LL-entries support
> dmaengine: dw-edma: Relax driver config settings
> PCI: dwc: Set coherent DMA-mask on MSI-address allocation
> PCI: bt1: Set 64-bit DMA-mask
> PCI: dwc: Add DW eDMA engine support
>
> drivers/dma/dw-edma/Kconfig | 5 +-
> drivers/dma/dw-edma/dw-edma-core.c | 196 ++++-----
> drivers/dma/dw-edma/dw-edma-core.h | 10 +-
> drivers/dma/dw-edma/dw-edma-pcie.c | 56 ++-
> drivers/dma/dw-edma/dw-edma-v0-core.c | 121 +++---
> drivers/dma/dw-edma/dw-edma-v0-core.h | 1 -
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 372 ++++++++----------
> drivers/dma/dw-edma/dw-edma-v0-debugfs.h | 5 -
> drivers/pci/controller/dwc/pcie-bt1.c | 4 +
> .../pci/controller/dwc/pcie-designware-ep.c | 12 +-
> .../pci/controller/dwc/pcie-designware-host.c | 24 +-
> drivers/pci/controller/dwc/pcie-designware.c | 195 +++++++++
> drivers/pci/controller/dwc/pcie-designware.h | 21 +
> include/linux/dma/edma.h | 25 +-
> include/linux/dmaengine.h | 2 +-
> 15 files changed, 652 insertions(+), 397 deletions(-)
Should I take the series in the PCI tree or do you prefer me acking
the relevant patches ?
Thanks,
Lorenzo
On Mon, Jan 16, 2023 at 10:52:29AM +0100, Lorenzo Pieralisi wrote:
> On Fri, Jan 13, 2023 at 08:13:42PM +0300, Serge Semin wrote:
> > This is a final patchset in the series created in the framework of
> > my Baikal-T1 PCIe/eDMA-related work:
> >
> > [1: Done v5] PCI: dwc: Various fixes and cleanups
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Merged: kernel 6.0-rc1
> > [2: Done v4] PCI: dwc: Add hw version and dma-ranges support
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Merged: kernel 6.0-rc1
> > [3: Done v7] PCI: dwc: Add generic resources and Baikal-T1 support
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Merged: kernel 6.2-rc1
> > [4: In-review v9] dmaengine: dw-edma: Add RP/EP local DMA controllers support
> > Link: ---you are looking at it---
> >
> > Note it is very recommended to merge the patchsets in the same order as
> > they are listed in the set above in order to have them applied smoothly.
> > Since the patchsets 1-3 have already been merged into the mainline kernel
> > this series can be applied via any DMA-engine or PCI repos.
> >
> > Here is a short summary regarding this patchset. The series starts with
> > fixes patches. We discovered that the dw-edma-pcie.c driver incorrectly
> > initializes the LL/DT base addresses for the platforms with not matching
> > CPU and PCIe memory spaces. It is fixed by using the pci_bus_address()
> > method to get a correct base address. After that you can find a series of
> > the interleaved xfers fixes. It turned out the interleaved transfers
> > implementation didn't work quite correctly from the very beginning for
> > instance missing src/dst addresses initialization, etc. In the framework
> > of the next two patches we suggest to add a new platform-specific
> > callback - pci_address() and use it to convert the CPU address to the PCIe
> > space address. It is at least required for the DW eDMA remote End-point
> > setup on the platforms with not-matching CPU/PCIe address spaces. In case
> > of the DW eDMA local RP/EP setup the conversion will be done automatically
> > by the outbound iATU (if no DMA-bypass flag is specified for the
> > corresponding iATU window). Then we introduce a set of the patches to make
> > the DebugFS part of the code supporting the multi-eDMA controllers
> > platforms. It starts with several cleanup patches and is closed joining
> > the Read/Write channels into a single DMA-device as they originally should
> > have been. After that you can find the patches with adding the non-atomic
> > io-64 methods usage, dropping DT-region descriptors allocation, replacing
> > chip IDs with the device name. In addition to that in order to have the
> > eDMA embedded into the DW PCIe RP/EP supported we need to bypass the
> > dma-ranges-based memory ranges mapping since in case of the root port DT
> > node it's applicable for the peripheral PCIe devices only. Finally at the
> > series closure we introduce a generic DW eDMA controller support being
> > available in the DW PCIe Root Port/Endpoint driver.
> >
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Changelog v2:
> > - Drop the patches:
> > [PATCH 1/25] dmaengine: dw-edma: Drop dma_slave_config.direction field usage
> > [PATCH 2/25] dmaengine: dw-edma: Fix eDMA Rd/Wr-channels and DMA-direction semantics
> > since they are going to be merged in in the framework of the
> > Frank's patchset.
> > - Add a new patch: "dmaengine: dw-edma: Release requested IRQs on
> > failure."
> > - Drop __iomem qualifier from the struct dw_edma_debugfs_entry instance
> > definition in the dw_edma_debugfs_u32_get() method. (@Manivannan)
> > - Add a new patch: "dmaengine: dw-edma: Rename DebugFS dentry variables to
> > 'dent'." (@Manivannan)
> > - Slightly extend the eDMA name array size. (@Manivannan)
> > - Change the specific DMA mapping comment a bit to being
> > clearer. (@Manivannan)
> > - Add a new patch: "PCI: dwc: Add generic iATU/eDMA CSRs space detection
> > method."
> > - Don't fail eDMA detection procedure if the DW eDMA driver couldn't probe
> > device. That happens if the driver is disabled. (@Manivannan)
> > - Add "dma" registers resource mapping procedure. (@Manivannan)
> > - Move the eDMA CSRs space detection into the dw_pcie_map_detect() method.
> > - Remove eDMA on the dw_pcie_ep_init() internal errors. (@Manivannan)
> > - Remove eDMA in the dw_pcie_ep_exit() method.
> > - Move the dw_pcie_edma_detect() method execution to the tail of the
> > dw_pcie_ep_init() function.
> >
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Changelog v3:
> > - Conditionally set dchan->dev->device.dma_coherent field since it can
> > be missing on some platforms. (@Manivannan)
> > - Drop the patch: "PCI: dwc: Add generic iATU/eDMA CSRs space detection
> > method". A similar modification has been done in another patchset.
> > - Add more comprehensive and less regression prune eDMA block detection
> > procedure.
> > - Drop the patch: "dma-direct: take dma-ranges/offsets into account in
> > resource mapping". It will be separately reviewed.
> > - Remove Manivannan tb tag from the modified patches.
> > - Rebase onto the kernel v5.18.
> >
> > Link: https://lore.kernel.org/linux-pci/[email protected]
> > Changelog v4:
> > - Rabase onto the laters Frank Li series:
> > Link: https://lore.kernel.org/all/[email protected]/
> > - Add Vinod' Ab-tag.
> > - Rebase onto the kernel v5.19-rcX.
> >
> > Link: https://lore.kernel.org/linux-pci/[email protected]
> > Changelog v5:
> > - Just resend.
> > - Rebase onto the kernel v6.0-rc2.
> >
> > Link: https://lore.kernel.org/linux-pci/[email protected]
> > Changelog v6:
> > - Fix some patchlog and in-line comments misspells. (@Bjorn)
> > - Directly call *_dma_configure() method on the DW eDMA channel child
> > device used for the DMA buffers mapping. (@Robin)
> > - Explicitly set the DMA-mask of the child device in the channel
> > allocation proecedure. (@Robin)
> > - Rebase onto the kernel v6.1-rc3.
> >
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Changelog v7:
> > - Activate the mapping auto-detection procedure for IP-cores older than
> > 5.40a. The viewport-based access has been removed since that
> > version. (@Yoshihiro)
> > - Drop the patch
> > [PATCH v6 22/24] dmaengine: dw-edma: Bypass dma-ranges mapping for the local setup
> > since the problem has been fixed in the commit f1ad5338a4d5 ("of: Fix
> > "dma-ranges" handling for bus controllers"). (@Robin)
> > - Add a new patch:
> > [PATCH v7 23/25] PCI: dwc: Restore DMA-mask after MSI-data allocation
> > (@Robin)
> > - Add a new patch:
> > [PATCH v7 24/25] PCI: bt1: Set 64-bit DMA-mask
> > (@Robin)
> >
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Changelog v8:
> > - Add a new patch:
> > [PATCH v8 23/26] dmaengine: dw-edma: Relax driver config settings
> > (@tbot)
> > - Replace the patch
> > [PATCH v7 23/25] PCI: dwc: Restore DMA-mask after MSI-data allocation
> > with a new one:
> > [PATCH v8 24/26] PCI: dwc: Set coherent DMA-mask on MSI-address allocation
> > (@Robin, @Christoph)
> >
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Changelog v9:
> > - Add a new patch:
> > [PATCH v9 23/27] dmaengine: dw-edma: Add mem-mapped LL-entries support
> > (@tbot)
> > - Rebase onto the kernel 6.2-rc3.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> > Cc: Alexey Malahov <[email protected]>
> > Cc: Pavel Parkhomenko <[email protected]>
> > Cc: "Krzysztof Wilczyński" <[email protected]>
> > Cc: caihuoqing <[email protected]>
> > Cc: Yoshihiro Shimoda <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> >
> > Serge Semin (27):
> > dmaengine: Fix dma_slave_config.dst_addr description
> > dmaengine: dw-edma: Release requested IRQs on failure
> > dmaengine: dw-edma: Convert ll/dt phys-address to PCIe bus/DMA address
> > dmaengine: dw-edma: Fix missing src/dst address of the interleaved
> > xfers
> > dmaengine: dw-edma: Don't permit non-inc interleaved xfers
> > dmaengine: dw-edma: Fix invalid interleaved xfers semantics
> > dmaengine: dw-edma: Add CPU to PCIe bus address translation
> > dmaengine: dw-edma: Add PCIe bus address getter to the remote EP
> > glue-driver
> > dmaengine: dw-edma: Drop chancnt initialization
> > dmaengine: dw-edma: Fix DebugFS reg entry type
> > dmaengine: dw-edma: Stop checking debugfs_create_*() return value
> > dmaengine: dw-edma: Add dw_edma prefix to the DebugFS nodes descriptor
> > dmaengine: dw-edma: Convert DebugFS descs to being kz-allocated
> > dmaengine: dw-edma: Rename DebugFS dentry variables to 'dent'
> > dmaengine: dw-edma: Simplify the DebugFS context CSRs init procedure
> > dmaengine: dw-edma: Move eDMA data pointer to DebugFS node descriptor
> > dmaengine: dw-edma: Join Write/Read channels into a single device
> > dmaengine: dw-edma: Use DMA-engine device DebugFS subdirectory
> > dmaengine: dw-edma: Use non-atomic io-64 methods
> > dmaengine: dw-edma: Drop DT-region allocation
> > dmaengine: dw-edma: Replace chip ID number with device name
> > dmaengine: dw-edma: Skip cleanup procedure if no private data found
> > dmaengine: dw-edma: Add mem-mapped LL-entries support
> > dmaengine: dw-edma: Relax driver config settings
> > PCI: dwc: Set coherent DMA-mask on MSI-address allocation
> > PCI: bt1: Set 64-bit DMA-mask
> > PCI: dwc: Add DW eDMA engine support
> >
> > drivers/dma/dw-edma/Kconfig | 5 +-
> > drivers/dma/dw-edma/dw-edma-core.c | 196 ++++-----
> > drivers/dma/dw-edma/dw-edma-core.h | 10 +-
> > drivers/dma/dw-edma/dw-edma-pcie.c | 56 ++-
> > drivers/dma/dw-edma/dw-edma-v0-core.c | 121 +++---
> > drivers/dma/dw-edma/dw-edma-v0-core.h | 1 -
> > drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 372 ++++++++----------
> > drivers/dma/dw-edma/dw-edma-v0-debugfs.h | 5 -
> > drivers/pci/controller/dwc/pcie-bt1.c | 4 +
> > .../pci/controller/dwc/pcie-designware-ep.c | 12 +-
> > .../pci/controller/dwc/pcie-designware-host.c | 24 +-
> > drivers/pci/controller/dwc/pcie-designware.c | 195 +++++++++
> > drivers/pci/controller/dwc/pcie-designware.h | 21 +
> > include/linux/dma/edma.h | 25 +-
> > include/linux/dmaengine.h | 2 +-
> > 15 files changed, 652 insertions(+), 397 deletions(-)
>
> Should I take the series in the PCI tree or do you prefer me acking
> the relevant patches ?
You taking the series in the PCI tree would be preferable choice since
it was what we agreed to do for all the denoted in the cover-letter
patchsets.
-Serge(y)
>
> Thanks,
> Lorenzo
On Fri, 13 Jan 2023 20:13:42 +0300, Serge Semin wrote:
> This is a final patchset in the series created in the framework of
> my Baikal-T1 PCIe/eDMA-related work:
>
> [1: Done v5] PCI: dwc: Various fixes and cleanups
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Merged: kernel 6.0-rc1
> [2: Done v4] PCI: dwc: Add hw version and dma-ranges support
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Merged: kernel 6.0-rc1
> [3: Done v7] PCI: dwc: Add generic resources and Baikal-T1 support
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Merged: kernel 6.2-rc1
> [4: In-review v9] dmaengine: dw-edma: Add RP/EP local DMA controllers support
> Link: ---you are looking at it---
>
> [...]
Applied to pci/dwc, thanks!
[01/27] dmaengine: Fix dma_slave_config.dst_addr description
https://git.kernel.org/lpieralisi/pci/c/064f216c0320
[02/27] dmaengine: dw-edma: Release requested IRQs on failure
https://git.kernel.org/lpieralisi/pci/c/0abe230898c5
[03/27] dmaengine: dw-edma: Convert ll/dt phys-address to PCIe bus/DMA address
https://git.kernel.org/lpieralisi/pci/c/e9bff321a78e
[04/27] dmaengine: dw-edma: Fix missing src/dst address of the interleaved xfers
https://git.kernel.org/lpieralisi/pci/c/dba023658bb8
[05/27] dmaengine: dw-edma: Don't permit non-inc interleaved xfers
https://git.kernel.org/lpieralisi/pci/c/66fd2f907b5e
[06/27] dmaengine: dw-edma: Fix invalid interleaved xfers semantics
https://git.kernel.org/lpieralisi/pci/c/669e5b693ed0
[07/27] dmaengine: dw-edma: Add CPU to PCIe bus address translation
https://git.kernel.org/lpieralisi/pci/c/3e52c2d7440a
[08/27] dmaengine: dw-edma: Add PCIe bus address getter to the remote EP glue-driver
https://git.kernel.org/lpieralisi/pci/c/262be8be4446
[09/27] dmaengine: dw-edma: Drop chancnt initialization
https://git.kernel.org/lpieralisi/pci/c/cbc8047b3c14
[10/27] dmaengine: dw-edma: Fix DebugFS reg entry type
https://git.kernel.org/lpieralisi/pci/c/09d4dc211c98
[11/27] dmaengine: dw-edma: Stop checking debugfs_create_*() return value
https://git.kernel.org/lpieralisi/pci/c/db75236638be
[12/27] dmaengine: dw-edma: Add dw_edma prefix to the DebugFS nodes descriptor
https://git.kernel.org/lpieralisi/pci/c/3e2b815a909a
[13/27] dmaengine: dw-edma: Convert DebugFS descs to being kz-allocated
https://git.kernel.org/lpieralisi/pci/c/65fc08f34f1b
[14/27] dmaengine: dw-edma: Rename DebugFS dentry variables to 'dent'
https://git.kernel.org/lpieralisi/pci/c/12f5703e8f3e
[15/27] dmaengine: dw-edma: Simplify the DebugFS context CSRs init procedure
https://git.kernel.org/lpieralisi/pci/c/7d087c23ce0b
[16/27] dmaengine: dw-edma: Move eDMA data pointer to DebugFS node descriptor
https://git.kernel.org/lpieralisi/pci/c/75ea6127b19a
[17/27] dmaengine: dw-edma: Join Write/Read channels into a single device
https://git.kernel.org/lpieralisi/pci/c/f19aba4d6c56
[18/27] dmaengine: dw-edma: Use DMA-engine device DebugFS subdirectory
https://git.kernel.org/lpieralisi/pci/c/5e1c2b6d9d6e
[19/27] dmaengine: dw-edma: Use non-atomic io-64 methods
https://git.kernel.org/lpieralisi/pci/c/647c614949f3
[20/27] dmaengine: dw-edma: Drop DT-region allocation
https://git.kernel.org/lpieralisi/pci/c/f27b5d39fdf2
[21/27] dmaengine: dw-edma: Replace chip ID number with device name
https://git.kernel.org/lpieralisi/pci/c/e2d483f30e93
[22/27] dmaengine: dw-edma: Skip cleanup procedure if no private data found
https://git.kernel.org/lpieralisi/pci/c/6bdc8b38c1c3
[23/27] dmaengine: dw-edma: Add mem-mapped LL-entries support
https://git.kernel.org/lpieralisi/pci/c/06a2a118d36f
[24/27] dmaengine: dw-edma: Relax driver config settings
https://git.kernel.org/lpieralisi/pci/c/9dd4a16f2540
[25/27] PCI: dwc: Set coherent DMA-mask on MSI-address allocation
https://git.kernel.org/lpieralisi/pci/c/b838544be814
[26/27] PCI: bt1: Set 64-bit DMA-mask
https://git.kernel.org/lpieralisi/pci/c/2484fb4c8581
[27/27] PCI: dwc: Add DW eDMA engine support
https://git.kernel.org/lpieralisi/pci/c/95624672bb3e
Thanks,
Lorenzo
On Fri, Jan 13, 2023 at 08:14:07PM +0300, Serge Semin wrote:
> The MSI target address requires to be reserved within the lowest 4GB
> memory in order to support the PCIe peripherals with no 64-bit MSI TLPs
> support. Since the allocation is done from the DMA-coherent memory let's
> modify the allocation procedure to setting the coherent DMA-mask only and
> avoiding the streaming DMA-mask modification. Thus at least the streaming
> DMA operations would work with no artificial limitations. It will be
> specifically useful for the eDMA-capable controllers so the corresponding
> DMA-engine clients would map the DMA buffers with no need in the SWIOTLB
> intervention for the buffers allocated above the 4GB memory region.
>
> While at it let's add a brief comment about the reason of having the MSI
> target address allocated from the DMA-coherent memory limited with the 4GB
> upper bound.
>
> Signed-off-by: Serge Semin <[email protected]>
> Reviewed-by: Robin Murphy <[email protected]>
>
> ---
>
> Changelog v8:
> - This is a new patch added on v8 stage of the series.
> (@Robin, @Christoph)
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 3ab6ae3712c4..e10608af39b4 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -366,7 +366,16 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> dw_chained_msi_isr, pp);
> }
>
> - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> + /*
> + * Even though the iMSI-RX Module supports 64-bit addresses some
> + * peripheral PCIe devices may lack the 64-bit messages support. In
> + * order not to miss MSI TLPs from those devices the MSI target address
> + * has to be reserved within the lowest 4GB.
> + * Note until there is a better alternative found the reservation is
> + * done by allocating from the artificially limited DMA-coherent
> + * memory.
> + */
> + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
We used to restrict both streaming and coherent DMA masks to 32 bits.
Now we will only restrict coherent DMA to 32 bits.
So in essence this change removes a restriction on the streaming DMA
mask, right?
And I guess bt1 is the only driver where this will make a difference
(after the next patch) because no other dwc drivers set their own DMA
masks?
> if (ret)
> dev_warn(dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
>
> --
> 2.39.0
>
>
On Fri, Jan 13, 2023 at 08:13:50PM +0300, Serge Semin wrote:
> In general the Synopsys PCIe EndPoint IP prototype kit can be attached to
> a PCIe bus with any PCIe Host controller including to the one with
> distinctive from CPU address space. Due to that we need to make sure that
> the source and destination addresses of the DMA-slave devices are properly
> converted to the PCIe bus address space, otherwise the DMA transaction
> will not only work as expected, but may cause the memory corruption with
> subsequent system crash. Let's do that by introducing a new
> dw_edma_pcie_address() method defined in the dw-edma-pcie.c, which will
> perform the denoted translation by using the pcibios_resource_to_bus()
> method.
>
> Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
> Signed-off-by: Serge Semin <[email protected]>
> Reviewed-by: Manivannan Sadhasivam <[email protected]>
> Tested-by: Manivannan Sadhasivam <[email protected]>
> Acked-by: Vinod Koul <[email protected]>
>
> ---
>
> Note this patch depends on the patch "dmaengine: dw-edma: Add CPU to PCIe
> bus address translation" from this series.
> ---
> drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 04c95cba1244..f530bacfd716 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -95,8 +95,23 @@ static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
> return pci_irq_vector(to_pci_dev(dev), nr);
> }
>
> +static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct pci_bus_region region;
> + struct resource res = {
> + .flags = IORESOURCE_MEM,
> + .start = cpu_addr,
> + .end = cpu_addr,
> + };
> +
> + pcibios_resource_to_bus(pdev->bus, ®ion, &res);
> + return region.start;
> +}
This doesn't look DW-specific. Do you expect other implementations
that are specific, or could this be a generic function that shares
some implementation with pci_bus_address()?
> static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> .irq_vector = dw_edma_pcie_irq_vector,
> + .pci_address = dw_edma_pcie_address,
> };
>
> static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
> --
> 2.39.0
>
>
On Fri, Jan 13, 2023 at 08:14:01PM +0300, Serge Semin wrote:
> Instead of splitting the 64-bits IOs up into two 32-bits ones it's
> possible to use the already available non-atomic readq/writeq methods
> implemented exactly for such cases. They are defined in the dedicated
> header files io-64-nonatomic-lo-hi.h/io-64-nonatomic-hi-lo.h. So in case
> if the 64-bits readq/writeq methods are unavailable on some platforms at
> consideration, the corresponding drivers can have any of these headers
> included and stop locally re-implementing the 64-bits IO accessors taking
> into account the non-atomic nature of the included methods. Let's do that
> in the DW eDMA driver too. Note by doing so we can discard the
> CONFIG_64BIT config ifdefs from the code.
>
> Signed-off-by: Serge Semin <[email protected]>
> Reviewed-by: Manivannan Sadhasivam <[email protected]>
> Tested-by: Manivannan Sadhasivam <[email protected]>
> Acked-by: Vinod Koul <[email protected]>
> ---
> drivers/dma/dw-edma/dw-edma-v0-core.c | 55 +++++++++------------------
> 1 file changed, 18 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 66f296daac5a..51a34b43434c 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -8,6 +8,8 @@
>
> #include <linux/bitfield.h>
>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +
> #include "dw-edma-core.h"
> #include "dw-edma-v0-core.h"
> #include "dw-edma-v0-regs.h"
> @@ -53,8 +55,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> SET_32(dw, rd_##name, value); \
> } while (0)
>
> -#ifdef CONFIG_64BIT
> -
> #define SET_64(dw, name, value) \
> writeq(value, &(__dw_regs(dw)->name))
>
> @@ -80,8 +80,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> SET_64(dw, rd_##name, value); \
> } while (0)
>
> -#endif /* CONFIG_64BIT */
Great to get rid of these #ifdefs!
Am I missing something? It looks like SET_64 is used only by
SET_RW_64 and SET_BOTH_64, and neither of *them is used at all.
Similarly for GET_64 and GET_RW_64.
So maybe we could get rid of everything inside the #ifdefs as well?
> #define SET_COMPAT(dw, name, value) \
> writel(value, &(__dw_regs(dw)->type.unroll.name))
>
> @@ -164,14 +162,13 @@ static inline u32 readl_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> #define SET_LL_32(ll, value) \
> writel(value, ll)
>
> -#ifdef CONFIG_64BIT
> -
> static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> u64 value, void __iomem *addr)
> {
> + unsigned long flags;
> +
> if (dw->chip->mf == EDMA_MF_EDMA_LEGACY) {
> u32 viewport_sel;
> - unsigned long flags;
>
> raw_spin_lock_irqsave(&dw->lock, flags);
>
> @@ -181,22 +178,22 @@ static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
>
> writel(viewport_sel,
> &(__dw_regs(dw)->type.legacy.viewport_sel));
> - writeq(value, addr);
> + }
> +
> + writeq(value, addr);
>
> + if (dw->chip->mf == EDMA_MF_EDMA_LEGACY)
> raw_spin_unlock_irqrestore(&dw->lock, flags);
> - } else {
> - writeq(value, addr);
> - }
This is basically a cosmetic change unrelated to the commit log. I
don't really object to the change, although I think it's of dubious
value to remove the repetition of the writeq() at the cost of adding
another "if" and unlock.
Lorenzo already applied this, so it's OK as-is unless you think it's
worth reworking to drop the unused stuff mentioned above, in which
case this rearrangement could be moved to a separate patch to make
both of them more focused.
Bjorn
On Fri, Jan 20, 2023 at 05:29:20PM -0600, Bjorn Helgaas wrote:
> On Fri, Jan 13, 2023 at 08:13:50PM +0300, Serge Semin wrote:
> > In general the Synopsys PCIe EndPoint IP prototype kit can be attached to
> > a PCIe bus with any PCIe Host controller including to the one with
> > distinctive from CPU address space. Due to that we need to make sure that
> > the source and destination addresses of the DMA-slave devices are properly
> > converted to the PCIe bus address space, otherwise the DMA transaction
> > will not only work as expected, but may cause the memory corruption with
> > subsequent system crash. Let's do that by introducing a new
> > dw_edma_pcie_address() method defined in the dw-edma-pcie.c, which will
> > perform the denoted translation by using the pcibios_resource_to_bus()
> > method.
> >
> > Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
> > Signed-off-by: Serge Semin <[email protected]>
> > Reviewed-by: Manivannan Sadhasivam <[email protected]>
> > Tested-by: Manivannan Sadhasivam <[email protected]>
> > Acked-by: Vinod Koul <[email protected]>
> >
> > ---
> >
> > Note this patch depends on the patch "dmaengine: dw-edma: Add CPU to PCIe
> > bus address translation" from this series.
> > ---
> > drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> > index 04c95cba1244..f530bacfd716 100644
> > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -95,8 +95,23 @@ static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
> > return pci_irq_vector(to_pci_dev(dev), nr);
> > }
> >
> > +static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + struct pci_bus_region region;
> > + struct resource res = {
> > + .flags = IORESOURCE_MEM,
> > + .start = cpu_addr,
> > + .end = cpu_addr,
> > + };
> > +
> > + pcibios_resource_to_bus(pdev->bus, ®ion, &res);
> > + return region.start;
> > +}
>
> This doesn't look DW-specific. Do you expect other implementations
> that are specific, or could this be a generic function that shares
> some implementation with pci_bus_address()?
I have doubts any other implementation would need such method of the
address translation. It's specific to the remote DW eDMA setup (DW
eDMA embedded into a PCIe end-point and accessed from a PCIe
host-side), which support is implemented in the
drivers/dma/dw-edma/dw-edma-pcie.c driver and it doesn't seem to be a
widespread type of devices anyway. The only known case is the Synopsys
PCIe EndPoint IP prototype kit mentioned in the initial DW eDMA driver
commit.
In case of the local DW eDMA setup (DW eDMA embedded into the DW PCIe
Root Port/Complex/End-point controllers and accessible via the
CPU-side of the controllers) the CPU->PCIe address translation is
performed by the outbound iATU engine if the DMA_BYPASS flag is
cleared in the outbound iATU windows control register. The
__dw_pcie_prog_outbound_atu() function (see pcie-designware.c) doesn't
set that flag. That's why we can freely omit the
dw_edma_core_ops.pci_address callback.
On the other hand the DW PCIe controller can have a customized ATU
(see dw_pcie_rp.bridge->child_ops can be pre-initialized by the DW
PCIe low-level drivers). In that case I can't predict of how the eDMA
engine would work. It can be as much similar to the iATU
DMA_BYPASS-flag-less case or work on the vendor-specific basis. It's
up to LLDD to implement the proper dw_edma_core_ops.pci_address
callback then.
-Serge(y)
>
> > static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> > .irq_vector = dw_edma_pcie_irq_vector,
> > + .pci_address = dw_edma_pcie_address,
> > };
> >
> > static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
> > --
> > 2.39.0
> >
> >
On Fri, Jan 20, 2023 at 06:54:01PM -0600, Bjorn Helgaas wrote:
> On Fri, Jan 13, 2023 at 08:14:01PM +0300, Serge Semin wrote:
> > Instead of splitting the 64-bits IOs up into two 32-bits ones it's
> > possible to use the already available non-atomic readq/writeq methods
> > implemented exactly for such cases. They are defined in the dedicated
> > header files io-64-nonatomic-lo-hi.h/io-64-nonatomic-hi-lo.h. So in case
> > if the 64-bits readq/writeq methods are unavailable on some platforms at
> > consideration, the corresponding drivers can have any of these headers
> > included and stop locally re-implementing the 64-bits IO accessors taking
> > into account the non-atomic nature of the included methods. Let's do that
> > in the DW eDMA driver too. Note by doing so we can discard the
> > CONFIG_64BIT config ifdefs from the code.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> > Reviewed-by: Manivannan Sadhasivam <[email protected]>
> > Tested-by: Manivannan Sadhasivam <[email protected]>
> > Acked-by: Vinod Koul <[email protected]>
> > ---
> > drivers/dma/dw-edma/dw-edma-v0-core.c | 55 +++++++++------------------
> > 1 file changed, 18 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > index 66f296daac5a..51a34b43434c 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > @@ -8,6 +8,8 @@
> >
> > #include <linux/bitfield.h>
> >
> > +#include <linux/io-64-nonatomic-lo-hi.h>
> > +
> > #include "dw-edma-core.h"
> > #include "dw-edma-v0-core.h"
> > #include "dw-edma-v0-regs.h"
> > @@ -53,8 +55,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> > SET_32(dw, rd_##name, value); \
> > } while (0)
> >
> > -#ifdef CONFIG_64BIT
> > -
> > #define SET_64(dw, name, value) \
> > writeq(value, &(__dw_regs(dw)->name))
> >
> > @@ -80,8 +80,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> > SET_64(dw, rd_##name, value); \
> > } while (0)
> >
> > -#endif /* CONFIG_64BIT */
>
> Great to get rid of these #ifdefs!
>
> Am I missing something? It looks like SET_64 is used only by
> SET_RW_64 and SET_BOTH_64, and neither of *them is used at all.
>
> Similarly for GET_64 and GET_RW_64.
>
> So maybe we could get rid of everything inside the #ifdefs as well?
Even though these macros are indeed unused in the driver they are
still a part of the DW eDMA CSRs access interface. In particular they
are supposed to be used to access the 64-bit registers declared in the
dw_edma_v0_regs, dw_edma_v0_unroll and dw_edma_v0_ch_regs structures.
So until the interface is converted to a more preferable direct MMIO
usage without any packed-structures I'd rather leave these macros
be.
>
> > #define SET_COMPAT(dw, name, value) \
> > writel(value, &(__dw_regs(dw)->type.unroll.name))
> >
> > @@ -164,14 +162,13 @@ static inline u32 readl_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> > #define SET_LL_32(ll, value) \
> > writel(value, ll)
> >
> > -#ifdef CONFIG_64BIT
> > -
> > static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> > u64 value, void __iomem *addr)
> > {
> > + unsigned long flags;
> > +
> > if (dw->chip->mf == EDMA_MF_EDMA_LEGACY) {
> > u32 viewport_sel;
> > - unsigned long flags;
> >
> > raw_spin_lock_irqsave(&dw->lock, flags);
> >
> > @@ -181,22 +178,22 @@ static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> >
> > writel(viewport_sel,
> > &(__dw_regs(dw)->type.legacy.viewport_sel));
> > - writeq(value, addr);
> > + }
> > +
> > + writeq(value, addr);
> >
> > + if (dw->chip->mf == EDMA_MF_EDMA_LEGACY)
> > raw_spin_unlock_irqrestore(&dw->lock, flags);
> > - } else {
> > - writeq(value, addr);
> > - }
>
> This is basically a cosmetic change unrelated to the commit log. I
> don't really object to the change, although I think it's of dubious
> value to remove the repetition of the writeq() at the cost of adding
> another "if" and unlock.
>
The denoted change is basically a leftover of the originally more
complex modification:
https://lore.kernel.org/linux-pci/[email protected]
for which the update made more sense. But then the corresponding flag
was dropped from the Frank' patchset and I had to reduce the patch
to the code above.
Though I agree with you in both aspects. Indeed the value of the
change is questionable and it isn't related to the commit log.
> Lorenzo already applied this, so it's OK as-is unless you think it's
> worth reworking to drop the unused stuff mentioned above, in which
> case this rearrangement could be moved to a separate patch to make
> both of them more focused.
As I said above I'd rather leave the 64-bit accessors be until the
packed structure-based interface is removed from the driver.
Regarding the QWORD accessors update. If @Lorenzo agrees to replace
the already applied v9 patchset with a new one I can resubmit the
series with no rearrangement above.
-Serge(y)
>
> Bjorn
On Fri, Jan 20, 2023 at 03:59:52PM -0600, Bjorn Helgaas wrote:
> On Fri, Jan 13, 2023 at 08:14:07PM +0300, Serge Semin wrote:
> > The MSI target address requires to be reserved within the lowest 4GB
> > memory in order to support the PCIe peripherals with no 64-bit MSI TLPs
> > support. Since the allocation is done from the DMA-coherent memory let's
> > modify the allocation procedure to setting the coherent DMA-mask only and
> > avoiding the streaming DMA-mask modification. Thus at least the streaming
> > DMA operations would work with no artificial limitations. It will be
> > specifically useful for the eDMA-capable controllers so the corresponding
> > DMA-engine clients would map the DMA buffers with no need in the SWIOTLB
> > intervention for the buffers allocated above the 4GB memory region.
> >
> > While at it let's add a brief comment about the reason of having the MSI
> > target address allocated from the DMA-coherent memory limited with the 4GB
> > upper bound.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> > Reviewed-by: Robin Murphy <[email protected]>
> >
> > ---
> >
> > Changelog v8:
> > - This is a new patch added on v8 stage of the series.
> > (@Robin, @Christoph)
> > ---
> > drivers/pci/controller/dwc/pcie-designware-host.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 3ab6ae3712c4..e10608af39b4 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -366,7 +366,16 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> > dw_chained_msi_isr, pp);
> > }
> >
> > - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > + /*
> > + * Even though the iMSI-RX Module supports 64-bit addresses some
> > + * peripheral PCIe devices may lack the 64-bit messages support. In
> > + * order not to miss MSI TLPs from those devices the MSI target address
> > + * has to be reserved within the lowest 4GB.
> > + * Note until there is a better alternative found the reservation is
> > + * done by allocating from the artificially limited DMA-coherent
> > + * memory.
> > + */
> > + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
>
> We used to restrict both streaming and coherent DMA masks to 32 bits.
> Now we will only restrict coherent DMA to 32 bits.
>
> So in essence this change removes a restriction on the streaming DMA
> mask, right?
Right.
>
> And I guess bt1 is the only driver where this will make a difference
> (after the next patch) because no other dwc drivers set their own DMA
> masks?
Right. But that's only because I am sure the Baikal-T1 PCIe eDMA
engine is able to reach memory above 4GB limits. If you know any other
DW PCIe RP/EP controller with eDMA-capability and which can work with
the memory space wider than 4GB then you can update it' streaming
DMA-mask too. Note lacking to do so doesn't mean the eDMA-engine
driver won't work for them. The eDMA-engine will be forced to work
with the lowest 4GB space to which the upper memory will be mapped via
SWIOTLB.
-Serge(y)
>
> > if (ret)
> > dev_warn(dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
> >
> > --
> > 2.39.0
> >
> >
On Sun, Jan 22, 2023 at 02:03:59AM +0300, Serge Semin wrote:
> On Fri, Jan 20, 2023 at 06:54:01PM -0600, Bjorn Helgaas wrote:
> > On Fri, Jan 13, 2023 at 08:14:01PM +0300, Serge Semin wrote:
> > > Instead of splitting the 64-bits IOs up into two 32-bits ones it's
> > > possible to use the already available non-atomic readq/writeq methods
> > > implemented exactly for such cases. They are defined in the dedicated
> > > header files io-64-nonatomic-lo-hi.h/io-64-nonatomic-hi-lo.h. So in case
> > > if the 64-bits readq/writeq methods are unavailable on some platforms at
> > > consideration, the corresponding drivers can have any of these headers
> > > included and stop locally re-implementing the 64-bits IO accessors taking
> > > into account the non-atomic nature of the included methods. Let's do that
> > > in the DW eDMA driver too. Note by doing so we can discard the
> > > CONFIG_64BIT config ifdefs from the code.
> > >
> > > Signed-off-by: Serge Semin <[email protected]>
> > > Reviewed-by: Manivannan Sadhasivam <[email protected]>
> > > Tested-by: Manivannan Sadhasivam <[email protected]>
> > > Acked-by: Vinod Koul <[email protected]>
> > > ---
> > > drivers/dma/dw-edma/dw-edma-v0-core.c | 55 +++++++++------------------
> > > 1 file changed, 18 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > index 66f296daac5a..51a34b43434c 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > @@ -8,6 +8,8 @@
> > >
> > > #include <linux/bitfield.h>
> > >
> > > +#include <linux/io-64-nonatomic-lo-hi.h>
> > > +
> > > #include "dw-edma-core.h"
> > > #include "dw-edma-v0-core.h"
> > > #include "dw-edma-v0-regs.h"
> > > @@ -53,8 +55,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> > > SET_32(dw, rd_##name, value); \
> > > } while (0)
> > >
> > > -#ifdef CONFIG_64BIT
> > > -
> > > #define SET_64(dw, name, value) \
> > > writeq(value, &(__dw_regs(dw)->name))
> > >
> > > @@ -80,8 +80,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> > > SET_64(dw, rd_##name, value); \
> > > } while (0)
> > >
> > > -#endif /* CONFIG_64BIT */
> >
>
> > Great to get rid of these #ifdefs!
> >
> > Am I missing something? It looks like SET_64 is used only by
> > SET_RW_64 and SET_BOTH_64, and neither of *them is used at all.
> >
> > Similarly for GET_64 and GET_RW_64.
> >
> > So maybe we could get rid of everything inside the #ifdefs as well?
>
> Even though these macros are indeed unused in the driver they are
> still a part of the DW eDMA CSRs access interface. In particular they
> are supposed to be used to access the 64-bit registers declared in the
> dw_edma_v0_regs, dw_edma_v0_unroll and dw_edma_v0_ch_regs structures.
> So until the interface is converted to a more preferable direct MMIO
> usage without any packed-structures I'd rather leave these macros
> be.
> ...
> As I said above I'd rather leave the 64-bit accessors be until the
> packed structure-based interface is removed from the driver.
I wouldn't bother polishing something that's unused since it can't be
tested, it's easy to resurrect from the history if/when it becomes
necessary, and it makes it much harder to connect the commit log with
the code change. But this isn't a drivers/pci change, so I'm fine
with it since Vinod acked it.
I guess the point is that when !CONFIG_64BIT, there was no writeq() so
we used SET_LL_32 twice. linux/io-64-nonatomic-lo-hi.h provides that
writeq() implementation, so we can define and use SET_LL_64 for that
case.
Bjorn
On Mon, Jan 23, 2023 at 10:37:09AM -0600, Bjorn Helgaas wrote:
> On Sun, Jan 22, 2023 at 02:03:59AM +0300, Serge Semin wrote:
> > On Fri, Jan 20, 2023 at 06:54:01PM -0600, Bjorn Helgaas wrote:
> > > On Fri, Jan 13, 2023 at 08:14:01PM +0300, Serge Semin wrote:
> > > > Instead of splitting the 64-bits IOs up into two 32-bits ones it's
> > > > possible to use the already available non-atomic readq/writeq methods
> > > > implemented exactly for such cases. They are defined in the dedicated
> > > > header files io-64-nonatomic-lo-hi.h/io-64-nonatomic-hi-lo.h. So in case
> > > > if the 64-bits readq/writeq methods are unavailable on some platforms at
> > > > consideration, the corresponding drivers can have any of these headers
> > > > included and stop locally re-implementing the 64-bits IO accessors taking
> > > > into account the non-atomic nature of the included methods. Let's do that
> > > > in the DW eDMA driver too. Note by doing so we can discard the
> > > > CONFIG_64BIT config ifdefs from the code.
> > > >
> > > > Signed-off-by: Serge Semin <[email protected]>
> > > > Reviewed-by: Manivannan Sadhasivam <[email protected]>
> > > > Tested-by: Manivannan Sadhasivam <[email protected]>
> > > > Acked-by: Vinod Koul <[email protected]>
> > > > ---
> > > > drivers/dma/dw-edma/dw-edma-v0-core.c | 55 +++++++++------------------
> > > > 1 file changed, 18 insertions(+), 37 deletions(-)
> > > >
> > > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > > index 66f296daac5a..51a34b43434c 100644
> > > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > > @@ -8,6 +8,8 @@
> > > >
> > > > #include <linux/bitfield.h>
> > > >
> > > > +#include <linux/io-64-nonatomic-lo-hi.h>
> > > > +
> > > > #include "dw-edma-core.h"
> > > > #include "dw-edma-v0-core.h"
> > > > #include "dw-edma-v0-regs.h"
> > > > @@ -53,8 +55,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> > > > SET_32(dw, rd_##name, value); \
> > > > } while (0)
> > > >
> > > > -#ifdef CONFIG_64BIT
> > > > -
> > > > #define SET_64(dw, name, value) \
> > > > writeq(value, &(__dw_regs(dw)->name))
> > > >
> > > > @@ -80,8 +80,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> > > > SET_64(dw, rd_##name, value); \
> > > > } while (0)
> > > >
> > > > -#endif /* CONFIG_64BIT */
> > >
> >
> > > Great to get rid of these #ifdefs!
> > >
> > > Am I missing something? It looks like SET_64 is used only by
> > > SET_RW_64 and SET_BOTH_64, and neither of *them is used at all.
> > >
> > > Similarly for GET_64 and GET_RW_64.
> > >
> > > So maybe we could get rid of everything inside the #ifdefs as well?
> >
> > Even though these macros are indeed unused in the driver they are
> > still a part of the DW eDMA CSRs access interface. In particular they
> > are supposed to be used to access the 64-bit registers declared in the
> > dw_edma_v0_regs, dw_edma_v0_unroll and dw_edma_v0_ch_regs structures.
> > So until the interface is converted to a more preferable direct MMIO
> > usage without any packed-structures I'd rather leave these macros
> > be.
> > ...
>
> > As I said above I'd rather leave the 64-bit accessors be until the
> > packed structure-based interface is removed from the driver.
>
> I wouldn't bother polishing something that's unused since it can't be
> tested, it's easy to resurrect from the history if/when it becomes
> necessary, and it makes it much harder to connect the commit log with
> the code change. But this isn't a drivers/pci change, so I'm fine
> with it since Vinod acked it.
>
> I guess the point is that when !CONFIG_64BIT, there was no writeq() so
> we used SET_LL_32 twice. linux/io-64-nonatomic-lo-hi.h provides that
> writeq() implementation, so we can define and use SET_LL_64 for that
> case.
Right. That's what is done in this patch.
-Serge(y)
>
> Bjorn