Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: drivers/dma/pl330.c:2981:9: warning: Identical condition 'ret',
 second condition is always false
To:     Vinod Koul <vkoul@kernel.org>, kernel test robot <lkp@intel.com>,
        Sugar Zhang <sugar.zhang@rock-chips.com>
Cc:     Ulf Hansson <ulf.hansson@linaro.org>, kbuild-all@lists.01.org,
        linux-kernel@vger.kernel.org
From:   Marek Szyprowski <m.szyprowski@samsung.com>
Message-ID: <9e7612e8-41a5-460d-5492-12c5ef81b914@samsung.com>
Date:   Mon, 24 Aug 2020 15:50:06 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
        Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <20200808125940.GR12965@vkoul-mobl>
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <202008081010.0bKKNfQp%lkp@intel.com>
        <CGME20200808125949eucas1p2d02178719f81ee811aae13097d382670@eucas1p2.samsung.com>
        <20200808125940.GR12965@vkoul-mobl>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Vinod,

On 08.08.2020 14:59, Vinod Koul wrote:
> On 08-08-20, 10:53, kernel test robot wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> head:   30185b69a2d533c4ba6ca926b8390ce7de495e29
>> commit: a39cddc9e3775100100a4272feed64faac831be9 dmaengine: pl330: Drop boilerplate code for suspend/resume
>> date:   8 months ago
>> compiler: aarch64-linux-gcc (GCC) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>>
>> cppcheck warnings: (new ones prefixed by >>)
>>
>>>> drivers/dma/pl330.c:2981:9: warning: Identical condition 'ret', second condition is always false [identicalConditionAfterEarlyExit]
>>      return ret;
>>             ^
>>     drivers/dma/pl330.c:2976:6: note: first condition
>>      if (ret)
>>          ^
>>     drivers/dma/pl330.c:2981:9: note: second condition
>>      return ret;
> This one could be fixed by making this as return 0, but is harmless

Right, with CONFIG_PM disabled, pm_runtime_force_resume() is noop, what 
causes the above warning.

>>             ^
>>>> drivers/dma/pl330.c:2798:23: warning: Variable 'desc->rqcfg.brst_len' is reassigned a value before the old one has been used. [redundantAssignment]
>>      desc->rqcfg.brst_len = get_burst_len(desc, len);
>>                           ^
>>     drivers/dma/pl330.c:2796:24: note: Variable 'desc->rqcfg.brst_len' is reassigned a value before the old one has been used.
>>       desc->rqcfg.brst_len = 1;
>>                            ^
>>     drivers/dma/pl330.c:2798:23: note: Variable 'desc->rqcfg.brst_len' is reassigned a value before the old one has been used.
>>      desc->rqcfg.brst_len = get_burst_len(desc, len);
> This one actually seems like a bug. Reading the code I think
> get_burst_len() should be called first before checking if burst size is
> smaller and setting to 1 in that case
>
> Sugar Zhang, Marek Szyprowski can you folks check this?

Indeed. It look that the commit 137bd11090d89b added 
desc->rqcfg.brst_len = 1 assignment before the desc->rqcfg.brst_len is 
set. Maybe this was a result of the broken rebase or so. No idea. It 
makes sense to switch the order and call desc->rqcfg.brst_len = 
get_burst_len(desc, len) first. I can send a patch if you want.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland