From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Armstrong Subject: Re: [RESEND,2/5] dmaengine: Add ADM driver Date: Fri, 23 Dec 2016 20:53:09 +0100 Message-ID: <585D80A5.8090801@baylibre.com> References: <1467150186-11427-3-git-send-email-twp@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return-path: Received: from mail-wj0-f174.google.com ([209.85.210.174]:35457 "EHLO mail-wj0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932305AbcLWTyb (ORCPT ); Fri, 23 Dec 2016 14:54:31 -0500 Received: by mail-wj0-f174.google.com with SMTP id v7so260073846wjy.2 for ; Fri, 23 Dec 2016 11:53:11 -0800 (PST) In-Reply-To: Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Zoran Markovic , "linux-arm-msm@vger.kernel.org" Cc: "linux@qca.qualcomm.com" , "andy.gross@linaro.org" , "twp@codeaurora.org" , "vinod.koul@intel.com" , "dan.j.williams@intel.com" , "okaya@codeaurora.org" , "andy.shevchenko@gmail.com" Le 22/12/2016 18:55, Zoran Markovic a écrit : >> Add the DMA engine driver for the QCOM Application Data Mover (ADM) DMA >> controller found in the MSM8x60 and IPQ/APQ8064 platforms. > > With minor changes I got this working on MDM9615, using qcom_nand. Below are the changes I had to make, please consider them. Patches for MDM9615 NAND are pending. > >> +static struct dma_async_tx_descriptor *adm_prep_slave_sg(struct dma_chan *chan, >> + struct scatterlist *sgl, unsigned int sg_len, >> + enum dma_transfer_direction direction, unsigned long flags, >> + void *context) >> +{ > ... > >> + /* if using flow control, validate burst and crci values */ >> + if (achan->slave.device_fc) { >> + >> + blk_size = adm_get_blksize(burst); >> + if (blk_size < 0) { >> + dev_err(adev->dev, "invalid burst value: %d\n", >> + burst); >> + return ERR_PTR(-EINVAL); > Return NULL here, most DMA clients (including qcom_nand) expect NULL if prep_slave_sg() fails. >> + } >> + >> + crci = achan->slave.slave_id & 0xf; >> + if (!crci || achan->slave.slave_id > 0x1f) { >> + dev_err(adev->dev, "invalid crci value\n"); >> + return ERR_PTR(-EINVAL); > Ditto above. >> + } >> + } >> + >> + /* iterate through sgs and compute allocation size of structures */ >> + for_each_sg(sgl, sg, sg_len, i) { >> + if (achan->slave.device_fc) { >> + box_count += DIV_ROUND_UP(sg_dma_len(sg) / burst, >> + ADM_MAX_ROWS); >> + if (sg_dma_len(sg) % burst) >> + single_count++; >> + } else { >> + single_count += DIV_ROUND_UP(sg_dma_len(sg), >> + ADM_MAX_XFER); >> + } >> + } >> + >> + async_desc = kzalloc(sizeof(*async_desc), GFP_NOWAIT); >> + if (!async_desc) >> + return ERR_PTR(-ENOMEM); > Ditto above. > >> + >> + if (crci) >> + async_desc->mux = achan->slave.slave_id & ADM_CRCI_MUX_SEL ? >> + ADM_CRCI_CTL_MUX_SEL : 0; >> + async_desc->crci = crci; >> + async_desc->blk_size = blk_size; >> + async_desc->dma_len = single_count * sizeof(struct adm_desc_hw_single) + >> + box_count * sizeof(struct adm_desc_hw_box) + >> + sizeof(*cple) + 2 * ADM_DESC_ALIGN; >> + >> + async_desc->cpl = dma_alloc_writecombine(adev->dev, async_desc->dma_len, >> + &async_desc->dma_addr, GFP_NOWAIT); > Under pressure this allocation might fail, resulting in NAND errors. I handled it using wait_event_timeout() to wait until buffers are available. Either that, or clients such as qcom_nand need to handle this failure. > >> + >> + if (!async_desc->cpl) { >> + kfree(async_desc); >> + return ERR_PTR(-ENOMEM); > Return NULL. >> + } > ... >> +} > ... > >> +static void adm_dma_free_desc(struct virt_dma_desc *vd) { >> + struct adm_async_desc *async_desc = container_of(vd, >> + struct adm_async_desc, vd); >> + >> + dma_free_writecombine(async_desc->adev->dev, async_desc->dma_len, >> + async_desc->cpl, async_desc->dma_addr); >> + kfree(async_desc); > Do wake_up() here to signal buffer availability. >> +} > > Regards, Zoran > Hi Zoran, These are also the fixes we needed to make the ADM work on MDM9615. Andy, do you plan to resend this driver ? Neil