> -----Original Message----- > From: Menon, Nishanth > Sent: Wednesday, October 27, 2010 7:56 PM > To: G, Manjunath Kondaiah > Cc: linux-omap@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Cousson, Benoit; Kevin > Hilman; Shilimkar, Santosh > Subject: RE: [PATCH v3 01/13] OMAP: DMA: Replace read/write > macros with functions > [...] > > This approach is as per i2c driver as suggested by kevin. > > http://www.spinics.net/lists/linux-omap/msg36446.html > Thanks for pointing this out. I2c has what 18 registers while > dma has over 40 registers :( patch 11 [1] now I understand > this step -> this merges > together at later patchset - it starts to make sense now. It > becomes reg_map. Thanks - looks like a good change in the > eventual code. > > > [...] > > > > +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]); > > > > + } > > > > +} > Eventual code looks like this: > > 62 static inline void dma_write(u32 val, int reg, int lch) > 63 { > 64 if (d->dev_caps & IS_WORD_16) { > 65 if (reg >= CH_COMMON_START) > 66 __raw_writew(val, dma_base + > 67 (reg_map[reg] + 0x40 * lch)); > 68 else > 69 __raw_writew(val, dma_base + reg_map[reg]); > 70 } else { > 71 if (reg > CH_COMMON_START) > 72 __raw_writel(val, dma_base + > 73 (reg_map[reg] + 0x60 * lch)); > 74 else > 75 __raw_writel(val, dma_base + reg_map[reg]); > 76 } > 77 } > I don't really see how inline will really help here! > > > > > + > > > > +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? > > > > Detailed discussion and alignment can be found at: > > http://www.spinics.net/lists/linux-omap/thrd6.html > Better link: > http://marc.info/?t=128264802300006&r=1&w=2 > > > > > Search for: > > [PATCH v2 07/11] OMAP2/3/4: DMA: HWMOD: Device registration > > http://marc.info/?l=linux-omap&m=128464661906497&w=2 > my question is slightly different here - debate of suggestion > to use inline > is based on the size of code involved, the discussion in the thread > discussed around 3 lines of code, which made sense, > unfortunately, the > thread does not answer my question unfortunately for *this* > specific patch > - OR do you wish to point me to some specific link which answers this? for plat-omap dma code, these read/write api's can be replaced with function pointers. -Manjunath -- 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
