Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: rhkNJ16n39RfQQ3rVSwMRqIWWmyu0+vVxDm8gLJkIKon4vnqY/WW4CD5OddtKJ5OhtZI6SGSg9
 Iyy4FW4VW8xA==
IronPort-SDR: xbS+36t5bYkJ7JvknkIxNYX9q3vs2QSllbBiEdPv5/RRvXG9EJYADc2WkD7NnTZawAFYiDj5aQ
 6oJeC00s2jqQ==
From:   "Sia, Jee Heng" <jee.heng.sia@intel.com>
To:     Vinod Koul <vkoul@kernel.org>
CC:     "Eugeniy.Paltsev@synopsys.com" <Eugeniy.Paltsev@synopsys.com>,
        "andriy.shevchenko@linux.intel.com" 
        <andriy.shevchenko@linux.intel.com>,
        "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2 07/15] dmaegine: dw-axi-dmac: Support
 device_prep_dma_cyclic()
Thread-Topic: [PATCH v2 07/15] dmaegine: dw-axi-dmac: Support
 device_prep_dma_cyclic()
Thread-Index: AQHWtnynUP/bz1/OVU2QzISZuNMgjKnCKU2A
Date:   Wed, 11 Nov 2020 01:38:32 +0000
Message-ID: <CO1PR11MB5026FE7E33170CA8C639D204DAE80@CO1PR11MB5026.namprd11.prod.outlook.com>
References: <20201027063858.4877-1-jee.heng.sia@intel.com>
 <20201027063858.4877-8-jee.heng.sia@intel.com>
 <20201109094216.GC3171@vkoul-mobl>
In-Reply-To: <20201109094216.GC3171@vkoul-mobl>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.5.1.3
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 5ae4ae52-63c4-456a-217d-08d885e28018
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Nov 2020 01:38:32.9958
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: B/iklbGccIbOGgU8DI3/yXBkDeqyQGLdQ9oq0mKUPB1mWQlYW1+XluJDgp6Ke9QcTcFKVnebmJ4DncPTmfchjA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR11MB1806
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: 09 November 2020 5:42 PM
> To: Sia, Jee Heng <jee.heng.sia@intel.com>
> Cc: Eugeniy.Paltsev@synopsys.com; andriy.shevchenko@linux.intel.com;
> dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 07/15] dmaegine: dw-axi-dmac: Support
> device_prep_dma_cyclic()
>=20
> On 27-10-20, 14:38, Sia Jee Heng wrote:
> > Add support for device_prep_dma_cyclic() callback function to benefit
> > DMA cyclic client, for example ALSA.
> >
> > Existing AxiDMA driver only support data transfer between memory to
> memory.
> > Data transfer between device to memory and memory to device in cyclic
> > mode would failed if this interface is not supported by the AxiDMA driv=
er.
> >
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Sia Jee Heng <jee.heng.sia@intel.com>
> > ---
> >  .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 182 +++++++++++++++++-
> >  drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |   2 +
> >  2 files changed, 177 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> > b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> > index 1124c97025f2..9e574753aaf0 100644
> > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> > @@ -15,6 +15,8 @@
> >  #include <linux/err.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/io-64-nonatomic-lo-hi.h>
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> > @@ -575,6 +577,135 @@ dma_chan_prep_dma_memcpy(struct dma_chan
> *dchan, dma_addr_t dst_adr,
> >  	return NULL;
> >  }
> >
> > +static struct dma_async_tx_descriptor *
> > +dw_axi_dma_chan_prep_cyclic(struct dma_chan *dchan, dma_addr_t
> dma_addr,
> > +			    size_t buf_len, size_t period_len,
> > +			    enum dma_transfer_direction direction,
> > +			    unsigned long flags)
> > +{
> > +	struct axi_dma_chan *chan =3D dchan_to_axi_dma_chan(dchan);
> > +	u32 data_width =3D BIT(chan->chip->dw->hdata->m_data_width);
> > +	struct axi_dma_hw_desc *hw_desc =3D NULL;
> > +	struct axi_dma_desc *desc =3D NULL;
> > +	dma_addr_t src_addr =3D dma_addr;
> > +	u32 num_periods =3D buf_len / period_len;
> > +	unsigned int reg_width;
> > +	unsigned int mem_width;
> > +	dma_addr_t reg;
> > +	unsigned int i;
> > +	u32 ctllo, ctlhi;
> > +	size_t block_ts;
> > +	u64 llp =3D 0;
> > +	u8 lms =3D 0; /* Select AXI0 master for LLI fetching */
> > +
> > +	block_ts =3D chan->chip->dw->hdata->block_size[chan->id];
> > +
> > +	mem_width =3D __ffs(data_width | dma_addr | period_len);
> > +	if (mem_width > DWAXIDMAC_TRANS_WIDTH_32)
> > +		mem_width =3D DWAXIDMAC_TRANS_WIDTH_32;
> > +
> > +	desc =3D axi_desc_alloc(num_periods);
> > +	if (unlikely(!desc))
> > +		goto err_desc_get;
> > +
> > +	chan->direction =3D direction;
> > +	desc->chan =3D chan;
> > +	chan->cyclic =3D true;
> > +
> > +	switch (direction) {
> > +	case DMA_MEM_TO_DEV:
> > +		reg_width =3D __ffs(chan->config.dst_addr_width);
> > +		reg =3D chan->config.dst_addr;
> > +		ctllo =3D reg_width << CH_CTL_L_DST_WIDTH_POS |
> > +			DWAXIDMAC_CH_CTL_L_NOINC <<
> CH_CTL_L_DST_INC_POS |
> > +			DWAXIDMAC_CH_CTL_L_INC <<
> CH_CTL_L_SRC_INC_POS;
> > +		break;
> > +	case DMA_DEV_TO_MEM:
> > +		reg_width =3D __ffs(chan->config.src_addr_width);
> > +		reg =3D chan->config.src_addr;
> > +		ctllo =3D reg_width << CH_CTL_L_SRC_WIDTH_POS |
> > +			DWAXIDMAC_CH_CTL_L_INC <<
> CH_CTL_L_DST_INC_POS |
> > +			DWAXIDMAC_CH_CTL_L_NOINC <<
> CH_CTL_L_SRC_INC_POS;
> > +		break;
> > +	default:
> > +		return NULL;
> > +	}
> > +
> > +	for (i =3D 0; i < num_periods; i++) {
> > +		hw_desc =3D &desc->hw_desc[i];
> > +
> > +		hw_desc->lli =3D axi_desc_get(chan, &hw_desc->llp);
> > +		if (unlikely(!hw_desc->lli))
> > +			goto err_desc_get;
> > +
> > +		if (direction =3D=3D DMA_MEM_TO_DEV)
> > +			block_ts =3D period_len >> mem_width;
> > +		else
> > +			block_ts =3D period_len >> reg_width;
> > +
> > +		ctlhi =3D CH_CTL_H_LLI_VALID;
> > +		if (chan->chip->dw->hdata->restrict_axi_burst_len) {
> > +			u32 burst_len =3D chan->chip->dw->hdata-
> >axi_rw_burst_len;
> > +
> > +			ctlhi |=3D (CH_CTL_H_ARLEN_EN |
> > +				burst_len << CH_CTL_H_ARLEN_POS |
> > +				CH_CTL_H_AWLEN_EN |
> > +				burst_len << CH_CTL_H_AWLEN_POS);
> > +		}
> > +
> > +		hw_desc->lli->ctl_hi =3D cpu_to_le32(ctlhi);
> > +
> > +		if (direction =3D=3D DMA_MEM_TO_DEV)
> > +			ctllo |=3D mem_width << CH_CTL_L_SRC_WIDTH_POS;
> > +		else
> > +			ctllo |=3D mem_width << CH_CTL_L_DST_WIDTH_POS;
> > +
> > +		if (direction =3D=3D DMA_MEM_TO_DEV) {
> > +			write_desc_sar(hw_desc, src_addr);
> > +			write_desc_dar(hw_desc, reg);
> > +		} else {
> > +			write_desc_sar(hw_desc, reg);
> > +			write_desc_dar(hw_desc, src_addr);
> > +		}
> > +
> > +		hw_desc->lli->block_ts_lo =3D cpu_to_le32(block_ts - 1);
> > +
> > +		ctllo |=3D (DWAXIDMAC_BURST_TRANS_LEN_4 <<
> CH_CTL_L_DST_MSIZE_POS |
> > +			  DWAXIDMAC_BURST_TRANS_LEN_4 <<
> CH_CTL_L_SRC_MSIZE_POS);
> > +		hw_desc->lli->ctl_lo =3D cpu_to_le32(ctllo);
> > +
> > +		set_desc_src_master(hw_desc);
> > +
> > +		/*
> > +		 * Set end-of-link to the linked descriptor, so that cyclic
> > +		 * callback function can be triggered during interrupt.
> > +		 */
> > +		set_desc_last(hw_desc);
> > +
> > +		src_addr +=3D period_len;
> > +	}
>=20
> apart from this bit and use of periods instead of sg_list this seems very=
 similar to
> slave handler, so can you please move common bits to helpers and
> remove/reduce duplicate code
[>>] sure, will try to reduce the common code.=20
>=20
> --
> ~Vinod