| Message ID | 20220218181226.431098-5-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RZN1 DMA support | expand |
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. > +};
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
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
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
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 --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);
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