Re: [RFC PATCH 01/10] dma-engine: sun4i: Add a quirk to support different chips

From: Maxime Ripard
Date: Mon Dec 03 2018 - 05:54:57 EST

Next message: Jan Beulich: "Re: [Xen-devel] [PATCH 1/2] xen-pciback: Fix error return in bar_write() and rom_write()"
Previous message: Ingo Molnar: "Re: [PATCH] tools: Fix diverse typos"
In reply to: Mesih Kilinc: "[RFC PATCH 01/10] dma-engine: sun4i: Add a quirk to support different chips"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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 <mesihkilinc@xxxxxxxxx>
> ---
> 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 <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/of_dma.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> @@ -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

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Attachment: signature.asc
Description: PGP signature

Next message: Jan Beulich: "Re: [Xen-devel] [PATCH 1/2] xen-pciback: Fix error return in bar_write() and rom_write()"
Previous message: Ingo Molnar: "Re: [PATCH] tools: Fix diverse typos"
In reply to: Mesih Kilinc: "[RFC PATCH 01/10] dma-engine: sun4i: Add a quirk to support different chips"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]