On Wed, May 18, 2016 at 01:17:32PM +0530, Kedareswara rao Appana wrote: > + if (chan->cyclic) { > + if (chan->ext_addr) > + dma_writeq(chan, XILINX_DMA_REG_TAILDESC, > + chan->cyclic_seg_v->phys); > + else > + dma_ctrl_write(chan, XILINX_DMA_REG_TAILDESC, > + chan->cyclic_seg_v->phys); > + } else { > + if (chan->ext_addr) > + dma_writeq(chan, XILINX_DMA_REG_TAILDESC, > + tail_segment->phys); > + else > + dma_ctrl_write(chan, XILINX_DMA_REG_TAILDESC, > + tail_segment->phys); this looks ugly and repeated few times. Why not have xilinx_write() which does either dma_writeq or dma_ctrl_write based on channel.. > + if (chan->ext_addr) { > + hw->buf_addr = lower_32_bits(buf_addr + > + sg_used + (period_len * i)); > + hw->buf_addr_msb = upper_32_bits(buf_addr + > + sg_used + (period_len * i)); > + } else { > + hw->buf_addr = buf_addr + sg_used + > + (period_len * i); > + } similar wrapper here would make code more readable -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html