[U-Boot] Please pull u-boot-ti/master

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hi Stefan,

On Fri, 06 Dec 2013 14:38:58 +0100, Stefan Roese <sr@denx.de> 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.