From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mugunthan V N Subject: Re: [PATCH v2 2/4] drivers: iio: ti_am335x_adc: add dma support Date: Wed, 5 Oct 2016 13:47:55 +0530 Message-ID: <1ffce826-b94a-5b9f-ffb4-ba2d0324b246@ti.com> References: <20161003130318.12591-1-mugunthanvnm@ti.com> <20161003130318.12591-3-mugunthanvnm@ti.com> <0ff2c6ef-3fa1-dc8b-fb6e-1a4dcfea8ea9@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-iio-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Peter Ujfalusi , linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 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" , linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sekhar Nori , John Syne List-Id: linux-omap@vger.kernel.org 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