[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1ffce826-b94a-5b9f-ffb4-ba2d0324b246@ti.com>
Date: Wed, 5 Oct 2016 13:47:55 +0530
From: Mugunthan V N <mugunthanvnm@...com>
To: Peter Ujfalusi <peter.ujfalusi@...com>, <linux-iio@...r.kernel.org>
CC: Tony Lindgren <tony@...mide.com>, Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Russell King <linux@...linux.org.uk>,
Jonathan Cameron <jic23@...nel.org>,
Hartmut Knaack <knaack.h@....de>,
Lars-Peter Clausen <lars@...afoo.de>,
Peter Meerwald-Stadler <pmeerw@...erw.net>,
Lee Jones <lee.jones@...aro.org>, Vignesh R <vigneshr@...com>,
"Andrew F . Davis" <afd@...com>, <linux-omap@...r.kernel.org>,
<devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, Sekhar Nori <nsekhar@...com>,
John Syne <john3909@...il.com>
Subject: Re: [PATCH v2 2/4] drivers: iio: ti_am335x_adc: add dma support
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
Powered by blists - more mailing lists