linux-kernel - Re: [PATCH v3 6/6] spi: tegra210-quad: Add support for internal DMA

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <05e4997c-631f-4906-9422-5af257c153e8@nvidia.com>
Date: Wed, 16 Apr 2025 15:08:17 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>,
 Vishwaroop A <va@...dia.com>, thierry.reding@...il.com,
 skomatineni@...dia.com, ldewangan@...dia.com, broonie@...nel.org,
 linux-spi@...r.kernel.org, linux-tegra@...r.kernel.org,
 linux-kernel@...r.kernel.org, kyarlagadda@...dia.com, smangipudi@...dia.com
Subject: Re: [PATCH v3 6/6] spi: tegra210-quad: Add support for internal DMA


On 16/04/2025 12:57, Mukesh Kumar Savaliya wrote:

...

>> @@ -1384,41 +1419,43 @@ static irqreturn_t 
>> handle_dma_based_xfer(struct tegra_qspi *tqspi)
>>       unsigned int total_fifo_words;
>>       unsigned long flags;
>>       long wait_status;
>> -    int err = 0;
>> +    int num_errors = 0;
>>       if (tqspi->cur_direction & DATA_DIR_TX) {
>>           if (tqspi->tx_status) {
>> -            dmaengine_terminate_all(tqspi->tx_dma_chan);
>> -            err += 1;
>> -        } else {
>> +            if (tqspi->tx_dma_chan)
>> +                dmaengine_terminate_all(tqspi->tx_dma_chan);
>> +            num_errors++;
>> +        } else if (tqspi->tx_dma_chan) {
>>               wait_status = wait_for_completion_interruptible_timeout(
>>                   &tqspi->tx_dma_complete, QSPI_DMA_TIMEOUT);
>>               if (wait_status <= 0) {
>>                   dmaengine_terminate_all(tqspi->tx_dma_chan);
>>                   dev_err(tqspi->dev, "failed TX DMA transfer\n");
>> -                err += 1;
>> +                num_errors++;
> what do you do with incrementing this local num_errors ?
> I don't see any post processing .
> Also all are are mutually exclusive due to if/else, not sequential. So 
> could not get any specific usage.

This is really legacy code and yes I am not too thrilled with it either. 
At the same time this change is simply making the variable name a bit 
more clear. It is not critical that this is changed as part of this 
patch, but does not hurt either. I don't have strong feelings on this.

Jon

-- 
nvpublic