Subject: Re: drivers/clocksource/timer-pistachio.c:74:22: warning: variable
 'overflow' set but not used
To:     Drew Fustini <drew@beagleboard.org>
Cc:     kernel test robot <lkp@intel.com>, kbuild-all@lists.01.org,
        linux-kernel@vger.kernel.org,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>
References: <202111120738.oAGjK5L8-lkp@intel.com> <20211114212938.GA317441@x1>
 <552daf37-c5eb-6634-7456-3fa39c74ebcb@linaro.org>
 <20211115010253.GA325926@x1>
From:   Daniel Lezcano <daniel.lezcano@linaro.org>
Message-ID: <e2632d73-8bf2-52d9-e7a0-e7fec21290e6@linaro.org>
Date:   Mon, 15 Nov 2021 13:22:06 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <20211115010253.GA325926@x1>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 15/11/2021 02:02, Drew Fustini wrote:
> On Sun, Nov 14, 2021 at 11:17:38PM +0100, Daniel Lezcano wrote:
>> On 14/11/2021 22:29, Drew Fustini wrote:
>>> On Fri, Nov 12, 2021 at 07:05:48AM +0800, kernel test robot wrote:
>>>> Hi Drew,
>>>>
>>>> FYI, the error/warning still remains.
>>>>
>>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>>>> head:   debe436e77c72fcee804fb867f275e6d31aa999c
>>>> commit: a47d7ef4550d08fb428ea4c3f1a9c71674212208 clocksource/drivers/pistachio: Fix trivial typo
>>>> date:   7 months ago
>>>> config: mips-randconfig-r012-20210927 (attached as .config)
>>>> compiler: mips-linux-gcc (GCC) 11.2.0
>>>> reproduce (this is a W=1 build):
>>>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>>         chmod +x ~/bin/make.cross
>>>>         # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a47d7ef4550d08fb428ea4c3f1a9c71674212208
>>>>         git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>>>         git fetch --no-tags linus master
>>>>         git checkout a47d7ef4550d08fb428ea4c3f1a9c71674212208
>>>>         # save the attached .config to linux build tree
>>>>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=mips 
>>>>
>>>> If you fix the issue, kindly add following tag as appropriate
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>>    drivers/clocksource/timer-pistachio.c: In function 'pistachio_clocksource_read_cycles':
>>>>>> drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used [-Wunused-but-set-variable]
>>>>       74 |         u32 counter, overflow;
>>>>          |                      ^~~~~~~~
>>>>
>>>>
>>>> vim +/overflow +74 drivers/clocksource/timer-pistachio.c
>>>>
>>>>     69	
>>>>     70	static u64 notrace
>>>>     71	pistachio_clocksource_read_cycles(struct clocksource *cs)
>>>>     72	{
>>>>     73		struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs);
>>>>   > 74		u32 counter, overflow;
>>>>     75		unsigned long flags;
>>>>     76	
>>>>     77		/*
>>>>     78		 * The counter value is only refreshed after the overflow value is read.
>>>>     79		 * And they must be read in strict order, hence raw spin lock added.
>>>>     80		 */
>>>>     81	
>>>>     82		raw_spin_lock_irqsave(&pcs->lock, flags);
>>>>     83		overflow = gpt_readl(pcs->base, TIMER_CURRENT_OVERFLOW_VALUE, 0);
>>
>> overflow is set here but then never reused in the function. So a call to
>> gpt_readl without getting the return value is fine.
>>
>> The warning is different from 'variable is unused'
> 
> Thank you for pointing that out.  I was reading it incorrectly.
> 
> As for this function, it seems that warning is a false positive as
> the act of reading TIMER_CURRENT_OVERFLOW_VALUE causes the hardware
> to refresh TIMER_CURRENT_VALUE.

The warning is actually not a false positive. It just state the variable
is unused after setting it which is actually correct.

The indirect warning is about the behavior of the hardware where the
counter is refreshed only when the overflow is read.

May be adding the may_be_unused annotation to the overflow variable can
silent the warning?


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog