Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v1] dmaengine: tegra-apb: Error out if DMA_PREP_INTERRUPT
 flag is unset
To:     Jon Hunter <jonathanh@nvidia.com>,
        Laxman Dewangan <ldewangan@nvidia.com>,
        Vinod Koul <vkoul@kernel.org>,
        Thierry Reding <thierry.reding@gmail.com>,
        Ben Dooks <ben.dooks@codethink.co.uk>
Cc:     dmaengine@vger.kernel.org, linux-tegra@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20190529214355.15339-1-digetx@gmail.com>
 <9b0e0d20-6386-a38a-1347-4264d249cb44@nvidia.com>
From:   Dmitry Osipenko <digetx@gmail.com>
Message-ID: <359abcbb-b3bd-ad32-9f35-15bb9b25f3b0@gmail.com>
Date:   Thu, 30 May 2019 20:32:17 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <9b0e0d20-6386-a38a-1347-4264d249cb44@nvidia.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

30.05.2019 15:01, Jon Hunter пишет:
> 
> On 29/05/2019 22:43, Dmitry Osipenko wrote:
>> Apparently driver was never tested with DMA_PREP_INTERRUPT flag being
>> unset since it completely disables interrupt handling instead of skipping
>> the callbacks invocations, hence putting channel into unusable state.
>>
>> The flag is always set by all of kernel drivers that use APB DMA, so let's
>> error out in otherwise case for consistency. It won't be difficult to
>> support that case properly if ever will be needed.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/dma/tegra20-apb-dma.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>> index cf462b1abc0b..2c84a660ba36 100644
>> --- a/drivers/dma/tegra20-apb-dma.c
>> +++ b/drivers/dma/tegra20-apb-dma.c
>> @@ -988,8 +988,12 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg(
>>  		csr |= tdc->slave_id << TEGRA_APBDMA_CSR_REQ_SEL_SHIFT;
>>  	}
>>  
>> -	if (flags & DMA_PREP_INTERRUPT)
>> +	if (flags & DMA_PREP_INTERRUPT) {
>>  		csr |= TEGRA_APBDMA_CSR_IE_EOC;
>> +	} else {
>> +		WARN_ON_ONCE(1);
>> +		return NULL;
>> +	}
>>  
>>  	apb_seq |= TEGRA_APBDMA_APBSEQ_WRAP_WORD_1;
>>  
>> @@ -1131,8 +1135,12 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_dma_cyclic(
>>  		csr |= tdc->slave_id << TEGRA_APBDMA_CSR_REQ_SEL_SHIFT;
>>  	}
>>  
>> -	if (flags & DMA_PREP_INTERRUPT)
>> +	if (flags & DMA_PREP_INTERRUPT) {
>>  		csr |= TEGRA_APBDMA_CSR_IE_EOC;
>> +	} else {
>> +		WARN_ON_ONCE(1);
>> +		return NULL;
>> +	}
>>  
>>  	apb_seq |= TEGRA_APBDMA_APBSEQ_WRAP_WORD_1;
> 
> Looks good to me.
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> 
> Cheers
> Jon
> 

Thanks