Message ID | 20220412193936.63355-6-miquel.raynal@bootlin.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | RZN1 DMA support | expand |
Hi Miquel, On Tue, Apr 12, 2022 at 9:39 PM Miquel Raynal <miquel.raynal@bootlin.com> 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. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Thanks for your patch! > --- /dev/null > +++ b/drivers/dma/dw/rzn1-dmamux.c > @@ -0,0 +1,160 @@ > +// 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/bitops.h> > +#include <linux/of_device.h> > +#include <linux/of_dma.h> > +#include <linux/slab.h> > +#include <linux/soc/renesas/r9a06g032-sysctrl.h> > +#include <linux/types.h> > + > +#define RNZ1_DMAMUX_NCELLS 6 > +#define RZN1_DMAMUX_LINES 64 > +#define RZN1_DMAMUX_MAX_LINES 16 > + > +struct rzn1_dmamux_data { > + struct dma_router dmarouter; > + unsigned long *used_chans; Why a pointer? > +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; > + > + dmamux->used_chans = devm_bitmap_zalloc(&pdev->dev, 2 * RZN1_DMAMUX_MAX_LINES, > + GFP_KERNEL); ... Oh, you want to allocate the bitmap separately, although you know it's just a single long. You might as well declare it in rzn1_dmamux_data as: unsigned long used_chans[BITS_TO_LONGS(2 * RZN1_DMAMUX_MAX_LINES)]; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, geert@linux-m68k.org wrote on Wed, 13 Apr 2022 09:53:09 +0200: > Hi Miquel, > > On Tue, Apr 12, 2022 at 9:39 PM Miquel Raynal <miquel.raynal@bootlin.com> 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. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > Thanks for your patch! > > > --- /dev/null > > +++ b/drivers/dma/dw/rzn1-dmamux.c > > @@ -0,0 +1,160 @@ > > +// 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/bitops.h> > > +#include <linux/of_device.h> > > +#include <linux/of_dma.h> > > +#include <linux/slab.h> > > +#include <linux/soc/renesas/r9a06g032-sysctrl.h> > > +#include <linux/types.h> > > + > > +#define RNZ1_DMAMUX_NCELLS 6 > > +#define RZN1_DMAMUX_LINES 64 > > +#define RZN1_DMAMUX_MAX_LINES 16 > > + > > +struct rzn1_dmamux_data { > > + struct dma_router dmarouter; > > + unsigned long *used_chans; > > Why a pointer? > > > +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; > > + > > + dmamux->used_chans = devm_bitmap_zalloc(&pdev->dev, 2 * RZN1_DMAMUX_MAX_LINES, > > + GFP_KERNEL); > > ... Oh, you want to allocate the bitmap separately, although you > know it's just a single long. > > You might as well declare it in rzn1_dmamux_data as: > > unsigned long used_chans[BITS_TO_LONGS(2 * RZN1_DMAMUX_MAX_LINES)]; I've done that in versions v1..v8 and it was explicitly requested by Ilpo that I used something more specific like a bitmap (or an idr, but I don't think it fits well here). So now I'm using a bitmap... Thanks, Miquèl
Hi Miquel, On Wed, Apr 13, 2022 at 10:00 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > geert@linux-m68k.org wrote on Wed, 13 Apr 2022 09:53:09 +0200: > > On Tue, Apr 12, 2022 at 9:39 PM Miquel Raynal <miquel.raynal@bootlin.com> 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. > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > Thanks for your patch! > > > > > --- /dev/null > > > +++ b/drivers/dma/dw/rzn1-dmamux.c > > > @@ -0,0 +1,160 @@ > > > +// 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/bitops.h> > > > +#include <linux/of_device.h> > > > +#include <linux/of_dma.h> > > > +#include <linux/slab.h> > > > +#include <linux/soc/renesas/r9a06g032-sysctrl.h> > > > +#include <linux/types.h> > > > + > > > +#define RNZ1_DMAMUX_NCELLS 6 > > > +#define RZN1_DMAMUX_LINES 64 > > > +#define RZN1_DMAMUX_MAX_LINES 16 > > > + > > > +struct rzn1_dmamux_data { > > > + struct dma_router dmarouter; > > > + unsigned long *used_chans; > > > > Why a pointer? > > > > > +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; > > > + > > > + dmamux->used_chans = devm_bitmap_zalloc(&pdev->dev, 2 * RZN1_DMAMUX_MAX_LINES, > > > + GFP_KERNEL); > > > > ... Oh, you want to allocate the bitmap separately, although you > > know it's just a single long. > > > > You might as well declare it in rzn1_dmamux_data as: > > > > unsigned long used_chans[BITS_TO_LONGS(2 * RZN1_DMAMUX_MAX_LINES)]; > > I've done that in versions v1..v8 and it was explicitly requested by > Ilpo that I used something more specific like a bitmap (or an idr, but > I don't think it fits well here). So now I'm using a bitmap... Right, my bad ;-) DECLARE_BITMAP(used_chans, 2 * RZN1_DMAMUX_MAX_LINES); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, Apr 13, 2022 at 09:53:09AM +0200, Geert Uytterhoeven wrote: > On Tue, Apr 12, 2022 at 9:39 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: ... > You might as well declare it in rzn1_dmamux_data as: > > unsigned long used_chans[BITS_TO_LONGS(2 * RZN1_DMAMUX_MAX_LINES)]; But better to use proper macro instead of open coding this...
On Wed, Apr 13, 2022 at 10:05:43AM +0200, Geert Uytterhoeven wrote: > On Wed, Apr 13, 2022 at 10:00 AM Miquel Raynal > <miquel.raynal@bootlin.com> wrote: ... > DECLARE_BITMAP(used_chans, 2 * RZN1_DMAMUX_MAX_LINES); Yep, this one.
Hi Andy, Geert, andriy.shevchenko@linux.intel.com wrote on Wed, 13 Apr 2022 13:41:23 +0300: > On Wed, Apr 13, 2022 at 10:05:43AM +0200, Geert Uytterhoeven wrote: > > On Wed, Apr 13, 2022 at 10:00 AM Miquel Raynal > > <miquel.raynal@bootlin.com> wrote: > > ... > > > DECLARE_BITMAP(used_chans, 2 * RZN1_DMAMUX_MAX_LINES); I'll make the update. Do you mind if I send only this patch as a v11? There is no change in any of the other patches anyway and I think I've received all the acks required. Thanks, Miquèl
Hi Andy, andriy.shevchenko@linux.intel.com wrote on Wed, 13 Apr 2022 13:40:27 +0300: > On Wed, Apr 13, 2022 at 09:53:09AM +0200, Geert Uytterhoeven wrote: > > On Tue, Apr 12, 2022 at 9:39 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > ... > > > You might as well declare it in rzn1_dmamux_data as: > > > > unsigned long used_chans[BITS_TO_LONGS(2 * RZN1_DMAMUX_MAX_LINES)]; Now at the top of the driver I have #define RZN1_DMAMUX_LINES 64 #define RZN1_DMAMUX_MAX_LINES 16 Which does not look right. So FYI I decided to change them to #define RZN1_DMAMUX_MAX_LINES 64 #define RZN1_DMAMUX_LINES_PER_CTLR 16 which feels much more clear. Thanks, Miquèl
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..e1796015f213 100644 --- a/drivers/dma/dw/Makefile +++ b/drivers/dma/dw/Makefile @@ -9,3 +9,5 @@ dw_dmac-$(CONFIG_OF) += of.o obj-$(CONFIG_DW_DMAC_PCI) += dw_dmac_pci.o dw_dmac_pci-y := pci.o + +obj-$(CONFIG_RZN1_DMAMUX) += rzn1-dmamux.o diff --git a/drivers/dma/dw/rzn1-dmamux.c b/drivers/dma/dw/rzn1-dmamux.c new file mode 100644 index 000000000000..95db9b7cf88b --- /dev/null +++ b/drivers/dma/dw/rzn1-dmamux.c @@ -0,0 +1,160 @@ +// 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/bitops.h> +#include <linux/of_device.h> +#include <linux/of_dma.h> +#include <linux/slab.h> +#include <linux/soc/renesas/r9a06g032-sysctrl.h> +#include <linux/types.h> + +#define RNZ1_DMAMUX_NCELLS 6 +#define RZN1_DMAMUX_LINES 64 +#define RZN1_DMAMUX_MAX_LINES 16 + +struct rzn1_dmamux_data { + struct dma_router dmarouter; + unsigned long *used_chans; +}; + +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); + + clear_bit(map->req_idx, dmamux->used_chans); + + 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 != RNZ1_DMAMUX_NCELLS) + 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_MAX_LINES) { + 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_MAX_LINES) != 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_MAX_LINES ? 1 : 0; + 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); + + if (test_and_set_bit(map->req_idx, dmamux->used_chans)) { + ret = -EBUSY; + goto free_map; + } + + mask = BIT(map->req_idx); + ret = r9a06g032_sysctrl_set_dmamux(mask, val ? mask : 0); + if (ret) + goto clear_bitmap; + + return map; + +clear_bitmap: + clear_bit(map->req_idx, dmamux->used_chans); +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; + + dmamux->used_chans = devm_bitmap_zalloc(&pdev->dev, 2 * RZN1_DMAMUX_MAX_LINES, + GFP_KERNEL); + if (!dmamux->used_chans) + return -ENOMEM; + + 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");