Re: [PATCH v10 5/9] dmaengine: dw: dmamux: Introduce RZN1 DMA router support

Miquel Raynal
2022-04-13T08:01:16Z

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