Hi Andy, andriy.shevchenko@xxxxxxxxxxxxxxx wrote on Tue, 5 Apr 2022 17:50:57 +0300: > On Tue, Apr 05, 2022 at 10:19:07AM +0200, 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. > > ... > > > dw_dmac-y := platform.o > > dw_dmac-$(CONFIG_OF) += of.o > > > +obj-$(CONFIG_RZN1_DMAMUX) += rzn1-dmamux.o > > I would move it down in the file to distinguish more generic drivers > from specific quirks. Ok. > > > obj-$(CONFIG_DW_DMAC_PCI) += dw_dmac_pci.o > > dw_dmac_pci-y := pci.o > > ... > > > +#define RZN1_DMAMUX_SPLIT 16 > > I would name it more explicitly: > > #define RZN1_DMAMUX_SPLIT_1_0 16 I am sorry but I don't understand this suffix, which probably means that it is not as clear as we wish. Do you mind if I stick to RZN1_DMAMUX_SPLIT? > ... > > > + dmac_idx = map->req_idx < RZN1_DMAMUX_SPLIT ? 0 : 1; > > ...and hence use positive conditional: > > dmac_idx = map->req_idx >= RZN1_DMAMUX_SPLIT_1_0 ? 1 : 0; I will. > > With above addressed > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Thanks, Miquèl