Hi Frank,
+ Dmitry
On Tue, Mar 01, 2022 at 09:26:42PM -0600, Frank Li wrote:
> API dw_edma_probe(struct dw_edma_chip *chip) defined at include/linux/dma,
> but struct dw_edma_chip have a hidden internal struct dw_edma *dw,
> supposed dw should be allocated in API dw_edma_probe().
>
> @dw: struct dw_edma that is filed by dw_edma_probe()
>
> but dw need allocate and fill chip related information before call dw_edma_probe()
> in current code. See ref
> drivers/dma/dw-edma/dw-edma-pci.c
>
> Move chip related information from dw-edma-core.h to edma.h
> allocate memory inside dw_edma_probe()
>
> Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
I started reviewing your eDMA patches because me and my colleague Dmitry are
also working on the eDMA support for Qualcomm platforms. By looking at the eDMA
driver in mainline, it looks like there is a bit of cleanup required. So for
avoiding the duplication of efforts, let's work together for fixing the eDMA
driver and add support for respective platforms.
Please CC us for all of your future patches as well. We will also share the
patches so that all can be combined into a single series.
Thanks,
Mani
> ---
> drivers/dma/dw-edma/dw-edma-core.c | 31 ++++++++++++---
> drivers/dma/dw-edma/dw-edma-core.h | 28 +++-----------
> drivers/dma/dw-edma/dw-edma-v0-core.c | 2 +-
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 2 +-
> include/linux/dma/edma.h | 48 +++++++++++++++++++++++-
> 5 files changed, 78 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 53289927dd0d6..029085c035067 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -905,19 +905,32 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> if (!dev)
> return -EINVAL;
>
> - dw = chip->dw;
> - if (!dw || !dw->irq || !dw->ops || !dw->ops->irq_vector)
> + dw = devm_kzalloc(dev, sizeof(*dw), GFP_KERNEL);
> + if (!dw)
> + return -ENOMEM;
> +
> + chip->dw = dw;
> +
> + if (!chip->nr_irqs || !chip->ops)
> return -EINVAL;
>
> raw_spin_lock_init(&dw->lock);
>
> - dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt,
> + dw->rg_region = &chip->rg_region;
> + dw->ll_region_wr = chip->ll_region_wr;
> + dw->ll_region_rd = chip->ll_region_rd;
> + dw->dt_region_wr = chip->dt_region_wr;
> + dw->dt_region_rd = chip->dt_region_rd;
> +
> + dw->mf = chip->mf;
> +
> + dw->wr_ch_cnt = min_t(u16, chip->wr_ch_cnt,
> dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
> - dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
> + dw->wr_ch_cnt = min_t(u16, chip->wr_ch_cnt, EDMA_MAX_WR_CH);
>
> - dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt,
> + dw->rd_ch_cnt = min_t(u16, chip->rd_ch_cnt,
> dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
> - dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
> + dw->rd_ch_cnt = min_t(u16, chip->rd_ch_cnt, EDMA_MAX_RD_CH);
>
> if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
> return -EINVAL;
> @@ -936,6 +949,12 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> /* Disable eDMA, only to establish the ideal initial conditions */
> dw_edma_v0_core_off(dw);
>
> + dw->nr_irqs = chip->nr_irqs;
> + dw->ops = chip->ops;
> + dw->irq = devm_kcalloc(dev, dw->nr_irqs, sizeof(*dw->irq), GFP_KERNEL);
> + if (!dw->irq)
> + return -ENOMEM;
> +
> /* Request IRQs */
> err = dw_edma_irq_request(chip, &wr_alloc, &rd_alloc);
> if (err)
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 60316d408c3e0..8ca195814a878 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -15,20 +15,12 @@
> #include "../virt-dma.h"
>
> #define EDMA_LL_SZ 24
> -#define EDMA_MAX_WR_CH 8
> -#define EDMA_MAX_RD_CH 8
>
> enum dw_edma_dir {
> EDMA_DIR_WRITE = 0,
> EDMA_DIR_READ
> };
>
> -enum dw_edma_map_format {
> - EDMA_MF_EDMA_LEGACY = 0x0,
> - EDMA_MF_EDMA_UNROLL = 0x1,
> - EDMA_MF_HDMA_COMPAT = 0x5
> -};
> -
> enum dw_edma_request {
> EDMA_REQ_NONE = 0,
> EDMA_REQ_STOP,
> @@ -57,12 +49,6 @@ struct dw_edma_burst {
> u32 sz;
> };
>
> -struct dw_edma_region {
> - phys_addr_t paddr;
> - void __iomem *vaddr;
> - size_t sz;
> -};
> -
> struct dw_edma_chunk {
> struct list_head list;
> struct dw_edma_chan *chan;
> @@ -109,10 +95,6 @@ struct dw_edma_irq {
> struct dw_edma *dw;
> };
>
> -struct dw_edma_core_ops {
> - int (*irq_vector)(struct device *dev, unsigned int nr);
> -};
> -
> struct dw_edma {
> char name[20];
>
> @@ -122,11 +104,11 @@ struct dw_edma {
> struct dma_device rd_edma;
> u16 rd_ch_cnt;
>
> - struct dw_edma_region rg_region; /* Registers */
> - struct dw_edma_region ll_region_wr[EDMA_MAX_WR_CH];
> - struct dw_edma_region ll_region_rd[EDMA_MAX_RD_CH];
> - struct dw_edma_region dt_region_wr[EDMA_MAX_WR_CH];
> - struct dw_edma_region dt_region_rd[EDMA_MAX_RD_CH];
> + struct dw_edma_region *rg_region; /* Registers */
> + struct dw_edma_region *ll_region_wr;
> + struct dw_edma_region *ll_region_rd;
> + struct dw_edma_region *dt_region_wr;
> + struct dw_edma_region *dt_region_rd;
>
> struct dw_edma_irq *irq;
> int nr_irqs;
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 329fc2e57b703..884ba55fbd530 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -25,7 +25,7 @@ enum dw_edma_control {
>
> static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> {
> - return dw->rg_region.vaddr;
> + return dw->rg_region->vaddr;
> }
>
> #define SET_32(dw, name, value) \
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
> index 4b3bcffd15ef1..a42047791e727 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
> @@ -288,7 +288,7 @@ void dw_edma_v0_debugfs_on(struct dw_edma_chip *chip)
> if (!dw)
> return;
>
> - regs = dw->rg_region.vaddr;
> + regs = dw->rg_region->vaddr;
> if (!regs)
> return;
>
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index cab6e18773dad..fdb19c717aa09 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -12,19 +12,63 @@
> #include <linux/device.h>
> #include <linux/dmaengine.h>
>
> +#define EDMA_MAX_WR_CH 8
> +#define EDMA_MAX_RD_CH 8
> +
> struct dw_edma;
>
> +struct dw_edma_region {
> + phys_addr_t paddr;
> + void __iomem *vaddr;
> + size_t sz;
> +};
> +
> +struct dw_edma_core_ops {
> + int (*irq_vector)(struct device *dev, unsigned int nr);
> +};
> +
> +enum dw_edma_map_format {
> + EDMA_MF_EDMA_LEGACY = 0x0,
> + EDMA_MF_EDMA_UNROLL = 0x1,
> + EDMA_MF_HDMA_COMPAT = 0x5
> +};
> +
> /**
> * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> * @dev: struct device of the eDMA controller
> * @id: instance ID
> - * @irq: irq line
> + * @nr_irqs: total dma irq number
> + * reg64bit if support 64bit write to register
> + * @ops DMA channel to IRQ number mapping
> + * @wr_ch_cnt DMA write channel number
> + * @rd_ch_cnt DMA read channel number
> + * @rg_region DMA register region
> + * @ll_region_wr DMA descriptor link list memory for write channel
> + * @ll_region_rd DMA descriptor link list memory for read channel
> + * @mf DMA register map format
> * @dw: struct dw_edma that is filed by dw_edma_probe()
> */
> struct dw_edma_chip {
> struct device *dev;
> int id;
> - int irq;
> + int nr_irqs;
> + const struct dw_edma_core_ops *ops;
> +
> + u16 wr_ch_cnt;
> + u16 rd_ch_cnt;
> +
> + struct dw_edma_region rg_region; /* Registers */
> +
> + /* link list address */
> + struct dw_edma_region ll_region_wr[EDMA_MAX_WR_CH];
> + struct dw_edma_region ll_region_rd[EDMA_MAX_RD_CH];
> +
> + /* data region */
> + struct dw_edma_region dt_region_wr[EDMA_MAX_WR_CH];
> + struct dw_edma_region dt_region_rd[EDMA_MAX_RD_CH];
> +
> + enum dw_edma_map_format mf;
> +
> struct dw_edma *dw;
> };
>
> --
> 2.24.0.rc1
>
