Subject: Re: [PATCH v2 2/4] drivers: iio: ti_am335x_adc: add dma support
To: Mugunthan V N <mugunthanvnm@ti.com>, <linux-iio@vger.kernel.org>
References: <20161003130318.12591-1-mugunthanvnm@ti.com>
 <20161003130318.12591-3-mugunthanvnm@ti.com>
 <0ff2c6ef-3fa1-dc8b-fb6e-1a4dcfea8ea9@ti.com>
 <e9c3108e-ffd1-472a-a2d3-2948665de217@ti.com>
CC: Tony Lindgren <tony@atomide.com>, Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Russell King <linux@armlinux.org.uk>,
        Jonathan Cameron <jic23@kernel.org>, Hartmut Knaack <knaack.h@gmx.de>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
        Lee Jones <lee.jones@linaro.org>, Vignesh R <vigneshr@ti.com>,
        "Andrew F . Davis" <afd@ti.com>, <linux-omap@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, Sekhar Nori <nsekhar@ti.com>,
        John Syne <john3909@gmail.com>
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
Message-ID: <de6bbf1c-e9e2-83b1-73c2-49aa64cc939c@ti.com>
Date: Wed, 5 Oct 2016 09:31:23 +0300
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: <e9c3108e-ffd1-472a-a2d3-2948665de217@ti.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2194
Lines: 79

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);

> +	}
> 
> 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;


-- 
Péter