Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1537430

Re: [PATCH] spi: davinci: Allow device tree devices to use DMA

From David Lechner <david@lechnology.com>
Newsgroups linux.kernel
Subject Re: [PATCH] spi: davinci: Allow device tree devices to use DMA
Date 2016-12-07 03:30 +0100
Message-ID <sLA9z-1Ws-3@gated-at.bofh.it> (permalink)
References <sF5Lb-8vM-3@gated-at.bofh.it> <sFA2D-2VT-29@gated-at.bofh.it> <sFDMS-6n8-23@gated-at.bofh.it> <sFSiS-7gi-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 11/21/2016 02:37 AM, Sekhar Nori wrote:
> On Sunday 20 November 2016 10:31 PM, David Lechner wrote:
>
>> On 11/20/2016 06:59 AM, Sekhar Nori wrote:
>
>>> On Saturday 19 November 2016 10:11 AM, David Lechner wrote:
>
>>>> @@ -400,6 +401,9 @@ static int davinci_spi_of_setup(struct spi_device
>>>> *spi)
>>>>          if (!of_property_read_u32(np, "ti,spi-wdelay", &prop))
>>>>              spicfg->wdelay = (u8)prop;
>>>>          spi->controller_data = spicfg;
>>>> +        /* Use DMA for device if master supports it */
>>>> +        if (dspi->dma_rx)
>>>
>>> This should be
>>>
>>>         if (!(IS_ERR(dpsi->dma_rx) || IS_ERR(dspi->dma_tx))
>>
>>
>> There is the following code in davinci_spi_probe():
>>
>>     ret = davinci_spi_request_dma(dspi);
>>     if (ret == -EPROBE_DEFER) {
>>         goto free_clk;
>>     } else if (ret) {
>>         dev_info(&pdev->dev, "DMA is not supported (%d)\n", ret);
>>         dspi->dma_rx = NULL;
>>         dspi->dma_tx = NULL;
>>     }
>>
>> So, I does not look like it is possible to get anything other than NULL
>> or a valid pointer for dpsi->dma_rx and that checking dpsi->dma_tx is
>> not necessary.
>>
>> So, I think if (dspi->dma_rx) is sufficient. In fact the same check is
>> used during unwinding if the probe function fails.
>
> You are right, I see it now. Setting dma_rx to NULL overriding the error
> value is confusing since dma_request_chan() itself does not use NULL as
> an error value.
>
> I think it is better to fix the existing code to remove the NULL
> overwrite and use IS_ERR() instead. You should probably wait for some
> feedback from the SPI maintainer though.
>
> Thanks,
> Sekhar
>


There don't seem to be any further comments. Using NULL here makes more 
sense to me, so I am inclined to leave this patch as-is.

Back to linux.kernel | Previous | Next | Find similar | Unroll thread


Thread

Re: [PATCH] spi: davinci: Allow device tree devices to use DMA David Lechner <david@lechnology.com> - 2016-12-07 03:30 +0100

csiph-web