diff mbox series

[4/8] dma: dmamux: Introduce RZN1 DMA router support

Message ID 20220218181226.431098-5-miquel.raynal@bootlin.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series RZN1 DMA support | expand

Commit Message

Miquel Raynal Feb. 18, 2022, 6:12 p.m. UTC 
The Renesas RZN1 DMA IP is a based on a DW core, with eg. an additional
dmamux register located in the system control area which can take up to
32 requests (16 per DMA controller). Each DMA channel can be wired to
two different peripherals.

We need two additional information from the 'dmas' property: the channel
(bit in the dmamux register) that must be accessed and the value of the
mux for this channel.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/dma/dw/Makefile |   2 +-
 drivers/dma/dw/dmamux.c | 175 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 drivers/dma/dw/dmamux.c

Comments

Andy Shevchenko Feb. 20, 2022, 10:56 a.m. UTC | #1 
On Fri, Feb 18, 2022 at 07:12:22PM +0100, Miquel Raynal wrote:
> The Renesas RZN1 DMA IP is a based on a DW core, with eg. an additional
> dmamux register located in the system control area which can take up to
> 32 requests (16 per DMA controller). Each DMA channel can be wired to
> two different peripherals.
> 
> We need two additional information from the 'dmas' property: the channel
> (bit in the dmamux register) that must be accessed and the value of the
> mux for this channel.

...

> +dw_dmac-y			:= platform.o dmamux.o

We do not need this on other platforms, please make sure we have no dangling
code on, e.g., x86.

...

> +	/* The of_node_put() will be done in the core for the node */
> +	master = map->req_idx < dmamux->dmac_requests ? 0 : 1;

The opposite conditional will be better, no?`

...

> +	dmamux->used_chans |= BIT(map->req_idx);
> +	ret = r9a06g032_syscon_set_dmamux(BIT(map->req_idx),
> +					  val ? BIT(map->req_idx) : 0);


Cleaner to do

	u32 mask = BIT(...);
	...

	dmamux->used_chans |= mask;
	ret = r9a06g032_syscon_set_dmamux(mask, val ? mask : 0);

...

> +static const struct of_device_id rzn1_dmac_match[] __maybe_unused = {
> +	{ .compatible = "renesas,rzn1-dma", },
> +	{},

No comma for terminator entry.

> +};

...

> +	if (!node)
> +		return -ENODEV;

Dup check, why not to simply try for phandle first?

...

> +	if (of_property_read_u32(dmac_node, "dma-requests",
> +				 &dmamux->dmac_requests)) {

One line?

> +		dev_err(&pdev->dev, "Missing DMAC requests information\n");
> +		of_node_put(dmac_node);
> +		return -EINVAL;

First put node, then simply use dev_err_probe().

> +	}

...

> +static const struct of_device_id rzn1_dmamux_match[] = {
> +	{ .compatible = "renesas,rzn1-dmamux", },
> +	{},

No comma.

> +};
kernel test robot Feb. 20, 2022, 9:22 p.m. UTC | #2 
Hi Miquel,

I love your patch! Yet something to improve:

[auto build test ERROR on geert-renesas-devel/next]
[also build test ERROR on geert-renesas-drivers/renesas-clk robh/for-next linus/master v5.17-rc4 next-20220217]
[cannot apply to vkoul-dmaengine/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Miquel-Raynal/RZN1-DMA-support/20220220-182519
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git next
config: microblaze-randconfig-r026-20220220 (https://download.01.org/0day-ci/archive/20220221/202202210543.GR8q2zw2-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/df0b7e58b46473e407c2c552f843d0628ad6875d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Miquel-Raynal/RZN1-DMA-support/20220220-182519
        git checkout df0b7e58b46473e407c2c552f843d0628ad6875d
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=microblaze SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   microblaze-linux-ld: drivers/dma/dw/dmamux.o: in function `rzn1_dmamux_init':
>> (.text+0x45c): multiple definition of `init_module'; drivers/dma/dw/platform.o:(.init.text+0x0): first defined here

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Feb. 21, 2022, 4:15 a.m. UTC | #3 
Hi Miquel,

I love your patch! Yet something to improve:

[auto build test ERROR on geert-renesas-devel/next]
[also build test ERROR on geert-renesas-drivers/renesas-clk robh/for-next linus/master v5.17-rc5 next-20220217]
[cannot apply to vkoul-dmaengine/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Miquel-Raynal/RZN1-DMA-support/20220220-182519
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git next
config: ia64-allmodconfig (https://download.01.org/0day-ci/archive/20220221/202202211236.07FjzSmp-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/df0b7e58b46473e407c2c552f843d0628ad6875d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Miquel-Raynal/RZN1-DMA-support/20220220-182519
        git checkout df0b7e58b46473e407c2c552f843d0628ad6875d
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ia64-linux-ld: drivers/dma/dw/dmamux.o: in function `rzn1_dmamux_init':
>> dmamux.c:(.text+0x9c0): multiple definition of `init_module'; drivers/dma/dw/platform.o:platform.c:(.init.text+0x0): first defined here

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Miquel Raynal Feb. 21, 2022, 3:13 p.m. UTC | #4 
Hi Andy,

andriy.shevchenko@linux.intel.com wrote on Sun, 20 Feb 2022 12:56:02
+0200:

> On Fri, Feb 18, 2022 at 07:12:22PM +0100, Miquel Raynal wrote:
> > The Renesas RZN1 DMA IP is a based on a DW core, with eg. an additional
> > dmamux register located in the system control area which can take up to
> > 32 requests (16 per DMA controller). Each DMA channel can be wired to
> > two different peripherals.
> > 
> > We need two additional information from the 'dmas' property: the channel
> > (bit in the dmamux register) that must be accessed and the value of the
> > mux for this channel.  
> 
> ...

Thanks for the review!

> 
> > +dw_dmac-y			:= platform.o dmamux.o  
> 
> We do not need this on other platforms, please make sure we have no dangling
> code on, e.g., x86.
> 
> ...
> 
> > +	/* The of_node_put() will be done in the core for the node */
> > +	master = map->req_idx < dmamux->dmac_requests ? 0 : 1;  
> 
> The opposite conditional will be better, no?`

I guess this is a matter of taste. I prefer the current writing but I
will change it.

> 
> ...
> 
> > +	dmamux->used_chans |= BIT(map->req_idx);
> > +	ret = r9a06g032_syscon_set_dmamux(BIT(map->req_idx),
> > +					  val ? BIT(map->req_idx) : 0);  
> 
> 
> Cleaner to do
> 
> 	u32 mask = BIT(...);
> 	...
> 
> 	dmamux->used_chans |= mask;
> 	ret = r9a06g032_syscon_set_dmamux(mask, val ? mask : 0);

Fine.

> 
> ...
> 
> > +static const struct of_device_id rzn1_dmac_match[] __maybe_unused = {
> > +	{ .compatible = "renesas,rzn1-dma", },
> > +	{},  
> 
> No comma for terminator entry.

Mmh, ok.

> 
> > +};  
> 
> ...
> 
> > +	if (!node)
> > +		return -ENODEV;  
> 
> Dup check, why not to simply try for phandle first?

I'll drop it.

> 
> ...
> 
> > +	if (of_property_read_u32(dmac_node, "dma-requests",
> > +				 &dmamux->dmac_requests)) {  
> 
> One line?

Ok.

> 
> > +		dev_err(&pdev->dev, "Missing DMAC requests information\n");
> > +		of_node_put(dmac_node);
> > +		return -EINVAL;  
> 
> First put node, then simply use dev_err_probe().

I don't get the point here. dev_err_probe() is useful when -EPROBE_DEFER
can be returned, right? I don't understand what it would bring here nor
how I should use it to simplify error handling.

> 
> > +	}  
> 
> ...
> 
> > +static const struct of_device_id rzn1_dmamux_match[] = {
> > +	{ .compatible = "renesas,rzn1-dmamux", },
> > +	{},  
> 
> No comma.

Ok.

> 
> > +};  
> 


Thanks,
Miquèl
Andy Shevchenko Feb. 21, 2022, 4:47 p.m. UTC | #5 
On Mon, Feb 21, 2022 at 04:13:20PM +0100, Miquel Raynal wrote:
> andriy.shevchenko@linux.intel.com wrote on Sun, 20 Feb 2022 12:56:02
> +0200:
> > On Fri, Feb 18, 2022 at 07:12:22PM +0100, Miquel Raynal wrote:

...

> > > +		dev_err(&pdev->dev, "Missing DMAC requests information\n");
> > > +		of_node_put(dmac_node);
> > > +		return -EINVAL;  
> > 
> > First put node, then simply use dev_err_probe().
> 
> I don't get the point here. dev_err_probe() is useful when -EPROBE_DEFER
> can be returned, right? I don't understand what it would bring here nor
> how I should use it to simplify error handling.

Less LOCs, and it's fine to call it here. This usecase is described in the
dev_err_probe() documentation.
diff mbox series

Patch

diff --git a/drivers/dma/dw/Makefile b/drivers/dma/dw/Makefile
index a6f358ad8591..d8cfbf36b381 100644
--- a/drivers/dma/dw/Makefile
+++ b/drivers/dma/dw/Makefile
@@ -4,7 +4,7 @@  dw_dmac_core-y			:= core.o dw.o idma32.o
 dw_dmac_core-$(CONFIG_ACPI)	+= acpi.o
 
 obj-$(CONFIG_DW_DMAC)		+= dw_dmac.o
-dw_dmac-y			:= platform.o
+dw_dmac-y			:= platform.o dmamux.o
 dw_dmac-$(CONFIG_OF)		+= of.o
 
 obj-$(CONFIG_DW_DMAC_PCI)	+= dw_dmac_pci.o
diff --git a/drivers/dma/dw/dmamux.c b/drivers/dma/dw/dmamux.c
new file mode 100644
index 000000000000..30de776f195e
--- /dev/null
+++ b/drivers/dma/dw/dmamux.c
@@ -0,0 +1,175 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Schneider-Electric
+ * Author: Miquel Raynal <miquel.raynal@bootlin.com
+ * Based on TI crossbar driver written by Peter Ujfalusi <peter.ujfalusi@ti.com>
+ */
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_dma.h>
+#include <linux/soc/renesas/r9a06g032-syscon.h>
+
+#define RZN1_DMAMUX_LINES	64
+
+struct rzn1_dmamux_data {
+	struct dma_router dmarouter;
+	unsigned int dmac_requests;
+	unsigned int dmamux_requests;
+	u32 used_chans;
+	struct mutex lock;
+};
+
+struct rzn1_dmamux_map {
+	unsigned int req_idx;
+};
+
+static void rzn1_dmamux_free(struct device *dev, void *route_data)
+{
+	struct rzn1_dmamux_data *dmamux = dev_get_drvdata(dev);
+	struct rzn1_dmamux_map *map = route_data;
+
+	dev_dbg(dev, "Unmapping DMAMUX request %u\n", map->req_idx);
+
+	mutex_lock(&dmamux->lock);
+	dmamux->used_chans &= ~BIT(map->req_idx);
+	mutex_unlock(&dmamux->lock);
+
+	kfree(map);
+}
+
+static void *rzn1_dmamux_route_allocate(struct of_phandle_args *dma_spec,
+					struct of_dma *ofdma)
+{
+	struct platform_device *pdev = of_find_device_by_node(ofdma->of_node);
+	struct rzn1_dmamux_data *dmamux = platform_get_drvdata(pdev);
+	struct rzn1_dmamux_map *map;
+	unsigned int master, chan, val;
+	int ret;
+
+	map = kzalloc(sizeof(*map), GFP_KERNEL);
+	if (!map)
+		return ERR_PTR(-ENOMEM);
+
+	if (dma_spec->args_count != 6)
+		return ERR_PTR(-EINVAL);
+
+	chan = dma_spec->args[0];
+	map->req_idx = dma_spec->args[4];
+	val = dma_spec->args[5];
+	dma_spec->args_count -= 2;
+
+	if (chan >= dmamux->dmac_requests) {
+		dev_err(&pdev->dev, "Invalid DMA request line: %d\n", chan);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (map->req_idx >= dmamux->dmamux_requests ||
+	    map->req_idx % dmamux->dmac_requests != chan) {
+		dev_err(&pdev->dev, "Invalid MUX request line: %d\n", map->req_idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* The of_node_put() will be done in the core for the node */
+	master = map->req_idx < dmamux->dmac_requests ? 0 : 1;
+	dma_spec->np = of_parse_phandle(ofdma->of_node, "dma-masters", master);
+	if (!dma_spec->np) {
+		dev_err(&pdev->dev, "Can't get DMA master\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	dev_dbg(&pdev->dev, "Mapping DMAMUX request %u to DMAC%u request %u\n",
+		map->req_idx, master, chan);
+
+	mutex_lock(&dmamux->lock);
+	dmamux->used_chans |= BIT(map->req_idx);
+	ret = r9a06g032_syscon_set_dmamux(BIT(map->req_idx),
+					  val ? BIT(map->req_idx) : 0);
+	mutex_unlock(&dmamux->lock);
+	if (ret) {
+		rzn1_dmamux_free(&pdev->dev, map);
+		return ERR_PTR(ret);
+	}
+
+	return map;
+}
+
+static const struct of_device_id rzn1_dmac_match[] __maybe_unused = {
+	{ .compatible = "renesas,rzn1-dma", },
+	{},
+};
+
+static int rzn1_dmamux_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	const struct of_device_id *match;
+	struct device_node *dmac_node;
+	struct rzn1_dmamux_data *dmamux;
+
+	if (!node)
+		return -ENODEV;
+
+	dmamux = devm_kzalloc(&pdev->dev, sizeof(*dmamux), GFP_KERNEL);
+	if (!dmamux)
+		return -ENOMEM;
+
+	mutex_init(&dmamux->lock);
+
+	dmac_node = of_parse_phandle(node, "dma-masters", 0);
+	if (!dmac_node) {
+		dev_err(&pdev->dev, "Can't get DMA master node\n");
+		return -ENODEV;
+	}
+
+	match = of_match_node(rzn1_dmac_match, dmac_node);
+	if (!match) {
+		dev_err(&pdev->dev, "DMA master is not supported\n");
+		of_node_put(dmac_node);
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(dmac_node, "dma-requests",
+				 &dmamux->dmac_requests)) {
+		dev_err(&pdev->dev, "Missing DMAC requests information\n");
+		of_node_put(dmac_node);
+		return -EINVAL;
+	}
+	of_node_put(dmac_node);
+
+	if (of_property_read_u32(node, "dma-requests",
+				 &dmamux->dmamux_requests)) {
+		dev_err(&pdev->dev, "Missing DMA mux requests information\n");
+		return -EINVAL;
+	}
+
+	dmamux->dmarouter.dev = &pdev->dev;
+	dmamux->dmarouter.route_free = rzn1_dmamux_free;
+
+	platform_set_drvdata(pdev, dmamux);
+
+	return of_dma_router_register(node, rzn1_dmamux_route_allocate,
+				      &dmamux->dmarouter);
+}
+
+static const struct of_device_id rzn1_dmamux_match[] = {
+	{ .compatible = "renesas,rzn1-dmamux", },
+	{},
+};
+
+static struct platform_driver rzn1_dmamux_driver = {
+	.driver = {
+		.name = "renesas,rzn1-dmamux",
+		.of_match_table = rzn1_dmamux_match,
+	},
+	.probe	= rzn1_dmamux_probe,
+};
+
+static int rzn1_dmamux_init(void)
+{
+	return platform_driver_register(&rzn1_dmamux_driver);
+}
+arch_initcall(rzn1_dmamux_init);