On 1/9/20 11:32 AM, Peter Ujfalusi wrote: > > > On 09/01/2020 11.13, Fabrice Gasnier wrote: >> On 1/7/20 12:45 PM, Peter Ujfalusi wrote: >>> dma_request_slave_channel() is a wrapper on top of dma_request_chan() >>> eating up the error code. >>> >>> By using dma_request_chan() directly the driver can support deferred >>> probing against DMA. >>> >>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx> >>> --- >>> Hi, >>> >>> Changes since v1: >>> - Fall back to IRQ mode for ADC only in case of ENODEV >> >> Hi Peter, >> >> Thanks for the patch, >> >> Please find a minor comment here after. Apart from that, you can add my: >> >> Acked-by: Fabrice Gasnier <fabrice.gasnier@xxxxxx> >> >> >>> >>> Regards, >>> Peter >>> >>> drivers/iio/adc/stm32-dfsdm-adc.c | 21 +++++++++++++++++---- >>> 1 file changed, 17 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c >>> index e493242c266e..74a2211bdff4 100644 >>> --- a/drivers/iio/adc/stm32-dfsdm-adc.c >>> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c >>> @@ -1383,9 +1383,13 @@ static int stm32_dfsdm_dma_request(struct iio_dev *indio_dev) >>> { >>> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev); >>> >>> - adc->dma_chan = dma_request_slave_channel(&indio_dev->dev, "rx"); >>> - if (!adc->dma_chan) >>> - return -EINVAL; >>> + adc->dma_chan = dma_request_chan(&indio_dev->dev, "rx"); >>> + if (IS_ERR(adc->dma_chan)) { >>> + int ret = PTR_ERR(adc->dma_chan); >>> + >>> + adc->dma_chan = NULL; >>> + return ret; >> >> You may "return PTR_ERR(adc->dma_chan);" directly here. > > I don't make decision here on behalf of the adc path on to go forward w/ > or w/o DMA support and if we go ahead the stm32_dfsdm_dma_release() > needs the dma_chan to be NULL in case we don't use DMA. > > It is much cleaner to set dma_chan to NULL here than doing it in other > paths. Hi Peter, Sorry I wasn't clear enough. I agree with you. I was suggesting only, talking about the 'ret' variable. It may be removed to spare a few lines :-). if (IS_ERR(adc->dma_chan)) { adc->dma_chan = NULL; return PTR_ERR(adc->dma_chan); } I'm okay both ways. > >> >> Best Regards, >> Fabrice >> >>> + } >>> >>> adc->rx_buf = dma_alloc_coherent(adc->dma_chan->device->dev, >>> DFSDM_DMA_BUFFER_SIZE, >>> @@ -1509,7 +1513,16 @@ static int stm32_dfsdm_adc_init(struct iio_dev *indio_dev) >>> init_completion(&adc->completion); >>> >>> /* Optionally request DMA */ >>> - if (stm32_dfsdm_dma_request(indio_dev)) { >>> + ret = stm32_dfsdm_dma_request(indio_dev); >>> + if (ret) { >>> + if (ret != -ENODEV) { >>> + if (ret != -EPROBE_DEFER) >>> + dev_err(&indio_dev->dev, >>> + "DMA channel request failed with %d\n", >>> + ret); >>> + return ret; >>> + } >>> + >>> dev_dbg(&indio_dev->dev, "No DMA support\n"); >>> return 0; >>> } >>> > > - Péter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >