From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Fri, 6 Dec 2013 15:23:49 +0100 Subject: [U-Boot] Please pull u-boot-ti/master In-Reply-To: <52A1D372.1010405@denx.de> References: <20131204220630.GO420@bill-the-cat> <20131206093316.432a8026@lilith> <52A1A211.3080109@denx.de> <20131206130606.1f3dcb3c@lilith> <52A1D372.1010405@denx.de> Message-ID: <20131206152349.359dc11a@lilith> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Stefan, On Fri, 06 Dec 2013 14:38:58 +0100, Stefan Roese wrote: > Hi Albert, > > On 06.12.2013 13:06, Albert ARIBAUD wrote: > >> On 06.12.2013 10:20, Yegor Yefremov wrote: > >>>> This causes am3517_evm to fail with warnings: > >>>> > >>>> am3517evm.c: In function 'misc_init_r': > >>>> am3517evm.c:106:6: warning: unused variable 'reset' [-Wunused-variable] > >>>> am3517evm.c:105:24: warning: unused variable 'ctr' [-Wunused-variable] > >>>> am3517evm.c: In function 'misc_init_r': > >>>> am3517evm.c:106:6: warning: unused variable 'reset' [-Wunused-variable] > >>>> am3517evm.c:105:24: warning: unused variable 'ctr' [-Wunused-variable] > >>>> > >>>> These are trivial, but I'd like them fixed; lingering warnings are a > >>>> pain to manage when doing large-scale test build. > >>>> > >>>> These ones come from commit ae98bbeb, that is : > >>>> > >>>>> Yegor Yefremov (1): > >>>>> am3517_evm: activate Ethernet PHY > >>>> > >>>> Yegor, can you fix this? Thanks. > >>> > >>> v2 sent. I've put those variables under if defined statement, so the > >>> warning has gone. > >> > >> In general its preferred to use the __maybe_unused statement instead of > >> adding more #ifdef's. Like this: > >> > >> + __maybe_unused volatile unsigned int ctr; > >> + __maybe_unused u32 reset; > >> > >> Care to send a v3 for this patch? > >> > >> Thanks, > >> Stefan > >> > > > > Hmm, if the variables are used under a clearly defined conditional, I > > prefer them to be defined under than conditional too. This makes > > it clearer what is affected when removing the conditional. > > I personally still prefer the __maybe_unused version. And I have seen > other U-Boot developers also using it lately more frequently. The > removed #ifdef outweighs the added __maybe_unused for my taste. I respect your preference. However, my own taste is for making explicit rather than implicit the reason why a variable is marked possibly unused, therefore I would prefer the declarations to be under the same conditional as their use. Note however that in case where one cannot readily see the conditions under which the variable is unused (e.g. if the variable is an argument to a macro which may or may not use it), I'm ok with using the attribute 'maybe_unused'. Note also that in the case at hand, a solution may be to declare the variables inside a block under the existing conditional; that block could enclose the three lines below the "ensure that the module is out of reset" comment. The declarations would then require neither conditionals or attributes (and would be topologically as close to the use as possible). > Thanks, > Stefan Amicalement, -- Albert.