On Thu, Mar 03, 2022 at 12:46:33PM -0600, Frank Li wrote:
> Allow user don't enable remote MSI irq.
> PCI ep probe dma locally, don't want to raise irq
> to remote PCI host.
>
> Add option allow force 32bit register access even at
> 64bit system. i.MX8 hardware only allowed 32bit register
> access.
>
> Add option allow EP side probe dma. remote side dma is
> continue physical memory, local memory is scatter list.
s/Allow user don't enable remote MSI/Allow user to prevent use of remote MSI/ ?
s/irq/IRQ/ (several, and in comment below)
s/ep/EP/ (capitalize consistently)
s/dma/DMA/ (several)
s/Add option allow force 32bit/Add option to force 32-bit/
s/even at 64bit system/even on 64-bit systems/
s/remote side/Remote side/ (start sentence with capital letter)
s/Add option allow EP side probe dma/Add option to allow EP to probe DMA/
(I'm not quite sure what this means).
s/continue physical memory/contiguous physical memory/ ?
Rewrap above to fill 75 columns.
> Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
> ---
> Change from v1 to v2
> - none
>
> drivers/dma/dw-edma/dw-edma-core.c | 7 ++++++-
> drivers/dma/dw-edma/dw-edma-v0-core.c | 14 ++++++++++----
> include/linux/dma/edma.h | 7 +++++++
> 3 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 0cb66434f9e14..a43bb26c8bf96 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -336,6 +336,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
> struct dw_edma_desc *desc;
> u32 cnt = 0;
> int i;
> + bool b;
>
> if (!chan->configured)
> return NULL;
> @@ -424,7 +425,11 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
> chunk->ll_region.sz += burst->sz;
> desc->alloc_sz += burst->sz;
>
> - if (chan->dir == EDMA_DIR_WRITE) {
> + b = (chan->dir == EDMA_DIR_WRITE);
> + if (chan->chip->flags & DW_EDMA_CHIP_LOCAL_EP)
> + b = !b;
> +
> + if (b) {
> burst->sar = src_addr;
> if (xfer->type == EDMA_XFER_CYCLIC) {
> burst->dar = xfer->xfer.cyclic.paddr;
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 6e2f83e31a03a..d5c2415e2c616 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -307,13 +307,18 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir
> static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> {
> struct dw_edma_burst *child;
> + struct dw_edma_chan *chan = chunk->chan;
> struct dw_edma_v0_lli __iomem *lli;
> struct dw_edma_v0_llp __iomem *llp;
> u32 control = 0, i = 0;
> + u32 rie = 0;
> int j;
>
> lli = chunk->ll_region.vaddr;
>
> + if (!(chan->chip->flags & DW_EDMA_CHIP_NO_MSI))
> + rie = DW_EDMA_V0_RIE;
> +
> if (chunk->cb)
> control = DW_EDMA_V0_CB;
>
> @@ -321,7 +326,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> list_for_each_entry(child, &chunk->burst->list, list) {
> j--;
> if (!j)
> - control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> + control |= (DW_EDMA_V0_LIE | rie);
Could drop "rie" and do this, which would keep DW_EDMA_CHIP_NO_MSI
closer to where it's used:
if (!j) {
control |= DW_EDMA_V0_LIE;
if (!(chan->chip->flags & DW_EDMA_CHIP_NO_MSI))
control |= DW_EDMA_V0_RIE;
}
> /* Channel control */
> SET_LL_32(&lli[i].control, control);
> @@ -420,15 +425,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> SET_CH_32(chip, chan->dir, chan->id, ch_control1,
> (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
> /* Linked list */
> - #ifdef CONFIG_64BIT
> + if (!(chan->chip->flags & DW_EDMA_CHIP_REG32BIT) &&
> + IS_ENABLED(CONFIG_64BIT)) {
> SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> chunk->ll_region.paddr);
> - #else /* CONFIG_64BIT */
> + } else {
> SET_CH_32(chip, chan->dir, chan->id, llp.lsb,
> lower_32_bits(chunk->ll_region.paddr));
> SET_CH_32(chip, chan->dir, chan->id, llp.msb,
> upper_32_bits(chunk->ll_region.paddr));
> - #endif /* CONFIG_64BIT */
I think this would read better if you reversed the sense:
if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) ||
!IS_ENABLED(CONFIG_64BIT)) {
SET_CH_32(...);
SET_CH_32(...);
} else {
SET_CH_64(...);
}
> + }
> }
> /* Doorbell */
> SET_RW_32(chip, chan->dir, doorbell,
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index fcfbc0f47f83d..e74ee15d9832a 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -33,6 +33,10 @@ enum dw_edma_map_format {
> EDMA_MF_HDMA_COMPAT = 0x5
> };
>
> +#define DW_EDMA_CHIP_NO_MSI BIT(0)
> +#define DW_EDMA_CHIP_REG32BIT BIT(1)
> +#define DW_EDMA_CHIP_LOCAL_EP BIT(2)
> +
> /**
> * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> * @dev: struct device of the eDMA controller
> @@ -40,6 +44,8 @@ enum dw_edma_map_format {
> * @nr_irqs: total dma irq number
> * reg64bit if support 64bit write to register
> * @ops DMA channel to IRQ number mapping
> + * @flags - DW_EDMA_CHIP_NO_MSI can't generate remote MSI irq
> + * - DW_EDMA_CHIP_REG32BIT only support 32bit register write
Looks like DW_EDMA_CHIP_LOCAL_EP should be documented here, too?
> * @wr_ch_cnt DMA write channel number
> * @rd_ch_cnt DMA read channel number
> * @rg_region DMA register region
> @@ -53,6 +59,7 @@ struct dw_edma_chip {
> int id;
> int nr_irqs;
> const struct dw_edma_core_ops *ops;
> + u32 flags;
>
> void __iomem *reg_base;
>
> --
> 2.24.0.rc1
>
