Hi, (you don't really need the RFC tag. RFC tags are usually meant to ask comments on the general approach, not the implementation). On Mon, Dec 03, 2018 at 12:23:08AM +0300, Mesih Kilinc wrote: > Allwinner suniv F1C100s has similar DMA engine to sun4i. Several > registers has different addresses. Total dma channels, endpoint counts > and max burst counts are also different. > > In order to support F1C100s add a quirk structure to hold IC specific > data. > > Signed-off-by: Mesih Kilinc > --- > drivers/dma/sun4i-dma.c | 138 +++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 106 insertions(+), 32 deletions(-) > > diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c > index f4ed3f1..e86b424 100644 > --- a/drivers/dma/sun4i-dma.c > +++ b/drivers/dma/sun4i-dma.c > @@ -16,6 +16,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -34,6 +35,8 @@ > #define SUN4I_DMA_CFG_SRC_ADDR_MODE(mode) ((mode) << 5) > #define SUN4I_DMA_CFG_SRC_DRQ_TYPE(type) (type) > > +#define SUN4I_MAX_BURST 8 > + > /** Normal DMA register values **/ > > /* Normal DMA source/destination data request type values */ > @@ -126,6 +129,32 @@ > SUN4I_DDMA_PARA_DST_WAIT_CYCLES(2) | \ > SUN4I_DDMA_PARA_SRC_WAIT_CYCLES(2)) > > +/* > + * Hardware channels / ports representation > + * > + * The hardware is used in several SoCs, with differing numbers > + * of channels and endpoints. This structure ties those numbers > + * to a certain compatible string. > + */ > +struct sun4i_dma_config { > + u32 ndma_nr_max_channels; > + u32 ndma_nr_max_vchans; > + > + u32 ddma_nr_max_channels; > + u32 ddma_nr_max_vchans; > + > + u32 dma_nr_max_channels; > + > + void (*set_dst_data_width)(u32 *p_cfg, s8 data_width); > + void (*set_src_data_width)(u32 *p_cfg, s8 data_width); This should be indented with tabs, not spaces. > + int (*convert_burst)(u32 maxburst); > + > + u8 ndma_drq_sdram; > + u8 ddma_drq_sdram; > + > + u8 max_burst; You'd be better off using a bitmask wit hthe supported bursts, like we're doing in sun6i-dma. > +}; > + > struct sun4i_dma_pchan { > /* Register base of channel */ > void __iomem *base; > @@ -163,7 +192,7 @@ struct sun4i_dma_contract { > }; > > struct sun4i_dma_dev { > - DECLARE_BITMAP(pchans_used, SUN4I_DMA_NR_MAX_CHANNELS); > + unsigned long *pchans_used; > struct dma_device slave; > struct sun4i_dma_pchan *pchans; > struct sun4i_dma_vchan *vchans; > @@ -171,6 +200,7 @@ struct sun4i_dma_dev { > struct clk *clk; > int irq; > spinlock_t lock; > + const struct sun4i_dma_config *cfg; This should be aligned to the rest of the members, using tabs. > }; > > static struct sun4i_dma_dev *to_sun4i_dma_dev(struct dma_device *dev) > @@ -193,7 +223,17 @@ static struct device *chan2dev(struct dma_chan *chan) > return &chan->dev->device; > } > > -static int convert_burst(u32 maxburst) > +static void set_dst_data_width_a10(u32 *p_cfg, s8 data_width) > +{ > + *p_cfg |= SUN4I_DMA_CFG_DST_DATA_WIDTH(data_width); > +} > + > +static void set_src_data_width_a10(u32 *p_cfg, s8 data_width) > +{ > + *p_cfg |= SUN4I_DMA_CFG_SRC_DATA_WIDTH(data_width); > +} > + > +static int convert_burst_a10(u32 maxburst) > { > if (maxburst > 8) > return -EINVAL; > @@ -226,15 +266,15 @@ static struct sun4i_dma_pchan *find_and_use_pchan(struct sun4i_dma_dev *priv, > int i, max; > > /* > - * pchans 0-SUN4I_NDMA_NR_MAX_CHANNELS are normal, and > - * SUN4I_NDMA_NR_MAX_CHANNELS+ are dedicated ones > + * pchans 0-priv->cfg->ndma_nr_max_channels are normal, and > + * priv->cfg->ndma_nr_max_channels+ are dedicated ones This should be next to the structure you just created. > */ > if (vchan->is_dedicated) { > - i = SUN4I_NDMA_NR_MAX_CHANNELS; > - max = SUN4I_DMA_NR_MAX_CHANNELS; > + i = priv->cfg->ndma_nr_max_channels; > + max = priv->cfg->dma_nr_max_channels; > } else { > i = 0; > - max = SUN4I_NDMA_NR_MAX_CHANNELS; > + max = priv->cfg->ndma_nr_max_channels; > } > > spin_lock_irqsave(&priv->lock, flags); > @@ -437,6 +477,7 @@ generate_ndma_promise(struct dma_chan *chan, dma_addr_t src, dma_addr_t dest, > size_t len, struct dma_slave_config *sconfig, > enum dma_transfer_direction direction) > { > + struct sun4i_dma_dev *priv = to_sun4i_dma_dev(chan->device); > struct sun4i_dma_promise *promise; > int ret; > > @@ -460,13 +501,13 @@ generate_ndma_promise(struct dma_chan *chan, dma_addr_t src, dma_addr_t dest, > sconfig->src_addr_width, sconfig->dst_addr_width); > > /* Source burst */ > - ret = convert_burst(sconfig->src_maxburst); > + ret = priv->cfg->convert_burst(sconfig->src_maxburst); > if (ret < 0) > goto fail; > promise->cfg |= SUN4I_DMA_CFG_SRC_BURST_LENGTH(ret); > > /* Destination burst */ > - ret = convert_burst(sconfig->dst_maxburst); > + ret = priv->cfg->convert_burst(sconfig->dst_maxburst); > if (ret < 0) > goto fail; > promise->cfg |= SUN4I_DMA_CFG_DST_BURST_LENGTH(ret); > @@ -475,13 +516,13 @@ generate_ndma_promise(struct dma_chan *chan, dma_addr_t src, dma_addr_t dest, > ret = convert_buswidth(sconfig->src_addr_width); > if (ret < 0) > goto fail; > - promise->cfg |= SUN4I_DMA_CFG_SRC_DATA_WIDTH(ret); > + priv->cfg->set_src_data_width(&promise->cfg, ret); > > /* Destination bus width */ > ret = convert_buswidth(sconfig->dst_addr_width); > if (ret < 0) > goto fail; > - promise->cfg |= SUN4I_DMA_CFG_DST_DATA_WIDTH(ret); > + priv->cfg->set_dst_data_width(&promise->cfg, ret); > > return promise; > > @@ -503,6 +544,7 @@ static struct sun4i_dma_promise * > generate_ddma_promise(struct dma_chan *chan, dma_addr_t src, dma_addr_t dest, > size_t len, struct dma_slave_config *sconfig) > { > + struct sun4i_dma_dev *priv = to_sun4i_dma_dev(chan->device); > struct sun4i_dma_promise *promise; > int ret; > > @@ -517,13 +559,13 @@ generate_ddma_promise(struct dma_chan *chan, dma_addr_t src, dma_addr_t dest, > SUN4I_DDMA_CFG_BYTE_COUNT_MODE_REMAIN; > > /* Source burst */ > - ret = convert_burst(sconfig->src_maxburst); > + ret = priv->cfg->convert_burst(sconfig->src_maxburst); > if (ret < 0) > goto fail; > promise->cfg |= SUN4I_DMA_CFG_SRC_BURST_LENGTH(ret); > > /* Destination burst */ > - ret = convert_burst(sconfig->dst_maxburst); > + ret = priv->cfg->convert_burst(sconfig->dst_maxburst); > if (ret < 0) > goto fail; > promise->cfg |= SUN4I_DMA_CFG_DST_BURST_LENGTH(ret); > @@ -532,13 +574,13 @@ generate_ddma_promise(struct dma_chan *chan, dma_addr_t src, dma_addr_t dest, > ret = convert_buswidth(sconfig->src_addr_width); > if (ret < 0) > goto fail; > - promise->cfg |= SUN4I_DMA_CFG_SRC_DATA_WIDTH(ret); > + priv->cfg->set_src_data_width(&promise->cfg, ret); > > /* Destination bus width */ > ret = convert_buswidth(sconfig->dst_addr_width); > if (ret < 0) > goto fail; > - promise->cfg |= SUN4I_DMA_CFG_DST_DATA_WIDTH(ret); > + priv->cfg->set_dst_data_width(&promise->cfg, ret); > > return promise; > > @@ -615,6 +657,7 @@ static struct dma_async_tx_descriptor * > sun4i_dma_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, > dma_addr_t src, size_t len, unsigned long flags) > { > + struct sun4i_dma_dev *priv = to_sun4i_dma_dev(chan->device); > struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan); > struct dma_slave_config *sconfig = &vchan->cfg; > struct sun4i_dma_promise *promise; > @@ -631,8 +674,8 @@ sun4i_dma_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, > */ > sconfig->src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; > sconfig->dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; > - sconfig->src_maxburst = 8; > - sconfig->dst_maxburst = 8; > + sconfig->src_maxburst = priv->cfg->max_burst; > + sconfig->dst_maxburst = priv->cfg->max_burst; > > if (vchan->is_dedicated) > promise = generate_ddma_promise(chan, src, dest, len, sconfig); > @@ -647,11 +690,13 @@ sun4i_dma_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, > > /* Configure memcpy mode */ > if (vchan->is_dedicated) { > - promise->cfg |= SUN4I_DMA_CFG_SRC_DRQ_TYPE(SUN4I_DDMA_DRQ_TYPE_SDRAM) | > - SUN4I_DMA_CFG_DST_DRQ_TYPE(SUN4I_DDMA_DRQ_TYPE_SDRAM); > + promise->cfg |= > + SUN4I_DMA_CFG_SRC_DRQ_TYPE(priv->cfg->ddma_drq_sdram) | > + SUN4I_DMA_CFG_DST_DRQ_TYPE(priv->cfg->ddma_drq_sdram); > } else { > - promise->cfg |= SUN4I_DMA_CFG_SRC_DRQ_TYPE(SUN4I_NDMA_DRQ_TYPE_SDRAM) | > - SUN4I_DMA_CFG_DST_DRQ_TYPE(SUN4I_NDMA_DRQ_TYPE_SDRAM); > + promise->cfg |= > + SUN4I_DMA_CFG_SRC_DRQ_TYPE(priv->cfg->ndma_drq_sdram) | > + SUN4I_DMA_CFG_DST_DRQ_TYPE(priv->cfg->ndma_drq_sdram); > } > > /* Fill the contract with our only promise */ > @@ -666,6 +711,7 @@ sun4i_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf, size_t len, > size_t period_len, enum dma_transfer_direction dir, > unsigned long flags) > { > + struct sun4i_dma_dev *priv = to_sun4i_dma_dev(chan->device); > struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan); > struct dma_slave_config *sconfig = &vchan->cfg; > struct sun4i_dma_promise *promise; > @@ -701,7 +747,7 @@ sun4i_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf, size_t len, > if (dir == DMA_MEM_TO_DEV) { > src = buf; > dest = sconfig->dst_addr; > - endpoints = SUN4I_DMA_CFG_SRC_DRQ_TYPE(SUN4I_NDMA_DRQ_TYPE_SDRAM) | > + endpoints = SUN4I_DMA_CFG_SRC_DRQ_TYPE(priv->cfg->ndma_drq_sdram) | > SUN4I_DMA_CFG_DST_DRQ_TYPE(vchan->endpoint) | > SUN4I_DMA_CFG_DST_ADDR_MODE(SUN4I_NDMA_ADDR_MODE_IO); > } else { > @@ -709,7 +755,7 @@ sun4i_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf, size_t len, > dest = buf; > endpoints = SUN4I_DMA_CFG_SRC_DRQ_TYPE(vchan->endpoint) | > SUN4I_DMA_CFG_SRC_ADDR_MODE(SUN4I_NDMA_ADDR_MODE_IO) | > - SUN4I_DMA_CFG_DST_DRQ_TYPE(SUN4I_NDMA_DRQ_TYPE_SDRAM); > + SUN4I_DMA_CFG_DST_DRQ_TYPE(priv->cfg->ndma_drq_sdram); > } > > /* > @@ -772,6 +818,7 @@ sun4i_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, > unsigned int sg_len, enum dma_transfer_direction dir, > unsigned long flags, void *context) > { > + struct sun4i_dma_dev *priv = to_sun4i_dma_dev(chan->device); > struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan); > struct dma_slave_config *sconfig = &vchan->cfg; > struct sun4i_dma_promise *promise; > @@ -797,11 +844,11 @@ sun4i_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, > if (vchan->is_dedicated) { > io_mode = SUN4I_DDMA_ADDR_MODE_IO; > linear_mode = SUN4I_DDMA_ADDR_MODE_LINEAR; > - ram_type = SUN4I_DDMA_DRQ_TYPE_SDRAM; > + ram_type = priv->cfg->ddma_drq_sdram; > } else { > io_mode = SUN4I_NDMA_ADDR_MODE_IO; > linear_mode = SUN4I_NDMA_ADDR_MODE_LINEAR; > - ram_type = SUN4I_NDMA_DRQ_TYPE_SDRAM; > + ram_type = priv->cfg->ndma_drq_sdram; > } > > if (dir == DMA_MEM_TO_DEV) > @@ -1130,6 +1177,10 @@ static int sun4i_dma_probe(struct platform_device *pdev) > if (!priv) > return -ENOMEM; > > + priv->cfg = of_device_get_match_data(&pdev->dev); > + if (!priv->cfg) > + return -ENODEV; > + > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > priv->base = devm_ioremap_resource(&pdev->dev, res); > if (IS_ERR(priv->base)) > @@ -1178,23 +1229,26 @@ static int sun4i_dma_probe(struct platform_device *pdev) > > priv->slave.dev = &pdev->dev; > > - priv->pchans = devm_kcalloc(&pdev->dev, SUN4I_DMA_NR_MAX_CHANNELS, > + priv->pchans = devm_kcalloc(&pdev->dev, priv->cfg->dma_nr_max_channels, > sizeof(struct sun4i_dma_pchan), GFP_KERNEL); > priv->vchans = devm_kcalloc(&pdev->dev, SUN4I_DMA_NR_MAX_VCHANS, > sizeof(struct sun4i_dma_vchan), GFP_KERNEL); > - if (!priv->vchans || !priv->pchans) > + priv->pchans_used = devm_kcalloc(&pdev->dev, > + BITS_TO_LONGS(priv->cfg->dma_nr_max_channels), > + sizeof(unsigned long), GFP_KERNEL); I'm not sure we really need a dynamic allocation here. Just keep the bitmap, and use the bigger size. Thanks! Maxime