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

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

Hi Andy,

andriy.shevchenko@linux.intel.com 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@linux.intel.com>
> 


Thanks,
Miquèl