Re: [PATCH v3 01/13] OMAP: DMA: Replace read/write macros with functions

[Kevin Hilman <khilman@deeprootsystems.com>]

"G, Manjunath Kondaiah" <manjugk@ti.com> writes:

> The low level read/write macros are replaced with static inline functions
> and register offsets are handled through static register offset tables
> mapped through enumeration constants.
>
> The objective of this patch is to prepare for omap dma driver cleanup
> and dma hwmod implementation. The code optimization and moving machine
> specific code to respective mach-omapx dma file will be handled in the
> rest of this patch series.

[...]

> -#define dma_write(val, reg)						\
> -({									\
> -	if (cpu_class_is_omap1())					\
> -		__raw_writew((u16)(val), omap_dma_base + OMAP1_DMA_##reg); \
> -	else								\
> -		__raw_writel((val), omap_dma_base + OMAP_DMA4_##reg);	\
> -})
> +static inline void dma_write(u32 val, int reg, int lch)
> +{
> +	if (cpu_class_is_omap1()) {

Reminder: any use of cpu_is_* checks outside of init code will be NAK'd.

cpu_is_* check do not belong at runtime, especially in a crucial path
like this.  

> +		if (reg > OMAP1_CH_COMMON_START)
> +			__raw_writew(val, dma_base +
> +				(reg_map_omap1[reg] + 0x40 * lch));
> +		else
> +			__raw_writew(val, dma_base + reg_map_omap1[reg]);
> +	} else {
> +		if (reg > OMAP2_CH_COMMON_START)
> +			__raw_writel(val, dma_base +
> +				(reg_map_omap2[reg] + 0x60 * lch));
> +		else
> +			__raw_writel(val, dma_base + reg_map_omap2[reg]);
> +	}
> +}

The register base offset, register array and the stride (offset between
instances: 0x40 or 0x60) are detectable at init time, and there's no
reason to have conditional code for them.  IOW, they should be init time
constants.  Doing so would greatly simply these functions.  In fact the
CH_COMMON_START could also be an init time constant as well.  So, given
the following init_time constants: dma_ch_common_start, dma_stride,
reg_map, the above would be reduced to something actually worth
inlining, for example (not actually tested):

static inline void dma_write(u32 val, int reg, int lch)
{
        u8 stride = (reg > dma_ch_common_start) ? dma_stride : 0;

        __raw_writel(val, dma_base + (reg_map[reg] + stride * lch));
}

Same applies to dma_read().

Kevin