DMA Engine development
 help / color / mirror / Atom feed
* [06/14] dmaengine: dma-jz4780: Add support for the JZ4725B SoC
From: PrasannaKumar Muralidharan @ 2018-07-04 16:55 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Vinod Koul, Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton,
	James Hogan, Zubair Lutfullah Kakakhel, Mathieu Malaterre,
	Daniel Silsby, dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Linux-MIPS

On 3 July 2018 at 18:02, Paul Cercueil <paul@crapouillou.net> wrote:
> The JZ4725B has one DMA core starring six DMA channels.
> As for the JZ4770, each DMA channel's clock can be enabled with
> a register write, the difference here being that once started, it
> is not possible to turn it off.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  Documentation/devicetree/bindings/dma/jz4780-dma.txt | 1 +
>  drivers/dma/dma-jz4780.c                             | 6 ++++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/dma/jz4780-dma.txt b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
> index d7ca3f925fdf..5d302b488e88 100644
> --- a/Documentation/devicetree/bindings/dma/jz4780-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
> @@ -5,6 +5,7 @@ Required properties:
>  - compatible: Should be one of:
>    * ingenic,jz4780-dma
>    * ingenic,jz4770-dma
> +  * ingenic,jz4725b-dma
>    * ingenic,jz4740-dma
>  - reg: Should contain the DMA channel registers location and length, followed
>    by the DMA controller registers location and length.
> diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
> index ccadbe61dde7..922e4031e70e 100644
> --- a/drivers/dma/dma-jz4780.c
> +++ b/drivers/dma/dma-jz4780.c
> @@ -134,6 +134,7 @@ struct jz4780_dma_chan {
>
>  enum jz_version {
>         ID_JZ4740,
> +       ID_JZ4725B,
>         ID_JZ4770,
>         ID_JZ4780,
>  };
> @@ -204,6 +205,8 @@ static inline void jz4780_dma_chan_enable(struct jz4780_dma_dev *jzdma,
>  {
>         if (jzdma->version == ID_JZ4770)
>                 jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKES, BIT(chn));
> +       else if (jzdma->version == ID_JZ4725B)
> +               jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKE, BIT(chn));
>  }
>
>  static inline void jz4780_dma_chan_disable(struct jz4780_dma_dev *jzdma,
> @@ -249,6 +252,7 @@ static void jz4780_dma_desc_free(struct virt_dma_desc *vdesc)
>
>  static const unsigned int jz4780_dma_ord_max[] = {
>         [ID_JZ4740] = 5,
> +       [ID_JZ4725B] = 5,
>         [ID_JZ4770] = 6,
>         [ID_JZ4780] = 7,
>  };
> @@ -804,12 +808,14 @@ static struct dma_chan *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
>
>  static const unsigned int jz4780_dma_nb_channels[] = {
>         [ID_JZ4740] = 6,
> +       [ID_JZ4725B] = 6,
>         [ID_JZ4770] = 6,
>         [ID_JZ4780] = 32,
>  };
>
>  static const struct of_device_id jz4780_dma_dt_match[] = {
>         { .compatible = "ingenic,jz4740-dma", .data = (void *)ID_JZ4740 },
> +       { .compatible = "ingenic,jz4725b-dma", .data = (void *)ID_JZ4725B },
>         { .compatible = "ingenic,jz4770-dma", .data = (void *)ID_JZ4770 },
>         { .compatible = "ingenic,jz4780-dma", .data = (void *)ID_JZ4780 },
>         {},
> --
> 2.18.0
>
>

Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [07/14] dmaengine: dma-jz4780: Enable Fast DMA to the AIC
From: PrasannaKumar Muralidharan @ 2018-07-04 17:00 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Vinod Koul, Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton,
	James Hogan, Zubair Lutfullah Kakakhel, Mathieu Malaterre,
	Daniel Silsby, dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Linux-MIPS

On 3 July 2018 at 18:02, Paul Cercueil <paul@crapouillou.net> wrote:
> With the fast DMA bit set, the DMA will transfer twice as much data
> per clock period to the AIC, so there is little point not to set it.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/dma/dma-jz4780.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
> index 922e4031e70e..7ee2c121948f 100644
> --- a/drivers/dma/dma-jz4780.c
> +++ b/drivers/dma/dma-jz4780.c
> @@ -52,6 +52,7 @@
>  #define JZ_DMA_DMAC_DMAE       BIT(0)
>  #define JZ_DMA_DMAC_AR         BIT(2)
>  #define JZ_DMA_DMAC_HLT                BIT(3)
> +#define JZ_DMA_DMAC_FAIC       BIT(27)
>  #define JZ_DMA_DMAC_FMSC       BIT(31)
>
>  #define JZ_DMA_DRT_AUTO                0x8
> @@ -929,8 +930,8 @@ static int jz4780_dma_probe(struct platform_device *pdev)
>          * Also set the FMSC bit - it increases MSC performance, so it makes
>          * little sense not to enable it.
>          */
> -       jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC,
> -                         JZ_DMA_DMAC_DMAE | JZ_DMA_DMAC_FMSC);
> +       jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC, JZ_DMA_DMAC_DMAE |
> +                              JZ_DMA_DMAC_FAIC | JZ_DMA_DMAC_FMSC);
>
>         if (jzdma->version == ID_JZ4780)
>                 jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMACP, 0);
> --
> 2.18.0
>
>

Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>.
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* dmaengine: fsl-edma-common: Add missing header module.h
From: Vinod Koul @ 2018-07-05  5:57 UTC (permalink / raw)
  To: dmaengine; +Cc: angelo, Vinod Koul

We added MODULE_LICENSE but missed adding the header, so add it

drivers/dma/fsl-edma-common.c:672:16: error:
	expected declaration specifiers or '...' before string constant
 MODULE_LICENSE("GPL v2");
                ^~~~~~~~

Fixes: 7565699b2d21 ("dmaengine: fsl-edma-common: Add missing MODULE_LICENSE")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/dma/fsl-edma-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index fd4333995a0e..17e386911825 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -4,6 +4,7 @@
 // Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>
 
 #include <linux/dmapool.h>
+#include <linux/module.h>
 #include <linux/slab.h>
 
 #include "fsl-edma-common.h"

^ permalink raw reply related

* [v2] dmaengine: rcar-dmac: convert to SPDX identifiers
From: Laurent Pinchart @ 2018-07-05  8:30 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Vinod, Geert Uytterhoeven, Dan Williams, Magnus Damm,
	Linux-Renesas, Kieran Bingham, Geert Uytterhoeven, dmaengine,
	Hiroki Negishi

Hi Morimoto-san,

Thank you for the patch.

On Wednesday, 4 July 2018 03:34:10 EEST Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> This patch is using C++ comment style for SPDX line only,
> because driver author want it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> I posted this because I don't know which style was OK.
> Please pick-up best side patch
> 
> v1 -> v2
> 
>  - used // for SPDX line only
> 
>  drivers/dma/sh/rcar-dmac.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 79d14af..ebd160d 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1,13 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   * Renesas R-Car Gen2 DMA Controller Driver
>   *
>   * Copyright (C) 2014 Renesas Electronics Inc.
>   *
>   * Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> - *
> - * This is free software; you can redistribute it and/or modify
> - * it under the terms of version 2 of the GNU General Public License as
> - * published by the Free Software Foundation.
>   */
> 
>  #include <linux/delay.h>

^ permalink raw reply

* [02/14] dmaengine: dma-jz4780: Separate chan/ctrl registers
From: Paul Cercueil @ 2018-07-05 18:23 UTC (permalink / raw)
  To: Paul Burton
  Cc: Vinod Koul, Rob Herring, Mark Rutland, Ralf Baechle, James Hogan,
	Zubair Lutfullah Kakakhel, Mathieu Malaterre, Daniel Silsby,
	dmaengine, devicetree, linux-kernel, linux-mips

Hi Paul,

> Hi Paul,
> 
> On Tue, Jul 03, 2018 at 02:32:02PM +0200, Paul Cercueil wrote:
>>  @@ -804,9 +818,19 @@ static int jz4780_dma_probe(struct 
>> platform_device *pdev)
>>   		return -EINVAL;
>>   	}
>> 
>>  -	jzdma->base = devm_ioremap_resource(dev, res);
>>  -	if (IS_ERR(jzdma->base))
>>  -		return PTR_ERR(jzdma->base);
>>  +	jzdma->chn_base = devm_ioremap_resource(dev, res);
>>  +	if (IS_ERR(jzdma->chn_base))
>>  +		return PTR_ERR(jzdma->chn_base);
>>  +
>>  +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>  +	if (!res) {
>>  +		dev_err(dev, "failed to get I/O memory\n");
>>  +		return -EINVAL;
>>  +	}
>>  +
>>  +	jzdma->ctrl_base = devm_ioremap_resource(dev, res);
>>  +	if (IS_ERR(jzdma->ctrl_base))
>>  +		return PTR_ERR(jzdma->ctrl_base);
> 
> Could we have this failure case fall back to the existing behaviour
> where we only have a single resource covering all the registers? That
> would avoid breaking bisection between this patch & the one that 
> updates
> the JZ4780 DT.
> 
> For example:
> 
> 	#define JZ4780_DMA_CTRL_OFFSET	0x1000
> 
> 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> 	if (res) {
> 		jzdma->ctrl_base = devm_ioremap_resource(dev, res);
> 		if (IS_ERR(jzdma->ctrl_base))
> 			return PTR_ERR(jzdma->ctrl_base);
> 	} else {
> 		jzdma->ctrl_base = jzdma->chn_base + JZ4780_DMA_CTRL_OFFSET;
> 	}
> 
> Then you could remove the fallback after patch 13, to end up with the
> same code you have now but without breaking bisection.
> 
> Most correct might be to move patch 13 to right after this one, so 
> that
> the JZ4780-specific fallback can be removed before adding support for
> any of the other SoCs.

Sure, I can do that for the V2.

Thanks,
-Paul
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [01/14] dmaengine: dma-jz4780: Avoid hardcoding number of channels
From: Paul Cercueil @ 2018-07-05 18:26 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: Vinod Koul, Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton,
	James Hogan, Zubair Lutfullah Kakakhel, Mathieu Malaterre,
	Daniel Silsby, dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Linux-MIPS

Hi PrasannaKumar,

> Hi Paul,
> 
> On 3 July 2018 at 18:02, Paul Cercueil <paul@crapouillou.net> wrote:
>>  As part of the work to support various other Ingenic JZ47xx SoC 
>> versions,
>>  which don't feature the same number of DMA channels per core, we now
>>  deduce the number of DMA channels available from the devicetree
>>  compatible string.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   drivers/dma/dma-jz4780.c | 53 
>> +++++++++++++++++++++++++++++-----------
>>   1 file changed, 39 insertions(+), 14 deletions(-)
>> 
>>  diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
>>  index 85820a2d69d4..b40f491f0367 100644
>>  --- a/drivers/dma/dma-jz4780.c
>>  +++ b/drivers/dma/dma-jz4780.c
>>  @@ -16,6 +16,7 @@
>>   #include <linux/interrupt.h>
>>   #include <linux/module.h>
>>   #include <linux/of.h>
>>  +#include <linux/of_device.h>
>>   #include <linux/of_dma.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/slab.h>
>>  @@ -23,8 +24,6 @@
>>   #include "dmaengine.h"
>>   #include "virt-dma.h"
>> 
>>  -#define JZ_DMA_NR_CHANNELS     32
>>  -
>>   /* Global registers. */
>>   #define JZ_DMA_REG_DMAC                0x1000
>>   #define JZ_DMA_REG_DIRQP       0x1004
>>  @@ -135,14 +134,20 @@ struct jz4780_dma_chan {
>>          unsigned int curr_hwdesc;
>>   };
>> 
>>  +enum jz_version {
>>  +       ID_JZ4780,
>>  +};
>>  +
>>   struct jz4780_dma_dev {
>>          struct dma_device dma_device;
>>          void __iomem *base;
>>          struct clk *clk;
>>          unsigned int irq;
>>  +       unsigned int nb_channels;
>>  +       enum jz_version version;
>> 
>>          uint32_t chan_reserved;
>>  -       struct jz4780_dma_chan chan[JZ_DMA_NR_CHANNELS];
>>  +       struct jz4780_dma_chan chan[];
> 
> Looks like a variable length array in struct. I think there is some
> effort to remove the usage of VLA. Can you revisit this? I may be
> wrong, please feel free to correct.

Are you sure? It's the first time I hear about it.
Could anybody confirm?

>>   };
>> 
>>   struct jz4780_dma_filter_data {
>>  @@ -648,7 +653,7 @@ static irqreturn_t jz4780_dma_irq_handler(int 
>> irq, void *data)
>> 
>>          pending = jz4780_dma_readl(jzdma, JZ_DMA_REG_DIRQP);
>> 
>>  -       for (i = 0; i < JZ_DMA_NR_CHANNELS; i++) {
>>  +       for (i = 0; i < jzdma->nb_channels; i++) {
>>                  if (!(pending & (1<<i)))
>>                          continue;
>> 
>>  @@ -728,7 +733,7 @@ static struct dma_chan 
>> *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
>>          data.channel = dma_spec->args[1];
>> 
>>          if (data.channel > -1) {
>>  -               if (data.channel >= JZ_DMA_NR_CHANNELS) {
>>  +               if (data.channel >= jzdma->nb_channels) {
>>                          dev_err(jzdma->dma_device.dev,
>>                                  "device requested non-existent 
>> channel %u\n",
>>                                  data.channel);
>>  @@ -752,19 +757,45 @@ static struct dma_chan 
>> *jz4780_of_dma_xlate(struct of_phandle_args *dma_spec,
>>          }
>>   }
>> 
>>  +static const unsigned int jz4780_dma_nb_channels[] = {
>>  +       [ID_JZ4780] = 32,
>>  +};
>>  +
>>  +static const struct of_device_id jz4780_dma_dt_match[] = {
>>  +       { .compatible = "ingenic,jz4780-dma", .data = (void 
>> *)ID_JZ4780 },
>>  +       {},
>>  +};
>>  +MODULE_DEVICE_TABLE(of, jz4780_dma_dt_match);
>>  +
>>   static int jz4780_dma_probe(struct platform_device *pdev)
>>   {
>>          struct device *dev = &pdev->dev;
>>  +       const struct of_device_id *of_id = of_match_device(
>>  +                       jz4780_dma_dt_match, dev);
>>          struct jz4780_dma_dev *jzdma;
>>          struct jz4780_dma_chan *jzchan;
>>          struct dma_device *dd;
>>          struct resource *res;
>>  +       enum jz_version version;
>>  +       unsigned int nb_channels;
>>          int i, ret;
>> 
>>  -       jzdma = devm_kzalloc(dev, sizeof(*jzdma), GFP_KERNEL);
>>  +       if (of_id)
>>  +               version = (enum jz_version)of_id->data;
>>  +       else
>>  +               version = ID_JZ4780; /* Default when not probed 
>> from DT */
>>  +
>>  +       nb_channels = jz4780_dma_nb_channels[version];
>>  +
>>  +       jzdma = devm_kzalloc(dev, sizeof(*jzdma)
>>  +                               + sizeof(*jzdma->chan) * 
>> nb_channels,
>>  +                               GFP_KERNEL);
>>          if (!jzdma)
>>                  return -ENOMEM;
>> 
>>  +       jzdma->nb_channels = nb_channels;
>>  +       jzdma->version = version;
>>  +
>>          platform_set_drvdata(pdev, jzdma);
>> 
>>          res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  @@ -839,7 +870,7 @@ static int jz4780_dma_probe(struct 
>> platform_device *pdev)
>> 
>>          INIT_LIST_HEAD(&dd->channels);
>> 
>>  -       for (i = 0; i < JZ_DMA_NR_CHANNELS; i++) {
>>  +       for (i = 0; i < jzdma->nb_channels; i++) {
>>                  jzchan = &jzdma->chan[i];
>>                  jzchan->id = i;
>> 
>>  @@ -884,19 +915,13 @@ static int jz4780_dma_remove(struct 
>> platform_device *pdev)
>> 
>>          free_irq(jzdma->irq, jzdma);
>> 
>>  -       for (i = 0; i < JZ_DMA_NR_CHANNELS; i++)
>>  +       for (i = 0; i < jzdma->nb_channels; i++)
>>                  tasklet_kill(&jzdma->chan[i].vchan.task);
>> 
>>          dma_async_device_unregister(&jzdma->dma_device);
>>          return 0;
>>   }
>> 
>>  -static const struct of_device_id jz4780_dma_dt_match[] = {
>>  -       { .compatible = "ingenic,jz4780-dma", .data = NULL },
>>  -       {},
>>  -};
>>  -MODULE_DEVICE_TABLE(of, jz4780_dma_dt_match);
>>  -
>>   static struct platform_driver jz4780_dma_driver = {
>>          .probe          = jz4780_dma_probe,
>>          .remove         = jz4780_dma_remove,
>>  --
>>  2.18.0
>> 
>>
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [02/14] dmaengine: dma-jz4780: Separate chan/ctrl registers
From: Paul Cercueil @ 2018-07-05 21:45 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: Vinod Koul, Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton,
	James Hogan, Zubair Lutfullah Kakakhel, Mathieu Malaterre,
	Daniel Silsby, dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Linux-MIPS

> Paul,
> 
> On 3 July 2018 at 18:02, Paul Cercueil <paul@crapouillou.net> wrote:
>>  The register area of the JZ4780 DMA core can be split into different
>>  sections for different purposes:
>> 
>>  * one set of registers is used to perform actions at the DMA core 
>> level,
>>  that will generally affect all channels;
>> 
>>  * one set of registers per DMA channel, to perform actions at the 
>> DMA
>>  channel level, that will only affect the channel in question.
>> 
>>  The problem rises when trying to support new versions of the JZ47xx
>>  Ingenic SoC. For instance, the JZ4770 has two DMA cores, each one
>>  with six DMA channels, and the register sets are interleaved:
>>  <DMA0 chan regs> <DMA1 chan regs> <DMA0 ctrl regs> <DMA1 ctrl regs>
>> 
>>  By using one memory resource for the channel-specific registers and
>>  one memory resource for the core-specific registers, we can support
>>  the JZ4770, by initializing the driver once per DMA core with 
>> different
>>  addresses.
> 
> As per my understanding device tree should be modified only when
> hardware changes. This looks the other way around. It must be possible
> to achieve what you are trying to do in this patch without changing
> the device tree.

I would agree that devicetree has an ABI that we shouldn't break if
possible.

However DTS support for all the Ingenic SoCs/boards is far from being
complete, and more importantly, all Ingenic-based boards compile the DTS
file within the kernel; so breaking the ABI is not (yet) a problem, and
we should push the big changes right now while it's still possible.

>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   .../devicetree/bindings/dma/jz4780-dma.txt    |   6 +-
>>   drivers/dma/dma-jz4780.c                      | 106 
>> +++++++++++-------
>>   2 files changed, 69 insertions(+), 43 deletions(-)
>> 
>>  diff --git a/Documentation/devicetree/bindings/dma/jz4780-dma.txt 
>> b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
>>  index f25feee62b15..f9b1864f5b77 100644
>>  --- a/Documentation/devicetree/bindings/dma/jz4780-dma.txt
>>  +++ b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
>>  @@ -3,7 +3,8 @@
>>   Required properties:
>> 
>>   - compatible: Should be "ingenic,jz4780-dma"
>>  -- reg: Should contain the DMA controller registers location and 
>> length.
>>  +- reg: Should contain the DMA channel registers location and 
>> length, followed
>>  +  by the DMA controller registers location and length.
>>   - interrupts: Should contain the interrupt specifier of the DMA 
>> controller.
>>   - interrupt-parent: Should be the phandle of the interrupt 
>> controller that
>>   - clocks: Should contain a clock specifier for the JZ4780 PDMA 
>> clock.
>>  @@ -22,7 +23,8 @@ Example:
>> 
>>   dma: dma@13420000 {
>>          compatible = "ingenic,jz4780-dma";
>>  -       reg = <0x13420000 0x10000>;
>>  +       reg = <0x13420000 0x400
>>  +              0x13421000 0x40>;
>> 
>>          interrupt-parent = <&intc>;
>>          interrupts = <10>;
>>  diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
>>  index b40f491f0367..4d234caf5d62 100644
>>  --- a/drivers/dma/dma-jz4780.c
>>  +++ b/drivers/dma/dma-jz4780.c
>>  @@ -25,26 +25,26 @@
>>   #include "virt-dma.h"
>> 
>>   /* Global registers. */
>>  -#define JZ_DMA_REG_DMAC                0x1000
>>  -#define JZ_DMA_REG_DIRQP       0x1004
>>  -#define JZ_DMA_REG_DDR         0x1008
>>  -#define JZ_DMA_REG_DDRS                0x100c
>>  -#define JZ_DMA_REG_DMACP       0x101c
>>  -#define JZ_DMA_REG_DSIRQP      0x1020
>>  -#define JZ_DMA_REG_DSIRQM      0x1024
>>  -#define JZ_DMA_REG_DCIRQP      0x1028
>>  -#define JZ_DMA_REG_DCIRQM      0x102c
>>  +#define JZ_DMA_REG_DMAC                0x00
>>  +#define JZ_DMA_REG_DIRQP       0x04
>>  +#define JZ_DMA_REG_DDR         0x08
>>  +#define JZ_DMA_REG_DDRS                0x0c
>>  +#define JZ_DMA_REG_DMACP       0x1c
>>  +#define JZ_DMA_REG_DSIRQP      0x20
>>  +#define JZ_DMA_REG_DSIRQM      0x24
>>  +#define JZ_DMA_REG_DCIRQP      0x28
>>  +#define JZ_DMA_REG_DCIRQM      0x2c
>> 
>>   /* Per-channel registers. */
>>   #define JZ_DMA_REG_CHAN(n)     (n * 0x20)
>>  -#define JZ_DMA_REG_DSA(n)      (0x00 + JZ_DMA_REG_CHAN(n))
>>  -#define JZ_DMA_REG_DTA(n)      (0x04 + JZ_DMA_REG_CHAN(n))
>>  -#define JZ_DMA_REG_DTC(n)      (0x08 + JZ_DMA_REG_CHAN(n))
>>  -#define JZ_DMA_REG_DRT(n)      (0x0c + JZ_DMA_REG_CHAN(n))
>>  -#define JZ_DMA_REG_DCS(n)      (0x10 + JZ_DMA_REG_CHAN(n))
>>  -#define JZ_DMA_REG_DCM(n)      (0x14 + JZ_DMA_REG_CHAN(n))
>>  -#define JZ_DMA_REG_DDA(n)      (0x18 + JZ_DMA_REG_CHAN(n))
>>  -#define JZ_DMA_REG_DSD(n)      (0x1c + JZ_DMA_REG_CHAN(n))
>>  +#define JZ_DMA_REG_DSA         0x00
>>  +#define JZ_DMA_REG_DTA         0x04
>>  +#define JZ_DMA_REG_DTC         0x08
>>  +#define JZ_DMA_REG_DRT         0x0c
>>  +#define JZ_DMA_REG_DCS         0x10
>>  +#define JZ_DMA_REG_DCM         0x14
>>  +#define JZ_DMA_REG_DDA         0x18
>>  +#define JZ_DMA_REG_DSD         0x1c
>> 
>>   #define JZ_DMA_DMAC_DMAE       BIT(0)
>>   #define JZ_DMA_DMAC_AR         BIT(2)
>>  @@ -140,7 +140,8 @@ enum jz_version {
>> 
>>   struct jz4780_dma_dev {
>>          struct dma_device dma_device;
>>  -       void __iomem *base;
>>  +       void __iomem *chn_base;
>>  +       void __iomem *ctrl_base;
>>          struct clk *clk;
>>          unsigned int irq;
>>          unsigned int nb_channels;
>>  @@ -174,16 +175,28 @@ static inline struct jz4780_dma_dev 
>> *jz4780_dma_chan_parent(
>>                              dma_device);
>>   }
>> 
>>  -static inline uint32_t jz4780_dma_readl(struct jz4780_dma_dev 
>> *jzdma,
>>  +static inline uint32_t jz4780_dma_chn_readl(struct jz4780_dma_dev 
>> *jzdma,
>>  +       unsigned int chn, unsigned int reg)
>>  +{
>>  +       return readl(jzdma->chn_base + reg + JZ_DMA_REG_CHAN(chn));
>>  +}
>>  +
>>  +static inline void jz4780_dma_chn_writel(struct jz4780_dma_dev 
>> *jzdma,
>>  +       unsigned int chn, unsigned int reg, uint32_t val)
>>  +{
>>  +       writel(val, jzdma->chn_base + reg + JZ_DMA_REG_CHAN(chn));
>>  +}
>>  +
>>  +static inline uint32_t jz4780_dma_ctrl_readl(struct jz4780_dma_dev 
>> *jzdma,
>>          unsigned int reg)
>>   {
>>  -       return readl(jzdma->base + reg);
>>  +       return readl(jzdma->ctrl_base + reg);
>>   }
>> 
>>  -static inline void jz4780_dma_writel(struct jz4780_dma_dev *jzdma,
>>  +static inline void jz4780_dma_ctrl_writel(struct jz4780_dma_dev 
>> *jzdma,
>>          unsigned int reg, uint32_t val)
>>   {
>>  -       writel(val, jzdma->base + reg);
>>  +       writel(val, jzdma->ctrl_base + reg);
>>   }
>> 
>>   static struct jz4780_dma_desc *jz4780_dma_desc_alloc(
>>  @@ -478,17 +491,18 @@ static void jz4780_dma_begin(struct 
>> jz4780_dma_chan *jzchan)
>>          }
>> 
>>          /* Use 8-word descriptors. */
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DCS(jzchan->id), 
>> JZ_DMA_DCS_DES8);
>>  +       jz4780_dma_chn_writel(jzdma, jzchan->id,
>>  +                             JZ_DMA_REG_DCS, JZ_DMA_DCS_DES8);
>> 
>>          /* Write descriptor address and initiate descriptor fetch. 
>> */
>>          desc_phys = jzchan->desc->desc_phys +
>>                      (jzchan->curr_hwdesc * 
>> sizeof(*jzchan->desc->desc));
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DDA(jzchan->id), 
>> desc_phys);
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DDRS, BIT(jzchan->id));
>>  +       jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DDA, 
>> desc_phys);
>>  +       jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DDRS, 
>> BIT(jzchan->id));
>> 
>>          /* Enable the channel. */
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DCS(jzchan->id),
>>  -                         JZ_DMA_DCS_DES8 | JZ_DMA_DCS_CTE);
>>  +       jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DCS,
>>  +                             JZ_DMA_DCS_DES8 | JZ_DMA_DCS_CTE);
>>   }
>> 
>>   static void jz4780_dma_issue_pending(struct dma_chan *chan)
>>  @@ -514,7 +528,7 @@ static int jz4780_dma_terminate_all(struct 
>> dma_chan *chan)
>>          spin_lock_irqsave(&jzchan->vchan.lock, flags);
>> 
>>          /* Clear the DMA status and stop the transfer. */
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DCS(jzchan->id), 0);
>>  +       jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DCS, 0);
>>          if (jzchan->desc) {
>>                  vchan_terminate_vdesc(&jzchan->desc->vdesc);
>>                  jzchan->desc = NULL;
>>  @@ -563,8 +577,8 @@ static size_t jz4780_dma_desc_residue(struct 
>> jz4780_dma_chan *jzchan,
>>                  residue += desc->desc[i].dtc << 
>> jzchan->transfer_shift;
>> 
>>          if (next_sg != 0) {
>>  -               count = jz4780_dma_readl(jzdma,
>>  -                                        
>> JZ_DMA_REG_DTC(jzchan->id));
>>  +               count = jz4780_dma_chn_readl(jzdma, jzchan->id,
>>  +                                        JZ_DMA_REG_DTC);
>>                  residue += count << jzchan->transfer_shift;
>>          }
>> 
>>  @@ -611,8 +625,8 @@ static void jz4780_dma_chan_irq(struct 
>> jz4780_dma_dev *jzdma,
>> 
>>          spin_lock(&jzchan->vchan.lock);
>> 
>>  -       dcs = jz4780_dma_readl(jzdma, JZ_DMA_REG_DCS(jzchan->id));
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DCS(jzchan->id), 0);
>>  +       dcs = jz4780_dma_chn_readl(jzdma, jzchan->id, 
>> JZ_DMA_REG_DCS);
>>  +       jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DCS, 0);
>> 
>>          if (dcs & JZ_DMA_DCS_AR) {
>>                  dev_warn(&jzchan->vchan.chan.dev->device,
>>  @@ -651,7 +665,7 @@ static irqreturn_t jz4780_dma_irq_handler(int 
>> irq, void *data)
>>          uint32_t pending, dmac;
>>          int i;
>> 
>>  -       pending = jz4780_dma_readl(jzdma, JZ_DMA_REG_DIRQP);
>>  +       pending = jz4780_dma_ctrl_readl(jzdma, JZ_DMA_REG_DIRQP);
>> 
>>          for (i = 0; i < jzdma->nb_channels; i++) {
>>                  if (!(pending & (1<<i)))
>>  @@ -661,12 +675,12 @@ static irqreturn_t jz4780_dma_irq_handler(int 
>> irq, void *data)
>>          }
>> 
>>          /* Clear halt and address error status of all channels. */
>>  -       dmac = jz4780_dma_readl(jzdma, JZ_DMA_REG_DMAC);
>>  +       dmac = jz4780_dma_ctrl_readl(jzdma, JZ_DMA_REG_DMAC);
>>          dmac &= ~(JZ_DMA_DMAC_HLT | JZ_DMA_DMAC_AR);
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DMAC, dmac);
>>  +       jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC, dmac);
>> 
>>          /* Clear interrupt pending status. */
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DIRQP, 0);
>>  +       jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DIRQP, 0);
>> 
>>          return IRQ_HANDLED;
>>   }
>>  @@ -804,9 +818,19 @@ static int jz4780_dma_probe(struct 
>> platform_device *pdev)
>>                  return -EINVAL;
>>          }
>> 
>>  -       jzdma->base = devm_ioremap_resource(dev, res);
>>  -       if (IS_ERR(jzdma->base))
>>  -               return PTR_ERR(jzdma->base);
>>  +       jzdma->chn_base = devm_ioremap_resource(dev, res);
>>  +       if (IS_ERR(jzdma->chn_base))
>>  +               return PTR_ERR(jzdma->chn_base);
>>  +
>>  +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>  +       if (!res) {
>>  +               dev_err(dev, "failed to get I/O memory\n");
>>  +               return -EINVAL;
>>  +       }
>>  +
>>  +       jzdma->ctrl_base = devm_ioremap_resource(dev, res);
>>  +       if (IS_ERR(jzdma->ctrl_base))
>>  +               return PTR_ERR(jzdma->ctrl_base);
>> 
>>          ret = platform_get_irq(pdev, 0);
>>          if (ret < 0) {
>>  @@ -864,9 +888,9 @@ static int jz4780_dma_probe(struct 
>> platform_device *pdev)
>>           * Also set the FMSC bit - it increases MSC performance, so 
>> it makes
>>           * little sense not to enable it.
>>           */
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DMAC,
>>  +       jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC,
>>                            JZ_DMA_DMAC_DMAE | JZ_DMA_DMAC_FMSC);
>>  -       jz4780_dma_writel(jzdma, JZ_DMA_REG_DMACP, 0);
>>  +       jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMACP, 0);
>> 
>>          INIT_LIST_HEAD(&dd->channels);
>> 
>>  --
>>  2.18.0
>> 
>> 
> 
> Regards,
> PrasannaKumar
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [2/3] k3dma: add support to reserved minimum channels
From: Guodong Xu @ 2018-07-06  3:05 UTC (permalink / raw)
  To: vkoul
  Cc: Rob Herring, Mark Rutland, dan.j.williams, liyu65, Suzhuangluan,
	xuhongtao (A), zhongkaihua, Xuezhiliang, xupeng (Q), sunliang10,
	Fengbaopeng (kevin, Kirin Solution Dept), dmaengine, devicetree,
	linux-kernel

On Thu, Jun 28, 2018 at 2:02 PM Vinod <vkoul@kernel.org> wrote:
>
> On 22-06-18, 11:24, Guodong Xu wrote:
> > From: Li Yu <liyu65@hisilicon.com>
> >
> > On k3 series of SoC, DMA controller reserves some channels for
> > other on-chip coprocessors. By adding support to dma_min_chan, kernel
> > will not be able to use these reserved channels.
> >
> > One example is on Hi3660 platform, channel 0 is reserved to lpm3.
> >
> > Please also refer to Documentation/devicetree/bindings/dma/k3dma.txt
>
> and if some other platform has channel X marked for co-processor, maybe
> a last channel or something in middle, how will this work then?
>
Hi, Vinod

Sorry for delayed response. We checked with Kirin hardware design
team, so far their design strategy is all Kirin SoC series reserve
only from minimum side, saying channel 0, then 1, then 2. That impacts
the current SoC in upstreaming, Kirin960 (Hi3660), and next versions
in Kirin SoC, Kirin970 and 980, which may hit upstream later.

> I am thinking this should be a mask, rather than min.
>

So, since this driver k3dma.c is only used by Kirin SoC DMA
controllers, I would prefer to keep the current design dma_min_chan
unchanged.

What do you think?

-Guodong


> >
> > Signed-off-by: Li Yu <liyu65@hisilicon.com>
> > Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
> > ---
> >  drivers/dma/k3dma.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
> > index fa31cccbe04f..13cec12742e3 100644
> > --- a/drivers/dma/k3dma.c
> > +++ b/drivers/dma/k3dma.c
> > @@ -113,6 +113,7 @@ struct k3_dma_dev {
> >       struct dma_pool         *pool;
> >       u32                     dma_channels;
> >       u32                     dma_requests;
> > +     u32                     dma_min_chan;
> >       unsigned int            irq;
> >  };
> >
> > @@ -309,7 +310,7 @@ static void k3_dma_tasklet(unsigned long arg)
> >
> >       /* check new channel request in d->chan_pending */
> >       spin_lock_irq(&d->lock);
> > -     for (pch = 0; pch < d->dma_channels; pch++) {
> > +     for (pch = d->dma_min_chan; pch < d->dma_channels; pch++) {
> >               p = &d->phy[pch];
> >
> >               if (p->vchan == NULL && !list_empty(&d->chan_pending)) {
> > @@ -326,7 +327,7 @@ static void k3_dma_tasklet(unsigned long arg)
> >       }
> >       spin_unlock_irq(&d->lock);
> >
> > -     for (pch = 0; pch < d->dma_channels; pch++) {
> > +     for (pch = d->dma_min_chan; pch < d->dma_channels; pch++) {
> >               if (pch_alloc & (1 << pch)) {
> >                       p = &d->phy[pch];
> >                       c = p->vchan;
> > @@ -825,6 +826,8 @@ static int k3_dma_probe(struct platform_device *op)
> >                               "dma-channels", &d->dma_channels);
> >               of_property_read_u32((&op->dev)->of_node,
> >                               "dma-requests", &d->dma_requests);
> > +             of_property_read_u32((&op->dev)->of_node,
> > +                             "dma-min-chan", &d->dma_min_chan);
> >       }
> >
> >       d->clk = devm_clk_get(&op->dev, NULL);
> > @@ -848,12 +851,12 @@ static int k3_dma_probe(struct platform_device *op)
> >               return -ENOMEM;
> >
> >       /* init phy channel */
> > -     d->phy = devm_kcalloc(&op->dev,
> > -             d->dma_channels, sizeof(struct k3_dma_phy), GFP_KERNEL);
> > +     d->phy = devm_kcalloc(&op->dev, (d->dma_channels - d->dma_min_chan),
> > +                     sizeof(struct k3_dma_phy), GFP_KERNEL);
> >       if (d->phy == NULL)
> >               return -ENOMEM;
> >
> > -     for (i = 0; i < d->dma_channels; i++) {
> > +     for (i = d->dma_min_chan; i < d->dma_channels; i++) {
> >               struct k3_dma_phy *p = &d->phy[i];
> >
> >               p->idx = i;
> > --
> > 2.17.1
>
> --
> ~Vinod
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [1/3] dt-bindings: k3dma: add optional property dma_min_chan
From: Guodong Xu @ 2018-07-06  3:17 UTC (permalink / raw)
  To: Rob Herring
  Cc: vkoul, Mark Rutland, dan.j.williams, liyu65, Suzhuangluan,
	xuhongtao (A), zhongkaihua, Xuezhiliang, xupeng (Q), sunliang10,
	Fengbaopeng (kevin, Kirin Solution Dept), dmaengine, devicetree,
	linux-kernel

On Wed, Jul 4, 2018 at 9:14 AM Guodong Xu <guodong.xu@linaro.org> wrote:
>
> On Wed, Jul 4, 2018 at 2:54 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Jun 22, 2018 at 11:24:14AM +0800, Guodong Xu wrote:
> > > From: Li Yu <liyu65@hisilicon.com>
> > >
> > > Add optional property dma_min_chan for k3dma.
> > >
> > > Signed-off-by: Li Yu <liyu65@hisilicon.com>
> > > ---
> > >  Documentation/devicetree/bindings/dma/k3dma.txt | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/t b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > index 4945aeac4dc4..2fa1370c3173 100644
> > > --- a/Documentation/devicetree/bindings/dma/k3dma.txt
> > > +++ b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > @@ -12,6 +12,11 @@ Required properties:
> > >               have specific request line
> > >  - clocks: clock required
> > >
> > > +Optional properties:
> > > +- dma_min_chan: the minimum number of DMA channel which begin to use
> > > +             the default value is 0, but in some platform is
> > > +             configured 1, like hi3660 platform
> >
> > Can't this be implied by the compatible?
> >
>
> No. "hisilicon,k3-dma-1.0" can work with series of hisilicon kirin
> SoC. And each has different reservation of channels for on-chip
> coprocessors.
>
> > If not, needs vendor prefix and don't use '_' in property names.
> >
>
> Sure, thanks. Will change that when design new property. As Vinod
> suggested, it makes sense to change this to a mask.
>

After checking with Kirin SoC design team, I prefer to stay with
minimum channel number instead of mask. So, I will change this
property to:

hisilicon,dma-min-chan

-Guodong



>
> -Guodong
>
> > Rob
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [v2,1/4] dt-bindings: k3dma: add optional property hisilicon,dma-min-chan
From: Guodong Xu @ 2018-07-06  3:55 UTC (permalink / raw)
  To: vkoul, robh+dt, mark.rutland, dan.j.williams, xuwei5,
	catalin.marinas, will.deacon
  Cc: liyu65, suzhuangluan, xuhongtao8, zhongkaihua, xuezhiliang,
	xupeng7, sunliang10, fengbaopeng, dmaengine, devicetree,
	linux-kernel, linux-arm-kernel, Guodong Xu

From: Li Yu <liyu65@hisilicon.com>

Add optional property hisilicon,dma-min-chan for k3dma.

Signed-off-by: Li Yu <liyu65@hisilicon.com>
Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
---
 Documentation/devicetree/bindings/dma/k3dma.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
index 4945aeac4dc4..f34202a80f3c 100644
--- a/Documentation/devicetree/bindings/dma/k3dma.txt
+++ b/Documentation/devicetree/bindings/dma/k3dma.txt
@@ -12,6 +12,11 @@ Required properties:
 		have specific request line
 - clocks: clock required
 
+Optional properties:
+- hisilicon,dma-min-chan: the minimum DMA channel number which is usable
+		Default value is 0, but in some platform it is
+		configured 1, like in hi3660 platform
+
 Example:
 
 Controller:
@@ -21,6 +26,7 @@ Controller:
 			#dma-cells = <1>;
 			dma-channels = <16>;
 			dma-requests = <27>;
+			hisilicon,dma-min-chan = <1>;
 			interrupts = <0 12 4>;
 			clocks = <&pclk>;
 		};

^ permalink raw reply related

* [v2,2/4] k3dma: add support to reserved minimum channels
From: Guodong Xu @ 2018-07-06  3:55 UTC (permalink / raw)
  To: vkoul, robh+dt, mark.rutland, dan.j.williams, xuwei5,
	catalin.marinas, will.deacon
  Cc: liyu65, suzhuangluan, xuhongtao8, zhongkaihua, xuezhiliang,
	xupeng7, sunliang10, fengbaopeng, dmaengine, devicetree,
	linux-kernel, linux-arm-kernel, Guodong Xu

From: Li Yu <liyu65@hisilicon.com>

On k3 series of SoC, DMA controller reserves some channels for
other on-chip coprocessors. By reading property "hisilicon,dma-min-chan"
from dts node, kernel will not use these reserved channels.

As an example, on Hi3660, channel 0 is reserved for lpm3.

Refer to Documentation/devicetree/bindings/dma/k3dma.txt for more
information.

Signed-off-by: Li Yu <liyu65@hisilicon.com>
Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
---
 drivers/dma/k3dma.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index fa31cccbe04f..33efb541acb2 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -113,6 +113,7 @@ struct k3_dma_dev {
 	struct dma_pool		*pool;
 	u32			dma_channels;
 	u32			dma_requests;
+	u32			dma_min_chan;
 	unsigned int		irq;
 };
 
@@ -309,7 +310,7 @@ static void k3_dma_tasklet(unsigned long arg)
 
 	/* check new channel request in d->chan_pending */
 	spin_lock_irq(&d->lock);
-	for (pch = 0; pch < d->dma_channels; pch++) {
+	for (pch = d->dma_min_chan; pch < d->dma_channels; pch++) {
 		p = &d->phy[pch];
 
 		if (p->vchan == NULL && !list_empty(&d->chan_pending)) {
@@ -326,7 +327,7 @@ static void k3_dma_tasklet(unsigned long arg)
 	}
 	spin_unlock_irq(&d->lock);
 
-	for (pch = 0; pch < d->dma_channels; pch++) {
+	for (pch = d->dma_min_chan; pch < d->dma_channels; pch++) {
 		if (pch_alloc & (1 << pch)) {
 			p = &d->phy[pch];
 			c = p->vchan;
@@ -825,6 +826,8 @@ static int k3_dma_probe(struct platform_device *op)
 				"dma-channels", &d->dma_channels);
 		of_property_read_u32((&op->dev)->of_node,
 				"dma-requests", &d->dma_requests);
+		of_property_read_u32((&op->dev)->of_node,
+				"hisilicon,dma-min-chan", &d->dma_min_chan);
 	}
 
 	d->clk = devm_clk_get(&op->dev, NULL);
@@ -848,12 +851,12 @@ static int k3_dma_probe(struct platform_device *op)
 		return -ENOMEM;
 
 	/* init phy channel */
-	d->phy = devm_kcalloc(&op->dev,
-		d->dma_channels, sizeof(struct k3_dma_phy), GFP_KERNEL);
+	d->phy = devm_kcalloc(&op->dev, (d->dma_channels - d->dma_min_chan),
+			sizeof(struct k3_dma_phy), GFP_KERNEL);
 	if (d->phy == NULL)
 		return -ENOMEM;
 
-	for (i = 0; i < d->dma_channels; i++) {
+	for (i = d->dma_min_chan; i < d->dma_channels; i++) {
 		struct k3_dma_phy *p = &d->phy[i];
 
 		p->idx = i;

^ permalink raw reply related

* [v2,3/4] arm64: dts: hi3660: update property name hisilicon,dma-min-chan
From: Guodong Xu @ 2018-07-06  3:55 UTC (permalink / raw)
  To: vkoul, robh+dt, mark.rutland, dan.j.williams, xuwei5,
	catalin.marinas, will.deacon
  Cc: liyu65, suzhuangluan, xuhongtao8, zhongkaihua, xuezhiliang,
	xupeng7, sunliang10, fengbaopeng, dmaengine, devicetree,
	linux-kernel, linux-arm-kernel, Guodong Xu

Update property name dma-min-chan to "hisilicon,dma-min-chan"

Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index 8d477dcbfa58..0cec26976eb6 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -537,7 +537,7 @@
 			#dma-cells = <1>;
 			dma-channels = <16>;
 			dma-requests = <32>;
-			dma-min-chan = <1>;
+			hisilicon,dma-min-chan = <1>;
 			interrupts = <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&crg_ctrl HI3660_CLK_GATE_DMAC>;
 			dma-no-cci;

^ permalink raw reply related

* [v2,4/4] k3dma: delete axi_config
From: Guodong Xu @ 2018-07-06  3:55 UTC (permalink / raw)
  To: vkoul, robh+dt, mark.rutland, dan.j.williams, xuwei5,
	catalin.marinas, will.deacon
  Cc: liyu65, suzhuangluan, xuhongtao8, zhongkaihua, xuezhiliang,
	xupeng7, sunliang10, fengbaopeng, dmaengine, devicetree,
	linux-kernel, linux-arm-kernel, Guodong Xu

From: Li Yu <liyu65@hisilicon.com>

Axi_config controls whether DMA resources can be accessed in non-secure
mode, such as linux kernel. The setting is actually done in
bootloader stage.

This patch removes axi_config from k3dma driver.

Signed-off-by: Li Yu <liyu65@hisilicon.com>
Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
---
 drivers/dma/k3dma.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index 33efb541acb2..4542e703ec85 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -52,8 +52,6 @@
 #define CX_SRC			0x814
 #define CX_DST			0x818
 #define CX_CFG			0x81c
-#define AXI_CFG			0x820
-#define AXI_CFG_DEFAULT		0x201201
 
 #define CX_LLI_CHAIN_EN		0x2
 #define CX_CFG_EN		0x1
@@ -158,7 +156,6 @@ static void k3_dma_set_desc(struct k3_dma_phy *phy, struct k3_desc_hw *hw)
 	writel_relaxed(hw->count, phy->base + CX_CNT0);
 	writel_relaxed(hw->saddr, phy->base + CX_SRC);
 	writel_relaxed(hw->daddr, phy->base + CX_DST);
-	writel_relaxed(AXI_CFG_DEFAULT, phy->base + AXI_CFG);
 	writel_relaxed(hw->config, phy->base + CX_CFG);
 }
 

^ permalink raw reply related

* dmaengine: nbpfaxi: Mark expected switch fall-through
From: Vinod Koul @ 2018-07-06  5:56 UTC (permalink / raw)
  To: Gustavo A. R. Silva; +Cc: Dan Williams, dmaengine, linux-kernel

On 02-07-18, 13:06, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/dma/nbpfaxi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma/nbpfaxi.c b/drivers/dma/nbpfaxi.c
> index 2f9974d..8c7b2e8 100644
> --- a/drivers/dma/nbpfaxi.c
> +++ b/drivers/dma/nbpfaxi.c
> @@ -479,6 +479,7 @@ static size_t nbpf_xfer_size(struct nbpf_device *nbpf,
>  
>  	default:
>  		pr_warn("%s(): invalid bus width %u\n", __func__, width);
> +		/* fall through */

Hmm this looks okay but am not able to trigger this warning..(used W=1) Did you
see this warning on your build, if so what options?

^ permalink raw reply

* [v2] dmaengine: imx-sdma: add missing structure description
From: Vinod Koul @ 2018-07-06  5:58 UTC (permalink / raw)
  To: Robin Gong; +Cc: dan.j.williams, dmaengine, linux-kernel, linux-imx

On 04-07-18, 18:06, Robin Gong wrote:
> Some member description or colons missing cause build warning with
> 'W=1' as below:
> 
> drivers/dma/imx-sdma.c:326: warning: Function parameter or member 'vd' not described in 'sdma_desc'
> drivers/dma/imx-sdma.c:326: warning: Function parameter or member 'num_bd' not described in 'sdma_desc'
> drivers/dma/imx-sdma.c:326: warning: Function parameter or member 'bd_phys' not described in 'sdma_desc'
> drivers/dma/imx-sdma.c:326: warning: Function parameter or member 'buf_tail' not described in 'sdma_desc'
> drivers/dma/imx-sdma.c:326: warning: Function parameter or member 'buf_ptail' not described in 'sdma_desc'
> drivers/dma/imx-sdma.c:326: warning: Function parameter or member 'period_len' not described in 'sdma_desc'
> drivers/dma/imx-sdma.c:326: warning: Function parameter or member 'chn_real_count' not described in 'sdma_desc'
> drivers/dma/imx-sdma.c:326: warning: Function parameter or member 'chn_count' not described in 'sdma_desc

Applied, thanks

^ permalink raw reply

* dma: add support for reporting pause and resume separately
From: Vinod Koul @ 2018-07-06  6:00 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Marek Szyprowski, Mark Brown, Lars-Peter Clausen, dmaengine,
	ALSA Development Mailing List, Linux Kernel Mailing List,
	Dan Williams, Bartlomiej Zolnierkiewicz

On 04-07-18, 13:38, Andy Shevchenko wrote:
> On Wed, Jul 4, 2018 at 10:00 AM, Vinod <vkoul@kernel.org> wrote:
> > Hi Marek,
> >
> > On 02-07-18, 15:08, Marek Szyprowski wrote:
> >> 'cmd_pause' DMA channel capability means that respective DMA engine
> >> supports both pausing and resuming given DMA channel. However, in some
> >> cases it is important to know if DMA channel can be paused without the
> >> need to resume it. This is a typical requirement for proper residue
> >> reading on transfer timeout in UART drivers. There are also some DMA
> >> engines with limited hardware, which doesn't really support resuming.
> >
> > Am curious given that your hardware does not support resume, what was motivation
> > for adding pause?
> >
> >> Reporting pause and resume capabilities separately allows UART drivers to
> >> properly check for the really required capabilities and operate in DMA
> >> mode also in systems with limited DMA hardware. On the other hand drivers,
> >> which rely on full channel suspend/resume support, should now check for
> >> both 'pause' and 'resume' features.
> >>
> >> Existing clients of dma_get_slave_caps() have been checked and the only
> >> driver which rely on proper channel resuming is soc-generic-dmaengine-pcm
> >> driver, which has been updated to check the newly added capability.
> >> Existing 'cmd_pause' now only indicates that DMA engine support pausing
> >> given DMA channel.
> >
> > The change looks fine to me. I was hoping that serial would also check
> > this..
> 
> AFAIU serial does not need to resume.

I meant the serial should check for Pause capability..

^ permalink raw reply

* [v2] dmaengine: rcar-dmac: convert to SPDX identifiers
From: Vinod Koul @ 2018-07-06  6:02 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Geert Uytterhoeven, Dan Williams, Magnus Damm, Linux-Renesas,
	Laurent Pinchart, Kieran Bingham, Geert Uytterhoeven, dmaengine,
	Hiroki Negishi

On 04-07-18, 00:34, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> This patch is using C++ comment style for SPDX line only,
> because driver author want it.

Applied, thanks

^ permalink raw reply

* [2/3] k3dma: add support to reserved minimum channels
From: Vinod Koul @ 2018-07-06  6:09 UTC (permalink / raw)
  To: Guodong Xu
  Cc: Rob Herring, Mark Rutland, dan.j.williams, liyu65, Suzhuangluan,
	xuhongtao (A), zhongkaihua, Xuezhiliang, xupeng (Q), sunliang10,
	Fengbaopeng (kevin, Kirin Solution Dept), dmaengine, devicetree,
	linux-kernel

On 06-07-18, 11:05, Guodong Xu wrote:
> On Thu, Jun 28, 2018 at 2:02 PM Vinod <vkoul@kernel.org> wrote:
> >
> > On 22-06-18, 11:24, Guodong Xu wrote:
> > > From: Li Yu <liyu65@hisilicon.com>
> > >
> > > On k3 series of SoC, DMA controller reserves some channels for
> > > other on-chip coprocessors. By adding support to dma_min_chan, kernel
> > > will not be able to use these reserved channels.
> > >
> > > One example is on Hi3660 platform, channel 0 is reserved to lpm3.
> > >
> > > Please also refer to Documentation/devicetree/bindings/dma/k3dma.txt
> >
> > and if some other platform has channel X marked for co-processor, maybe
> > a last channel or something in middle, how will this work then?
> >
> Hi, Vinod
> 
> Sorry for delayed response. We checked with Kirin hardware design
> team, so far their design strategy is all Kirin SoC series reserve
> only from minimum side, saying channel 0, then 1, then 2. That impacts
> the current SoC in upstreaming, Kirin960 (Hi3660), and next versions
> in Kirin SoC, Kirin970 and 980, which may hit upstream later.

And what guarantees that they will not change their mind..

> > I am thinking this should be a mask, rather than min.
> >
> 
> So, since this driver k3dma.c is only used by Kirin SoC DMA
> controllers, I would prefer to keep the current design dma_min_chan
> unchanged.
> 
> What do you think?

I would still prefer bitmask to expose the channels you are supposed to
use

^ permalink raw reply

* dma: add support for reporting pause and resume separately
From: Marek Szyprowski @ 2018-07-06  6:22 UTC (permalink / raw)
  To: Vinod, Andy Shevchenko
  Cc: Mark Brown, Lars-Peter Clausen, dmaengine,
	ALSA Development Mailing List, Linux Kernel Mailing List,
	Dan Williams, Bartlomiej Zolnierkiewicz

Hi Vinod,

On 2018-07-06 08:00, Vinod wrote:
> On 04-07-18, 13:38, Andy Shevchenko wrote:
>> On Wed, Jul 4, 2018 at 10:00 AM, Vinod <vkoul@kernel.org> wrote:
>>> Hi Marek,
>>>
>>> On 02-07-18, 15:08, Marek Szyprowski wrote:
>>>> 'cmd_pause' DMA channel capability means that respective DMA engine
>>>> supports both pausing and resuming given DMA channel. However, in some
>>>> cases it is important to know if DMA channel can be paused without the
>>>> need to resume it. This is a typical requirement for proper residue
>>>> reading on transfer timeout in UART drivers. There are also some DMA
>>>> engines with limited hardware, which doesn't really support resuming.
>>> Am curious given that your hardware does not support resume, what was motivation
>>> for adding pause?
>>>
>>>> Reporting pause and resume capabilities separately allows UART drivers to
>>>> properly check for the really required capabilities and operate in DMA
>>>> mode also in systems with limited DMA hardware. On the other hand drivers,
>>>> which rely on full channel suspend/resume support, should now check for
>>>> both 'pause' and 'resume' features.
>>>>
>>>> Existing clients of dma_get_slave_caps() have been checked and the only
>>>> driver which rely on proper channel resuming is soc-generic-dmaengine-pcm
>>>> driver, which has been updated to check the newly added capability.
>>>> Existing 'cmd_pause' now only indicates that DMA engine support pausing
>>>> given DMA channel.
>>> The change looks fine to me. I was hoping that serial would also check
>>> this..
>> AFAIU serial does not need to resume.
> I meant the serial should check for Pause capability..

Yes, I want to add such check to Samsung Exynos serial driver once this 
change is
merged. PL330 DMA engine (which is used in Samsung Exynos SoCs) supports 
only
pause, it is not possible to check that functionality from serial driver 
yet.

Best regards

^ permalink raw reply

* dmaengine: nbpfaxi: Mark expected switch fall-through
From: Gustavo A. R. Silva @ 2018-07-06 11:47 UTC (permalink / raw)
  To: Vinod; +Cc: Dan Williams, dmaengine, linux-kernel

Hi Vinod,

On 07/06/2018 12:56 AM, Vinod wrote:
> On 02-07-18, 13:06, Gustavo A. R. Silva wrote:
>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>> where we are expecting to fall through.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>  drivers/dma/nbpfaxi.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/dma/nbpfaxi.c b/drivers/dma/nbpfaxi.c
>> index 2f9974d..8c7b2e8 100644
>> --- a/drivers/dma/nbpfaxi.c
>> +++ b/drivers/dma/nbpfaxi.c
>> @@ -479,6 +479,7 @@ static size_t nbpf_xfer_size(struct nbpf_device *nbpf,
>>  
>>  	default:
>>  		pr_warn("%s(): invalid bus width %u\n", __func__, width);
>> +		/* fall through */
> 
> Hmm this looks okay but am not able to trigger this warning..(used W=1) Did you
> see this warning on your build, if so what options?
> 

Add this to your Makefile:

KBUILD_CFLAGS  += $(call cc-option,-Wimplicit-fallthrough=2)

Thanks
---
Gustavo
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* dma: stm32: replace "%p" with "%pK"
From: Benjamin Gaignard @ 2018-07-06 13:02 UTC (permalink / raw)
  To: vkoul, dan.j.williams, mcoquelin.stm32, alexandre.torgue
  Cc: dmaengine, linux-arm-kernel, linux-kernel, Benjamin Gaignard

The format specifier "%p" can leak kernel addresses.
Use "%pK" instead.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 drivers/dma/stm32-dma.c  | 4 ++--
 drivers/dma/stm32-mdma.c | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index 8c5807362a25..379e8d534e61 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -594,7 +594,7 @@ static void stm32_dma_start_transfer(struct stm32_dma_chan *chan)
 
 	chan->busy = true;
 
-	dev_dbg(chan2dev(chan), "vchan %p: started\n", &chan->vchan);
+	dev_dbg(chan2dev(chan), "vchan %pK: started\n", &chan->vchan);
 }
 
 static void stm32_dma_configure_next_sg(struct stm32_dma_chan *chan)
@@ -693,7 +693,7 @@ static void stm32_dma_issue_pending(struct dma_chan *c)
 
 	spin_lock_irqsave(&chan->vchan.lock, flags);
 	if (vchan_issue_pending(&chan->vchan) && !chan->desc && !chan->busy) {
-		dev_dbg(chan2dev(chan), "vchan %p: issued\n", &chan->vchan);
+		dev_dbg(chan2dev(chan), "vchan %pK: issued\n", &chan->vchan);
 		stm32_dma_start_transfer(chan);
 
 	}
diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c
index 9dc450b7ace6..06dd1725375e 100644
--- a/drivers/dma/stm32-mdma.c
+++ b/drivers/dma/stm32-mdma.c
@@ -1170,7 +1170,7 @@ static void stm32_mdma_start_transfer(struct stm32_mdma_chan *chan)
 
 	chan->busy = true;
 
-	dev_dbg(chan2dev(chan), "vchan %p: started\n", &chan->vchan);
+	dev_dbg(chan2dev(chan), "vchan %pK: started\n", &chan->vchan);
 }
 
 static void stm32_mdma_issue_pending(struct dma_chan *c)
@@ -1183,7 +1183,7 @@ static void stm32_mdma_issue_pending(struct dma_chan *c)
 	if (!vchan_issue_pending(&chan->vchan))
 		goto end;
 
-	dev_dbg(chan2dev(chan), "vchan %p: issued\n", &chan->vchan);
+	dev_dbg(chan2dev(chan), "vchan %pK: issued\n", &chan->vchan);
 
 	if (!chan->desc && !chan->busy)
 		stm32_mdma_start_transfer(chan);
@@ -1203,7 +1203,7 @@ static int stm32_mdma_pause(struct dma_chan *c)
 	spin_unlock_irqrestore(&chan->vchan.lock, flags);
 
 	if (!ret)
-		dev_dbg(chan2dev(chan), "vchan %p: pause\n", &chan->vchan);
+		dev_dbg(chan2dev(chan), "vchan %pK: pause\n", &chan->vchan);
 
 	return ret;
 }
@@ -1240,7 +1240,7 @@ static int stm32_mdma_resume(struct dma_chan *c)
 
 	spin_unlock_irqrestore(&chan->vchan.lock, flags);
 
-	dev_dbg(chan2dev(chan), "vchan %p: resume\n", &chan->vchan);
+	dev_dbg(chan2dev(chan), "vchan %pK: resume\n", &chan->vchan);
 
 	return 0;
 }

^ permalink raw reply related

* dma: stm32: replace "%p" with "%pK"
From: Geert Uytterhoeven @ 2018-07-06 19:38 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: vkoul, Dan Williams, Maxime Coquelin, Alexandre Torgue, dmaengine,
	Linux Kernel Mailing List, Linux ARM, Benjamin Gaignard

Hi Benjamin,

On Fri, Jul 6, 2018 at 3:03 PM Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> The format specifier "%p" can leak kernel addresses.
> Use "%pK" instead.

Still? Isn't the value randomized these days?

Gr{oetje,eeting}s,

                        Geert

^ permalink raw reply

* [02/14] dmaengine: dma-jz4780: Separate chan/ctrl registers
From: PrasannaKumar Muralidharan @ 2018-07-07  7:27 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Vinod Koul, Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton,
	James Hogan, Zubair Lutfullah Kakakhel, Mathieu Malaterre,
	Daniel Silsby, dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Linux-MIPS

On 6 July 2018 at 03:15, Paul Cercueil <paul@crapouillou.net> wrote:
>
>
>> Paul,
>>
>> On 3 July 2018 at 18:02, Paul Cercueil <paul@crapouillou.net> wrote:
>>>
>>>  The register area of the JZ4780 DMA core can be split into different
>>>  sections for different purposes:
>>>
>>>  * one set of registers is used to perform actions at the DMA core level,
>>>  that will generally affect all channels;
>>>
>>>  * one set of registers per DMA channel, to perform actions at the DMA
>>>  channel level, that will only affect the channel in question.
>>>
>>>  The problem rises when trying to support new versions of the JZ47xx
>>>  Ingenic SoC. For instance, the JZ4770 has two DMA cores, each one
>>>  with six DMA channels, and the register sets are interleaved:
>>>  <DMA0 chan regs> <DMA1 chan regs> <DMA0 ctrl regs> <DMA1 ctrl regs>
>>>
>>>  By using one memory resource for the channel-specific registers and
>>>  one memory resource for the core-specific registers, we can support
>>>  the JZ4770, by initializing the driver once per DMA core with different
>>>  addresses.
>>
>>
>> As per my understanding device tree should be modified only when
>> hardware changes. This looks the other way around. It must be possible
>> to achieve what you are trying to do in this patch without changing
>> the device tree.
>
>
> I would agree that devicetree has an ABI that we shouldn't break if
> possible.
>
> However DTS support for all the Ingenic SoCs/boards is far from being
> complete, and more importantly, all Ingenic-based boards compile the DTS
> file within the kernel; so breaking the ABI is not (yet) a problem, and
> we should push the big changes right now while it's still possible.

Completely agree with you in this. Let's wait and see what DT maintainer's view.
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [01/14] dmaengine: dma-jz4780: Avoid hardcoding number of channels
From: PrasannaKumar Muralidharan @ 2018-07-07  7:34 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Vinod Koul, Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton,
	James Hogan, Zubair Lutfullah Kakakhel, Mathieu Malaterre,
	Daniel Silsby, dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Linux-MIPS

On 5 July 2018 at 23:56, Paul Cercueil <paul@crapouillou.net> wrote:
> Hi PrasannaKumar,
>
>
>> Hi Paul,
>>
>> On 3 July 2018 at 18:02, Paul Cercueil <paul@crapouillou.net> wrote:
>>>
>>>  As part of the work to support various other Ingenic JZ47xx SoC
>>> versions,
>>>  which don't feature the same number of DMA channels per core, we now
>>>  deduce the number of DMA channels available from the devicetree
>>>  compatible string.
>>>
>>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>>  ---
>>>   drivers/dma/dma-jz4780.c | 53 +++++++++++++++++++++++++++++-----------
>>>   1 file changed, 39 insertions(+), 14 deletions(-)
>>>
>>>  diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
>>>  index 85820a2d69d4..b40f491f0367 100644
>>>  --- a/drivers/dma/dma-jz4780.c
>>>  +++ b/drivers/dma/dma-jz4780.c
>>>  @@ -16,6 +16,7 @@
>>>   #include <linux/interrupt.h>
>>>   #include <linux/module.h>
>>>   #include <linux/of.h>
>>>  +#include <linux/of_device.h>
>>>   #include <linux/of_dma.h>
>>>   #include <linux/platform_device.h>
>>>   #include <linux/slab.h>
>>>  @@ -23,8 +24,6 @@
>>>   #include "dmaengine.h"
>>>   #include "virt-dma.h"
>>>
>>>  -#define JZ_DMA_NR_CHANNELS     32
>>>  -
>>>   /* Global registers. */
>>>   #define JZ_DMA_REG_DMAC                0x1000
>>>   #define JZ_DMA_REG_DIRQP       0x1004
>>>  @@ -135,14 +134,20 @@ struct jz4780_dma_chan {
>>>          unsigned int curr_hwdesc;
>>>   };
>>>
>>>  +enum jz_version {
>>>  +       ID_JZ4780,
>>>  +};
>>>  +
>>>   struct jz4780_dma_dev {
>>>          struct dma_device dma_device;
>>>          void __iomem *base;
>>>          struct clk *clk;
>>>          unsigned int irq;
>>>  +       unsigned int nb_channels;
>>>  +       enum jz_version version;
>>>
>>>          uint32_t chan_reserved;
>>>  -       struct jz4780_dma_chan chan[JZ_DMA_NR_CHANNELS];
>>>  +       struct jz4780_dma_chan chan[];
>>
>>
>> Looks like a variable length array in struct. I think there is some
>> effort to remove the usage of VLA. Can you revisit this? I may be
>> wrong, please feel free to correct.
>
>
> Are you sure? It's the first time I hear about it.
> Could anybody confirm?

Please see [1] for info.

Variable Length Arrays in struct is expressly forbidden in C99, C11.
Clang does not support it. To make kernel compile with Clang few
people are trying to remove/reduce VLAIS usage.

1. https://blog.linuxplumbersconf.org/2013/ocw/system/presentations/1221/original/VLAIS.pdf
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [01/14] dmaengine: dma-jz4780: Avoid hardcoding number of channels
From: Paul Cercueil @ 2018-07-07 11:01 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: Vinod Koul, Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton,
	James Hogan, Zubair Lutfullah Kakakhel, Mathieu Malaterre,
	Daniel Silsby, dmaengine,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Linux-MIPS

Le sam. 7 juil. 2018 à 9:34, PrasannaKumar Muralidharan 
<prasannatsmkumar@gmail.com> a écrit :
> On 5 July 2018 at 23:56, Paul Cercueil <paul@crapouillou.net> wrote:
>>  Hi PrasannaKumar,
>> 
>> 
>>>  Hi Paul,
>>> 
>>>  On 3 July 2018 at 18:02, Paul Cercueil <paul@crapouillou.net> 
>>> wrote:
>>>> 
>>>>   As part of the work to support various other Ingenic JZ47xx SoC
>>>>  versions,
>>>>   which don't feature the same number of DMA channels per core, we 
>>>> now
>>>>   deduce the number of DMA channels available from the devicetree
>>>>   compatible string.
>>>> 
>>>>   Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>>>   ---
>>>>    drivers/dma/dma-jz4780.c | 53 
>>>> +++++++++++++++++++++++++++++-----------
>>>>    1 file changed, 39 insertions(+), 14 deletions(-)
>>>> 
>>>>   diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
>>>>   index 85820a2d69d4..b40f491f0367 100644
>>>>   --- a/drivers/dma/dma-jz4780.c
>>>>   +++ b/drivers/dma/dma-jz4780.c
>>>>   @@ -16,6 +16,7 @@
>>>>    #include <linux/interrupt.h>
>>>>    #include <linux/module.h>
>>>>    #include <linux/of.h>
>>>>   +#include <linux/of_device.h>
>>>>    #include <linux/of_dma.h>
>>>>    #include <linux/platform_device.h>
>>>>    #include <linux/slab.h>
>>>>   @@ -23,8 +24,6 @@
>>>>    #include "dmaengine.h"
>>>>    #include "virt-dma.h"
>>>> 
>>>>   -#define JZ_DMA_NR_CHANNELS     32
>>>>   -
>>>>    /* Global registers. */
>>>>    #define JZ_DMA_REG_DMAC                0x1000
>>>>    #define JZ_DMA_REG_DIRQP       0x1004
>>>>   @@ -135,14 +134,20 @@ struct jz4780_dma_chan {
>>>>           unsigned int curr_hwdesc;
>>>>    };
>>>> 
>>>>   +enum jz_version {
>>>>   +       ID_JZ4780,
>>>>   +};
>>>>   +
>>>>    struct jz4780_dma_dev {
>>>>           struct dma_device dma_device;
>>>>           void __iomem *base;
>>>>           struct clk *clk;
>>>>           unsigned int irq;
>>>>   +       unsigned int nb_channels;
>>>>   +       enum jz_version version;
>>>> 
>>>>           uint32_t chan_reserved;
>>>>   -       struct jz4780_dma_chan chan[JZ_DMA_NR_CHANNELS];
>>>>   +       struct jz4780_dma_chan chan[];
>>> 
>>> 
>>>  Looks like a variable length array in struct. I think there is some
>>>  effort to remove the usage of VLA. Can you revisit this? I may be
>>>  wrong, please feel free to correct.
>> 
>> 
>>  Are you sure? It's the first time I hear about it.
>>  Could anybody confirm?
> 
> Please see [1] for info.
> 
> Variable Length Arrays in struct is expressly forbidden in C99, C11.
> Clang does not support it. To make kernel compile with Clang few
> people are trying to remove/reduce VLAIS usage.
> 
> 1. 
> https://blog.linuxplumbersconf.org/2013/ocw/system/presentations/1221/original/VLAIS.pdf

I read it, and my structure is not a VLAIS; my "chan" array is a 
flexible
array, its sizeof() is 0, so the sizeof() of the structure is constant.

See page 6 of the PDF, about alternatives to VLAIS:
"If possible use a flexible array member and move the array to the end 
of
the struct"
Which is what I am doing here.

-Paul
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox