G, Manjunath Kondaiah had written, on 10/26/2010 08:25 AM, the following:
[...]
diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index f5c5b8d..77241e2 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -40,6 +40,147 @@
#undef DEBUG
+enum {
+ GCR1 = 0, GSCR, GRST, HW_ID,
+ PCH2_ID, PCH0_ID, PCH1_ID, PCHG_ID,
+ PCHD_ID, CAPS1_0_U, CAPS1_0_L, CAPS1_1_U,
+ CAPS1_1_L, CAPS1_2, CAPS1_3, CAPS1_4,
+ PCH2_SR, PCH0_SR, PCH1_SR, PCHD_SR,
[...]
+};
+
+static u16 reg_map_omap1[] = {
+ [GCR1] = 0x400,
+ [GSCR] = 0x404,
+ [GRST] = 0x408,
+ [HW_ID] = 0x442,
+ [PCH2_ID] = 0x444,
+ [PCH0_ID] = 0x446,
+ [PCH1_ID] = 0x448,
+ [PCHG_ID] = 0x44a,
+ [PCHD_ID] = 0x44c,
[...]
+ [LCH_CTRL] = 0x2a,
+};
+
+enum {
+ REVISION = 0, GCR2, IRQSTATUS_L0, IRQSTATUS_L1,
+ IRQSTATUS_L2, IRQSTATUS_L3, IRQENABLE_L0, IRQENABLE_L1,
[...]
+
+ /* OMAP4 specific registers */
+ CDP, CNDP, CCDN,
+
+ OMAP2_CH_COMMON_END,
+};
+
+static u16 reg_map_omap2[] = {
+ [REVISION] = 0x00,
+ [GCR2] = 0x78,
+ [IRQSTATUS_L0] = 0x08,
+ [IRQSTATUS_L1] = 0x0c,
[..]
+ /* OMAP4 specific registers */
+ [CDP] = 0xd0,
+ [CNDP] = 0xd4,
+ [CCDN] = 0xd8,
+};
+
dumb question: any reason why a struct wont do?
struct reg_map_omap2 {
u16 revision;
...
...
}
[..]
-#define dma_read(reg) \
-({ \
- u32 __val; \
- if (cpu_class_is_omap1()) \
- __val = __raw_readw(omap_dma_base + OMAP1_DMA_##reg); \
- else \
- __val = __raw_readl(omap_dma_base + OMAP_DMA4_##reg); \
- __val; \
-})
-
-#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()) {
+ 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]);
+ }
+}
+
+static inline u32 dma_read(int reg, int lch)
+{
+ u32 val;
+
+ if (cpu_class_is_omap1()) {
+ if (reg > OMAP1_CH_COMMON_START)
+ val = __raw_readw(dma_base +
+ (reg_map_omap1[reg] + 0x40 * lch));
+ else
+ val = __raw_readw(dma_base + reg_map_omap1[reg]);
+ } else {
+ if (reg > OMAP2_CH_COMMON_START)
+ val = __raw_readl(dma_base +
+ (reg_map_omap2[reg] + 0x60 * lch));
+ else
+ val = __raw_readl(dma_base + reg_map_omap2[reg]);
+ }
+ return val;
+}
What is the benefit of using inline function here? would'nt we increase
code size? cant we use a function pointer initialized to class1 or rest?
Quote from CodingStyle (15):
"A reasonable rule of thumb is to not put inline at functions that have more
than 3 lines of code in them. An exception to this rule are the cases where
a parameter is known to be a compiletime constant, and as a result of this
constantness you *know* the compiler will be able to optimize most of your
function away at compile time. For a good example of this later case, see
the kmalloc() inline function.
"
is the expectation that cpu_class_is_omap1 compile time constant hence
the actual compiled in code is smaller -candidate for inline?
#ifdef CONFIG_ARCH_OMAP15XX
/* Returns 1 if the DMA module is in OMAP1510-compatible mode, 0 otherwise */
@@ -209,11 +369,10 @@ static inline void set_gdma_dev(int req, int dev)
/* Omap1 only */
static void clear_lch_regs(int lch)
{
- int i;
- void __iomem *lch_base = omap_dma_base + OMAP1_DMA_CH_BASE(lch);
+ int i = OMAP1_CH_COMMON_START;
- for (i = 0; i < 0x2c; i += 2)
- __raw_writew(0, lch_base + i);
+ for (; i <= OMAP1_CH_COMMON_END; i += 1)
+ dma_write(0, i, lch);
}
void omap_set_dma_priority(int lch, int dst_port, int priority)
@@ -248,12 +407,12 @@ void omap_set_dma_priority(int lch, int dst_port, int priority)
if (cpu_class_is_omap2()) {
u32 ccr;
- ccr = dma_read(CCR(lch));
+ ccr = dma_read(CCR2, lch);
if (priority)
ccr |= (1 << 6);
else
ccr &= ~(1 << 6);
- dma_write(ccr, CCR(lch));
+ dma_write(ccr, CCR2, lch);
}
}
EXPORT_SYMBOL(omap_set_dma_priority);
@@ -264,31 +423,36 @@ void omap_set_dma_transfer_params(int lch, int data_type, int elem_count,
{
u32 l;
- l = dma_read(CSDP(lch));
- l &= ~0x03;
- l |= data_type;
- dma_write(l, CSDP(lch));
-
if (cpu_class_is_omap1()) {
u16 ccr;
- ccr = dma_read(CCR(lch));
+ l = dma_read(CSDP1, lch);
+ l &= ~0x03;
+ l |= data_type;
+ dma_write(l, CSDP1, lch);
+
+ ccr = dma_read(CCR1, lch);
ccr &= ~(1 << 5);
if (sync_mode == OMAP_DMA_SYNC_FRAME)
ccr |= 1 << 5;
- dma_write(ccr, CCR(lch));
+ dma_write(ccr, CCR1, lch);
- ccr = dma_read(CCR2(lch));
+ ccr = dma_read(CCR1_2, lch);
ccr &= ~(1 << 2);
if (sync_mode == OMAP_DMA_SYNC_BLOCK)
ccr |= 1 << 2;
- dma_write(ccr, CCR2(lch));
+ dma_write(ccr, CCR1_2, lch);
}
if (cpu_class_is_omap2() && dma_trigger) {
u32 val;
- val = dma_read(CCR(lch));
+ l = dma_read(CSDP2, lch);
+ l &= ~0x03;
+ l |= data_type;
+ dma_write(l, CSDP2, lch);
This seems to me to be a replication mess :( compared to the original
CSDP(lch) - which mapped to class1 or class2 as needed - same elsewhere.
Code is becoming pretty much needing a refactoring as a result :(
[...]
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
