Re: [PATCH v2 2/9] ARM: shmobile: r8a7790: add dma defines for sys and audio dmacs

[Simon Horman <horms@verge.net.au>]

On Tue, Apr 15, 2014 at 12:10:00AM +0200, Arnd Bergmann wrote:
> On Monday 14 April 2014 22:35:05 Ben Dooks wrote:
> > Add the DMA resource IDs for the R8A7790 Audio and SYS DMA controllers
> > for use when specifying DMA handles.
> > 
> > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> 
> I really hate these lists in bindings:
> 
> > +#define R8A7790_DMA_SCIFA0_TX	(0x21)
> > +#define R8A7790_DMA_SCIFA0_RX	(0x22)
> > +#define R8A7790_DMA_SCIFA1_TX	(0x25)
> > +#define R8A7790_DMA_SCIFA1_RX	(0x26)
> > +#define R8A7790_DMA_SCIFA2_TX	(0x27)
> > +#define R8A7790_DMA_SCIFA2_RX	(0x28)
> > +
> > +#define R8A7790_DMA_SCIFB0_TX	(0x3D)
> > +#define R8A7790_DMA_SCIFB0_RX	(0x3E)
> > +#define R8A7790_DMA_SCIFB1_TX	(0x19)
> > +#define R8A7790_DMA_SCIFB1_RX	(0x1A)
> > +#define R8A7790_DMA_SCIFB2_TX	(0x1D)
> > +#define R8A7790_DMA_SCIFB2_RX	(0x1E)
> 
> These are all hardware constants, they should come from the data sheet and
> get put into the dtsi file. The driver doesn't care what they are, and
> the binding doesn't care, this only serves to make the binding less
> generic.

They serve to make the dts file significantly more readable and
give some small level of compile-time checking to the dts file.

I am entirely in favour of them and its the direction that we
have been moving in for shmobile bindings.

> > +#define CHCR_RX_32BIT	SHDMA_ARM_CHCR_RX(SHDMA_ARM_SZ_32BIT)
> > +#define CHCR_TX_32BIT	SHDMA_ARM_CHCR_TX(SHDMA_ARM_SZ_32BIT)
> > +#define CHCR_RX_256BIT	SHDMA_ARM_CHCR_RX(SHDMA_ARM_SZ_256BIT)
> > +#define CHCR_TX_256BIT	SHDMA_ARM_CHCR_TX(SHDMA_ARM_SZ_256BIT)
> 
> For these, it's different: There is nothing wrong putting macros like
> these into DT, because they will be the same across all SoCs.
> 
> However, as I mentioned in my reply to patch 8, I don't think that information
> belongs into DT, and it should be in the device driver instead, because
> that already knows the direction and transfer width and has to set it
> through the slave config interface anyway.
> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>