[U-Boot] [PATCH 9/9] arch/arm/include/asm/arch-omap5/clocks.h: Fix GCC 4.2 warnings

[Anatolij Gustschin]

On Sun, 4 Dec 2011 12:30:40 +0100
Marek Vasut <marek.vasut at gmail.com> wrote:

> > Fix:
> > clocks.c: In function 'setup_post_dividers':
> > clocks.c:175: warning: comparison is always true due to limited range of
> > data type
> > clocks.c:177: warning: comparison is always true due to limited range of
> > data type
> > clocks.c:179: warning: comparison is always true due to limited range of
> > data type
> > clocks.c:181: warning: comparison is always true due to limited range of
> > data type
> > clocks.c:183: warning: comparison is always true due to limited range of
> > data type
> > clocks.c:185: warning: comparison is always true due to limited range of
> > data type
> > clocks.c:187: warning: comparison is always true due to limited range of
> > data type
> > clocks.c:189: warning: comparison is always true due to limited range of
> > data type
> > 
> > Signed-off-by: Anatolij Gustschin <agust at denx.de>
> > Cc: sricharan <r.sricharan at ti.com>
> > Cc: Tom Rini <trini at ti.com>
> > ---
> > Some notes:
> > 
> >  - GCC v4.5.1 didn't warn here
> >  - GCC v4.6.1 seems to have a bug and can't compile this code:
> >    clocks.c: In function 'enable_non_essential_clocks':
> >    clocks.c:349:13: internal compiler error: in decode_addr_const, at
> > varasm.c:2632
> > 
> >  arch/arm/include/asm/arch-omap5/clocks.h |   16 ++++++++--------
> >  1 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/arch-omap5/clocks.h
> > b/arch/arm/include/asm/arch-omap5/clocks.h index fa99f65..d0e6dd6 100644
> > --- a/arch/arm/include/asm/arch-omap5/clocks.h
> > +++ b/arch/arm/include/asm/arch-omap5/clocks.h
> > @@ -686,14 +686,14 @@ struct dpll_regs {
> >  struct dpll_params {
> >  	u32 m;
> >  	u32 n;
> > -	u8 m2;
> > -	u8 m3;
> > -	u8 h11;
> > -	u8 h12;
> > -	u8 h13;
> > -	u8 h14;
> > -	u8 h22;
> > -	u8 h23;
> > +	s8 m2;
> > +	s8 m3;
> > +	s8 h11;
> > +	s8 h12;
> > +	s8 h13;
> > +	s8 h14;
> > +	s8 h22;
> > +	s8 h23;
> >  };
> > 
> >  extern struct omap5_prcm_regs *const prcm;
> 
> Make clock registers a signed type? whoa

No, we don't make registers a signed type. This is parameters structure
for some parameter tables containing -1 as an indicator that the
parameter shouldn't be written to the register. Using unsigned type
for structure field results in parameter value 255:

static const struct dpll_params per_dpll_params_768mhz[NUM_SYS_CLKS] = {
        {32, 0, 4, 3, 6, 4, -1, 2, -1, -1},             /* 12 MHz   */
        {-1, -1, -1, -1, -1, -1, -1, -1, -1, -1},       /* 13 MHz   */
        {160, 6, 4, 3, 6, 4, -1, 2, -1, -1},            /* 16.8 MHz */
        {20, 0, 4, 3, 6, 4, -1, 2, -1, -1},             /* 19.2 MHz */
        {192, 12, 4, 3, 6, 4, -1, 2, -1, -1},           /* 26 MHz   */
        {-1, -1, -1, -1, -1, -1, -1, -1, -1, -1},       /* 27 MHz   */
        {10, 0, 4, 3, 6, 4, -1, 2, -1, -1}              /* 38.4 MHz */
};

The code then checks:

void setup_post_dividers(u32 *const base, const struct dpll_params *params)
{
        struct dpll_regs *const dpll_regs = (struct dpll_regs *)base;
        
        /* Setup post-dividers */
        if (params->m2 >= 0)
                writel(params->m2, &dpll_regs->cm_div_m2_dpll);
        if (params->m3 >= 0)
                writel(params->m3, &dpll_regs->cm_div_m3_dpll);
        if (params->h11 >= 0)
                writel(params->h11, &dpll_regs->cm_div_h11_dpll);
        if (params->h12 >= 0)
                writel(params->h12, &dpll_regs->cm_div_h12_dpll);
        if (params->h13 >= 0)
                writel(params->h13, &dpll_regs->cm_div_h13_dpll);
        if (params->h14 >= 0)
                writel(params->h14, &dpll_regs->cm_div_h14_dpll);
        if (params->h22 >= 0)
                writel(params->h22, &dpll_regs->cm_div_h22_dpll);
        if (params->h23 >= 0)
                writel(params->h23, &dpll_regs->cm_div_h23_dpll);
}

The result is that the registers will always be written to, since
the comparison is always true. This is apparently not intended in
the code.

The actual registers structure 'struct dpll_regs' uses unsigned type.

This sneaked in in the commit 2e5ba489 adding omap5 clock support.
The similar parameter structure for omap4 used signed type for the
fields in question.

Newer gcc doesn't warn here unless -Wextra option is used.

Thanks,
Anatolij