From: Maxime Ripard <maxime.ripard@bootlin.com>
To: Mesih Kilinc <mesihkilinc@gmail.com>
Cc: dmaengine@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org,
linux-sunxi@googlegroups.com, Vinod Koul <vkoul@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>, Chen-Yu Tsai <wens@csie.org>,
Liam Girdwood <lgirdwood@gmail.com>,
Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
Takashi Iwai <tiwai@suse.com>
Subject: Re: [RFC PATCH 01/10] dma-engine: sun4i: Add a quirk to support different chips
Date: Mon, 3 Dec 2018 11:54:40 +0100 [thread overview]
Message-ID: <20181203105440.jxy6ue75ic2fc5t5@flea> (raw)
In-Reply-To: <864e28404a31ba24094f74fd060d11c16562e965.1543782328.git.mesihkilinc@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 12664 bytes --]
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@gmail.com>
> ---
> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2018-12-03 10:54 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-02 21:23 [RFC PATCH 00/10] Add support for DMA and audio codec of F1C100s Mesih Kilinc
2018-12-02 21:23 ` [RFC PATCH 01/10] dma-engine: sun4i: Add a quirk to support different chips Mesih Kilinc
2018-12-03 10:54 ` Maxime Ripard [this message]
2019-01-04 15:38 ` Vinod Koul
2018-12-02 21:23 ` [RFC PATCH 02/10] dma-engine: sun4i: Add has_reset option to quirk Mesih Kilinc
2018-12-02 21:23 ` [RFC PATCH 03/10] dt-bindings: dmaengine: Add Allwinner suniv F1C100s DMA Mesih Kilinc
2018-12-19 14:03 ` Rob Herring
2018-12-02 21:23 ` [RFC PATCH 04/10] dma-engine: sun4i: Add support for Allwinner suniv F1C100s Mesih Kilinc
2018-12-03 10:56 ` Maxime Ripard
2018-12-03 11:00 ` Maxime Ripard
2018-12-02 21:23 ` [RFC PATCH 05/10] ARM: dts: suniv: f1c100s: Add support for DMA Mesih Kilinc
2018-12-02 21:23 ` [RFC PATCH 06/10] ASoC: sun4i-codec: Add DMA Max Burst field Mesih Kilinc
2018-12-02 21:23 ` [RFC PATCH 07/10] dt-bindigs: sound: Add Allwinner suniv F1C100s Audio Codec Mesih Kilinc
2018-12-19 14:04 ` Rob Herring
2018-12-02 21:23 ` [RFC PATCH 08/10] ASoC: sun4i-codec: Add support for Allwinner suniv F1C100s Mesih Kilinc
2018-12-02 21:23 ` [RFC PATCH 09/10] ARM: dts: suniv: f1c100s: Add support for Audio Codec Mesih Kilinc
2018-12-02 21:23 ` [RFC PATCH 10/10] ARM: dts: suniv: f1c100s: Activate Audio Codec for Lichee Pi Nano Mesih Kilinc
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181203105440.jxy6ue75ic2fc5t5@flea \
--to=maxime.ripard@bootlin.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dmaengine@vger.kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sunxi@googlegroups.com \
--cc=mark.rutland@arm.com \
--cc=mesihkilinc@gmail.com \
--cc=perex@perex.cz \
--cc=robh+dt@kernel.org \
--cc=tiwai@suse.com \
--cc=vkoul@kernel.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).