linux-kernel - Re: [PATCH v2 2/4] drivers: iio: ti_am335x

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d059db2e-c5fc-f1a9-47ad-0ffbb0c4f86b@ti.com>
Date:   Wed, 5 Oct 2016 14:22:38 +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 02:16 PM, Peter Ujfalusi wrote:
> On 10/05/16 11:17, Mugunthan V N wrote:
>> 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))*?
> 
> Oh, sorry. Your version was fine as you NULL the dma->chan before the return.
> 
>>
>>>
>>>> +	}
>>>>
>>>> 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?
> 
> Yes, exactly. If we fail to get the DMA channel we should not use the DMA and
> we only want to handle the deferred probing.
> 

Okay, will update accordingly in next version.

Regards
Mugunthan V N