From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752222AbcFGGKa (ORCPT ); Tue, 7 Jun 2016 02:10:30 -0400 Received: from mga11.intel.com ([192.55.52.93]:36372 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751926AbcFGGK2 (ORCPT ); Tue, 7 Jun 2016 02:10:28 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,432,1459839600"; d="scan'208";a="823091206" Date: Tue, 7 Jun 2016 11:47:01 +0530 From: Vinod Koul To: Kedareswara rao Appana Cc: dan.j.williams@intel.com, michal.simek@xilinx.com, soren.brinkmann@xilinx.com, appanad@xilinx.com, moritz.fischer@ettus.com, laurent.pinchart@ideasonboard.com, luis@debethencourt.com, punnaia@xilinx.com, dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 3/4] dmaengine: vdma: Add 64 bit addressing support for the axi dma Message-ID: <20160607061701.GG16910@localhost> References: <1463557653-1687-1-git-send-email-appanad@xilinx.com> <1463557653-1687-4-git-send-email-appanad@xilinx.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1463557653-1687-4-git-send-email-appanad@xilinx.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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