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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html