Hi Ilpo, Andy, ilpo.jarvinen@xxxxxxxxxxxxxxx wrote on Fri, 8 Apr 2022 15:38:48 +0300 (EEST): > On Fri, 8 Apr 2022, Andy Shevchenko wrote: > > > 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. > > Yes, and yes, the mutex still has to be unlocked on that error path. > > > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> I've done the modification, thanks for your feedback. Thanks, Miquèl
