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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, Apr 08, 2022 at 12:55:47PM +0300, Ilpo Järvinen wrote:
> On Wed, 6 Apr 2022, 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.

> > +	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;
> 
> Now that I check this again, I'm not sure why dmamux->used_chans |= mask; 
> couldn't be done after r9a06g032_sysctrl_set_dmamux() call so this 
> rollback of it wouldn't be necessary.

I would still need the mutex unlock which I believe is down path there under
some other label. Hence you are proposing something like

	mask = BIT(map->req_idx);

	mutex_lock(&dmamux->lock);
	ret = r9a06g032_sysctrl_set_dmamux(mask, val ? mask : 0);
	if (ret)
		goto err_unlock; // or whatever label is

	dmamux->used_chans |= mask;
	mutex_unlock(&dmamux->lock);

	return map;

Is that correct? If so, I don't see impediments either.

> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>


-- 
With Best Regards,
Andy Shevchenko