On Sun, 4 Dec 2011 12:30:40 +0100 Marek Vasut marek.vasut@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@denx.de Cc: sricharan r.sricharan@ti.com Cc: Tom Rini trini@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