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

[Miquel Raynal <miquel.raynal@bootlin.com>]

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