| Message ID | 20220405081911.1349563-6-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | RZN1 DMA support | expand |
On Tue, Apr 05, 2022 at 10:19:07AM +0200, Miquel Raynal wrote: > The Renesas RZN1 DMA IP is 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 > dw_dmac-$(CONFIG_OF) += of.o > +obj-$(CONFIG_RZN1_DMAMUX) += rzn1-dmamux.o I would move it down in the file to distinguish more generic drivers from specific quirks. > obj-$(CONFIG_DW_DMAC_PCI) += dw_dmac_pci.o > dw_dmac_pci-y := pci.o ... > +#define RZN1_DMAMUX_SPLIT 16 I would name it more explicitly: #define RZN1_DMAMUX_SPLIT_1_0 16 ... > + dmac_idx = map->req_idx < RZN1_DMAMUX_SPLIT ? 0 : 1; ...and hence use positive conditional: dmac_idx = map->req_idx >= RZN1_DMAMUX_SPLIT_1_0 ? 1 : 0; With above addressed Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Hi Andy, andriy.shevchenko@linux.intel.com wrote on Tue, 5 Apr 2022 17:50:57 +0300: > On Tue, Apr 05, 2022 at 10:19:07AM +0200, Miquel Raynal wrote: > > The Renesas RZN1 DMA IP is 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 > > dw_dmac-$(CONFIG_OF) += of.o > > > +obj-$(CONFIG_RZN1_DMAMUX) += rzn1-dmamux.o > > I would move it down in the file to distinguish more generic drivers > from specific quirks. Ok. > > > obj-$(CONFIG_DW_DMAC_PCI) += dw_dmac_pci.o > > dw_dmac_pci-y := pci.o > > ... > > > +#define RZN1_DMAMUX_SPLIT 16 > > I would name it more explicitly: > > #define RZN1_DMAMUX_SPLIT_1_0 16 I am sorry but I don't understand this suffix, which probably means that it is not as clear as we wish. Do you mind if I stick to RZN1_DMAMUX_SPLIT? > ... > > > + dmac_idx = map->req_idx < RZN1_DMAMUX_SPLIT ? 0 : 1; > > ...and hence use positive conditional: > > dmac_idx = map->req_idx >= RZN1_DMAMUX_SPLIT_1_0 ? 1 : 0; I will. > > With above addressed > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Thanks, Miquèl
On Wed, Apr 06, 2022 at 09:49:08AM +0200, Miquel Raynal wrote: > andriy.shevchenko@linux.intel.com wrote on Tue, 5 Apr 2022 17:50:57 > +0300: > > On Tue, Apr 05, 2022 at 10:19:07AM +0200, Miquel Raynal wrote: ... > > > +#define RZN1_DMAMUX_SPLIT 16 > > > > I would name it more explicitly: > > > > #define RZN1_DMAMUX_SPLIT_1_0 16 > > I am sorry but I don't understand this suffix, which probably means > that it is not as clear as we wish. Do you mind if I stick to > RZN1_DMAMUX_SPLIT? The suffix to show that this is the value between part 0 (indexed by 0) and part 1 (indexed by 1) as far as I can see they are different by size. Since they are not equal, the original name without suffix is confusing (I would expect indexing up to 4 in such case).
Hi Andy, andriy.shevchenko@linux.intel.com wrote on Wed, 6 Apr 2022 11:59:05 +0300: > On Wed, Apr 06, 2022 at 09:49:08AM +0200, Miquel Raynal wrote: > > andriy.shevchenko@linux.intel.com wrote on Tue, 5 Apr 2022 17:50:57 > > +0300: > > > On Tue, Apr 05, 2022 at 10:19:07AM +0200, Miquel Raynal wrote: > > ... > > > > > +#define RZN1_DMAMUX_SPLIT 16 > > > > > > I would name it more explicitly: > > > > > > #define RZN1_DMAMUX_SPLIT_1_0 16 > > > > I am sorry but I don't understand this suffix, which probably means > > that it is not as clear as we wish. Do you mind if I stick to > > RZN1_DMAMUX_SPLIT? > > The suffix to show that this is the value between part 0 (indexed by 0) and > part 1 (indexed by 1) as far as I can see they are different by size. Since > they are not equal, the original name without suffix is confusing (I would > expect indexing up to 4 in such case). They are equivalent in size (0-15/16-31), or aren't we talking about the same thing? Thanks, Miquèl
On Wed, Apr 06, 2022 at 02:13:46PM +0200, Miquel Raynal wrote: > andriy.shevchenko@linux.intel.com wrote on Wed, 6 Apr 2022 11:59:05 > +0300: > > On Wed, Apr 06, 2022 at 09:49:08AM +0200, Miquel Raynal wrote: > > > andriy.shevchenko@linux.intel.com wrote on Tue, 5 Apr 2022 17:50:57 > > > +0300: > > > > On Tue, Apr 05, 2022 at 10:19:07AM +0200, Miquel Raynal wrote: ... > > > > > +#define RZN1_DMAMUX_SPLIT 16 > > > > > > > > I would name it more explicitly: > > > > > > > > #define RZN1_DMAMUX_SPLIT_1_0 16 > > > > > > I am sorry but I don't understand this suffix, which probably means > > > that it is not as clear as we wish. Do you mind if I stick to > > > RZN1_DMAMUX_SPLIT? > > > > The suffix to show that this is the value between part 0 (indexed by 0) and > > part 1 (indexed by 1) as far as I can see they are different by size. Since > > they are not equal, the original name without suffix is confusing (I would > > expect indexing up to 4 in such case). > > They are equivalent in size (0-15/16-31), or aren't we talking about the > same thing? Hmm... I have misread something then. To clarify that the portions are equal perhaps we can name it MAX_LINES or so instead of SPLIT?
diff --git a/drivers/dma/dw/Kconfig b/drivers/dma/dw/Kconfig index db25f9b7778c..a9828ddd6d06 100644 --- a/drivers/dma/dw/Kconfig +++ b/drivers/dma/dw/Kconfig @@ -16,6 +16,15 @@ config DW_DMAC Support the Synopsys DesignWare AHB DMA controller. This can be integrated in chips such as the Intel Cherrytrail. +config RZN1_DMAMUX + tristate "Renesas RZ/N1 DMAMUX driver" + depends on DW_DMAC + depends on ARCH_RZN1 || COMPILE_TEST + help + Support the Renesas RZ/N1 DMAMUX which is located in front of + the Synopsys DesignWare AHB DMA controller located on Renesas + SoCs. + config DW_DMAC_PCI tristate "Synopsys DesignWare AHB DMA PCI driver" depends on PCI diff --git a/drivers/dma/dw/Makefile b/drivers/dma/dw/Makefile index a6f358ad8591..8025f75e589c 100644 --- a/drivers/dma/dw/Makefile +++ b/drivers/dma/dw/Makefile @@ -7,5 +7,7 @@ obj-$(CONFIG_DW_DMAC) += dw_dmac.o dw_dmac-y := platform.o dw_dmac-$(CONFIG_OF) += of.o +obj-$(CONFIG_RZN1_DMAMUX) += rzn1-dmamux.o + obj-$(CONFIG_DW_DMAC_PCI) += dw_dmac_pci.o dw_dmac_pci-y := pci.o diff --git a/drivers/dma/dw/rzn1-dmamux.c b/drivers/dma/dw/rzn1-dmamux.c new file mode 100644 index 000000000000..a3313f268275 --- /dev/null +++ b/drivers/dma/dw/rzn1-dmamux.c @@ -0,0 +1,157 @@ +// 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/of_device.h> +#include <linux/of_dma.h> +#include <linux/slab.h> +#include <linux/soc/renesas/r9a06g032-sysctrl.h> + +#define RZN1_DMAMUX_LINES 64 +#define RZN1_DMAMUX_SPLIT 16 + +struct rzn1_dmamux_data { + struct dma_router dmarouter; + 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 dmac_idx, chan, val; + u32 mask; + int ret; + + if (dma_spec->args_count != 6) + return ERR_PTR(-EINVAL); + + map = kzalloc(sizeof(*map), GFP_KERNEL); + if (!map) + return ERR_PTR(-ENOMEM); + + chan = dma_spec->args[0]; + map->req_idx = dma_spec->args[4]; + val = dma_spec->args[5]; + dma_spec->args_count -= 2; + + if (chan >= RZN1_DMAMUX_SPLIT) { + dev_err(&pdev->dev, "Invalid DMA request line: %u\n", chan); + ret = -EINVAL; + goto free_map; + } + + if (map->req_idx >= RZN1_DMAMUX_LINES || + (map->req_idx % RZN1_DMAMUX_SPLIT) != chan) { + dev_err(&pdev->dev, "Invalid MUX request line: %u\n", map->req_idx); + ret = -EINVAL; + goto free_map; + } + + dmac_idx = map->req_idx < RZN1_DMAMUX_SPLIT ? 0 : 1; + dma_spec->np = of_parse_phandle(ofdma->of_node, "dma-masters", dmac_idx); + if (!dma_spec->np) { + dev_err(&pdev->dev, "Can't get DMA master\n"); + ret = -EINVAL; + goto free_map; + } + + dev_dbg(&pdev->dev, "Mapping DMAMUX request %u to DMAC%u request %u\n", + map->req_idx, dmac_idx, chan); + + mask = BIT(map->req_idx); + mutex_lock(&dmamux->lock); + dmamux->used_chans |= mask; + ret = r9a06g032_sysctrl_set_dmamux(mask, val ? mask : 0); + if (ret) + goto release_chan_and_unlock; + + mutex_unlock(&dmamux->lock); + + return map; + +release_chan_and_unlock: + dmamux->used_chans &= ~mask; + mutex_unlock(&dmamux->lock); +free_map: + kfree(map); + + return ERR_PTR(ret); +} + +static const struct of_device_id rzn1_dmac_match[] = { + { .compatible = "renesas,rzn1-dma" }, + {} +}; + +static int rzn1_dmamux_probe(struct platform_device *pdev) +{ + struct device_node *mux_node = pdev->dev.of_node; + const struct of_device_id *match; + struct device_node *dmac_node; + struct rzn1_dmamux_data *dmamux; + + dmamux = devm_kzalloc(&pdev->dev, sizeof(*dmamux), GFP_KERNEL); + if (!dmamux) + return -ENOMEM; + + mutex_init(&dmamux->lock); + + dmac_node = of_parse_phandle(mux_node, "dma-masters", 0); + if (!dmac_node) + return dev_err_probe(&pdev->dev, -ENODEV, "Can't get DMA master node\n"); + + match = of_match_node(rzn1_dmac_match, dmac_node); + of_node_put(dmac_node); + if (!match) + return dev_err_probe(&pdev->dev, -EINVAL, "DMA master is not supported\n"); + + dmamux->dmarouter.dev = &pdev->dev; + dmamux->dmarouter.route_free = rzn1_dmamux_free; + + platform_set_drvdata(pdev, dmamux); + + return of_dma_router_register(mux_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, +}; +module_platform_driver(rzn1_dmamux_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com"); +MODULE_DESCRIPTION("Renesas RZ/N1 DMAMUX driver");
The Renesas RZN1 DMA IP is 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/Kconfig | 9 ++ drivers/dma/dw/Makefile | 2 + drivers/dma/dw/rzn1-dmamux.c | 157 +++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 drivers/dma/dw/rzn1-dmamux.c