Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754536AbcJEIS7 (ORCPT ); Wed, 5 Oct 2016 04:18:59 -0400 Received: from bear.ext.ti.com ([198.47.19.11]:53163 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751980AbcJEIS4 (ORCPT ); Wed, 5 Oct 2016 04:18:56 -0400 Subject: Re: [PATCH v2 2/4] drivers: iio: ti_am335x_adc: add dma support To: Peter Ujfalusi , References: <20161003130318.12591-1-mugunthanvnm@ti.com> <20161003130318.12591-3-mugunthanvnm@ti.com> <0ff2c6ef-3fa1-dc8b-fb6e-1a4dcfea8ea9@ti.com> CC: Tony Lindgren , Rob Herring , Mark Rutland , Russell King , Jonathan Cameron , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Lee Jones , Vignesh R , "Andrew F . Davis" , , , , , Sekhar Nori , John Syne From: Mugunthan V N Message-ID: <1ffce826-b94a-5b9f-ffb4-ba2d0324b246@ti.com> Date: Wed, 5 Oct 2016 13:47:55 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2523 Lines: 85 On Wednesday 05 October 2016 12:01 PM, Peter Ujfalusi wrote: > On 10/05/16 09:21, Mugunthan V N wrote: >> On Tuesday 04 October 2016 02:02 PM, Peter Ujfalusi wrote: >>> On 10/03/16 16:03, Mugunthan V N wrote: >>>> +static int tiadc_request_dma(struct platform_device *pdev, >>>> + struct tiadc_device *adc_dev) >>>> +{ >>>> + struct tiadc_dma *dma = &adc_dev->dma; >>>> + dma_cap_mask_t mask; >>>> + >>>> + /* Default slave configuration parameters */ >>>> + dma->conf.direction = DMA_DEV_TO_MEM; >>>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; >>>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1; >>>> + >>>> + dma_cap_zero(mask); >>>> + dma_cap_set(DMA_CYCLIC, mask); >>>> + >>>> + /* Get a channel for RX */ >>>> + dma->chan = dma_request_chan(adc_dev->mfd_tscadc->dev, "fifo1"); >>>> + if (!dma->chan) >>>> + return -ENODEV; >>> >>> dma_request_chan() ERR_PTR in case of failure, never NULL. You should reuse >>> the returned error code to support deferred probing. >> >> Will fix this in v3. >> >>> >>>> + >>>> + /* RX buffer */ >>>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE, >>>> + &dma->addr, GFP_KERNEL); >>>> + if (!dma->buf) >>>> + goto err; >>>> + >>>> + return 0; >>>> +err: >>>> + dma_release_channel(dma->chan); >>>> + >>>> + return -ENOMEM; >>>> +} >>>> + >>>> static int tiadc_parse_dt(struct platform_device *pdev, >>>> struct tiadc_device *adc_dev) >>>> { >>>> @@ -512,8 +639,14 @@ static int tiadc_probe(struct platform_device *pdev) >>>> >>>> platform_set_drvdata(pdev, indio_dev); >>>> >>>> + err = tiadc_request_dma(pdev, adc_dev); >>>> + if (err && err != -ENODEV) >>>> + goto err_dma; >>> >>> You should handle the deferred probing for DMA channel. >> >> + dma->chan = dma_request_chan(adc_dev->mfd_tscadc->dev, "fifo1"); >> + if (IS_ERR(dma->chan)) { >> + int ret = PTR_ERR(dma->chan); >> + >> + dma->chan = NULL; >> + return ret; > > You don't need the 'ret' variable: > return PTR_ERR(dma->chan); So you mean change all *if (dma->chan)* to *if (IS_ERR(dma->chan))*? > >> + } >> >> With this probe defer will be taken care and ADC will continue without >> DMA when request channel returns -ENODEV. > > I would rather have explicit check for deferred probe: > > err = tiadc_request_dma(pdev, adc_dev); > if (err && err == -EPROBE_DEFER) > goto err_dma; But in this case any other failure other than -EPROBE_DEFER will be masked and ADC will be in PIO mode? Regards Mugunthan V N