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@xxxxxxxxxxxxxxx> -- With Best Regards, Andy Shevchenko
