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

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

Hi Vinod,


> > +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);  
> 
> Why not use idr or bitmap for this. Hint: former does locking as well

I've changed the code to use a proper bitmap.

> 
> > +
> > +	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)  
> 
> magic

Defined.

> 
> > +		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;
> > +

[...]

> > +	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);  
> 
> I guess due to this it would be merged by whosoever merges this api.
> Please mention this in cover letter and how you propose this should be
> merged

Yes, the cover letter mentions this issue, but since then Geert
proposed to take everything through the renesas trees, which I agree
with. I will send a v9 and if you agree with it please provide your Ack
so that Geert can take it.

Thanks,
Miquèl